From ad3444ce507290fb949d9656b243658dcbae0b5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Tue, 8 Sep 2026 17:09:44 -0600 Subject: [PATCH] Memory partitions: correct the range, the error codes, and the heap The new sysmem tests run the same partition sweep from both privilege levels, which settles several things that were guesses: The valid range is 1-6, not 1-9-except-7. sceKernelCreateVpl, CreateFpl, CreateMsgPipe and AllocPartitionMemory all let 8 and 9 through to the permission check, so a caller asking for partition 8 got ILLEGAL_PERM where hardware says ILLEGAL_ARGUMENT. Privilege changes the permission check, not the range - 1, 3 and 4 are refused from user mode and work from kernel mode in every one of these APIs, which is the evidence the earlier BlockAllocatorFromID change was missing. sceKernelAllocPartitionMemory reports an out-of-range partition differently depending on which entry point was used - ILLEGAL_ARGUMENT through SysMemUserForUser, ILLEGAL_PARTITION through SysMemForKernel. Both NIDs land on the same function here, and hleIsKernelMode() is precisely "came in through the kernel NID", so it picks the right one. sceKernelCreateHeap had four "TODO: Validate error code" comments and no test at all - it's kernel-only, which is why. All four are now recorded: out-of-range partitions are ILLEGAL_PARTITION, a size of zero or less is HEAPBLOCK_ALLOC_FAILED before anything is allocated, a NULL name is refused with ERROR, and flags really are ignored. sceKernelAllocHeapMemoryWithOption had its validation backwards: the option struct's size field isn't checked at all, while the alignment must be a power of two from 4 to 0x80. Not fixed, and split into sysmem/kernel/heapgrow in tests_next: a real heap will hand out a block larger than the heap itself, so the size isn't a cap. Ours is a fixed allocator over the reserved block. Worth establishing how far the real one grows before implementing that. Risk: the range change makes partitions 8 and 9 fail earlier and with a different code than before. Nothing in tests_good depended on the old behaviour except two expectations that had drifted from hardware, corrected in the submodule. --- Core/HLE/sceKernelHeap.cpp | 43 +++++++++++++++++++++++++---------- Core/HLE/sceKernelMemory.cpp | 20 ++++++++++++---- Core/HLE/sceKernelMsgPipe.cpp | 5 +++- pspautotests | 2 +- test.py | 4 ++++ 5 files changed, 56 insertions(+), 18 deletions(-) diff --git a/Core/HLE/sceKernelHeap.cpp b/Core/HLE/sceKernelHeap.cpp index 7b382124e9..ce39185204 100644 --- a/Core/HLE/sceKernelHeap.cpp +++ b/Core/HLE/sceKernelHeap.cpp @@ -46,18 +46,33 @@ struct KernelHeap : public KernelObject { }; static int sceKernelCreateHeap(int partitionId, int size, int flags, const char *Name) { - u32 allocSize = (size + 3) & ~3; + // Everything below is recorded by pspautotests sysmem/kernel/heap, which is the first test + // this API has ever had - these used to be guesses. + // + // Only partitions 1-6 exist, and anything else is ILLEGAL_PARTITION rather than the + // ILLEGAL_ARGUMENT this used to return. Note the test can't cover partition 5: creating a + // heap in the volatile partition takes a real PSP down hard enough to need a reboot. + if (partitionId < 1 || partitionId > 6) + return hleLogWarning(Log::sceKernel, SCE_KERNEL_ERROR_ILLEGAL_PARTITION, "invalid partition %d", partitionId); BlockAllocator *allocator = BlockAllocatorFromID(partitionId); - // TODO: Validate error code. if (!allocator) - return hleLogError(Log::sceKernel, SCE_KERNEL_ERROR_ILLEGAL_ARGUMENT, "invalid partition"); + return hleLogWarning(Log::sceKernel, SCE_KERNEL_ERROR_ILLEGAL_PARTITION, "invalid partition %d", partitionId); - // TODO: This should probably actually use flags? Name? + // A zero or negative size is refused outright, before anything is allocated. + if (size <= 0) + return hleLogWarning(Log::sceKernel, SCE_KERNEL_ERROR_HEAPBLOCK_ALLOC_FAILED, "invalid size %d", size); + // A name is required, unlike most of the kernel object constructors. + if (!Name) + return hleLogWarning(Log::sceKernel, SCE_KERNEL_ERROR_ERROR, "invalid name"); + + u32 allocSize = (size + 3) & ~3; + + // flags really is ignored - the test sweeps -1, 0, 1, 2, 3, 4, 0x100 and 0x1000 and every + // one of them creates a heap. u32 addr = allocator->Alloc(allocSize, g_fromBottom, StringFromFormat("KernelHeap/%s", Name).c_str()); if (addr == (u32)-1) { - // TODO: Validate error code. - return hleLogError(Log::sceKernel, SCE_KERNEL_ERROR_NO_MEMORY, "fFailed to allocate %d bytes of memory", size); + return hleLogError(Log::sceKernel, SCE_KERNEL_ERROR_NO_MEMORY, "failed to allocate %d bytes of memory", size); } KernelHeap *heap = new KernelHeap(); @@ -65,7 +80,7 @@ static int sceKernelCreateHeap(int partitionId, int size, int flags, const char heap->partitionId = partitionId; heap->flags = flags; - heap->name = Name ? Name : ""; // Not sure if this needs validation. + heap->name = Name; heap->size = allocSize; heap->address = addr; heap->alloc.Init(heap->address + 128, heap->size - 128, true); @@ -152,12 +167,16 @@ static int sceKernelAllocHeapMemoryWithOption(int heapId, u32 memSize, u32 param if (paramsPtr != 0) { if (!Memory::IsValid4AlignedRange(paramsPtr, 8)) return hleLogError(Log::sceKernel, 0, "invalid paramsPtr"); - u32 size = Memory::ReadUnchecked_U32(paramsPtr); // size of the params struct - if (size < 8) - return hleLogError(Log::sceKernel, 0, "invalid param size"); - if (size > 8) - WARN_LOG(Log::HLE, "sceKernelAllocHeapMemoryWithOption(): unexpected param size %d", size); + // The size field is not validated at all - sysmem/kernel/heap sweeps 0, 4, 8, 12 and + // 0x100 through here and every one of them allocates. Only the alignment matters. grain = Memory::ReadUnchecked_U32(paramsPtr + 4); + // And it has to be a power of two from 4 to 0x80. 0 means "no preference", 1 and 2 are + // refused just as firmly as 0x100 and up. + if (grain == 0) { + grain = 4; + } else if (grain < 4 || grain > 0x80 || (grain & (grain - 1)) != 0) { + return hleLogWarning(Log::sceKernel, 0, "invalid alignment %d", grain); + } } // There's 8 bytes at the end of every block, reserved. memSize += 8; diff --git a/Core/HLE/sceKernelMemory.cpp b/Core/HLE/sceKernelMemory.cpp index ac0ec377ff..135fc808bf 100644 --- a/Core/HLE/sceKernelMemory.cpp +++ b/Core/HLE/sceKernelMemory.cpp @@ -525,7 +525,10 @@ static void __KernelSortFplThreads(FPL *fpl) int sceKernelCreateFpl(const char *name, u32 mpid, u32 attr, u32 blockSize, u32 numBlocks, u32 optPtr) { if (!name) return hleLogWarning(Log::sceKernel, SCE_KERNEL_ERROR_NO_MEMORY, "invalid name"); - if (mpid < 1 || mpid > 9 || mpid == 7) + // Only partitions 1-6 exist. sysmem/partitions and its kernel-mode twin record 7 and up + // coming back ILLEGAL_ARGUMENT from both privilege levels; what privilege changes is the + // permission check below, not the range. + if (mpid < 1 || mpid > 6) return hleLogWarning(Log::sceKernel, SCE_KERNEL_ERROR_ILLEGAL_ARGUMENT, "invalid partition %d", mpid); BlockAllocator *allocator = BlockAllocatorFromID(mpid); @@ -882,8 +885,14 @@ int sceKernelAllocPartitionMemory(int partition, const char *name, int type, u32 if ((addr & (addr - 1)) != 0 || addr == 0) return hleLogWarning(Log::sceKernel, SCE_KERNEL_ERROR_ILLEGAL_ALIGNMENT_SIZE, "invalid alignment %x", addr); } - if (partition < 1 || partition > 9 || partition == 7) - return hleLogWarning(Log::sceKernel, SCE_KERNEL_ERROR_ILLEGAL_ARGUMENT, "invalid partition %x", partition); + // SysMemUserForUser and SysMemForKernel both land here, and sysmem/partitions shows they + // report an out-of-range partition differently - ILLEGAL_ARGUMENT from the user entry point, + // ILLEGAL_PARTITION from the kernel one. hleIsKernelMode() is exactly "came in through the + // kernel NID", which is the distinction being made. + if (partition < 1 || partition > 6) { + const u32 error = hleIsKernelMode() ? SCE_KERNEL_ERROR_ILLEGAL_PARTITION : SCE_KERNEL_ERROR_ILLEGAL_ARGUMENT; + return hleLogWarning(Log::sceKernel, error, "invalid partition %x", partition); + } BlockAllocator *allocator = BlockAllocatorFromID(partition); if (allocator == nullptr) @@ -1311,7 +1320,10 @@ static void __KernelSortVplThreads(VPL *vpl) SceUID sceKernelCreateVpl(const char *name, int partition, u32 attr, u32 vplSize, u32 optPtr) { if (!name) return hleLogWarning(Log::sceKernel, SCE_KERNEL_ERROR_ERROR, "invalid name"); - if (partition < 1 || partition > 9 || partition == 7) + // Only partitions 1-6 exist. sysmem/partitions and its kernel-mode twin record 7 and up + // coming back ILLEGAL_ARGUMENT from both privilege levels; what privilege changes is the + // permission check below, not the range. + if (partition < 1 || partition > 6) return hleLogWarning(Log::sceKernel, SCE_KERNEL_ERROR_ILLEGAL_ARGUMENT, "invalid partition %d", partition); BlockAllocator *allocator = BlockAllocatorFromID(partition); diff --git a/Core/HLE/sceKernelMsgPipe.cpp b/Core/HLE/sceKernelMsgPipe.cpp index fa4d335cd3..47110b2752 100644 --- a/Core/HLE/sceKernelMsgPipe.cpp +++ b/Core/HLE/sceKernelMsgPipe.cpp @@ -622,7 +622,10 @@ void __KernelMsgPipeDoState(PointerWrap &p) int sceKernelCreateMsgPipe(const char *name, int partition, u32 attr, u32 size, u32 optionsPtr) { if (!name) return hleLogWarning(Log::sceKernel, SCE_KERNEL_ERROR_NO_MEMORY, "invalid name"); - if (partition < 1 || partition > 9 || partition == 7) + // Only partitions 1-6 exist. sysmem/partitions and its kernel-mode twin record 7 and up + // coming back ILLEGAL_ARGUMENT from both privilege levels; what privilege changes is the + // permission check below, not the range. + if (partition < 1 || partition > 6) return hleLogWarning(Log::sceKernel, SCE_KERNEL_ERROR_ILLEGAL_ARGUMENT, "invalid partition %d", partition); BlockAllocator *allocator = BlockAllocatorFromID(partition); diff --git a/pspautotests b/pspautotests index 7c5d150d40..dcc31bc21c 160000 --- a/pspautotests +++ b/pspautotests @@ -1 +1 @@ -Subproject commit 7c5d150d40ab585d8bc1b5f60f5673eb56620ac9 +Subproject commit dcc31bc21cab554c8cff226898a93c84e7b90ae3 diff --git a/test.py b/test.py index 5caaed33a2..e66c040a90 100755 --- a/test.py +++ b/test.py @@ -249,6 +249,9 @@ tests_good = [ "sysmem/freesize", "sysmem/memblock", "sysmem/sysmem", + "sysmem/partitions", + "sysmem/kernel/partitions", + "sysmem/kernel/heap", "sysmem/volatile", "threads/alarm/alarm", "threads/alarm/cancel/cancel", @@ -458,6 +461,7 @@ tests_next = [ "intr/registersub", "intr/releasesub", "intr/waits", + "sysmem/kernel/heapgrow", "io/file/file", "io/io/io", "io/iodrv/iodrv",