diff --git a/pcsx2/arm64/iMMI-arm64.cpp b/pcsx2/arm64/iMMI-arm64.cpp index ac7087e4ed..880e1a34cf 100644 --- a/pcsx2/arm64/iMMI-arm64.cpp +++ b/pcsx2/arm64/iMMI-arm64.cpp @@ -1350,13 +1350,27 @@ void recQFSRV() // Adjacent-source fast path: when Rs == Rt+1 the 256-bit // {Rt:Rs} window already exists contiguously in the GPR array - // (GPR.r[Rt] immediately precedes GPR.r[Rt+1]==GPR.r[Rs], 32 bytes), now - // memory-coherent after the flushes above. Read the unaligned 128 bits - // directly at &GPR.r[Rt] + sa and skip the two temp stores. sa is 0..15 so - // the load stays within the two registers' 32 bytes. Gate on Rt != 0 to avoid - // depending on GPR.r[0] holding zero in memory (the slow path Movi's it). + // (GPR.r[Rt] immediately precedes GPR.r[Rt+1]==GPR.r[Rs], 32 bytes). + // Read the unaligned 128 bits directly at &GPR.r[Rt] + sa and skip the two + // temp stores. sa is 0..15 so the load stays within the two registers' 32 + // bytes. Gate on Rt != 0 to avoid depending on GPR.r[0] holding zero in + // memory (the slow path Movi's it). if (_Rt_ != 0 && _Rs_ == _Rt_ + 1) { + // The flushes above do NOT make this window memory-coherent: mmiFlushReg + // is _deleteEEreg, which reconciles const-prop and the scalar/NEON slots + // and never touches the pins. Under lazy-dirty the pin is authoritative + // for UD[0] and armStoreEERegPtrRaw elides the canonical store, so a + // pinned source's lower half in memory is routinely stale here. Unlike + // every other raw quad-load site we cannot merge after the load — the + // read straddles two guest registers — so flush the two pins the window + // actually covers. It covers exactly r[Rt] and r[Rt+1]: sa <= 15 over + // their 32 bytes. Four adjacent pairs are both-pinned — ($at,$v0) + // ($v0,$v1) ($v1,$a0) ($a0,$a1) — and eight more have one pinned + // operand, which is the register range a funnel-shift memcpy uses. (SM-010) + armFlushEEGPRPin(_Rt_); + armFlushEEGPRPin(_Rs_); + armLoadEERegPtr(RWSCRATCH, &cpuRegs.sa); // Clamp sa to 0..15 before indexing host memory. MTSA masks at the // write, cpuRegs.sa can't hold >= 16 and this is belt-and-braces. diff --git a/pcsx2/arm64/iR5900-arm64.h b/pcsx2/arm64/iR5900-arm64.h index 2558f4e0cf..157ccc8cf3 100644 --- a/pcsx2/arm64/iR5900-arm64.h +++ b/pcsx2/arm64/iR5900-arm64.h @@ -343,6 +343,19 @@ static __fi void armFlushEEGPRPins() armAsm->Str(pin.host, armCpuRegMem(&cpuRegs.GPR.r[pin.gpr].UD[0])); } +// Flush ONE pin mirror back to canonical memory. Not a dirty-subset heuristic +// (see the note above) — this is for the emitter that reads a STATICALLY KNOWN +// guest-GPR memory window raw, where neither of the two normal coherence tools +// applies: pin substitution (armLoadEERegPtr) can't serve an unaligned read, +// and the post-load lane merge (armMergeEEResidentIntoQuad) can't fix a quad +// that straddles two guest registers. The caller must name every register its +// window covers. Emits nothing when gpr is not pinned. +static __fi void armFlushEEGPRPin(int gpr) +{ + if (const vixl::aarch64::Register* pin = armEEPinForGPR(gpr)) + armAsm->Str(*pin, armCpuRegMem(&cpuRegs.GPR.r[gpr].UD[0])); +} + // Flush only the CALLER-saved pins. Required before any C call that is // followed by armReloadEEClobberedPins: the reload reads canonical memory, // which under lazy-dirty is stale until flushed — the pair would otherwise diff --git a/tests/ctest/core/recompilers/ee_rec_mmi_coherence_tests.cpp b/tests/ctest/core/recompilers/ee_rec_mmi_coherence_tests.cpp index f1b23d78bd..bc7f1cafec 100644 --- a/tests/ctest/core/recompilers/ee_rec_mmi_coherence_tests.cpp +++ b/tests/ctest/core/recompilers/ee_rec_mmi_coherence_tests.cpp @@ -1214,3 +1214,80 @@ TEST(EeRecMmiCoherence, SqFaultPathStoresResidentQuad) faulted |= vtlb_IsFaultingPC(a); EXPECT_TRUE(faulted) << "MMIO SQ did not take the fastmem-fault/backpatch path"; } + +// ============================================================================ +// Raw guest-GPR memory reads vs the lazy-dirty pins (SM-010) +// ============================================================================ +// +// The pins (kEEPinTable) mirror GPR.r[n].UD[0] for nine hot guest regs, and +// under lazy-dirty the PIN is authoritative: _eeStoreGPRDestReg resolves a +// pinned dest to the pin itself and armStoreEERegPtrRaw then emits NOTHING, so +// GPR.r[n].UD[0] in memory is stale until a seam flushes (armFlushEEGPRPins). +// +// mmiFlushReg is NOT such a seam — it reconciles const-prop, the scalar slot +// and the NEON slot (_deleteEEreg) and never touches the pins. So any MMI path +// that reads guest-GPR memory RAW, rather than via mmiLoadReg (which merges +// through armMergeEEResidentIntoQuad) or armLoadEERegPtr (which substitutes the +// pin), reads a stale lower half. +// +// recQFSRV's adjacent-source fast path (Rs == Rt+1) is exactly that: it takes +// &GPR.r[Rt] and Ldr's 128 unaligned bits across the two registers. Four +// adjacent pairs are BOTH pinned — ($at,$v0) ($v0,$v1) ($v1,$a0) ($a0,$a1) — +// and eight more have one pinned operand, which is precisely the register range +// a funnel-shift memcpy loop uses. + +// Rt is pinned ($a0 → x27) and written earlier in the same block, so its +// canonical memory slot is stale when QFSRV's fast path reads it. +TEST(EeRecMmiCoherence, QfsrvAdjacentFastPathSeesDirtyPinnedRt) +{ + EeRecTestHarness h; + h.SetMmiPair(reg::a0, 0x1122'3344'5566'7788ull, 0x99AA'BBCC'DDEE'FF00ull); // Rt + h.SetMmiPair(reg::a1, 0xAABB'CCDD'1122'3344ull, 0x5566'7788'99AA'BBCCull); // Rs == Rt+1 + h.SetGpr64(reg::t0, 0xF0E0'D0C0'B0A0'9080ull); + h.LoadProgram({ + ee::MTSAB(reg::zero, 4), + OR (reg::a0, reg::t0, reg::zero), // pinned write: pin dirty, memory stale + ee::QFSRV(reg::v0, reg::a1, reg::a0), + }); + h.Run(); + // {Rs:Rt} >> 4 bytes with Rt.UD[0] = the value just written. + h.ExpectMmiPair(reg::v0, 0xDDEE'FF00'F0E0'D0C0ull, 0x1122'3344'99AA'BBCCull); + // A stale read yields the pre-write Rt bytes: UD[0] == 0xDDEEFF0011223344. +} + +// Same defect through the other operand: Rs is pinned ($a1 → x21) and dirty, +// so the upper half of the funnel result comes from stale memory. +TEST(EeRecMmiCoherence, QfsrvAdjacentFastPathSeesDirtyPinnedRs) +{ + EeRecTestHarness h; + h.SetMmiPair(reg::a0, 0x1122'3344'5566'7788ull, 0x99AA'BBCC'DDEE'FF00ull); // Rt + h.SetMmiPair(reg::a1, 0xAABB'CCDD'1122'3344ull, 0x5566'7788'99AA'BBCCull); // Rs == Rt+1 + h.SetGpr64(reg::t0, 0xF0E0'D0C0'B0A0'9080ull); + h.LoadProgram({ + ee::MTSAB(reg::zero, 4), + OR (reg::a1, reg::t0, reg::zero), // pinned write to Rs + ee::QFSRV(reg::v0, reg::a1, reg::a0), + }); + h.Run(); + h.ExpectMmiPair(reg::v0, 0xDDEE'FF00'1122'3344ull, 0xB0A0'9080'99AA'BBCCull); + // A stale read yields the pre-write Rs bytes: UD[1] == 0x1122334499AABBCC. +} + +// Green control: identical dirty-pin setup, but Rs != Rt+1 so the temp-buffer +// path runs. That path loads through mmiLoadReg, which merges the dirty pin — +// it must stay correct both before and after the fast path is gated, proving +// the divergence above belongs to the fast path and not to the expectations. +TEST(EeRecMmiCoherence, QfsrvNonAdjacentPathMergesDirtyPin) +{ + EeRecTestHarness h; + h.SetMmiPair(reg::a0, 0x1122'3344'5566'7788ull, 0x99AA'BBCC'DDEE'FF00ull); // Rt + h.SetMmiPair(reg::a2, 0xAABB'CCDD'1122'3344ull, 0x5566'7788'99AA'BBCCull); // Rs != Rt+1 + h.SetGpr64(reg::t0, 0xF0E0'D0C0'B0A0'9080ull); + h.LoadProgram({ + ee::MTSAB(reg::zero, 4), + OR (reg::a0, reg::t0, reg::zero), + ee::QFSRV(reg::v0, reg::a2, reg::a0), + }); + h.Run(); + h.ExpectMmiPair(reg::v0, 0xDDEE'FF00'F0E0'D0C0ull, 0x1122'3344'99AA'BBCCull); +}