Re: [PATCH 03/17] rust: pci: expose the allocated interrupt type

["Danilo Krummrich" <dakr@kernel.org>]

On Sun Aug 9, 2026 at 11:42 PM CEST, John Hubbard wrote:
> Yes, please. Then my patches 2 and 3 collapse into a single patch that
> adds count(), irq_type() and an index-to-IrqVector accessor. Or, they go
> away entirely if you end up putting those on the type yourself.

I sent out a patch series [1] for this, applied your nova-core patches on top of
it and resolved the conflicts with the diff below (also pushed a branch in [2]).

Note that I optimized for a clean diff and not for optimal code. I think there
are more improvements we can make; I will comment on the corresponding patches
of this series.

[1] https://lore.kernel.org/all/20260810224800.2314458-1-dakr@kernel.org/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=nova/irq

diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index 3fbf117a99ee..21d7df0744ed 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -20,7 +20,10 @@

 use crate::{
     gpu::Gpu,
-    irq::gsp::GspIrq, //
+    irq::{
+        gsp::GspIrq,
+        SubtreeVectors, //
+    },
 };

 /// Counter for generating unique auxiliary device IDs.
@@ -39,6 +42,12 @@ pub(crate) struct NovaCore<'bound> {
     bar: pci::Bar<'bound, BAR0_SIZE>,
     #[allow(clippy::type_complexity)]
     _reg: auxiliary::Registration<'bound, ForLt!(())>,
+    /// 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: &'bound SubtreeVectors<'bound>,
+    /// PCI interrupt vector allocation. Dropped last (struct field drop order).
+    #[pin]
+    vectors: SubtreeVectors<'bound>,
 }

 pub(crate) struct NovaCoreDriver;
@@ -87,38 +96,34 @@ fn probe<'bound>(
             pdev.enable_device_mem()?;
             pdev.set_master();

-            // A PCI device has one interrupt vector allocation, so it is made here for every
-            // subtree nova-core services, and each handler takes the vector for its own subtree.
-            let vectors = crate::irq::alloc_vectors(pdev, crate::irq::gsp::GSP_SUBTREE)?;
-            let gsp_vector = vectors.vector_for(crate::irq::gsp::GSP_SUBTREE)?;
-            let irq_type = vectors.irq_type();
-
             Ok(try_pin_init!(NovaCore {
+                vectors: crate::irq::alloc_vectors(pdev, crate::irq::gsp::GSP_SUBTREE)?,
+                // SAFETY: `vectors` is initialized above, lives at a pinned stable address, and
+                // is dropped after all fields that use `vectors_ref` (struct field drop order).
+                vectors_ref: unsafe { &*core::ptr::from_ref(vectors.as_ref().get_ref()) },
                 bar: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"nova-core/bar0")?,
                 // TODO: Use `&bar` self-referential pin-init syntax once available.
                 //
                 // SAFETY: `bar` is initialized before this expression is evaluated
                 // (`try_pin_init!()` initializes fields in the order they appear here), lives at a
                 // pinned stable address, and is dropped after `gpu` (struct field drop order).
-                gpu <- Gpu::new(pdev, unsafe { &*core::ptr::from_ref(bar) }, vectors),
+                gpu <- Gpu::new(pdev, unsafe { &*core::ptr::from_ref(bar) }, vectors_ref),
                 // Quiesce the interrupt tree before registering the handler below.
                 _: {
                     // SAFETY: as for the `bar` borrow above.
                     let bar = unsafe { &*core::ptr::from_ref(bar) };
-                    crate::irq::gsp::quiesce(bar, gpu.chipset(), irq_type);
+                    crate::irq::gsp::quiesce(bar, gpu.chipset(), vectors_ref.irq_type());
                 },
                 // Register the permanent GSP SWGEN0 handler before enabling the interrupt.
                 //
-                // SAFETY: `bar` is initialized before this expression is evaluated, lives at a
-                // pinned stable address, and is dropped after `_gsp_irq` (declared first, so
-                // dropped first), so the handler's borrow stays valid for its whole lifetime.
-                // `_gsp_irq` is stored in `NovaCore`, whose `Drop` runs `free_irq`, so the
-                // registration is never leaked.
+                // SAFETY: `bar` and `vectors` are initialized and pinned (see above). `_gsp_irq`
+                // is declared before `vectors` in the struct, so it is dropped first, ensuring
+                // `free_irq` runs before the vectors are freed. The registration is stored in
+                // `NovaCore` and never leaked.
                 _gsp_irq <- unsafe {
                     GspIrq::new(
                         pdev,
-                        gsp_vector,
-                        irq_type,
+                        vectors_ref,
                         &*core::ptr::from_ref(bar),
                         gpu.cmdq(),
                         gpu.chipset(),
@@ -129,7 +134,7 @@ fn probe<'bound>(
                 _: {
                     // SAFETY: as for the `bar` borrow above.
                     let bar = unsafe { &*core::ptr::from_ref(bar) };
-                    crate::irq::gsp::enable(bar, gpu.chipset(), irq_type);
+                    crate::irq::gsp::enable(bar, gpu.chipset(), vectors_ref.irq_type());
                     gpu.cmdq().drain()?;
                 },
                 _reg: auxiliary::Registration::new(
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 9700eff6db86..11a66a597298 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -339,7 +339,7 @@ pub(crate) fn cmdq(&self) -> Arc<Cmdq> {
     pub(crate) fn new(
         pdev: &'gpu pci::Device<device::Core<'_>>,
         bar: Bar0<'gpu>,
-        vectors: SubtreeVectors<'gpu>,
+        vectors: &'gpu SubtreeVectors<'gpu>,
     ) -> impl PinInit<Self, Error> + 'gpu {
         let dev = pdev.as_ref();

diff --git a/drivers/gpu/nova-core/irq.rs b/drivers/gpu/nova-core/irq.rs
index ddf322f2e623..6f2ab85ccdc9 100644
--- a/drivers/gpu/nova-core/irq.rs
+++ b/drivers/gpu/nova-core/irq.rs
@@ -16,6 +16,7 @@

 use kernel::{
     device::Bound,
+    irq,
     pci::{
         self,
         IrqType,
@@ -28,9 +29,8 @@
 ///
 /// MSI-X raises a separate table entry per subtree, so subtree `N` arrives on entry `N`. MSI has a
 /// single message that every subtree raises, so all of them arrive on the one allocated entry.
-#[derive(Clone, Copy)]
 pub(crate) struct SubtreeVectors<'a> {
-    vectors: pci::IrqAllocation<'a>,
+    vectors: pci::IrqVectorRegistration<'a>,
     /// `TOP` bit of every subtree nova-core services.
     serviced: u32,
 }
@@ -41,18 +41,21 @@ pub(crate) fn irq_type(&self) -> IrqType {
         self.vectors.irq_type()
     }

-    /// Returns the vector that delivers `subtree`, a single `TOP` bit of the form
-    /// `interrupt_tree::vector_subtree_mask` returns.
+    /// Returns an [`irq::IrqRequest`] for the vector that delivers `subtree`.
     ///
     /// # Errors
     ///
     /// `EINVAL` if `subtree` names anything other than a single subtree nova-core services.
-    pub(crate) fn vector_for(&self, subtree: u32) -> Result<pci::IrqVector<'a>> {
+    pub(crate) fn request_for(
+        &self,
+        subtree: u32,
+    ) -> Result<irq::IrqRequest<'_, &'_ pci::IrqVectorRegistration<'_>>> {
         if subtree.count_ones() != 1 || subtree & self.serviced == 0 {
             return Err(EINVAL);
         }

-        self.vectors.vector(entry_index(self.irq_type(), subtree))
+        self.vectors
+            .request(entry_index(self.irq_type(), subtree) as usize)
     }
 }

diff --git a/drivers/gpu/nova-core/irq/doorbell_test.rs b/drivers/gpu/nova-core/irq/doorbell_test.rs
index bfdfee732892..2c123d0cdc52 100644
--- a/drivers/gpu/nova-core/irq/doorbell_test.rs
+++ b/drivers/gpu/nova-core/irq/doorbell_test.rs
@@ -144,7 +144,13 @@ struct SelftestGuard<'a, 'r> {
     bar: Bar0<'a>,
     tree: Tree,
     doorbell: LeafIndex,
-    reg: Option<Pin<KBox<irq::Registration<'r, DoorbellTestHandler<'a>>>>>,
+    reg: Option<
+        Pin<
+            KBox<
+                irq::Registration<'r, DoorbellTestHandler<'a>, &'r pci::IrqVectorRegistration<'r>>,
+            >,
+        >,
+    >,
 }

 impl<'a, 'r> SelftestGuard<'a, 'r> {
@@ -195,11 +201,11 @@ pub(crate) fn run_selftest<'a>(
     pdev: &'a pci::Device<Bound>,
     bar: Bar0<'a>,
     chipset: Chipset,
-    vectors: SubtreeVectors<'_>,
+    vectors: &'a SubtreeVectors<'a>,
 ) -> Result {
     // The interrupt type decides how the handler rearms delivery, so the tree takes it from
     // probe's allocation.
-    let vector = vectors.vector_for(DOORBELL_SUBTREE)?;
+    let request = vectors.request_for(DOORBELL_SUBTREE)?;
     let irq_type = vectors.irq_type();
     let tree = Tree::new(chipset, irq_type, DOORBELL_SUBTREE);
     let doorbell = LeafIndex::new::<DOORBELL_LEAF>();
@@ -252,7 +258,14 @@ pub(crate) fn run_selftest<'a>(
         // SAFETY: the registration is owned by `guard` 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 { pdev.request_irq(vector, irq::Flags::TRIGGER_NONE, c"nova-core", handler_init) },
+        unsafe {
+            irq::Registration::new(
+                request,
+                irq::Flags::TRIGGER_NONE,
+                c"nova-core",
+                handler_init,
+            )
+        },
         GFP_KERNEL,
     )?;

diff --git a/drivers/gpu/nova-core/irq/gsp.rs b/drivers/gpu/nova-core/irq/gsp.rs
index ecd716b92d4e..558f944c4c0d 100644
--- a/drivers/gpu/nova-core/irq/gsp.rs
+++ b/drivers/gpu/nova-core/irq/gsp.rs
@@ -202,22 +202,21 @@ fn handle_threaded(&self) -> irq::IrqReturn {
 #[pin_data(PinnedDrop)]
 pub(crate) struct GspIrq<'a> {
     #[pin]
-    reg: irq::ThreadedRegistration<'a, GspInterrupt<'a>>,
+    reg: irq::ThreadedRegistration<'a, GspInterrupt<'a>, &'a pci::IrqVectorRegistration<'a>>,
     /// Borrowed BAR0 and the interrupt tree, used by the teardown to disable the GSP source.
     bar: Bar0<'a>,
     tree: Tree,
 }

 impl<'a> GspIrq<'a> {
-    /// Registers the GSP SWGEN0 threaded handler on `vector`.
+    /// Registers the GSP SWGEN0 threaded handler for the GSP subtree in `vectors`.
     ///
     /// # Safety
     ///
     /// The caller must not leak the returned value: its [`Drop`] runs `free_irq`.
     pub(crate) unsafe fn new(
         pdev: &'a pci::Device<device::Bound>,
-        vector: pci::IrqVector<'a>,
-        irq_type: pci::IrqType,
+        vectors: &'a super::SubtreeVectors<'a>,
         bar: Bar0<'a>,
         cmdq: Arc<Cmdq>,
         chipset: Chipset,
@@ -227,15 +226,15 @@ pub(crate) unsafe fn new(
             // SAFETY: the caller guarantees the returned `GspIrq` is not leaked, so this
             // registration's `Drop` (`free_irq`) always runs.
             reg <- unsafe {
-                pdev.request_threaded_irq(
-                    vector,
+                irq::ThreadedRegistration::new(
+                    vectors.request_for(GSP_SUBTREE)?,
                     irq::Flags::TRIGGER_NONE,
                     c"nova-core",
-                    GspInterrupt::new(bar, cmdq, chipset, irq_type, dev),
+                    GspInterrupt::new(bar, cmdq, chipset, vectors.irq_type(), dev),
                 )
             },
             bar,
-            tree: Tree::new(chipset, irq_type, GSP_SUBTREE),
+            tree: Tree::new(chipset, vectors.irq_type(), GSP_SUBTREE),
         })
     }
 }