Repository navigation
fix(agc): import GPU-mapped guest memory when it is mapped - #1961
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
The new observer misses an important map path. DoMapDirect/DoMapAnon first call RemapFixedIntoRegistered for a fixed mapping over an existing reservation; that path calls Mutation::Protect(..., gpu, ...) rather than Mutation::Add. GuestAllocationsProtect_nid_postfix updates the range but never appends it to MutationState::gpuMapped, so the observer does not import a reserved range when it is mapped with GPU access. The new test only covers Add, so it does not exercise this path. Please notify the observer for GPU-enabled fixed remaps (while preserving the documented deferred behavior for plain sceKernelMprotect, if intended) and add a reserve-then-fixed-map regression test. I will re-review the updated head.
9a402fb to
a050a7c
Compare
|
Thanks, A fixed map over a reservation goes through RemapFixedIntoRegistered, which uses Protect instead of Add, so the observer never saw it. Fixed remaps with GPU access now get reported too (Mutation::NoteGpuMapping). Plain sceKernelMprotect still isn't reported, so that stays deferred to first use like the description says. I added the regression test you suggested for in guest_memory, it reserves two pages and maps them fixed and checks that only the GPU-mapped one is reported |
MarcZX10
left a comment
There was a problem hiding this comment.
Re-reviewed the updated head and the follow-up against the earlier request. GPU-enabled fixed remaps over reservations now call NoteGpuMapping after the registry mutation, while ordinary sceKernelMprotect remains deferred. The regression test covers reserve → fixed CPU map → fixed GPU map, verifies only the GPU range is observed, and also checks the observer can lease the registry after the lock is released. Linux and Windows CI and Conventions pass. This change depends on #1905 as documented; no remaining blocker found.
|
Native Windows run, since the description only covers Windows under Wine. Setup: Windows 10 (19045), RTX 3060 Laptop (NVIDIA 616.92), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release. Head
No title was run. 🤖 Addressed by Claude Code |
|
I tried this with Ghost of Yōtei (PPSA26344) on Windows with an RTX 3080 Laptop GPU. It was a backport onto our older pre-#639 test tree, not current main, so treat the numbers as indicative. The change does what it says: the largest early-world address rebuild went from 19.9 s to 25 ms. But the run as a whole got a lot slower. The world showed up at 549 s instead of 278 s, and the first 23 world frames averaged 15.9 s against 7.2 s on the same build with the change switched off. The cost is per mapping. Yōtei makes about 43.7k GPU mappings of 128 KiB each, and every Importing the range the hook is handed directly, or reconciling once per batch of mappings rather than on every call, would probably keep the gain without the cost. I'm happy to rerun a revised version. Numbers (same build, switch on/off)
On its own, #1905 is neutral for speed here: only 182 of about 40.5k ranges lack GPU access. It does lower the import peak by about 711 MiB, which is nice since we sit right at the host-import ceiling. No new skips or errors in any of the runs. |
|
Celegans12's run above shows the cost of importing at map: each GPU mapping takes a fresh registry lease and runs |
a050a7c to
7b62142
Compare
|
Thanks for the numbers @Celegans12, the cause was what you described. Reworked, and rebased on main together with #1905 (@oneandonlydean):
What's left is mostly the Vulkan import itself. The draw-time reconcile after a registry change is main's and still walks every import. If you can rerun Yotei with this I'd like to see how it compares. |
7b62142 to
de9239f
Compare
de9239f to
78466e9
Compare
78466e9 to
90094af
Compare
90094af to
1156317
Compare
|
007 First Light (PPSA11386 v01.018.000), start-up only, on the reworked head 1156317. Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ryzen 9 7950X3D, RTX 3080 with NVIDIA 615.71.09), Release with Setup:
AI-assisted: yes (Claude Code) |
1156317 to
81fac84
Compare
|
Independent validation on real hardware Built and ran the PR head
Results:
One failure I tracked down (not a regression from this PR): the full Honest limits of what I verified:
Written with AI assistance. |
The registry calls an observer on the mapping thread, after the registry lock is released, with the ranges a kernel mapping gave GPU access: direct and flexible maps with it, and fixed remaps with it over a registered range (a reservation). The mutation looks them up while it still holds the lock and hands them over weakly, with the registry generation it ended at, so the observer does not keep a range alive. The driver imports those ranges on its current device, so an address-based build finds the import instead of pinning the memory on the queue that first uses it. A range that gains GPU access through sceKernelMprotect, and AnyPS5's own libc heap, are still imported on first use. The import at map does not reconcile the other imports with the registry, so its cost does not grow with the number of imports. It skips a range that is gone, and any range when the imports were reconciled with a newer registry since; the next lookup imports those as before. It installs the pin waiter as the other import paths do, so unmapping a range imported at map retires its import. The live import bytes are kept as a running total instead of being summed over every import. Once the import table belongs to the current device the import takes only the table's lock; the first import on a device (which runs the write-watch probe) and a device that was just replaced go through the GPU lock. Clearing the observer waits for calls already running. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Rebased onto current |
81fac84 to
be1f501
Compare
What
An address-based build imports every registered allocation with GPU access that it leases, whole, the first time it builds with it. Pinning a large allocation takes about 0.3 s per GiB here (the kernel faults in and zeroes each page), and it runs on the queue that first uses the allocation, under the device lock.
GuestAllocationsSetGpuMapObserver_nid_postfix) on the mapping thread, after the registry lock is released, with the ranges a kernel mapping gave GPU access: direct and flexible maps with it (Mutation::NoteGpuMapping), and fixed maps with it over a registered range such as a reservation. The mutation looks the ranges up while it still holds the lock and hands them over as weak pointers, with the registry generation it ended at, so the observer never keeps a range alive (a concurrent unmap does not wait for it). AnyPS5's own libc heap is registered with GPU access but is not reported.ImportMappedRanges), the same whole-range import an address-based build makes, without reconciling the other imports with the registry, so the work per mapping does not depend on how many imports there are. It skips a range that is already gone, and every range when the imports were reconciled with a newer registry since; the next lookup imports those as before. It installs the pin waiter as the other import paths do, so unmapping a range imported at map retires its import (fix(agc): retire host imports before guest unmap #2575); without it, unmapping a range imported at map before any lookup waited forAPS5_PIN_WAIT_MS(60 s) and failed. The live import bytes are a running total instead of a sum over every import. With no device yet, nothing happens and the first use imports as before. A failed import is logged per range and handled as a failed import is today.sceKernelMprotectis still imported on first use.Sources:
src/core/memory.cppcallsrasterizer->MapMemorywhen a mapping with GPU access is made, and KytyPS5src/kernel/memory.cppMapGpuRangecallsRenderContext::MapMemoryat map. Both only record the range, because their buffer caches upload on use. AnyPS5's address-based builds bind host memory in place, so the matching work at map is the import.PPSA36082
While loading it maps 512 MiB, 6 GiB and 128 MiB of direct memory with
0x32. Its 512 MiB upload pool is freed only after a GPU fence. Without this change, its first address-based build pins those allocations on the queue the loader's fence waits on, and the pool overflows at about 2.0 s, before any import finishes (0/3). With it, the imports run on the mapping thread (done by about 0.9 s, 2.8 s and 2.8 s) and nothing is imported on the fence path. Together with #1959 (formatted buffer accesses) and #1960 (imports leave pages unprotected), it reaches the main menu in 10/10 runs (40 s each, pool peak 340-391 MiB of 512); without this change, 0/3 overflow at about 2.0 s. Three of the ten later stopped at "prepared shader artifact is missing for the requested static ABI", a separate issue after the menu.PPSA36082 also needs an import budget of at least 6784 MiB, which this change does not need: with main's 6 GiB, the 6 GiB allocation is refused at map (and logged) because 512 MiB is already imported. #290 sets the budget to max(6 GiB, RAM/2); these runs used
APS5_HOST_IMPORT_MIB=8192in its place. With #290's import windows, a request that covers a whole registered range, as this one does, still imports the whole range, which is what an address-based build imports. The runs were on main f145139 with #861, #1367, #1249, #1905, #1959 and #1960.Tested
Linux (CachyOS), x86-64, GCC, 7800X3D / RTX 4090.
guest_memory: a libc heap allocation is not reported; a direct map with3is not reported; one with0x32is reported once, with its address and size, and the observer can take a registry lease; an mprotect to0x13is not reported. Over a reservation, a fixed map with3is not reported and one with0x32is reported once with its address and size. Clearing the observer while a call runs returns only after it finished. Never reporting, reporting maps without GPU access, reporting the heap, not reporting direct maps, reporting with the registry still locked (it deadlocks; the test times out), not reporting fixed GPU maps, reporting every fixed map, and clearing without waiting each fail it.agc_driver_host_import_at_map(new): a mapping imported before any lookup is retired when it is unmapped; a mapped allocation is imported on that device, also after another mapping changed and when a range is mapped again; a mapping unregistered before its import, and an import older than the last reconcile, are not imported; with the device lock held by another thread, a mapping on a device that already imports is imported. Not installing the pin waiter, dropping the generation check, and always taking the device lock each fail it.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)