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

Common: Use Mach VM routines for memory mapping #11308

Merged
merged 1 commit into from
May 30, 2024
Merged

Conversation

stenzek
Copy link
Contributor

@stenzek stenzek commented May 29, 2024

Description of Changes

MacOS does not support an equivalent of MAP_FIXED_NOREPLACE via mmap(), which means that our usage for allocating PCSX2's memory map is not thread-safe.

#11282 fixed this for Linux, this PR implements it for MacOS.

I'll follow up later with a signal handler tidy-up, that way we can skip compiling everything in the Linux/ directory on MacOS.

Rationale behind Changes

Preventing a race condition that can cause memory corruption in PCSX2 on startup, while it allocates its memory map.

Suggested Testing Steps

Make sure Mac builds still boot.

@TellowKrinkle I've tagged you for review in case you know anything about these system calls. Apple's documentation is all blank pages, I ended up using the published XNU source as a reference...

MacOS does not support an equivalent of MAP_FIXED_NOREPLACE via mmap(),
which means that our usage for allocating PCSX2's memory map is not
thread-safe.
Copy link
Member

@TellowKrinkle TellowKrinkle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not super familiar with mach's memory management APIs but it seems reasonable (untested)

One part I do have a question about is the part that's just getting moved around, but since pthread_jit_write_protect_np is per-thread while s_code_write_depth is static, do/will we have something in place to make sure no two threads attempt to write code at the same time (e.g. ee and sw renderer)?
(Don't have to address it here/now, since it won't affect anyone on x86)
Edit: I'm apparently blind

@stenzek
Copy link
Contributor Author

stenzek commented May 30, 2024

All good :) I've tested on Sonoma (M2 Pro) and it seems fine.

The only potential issue is mmap() vs mach_vm_deallocate() on unmap, but that only affects the Apple Silicon path, I'll resolve that later.

@stenzek stenzek merged commit ecbe239 into PCSX2:master May 30, 2024
12 checks passed
@stenzek stenzek deleted the alloc branch May 30, 2024 03:39
@atarixle
Copy link

atarixle commented Jul 2, 2024

Sorry to say that, but this does not work on my 2014 MacBook Pro 13". It runs with Sonoma and Open Core Legacy Patcher. I do not know if this is an issue with Open Core Legacy Patcher, BUT PCSX2-v1.7.5849 introduces this feature, while PCSX2-v1.7.5847 runs flawlessly.
Anybody with an unpatched Mac please test this for me! Btw, does PCSX2 still work on Big Sur (this is the last OS that supports my MacBook Pro 2014 natively).

@CatGreen90
Copy link

Latest (1.7.5953) still works for me on a 15" MBP from 2014 running Big Sur.

@atarixle
Copy link

atarixle commented Jul 3, 2024

So, in Big Sur it works (your observation), in OCLP/Sonoma it doesn't (my observation). Anybody able to try if it works in vanilla Sonoma (Intel)?

@DigitalMajestic
Copy link
Contributor

2019 MBP (Intel), macOS 14.4 (Sonoma), 1.7.5955 works for me.

@atarixle
Copy link

atarixle commented Jul 4, 2024

I guess as I run OCLP/Sonoma my system is out of support in this case and this error is irrelevant on regular systems.

@TellowKrinkle
Copy link
Member

TellowKrinkle commented Jul 5, 2024

We had another user with a similar issue on a Ryzen hackintosh, check what SIP flags you have enabled, apparently some of them break this.

@atarixle
Copy link

atarixle commented Jul 5, 2024

here is what my Mac's OCLP/Sonoma state is like:

$ csrutil status
System Integrity Protection status: unknown (Custom Configuration).

Configuration:
  Apple Internal: disabled
  Kext Signing: disabled
  Filesystem Protections: disabled
  Debugging Restrictions: enabled
  DTrace Restrictions: enabled
  NVRAM Protections: enabled
  BaseSystem Verification: enabled

This is an unsupported configuration, likely to break in the future and leave your machine in an unknown state.

@jimjamyaha
Copy link

I also have this problem. Running 14.6.1 Sonoma, OCLP nightly build 1.6.0.

PCSX2 only works on version v1.7.5847.

SIP settings are

Screenshot 2024-08-20 at 00 10 12

@TellowKrinkle
Copy link
Member

@jimjamyaha Can you check if #11734 helps?

@atarixle
Copy link

atarixle commented Aug 28, 2024

@TellowKrinkle #11734 works on my MacBook Pro 2014 Sonoma 14.6.1 OCLP.

I just saw that you can set the SIP bits as you want before root-patching. What are these bits set like on a supported Mac with Sonoma?

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

Successfully merging this pull request may close these issues.

6 participants