Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linux Support #1430

Open
bwrsandman opened this issue Aug 14, 2019 · 23 comments
Open

Linux Support #1430

bwrsandman opened this issue Aug 14, 2019 · 23 comments

Comments

@bwrsandman
Copy link
Contributor

@bwrsandman bwrsandman commented Aug 14, 2019

Current State

Before

Xenia on Linux compiles for Travis tests in order to check C++ errors, style and very basic functionality. It has some nice abstract function definition. Most of these definitions are stubs with failing asserts which mean running Xenia in debug causes SIGABRT and running in release will eventually crash due to missing implementation.

The biggest missing part is the threading. Without threading implementation, the Xenia UI can't run and wait. The emulated threads and synchronization primitives which are built on top of these cannot work properly.

There is also PPC CPU emulation discrepancies between Windows and Linux. The registers used on 64-bit Windows and 64-bit Linux are not the same, therefore proper care must be made to use the correct registers when transitioning from guest to host code and vice-versa.

Memory mapping needs a bit of work due to the differences in shared memory and explicit memory ranges having different parameters and prefixes on Linux.

The GTK windowing and its interaction with Vulkan needs to fixed for several bugs.

The use of paths and the string functions need to better specify NT-style line endings (\) which are used by guest and Windows host. In the case of a Linux host, Unix line endings must be used when specifying guest paths.

Linux debugging and stack walking functions need to be implemented.

Approach to Adding Support

My approach to fixing the following problem has been to first add unit tests (and to use the ones already there) to the already working Windows implementation in order to have a behavioural ground truth. Then I add implementation to Linux which satisfy these tests. Once the Linux implementation pass the tests, I activate those tests on Travis in order to prevent future regressions.

Active PRs

Here are the PRs that I have worked on to have native Linux working. Some of these are built on top of the work of others and I preserved the authorship in the commits.
They are in descending order of importance.

Things left to do

If anyone feels like contributing to the port there are a few areas which have not yet been addressed:

  • Vulkan Graphics
  • Auto Reset events do not work as expected
  • There is a bug in Event to be fixed

New State

After
@JoelLinn Input

The fixes allow for all unit tests in the project to run and pass on Linux with clang.

The Xenia UI works and can select files with the open dialog.

Loading a xex will work up until emulation.

Calling a host function works as well as host calling a guest function.

Logging works.

Loading a ROM will eventually crash during emulation.

To try all the changes together:

$ git remote add bwrsandman git@github.com:bwrsandman/xenia.git
$ git fetch bwrsandman
$ git merge --no-ff --no-edit bwrsandman/linux_threads
$ git merge --no-ff --no-edit bwrsandman/linux_cpu
$ git merge --no-ff --no-edit -X theirs bwrsandman/linux_memory
$ git merge --no-ff --no-edit bwrsandman/linux_windowing
$ git merge --no-ff --no-edit -X theirs bwrsandman/linux_filesystem
$ git merge --no-ff --no-edit bwrsandman/linux_debug
$ git merge --no-ff --no-edit bwrsandman/linux_stack_walker
$ # Resolving the conflict in .gdbinit in a union-like way
$ sed -i '/^<<<<<<</d;/^=======/d;/^>>>>>>>/d' .gdbinit
$ git add .gdbinit
$ git commit --no-edit
@Prism019

This comment has been minimized.

Copy link
Contributor

@Prism019 Prism019 commented Aug 31, 2019

linux_stack_walker has a merge conflict in .gdbinit... should probably use -X theirs for that merge?

@bwrsandman

This comment has been minimized.

Copy link
Contributor Author

@bwrsandman bwrsandman commented Aug 31, 2019

The .gdbinit conflict has to be resolved without an -X strategy.
Unfortunately, using -X theirs will replace the changes from linux_threading and we need both changes from linux_threading (gdb ignoring custom signals from the threading implementation) and those from linux_stack_walker (gdb ignoring custom signal for stack retrieval).
The correct strategy would be -X union but git doesn't currently support that for merge yet.

You can add this to the commands to auto-resolve in a union-like way:

$ sed -i '/^<<<<<<</d;/^=======/d;/^>>>>>>>/d' .gdbinit
$ git add .gdbinit
$ git commit --no-edit
@bwrsandman

This comment has been minimized.

Copy link
Contributor Author

@bwrsandman bwrsandman commented Aug 31, 2019

Oh yeah, I should add: Use gdb for debugging, lldb is painfully slow and I'm not sure why.

@JoelLinn

This comment has been minimized.

Copy link
Contributor

@JoelLinn JoelLinn commented Oct 13, 2019

What is the current state of these merge requests? There seem to be only minor objections from the maintainers. This work is the basis for all further Linux development.

@Prism019

This comment has been minimized.

Copy link
Contributor

@Prism019 Prism019 commented Oct 14, 2019

Games still do not run. There are also some threading issues (race conditions?) with exiting the emulator. There's also the case of wide chars being different sizes between the guest and host (16 bit vs 32 bit, respectively).

@bwrsandman

This comment has been minimized.

Copy link
Contributor Author

@bwrsandman bwrsandman commented Oct 14, 2019

I wasn't aware of race conditions...

@JoelLinn

This comment has been minimized.

Copy link
Contributor

@JoelLinn JoelLinn commented Oct 14, 2019

To my knowledge merging this does not break anything, it only improves things.
It would make it easier for others to submit further fixes for linux - doing so right now would probably make them dependent on these PRs anyways.

@JoelLinn

This comment has been minimized.

Copy link
Contributor

@JoelLinn JoelLinn commented Oct 15, 2019

Audio

A cross platform audio back-end is implemented in PR #1498.
It is however untested on Linux (since emulation crashes at the time of writing).
It was way easier than expected to implement this.

This comment was edited since my previous thoughts are no longer relevant

@JoelLinn

This comment has been minimized.

Copy link
Contributor

@JoelLinn JoelLinn commented Oct 20, 2019

Input

Input has been implemented in PR #1493

@bwrsandman

This comment has been minimized.

Copy link
Contributor Author

@bwrsandman bwrsandman commented Oct 21, 2019

Great work!

@bwrsandman

This comment has been minimized.

Copy link
Contributor Author

@bwrsandman bwrsandman commented Nov 7, 2019

The window title issue was fixed 0fdbcde
It wasn't related to the different width of wchar. Xenia gets the title correctly from the game file. It was just the printf token which was using regular strings instead of wide strings.

@JoelLinn

This comment has been minimized.

Copy link
Contributor

@JoelLinn JoelLinn commented Nov 7, 2019

Is this it?
I would argue there must be a number of places where sizeof(wchar_t) == 2 is assumed.
E.g. XBox buffers converted to c++ (w)strings and vice versa.

@bwrsandman

This comment has been minimized.

Copy link
Contributor Author

@bwrsandman bwrsandman commented Nov 7, 2019

Perhaps in src/xenia/kernel/util/shim_utils.h but from my tests running this app, it's only ansi strings being called.

@JoelLinn

This comment has been minimized.

Copy link
Contributor

@JoelLinn JoelLinn commented Nov 7, 2019

Ok a quick static analysis on which code segments assume wchar_t on the host and uint16_t of the guest to be equal:
(May not be complete)

template <>
inline std::wstring load_and_swap<std::wstring>(const void* mem) {
std::wstring value;
for (int i = 0;; ++i) {
auto c =
xe::load_and_swap<uint16_t>(reinterpret_cast<const uint16_t*>(mem) + i);
if (!c) {
break;
}
value.push_back(static_cast<wchar_t>(c));
}
return value;
}

template <>
inline void store_and_swap<std::wstring>(void* mem, const std::wstring& value) {
for (auto i = 0; i < value.size(); ++i) {
xe::store_and_swap<uint16_t>(reinterpret_cast<uint16_t*>(mem) + i,
value[i]);
}
}

inline std::wstring TranslateUnicodeString(
const Memory* memory, const X_UNICODE_STRING* unicode_string) {
if (!unicode_string) {
return L"";
}
uint16_t length = unicode_string->length;
if (!length) {
return L"";
}
const xe::be<uint16_t>* guest_string =
memory->TranslateVirtual<const xe::be<uint16_t>*>(
unicode_string->pointer);
std::wstring translated_string;
translated_string.reserve(length);
for (uint16_t i = 0; i < length; ++i) {
translated_string += wchar_t(uint16_t(guest_string[i]));
}
return translated_string;
}

What about using

std::wstring_convert<std::codecvt_utf16<char32_t>, char32_t>

in the case of 4 byte wchar_t platforms to translate between host and guest?

Another way would be to use std::u16string all around - That would however introduce other problems I guess.

@bwrsandman

This comment has been minimized.

Copy link
Contributor Author

@bwrsandman bwrsandman commented Nov 7, 2019

I went and ran with std::u16string everywhere. No success. It seems to crash at the same place.
0eba293

@Prism019

This comment has been minimized.

Copy link
Contributor

@Prism019 Prism019 commented Nov 7, 2019

That happened to me too on my local branch when i did the conversion. Trying to debug this in GDB is a nightmare though.. when I try to run dolphin.xex, the stack is clobbered when it crashes, so I don't know what to look at in IDA to try and figure out the crash.

@JoelLinn

This comment has been minimized.

Copy link
Contributor

@JoelLinn JoelLinn commented Nov 7, 2019

I went and ran with std::u16string everywhere. No success. It seems to crash at the same place.
0eba293

Eww...
Makes me wonder what the places are where std:wstrings interact with Linux APIs
Maybe use 4 byte in xenia and convert to 1 byte UTF8 when needed.
We would need to do that anyways when using 2 byte strings in xenia.
Downside is one more conversion between xenia and guest (4 byte <> 2 byte).

Generally speaking, a clean solution on Linux is to use std::string with UTF8 encoding but that gets pretty messy when you have ansistrings around that are also 1 byte long like we have.

@Prism019

This comment has been minimized.

Copy link
Contributor

@Prism019 Prism019 commented Dec 2, 2019

For anyone not following along on the #dev-linux channel on the discord, I've gotten the game emulation to be stable. I'm going to check to make sure the fixes don't interfere with Windows, then open a new PR to get the changes merged.

Note: Vulkan Graphics and SDL Audio do not work as of now. SDL Input may work but I have no way of testing it in a game.

@Prism019

This comment has been minimized.

Copy link
Contributor

@Prism019 Prism019 commented Dec 3, 2019

#1517 should fix the segfaulting while running emulations.

@bwrsandman

This comment has been minimized.

Copy link
Contributor Author

@bwrsandman bwrsandman commented Dec 4, 2019

There are some conflicts with #1339 I don't know if you tested with both, I'm applying #1517 after #1339 with the -X theirs strategy.
What are your thoughts?

@bwrsandman

This comment has been minimized.

Copy link
Contributor Author

@bwrsandman bwrsandman commented Dec 7, 2019

There seems to be a bug in the auto reset events in the threading PR.
The notify one signal does not seem to unblock the main thread causing a deadlock.
More tests need to be made to have the behaviour on par with windows.
See the threading PR for more details.

@sl1pkn07

This comment has been minimized.

Copy link

@sl1pkn07 sl1pkn07 commented Mar 2, 2020

seems conflict #1339 with #1397 and #1405 with #1317

greetings

@bwrsandman

This comment has been minimized.

Copy link
Contributor Author

@bwrsandman bwrsandman commented Mar 3, 2020

Rebased and fixed the conflicts.
I did minimal testing as I don't have a lot of time to dedicate to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.