Skip to content

G15: fix stamp wrap and VM slot exhaustion - #233

Closed
erykwieliczko wants to merge 2 commits into
integration/aurorafrom
fix/g15-cts-stability
Closed

erykwieliczko wants to merge 2 commits into
integration/aurorafrom
fix/g15-cts-stability

Conversation

@erykwieliczko

@erykwieliczko erykwieliczko commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator
Wrap compute, TA and fragment stamps through zero, keeping absent waits and completed progress separate from zero stamps.

Bind ASIDs without waiting at publication and release them when published
prefixes retire. Retry temporary exhaustion without failing healthy VMs.
Idle VMs no longer reserve hardware slots.

Rebased onto integration/aurora at 937b727ef9a3a9ff09489d597ef4b28f3b6fdbfa.
This PR's new head is 8525dd8e380a708356ec1ffaef0bbdf266cb328b;
#238 remains stacked on it. Both patches are unchanged by the rebase.

The combined stack is packaged and booted on J514S: kernel
7.2.11-m3plasma-8824dd6-ARCH with Mesa git-eaf69705c3 from
omacom/mesa#17. Kernel host checks: 80 pass, 2 ignored;
50 GEM lifetime cases and Settings/theme checks pass. The broader CTS sample
has 7,054 passes, and the 16-set group has 2,694 passes. The seven-case
regression group has six passes and one wrong-result flake that passed its
retry (uaddcarry.uint_highp_geometry); its cause is unresolved. Full CTS
on this rebased stack is in progress, not yet qualified as a full pass.

Earlier validation included 100 submitting VMs through one ASID, split jobs,
PB reclaim, recovered-hang isolation and 13 million mixed compute/render
pairs across all stamp wraps. The historical full Vulkan 1.4 run had
3,247,552 cases, zero failures/crashes on kernel m3plasma-5782625 and
Mesa git-ea2c18f45c; that result belongs to the pre-rebase stack.

@maralcbr maralcbr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @erykwieliczko, this is a careful fix and the CTS evidence is strong. The stamp change is right: stamp_seed now follows the firmware's 2^24-step cycle through zero (encode.rs:68-70), and every place that used 0 to mean "no wait" or "done" is now an Option (engine.rs:161,392, recovery/logic.rs:148-152, poll.rs:506-525). I found no zero sentinel left on stamps or targets in g15/. The compute cell's initial 0 (graph.rs:255) can't read as reached, because the range only spans zero after about 4.1M computes. Per-publication binding also holds up. Publication only appends behind flights of the same VM (engine.rs:566-575), so a horizon j2.bind() reuses the VM's binding and can't hit ENOSPC. Bindings drop only once next == sent, and an unbound VM's unmaps still invalidate its last slot through bind_token (mmu.rs:752-771), so reusing a token is safe.

One thing before merge:

  • Three G15 host suites no longer build or pass. CI's "host tests (tools)" job fails on tools/asahi/g15-input-error-tests.py, g15-zero-words-tests.py and g15-pb-growth-tests.py. I get the same result locally with the host-tests.json arguments: all three exit 1 at 5782625d6e19 and 0 at 13c3371ce23d. They extract the production files, so their mocks need to follow the new interfaces:
    • g15-pb-growth-tests.py:626-642: the mock VmState needs id and the mock Job needs slot(), since pools.rs:628 and :916 now use them (E0609/E0599 today).
    • g15-input-error-tests.py:63: the mock Job needs release_binding(), which job.rs:152 now calls. The control fails as NOBUILD.
    • g15-zero-words-tests.py:29: please refresh the hw.rs hash, with a note that the change only removes MAX_VMS. Keep the T6030 function hash at :33; it still matches.

Smaller things:

  • The "until reboot" wording is too broad (vm.rs:16-17, uat.rs:28-29). Only jobs retained after a fail-stop keep their slot until reboot. A recovered hang's failed jobs aren't retained (recovery.rs:351-353), so their binding goes when the scheduler drops the last reference, as the comment at engine.rs:684-685 already says. Suggest: "failed published jobs keep it until their last reference drops (until reboot for jobs retained after a fail-stop)".
  • nit: rustfmt --check wants braces around the compute arm at engine.rs:923.

Behaviour changes to note in the PR body: VM creation is no longer capped at 63 (MAX_VMS is gone), and with robust_isolation=1 many VMs now share the one slot with an ASID flush per switch, instead of the second VM getting ENOSPC.

Checked: tools/testing/selftests/drivers/gpu/asahi-g15/run.py passes locally (76 ok, 2 ignored, including the new firmware_stamp_wrap and progress_at_zero_wrap). check-commits.py is clean for both commits. In CI, package, KUnit, boot, compile, device trees and the aurora-sep host tests pass. Only the tools group fails, with the three suites above. With those fixed and the tools group green, I'll approve. For release, not this merge: we have no M3 in our lab, so G15 hardware coverage rests on your M3 Pro runs. A short smoke test on a second G15 machine before release would help.

//! lives; at most [`super::hw::sizes::MAX_VMS`] such VMs (ENOSPC beyond, checked before
//! binding so the slot allocator never blocks).
//! - leave it unbound until a runnable job needs a TTBAT slot. Each published job holds
//! its binding through completion; failed published jobs retain theirs until reboot.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only fail-stop retained jobs keep the binding until reboot. A recovered hang's failed jobs release it when their last reference drops (recovery.rs:351-353). Suggest "until their last reference drops (until reboot for jobs retained after a fail-stop)".

//! User VMs are bound to exclusive TTBAT slots for their lifetime through `mmu::Uat::bind`
//! (`vm.rs`); the shared layer writes TTBAT `(root | 1 | slot << 48, TABLES | 1 |
//! Jobs acquire exclusive TTBAT slots without waiting at publication (`mmu::Uat::try_bind`)
//! and return them after successful retirement; failed published jobs retain their slots.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here: "failed published jobs retain their slots" holds only for fail-stop retained jobs.

let encoded = (|| {
for k in 0..n2 {
match command(&j2, k).ok_or(EINVAL)? {
Command::Compute(c) => self.prefix_compute(&j2, c, &mut pf, f, false)?,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: rustfmt --check wants this arm in braces.

.iter()
.enumerate()
.any(|(i, f)| self.receipts(i).renders != 0 && f.job.vm.slot == ctx);
.any(|(i, f)| self.receipts(i).renders != 0 && f.job.slot() == ctx);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

g15-pb-growth-tests.py extracts this file, and its mock Job has no slot() (and the mock VmState has no id for :628), so the suite no longer builds.

}
}
if result.is_ok() || !engine {
self.release_binding();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

g15-input-error-tests.py's mock Job needs release_binding() for this call; its control fails as NOBUILD.

@@ -518,8 +518,6 @@ pub(crate) mod sizes {
pub(crate) const AHEAD_RECORDS: u32 = 64;
/// Policy bound on logical queues reported in QUEUE_LIMITS (not a firmware limit).
pub(crate) const MAX_QUEUES: u32 = 58;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing MAX_VMS changes the hw.rs hash pinned at g15-zero-words-tests.py:29; please refresh it with a note.

@scottjones scottjones left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Eryk, this is a nice cleanup, and it follows on well from your fixes in #202 and #214. We'd like the tests sorted before merge; the driver logic looks right to us.

Moving the ASID to publication and dropping the 63 VM cap makes sense, since only one VM is ever in flight. The stamp wrap logic also looks right to us. We built the asahi driver at the head and at the merge base (G15 only, G15 with M3_T8122, and M3_T8122 only) with W=1. There are no new errors, and the G15 builds have no new warnings.

Blocker: host tests are red. host tests (tools) fails in CI run 40253690456, and we see the same thing locally. All three pass on the merge base:

  • tools/asahi/g15-input-error-tests.py: the stub Job has no release_binding(), so the control build fails.
  • tools/asahi/g15-pb-growth-tests.py: the stub Job has no slot() and the stub VmState has no id.
  • tools/asahi/g15-zero-words-tests.py: the hw.rs golden hash changed because MAX_VMS was removed. The hash in the test needs a deliberate refresh.

Nothing on the host tests the binding lifecycle. The new stamp tests are good. We mutated stamp_reached, stamp_seed, pending_stamp and the cwait check, and each mutation made a test fail. The slot changes are different. We made each of these changes one at a time, and all six G15 suites that still build passed every time:

  • removed job.release_binding() in retire_oldest
  • made Job::finish release the binding on engine failures too
  • removed slot_wait = true on ENOSPC

The second one is the dangerous one: it hands a failed job's ASID to another VM. A small host test around Job::bind/release_binding and the retire/finish rules would catch it. Even one that only pins "a failed engine completion keeps its binding" would do. Your hardware runs are convincing, but nothing guards this on the host.

Two questions we couldn't settle by reading, so they rest on your hardware runs. Could you confirm them?

  • Stamp 0: the firmware doesn't treat a stamp of 0 specially? The old code avoided it on purpose.
  • Per-context firmware state: now that a slot moves between VMs after every job, does any per-context state outlive a flight's retirement? We're thinking of InitBM's ctx and the PB growth requests keyed by it (pools.rs:910-916). Before, a VM kept its slot for life.

Just so you know: on ENOSPC the publish loop stops at the picked job, so jobs whose VM already holds a slot wait behind it (engine.rs:586). It only happens under exhaustion and doesn't last long, so it's fine as is.

Small one: with M3_T8122=y and G15=n, Uat::try_bind is unused and W=1 warns that it's never used. A #[cfg] or #[allow(dead_code)] would keep G15-off builds clean.

@maralcbr maralcbr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 5782625 (source only; no build or hardware on my side).

The direction is good: binding ASIDs at publication removes the 63-live-VM cap, and making absent waits and progress Options instead of overloading 0 is the right way to let stamps wrap through zero. The CTS result and the wrap stress run are strong evidence. Before merge:

1. CI is red (host tests (tools), run 40253690456). Three G15 host suites fail:

  • g15-input-error-tests.py: the extracted model of finish() now calls Job::release_binding(), which the model's Job doesn't have.
  • g15-pb-growth-tests.py: the extracted code now uses Job::slot() and VmState::id, missing from the model.
  • g15-zero-words-tests.py: the whole-file golden hash of hw.rs is stale, since MAX_VMS was removed.
    These are broken host checks, not a production build failure, but they need updating with the refactor.

2. Build evidence for G15. The ci / compile job annotates all ten changed G15 Rust files as "not compiled by this job". Your m3plasma-5782625 kernel implies it builds, but please attach a build of this head with G15 enabled (or point at the job that compiles it).

3. Zero stamp at the barriers. At wrap, encode::barrier() sends a literal WAIT = 0 to the firmware, because barriers are emitted by predecessor type, not by value. If the firmware treated 0 as "no wait", ordering would quietly loosen exactly at the wrap, and CTS or throughput runs would rarely catch it. The first zero targets fall at about ordinal 4,128,768 (compute), 8,781,824 (TA) and 12,779,520 (FS). Did the 13M-pair run check dependent output across those transitions, for TA→FS, FS→compute and compute→compute, with plain Stamp barriers rather than only Hold (which also holds the predecessor and could mask a bad stamp comparison)?

4. A job resuming on a different ASID. The code looks sound: bindings are refcounted per VM, so a live flight keeps its slot. But ordinary split-job runs probably keep getting the preferred slot back. An instrumented split-job test that forces one job to resume on a different ASID number, and checks its output, would close this.

5. Doc nit. vm.rs (and the uat.rs comment) say failed published jobs keep their binding until reboot. In the code, a recovered hang completes the job and releases the binding once its references drop; only fail-stop retains it. Worth spelling out, since it matters for slot exhaustion.

Include zero in the firmware stamp cycle. Keep absent dependencies and completed progress separate from wrapped zero stamps.

Generated-by: LLM
Signed-off-by: Eryk Wieliczko <eryk@wieliczko.ninja>
Idle VMs must not reserve hardware ASIDs. Bind without waiting at publication, release at successful retirement, and retain failed published jobs. Logical VM creation is limited by memory rather than slot count.

Generated-by: LLM
Signed-off-by: Eryk Wieliczko <eryk@wieliczko.ninja>

@chriskearney chriskearney left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 8525dd8e380a708356ec1ffaef0bbdf266cb328b against 937b727ef9a3a9ff09489d597ef4b28f3b6fdbfa.

Requesting changes because the current head still breaks three host suites. I checked the actual tools-job log, which identifies merge b62f22a3 of this head into the target:

  • g15-input-error-tests.py: the control is NOBUILD because the mock Job lacks release_binding().
  • g15-pb-growth-tests.py: the mock Job::slot() and VmState::id are missing.
  • g15-zero-words-tests.py: the hw.rs golden hash needs the deliberate update for removal of MAX_VMS.

All three exit 1 where the runner expects 0. The current patch contains no tools/asahi updates, so the earlier reviewer findings remain current after the rebase. Please update the mocks/hash and get the tools job green. Preserve real runtime mutation checks; a control that fails compilation is not a successful negative test. The binding-retention-on-engine-failure coverage requested in the existing review also remains relevant.

The production changes look coherent in our bounded source review: absent progress is distinct from stamp zero, publication acquires the binding, and retirement releases it only after published work retires. Our four CPU stamp-helper controls passed. We also completed native-provider benchmarks and normal returns on J516S/J293/J313 with the combined #233+#238 and Mesa #17 stack; the detailed table is in our #238 review. M3 Quake measured 186.5 → 658.7 FPS, while the two M1 Quake comparisons were roughly flat. That is positive combined-stack evidence, not an isolated attribution to this PR or hardware proof of stamp-wrap/binding-pressure correctness. Existing OMT/log warnings and incomplete CTS qualification are retained in that report.

The benchmark result does not override the current CI failure or resolve the first-attempt wrong-draw evidence documented by the author.

malik-na added a commit to malik-na/linux-aurora that referenced this pull request Oct 27, 2026
…cleanly; 58-queue cap found

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@malik-na

Copy link
Copy Markdown
Member

Tested on J516C (M3 Max, T6031 / G15C): head 8525dd8e380a, the running G15 config unchanged, Mesa aurora-silicon/mesa aurora-wip 4911479 (not omacom/mesa#17). Firmware READY, display and desktop come up, no G15 warnings, faults or recoveries during the runs below.

Stamp wrap: pass. 4,300,032 compute command buffers (64 per submit, every result checked) took the engine's compute count from 21,359 to 4,321,391, past the wrap at 4,128,768 (0xc1000000 + n × 0x100). All results correct, recoveries 0 hang_kills 0, no kernel lines. TA/FS wraps and mixed render/compute across a wrap were not covered.

VM slots: pass. N processes each open a Vulkan device, all wait until every device is open, then each runs 50 checked submits.

Kernel N Open at once Pass
integration/aurora 7b96998 100 23 23 (77 VM_CREATE ENOSPC)
this PR 100 100 53
this PR 200 200 53

The PR-side failures are not ASIDs: each failing process gets DRM_IOCTL_ASAHI_QUEUE_CREATE = ENOMEM from the device-wide MAX_QUEUES = 58 limit, which Mesa reports as VK_ERROR_DEVICE_LOST at the first submit. Not part of this PR, but two notes:

  • With 58 queues below 63 ASIDs, normal clients can no longer exhaust ASIDs, so the try_bind ENOSPC → EAGAIN retry is not exercised this way.
  • In publish_prefix_into the horizon member's j2.bind()? passes ENOSPC up without the EAGAIN mapping that publish_prefix applies to the leading job, and the group is then completed with that error. Unreachable while queues < ASIDs, but it could matter if the queue limit is raised.

Separately, ENOMEM for an advertised count limit is a surprising errno; happy to open an issue for it.

malik-na added a commit to malik-na/linux-aurora that referenced this pull request Oct 27, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chriskearney

Copy link
Copy Markdown
Collaborator

Superseded by #253, now merged as 213f627205d9fe64b2f41607ca21266a5bb8ee2f after Marcelo's approval and passing PR and merge-queue CI.

Eryk's d82144ad and 8525dd8e are carried with their original authorship, sign-offs, provenance and unchanged stable patch IDs. The follow-ups repair the stale host fixtures and add binding/PMP error controls; this branch and its hardware evidence remain here for reference. Mixed-workload stamp-wrap and different-ASID resume coverage remain nonblocking follow-ups, as recorded in Marcelo's carry review. Closing this original PR to avoid merging the same changes twice. Thanks, Eryk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants