diff --git a/src/graphics/guest_gpu/graphicsRun.cpp b/src/graphics/guest_gpu/graphicsRun.cpp index 3b8315307..639336aa0 100644 --- a/src/graphics/guest_gpu/graphicsRun.cpp +++ b/src/graphics/guest_gpu/graphicsRun.cpp @@ -689,7 +689,7 @@ void CommandProcessor::ProcessPm4(Pm4Execution& execution) { const auto opcode = (packet_header >> 8u) & 0xffu; EXIT_NOT_IMPLEMENTED(remaining_dw > total_dw); - if (packet_header == 0x80000000u) { + if (packet_header == 0x80000000u || Pm4::PacketSizeDw(packet_header) == 1u) { cursor.offset_dw++; execution.m_made_progress = true; continue; diff --git a/src/graphics/guest_gpu/pm4.cpp b/src/graphics/guest_gpu/pm4.cpp index 551da30a2..554b33255 100644 --- a/src/graphics/guest_gpu/pm4.cpp +++ b/src/graphics/guest_gpu/pm4.cpp @@ -120,15 +120,15 @@ void DumpPm4PacketStream(Common::File* file, const uint32_t* cmd_buffer, uint32_ uint32_t len = 0; const auto packet_type = static_cast(cmd_id >> 30u); - // Type-2 packets are header-only padding; every other packet type requires a body. - EXIT_NOT_IMPLEMENTED(dw < 2 && packet_type != PacketType::Type2); + EXIT_NOT_IMPLEMENTED(dw < 2 && packet_type != PacketType::Type2 && + PacketSizeDw(cmd_id) != 1u); switch (packet_type) { case PacketType::Type3: { const bool sh_gx = (cmd_id & 0x2u) == 0; const uint8_t op = ((cmd_id >> 8u) & 0xffu); const auto r = KYTY_PM4_R(cmd_id); - len = ((cmd_id >> 16u) & 0x3fffu) + 1; + len = PacketSizeDw(cmd_id) - 1u; EXIT_NOT_IMPLEMENTED(len >= dw); diff --git a/src/graphics/guest_gpu/pm4.h b/src/graphics/guest_gpu/pm4.h index 8c724108d..80d190226 100644 --- a/src/graphics/guest_gpu/pm4.h +++ b/src/graphics/guest_gpu/pm4.h @@ -19,6 +19,10 @@ class File; namespace Libs::Graphics::Pm4 { +constexpr uint32_t PacketSizeDw(uint32_t header) { + return (header & 0x3fffff00u) == 0x3fff1000u ? 1u : KYTY_PM4_LEN(header); +} + constexpr uint32_t IT_NOP = 0x10; constexpr uint32_t IT_SET_BASE = 0x11; constexpr uint32_t IT_CLEAR_STATE = 0x12; diff --git a/src/libs/agc.cpp b/src/libs/agc.cpp index 49a327c4a..151c5546e 100644 --- a/src/libs/agc.cpp +++ b/src/libs/agc.cpp @@ -1730,7 +1730,7 @@ int KYTY_SYSV_ABI AgcDriverRegisterWorkloadStream(uint32_t stream_id, const void } uint32_t* KYTY_SYSV_ABI AgcCbNop(CommandBuffer* buf, uint32_t size_in_dwords) { - if (buf == nullptr || size_in_dwords < 2) { + if (buf == nullptr || size_in_dwords == 0) { return nullptr; } @@ -1741,9 +1741,6 @@ uint32_t* KYTY_SYSV_ABI AgcCbNop(CommandBuffer* buf, uint32_t size_in_dwords) { } cmd[0] = KYTY_PM4(size_in_dwords, Pm4::IT_NOP, Pm4::R_ZERO); - if (size_in_dwords > 1) { - memset(cmd + 1, 0, static_cast(size_in_dwords - 1) * 4); - } return cmd; } @@ -3678,12 +3675,7 @@ int KYTY_SYSV_ABI AgcDmaDataPatchSetSrcAddressOrOffsetOrImmediate( } uint32_t KYTY_SYSV_ABI AgcGetPacketSize(uint32_t* packet) { - const auto cmd_id = packet[0]; - if ((cmd_id & 0x3fffff00u) == 0x3fff1000u) { - return 1; - } - - return KYTY_PM4_LEN(cmd_id); + return Pm4::PacketSizeDw(packet[0]); } int KYTY_SYSV_ABI AgcSetPacketPredication(uint32_t* packet, uint32_t predication) { @@ -3725,12 +3717,7 @@ int KYTY_SYSV_ABI AgcSetRangePredication(uint32_t* start, const volatile uint32_ const auto cmd_id = packet[0]; packet[0] = (cmd_id & ~1u) | predication_bit; - auto size = KYTY_PM4_LEN(cmd_id); - if ((cmd_id & 0x3fffff00u) == 0x3fff1000u) { - size = 1; - } - - packet_va += size * sizeof(uint32_t); + packet_va += Pm4::PacketSizeDw(cmd_id) * sizeof(uint32_t); packet = reinterpret_cast(packet_va); } diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index 870dfd4d8..8748ecb01 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/tests/ShaderRecompilerComputeTests.cpp @@ -1,5 +1,6 @@ #include "common/assert.h" #include "common/emulatorConfig.h" +#include "common/file.h" #include "common/hostException.h" #include "common/logging/log.h" #include "common/subsystems.h" @@ -41778,6 +41779,43 @@ void CheckPm4IndirectControlFlow(RenderContext &renderer) { static_cast(address(target.data())), static_cast(address(target.data()) >> 32u), control}; }; + // A one-word NOP must leave the following completion write reachable. + std::array nop_commands{}; + CommandBufferLayout nop_dcb{nop_commands.data(), nop_commands.data() + nop_commands.size(), + nop_commands.data(), nop_commands.data() + nop_commands.size(), nullptr, nullptr, 0}; + auto* nop_cb = reinterpret_cast(&nop_dcb); + Require(name, "zero-sized NOP", Gen5::AgcCbNop(nop_cb, 0) == nullptr && + nop_dcb.cursor_up == nop_commands.data() && nop_commands[0] == 0, + "zero-sized reservation changed the command buffer"); + Require(name, "empty command list NOP", Gen5::AgcCbNop(nop_cb, 1) == nop_commands.data() && + nop_dcb.cursor_up == nop_commands.data() + 1 && nop_commands[0] == 0xffff1000u && + Gen5::AgcCbNopGetSize(1) == 4 && Gen5::AgcGetPacketSize(nop_commands.data()) == 1, + "native NOP(1) did not reserve one DWORD"); + const auto completion = write(&selected, 7); + std::copy(completion.begin(), completion.end(), nop_commands.begin() + 1); + nop_commands.back() = 0xffff1000u; + for (uint32_t predication : {1u, 0u}) { + Gen5::AgcSetRangePredication(nop_commands.data(), nop_commands.data() + nop_commands.size(), predication); + Require(name, "NOP range predication", nop_commands[0] == (0xffff1000u | predication) && + nop_commands.back() == (0xffff1000u | predication) && + nop_commands[1] == (completion[0] | predication) && nop_commands[5] == 7, + "range predication skipped a packet or modified a payload"); + Pm4Execution execution; + selected = 0; + Require(name, "NOP and completion execution", + processor.Process(execution, nop_commands) == Pm4ProcessResult::Complete && selected == 7, + "one-DWORD NOP prevented the completion write or overread the stream end"); + } + Common::File dump; + Require(name, "NOP dump file", dump.CreateInMem(), "could not create PM4 dump"); + Pm4::DumpPm4PacketStream(&dump, nop_commands.data(), 0, nop_commands.size()); + std::array payload{0, 0x12345678u, 0x9abcdef0u}; + CommandBufferLayout payload_dcb{payload.data(), payload.data() + payload.size(), + payload.data(), payload.data() + payload.size(), nullptr, nullptr, 0}; + Require(name, "NOP payload reservation", + Gen5::AgcCbNop(reinterpret_cast(&payload_dcb), 3) == payload.data() && + payload == std::array{0xc0011000u, 0x12345678u, 0x9abcdef0u}, + "NOP emitter overwrote the caller's reserved payload"); const auto then_commands = write(&selected, 11); const auto else_commands = write(&selected, 33); const auto branch_suffix = write(&selected, 44);