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 MFC: Implement MFC commands execution shuffling #8514

Merged
merged 1 commit into from Sep 17, 2021

Conversation

elad335
Copy link
Contributor

@elad335 elad335 commented Jun 27, 2020

A brief summary on what is MFC and what was so problematic about emulating it accurately:
The MFC (Memory Flow Controller) is what allows the SPU to perform reads and writes to memory outside of its own small memory "cache".
Unlike literally all processors, no CPU is known to be able to see the status of completion of its reads/writes, nor perform them out of order of its own execution.
Meaning if we write the value y to address x, then read from it. We expect on all processors to read the value y (assuming no concurrent writes involved) right? Wrong! SPU is the exception to this basic fundamental rule in processors.
Infact, the reads and writes the MFC is performing are executed at unpredicatable timing, without ordering to its own reads/writes (unless barriers are involved).
In Narcor Terror and The Expandables games, as a result of a bug, a dependancy of this feature developed on real ps3. The SPU thread would enqueue an MFC command which reads memory from the main memory into its memory "cache" (the local storage) but immediately afterwards write to the local storage directly as well with different values.
This is basically a race condition between the MFC and the SPU, who wouldwrite first the value? in rpcs3 it turns out to be the MFC writing the wrong value and crashing, because in rpcs3 all MFC transfers are executed exactly when you order them, like all other processors. But in real ps3 the answer differ and whos actually wins this race is the SPU, writing the correct value and continueing.
In Resonance Of Fate, something more extreme happens, where the SPU would enqueue multiple reads and a write to the same address on the local storage without any barriers, in a loop.
The correct value written to the main memory is not the one of the last command executed, but one before it.
In rpcs3 the value written is of the last command, but on ps3 the answer is "random", it may take a few attempts to finally use the correct command and continuing execition normally.

To solve both issues at the same time, we need to implement two things:

  1. A way to postpone commands to not execute immedaitely.
  2. Select racdomly the command to be next executed from the queue of transfers, regardless of which issued first.

I did some thought and instead of adding another thread like "MFC threads" would be a total waste of HW resources and performance would drop siginicantly.
So I made the same SPU thread execute the transfers later in its execution.
Two new settings were added: "MFC Transfers Shuffling Max Commands" (controlling max command to shuffle and postpone)
and "MFC Transfers Timeout" (timeout for enqueued commands to be executed, 0 means no timeout)

List of fixed games:

Resonance Of Fate (Intro -> Ingame)
Narco Terror (Nothing -> Ingame)
The Expandables: The Video Game (Nothing -> Ingame)

For all the games fixed here set max MFC commands to 1 and MFC transfers timeout to 0.

image
image

Fixes #8513
Fixes #5504
NOTE: Currently only implemented on SPU interpreters.

@elad335 elad335 changed the title SPU MFC: Implement MFC commands executions shuffling SPU MFC: Implement MFC commands execution shuffeling Jun 27, 2020
@elad335
Copy link
Contributor Author

elad335 commented Jun 27, 2020

There are no longer games in "Nothing" section when this pr is merged, obsolete category.

@elad335
Copy link
Contributor Author

elad335 commented Sep 10, 2021

Rebased.

@elad335 elad335 marked this pull request as ready for review September 10, 2021 17:52
// Process enqueued commands
while (true)
{
static_cast<void>(std::remove_if(mfc_queue + 0, mfc_queue + mfc_size, [&](spu_mfc_cmd& args)
Copy link
Member

Choose a reason for hiding this comment

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

Can you put remove_if lambda into a variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

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