From 498c00deb31c3f9a08920ba52e95ae06fb954139 Mon Sep 17 00:00:00 2001 From: NaitikJoshiPro <190247830+NaitikJoshiPro@users.noreply.github.com> Date: Wed, 14 Oct 2026 14:43:43 +0530 Subject: [PATCH] fix(agc): read unregistered graphics programs beside registered shaders Since 39fdc48b5 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 --- .../include/Driver/Shaders/ShaderRegistry.hpp | 4 ++- .../Execution/src/Driver/Draw/DrawDecode.cpp | 2 +- .../src/Driver/Shaders/ShaderRegistry.cpp | 36 +++++++++++++++++-- .../libs/prx/libSceAgcDriver/tests/Submit.cpp | 29 +++++++++++++-- .../tests/execution/PreparedGraphics.cpp | 8 ++++- .../PreparedUnregisteredGraphics.cpp | 12 +++---- 6 files changed, 76 insertions(+), 15 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp b/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp index 9aa51c4f40e5..60a9fee23755 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp +++ b/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp @@ -87,6 +87,7 @@ struct ShaderSnapshot { std::shared_ptr registeredState; std::shared_ptr> workgroupUse = std::make_shared>(0u); std::shared_ptr> positionVectorUse = std::make_shared>(0u); + std::shared_ptr> rawGraphicsNoted = std::make_shared>(false); std::optional userDataTailRegister; }; @@ -94,8 +95,9 @@ struct ShaderSnapshot { std::shared_ptr ReadRawShader(std::uint64_t address); std::shared_ptr ReadUserDataTailContinuation(std::span code, std::uint32_t userDataBase, std::span userData); std::shared_ptr ProgramSnapshot(const ShaderRegistry& shaders, std::uint64_t address, std::initializer_list types); -void NoteRawGraphicsDraw(); +void NoteRawGraphicsDraw(const ShaderSnapshot& snapshot); bool RawGraphicsDrawn(); +bool RawGraphicsDrawnIn(std::uint64_t begin, std::uint64_t end); bool ProgramUsesWorkgroup(const ShaderSnapshot& snapshot, std::size_t codeOffset); // The PA_CL_VS_OUT_CNTL position export bits a vertex program is compiled with: none for a program // without pos1..pos3 exports, so its artifact does not depend on them. diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawDecode.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawDecode.cpp index f4a26ead03de..34177240c4a2 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawDecode.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawDecode.cpp @@ -65,7 +65,7 @@ void DecodeGraphicsPrograms(DrawDecode& decoded, const QueueState& queue, const const auto& snapshot = *program; require((address - snapshot.codeAddress) % sizeof(std::uint32_t) == 0, "graphics entry point is not dword aligned"); require(snapshot.header.empty() || snapshot.type == type, "graphics program refers to an incompatible shader binary type"); - if (!nullPixel && snapshot.header.empty()) NoteRawGraphicsDraw(); + if (!nullPixel && snapshot.header.empty()) NoteRawGraphicsDraw(snapshot); const auto codeOffset = static_cast((address - snapshot.codeAddress) / sizeof(std::uint32_t)); DrawProgram result{ {stage, address, std::span(snapshot.code).subspan(codeOffset), snapshot.headerAddress, snapshot.header}, diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp index 30a4685d053b..2ba97ad85a71 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp @@ -24,7 +24,10 @@ #include #include #include +#include #include +#include +#include #include #include #ifdef __GLIBC__ @@ -142,15 +145,35 @@ std::shared_ptr ReadUserDataTailContinuation(std::span ProgramSnapshot(const ShaderRegistry& shaders, std::uint64_t address, std::initializer_list types) { if (auto registered = RegisteredProgram(shaders, address, types)) return registered; - require(shaders.empty(), "graphics program does not belong to a compatible registered shader"); return ReadRawShader(address); } namespace { std::atomic rawGraphicsDrawn{false}; +std::mutex rawGraphicsMutex; +std::map rawGraphicsRanges; } -void NoteRawGraphicsDraw() { +void NoteRawGraphicsDraw(const ShaderSnapshot& snapshot) { + if (snapshot.rawGraphicsNoted->load(std::memory_order_acquire)) return; + auto begin = snapshot.codeAddress; + auto end = snapshot.codeAddress + snapshot.code.size() * sizeof(std::uint32_t); + { + std::lock_guard lock(rawGraphicsMutex); + auto next = rawGraphicsRanges.upper_bound(begin); + if (next != rawGraphicsRanges.begin() && std::prev(next)->second >= begin) { + const auto previous = std::prev(next); + begin = previous->first; + end = std::max(end, previous->second); + rawGraphicsRanges.erase(previous); + } + while (next != rawGraphicsRanges.end() && next->first <= end) { + end = std::max(end, next->second); + next = rawGraphicsRanges.erase(next); + } + rawGraphicsRanges.emplace(begin, end); + } + snapshot.rawGraphicsNoted->store(true, std::memory_order_release); rawGraphicsDrawn.store(true, std::memory_order_relaxed); } @@ -158,6 +181,13 @@ bool RawGraphicsDrawn() { return rawGraphicsDrawn.load(std::memory_order_relaxed); } +bool RawGraphicsDrawnIn(std::uint64_t begin, std::uint64_t end) { + if (begin >= end) return false; + std::lock_guard lock(rawGraphicsMutex); + const auto next = rawGraphicsRanges.lower_bound(end); + return next != rawGraphicsRanges.begin() && std::prev(next)->second > begin; +} + namespace { void AwaitRegisteredPreparation(PreparedShaders& prepared, std::unique_lock& lock) { @@ -1434,7 +1464,6 @@ void Driver::RegisterShader(const Shader* shader) { PerformanceTimer timing("Shader.Register"); ShaderPreparationTransaction transaction; CheckFailure(); - require(!RawGraphicsDrawn(), "a shader was registered after graphics programs were drawn without registration"); GuestMemory::CheckRange(shader, sizeof(Shader), 1); Shader fields; std::memcpy(&fields, static_cast(shader), sizeof(Shader)); @@ -1444,6 +1473,7 @@ void Driver::RegisterShader(const Shader* shader) { GuestMemory::CheckRange(shader, fields.header_size, 1); const auto* code = const_cast(fields.code); GuestMemory::CheckRange(code, fields.shader_size, 256); + require(!RawGraphicsDrawnIn(reinterpret_cast(code), reinterpret_cast(code) + fields.shader_size), "a shader was registered over a graphics program that was drawn without registration"); ShaderSnapshot snapshot{reinterpret_cast(code), reinterpret_cast(shader), fields.type, {}, {}}; snapshot.code.resize(fields.shader_size / sizeof(std::uint32_t)); std::memcpy(snapshot.code.data(), code, fields.shader_size); diff --git a/core/libs/prx/libSceAgcDriver/tests/Submit.cpp b/core/libs/prx/libSceAgcDriver/tests/Submit.cpp index d08d98999a7b..f4fe92812001 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Submit.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Submit.cpp @@ -671,9 +671,10 @@ void testProgramSnapshots() { shaders[registeredAddress] = {entry}; check(AgcDriver::DriverDetail::ProgramSnapshot(shaders, registeredAddress, {2}) == entry, "a registered program was not resolved to its registration"); check(AgcDriver::DriverDetail::ProgramSnapshot(shaders, registeredAddress + 4 * 8, {2}) == entry, "an entry inside registered code was not resolved to its registration"); - check(!expectFailure([&] { static_cast(AgcDriver::DriverDetail::ProgramSnapshot(shaders, rawAddress, {2})); }).empty(), "an unregistered program was read raw while shaders are registered"); - const auto unregistered = AgcDriver::DriverDetail::ProgramSnapshot({}, rawAddress, {2}); - check(unregistered != entry && unregistered->codeAddress == rawAddress && unregistered->header.empty() && unregistered->code.size() == 2, "an empty registry did not read the program as raw code"); + const auto unregistered = AgcDriver::DriverDetail::ProgramSnapshot(shaders, rawAddress, {2}); + check(unregistered != entry && unregistered->codeAddress == rawAddress && unregistered->header.empty() && unregistered->code.size() == 2, "an unregistered program beside registered shaders was not read as raw code"); + check(AgcDriver::DriverDetail::ProgramSnapshot({}, rawAddress, {2}) == unregistered, "an empty registry did not read the program as raw code"); + check(!expectFailure([&] { static_cast(AgcDriver::DriverDetail::ProgramSnapshot(shaders, registeredAddress, {1})); }).empty(), "a registered program was accepted as another shader type"); check(AgcDriver::DriverDetail::ProgramSnapshot({}, rawAddress, {2}) == unregistered, "unchanged raw code lost its snapshot identity"); check(!expectFailure([] { static_cast(AgcDriver::DriverDetail::ProgramSnapshot({}, 0, {2})); }).empty(), "an unmapped unregistered program was accepted"); AgcDriver::DriverDetail::DrawDecode decode{}; @@ -690,6 +691,27 @@ void testProgramSnapshots() { check(AgcDriver::DriverDetail::DecodeProgramsCurrent(decode, shaders), "a decode with the rewritten program was not current"); } +void testRawGraphicsRanges() { + using AgcDriver::DriverDetail::RawGraphicsDrawnIn; + constexpr std::uint64_t base = 0x7e0000000000ull; + const auto note = [](std::uint64_t address, std::size_t dwords) { + AgcDriver::DriverDetail::ShaderSnapshot snapshot{address, 0, 0, std::vector(dwords, 0xbf810000u), {}}; + AgcDriver::DriverDetail::NoteRawGraphicsDraw(snapshot); + check(snapshot.rawGraphicsNoted->load(), "a raw graphics program was not marked as noted"); + AgcDriver::DriverDetail::NoteRawGraphicsDraw(snapshot); + }; + note(base + 0x100, 0x10); + note(base, 0x100); + note(base + 0x3f0, 0x8); + note(base + 0x800, 0x1); + check(RawGraphicsDrawnIn(base + 0x200, base + 0x210), "code inside a raw program that contains a smaller one was not reported"); + check(RawGraphicsDrawnIn(base + 0x40c, base + 0x500), "the end of a raw program that overlaps another was not reported"); + check(!RawGraphicsDrawnIn(base + 0x410, base + 0x800), "code between raw programs was reported"); + check(!RawGraphicsDrawnIn(base - 4, base), "code before a raw program was reported"); + check(RawGraphicsDrawnIn(base + 0x700, base + 0x900), "a range covering a raw program was not reported"); + check(!RawGraphicsDrawnIn(base + 0x804, base + 0x808) && !RawGraphicsDrawnIn(base + 0x200, base + 0x200), "code after a raw program or an empty range was reported"); +} + void testProgramWorkgroupUse() { const auto snapshot = [](std::vector code) { AgcDriver::DriverDetail::ShaderSnapshot result{0x1000, 0, 2, std::move(code), {}}; @@ -860,6 +882,7 @@ int main(int argc, char** argv) { check(!unterminated.empty() && bounded->code.size() == 64, "raw compute crossed inaccessible memory or missed its last instruction"); #endif testProgramSnapshots(); + testRawGraphicsRanges(); testProgramWorkgroupUse(); testVertexTailSnapshots(); testRegisteredVertexWorkgroupRouting(); diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/PreparedGraphics.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/PreparedGraphics.cpp index b296b49bebd1..1242c30e3389 100644 --- a/core/libs/prx/libSceAgcDriver/tests/execution/PreparedGraphics.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/execution/PreparedGraphics.cpp @@ -304,7 +304,13 @@ void Check(AgcDriver::VulkanDevice& device, AgcDriver::Graphics::ShaderPath path registry.erase(late.snapshot->codeAddress); } ShaderRegistry invalidRegistry; - invalidRegistry[1] = {std::make_shared(ShaderSnapshot{1, 0, 2, {}, {}})}; + for (const auto& [address, snapshots] : registry) { + for (const auto& snapshot : snapshots) { + auto compute = *snapshot; + compute.type = 0; + invalidRegistry[address].push_back(std::make_shared(std::move(compute))); + } + } DrawDecode invalid{}; invalid.state = prepared.state; Reject([&] { DecodeGraphicsPrograms(invalid, queue, invalidRegistry, true, true); }, "registered"); diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/PreparedUnregisteredGraphics.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/PreparedUnregisteredGraphics.cpp index 19154673951d..df68d87509e9 100644 --- a/core/libs/prx/libSceAgcDriver/tests/execution/PreparedUnregisteredGraphics.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/execution/PreparedUnregisteredGraphics.cpp @@ -51,13 +51,13 @@ void Check(AgcDriver::VulkanDevice& device) { Require(draw.programs.size() == 2 && draw.roles.back() == ProgramRole::Fragment, "unregistered graphics programs did not decode as a vertex and a fragment stage"); for (const auto& program : draw.programs) Require(program.snapshot != nullptr && program.snapshot->header.empty() && program.snapshot->prepared->entries.empty(), "an unregistered graphics program was not read as raw code"); Require(RawGraphicsDrawn(), "a draw of unregistered graphics programs was not noted"); - bool refused = false; - try { - Driver::Get().RegisterShader(nullptr); - } catch (const std::exception& error) { - refused = std::string(error.what()).find("registered after graphics programs were drawn") != std::string::npos; + for (const auto& program : draw.programs) { + const auto begin = program.snapshot->codeAddress; + Require(RawGraphicsDrawnIn(begin, begin + 4u) && RawGraphicsDrawnIn(begin - 4u, begin + 1u), "a raw-drawn program's code is not recorded"); + Require(!RawGraphicsDrawnIn(begin - 4u, begin), "the raw-drawn range starts before the program"); } - Require(refused, "a shader registration after unregistered graphics draws was accepted"); + alignas(256) static std::array Elsewhere{0xbf810000u, 0xbf810000u, 0xbf810000u, 0xbf810000u}; + Require(!RawGraphicsDrawnIn(reinterpret_cast(Elsewhere.data()), reinterpret_cast(Elsewhere.data()) + sizeof(Elsewhere)), "unrelated code reads as drawn raw"); const auto target = device.Target(); std::vector linked; std::vector memory;