Re: [PATCH v3 03/14] gpu: nova-core: add the GIN vector and subtree newtypes

On Thu Sep 3, 2026 at 12:15 PM JST, John Hubbard wrote:

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.