From aba2617a4215f6233fdd8b2d7c98baa14c130fce Mon Sep 17 00:00:00 2001 From: drako <98249188+drakolordx7@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:14:26 -0500 Subject: [PATCH] Wait for IOP threads in WaitSema outside a thread context RPC server functions and module start routines run synchronously (IopRpcBridge::handleRpc / loadImage -> callFunction), outside any IOP thread. WaitSema there found the count at 0, had no thread to block, and returned at once without taking the semaphore. The caller then ran its critical section while the IOP thread that holds the semaphore was preempted inside its own, because IOP threads are cut off wherever their slice ends. Found in Killzone's PFILE_R.IRX: the read-request RPC appends to a streaming queue under a semaphore, and the streaming thread removes finished requests under the same one. An append that landed between the streaming thread's `next = cur->next` and `if (next == 0) tail = 0` was dropped, the completion SIF command was never sent, and the EE waited forever (intermittent boot/loading hang). With the thread states traced at each contended wait, the holder was preempted inside the critical section in every hung boot. 5 of 12 headless boots hung before this change, 0 of 12 after. Do what the real IOP does: the waiting code blocks and the holder runs until it signals. waitSemaphoreOutsideThread runs the IOP scheduler (threads, due interrupts, callbacks, timers) until the semaphore count is above 0, then WaitSema takes it as usual. While such a wait is active, SignalSema keeps the count for the waiter and ends the signalling thread's slice, so another thread cannot take the semaphore back first. The wait is skipped from interrupt handlers and guest callbacks and while the scheduler is already running, and it gives up after one second of IOP time with a warning (the previous behaviour). The body of runCycles' loop moves into scheduleStep(), shared with the wait. The new test starts a module whose start routine calls WaitSema on an empty semaphore that a thread signals after writing a marker; it fails without the change. A second case with no signaller checks the timeout. Made by drakolord and assisted with Claude Code. --- ps2xIOP/src/emulator/core/iop_kernel.cpp | 18 +++- ps2xIOP/src/emulator/core/iop_kernel.h | 8 ++ ps2xIOP/src/emulator/iop_emulator.cpp | 96 ++++++++++++++++----- ps2xIOP/tests/iop_emulator_tests.cpp | 105 +++++++++++++++++++++++ 4 files changed, 204 insertions(+), 23 deletions(-) diff --git a/ps2xIOP/src/emulator/core/iop_kernel.cpp b/ps2xIOP/src/emulator/core/iop_kernel.cpp index 4832d8f5b..477c8d118 100644 --- a/ps2xIOP/src/emulator/core/iop_kernel.cpp +++ b/ps2xIOP/src/emulator/core/iop_kernel.cpp @@ -32,6 +32,7 @@ namespace ps2x::iop::detail m_nextSemaphoreId = 1; m_nextEventFlagId = 1; m_currentThread = nullptr; + m_outsideSemaphoreWait = 0; } bool IopKernel::dispatchThreadImport(uint16_t ordinal, IopCpuState &cpu, uint64_t currentCycle) @@ -387,7 +388,16 @@ namespace ps2x::iop::detail } if (it->second.current < it->second.maximum) ++it->second.current; - wakeOneSemaphore(id); + if (m_outsideSemaphoreWait == id) + { + // Code outside any thread is blocked on this semaphore (see setOutsideSemaphoreWait). It stands for + // a thread that outranks the signaller: keep the count for it and end the signaller's slice so the + // signaller cannot take the semaphore back first. + if (m_currentThread != nullptr && &cpu == &m_currentThread->cpu) // not from an interrupt handler + cpu.yielded = true; + } + else + wakeOneSemaphore(id); setV0(0); return true; } @@ -652,6 +662,12 @@ namespace ps2x::iop::detail } } + int IopKernel::semaphoreCount(int id) const + { + const auto semaphore = m_semaphores.find(id); + return semaphore == m_semaphores.end() ? -1 : semaphore->second.current; + } + void IopKernel::sleepCurrent(IopCpuState &cpu) { if (m_currentThread == nullptr) diff --git a/ps2xIOP/src/emulator/core/iop_kernel.h b/ps2xIOP/src/emulator/core/iop_kernel.h index 497710244..2831d823d 100644 --- a/ps2xIOP/src/emulator/core/iop_kernel.h +++ b/ps2xIOP/src/emulator/core/iop_kernel.h @@ -67,6 +67,13 @@ namespace ps2x::iop::detail void terminateThreadsInRange(uint32_t base, uint32_t size); [[nodiscard]] size_t threadCount() const noexcept { return m_threads.size(); } + [[nodiscard]] bool inThread() const noexcept { return m_currentThread != nullptr; } + // Current count of a semaphore, -1 for an unknown id. + [[nodiscard]] int semaphoreCount(int id) const; + // Code running outside any IOP thread (RPC server functions, module start routines) that must wait for a + // semaphore: while set, SignalSema(id) keeps the count for that waiter instead of waking a kernel waiter, + // and ends the signalling thread's slice. 0 = none. + void setOutsideSemaphoreWait(int id) noexcept { m_outsideSemaphoreWait = id; } private: struct Semaphore @@ -99,5 +106,6 @@ namespace ps2x::iop::detail uint32_t m_nextSemaphoreId = 1; uint32_t m_nextEventFlagId = 1; IopThread *m_currentThread = nullptr; + int m_outsideSemaphoreWait = 0; }; } diff --git a/ps2xIOP/src/emulator/iop_emulator.cpp b/ps2xIOP/src/emulator/iop_emulator.cpp index e5f63a9fe..8252b0c57 100644 --- a/ps2xIOP/src/emulator/iop_emulator.cpp +++ b/ps2xIOP/src/emulator/iop_emulator.cpp @@ -274,6 +274,8 @@ namespace ps2x::iop::detail } if (iequals(call.library, "thsemap")) { + if (call.ordinal == 8u && !kernel.inThread()) + waitSemaphoreOutsideThread(static_cast(a0)); return kernel.dispatchSemaphoreImport(call.ordinal, cpu) ? ImportDisposition::Handled : ImportDisposition::Missing; @@ -553,34 +555,48 @@ namespace ps2x::iop::detail servicingGuestCallbacks = false; } + struct SchedulerGuard + { + bool &flag; + const bool previous; + explicit SchedulerGuard(bool &f) : flag(f), previous(f) { flag = true; } + ~SchedulerGuard() { flag = previous; } + }; + + // One scheduler step toward `target`: service due interrupts, callbacks and timers, then run the best ready + // thread for a slice or, with every thread idle, advance the clock to the next event (at most `target`). + void scheduleStep(uint64_t target) + { + servicePendingDmaInterrupts(); + servicePendingGuestCallbacks(); + timrman.serviceDue(totalCycles, *this); + IopThread *next = kernel.beginNextReady(totalCycles); + if (!next) + { + uint64_t nextWake = kernel.nextWakeCycle(target); + for (const auto &[irq, completionCycle] : pendingDmaInterrupts) + nextWake = std::min(nextWake, completionCycle); + if (!pendingGuestCallbacks.empty()) + nextWake = std::min(nextWake, pendingGuestCallbacks.begin()->first); + nextWake = timrman.nextEventCycle(nextWake); + totalCycles = std::max(totalCycles + 1u, std::min(target, nextWake)); + return; + } + const uint64_t before = totalCycles; + runCpu(next->cpu, static_cast(std::min(kDefaultSlice, target - totalCycles))); + kernel.endTimeslice(*next, kThreadReturnSentinel); + if (totalCycles == before) + ++totalCycles; + } + void runCycles(uint64_t cycles) noexcept { try { + const SchedulerGuard guard{inScheduler}; const uint64_t target = totalCycles + cycles; while (totalCycles < target) - { - servicePendingDmaInterrupts(); - servicePendingGuestCallbacks(); - timrman.serviceDue(totalCycles, *this); - IopThread *next = kernel.beginNextReady(totalCycles); - if (!next) - { - uint64_t nextWake = kernel.nextWakeCycle(target); - for (const auto &[irq, completionCycle] : pendingDmaInterrupts) - nextWake = std::min(nextWake, completionCycle); - if (!pendingGuestCallbacks.empty()) - nextWake = std::min(nextWake, pendingGuestCallbacks.begin()->first); - nextWake = timrman.nextEventCycle(nextWake); - totalCycles = std::max(totalCycles + 1u, std::min(target, nextWake)); - continue; - } - const uint64_t before = totalCycles; - runCpu(next->cpu, static_cast(std::min(kDefaultSlice, target - totalCycles))); - kernel.endTimeslice(*next, kThreadReturnSentinel); - if (totalCycles == before) - ++totalCycles; - } + scheduleStep(target); } catch (...) { @@ -588,6 +604,41 @@ namespace ps2x::iop::detail } } + // RPC server functions and module start routines run synchronously (callFunction), outside any IOP thread. + // A WaitSema there that finds the count at 0 cannot block, so the caller used to go on without the semaphore + // while the IOP thread holding it was preempted inside its critical section. On a real IOP the calling + // thread blocks and the holder runs until it signals. Do the same: run the IOP scheduler (threads, due + // interrupts, callbacks and timers) until the semaphore is signalled, then let WaitSema take it. Not from + // an interrupt handler or guest callback, nor while the scheduler is already running; gives up after one + // second of IOP time. + void waitSemaphoreOutsideThread(int id) + { + if (kernel.semaphoreCount(id) != 0) + return; // available, or an unknown id (WaitSema reports it) + if (inScheduler || servicingDmaInterrupts || servicingGuestCallbacks) + return; + const SchedulerGuard guard{inScheduler}; + const uint64_t limit = totalCycles + kIopClockHz; + kernel.setOutsideSemaphoreWait(id); + try + { + while (kernel.semaphoreCount(id) == 0 && totalCycles < limit) + scheduleStep(limit); + } + catch (...) + { + kernel.setOutsideSemaphoreWait(0); + throw; + } + kernel.setOutsideSemaphoreWait(0); + if (kernel.semaphoreCount(id) == 0) + { + std::ostringstream out; + out << "[IOP] WaitSema(" << id << ") outside a thread timed out"; + log(LogLevel::Warning, out.str()); + } + } + ModuleLoadResult loadImage(std::string path, std::span image, const void *arguments, uint32_t argumentSize) { ModuleLoadResult result{true, -1, -1}; @@ -706,6 +757,7 @@ namespace ps2x::iop::detail std::string lastError; bool servicingDmaInterrupts = false; bool servicingGuestCallbacks = false; + bool inScheduler = false; // runCycles or an outside-thread semaphore wait is running IOP threads uint32_t callDepth = 0u; GuestCallback secrMcCommandHandler; GuestCallback secrMcDevIdHandler; diff --git a/ps2xIOP/tests/iop_emulator_tests.cpp b/ps2xIOP/tests/iop_emulator_tests.cpp index 44c249499..1710808dc 100644 --- a/ps2xIOP/tests/iop_emulator_tests.cpp +++ b/ps2xIOP/tests/iop_emulator_tests.cpp @@ -615,6 +615,90 @@ namespace std::memcpy(host.guest.data() + address + codeOffset, segment.data(), segment.size()); } + // The module start routine runs outside any IOP thread. It creates a semaphore (count 0), starts a thread + // that writes a marker and then signals the semaphore, and calls WaitSema. It returns the marker as read right + // after WaitSema: 1 when the wait ran the thread, 0 when it returned without waiting. With startWorker false the + // thread is never started, so nothing signals the semaphore. + void writeSemaphoreWaitIrx(TestHost &host, uint32_t address, bool startWorker = true) + { + constexpr uint32_t codeOffset = 0x100u; + constexpr uint32_t loadAddress = 0x00010000u; + constexpr uint32_t workerAddress = loadAddress + 0x100u; + constexpr uint32_t thbaseTableAddress = loadAddress + 0x180u; + constexpr uint32_t createThreadStub = thbaseTableAddress + 20u; + constexpr uint32_t startThreadStub = createThreadStub + 8u; + constexpr uint32_t thsemapTableAddress = loadAddress + 0x1C0u; + constexpr uint32_t createSemaStub = thsemapTableAddress + 20u; + constexpr uint32_t signalSemaStub = createSemaStub + 8u; + constexpr uint32_t waitSemaStub = signalSemaStub + 8u; + + ElfHeader header{}; + header.ident[0] = 0x7Fu; header.ident[1] = 'E'; header.ident[2] = 'L'; header.ident[3] = 'F'; + header.ident[4] = 1u; header.ident[5] = 1u; header.ident[6] = 1u; + header.type = 2u; header.machine = 8u; header.version = 1u; + header.entry = loadAddress; header.phoff = sizeof(ElfHeader); + header.ehsize = sizeof(ElfHeader); header.phentsize = sizeof(ProgramHeader); header.phnum = 1u; + + ProgramHeader program{}; + program.type = 1u; program.offset = codeOffset; program.vaddr = loadAddress; program.paddr = loadAddress; + program.filesz = 0x500u; program.memsz = 0x500u; program.flags = 7u; program.align = 4u; + + const auto jal = [](uint32_t target) { return 0x0C000000u | ((target >> 2u) & 0x03FFFFFFu); }; + const uint32_t entry[] = { + 0x27BDFFE0u, 0xAFBF001Cu, 0xAFB00018u, // addiu sp, sp, -0x20; sw ra, 0x1c(sp); sw s0, 0x18(sp) + 0x3C040001u, 0x34840300u, jal(createSemaStub), 0x00000000u, // CreateSema(0x10300) + 0x00408021u, // move s0, v0 + 0x3C080001u, 0x35080400u, 0xAD100004u, // sw s0, 4(0x10400): semaphore id for the thread + 0x3C040001u, 0x34840320u, jal(createThreadStub), 0x00000000u, // CreateThread(0x10320) + 0x00402021u, 0x00002821u, startWorker ? jal(startThreadStub) : 0x00000000u, 0x00000000u, // StartThread(id, 0) + 0x02002021u, jal(waitSemaStub), 0x00000000u, // WaitSema(s0) + 0x3C080001u, 0x35080400u, 0x8D090000u, 0x00000000u, // t1 = marker (load delay slot) + 0x01201021u, // move v0, t1 + 0x8FB00018u, 0x8FBF001Cu, 0x27BD0020u, 0x03E00008u, 0x00000000u, + }; + const uint32_t worker[] = { + 0x27BDFFF0u, 0xAFBF000Cu, // addiu sp, sp, -0x10; sw ra, 0xc(sp) + 0x3C080001u, 0x35080400u, // t0 = 0x10400 + 0x24090001u, 0xAD090000u, // marker = 1 + 0x8D040004u, jal(signalSemaStub), 0x00000000u, // SignalSema(semaphore id) + 0x8FBF000Cu, 0x27BD0010u, 0x03E00008u, 0x00000000u, + }; + const uint32_t thbaseImports[] = { + 0x41E00000u, 0u, 0x00000101u, + 0x61626874u, 0x00006573u, // "thbase" + 0x03E00008u, 0x24000004u, // CreateThread + 0x03E00008u, 0x24000006u, // StartThread + 0u, 0u, + }; + const uint32_t thsemapImports[] = { + 0x41E00000u, 0u, 0x00000101u, + 0x65736874u, 0x0070616Du, // "thsemap" + 0x03E00008u, 0x24000004u, // CreateSema + 0x03E00008u, 0x24000006u, // SignalSema + 0x03E00008u, 0x24000008u, // WaitSema + 0u, 0u, + }; + const uint32_t semaphoreDescriptor[] = {0u, 0u, 0u, 1u}; + const uint32_t workerDescriptor[] = {0u, 0u, workerAddress, 0x400u, 20u}; + + std::vector segment(program.filesz, 0u); + const auto put = [&](uint32_t offset, const void *data, size_t size) + { + std::memcpy(segment.data() + offset, data, size); + }; + put(0u, entry, sizeof(entry)); + put(0x100u, worker, sizeof(worker)); + put(0x180u, thbaseImports, sizeof(thbaseImports)); + put(0x1C0u, thsemapImports, sizeof(thsemapImports)); + put(0x300u, semaphoreDescriptor, sizeof(semaphoreDescriptor)); + put(0x320u, workerDescriptor, sizeof(workerDescriptor)); + + std::memset(host.guest.data() + address, 0, codeOffset + program.filesz); + std::memcpy(host.guest.data() + address, &header, sizeof(header)); + std::memcpy(host.guest.data() + address + sizeof(header), &program, sizeof(program)); + std::memcpy(host.guest.data() + address + codeOffset, segment.data(), segment.size()); + } + void writeMcmanRegistrationIrx(TestHost &host, uint32_t address) { constexpr uint32_t codeOffset = 0x100u; @@ -1051,6 +1135,27 @@ int main() if (!expect(lowPriorityMarker == 1u, "WaitVblankEnd returned immediately and starved a lower-priority IOP thread")) return 1; + iop.reset(); + host.logs.clear(); + writeSemaphoreWaitIrx(host, 0x100u); + const ModuleLoadResult semaphoreWait = iop.loadModuleBuffer(0x100u); + if (!expect(semaphoreWait.handled && semaphoreWait.startResult == 1, + "WaitSema outside an IOP thread returned without waiting for the thread that signals it")) return 1; + const auto timedOutWait = [&host]() + { + return std::any_of(host.logs.begin(), host.logs.end(), + [](const std::string &message) { return message.find("timed out") != std::string::npos; }); + }; + if (!expect(!timedOutWait(), "WaitSema outside an IOP thread timed out although a thread signalled")) return 1; + + iop.reset(); + host.logs.clear(); + writeSemaphoreWaitIrx(host, 0x100u, false); + const ModuleLoadResult semaphoreTimeout = iop.loadModuleBuffer(0x100u); + if (!expect(semaphoreTimeout.handled && semaphoreTimeout.startResult == 0, + "WaitSema outside an IOP thread with no signaller did not return")) return 1; + if (!expect(timedOutWait(), "WaitSema outside an IOP thread with no signaller did not report a timeout")) return 1; + iop.reset(); host.logs.clear(); writeMcmanRegistrationIrx(host, 0x100u);