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.

Sign up for GitHub

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

Jump to bottom

DSP: Make ARAM memory circular in Wii mode #6368

Closed
wants to merge 2 commits into from
Closed

DSP: Make ARAM memory circular in Wii mode #6368

wants to merge 2 commits into from

Conversation

sepalani
Copy link
Contributor

@sepalani sepalani commented Feb 9, 2018

Apparently ARAM is supposed to be circular, however, with the use of Memory::Read_U8 which leads to a GetPointer call later on will have invalid RAM address ranges returning nullptr in Wii mode:

  • 0x01_80_00_00 to 0x02_00_00_00,
  • 0x03_80_00_00 to 0x04_00_00_00,
  • etc.

In sum, every GetPointer's MEM1 address located between REALRAM_SIZE and RAM_SIZE will return nullptr.

These are crashing Dolphin and despite them being invalid RAM addresses, they should be valid ARAM addresses. That's why the RAM pointer is used instead, just like the DSPHLE implementation.

This PR fixes the debugger's MemoryView crashing while looking at an address between REALRAM_SIZE and RAM_SIZE. It might also fix potential DSP related crashes.

Ready to be reviewed.

@sepalani sepalani changed the title DSPLLE: Make ARAM memory circular in Wii mode DSP: Make ARAM memory circular in Wii mode Feb 9, 2018
@sepalani sepalani mentioned this pull request Feb 9, 2018
Closed
@degasus
Copy link
Member

degasus commented Feb 19, 2018

The title is wrong. The ARAM is already circular, but this PR changes the behavior of the addressable but not present memory. Please keep in mind that there is only 24 MB of memory and that the missing 8 MB will just return random data.

What do you think about catching the nullptr and returning 0 instead?

@sepalani
Copy link
Contributor Author

sepalani commented Feb 19, 2018 •
edited
Loading

@degasus
I'm fine with any alternative. The previous PR (#5924) was checking the address validity but didn't make sense in a way since all addresses should be valid (I haven't hardware tested it, however). Nonetheless, it didn't fix the issue if such address is requested by the game and might lead to a crash.

This one, might not be the appropriate way to fix it but is consistent with the already implemented HLE version.

You're also right, the title is a bit misleading since this one handle the case where it's located in RAM. I'll come up with a better title, ASAP.

@delroth @booto
Any thought on this?

I don't mind returning zero for these ranges, even if it would make more sense to me to make them circular. Especially, if there might be games relying on edgy behaviour like this.

@sepalani
Copy link
Contributor Author

This DSPSpy test is still WIP. This test makes Dolphin (DSP LLE) crash without this PR:
DSPSpy_wii.zip. However, while the test can be executed on Dolphin, it can't on real hardware due to "exception 3". I'll have to see how to properly setup this in order to use the read/write from 0xffd3 feature and maybe add a label for it in the DSPTable.

@sepalani
Copy link
Contributor Author

Closed in favor of #7740 which seems to be a better alternative.

@sepalani sepalani closed this Jan 26, 2019
@booto
Copy link
Contributor

booto commented Jan 26, 2019

Sorry, I had forgotten about this PR. I plan #7740 to be a little bit of a WIP testbed for trying to experiment with ARAM stuff. I would be curious about how well #7740 deals with the custom DSP code you included in this PR (vs. hardware).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sepalani @degasus @booto