Re: [PATCH v3 07/14] gpu: nova-core: add an interrupt delivery self-test

[Alexandre Courbot]

On Thu Sep 3, 2026 at 12:15 PM JST, John Hubbard wrote:
> A GPU interrupt can be lost in the MSI or MSI-X allocation, in the GIN
> tree's enable bits, or in the rearm. Every one of those failures looks
> the same to the driver: no interrupt arrives, and nothing in the symptom
> says which one broke.
>
> Add an optional probe-time self-test that injects the CPU doorbell
> through the GIN software trigger. One injection would pass even with a
> broken rearm, because the first message-signaled interrupt arrives
> whether the driver rearms or not. The test injects twice, and waits for
> the first handler to rearm before it injects again.
>
> Run it before GSP boot on a quiesced tree, and fail probe unless exactly
> two deliveries arrive, each delivery finds only the doorbell pending,
> and the leaf ends clear. Under MSI-X the injected subtree has its own
> table entry, so the delivery exercises that entry too.
>
> Allocate the PCI interrupt vectors alongside the GPU's other resources
> rather than in the test, because the vectors are allocated once for the
> whole PCI device rather than per handler. The test takes the vector for
> the subtree it services.
>
> Assisted-by: Cursor:claude-opus-5
> Reviewed-by: Will Pierce <wpierce@xxxxxxxxxx>
> 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               |  15 +
>  drivers/gpu/nova-core/gpu.rs                |  25 ++
>  drivers/gpu/nova-core/irq.rs                |   9 +
>  drivers/gpu/nova-core/irq/doorbell_test.rs  | 301 ++++++++++++++++++++
>  drivers/gpu/nova-core/irq/interrupt_tree.rs |   2 +-
>  drivers/gpu/nova-core/nova_core.rs          |   2 +-
>  6 files changed, 352 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 f918f69e0599..7198fae6b6f4 100644
> --- a/drivers/gpu/nova-core/Kconfig
> +++ b/drivers/gpu/nova-core/Kconfig
> @@ -15,3 +15,18 @@ config NOVA_CORE
>  	  This driver is work in progress and may not be functional.
>  
>  	  If M is selected, the module will be called nova-core.
> +
> +config NOVA_CORE_IRQ_SELFTEST
> +	bool "Nova Core interrupt delivery self-test"
> +	depends on NOVA_CORE
> +	help
> +	  Run an interrupt delivery self-test during nova-core probe. It
> +	  injects a known vector through the GPU interrupt controller's
> +	  software trigger and confirms the interrupt reaches the driver's
> +	  handler, validating the PCI interrupt path from the GPU to the CPU
> +	  with no dependency on GSP firmware. The result is printed to dmesg.
> +
> +	  If the test fails, the PCI probe fails and the driver does not load.
> +
> +	  This is intended for driver bring-up and for debugging PCI, MSI, or
> +	  passthrough setups. If unsure, say N.

With the PRAMIN series merged, there is now a global
`NOVA_CORE_SELFTESTS` Kconfig option - let's leverage it.

(see also if the added assertion macros are useful for this test)

> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index e1ac8ee9ba4d..8a9bc4baf9ac 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -29,6 +29,7 @@
>          Gsp,
>          GspBootContext, //
>      },
> +    irq::SubtreeVectors,
>      vgpu::VgpuManager, //
>  };
>  
> @@ -292,6 +293,12 @@ pub(crate) struct Gpu<'gpu> {
>      /// Must be kept declared *after* `gsp_resources`, as the latter's `PinnedDrop` implementation
>      /// requires the sysmem flush page to be in place.
>      sysmem_flush: SysmemFlush<'gpu>,
> +    /// Self-referential borrow of `vectors`, so this does not have to be repeated in the
> +    /// constructor. Will go away with self-referential pin-init.
> +    vectors_ref: &'gpu SubtreeVectors<'gpu>,
> +    /// PCI interrupt vector allocation. Dropped last (struct field drop order).
> +    #[pin]
> +    vectors: SubtreeVectors<'gpu>,
>  }
>  
>  #[pinned_drop]
> @@ -330,6 +337,12 @@ pub(crate) fn new<'a>(
>          let dev = pdev.as_ref();
>  
>          try_pin_init!(Self {
> +            vectors: crate::irq::alloc_vectors(pdev, crate::irq::SERVICED_SUBTREE.into())?,
> +
> +            // SAFETY: `vectors` is initialized above, lives at a pinned stable address, and is
> +            // dropped after every field that uses `vectors_ref` (struct field drop order).
> +            vectors_ref: unsafe { &*core::ptr::from_ref(vectors.as_ref().get_ref()) },
> +
>              spec: Spec::new(dev, bar).inspect(|spec| {
>                  dev_info!(dev,"NVIDIA ({})\n", spec);
>              })?,
> @@ -347,6 +360,18 @@ pub(crate) fn new<'a>(
>                      .inspect_err(|_| dev_err!(dev, "GFW boot did not complete\n"))?;
>              },
>  
> +            // Validate the MSI interrupt path before booting GSP, when the self-test is
> +            // enabled. This runs on a quiesced interrupt tree with no GSP state present, so it
> +            // never observes or clears GSP or PRIV_RING interrupts.
> +            _: {
> +                // `vectors_ref` exists for the self-test below, which this configuration omits.
> +                #[cfg(not(CONFIG_NOVA_CORE_IRQ_SELFTEST))]
> +                let _ = vectors_ref;
> +
> +                #[cfg(CONFIG_NOVA_CORE_IRQ_SELFTEST)]
> +                crate::irq::doorbell_test::run_selftest(pdev, bar, spec.chipset, vectors_ref)?;
> +            },
> +

The placement here looks a bit off to me. We are creating the vectors
earlier than we need to, and there is a significant issue with passing
them to `run_selftest`. I'll come back to this in `run_selftest`, but
long story short, the `vectors_ref` argument will go away.

This means that you can now do what the PRAMIN series did and run the
selftests from `driver.rs`. Unfortunately you cannot use the same
anchor, as the PRAMIN tests need to run after the `Gpu` instance is
created to obtain the VRAM regions, whereas the IRQ tests are the
opposite and need to run *before* that. So I guess the right anchor will
be right after the `bar` is created, and you'll need to create a local
`Spec` to extract the chipset from, which is no big deal.

Although we may want to check whether the selftest requires
`wait_gfw_boot_completion` to have completed, and move that one out of
the Gpu constructor as well if needed?

Regardless, as a consequence of the selftest acquiring its own vectors,
the creation of `vectors` and `vectors_ref` can also be deferred to
patch 12.

>              // Initialize this early because `gsp_resources` depends on it.
>              sysmem_flush: SysmemFlush::register(dev, bar, spec.chipset)?,
>  
> diff --git a/drivers/gpu/nova-core/irq.rs b/drivers/gpu/nova-core/irq.rs
> index f6ba883d72c5..f44897692b74 100644
> --- a/drivers/gpu/nova-core/irq.rs
> +++ b/drivers/gpu/nova-core/irq.rs
> @@ -8,6 +8,8 @@
>  //!
>  //! See `Documentation/gpu/nova/core/interrupts.rst`.
>  
> +#[cfg(CONFIG_NOVA_CORE_IRQ_SELFTEST)]
> +pub(crate) mod doorbell_test;
>  mod hal;
>  mod interrupt_tree;
>  mod regs;
> @@ -25,10 +27,17 @@
>  use crate::num;
>  
>  use interrupt_tree::{
> +    GinVector,
>      Subtree,
>      SubtreeSet, //
>  };
>  
> +/// The subtree nova-core allocates PCI vectors for.
> +///
> +/// Every source nova-core services latches in this one subtree, so a single allocation covers all
> +/// of them.
> +pub(crate) const SERVICED_SUBTREE: Subtree = GinVector::new::<129>().subtree();
> +
>  /// The message-signaled interrupt type a vector allocation obtained.
>  ///
>  /// nova-core allocates MSI-X or MSI and nothing else, so the level-triggered INTx that
> 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..a232a83b62f6
> --- /dev/null
> +++ b/drivers/gpu/nova-core/irq/doorbell_test.rs
> @@ -0,0 +1,301 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +
> +//! Interrupt delivery self-test, driven through the CPU doorbell vector.
> +//!
> +//! Exercises the whole PCI interrupt path (GPU to PCIe to CPU to handler) with no GSP dependency:
> +//! it injects a known vector through the GIN software trigger and confirms the handler runs. Two
> +//! interrupts are triggered one at a time, which also covers the rearm that every delivery after
> +//! the first depends on. Gated behind `CONFIG_NOVA_CORE_IRQ_SELFTEST` and run before GSP boot, so
> +//! it never observes or clears GSP interrupt state.
> +//!
> +//! See `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, //
> +    },
> +    SubtreeVectors, //
> +};
> +
> +use crate::{
> +    driver::Bar0,
> +    gpu::Chipset, //
> +};
> +
> +/// Fixed vector for the CPU doorbell.
> +///
> +/// The resource manager pins the CPU doorbell to this vector on every supported chip, so nova-core

In general we want to say GSP-RM instead of "resource manager" for
precision.

Though actually it looks that contrary to the GSP vector, the doorbell
one is hardwired - which is what allows it to be used before GSP-RM is
running.

> +/// uses the constant directly instead of discovering it at runtime.
> +const DOORBELL_VECTOR: GinVector = GinVector::new::<129>();
> +
> +/// Subtree carrying the doorbell vector, and the only subtree this test services.
> +///
> +/// Derived from the vector so that changing `DOORBELL_VECTOR` moves the subtree it enables and the
> +/// handler together.
> +const DOORBELL_SUBTREE: Subtree = DOORBELL_VECTOR.subtree();
> +
> +/// Time allowed for each of the two deliveries to arrive.
> +const DELIVERY_TIMEOUT_MS: time::Msecs = 1000;
> +
> +/// Interrupt handler installed by the self-test.
> +///
> +/// Services the doorbell the way a notification source is serviced: it clears its own leaf bit and
> +/// rearms PCI interrupt delivery, leaving the rest of the tree untouched. It records the leaf's
> +/// pending bits seen on each of the first two deliveries and signals the matching completion.
> +#[pin_data]
> +struct DoorbellTestHandler<'a> {
> +    /// The interrupt tree, which carries the borrowed BAR0 that register access needs.
> +    tree: Tree<'a>,
> +    /// Signalled by the first delivery.
> +    #[pin]
> +    first: Completion,
> +    /// Signalled by the second delivery.
> +    #[pin]
> +    second: Completion,
> +    /// Count of deliveries this handler has serviced.
> +    irq_count: Atomic<u32>,
> +    /// Doorbell leaf's pending bits observed on the first delivery.
> +    first_pending: Atomic<u32>,
> +    /// Doorbell leaf's pending bits observed on the second delivery.
> +    second_pending: Atomic<u32>,
> +}
> +
> +impl irq::Handler for DoorbellTestHandler<'_> {
> +    fn handle(&self) -> irq::IrqReturn {
> +        // Clear only this handler's own bit and leave `TOP_EN` alone. A full walk disables and
> +        // enables the tree, which produces a delivery edge by itself and would hide a missing PCI
> +        // interrupt rearm.
> +        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 signalling, so delivery is possible again by the time the waiting thread
> +        // triggers the next vector.
> +        self.tree.rearm_pci_irq(DOORBELL_SUBTREE);
> +
> +        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
> +    }
> +}
> +
> +/// Everything the running self-test owns, torn down in declaration order.
> +///
> +/// That order is what every exit path, including an early error, needs: disabling the leaf stops
> +/// new deliveries, dropping the registration runs `free_irq()`, which waits for a handler still in
> +/// flight, and only then are the tree's subtrees disabled, so a late handler cannot rearm them.
> +struct SelftestResources<'a, 'r> {
> +    _leaf_guard: LeafEnableGuard<'a>,
> +    reg: Pin<KBox<irq::Registration<'r, DoorbellTestHandler<'a>>>>,
> +    _top_guard: TopEnableGuard<'a>,
> +}
> +
> +impl<'a> SelftestResources<'a, '_> {
> +    /// Returns the registered handler.
> +    fn handler(&self) -> &DoorbellTestHandler<'a> {
> +        self.reg.handler()
> +    }
> +
> +    /// Disables the doorbell source and waits for a handler already running on another CPU.
> +    ///
> +    /// On return no further delivery can reach the handler, so its counters and the doorbell
> +    /// leaf hold their final values.
> +    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.
> +///
> +/// Quiesces the interrupt tree, registers a temporary handler, and injects the doorbell vector
> +/// through the GIN software trigger twice, one delivery at a time. This validates the PCI
> +/// interrupt path from GIN to the ISR without GSP firmware, including the rearm without which only
> +/// the first interrupt would arrive. The handler, its IRQ registration, and all tree state are
> +/// torn down before this returns.
> +///
> +/// # Errors
> +///
> +/// `EINVAL` if the doorbell's subtree is not one nova-core services. `EIO` if the doorbell is
> +/// already pending before the test, if the delivery count is not two, if the doorbell bit is still
> +/// set once the source is stopped, or if either delivery found a pending bit other than the
> +/// doorbell. `ETIMEDOUT` if either delivery does not arrive within the timeout.
> +pub(crate) fn run_selftest<'a>(
> +    pdev: &'a pci::Device<Bound>,
> +    bar: Bar0<'a>,
> +    chipset: Chipset,
> +    vectors: &'a SubtreeVectors<'a>,

So here is the problem with passing `vectors`: the vectors created by
`gpu.rs` correspond to the GSP interrupt, while the selftest uses the
doorbell one. By pure coincidence they happen to be on the same subtree,
so the test receives the interrupt as expected, but that's just luck
and we shouldn't rely on that!

`run_selftest` can and should create its own (accurate) vectors and drop them in
the end so the GPU driver takes over afterwards. It's just one extra line:

  let vectors = crate::irq::alloc_vectors(pdev, DOORBELL_SUBTREE.into())?;

And with that you can also make `Tree::new()` take a `&SubtreeVectors`
instead of two `msi_type` and `serviced` arguments, making the API a bit
more consistent.

> +) -> Result {
> +    // The interrupt type decides how the handler rearms delivery, so the tree takes it from
> +    // probe's allocation.
> +    let request = vectors.request_for(DOORBELL_SUBTREE)?;
> +    let msi_type = vectors.msi_type();
> +    let tree = Tree::new(bar, chipset, msi_type, DOORBELL_SUBTREE.into())?;
> +    let doorbell = DOORBELL_VECTOR.leaf_index();
> +    let doorbell_mask = DOORBELL_VECTOR.leaf_mask();
> +
> +    // Under MSI-X the subtree index is also the table entry the delivery arrives on, so a pass
> +    // shows that the per-subtree routing works. Under MSI every subtree shares one entry.
> +    dev_info!(
> +        pdev.as_ref(),
> +        "interrupt self-test: starting on vector {}, subtree {}, with {:?}\n",
> +        DOORBELL_VECTOR.into_raw(),
> +        DOORBELL_SUBTREE.index(),
> +        msi_type,
> +    );
> +
> +    // No delivery may reach the CPU before a handler is registered, and a vector left enabled by
> +    // boot would fail the pending checks below. `drain` leaves the top level disabled.
> +    tree.disable_all_leaves();
> +    tree.drain();
> +
> +    // A delivery can be credited to the trigger below only if the vector starts out clear, so
> +    // refuse to run otherwise.
> +    let pre_pending = tree.read_pending(doorbell).vectors();
> +    if pre_pending.contains(doorbell_mask) {
> +        dev_warn!(

This is an error so this should be `dev_err`.

> +            pdev.as_ref(),
> +            "interrupt self-test: failed, vector {} already pending (leaf[{}] pending {:#x})\n",
> +            DOORBELL_VECTOR.into_raw(),
> +            doorbell.get(),
> +            pre_pending.into_raw(),
> +        );
> +        return Err(EIO);
> +    }
> +
> +    let handler_init = try_pin_init!(DoorbellTestHandler {
> +        tree,
> +        first <- Completion::new(),
> +        second <- Completion::new(),
> +        irq_count: Atomic::new(0),
> +        first_pending: Atomic::new(0),
> +        second_pending: Atomic::new(0),
> +    }? Error);
> +
> +    // Register the handler before allowing any source to fire.
> +    let reg = KBox::pin_init(
> +        // SAFETY: the registration is owned by `resources` below and dropped before this function
> +        // returns, so its `Drop` (which calls `free_irq()`) always runs and the registration is
> +        // never leaked or `mem::forget`-ed.
> +        unsafe {
> +            irq::Registration::new(
> +                request,
> +                irq::Flags::TRIGGER_NONE,
> +                c"nova-core",

Let's use `c"nova-core-selftest"` to differentiate from the driver's actual registration.

> +                handler_init,
> +            )
> +        },
> +        GFP_KERNEL,
> +    )?;
> +
> +    // From here every exit must tear down the source, the registration, and the tree. The fields
> +    // are initialized in the order the hardware requires, which is the reverse of the declaration
> +    // order that tears them down: the handler is registered above before either source is
> +    // enabled, the leaf next, and the top level last.
> +    let resources = SelftestResources {
> +        _leaf_guard: reg
> +            .handler()
> +            .tree
> +            .enable_leaf_guarded(doorbell, doorbell_mask),
> +        _top_guard: reg.handler().tree.enable_top_guarded(),
> +        reg,
> +    };
> +    let handler = resources.handler();
> +
> +    handler.tree.trigger(DOORBELL_VECTOR)?;
> +    let mut completed = handler
> +        .first
> +        .wait_for_completion_timeout(time::msecs_to_jiffies(DELIVERY_TIMEOUT_MS))
> +        .is_some();
> +
> +    // Trigger the second interrupt only once the first handler has cleared its leaf bit and
> +    // rearmed, so the two cannot coalesce into one delivery and a handler that never rearms
> +    // cannot pass.
> +    if completed {
> +        handler.tree.trigger(DOORBELL_VECTOR)?;
> +        completed = handler
> +            .second
> +            .wait_for_completion_timeout(time::msecs_to_jiffies(DELIVERY_TIMEOUT_MS))
> +            .is_some();
> +    }
> +
> +    // Stop the source and wait out any handler still running, so the values read below are the
> +    // final ones.
> +    resources.quiesce_source();
> +
> +    let count = handler.irq_count.load(Relaxed);
> +    let first_pending = LeafMask::from_raw(handler.first_pending.load(Relaxed));
> +    let second_pending = LeafMask::from_raw(handler.second_pending.load(Relaxed));
> +    let residual = handler.tree.read_pending(doorbell).vectors();
> +
> +    // The self-test runs before GSP boot on a leaf that `drain` has just cleared, and nothing
> +    // triggers the vector after the second delivery, so each delivery must find the doorbell bit
> +    // and nothing else, and the leaf must end clear.
> +    if completed
> +        && count == 2
> +        && first_pending == doorbell_mask
> +        && second_pending == doorbell_mask
> +        && !residual.contains(doorbell_mask)
> +    {
> +        dev_info!(
> +            pdev.as_ref(),
> +            "interrupt self-test: passed, subtree {}, {} deliveries\n",
> +            DOORBELL_SUBTREE.index(),
> +            count,
> +        );
> +        Ok(())
> +    } else {
> +        dev_warn!(

Here also we should use `dev_err` imho.