From b38c8d6ae8089e38f4c0789cd828726482754555 Mon Sep 17 00:00:00 2001 From: LuisBiceps <54749472+LuisBiceps@users.noreply.github.com> Date: Fri, 9 Oct 2026 11:45:53 +0800 Subject: [PATCH] fix(agc): correct the default tessellation factor ring address The hardware findings give 0xF_F000_0000, which is 0xFF0000000, not 0xFF00000000. Check the ring across threads, at ps5-opengl's size of 0x1b000 and with a canary after the outputs, and say in the debt entry that nothing is written to the ring. --- .../State/src/Configuration.cpp | 2 +- .../tests/TessellationFactorRing.cpp | 34 ++++++++++++------- docs/dev/TechnicalDebt.md | 2 +- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/State/src/Configuration.cpp b/core/libs/prx/libSceAgcDriver/State/src/Configuration.cpp index ade49a0bf..468f92e98 100644 --- a/core/libs/prx/libSceAgcDriver/State/src/Configuration.cpp +++ b/core/libs/prx/libSceAgcDriver/State/src/Configuration.cpp @@ -10,7 +10,7 @@ namespace { struct TessellationFactorRing { - uintptr_t base = 0xff00000000; + uintptr_t base = 0xff0000000; uint32_t size = 0x20000; }; diff --git a/core/libs/prx/libSceAgcDriver/tests/TessellationFactorRing.cpp b/core/libs/prx/libSceAgcDriver/tests/TessellationFactorRing.cpp index 8cadaa87f..f302a0d11 100644 --- a/core/libs/prx/libSceAgcDriver/tests/TessellationFactorRing.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/TessellationFactorRing.cpp @@ -2,6 +2,7 @@ #include #include #include +#include extern "C" { int APS5_VABI sceAgcDriverSetTFRing(const volatile void* base, uint32_t size); @@ -10,6 +11,14 @@ int APS5_VABI sceAgcDriverGetTFRing(uintptr_t* base, uint32_t* size); namespace { +constexpr uint32_t Canary = 0xa5a5a5a5; + +struct RingOutput { + uintptr_t base = ~uintptr_t{0}; + uint32_t size = 0; + uint32_t canary = Canary; +}; + void Require(bool value) { if (!value) std::abort(); } @@ -22,27 +31,26 @@ bool Rejects(TAction action) { } void RequireRing(uintptr_t expectedBase, uint32_t expectedSize) { - uintptr_t base = 0; - uint32_t size = 0; - Require(sceAgcDriverGetTFRing(&base, &size) == 0); - Require(base == expectedBase && size == expectedSize); + RingOutput output; + Require(sceAgcDriverGetTFRing(&output.base, &output.size) == 0); + Require(output.base == expectedBase && output.size == expectedSize && output.canary == Canary); } } int main() { - alignas(256) static uint8_t ring[0x4000]; + alignas(256) static uint8_t ring[0x20000]; const auto ringBase = reinterpret_cast(ring); - RequireRing(0xff00000000, 0x20000); - Require(sceAgcDriverSetTFRing(ring, sizeof(ring)) == 0); - RequireRing(ringBase, sizeof(ring)); - Require(sceAgcDriverSetTFRing(ring + 0x1000, 0x2000) == 0); + RequireRing(0xff0000000, 0x20000); + Require(sceAgcDriverSetTFRing(ring, 0x1b000) == 0); + RequireRing(ringBase, 0x1b000); + std::thread([&] { RequireRing(ringBase, 0x1b000); }).join(); + std::thread([] { Require(sceAgcDriverSetTFRing(ring + 0x1000, 0x2000) == 0); }).join(); RequireRing(ringBase + 0x1000, 0x2000); Require(Rejects([] { sceAgcDriverSetTFRing(nullptr, 0x2000); })); Require(Rejects([] { sceAgcDriverSetTFRing(ring, 0); })); RequireRing(ringBase + 0x1000, 0x2000); - uintptr_t base = 0; - uint32_t size = 0; - Require(Rejects([&] { sceAgcDriverGetTFRing(nullptr, &size); })); - Require(Rejects([&] { sceAgcDriverGetTFRing(&base, nullptr); })); + RingOutput output; + Require(Rejects([&] { sceAgcDriverGetTFRing(nullptr, &output.size); })); + Require(Rejects([&] { sceAgcDriverGetTFRing(&output.base, nullptr); })); } diff --git a/docs/dev/TechnicalDebt.md b/docs/dev/TechnicalDebt.md index 550ff804f..f1837e104 100644 --- a/docs/dev/TechnicalDebt.md +++ b/docs/dev/TechnicalDebt.md @@ -334,7 +334,7 @@ Throughout the project, every function at every stage either **does exactly what - [Vector buffer offsets that are not a multiple of 4](../../core/shader/recompiler/SpirvBackend/src/SpirvMemory/SpirvMemoryInstructions.cpp) (shader recompiler) - dword accesses count whole dwords from the V# base, where the hardware reads the exact bytes at base + offset. - [Min/max S# reduction](../../core/libs/prx/libSceAgcDriver/Graphics/src/GuestSamplerResource.cpp) (libSceAgcDriver) - `FILTER_MODE` 1 and 2 use Vulkan's min and max reduction (the same `SQ_IMG_FILTER_MODE` values RADV programs). Measured on RDNA2 (gfx1035), not on the console: only texels of non-zero weight are reduced, and the weight is taken after rounding to the 8-bit subtexel grid (a sample within 1/512 texel of a centre reduces that texel alone); a driver with other `subTexelPrecisionBits` reduces a different set there. Gathers ignore the mode. - [Min/max S#](../../core/libs/prx/libSceAgcDriver/Graphics/src/GuestSamplerResource.cpp) (libSceAgcDriver, shader recompiler) - throws with anisotropy, a linear mip filter, depth or color-texture compare, or bilinear filtering of an image the recompiler point-samples (sint, converted, depth-bits). -- [Tessellation factor ring](../../core/libs/prx/libSceAgcDriver/State/src/Configuration.cpp) (libSceAgcDriver) - `sceAgcDriverSetTFRing` records the ring and `sceAgcDriverGetTFRing` reports it. Before a title sets one, it reports the console's default, 0xFF00000000 and 128 KiB, as measured in [PS5_Vulkan's hardware findings](https://github.com/mihawk-99/PS5_Vulkan/blob/fde9e3798c2ec1d8fccc90d5067cd6060490607f/docs/HARDWARE_FINDINGS.md). The recorded ring is not used: tessellated draws run as Vulkan tessellation stages, which keep their factors on the host. A null base or a zero size throws; [ps5-opengl's AGC runtime](https://github.com/blackbearreloaded/ps5-opengl/blob/67c873f985eaefe0a72f713529e5632eaa01139c/src/platform/ps5_agc_native_runtime.c) calls `sceAgcDriverSetTFRing(0, 0)` in its cleanup, and what that does on the console is unknown. +- [Tessellation factor ring](../../core/libs/prx/libSceAgcDriver/State/src/Configuration.cpp) (libSceAgcDriver) - `sceAgcDriverSetTFRing` records the ring and `sceAgcDriverGetTFRing` reports it. Before a title sets one, it reports the console's default, 0xFF0000000 and 128 KiB, as measured in [PS5_Vulkan's hardware findings](https://github.com/mihawk-99/PS5_Vulkan/blob/fde9e3798c2ec1d8fccc90d5067cd6060490607f/docs/HARDWARE_FINDINGS.md). Nothing is written to the recorded ring: tessellated draws run as Vulkan tessellation stages, which keep their factors on the host, so a title that reads tessellation factors from the ring finds none. A null base or a zero size throws; [ps5-opengl's AGC runtime](https://github.com/blackbearreloaded/ps5-opengl/blob/67c873f985eaefe0a72f713529e5632eaa01139c/src/platform/ps5_agc_native_runtime.c) calls `sceAgcDriverSetTFRing(0, 0)` in its cleanup, and what that does on the console is unknown. - [Raw `buffer_load_dwordx2`](../../core/shader/recompiler/SpirvBackend/src/SpirvMemory/SpirvMemoryInstructions.cpp) is one 64-bit access on the hardware (with or without glc), so it never sees half of a `buffer_atomic_swap_x2`. Only a glc load that is 8-byte aligned and in bounds, in a program that has the 64-bit storage view (it uses 64-bit buffer atomics), is read with one 64-bit atomic load; every other dwordx2 load is two dword loads and can tear against a concurrent 64-bit write: loads without glc, and loads in a program without 64-bit buffer atomics while another shader writes the pair with `swap_x2`. - [DS src2 operations](../../core/shader/recompiler/Translation/src/MemoryInstructions/SharedMemoryInstructions.cpp) (`ds_*_src2_*`, GFX10.1 only) read `DS[B]`, with `B = A + 4 * sext(offset1[6:0]:offset0)`, and then update `DS[A]` with the matching 32-bit atomic, so the read of `B` is not atomic with the update of `A`. The form that takes the offset from the address (`offset1[7]` set), the 64-bit forms and GDS throw: whether the LDS address keeps `A[31:17]` in that form, and whether the 64-bit forms scale the offset by 4 or 8, is not documented. `ds_min_src2_f32`, `ds_max_src2_f32` and `ds_add_src2_f32` throw: on RDNA2 (gfx1035, the oracle's hardware, which has no src2 opcodes) `ds_min_f32`, `ds_max_f32` and `ds_add_f32` ignore `IEEE_MODE` but depend on whether f32 denormals are flushed (28, 28 and 12 of 196 special rows differ), and that is not modelled for LDS float atomics yet (#981). - [wcsrtombs_s](../../core/libs/prx/libc/src/Multibyte.cpp) (libc) follows the console's `libc.prx`, but a runtime-constraint violation only returns its error: the default handler there writes the message and a newline to `stderr` before returning, which is not done here. Conversion uses the C locale of `wcrtomb` (`MB_CUR_MAX` 1), so the path where a multibyte character does not fit the remaining space is not reached