Repository navigation
fix(agc): read unregistered graphics programs beside registered shaders - #5751
adriadam10 merged 1 commit into
Conversation
CnRJay
left a comment
There was a problem hiding this comment.
Letting raw programs coexist with registered ones, and narrowing the RegisterShader refusal to overlapping code, is a sensible rule. Two problems before this can be judged:
- It hasn't run on the backend it changes. The only runtime testing is on an out-of-tree Metal port (NaitikJoshiPro/AnyPS5-mac). "Not run: the Vulkan backend" means
agc_driver,agc_driver_prepared_unregistered_graphicsandagc_driver_prepared_graphicshaven't been run on any configuration this repository ships. Please run them on Linux or Windows (CI on your fork is enough) and say so. A SuperTuxKart result on the Metal port is evidence for the port, not for this tree. - A global mutex in the draw decode.
NoteRawGraphicsDrawwas a relaxed atomic store. It now takesrawGraphicsMutexand does astd::mapinsert fromDecodeGraphicsPrograms, which runs on every draw-decode miss of an unregistered program. With radeonsi-on-oops, the case this PR is for, every GL program is raw, so every queue worker that decodes serializes on that lock. The range only needs recording once per program: doing it where the program is first read raw (ReadRawShader), or behind a flag on the snapshot, keeps the decode path lock-free as it was.RawGraphicsDrawnInalso walks every recorded range belowend. It should useupper_boundand look at the predecessor, as the other range maps in the driver do. It only runs at registration, but it is linear in the number of raw programs.
Fork-only workflow, not part of any pull request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since 39fdc48 a graphics program outside every registered shader was read raw only while the registry was empty, and any RegisterShader after the first raw draw threw. A title that registers some shaders through sceAgcCreateShader and also draws programs it never registers stops at its first such draw with "graphics program does not belong to a compatible registered shader": oops-sdk homebrew registers its display shaders while radeonsi (oops-mesa) submits raw programs. Graphics programs now resolve as compute dispatches already do: an address inside a registered shader resolves to that registration (a registration of another type still throws), any other address is read raw. RegisterShader throws only when the new shader's code overlaps a graphics program that was drawn raw, where the program would get a second identity (the draw cache revalidates raw programs by reading them raw), instead of after any raw draw. The drawn range is recorded once per raw snapshot: a flag on the snapshot keeps later decodes of the same program off the lock. Recorded ranges are merged, so the overlap check looks only at the predecessor of the queried end. Tests: agc_driver_prepared_graphics (a draw whose programs are registered as compute shaders is refused, where it used an address outside every registration before), agc_driver (ProgramSnapshot reads raw beside registered shaders and refuses a registered program of another type; nested, overlapping and adjacent raw ranges), agc_driver_prepared_unregistered_graphics (raw-drawn ranges recorded exactly, unrelated code not reported). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6eb61f3 to
498c00d
Compare
|
Rebased on
The description's testing section is updated. |
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Checked the diff against Built and ran the libSceAgcDriver scope on the merged tree: |
What
Since 39fdc48, an unregistered (raw) graphics program is read only while the shader registry is empty. Once any shader is registered, every draw of a program outside the registered code throws
graphics program does not belong to a compatible registered shader.RegisterShaderalso throws after the first raw draw anywhere.Compute dispatches don't have this restriction: they resolve a registered program by its registration and read any other address raw. Some titles need both kinds of program. Homebrew built on the oops-sdk does: its display path (
src/agc/agc_display.c) registers its shaders throughsceAgcCreateShader, and radeonsi (oops-mesa) submits raw programs for every GL draw. SuperTuxKart (STKT00001, project-oops/oops-apps) stops at its first GL draw with the error above.Fix. Graphics programs now resolve the way compute does:
RegisteredProgram.The concern behind the old
RegisterShadercheck stays covered. A draw keeps a raw snapshot that the draw cache revalidates by reading it raw again, so a registration over the same code would give one program two identities. The code range of every raw program that is drawn is recorded, andRegisterShaderthrows only when the new shader's code overlaps one of those ranges (a shader was registered over a graphics program that was drawn without registration). Registrations elsewhere are accepted. The range is recorded once per raw snapshot: a flag on the snapshot keeps later decodes of the same program off the lock, so the decode path takes no new lock after a program's first draw. Recorded ranges are merged when they overlap or touch, so the check at registration looks only at the predecessor of the queried end.Concrete case.
agc_driver(testProgramSnapshots) now resolves a raw address while another shader is registered. On currentmain,ProgramSnapshotthrows for that address. The test also checks that a registered program requested as another type is still refused, and checks the range bookkeeping with nested, overlapping and adjacent ranges.agc_driver_prepared_graphicsused an address outside every registration as its invalid case; it now registers the draw's programs as compute shaders, which is still refused.agc_driver_prepared_unregistered_graphicsnow checks that the raw-drawn ranges are recorded exactly, starting at the program and not before it, and that unrelated code doesn't count as drawn raw. Before, it checked that any later registration was refused.Tested
main(ab9a476; the commits since then touch only the relinker and CI tools) withANYPS5_REQUIRE_VULKAN=1.agc_driver,agc_driver_prepared_graphics(and its_prebuild_unset,_prebuild_0and_prebuild_1variants) andagc_driver_prepared_unregistered_graphicspass: 6 of 6, no skips. The workflow is fork-only and is not part of this PR.main, SuperTuxKart stops earlier than this draw:sceAgcInit(state, version)(fix(agc): return 0 for out-of-range sceAgcInit without table lookup #5163), plus PM4 and register states radeonsi uses that the driver still refuses. So this change on its own unblocks no title.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)🤖 Generated with Claude Code