Rust for Linux List
 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; 8+ 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] 8+ 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 16:52 ` [PATCH v3 2/5] rust: pci: resolve IRQ in index() and embed IrqRequest in IrqVector Danilo Krummrich
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ 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] 8+ 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 16:52 ` [PATCH v3 3/5] rust: pci: remove request_irq() and request_threaded_irq() from Device Danilo Krummrich
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 8+ 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] 8+ 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 16:52 ` [PATCH v3 4/5] PCI: Add pci_irq_type() to query the allocated interrupt type Danilo Krummrich
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ 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] 8+ 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 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, 0 replies; 8+ 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] 8+ 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:02 ` [PATCH v3 0/5] Rework PCI IRQ vector code Gary Guo
  5 siblings, 0 replies; 8+ 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] 8+ 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
  0 siblings, 0 replies; 8+ 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] 8+ 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; 8+ 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] 8+ messages in thread

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

Thread overview: 8+ 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 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 16:52 ` [PATCH v3 3/5] rust: pci: remove request_irq() and request_threaded_irq() from Device Danilo Krummrich
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 ` [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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox