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>
Subject: Re: [PATCH v3 03/14] gpu: nova-core: add the GIN vector and subtree newtypes
Date: Sat, 05 Sep 2026 10:39:17 +0900 [thread overview]
Message-ID: <DL6ZZI0912ZG.2AY2OE6P0LIQZ@nvidia.com> (raw)
In-Reply-To: <20260903031514.1515905-4-jhubbard@nvidia.com>
On Thu Sep 3, 2026 at 12:15 PM JST, John Hubbard wrote:
> A GIN vector's number fixes its position in the interrupt tree: it
> latches in leaf vector / 32 at bit vector % 32, in subtree vector / 64.
> A tree implements either 8 or 16 leaves, which sets both its subtree
> count and its highest usable vector.
>
> Each of those is a bare bit pattern, so a leaf mask and a TOP bit are
> interchangeable to the compiler.
>
> Add a type for each: a vector, a leaf index, a set of vectors within one
> leaf, one subtree, a set of subtrees, and a leaf count. A vector
> converts to its own leaf, bit and subtree. A leaf count yields the
> subtree set it implements.
>
> Suggested-by: Danilo Krummrich <dakr@kernel.org>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
> drivers/gpu/nova-core/irq.rs | 11 +
> drivers/gpu/nova-core/irq/interrupt_tree.rs | 242 ++++++++++++++++++++
> drivers/gpu/nova-core/nova_core.rs | 2 +
> 3 files changed, 255 insertions(+)
> create mode 100644 drivers/gpu/nova-core/irq.rs
> create mode 100644 drivers/gpu/nova-core/irq/interrupt_tree.rs
>
> diff --git a/drivers/gpu/nova-core/irq.rs b/drivers/gpu/nova-core/irq.rs
> new file mode 100644
> index 000000000000..f27952ff747b
> --- /dev/null
> +++ b/drivers/gpu/nova-core/irq.rs
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +
> +//! GPU interrupt support.
> +//!
> +//! GIN, the GPU Interrupt and Notification unit, is the GPU's interrupt controller: a two-level
> +//! tree of pending and enable registers, one tree per PCIe function.
> +//!
> +//! See `Documentation/gpu/nova/core/interrupts.rst`.
> +
> +mod interrupt_tree;
> diff --git a/drivers/gpu/nova-core/irq/interrupt_tree.rs b/drivers/gpu/nova-core/irq/interrupt_tree.rs
> new file mode 100644
> index 000000000000..5aa447cf0ec4
> --- /dev/null
> +++ b/drivers/gpu/nova-core/irq/interrupt_tree.rs
> @@ -0,0 +1,242 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +
> +//! Vector addressing in the GIN CPU interrupt tree.
> +//!
> +//! A vector's number fixes where it latches: leaf `vector / 32` at bit `vector % 32`, and that
> +//! leaf belongs to subtree `vector / 64`. The types here keep those three views apart, so a leaf
> +//! index, a set of vectors within one leaf, and a `TOP` bit cannot stand in for one another.
> +
> +use kernel::{
> + num::Bounded,
> + prelude::*, //
> +};
> +
> +use crate::num;
> +
> +/// Number of bits a leaf index occupies, covering the `0..16` leaf register arrays.
> +const LEAF_INDEX_BITS: u32 = 4;
These constant declarations are a bit inconsistent - we are using number
of bits here, number of elements there. Let's harmonize on number of
elements and use `ilog2()` to convert to number of bits where needed.
Consequently, this constant can be removed.
> +
> +/// Index of a leaf register, bounded to the `0..16` range covered by the leaf register arrays.
> +pub(super) type LeafIndex = Bounded<usize, LEAF_INDEX_BITS>;
Let's group this together with the other types (for instance, before
`LeafCount`), and use `Bounded<usize, { MAX_NUM_LEAVES.ilog2() }>`.
(see below for `MAX_NUM_LEAVES`)
> +
> +/// Number of vectors one leaf register carries, one per bit.
> +const VECTORS_PER_LEAF: u32 = 32;
Let's use `u32::BITS` here.
> +
> +/// Number of leaves one subtree covers.
> +const LEAVES_PER_SUBTREE: u32 = 2;
And with the following two constants we have everything we need to
derive the rest:
/// Maximum number of subtrees.
const MAX_NUM_SUBTREES: u32 = 8;
/// Maximum number of leaves.
const MAX_NUM_LEAVES: u32 = MAX_NUM_SUBTREES * LEAVES_PER_SUBTREE;
> +
> +/// Number of bits that address any vector the widest supported tree carries.
> +const VECTOR_BITS: u32 = 9;
We can now derive this one as:
const VECTOR_BITS: u32 = (MAX_NUM_LEAVES * VECTORS_PER_LEAF).ilog2();
> +
> +const _: () = assert!(1 << VECTOR_BITS == LeafCount::Sixteen.vector_count());
You will want to use `static_assert` here. I'd also suggest moving this
to after the declaration of `LeafCount` (since that's what it tests),
and adding a comment to explain why we do this integrity check.
> +
> +/// Width of the vector field in the leaf trigger register.
> +const TRIGGER_VECTOR_BITS: u32 = 12;
Let's also derive from the register's constants:
const TRIGGER_VECTOR_BITS: u32 = {
let range = NV_VIRTUAL_FUNCTION_PRIV_CPU_INTR_LEAF_TRIGGER::VECTOR_RANGE;
num::u8_as_u32(*range.end() - *range.start() + 1)
};
This would need to be introduced in the next patch since the register
doesn't exist yet, along with the conversion to it from `GinVector`, but
that's actually the right time to introduce these.
> +
> +/// Number of leaves a tree implements.
> +///
> +/// Every supported part implements one of these two counts, and the interrupt HAL names the one
> +/// its architecture uses.
> +#[derive(Clone, Copy, Debug, Eq, PartialEq)]
> +#[repr(usize)]
> +pub(super) enum LeafCount {
> + /// Turing through Ada.
> + Eight = 8,
> +
> + /// Hopper and later.
> + Sixteen = 16,
> +}
> +
> +impl LeafCount {
> + /// Returns the number of leaves.
> + pub(super) const fn into_u32(self) -> u32 {
> + // CAST: both discriminants are 16 or below.
> + self as u32
> + }
> +
> + /// Returns the number of leaves, in the type that indexes the leaf register arrays.
> + pub(super) const fn into_raw(self) -> usize {
> + num::u32_as_usize(self.into_u32())
> + }
> +
> + /// Returns the number of subtrees, each of which covers two leaves.
> + pub(super) const fn subtree_count(self) -> u32 {
> + self.into_u32() / LEAVES_PER_SUBTREE
> + }
> +
> + /// Returns the set of every subtree a tree of this size implements.
> + pub(super) const fn subtree_set(self) -> SubtreeSet {
> + SubtreeSet((1u32 << self.subtree_count()) - 1)
> + }
> +
> + /// Returns the number of vectors a tree of this size carries.
> + pub(super) const fn vector_count(self) -> u32 {
> + self.into_u32() * VECTORS_PER_LEAF
> + }
> +}
> +
> +/// Set of vectors within one leaf, one bit per vector.
> +#[derive(Clone, Copy, Debug, Eq, PartialEq)]
> +pub(super) struct LeafMask(u32);
> +
> +impl LeafMask {
> + /// Returns the mask with every vector of the leaf set.
> + pub(super) const fn all() -> Self {
> + Self(u32::MAX)
> + }
> +
> + /// Returns the mask holding the vectors set in `raw`.
> + pub(super) const fn from_raw(raw: u32) -> Self {
> + Self(raw)
> + }
> +
> + /// Returns the mask as the value the leaf registers take.
> + pub(super) const fn into_raw(self) -> u32 {
> + self.0
> + }
> +
> + /// Returns whether no vector is set.
> + pub(super) const fn is_empty(self) -> bool {
> + self.0 == 0
> + }
> +
> + /// Returns whether every vector set in `other` is also set here.
> + pub(super) const fn contains(self, other: Self) -> bool {
> + self.0 & other.0 == other.0
> + }
> +}
> +
> +/// One subtree, named by its `TOP` bit.
> +///
> +/// # Invariants
> +///
> +/// Exactly one bit is set.
> +#[derive(Clone, Copy, Debug, Eq, PartialEq)]
> +pub(super) struct Subtree(u32);
> +
> +impl Subtree {
Every time we build a `Subtree` we need an `// INVARIANT:` block. Let's
add and use a constructor to enforce the invariant from a single place.
const fn new(idx: u32) -> Self {
// INVARIANT: a shift of `1` leaves exactly one bit set.
Self(1 << idx)
}
The constructor can remain private.
> + /// Returns this subtree's index within the tree.
> + ///
> + /// Under MSI-X this is also the index of the allocated entry the subtree raises.
> + pub(super) const fn index(self) -> u32 {
> + self.0.trailing_zeros()
> + }
> +
> + /// Returns the subtree as the value the `TOP` enable registers take.
> + pub(super) const fn into_raw(self) -> u32 {
> + self.0
> + }
> +}
> +
> +/// Set of subtrees, one bit per subtree, in the layout the `TOP` enable registers take.
> +#[derive(Clone, Copy, Debug, Eq, PartialEq)]
> +pub(super) struct SubtreeSet(u32);
> +
> +impl SubtreeSet {
> + /// Returns whether `subtree` belongs to this set.
> + pub(super) const fn contains(self, subtree: Subtree) -> bool {
> + self.0 & subtree.into_raw() != 0
> + }
> +
> + /// Returns whether the set holds no subtree.
> + pub(super) const fn is_empty(self) -> bool {
> + self.0 == 0
> + }
> +
> + /// Returns the subtrees present in both sets.
> + pub(super) const fn intersection(self, other: Self) -> Self {
> + Self(self.0 & other.0)
> + }
> +
> + /// Returns the number of subtrees counted from subtree `0` through the highest one in this
> + /// set, which is `0` for an empty set.
> + pub(super) const fn span(self) -> u32 {
> + u32::BITS - self.0.leading_zeros()
> + }
> +}
> +
> +impl From<Subtree> for SubtreeSet {
> + fn from(subtree: Subtree) -> Self {
> + Self(subtree.into_raw())
> + }
> +}
> +
> +/// A GIN interrupt vector, bounded to the widest tree any supported part implements.
> +#[derive(Clone, Copy, Debug, Eq, PartialEq)]
> +pub(super) struct GinVector(Bounded<u32, VECTOR_BITS>);
I was contemplating that maybe we could turn this type into a bitfield,
since that's really what it is and its methods do bit manipulation, but
hit a wall due to `const` requirements that cannot be met. Just
mentioning it before someone else spends their time on the same idea. :)
> +
> +impl GinVector {
> + /// Returns the vector numbered `VECTOR`.
> + ///
> + /// Fails at build time if `VECTOR` lies outside the widest tree any supported part
> + /// implements.
> + pub(super) const fn new<const VECTOR: u32>() -> Self {
> + Self(Bounded::<u32, VECTOR_BITS>::new::<VECTOR>())
> + }
> +
> + /// Returns the vector number.
> + pub(super) const fn into_raw(self) -> u32 {
> + self.0.get()
> + }
> +
> + /// Returns the leaf that carries this vector.
> + pub(super) fn leaf_index(self) -> LeafIndex {
> + // CALC: `self.0 / VECTORS_PER_LEAF`.
> + self.0.shr::<{ VECTORS_PER_LEAF.ilog2() }, _>().cast()
> + }
> +
> + /// Returns this vector's bit within its leaf.
> + pub(super) const fn leaf_mask(self) -> LeafMask {
> + LeafMask(1 << (self.0.get() % VECTORS_PER_LEAF))
> + }
> +
> + /// Returns the subtree that carries this vector.
> + pub(super) const fn subtree(self) -> Subtree {
> + // INVARIANT: a shift of `1` leaves exactly one bit set.
> + Subtree(1 << (self.0.get() / (VECTORS_PER_LEAF * LEAVES_PER_SUBTREE)))
Here I wanted to use `Bounded::shr` as well, but we would lose the
`const` and we need it... Can't wait for const ops traits. :( That's
also why we cannot turn `GinVector` into a regular bitfield.
> + }
> +
> + /// Checks that this vector lies within a tree of `leaves` leaves.
> + ///
> + /// # Errors
> + ///
> + /// `EINVAL` if the vector lies beyond the last leaf such a tree implements.
> + pub(super) const fn validate(self, leaves: LeafCount) -> Result {
> + if self.0.get() >= leaves.vector_count() {
> + return Err(EINVAL);
> + }
> +
> + Ok(())
> + }
> +}
> +
> +impl From<Bounded<u32, 32>> for LeafMask {
> + fn from(vectors: Bounded<u32, 32>) -> Self {
> + Self(vectors.get())
> + }
> +}
> +
> +impl From<LeafMask> for Bounded<u32, 32> {
> + fn from(vectors: LeafMask) -> Self {
> + vectors.0.into()
> + }
> +}
> +
> +impl From<Bounded<u32, 32>> for SubtreeSet {
> + fn from(subtrees: Bounded<u32, 32>) -> Self {
> + Self(subtrees.get())
> + }
> +}
> +
> +impl From<SubtreeSet> for Bounded<u32, 32> {
> + fn from(subtrees: SubtreeSet) -> Self {
> + subtrees.0.into()
> + }
> +}
> +
> +impl From<GinVector> for Bounded<u32, TRIGGER_VECTOR_BITS> {
> + fn from(vector: GinVector) -> Self {
> + vector.0.extend()
> + }
> +}
Let's keep the impl blocks for a given type grouped together.
next prev parent reply other threads:[~2026-09-05 1:39 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-03 3:14 [PATCH v3 00/14] nova-core: GPU interrupt support and GSP event delivery John Hubbard
2026-09-03 3:15 ` [PATCH v3 01/14] rust: pci: declare IrqType and IrqTypes with impl_flags John Hubbard
2026-09-03 3:15 ` [PATCH v3 02/14] rust: sync: completion: add wait_for_completion_timeout() John Hubbard
2026-09-03 3:15 ` [PATCH v3 03/14] gpu: nova-core: add the GIN vector and subtree newtypes John Hubbard
2026-09-05 1:39 ` Alexandre Courbot [this message]
2026-09-03 3:15 ` [PATCH v3 04/14] gpu: nova-core: add the GIN CPU interrupt tree and MSI EOI registers John Hubbard
2026-09-03 3:15 ` [PATCH v3 05/14] gpu: nova-core: add the per-architecture GIN CPU interrupt HAL John Hubbard
2026-09-03 3:15 ` [PATCH v3 06/14] gpu: nova-core: add the GIN interrupt tree and allocate its vectors John Hubbard
2026-09-03 3:15 ` [PATCH v3 07/14] gpu: nova-core: add an interrupt delivery self-test John Hubbard
2026-09-03 3:29 ` sashiko-bot
2026-09-03 3:57 ` John Hubbard
2026-09-03 3:15 ` [PATCH v3 08/14] gpu: nova-core: log GSP events instead of discarding them John Hubbard
2026-09-03 3:15 ` [PATCH v3 09/14] gpu: nova-core: recover the GSP receive path from corrupt framing John Hubbard
2026-09-04 10:53 ` Alexandre Courbot
2026-09-04 11:17 ` Gary Guo
2026-09-04 13:45 ` Alexandre Courbot
2026-09-03 3:15 ` [PATCH v3 10/14] gpu: nova-core: bound a GSP wait by a single deadline John Hubbard
2026-09-04 11:13 ` Alexandre Courbot
2026-09-04 11:26 ` Gary Guo
2026-09-04 13:32 ` Alexandre Courbot
2026-09-04 13:41 ` Gary Guo
2026-09-03 3:15 ` [PATCH v3 11/14] gpu: nova-core: add the falcon interrupt status and routing registers John Hubbard
2026-09-03 3:15 ` [PATCH v3 12/14] gpu: nova-core: drive GSP events with the SWGEN0 interrupt John Hubbard
2026-09-03 3:28 ` sashiko-bot
2026-09-03 3:55 ` John Hubbard
2026-09-04 1:53 ` John Hubbard
2026-09-03 3:15 ` [PATCH v3 13/14] gpu: nova-core: add KUnit tests for the interrupt tree and HALs John Hubbard
2026-09-03 3:15 ` [PATCH v3 14/14] gpu: nova-core: document the GIN interrupt controller and GSP events John Hubbard
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=DL6ZZI0912ZG.2AY2OE6P0LIQZ@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=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