All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/5] Rework PCI IRQ vector code
@ 2026-08-13 16:52 Danilo Krummrich
  2026-08-13 16:52 ` [PATCH v3 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type Danilo Krummrich
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Danilo Krummrich @ 2026-08-13 16:52 UTC (permalink / raw)
  To: bhelgaas, dakr, kwilczynski, aliceryhl, daniel.almeida, ojeda,
	boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross, tamird,
	acourbot, work, jhubbard, ttabi, apopple, ecourtney, shashanks,
	zhiw
  Cc: driver-core, linux-pci, rust-for-linux, linux-kernel

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 index() 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 v3:
  - Rename s/count/len/, s/vector()/index()/, s/vector_count()/len()/.
  - Remove redundant range check in index().
  - Add missing #[inline].

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 index() 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 | 220 ++++++++++++++++++-----------------------
 4 files changed, 128 insertions(+), 125 deletions(-)


base-commit: dbaafe9cc56a996931eedfe043eb34418cc9cd9b
-- 
2.55.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type
  2026-08-13 16:52 [PATCH v3 0/5] Rework PCI IRQ vector code Danilo Krummrich
@ 2026-08-13 16:52 ` Danilo Krummrich
  2026-08-13 17:06   ` sashiko-bot
  2026-08-13 16:52 ` [PATCH v3 2/5] rust: pci: resolve IRQ in index() and embed IrqRequest in IrqVector Danilo Krummrich
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Danilo Krummrich @ 2026-08-13 16:52 UTC (permalink / raw)
  To: bhelgaas, dakr, kwilczynski, aliceryhl, daniel.almeida, ojeda,
	boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross, tamird,
	acourbot, work, jhubbard, ttabi, apopple, ecourtney, shashanks,
	zhiw
  Cc: driver-core, linux-pci, rust-for-linux, linux-kernel

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 index() 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.

Tested-by: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/pci.rs     |   3 +-
 rust/kernel/pci/irq.rs | 128 ++++++++++++++++++++++-------------------
 2 files changed, 70 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..daba86505cd2 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,83 @@ 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 `len` interrupt vectors.
+pub struct IrqVectorRegistration<'a> {
+    dev: &'a Device<Bound>,
+    len: 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]
+    #[allow(clippy::len_without_is_empty)]
+    pub fn len(&self) -> usize {
+        self.len.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`.
+    ///
+    /// Returns [`EINVAL`] if the `index` is out of bounds for the length reported by
+    /// [`Self::len()`].
+    #[inline]
+    pub fn index(&self, index: usize) -> Result<IrqVector<'_>> {
+        if index >= self.len.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 +208,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 +227,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 +251,20 @@ 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 len = NonZero::new(ret as usize).ok_or(EINVAL)?;
+
+        // INVARIANT: `pci_alloc_irq_vectors()` allocated `len` vectors for `self`.
+        Ok(IrqVectorRegistration { dev: self, len })
     }
 }
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 2/5] rust: pci: resolve IRQ in index() and embed IrqRequest in IrqVector
  2026-08-13 16:52 [PATCH v3 0/5] Rework PCI IRQ vector code Danilo Krummrich
  2026-08-13 16:52 ` [PATCH v3 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type Danilo Krummrich
@ 2026-08-13 16:52 ` Danilo Krummrich
  2026-08-13 17:00   ` Gary Guo
  2026-08-13 17:11   ` sashiko-bot
  2026-08-13 16:52 ` [PATCH v3 3/5] rust: pci: remove request_irq() and request_threaded_irq() from Device Danilo Krummrich
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 13+ messages in thread
From: Danilo Krummrich @ 2026-08-13 16:52 UTC (permalink / raw)
  To: bhelgaas, dakr, kwilczynski, aliceryhl, daniel.almeida, ojeda,
	boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross, tamird,
	acourbot, work, jhubbard, ttabi, apopple, ecourtney, shashanks,
	zhiw
  Cc: driver-core, linux-pci, rust-for-linux, linux-kernel

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

IrqVector now embeds the resolved IrqRequest and a reference to the
IrqVectorRegistration. The conversion to IrqRequest is infallible, which
removes the need for pin_init_scope() in request_irq() /
request_threaded_irq().

Tested-by: John Hubbard <jhubbard@nvidia.com>
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 | 67 +++++++++++++++---------------------------
 1 file changed, 23 insertions(+), 44 deletions(-)

diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
index daba86505cd2..81b74c4c17d9 100644
--- a/rust/kernel/pci/irq.rs
+++ b/rust/kernel/pci/irq.rs
@@ -68,32 +68,25 @@ 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::index`] 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,
 }
 
 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 }
-    }
-
-    /// Returns the raw vector index.
-    fn index(&self) -> u32 {
-        self.index
+    unsafe fn new(request: IrqRequest<'a>, reg: &'a IrqVectorRegistration<'a>) -> Self {
+        Self { request, reg }
     }
 
     /// Returns the [`IrqVectorRegistration`] this vector was derived from.
@@ -103,17 +96,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
     }
 }
 
@@ -146,13 +132,14 @@ pub fn len(&self) -> usize {
     /// [`Self::len()`].
     #[inline]
     pub fn index(&self, index: usize) -> Result<IrqVector<'_>> {
-        if index >= self.len.get() {
-            return Err(EINVAL);
+        // 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: `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: `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) })
     }
 }
 
@@ -179,12 +166,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.
@@ -200,12 +183,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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 3/5] rust: pci: remove request_irq() and request_threaded_irq() from Device
  2026-08-13 16:52 [PATCH v3 0/5] Rework PCI IRQ vector code Danilo Krummrich
  2026-08-13 16:52 ` [PATCH v3 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type Danilo Krummrich
  2026-08-13 16:52 ` [PATCH v3 2/5] rust: pci: resolve IRQ in index() and embed IrqRequest in IrqVector Danilo Krummrich
@ 2026-08-13 16:52 ` Danilo Krummrich
  2026-08-13 17:05   ` sashiko-bot
  2026-08-13 16:52 ` [PATCH v3 4/5] PCI: Add pci_irq_type() to query the allocated interrupt type Danilo Krummrich
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Danilo Krummrich @ 2026-08-13 16:52 UTC (permalink / raw)
  To: bhelgaas, dakr, kwilczynski, aliceryhl, daniel.almeida, ojeda,
	boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross, tamird,
	acourbot, work, jhubbard, ttabi, apopple, ecourtney, shashanks,
	zhiw
  Cc: driver-core, linux-pci, rust-for-linux, linux-kernel

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.

Tested-by: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/pci/irq.rs | 48 +++++-------------------------------------
 1 file changed, 5 insertions(+), 43 deletions(-)

diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
index 81b74c4c17d9..b6d1699ee3eb 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,8 @@ const fn as_raw(self) -> u32 {
 
 /// A resolved IRQ vector from a PCI interrupt vector allocation.
 ///
-/// Created by [`IrqVectorRegistration::index`] 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::index`]. Convert to [`IrqRequest`] via [`From`] to register
+/// a handler with [`irq::Registration::new`](crate::irq::Registration::new).
 pub struct IrqVector<'a> {
     request: IrqRequest<'a>,
     reg: &'a IrqVectorRegistration<'a>,
@@ -153,40 +149,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.
@@ -195,8 +157,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::index`] to obtain an [`IrqVector`] for a given vector
+    /// index.
     ///
     /// # Arguments
     ///
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 4/5] PCI: Add pci_irq_type() to query the allocated interrupt type
  2026-08-13 16:52 [PATCH v3 0/5] Rework PCI IRQ vector code Danilo Krummrich
                   ` (2 preceding siblings ...)
  2026-08-13 16:52 ` [PATCH v3 3/5] rust: pci: remove request_irq() and request_threaded_irq() from Device Danilo Krummrich
@ 2026-08-13 16:52 ` Danilo Krummrich
  2026-08-13 17:04   ` sashiko-bot
  2026-08-13 16:52 ` [PATCH v3 5/5] rust: pci: expose " Danilo Krummrich
  2026-08-13 17:02 ` [PATCH v3 0/5] Rework PCI IRQ vector code Gary Guo
  5 siblings, 1 reply; 13+ messages in thread
From: Danilo Krummrich @ 2026-08-13 16:52 UTC (permalink / raw)
  To: bhelgaas, dakr, kwilczynski, aliceryhl, daniel.almeida, ojeda,
	boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross, tamird,
	acourbot, work, jhubbard, ttabi, apopple, ecourtney, shashanks,
	zhiw
  Cc: driver-core, linux-pci, rust-for-linux, linux-kernel

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>
Tested-by: John Hubbard <jhubbard@nvidia.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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 5/5] rust: pci: expose the allocated interrupt type
  2026-08-13 16:52 [PATCH v3 0/5] Rework PCI IRQ vector code Danilo Krummrich
                   ` (3 preceding siblings ...)
  2026-08-13 16:52 ` [PATCH v3 4/5] PCI: Add pci_irq_type() to query the allocated interrupt type Danilo Krummrich
@ 2026-08-13 16:52 ` Danilo Krummrich
  2026-08-13 17:07   ` sashiko-bot
  2026-08-13 17:02 ` [PATCH v3 0/5] Rework PCI IRQ vector code Gary Guo
  5 siblings, 1 reply; 13+ messages in thread
From: Danilo Krummrich @ 2026-08-13 16:52 UTC (permalink / raw)
  To: bhelgaas, dakr, kwilczynski, aliceryhl, daniel.almeida, ojeda,
	boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross, tamird,
	acourbot, work, jhubbard, ttabi, apopple, ecourtney, shashanks,
	zhiw
  Cc: driver-core, linux-pci, rust-for-linux, linux-kernel

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.

Tested-by: John Hubbard <jhubbard@nvidia.com>
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 | 23 +++++++++++++++++++++++
 2 files changed, 28 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 b6d1699ee3eb..6741046ec1c0 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.
@@ -90,6 +100,12 @@ unsafe fn new(request: IrqRequest<'a>, reg: &'a IrqVectorRegistration<'a>) -> Se
     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()
+    }
 }
 
 impl<'a> From<IrqVector<'a>> for IrqRequest<'a> {
@@ -122,6 +138,13 @@ pub fn len(&self) -> usize {
         self.len.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`.
     ///
     /// Returns [`EINVAL`] if the `index` is out of bounds for the length reported by
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/5] rust: pci: resolve IRQ in index() and embed IrqRequest in IrqVector
  2026-08-13 16:52 ` [PATCH v3 2/5] rust: pci: resolve IRQ in index() and embed IrqRequest in IrqVector Danilo Krummrich
@ 2026-08-13 17:00   ` Gary Guo
  2026-08-13 17:11   ` sashiko-bot
  1 sibling, 0 replies; 13+ messages in thread
From: Gary Guo @ 2026-08-13 17:00 UTC (permalink / raw)
  To: Danilo Krummrich, bhelgaas, kwilczynski, aliceryhl,
	daniel.almeida, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg,
	tmgross, tamird, acourbot, work, jhubbard, ttabi, apopple,
	ecourtney, shashanks, zhiw
  Cc: driver-core, linux-pci, rust-for-linux, linux-kernel

On Thu Aug 13, 2026 at 5:52 PM BST, Danilo Krummrich wrote:
> Move the pci_irq_vector() call from the TryInto<IrqRequest> impl into
> IrqVectorRegistration::index(), so the IRQ number is resolved eagerly.
>
> IrqVector now embeds the resolved IrqRequest and a reference to the
> IrqVectorRegistration. The conversion to IrqRequest is infallible, which
> removes the need for pin_init_scope() in request_irq() /
> request_threaded_irq().
>
> Tested-by: John Hubbard <jhubbard@nvidia.com>
> 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 | 67 +++++++++++++++---------------------------
>  1 file changed, 23 insertions(+), 44 deletions(-)
>
> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index daba86505cd2..81b74c4c17d9 100644
> --- a/rust/kernel/pci/irq.rs
> +++ b/rust/kernel/pci/irq.rs
> @@ -68,32 +68,25 @@ 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::index`] 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,
>  }
>  
>  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 }
> -    }
> -
> -    /// Returns the raw vector index.
> -    fn index(&self) -> u32 {
> -        self.index
> +    unsafe fn new(request: IrqRequest<'a>, reg: &'a IrqVectorRegistration<'a>) -> Self {
> +        Self { request, reg }
>      }
>  
>      /// Returns the [`IrqVectorRegistration`] this vector was derived from.
> @@ -103,17 +96,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> {

I feel that this is actually one of the prime candidate of `DerefMove` when (or
if) Rust adds that.

If we have !Leak` in the langauge, then we can drop the unsafe on
`irq::Registration::new`, then we can move that to become a method on
`IrqRequest`; if we also have `DerefMove`, then you'd be able to do

    irq_vector.request_thread_irq(...)

and this will look super clean.

That said, we have neither `DerefMove` nor `!Leak`, so a unsafe constructor + a
`.into()` does sound like the best option so far. But one can dream :)

Best,
Gary

> +    #[inline]
> +    fn from(vector: IrqVector<'a>) -> Self {
> +        vector.request
>      }
>  }


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/5] Rework PCI IRQ vector code
  2026-08-13 16:52 [PATCH v3 0/5] Rework PCI IRQ vector code Danilo Krummrich
                   ` (4 preceding siblings ...)
  2026-08-13 16:52 ` [PATCH v3 5/5] rust: pci: expose " Danilo Krummrich
@ 2026-08-13 17:02 ` Gary Guo
  5 siblings, 0 replies; 13+ messages in thread
From: Gary Guo @ 2026-08-13 17:02 UTC (permalink / raw)
  To: Danilo Krummrich, bhelgaas, kwilczynski, aliceryhl,
	daniel.almeida, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg,
	tmgross, tamird, acourbot, work, jhubbard, ttabi, apopple,
	ecourtney, shashanks, zhiw
  Cc: driver-core, linux-pci, rust-for-linux, linux-kernel

On Thu Aug 13, 2026 at 5:52 PM BST, 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 index() 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 v3:
>   - Rename s/count/len/, s/vector()/index()/, s/vector_count()/len()/.
>   - Remove redundant range check in index().
>   - Add missing #[inline].
>
> 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 index() 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

For the series:

Reviewed-by: Gary Guo <gary@garyguo.net>

I think the duplicate irq alloc issue is still worth solving, but I agree that
it should be a separate series.

>   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 | 220 ++++++++++++++++++-----------------------
>  4 files changed, 128 insertions(+), 125 deletions(-)
>
>
> base-commit: dbaafe9cc56a996931eedfe043eb34418cc9cd9b



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 4/5] PCI: Add pci_irq_type() to query the allocated interrupt type
  2026-08-13 16:52 ` [PATCH v3 4/5] PCI: Add pci_irq_type() to query the allocated interrupt type Danilo Krummrich
@ 2026-08-13 17:04   ` sashiko-bot
  0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-13 17:04 UTC (permalink / raw)
  To: Danilo Krummrich; +Cc: ojeda, linux-pci

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing dummy implementation of pci_irq_type() for !CONFIG_PCI configurations
--

--- Patch [4]: [PATCH v3 4/5] PCI: Add pci_irq_type() to query the allocated interrupt type ---
commit dc3ae5a104b31d99bffe0b8a2992cb68e0130731
Author: Danilo Krummrich <dakr@kernel.org>

PCI: Add pci_irq_type() to query the allocated interrupt type

This patch adds a helper function to return the interrupt type (MSI-X, MSI,
or INTX) that the PCI core selected after a successful call to
pci_alloc_irq_vectors(). It provides a common way for drivers to query this
information without directly accessing the pci_dev bitfields.

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

> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 64b308b6e61c1..80b8561b5be0b 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
[ ... ]
> @@ -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

[Severity: Low]
Does this also need a dummy implementation in the !CONFIG_PCI block?

This patch adds a fallback implementation for when CONFIG_PCI_MSI is disabled,
but misses adding a stub for when CONFIG_PCI is completely disabled. Other IRQ
vector API functions provide stubs in the !CONFIG_PCI block to ensure drivers
compile cleanly on architectures without PCI support:

include/linux/pci.h:
static inline int
pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
		      unsigned int max_vecs, unsigned int flags)
{
	return -ENOSPC;
}

static inline void pci_free_irq_vectors(struct pci_dev *dev)
{
}
#endif /* CONFIG_PCI */

Could the lack of a pci_irq_type() stub in include/linux/pci.h introduce a
compile regression when building a kernel with CONFIG_PCI=n where a generic
driver calls pci_irq_type()?

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/5] rust: pci: remove request_irq() and request_threaded_irq() from Device
  2026-08-13 16:52 ` [PATCH v3 3/5] rust: pci: remove request_irq() and request_threaded_irq() from Device Danilo Krummrich
@ 2026-08-13 17:05   ` sashiko-bot
  0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-13 17:05 UTC (permalink / raw)
  To: Danilo Krummrich; +Cc: ojeda, linux-pci

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The new `IrqVectorRegistration` lifetime design creates a self-referential lifetime trap, making it practically impossible for drivers to safely store their IRQ handler registrations in their driver data structures.
--

--- Patch [3]: [PATCH v3 3/5] rust: pci: remove request_irq() and request_threaded_irq() from Device ---
commit 391f2070be9b225ebd0a122e28fa0091732c94e1
Author: Danilo Krummrich <dakr@kernel.org>

rust: pci: remove request_irq() and request_threaded_irq() from Device

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.

> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index 81b74c4c17d98..b6d1699ee3ebf 100644
> --- a/rust/kernel/pci/irq.rs
> +++ b/rust/kernel/pci/irq.rs
> @@ -70,9 +67,8 @@ const fn as_raw(self) -> u32 {
>  
>  /// A resolved IRQ vector from a PCI interrupt vector allocation.
>  ///
> -/// Created by [`IrqVectorRegistration::index`] 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::index`]. Convert to [`IrqRequest`] via [`From`] to register
> +/// a handler with [`irq::Registration::new`](crate::irq::Registration::new).
>  pub struct IrqVector<'a> {
>      request: IrqRequest<'a>,
>      reg: &'a IrqVectorRegistration<'a>,
>  }

[Severity: High]
Does this new IrqVectorRegistration lifetime design create a self-referential
lifetime trap that makes it impossible for drivers to safely store their IRQ
handler registrations?

Because IrqVector uses a single lifetime parameter 'a for both its embedded
IrqRequest<'a> and its borrow of the registration &'a IrqVectorRegistration<'a>,
the returned IrqRequest is forced to inherit the short lifetime of the method
call's borrow of the IrqVectorRegistration when converted via From:

rust/kernel/pci/irq.rs:
impl<'a> From<IrqVector<'a>> for IrqRequest<'a> {
    #[inline]
    fn from(vector: IrqVector<'a>) -> Self {
        vector.request
    }
}

This conversion propagates the strictly constrained borrow lifetime into the
IrqRequest, permanently tying any resulting irq::Registration to the local
scope of the IrqVectorRegistration. 

For a driver to return from its probe() function, it must store both the vector
registration and the IRQ handler registration in its main device struct.
However, this creates a self-referential struct (the IRQ registration borrows
the sibling vector registration field), which is forbidden in safe Rust.

There is no devres escape hatch because devres::register requires 'static, and
DevresLt cannot encapsulate a borrow of a local field.

This affects all PCI drivers attempting to use the new API, breaking the
primary use case of the subsystem.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type
  2026-08-13 16:52 ` [PATCH v3 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type Danilo Krummrich
@ 2026-08-13 17:06   ` sashiko-bot
  0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-13 17:06 UTC (permalink / raw)
  To: Danilo Krummrich; +Cc: linux-pci, ojeda

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The lifetime linkage between `IrqVectorRegistration`, `IrqVector`, and `irq::Registration` forces a self-referential struct design, making the API unusable for device drivers.
--

--- Patch [1]: [PATCH v3 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type ---
commit d29a976d8fb0d09c0565565666049353a896a317
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. It modifies IrqVector to hold a
reference to the registration, preventing the allocation from being dropped
while vectors are still in use.

> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index fea484dcf9cf2..daba86505cd2b 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,
>  }
[ ... ]
> +    pub fn index(&self, index: usize) -> Result<IrqVector<'_>> {
> +        if index >= self.len.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) })
>      }

[Severity: High]
Could this introduce a regression by creating a self-referential struct
requirement for device drivers?

A typical device driver must maintain both the IrqVectorRegistration (to keep
vectors allocated) and the irq::Registration (to keep handlers active) within
its long-lived state struct across the driver lifecycle.

Looking at IrqVectorRegistration::index() in rust/kernel/pci/irq.rs, it returns
an IrqVector tied to the local borrow of self. This lifetime dependency
propagates to irq::Registration.

Storing both objects in the same driver struct requires a self-referential
design, which cannot be safely modeled using the kernel's pin_init macro.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 5/5] rust: pci: expose the allocated interrupt type
  2026-08-13 16:52 ` [PATCH v3 5/5] rust: pci: expose " Danilo Krummrich
@ 2026-08-13 17:07   ` sashiko-bot
  0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-13 17:07 UTC (permalink / raw)
  To: Danilo Krummrich; +Cc: linux-pci, ojeda

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] rust_helper_pci_irq_type() causes a build failure when CONFIG_PCI is disabled because pci_irq_type() is undefined.
--

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

rust: pci: expose the allocated interrupt type

This commit adds irq_type() on IrqVectorRegistration and IrqVector, wrapping
the new pci_irq_type() C function. It allows a driver to query the allocated
interrupt type (MSI-X, MSI, or INTx).

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

> diff --git a/rust/helpers/pci.c b/rust/helpers/pci.c
> index e44905317d759..23b06becb4489 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);
> +}
> +

[Severity: Low]
Does this code cause a build failure when CONFIG_PCI is disabled?

It looks like this regression could result in an implicit declaration error,
because there is no fallback stub for pci_irq_type() in the CONFIG_PCI=n
block of include/linux/pci.h, and this helper is unconditionally compiled here.

[ ... ]

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/5] rust: pci: resolve IRQ in index() and embed IrqRequest in IrqVector
  2026-08-13 16:52 ` [PATCH v3 2/5] rust: pci: resolve IRQ in index() and embed IrqRequest in IrqVector Danilo Krummrich
  2026-08-13 17:00   ` Gary Guo
@ 2026-08-13 17:11   ` sashiko-bot
  1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-13 17:11 UTC (permalink / raw)
  To: Danilo Krummrich; +Cc: linux-pci, ojeda

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

New issues:
- [High] The API design forces a self-referential lifetime bound, making it impossible to safely store both the vector registration and the IRQ handler in a driver struct, naturally leading to Use-After-Free during teardown.

Pre-existing issues:
- [High] Multiple concurrent `IrqVectorRegistration` instances can be created, leading to a Use-After-Free of active IRQ vectors on teardown.
--

--- Patch [2]: [PATCH v3 2/5] rust: pci: resolve IRQ in index() and embed IrqRequest in IrqVector ---
commit 517346220f40fc029bc56afcf3a1427744cf6f09
Author: Danilo Krummrich <dakr@kernel.org>

rust: pci: resolve IRQ in index() and embed IrqRequest in IrqVector

This commit eagerly resolves the IRQ number by moving the pci_irq_vector()
call into IrqVectorRegistration::index(). It embeds the resolved IrqRequest
into IrqVector, making the conversion infallible. This removes the need for
pin_init_scope() in request_irq() and request_threaded_irq().

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

> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index daba86505cd2b..81b74c4c17d98 100644
> --- a/rust/kernel/pci/irq.rs
> +++ b/rust/kernel/pci/irq.rs
> @@ -68,32 +68,25 @@ const fn as_raw(self) -> u32 {
[ ... ]
>  pub struct IrqVector<'a> {
> -    dev: &'a Device<Bound>,
> +    request: IrqRequest<'a>,
>      reg: &'a IrqVectorRegistration<'a>,
> -    index: u32,
>  }

[Severity: High]
Does this struct definition force a self-referential lifetime bound that makes
it impossible to safely store both the vector registration and the IRQ handler
in a driver struct?

Because IrqVector<'a> borrows IrqVectorRegistration<'a> with the exact same
lifetime 'a as the device, request_irq() requires the resulting
irq::Registration to share this lifetime. A long-lived driver struct would
have to hold both the IrqVectorRegistration and the Registration that
borrows it.

This creates a self-referential struct forbidden by Safe Rust. If a developer
uses unsafe to bypass the restrictions to get it to compile, Rust drops
struct fields in declaration order. If IrqVectorRegistration is declared
before Registration, pci_free_irq_vectors() will execute before free_irq(),
destroying MSI vectors while the IRQ handler is still registered and active
in the system.

[ ... ]
> @@ -200,12 +183,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.

[Severity: High]
This is a pre-existing issue, but can multiple concurrent
IrqVectorRegistration instances be created, leading to a Use-After-Free of
active IRQ vectors on teardown?

Since Device::alloc_irq_vectors() takes an immutable reference (&self), a
driver can call it multiple times. If a driver allocates MSI vectors and
then falls back to allocating INTx vectors, the C API
(pci_alloc_irq_vectors_affinity) successfully processes the INTx request
without checking if MSI is already enabled. This returns a second
IrqVectorRegistration.

When this second registration goes out of scope and drops, its Drop
implementation calls pci_free_irq_vectors(), which disables and frees the
global MSI state. The first registration's active IRQ handlers are then
left operating on freed hardware state and descriptors.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-08-13 17:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 16:52 [PATCH v3 0/5] Rework PCI IRQ vector code Danilo Krummrich
2026-08-13 16:52 ` [PATCH v3 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type Danilo Krummrich
2026-08-13 17:06   ` sashiko-bot
2026-08-13 16:52 ` [PATCH v3 2/5] rust: pci: resolve IRQ in index() and embed IrqRequest in IrqVector Danilo Krummrich
2026-08-13 17:00   ` Gary Guo
2026-08-13 17:11   ` sashiko-bot
2026-08-13 16:52 ` [PATCH v3 3/5] rust: pci: remove request_irq() and request_threaded_irq() from Device Danilo Krummrich
2026-08-13 17:05   ` sashiko-bot
2026-08-13 16:52 ` [PATCH v3 4/5] PCI: Add pci_irq_type() to query the allocated interrupt type Danilo Krummrich
2026-08-13 17:04   ` sashiko-bot
2026-08-13 16:52 ` [PATCH v3 5/5] rust: pci: expose " Danilo Krummrich
2026-08-13 17:07   ` sashiko-bot
2026-08-13 17:02 ` [PATCH v3 0/5] Rework PCI IRQ vector code Gary Guo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.