Repository navigation
hk: give G17P host-read and aliased-memory flushes memory scope - #20
Open
DjDeveloperr wants to merge 3 commits into
Open
DjDeveloperr wants to merge 3 commits into
DjDeveloperr wants to merge 3 commits into
Conversation
The strong CDM barrier (flags 5) makes compute and transfer writes visible only to later reads through the same virtual address, because the G17P caches are tagged by virtual address. A read of the same memory through a second mapping in the same command returns stale memory. That second mapping can be a sparse binding next to a plain buffer over the same pages, or a sparse buffer's read-only shadow. Bit 4 of the barrier word adds memory scope. On a Neo (A18 Pro), one command buffer fills pages through a sparse buffer, records a transfer barrier, then copies the same pool pages through a plain buffer. Pages read back stale, by barrier word: - 0x60001560 (flags 5): 494 of 526 - 0x60001570 (bit 4): 0 of 526 - 0x6000157c (bits 4:2, the HAL300 memory record): 0 of 526 - 0x600215fe (the G15 wide-visibility word): 0 of 526 - 0x60001564 or 0x60001568 (bit 2 or bit 3 alone): 494 and 495 of 526 Bit 4 also fixes the reverse direction, shader stores, and reads of pages the GPU had already cached through the second mapping. Accept the HAL300 record (bits 4:2) on G17P as well, and name the bits.
On G17P a strong CDM flush leaves written data in the GPU caches, which are tagged by virtual address. Two cases depended on more than that. Host availability: a barrier to HOST_READ records hk's strong cache-maintenance flush, which never reached memory on G17P. The end of the command usually wrote the data back before completion was reported, but not when render work shared the queue. gpubench-stress queue.depth runs 32 compute and 32 render submissions on one queue. Compute results were read back stale 1-3 times per 4-second run, in every run. Tracepoints showed each stale submission had finished on the GPU 180-700 us before the host read it, and the kernel had retired it from its completion stamp 35-240 us before the read. The data reached memory 0.1-1.5 ms later. With the render ring on a second queue, or with a memory-scope host flush, three runs each saw no stale result in ~135000 completions. Aliased memory: sparse buffers and images, and capture/replay buffers, map their memory at an address of their own as well. In sparse.churn, a copy through a plain buffer over the pool read stale data for 494 of 526 pages written through the sparse buffer. That happens after a barrier in the same command buffer, whichever direction or kind of write is involved. Splitting the stream into two kernel commands of the same submission did not help. So: - hk_cdm_visibility_flush() records the strong flush with memory scope (AGX3_CDM_BARRIER_FLUSH_MEMORY) on request. It strengthens a trailing strong record in place, as a weak one already is. Inline stream concatenation keeps the scope of the record it folds into. - Host-read availability always asks for memory scope on G17P. - Compute and transfer dependencies ask for it only while the device holds a resource with its own GPU address (hk_cdm_alias_scope()). This covers pipeline barriers, the deferred-clear barrier, the transfer-before-graphics ordering, and vkCmdSetEvent2. Devices that never create such a resource record the same barriers as before. M5 already gives every strong flush memory scope. Other chips are unchanged. In a microbenchmark of 1 and 64 dispatch-plus-barrier pairs per command buffer, the memory-scope form cost nothing measurable: 106-109 us and 167-172 us per command buffer either way.
[Medium impact] The PR appears safe to merge; the added tests address the previous coverage gap. SummaryThe PR gives G17P barriers memory scope for host reads and reads through another GPU address.
Reviews (2) · Last reviewed commit: "hk: test memory scope across CDM stream ..." · Reviewed by Greptile |
hk_cs_merge_cdm() folds the next stream's opening strong flush into a trailing barrier and must keep the memory scope that record carried, and keep memory_flush_end exact for later merges and cache flushes. The merge test only checked metadata and links, so dropping either would have passed it and brought back stale reads through another mapping. Cover a memory-scoped tail folded with a plain opening barrier, a barrier-only second stream, repeated merges, a memory-scoped opener folded onto a plain tail and an unfolded copy, checking each barrier word and memory_flush_end. With more than one call site the merge stays out of line, so the test stubs hk_drm_attachment_set_add() like the link emitter. The test fails when the fold drops the memory bit, and when a barrier-only fold loses the marker. asahi meson suite: 107 tests pass. (cherry picked from commit c22e446)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirror of aurora-silicon#98 (merged into aurora-wip as deed89d), cherry-picked onto integration/aurora without conflicts. On this branch it builds with
-Dwerror=trueand passes the asahi unit tests 76/76. The hardware results below were measured on the aurora-wip build of the same two commits.On G17P (A18 Pro / Neo), a barrier made written data visible only to later reads through the same GPU virtual address. This PR gives the barrier's flush memory scope where a second address or the host can read the data. Both failures were found by the new gpubench stress suite (aurora-silicon/gpubench#4).
What broke
Root cause
The G17P GPU caches are tagged by virtual address. The strong CDM barrier hk records for a dependency (flags 5, word
0x60001560) makes writes visible only through the same mapping. It does not write the data back to memory.This is not the kernel's 50 ms stamp fallback. Tracepoints on four stale runs showed each stale compute submission retired from its normal completion stamp 35–240 µs before the host read it. The GPU had finished it 180–700 µs before the read.
Evidence
A diagnostic override of hk's strong-flush word was tested on the Neo with sparse.churn, 20 iterations, 526 pages:
0x60001560(flags 5, today)0x60001570(+ bit 4)0x6000157c(+ bits 4:2, M5's memory-scope record)0x600215fe(the G15/G16G wide-visibility word)0x60001564/0x60001568(bit 2 / bit 3 alone)flush_memory, HAL300) on every strong flush, and the existing packing test notes that ordering flags alone do not make writes visible through another mapping.Fix
AGX3_CDM_BARRIER_FLUSH_MEMORY, bits 4:2) on G17P as well. Each bit is documented. G17P keeps the three-bit form that the HAL300 and G15 words share.hk_cdm_visibility_flush()records the strong flush with memory scope on request. A trailing strong record without it is strengthened in place, the same way a weak record already is. Inline stream concatenation keeps the scope of the record it folds into.vkCmdSetEvent2. Devices that never create such a resource record exactly the barriers they did before.The gate reads device state when a barrier is recorded. Any resource the command buffer references exists by then. It is counted from creation, so a later
vkQueueBindSparsethat remaps it is covered. The memory-scope flush writes back every dirty line, so writes recorded in an earlier command buffer are covered too.Not covered: an aliasing resource created after the command buffer was recorded and reached only through a device address the command buffer reads indirectly. This is legal Vulkan but unusual. Closing it would mean gating on the enabled features (
sparseBinding,bufferDeviceAddressCaptureReplay) instead. DXVK and vkd3d-proton enable those whenever they are offered, so that gate would put the flush cost on every Proton game. I chose not to do that.Qualification
Neo (A18 Pro), stock kernel kit (integration/aurora + #563), this branch built with
-Dwerror=true:sparse.churnpassed 6/6 runs at about 1.18M checks each. Baseline fails at iteration 0./var/tmp/cts/run.py, 6 workers.The first synchronization pass had two
rc=-9crashes insignal_order.timeline_semaphore.write_image_*, after a GPU timeout under 6-worker load. Both pass 3/3 in isolation on this branch and on the baseline. The baseline shows the same crash class in its own full pass, and a full rerun on this branch was clean.