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

Reimplement RSX reservation access #8983

Merged
merged 1 commit into from Oct 25, 2020
Merged

Reimplement RSX reservation access #8983

merged 1 commit into from Oct 25, 2020

Conversation

@kd-11
Copy link
Contributor

@kd-11 kd-11 commented Sep 26, 2020

This PR adds block-level reservation management for RSX vs CELL. The basic design philosophy here is that when "Accurate RSX reservations" is enabled, access to memory shared between CELL and RSX is accessed in an exclusive manner. Multiple CELL threads are free to simultaneously manipulate memory as they have their own synchronization mechanisms, but RSX accesses said memory in an exclusive manner. This fixes a lot of problems where SPU/PPU has a reservation line but data is clobbered by RSX DMA operations. TSX normally fixes this issue for capable CPUs which is why some users can play affected games at good performance with no issues.
The advantage of doing it this way of course is performance, the performance hit is now much less and games that require the option to work see comparable performance to using TSX, being only slightly behind.
While this change is now heavily simplified and reuses existing sync primitives, it was previously overly complicated with custom page-locking mechanism but that solution was scrapped as it was overdesigned and broke very easily. The new version also has slightly better performance (about 1 fps or so).

To Testers: Please focus on games that require the "Accurate RSX reservations" option. If you have TSX, compare TSX on vs TSX off + the option enabled.

Copy link
Contributor

@elad335 elad335 left a comment

I think you need to add it to texture faults handling as well.

@kd-11
Copy link
Contributor Author

@kd-11 kd-11 commented Sep 26, 2020

Those are atomic as the pages are no-access. Flushing is done in mirror space so no way we can have contesting writes.

rpcs3/Emu/RSX/rsx_methods.cpp Outdated Show resolved Hide resolved
rpcs3/Emu/RSX/RSXThread.h Outdated Show resolved Hide resolved
@elad335
Copy link
Contributor

@elad335 elad335 commented Sep 28, 2020

Is there a opential deadlock here where SPU enters texture cache fault handler for adderess X on a reservation then when RSX waits on the same address to be unlocked it cannot handle the texture cache fault and the SPU and RSX freezes because they wait for each other forever?

@xddxd
Copy link
Contributor

@xddxd xddxd commented Sep 28, 2020

On this PR L.A Noire locks up in the menu with reservations enabled.
Master: RPCS3.log.gz
PR: rsrv.log.gz

@kd-11 kd-11 changed the title [TESTERS NEEDED] Reimplement RSX reservation access [TESTERS NEEDED][WIP] Reimplement RSX reservation access Sep 28, 2020
@kd-11 kd-11 force-pushed the kd-11:reservations_crap branch 2 times, most recently from 5e13b5b to 677b3b6 Oct 6, 2020
@kd-11
Copy link
Contributor Author

@kd-11 kd-11 commented Oct 6, 2020

FTR, deadlock on RSX-is-asleep-while-SPU-thread-is-in-exception-handling was always a problem. Fixing it in a straightforward manner would be terrible and allows other threads to essentially starve RSX of the locks, which means a custom primitive with first come first serve is needed that optionally invokes a callback while waiting and still maintains request order. This is easy to do on *nix but not so much on windows so I need to think of something that plays a bit nicer and doesn't scare people. I have a few ideas I'd love to try out; we'll see which implementation is the winner.

@kd-11 kd-11 changed the title [TESTERS NEEDED][WIP] Reimplement RSX reservation access Reimplement RSX reservation access Oct 8, 2020
@kd-11
Copy link
Contributor Author

@kd-11 kd-11 commented Oct 8, 2020

@xddxd I forgot to request a retest, I fixed a potential deadlock.

@xddxd
Copy link
Contributor

@xddxd xddxd commented Oct 8, 2020

Same behavior.

@kd-11 kd-11 removed the In Progress label Oct 21, 2020
@kd-11 kd-11 force-pushed the kd-11:reservations_crap branch from fbfdd13 to e8390a3 Oct 25, 2020
rpcs3/Emu/Cell/SPUThread.cpp Outdated Show resolved Hide resolved
@kd-11 kd-11 force-pushed the kd-11:reservations_crap branch from e8390a3 to ffa2397 Oct 25, 2020
@kd-11 kd-11 merged commit 18ca3ed into RPCS3:master Oct 25, 2020
6 checks passed
6 checks passed
FreeBSD 12.2 (snapshot) Task Summary
Details
RPCS3.rpcs3 Build #20201025.19 succeeded
Details
RPCS3.rpcs3 (Linux_Build Clang) Linux_Build Clang succeeded
Details
RPCS3.rpcs3 (Linux_Build GCC) Linux_Build GCC succeeded
Details
RPCS3.rpcs3 (Windows_Build) Windows_Build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kd-11 kd-11 mentioned this pull request Oct 25, 2020
0 of 1 task complete
@kd-11
Copy link
Contributor Author

@kd-11 kd-11 commented Oct 25, 2020

L.A Noire discussion moved to #9134
Unfortunately that title is so unstable that debugging becomes hit or miss. Sometimes it seems to be working then it is not, then you change something insignificant and it seems to work again only to break after a few minutes - it's not a reasonable target for this kind of investigation.

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

Successfully merging this pull request may close these issues.

None yet

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