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

SPU: Validate reservation in GET commands (Accurate DMA) #9062

Merged
merged 5 commits into from
Oct 12, 2020

Conversation

elad335
Copy link
Contributor

@elad335 elad335 commented Oct 12, 2020

This originally striked me as I realized PPU accurate 128byte reservation mode does not work properly when there are two or more loads from the same address in the atomic loop. Why? because if one loads reads data X and the other reads data Y, if the reservation data there is X for example, meaning the data was changed twice by other thread (X ->Y ->X) the reservation store can even succeed in this case on rpcs3 although clearly the writes occured and the reservation store should fail in this case.

Now this pr doesnt fix, but it fixes a similar case in SPU side, where someone can use GET inside an atomic loop (essentialy reading the data twice as GETLLAR already read it) so add reservation validation checks there if this case is encountered.

Only affects Accurate SPU DMA as mentioned in the title.

raddr = 0;
}

alignas(64) u8 temp[128];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Temporary storage, similar to a recent bugfix in GETLLAR which doesnt rely on LS to store constant data because its volatile.

@bevanweiss
Copy link
Contributor

Is there something similar to PAGE_GUARD in the Linux/other OS world?
https://docs.microsoft.com/en-us/windows/win32/memory/creating-guard-pages

It seems like it might be an 'easier' way to detect if a write occurs to the reservation range.
The STATUS_GUARD_PAGE_VIOLATION exception could be caught (for the page containing the reservation), and checked against the reservation address range. If it's a match, then the flags for the reservation 'fault' are set, which the PUTLL.. opcodes should look at prior to performing their actions. If it's not a match, then the PAGE_GUARD is setup again.

@elad335
Copy link
Contributor Author

elad335 commented Oct 12, 2020

memory protection is probably very slow, you can perf test if you want.

@kd-11
Copy link
Contributor

kd-11 commented Oct 12, 2020

I can confirm memory protection is painfully slow. It is already a major bottleneck for texture cache management, I'm horrified to think how it can impact common code execution paths.

@Nekotekina Nekotekina merged commit 95c1443 into RPCS3:master Oct 12, 2020
fengjixuchui added a commit to fengjixuchui/rpcs3 that referenced this pull request Oct 12, 2020
SPU: Validate reservation in GET commands (Accurate DMA) (RPCS3#9062)
@elad335 elad335 deleted the patch-17 branch October 23, 2020 10:41
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.

None yet

4 participants