[PATCH v2 0/5] Rework PCI IRQ vector code

[PATCH v2 0/5] Rework PCI IRQ vector code

[Danilo Krummrich]

This series reworks the Rust PCI interrupt vector abstractions, motivated by
review feedback on the nova-core interrupt support series [1].

Convert IrqVectorRegistration to a lifetime-managed owning type, replacing the
devres-based approach. Since vector() borrows the registration, the returned
IrqVector inherits that lifetime, preventing the allocation from being dropped
while any handler is live.

IrqVector embeds a resolved IrqRequest, making the conversion infallible. The
request_irq()/request_threaded_irq() wrappers on Device are removed since their
&self receiver could refer to an unrelated device.

Add pci_irq_type() as a C function in include/linux/pci.h, replacing open-coded
checks across drivers [2], and wrap it for Rust.

[1] https://lore.kernel.org/all/20260808031120.363869-1-jhubbard@nvidia.com/
[2] https://elixir.bootlin.com/linux/v7.1/source/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c#L196

Changes in v2:
  - Drop the IrqRequestAnchor approach and keep IrqVector as a new type over
    IrqRequest.

Danilo Krummrich (5):
  rust: pci: convert IrqVectorRegistration to a lifetime-managed owning
    type
  rust: pci: resolve IRQ in vector() and embed IrqRequest in IrqVector
  rust: pci: remove request_irq() and request_threaded_irq() from Device
  PCI: add pci_irq_type() to query the allocated interrupt type
  rust: pci: expose the allocated interrupt type

 include/linux/pci.h    |  25 +++++
 rust/helpers/pci.c     |   5 +
 rust/kernel/pci.rs     |   3 +-
 rust/kernel/pci/irq.rs | 234 ++++++++++++++++++++---------------------
 4 files changed, 144 insertions(+), 123 deletions(-)


base-commit: dbaafe9cc56a996931eedfe043eb34418cc9cd9b
-- 
2.55.0

[PATCH v2 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type

[Danilo Krummrich]

Convert IrqVectorRegistration from a devres-managed internal type to a
lifetime-annotated type that owns the PCI interrupt vector allocation.
Dropping it frees the vectors.

IrqVector gains a reference to the IrqVectorRegistration it was derived
from. Since vector() borrows the registration, the compiler prevents the
allocation from being dropped while any IrqVector (and hence any
irq::Registration built from it) is still live.

alloc_irq_vectors() returns IrqVectorRegistration<'_> directly, giving
drivers explicit control over the allocation lifetime, which is needed
by net and block drivers that re-allocate vectors at runtime, e.g.
during queue reconfiguration or device recovery.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/pci.rs     |   3 +-
 rust/kernel/pci/irq.rs | 127 ++++++++++++++++++++++-------------------
 2 files changed, 69 insertions(+), 61 deletions(-)

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index c6417af2bb17..2757a0cc0f11 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -51,7 +51,8 @@
 pub use self::irq::{
     IrqType,
     IrqTypes,
-    IrqVector, //
+    IrqVector,
+    IrqVectorRegistration, //
 };
 
 /// An adapter for the registration of PCI drivers.
diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
index fea484dcf9cf..8e0651587829 100644
--- a/rust/kernel/pci/irq.rs
+++ b/rust/kernel/pci/irq.rs
@@ -7,17 +7,14 @@
     bindings,
     device,
     device::Bound,
-    devres,
     error::to_result,
     irq::{
         self,
         IrqRequest, //
     },
-    prelude::*,
-    str::CStr,
-    sync::aref::ARef, //
+    prelude::*, //
 };
-use core::ops::RangeInclusive;
+use core::num::NonZero;
 
 /// IRQ type flags for PCI interrupt allocation.
 #[derive(Debug, Clone, Copy)]
@@ -78,6 +75,7 @@ const fn as_raw(self) -> u32 {
 #[derive(Clone, Copy)]
 pub struct IrqVector<'a> {
     dev: &'a Device<Bound>,
+    reg: &'a IrqVectorRegistration<'a>,
     index: u32,
 }
 
@@ -86,87 +84,81 @@ impl<'a> IrqVector<'a> {
     ///
     /// # Safety
     ///
-    /// - `index` must be a valid IRQ vector index for `dev`.
-    /// - `dev` must point to a [`Device`] that has successfully allocated IRQ vectors.
-    unsafe fn new(dev: &'a Device<Bound>, index: u32) -> Self {
-        Self { dev, index }
+    /// - `index` must be a valid IRQ vector index for `reg`.
+    /// - `dev` must be the device `reg` was allocated from.
+    #[inline]
+    unsafe fn new(dev: &'a Device<Bound>, reg: &'a IrqVectorRegistration<'a>, index: u32) -> Self {
+        Self { dev, reg, index }
     }
 
     /// Returns the raw vector index.
     fn index(&self) -> u32 {
         self.index
     }
+
+    /// Returns the [`IrqVectorRegistration`] this vector was derived from.
+    pub fn vectors(&self) -> &'a IrqVectorRegistration<'a> {
+        self.reg
+    }
 }
 
 impl<'a> TryInto<IrqRequest<'a>> for IrqVector<'a> {
     type Error = Error;
 
     fn try_into(self) -> Result<IrqRequest<'a>> {
-        // SAFETY: `self.as_raw` returns a valid pointer to a `struct pci_dev`.
+        // SAFETY: `self.dev.as_raw()` returns a valid pointer to a `struct pci_dev`.
         let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), self.index()) };
         if irq < 0 {
             return Err(crate::error::Error::from_errno(irq));
         }
-        // SAFETY: `irq` is guaranteed to be a valid IRQ number for `&self`.
+        // SAFETY: `irq` is guaranteed to be a valid IRQ number for `self.dev`.
         Ok(unsafe { IrqRequest::new(self.dev.as_ref(), irq as u32) })
     }
 }
 
-/// Represents an IRQ vector allocation for a PCI device.
+/// An allocation of PCI interrupt vectors for a device.
 ///
-/// This type ensures that IRQ vectors are properly allocated and freed by
-/// tying the allocation to the lifetime of this registration object.
+/// This type owns the vector allocation; dropping it frees the vectors. IRQ handlers borrow from
+/// this registration and must be dropped before it is.
 ///
 /// # Invariants
 ///
-/// The [`Device`] has successfully allocated IRQ vectors.
-struct IrqVectorRegistration {
-    dev: ARef<Device>,
+/// `dev` has an allocation of `count` interrupt vectors.
+pub struct IrqVectorRegistration<'a> {
+    dev: &'a Device<Bound>,
+    count: NonZero<usize>,
 }
 
-impl IrqVectorRegistration {
-    /// Allocate and register IRQ vectors for the given PCI device.
+impl<'a> IrqVectorRegistration<'a> {
+    /// Returns the number of allocated vectors.
     ///
-    /// Allocates IRQ vectors and registers them with devres for automatic cleanup.
-    /// Returns a range of valid IRQ vectors.
-    fn register<'a>(
-        dev: &'a Device<Bound>,
-        min_vecs: u32,
-        max_vecs: u32,
-        irq_types: IrqTypes,
-    ) -> Result<RangeInclusive<IrqVector<'a>>> {
-        // SAFETY:
-        // - `dev.as_raw()` is guaranteed to be a valid pointer to a `struct pci_dev`
-        //   by the type invariant of `Device`.
-        // - `pci_alloc_irq_vectors` internally validates all other parameters
-        //   and returns error codes.
-        let ret = unsafe {
-            bindings::pci_alloc_irq_vectors(dev.as_raw(), min_vecs, max_vecs, irq_types.as_raw())
-        };
-
-        to_result(ret)?;
-        let count = ret as u32;
-
-        // SAFETY:
-        // - `pci_alloc_irq_vectors` returns the number of allocated vectors on success.
-        // - Vectors are 0-based, so valid indices are [0, count-1].
-        // - `pci_alloc_irq_vectors` guarantees `count >= min_vecs > 0`, so both `0` and
-        //   `count - 1` are valid IRQ vector indices for `dev`.
-        let range = unsafe { IrqVector::new(dev, 0)..=IrqVector::new(dev, count - 1) };
+    /// This is at least the `min_vecs` that [`Device::alloc_irq_vectors`] was asked for.
+    #[inline]
+    pub fn vector_count(&self) -> usize {
+        self.count.get()
+    }
 
-        // INVARIANT: The IRQ vector allocation for `dev` above was successful.
-        let irq_vecs = Self { dev: dev.into() };
-        devres::register(dev.as_ref(), irq_vecs, GFP_KERNEL)?;
+    /// Returns the [`IrqVector`] at `index`.
+    ///
+    /// The returned [`IrqVector`] borrows from this registration, ensuring the vector allocation
+    /// remains live while any handler is registered on it.
+    #[inline]
+    pub fn vector(&self, index: usize) -> Result<IrqVector<'_>> {
+        if index >= self.count.get() {
+            return Err(EINVAL);
+        }
 
-        Ok(range)
+        // SAFETY: `index` is within bounds of this registration's allocation, and `self.dev` is
+        // the device it was allocated from.
+        Ok(unsafe { IrqVector::new(self.dev, self, index as u32) })
     }
 }
 
-impl Drop for IrqVectorRegistration {
+impl Drop for IrqVectorRegistration<'_> {
+    #[inline]
     fn drop(&mut self) {
-        // SAFETY:
-        // - By the type invariant, `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`.
-        // - `self.dev` has successfully allocated IRQ vectors.
+        // SAFETY: By the type invariant, `self.dev.as_raw()` is a valid pointer to a
+        // `struct pci_dev` that has successfully allocated IRQ vectors.
         unsafe { bindings::pci_free_irq_vectors(self.dev.as_raw()) };
     }
 }
@@ -214,15 +206,16 @@ pub unsafe fn request_threaded_irq<'a, T: crate::irq::ThreadedHandler + 'a>(
         })
     }
 
-    /// Allocate IRQ vectors for this PCI device with automatic cleanup.
+    /// Allocate IRQ vectors for this PCI device.
     ///
     /// Allocates between `min_vecs` and `max_vecs` interrupt vectors for the device.
     /// The allocation will use MSI-X, MSI, or INTx interrupts based on the `irq_types`
     /// parameter and hardware capabilities. When multiple types are specified, the kernel
     /// will try them in order of preference: MSI-X first, then MSI, then INTx interrupts.
     ///
-    /// The allocated vectors are automatically freed when the device is unbound, using the
-    /// devres (device resource management) system.
+    /// The allocated vectors are freed when the returned [`IrqVectorRegistration`] is dropped.
+    /// IRQ handlers registered via [`Self::request_irq`] or [`Self::request_threaded_irq`]
+    /// borrow from the registration, so the compiler ensures they are freed first.
     ///
     /// # Arguments
     ///
@@ -232,8 +225,8 @@ pub unsafe fn request_threaded_irq<'a, T: crate::irq::ThreadedHandler + 'a>(
     ///
     /// # Returns
     ///
-    /// Returns a range of IRQ vectors that were successfully allocated, or an error if the
-    /// allocation fails or cannot meet the minimum requirement.
+    /// Returns the IRQ vector registration, or an error if `min_vecs` vectors cannot be
+    /// allocated.
     ///
     /// # Examples
     ///
@@ -256,7 +249,21 @@ pub fn alloc_irq_vectors(
         min_vecs: u32,
         max_vecs: u32,
         irq_types: IrqTypes,
-    ) -> Result<RangeInclusive<IrqVector<'_>>> {
-        IrqVectorRegistration::register(self, min_vecs, max_vecs, irq_types)
+    ) -> Result<IrqVectorRegistration<'_>> {
+        // SAFETY:
+        // - `self.as_raw()` is guaranteed to be a valid pointer to a `struct pci_dev`
+        //   by the type invariant of `Device`.
+        // - `pci_alloc_irq_vectors` internally validates all other parameters
+        //   and returns error codes.
+        let ret = unsafe {
+            bindings::pci_alloc_irq_vectors(self.as_raw(), min_vecs, max_vecs, irq_types.as_raw())
+        };
+
+        to_result(ret)?;
+
+        let count = NonZero::new(ret as usize).ok_or(EINVAL)?;
+
+        // INVARIANT: `pci_alloc_irq_vectors()` allocated `count` vectors for `self`.
+        Ok(IrqVectorRegistration { dev: self, count })
     }
 }
-- 
2.55.0

Re: [PATCH v2 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The lifetime bounds introduced on `IrqVectorRegistration` and `IrqVector` force an impossible self-referential struct, rendering the API unusable in safe Rust.

Pre-existing issues:
- [High] Concurrent calls to `alloc_irq_vectors` allow unsynchronized mutation of PCI device state, leading to data races and kernel memory corruption.
--

--- Patch [1]: [PATCH v2 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type ---
commit c70c8b630ade05d17f2339a929d25258bdd64514
Author: Danilo Krummrich <dakr@kernel.org>

rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type

This commit converts IrqVectorRegistration to a lifetime-annotated type that
owns the PCI interrupt vector allocation. Dropping it frees the vectors, and
alloc_irq_vectors() is updated to return IrqVectorRegistration directly.

> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index fea484dcf9cf2..8e06515878290 100644
> --- a/rust/kernel/pci/irq.rs
> +++ b/rust/kernel/pci/irq.rs
> @@ -78,6 +75,7 @@ const fn as_raw(self) -> u32 {
>  #[derive(Clone, Copy)]
>  pub struct IrqVector<'a> {
>      dev: &'a Device<Bound>,
> +    reg: &'a IrqVectorRegistration<'a>,
>      index: u32,
>  }

[Severity: High]
Will this lifetime bound prevent drivers from storing their PCI interrupts in a
device data structure?

If a driver calls alloc_irq_vectors() to get an IrqVectorRegistration, borrows
it via vector() to create an irq::Registration, and then tries to store both in
its driver data struct, the compiler will reject it. Safe Rust does not allow
the registration and the IRQ handler to be stored in the same struct due to
these self-referential lifetime constraints. 

This seems to force drivers to either leak memory or drop the registration
prematurely, which would instantly free the interrupts via
pci_free_irq_vectors().

[ ... ]

> @@ -256,7 +249,21 @@ pub fn alloc_irq_vectors(
>          &self,
>          min_vecs: u32,
>          max_vecs: u32,
>          irq_types: IrqTypes,
> -    ) -> Result<RangeInclusive<IrqVector<'_>>> {
> -        IrqVectorRegistration::register(self, min_vecs, max_vecs, irq_types)
> +    ) -> Result<IrqVectorRegistration<'_>> {

[Severity: High]
This is a pre-existing issue, but is it safe for alloc_irq_vectors to take an
immutable reference when it mutates PCI device state?

Because alloc_irq_vectors takes &self (which is Sync), a driver could expose
concurrent sysfs or ioctl endpoints that trigger reallocation on the same
Device. 

The underlying C function pci_alloc_irq_vectors checks and modifies
dev->msix_enabled without locking. This can cause a data race, leading to
double-allocation of MSI-X descriptors, leaking of ioremap mappings, and
corruption of internal PCI device state.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260811233952.3000968-1-dakr@kernel.org?part=1

Re: [PATCH v2 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type

[Gary Guo]

On Wed Aug 12, 2026 at 12:39 AM BST, Danilo Krummrich wrote:
> Convert IrqVectorRegistration from a devres-managed internal type to a
> lifetime-annotated type that owns the PCI interrupt vector allocation.
> Dropping it frees the vectors.
>
> IrqVector gains a reference to the IrqVectorRegistration it was derived
> from. Since vector() borrows the registration, the compiler prevents the
> allocation from being dropped while any IrqVector (and hence any
> irq::Registration built from it) is still live.
>
> alloc_irq_vectors() returns IrqVectorRegistration<'_> directly, giving
> drivers explicit control over the allocation lifetime, which is needed
> by net and block drivers that re-allocate vectors at runtime, e.g.
> during queue reconfiguration or device recovery.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/pci.rs     |   3 +-
>  rust/kernel/pci/irq.rs | 127 ++++++++++++++++++++++-------------------
>  2 files changed, 69 insertions(+), 61 deletions(-)
>
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index c6417af2bb17..2757a0cc0f11 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -51,7 +51,8 @@
>  pub use self::irq::{
>      IrqType,
>      IrqTypes,
> -    IrqVector, //
> +    IrqVector,
> +    IrqVectorRegistration, //
>  };
>  
>  /// An adapter for the registration of PCI drivers.
> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index fea484dcf9cf..8e0651587829 100644
> --- a/rust/kernel/pci/irq.rs
> +++ b/rust/kernel/pci/irq.rs
> @@ -7,17 +7,14 @@
>      bindings,
>      device,
>      device::Bound,
> -    devres,
>      error::to_result,
>      irq::{
>          self,
>          IrqRequest, //
>      },
> -    prelude::*,
> -    str::CStr,
> -    sync::aref::ARef, //
> +    prelude::*, //
>  };
> -use core::ops::RangeInclusive;
> +use core::num::NonZero;
>  
>  /// IRQ type flags for PCI interrupt allocation.
>  #[derive(Debug, Clone, Copy)]
> @@ -78,6 +75,7 @@ const fn as_raw(self) -> u32 {
>  #[derive(Clone, Copy)]
>  pub struct IrqVector<'a> {
>      dev: &'a Device<Bound>,
> +    reg: &'a IrqVectorRegistration<'a>,

The registration has a refence to the device so we don't need to keep both reg
and dev?

>      index: u32,
>  }
>  
> @@ -86,87 +84,81 @@ impl<'a> IrqVector<'a> {
>      ///
>      /// # Safety
>      ///
> -    /// - `index` must be a valid IRQ vector index for `dev`.
> -    /// - `dev` must point to a [`Device`] that has successfully allocated IRQ vectors.
> -    unsafe fn new(dev: &'a Device<Bound>, index: u32) -> Self {
> -        Self { dev, index }
> +    /// - `index` must be a valid IRQ vector index for `reg`.
> +    /// - `dev` must be the device `reg` was allocated from.
> +    #[inline]
> +    unsafe fn new(dev: &'a Device<Bound>, reg: &'a IrqVectorRegistration<'a>, index: u32) -> Self {
> +        Self { dev, reg, index }
>      }
>  
>      /// Returns the raw vector index.
>      fn index(&self) -> u32 {
>          self.index
>      }
> +
> +    /// Returns the [`IrqVectorRegistration`] this vector was derived from.

#[inline]

> +    pub fn vectors(&self) -> &'a IrqVectorRegistration<'a> {
> +        self.reg
> +    }
>  }
>  
>  impl<'a> TryInto<IrqRequest<'a>> for IrqVector<'a> {
>      type Error = Error;
>  
>      fn try_into(self) -> Result<IrqRequest<'a>> {
> -        // SAFETY: `self.as_raw` returns a valid pointer to a `struct pci_dev`.
> +        // SAFETY: `self.dev.as_raw()` returns a valid pointer to a `struct pci_dev`.
>          let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), self.index()) };
>          if irq < 0 {
>              return Err(crate::error::Error::from_errno(irq));
>          }
> -        // SAFETY: `irq` is guaranteed to be a valid IRQ number for `&self`.
> +        // SAFETY: `irq` is guaranteed to be a valid IRQ number for `self.dev`.
>          Ok(unsafe { IrqRequest::new(self.dev.as_ref(), irq as u32) })
>      }
>  }
>  
> -/// Represents an IRQ vector allocation for a PCI device.
> +/// An allocation of PCI interrupt vectors for a device.
>  ///
> -/// This type ensures that IRQ vectors are properly allocated and freed by
> -/// tying the allocation to the lifetime of this registration object.
> +/// This type owns the vector allocation; dropping it frees the vectors. IRQ handlers borrow from
> +/// this registration and must be dropped before it is.
>  ///
>  /// # Invariants
>  ///
> -/// The [`Device`] has successfully allocated IRQ vectors.
> -struct IrqVectorRegistration {
> -    dev: ARef<Device>,
> +/// `dev` has an allocation of `count` interrupt vectors.
> +pub struct IrqVectorRegistration<'a> {
> +    dev: &'a Device<Bound>,
> +    count: NonZero<usize>,

I wonder if it should be called "len" as I view this as a collection of IRQ
vectors.

>  }
>  
> -impl IrqVectorRegistration {
> -    /// Allocate and register IRQ vectors for the given PCI device.
> +impl<'a> IrqVectorRegistration<'a> {
> +    /// Returns the number of allocated vectors.
>      ///
> -    /// Allocates IRQ vectors and registers them with devres for automatic cleanup.
> -    /// Returns a range of valid IRQ vectors.
> -    fn register<'a>(
> -        dev: &'a Device<Bound>,
> -        min_vecs: u32,
> -        max_vecs: u32,
> -        irq_types: IrqTypes,
> -    ) -> Result<RangeInclusive<IrqVector<'a>>> {
> -        // SAFETY:
> -        // - `dev.as_raw()` is guaranteed to be a valid pointer to a `struct pci_dev`
> -        //   by the type invariant of `Device`.
> -        // - `pci_alloc_irq_vectors` internally validates all other parameters
> -        //   and returns error codes.
> -        let ret = unsafe {
> -            bindings::pci_alloc_irq_vectors(dev.as_raw(), min_vecs, max_vecs, irq_types.as_raw())
> -        };
> -
> -        to_result(ret)?;
> -        let count = ret as u32;
> -
> -        // SAFETY:
> -        // - `pci_alloc_irq_vectors` returns the number of allocated vectors on success.
> -        // - Vectors are 0-based, so valid indices are [0, count-1].
> -        // - `pci_alloc_irq_vectors` guarantees `count >= min_vecs > 0`, so both `0` and
> -        //   `count - 1` are valid IRQ vector indices for `dev`.
> -        let range = unsafe { IrqVector::new(dev, 0)..=IrqVector::new(dev, count - 1) };
> +    /// This is at least the `min_vecs` that [`Device::alloc_irq_vectors`] was asked for.
> +    #[inline]
> +    pub fn vector_count(&self) -> usize {
> +        self.count.get()
> +    }
>  
> -        // INVARIANT: The IRQ vector allocation for `dev` above was successful.
> -        let irq_vecs = Self { dev: dev.into() };
> -        devres::register(dev.as_ref(), irq_vecs, GFP_KERNEL)?;
> +    /// Returns the [`IrqVector`] at `index`.
> +    ///
> +    /// The returned [`IrqVector`] borrows from this registration, ensuring the vector allocation
> +    /// remains live while any handler is registered on it.

nit: I think this is redundant information as it's just stating what the
signature already conveys.

> +    #[inline]
> +    pub fn vector(&self, index: usize) -> Result<IrqVector<'_>> {
> +        if index >= self.count.get() {
> +            return Err(EINVAL);
> +        }

Given that the error is for out-of-bound access only, perhaps return `Option`
like `get()` function of various containers?

>  
> -        Ok(range)
> +        // SAFETY: `index` is within bounds of this registration's allocation, and `self.dev` is
> +        // the device it was allocated from.
> +        Ok(unsafe { IrqVector::new(self.dev, self, index as u32) })
>      }
>  }
>  
> -impl Drop for IrqVectorRegistration {
> +impl Drop for IrqVectorRegistration<'_> {
> +    #[inline]
>      fn drop(&mut self) {
> -        // SAFETY:
> -        // - By the type invariant, `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`.
> -        // - `self.dev` has successfully allocated IRQ vectors.
> +        // SAFETY: By the type invariant, `self.dev.as_raw()` is a valid pointer to a
> +        // `struct pci_dev` that has successfully allocated IRQ vectors.
>          unsafe { bindings::pci_free_irq_vectors(self.dev.as_raw()) };
>      }
>  }
> @@ -214,15 +206,16 @@ pub unsafe fn request_threaded_irq<'a, T: crate::irq::ThreadedHandler + 'a>(
>          })
>      }
>  
> -    /// Allocate IRQ vectors for this PCI device with automatic cleanup.
> +    /// Allocate IRQ vectors for this PCI device.
>      ///
>      /// Allocates between `min_vecs` and `max_vecs` interrupt vectors for the device.
>      /// The allocation will use MSI-X, MSI, or INTx interrupts based on the `irq_types`
>      /// parameter and hardware capabilities. When multiple types are specified, the kernel
>      /// will try them in order of preference: MSI-X first, then MSI, then INTx interrupts.
>      ///
> -    /// The allocated vectors are automatically freed when the device is unbound, using the
> -    /// devres (device resource management) system.
> +    /// The allocated vectors are freed when the returned [`IrqVectorRegistration`] is dropped.
> +    /// IRQ handlers registered via [`Self::request_irq`] or [`Self::request_threaded_irq`]
> +    /// borrow from the registration, so the compiler ensures they are freed first.
>      ///
>      /// # Arguments
>      ///
> @@ -232,8 +225,8 @@ pub unsafe fn request_threaded_irq<'a, T: crate::irq::ThreadedHandler + 'a>(
>      ///
>      /// # Returns
>      ///
> -    /// Returns a range of IRQ vectors that were successfully allocated, or an error if the
> -    /// allocation fails or cannot meet the minimum requirement.
> +    /// Returns the IRQ vector registration, or an error if `min_vecs` vectors cannot be
> +    /// allocated.
>      ///
>      /// # Examples
>      ///
> @@ -256,7 +249,21 @@ pub fn alloc_irq_vectors(
>          min_vecs: u32,
>          max_vecs: u32,
>          irq_types: IrqTypes,
> -    ) -> Result<RangeInclusive<IrqVector<'_>>> {
> -        IrqVectorRegistration::register(self, min_vecs, max_vecs, irq_types)
> +    ) -> Result<IrqVectorRegistration<'_>> {
> +        // SAFETY:
> +        // - `self.as_raw()` is guaranteed to be a valid pointer to a `struct pci_dev`
> +        //   by the type invariant of `Device`.
> +        // - `pci_alloc_irq_vectors` internally validates all other parameters
> +        //   and returns error codes.
> +        let ret = unsafe {
> +            bindings::pci_alloc_irq_vectors(self.as_raw(), min_vecs, max_vecs, irq_types.as_raw())
> +        };
> +
> +        to_result(ret)?;
> +
> +        let count = NonZero::new(ret as usize).ok_or(EINVAL)?;

I don't think `ret` can ever be zero. `expect` or `new_unchecked()` perhaps?

Best,
Gary

> +
> +        // INVARIANT: `pci_alloc_irq_vectors()` allocated `count` vectors for `self`.
> +        Ok(IrqVectorRegistration { dev: self, count })
>      }
>  }

Re: [PATCH v2 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type

[Danilo Krummrich]

On Wed Aug 12, 2026 at 6:26 PM CEST, Gary Guo wrote:
>>  /// IRQ type flags for PCI interrupt allocation.
>>  #[derive(Debug, Clone, Copy)]
>> @@ -78,6 +75,7 @@ const fn as_raw(self) -> u32 {
>>  #[derive(Clone, Copy)]
>>  pub struct IrqVector<'a> {
>>      dev: &'a Device<Bound>,
>> +    reg: &'a IrqVectorRegistration<'a>,
>
> The registration has a refence to the device so we don't need to keep both reg
> and dev?

In this patch dev is still needed for the TryInto impl, but a subsquent patch
does remove it in favor of IrqRequest.

>> +pub struct IrqVectorRegistration<'a> {
>> +    dev: &'a Device<Bound>,
>> +    count: NonZero<usize>,
>
> I wonder if it should be called "len" as I view this as a collection of IRQ
> vectors.

Either sounds good to me.

>> +    #[inline]
>> +    pub fn vector(&self, index: usize) -> Result<IrqVector<'_>> {
>> +        if index >= self.count.get() {
>> +            return Err(EINVAL);
>> +        }
>
> Given that the error is for out-of-bound access only, perhaps return `Option`
> like `get()` function of various containers?

It's not really a case a driver would handle other than just follow it up with
.ok_or(EINVAL)? anyways, so I'd like to keep that.

(Further consideration on a subsequent patch.)

>> @@ -256,7 +249,21 @@ pub fn alloc_irq_vectors(
>>          min_vecs: u32,
>>          max_vecs: u32,
>>          irq_types: IrqTypes,
>> -    ) -> Result<RangeInclusive<IrqVector<'_>>> {
>> -        IrqVectorRegistration::register(self, min_vecs, max_vecs, irq_types)
>> +    ) -> Result<IrqVectorRegistration<'_>> {
>> +        // SAFETY:
>> +        // - `self.as_raw()` is guaranteed to be a valid pointer to a `struct pci_dev`
>> +        //   by the type invariant of `Device`.
>> +        // - `pci_alloc_irq_vectors` internally validates all other parameters
>> +        //   and returns error codes.
>> +        let ret = unsafe {
>> +            bindings::pci_alloc_irq_vectors(self.as_raw(), min_vecs, max_vecs, irq_types.as_raw())
>> +        };
>> +
>> +        to_result(ret)?;
>> +
>> +        let count = NonZero::new(ret as usize).ok_or(EINVAL)?;
>
> I don't think `ret` can ever be zero. `expect` or `new_unchecked()` perhaps?

Correct, but I don't see a reason to BUG_ON() for this. A WARN_ON() makes sense,
but I see this to be the job of the C API making the promise.

We could use new_unchecked(), but since this method is fallible already and not
a hot path, I don't think it's worth.

Re: [PATCH v2 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type

[Gary Guo]

On Wed Aug 12, 2026 at 6:37 PM BST, Danilo Krummrich wrote:
> On Wed Aug 12, 2026 at 6:26 PM CEST, Gary Guo wrote:
>>>  /// IRQ type flags for PCI interrupt allocation.
>>>  #[derive(Debug, Clone, Copy)]
>>> @@ -78,6 +75,7 @@ const fn as_raw(self) -> u32 {
>>>  #[derive(Clone, Copy)]
>>>  pub struct IrqVector<'a> {
>>>      dev: &'a Device<Bound>,
>>> +    reg: &'a IrqVectorRegistration<'a>,
>>
>> The registration has a refence to the device so we don't need to keep both reg
>> and dev?
>
> In this patch dev is still needed for the TryInto impl, but a subsquent patch
> does remove it in favor of IrqRequest.
>
>>> +pub struct IrqVectorRegistration<'a> {
>>> +    dev: &'a Device<Bound>,
>>> +    count: NonZero<usize>,
>>
>> I wonder if it should be called "len" as I view this as a collection of IRQ
>> vectors.
>
> Either sounds good to me.
>
>>> +    #[inline]
>>> +    pub fn vector(&self, index: usize) -> Result<IrqVector<'_>> {
>>> +        if index >= self.count.get() {
>>> +            return Err(EINVAL);
>>> +        }
>>
>> Given that the error is for out-of-bound access only, perhaps return `Option`
>> like `get()` function of various containers?
>
> It's not really a case a driver would handle other than just follow it up with
> .ok_or(EINVAL)? anyways, so I'd like to keep that.

Well, I'd expect some drivers want to do `.vector(v).expect()` rather than just
propagating the error if `v` is a constant that is less than `min_vecs`..

Best,
Gary

>
> (Further consideration on a subsequent patch.)
>
>>> @@ -256,7 +249,21 @@ pub fn alloc_irq_vectors(
>>>          min_vecs: u32,
>>>          max_vecs: u32,
>>>          irq_types: IrqTypes,
>>> -    ) -> Result<RangeInclusive<IrqVector<'_>>> {
>>> -        IrqVectorRegistration::register(self, min_vecs, max_vecs, irq_types)
>>> +    ) -> Result<IrqVectorRegistration<'_>> {
>>> +        // SAFETY:
>>> +        // - `self.as_raw()` is guaranteed to be a valid pointer to a `struct pci_dev`
>>> +        //   by the type invariant of `Device`.
>>> +        // - `pci_alloc_irq_vectors` internally validates all other parameters
>>> +        //   and returns error codes.
>>> +        let ret = unsafe {
>>> +            bindings::pci_alloc_irq_vectors(self.as_raw(), min_vecs, max_vecs, irq_types.as_raw())
>>> +        };
>>> +
>>> +        to_result(ret)?;
>>> +
>>> +        let count = NonZero::new(ret as usize).ok_or(EINVAL)?;
>>
>> I don't think `ret` can ever be zero. `expect` or `new_unchecked()` perhaps?
>
> Correct, but I don't see a reason to BUG_ON() for this. A WARN_ON() makes sense,
> but I see this to be the job of the C API making the promise.
>
> We could use new_unchecked(), but since this method is fallible already and not
> a hot path, I don't think it's worth.

Re: [PATCH v2 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type

[Danilo Krummrich]

On Wed Aug 12, 2026 at 8:11 PM CEST, Gary Guo wrote:
> Well, I'd expect some drivers want to do `.vector(v).expect()` rather than just
> propagating the error if `v` is a constant that is less than `min_vecs`..

This is nothing we want drivers to do; this API is only ever called from a
fallible context anyway and propagating costs nothing, but on the other hand, if
the driver gets it wrong, we'd BUG() the whole kernel for no value.

Re: [PATCH v2 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type

[Gary Guo]

On Wed Aug 12, 2026 at 7:47 PM BST, Danilo Krummrich wrote:
> On Wed Aug 12, 2026 at 8:11 PM CEST, Gary Guo wrote:
>> Well, I'd expect some drivers want to do `.vector(v).expect()` rather than just
>> propagating the error if `v` is a constant that is less than `min_vecs`..
>
> This is nothing we want drivers to do; this API is only ever called from a
> fallible context anyway and propagating costs nothing, but on the other hand, if
> the driver gets it wrong, we'd BUG() the whole kernel for no value.

Well, if you ask for an interrupt and got one, you'd better got one! If the case
is actually "we'd BUG() the kernel", then it probably should because something
is catastrophically wrong. I'd even consider `unwrap_unchecked` to be valid
there and in my view `BUG()` is less damaging then UB.

I don't like the fact that we propagate error code because we can. Propagating
error comes with a cost: it's one extra control flow that developer needs to
consider; more code is generated because the destructors that's currently
available still needs to be executed; and the code will have 0% coverage because
it'd never occur as

    /// `dev` has an allocation of `count` interrupt vectors

is the type invariant of `IrqVectorRegistration`.

If you write additional error checks in C people will complain it's adding dead
code. I don't want Rust's strong type system to be penalizing by forcing error
checks to places where trhey don't belong. I am not seeing `expect()` as the
equivalent of `BUG_ON`; it's just the way you deal with a stronger type system. 

Best,
Gary

Re: [PATCH v2 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type

[Danilo Krummrich]

On Wed Aug 12, 2026 at 9:01 PM CEST, Gary Guo wrote:
> On Wed Aug 12, 2026 at 7:47 PM BST, Danilo Krummrich wrote:
>> On Wed Aug 12, 2026 at 8:11 PM CEST, Gary Guo wrote:
>>> Well, I'd expect some drivers want to do `.vector(v).expect()` rather than just
>>> propagating the error if `v` is a constant that is less than `min_vecs`..
>>
>> This is nothing we want drivers to do; this API is only ever called from a
>> fallible context anyway and propagating costs nothing, but on the other hand, if
>> the driver gets it wrong, we'd BUG() the whole kernel for no value.
>
> Well, if you ask for an interrupt and got one, you'd better got one! If the case
> is actually "we'd BUG() the kernel", then it probably should because something
> is catastrophically wrong. I'd even consider `unwrap_unchecked` to be valid
> there and in my view `BUG()` is less damaging then UB.

If the driver calls vector().expect() on an index that is actually out of
bounds, because someone made a mistake, e.g. because min_vecs changed and people
forgot to update the code, then nothing went "catastrophically wrong" to a point
that we need to BUG() the whole kernel.

> I don't like the fact that we propagate error code because we can. Propagating
> error comes with a cost: it's one extra control flow that developer needs to
> consider; more code is generated because the destructors that's currently
> available still needs to be executed; and the code will have 0% coverage because
> it'd never occur as
>
>     /// `dev` has an allocation of `count` interrupt vectors
>
> is the type invariant of `IrqVectorRegistration`.

I do not disagree; those points are all valid, but I think it is a case by case
question.

For drivers and in an already fallible cold path, I don't see a lot of value in
compromising on robustness against human mistakes for those reasons.

Quite some drivers are poorly maintained and patches don't receive a lot of
review before they are thrown in; Rust has a chance to significantly compensate
the downsides of a monolithic kernel by increasing the robustness of this
weakest part.

If we encourage drivers to use accessors that potentially end up in BUG() for
cases where it doesn't provide significant value, we may also diminish the
potential for additional robustness.

[PATCH v2 2/5] rust: pci: resolve IRQ in vector() and embed IrqRequest in IrqVector

[Danilo Krummrich]

Move the pci_irq_vector() call from the TryInto<IrqRequest> impl into
IrqVectorRegistration::vector(), so the IRQ number is resolved eagerly.

IrqVector now embeds the resolved IrqRequest and the vector index. The
conversion to IrqRequest is infallible (From instead of TryInto), which
removes the need for pin_init_scope in request_irq/request_threaded_irq.

Inspired-by: John Hubbard <jhubbard@nvidia.com>
Link: https://lore.kernel.org/all/20260808031120.363869-3-jhubbard@nvidia.com/
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/pci/irq.rs | 77 ++++++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 40 deletions(-)

diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
index 8e0651587829..305701440114 100644
--- a/rust/kernel/pci/irq.rs
+++ b/rust/kernel/pci/irq.rs
@@ -68,31 +68,39 @@ const fn as_raw(self) -> u32 {
     }
 }
 
-/// Represents an allocated IRQ vector for a specific PCI device.
+/// A resolved IRQ vector from a PCI interrupt vector allocation.
 ///
-/// This type ties an IRQ vector to the device it was allocated for,
-/// ensuring the vector is only used with the correct device.
-#[derive(Clone, Copy)]
+/// Created by [`IrqVectorRegistration::vector`] and consumed by [`Device::request_irq`] or
+/// [`Device::request_threaded_irq`]. Borrows the [`IrqVectorRegistration`] it was derived from,
+/// so the allocation stays live until the handler is freed.
 pub struct IrqVector<'a> {
-    dev: &'a Device<Bound>,
+    request: IrqRequest<'a>,
     reg: &'a IrqVectorRegistration<'a>,
-    index: u32,
+    index: usize,
 }
 
 impl<'a> IrqVector<'a> {
-    /// Creates a new [`IrqVector`] for the given device and index.
+    /// Creates a new [`IrqVector`] with an already resolved [`IrqRequest`].
     ///
     /// # Safety
     ///
-    /// - `index` must be a valid IRQ vector index for `reg`.
-    /// - `dev` must be the device `reg` was allocated from.
+    /// `request` must have been resolved from `reg`.
     #[inline]
-    unsafe fn new(dev: &'a Device<Bound>, reg: &'a IrqVectorRegistration<'a>, index: u32) -> Self {
-        Self { dev, reg, index }
+    unsafe fn new(
+        request: IrqRequest<'a>,
+        reg: &'a IrqVectorRegistration<'a>,
+        index: usize,
+    ) -> Self {
+        Self {
+            request,
+            reg,
+            index,
+        }
     }
 
-    /// Returns the raw vector index.
-    fn index(&self) -> u32 {
+    /// Returns the vector index within the allocation.
+    #[inline]
+    pub fn index(&self) -> usize {
         self.index
     }
 
@@ -102,17 +110,10 @@ pub fn vectors(&self) -> &'a IrqVectorRegistration<'a> {
     }
 }
 
-impl<'a> TryInto<IrqRequest<'a>> for IrqVector<'a> {
-    type Error = Error;
-
-    fn try_into(self) -> Result<IrqRequest<'a>> {
-        // SAFETY: `self.dev.as_raw()` returns a valid pointer to a `struct pci_dev`.
-        let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), self.index()) };
-        if irq < 0 {
-            return Err(crate::error::Error::from_errno(irq));
-        }
-        // SAFETY: `irq` is guaranteed to be a valid IRQ number for `self.dev`.
-        Ok(unsafe { IrqRequest::new(self.dev.as_ref(), irq as u32) })
+impl<'a> From<IrqVector<'a>> for IrqRequest<'a> {
+    #[inline]
+    fn from(vector: IrqVector<'a>) -> Self {
+        vector.request
     }
 }
 
@@ -142,15 +143,19 @@ pub fn vector_count(&self) -> usize {
     ///
     /// The returned [`IrqVector`] borrows from this registration, ensuring the vector allocation
     /// remains live while any handler is registered on it.
-    #[inline]
     pub fn vector(&self, index: usize) -> Result<IrqVector<'_>> {
         if index >= self.count.get() {
             return Err(EINVAL);
         }
 
-        // SAFETY: `index` is within bounds of this registration's allocation, and `self.dev` is
-        // the device it was allocated from.
-        Ok(unsafe { IrqVector::new(self.dev, self, index as u32) })
+        // SAFETY: `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`.
+        let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), index as u32) };
+        if irq < 0 {
+            return Err(Error::from_errno(irq));
+        }
+
+        // SAFETY: `irq` is a valid IRQ number for `self.dev`, resolved from this registration.
+        Ok(unsafe { IrqVector::new(IrqRequest::new(self.dev.as_ref(), irq as u32), self, index) })
     }
 }
 
@@ -177,12 +182,8 @@ pub unsafe fn request_irq<'a, T: crate::irq::Handler + 'a>(
         name: &'static CStr,
         handler: impl PinInit<T, Error> + 'a,
     ) -> impl PinInit<irq::Registration<'a, T>, Error> + 'a {
-        pin_init::pin_init_scope(move || {
-            let request = vector.try_into()?;
-
-            // SAFETY: Caller guarantees the Registration will not be leaked.
-            Ok(unsafe { irq::Registration::<T>::new(request, flags, name, handler) })
-        })
+        // SAFETY: Caller guarantees the Registration will not be leaked.
+        unsafe { irq::Registration::<T>::new(vector.into(), flags, name, handler) }
     }
 
     /// Returns a [`kernel::irq::ThreadedRegistration`] for the given IRQ vector.
@@ -198,12 +199,8 @@ pub unsafe fn request_threaded_irq<'a, T: crate::irq::ThreadedHandler + 'a>(
         name: &'static CStr,
         handler: impl PinInit<T, Error> + 'a,
     ) -> impl PinInit<irq::ThreadedRegistration<'a, T>, Error> + 'a {
-        pin_init::pin_init_scope(move || {
-            let request = vector.try_into()?;
-
-            // SAFETY: Caller guarantees the Registration will not be leaked.
-            Ok(unsafe { irq::ThreadedRegistration::<T>::new(request, flags, name, handler) })
-        })
+        // SAFETY: Caller guarantees the Registration will not be leaked.
+        unsafe { irq::ThreadedRegistration::<T>::new(vector.into(), flags, name, handler) }
     }
 
     /// Allocate IRQ vectors for this PCI device.
-- 
2.55.0

Re: [PATCH v2 2/5] rust: pci: resolve IRQ in vector() and embed IrqRequest in IrqVector

[sashiko-bot]

> Move the pci_irq_vector() call from the TryInto<IrqRequest> impl into
> IrqVectorRegistration::vector(), so the IRQ number is resolved eagerly.
> 
> IrqVector now embeds the resolved IrqRequest and the vector index. The
> conversion to IrqRequest is infallible (From instead of TryInto), which
> removes the need for pin_init_scope in request_irq/request_threaded_irq.
> 
> Inspired-by: John Hubbard <jhubbard@nvidia.com>
> Link: https://lore.kernel.org/all/20260808031120.363869-3-jhubbard@nvidia.com/
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260811233952.3000968-1-dakr@kernel.org?part=2

Re: [PATCH v2 2/5] rust: pci: resolve IRQ in vector() and embed IrqRequest in IrqVector

[Gary Guo]

On Wed Aug 12, 2026 at 12:39 AM BST, Danilo Krummrich wrote:
> Move the pci_irq_vector() call from the TryInto<IrqRequest> impl into
> IrqVectorRegistration::vector(), so the IRQ number is resolved eagerly.
>
> IrqVector now embeds the resolved IrqRequest and the vector index. The
> conversion to IrqRequest is infallible (From instead of TryInto), which
> removes the need for pin_init_scope in request_irq/request_threaded_irq.
>
> Inspired-by: John Hubbard <jhubbard@nvidia.com>
> Link: https://lore.kernel.org/all/20260808031120.363869-3-jhubbard@nvidia.com/
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/pci/irq.rs | 77 ++++++++++++++++++++----------------------
>  1 file changed, 37 insertions(+), 40 deletions(-)
>
> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index 8e0651587829..305701440114 100644
> --- a/rust/kernel/pci/irq.rs
> +++ b/rust/kernel/pci/irq.rs
>
> [snip]
>
> -impl<'a> TryInto<IrqRequest<'a>> for IrqVector<'a> {
> -    type Error = Error;
> -
> -    fn try_into(self) -> Result<IrqRequest<'a>> {
> -        // SAFETY: `self.dev.as_raw()` returns a valid pointer to a `struct pci_dev`.
> -        let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), self.index()) };
> -        if irq < 0 {
> -            return Err(crate::error::Error::from_errno(irq));
> -        }
> -        // SAFETY: `irq` is guaranteed to be a valid IRQ number for `self.dev`.
> -        Ok(unsafe { IrqRequest::new(self.dev.as_ref(), irq as u32) })
> +impl<'a> From<IrqVector<'a>> for IrqRequest<'a> {
> +    #[inline]
> +    fn from(vector: IrqVector<'a>) -> Self {
> +        vector.request
>      }
>  }
>  
> @@ -142,15 +143,19 @@ pub fn vector_count(&self) -> usize {
>      ///
>      /// The returned [`IrqVector`] borrows from this registration, ensuring the vector allocation
>      /// remains live while any handler is registered on it.
> -    #[inline]
>      pub fn vector(&self, index: usize) -> Result<IrqVector<'_>> {
>          if index >= self.count.get() {
>              return Err(EINVAL);
>          }
>  
> -        // SAFETY: `index` is within bounds of this registration's allocation, and `self.dev` is
> -        // the device it was allocated from.
> -        Ok(unsafe { IrqVector::new(self.dev, self, index as u32) })
> +        // SAFETY: `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`.
> +        let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), index as u32) };
> +        if irq < 0 {
> +            return Err(Error::from_errno(irq));
> +        }

Correct me if I'm wrong, but I believe that it's impossible for `pci_irq_vector`
once we have allocated vector and the index is in bounds. (If that's not the
case, we should ideally fix that instead.)

So I think we should just `.expect()` on the error in `Into`.

Best,
Gary

> +
> +        // SAFETY: `irq` is a valid IRQ number for `self.dev`, resolved from this registration.
> +        Ok(unsafe { IrqVector::new(IrqRequest::new(self.dev.as_ref(), irq as u32), self, index) })
>      }
>  }
>  

Re: [PATCH v2 2/5] rust: pci: resolve IRQ in vector() and embed IrqRequest in IrqVector

[Danilo Krummrich]

On Wed Aug 12, 2026 at 6:38 PM CEST, Gary Guo wrote:
> On Wed Aug 12, 2026 at 12:39 AM BST, Danilo Krummrich wrote:
>>      pub fn vector(&self, index: usize) -> Result<IrqVector<'_>> {
>>          if index >= self.count.get() {
>>              return Err(EINVAL);
>>          }
>>  
>> -        // SAFETY: `index` is within bounds of this registration's allocation, and `self.dev` is
>> -        // the device it was allocated from.
>> -        Ok(unsafe { IrqVector::new(self.dev, self, index as u32) })
>> +        // SAFETY: `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`.
>> +        let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), index as u32) };
>> +        if irq < 0 {
>> +            return Err(Error::from_errno(irq));
>> +        }
>
> Correct me if I'm wrong, but I believe that it's impossible for `pci_irq_vector`
> once we have allocated vector and the index is in bounds. (If that's not the
> case, we should ideally fix that instead.)

You are correct, as of now it is unreachable with the index check above.

> So I think we should just `.expect()` on the error in `Into`.

I don't agree with the conclusion; I don't want this code to rely on an
implementation detail of pci_irq_vector(), which (even though unlikely) could
theoretically change.

If we want to remove the redundancy, then we could maybe drop the index check
above.

(I also prefer IrqVector to be a new type over IrqRequest, as it also guarantees
type wise that a valid IrqVector will always transform into a valid IrqRequest.)

Re: [PATCH v2 2/5] rust: pci: resolve IRQ in vector() and embed IrqRequest in IrqVector

[Gary Guo]

On Wed Aug 12, 2026 at 6:44 PM BST, Danilo Krummrich wrote:
> On Wed Aug 12, 2026 at 6:38 PM CEST, Gary Guo wrote:
>> On Wed Aug 12, 2026 at 12:39 AM BST, Danilo Krummrich wrote:
>>>      pub fn vector(&self, index: usize) -> Result<IrqVector<'_>> {
>>>          if index >= self.count.get() {
>>>              return Err(EINVAL);
>>>          }
>>>  
>>> -        // SAFETY: `index` is within bounds of this registration's allocation, and `self.dev` is
>>> -        // the device it was allocated from.
>>> -        Ok(unsafe { IrqVector::new(self.dev, self, index as u32) })
>>> +        // SAFETY: `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`.
>>> +        let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), index as u32) };
>>> +        if irq < 0 {
>>> +            return Err(Error::from_errno(irq));
>>> +        }
>>
>> Correct me if I'm wrong, but I believe that it's impossible for `pci_irq_vector`
>> once we have allocated vector and the index is in bounds. (If that's not the
>> case, we should ideally fix that instead.)
>
> You are correct, as of now it is unreachable with the index check above.
>
>> So I think we should just `.expect()` on the error in `Into`.
>
> I don't agree with the conclusion; I don't want this code to rely on an
> implementation detail of pci_irq_vector(), which (even though unlikely) could
> theoretically change.

I think this is expected use pattern of `pci_irq_vector`. Many C code don't
check the return code at all. If we want to mirror what C code do, we can also
just drop this error code check and rely on `irq as u32` below doing the correct
thing.

For both this and the EINVAL case for patch 1, my reasoning is that if the error
is never going to happen, then the code shouldn't be written as if it does, as
it will only add confusion to people reading the code.

I view these essentially as invariants, just not spelled out because it's
written in another language. In this case, basically you can say that
`pci_irq_vector(dev, index)` being successful is an invariant of
`IrqVectorRegistration` type.

>
> If we want to remove the redundancy, then we could maybe drop the index check
> above.

I think the index check should stay.

Best,
Gary

>
> (I also prefer IrqVector to be a new type over IrqRequest, as it also guarantees
> type wise that a valid IrqVector will always transform into a valid IrqRequest.)

Re: [PATCH v2 2/5] rust: pci: resolve IRQ in vector() and embed IrqRequest in IrqVector

[Danilo Krummrich]

On Wed Aug 12, 2026 at 8:09 PM CEST, Gary Guo wrote:
> On Wed Aug 12, 2026 at 6:44 PM BST, Danilo Krummrich wrote:
>> On Wed Aug 12, 2026 at 6:38 PM CEST, Gary Guo wrote:
>>> On Wed Aug 12, 2026 at 12:39 AM BST, Danilo Krummrich wrote:
>>>>      pub fn vector(&self, index: usize) -> Result<IrqVector<'_>> {
>>>>          if index >= self.count.get() {
>>>>              return Err(EINVAL);
>>>>          }
>>>>  
>>>> -        // SAFETY: `index` is within bounds of this registration's allocation, and `self.dev` is
>>>> -        // the device it was allocated from.
>>>> -        Ok(unsafe { IrqVector::new(self.dev, self, index as u32) })
>>>> +        // SAFETY: `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`.
>>>> +        let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), index as u32) };
>>>> +        if irq < 0 {
>>>> +            return Err(Error::from_errno(irq));
>>>> +        }
>>>
>>> Correct me if I'm wrong, but I believe that it's impossible for `pci_irq_vector`
>>> once we have allocated vector and the index is in bounds. (If that's not the
>>> case, we should ideally fix that instead.)
>>
>> You are correct, as of now it is unreachable with the index check above.
>>
>>> So I think we should just `.expect()` on the error in `Into`.
>>
>> I don't agree with the conclusion; I don't want this code to rely on an
>> implementation detail of pci_irq_vector(), which (even though unlikely) could
>> theoretically change.
>
> I think this is expected use pattern of `pci_irq_vector`. Many C code don't
> check the return code at all. If we want to mirror what C code do, we can also
> just drop this error code check and rely on `irq as u32` below doing the correct
> thing.
>
> For both this and the EINVAL case for patch 1, my reasoning is that if the error
> is never going to happen, then the code shouldn't be written as if it does, as
> it will only add confusion to people reading the code.
>
> I view these essentially as invariants, just not spelled out because it's
> written in another language. In this case, basically you can say that
> `pci_irq_vector(dev, index)` being successful is an invariant of
> `IrqVectorRegistration` type.

All this only checks out if we keep open-coding the range check and therefore
rely on implementation details of pci_irq_vector().

>>
>> If we want to remove the redundancy, then we could maybe drop the index check
>> above.
>
> I think the index check should stay.

Again, this seems backwards, why would we want to open-code a check that
pci_irq_vector() already does and subsequently rely on this implementation
detail?

>>
>> (I also prefer IrqVector to be a new type over IrqRequest, as it also guarantees
>> type wise that a valid IrqVector will always transform into a valid IrqRequest.)

[PATCH v2 3/5] rust: pci: remove request_irq() and request_threaded_irq() from Device

[Danilo Krummrich]

Remove the thin wrappers on Device<Bound> that only forwarded to
irq::Registration::new() and irq::ThreadedRegistration::new(). With
IrqVector embedding a resolved IrqRequest, the conversion is infallible
and drivers call irq::Registration::new(vector.into(), ...) directly.

Unlike the platform equivalents, which combine a fallible IRQ lookup
with handler registration, the PCI wrappers add no value beyond
namespacing. They also introduce a redundant device reference.
IrqVector already carries a device borrow through its embedded
IrqRequest, yet the wrappers required a second, potentially unrelated,
&self receiver.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/pci/irq.rs | 50 ++++++------------------------------------
 1 file changed, 7 insertions(+), 43 deletions(-)

diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
index 305701440114..b3dce5b49d57 100644
--- a/rust/kernel/pci/irq.rs
+++ b/rust/kernel/pci/irq.rs
@@ -8,10 +8,7 @@
     device,
     device::Bound,
     error::to_result,
-    irq::{
-        self,
-        IrqRequest, //
-    },
+    irq::IrqRequest,
     prelude::*, //
 };
 use core::num::NonZero;
@@ -70,9 +67,10 @@ const fn as_raw(self) -> u32 {
 
 /// A resolved IRQ vector from a PCI interrupt vector allocation.
 ///
-/// Created by [`IrqVectorRegistration::vector`] and consumed by [`Device::request_irq`] or
-/// [`Device::request_threaded_irq`]. Borrows the [`IrqVectorRegistration`] it was derived from,
-/// so the allocation stays live until the handler is freed.
+/// Created by [`IrqVectorRegistration::vector`]. Convert to [`IrqRequest`] via [`From`] to register
+/// a handler with [`irq::Registration::new`](crate::irq::Registration::new). Borrows the
+/// [`IrqVectorRegistration`] it was derived from, so the allocation stays live until the handler is
+/// freed.
 pub struct IrqVector<'a> {
     request: IrqRequest<'a>,
     reg: &'a IrqVectorRegistration<'a>,
@@ -169,40 +167,6 @@ fn drop(&mut self) {
 }
 
 impl Device<device::Bound> {
-    /// Returns a [`kernel::irq::Registration`] for the given IRQ vector.
-    ///
-    /// # Safety
-    ///
-    /// Callers must not `mem::forget()` the resulting [`irq::Registration`] or otherwise prevent
-    /// its [`Drop`] implementation from running.
-    pub unsafe fn request_irq<'a, T: crate::irq::Handler + 'a>(
-        &'a self,
-        vector: IrqVector<'a>,
-        flags: irq::Flags,
-        name: &'static CStr,
-        handler: impl PinInit<T, Error> + 'a,
-    ) -> impl PinInit<irq::Registration<'a, T>, Error> + 'a {
-        // SAFETY: Caller guarantees the Registration will not be leaked.
-        unsafe { irq::Registration::<T>::new(vector.into(), flags, name, handler) }
-    }
-
-    /// Returns a [`kernel::irq::ThreadedRegistration`] for the given IRQ vector.
-    ///
-    /// # Safety
-    ///
-    /// Callers must not `mem::forget()` the resulting [`irq::ThreadedRegistration`] or otherwise
-    /// prevent its [`Drop`] implementation from running.
-    pub unsafe fn request_threaded_irq<'a, T: crate::irq::ThreadedHandler + 'a>(
-        &'a self,
-        vector: IrqVector<'a>,
-        flags: irq::Flags,
-        name: &'static CStr,
-        handler: impl PinInit<T, Error> + 'a,
-    ) -> impl PinInit<irq::ThreadedRegistration<'a, T>, Error> + 'a {
-        // SAFETY: Caller guarantees the Registration will not be leaked.
-        unsafe { irq::ThreadedRegistration::<T>::new(vector.into(), flags, name, handler) }
-    }
-
     /// Allocate IRQ vectors for this PCI device.
     ///
     /// Allocates between `min_vecs` and `max_vecs` interrupt vectors for the device.
@@ -211,8 +175,8 @@ pub unsafe fn request_threaded_irq<'a, T: crate::irq::ThreadedHandler + 'a>(
     /// will try them in order of preference: MSI-X first, then MSI, then INTx interrupts.
     ///
     /// The allocated vectors are freed when the returned [`IrqVectorRegistration`] is dropped.
-    /// IRQ handlers registered via [`Self::request_irq`] or [`Self::request_threaded_irq`]
-    /// borrow from the registration, so the compiler ensures they are freed first.
+    /// Use [`IrqVectorRegistration::vector`] to obtain an [`IrqVector`] for a given vector
+    /// index.
     ///
     /// # Arguments
     ///
-- 
2.55.0

Re: [PATCH v2 3/5] rust: pci: remove request_irq() and request_threaded_irq() from Device

[sashiko-bot]

> Remove the thin wrappers on Device<Bound> that only forwarded to
> irq::Registration::new() and irq::ThreadedRegistration::new(). With
> IrqVector embedding a resolved IrqRequest, the conversion is infallible
> and drivers call irq::Registration::new(vector.into(), ...) directly.
> 
> Unlike the platform equivalents, which combine a fallible IRQ lookup
> with handler registration, the PCI wrappers add no value beyond
> namespacing. They also introduce a redundant device reference.
> IrqVector already carries a device borrow through its embedded
> IrqRequest, yet the wrappers required a second, potentially unrelated,
> &self receiver.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260811233952.3000968-1-dakr@kernel.org?part=3

[PATCH v2 4/5] PCI: Add pci_irq_type() to query the allocated interrupt type

[Danilo Krummrich]

Add a helper that returns PCI_IRQ_MSIX, PCI_IRQ_MSI, or PCI_IRQ_INTX
based on the interrupt type the PCI core selected after
pci_alloc_irq_vectors().

Several drivers already open-code this check against pdev->msix_enabled
and pdev->msi_enabled, or even open code this helper [1].

A common helper avoids the duplication and keeps drivers from accessing
the bitfield directly (see also [2]).

Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Link: https://elixir.bootlin.com/linux/v7.1/source/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c#L196 [1]
Inspired-by: John Hubbard <jhubbard@nvidia.com>
Link: https://lore.kernel.org/all/DKKG2QM3YJYB.Z2H2B2UXJ75N@kernel.org/ [2]
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 include/linux/pci.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 64b308b6e61c..80b8561b5be0 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1783,6 +1783,26 @@ void pci_free_irq_vectors(struct pci_dev *dev);
 int pci_irq_vector(struct pci_dev *dev, unsigned int nr);
 const struct cpumask *pci_irq_get_affinity(struct pci_dev *pdev, int vec);
 
+/**
+ * pci_irq_type - Get the interrupt type of a PCI device
+ * @pdev: the PCI device to operate on
+ *
+ * Discriminate the interrupt type the PCI core selected for this device
+ * after a successful pci_alloc_irq_vectors() call.
+ *
+ * Return: %PCI_IRQ_MSIX, %PCI_IRQ_MSI, or %PCI_IRQ_INTX.
+ */
+static inline unsigned int pci_irq_type(struct pci_dev *pdev)
+{
+	if (pdev->msix_enabled)
+		return PCI_IRQ_MSIX;
+
+	if (pdev->msi_enabled)
+		return PCI_IRQ_MSI;
+
+	return PCI_IRQ_INTX;
+}
+
 #else
 static inline int pci_msi_vec_count(struct pci_dev *dev) { return -ENOSYS; }
 static inline void pci_disable_msi(struct pci_dev *dev) { }
@@ -1845,6 +1865,11 @@ static inline const struct cpumask *pci_irq_get_affinity(struct pci_dev *pdev,
 {
 	return cpu_possible_mask;
 }
+
+static inline unsigned int pci_irq_type(struct pci_dev *pdev)
+{
+	return PCI_IRQ_INTX;
+}
 #endif
 
 /**
-- 
2.55.0

Re: [PATCH v2 4/5] PCI: Add pci_irq_type() to query the allocated interrupt type

[sashiko-bot]

> Add a helper that returns PCI_IRQ_MSIX, PCI_IRQ_MSI, or PCI_IRQ_INTX
> based on the interrupt type the PCI core selected after
> pci_alloc_irq_vectors().
> 
> Several drivers already open-code this check against pdev->msix_enabled
> and pdev->msi_enabled, or even open code this helper [1].
> 
> A common helper avoids the duplication and keeps drivers from accessing
> the bitfield directly (see also [2]).
> 
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> Link: https://elixir.bootlin.com/linux/v7.1/source/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c#L196 [1]
> Inspired-by: John Hubbard <jhubbard@nvidia.com>
> Link: https://lore.kernel.org/all/DKKG2QM3YJYB.Z2H2B2UXJ75N@kernel.org/ [2]
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260811233952.3000968-1-dakr@kernel.org?part=4

[PATCH v2 5/5] rust: pci: expose the allocated interrupt type

[Danilo Krummrich]

Add irq_type() on IrqVectorRegistration and IrqVector, wrapping the new
pci_irq_type() C function. A driver whose interrupt acknowledgment
depends on the type (MSI-X vs MSI vs INTx) queries it here rather than
assuming which type the PCI core selected.

Suggested-by: John Hubbard <jhubbard@nvidia.com>
Link: https://lore.kernel.org/all/20260808031120.363869-4-jhubbard@nvidia.com/
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/helpers/pci.c     |  5 +++++
 rust/kernel/pci/irq.rs | 22 ++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/rust/helpers/pci.c b/rust/helpers/pci.c
index e44905317d75..23b06becb448 100644
--- a/rust/helpers/pci.c
+++ b/rust/helpers/pci.c
@@ -24,6 +24,11 @@ __rust_helper bool rust_helper_dev_is_pci(const struct device *dev)
 	return dev_is_pci(dev);
 }
 
+__rust_helper unsigned int rust_helper_pci_irq_type(struct pci_dev *pdev)
+{
+	return pci_irq_type(pdev);
+}
+
 #ifndef CONFIG_PCI_MSI
 __rust_helper int rust_helper_pci_alloc_irq_vectors(struct pci_dev *dev,
 						    unsigned int min_vecs,
diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
index b3dce5b49d57..41059b922492 100644
--- a/rust/kernel/pci/irq.rs
+++ b/rust/kernel/pci/irq.rs
@@ -33,6 +33,16 @@ const fn as_raw(self) -> u32 {
             IrqType::MsiX => bindings::PCI_IRQ_MSIX,
         }
     }
+
+    /// Construct from raw value.
+    #[inline]
+    const fn from_raw(raw: u32) -> Self {
+        match raw {
+            bindings::PCI_IRQ_MSIX => IrqType::MsiX,
+            bindings::PCI_IRQ_MSI => IrqType::Msi,
+            _ => IrqType::Intx,
+        }
+    }
 }
 
 /// Set of IRQ types that can be used for PCI interrupt allocation.
@@ -106,6 +116,11 @@ pub fn index(&self) -> usize {
     pub fn vectors(&self) -> &'a IrqVectorRegistration<'a> {
         self.reg
     }
+
+    /// Returns the interrupt type the PCI core selected for this vector's allocation.
+    pub fn irq_type(&self) -> IrqType {
+        self.reg.irq_type()
+    }
 }
 
 impl<'a> From<IrqVector<'a>> for IrqRequest<'a> {
@@ -137,6 +152,13 @@ pub fn vector_count(&self) -> usize {
         self.count.get()
     }
 
+    /// Returns the interrupt type the PCI core selected for this allocation.
+    #[inline]
+    pub fn irq_type(&self) -> IrqType {
+        // SAFETY: `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`.
+        IrqType::from_raw(unsafe { bindings::pci_irq_type(self.dev.as_raw()) })
+    }
+
     /// Returns the [`IrqVector`] at `index`.
     ///
     /// The returned [`IrqVector`] borrows from this registration, ensuring the vector allocation
-- 
2.55.0

Re: [PATCH v2 5/5] rust: pci: expose the allocated interrupt type

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The `irq_type` method on `IrqVector` is missing the `#[inline]` attribute, which is required by the subsystem guidelines for small forwarding functions in abstractions.
--

--- Patch [5]: [PATCH v2 5/5] rust: pci: expose the allocated interrupt type ---
commit 2729ea5a5c9bd73b2db961974f58b91749e6fbc9
Author: Danilo Krummrich <dakr@kernel.org>

rust: pci: expose the allocated interrupt type

This commit adds irq_type() methods to IrqVectorRegistration and IrqVector to
wrap the pci_irq_type() C function. This allows drivers to query the interrupt
type rather than assuming which type the PCI core selected.

Link: https://lore.kernel.org/all/20260808031120.363869-4-jhubbard@nvidia.com/

> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index b3dce5b49d571..41059b922492e 100644
> --- a/rust/kernel/pci/irq.rs
> +++ b/rust/kernel/pci/irq.rs
[ ... ]
> @@ -106,6 +116,11 @@ pub fn index(&self) -> usize {
>      pub fn vectors(&self) -> &'a IrqVectorRegistration<'a> {
>          self.reg
>      }
> +
> +    /// Returns the interrupt type the PCI core selected for this vector's allocation.
> +    pub fn irq_type(&self) -> IrqType {
> +        self.reg.irq_type()
> +    }

[Severity: Low]
This isn't a bug, but should this method have an #[inline] annotation?

The Rust subsystem guidelines specify that small or forwarding functions in
abstractions should be annotated with #[inline]. Since this just forwards to
self.reg.irq_type(), adding the annotation would ensure it aligns with the
coding guidelines.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260811233952.3000968-1-dakr@kernel.org?part=5

Re: [PATCH v2 5/5] rust: pci: expose the allocated interrupt type

[Gary Guo]

On Wed Aug 12, 2026 at 12:39 AM BST, Danilo Krummrich wrote:
> Add irq_type() on IrqVectorRegistration and IrqVector, wrapping the new
> pci_irq_type() C function. A driver whose interrupt acknowledgment
> depends on the type (MSI-X vs MSI vs INTx) queries it here rather than
> assuming which type the PCI core selected.
>
> Suggested-by: John Hubbard <jhubbard@nvidia.com>
> Link: https://lore.kernel.org/all/20260808031120.363869-4-jhubbard@nvidia.com/
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/helpers/pci.c     |  5 +++++
>  rust/kernel/pci/irq.rs | 22 ++++++++++++++++++++++
>  2 files changed, 27 insertions(+)
>
> diff --git a/rust/helpers/pci.c b/rust/helpers/pci.c
> index e44905317d75..23b06becb448 100644
> --- a/rust/helpers/pci.c
> +++ b/rust/helpers/pci.c
> @@ -24,6 +24,11 @@ __rust_helper bool rust_helper_dev_is_pci(const struct device *dev)
>  	return dev_is_pci(dev);
>  }
>  
> +__rust_helper unsigned int rust_helper_pci_irq_type(struct pci_dev *pdev)
> +{
> +	return pci_irq_type(pdev);
> +}
> +
>  #ifndef CONFIG_PCI_MSI
>  __rust_helper int rust_helper_pci_alloc_irq_vectors(struct pci_dev *dev,
>  						    unsigned int min_vecs,
> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index b3dce5b49d57..41059b922492 100644
> --- a/rust/kernel/pci/irq.rs
> +++ b/rust/kernel/pci/irq.rs
> @@ -33,6 +33,16 @@ const fn as_raw(self) -> u32 {
>              IrqType::MsiX => bindings::PCI_IRQ_MSIX,
>          }
>      }
> +
> +    /// Construct from raw value.
> +    #[inline]
> +    const fn from_raw(raw: u32) -> Self {
> +        match raw {
> +            bindings::PCI_IRQ_MSIX => IrqType::MsiX,
> +            bindings::PCI_IRQ_MSI => IrqType::Msi,
> +            _ => IrqType::Intx,
> +        }
> +    }
>  }
>  
>  /// Set of IRQ types that can be used for PCI interrupt allocation.
> @@ -106,6 +116,11 @@ pub fn index(&self) -> usize {
>      pub fn vectors(&self) -> &'a IrqVectorRegistration<'a> {
>          self.reg
>      }
> +
> +    /// Returns the interrupt type the PCI core selected for this vector's allocation.

#[inline]

> +    pub fn irq_type(&self) -> IrqType {
> +        self.reg.irq_type()

Do you expect people to call this on the `IrqVetor` (or even
`IrqVectorRegistration`)? This is really a property of the device, and not on a
specific IRQ vector/allocation.

Asking this because I think we can avoid keeping reference to `reg` if we don't
need this and `vectors` (just keep `&'a Device<Bound>`; the mere signature of
`IrqVectorRegistration::vector` will ensure the correct lifetime)

Best,
Gary

> +    }
>  }
>  
>  impl<'a> From<IrqVector<'a>> for IrqRequest<'a> {
> @@ -137,6 +152,13 @@ pub fn vector_count(&self) -> usize {
>          self.count.get()
>      }
>  
> +    /// Returns the interrupt type the PCI core selected for this allocation.
> +    #[inline]
> +    pub fn irq_type(&self) -> IrqType {
> +        // SAFETY: `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`.
> +        IrqType::from_raw(unsafe { bindings::pci_irq_type(self.dev.as_raw()) })
> +    }
> +
>      /// Returns the [`IrqVector`] at `index`.
>      ///
>      /// The returned [`IrqVector`] borrows from this registration, ensuring the vector allocation

Re: [PATCH v2 5/5] rust: pci: expose the allocated interrupt type

[Danilo Krummrich]

On Wed Aug 12, 2026 at 6:44 PM CEST, Gary Guo wrote:
> On Wed Aug 12, 2026 at 12:39 AM BST, Danilo Krummrich wrote:
>> +    pub fn irq_type(&self) -> IrqType {
>> +        self.reg.irq_type()
>
> Do you expect people to call this on the `IrqVetor` (or even
> `IrqVectorRegistration`)? This is really a property of the device, and not on a
> specific IRQ vector/allocation.

You are not wrong, but the C API sets msix_enabled and msi_enabled in
pci_alloc_irq_vectors() and clears them in pci_free_irq_vectors().

So, if we'd put it on Device<Bound> it'd be a bit of a footgun because it would
potentially yield different results depending on whether it is called before or
after obtaining an IrqVectorRegistration.

Re: [PATCH v2 5/5] rust: pci: expose the allocated interrupt type

[Gary Guo]

On Wed Aug 12, 2026 at 6:57 PM BST, Danilo Krummrich wrote:
> On Wed Aug 12, 2026 at 6:44 PM CEST, Gary Guo wrote:
>> On Wed Aug 12, 2026 at 12:39 AM BST, Danilo Krummrich wrote:
>>> +    pub fn irq_type(&self) -> IrqType {
>>> +        self.reg.irq_type()
>>
>> Do you expect people to call this on the `IrqVetor` (or even
>> `IrqVectorRegistration`)? This is really a property of the device, and not on a
>> specific IRQ vector/allocation.
>
> You are not wrong, but the C API sets msix_enabled and msi_enabled in
> pci_alloc_irq_vectors() and clears them in pci_free_irq_vectors().

Right, then putting it on `IrqVectorRegistration` does make sense. Speaking of
which, what prevents people from calling pci_alloc_irq_vectors twice with
different irq types or with both IrqType::Intx?

Best,
Gary

Re: [PATCH v2 5/5] rust: pci: expose the allocated interrupt type

[Danilo Krummrich]

On Wed Aug 12, 2026 at 8:16 PM CEST, Gary Guo wrote:
> On Wed Aug 12, 2026 at 6:57 PM BST, Danilo Krummrich wrote:
>> On Wed Aug 12, 2026 at 6:44 PM CEST, Gary Guo wrote:
>>> On Wed Aug 12, 2026 at 12:39 AM BST, Danilo Krummrich wrote:
>>>> +    pub fn irq_type(&self) -> IrqType {
>>>> +        self.reg.irq_type()
>>>
>>> Do you expect people to call this on the `IrqVetor` (or even
>>> `IrqVectorRegistration`)? This is really a property of the device, and not on a
>>> specific IRQ vector/allocation.
>>
>> You are not wrong, but the C API sets msix_enabled and msi_enabled in
>> pci_alloc_irq_vectors() and clears them in pci_free_irq_vectors().
>
> Right, then putting it on `IrqVectorRegistration` does make sense. Speaking of
> which, what prevents people from calling pci_alloc_irq_vectors twice with
> different irq types or with both IrqType::Intx?

Most cases are fine: INTX is idempotent; MSI and MSIX mixed up should be caught
by the C API already.

I'm aware of a preexisting issue with mixing INTX and either MSI or MSIX, since
calling pci_free_irq_vectors() on the INTX one would also disable MSI/MSIX.

I plan to fix it up when fixing the C bitfied issue [1] with an additional flag.

[1] https://lore.kernel.org/all/DKKG2QM3YJYB.Z2H2B2UXJ75N@kernel.org/

Re: [PATCH v2 0/5] Rework PCI IRQ vector code

[John Hubbard]

On 8/11/26 4:39 PM, Danilo Krummrich wrote:
> This series reworks the Rust PCI interrupt vector abstractions, motivated by
> review feedback on the nova-core interrupt support series [1].
> 
> Convert IrqVectorRegistration to a lifetime-managed owning type, replacing the
> devres-based approach. Since vector() borrows the registration, the returned
> IrqVector inherits that lifetime, preventing the allocation from being dropped
> while any handler is live.
> 
> IrqVector embeds a resolved IrqRequest, making the conversion infallible. The
> request_irq()/request_threaded_irq() wrappers on Device are removed since their
> &self receiver could refer to an unrelated device.
> 
> Add pci_irq_type() as a C function in include/linux/pci.h, replacing open-coded
> checks across drivers [2], and wrap it for Rust.
> 
> [1] https://lore.kernel.org/all/20260808031120.363869-1-jhubbard@nvidia.com/
> [2] https://elixir.bootlin.com/linux/v7.1/source/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c#L196
> 
> Changes in v2:
>   - Drop the IrqRequestAnchor approach and keep IrqVector as a new type over
>     IrqRequest.
> 

I've integrated this version into my work-in-progress v2 patchset for interrupt
support for nova[1], and it still passes tests on some real GPU hardware.

For example:

$ dmesg -t | grep -B1 'interrupt self-test'
nova-core 0000:c1:00.0: NVIDIA (Chipset: TU117, Architecture: Turing, Revision: a.1)
nova-core 0000:c1:00.0: interrupt self-test: starting on vector 129, subtree 2, with Msi
nova-core 0000:c1:00.0: interrupt self-test: passed, subtree 2, 2 deliveries
--
nova-core 0000:c2:00.0: NVIDIA (Chipset: GB202, Architecture: BlackwellGB20x, Revision: a.1)
nova-core 0000:c2:00.0: interrupt self-test: starting on vector 129, subtree 2, with Msi
nova-core 0000:c2:00.0: interrupt self-test: passed, subtree 2, 2 deliveries
--
nova-core 0000:01:00.0: NVIDIA (Chipset: GA104, Architecture: Ampere, Revision: a.1)
nova-core 0000:01:00.0: interrupt self-test: starting on vector 129, subtree 2, with Msi
nova-core 0000:01:00.0: interrupt self-test: passed, subtree 2, 2 deliveries

So for the series, please feel free to add:

Tested-by: John Hubbard <jhubbard@nvidia.com>


[1] https://github.com/johnhubbard/linux/tree/nova-core-gin-interrupt-tree-v2

thanks,
-- 
John Hubbard