From 61eccdbbee803c327221c126698d027e38f8c906 Mon Sep 17 00:00:00 2001 From: Scott Jones Date: Sun, 25 Oct 2026 20:36:35 -0400 Subject: [PATCH 1/3] drm/asahi: m3: start the GPU firmware once per boot A second start of the T8122 runtime in one boot (unbind and rebind, a module reload, or a retry after a failed start) hands the firmware freed tables. Runtime::drop and the failed-start arm free the adopted lower root once ASC has stopped, but TTBAT context 0 still names it, and the next start sets RUN before adopt_uat rewrites context 0. stop_asc only clears RUN, so there is no firmware state to restart from anyway. Refuse every start after the first one of the boot instead. The latch is claimed just before the first ASC start and is never released, and claiming it pins the module, so a reload can't forget it. An unbind and rebind now ends in a clean refusal rather than a second firmware boot, and a reboot is the only way to start the GPU again. Pinning the module also means it can't be unloaded right after an unbind while a completion work item is still queued, which was the reason for draining that work in Backend::stop. Reported-by: Marcelo Alcantara Generated-by: LLM Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Scott Jones --- drivers/gpu/drm/asahi/m3/m3_batch_policy.rs | 11 +++++++++++ drivers/gpu/drm/asahi/m3/m3_runtime.rs | 11 +++++++++++ tools/asahi/m3-batching-tests.rs | 13 +++++++++++++ 3 files changed, 35 insertions(+) diff --git a/drivers/gpu/drm/asahi/m3/m3_batch_policy.rs b/drivers/gpu/drm/asahi/m3/m3_batch_policy.rs index 851b4e79f421e6..e7a5d6ad3eecb0 100644 --- a/drivers/gpu/drm/asahi/m3/m3_batch_policy.rs +++ b/drivers/gpu/drm/asahi/m3/m3_batch_policy.rs @@ -87,6 +87,17 @@ pub(crate) const fn retain_runtime(stop_failed: bool, gpu_pending: bool) -> bool stop_failed || gpu_pending } +/// One firmware start per boot. `stop_asc` only clears RUN: it doesn't reset the firmware or +/// give back a reusable RTKit or InitData state, and TTBAT context 0 still names the lower root +/// the first start adopted, which is freed once ASC has stopped. Claimed just before the first +/// ASC start and never released. +pub(crate) struct StartLatch(core::sync::atomic::AtomicBool); +impl StartLatch { + pub(crate) const fn new() -> Self { Self(core::sync::atomic::AtomicBool::new(false)) } + /// True for the first claim only. + pub(crate) fn claim(&self) -> bool { !self.0.swap(true, core::sync::atomic::Ordering::AcqRel) } +} + #[derive(Default)] pub(crate) struct PassCosts { recent: [Option<(u64, u64)>; 8] } impl PassCosts { diff --git a/drivers/gpu/drm/asahi/m3/m3_runtime.rs b/drivers/gpu/drm/asahi/m3/m3_runtime.rs index 28bbea3c86889b..234678e9aa2cd1 100644 --- a/drivers/gpu/drm/asahi/m3/m3_runtime.rs +++ b/drivers/gpu/drm/asahi/m3/m3_runtime.rs @@ -974,6 +974,8 @@ impl Inner { // Registration and Mutex initialization otherwise duplicates it on the bounded // kernel stack. ManuallyDrop retains the whole allocation on failed ASC stop. pub(crate) struct Runtime { inner: ManuallyDrop> } +/// Claimed before this boot's first ASC start. +static FIRMWARE_START: policy::StartLatch = policy::StartLatch::new(); impl Runtime { #[inline(never)] pub(crate) fn new(pdev: &platform::Device>, drm: &driver::AsahiDrmDevice, device: Device, contents: crate::m3::m3_adt_config::Contents) -> Result { @@ -995,6 +997,15 @@ impl Runtime { if crate::m3::m3_adt_config::stop_before_asc(pdev.as_ref()) { return Err(ENODEV); } + // 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 root of this one. Unloading + // is blocked once the latch is claimed, so a reload can't forget it. + if !FIRMWARE_START.claim() { + 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); + } + // SAFETY: never released: the latch must outlive this instance of the module. + unsafe { kernel::bindings::__module_get(crate::THIS_MODULE.as_ptr()) }; // Whether the coprocessor runs and offers its endpoints, for the T8122 verdict. let mut started=false; // The adopted UAT and its window live outside the closure: once TTBAT context 0 names diff --git a/tools/asahi/m3-batching-tests.rs b/tools/asahi/m3-batching-tests.rs index a6eb52bbf4fe7c..f7db83f75862ce 100644 --- a/tools/asahi/m3-batching-tests.rs +++ b/tools/asahi/m3-batching-tests.rs @@ -163,3 +163,16 @@ fn experimental_air_is_active_while_pro_retains_serial_default() { assert!(default_depth(true)<=SLOTS); assert_eq!(MAX_PACKETS,8); } + +#[test] +fn firmware_starts_once_per_boot() { + let latch=StartLatch::new(); + assert!(latch.claim()); + // Unbind and rebind, a reload or a retry: every later start is refused. + for _ in 0..3 {assert!(!latch.claim());} + // Concurrent probes: exactly one claims it. + let latch=std::sync::Arc::new(StartLatch::new()); + let claims:usize=(0..8).map(|_|{let l=latch.clone();std::thread::spawn(move||l.claim())}) + .collect::>().into_iter().map(|t|usize::from(t.join().unwrap())).sum(); + assert_eq!(claims,1); +} From 6246daaee9d2cf286995edea7f59c642113a9b76 Mon Sep 17 00:00:00 2001 From: Scott Jones Date: Sun, 25 Oct 2026 20:37:29 -0400 Subject: [PATCH 2/3] drm/asahi: m3: don't quarantine VMs for packets the firmware never saw Since a failed job's VM is quarantined before its fence signals, Inner::fail_pending quarantines every pending packet's VM, including packets the firmware never saw: it finishes them without releasing their job guards. A check against the active batches can't tell them apart, because a pending packet can be partly published and fail_active drains the active batches before it calls fail_pending. Give each packet a flag that is set before the first firmware-visible change of any batch that includes it: before the producer state of a batch changes, so partial ring appends that overlapping firmware may consume count as published. fail_pending releases the guard only for a packet whose flag is unset. Everything else keeps quarantining as before, including on ECANCELED: cancellation only withdraws a packet's unpublished remainder, and its published commands may still be running. Reported-by: Marcelo Alcantara Generated-by: LLM Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Scott Jones --- drivers/gpu/drm/asahi/m3/m3_batch_policy.rs | 6 ++++++ drivers/gpu/drm/asahi/m3/m3_runtime.rs | 14 ++++++++++++-- drivers/gpu/drm/asahi/m3/m3_submit.rs | 9 ++++++++- tools/asahi/m3-batching-tests.rs | 14 ++++++++++++++ 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/asahi/m3/m3_batch_policy.rs b/drivers/gpu/drm/asahi/m3/m3_batch_policy.rs index e7a5d6ad3eecb0..0d8cf9056d44f1 100644 --- a/drivers/gpu/drm/asahi/m3/m3_batch_policy.rs +++ b/drivers/gpu/drm/asahi/m3/m3_batch_policy.rs @@ -83,6 +83,12 @@ pub(crate) const fn packet_retired(pending: bool, active: bool) -> bool { !pending && !active } +/// A failed packet that no batch ever included can drop its VM job guard: the firmware never saw +/// it. Any other keeps the guard, and its VM stays quarantined, until the runtime drops it. +pub(crate) const fn release_unpublished(may_be_published: bool) -> bool { + !may_be_published +} + pub(crate) const fn retain_runtime(stop_failed: bool, gpu_pending: bool) -> bool { stop_failed || gpu_pending } diff --git a/drivers/gpu/drm/asahi/m3/m3_runtime.rs b/drivers/gpu/drm/asahi/m3/m3_runtime.rs index 234678e9aa2cd1..aeb483cce866d5 100644 --- a/drivers/gpu/drm/asahi/m3/m3_runtime.rs +++ b/drivers/gpu/drm/asahi/m3/m3_runtime.rs @@ -169,9 +169,15 @@ impl Inner { } } impl Inner { - /// Fail every packet that was never published. Their fences signal with the error. + /// Fail every packet not yet fully published. Their fences signal with the error. One that no + /// batch ever included releases its VM job guard first, so its VM isn't quarantined for work + /// the firmware never saw. One that may have been published keeps it: a partly published + /// packet, or one whose batch `fail_active` has already drained from `active`. fn fail_pending(&mut self, error: Error) { - for (packet,_) in self.pending.drain_all() {packet.finish(Err(error));} + for (packet,_) in self.pending.drain_all() { + if policy::release_unpublished(packet.may_be_published()) {packet.release_guard();} + packet.finish(Err(error)); + } } /// A published batch failed or never retired: latch the fault, log it, and fail its /// packets. The packets stay retained (with their VM job guards) until teardown, because @@ -374,6 +380,9 @@ impl Inner { for (p,_) in entries { if !self.packets.iter().any(|q| Arc::ptr_eq(q,p)) {self.packets.push(p.clone(),GFP_KERNEL).map_err(|_|early(ENOMEM))?;} } + // Before any producer state changes: an append the overlapping firmware consumes, or a + // partial publication, makes these packets visible to it. + for (p,_) in entries {p.mark_published();} let found=self.jobs.iter().position(|job| matches!((job,control), (NativeJob::Compute(_),crate::m3::m3_submit::Command::Compute(_)) | (NativeJob::Render(_),crate::m3::m3_submit::Command::Render{..}))); @@ -519,6 +528,7 @@ impl Inner { // Keep the packet before any producer mutation. No other batch is live. self.packets.clear(); self.packets.push(packet.clone(),GFP_KERNEL).map_err(|_|early(ENOMEM))?; + packet.mark_published(); let mut render_prepared=false; let mut compute_prepared=false; let prepared=(||->Result { diff --git a/drivers/gpu/drm/asahi/m3/m3_submit.rs b/drivers/gpu/drm/asahi/m3/m3_submit.rs index ca16e18f10fbf5..ffc57c77afcb20 100644 --- a/drivers/gpu/drm/asahi/m3/m3_submit.rs +++ b/drivers/gpu/drm/asahi/m3/m3_submit.rs @@ -43,6 +43,9 @@ pub(crate) struct Packet { pub(crate) wide_visibility:KVec, completion:UserFence, finish_claimed: AtomicBool, + /// Set before a batch with any of this packet's commands can change firmware-visible state, + /// and never cleared. + published: AtomicBool, /// Queue urgency (3 = UAPI realtime .. 0 = UAPI low): more urgent packets are published first. pub(crate) urgency:u8, /// Who submitted the packet, for the failure log only. @@ -78,6 +81,10 @@ impl Packet { } /// Called only after all published commands retired or before any publication. pub(crate) fn release_guard(&self) {drop(self.vm_job.lock().take());} + /// Called before the first firmware-visible change of a batch that includes this packet. + pub(crate) fn mark_published(&self) {self.published.store(true,Ordering::Relaxed);} + /// Whether the firmware may have seen any of this packet's commands. + pub(crate) fn may_be_published(&self)->bool {self.published.load(Ordering::Relaxed)} pub(crate) fn retired(&self) {self.release_guard();self.finish(Ok(()));} pub(crate) fn finish(&self,result:Result) { // Retirement, cancellation and a queued scheduler timeout can race. @@ -232,7 +239,7 @@ impl queue::Queue for Queue { parser.finish().map_err(|_|EINVAL)?; let packet=Arc::new(Packet{vm:self.vm.clone(),commands,timestamps,attachments:flushes,wide_visibility:visibility, completion:self.fences.new_fence(0,Completion)?.into(), - finish_claimed:AtomicBool::new(false),urgency:self.urgency,owner:Owner::current(), + finish_claimed:AtomicBool::new(false),published:AtomicBool::new(false),urgency:self.urgency,owner:Owner::current(), vm_job:KBox::pin_init(new_mutex!(Some(vm_job)),GFP_KERNEL)?},GFP_KERNEL)?; let mut job=self.entity.new_job(1,Job{shared:self.shared.clone(),packet})?; for sync in syncs.drain(0..in_sync_count) {if let Some(f)=sync.fence {job.add_dependency(f)?;}} diff --git a/tools/asahi/m3-batching-tests.rs b/tools/asahi/m3-batching-tests.rs index f7db83f75862ce..2812989e51f9e6 100644 --- a/tools/asahi/m3-batching-tests.rs +++ b/tools/asahi/m3-batching-tests.rs @@ -176,3 +176,17 @@ fn firmware_starts_once_per_boot() { .collect::>().into_iter().map(|t|usize::from(t.join().unwrap())).sum(); assert_eq!(claims,1); } + +#[test] +fn failed_pending_packets_quarantine_only_what_the_firmware_may_have_seen() { + // (in a batch fail_active drained, partly published and still pending, never in a batch) + struct Packet {published:bool,guard:bool} + let mut active=vec![Packet{published:true,guard:true}]; + let mut pending=vec![Packet{published:true,guard:true},Packet{published:false,guard:true}]; + // fail_active drains active before fail_pending: a check against active can't see these. + let drained:Vec<_>=active.drain(..).collect(); + for p in pending.iter_mut() {if release_unpublished(p.published) {p.guard=false;}} + assert!(drained[0].guard); + assert!(pending[0].guard,"a partly published packet keeps its guard"); + assert!(!pending[1].guard,"a never-published packet releases its guard"); +} From 80958cd5a495298a3e7119d0a4daa4f4d513298e Mon Sep 17 00:00:00 2001 From: Scott Jones Date: Sun, 25 Oct 2026 22:37:22 -0400 Subject: [PATCH 3/3] drm/asahi: m3: take the start pin with try_module_get The one-start-per-boot latch pinned the module with __module_get after claiming it. A first probe that isn't the module-init one (deferred, async or a sysfs bind) can run while an ordinary rmmod is under way: once try_stop_module has dropped the reference and marked the module GOING, __module_get revives the count unconditionally, the probe starts ASC, and the module still unloads. The next load then gets a fresh latch and starts the firmware a second time. Take the reference with try_module_get before claiming the latch, and start nothing when the module is already being removed. When another start already holds the latch, drop only the reference this probe just took; the winning claim keeps its reference, so an ordinary unload stays blocked. A forced unload, or a module init that fails after the probe started ASC, still frees the module regardless of references; neither is new here. The order lives in the batch policy (claim_start), so the host test checks it: a pin that fails claims nothing, a start that loses the claim gives back only its own reference, and the winner keeps its reference. Reported-by: Chris Kearney <303316+chriskearney@users.noreply.github.com> Generated-by: LLM Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Scott Jones --- drivers/gpu/drm/asahi/m3/m3_batch_policy.rs | 25 +++++++++++++ drivers/gpu/drm/asahi/m3/m3_runtime.rs | 39 +++++++++++++++++---- tools/asahi/m3-batching-tests.rs | 21 +++++++++++ 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/asahi/m3/m3_batch_policy.rs b/drivers/gpu/drm/asahi/m3/m3_batch_policy.rs index 0d8cf9056d44f1..27295660640ae4 100644 --- a/drivers/gpu/drm/asahi/m3/m3_batch_policy.rs +++ b/drivers/gpu/drm/asahi/m3/m3_batch_policy.rs @@ -104,6 +104,31 @@ impl StartLatch { pub(crate) fn claim(&self) -> bool { !self.0.swap(true, core::sync::atomic::Ordering::AcqRel) } } +/// Why a start was refused before ASC. +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum StartRefusal { + /// The module is being removed: nothing is claimed or started. + Removing, + /// This boot's start already happened: the extra module reference is dropped again. + AlreadyStarted, +} + +/// Pins the module, then claims `latch`. The pin comes first, so a probe racing an ordinary +/// rmmod fails closed instead of reviving the module's reference count. A start that loses the +/// claim drops only its own pin; the winner keeps its pin for good, so a reload can't forget the +/// latch. +pub(crate) fn claim_start(latch: &StartLatch, pin: impl FnOnce() -> bool, unpin: impl FnOnce()) + -> Result<(), StartRefusal> { + if !pin() { + return Err(StartRefusal::Removing); + } + if !latch.claim() { + unpin(); + return Err(StartRefusal::AlreadyStarted); + } + Ok(()) +} + #[derive(Default)] pub(crate) struct PassCosts { recent: [Option<(u64, u64)>; 8] } impl PassCosts { diff --git a/drivers/gpu/drm/asahi/m3/m3_runtime.rs b/drivers/gpu/drm/asahi/m3/m3_runtime.rs index aeb483cce866d5..1a366d7f4b3407 100644 --- a/drivers/gpu/drm/asahi/m3/m3_runtime.rs +++ b/drivers/gpu/drm/asahi/m3/m3_runtime.rs @@ -986,6 +986,24 @@ impl Inner { pub(crate) struct Runtime { inner: ManuallyDrop> } /// Claimed before this boot's first ASC start. static FIRMWARE_START: policy::StartLatch = policy::StartLatch::new(); +/// Takes a module reference, unless the module is already being removed. +#[cfg(CONFIG_MODULE_UNLOAD)] +fn pin_module() -> bool { + // SAFETY: THIS_MODULE is this module, whose code is running; try_module_get fails once + // removal has begun instead of reviving the reference count. + unsafe { kernel::bindings::try_module_get(crate::THIS_MODULE.as_ptr()) } +} +/// Without module unloading there is no removal to race. +#[cfg(not(CONFIG_MODULE_UNLOAD))] +fn pin_module() -> bool { true } +/// Drops a reference [`pin_module`] took. +#[cfg(CONFIG_MODULE_UNLOAD)] +fn unpin_module() { + // SAFETY: called only to drop a reference pin_module took. + unsafe { kernel::bindings::module_put(crate::THIS_MODULE.as_ptr()) }; +} +#[cfg(not(CONFIG_MODULE_UNLOAD))] +fn unpin_module() {} impl Runtime { #[inline(never)] pub(crate) fn new(pdev: &platform::Device>, drm: &driver::AsahiDrmDevice, device: Device, contents: crate::m3::m3_adt_config::Contents) -> Result { @@ -1008,14 +1026,21 @@ impl Runtime { return Err(ENODEV); } // 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 root of this one. Unloading - // is blocked once the latch is claimed, so a reload can't forget it. - if !FIRMWARE_START.claim() { - 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); + // would set RUN while TTBAT context 0 still names the freed root of this one. An ordinary + // (not forced) unload is blocked once the latch is claimed, so a reload can't forget it. + // The pin comes first: a later probe can race an ordinary rmmod, and once removal has + // begun nothing starts. + match policy::claim_start(&FIRMWARE_START, pin_module, unpin_module) { + Ok(()) => {} + Err(policy::StartRefusal::Removing) => { + dev_err!(pdev.as_ref(), "M3: the module is being removed; GPU not started\n"); + return Err(ENODEV); + } + Err(policy::StartRefusal::AlreadyStarted) => { + 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); + } } - // SAFETY: never released: the latch must outlive this instance of the module. - unsafe { kernel::bindings::__module_get(crate::THIS_MODULE.as_ptr()) }; // Whether the coprocessor runs and offers its endpoints, for the T8122 verdict. let mut started=false; // The adopted UAT and its window live outside the closure: once TTBAT context 0 names diff --git a/tools/asahi/m3-batching-tests.rs b/tools/asahi/m3-batching-tests.rs index 2812989e51f9e6..d9a0a35c04dd00 100644 --- a/tools/asahi/m3-batching-tests.rs +++ b/tools/asahi/m3-batching-tests.rs @@ -190,3 +190,24 @@ fn failed_pending_packets_quarantine_only_what_the_firmware_may_have_seen() { assert!(pending[0].guard,"a partly published packet keeps its guard"); assert!(!pending[1].guard,"a never-published packet releases its guard"); } + +#[test] +fn the_start_pins_the_module_before_claiming_and_fails_closed() { + use std::cell::Cell; + let latch=StartLatch::new(); + let refs=Cell::new(1i32); // the module's base reference + let pin_ok=||{refs.set(refs.get()+1);true}; + let unpin=||refs.set(refs.get()-1); + // rmmod has begun: no pin, no claim, no start, and the reference count is untouched. + assert_eq!(claim_start(&latch,||false,unpin),Err(StartRefusal::Removing)); + assert_eq!(refs.get(),1); + // The first live start wins and keeps its pin. + assert_eq!(claim_start(&latch,pin_ok,unpin),Ok(())); + assert_eq!(refs.get(),2); + // A later start (rebind, retry) loses the claim and drops only its own pin. + assert_eq!(claim_start(&latch,pin_ok,unpin),Err(StartRefusal::AlreadyStarted)); + assert_eq!(refs.get(),2); + // A later start racing rmmod still claims nothing. + assert_eq!(claim_start(&latch,||false,||panic!("nothing to unpin")),Err(StartRefusal::Removing)); + assert_eq!(refs.get(),2); +}