Repository navigation
fix(agc): accept GDS DMA_DATA register address controls - #16345
AlexWhitehouse wants to merge 1 commit into
Conversation
Normalize only GDS endpoints with both register-space and no-increment controls to the existing consecutive GDS transfer path. Preserve all other selector rejections and range checks. Cover execution, store resolution, decoding and unsupported combinations independently of testCopies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Correction (rerun): my first run rebuilt only the main test target, but the test pattern also matched the test from 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
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 (4, not counted against this PR)Validation IDs already present on main (9 IDs in 587 tests)Skipped (52)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. |
milescoviello
left a comment
There was a problem hiding this comment.
Checked that the normalisation reaches every consumer on main. Nothing reads the SAS/SAIC/DAS/DAIC bits outside dmaSource/dmaDestination. The users of the selectors all go through Pm4::DmaSource/Pm4::DmaDestination:
- the pipeline overlap check for
DMA_DATAinPacketExecution.cpp(the GDS source range againstDrawPipeline::Queue0()); - the GDS copies and the GDS-destination checks in
PacketMemory.cpp(gdsDestination,gdsWritesPending, the store-limit split); describeDmaEndintools/Pm4Analysis.cpp.
So a controlled GDS endpoint takes the same paths as a plain one everywhere, not just in validation. The refusal loop in the test covers the other 11 selector and control combinations. testDmaGdsAddressControls is CPU-only, so a GPU run adds nothing beyond the CI result above.
AI-assisted: yes (Claude Code)
What
A PM4
DMA_DATA(opcode 0x50) with a GDS endpoint was refused whenever that endpoint's address-control bits were set.dmaSource()anddmaDestination()incore/libs/prx/libSceAgcDriver/Execution/src/Pm4.cppfold SAS/SAIC (packet[6]bits 26 and 28) and DAS/DAIC (bits 27 and 29) into the selector. So GDS (selector 1) with both bits set became selector 13, and validation rejected it withDMA_DATA register source is not implementedorDMA_DATA register destination is not implemented.That is the encoding Mesa's radeonsi emits for every GDS DMA. Mesa 24.0.0
si_cp_dma.c, lines 72–86:DST_SEL(GDS) | DAS(REGISTER) | DAIC(NO_INCREMENT);SRC_SEL(GDS) | SAS(REGISTER) | SAIC(NO_INCREMENT);The bit positions match PAL's GFX9+ packet definition at c5e8000 (selectors, lines 1238–1249; address controls, lines 1285–1289) and Mesa's
pkt3.json.Change:
ResolveStore,DecodeStores, execution and capture therefore treat those packets as the consecutive GDS transfers the driver already performs with clear controls.DMA_DATAare not implemented here.docs/dev/debt/libSceAgcDriver.mdentry records the accepted encodings, the Mesa source, that PS5 is not measured, and what still throws.Tested
testDmaGdsAddressControlsincore/libs/prx/libSceAgcDriver/tests/Pm4.cpp(ctestagc_driver_pm4). Existing assertions are unchanged, andtestCopiesis untouched.Validate,ResolveStoreandDecodeStoresgive the same result as the clear-control GDS packet, and a GDS destination reports no guest memory store.ANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, over the 18 tests matchingagc_driver_pm4|agc_driver_recorder|agc_driver_register_key:mainpasses 18/18. With only the new test added,agc_driver_pm4fails withDMA_DATA register destination is not implemented; a source-only variant fails withDMA_DATA register source is not implemented;VUID-lines and no missing-layer messages.DMA_DATA register source is not implemented.agc_driver_pm4_ring_report_heapwhere Linux registersagc_driver_pm4_priority_refused;main'sPm4.cpp,agc_driver_pm4fails withDMA_DATA register destination is not implemented;VUID-lines and no missing-layer messages. AVK_LOADER_DEBUG=layerrerun ofagc_driver_recorderconfirmed that the validation layer was inserted at instance and device level.python3 tools/check_conventions.py --base origin/main: all conventions checks passed.python3 tools/pr_overlap.py --branch fix/agc-dma-data-gds-address-controls: no confirmed overlap.main.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