From 98ed835123ea6c198f52939116e5d66d41d9bc70 Mon Sep 17 00:00:00 2001 From: alfalfa-star Date: Tue, 6 Oct 2026 16:12:31 +0200 Subject: [PATCH] feat(agc): run COND_INDIRECT_BUFFER branches whose comparison always passes PPSA14632 (Unreal Engine) writes sceAgcCbBranch(mode 1, compare function 0, mask 0, reference 0) with a then buffer and a null else buffer of 0 dwords. It leaves the compare address uninitialised, so it is sometimes only 4-byte aligned. The writer rejected the null else buffer and the misaligned compare address, and the driver accepted opcode 0x3f only as the 4-dword INDIRECT_BUFFER, so the 14-dword branch would have failed at submission. sceAgcCbBranch no longer checks the compare address of an always-pass branch, which reads nothing, or the address of a buffer of 0 dwords. The driver runs the first buffer of an always-pass COND_INDIRECT_BUFFER in modes 1 and 2 (if-then and if-then-else) in place of the packet. A comparison, a predicated branch and other modes throw. Co-Authored-By: Claude Opus 5.5 --- core/libs/prx/libSceAgc/Cb/src/Branch.cpp | 6 +-- core/libs/prx/libSceAgc/tests/BranchPatch.cpp | 16 ++++++++ .../src/Driver/Queues/Submission.cpp | 17 +++++++++ core/libs/prx/libSceAgcDriver/tests/Pm4.cpp | 37 +++++++++++++++++++ docs/dev/TechnicalDebt.md | 1 + 5 files changed, 74 insertions(+), 3 deletions(-) diff --git a/core/libs/prx/libSceAgc/Cb/src/Branch.cpp b/core/libs/prx/libSceAgc/Cb/src/Branch.cpp index 649b810f6..b300fb533 100644 --- a/core/libs/prx/libSceAgc/Cb/src/Branch.cpp +++ b/core/libs/prx/libSceAgc/Cb/src/Branch.cpp @@ -18,9 +18,9 @@ std::uint32_t* APS5_VABI sceAgcCbBranch(CommandBuffer* buf, std::uint8_t mode, s const auto address = reinterpret_cast(compareAddress); const auto first = reinterpret_cast(buffer1); const auto second = reinterpret_cast(buffer2); - Agc::Command::CheckAddress(address, 8, __func__); - Agc::Command::CheckAddress(first, 4, __func__); - Agc::Command::CheckAddress(second, 4, __func__); + if (compareFunction != 0) Agc::Command::CheckAddress(address, 8, __func__); + if (first != 0 || sizeInDwords1 != 0) Agc::Command::CheckAddress(first, 4, __func__); + if (second != 0 || sizeInDwords2 != 0) Agc::Command::CheckAddress(second, 4, __func__); return Agc::Command::Emit(buf, 0x3fu, {mode | (static_cast(compareFunction) << 8u), static_cast(address), static_cast(address >> 32u), static_cast(mask), static_cast(mask >> 32u), static_cast(reference), static_cast(reference >> 32u), static_cast(first), static_cast(first >> 32u), sizeInDwords1 | (static_cast(cachePolicy1) << 28u), static_cast(second), static_cast(second >> 32u), sizeInDwords2 | (static_cast(cachePolicy2) << 28u)}, __func__); } diff --git a/core/libs/prx/libSceAgc/tests/BranchPatch.cpp b/core/libs/prx/libSceAgc/tests/BranchPatch.cpp index 0062068af..2382676ee 100644 --- a/core/libs/prx/libSceAgc/tests/BranchPatch.cpp +++ b/core/libs/prx/libSceAgc/tests/BranchPatch.cpp @@ -58,6 +58,21 @@ void testWriter() { check(storage.packet[expected.size()] == 0xabcdef01u, "branch command overwrote following word"); } +void testUnusedFields() { + std::array words{}; + CommandBuffer buffer{words.data(), words.data() + words.size(), words.data(), words.data() + words.size(), nullptr, nullptr, 0}; + auto* packet = sceAgcCbBranch(&buffer, 1, 0, compareAt(0x0000001234567894ull), 0, 0, 0, targetAt(0x0000002233445564ull), 0x123u, 0, nullptr, 0); + const std::array expected{0xc00c3f00u, 0x1u, 0x34567894u, 0x12u, 0u, 0u, 0u, 0u, 0x33445564u, 0x22u, 0x123u, 0u, 0u, 0u}; + check(std::equal(expected.begin(), expected.end(), packet), "an always-taken branch without an else target was not written"); + packet = sceAgcCbBranch(&buffer, 1, 0, nullptr, 0, 0, 0, nullptr, 0, 0, nullptr, 0); + check(packet[1] == 1u && packet[2] == 0u && packet[8] == 0u && packet[10] == 0u, "an empty always-taken branch was not written"); + const auto cursor = buffer.cursor_up; + expectFailure([&] { sceAgcCbBranch(&buffer, 1, 3, compareAt(0x0000001234567894ull), 0, 0, 0, targetAt(0x2000u), 1, 0, nullptr, 0); }); + expectFailure([&] { sceAgcCbBranch(&buffer, 1, 0, nullptr, 0, 0, 0, nullptr, 1, 0, nullptr, 0); }); + expectFailure([&] { sceAgcCbBranch(&buffer, 2, 0, nullptr, 0, 0, 0, targetAt(0x2000u), 1, 0, nullptr, 1); }); + check(buffer.cursor_up == cursor, "a rejected branch advanced the command buffer"); +} + void testCompareAddress() { Storage storage; auto expected = storage.words; @@ -109,6 +124,7 @@ void testMalformedPackets() { int main() { try { testWriter(); + testUnusedFields(); testCompareAddress(); testTarget(sceAgcBranchPatchSetThenTarget, 8, 0x10000000u, "then target setter wrote the wrong words"); testTarget(sceAgcBranchPatchSetElseTarget, 11, 0x20000000u, "else target setter wrote the wrong words"); diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Queues/Submission.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Queues/Submission.cpp index cdc6d68b8..08f5cee6c 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Queues/Submission.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Queues/Submission.cpp @@ -43,6 +43,23 @@ bool Driver::copySegment(Submission& submission, const std::uint32_t* guest, std } reach(cursor, cursor + count); const auto opcode = (header >> 8u) & 0xffu; + if (opcode == 0x3fu && count == 14) { + const auto mode = guest[cursor + 1] & 3u; + require(mode == 1u || mode == 2u, "invalid COND_INDIRECT_BUFFER mode"); + require(((guest[cursor + 1] >> 8u) & 7u) == 0u, "a COND_INDIRECT_BUFFER with a comparison is not implemented"); + require(!Pm4::Predicated(header), "a predicated COND_INDIRECT_BUFFER is not implemented"); + const auto* target = reinterpret_cast(static_cast(guest[cursor + 8] & ~3u) | (static_cast(guest[cursor + 9] & 0xffffu) << 32u)); + const std::size_t targetWords = guest[cursor + 10] & 0xfffffu; + if (targetWords != 0) { + GuestMemory::CheckRange(target, targetWords * sizeof(std::uint32_t), alignof(std::uint32_t)); + if (copySegment(submission, target, targetWords, budget)) { + require(guarded.empty(), "a REWIND inside a conditional execution range is not implemented"); + return true; + } + } + cursor += count; + continue; + } if (opcode == 0x3fu) { require(count == 4, "invalid INDIRECT_BUFFER size"); const auto* target = reinterpret_cast(static_cast(guest[cursor + 1] & ~3u) | (static_cast(guest[cursor + 2] & 0xffffu) << 32u)); diff --git a/core/libs/prx/libSceAgcDriver/tests/Pm4.cpp b/core/libs/prx/libSceAgcDriver/tests/Pm4.cpp index 3af51bbd8..e8b1d74e1 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Pm4.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Pm4.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #ifdef _WIN32 #ifndef NOMINMAX @@ -737,6 +738,41 @@ void testConditionalSubmission() { check(results[13] == 0, "a rejected conditional submission executed a guarded packet"); } +std::vector branch(std::uint32_t mode, std::uint32_t function, const std::vector* first, const std::vector* second) { + const auto address = [](const std::vector* target) { return target ? reinterpret_cast(target->data()) : std::uintptr_t{0}; }; + const auto size = [](const std::vector* target) { return target ? static_cast(target->size()) : 0u; }; + return makePacket(0x3f, {mode | (function << 8u), 0, 0, 0, 0, 0, 0, static_cast(address(first)), static_cast(address(first) >> 32u), size(first), static_cast(address(second)), static_cast(address(second) >> 32u), size(second)}); +} + +void testBranchSubmission() { + static std::array results{}; + static std::vector first, second; + results.fill(0); + first = joinPackets({writeWord(results[0], 71)}); + second = joinPackets({writeWord(results[1], 72)}); + auto words = joinPackets({branch(1, 0, &first, nullptr), writeWord(results[2], 73)}); + submitWords(words); + AgcDriverWaitIdle_nid_postfix(); + check(results[0] == 71 && results[1] == 0 && results[2] == 73, "an always-taken if-then COND_INDIRECT_BUFFER did not run its buffer"); + results.fill(0); + words = joinPackets({branch(2, 0, &first, &second), writeWord(results[2], 74)}); + submitWords(words); + AgcDriverWaitIdle_nid_postfix(); + check(results[0] == 71 && results[1] == 0 && results[2] == 74, "an always-taken if-then-else COND_INDIRECT_BUFFER ran the wrong buffer"); + results.fill(0); + words = joinPackets({branch(1, 0, nullptr, nullptr), writeWord(results[2], 75)}); + submitWords(words); + AgcDriverWaitIdle_nid_postfix(); + check(results[2] == 75, "an empty COND_INDIRECT_BUFFER skipped the next packet"); + for (const auto& [mode, function, text] : {std::tuple{1u, 3u, "with a comparison"}, std::tuple{0u, 0u, "invalid COND_INDIRECT_BUFFER mode"}}) { + results.fill(0); + words = joinPackets({writeWord(results[3], 1), branch(mode, function, &first, nullptr)}); + expectFailure([&] { submitWords(words); }, text); + AgcDriverWaitIdle_nid_postfix(); + check(results[0] == 0 && results[3] == 0, "a rejected COND_INDIRECT_BUFFER submission executed a packet"); + } +} + void testPredicatedSubmission() { alignas(16) std::uint64_t flag[2] = {0, 0}; alignas(16) std::array written{}; @@ -819,6 +855,7 @@ int main(int argc, char** argv) { testDriverSubmission(); testPredicatedSubmission(); testConditionalSubmission(); + testBranchSubmission(); LibcRunShutdown_nid_postfix(); std::puts("PM4 catalog, registers, state, memory, conditional execution and submission tests passed"); return 0; diff --git a/docs/dev/TechnicalDebt.md b/docs/dev/TechnicalDebt.md index 645d976a4..b712e7139 100644 --- a/docs/dev/TechnicalDebt.md +++ b/docs/dev/TechnicalDebt.md @@ -146,6 +146,7 @@ Throughout the project, every function at every stage either **does exactly what - [COND_EXEC](../../core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp) reads its condition after the earlier packets of its queue complete. Without PFP_SYNC_ME the console's graphics PFP reads it ahead of the ME, so a title that races the read sees a later value here. - A [flip inside a COND_EXEC range](../../core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/CommandValidation.cpp) throws: the flip is reserved with the video output when the buffer is submitted, and how the console withdraws a skipped one is unknown. - A [COND_EXEC range](../../core/libs/prx/libSceAgcDriver/Execution/src/Driver/Queues/Submission.cpp) must hold whole packets and end inside its command buffer. A chained INDIRECT_BUFFER or a REWIND inside a range throws, as what the CP does with the skipped dwords then is unknown. +- A [COND_INDIRECT_BUFFER](../../core/libs/prx/libSceAgcDriver/Execution/src/Driver/Queues/Submission.cpp) (`sceAgcCbBranch`) runs only with the always-pass compare function; a comparison or a predicated packet throws. Modes 1 and 2 run the first buffer, assumed to be the if-then and if-then-else modes of AMD's PM4 packet. [sceAgcCbBranch](../../core/libs/prx/libSceAgc/Cb/src/Branch.cpp) does not check the compare address of an always-pass branch or the address of an empty buffer, as PPSA14632 passes an uninitialised compare address and a null else buffer of 0 dwords. - [v_mullit_f32](../../core/shader/recompiler/Translation/src/FloatInstructions.cpp) follows the hardware measured with the MODE register at IEEE=1 and denormals kept (a signaling NaN src0 is quieted, a positive denormal src2 counts as positive): the recompiler does not read MODE, so other IEEE/denormal settings are not modelled. - [v_fma_legacy_f32](../../core/shader/recompiler/Translation/src/FloatInstructions.cpp) (VOP3 0x140) is assumed to be fused, which has not been measured on hardware. LLVM decodes this encoding as `v_mad_legacy_f32` for gfx1013, and `v_mac_legacy_f32` (VOP2 0x06) was measured to round its product before the add, so 0x140 may do the same. - The executable file that [relinker](../../core/relinker/elfpatcher/src/windows/WindowsPeWriter.cpp) generates opens the console when launched, which is inconvenient for playability.