[joshuaswarren]

Landing hold (29 October)

Current head 11ae82d8d8c379c2ae895806b4409537c5ceb25b must wait for #10 and a corrected and qualified #8. Chris identified a retained image-mapping lifetime defect in #8; #8's READY status has been withdrawn. Existing stack measurements below are bound to their recorded source versions and do not qualify a future integration with the #8 fix.

Earlier description and evidence

READY TO MERGE after #10 and #8.

Head: 11ae82d8d8c379c2ae895806b4409537c5ceb25b.

GitHub reports MERGEABLE against main: no text conflicts. This is not a test pass.

The main-based head built and passed nine allocator checks.

Device result at this exact head, 13-inch M1, one boot, same CTS binary for both arms, dEQP-VK binding_model chunk 0 (1,000 cases):

	main 7cda7850ed	main + #10 + #8 + #9 (local merge of the three heads, tip 59c25f62c9d)
Pass	340	462
NotSupported	538	538
ResourceError	122	0

All 122 differences are ResourceError to Pass. No case gets worse. Before the run, each arm was checked to load its own driver file. The table shows the three PRs together, not #9 alone. It is one targeted chunk, not full CTS, and the CTS binary's source commit is not independently verified. An Aurora #18 result does not stand in for this one. Landing order: #10, #8, then #9.

Earlier review, source and test notes are kept below with their original bindings. The line below that says the main-based build was not run on a device is superseded by the result above. They do not change this status otherwise.

---

What this fixes

A buffer created with VK_BUFFER_CREATE_DEVICE_ADDRESS_CAPTURE_REPLAY_BIT cannot be replayed. In the first 1000 cases of the conformance group binding_model, every capture and replay case fails. vkCreateBuffer returns VK_ERROR_OUT_OF_DEVICE_MEMORY for a buffer that the test created with a captured address.

The cause is in the address heap. Capture and replay buffers take their addresses from the same heap as every other allocation. In the old code the capture device hands out addresses from the top of the heap. The replay device later asks for the same addresses. In the meantime it gives the same kind of addresses to other allocations. The trace does not say which ones. When one of those sits on a captured address, the replay of the buffer that owns that address fails.

I traced three conformance cases with a throwaway patch that logs every address allocation. In each one the failing request was the last address allocation of the run. It was a fixed-address request on the replay device. It came after 68, 73 and 2 earlier fixed requests had succeeded. The range it asked for overlapped a range that a non-fixed allocation had taken a moment before. No replay buffer was in the way. In the smallest case the request was for 0x8000 bytes at 0x2ffff7c000. The call just before it was a non-fixed allocation of 0x8000 bytes at exactly that address.

What this changes

• The lowest 4 GiB of the read-write user address window is reserved for capture and replay buffers. It has its own heap, and no other allocation uses it, so a captured address is free when it is replayed.
• The range is a fixed part of the window, so a capture made in one process replays in another on the same machine and kernel.
• Each buffer keeps its guard page. The allocation stores a flag, and free returns the range to the heap it came from.
• The read-only shadow of an address stays at the same offset, because the range is inside the read-write half of the window.
• An address that cannot be in the range, or that is in use, returns VK_ERROR_INVALID_OPAQUE_CAPTURE_ADDRESS. The reference page for vkCreateBuffer lists this code. A capture request with no address that finds the range full returns VK_ERROR_OUT_OF_DEVICE_MEMORY.
• agx_va_alloc refuses a fixed capture request outside the range without calling the heap code, which asserts on a range that wraps.
• A test of the allocator that needs no GPU is added as test-agx-va.c.

Limits of this change

• The 4 GiB range is a cumulative limit for all live capture and replay buffers on a device, guards included. An application that creates more than 4 GiB of buffers with the capture and replay flag now gets VK_ERROR_OUT_OF_DEVICE_MEMORY where it did not before. Buffers without the flag are not limited. The ordinary heap is 4 GiB smaller.
• A capture address works only on the same machine and kernel. The window start comes from the kernel's VM start, so it can differ between kernels and chips.
• An address captured by an earlier driver lies in the ordinary window. Its replay is now refused with VK_ERROR_INVALID_OPAQUE_CAPTURE_ADDRESS.

Test results

The allocator test is plain C and I ran it by hand with assertions on. All 9 cases pass. On a model of the old behavior, with the reservation turned off, 6 of the 9 cases fail.

A small test program, run on the 16-inch M1 Max, the new arm as a native build and the control as the built package, one boot. It captures 100 buffers in one process and replays them in a fresh process, in reverse order, with a memory allocation between replays, as the conformance test does.

	Main with #8	This change
Replay in a fresh process	74 of 100 match, then VK_ERROR_OUT_OF_DEVICE_MEMORY	100 of 100 match
seed_0, seed_2 and one replay compute case	ResourceError	Pass

In the same program this change returns the expected code for an address in the ordinary window, a captured address moved up by 4 GiB, an address in use, a 5 GiB buffer with an address (VK_ERROR_INVALID_OPAQUE_CAPTURE_ADDRESS), and a second 3 GiB buffer without an address (VK_ERROR_OUT_OF_DEVICE_MEMORY). A buffer destroyed and created again at the same captured address on the same device gets the same address both times, and both memory binds succeed. That test used one size and one recreate. It did not access the recreated buffer from the GPU. I did not read the return value of the scratch bind, which the code ignores.

The first 1000 cases of the binding_model group (chunk 0 of 151), with the library bytes of built packages, one run per arm. The 16-inch M1 Max and the 13-inch M1 give the same table.

	Control	This change
Pass	340	462
NotSupported	538	538
ResourceError	122	0

Exactly 122 of the 1000 rows differ on both chips, all from ResourceError to Pass, and 878 are unchanged. No row gets worse. The 122 rows are the 10 capture_replay_stress seeds and 112 buffer_device_address replay cases.

The control is the package of main with the change from #8 added. The candidate package is built from the commit on top of #8. This branch carries the same change on main. git patch-id is equal for the two commits, and the main-based commit built natively without errors. I did not run the main-based build on a device.

What this does not show

• This is one chunk, 1000 of 3,253,584 cases. Later chunks of the group, which hold more buffer address cases, were not run.
• The 13-inch M1 ran only the chunk above. The other tests ran on the 16-inch M1 Max.
• The virtio path compiles but did not run.
• The Gallium driver was not built.
• The meson test target for the allocator test was not built.
• Opaque capture addresses of memory objects were not traced.

[joshuaswarren]

GREEN at 11ae82d, with two non-blocking notes. I read the full diff against main (7cda785) and ran no build.

Checked:

• The patch is the same change I reviewed on top of honeykrisp: map placed memory at the allocation size #8. git patch-id is equal for the two commits, so the earlier review and the device runs on the package build apply to this diff.
• The numbers in the body match the raw per-chunk results from both machines. 340 Pass, 538 NotSupported and 122 ResourceError on the control. 462, 538 and 0 on the candidate. The 122 differing rows are 10 capture_replay_stress seeds and 112 replay cases, all ResourceError to Pass, with no row getting worse. The case list has 3,253,584 cases and the group has 151 chunks.
• The capture range check cannot wrap. The guard is added before the size is used. hk_CreateBuffer and agx_va_alloc agree on the guard.
• The new heap is created in one place and torn down in one place. Free picks the heap from the stored flag, so a freed capture address stays reserved.
• The read-only shadow arithmetic is unchanged, because the range sits inside the read-write half of the window.
• This branch merges with main and with the honeykrisp: map placed memory at the allocation size #8 head without conflicts.

Notes:

1. The main-based commit built natively but did not run on a device. Only the commit on top of honeykrisp: map placed memory at the allocation size #8 ran, as the body says. The diff is the same patch, so I accept this.
2. Destroying a capture buffer and creating it again at the same address was tested with one size and one recreate, with no GPU access to the new buffer. agx_va_free unbinds only the read-write range. This is not a regression, and the body states the test limit.

Merging is the maintainers' call.

[joshuaswarren]

Both notes are open items, and the body already states both. I did not change the branch.

1. The main-based commit has not run on a device. Any change to the commit gives a new head id, and the library bytes carry that id. The result above is tied to the library bytes built from the commit on top of honeykrisp: map placed memory at the allocation size #8. I can run the main-based commit once main moves or on request.

2. The destroy and create test used one size and one recreate. I can add more sizes and a GPU access to the recreated buffer as a separate test if the maintainers want it.

[joshuaswarren]

Maintainer routing request for PR 9. This is a request for a merge contact, not a claim that review is finished.

It makes capture and replay buffers replayable in a fresh process. What was run on the head 11ae82d8d8c3 and what was not run is in the description and in the two review comments above; I have not changed it since. PR 9 belongs to a merge order of PR 8, PR 9 and PR 10, then PR 6, then PR 11. A whole-suite run of PR 6 and the PR 11 commit on top of PRs 8, 9 and 10, against PRs 8, 9 and 10 alone, is queued. Its results will be posted on the PR 6 thread.

Question: who can merge into this repository's main branch, and in which of these PRs would you like to start? My account has pull access only.

[joshuaswarren]

Second routing request for PR 9, about 14 hours after the first. It is still a request for a merge contact, not a claim that review is finished.

Nothing on the head has changed since my first note above. The review notes and the list of what was run are in the description and the two review comments.

Question: who can merge into this repository's main branch? If there is a different place to ask, a link is enough. My account has pull access only, so I cannot merge or mark anything ready.

[joshuaswarren]

Status, 25 Oct: unchanged since 16 Oct, no review yet. It is in both arms of the #6 whole-suite run (main plus #8, #9, #10), so the binding_model capture and replay cases are counted equally on both sides. The #12 notes ask for an integration review against the native and Air allocation reservations; I will do that once the suite is running. It can land right after #8, independent of #6.

[joshuaswarren]

Status, 25 Oct, for the landing order (#10 first, then #8, then #9, then #6).

Head 11ae82d8d8c3. It merges cleanly after #10 and #8 on main 7cda7850ed. It does not merge on integration/aurora: four files conflict (agx_bo.h, agx_device.c, agx_device.h, agx_va.c), because that branch has its own capture and replay heap. I have a reconciled version for that branch in preparation. It reuses that heap and adds the range check and the error codes of this PR.

MEASURED: this change is in the control arm of the whole-suite CTS of the #6 stack (16-inch M1 Max and 13-inch M1), with no Pass to Fail change.

NOT measured: the capture and replay CTS groups with only this change.

[joshuaswarren]

The version of this change for integration/aurora is ready: branch aurora-va-check in the fork, head fba14a0b25ea, one commit on 147a62f6190.

That branch already has a capture and replay heap, so this keeps it. The commit records the range, adds agx_va_capture_valid (the guard page counts as part of the allocation, and nothing wraps), refuses an invalid fixed request inside agx_va_alloc, and returns VK_ERROR_INVALID_OPAQUE_CAPTURE_ADDRESS from vkCreateBuffer for an address outside the range or already in use. The unit test covers the range, the validity cases, the in-use case, and that ordinary allocations never land in the range.

MEASURED, native arm64 build of this head on a 16-inch M1 Max:

• hk_va passes. meson test gives 121 pass (the base gives 120; the new test is the difference) and the same 19 failures as the base.
• The driver links and the AGX compiler unit tests pass.

NOT measured: a GPU run and the capture and replay CTS groups on this head.

[joshuaswarren]

Batch 1 for main: #10, then #8, then #9, in that order.

All three apply to main at 7cda7850ed with no conflicts. They touch different code and git applies them in any order. The order is the agreed landing order. The #9 device run below is on top of #10 and #8. The #10 and #8 results below were measured on each branch alone. We checked the three together as a local merge (tip 59c25f62c9d). #19 went into integration/aurora, not main, so this base has not moved.

PR	Head	Change	Device result at that head
#10	b87e7456	accept a NULL array of transform feedback counter buffers	three transform_feedback chunks, 2,719 cases, on a 16-inch M1 Max and a 13-inch M1: the 18 former crashes pass, no other change
#8	ddfb4cf1	map placed memory at the allocation size	memory chunk 0, 1000 cases, both chips: 4 Fail and 1 Crash become Pass, no other row differs
#9	11ae82d8	give capture and replay buffers their own address range	binding_model chunk 0, 1000 cases, 13-inch M1, run at this exact head: 122 ResourceError become Pass, no case gets worse

Stack check, one boot on a 13-inch M1, binding_model chunk 0: main gives 340 Pass, 538 NotSupported and 122 ResourceError. Main with #10, #8 and #9 gives 462 Pass, 538 NotSupported and 0 ResourceError. The 122 changes are all ResourceError to Pass. This shows the three together. It does not isolate #9 from #8 and #10.

These are targeted checks, not the full conformance suite. The CTS binary's source commit is not independently verified.

#6 is not part of this batch. Its run over the touched test groups at its current head has not finished, and three shader_object.performance timing cases still need a decision. We will post that result separately. #3, #4 and #5 are superseded by #6 and will be closed after it merges.

The author account on these PRs has pull-only access, so a maintainer has to merge. Tell us if you want a different order or a different check first.

[joshuaswarren]

@maralcbr could you review this current head for landing after #10 and #8? The exact-head test evidence and remaining scope limits are in the PR body. The requested landing order is #10, then #8, then #9. Our account cannot assign formal reviewers or merge here; please route this to a main-branch maintainer if needed.