diff --git a/Core/HLE/__sceAudio.cpp b/Core/HLE/__sceAudio.cpp index 763703bb15..2be45a25c2 100644 --- a/Core/HLE/__sceAudio.cpp +++ b/Core/HLE/__sceAudio.cpp @@ -42,6 +42,7 @@ #include "Core/HLE/sceAudio.h" #include "Core/HLE/sceKernel.h" #include "Core/HLE/sceKernelThread.h" +#include "Core/HLE/sceKernelInterrupt.h" #include "Core/Util/AudioFormat.h" // Should be used to lock anything related to the outAudioQueue. @@ -87,7 +88,12 @@ void __AudioCPUMHzChange() { audioHostIntervalCycles = (int)(usToCycles(1000000ULL) * 512 / hwSampleRate); } +// The mixer's DMA is still playing out the last block it was handed, which had samples in it. +// A buffer arriving then doesn't restart the DMA or get read early: it waits for the next block. +static bool mixerDMABusy; + void __AudioInit() { + mixerDMABusy = false; System_AudioResetStatCounters(); mixFrequency = 44100; srcFrequency = 0; @@ -113,7 +119,7 @@ void __AudioInit() { } void __AudioDoState(PointerWrap &p) { - auto s = p.Section("sceAudio", 1, 3); + auto s = p.Section("sceAudio", 1, 4); if (!s) return; @@ -180,6 +186,11 @@ void __AudioDoState(PointerWrap &p) { g_audioSRC.DoState(p); __AudioRoutingDoState(p); } + if (s >= 4) { + Do(p, mixerDMABusy); + } else { + mixerDMABusy = false; + } // For older states the routing modes were read back once per channel, above. __AudioCPUMHzChange(); @@ -234,7 +245,7 @@ static void __AudioReScheduleAfterWake() { } // Only channels 0-7. The SRC channel is on its own DMA that the mixer never touches, so the -// two start independently of each other. +// two start independently of each other. See mixerDMABusy for the block after the last one. static bool __AudioAnyChannelPlaying() { for (const AudioChannel &chan : g_audioChans) { if (chan.sampleAddress != 0) { @@ -294,7 +305,7 @@ u32 __AudioEnqueue(AudioChannel &chan, u32 samplePtr, int leftVol, int rightVol) chan.rightVolume = rightVol; } // Handing over a buffer while nothing was playing is what starts the DMA. - const bool startsDMA = samplePtr != 0 && !__AudioAnyChannelPlaying(); + const bool startsDMA = samplePtr != 0 && !__AudioAnyChannelPlaying() && !mixerDMABusy; // A null pointer is accepted and leaves the channel idle, but still counts as a buffer's // worth of remaining samples - which is the one case where the two rest-length calls // disagree with each other. @@ -314,7 +325,7 @@ void __AudioEnqueueOneshot(AudioChannel &chan, u32 samplePtr, u32 sampleCount, u chan.rightVolume = rightVol; chan.remainingSamples = sampleCount; - const bool startsDMA = samplePtr != 0 && !__AudioAnyChannelPlaying(); + const bool startsDMA = samplePtr != 0 && !__AudioAnyChannelPlaying() && !mixerDMABusy; chan.sampleAddress = samplePtr; if (startsDMA) { __AudioStartMixerDMA(); @@ -334,8 +345,13 @@ u32 __AudioEnqueueBlocking(AudioChannel &chan, u32 samplePtr, int leftVol, int r if (chan.waitingThread != 0) { return SCE_ERROR_AUDIO_CHANNEL_BUSY; } - if (!__KernelIsDispatchEnabled()) { - return SCE_KERNEL_ERROR_CAN_NOT_WAIT; + // The event flag wait fails at once with interrupts or dispatch disabled, or inside an + // interrupt. The driver returns that error without clearing the waiting flag it set, which + // leaves the channel busy for good (audio.prx, sceAudioOutputBlocking). That's deliberately not + // emulated: whether the channel was busy at that moment is timing, and a small difference in + // ours would drop a channel for the rest of the game where hardware wouldn't. + if (__IsInInterrupt() || !__KernelIsDispatchEnabled()) { + return __IsInInterrupt() ? SCE_KERNEL_ERROR_ILLEGAL_CONTEXT : SCE_KERNEL_ERROR_CAN_NOT_WAIT; } chan.waitingThread = __KernelGetCurThread(); @@ -414,13 +430,18 @@ u32 __AudioSRCEnqueueBlocking(AudioSRCChannel &chan, u32 samplePtr, int vol) { return 0; } - if (chan.completion) { - chan.completion = false; - return result; + // The event flag wait refuses before looking at the flag, so even a completion that's + // already there isn't taken - the buffer stays armed and the error comes back. + if (__IsInInterrupt()) { + return SCE_KERNEL_ERROR_ILLEGAL_CONTEXT; } if (!__KernelIsDispatchEnabled()) { return SCE_KERNEL_ERROR_CAN_NOT_WAIT; } + if (chan.completion) { + chan.completion = false; + return result; + } chan.waitingThreads.push_back(__KernelGetCurThread()); __KernelWaitCurThread(WAITTYPE_AUDIOCHANNEL, PSP_AUDIO_SRC_WAIT_ID, result, 0, false, "blocking audio"); @@ -608,6 +629,7 @@ void __AudioUpdate(bool resetRecording) { memset(mixBuffer, 0, hwBlockSize * 2 * sizeof(s32)); audioMixing = true; + mixerDMABusy = __AudioAnyChannelPlaying(); bool woke = false; for (AudioChannel &chan : g_audioChans) { // Deliberately not gated on `reserved`: sceAudioChRelease only clears the diff --git a/Core/HLE/sceAudio.cpp b/Core/HLE/sceAudio.cpp index 8c315bc079..0015d2731a 100644 --- a/Core/HLE/sceAudio.cpp +++ b/Core/HLE/sceAudio.cpp @@ -63,6 +63,10 @@ void AudioChannel::DoState(PointerWrap &p) { if (s >= 4) { Do(p, remainingSamples); Do(p, waitingThread); + // Briefly, a failed blocking wait left -1 here to keep the channel busy for good. + if (waitingThread < 0) { + waitingThread = 0; + } Do(p, waitingAddress); Do(p, waitingLeftVolume); Do(p, waitingRightVolume); diff --git a/Core/HLE/sceDisplay.cpp b/Core/HLE/sceDisplay.cpp index d657610979..49a78a14b6 100644 --- a/Core/HLE/sceDisplay.cpp +++ b/Core/HLE/sceDisplay.cpp @@ -135,8 +135,10 @@ std::map vblankPausedWaits; // STATE END -// The vblank period is 731.5 us (0.7315 ms) -const double vblankMs = 0.7315; +// tests/display/vblanklen measures 730-770us from sceDisplayWaitVblankStart returning until +// vblank ends, and an hcount of up to 14 inside it. The wait's own latency is in that, so this is +// the upper end. +const double vblankMs = 0.770; // These are guesses based on tests. const double vsyncStartMs = 0.5925; const double vsyncEndMs = 0.7265; diff --git a/Core/HLE/sceIo.cpp b/Core/HLE/sceIo.cpp index f9058bfcae..86e878da39 100644 --- a/Core/HLE/sceIo.cpp +++ b/Core/HLE/sceIo.cpp @@ -474,6 +474,9 @@ static void __IoAsyncNotify(u64 userdata, int cyclesLate) { if (f->closePending) { __IoFreeFd(fd, error); } + // Like any interrupt that wakes a thread, this dispatches right away, so a better + // priority waiter doesn't sit ready until something else happens to reschedule. + __KernelReSchedule("async io completed"); } } @@ -522,6 +525,7 @@ static void __IoSyncNotify(u64 userdata, int cyclesLate) { HLEKernel::ResumeFromWait(threadID, WAITTYPE_IO, fd, result); f->waitingSyncThreads.erase(std::remove(f->waitingSyncThreads.begin(), f->waitingSyncThreads.end(), threadID), f->waitingSyncThreads.end()); + __KernelReSchedule("io completed"); } static void __IoAsyncBeginCallback(SceUID threadID, SceUID prevCallbackId) { diff --git a/Core/HLE/sceKernelInterrupt.cpp b/Core/HLE/sceKernelInterrupt.cpp index db9ed3852f..90b16b7ab1 100644 --- a/Core/HLE/sceKernelInterrupt.cpp +++ b/Core/HLE/sceKernelInterrupt.cpp @@ -512,13 +512,65 @@ int __ReleaseSubIntrHandler(int intrNumber, int subIntrNumber) { return 0; } +// What a user mode caller finds on each interrupt, as interruptman.prx checks it: whether a driver +// has installed a handler for the interrupt at all, whether it has sub-interrupt slots, and whether +// user handlers are allowed in them. This is system state rather than a rule, read back from +// intr/registersub and intr/releasesub on a 6.61 PSP (running PSPLink, whose USB drivers may count.) +// Interrupt 8 had a handler in an older recording and doesn't now, so treat this as approximate. +enum class IntrUserAccess : u8 { + NO_HANDLER, // SCE_KERNEL_ERROR_NOTFOUND_HANDLER + NO_SUBS, // SCE_KERNEL_ERROR_ILLEGAL_INTRCODE, the sub number is always out of range + KERNEL_SUBS, // SCE_KERNEL_ERROR_ILLEGAL_INTRCODE to register, empty slots to release + USER, +}; + +static IntrUserAccess GetIntrUserAccess(u32 intrNumber) { + switch (intrNumber) { + case PSP_GE_INTR: + case PSP_VBLANK_INTR: + return IntrUserAccess::USER; + case 4: case 6: case 21: + return IntrUserAccess::KERNEL_SUBS; + case 7: case 10: case 12: case 15: case 16: case 17: case 18: case 19: case 20: case 22: + case 23: case 24: case 26: case 31: case 36: case 50: case 56: case 57: case 58: case 59: + case 60: case 61: case 65: + return IntrUserAccess::NO_SUBS; + default: + return IntrUserAccess::NO_HANDLER; + } +} + +// The vblank slots a user handler may take (the rest are the kernel's), and those the display +// driver already holds. +static bool IsUserVblankSubIntr(u32 subIntrNumber) { + return subIntrNumber < 16; +} +static bool IsKernelHeldVblankSubIntr(u32 subIntrNumber) { + return (subIntrNumber >= 18 && subIntrNumber <= 20) || (subIntrNumber >= 24 && subIntrNumber <= 26); +} + u32 sceKernelRegisterSubIntrHandler(u32 intrNumber, u32 subIntrNumber, u32 handler, u32 handlerArg) { if (intrNumber >= PSP_NUMBER_INTERRUPTS) { return hleLogError(Log::sceIntc, SCE_KERNEL_ERROR_ILLEGAL_INTRCODE, "invalid interrupt"); } + switch (GetIntrUserAccess(intrNumber)) { + case IntrUserAccess::NO_HANDLER: + return hleLogError(Log::sceIntc, SCE_KERNEL_ERROR_NOTFOUND_HANDLER, "no handler for this interrupt"); + case IntrUserAccess::NO_SUBS: + case IntrUserAccess::KERNEL_SUBS: + return hleLogError(Log::sceIntc, SCE_KERNEL_ERROR_ILLEGAL_INTRCODE, "no user subinterrupts"); + case IntrUserAccess::USER: + break; + } if (subIntrNumber >= PSP_NUMBER_SUBINTERRUPTS) { return hleLogError(Log::sceIntc, SCE_KERNEL_ERROR_ILLEGAL_INTRCODE, "invalid subinterrupt"); } + if (intrNumber == PSP_VBLANK_INTR) { + if (IsKernelHeldVblankSubIntr(subIntrNumber)) + return hleLogError(Log::sceIntc, SCE_KERNEL_ERROR_FOUND_HANDLER, "held by the kernel"); + if (!IsUserVblankSubIntr(subIntrNumber)) + return hleLogError(Log::sceIntc, SCE_KERNEL_ERROR_ILLEGAL_INTRCODE, "kernel only subinterrupt"); + } u32 error; SubIntrHandler *subIntrHandler = __RegisterSubIntrHandler(intrNumber, subIntrNumber, handler, handlerArg, error); @@ -539,9 +591,17 @@ u32 sceKernelReleaseSubIntrHandler(u32 intrNumber, u32 subIntrNumber) { if (intrNumber >= PSP_NUMBER_INTERRUPTS) { return hleLogError(Log::sceIntc, SCE_KERNEL_ERROR_ILLEGAL_INTRCODE, "invalid interrupt"); } - if (subIntrNumber >= PSP_NUMBER_SUBINTERRUPTS) { + const IntrUserAccess access = GetIntrUserAccess(intrNumber); + if (access == IntrUserAccess::NO_HANDLER) { + return hleLogError(Log::sceIntc, SCE_KERNEL_ERROR_NOTFOUND_HANDLER, "no handler for this interrupt"); + } + if (access == IntrUserAccess::NO_SUBS || subIntrNumber >= PSP_NUMBER_SUBINTERRUPTS) { return hleLogError(Log::sceIntc, SCE_KERNEL_ERROR_ILLEGAL_INTRCODE, "invalid subinterrupt"); } + // User code can't have put anything in the kernel's slots, and can't release what's there. + if (access == IntrUserAccess::KERNEL_SUBS || (intrNumber == PSP_VBLANK_INTR && !IsUserVblankSubIntr(subIntrNumber))) { + return hleLogError(Log::sceIntc, SCE_KERNEL_ERROR_NOTFOUND_HANDLER, "not a user subinterrupt"); + } u32 error = __ReleaseSubIntrHandler(intrNumber, subIntrNumber); return hleLogDebugOrError(Log::sceIntc, error); diff --git a/Core/HLE/sceKernelThread.cpp b/Core/HLE/sceKernelThread.cpp index b4132b0f9a..e5b8091d1c 100644 --- a/Core/HLE/sceKernelThread.cpp +++ b/Core/HLE/sceKernelThread.cpp @@ -1629,8 +1629,17 @@ static PSPThread *__KernelNextThread() { return 0; } +// Set by sceKernelStartThread when the new thread outranks the caller, for the reschedule right +// after it. The firmware hands the CPU straight to the new thread, even when a thread of better +// priority is ready but hasn't been dispatched (say, one that a sceKernelTerminateThread woke.) +// See tests/threads/threads/termsuspended. +static SceUID g_startThreadHandoff = 0; + void __KernelReSchedule(const char *reason) { + const SceUID handoff = g_startThreadHandoff; + g_startThreadHandoff = 0; + // First, let's check if there are any pending callbacks to trigger. // TODO: Could probably take this out of __KernelReSchedule() which is a bit hot. __KernelCheckCallbacks(); @@ -1643,6 +1652,16 @@ void __KernelReSchedule(const char *reason) return; } + if (handoff != 0) { + u32 error; + PSPThread *started = kernelObjects.Get(handoff, error); + if (started && started->nt.status == THREADSTATUS_READY) { + threadReadyQueue.remove(started->nt.currentPriority, handoff); + __KernelSwitchContext(started, reason); + return; + } + } + PSPThread *nextThread = __KernelNextThread(); if (nextThread) { __KernelSwitchContext(nextThread, reason); @@ -1900,7 +1919,13 @@ int __KernelCreateThread(const char *threadName, SceUID moduleID, u32 entry, u32 // Creating a thread resumes dispatch automatically. Probably can't create without it. dispatchEnabled = true; - hleEatCycles(32000); + // Measured by tests/threads/scheduling/costs: about 150us, plus filling the stack with 0xFF at + // around a cycle per byte - 1.3ms for a 256KB stack. + int createCycles = 32000; + if ((attr & PSP_THREAD_ATTR_NO_FILLSTACK) == 0 && stacksize > 0) { + createCycles += stacksize - stacksize / 64; + } + hleEatCycles(createCycles); // This won't schedule to the new thread, but it may to one woken from eating cycles. // Technically, this should not eat all at once, and reschedule in the middle, but that's hard. hleReSchedule("thread created"); @@ -1971,8 +1996,10 @@ int __KernelStartThread(SceUID threadToStartID, int argSize, u32 argBlockPtr, bo if (cur && cur->nt.currentPriority > startThread->nt.currentPriority) { KernelValidateThreadTarget(startThread->context.pc); __KernelChangeReadyState(cur, currentThread, true); - if (__InterruptsEnabled()) + if (__InterruptsEnabled()) { + g_startThreadHandoff = threadToStartID; hleReSchedule("thread started"); + } } // Starting a thread automatically resumes the dispatch thread if the new thread has worse priority. @@ -2219,6 +2246,8 @@ int sceKernelDeleteThread(int threadID) { return hleLogError(Log::sceKernel, SCE_KERNEL_ERROR_NOT_DORMANT); } + // 50-100us whatever the stack size, per tests/threads/scheduling/costs. + hleEatCycles(15000); return hleLogDebug(Log::sceKernel, __KernelDeleteThread(threadID, SCE_KERNEL_ERROR_THREAD_TERMINATED, "thread deleted")); } } diff --git a/Core/HLE/sceKernelTime.cpp b/Core/HLE/sceKernelTime.cpp index b0f8c6e4eb..e6576dd261 100644 --- a/Core/HLE/sceKernelTime.cpp +++ b/Core/HLE/sceKernelTime.cpp @@ -74,7 +74,6 @@ int sceKernelGetSystemTime(u32 sysclockPtr) } VERBOSE_LOG(Log::sceKernel, "sceKernelGetSystemTime(out:%16llx)", t); hleEatCycles(265); - hleReSchedule("system time"); return hleNoLog(0); } @@ -87,7 +86,6 @@ u32 sceKernelGetSystemTimeLow() if (PSP_CoreParameter().compat.flags().KernelGetSystemTimeLowEatMoreCycles) { hleEatCycles(70000); } - hleReSchedule("system time"); return hleNoLog((u32)t); } @@ -96,7 +94,6 @@ u64 sceKernelGetSystemTimeWide() u64 t = CoreTiming::GetGlobalTimeUsScaled(); VERBOSE_LOG(Log::sceKernel,"%i=sceKernelGetSystemTimeWide()",(u32)t); hleEatCycles(250); - hleReSchedule("system time"); return hleNoLog(t); } @@ -153,7 +150,6 @@ u32 sceKernelLibcClock() { u32 retVal = (u32) CoreTiming::GetGlobalTimeUs(); hleEatCycles(330); - hleReSchedule("libc clock"); return hleLogDebug(Log::sceKernel, retVal); } @@ -169,7 +165,6 @@ u32 sceKernelLibcTime(u32 outPtr) else if (outPtr != 0) return 0; - hleReSchedule("libc time"); return hleLogDebug(Log::sceKernel, t); } @@ -185,7 +180,6 @@ u32 sceKernelLibcGettimeofday(u32 timeAddr, u32 tzAddr) DEBUG_LOG(Log::sceKernel,"sceKernelLibcGettimeofday(%08x, %08x)", timeAddr, tzAddr); hleEatCycles(1885); - hleReSchedule("libc timeofday"); return hleNoLog(0); } diff --git a/Core/HLE/scePower.cpp b/Core/HLE/scePower.cpp index cd65db64fd..43ca06633c 100644 --- a/Core/HLE/scePower.cpp +++ b/Core/HLE/scePower.cpp @@ -433,16 +433,22 @@ static int sceKernelVolatileMemLock(int type, u32 paddr, u32 psize) { case SCE_KERNEL_ERROR_CAN_NOT_WAIT: { WARN_LOG(Log::HLE, "sceKernelVolatileMemLock(%i, %08x, %08x): dispatch disabled", type, paddr, psize); - Memory::WriteOrException_U32(0x08400000, paddr); - Memory::WriteOrException_U32(0x00400000, psize); + // Only through pointers that are there: intr/waits passes NULL and gets just the error. + if (Memory::IsValid4AlignedAddress(paddr)) + Memory::WriteUnchecked_U32(0x08400000, paddr); + if (Memory::IsValid4AlignedAddress(psize)) + Memory::WriteUnchecked_U32(0x00400000, psize); } break; case SCE_KERNEL_ERROR_ILLEGAL_CONTEXT: { WARN_LOG(Log::HLE, "sceKernelVolatileMemLock(%i, %08x, %08x): in interrupt", type, paddr, psize); - Memory::WriteOrException_U32(0x08400000, paddr); - Memory::WriteOrException_U32(0x00400000, psize); + // Only through pointers that are there: intr/waits passes NULL and gets just the error. + if (Memory::IsValid4AlignedAddress(paddr)) + Memory::WriteUnchecked_U32(0x08400000, paddr); + if (Memory::IsValid4AlignedAddress(psize)) + Memory::WriteUnchecked_U32(0x00400000, psize); } break; @@ -505,6 +511,10 @@ static u32 scePowerSetCpuClockFrequency(u32 cpufreq) { if (cpufreq == 0 || cpufreq > 333) { return hleLogWarning(Log::sceMisc, SCE_KERNEL_ERROR_INVALID_VALUE, "invalid frequency"); } + // The CPU can't run faster than the PLL it's divided from. + if ((u64)cpufreq * 1000000 > (u64)pllFreq) { + return hleLogWarning(Log::sceMisc, SCE_KERNEL_ERROR_INVALID_VALUE, "above the pll frequency"); + } if (GetLockedCPUSpeedMhz() > 0) { return hleLogDebug(Log::sceMisc, 0, "locked by user config at %i", GetLockedCPUSpeedMhz()); } @@ -551,7 +561,13 @@ static u32 scePowerGetBusClockFrequencyInt() { } static float scePowerGetCpuClockFrequencyFloat() { - float cpuFreq = CoreTiming::GetClockFrequencyHz() / 1000000.0f; + // The CPU runs at a multiple of pll/511, and the firmware works the value out in single + // precision, as pll * n / 511, rather than from whole Hz - which is off in the last digit + // (power/freq). + const double step = (double)pllFreq / 511.0; + const float steps = (float)std::round(CoreTiming::GetClockFrequencyHz() / step); + const float pllMhz = (float)(pllFreq / 1000000.0); + float cpuFreq = (pllMhz * steps) / 511.0f; DEBUG_LOG(Log::sceMisc, "%f=scePowerGetCpuClockFrequencyFloat()", (float)cpuFreq); return cpuFreq; } diff --git a/Core/HLE/sceUmd.cpp b/Core/HLE/sceUmd.cpp index d9fc257606..fec369bbc0 100644 --- a/Core/HLE/sceUmd.cpp +++ b/Core/HLE/sceUmd.cpp @@ -252,7 +252,9 @@ static void __UmdEndCallback(SceUID threadID, SceUID prevCallbackId) else { _dbg_assert_msg_(umdStatTimeoutEvent != -1, "Must have a umd timer"); - CoreTiming::ScheduleEvent(cyclesLeft, umdStatTimeoutEvent, __KernelGetCurThread()); + // A deadline of 0 means the wait has no timeout. + if (waitDeadline != 0) + CoreTiming::ScheduleEvent(cyclesLeft, umdStatTimeoutEvent, __KernelGetCurThread()); umdWaitingThreads.push_back(threadID); @@ -268,10 +270,15 @@ static int sceUmdCheckMedium() { return hleLogDebug(Log::sceKernel, retVal); } +// A user mode caller can't pass a kernel address. (mediaman.prx checks these with k1.) +static bool IsUserAddress(u32 addr) { + return (addr & 0x80000000) == 0; +} + static u32 sceUmdGetDiscInfo(u32 infoAddr) { DEBUG_LOG(Log::sceIo, "sceUmdGetDiscInfo(%08x)", infoAddr); - if (Memory::IsValidAddress(infoAddr)) { + if (Memory::IsValidRange(infoAddr, 8) && IsUserAddress(infoAddr) && IsUserAddress(infoAddr + 4)) { auto info = PSPPointer::Create(infoAddr); if (info->size != 8) return hleLogError(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT); @@ -283,9 +290,13 @@ static u32 sceUmdGetDiscInfo(u32 infoAddr) { } } -static int sceUmdActivate(u32 mode, const char *name) { +static int sceUmdActivate(u32 mode, u32 namePtr) { if (mode < 1 || mode > 2) return hleLogWarning(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT); + // The firmware compares the name before checking the pointer, so a bad one crashes there. + const char *name = namePtr != 0 && Memory::IsValidAddress(namePtr) ? Memory::GetCharPointer(namePtr) : nullptr; + if (!name || strncmp(name, "disc0:", 7) != 0 || !IsUserAddress(namePtr)) + return hleLogWarning(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT, "bad name"); __KernelUmdActivate(); @@ -295,11 +306,14 @@ static int sceUmdActivate(u32 mode, const char *name) { return hleLogDebug(Log::sceIo, 0); } -static int sceUmdDeactivate(u32 mode, const char *name) +static int sceUmdDeactivate(u32 mode, u32 namePtr) { // Why 18? No idea. if (mode > 18) return hleLogError(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT); + // Unlike sceUmdActivate(), the name isn't compared, and only mode 2 requires one. + if ((mode == 2 && namePtr == 0) || !IsUserAddress(namePtr)) + return hleLogError(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT, "bad name"); __KernelUmdDeactivate(); @@ -314,8 +328,7 @@ static u32 sceUmdRegisterUMDCallBack(u32 cbId) { int retVal = 0; - // TODO: If the callback is invalid, return SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT. - if (!kernelObjects.IsValid(cbId)) { + if (!kernelObjects.Is(cbId)) { retVal = SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT; } else { // There's only ever one. @@ -363,13 +376,18 @@ static void __UmdStatTimeout(u64 userdata, int cyclesLate) HLEKernel::RemoveWaitingThread(umdWaitingThreads, threadID); } -static void __UmdWaitStat(u32 timeout) +// The firmware waits on an event flag holding the drive state (mediaman.prx), and passes no +// timeout at all for 0, so that waits forever. +static void __UmdWaitStat(u32 timeout, bool callbacks) { - // This happens to be how the hardware seems to time things. - if (timeout <= 4) - timeout = 15; - else if (timeout <= 215) - timeout = 250; + if (timeout == 0) + return; + + // Measured on hardware. Oddly, the CB version doesn't have the shortest step. + if (timeout <= 1 && !callbacks) + timeout = 25; + else if (timeout <= 209) + timeout = 240; CoreTiming::ScheduleEvent(usToCycles((int) timeout), umdStatTimeoutEvent, __KernelGetCurThread()); } @@ -416,7 +434,7 @@ static int sceUmdWaitDriveStatWithTimer(u32 stat, u32 timeout) { hleEatCycles(520); if ((stat & __KernelUmdGetState()) == 0) { - __UmdWaitStat(timeout); + __UmdWaitStat(timeout, false); umdWaitingThreads.push_back(__KernelGetCurThread()); __KernelWaitCurThread(WAITTYPE_UMD, 1, stat, 0, false, "umd stat waited with timer"); return hleLogDebug(Log::sceIo, 0, "waiting"); @@ -441,11 +459,7 @@ static int sceUmdWaitDriveStatCB(u32 stat, u32 timeout) { hleEatCycles(520); hleCheckCurrentCallbacks(); if ((stat & __KernelUmdGetState()) == 0) { - if (timeout == 0) { - timeout = 8000; - } - - __UmdWaitStat(timeout); + __UmdWaitStat(timeout, true); umdWaitingThreads.push_back(__KernelGetCurThread()); __KernelWaitCurThread(WAITTYPE_UMD, 1, stat, 0, true, "umd stat waited"); return hleLogDebug(Log::sceIo, 0, "waiting"); @@ -519,10 +533,10 @@ static u32 sceUmdReplacePermit() { const HLEFunction sceUmdUser[] = { - {0XC6183D47, &WrapI_UC, "sceUmdActivate", 'i', "is"}, + {0XC6183D47, &WrapI_UU, "sceUmdActivate", 'i', "is"}, {0X6B4A146C, &WrapU_V, "sceUmdGetDriveStat", 'x', "" }, {0X46EBB729, &WrapI_V, "sceUmdCheckMedium", 'i', "" }, - {0XE83742BA, &WrapI_UC, "sceUmdDeactivate", 'i', "xs"}, + {0XE83742BA, &WrapI_UU, "sceUmdDeactivate", 'i', "xs"}, {0X8EF08FCE, &WrapI_U, "sceUmdWaitDriveStat", 'i', "x" }, {0X56202973, &WrapI_UU, "sceUmdWaitDriveStatWithTimer", 'i', "xx"}, {0X4A9E5E29, &WrapI_UU, "sceUmdWaitDriveStatCB", 'i', "xx"}, diff --git a/docs/pspautotests-hardware.md b/docs/pspautotests-hardware.md index a9369566c5..e961fad821 100644 --- a/docs/pspautotests-hardware.md +++ b/docs/pspautotests-hardware.md @@ -141,15 +141,23 @@ python3 test.py --graphics=software io/shortname/shortname builds with pspdev GCC 15 as of the "Make the tests build with a current pspdev toolchain" commit; if you hit a broken one anyway, `gentest.py -k` (`--keep`) skips `make` entirely and you can build your own target by hand with `make yourtest.prx`. -- **Rebuilding a `.prx` is not free, so don't regenerate one you didn't change.** The binaries are - committed and were built with a much older SDK. Rebuilding with the current toolchain grows them - by roughly a third (`testgp.prx`: 117 KB to 191 KB), and it can change what a test *does*: - `time_t` is 64-bit now, so `rtc/convert`'s `sceRtcSetTime_t(&pt, 62135596800ULL)` marshals - differently than the committed binary and stops matching its own `.expected`. Check - `git status` and revert any `.prx` you didn't mean to touch. -- **A `.prx` you did rebuild deserves a hardware run before you commit it.** Build it, run it, and - diff the output against the committed `.expected` - if it differs, decide whether the test - genuinely changed or whether the toolchain did. Note the committed `.expected` files have CRLF +- **When you touch one test, rebuild every `.prx` in its directory and re-record them all.** Many + committed binaries were built with a much older SDK, and a rebuild with the current toolchain can + change what a test *does*: `time_t` is 64-bit now, so `rtc/convert`'s + `sceRtcSetTime_t(&pt, 62135596800ULL)` marshals differently, and in `tests/intr` the SDK's + `ModuleMgrForUser` stubs, linked next to the directory's own import of that library, silently did + nothing - `waits` printed `sceKernelGetModuleId`'s result for `sceKernelStartModule`. Rebuilding + a whole directory at once turns such surprises up while you're there to explain them, rather + than one at a time whenever someone happens to touch a neighbour. So run a plain `make` in the + directory, record each test on hardware, and commit the binaries and `.expected` files together. + (The binaries also grow by roughly a third: `testgp.prx` went from 117 KB to 191 KB.) +- **Diff every re-recorded `.expected` against the old one before committing.** A difference means + the test changed on purpose, the toolchain changed what it does (fix the test), or the firmware + did. Many old recordings are likely from a PSP on an earlier firmware than today's 6.61, and a + driver change or a bug fix between versions shows up regardless of `sceKernelSetCompiledSdkVersion` + (`intr/registersub` depends on which drivers have hooked which interrupts: interrupt 8 had a + handler in the old recording and has none on 6.61). Say which in the commit message, and keep + the 6.61 result, since that's the firmware PPSSPP models. Note the committed `.expected` files have CRLF line endings (they were recorded on Windows) while a fresh run writes LF, so compare with `diff <(tr -d '\r' < __testoutput.txt) <(tr -d '\r' < the.expected)`. - **Each test PRX stays resident after it runs.** Run a handful back to back and the next diff --git a/docs/sceAudio.md b/docs/sceAudio.md index a99c7598c1..dd1c2308f5 100644 --- a/docs/sceAudio.md +++ b/docs/sceAudio.md @@ -59,11 +59,27 @@ r = enqueue(...); if (r != BUSY) return r; if (channel->waiting) return BUSY; // somebody else is already parked here channel->waiting = 1; -wait for this channel's bit in the driver's event flag; +r = wait for this channel's bit in the driver's event flag; +if (r < 0) return r; // waiting is left set! (not emulated) retry the enqueue; channel->waiting = 0; ``` +The error path is a real firmware bug. The event flag wait fails at once with interrupts or +dispatch disabled (`800201a7`) or inside an interrupt (`80020064`), and the driver returns that +without clearing `waiting`. Nothing ever clears it after that, so the channel stays busy for +good, and `sceAudioChRelease` refuses to release it. + +**The emulator returns the same errors but doesn't leave the flag set.** Whether the channel was +busy at that moment is a question of timing, and ours is close but not exact: emulating the bug +means a small scheduling difference could drop a channel for the rest of the game where hardware +wouldn't. No game can depend on losing a channel, so the risk only runs one way. +`tests/intr/waits` leaves the case out for this reason. + +The SRC channel's wait fails the same way, and since the flag is never looked at, even a +completion that was already there isn't taken: the buffer stays armed and the error comes back. +That one is emulated, since nothing is lost - the buffer plays out as usual. + **Only one thread can be parked on a channel.** That single flag is why a game that runs a movie thread and a sound-effect thread over one output gets sensible behavior on hardware and did not in the emulator: the loser is told the channel is busy and skips its turn, where an emulator that @@ -121,6 +137,11 @@ reset but no early read, since its DMA feeds the codec directly. Without this th `sceAudioGetChannelRestLen` right after an output would depend on where the timer happened to be, and would differ from run to run. +That only happens when the DMA is actually idle. After a block that had samples in it, the DMA +is still playing that block out, so a buffer handed over within it is not read early: it waits +for the next block like any other. (Also from `tests/intr/waits`, where a second 64-sample buffer +is still in the channel when the next call looks.) + Two things are still approximate, both below one mix block: - A descriptor retires when the DMA transfer finishes, which is slightly ahead of the audio diff --git a/pspautotests b/pspautotests index 33e59e29f2..0623f988bb 160000 --- a/pspautotests +++ b/pspautotests @@ -1 +1 @@ -Subproject commit 33e59e29f2aad3c7440e5b1afeab1715268d72dc +Subproject commit 0623f988bbd5f46d7a1311424faef0763c687eda diff --git a/test.py b/test.py index 237ec26b82..985ceacfb2 100755 --- a/test.py +++ b/test.py @@ -180,6 +180,7 @@ tests_good = [ "ctrl/sampling2/sampling2", "ctrl/vblank", "display/display", + "display/hcount", "display/vblankmulti", "display/isstate", "display/setframebuf", @@ -266,6 +267,8 @@ tests_good = [ "intr/intr", "intr/mfic", "intr/enablesub", + "intr/registersub", + "intr/releasesub", "intr/suspended", "intr/vblank/vblank", "io/cwd/cwd", @@ -291,6 +294,7 @@ tests_good = [ "mstick/mstick", "power/cpu", "power/power", + "power/freq", "power/volatile/lock", "power/volatile/trylock", "power/volatile/unlock", @@ -403,6 +407,8 @@ tests_good = [ "threads/threads/stackfree", "threads/threads/start", "threads/threads/suspend", + "threads/threads/terminate", + "threads/threads/termsuspended", "threads/threads/threadend", "threads/threads/threadmanidlist", "threads/threads/threadmanidtype", @@ -450,6 +456,7 @@ tests_good = [ "utility/savedata/getsize", "utility/savedata/makedata", "utility/systemparam/systemparam", + "umd/api/api", "umd/callbacks/umd", "umd/wait/wait", "umd/register", @@ -527,7 +534,6 @@ tests_next = [ "audio/output2/frequency", "audio/output2/rest", "ccc/convertstring", - "display/hcount", "font/fonttest", "font/charinfo", "font/newlib", @@ -569,8 +575,6 @@ tests_next = [ "gpu/texmtx/uvs", "gpu/textures/size", "gpu/triangle/triangle", - "intr/registersub", - "intr/releasesub", "intr/waits", "sysmem/kernel/heapgrow", "io/file/file", @@ -590,7 +594,6 @@ tests_next = [ #"modules/loadexec/loader", "net/http/http", "net/primary/ether", - "power/freq", "sysmem/partition", # These two mbx tests only appeared to work because they papered over bugs @@ -598,7 +601,6 @@ tests_next = [ "threads/scheduling/dispatch", "threads/scheduling/scheduling", "threads/threads/create", - "threads/threads/terminate", "threads/tls/memory", "threads/vpl/create", "umd/io/umd_io",