Re: [PATCH v4 08/17] gpu: nova-core: add an interrupt delivery self-test

[Alexandre Courbot]

On Sat Sep 12, 2026 at 1:43 PM JST, John Hubbard wrote:
> A GPU interrupt can be lost in the MSI or MSI-X allocation, in the GIN
> tree's enables, or in the rearm, and every one of those failures looks
> the same: no interrupt arrives, and nothing says which one broke.
>
> Add a probe-time self-test, built under NOVA_CORE_SELFTESTS, that
> latches the CPU doorbell vector through the GIN software trigger and
> waits for a registered handler to service it. One delivery would pass
> with a broken rearm, because the first message-signaled interrupt
> arrives whether or not the driver rearms, so the test triggers twice and
> waits for the first handler to finish before the second trigger. It runs
> after GFW boot and before GSP boot, on a quiesced tree, and fails probe
> unless both deliveries arrive, each finds only the doorbell pending, and
> the leaf ends clear.
>
> The doorbell has the same vector on every supported GPU, so the test
> names it without asking GSP-RM. It allocates the PCI vectors for the
> doorbell's subtree and releases them before returning, so under MSI-X
> the delivery also exercises that subtree's table entry.
>
> Assisted-by: LLM
> Co-developed-by: Joel Fernandes <joelagnelf@xxxxxxxxxx>
> Signed-off-by: Joel Fernandes <joelagnelf@xxxxxxxxxx>
> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
> ---
>  drivers/gpu/nova-core/Kconfig               |   5 +
>  drivers/gpu/nova-core/driver.rs             |   5 +
>  drivers/gpu/nova-core/irq.rs                |   2 +
>  drivers/gpu/nova-core/irq/doorbell_test.rs  | 266 ++++++++++++++++++++
>  drivers/gpu/nova-core/irq/interrupt_tree.rs |   2 +-
>  drivers/gpu/nova-core/nova_core.rs          |   2 +-
>  6 files changed, 280 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/gpu/nova-core/irq/doorbell_test.rs
>
> diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
> index 1934f17baa8b..2e11e46c99c7 100644
> --- a/drivers/gpu/nova-core/Kconfig
> +++ b/drivers/gpu/nova-core/Kconfig
> @@ -24,4 +24,9 @@ config NOVA_CORE_SELFTESTS
>  	help
>  	  Build the driver self-tests and run them when the GPU is probed.
>  
> +	  If the interrupt delivery test fails, the probe fails and the driver
> +	  does not bind to the GPU. A broken interrupt path would otherwise
> +	  show up later as a hang, far from its cause. Every other self-test
> +	  logs its failure and lets the probe continue.
> +
>  	  If unsure, say N.
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> index 15a44f9a6441..4400cae8c8ce 100644
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -119,6 +119,11 @@ fn probe<'bound>(
>                      let spec = Spec::new(pdev.as_ref(), bar)?;
>  
>                      gpu::wait_gfw_boot_completion(pdev.as_ref(), bar, spec.chipset)?;
> +
> +                    // The self-test disables and drains the whole tree, so it has to run before
> +                    // `Gpu::new` boots the GSP.
> +                    #[cfg(CONFIG_NOVA_CORE_SELFTESTS)]
> +                    crate::irq::doorbell_test::run_selftest(pdev, bar, spec.chipset)?;
>                  },
>                  // TODO: Use self-referential pin-init syntax once available.
>                  gpu <- Gpu::new(
> diff --git a/drivers/gpu/nova-core/irq.rs b/drivers/gpu/nova-core/irq.rs
> index 28f147641024..7fb7d9f2e237 100644
> --- a/drivers/gpu/nova-core/irq.rs
> +++ b/drivers/gpu/nova-core/irq.rs
> @@ -9,6 +9,8 @@
>  //!
>  //! See `Documentation/gpu/nova/core/interrupts.rst`.
>  
> +#[cfg(CONFIG_NOVA_CORE_SELFTESTS)]
> +pub(crate) mod doorbell_test;
>  mod hal;
>  mod interrupt_tree;
>  mod regs;
> diff --git a/drivers/gpu/nova-core/irq/doorbell_test.rs b/drivers/gpu/nova-core/irq/doorbell_test.rs
> new file mode 100644
> index 000000000000..a1f8b3cc377b
> --- /dev/null
> +++ b/drivers/gpu/nova-core/irq/doorbell_test.rs
> @@ -0,0 +1,266 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +
> +//! Interrupt delivery self-test.
> +//!
> +//! The test triggers the CPU doorbell vector from software, twice, and checks that each trigger
> +//! reaches a registered handler. It runs during probe under `CONFIG_NOVA_CORE_SELFTESTS`.
> +//!
> +//! See "Self-test" in `Documentation/gpu/nova/core/interrupts.rst`.
> +
> +use core::pin::Pin;
> +
> +use kernel::{
> +    device::Bound,
> +    irq,
> +    pci,
> +    prelude::*,
> +    sync::{
> +        atomic::{
> +            Atomic,
> +            Relaxed, //
> +        },
> +        Completion, //
> +    },
> +    time, //
> +};
> +
> +use super::interrupt_tree::{
> +    GinVector,
> +    LeafEnableGuard,
> +    LeafMask,
> +    Subtree,
> +    TopEnableGuard,
> +    Tree, //
> +};
> +
> +use crate::{
> +    driver::Bar0,
> +    gpu::Chipset,
> +    selftest_assert,
> +    selftest_assert_eq, //
> +};
> +
> +/// The CPU doorbell vector. Every supported GPU uses this number, so the test needs nothing from
> +/// GSP-RM, which is not running yet.
> +const DOORBELL_VECTOR: GinVector = GinVector::new::<129>();
> +
> +/// The only subtree that this test services.
> +const DOORBELL_SUBTREE: Subtree = DOORBELL_VECTOR.subtree();
> +
> +/// Time allowed for each delivery to arrive.
> +const DELIVERY_TIMEOUT_MS: time::Msecs = 1000;
> +
> +/// The self-test's interrupt handler.
> +///
> +/// It clears only the doorbell's bit, rearms delivery, and never walks the tree. A missing rearm
> +/// shows up as a timeout on the second delivery.
> +#[pin_data]
> +struct DoorbellTestHandler<'a> {
> +    tree: Tree<'a>,
> +    /// Completed by the first delivery.
> +    #[pin]
> +    first: Completion,
> +    /// Completed by the second delivery.
> +    #[pin]
> +    second: Completion,
> +    /// Deliveries that found the doorbell bit set.
> +    irq_count: Atomic<u32>,
> +    /// The doorbell leaf's pending bits, as read by the first delivery.
> +    first_pending: Atomic<u32>,
> +    /// The doorbell leaf's pending bits, as read by the second delivery.
> +    second_pending: Atomic<u32>,
> +}
> +
> +impl irq::Handler for DoorbellTestHandler<'_> {
> +    fn handle(&self) -> irq::IrqReturn {
> +        let leaf = self.tree.read_pending(DOORBELL_VECTOR.leaf_index());
> +        let pending = leaf.vectors();
> +        if !pending.contains(DOORBELL_VECTOR.leaf_mask()) {
> +            self.tree.rearm_pci_irq(DOORBELL_SUBTREE);
> +            return irq::IrqReturn::None;
> +        }
> +        leaf.clear_vectors(DOORBELL_VECTOR.leaf_mask());
> +
> +        let count = self.irq_count.fetch_add(1, Relaxed);
> +
> +        // Rearm before completing, since the waiting thread triggers the next doorbell as soon as
> +        // it wakes.
> +        self.tree.rearm_pci_irq(DOORBELL_SUBTREE);

Let's group the `clear_vectors` and `rearm_pci_irq` together and before
the `count` increase. Having the count increase in the middle breaks the
flow of the IRQ logic and there is no good reason to have it here.

> +
> +        match count {
> +            0 => {
> +                self.first_pending.store(pending.into_raw(), Relaxed);
> +                self.first.complete_all();
> +            }
> +            1 => {
> +                self.second_pending.store(pending.into_raw(), Relaxed);
> +                self.second.complete_all();
> +            }
> +            _ => (),
> +        }
> +
> +        irq::IrqReturn::Handled
> +    }
> +}
> +
> +/// The self-test's handler registration and the enables that deliver to it.
> +///
> +/// Drops in the order that "Enabling the GSP event" in
> +/// `Documentation/gpu/nova/core/interrupts.rst` requires: the vector is disabled, then the
> +/// handler is freed, then the subtree is disabled.
> +struct SelftestResources<'a, 'r> {
> +    _leaf_guard: LeafEnableGuard<'a>,
> +    reg: Pin<KBox<irq::Registration<'r, DoorbellTestHandler<'a>>>>,
> +    _top_guard: TopEnableGuard<'a>,
> +}
> +
> +impl<'a> SelftestResources<'a, '_> {
> +    fn handler(&self) -> &DoorbellTestHandler<'a> {
> +        self.reg.handler()
> +    }
> +
> +    /// Disables the doorbell vector and waits for a handler in flight on another CPU to finish.
> +    ///
> +    /// The handler's counters and the leaf's pending bits are final on return.
> +    fn quiesce_source(&self) {
> +        self.handler()
> +            .tree
> +            .disable_leaf(DOORBELL_VECTOR.leaf_index(), DOORBELL_VECTOR.leaf_mask());
> +        self.reg.synchronize();
> +    }
> +}
> +
> +/// Runs the interrupt delivery self-test.
> +///
> +/// Call this only during probe, before GSP boot: it disables every vector in the tree and clears
> +/// every pending bit. On return, the doorbell's subtree is disabled at `TOP`, and the test's PCI
> +/// vectors and handler are released.
> +///
> +/// # Errors
> +///
> +/// `EINVAL` if `chipset` does not implement the doorbell's subtree. `ETIMEDOUT` if a delivery
> +/// does not arrive within [`DELIVERY_TIMEOUT_MS`]. `EIO` if a self-test assertion fails.
> +/// Otherwise the error from allocating the PCI vectors or registering the handler.
> +pub(crate) fn run_selftest(pdev: &pci::Device<Bound>, bar: Bar0<'_>, chipset: Chipset) -> Result {
> +    let dev = pdev.as_ref();

This variable is superfluous, `pdev` can be passed as-is to all of the
macros and will be automatically deref'd.

> +
> +    let vectors = super::alloc_vectors(pdev, DOORBELL_SUBTREE.into())?;
> +    let request = vectors.request_for(DOORBELL_SUBTREE)?;
> +    let tree = Tree::new(bar, chipset, &vectors)?;

There is a `vectors.tree()` method introduced later in the series that
we should add earlier and use here instead of calling this constructor.