drm/asahi: g17: confine a GPU hang to the client that caused it

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.

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<bool> {
+        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<VmStatus>> {
+        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,

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<bool>;
+    /// 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<u128> { 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<Error>;
@@ -144,6 +152,8 @@ pub(super) struct State {
 pub(super) struct ServiceResult {
     pub(super) scan_error: Option<Error>,
     pub(super) target_error: Option<Error>,
+    /// 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<H: Host>(
     // 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<H: Host>(
     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<H: Host>(
     }
     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<H: Host>(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<H: Host>(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<Q: Queues>(queues: &mut Q, entries: Entries, blamed: Option<u8>, 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<Q: Queues>(queues: &mut Q, entries: Entries, blamed: Option<u8>, 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<Q: Queues>(queues: &mut Q, entries: Entries, blamed: Option<u8>, 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<Arc<VmStatus>>; MAX_GUILTY_VMS] = Default::default();
     let mut count = 0;
@@ -383,8 +407,15 @@ fn classify<Q: Queues>(queues: &mut Q, entries: Entries, blamed: Option<u8>, att
         }
     }
     if unresolved || (count == 0 && collateral == 0 && !no_culprit) {
-        return (0, blame);
+        return (0, blame, false);
     }
+    let known = |status: &Arc<VmStatus>| 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<Q: Queues>(queues: &mut Q, entries: Entries, blamed: Option<u8>, 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<VmStatus>); 4], [Option<Class>; 4]);
+
+    impl Queues for Owners {
+        fn render_scheduler_active(&self) -> bool { true }
+        fn render_pass_started(&self, _qid: u8, _stamp: u64) -> Option<bool> { None }
+        fn publish_recovery_targets(&self) -> Option<Error> { None }
+        fn oldest_spared(&self, qid: u8) -> Option<bool> {
+            self.0.iter().any(|(own, _)| *own == qid).then_some(false)
+        }
+        fn visit_owners(&self, qid: u8, visit: &mut dyn FnMut(&Arc<VmStatus>)) {
+            self.0.iter().filter(|(own, _)| *own == qid).for_each(|(_, status)| visit(status));
+        }
+        fn classify_work(&mut self, class: &dyn Fn(&Arc<VmStatus>) -> 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);

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<u128>,
+    ) -> 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);
+    }
 }