Repository navigation
Conversation
…oss a storage image's first write Six modes of agc_driver_dcc_keys_first_write_tests. Each uploads a storage image under 0000 keys in a watched block, collects the keys, lets the guest write in the same collect epoch, looks the image up again (the lookup must keep it under its clear keys) and marks it written, which stores uncompressed keys: - agc_driver_dcc_keys_rewrite_after_memo: every key replaced by another clear code; the lookup is answered by the memoized key proof; - agc_driver_dcc_keys_rewrite_after_refresh_proof: the same, the lookup answered by the refresh's own proof (required by the step); - agc_driver_dcc_keys_rewrite_after_epoch_proof: the same, the lookup answered inside the epoch of the last proof (required by the step; APS5_DRAW_PIPELINE=1); - agc_driver_dcc_keys_rewrite_to_register_clear: every key replaced by the register clear code; in these four the guest's keys must still be in memory after the first write; - agc_driver_dcc_keys_partial_rewrite_keeps_store: half of the keys replaced; the half the guest did not touch must read uncompressed after the first write; - agc_driver_dcc_keys_write_beside_keys_keeps_store: a guest write beside the keys in their page; every key must read uncompressed after the first write. The steps run with APS5_NO_HOST_IMPORT=1: an upload's host import ends the write watch over the test's block. Every skip names its reason. An unknown mode on the command line is an error. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…d the keys by another clear code
A storage image uploaded under fast-clear keys stores uncompressed keys at its first GPU write and at
its write-backs. Whether the keys are still that clear kind is decided earlier, by a lookup that can
answer from a memoized collect, from the refresh's own proof, or inside the epoch of its last proof
without collecting at all. When the guest replaced the keys by another clear code in between, the
image stays the one uploaded under the old code, and its store then puts uncompressed keys over
texels taken under the replaced code. Where the driver sees such a write it honours the guest's
code in either order. Seen before the first write, the image is taken under the new code (as stored
for the register clear code, whose keys stay). Seen once the image holds results, a fixed clear code
written over them wins ("new keys that are a clear code make every result dead on hardware too", for
images tracked in 64 KiB units), and the register clear code stays in memory once the first write has
stored its keys. Here the store covers the guest's code, which neither order gives outside the two
cases named below; this change gives the second.
The check is made at the store. When the proved key count is the stored key count, a key proof for
the uploaded kind exists, the caller holds the device lock and no recorded work writes the keys, the
key range is collected uncached; if the guest wrote it since the proof and the keys now read as
another clear code (0000, 0001, 1110, 1111 or the register clear), the store is left out and the
guest's clear stands. Any other reading of the keys (mixed, uncompressed, single) keeps the store,
and so does a write to other data in the keys' page.
For a fixed clear code, where the first write covers every texel or the two codes decode to the same
texels, the result without this change is still the first order, and the result with it is the
second.
GuestMemory::CpuWroteSince is the uncached collect and CPU-stamp compare of UnchangedSinceCollected
with the answer "no" where it cannot tell (no write watch, a failed collect, a generation of 0).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
milescoviello
left a comment
There was a problem hiding this comment.
One interaction with #15456 (the draw finisher by default), which edits neither file, so nothing will flag it as an overlap. On #15456's head ShaderResources::EpochProofs() returns DrawPipelineEnabled(), and that is true unless APS5_NO_DRAW_PIPELINE is set. Once it lands, every mode here runs with epoch proofs. --rewrite-after-memo and --rewrite-after-refresh-proof can then be answered by the epoch proof instead of the memoized collect or the refresh's own proof. Their names and the description would no longer match the path they test, while --rewrite-after-epoch-proof still passes. Setting APS5_NO_DRAW_PIPELINE=1 on the five non-epoch modes (and dropping APS5_DRAW_PIPELINE=1 from the epoch mode) would keep each mode on its own path, whichever of the two lands second.
The rest reads right to me: keysReplacedByClear only keeps the store off when the guest wrote the keys since the key proof's generation, nothing recorded is pending over them, and what they hold now is a different clear code.
AI-assisted: yes (Claude Code)
|
Windows check of head Setup: Windows 11 Enterprise 22621, WinLibs GCC 15.2 UCRT, Release, Radeon RX 6700 XT, AMD driver 26.10.1. Run through the Windows test queue with
The other six tests in the group pass in 3 of 3 runs: The Vulkan device was created in each run. Write watching differs on Windows: guest memory uses AI-assisted: yes. |
…brary and pin each mode to it The epoch mode read StorageTexture::epochRefreshes from the test executable. The counter is an inline static of a header and libSceAgcDriver is a shared library, so on Windows the executable and the library each hold their own copy: the test's copy never moved and the mode stopped at its premise check. The modes now read only through functions of the library: StorageTexture::RefreshesProved() and the thread's RefreshProved lookup outcome, which only the refresh's own proof raises. Each mode now requires the path its name says for the lookup after the key write: a refresh under the memoized collect raises neither count, the refresh's own proof raises both by one, and an answer inside the epoch of the last proof raises RefreshesProved alone. A mode whose lookup is answered on another path fails. A mode requires that the driver counts lookup outcomes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Thanks to both of you. The head is now Windows failure of #15456 (@milescoviello). Confirmed on its head
Checked on Linux (CachyOS, kernel 7.2.10), GCC 16.2.1, AMD Radeon RX 9070 XT (RADV GFX1201, Mesa 26.2.4) and lavapipe (llvmpipe, LLVM 23.1.1),
AI-assisted: yes (Claude Code) |
|
Follow-up Windows/AMD check of the new head Setup: Windows 11 Enterprise 22621, WinLibs GCC 15.2 UCRT, Release, Radeon RX 6700 XT, AMD driver 26.10.1. Run through the Windows test queue with
AI-assisted: yes. |
✅ NVIDIA GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validation IDs already present on main (9 IDs in 600 tests)Skipped (55)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
What
A storage image uploaded under fast-clear DCC keys stores uncompressed keys (0xff) at its first GPU write and at its write-backs. Whether the keys still are that clear kind is decided earlier, by the lookup, which can answer in three ways without seeing a guest write that landed after the thread's earlier collect of the key range in the same collect epoch: from the memoized collect behind the key proof, from the refresh's own proof, and, with
APS5_DRAW_PIPELINE, inside the epoch of the last proof with no collect at all.When the guest replaced the keys by another clear code in that window, the lookup keeps the image as uploaded under the old code, and the first write's store then puts uncompressed keys over texels taken under the replaced code. Where main sees such a write it honours the guest's code in either order. A lookup that sees it before the first write takes the image under the new code (as stored for the register clear code, whose keys stay): the order "clear before the draw". Once an image holds results, a fixed clear code written over them wins ("new keys that are a clear code make every result dead on hardware too", the comment over
droppableinStorageTexture::refresh, for images tracked in 64 KiB units), and the register clear code stays in memory once the first write has stored its keys: the order "clear after the draw". Here the store covers the guest's code, which neither order gives outside the two cases below. This PR gives the second.For a fixed clear code main's result is still the first order in two cases: the first write covers every texel of the surface, or the two codes decode to the same texels for the surface's format. There this PR gives the other order, the guest's clear after the draw.
test(agc): six modes of theagc_driver_dcc_keys_first_write_testsexecutable. Each uploads an image under 0000 keys in a watched block, collects the keys, lets the guest write in the same collect epoch, looks the image up again (the step requires that the lookup kept the clear keys) and marks it written. Each step also requires the path that answers that lookup, read through the driver library (StorageTexture::RefreshesProved()and the thread'sRefreshProvedlookup outcome): neither moves for a refresh under the memoized collect, both rise by one for the refresh's own proof, andRefreshesProved()alone rises for an answer inside the epoch of the last proof.agc_driver_dcc_keys_rewrite_after_memo: every key replaced by another clear code (1111); the lookup is answered by the memoized proof.agc_driver_dcc_keys_rewrite_after_refresh_proof: the same, the lookup answered by the refresh's own proof (required by the step).agc_driver_dcc_keys_rewrite_after_epoch_proof: the same, the lookup answered inside the epoch of the last proof (required by the step;APS5_DRAW_PIPELINE=1).On main these three fail with
AGC graphics: a storage image's first write stored uncompressed keys over keys the guest had replaced by another clear code after a memoized collect: 0 of 256 key bytes are 0xc0 (key 0 is 0xff).agc_driver_dcc_keys_rewrite_to_register_clear: every key replaced by the register clear code. On main it fails withAGC graphics: a storage image's first write stored uncompressed keys over keys the guest had replaced by the register clear code after a memoized collect: 0 of 256 key bytes are 0x20 (key 0 is 0xff).agc_driver_dcc_keys_partial_rewrite_keeps_store: half of the keys replaced. The half the guest did not touch must read uncompressed after the first write. The rewritten half is not checked (see Limits).agc_driver_dcc_keys_write_beside_keys_keeps_store: a guest write beside the keys, in their page. Every key must read uncompressed after the first write.These two pass on main and with the fix.
The steps run with
APS5_NO_HOST_IMPORT=1, because an upload's host import ends the write watch over the test's block; every skip prints its reason.fix(agc): the check is made at the store.StorageTexture::keysReplacedByClearruns inMarkDirtyand at the three write-back stores. It answers yes only when the proved key count is the stored key count, a key proof for the uploaded kind exists, the caller holds the device lock, no recorded work writes the keys, the guest wrote the key range since the proof (GuestMemory::CpuWroteSince, new: the uncached collect and CPU-stamp compare ofUnchangedSinceCollected, answering "no" where it cannot tell), and the keys read again as another clear code (0000, 0001, 1110, 1111 or the register clear). Then the store is left out and the guest's clear stands.Limits:
guestBytes / 256(single-level pipe-aligned R64KBX with 4-byte elements). For element sizes that do not divide 256 main reads single-colour keys as uncompressed and leaves them in place where it sees them; here the store covers them.MarkDirtyhas already taken the image out of its clear kind when the write-back runs.ProvedCurrentDccKeys(the register clear's proof) has the same window and is not changed here.Cost: one uncached collect of the key range per first write, and per write-back store that finds the image still under its clear kind, of an image that has a proof for its uploaded kind and no key tail; a read of the keys only when that collect finds a CPU write.
Related: #6209 (open) changes
ProvedCurrentDccKeysandProvedClearKeys, which this PR does not touch. #6186 (open) keeps CPU stores made after a deferred key store was recorded, the window after this one.Tested
Head 5287cda is this PR's three commits on upstream/main 3bd88ca (the third, test only, came after review: see the thread). The runs below are clean merges onto main 9f2b939: main + the two test commits, main + the head, and main + the head + the head of #15456.
ctest -V -R '^agc_driver_dcc_key'withVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation ANYPS5_NO_SHADER_CACHE=1 ANYPS5_REQUIRE_VULKAN=1, on an AMD Radeon RX 9070 XT (RADV GFX1201, Mesa 26.2.4) and on lavapipe (llvmpipe, LLVM 23.1.1, Mesa 26.2.4). The pattern matches the six new steps and the two existing key tests,agc_driver_dcc_keys_first_writeandagc_driver_dcc_key_presync.VUID-.VUID-.APS5_NO_DRAW_PIPELINEis set): all eight pass on both drivers, none skipped. NoVUID-.VK_LAYER_KHRONOS_validationinserted; no log reportswas not found but was requested by env var VK_INSTANCE_LAYERS.ctest -R '^agc'on the RX 9070 XT, without the validation layer, main + the head: 1 of 1352 fails,agc_driver_tessellation(segmentation fault), and 23 tests skip. On the earlier head b04dc41 over main 3bd88ca: 5 of 1349 failed with the test commit alone and 1 of 1349 with the fix; the four rewrite steps were the only difference in the failed list, with the same failing test and the same 23 skipped names as now.python3 tools/check_conventions.py --base upstream/main: passes.No title effect is claimed.
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