[scottjones]

Summary

These are the two fixes Marcelo asked for in #187, which need to land in or before the M3 Air switch-on (#230):

 Runtime::new
   require_stopped, RTKit state and transport
+  claim the once-per-boot latch, or refuse with EBUSY   # pins the module
   start_asc, adopt_uat, ...

 publish / publish_mixed
   retain the batch's packets
+  mark them "may have been published"
   append to the rings, doorbells

 Inner::fail_pending
   for each pending packet
+    release its job guard if no batch ever included it
     finish(Err)                                        # quarantines only if a guard is held

• One start per boot (61eccdbbee). A second start (unbind and rebind, a module reload, a retry after a failed start) would set RUN while TTBAT context 0 still names the freed lower root of the first one. Since stop_asc only clears RUN, we refuse it instead of trying to make a restart work. The latch is claimed just before the first ASC start, and claiming it takes a module reference that is never dropped, so a reload can't forget it. An unbind and rebind now ends in "M3: the GPU firmware already started once this boot and can't start again; reboot to start the GPU".
• Published packets only (6246daaee9). Each packet gets a flag that is set before any producer-state change of a batch that includes it, so a partial ring append counts. fail_pending releases the guard only when the flag is unset. ECANCELED still quarantines, as you asked.

This also covers the Backend::stop completion drain: a completion work item only exists once the runtime has started, and by then the module is pinned, so it can't be unloaded right after an unbind.

Marcelo, would you rather have these as one PR or two? They're independent commits, so splitting is easy.

Evidence

• Host policy tests (tools/asahi/m3-batching-tests.rs, Rust 1.85.0 as in the CI manifest): 11 pass, including two new ones.
  • The latch is claimed once; every later claim fails; of 8 concurrent claims, exactly one wins.
  • After fail_active has drained the active batch, a partly published pending packet keeps its guard and a never-published one releases it.
• Driver build (LLVM 22.1.8, Rust 1.85.0, W=1), against the untouched base 19f4965180, M3 only and M3 + G15: 0 errors in both. M3 only has 0 Rust warnings, the same as the base. M3 + G15 has 28, the same 28 g15/ sites as the base, none in m3/.
• Still to do: a hardware run. Neither change has run on hardware yet. On the J613 we'll boot this branch, check the GPU desktop, then unbind and rebind the GPU, expecting the refusal line and no firmware crash, then reboot and check the GPU starts again. We'll post the result here.

Merge Danger

Door: two-way

Blast Radius: M3 Air runtime

Both changes are in m3/, built only with CONFIG_DRM_ASAHI_M3_T8122. After the first T8122 start, rmmod asahi is refused until reboot.

🤖 Generated with Claude Code

[chriskearney]

One startup lifetime fix is needed before merging 6246daa: the permanent reference must be acquired with try_module_get, failing closed, before the latch/start.

A first deferred, async or sysfs probe can run while the module is LIVE. The Rust platform probe wrapper and C bind/probe path do not take a module reference. An ordinary rmmod can therefore drop the base reference and commit GOING before this probe reaches the unconditional __module_get.

The relevant ordering is source-verifiable:

1. try_stop_module checks/releases the reference and marks GOING. The subsequent exit/free path does not recheck it.
2. __module_get unconditionally increments; it can resurrect the count after removal has been committed. Driver unregister drains the in-flight bind/probe, so the probe can finish starting ASC before being unbound and unloaded.
3. The next module load gets a fresh static latch and can start the firmware again against the stale adopted context. FORCE_UNLOAD=n does not protect this ordinary-unload case. The synchronous module-init probe is protected by COMING; the later first-probe paths are the issue.

Please take try_module_get(THIS_MODULE) before claiming, return without claiming/starting on failure, and module_put only the temporary reference when another start already claimed the latch. Keep the winning reference permanently. The kernel's module-reference documentation describes this removal check.

The packet-publication/guard ordering checks out, including partial append errors, active-list drain and ECANCELED quarantine. Validation: 11 production policy tests pass, independent source-order controls pass, and a small exhaustive source-derived unload model admits the bad interleaving as written and rejects it with the proposed pin ordering. That model is not a kernel/hardware reproduction. Native M3-enabled software builds also succeed; no T8122 hardware result is claimed.

CI coverage note: this PR's package config leaves M3_T8122 disabled, so its green package job excludes the changed runtime/submit paths. The separate M3-enabled builds supply that compilation coverage; the final #236+#230 production composition is building separately.

[chriskearney]

The unconditional reference can be acquired after ordinary delete_module has already committed GOING. A racing first deferred/sysfs/async probe can then start, finish while unregister drains it, and unload; reloading loses this static latch. Use a fallible live-module pin before claiming/start, release only the extra reference when claim fails, and otherwise retain it permanently. The packet flag side looks sound.

[chriskearney]

A minimal replacement for the claim/pin block, preserving the existing fail-closed placement:

        // A deferred, async or sysfs probe may race an ordinary module removal.
        // Pin before claiming so a reload cannot forget an already-started firmware.
        // SAFETY: THIS_MODULE names this module, or is null when built in.
        if !unsafe { kernel::bindings::try_module_get(crate::THIS_MODULE.as_ptr()) } {
            dev_err!(pdev.as_ref(), "M3: the driver is being removed; GPU not started\n");
            return Err(ENODEV);
        }
        if !FIRMWARE_START.claim() {
            // SAFETY: release only this attempt's reference; the first start keeps its own.
            unsafe { kernel::bindings::module_put(crate::THIS_MODULE.as_ptr()) };
            dev_err!(pdev.as_ref(), "M3: the GPU firmware already started once this boot and can't start again; reboot to start the GPU\n");
            return Err(EBUSY);
        }
        // Keep the winning reference permanently so the latch lasts until reboot.

The unload interleaving model rejects both bad schedules with this ordering. This proposed replacement has not yet been natively compiled; the completed builds apply to the original6246 source. No branch changes from me.

[scottjones]

Agreed, thanks for catching this. Fixed in 80958cd5a4. The reference is now taken with try_module_get(THIS_MODULE) before the latch is claimed, and nothing is claimed or started if removal has begun. A start that loses the claim drops only its own reference, and the winner keeps its reference, so an ordinary unload stays blocked.

The order lives in the batch policy (claim_start), so the host test covers it:

• a failed pin claims nothing;
• a lost claim gives back its reference;
• the winner keeps its reference.

Four mutations of that order each fail the test. The two commits you reviewed are unchanged.

[chriskearney]

Re-reviewed 80958cd: the requested unload-race fix is correct. try_module_get now precedes the latch claim; a failed pin claims nothing, a losing claim releases only its temporary reference, and the winner retains its pin. The two remaining __module_get calls are reached only after that winning reference is held. Packet publication and guard handling are unchanged from the earlier review.

All 12 production policy tests pass with warnings denied on Rust 1.85.0, 1.93.1 and 1.98.1. Five independent mutations of the pin/claim/release ordering are rejected by the new regression test. The source-derived unload interleaving model also rejects the previously identified double-start paths with this ordering; this is host/source evidence, not a hardware reproduction.

Approving the source change. The updated M3-enabled production composition with #230 is being compiled separately; the earlier 6246 build does not cover this revision. No T8122 hardware qualification is claimed.

[chriskearney]

The updated native production composition of #236 at 80958cd with #230 at fc53911 built successfully. The actual Image and headers configs match, with Rust/G15/T8122/16KiB and module unloading enabled. The AArch64 Asahi module contains the new removing-module refusal, the first-start latch and try_module_get/module_put; all 458 undefined symbols resolve against the shipped Module.symvers. The exact policy suite passes 12/12. Actual compiler versions are clang/LLD 18.1.8 and Rust 1.85.0.

This validates the corrected runtime compilation; it is not a T8122 hardware result. Nonfatal rustfmt and header-helper strip diagnostics remain, and DKMS/header-tool execution was not tested.

[scottjones]

Hardware result, as promised. We ran this on our 13" M3 Air (J613), on kernel 7.2.11 built from 80958cd (this PR's head, so the merged content) with G15 and M3_T8122 on, booted with asahi.t8122_start=1.

• Boot: the GPU came up as usual: firmware-running, then the first render job completed.
• Unbind and rebind: we stopped the desktop first, so nothing held the GPU's DRM nodes.
  • Unbinding the GPU stopped the runtime.
  • Binding it again was refused with "M3: the GPU firmware already started once this boot and can't start again; reboot to start the GPU", and the probe failed with -16. ASC never ran a second time.
  • There was no firmware crash, fault or oops.
• Reboot: after a reboot on the same kernel, the GPU started again as usual (firmware-running, then the first render job completed).

Three things we noticed along the way:

1. The refused probe still touched the hardware. Before reaching the latch, it took the PMP power vote and wrote the SGX setup register (SGX+0xd14000 = 0x70001). The latch is in Runtime::new, after Device::new. ASC never started, so this isn't the stale-root problem. Still, refusing before Device::new would leave the GPU untouched on a second probe. We can send that as a small follow-up.
2. rmmod asahi was refused, but it doesn't prove the pin. With the desktop gone the module refcount was 4, not 1, so other references keep it loaded too.
3. The unbind logged two teardown errors: "MMU: ASC cache flush ... failed (err: ENODEV)" and "Handoff: expected flush state 2, got 1". The unbind still completed. This was our first unbind of this runtime, so we can't yet say whether every unbind logs them.