From 6abe17234b3df92c594a5f3ebc00197bfbd59100 Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Mon, 27 Jul 2026 06:43:54 -0400 Subject: [PATCH] fix(dma): transfer the VIF chain tag half for every tag id The DMA chain walker only appended a chain tag's upper-half vifcodes to the VIF stream for CNT/NEXT/CALL/RET/END tags, skipping REFE/REF/REFS entirely. That gate conflates the tag id, which says where a tag's payload lives, with the tag's own upper quadword, which hardware transfers for every tag id when CHCR.TTE is set. REF/REFS/REFE are exactly the tags most likely to put something meaningful in that half, since their payload lives elsewhere; dropping it desyncs the parser because the referenced payload gets decoded as vifcode instead of being consumed as the data it is. Make the append unconditional per tag on both VIF channels instead of gating it on id, since the latched CHCR does not reliably expose TTE here and gating on it would regress chains that work today. Fixing that alone regresses PATH2 image continuation, because the interpreter used to drain pending continuation data as a raw byte grab at the top of its loop, with no regard for whether those bytes were actually a vifcode. Now that a chain can legitimately contain a real vifcode between an image DIRECT and its continuation, move the drain into the DIRECT/DIRECTHL handler itself: a pending continuation is only ever satisfied by a follow-up DIRECT/DIRECTHL, out of the front of that vifcode's own payload. Also raise the chain-tag walk cap, which was low enough to silently truncate a legitimately long chain, and add a one-time diagnostic for when it is hit so a truncation is visible instead of silent. The cap itself stays, as the only guard against a self-referential NEXT tag hanging the walker. Four tests pin all three pieces directly against processVIF1Data and PS2Memory, each written and confirmed failing against the unmodified tree before the fix, with no game content involved. --- ps2xRuntime/src/lib/ps2_memory.cpp | 64 +++--- ps2xRuntime/src/lib/ps2_vif1_interpreter.cpp | 80 +++---- ps2xTest/src/ps2_memory_tests.cpp | 215 ++++++++++++++++++- 3 files changed, 290 insertions(+), 69 deletions(-) diff --git a/ps2xRuntime/src/lib/ps2_memory.cpp b/ps2xRuntime/src/lib/ps2_memory.cpp index 3d593285a..0a7085f4d 100644 --- a/ps2xRuntime/src/lib/ps2_memory.cpp +++ b/ps2xRuntime/src/lib/ps2_memory.cpp @@ -1178,7 +1178,7 @@ bool PS2Memory::writeIORegister(uint32_t address, uint32_t value) uint32_t asr1 = m_ioRegisters[channelBase + 0x50]; uint32_t asp = (chcr >> 4) & 0x3u; const bool tieEnabled = (chcr & (1u << 7)) != 0u; - const int kMaxChainTags = 4096; + const int kMaxChainTags = 65536; std::vector chainBuf; auto appendData = [&](uint32_t srcAddr, uint32_t qwCount) @@ -1216,27 +1216,18 @@ bool PS2Memory::writeIORegister(uint32_t address, uint32_t value) } }; - auto appendCompactVif1TagData = [&](uint32_t localTagAddr, uint32_t qwCount) - { - uint32_t tagPhys = 0u; - const bool tagScratch = isScratchpad(localTagAddr); - tagPhys = translateAddress(localTagAddr); - - const uint8_t *localBase = tagScratch ? m_scratchpad : m_rdram; - const uint32_t localMax = tagScratch ? PS2_SCRATCHPAD_SIZE : PS2_RAM_SIZE; - if (tagPhys + 16u > localMax) - return; - - // VIF packet helpers embed 8 bytes of VIF stream in the DMAtag's upper half. - chainBuf.insert(chainBuf.end(), localBase + tagPhys + 8u, localBase + tagPhys + 16u); - appendData(localTagAddr + 16u, qwCount); - }; - int tagsProcessed = 0; + bool chainTagCapHit = false; uint32_t lastTagUpper = (chcr >> 16) & 0xFFFFu; + const bool vifChannel = (channelBase == 0x10009000u || channelBase == 0x10008000u); - while (tagsProcessed < kMaxChainTags) + while (true) { + if (tagsProcessed >= kMaxChainTags) + { + chainTagCapHit = true; + break; + } const uint32_t currentTagAddr = tagAddr; const bool tagInSPR = isScratchpad(tagAddr); uint32_t physTag = 0; @@ -1340,25 +1331,40 @@ bool PS2Memory::writeIORegister(uint32_t address, uint32_t value) break; } - const bool compactVifLocalTag = - (channelBase == 0x10009000u || channelBase == 0x10008000u) && - (id == 1u || id == 2u || id == 5u || id == 6u || id == 7u); - if (compactVifLocalTag) - appendCompactVif1TagData(currentTagAddr, 0u); - - if (hasPayload) + if (vifChannel) { - if (compactVifLocalTag) - appendData(currentTagAddr + 16u, tagQwc); - else - appendData(dataAddr, tagQwc); + // A VIF chain tag's upper quadword holds two embedded vifcodes (for + // example STCYCL+UNPACK, or a DIRECT header). Hardware transfers it + // for every tag id when CHCR.TTE is set, regardless of where the + // tag's own payload lives, so append it unconditionally. physTag and + // tagBase were translated and the +16 bound validated at the top of + // the loop, so no re-translation is needed here. + chainBuf.insert(chainBuf.end(), + tagBase + physTag + 8u, tagBase + physTag + 16u); } + + if (hasPayload) + appendData(dataAddr, tagQwc); + if (irq && tieEnabled) endChain = true; if (endChain) break; } + if (chainTagCapHit) + { + static bool s_vifChainTagCapWarned = false; + if (!s_vifChainTagCapWarned) + { + s_vifChainTagCapWarned = true; + std::cerr << "PS2Memory: DMA chain walker hit the " << kMaxChainTags + << "-tag cap; truncating the chain to guard against a " + "self-referential NEXT tag hanging the walker." + << std::endl; + } + } + m_ioRegisters[channelBase + 0x30] = tagAddr; m_ioRegisters[channelBase + 0x40] = asr0; m_ioRegisters[channelBase + 0x50] = asr1; diff --git a/ps2xRuntime/src/lib/ps2_vif1_interpreter.cpp b/ps2xRuntime/src/lib/ps2_vif1_interpreter.cpp index c0af2a3ac..78980f327 100644 --- a/ps2xRuntime/src/lib/ps2_vif1_interpreter.cpp +++ b/ps2xRuntime/src/lib/ps2_vif1_interpreter.cpp @@ -265,37 +265,6 @@ void PS2Memory::processVIF1Data(const uint8_t *data, uint32_t sizeBytes) while (pos + 4 <= sizeBytes) { - if (m_vif1PendingPath2ImageQwc != 0u) - { - const uint32_t availableQw = (sizeBytes - pos) / 16u; - if (availableQw == 0u) - { - break; - } - - const uint32_t chunkQw = std::min(m_vif1PendingPath2ImageQwc, availableQw); - std::vector imagePacket(16u + static_cast(chunkQw) * 16u, 0u); - const uint64_t imageTag = - static_cast(chunkQw & 0x7FFFu) | - ((m_vif1PendingPath2ImageQwc == chunkQw) ? (1ull << 15) : 0ull) | - (static_cast(kGifFmtImage) << 58); - std::memcpy(imagePacket.data(), &imageTag, sizeof(imageTag)); - std::memcpy(imagePacket.data() + 16u, data + pos, static_cast(chunkQw) * 16u); - submitGifPacket(GifPathId::Path2, - imagePacket.data(), - static_cast(imagePacket.size()), - true, - m_vif1PendingPath2DirectHl); - - pos += chunkQw * 16u; - m_vif1PendingPath2ImageQwc -= chunkQw; - if (m_vif1PendingPath2ImageQwc == 0u) - { - m_vif1PendingPath2DirectHl = false; - } - continue; - } - uint32_t cmd; memcpy(&cmd, data + pos, 4); pos += 4; @@ -468,16 +437,51 @@ void PS2Memory::processVIF1Data(const uint8_t *data, uint32_t sizeBytes) if (qwCount > 0) { const bool directHl = (opcode == VIF_DIRECTHL); - submitGifPacket(GifPathId::Path2, data + pos, qwCount * 16, true, directHl); + uint32_t chunkPos = pos; + uint32_t remainingQw = qwCount; + + // A pending PATH2 image continuation from an earlier DIRECT/DIRECTHL is + // satisfied out of the FRONT of this packet's own payload, before any of + // it is reinterpreted as a fresh GIF tag. This keeps any vifcodes between + // the two DIRECTs (NOP padding, etc.) from being misread as image data. + if (m_vif1PendingPath2ImageQwc != 0u) + { + const uint32_t chunkQw = std::min(m_vif1PendingPath2ImageQwc, remainingQw); + std::vector imagePacket(16u + static_cast(chunkQw) * 16u, 0u); + const uint64_t imageTag = + static_cast(chunkQw & 0x7FFFu) | + ((m_vif1PendingPath2ImageQwc == chunkQw) ? (1ull << 15) : 0ull) | + (static_cast(kGifFmtImage) << 58); + std::memcpy(imagePacket.data(), &imageTag, sizeof(imageTag)); + std::memcpy(imagePacket.data() + 16u, data + chunkPos, static_cast(chunkQw) * 16u); + submitGifPacket(GifPathId::Path2, + imagePacket.data(), + static_cast(imagePacket.size()), + true, + m_vif1PendingPath2DirectHl); + + chunkPos += chunkQw * 16u; + remainingQw -= chunkQw; + m_vif1PendingPath2ImageQwc -= chunkQw; + if (m_vif1PendingPath2ImageQwc == 0u) + { + m_vif1PendingPath2DirectHl = false; + } + } - const uint32_t imageQw = gifImageQwcFromTag(data + pos, qwCount * 16u); - if (imageQw != 0u) + if (remainingQw > 0u) { - const uint32_t inlineImageQw = (qwCount > 0u) ? (qwCount - 1u) : 0u; - if (imageQw > inlineImageQw) + submitGifPacket(GifPathId::Path2, data + chunkPos, remainingQw * 16, true, directHl); + + const uint32_t imageQw = gifImageQwcFromTag(data + chunkPos, remainingQw * 16u); + if (imageQw != 0u) { - m_vif1PendingPath2ImageQwc = imageQw - inlineImageQw; - m_vif1PendingPath2DirectHl = directHl; + const uint32_t inlineImageQw = remainingQw - 1u; + if (imageQw > inlineImageQw) + { + m_vif1PendingPath2ImageQwc = imageQw - inlineImageQw; + m_vif1PendingPath2DirectHl = directHl; + } } } } diff --git a/ps2xTest/src/ps2_memory_tests.cpp b/ps2xTest/src/ps2_memory_tests.cpp index 3bcb7a5fd..cf022a295 100644 --- a/ps2xTest/src/ps2_memory_tests.cpp +++ b/ps2xTest/src/ps2_memory_tests.cpp @@ -1449,6 +1449,213 @@ void register_ps2_memory_tests() t.IsTrue((mem.readIORegister(0x1000E010u) & 0x2u) != 0u, "VIF1 DMA completion should raise D_STAT channel bit"); }); + tc.Run("VIF1 DMA chain transfers a REF tag half so its UNPACK executes", [](TestCase &t) + { + PS2Memory mem; + t.IsTrue(mem.initialize(), "PS2Memory initialize should succeed"); + std::memset(mem.getVU1Data(), 0, PS2_VU1_DATA_SIZE); + + constexpr uint32_t kVif1Ch = 0x10009000u; + constexpr uint32_t kTag0 = 0x00029000u; + constexpr uint32_t kTag1 = kTag0 + 0x20u; + constexpr uint32_t kRefData = 0x00029200u; + + uint8_t *rdram = mem.getRDRAM(); + + // REF (id 3): the referenced quadword lives at kRefData; the chain + // itself continues at kTag0+16, not at the referenced address. + writeDmaTag(rdram, kTag0, makeDmaTag(1u, 3u, kRefData, false)); + + // The tag's upper half is two real vifcodes: an implicit NOP (left + // zeroed by writeDmaTag) followed by an UNPACK V4-32 that claims the + // referenced quadword as its source data. + const uint32_t unpackCmd = makeVifCmd(0x6Cu, 1u, 0x0005u); + std::memcpy(rdram + kTag0 + 12u, &unpackCmd, sizeof(unpackCmd)); + + writeDmaTag(rdram, kTag1, makeDmaTag(0u, 7u, 0u, false)); // END, stops the chain. + + const uint32_t p0 = 0x11111111u; + const uint32_t p1 = 0x22222222u; + const uint32_t p2 = 0x33333333u; + const uint32_t p3 = 0x44444444u; + std::memcpy(rdram + kRefData + 0u, &p0, sizeof(p0)); + std::memcpy(rdram + kRefData + 4u, &p1, sizeof(p1)); + std::memcpy(rdram + kRefData + 8u, &p2, sizeof(p2)); + std::memcpy(rdram + kRefData + 12u, &p3, sizeof(p3)); + + t.IsTrue(mem.writeIORegister(kVif1Ch + 0x30u, kTag0), "write VIF1 TADR should succeed"); + t.IsTrue(mem.writeIORegister(kVif1Ch + 0x00u, 0x104u), "write VIF1 CHCR STR|CHAIN should succeed"); + + mem.processPendingTransfers(); + + const uint8_t *vu1 = mem.getVU1Data(); + const uint32_t dest = 5u * 16u; + uint32_t x = 0, y = 0, z = 0, w = 0; + std::memcpy(&x, vu1 + dest + 0u, 4); + std::memcpy(&y, vu1 + dest + 4u, 4); + std::memcpy(&z, vu1 + dest + 8u, 4); + std::memcpy(&w, vu1 + dest + 12u, 4); + t.Equals(x, p0, "REF tag upper-half UNPACK should land the referenced quadword at VU1 addr 5 (x)"); + t.Equals(y, p1, "REF tag upper-half UNPACK should land the referenced quadword at VU1 addr 5 (y)"); + t.Equals(z, p2, "REF tag upper-half UNPACK should land the referenced quadword at VU1 addr 5 (z)"); + t.Equals(w, p3, "REF tag upper-half UNPACK should land the referenced quadword at VU1 addr 5 (w)"); + }); + + tc.Run("VIF1 DMA chain does not desync MSCAL after a REF tag with a non-empty half", [](TestCase &t) + { + PS2Memory mem; + t.IsTrue(mem.initialize(), "PS2Memory initialize should succeed"); + + constexpr uint32_t kVif1Ch = 0x10009000u; + constexpr uint32_t kTagA = 0x00029400u; // REF + constexpr uint32_t kTagB = kTagA + 0x10u; // END, immediately follows (REF does not jump) + constexpr uint32_t kPoisonData = 0x00029600u; + + uint8_t *rdram = mem.getRDRAM(); + + // TagA: REF (id 3), qwc=1, referenced payload at kPoisonData. Its own + // upper half is a real NOP+UNPACK pair that claims the referenced + // quadword as UNPACK data, so the bytes at kPoisonData are never + // parsed as vifcodes when the tag half is transferred correctly. + writeDmaTag(rdram, kTagA, makeDmaTag(1u, 3u, kPoisonData, false)); + const uint32_t unpackCmd = makeVifCmd(0x6Cu, 1u, 0x0000u); + std::memcpy(rdram + kTagA + 12u, &unpackCmd, sizeof(unpackCmd)); + + // TagB: END (id 7), qwc=0. Its upper half carries an implicit NOP + // followed by MSCAL imm=100 (startPC = 100*8 = 800). + writeDmaTag(rdram, kTagB, makeDmaTag(0u, 7u, 0u, false)); + const uint32_t mscalCmd = makeVifCmd(0x14u, 0u, 100u); + std::memcpy(rdram + kTagB + 12u, &mscalCmd, sizeof(mscalCmd)); + + // If TagA's own half is dropped, these bytes become the start of the + // vifcode stream instead of UNPACK's source data: a V4-32 UNPACK + // requesting 4 vectors (64 bytes) that the short chain cannot supply, + // which runs the cursor past the end of the buffer. + const uint32_t poisonWord0 = makeVifCmd(0x6Cu, 4u, 0u); + std::memcpy(rdram + kPoisonData + 0u, &poisonWord0, sizeof(poisonWord0)); + std::memset(rdram + kPoisonData + 4u, 0, 12u); + + struct MscalCall + { + uint32_t startPC; + uint32_t top; + uint32_t itop; + }; + std::vector mscalCalls; + mem.setVu1MscalCallback([&](uint32_t startPC, uint32_t top, uint32_t itop) + { + mscalCalls.push_back({startPC, top, itop}); + }); + + t.IsTrue(mem.writeIORegister(kVif1Ch + 0x30u, kTagA), "write VIF1 TADR should succeed"); + t.IsTrue(mem.writeIORegister(kVif1Ch + 0x00u, 0x104u), "write VIF1 CHCR STR|CHAIN should succeed"); + + mem.processPendingTransfers(); + + t.Equals(mscalCalls.size(), static_cast(1u), "MSCAL after a transferred REF half should fire exactly once"); + if (!mscalCalls.empty()) + { + t.Equals(mscalCalls[0].startPC, 800u, "MSCAL imm=100 should compute startPC=800"); + } + }); + + tc.Run("VIF1 DIRECT continuation sources image data from its own payload, not intervening vifcodes", [](TestCase &t) + { + PS2Memory mem; + t.IsTrue(mem.initialize(), "PS2Memory initialize should succeed"); + + std::vector> captured; + mem.setGifPacketCallback([&](const uint8_t *data, uint32_t sizeBytes) + { + captured.emplace_back(data, data + sizeBytes); + }); + + // Packet 1: DIRECT with a GIF IMAGE tag claiming 3 quadwords total, + // but only 1 inline quadword follows -> arms a 2-quadword pending + // continuation. Followed by a real NOP vifcode. + const uint32_t directCmd1 = makeVifCmd(0x50u, 0u, 2u); // DIRECT, 2 QW payload + const std::vector inlineQw(16u, 0xA0u); + const uint32_t nopCmd = makeVifCmd(0x00u, 0u, 0x1234u); + + std::vector packet1; + appendU32(packet1, directCmd1); + appendU64(packet1, makeGifTag(3u, GIF_FMT_IMAGE, 0u)); // tag lo: nloop=3, flg=IMAGE + appendU64(packet1, 0u); // tag hi: unused when nreg=0 + packet1.insert(packet1.end(), inlineQw.begin(), inlineQw.end()); + appendU32(packet1, nopCmd); + + mem.processVIF1Data(packet1.data(), static_cast(packet1.size())); + t.Equals(mem.vif1_regs.code, nopCmd, + "the NOP between an IMAGE DIRECT and its continuation should execute as a vifcode, not be swallowed as image data"); + + // Packet 2: the continuation DIRECT. Its 3-quadword payload supplies + // the 2 remaining pending image quadwords up front, then 1 quadword + // of its own new content. + std::vector qw0(16u, 0xC1u); + std::vector qw1(16u, 0xC2u); + std::vector qw2(16u, 0xC3u); + std::vector packet2; + const uint32_t directCmd2 = makeVifCmd(0x50u, 0u, 3u); + appendU32(packet2, directCmd2); + packet2.insert(packet2.end(), qw0.begin(), qw0.end()); + packet2.insert(packet2.end(), qw1.begin(), qw1.end()); + packet2.insert(packet2.end(), qw2.begin(), qw2.end()); + + mem.processVIF1Data(packet2.data(), static_cast(packet2.size())); + + t.Equals(captured.size(), static_cast(3u), + "DIRECT#1, the image continuation, and the continuation's own remainder should each emit one GIF packet"); + if (captured.size() >= 2) + { + t.Equals(captured[1].size(), static_cast(48u), "continuation image packet should be a synthetic tag plus 2 QW"); + bool continuationOk = captured[1].size() == 48u; + if (continuationOk) + { + continuationOk = std::equal(qw0.begin(), qw0.end(), captured[1].begin() + 16) && + std::equal(qw1.begin(), qw1.end(), captured[1].begin() + 32); + } + t.IsTrue(continuationOk, "continuation image quadwords should come from the continuation DIRECT's own payload (qw0, qw1)"); + } + if (captured.size() >= 3) + { + t.IsTrue(captured[2].size() == 16u && std::equal(qw2.begin(), qw2.end(), captured[2].begin()), + "leftover payload after the continuation is satisfied should submit as the DIRECT's own content (qw2)"); + } + }); + + tc.Run("VIF1 DMA chain walks past 4096 tags to the real end of a longer chain", [](TestCase &t) + { + PS2Memory mem; + t.IsTrue(mem.initialize(), "PS2Memory initialize should succeed"); + + constexpr uint32_t kVif1Ch = 0x10009000u; + constexpr uint32_t kChainBase = 0x00030000u; + constexpr uint32_t kTagCount = 4200u; // > 4096, comfortably under 65536 + + uint8_t *rdram = mem.getRDRAM(); + + for (uint32_t i = 0; i < kTagCount - 1u; ++i) + { + // CNT (id 1), qwc=0: the chain continues immediately at the next tag. + writeDmaTag(rdram, kChainBase + i * 16u, makeDmaTag(0u, 1u, 0u, false)); + } + + // Final tag: END (id 7) with an ITOP vifcode in its upper half. Only + // reachable if the walker does not stop short at the old 4096 cap. + const uint32_t lastTagAddr = kChainBase + (kTagCount - 1u) * 16u; + writeDmaTag(rdram, lastTagAddr, makeDmaTag(0u, 7u, 0u, false)); + const uint32_t itopCmd = makeVifCmd(0x04u, 0u, 0x0077u); + std::memcpy(rdram + lastTagAddr + 12u, &itopCmd, sizeof(itopCmd)); + + t.IsTrue(mem.writeIORegister(kVif1Ch + 0x30u, kChainBase), "write VIF1 TADR should succeed"); + t.IsTrue(mem.writeIORegister(kVif1Ch + 0x00u, 0x104u), "write VIF1 CHCR STR|CHAIN should succeed"); + + mem.processPendingTransfers(); + + t.Equals(mem.vif1_regs.itops, 0x77u, + "a chain longer than the old 4096-tag cap should still reach and process its final tag"); + }); + tc.Run("GIF DMA chain CALL sources payload from TADR+16", [](TestCase &t) { PS2Memory mem; @@ -1788,7 +1995,7 @@ void register_ps2_memory_tests() t.IsTrue(imageOk, "VIF1 DIRECT image should update GS VRAM through GIF path2"); }); - tc.Run("VIF1 DIRECT image tag can continue with raw image qwords", [](TestCase &t) + tc.Run("VIF1 DIRECT image tag continues via a follow-up DIRECT", [](TestCase &t) { PS2Memory mem; t.IsTrue(mem.initialize(), "PS2Memory initialize should succeed"); @@ -1813,10 +2020,14 @@ void register_ps2_memory_tests() gs.writeRegister(GS_REG_TRXREG, (4ull << 0) | (1ull << 32)); gs.writeRegister(GS_REG_TRXDIR, 0ull); + // The image data continues in a second DIRECT, not as unwrapped raw + // quadwords: PATH2 image continuation is always carried by another + // DIRECT/DIRECTHL vifcode, the same way a real VIF chain would emit it. std::vector packet; appendU32(packet, makeVifCmd(0x50u, 0u, 1u)); // DIRECT 1 QW payload: GIF IMAGE tag only. appendU64(packet, makeGifTag(1u, GIF_FMT_IMAGE, 0u, true)); appendU64(packet, 0ull); + appendU32(packet, makeVifCmd(0x50u, 0u, 1u)); // continuation DIRECT: 1 QW of image data. for (uint32_t i = 0; i < 16u; ++i) { packet.push_back(static_cast(0xA0u + i)); @@ -1838,7 +2049,7 @@ void register_ps2_memory_tests() } } } - t.IsTrue(imageOk, "raw qwords after a DIRECT image tag should continue the PATH2 image upload"); + t.IsTrue(imageOk, "a follow-up DIRECT after a DIRECT image tag should continue the PATH2 image upload"); }); tc.Run("unaligned accesses throw", [](TestCase &t)