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
PowerPC: Refactor ReadFromHardware #10642
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lioncash
reviewed
May 7, 2022
This fixes a problem where Dolphin could crash if a non-translated read crossed the end of a physical memory region. The same change was applied to WriteToHardware in ecbce0a.
df57ad6
to
df2dd3d
Compare
lioncash
reviewed
May 7, 2022
This refactorization is done just to match the order that I made WriteToHardware use in 543ed8a. For WriteToHardware, it's important that things like MMIO and gather pipe are handled before we reach a special piece of code that only should get triggered for writes that hit memory directly, but for ReadFromHardware we don't have any code like that.
df2dd3d
to
ed40b43
Compare
Source/Core/Core/PowerPC/MMU.cpp
Outdated
| const u32 em_address_start_page = em_address & ~(HW_PAGE_SIZE - 1); | ||
| const u32 em_address_end_page = (em_address + sizeof(T) - 1) & ~(HW_PAGE_SIZE - 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worth creating a constant like PAGE_ALIGNMENT_MASK and using it here and in WriteToHardware?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
lioncash
approved these changes
May 7, 2022
|
Pada tanggal Sab, 7 Mei 2022 20.43, Mai M. ***@***.***>
menulis:
… ***@***.**** commented on this pull request.
------------------------------
In Source/Core/Core/PowerPC/MMU.cpp
<#10642 (comment)>:
> em_address = translated_addr.address;
}
- // TODO: Make sure these are safe for unaligned addresses.
+ if (flag == XCheckTLBFlag::Read && (em_address & 0xF8000000) == 0x08000000)
+ {
+ if (em_address < 0x0c000000)
+ return EFB_Read(em_address);
+ else
+ return (T)Memory::mmio_mapping->Read<typename std::make_unsigned<T>::type>(em_address);
Missed in previous review (my bad)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
When I was creating PR #9964, I ended up refactoring WriteToHardware a bunch. This PR applies most of those refactorizations to ReadFromHardware.
See commit descriptions for details.