diff --git a/drivers/gpu/drm/asahi/g17/completion.rs b/drivers/gpu/drm/asahi/g17/completion.rs index 90f6d77780bff..4d3d5e5a7904d 100644 --- a/drivers/gpu/drm/asahi/g17/completion.rs +++ b/drivers/gpu/drm/asahi/g17/completion.rs @@ -547,7 +547,7 @@ impl Completion { /// A firmware restart listed this command in its recovery table: it was in /// flight, so it may have run partly. Its stamps and completion record may /// still look like success. Unless a replay republishes it from the start, - /// it fails and the failure is always reported to the client. + /// it fails: its fence carries ECANCELED, and only a guilty VM is poisoned. pub(crate) fn note_interrupted(&self) { self.interrupted.store(true, Ordering::Release); } @@ -563,12 +563,13 @@ impl Completion { (error != ECANCELED || interrupted) && !self.spared() && !(interrupted && !self.guilty()) } - /// ECANCELED stays silent only for work that was never interrupted (closed - /// or cancelled by its owner); every other failure reaches the client. + /// A non-poisoning ECANCELED or ENODATA (work a recovery discarded or dropped) stays out of + /// the VM status, as the UAPI documents for work another VM's recovery discarded: + /// Honeykrisp reads any status error as a lost device. Every other failure reaches the client. fn report(&self, error: Error) { if self.poisons(error) { self.status.record(error); - } else if error != ECANCELED || self.interrupted() { + } else if !discarded(error) { self.status.report_failure(error); } } @@ -715,7 +716,7 @@ impl Completion { guard.quarantine(); } if self.spared() { - if error != ECANCELED || self.interrupted() { + if !discarded(error) { self.status.report_failure(error); } } else { @@ -726,6 +727,11 @@ impl Completion { } } +/// The errors of work a recovery discarded (ECANCELED) or dropped before it ran (ENODATA). +fn discarded(error: Error) -> bool { + error == ECANCELED || error == ENODATA +} + /// Collected under the device mutex and executed only after releasing it. /// Runtime queues establish the ownership witness before constructing `Retired` /// or `Release`; `FailedOwned` leaves their physical and VM ownership intact. diff --git a/drivers/gpu/drm/asahi/g17/queue/render.rs b/drivers/gpu/drm/asahi/g17/queue/render.rs index 2436cfd33ed35..b387d294cda5a 100644 --- a/drivers/gpu/drm/asahi/g17/queue/render.rs +++ b/drivers/gpu/drm/asahi/g17/queue/render.rs @@ -1297,6 +1297,30 @@ impl Pair { Some(observation::snapshot(&self.memory, ordinal).ok()?.pass[0] != 0) } + /// The oldest published group has neither its tiler start nor its fragment end written. + pub(crate) fn front_untouched(&self) -> Option { + let front = self.active.iter().find(|active| active.outer_published)?; + Some(observation::snapshot(&self.memory, front.ticket.ordinal).ok()?.untouched()) + } + + /// The owner of a published group whose tiler started and whose fragment end is unwritten. + /// Groups an earlier recovery listed or blamed are skipped: a restart drops started groups, + /// so their records stay half-written without anything running. + pub(crate) fn running_owner(&self) -> Option<&Arc> { + self.active + .iter() + .filter(|active| { + active.outer_published + && !active.packet.completion.interrupted() + && !active.packet.completion.blamed() + }) + .find(|active| { + observation::snapshot(&self.memory, active.ticket.ordinal) + .is_ok_and(|snapshot| snapshot.pass[0] != 0 && snapshot.pass[1] == 0) + }) + .map(|active| active.packet.completion.status()) + } + /// Firmware is halted and all graph borrowers have joined before this copy. pub(crate) fn publish_recovery_targets(&self) -> Result { self.memory.graph.publish_recovery_targets() @@ -1611,13 +1635,10 @@ impl Pair { let packet = front.packet.clone(); failing = failing || !packet.completion.begin_replay(); if failing { - if !packet.completion.spared() { - packet.completion.status().record(EIO); - if self.terminal.is_none() { - self.terminal = Some((packet.completion.status().clone(), EIO)); - } - } - defer(Deferred::Retired(packet.completion.clone(), Err(EIO)))?; + // decide() replays only innocent groups that never ran: like `fail_dropped`, + // they fail without poisoning their VM. Restarts other VMs caused dropped them. + packet.completion.classify(recovery::Class::Spared); + defer(Deferred::Retired(packet.completion.clone(), Err(ENODATA)))?; self.memory.retire(ordinal)?; self.active.pop().ok_or(EIO)?; continue; @@ -1634,8 +1655,8 @@ impl Pair { /// Firmware retired every group of the pair after a restart, but some were dropped /// unexecuted and cannot run again in order (see `replay::decide`). Every group fails now, /// the ones that ran too: they consumed output the dropped ones never wrote. The work is - /// innocent, so it fails without poisoning its VM, with ENODATA, which its owner sees, - /// instead of waiting for the render timeout to fail it with ETIMEDOUT. + /// innocent, so it fails without poisoning its VM: its fences carry ENODATA, which stays out + /// of the VM status, instead of waiting for the render timeout to fail it with ETIMEDOUT. fn fail_dropped( &mut self, host: &mut impl Host, @@ -1672,9 +1693,10 @@ impl Pair { /// A front group a restart interrupted after its tiler started, which firmware /// then retired without writing its fragment end, may have run partly. It is - /// not replayed (blending or storage writes could apply twice) and fails now, - /// reported to its owner, instead of waiting out the render timeout. Returns - /// None when the front is not such a group. + /// not replayed (blending or storage writes could apply twice) and fails now + /// instead of waiting out the render timeout: its fence carries ECANCELED, and + /// only a guilty VM is poisoned. Returns None when the front is not such a + /// group. fn settle_interrupted_front( &mut self, host: &mut impl Host, diff --git a/drivers/gpu/drm/asahi/g17/recovery.rs b/drivers/gpu/drm/asahi/g17/recovery.rs index ca8da8b03545f..d42c0e8f37efb 100644 --- a/drivers/gpu/drm/asahi/g17/recovery.rs +++ b/drivers/gpu/drm/asahi/g17/recovery.rs @@ -45,6 +45,8 @@ pub(super) struct Entries { /// Listed commands found by their exact stamp, and by the oldest-work fallback. pub(super) exact: u32, pub(super) fallback: u32, + /// QIDs with a listed command found by its exact stamp: that command was dropped. + exact_qids: u128, } impl Entries { @@ -92,6 +94,12 @@ pub(super) enum Class { pub(super) trait Queues { fn render_scheduler_active(&self) -> bool; fn render_pass_started(&self, qid: u8, stamp: u64) -> Option; + /// Read while firmware is halted, like `render_pass_started`. The QIDs of every pair of a VM + /// an earlier recovery found guilty with a render that started and did not finish. Without + /// one, and only when the oldest published render of the pair holding `blamed` had not + /// started, those of the one VM with such a render. None if no such VM, several, or any + /// other VM holds compute work, which these records cannot see. + fn render_culprit(&self, _blamed: u8) -> Option { None } /// Copy every retained queue's live producer to its recovery target, then /// issue a full barrier. A failed graph must not skip subsequent queues. fn publish_recovery_targets(&self) -> Option; @@ -144,6 +152,8 @@ pub(super) struct State { pub(super) struct ServiceResult { pub(super) scan_error: Option, pub(super) target_error: Option, + /// Every VM found guilty had failed before, and the restart dropped one of its commands. + pub(super) leftover: bool, } impl State { @@ -159,6 +169,9 @@ impl State { self.timeouts.note_unless_progress(now, safety::retired()) } pub(super) fn restart_limit(&mut self, now: u64) -> bool { self.restarts.note(now) } + /// A restart that only dropped leftover work of failed VMs (`ServiceResult::leftover`) + /// does not count toward the storm limit. Returns whether it was forgiven. + pub(super) fn forgive_restart(&mut self) -> bool { self.restarts.forgive() } pub(super) fn generation(&self) -> u64 { self.accepted } @@ -252,6 +265,11 @@ pub(super) fn service( // acknowledged; sampling them during classification can blame a bystander. let attribution = sources.attribute(request.blamed, |qid, stamp| host.render_pass_started(qid, stamp)); + let attribution = sources.host_fallback(attribution, + || host.render_culprit(request.blamed?)); + if let Attribution::Queues(qids) = attribution { + pr_warn!("G17: recovery gen {} blames queues {:#x}\n", request.generation, qids); + } let target_error = host.publish_recovery_targets(); host.transition(1, 2)?; let mut recovered = false; @@ -282,7 +300,7 @@ pub(super) fn service( host.recovery_mut().pending = None; let (listed, exact, fallback) = (entries.listed, entries.exact, entries.fallback); - let (cancelled, blame) = classify(host, entries, request.blamed, attribution); + let (cancelled, blame, leftover) = classify(host, entries, request.blamed, attribution); if listed != 0 { pr_warn!( "G17: GPU recovery: {} commands were in flight ({} matched by stamp, {} by queue); they fail instead of completing\n", @@ -307,7 +325,7 @@ pub(super) fn service( } let scan_error = host.rescan_compute().err(); host.release_retired_compute(); - Ok(ServiceResult { scan_error, target_error }) + Ok(ServiceResult { scan_error, target_error, leftover }) } /// Walks the packed recovery table up to its first invalid record and marks @@ -321,8 +339,12 @@ fn read_entries(host: &mut H) -> Entries { if !entries.include(flags) { break; } - match host.note_interrupted(((flags >> 4) & 0x7f) as u8, stamp) { - Interrupted::Exact => entries.exact += 1, + let qid = ((flags >> 4) & 0x7f) as u8; + match host.note_interrupted(qid, stamp) { + Interrupted::Exact => { + entries.exact += 1; + entries.exact_qids |= 1u128 << qid; + } Interrupted::Oldest => entries.fallback += 1, Interrupted::None => {} } @@ -336,7 +358,9 @@ fn read_entries(host: &mut H) -> Entries { /// Also returns whether the firmware's blamed QID is still guilty after attribution: a blamed /// render that had not started while others had (or with no culprit at all) is innocent, and its /// work is spared like that of every other innocent VM instead of failing with EIO. -fn classify(queues: &mut Q, entries: Entries, blamed: Option, attribution: Attribution) -> (u128, bool) { +/// Last, whether every guilty VM had failed before this restart and it dropped one of their +/// commands: a failed VM publishes nothing new, so such restarts end with its queued work. +fn classify(queues: &mut Q, entries: Entries, blamed: Option, attribution: Attribution) -> (u128, bool, bool) { let mut guilty_qids = entries.guilty | blamed.map_or(0, |qid| 1u128 << qid); let cancelled = entries.cancelled; match attribution { @@ -347,7 +371,7 @@ fn classify(queues: &mut Q, entries: Entries, blamed: Option, att let no_culprit = attribution == Attribution::NoCulprit; let blame = blamed.is_some_and(|qid| guilty_qids & (1u128 << qid) != 0); if guilty_qids == 0 && !no_culprit { - return (0, blame); + return (0, blame, false); } let mut guilty: [Option>; MAX_GUILTY_VMS] = Default::default(); let mut count = 0; @@ -383,8 +407,15 @@ fn classify(queues: &mut Q, entries: Entries, blamed: Option, att } } if unresolved || (count == 0 && collateral == 0 && !no_culprit) { - return (0, blame); + return (0, blame, false); } + let known = |status: &Arc| guilty[..count].iter().flatten().any(|known| Arc::ptr_eq(known, status)); + let mut dropped = false; + for qid in (0..QIDS).filter(|qid| entries.exact_qids & (1u128 << qid) != 0) { + queues.visit_owners(qid, &mut |status| dropped |= known(status)); + } + let leftover = count != 0 && dropped && guilty[..count].iter().flatten().all(|status| status.get() != 0); + guilty[..count].iter().flatten().for_each(|status| status.note_recovery_guilty()); let spared = queues.classify_work(&|status| { guilty[..count] .iter() @@ -394,9 +425,11 @@ fn classify(queues: &mut Q, entries: Entries, blamed: Option, att .filter(|known| Arc::ptr_eq(known, status)) .map(|_| Class::Guilty) }) - .unwrap_or(Class::Spared) + // A VM an earlier recovery found guilty stays guilty. Spared, its leftover work would + // make the next blame on it collateral, and that restart would count as a storm. + .unwrap_or(if status.recovery_guilty() { Class::Guilty } else { Class::Spared }) }); - (cancelled & !(guilty_qids & !collateral) & spared, blame) + (cancelled & !(guilty_qids & !collateral) & spared, blame, leftover) } @@ -487,6 +520,68 @@ mod tests { assert!(!host.blamed); } + /// Two VMs, each owning one render pair: `a` queues 40/41, `b` queues 60/61. + struct Owners([(u8, Arc); 4], [Option; 4]); + + impl Queues for Owners { + fn render_scheduler_active(&self) -> bool { true } + fn render_pass_started(&self, _qid: u8, _stamp: u64) -> Option { None } + fn publish_recovery_targets(&self) -> Option { None } + fn oldest_spared(&self, qid: u8) -> Option { + self.0.iter().any(|(own, _)| *own == qid).then_some(false) + } + fn visit_owners(&self, qid: u8, visit: &mut dyn FnMut(&Arc)) { + self.0.iter().filter(|(own, _)| *own == qid).for_each(|(_, status)| visit(status)); + } + fn classify_work(&mut self, class: &dyn Fn(&Arc) -> Class) -> u128 { + for (index, (_, status)) in self.0.iter().enumerate() { + self.1[index] = Some(class(status)); + } + 0 + } + fn note_interrupted(&mut self, _qid: u8, _stamp: u32) -> Interrupted { Interrupted::None } + fn settle_blamed_render(&mut self, _qid: u8, _error: Error) {} + fn settle_cancelled_renders(&mut self, _qids: u128) {} + fn rescan_compute(&mut self) -> Result { Ok(()) } + fn release_retired_compute(&mut self) {} + } + + fn owners() -> Owners { + let a = Arc::pin_init(VmStatus::new(), GFP_KERNEL).unwrap(); + let b = Arc::pin_init(VmStatus::new(), GFP_KERNEL).unwrap(); + Owners([(40, a.clone()), (41, a), (60, b.clone()), (61, b)], [None; 4]) + } + + fn leftover(queues: &mut Owners, exact_qid: u8) -> bool { + let entries = Entries { exact_qids: 1u128 << exact_qid, ..Entries::default() }; + classify(queues, entries, Some(40), Attribution::Queues(3u128 << 60)).2 + } + + #[test] + fn only_dropped_work_of_an_already_failed_vm_is_leftover() { + let mut queues = owners(); + // A first failure counts toward the storm. + assert!(!leftover(&mut queues, 60)); + queues.0[2].1.record(EIO); + assert!(leftover(&mut queues, 60)); + // The restart dropped nothing of the failed VM. + assert!(!leftover(&mut queues, 40)); + } + + #[test] + fn a_guilty_vm_is_not_spared_by_another_vms_recovery() { + let mut queues = owners(); + let entries = || Entries { exact_qids: 1u128 << 40, ..Entries::default() }; + // b failing for its own client does not make it guilty in a's recovery. + queues.0[2].1.record(EIO); + classify(&mut queues, entries(), Some(40), Attribution::Queues(3u128 << 40)); + assert_eq!(queues.1, [Some(Class::Guilty), Some(Class::Guilty), Some(Class::Spared), Some(Class::Spared)]); + // Once a recovery finds b guilty, a later recovery that blames a keeps b guilty. + classify(&mut queues, entries(), Some(60), Attribution::Queues(3u128 << 60)); + classify(&mut queues, entries(), Some(40), Attribution::Queues(3u128 << 40)); + assert_eq!(queues.1, [Some(Class::Guilty); 4]); + } + #[test] fn unknown_25g83_keys_still_preserve_firmware_blame() { let mut host = Halted::new(false); diff --git a/drivers/gpu/drm/asahi/g17/recovery/attribution.rs b/drivers/gpu/drm/asahi/g17/recovery/attribution.rs index 38ad8612a0b99..fdf0cd62cbd07 100644 --- a/drivers/gpu/drm/asahi/g17/recovery/attribution.rs +++ b/drivers/gpu/drm/asahi/g17/recovery/attribution.rs @@ -122,6 +122,21 @@ impl Sources { Attribution::Queues(guilty) } } + + /// When slot identity cannot check a timeout's blame (25G83 reads every key as 0, and a + /// single-slot mask has no peer), the host's own pass records still can: a blamed render + /// that never started was starved, not hung. `culprit` names the queues of the one VM whose + /// render started and did not finish; without one the firmware's blame stands. + pub(super) fn host_fallback( + &self, + attribution: Attribution, + culprit: impl FnOnce() -> Option, + ) -> Attribution { + if attribution != Attribution::Firmware || self.reason.is_none_or(|reason| reason == PAGE_FAULT) { + return attribution; + } + culprit().map_or(Attribution::Firmware, Attribution::Queues) + } } #[kernel::macros::kunit_tests(asahi_g17_recovery_sources)] @@ -173,4 +188,28 @@ mod tests { assert_eq!(sources.attribute(Some(38), |qid, _| Some(qid == 39)), Attribution::Firmware); } + + #[test] + fn host_records_move_a_keyless_timeout_blame_to_the_running_vm() { + let mut sources = Sources::sample(0b10, |_| Some((0, 2))); + sources.reason = Some(0); + let attribution = sources.attribute(Some(36), |_, _| panic!("unknown key")); + assert_eq!(attribution, Attribution::Firmware); + assert_eq!(sources.host_fallback(attribution, || Some(3 << 60)), Attribution::Queues(3 << 60)); + assert_eq!(sources.host_fallback(attribution, || None), Attribution::Firmware); + } + + #[test] + fn host_records_never_override_a_page_fault_or_slot_evidence() { + let mut sources = Sources::sample(0b10, |_| Some((0, 2))); + sources.reason = Some(PAGE_FAULT); + assert_eq!(sources.host_fallback(Attribution::Firmware, || panic!("page fault")), + Attribution::Firmware); + sources.reason = None; + assert_eq!(sources.host_fallback(Attribution::Firmware, || panic!("no reason")), + Attribution::Firmware); + sources.reason = Some(0); + assert_eq!(sources.host_fallback(Attribution::NoCulprit, || panic!("slot evidence")), + Attribution::NoCulprit); + } } diff --git a/drivers/gpu/drm/asahi/g17/recovery/safety.rs b/drivers/gpu/drm/asahi/g17/recovery/safety.rs index d71d3cc81553f..5c2ad346c9890 100644 --- a/drivers/gpu/drm/asahi/g17/recovery/safety.rs +++ b/drivers/gpu/drm/asahi/g17/recovery/safety.rs @@ -6,6 +6,9 @@ use core::sync::atomic::{AtomicU64, Ordering}; const WINDOW_NS: u64 = 30_000_000_000; const LIMIT: u8 = 8; +/// Restarts forgiven per window: one per group a render pair holds. Leftover work of a failed +/// VM ends on its own, but an unknown firmware loop must still reach the limit. +const FORGIVE_LIMIT: u8 = crate::hw::t8140::queues::RENDER_DEPTH; /// Commands retired successfully. A watchdog storm means the GPU stopped making progress; /// timeouts of overloaded clients while other work keeps retiring are not one. @@ -34,12 +37,13 @@ pub(crate) fn since_retired(now: u64) -> Option { pub(super) struct Storm { start: Option, count: u8, + forgiven: u8, stopped: bool, progress: u64, } impl Storm { pub(super) const fn new() -> Self { - Self { start: None, count: 0, stopped: false, progress: 0 } + Self { start: None, count: 0, forgiven: 0, stopped: false, progress: 0 } } /// Like [`Self::note`], but work retired since the previous event starts a new window. pub(super) fn note_unless_progress(&mut self, now: u64, progress: u64) -> bool { @@ -54,11 +58,21 @@ impl Storm { if self.start.is_none_or(|start| now.saturating_sub(start) >= WINDOW_NS) { self.start = Some(now); self.count = 0; + self.forgiven = 0; } self.count += 1; self.stopped = self.count >= LIMIT; self.stopped } + /// Takes back the latest event, at most [`FORGIVE_LIMIT`] times per window. A stop stays. + pub(super) fn forgive(&mut self) -> bool { + if self.stopped || self.count == 0 || self.forgiven >= FORGIVE_LIMIT { + return false; + } + self.count -= 1; + self.forgiven += 1; + true + } } #[kernel::macros::kunit_tests(asahi_g17_timeout_recovery_safety)] @@ -109,6 +123,23 @@ mod tests { for _ in 0..1024 { assert!(storm.note(u64::MAX)); } } #[test] + fn forgiven_restarts_are_bounded_per_window() { + let mut storm = Storm::new(); + for _ in 0..FORGIVE_LIMIT { + assert!(!storm.note(0)); + assert!(storm.forgive()); + } + assert!(!storm.note(0)); + assert!(!storm.forgive()); + for _ in 2..LIMIT { assert!(!storm.note(0)); } + assert!(storm.note(0)); + assert!(!storm.forgive()); + let mut storm = Storm::new(); + for _ in 0..FORGIVE_LIMIT { storm.note(0); storm.forgive(); } + assert!(!storm.note(WINDOW_NS)); + assert!(storm.forgive()); + } + #[test] fn recovery_table_ends_at_its_first_invalid_record() { use crate::g17::recovery::Entries; let mut entries = Entries::default(); diff --git a/drivers/gpu/drm/asahi/g17/runtime/events.rs b/drivers/gpu/drm/asahi/g17/runtime/events.rs index 3e779bba4fbbf..d6d811b410e89 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/events.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/events.rs @@ -158,6 +158,9 @@ impl recovery::Queues for Service<'_> { fn render_pass_started(&self, qid: u8, stamp: u64) -> Option { self.firmware.queues.render_pass_started(qid, stamp) } + fn render_culprit(&self, blamed: u8) -> Option { + self.firmware.queues.render_culprit(blamed) + } fn publish_recovery_targets(&self) -> Option { let mut failure = self.firmware.queues.publish_render_recovery_targets(); for queue in self @@ -323,6 +326,9 @@ impl Firmware { .set_generation(service.firmware.recovery.generation()); service.firmware.rings.invalidate_statistics(); let result = result?; + if result.leftover && service.firmware.recovery.forgive_restart() { + pr_warn!("G17: recovery dropped queued work of an already failed VM; not counted as a restart storm\n"); + } if let Some(error) = result.target_error { dev_warn!( shared.dev.as_ref(), diff --git a/drivers/gpu/drm/asahi/g17/runtime/render/recovery.rs b/drivers/gpu/drm/asahi/g17/runtime/render/recovery.rs index 7a6d987591c76..02ece8cf4025b 100644 --- a/drivers/gpu/drm/asahi/g17/runtime/render/recovery.rs +++ b/drivers/gpu/drm/asahi/g17/runtime/render/recovery.rs @@ -2,8 +2,8 @@ //! Recovery targets for retained render graphs while firmware is halted. -use crate::g17::runtime::Registry; -use kernel::prelude::*; +use crate::g17::{runtime::Registry, status::VmStatus}; +use kernel::{prelude::*, sync::Arc}; impl Registry { pub(in crate::g17::runtime) fn report_render_recovery(&self, qid: u8, port: &crate::g17::port::Port) { @@ -29,6 +29,23 @@ impl Registry { .pass_started(stamp) } + /// See `recovery::Queues::render_culprit`. + pub(in crate::g17::runtime) fn render_culprit(&self, blamed: u8) -> Option { + let pairs = || self.render.entries.iter().flatten().filter_map(|entry| entry.pair.as_ref()); + let untouched = pairs().find(|pair| pair.qids().contains(&blamed))?.front_untouched() == Some(true); + let running = pairs().filter_map(|pair| { + let qids = pair.qids().into_iter().fold(0, |mask, qid| mask | 1u128 << qid); + Some((qids, pair.running_owner()?)) + }); + culprit(running, untouched, |culprit| { + let mut foreign = false; + for queue in self.compute.iter().flatten().filter_map(|entry| entry.queue.as_deref()) { + queue.visit_owners(&mut |owner| foreign |= !Arc::ptr_eq(owner, culprit)); + } + foreign + }) + } + pub(in crate::g17::runtime) fn publish_render_recovery_targets(&self) -> Option { let mut failure = None; for pair in self.render.entries.iter().flatten() @@ -41,3 +58,64 @@ impl Registry { failure } } + +/// Picks the QIDs to blame from each pair's running owner. A VM an earlier recovery found guilty +/// comes first, even when the blamed render had started: its groups queued before that keep +/// hanging the GPU one per restart, while bystanders' renders whose tiler started wait behind +/// them, and that recovery failed its interrupted work. Otherwise the blamed render must be +/// `untouched`, exactly one VM may be running, and `foreign` must find no other VM's compute work. +fn culprit<'a>( + running: impl Iterator)>, + untouched: bool, + foreign: impl FnOnce(&Arc) -> bool, +) -> Option { + let (mut guilty, mut qids, mut several) = (0u128, 0u128, false); + let mut culprit: Option<&Arc> = None; + for (pair, owner) in running { + if owner.recovery_guilty() { + guilty |= pair; + } + several |= culprit.is_some_and(|known| !Arc::ptr_eq(known, owner)); + culprit = Some(owner); + qids |= pair; + } + if guilty != 0 { + return Some(guilty); + } + let culprit = culprit.filter(|_| untouched && !several)?; + (!foreign(culprit)).then_some(qids) +} + +#[kernel::macros::kunit_tests(asahi_g17_render_culprit)] +mod tests { + use super::*; + + fn vm() -> Arc { + Arc::pin_init(VmStatus::new(), GFP_KERNEL).unwrap() + } + + #[test] + fn a_guilty_vm_still_running_is_blamed_before_running_bystanders() { + let (spinner, bystander) = (vm(), vm()); + let running = || [(0b11, &bystander), (0b1100, &spinner)].into_iter(); + // Two VMs running and no earlier guilt: the firmware's blame stands, even when one of + // them failed only for its own client. + bystander.record(EIO); + assert_eq!(culprit(running(), true, |_| false), None); + spinner.note_recovery_guilty(); + assert_eq!(culprit(running(), true, |_| panic!("compute is not consulted")), Some(0b1100)); + // The bystander's blamed render whose tiler started is starved too. + assert_eq!(culprit(running(), false, |_| panic!("compute is not consulted")), Some(0b1100)); + } + + #[test] + fn one_running_vm_is_blamed_only_without_foreign_compute() { + let (a, b) = (vm(), vm()); + assert_eq!(culprit([(0b11, &a), (0b1100, &a)].into_iter(), true, |_| false), Some(0b1111)); + // A blamed render that had started may be the hang itself. + assert_eq!(culprit([(0b11, &a), (0b1100, &a)].into_iter(), false, |_| false), None); + assert_eq!(culprit([(0b11, &a)].into_iter(), true, |_| true), None); + assert_eq!(culprit([(0b11, &a), (0b1100, &b)].into_iter(), true, |_| false), None); + assert_eq!(culprit(core::iter::empty(), true, |_| false), None); + } +} diff --git a/drivers/gpu/drm/asahi/g17/status.rs b/drivers/gpu/drm/asahi/g17/status.rs index a065c5030b1b8..75df8ad0480ae 100644 --- a/drivers/gpu/drm/asahi/g17/status.rs +++ b/drivers/gpu/drm/asahi/g17/status.rs @@ -5,7 +5,7 @@ use crate::debug::*; use crate::gem; use core::sync::atomic::{AtomicBool, AtomicI32, Ordering}; -use kernel::{new_mutex, prelude::*, sync::Mutex}; +use kernel::{new_mutex, prelude::*, str::BStr, sync::Mutex, task::Pid}; const DEBUG_CLASS: DebugFlags = DebugFlags::Errors; @@ -22,6 +22,11 @@ pub(crate) struct VmStatus { client_closed: AtomicBool, /// A failure of this VM has been logged as a warning. warned: AtomicBool, + /// A GPU recovery found this VM's work guilty. + recovery_guilty: AtomicBool, + /// The process that created the VM, named in the failure report. + tgid: Pid, + comm: [u8; 16], /// The client's mapped status word, retained with its object only while the /// client's VM handle exists; released by `release_mirror` at VM close. #[pin] @@ -38,12 +43,23 @@ struct Mirror { unsafe impl Send for Mirror {} impl VmStatus { + /// Called from the creating client's ioctl: the VM records who it belongs to. pub(crate) fn new() -> impl PinInit { + let task = kernel::current!(); + // SAFETY: `comm` is a fixed array inside the live current task. A concurrent rename by + // another thread of the process may tear the copy, as with C's unlocked `current->comm`. + let comm: [u8; 16] = unsafe { + core::ptr::read_volatile(core::ptr::addr_of!((*task.as_ptr()).comm).cast()) + }; + let tgid = task.tgid(); pin_init!(Self { error: AtomicI32::new(0), reported_error: AtomicI32::new(0), client_closed: AtomicBool::new(false), warned: AtomicBool::new(false), + recovery_guilty: AtomicBool::new(false), + tgid, + comm, mirror <- new_mutex!(None, "Asahi VM status mirror"), }) } @@ -52,6 +68,16 @@ impl VmStatus { self.error.load(Ordering::Acquire) } + pub(crate) fn note_recovery_guilty(&self) { + self.recovery_guilty.store(true, Ordering::Release); + } + + /// A GPU recovery found this VM guilty: it hung or faulted the GPU, unlike a VM that + /// failed only for its own client. + pub(crate) fn recovery_guilty(&self) -> bool { + self.recovery_guilty.load(Ordering::Acquire) + } + /// Record permanent admission failure and report it to the client. #[track_caller] pub(crate) fn record(&self, error: Error) { @@ -98,11 +124,14 @@ impl VmStatus { ); } } else if !self.warned.swap(true, Ordering::AcqRel) { + let len = self.comm.iter().position(|&b| b == 0).unwrap_or(self.comm.len()); pr_warn!( - "GPU work of a client failed ({:?}, reported at {}:{}); the client sees a lost device\n", + "GPU work of a client failed ({:?}, reported at {}:{}, client {}[{}]); the client sees a lost device\n", error, file, - at.line() + at.line(), + BStr::from_bytes(&self.comm[..len]), + self.tgid ); } let error = match first { diff --git a/include/uapi/drm/asahi_drm.h b/include/uapi/drm/asahi_drm.h index 418522205ee6f..5641384b51bfe 100644 --- a/include/uapi/drm/asahi_drm.h +++ b/include/uapi/drm/asahi_drm.h @@ -447,8 +447,9 @@ enum drm_asahi_feature { * treating that fence as successful execution. Syncobj signal alone is not * an execution-success indication. A job cancelled (ECANCELED) because * another VM's recovery discarded it, an input did not signal in time, or - * its queue was closed, is not a failure of this VM: its fence carries the - * error, error stays 0, and its output is undefined. + * its queue was closed, or dropped (ENODATA) because a recovery could not + * run it again, is not a failure of this VM: its fence carries the error, + * error stays 0, and its output is undefined. * The feature bit is the availability contract; older kernels leave it zero. * Kernel errors for invalid requests: EINVAL flags/pad/size, ENOENT foreign or * missing vm_id, EFAULT inaccessible pointer. No output on invalid request.