Re: [PATCH v3 12/14] gpu: nova-core: drive GSP events with the SWGEN0 interrupt

[sashiko-bot]

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 <jhubbard@xxxxxxxxxx>

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.

> 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

[ ... ]

> +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@xxxxxxxxxx?part=12