From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "John Hubbard" <jhubbard@nvidia.com>
Cc: "Danilo Krummrich" <dakr@kernel.org>,
"Timur Tabi" <ttabi@nvidia.com>,
"Alistair Popple" <apopple@nvidia.com>,
"Eliot Courtney" <ecourtney@nvidia.com>,
"Zhi Wang" <zhiw@nvidia.com>, "David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Alex Gaynor" <alex.gaynor@gmail.com>,
"Boqun Feng" <boqun.feng@gmail.com>,
"Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <lossin@kernel.org>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"Trevor Gross" <tmgross@umich.edu>,
nova-gpu@lists.linux.dev, LKML <linux-kernel@vger.kernel.org>,
"Joel Fernandes" <joelagnelf@nvidia.com>
Subject: Re: [PATCH v4 08/17] gpu: nova-core: add an interrupt delivery self-test
Date: Mon, 21 Sep 2026 15:35:39 +0900 [thread overview]
Message-ID: <DLKSB4FK3UEE.531C393VK0ZV@nvidia.com> (raw)
In-Reply-To: <20260912044400.677097-9-jhubbard@nvidia.com>
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@nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
> 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.
next prev parent reply other threads:[~2026-09-21 6:35 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-12 4:43 [PATCH v4 00/17] nova-core: GPU interrupt support and GSP event delivery John Hubbard
2026-09-12 4:43 ` [PATCH v4 01/17] rust: pci: declare IrqType and IrqTypes with impl_flags John Hubbard
2026-09-12 4:43 ` [PATCH v4 02/17] rust: sync: completion: add wait_for_completion_timeout() John Hubbard
2026-09-12 4:43 ` [PATCH v4 03/17] gpu: nova-core: add the GIN vector, leaf and subtree types John Hubbard
2026-09-21 6:34 ` Alexandre Courbot
2026-09-12 4:43 ` [PATCH v4 04/17] gpu: nova-core: add the GIN CPU interrupt tree and MSI EOI registers John Hubbard
2026-09-12 4:43 ` [PATCH v4 05/17] gpu: nova-core: add the per-architecture GIN CPU interrupt HAL John Hubbard
2026-09-12 4:43 ` [PATCH v4 06/17] gpu: nova-core: add the GIN interrupt tree and allocate its vectors John Hubbard
2026-09-21 6:35 ` Alexandre Courbot
2026-09-12 4:43 ` [PATCH v4 07/17] gpu: nova-core: wait for GFW boot in probe, not in the Gpu constructor John Hubbard
2026-09-21 6:35 ` Alexandre Courbot
2026-09-12 4:43 ` [PATCH v4 08/17] gpu: nova-core: add an interrupt delivery self-test John Hubbard
2026-09-21 6:35 ` Alexandre Courbot [this message]
2026-09-12 4:43 ` [PATCH v4 09/17] gpu: nova-core: log GSP events instead of discarding them John Hubbard
2026-09-12 4:43 ` [PATCH v4 10/17] gpu: nova-core: stop re-parsing a bad GSP message John Hubbard
2026-09-21 6:32 ` Alexandre Courbot
2026-09-12 4:43 ` [PATCH v4 11/17] gpu: nova-core: return ENOMSG for an unmatched " John Hubbard
2026-09-12 4:43 ` [PATCH v4 12/17] gpu: nova-core: bound a GSP wait by a single deadline John Hubbard
2026-09-12 4:43 ` [PATCH v4 13/17] gpu: nova-core: add a GSP message queue drain John Hubbard
2026-09-12 4:43 ` [PATCH v4 14/17] gpu: nova-core: add the falcon interrupt registers and their HAL John Hubbard
2026-09-21 6:37 ` Alexandre Courbot
2026-09-12 4:43 ` [PATCH v4 15/17] gpu: nova-core: service GSP events from the SWGEN0 interrupt John Hubbard
2026-09-21 6:46 ` Alexandre Courbot
2026-09-21 7:20 ` Alexandre Courbot
2026-09-12 4:43 ` [PATCH v4 16/17] gpu: nova-core: add KUnit tests for the interrupt tree and HALs John Hubbard
2026-09-21 6:47 ` Alexandre Courbot
2026-09-12 4:44 ` [PATCH v4 17/17] gpu: nova-core: document the GIN interrupt controller and GSP events John Hubbard
2026-09-21 6:59 ` [PATCH v4 00/17] nova-core: GPU interrupt support and GSP event delivery Alexandre Courbot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DLKSB4FK3UEE.531C393VK0ZV@nvidia.com \
--to=acourbot@nvidia.com \
--cc=a.hindborg@kernel.org \
--cc=airlied@gmail.com \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=apopple@nvidia.com \
--cc=bhelgaas@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=dakr@kernel.org \
--cc=ecourtney@nvidia.com \
--cc=gary@garyguo.net \
--cc=jhubbard@nvidia.com \
--cc=joelagnelf@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=nova-gpu@lists.linux.dev \
--cc=ojeda@kernel.org \
--cc=simona@ffwll.ch \
--cc=tmgross@umich.edu \
--cc=ttabi@nvidia.com \
--cc=zhiw@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®