From 8985587c131f1f869d07966cf19460e35c2ef0cc Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Mon, 20 Jul 2026 20:02:31 -0400 Subject: [PATCH] fix(runtime): accept non-zero memory-card slots in isValidMcPortSlot isValidMcPortSlot() hardcoded slot == 0. It gates the ten port/slot-addressed sceMc* entry points (sceMcChdir, sceMcDelete, sceMcFormat, sceMcGetDir, sceMcGetInfo, sceMcMkdir, sceMcOpen, sceMcRename, sceMcSetFileInfo, sceMcUnformat), so each of those rejected any call that passed a non-zero slot with kMcResultNoEntry even when the requested port was in range. The fd-based calls (sceMcRead, sceMcWrite, sceMcSeek, sceMcFlush, sceMcClose) take an open descriptor rather than a (port, slot), so the guard never runs there directly -- but a descriptor can only come from sceMcOpen, which is gated, so they are unreachable once open fails; sceMcGetEntSpace is an unconditional stub and never consults the guard. Dragon Quest VIII probes its card with sceMcGetInfo at slot=1 on the "Checking memory card..." screen, so that one call failed and the whole memory-card subsystem became unreachable for the title ("a memory card was not found in slot 1"). This HLE keeps backing state per port (g_mcPorts, one entry per physical PS2 connector) and none per slot: slot never indexes any state, and getMcRootPath() ignores it. Widen the accept predicate to tolerate the whole non-negative slot address space and map any slot onto the port's single virtual card. A negative slot is not a real address and is still rejected; the port bound is unchanged. This imposes no upper slot bound: a slot beyond the four a real multitap could physically expose is accepted rather than rejected, which is harmless because slot indexes no state -- every slot already aliases the port's single card -- so a real title has no reason to reach one, and a title that somehow does is served that same card, no worse than the aliasing described above. This is deliberate tolerance, not advertisement. sceMcGetSlotMax is left at its existing no-multitap value of 1: the title addresses the slot directly and never consults it, and reporting more slots would advertise a multitap the HLE cannot distinctly back -- every slot aliases the one card, so an enumerator would see identical cards and a "copy slot 0 to slot 1" would self-overwrite. Tolerating a slot the guest addresses while advertising only the count actually modeled is the safe direction. The reasonable inference -- from the title's own fail-stop behaviour, not a hardware measurement -- is that retail hardware services this directly addressed slot=1 probe even with no multitap present. Adds regression tests driving the public libmc surface: sceMcGetInfo with a non-zero slot succeeds and reports the port's card; a negative slot, and an out-of-range or negative port, each return kMcResultNoEntry. A further test drives the save flow end to end -- sceMcOpen at slot 1 yields a usable descriptor and sceMcWrite/sceMcSeek/sceMcRead/sceMcClose round-trip a payload through it -- exercising the fd-based path that becomes reachable only once open at a non-zero slot succeeds. ps2x_tests 336/336. --- .../src/lib/Kernel/Stubs/MemoryCard.cpp | 6 +- ps2xTest/src/ps2_runtime_io_tests.cpp | 189 ++++++++++++++++++ 2 files changed, 194 insertions(+), 1 deletion(-) diff --git a/ps2xRuntime/src/lib/Kernel/Stubs/MemoryCard.cpp b/ps2xRuntime/src/lib/Kernel/Stubs/MemoryCard.cpp index 852416203..561e34bbc 100644 --- a/ps2xRuntime/src/lib/Kernel/Stubs/MemoryCard.cpp +++ b/ps2xRuntime/src/lib/Kernel/Stubs/MemoryCard.cpp @@ -96,7 +96,11 @@ namespace ps2_stubs bool isValidMcPortSlot(int32_t port, int32_t slot) { - return port >= 0 && port < static_cast(g_mcPorts.size()) && slot == 0; + // Backing state is per port (g_mcPorts); a slot indexes no state, + // so any non-negative slot maps onto the port's single card. + // A negative slot is not a valid address and is rejected. + return port >= 0 && port < static_cast(g_mcPorts.size()) && + slot >= 0; } std::filesystem::path getMcRootPath(int32_t port) diff --git a/ps2xTest/src/ps2_runtime_io_tests.cpp b/ps2xTest/src/ps2_runtime_io_tests.cpp index e57b8f6f3..504023286 100644 --- a/ps2xTest/src/ps2_runtime_io_tests.cpp +++ b/ps2xTest/src/ps2_runtime_io_tests.cpp @@ -509,6 +509,195 @@ void register_ps2_runtime_io_tests() t.Equals(readGuestS32(test.rdram.data(), formatAddr), 0, "sceMcGetInfo should report an unformatted card after sceMcUnformat"); }); + tc.Run("sceMcGetInfo accepts a non-zero slot on an in-range port", [](TestCase &t) + { + TestContext test; + + constexpr uint32_t typeAddr = GUEST_BUFFER_AREA_START + 0x900; + constexpr uint32_t freeAddr = GUEST_BUFFER_AREA_START + 0x904; + constexpr uint32_t formatAddr = GUEST_BUFFER_AREA_START + 0x908; + + // Establish a known-formatted card on port 0 via slot 0 (accepted + // by any predicate), so this test does not depend on global + // memory-card state left behind by earlier cases. + clearContext(test.ctx); + setRegU32(test.ctx, 4, 0u); + setRegU32(test.ctx, 5, 0u); + ps2_stubs::sceMcFormat(test.rdram.data(), &test.ctx, nullptr); + int32_t cmd = 0; + t.Equals(syncMc(test.rdram, &cmd), 0, "sceMcFormat should format port 0"); + + // A multitap addresses several slots per port; probing slot 1 on + // an in-range port must succeed and report that port's card. + clearContext(test.ctx); + setRegU32(test.ctx, 4, 0u); // port 0 + setRegU32(test.ctx, 5, 1u); // slot 1 + setRegU32(test.ctx, 6, typeAddr); + setRegU32(test.ctx, 7, freeAddr); + setRegU32(test.ctx, 29, GUEST_STACK_AREA_START); + writeStackArg(test.rdram, test.ctx, 0u, formatAddr); + ps2_stubs::sceMcGetInfo(test.rdram.data(), &test.ctx, nullptr); + + t.Equals(syncMc(test.rdram, &cmd), 0, "an in-range port must succeed for a non-zero slot"); + t.Equals(cmd, 0x01, "sceMcSync should report GETINFO as the last command"); + t.Equals(readGuestS32(test.rdram.data(), typeAddr), 2, "a non-zero slot should still report a PS2 memory card"); + t.Equals(readGuestS32(test.rdram.data(), formatAddr), 1, "a non-zero slot should still report the formatted card"); + }); + + tc.Run("sceMcGetInfo rejects a negative slot", [](TestCase &t) + { + TestContext test; + + constexpr uint32_t typeAddr = GUEST_BUFFER_AREA_START + 0x900; + constexpr uint32_t freeAddr = GUEST_BUFFER_AREA_START + 0x904; + constexpr uint32_t formatAddr = GUEST_BUFFER_AREA_START + 0x908; + + // A guest passing slot 0xFFFFFFFF arrives as int32_t -1; the lower + // bound must reject it with "no entry" (-4), even on an in-range + // port. This -4 can come only from the slot guard, whatever card + // state the process-global g_mcPorts happens to hold: for an + // in-range port the only other results sceMcGetInfo produces are + // success (0, formatted) and sceMcResNoFormat (-2, unformatted), + // both distinct from the guard's -4. So this also pins that the + // guard fires before any card-state logic -- a future reorder that + // consulted card state first would yield 0 or -2 and be caught. + clearContext(test.ctx); + setRegU32(test.ctx, 4, 0u); // port 0 + setRegU32(test.ctx, 5, 0xFFFFFFFFu); // slot -1 + setRegU32(test.ctx, 6, typeAddr); + setRegU32(test.ctx, 7, freeAddr); + setRegU32(test.ctx, 29, GUEST_STACK_AREA_START); + writeStackArg(test.rdram, test.ctx, 0u, formatAddr); + ps2_stubs::sceMcGetInfo(test.rdram.data(), &test.ctx, nullptr); + + int32_t cmd = 0; + t.Equals(syncMc(test.rdram, &cmd), -4, "a negative slot must report sceMcResNoEntry"); + t.Equals(readGuestS32(test.rdram.data(), typeAddr), 0, "a negative slot must not report a card type"); + }); + + tc.Run("sceMcGetInfo still rejects an out-of-range or negative port", [](TestCase &t) + { + TestContext test; + + constexpr uint32_t typeAddr = GUEST_BUFFER_AREA_START + 0x900; + constexpr uint32_t freeAddr = GUEST_BUFFER_AREA_START + 0x904; + constexpr uint32_t formatAddr = GUEST_BUFFER_AREA_START + 0x908; + + // Widening the slot check must not weaken the port bound: only + // ports 0 and 1 are emulated (g_mcPorts has two entries). Port 2 + // (above the range) and port -1 (below it) must both fail. + clearContext(test.ctx); + setRegU32(test.ctx, 4, 2u); // out-of-range port + setRegU32(test.ctx, 5, 0u); // slot 0 + setRegU32(test.ctx, 6, typeAddr); + setRegU32(test.ctx, 7, freeAddr); + setRegU32(test.ctx, 29, GUEST_STACK_AREA_START); + writeStackArg(test.rdram, test.ctx, 0u, formatAddr); + ps2_stubs::sceMcGetInfo(test.rdram.data(), &test.ctx, nullptr); + + int32_t cmd = 0; + t.Equals(syncMc(test.rdram, &cmd), -4, "an out-of-range port must still report sceMcResNoEntry"); + t.Equals(readGuestS32(test.rdram.data(), typeAddr), 0, "an out-of-range port must not report a card type"); + + clearContext(test.ctx); + setRegU32(test.ctx, 4, 0xFFFFFFFFu); // port -1 + setRegU32(test.ctx, 5, 0u); // slot 0 + setRegU32(test.ctx, 6, typeAddr); + setRegU32(test.ctx, 7, freeAddr); + setRegU32(test.ctx, 29, GUEST_STACK_AREA_START); + writeStackArg(test.rdram, test.ctx, 0u, formatAddr); + ps2_stubs::sceMcGetInfo(test.rdram.data(), &test.ctx, nullptr); + + t.Equals(syncMc(test.rdram, &cmd), -4, "a negative port must also report sceMcResNoEntry"); + t.Equals(readGuestS32(test.rdram.data(), typeAddr), 0, "a negative port must not report a card type"); + }); + + tc.Run("sceMcOpen reaches the save flow at a non-zero slot", [](TestCase &t) + { + TestContext test; + + const uint32_t dirAddr = GUEST_STRING_AREA_START + 0x400; + const uint32_t fileAddr = GUEST_STRING_AREA_START + 0x500; + const uint32_t writeBufAddr = GUEST_BUFFER_AREA_START + 0x300; + const uint32_t readBufAddr = GUEST_BUFFER_AREA_START + 0x500; + const std::string payload = "slot1 save flow"; + + writeGuestString(test.rdram.data(), dirAddr, "/SAVEDATA"); + writeGuestString(test.rdram.data(), fileAddr, "/SAVEDATA/slot1.bin"); + std::memcpy(test.rdram.data() + writeBufAddr, payload.data(), payload.size()); + + // Setup on port 0 through slot 0 (accepted by any predicate): a + // known-formatted card and the parent directory, so the test does + // not depend on global memory-card state left by earlier cases and + // fails only at the slot-1 open under the old predicate. + clearContext(test.ctx); + setRegU32(test.ctx, 4, 0u); + setRegU32(test.ctx, 5, 0u); + ps2_stubs::sceMcFormat(test.rdram.data(), &test.ctx, nullptr); + int32_t cmd = 0; + t.Equals(syncMc(test.rdram, &cmd), 0, "sceMcFormat should format port 0"); + + clearContext(test.ctx); + setRegU32(test.ctx, 4, 0u); + setRegU32(test.ctx, 5, 0u); + setRegU32(test.ctx, 6, dirAddr); + ps2_stubs::sceMcMkdir(test.rdram.data(), &test.ctx, nullptr); + t.Equals(syncMc(test.rdram, &cmd), 0, "sceMcMkdir should create the save directory"); + + // Headline flow: the title reaches its save file through the + // directly addressed slot 1. sceMcOpen is the only source of a + // memory-card descriptor, so if the slot guard rejects it the whole + // save/load chain is unreachable. Open at slot 1 must yield a + // usable descriptor. + clearContext(test.ctx); + setRegU32(test.ctx, 4, 0u); // port 0 + setRegU32(test.ctx, 5, 1u); // slot 1 + setRegU32(test.ctx, 6, fileAddr); + setRegU32(test.ctx, 7, PS2_FIO_O_RDWR | PS2_FIO_O_CREAT | PS2_FIO_O_TRUNC); + ps2_stubs::sceMcOpen(test.rdram.data(), &test.ctx, nullptr); + + const int32_t fd = syncMc(test.rdram, &cmd); + t.IsTrue(fd > 0, "sceMcOpen at slot 1 must return a usable descriptor"); + t.Equals(cmd, 0x02, "sceMcSync should report OPEN as the last command"); + + // With the descriptor in hand the fd-based ops carry the save + // through: write the payload, rewind, read it back. + clearContext(test.ctx); + setRegU32(test.ctx, 4, static_cast(fd)); + setRegU32(test.ctx, 5, writeBufAddr); + setRegU32(test.ctx, 6, static_cast(payload.size())); + ps2_stubs::sceMcWrite(test.rdram.data(), &test.ctx, nullptr); + t.Equals(syncMc(test.rdram, &cmd), static_cast(payload.size()), + "sceMcWrite should persist the full payload through the slot 1 descriptor"); + + clearContext(test.ctx); + setRegU32(test.ctx, 4, static_cast(fd)); + setRegU32(test.ctx, 5, 0u); + setRegU32(test.ctx, 6, PS2_FIO_SEEK_SET); + ps2_stubs::sceMcSeek(test.rdram.data(), &test.ctx, nullptr); + t.Equals(syncMc(test.rdram, &cmd), 0, "sceMcSeek should rewind the slot 1 descriptor"); + + std::memset(test.rdram.data() + readBufAddr, 0, payload.size()); + clearContext(test.ctx); + setRegU32(test.ctx, 4, static_cast(fd)); + setRegU32(test.ctx, 5, readBufAddr); + setRegU32(test.ctx, 6, static_cast(payload.size())); + ps2_stubs::sceMcRead(test.rdram.data(), &test.ctx, nullptr); + t.Equals(syncMc(test.rdram, &cmd), static_cast(payload.size()), + "sceMcRead should return the full payload through the slot 1 descriptor"); + + std::string readback(reinterpret_cast(test.rdram.data() + readBufAddr), payload.size()); + t.Equals(readback, payload, "the save written at slot 1 should round-trip byte for byte"); + + clearContext(test.ctx); + setRegU32(test.ctx, 4, static_cast(fd)); + ps2_stubs::sceMcClose(test.rdram.data(), &test.ctx, nullptr); + t.Equals(syncMc(test.rdram, &cmd), 0, "sceMcClose should release the slot 1 descriptor"); + + const std::filesystem::path hostPath = test.paths.mcRoot / "SAVEDATA" / "slot1.bin"; + t.IsTrue(std::filesystem::exists(hostPath), "the slot 1 save should land in the port's card under mcRoot"); + }); + tc.Run("sceMcEnd resets libmc state so sync reports no active command", [](TestCase &t) { TestContext test;