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

Debugger/Memory: Fix mem2 search #12193

Merged
merged 1 commit into from Sep 23, 2023

Conversation

malleoz
Copy link
Contributor

@malleoz malleoz commented Sep 22, 2023

Memory widget search functionality would not work for MEM2 region. Match() restricts our physical address from exceeding 0x01FFFFFF. This means that we can access cached/uncache MEM1 as our physical addresses are 0x00000000 - 0x017FFFFF. However, this logic does not work for MEM2, as our physical address is 0x10000000 to 0x117FFFFF (I believe). Searching starting from MEM2 does not show a match until 0xC0000000.

At first I assumed we would just modify memory_area, however, the call to MMU::HostIsRAMAddress() in EffectiveAddressSpaceAccessors::Search() appears to sufficiently check for valid memory bounds. Notably, we want to allow RAM searches for instances where you increase the size of MEM1 or MEM2 via Config->Advanced tab. In these instances, the current code does not allow for the upper bound to increase, whereas HostIsRAMAddress() eventually retrieves the m_memory.GetRamSizeReal() which factors in the possibly larger mem allocation.

The comment in Match() implies concern regarding the dcache. In testing, we either pagefault in MMU::IsRAMAddress() after the call to TranslateAddress() OR we reach EffectiveAddressSpaceAccessors::Matches(). In cases where we reach Matches(), we can just change the physical address check to require that we are not at or beyond dcache.

@duracellpoweredgames

This comment was marked as spam.

@malleoz
Copy link
Contributor Author

malleoz commented Sep 22, 2023

Applied a wider change. Rather than just allow MEM2, which was the original intention, it turns out that we can remove the check here altogether, except for restricting dcache attempted accesses.

Copy link
Contributor

@sepalani sepalani left a comment

Choose a reason for hiding this comment

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

Tested and the code LGTM.

This PR fixes MEM2 search using effective address space. It doesn't fix MEM2 search using physical address space, AFAICT.

@AdmiralCurtiss AdmiralCurtiss merged commit c0f690b into dolphin-emu:master Sep 23, 2023
11 checks passed
@malleoz malleoz deleted the malleo/mem2_search branch September 24, 2023 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants