From 7a9ebf019036a7fae177e3cc626625e903f4332d Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Tue, 27 Oct 2026 03:27:37 -0400 Subject: [PATCH] drm/asahi: kill a G17 context at most once A closed context qualifies for a render-only kill while one of its pairs holds executed but unstamped commands and no compute queue still runs it, switches away from it or killed it. A context killed through its compute queue normally leaves that state at once, since the acknowledged kill releases the pair in the same service pass, but only that timing kept the render-only path from killing it again: the compute queue's own record of the kill ends when the queue is rebound. A second kill of a killed context is harmful. In a diagnostic run that held the pair's release back for 100 ms after the kill, 31 contexts were killed twice, 0.1 to 2.5 ms apart, and twice the firmware recovered the GPU 0.3 ms after acknowledging the second kill, on a page fault in the killed pair's own tiling queue. The 35 contexts killed once in that run caused no recovery. Record on the context that a kill was published, by either path, and never publish a render-only kill for a context that has one. --- drivers/gpu/drm/asahi/g17/context.rs | 14 ++++++++ .../gpu/drm/asahi/g17/runtime/compute_exit.rs | 35 +++++++++++++++---- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/asahi/g17/context.rs b/drivers/gpu/drm/asahi/g17/context.rs index ba74140d4f243..0b3bd97fb42fe 100644 --- a/drivers/gpu/drm/asahi/g17/context.rs +++ b/drivers/gpu/drm/asahi/g17/context.rs @@ -114,6 +114,9 @@ pub(crate) struct Context { scheduler: Arc, scheduler_generation: u32, published: AtomicBool, + /// A kill of this context was published, through a compute queue or on its render pairs' + /// behalf. A context is killed at most once. + kill_published: AtomicBool, /// Advisory only: publication still checks the physical binding under the /// device mutex. False skips an inline attempt known to need admission. compute_binding_hint: AtomicBool, @@ -197,6 +200,7 @@ impl Context { scheduler, scheduler_generation, published: AtomicBool::new(false), + kill_published: AtomicBool::new(false), compute_binding_hint: AtomicBool::new(false), render_seen: AtomicBool::new(false), qos, @@ -316,6 +320,16 @@ impl Context { Ok(head) } + /// Record that a kill of this context was published (see `kill_published`). + pub(crate) fn note_kill_published(&self) { + self.kill_published.store(true, Ordering::Release); + } + + /// A kill of this context was published. + pub(crate) fn kill_published(&self) -> bool { + self.kill_published.load(Ordering::Acquire) + } + pub(crate) fn is_published(&self) -> bool { self.published.load(Ordering::Acquire) } diff --git a/drivers/gpu/drm/asahi/g17/runtime/compute_exit.rs b/drivers/gpu/drm/asahi/g17/runtime/compute_exit.rs index cd7f2999d0312..41960369b04a3 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/compute_exit.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/compute_exit.rs @@ -52,6 +52,16 @@ fn render_kill_target(listed: u128, own: u128) -> Option { (candidates != 0).then(|| candidates.trailing_zeros() as u8) } +/// A closed context may be killed on its render pairs' behalf once it has been closed for +/// [`RENDER_KILL_DELAY_NS`], it was never killed, a pair of it holds executed but unstamped +/// commands and no compute queue still runs it, switches away from it or killed it. A second kill +/// of a killed context made the firmware fault on the killed pair's own tiling queue within a +/// millisecond of acknowledging it: with the pair's release held back, a context killed through +/// its compute queue still looked like a render-only candidate. +fn render_kill_eligible(age: u64, killed: bool, executed: bool, compute_clear: bool) -> bool { + age >= RENDER_KILL_DELAY_NS && !killed && executed && compute_clear +} + /// The one admission owner of every pair a kill holds, if there is one. fn hold_owner(owners: impl Iterator>) -> Option { let mut owner = None; @@ -296,6 +306,7 @@ impl Firmware { PRESSED_KILL_NS.store(now.max(1), Ordering::Relaxed); } queue.record_exit(cookie, now); + context.note_kill_published(); crate::g17::trace::teardown(3, cookie, u64::from(context.id()), hold | u64::from(beside)); fence(Ordering::SeqCst); if let Err(error) = queue.announce_exit(now, || self.primary.notify(MSG_CONTROL_NOTIFY)) { @@ -315,12 +326,14 @@ impl Firmware { /// yet those pairs are released only after one: without this the context, its pairs and /// its memory stay until the processors stop. fn render_kill_candidate(&self, index: usize, context: &Arc, age: u64) -> bool { - age >= RENDER_KILL_DELAY_NS - && !self.queues.teardown.render_kill_published(index) - && self.queues.render.executed_slots(context) != 0 - && self.queues.compute.iter().flatten().filter_map(|entry| entry.queue.as_deref()).all(|queue| { + render_kill_eligible( + age, + self.queues.teardown.render_kill_published(index) || context.kill_published(), + self.queues.render.executed_slots(context) != 0, + self.queues.compute.iter().flatten().filter_map(|entry| entry.queue.as_deref()).all(|queue| { queue.idle_for_context(context) && !queue.switching_from(context) && !queue.exit_published(context) - }) + }), + ) } /// Kill a render-only candidate (`render_kill_candidate`). The rules of a failed @@ -393,6 +406,7 @@ impl Firmware { // Publication is irrevocable. The held pairs stay held until this context's // scheduler release is consumed (`release_killed_slots` at detach). self.queues.teardown.record_render_kill(index, cookie, now)?; + context.note_kill_published(); record_kill_wait(self.queues.teardown.take_kill_wait(index, now)); if beside { *pressed = false; @@ -491,7 +505,16 @@ impl Firmware { #[kernel::macros::kunit_tests(asahi_g17_render_kill_target)] mod render_kill_target_tests { - use super::{hold_owner, render_kill_target}; + use super::{hold_owner, render_kill_eligible, render_kill_target, RENDER_KILL_DELAY_NS}; + + #[test] + fn a_killed_context_is_never_killed_again() { + assert!(render_kill_eligible(RENDER_KILL_DELAY_NS, false, true, true)); + assert!(!render_kill_eligible(RENDER_KILL_DELAY_NS, true, true, true)); + assert!(!render_kill_eligible(RENDER_KILL_DELAY_NS - 1, false, true, true)); + assert!(!render_kill_eligible(RENDER_KILL_DELAY_NS, false, false, true)); + assert!(!render_kill_eligible(RENDER_KILL_DELAY_NS, false, true, false)); + } #[test] fn a_render_kill_holds_pairs_of_one_owner_only() {