Re: [PATCH v2 04/15] gpu: nova-core: add the GIN vector and subtree newtypes

["Alexandre Courbot" <acourbot@nvidia.com>]

On Sat Aug 29, 2026 at 10:22 AM 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                |   1 +
>  drivers/gpu/nova-core/irq/interrupt_tree.rs | 209 ++++++++++++++++++++
>  drivers/gpu/nova-core/nova_core.rs          |   1 +
>  3 files changed, 211 insertions(+)
>  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
> index 6656a1a23d59..3066ceeb850c 100644
> --- a/drivers/gpu/nova-core/irq.rs
> +++ b/drivers/gpu/nova-core/irq.rs
> @@ -8,4 +8,5 @@
>  //!
>  //! See `Documentation/gpu/nova/core/interrupts.rst`.
>  
> +mod interrupt_tree;
>  mod regs;
> 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..da24f3d35893
> --- /dev/null
> +++ b/drivers/gpu/nova-core/irq/interrupt_tree.rs
> @@ -0,0 +1,209 @@
> +// 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.

The dedicated types are really nice. Looking at the full series, I think
we can even push them further and unleash more of the guarantees they
give us. I'll comment on the relevant bits of code in the patches, but
one obvious instance is that the registers defined in patch 3 could use
these types for their fields, instead of having to convert them into raw
values and back again (which partially defeats the purpose of using
dedicated types as you can write e.g. a `SubtreeSet` into a register
meant to represent a `LeafMask`). For instance:

    pub(super) NV_VIRTUAL_FUNCTION_PRIV_CPU_INTR_LEAF(u32)[16] @ 0x00b81000 {
        31:0   vectors => LeafMask;
    }

    ...

    pub(super) NV_VIRTUAL_FUNCTION_PRIV_CPU_INTR_TOP_EN_SET(u32) @ 0x00b81608 {
        31:0   subtrees => SubtreeSet;
    }

This unleashes some code cleanups in the patches ahead - I'll comment on
them as I come across them. But basically this means that I think patch
3 should come *after* this one, so the registers can be defined with
their fields from the get-go.

This also means you will need simple implementations to convert LeafMask
and SubtreeSet from and to `Bounded<u32, 32>`, but they're as trivial as
they come, e.g.:

    impl From<Bounded<u32, 32>> for LeafMask {
        fn from(value: Bounded<u32, 32>) -> Self {
            Self(value.into())
        }
    }

    impl From<LeafMask> for Bounded<u32, 32> {
        fn from(value: LeafMask) -> Self {
            Bounded::from(value.0)
        }
    }

> +
> +use kernel::{
> +    num::Bounded,
> +    prelude::*, //
> +};
> +
> +/// Index of a leaf register, bounded to the `0..16` range covered by the leaf register arrays.
> +pub(super) type LeafIndex = Bounded<usize, 4>;
> +
> +/// Number of vectors one leaf register carries, one per bit.
> +const VECTORS_PER_LEAF: u32 = 32;
> +
> +/// Number of leaves one subtree covers.
> +const LEAVES_PER_SUBTREE: u32 = 2;
> +
> +/// Mask that bounds a leaf index to the leaf register arrays.
> +const LEAF_INDEX_MASK: usize = LeafCount::Sixteen.into_raw() - 1;
> +
> +/// 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_raw(self) -> usize {
> +        self as usize

`// CAST:` comment needed here - it just needs to state the obvious
though. :)

> +    }
> +
> +    /// Returns the number of subtrees, each of which covers two leaves.
> +    pub(super) const fn subtree_count(self) -> u32 {
> +        self as u32 / LEAVES_PER_SUBTREE

Here as well.

> +    }
> +
> +    /// 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 as u32 * VECTORS_PER_LEAF

And here as well - maybe we can have a `into_u32()` helper that does the
cast, and then `into_raw` can call `u32_as_usize` on it to avoid having
a `CAST` comment itself?

> +    }
> +}
> +
> +/// 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 {
> +    /// 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()
> +    }
> +
> +    /// Returns the set as the value the `TOP` enable registers take.
> +    pub(super) const fn into_raw(self) -> u32 {
> +        self.0
> +    }
> +}
> +
> +impl From<Subtree> for SubtreeSet {
> +    fn from(subtree: Subtree) -> Self {
> +        Self(subtree.into_raw())
> +    }
> +}
> +
> +/// A GIN interrupt vector.
> +///
> +/// # Invariants
> +///
> +/// The vector lies within the widest tree any supported part implements.
> +#[derive(Clone, Copy, Debug, Eq, PartialEq)]
> +pub(super) struct GinVector(u32);

Since the maximum number of vectors is 512, and as
`NV_VIRTUAL_FUNCTION_PRIV_CPU_INTR_LEAF_TRIGGER` is 12 bits, how about
storing the inner value as a `Bounded<u32, 9>`? This enforces the < 512
invariant using the type system and saves us one `try_with_vector` down
the road when we write `NV_VIRTUAL_FUNCTION_PRIV_CPU_INTR_LEAF_TRIGGER`.

> +
> +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 {
> +        build_assert!(VECTOR < LeafCount::Sixteen.vector_count());

This can be a `const_assert!` as it works with a generic argument.
Although I guess we won't need this anymore if we use `Bounded<u32,
9>`. The `Invariants` section of `GinVector` could also be dropped.

> +
> +        // INVARIANT: `VECTOR` is within the widest supported tree.
> +        Self(VECTOR)
> +    }
> +
> +    /// Returns the vector number.
> +    pub(super) const fn into_raw(self) -> u32 {
> +        self.0
> +    }
> +
> +    /// Returns the leaf that carries this vector.
> +    pub(super) fn leaf_index(self) -> LeafIndex {
> +        // By the type invariant the quotient is already below 16, so the mask changes nothing. It
> +        // is what proves the bound to `from_expr`.
> +        LeafIndex::from_expr(crate::num::u32_as_usize(self.0 / VECTORS_PER_LEAF) & LEAF_INDEX_MASK)

Let's `use crate::num` so these prefixes become just `num::` as there are
quite a few instances in later patches as well.