From 85af97ecdc0675313ce73a717ce7bdea1e2a7706 Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Sun, 26 Jul 2026 09:09:02 -0400 Subject: [PATCH] fix(gs): write PMODE/SMODE2 directly in sceGsResetGraph instead of through colliding A+D addresses sceGsResetGraph builds a GIF A+D packet to program the display-mode registers, and used addresses 0x41 and 0x42 for PMODE and SMODE2. Those addresses are not free A+D slots: GS_REG_SCISSOR_2 is 0x41 and GS_REG_ALPHA_1 is 0x42 in the GIF drawing-register address space, and writeRegister/processGIFPacket already handle them as SCISSOR_2/ALPHA_1 writes into the active drawing context. As a result the packet's PMODE/SMODE2 entries were reinterpreted as scissor and alpha-blend writes, the drawing context got silently reprogrammed, and PMODE/SMODE2 themselves never received the values the caller asked for. PMODE and SMODE2 are privileged registers, not GIF drawing registers, so they don't belong in the A+D stream at all. This routes them directly to the privileged GSRegisters struct instead, the same way applyGsDispEnv already does for the display-env registers. The A+D packet keeps only the five entries that don't collide with anything (DISPFB1/DISPLAY1/DISPFB2/DISPLAY2/BGCOLOR), shrunk accordingly. The other five privileged registers set through this same packet stay on the A+D path deliberately: they have dedicated non-drawing pass-through cases and never collided with anything, so migrating them here would be unrelated churn. If full consistency with the direct-write convention is wanted later, migrating all seven and dropping the DMA packet entirely is the natural follow-up, but it's a separate change. Added a regression test that seeds the drawing context through the same addresses the old code collided on, runs sceGsResetGraph, and asserts PMODE/SMODE2 land in privileged state while the drawing context is untouched. --- ps2xRuntime/src/lib/Kernel/Stubs/GS.cpp | 32 ++++++++------- ps2xTest/src/ps2_gs_tests.cpp | 54 +++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 15 deletions(-) diff --git a/ps2xRuntime/src/lib/Kernel/Stubs/GS.cpp b/ps2xRuntime/src/lib/Kernel/Stubs/GS.cpp index 0fd65656b..45b4550f1 100644 --- a/ps2xRuntime/src/lib/Kernel/Stubs/GS.cpp +++ b/ps2xRuntime/src/lib/Kernel/Stubs/GS.cpp @@ -989,34 +989,36 @@ namespace ps2_stubs if (runtime) { - uint32_t pktAddr = runtime->guestMalloc(128u, 16u); + // PMODE/SMODE2 are privileged registers, not GIF A+D drawing registers: + // their old A+D addresses 0x41/0x42 alias SCISSOR_2/ALPHA_1, so route them + // straight to privileged state (as applyGsDispEnv does) instead. + runtime->memory().gs().pmode = pmode; + runtime->memory().gs().smode2 = smode2; + + uint32_t pktAddr = runtime->guestMalloc(96u, 16u); if (pktAddr != 0u) { uint8_t *pkt = getMemPtr(rdram, pktAddr); if (pkt) { uint64_t *q = reinterpret_cast(pkt); - q[0] = makeGiftagAplusD(7u); + q[0] = makeGiftagAplusD(5u); q[1] = 0xEULL; - q[2] = pmode; - q[3] = 0x41ULL; - q[4] = smode2; - q[5] = 0x42ULL; + q[2] = dispfb; + q[3] = 0x59ULL; + q[4] = display; + q[5] = 0x5aULL; q[6] = dispfb; - q[7] = 0x59ULL; + q[7] = 0x5bULL; q[8] = display; - q[9] = 0x5aULL; - q[10] = dispfb; - q[11] = 0x5bULL; - q[12] = display; - q[13] = 0x5cULL; - q[14] = bgcolor; - q[15] = 0x5fULL; + q[9] = 0x5cULL; + q[10] = bgcolor; + q[11] = 0x5fULL; constexpr uint32_t GIF_CHANNEL = 0x1000A000; constexpr uint32_t CHCR_STR_MODE0 = 0x101u; auto &mem = runtime->memory(); mem.writeIORegister(GIF_CHANNEL + 0x10u, pktAddr); - mem.writeIORegister(GIF_CHANNEL + 0x20u, 8u); + mem.writeIORegister(GIF_CHANNEL + 0x20u, 6u); mem.writeIORegister(GIF_CHANNEL + 0x00u, CHCR_STR_MODE0); mem.processPendingTransfers(); runtime->guestFree(pktAddr); diff --git a/ps2xTest/src/ps2_gs_tests.cpp b/ps2xTest/src/ps2_gs_tests.cpp index 4c3e8ec1c..c82adfbf5 100644 --- a/ps2xTest/src/ps2_gs_tests.cpp +++ b/ps2xTest/src/ps2_gs_tests.cpp @@ -3343,6 +3343,60 @@ void register_ps2_gs_tests() "sceGsResetGraph should free its temporary GIF packet"); }); + tc.Run("sceGsResetGraph writes PMODE/SMODE2 to privileged state without clobbering the drawing context", [](TestCase &t) + { + PS2Runtime runtime; + t.IsTrue(runtime.memory().initialize(), "runtime memory initialize should succeed"); + // Bind the GS core to guest memory up front (the same one-time bind + // sceGsResetGraph triggers internally via syncCoreSubsystems()) so that + // seeding the drawing context below survives the call under test — + // otherwise the lazy first-time bind would reset the context we seeded. + t.IsTrue(runtime.syncCoreSubsystems(), "runtime core subsystems should sync"); + + constexpr uint16_t kScissorX0 = 0x111; + constexpr uint16_t kScissorX1 = 0x222; + constexpr uint16_t kScissorY0 = 0x333; + constexpr uint16_t kScissorY1 = 0x444; + const uint64_t kScissorSentinel = static_cast(kScissorX0) | + (static_cast(kScissorX1) << 16) | + (static_cast(kScissorY0) << 32) | + (static_cast(kScissorY1) << 48); + constexpr uint64_t kAlphaSentinel = 0x1122334455667788ULL; + + // Seed the drawing context directly through the same register addresses + // sceGsResetGraph's old (buggy) A+D packet used for PMODE/SMODE2. + runtime.gs().writeRegister(0x41, kScissorSentinel); + runtime.gs().writeRegister(0x42, kAlphaSentinel); + + // Poison privileged state so a positive assertion below is meaningful. + runtime.memory().gs().pmode = 0; + runtime.memory().gs().smode2 = 0; + + // Use the runtime's own guest memory (not a disconnected local buffer): + // the temporary GIF packet sceGsResetGraph builds and kicks is read back + // by the DMA engine through runtime memory, so the packet writer and the + // DMA reader must share the same backing storage for the kick to have any + // observable effect on GS context state. + uint8_t *const rdram = runtime.memory().getRDRAM(); + R5900Context ctx{}; + setRegU32(ctx, 4, 0u); + setRegU32(ctx, 5, 1u); + setRegU32(ctx, 6, 2u); + setRegU32(ctx, 7, 1u); + ps2_stubs::sceGsResetGraph(rdram, &ctx, &runtime); + + t.Equals(runtime.memory().gs().pmode, makePmode(1, 0, 0, 0, 0, 0x80), + "sceGsResetGraph writes PMODE to privileged state"); + t.Equals(runtime.memory().gs().smode2, static_cast((1u & 1u) | ((1u & 1u) << 1)), + "sceGsResetGraph writes SMODE2 to privileged state"); + + GSDebugSnapshot snap = runtime.gs().getDebugSnapshot(); + t.IsTrue(snap.ctx[1].scissor.x0 == kScissorX0 && snap.ctx[1].scissor.x1 == kScissorX1 && + snap.ctx[1].scissor.y0 == kScissorY0 && snap.ctx[1].scissor.y1 == kScissorY1, + "sceGsResetGraph leaves SCISSOR_2 untouched"); + t.Equals(snap.ctx[0].alpha, kAlphaSentinel, "sceGsResetGraph leaves ALPHA_1 untouched"); + }); + tc.Run("sceGsSyncV waits on VBlank and reports interlaced field parity", [](TestCase &t) { notifyRuntimeStop();