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

"Danilo Krummrich" <[email protected]>
Newsgroups dev.linux.lists.nova-gpu,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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/[email protected]/
[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),
         })
     }
 }
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.