Allow access to code memory for exefs mods #5518
Conversation
|
Download the artifacts for this pull request: Experimental GUI (Avalonia)GUI-less (SDL2)Only for Developers
|
|
Does that work with ARCropolis on Super Smash Bros Ultimate? The main reason it was disabled is because there is a bug on skyline that prevents it from working with code memory. It would try to allocate memory before the executable, which doesn't work in the emulator as it maps the executable at the base of the address space. A potential fix for this is simply not loading the executable at the base of address space, but that will break PPTC since it currently assumes all guest code addresses don't change. |
|
That just crashes with an invalid memory access: Ryujinx crash: ARCropolisI'm not a fan of having workarounds to make something work on Ryujinx that works on hardware without that workaround. Could we consider this a bug instead and solve this issue correctly? |
The behaviour that makes those mods crash is the official kernel behaviour. The official kernel does not allow code memory to be mapped to the same process it was created. This is a CFW modification that was made for homebrew, to allow them to create and map code memory in the same process, to allow generating and modifying its own code. For this reason I don't see it as much as a "workaround" to make ARCropolis work. In any case, I think the amount of people using SSBU mods is considerably higher than users of exlaunch based mods, it doesn't seems worth it for me to break one to fix the other. I think it would be better to make PPTC guest code relocation work, and then enable code memory for everything (maybe behind a Switch since it's not standard kernel behaviour). |
My word choice was bad. What I meant was that if this check was only implemented like that to make ARCropolis work, I don't think it's the right way to do this. I'd be more in favor of having the same behavior as AMS. If I understand the ARCropolis issue correctly we currently don't match the behavior of pm (or was that am?) for loading processes into memory. Shouldn't we address that instead? |
Yes, I think we should delete this
Yes, the Switch has ASLR and will load the executable at a random memory location. We always load it at a fixed location. Unfortunately PPTC currently has a dependency on that, it stores guest addresses directly, which means if it changes, the cached PPTC code will stop working. To make skyline work with code memory, we can simply load the executable at a higher fixed address (instead of random address) which would work with PPTC as the address would be still the same between runs, but it would still require a full PPTC invalidation as the address would be different from the existing caches. The ideal solution would be rewriting PPTC and solving that and the other issues it has (like #5130, fix the "NRO stutters", etc). |
|
BTW there is another bug on skyline that makes it crash when using the path that does not require code memory. It uses an invalid address with a cache invalidation instruction. The only reason it works is because we ignore cache maintenance instructions on the JIT right now, as we don't emulate CPU caches. But it does not work on Apple Hypervisor for this reason, and presumably wouldn't work on hardware either (without code memory). So that is something that should eventually be fixed on the JIT, we should at least throw for cache operations with invalid memory addresses. |
|
@gdkchan do you think the workaround that I added with my latest commit here is a good solution for now? |
I'm not a fan of adding even more special cases to make this work, but maybe that is fine for now, until PPTC is eventually improved. If you are going to do this, the existing About PPTC, instead of the migration path, maybe it's fine to just implement a way to do full PPTC invalidation. It doesn't take long to build so a full invalidation wouldn't be a big deal IMO. BTW this should also fix ARCropolis not working on mac with hypervisor, but I didn't test myself. |
For that we just need to change the InternalVersion for both
I'd prefer to replace it with the setting we discussed before, but don't think I should do that in this PR as well.
Sounds good to me! I'd like to do that, but since that requires a few more changes I'd like to do it in a separate PR. Is that fine? |
You can just keep the setting hardcoded to true without exposing it on the UI or adding to the config file. It can be exposed in the UI on a separate PR. That way the amount of changes should be small. |
* Allow access to code memory for exefs mods * Add ASLR workaround for Skyline * Hardcode allowCodeMemoryForJit to true
This PR fixes an issue where games would crash if mods utilizing JIT (most notably exlaunch) were used.
More context can be found at the linked issue.
Fixes #5466
Fixes #4354