Repository navigation
fix(agc): keep fused geometry programs out of the caller's memory - #5573
Conversation
sceAgcFuseShaderHalves copied both halves' code, the fused header and its tables into the caller's memory and registered the program at that address. PPSA02100 fuses different halves into the same memory a few calls apart, while draws that use the earlier result are still queued. The registry keeps one program per code address, so those draws were compiled from the later program: its code read a user-data word the bound layout does not have, and the device reported "BDA access failed" for the mesh stage. The PS5 5.02 module writes only the back half's register list into that memory (sceAgcGetFusedShaderSize reports eight bytes per register, alignment 4) and leaves the code where the halves are. The fused code, header and tables now live in memory libSceAgc owns, one program per distinct pair of halves, registered once. The caller's memory receives the fused register list and the size query reports that list. A second fusion of the same halves returns the same program; halves whose contents changed get a new one. The driver looked a header up by comparing its fields with the registered header, including the register list's address. A header whose register list is a copy elsewhere with the same contents now matches too, which is what the title passes after a fusion.
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
The overwrite diagnosis is compelling, and the scratch-reuse regression covers the original failure mode. One lifecycle question: FusedPrograms retains a full key copy and fused storage for every distinct half pair until process shutdown. Do the title runs show that the number/total size of distinct fusions stays bounded during a longer session? If not, this changes reusable caller scratch into process-lifetime storage and could accumulate across dynamically generated shader pairs.
|
Linux validation on PR head |
|
I measured it. A local build of this head with a counter (one line per new entry and the running total of key plus storage bytes) on both titles:
Both fuse when a shader pair is first set up, a handful of times in a session. In Stray the second call was a hit on the first entry, and the last new entry appeared 70% of the way through the log, so the map follows the pairs the title has loaded. What stays unbounded is a title that builds new halves at run time: each distinct pair would keep about 6 KB until exit. I have not seen one. I left eviction out because a draw queued earlier may still name an entry's code address, which is the failure this PR removes. Kena's run ended on |
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Thanks for measuring the lifetime retention. The two title runs each retain about 12.6 KB across two distinct fused programs, with later calls mostly reusing existing pairs. That addresses my lifecycle question for these observed workloads. The dynamic-pair case remains process-lifetime storage, and the PR explains why eviction could invalidate code still referenced by queued draws. I previously ran the focused fusion and prepared-shader suites on this exact head; I see no blocker from the retention data.
|
Windows test of #5573: Windows 10 (19045), MinGW-w64 GCC 15.2, Release, AI-assisted (Claude Code). |
What
Refs #5099. Stray (PPSA02100) and Kena (PPSA01746) stop on current
mainwithThe draw is a fused geometry shader.
sceAgcFuseShaderHalvescopied both halves' code, the fused header and its tables into the caller's memory and registered the program at that address. Stray fuses different halves into the same memory a few calls apart while draws that use the earlier result are still queued. The registry keeps one program per code address, so those draws were compiled from the later program, whose code reads a user-data word the bound layout does not have. WithAPS5_TRACE_SHADER_REGS=allone run shows two registrations at the same code address twelve log lines apart, with RSRC20x008b0018and then0x008b0010, and the failing draw carries the first value.The export in the PS5 5.02 module writes only the register list into that memory: it copies the back half's header, memcpys
num_sh_registers * 8bytes, patches the checksum, RSRC1, RSRC2 and program address entries from the front half, and its size query returnsnum_sh_registers * 8with alignment 4. No code goes there.This PR keeps the fused code, header and tables in memory libSceAgc owns, one program per distinct pair of halves, registered once. The caller's memory receives the fused register list and the size query reports that list. Fusing the same halves again returns the same program; halves whose contents changed get a new one.
The driver matched a header against the registered one field by field, including the register list's address. A header whose register list is an identical copy elsewhere now matches as well, which is what a title passes after a fusion. Without that every run stops on
rectangle ABI refers to a replaced shader header.This changes where #124 put the fused program; its code layout and register merge are untouched. PR 4826 edits another function in the same file. #5384 works around the same stop by recording the unreadable root as poison; with this change the read does not happen, so the two do not depend on each other.
Limits. The fused list still appends the front half's own SH registers as #124 does, so it can be longer than the real one; a title that sizes the memory without the size query would be overrun by those entries (before this PR it was overrun by the whole program). Geometry halves without memory still throw, where the real function patches the back half's list in place. A fused result whose register list the title has overwritten no longer matches its header and throws
refers to a replaced shader header; neither title does that.Tested
Arch Linux, GCC 16.2.1,
maind167000 with this PR.agc_shader_fusionfails onmainwithfused size is not the fused register list;agc_driver_prepared_shadersand its_indirect,_uncachedand_debugvariants fail without theShaderRegistry.cppchange withgraphics ABI refers to a replaced shader header.agc_shader_fusionagc_driver_prepared_shadersagc_driver_prepared_shaders_indirectagc_driver_prepared_shaders_uncachedagc_driver_prepared_shaders_debugagc_driver_prepared_shaders_debug_failureagc_driver_prepared_shaders_deferredagc_driver_prepared_shaders_deferred_undecodableagc_driver_prepared_shaders_registered_workgroupagc_driver_prepared_shaders_registered_workgroup_syncWindows Server 2025, WinLibs GCC 15.2 (no GPU): build and ctest, 1103 tests, 0 failed.
Stray, runs of 120 s on the RTX 5080, alternating builds of
main30f9612 and that commit with this PR:mainOn
maind167000 with this PR (the commit before a last change that leaves struct padding out of the cache key): Stray 4 of 4 reach 120 s, Kena 1 of 2; the other Kena run stops at 120 s onbuffer descriptor uses an unsupported type, a stop Kena also has without this change. No run hasBDA access failedorguest memory is not readable. On the older base Kena reached 120 s in 2 of 2 runs. I checked that the titles keep presenting; I did not compare pictures.On Intel Arc (Meteor Lake, Mesa 26.2.3 ANV) ThatsLeo ran this head on Stray: the
BDA access failed ... 0xdfac00000004lines and the stop of presents after the calibration screen are gone, one run (#5099 (comment)).Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)