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

DSPLLE: Add assertion for bad DMA alignment #11480

Merged
merged 2 commits into from Jan 26, 2023

Conversation

Pokechu22
Copy link
Contributor

This caused me a lot of trouble, as DSPSpy was breaking when compiled with -O0. It ended up being the case that dsp_code was placed at 0x800ba400 at -O2, 0x800ba320 at -O1, and 0x800bb3ee at -O0. I don't know if the alignment at -O1 and higher was just a coincidence or not.

The symptom of misaligned code was either no mail at all from the DSP (instead showing the DSP ROM's 0x8071feed mail) or the appearance of an exception occuring (instead showing dspspy exception handler's 0x8bad0001 mail). I also sometimes got hangs. My guess is that it just rounded the address up or down, but I'm not 100% sure of this and don't want to investigate it currently since I'm busy investigating edge cases that things actually rely on (somewhat). This behavior can be investigated further later.

DSPTool forced 64 byte alignment starting early on (f6474b9), but that was removed fairly quickly (6e61c32). In comparison, libogc's gcdsptool has had 32 byte alignment from early on too (2cea6d99ad518c963e24c3e28834473ff5312e69), and still does (although I'm not 100% sure whether that still applies, as they use bin2s instead of generating a header, and that change did result in the length not always being a multiple of 32). The libogc documentation says that pointers "have" to be aligned while lengths "should" be a multiple of 32, for what it's worth.

I haven't tested this extensively on real hardware, but I do know that bad things happen if the address isn't properly aligned, and libogc says it should be 32-byte aligned.
alignas is a C++ keyword since C++11, and can be used in C with a header too (although I don't know the details).
@delroth delroth merged commit f056cec into dolphin-emu:master Jan 26, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants