From 960d957f4fe5430eece3d99fc0d4d5649e848f64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Tue, 6 Apr 2021 01:35:56 +0200 Subject: [PATCH 1/2] MMU: Fix SDR updates being silently dropped in some cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While 6xx_pem.pdf §7.6.1.1 mentions that the number of trailing zeros in HTABORG must be equal to the number of trailing ones in the mask (i.e. HTABORG must be properly aligned), this is actually not a hard requirement. Real hardware will just OR the base address anyway. Ignoring SDR changes would lead to incorrect emulation. Logging a warning instead of dropping the SDR update silently is a saner behaviour. --- Source/Core/Core/PowerPC/MMU.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Source/Core/Core/PowerPC/MMU.cpp b/Source/Core/Core/PowerPC/MMU.cpp index 9714af5bc7..a8ed456de7 100644 --- a/Source/Core/Core/PowerPC/MMU.cpp +++ b/Source/Core/Core/PowerPC/MMU.cpp @@ -980,14 +980,16 @@ void SDRUpdated() { u32 htabmask = SDR1_HTABMASK(PowerPC::ppcState.spr[SPR_SDR]); if (!Common::IsValidLowMask(htabmask)) - { - return; - } + WARN_LOG_FMT(POWERPC, "Invalid HTABMASK: 0b{:032b}", htabmask); + + // While 6xx_pem.pdf §7.6.1.1 mentions that the number of trailing zeros in HTABORG + // must be equal to the number of trailing ones in the mask (i.e. HTABORG must be + // properly aligned), this is actually not a hard requirement. Real hardware will just OR + // the base address anyway. Ignoring SDR changes would lead to incorrect emulation. u32 htaborg = SDR1_HTABORG(PowerPC::ppcState.spr[SPR_SDR]); if (htaborg & htabmask) - { - return; - } + WARN_LOG_FMT(POWERPC, "Invalid HTABORG: htaborg=0x{:08x} htabmask=0x{:08x}", htaborg, htabmask); + PowerPC::ppcState.pagetable_base = htaborg << 16; PowerPC::ppcState.pagetable_hashmask = ((htabmask << 10) | 0x3ff); } From 49edd5f482036dec2fa0778366e2c4ea118d89a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Tue, 6 Apr 2021 01:41:55 +0200 Subject: [PATCH 2/2] MMU: Remove a bunch of useless swaps The swaps are confusing and don't accomplish much. It was originally written like this: u32 pte = bswap(*(u32*)&base_mem[pteg_addr]); then bswap was changed to Common::swap32, and then the array access was replaced with Memory::Read_U32, leading to the useless swaps. --- Source/Core/Core/PowerPC/MMU.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Core/Core/PowerPC/MMU.cpp b/Source/Core/Core/PowerPC/MMU.cpp index a8ed456de7..3adf10a294 100644 --- a/Source/Core/Core/PowerPC/MMU.cpp +++ b/Source/Core/Core/PowerPC/MMU.cpp @@ -1113,7 +1113,7 @@ static TranslateAddressResult TranslatePageAddress(const u32 address, const XChe // hash function no 1 "xor" .360 u32 hash = (VSID ^ page_index); - u32 pte1 = Common::swap32((VSID << 7) | api | PTE1_V); + u32 pte1 = (VSID << 7) | api | PTE1_V; for (int hash_func = 0; hash_func < 2; hash_func++) { @@ -1121,7 +1121,7 @@ static TranslateAddressResult TranslatePageAddress(const u32 address, const XChe if (hash_func == 1) { hash = ~hash; - pte1 |= PTE1_H << 24; + pte1 |= PTE1_H; } u32 pteg_addr = @@ -1129,7 +1129,7 @@ static TranslateAddressResult TranslatePageAddress(const u32 address, const XChe for (int i = 0; i < 8; i++, pteg_addr += 8) { - u32 pteg = Common::swap32(Memory::Read_U32(pteg_addr)); + const u32 pteg = Memory::Read_U32(pteg_addr); if (pte1 == pteg) {