Re: [PATCH v3 12/14] gpu: nova-core: drive GSP events with the SWGEN0 interrupt
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-03 03:28:16
Also in:
lkml
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unserviceable GSP falcon interrupts cause an infinite interrupt storm and CPU soft lockup.
--
commit e6648318ceeb9312972b220805b21b9370c7409e
Author: John Hubbard [off-list ref]
gpu: nova-core: drive GSP events with the SWGEN0 interrupt
This commit implements threaded IRQ handling for the GSP SWGEN0 event in
nova-core. It replaces polling with an interrupt handler that drains the
GSP-to-CPU message queue and clears the GIN leaf in hardirq context.
quoted hunk ↗ jump to hunk
diff --git a/drivers/gpu/nova-core/irq/gsp.rs b/drivers/gpu/nova-core/irq/gsp.rs new file mode 100644 index 0000000000000..174488565f790 --- /dev/null +++ b/drivers/gpu/nova-core/irq/gsp.rs
[ ... ]
quoted hunk ↗ jump to hunk
+impl irq::ThreadedHandler for GspInterrupt<'_> { + /// Top half: clears the GIN leaf, takes every falcon cause pending for the host, and rearms + /// PCI interrupt delivery. + fn handle(&self) -> irq::ThreadedIrqReturn { + let bar = self.bar; + + // Only service our own vector: require the GSP bit in the leaf and clear just that bit, so + // a co-pending vector in the same leaf stays pending for whoever services it. The subtree + // stays enabled, so there is no whole-tree disable and enable. + let leaf = self.tree.read_pending(GSP_INTR_0_VECTOR.leaf_index()); + if !leaf.vectors().contains(GSP_INTR_0_VECTOR.leaf_mask()) { + // Nothing to service, but nova-core is the only consumer of this PCI interrupt, so + // skipping the rearm here would silence every later interrupt as well. + self.tree.rearm_pci_irq(GSP_SUBTREE); + return irq::ThreadedIrqReturn::None; + } + leaf.clear_vectors(GSP_INTR_0_VECTOR.leaf_mask()); + + let status = GspFalcon::take_host_intr(bar, self.chipset); + + // A cause left latched holds the falcon's host-routed set non-empty, and the falcon + // signals the tree only on a transition of that set, so no later SWGEN0 would signal. + let unserviceable = status.with_swgen0(false); + if unserviceable.into_raw() != 0 { + // nova-core has no recovery path for a cause other than a posted message, for example + // a HALT from a GSP crash, so report it rather than discarding it. + dev_err!( + &self.dev, + "unserviceable GSP falcon interrupt, IRQSTAT {:#x}\n", + status.into_raw() + ); + GspFalcon::clear_intr(bar, unserviceable); + } + + // The leaf clear above consumed the tree's record of this interrupt, and the falcon signals + // the tree only on a transition of its host-routed causes, so a cause that arrived while + // this handler ran would never reach the CPU. Re-emit to supply that transition. + GspFalcon::retrigger_intr(bar, self.chipset); + + // Delivery resumes only after this, so it must happen on every path that services the + // vector, including the fault path above. + self.tree.rearm_pci_irq(GSP_SUBTREE);
[Severity: High] Does this lead to an interrupt storm if the GSP remains in a fault state like a HALT or WDT timeout? If there is no recovery path to reset the GSP, wouldn't the unserviceable interrupt immediately re-latch after GspFalcon::clear_intr() is called? The subsequent calls to GspFalcon::retrigger_intr() and self.tree.rearm_pci_irq() would then force a new edge to the GIN tree and re-enable PCI delivery, potentially trapping the CPU in an endless loop servicing the same unserviceable interrupt. Could the GIN leaf source be disabled here instead when encountering an unrecoverable fault? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260903031514.1515905-1-jhubbard@nvidia.com?part=12