[PATCH v2] rust: pci: Allocate and manage PCI interrupt vectors

[PATCH v2] rust: pci: Allocate and manage PCI interrupt vectors

[Joel Fernandes]

Add support to PCI rust module to allocate, free and manage IRQ vectors.
Integrate with devres for managing the allocated resources.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
Previous patch was here:
https://lore.kernel.org/all/20250910035415.381753-1-joelagnelf@nvidia.com/

 rust/kernel/pci.rs | 199 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 186 insertions(+), 13 deletions(-)

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 78271bf88cea..f97a6a36cf5e 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -6,8 +6,9 @@
 
 use crate::{
     bindings, container_of, device,
+    device::Bound,
     device_id::{RawDeviceId, RawDeviceIdIndex},
-    devres::Devres,
+    devres::{self, Devres},
     driver,
     error::{from_result, to_result, Result},
     io::{Io, IoRaw},
@@ -19,7 +20,7 @@
 };
 use core::{
     marker::PhantomData,
-    ops::Deref,
+    ops::{Deref, RangeInclusive},
     ptr::{addr_of_mut, NonNull},
 };
 use kernel::prelude::*;
@@ -28,6 +29,59 @@
 
 pub use self::id::{Class, ClassMask, Vendor};
 
+/// IRQ type flags for PCI interrupt allocation.
+#[derive(Debug, Clone, Copy)]
+pub enum IrqType {
+    /// Legacy INTx interrupts
+    Legacy,
+    /// Message Signaled Interrupts (MSI)
+    Msi,
+    /// Extended Message Signaled Interrupts (MSI-X)
+    MsiX,
+}
+
+impl IrqType {
+    /// Convert to the corresponding kernel flags
+    const fn as_raw(self) -> u32 {
+        match self {
+            IrqType::Legacy => bindings::PCI_IRQ_INTX,
+            IrqType::Msi => bindings::PCI_IRQ_MSI,
+            IrqType::MsiX => bindings::PCI_IRQ_MSIX,
+        }
+    }
+}
+
+/// Set of IRQ types that can be used for PCI interrupt allocation.
+#[derive(Debug, Clone, Copy, Default)]
+pub struct IrqTypes(u32);
+
+impl IrqTypes {
+    /// Create a set containing all IRQ types (MSI-X, MSI, and Legacy)
+    pub const fn all() -> Self {
+        Self(bindings::PCI_IRQ_ALL_TYPES)
+    }
+
+    /// Build a set of IRQ types
+    ///
+    /// # Examples
+    ///
+    /// ```ignore
+    /// // Create a set with only MSI and MSI-X (no legacy interrupts)
+    /// let msi_only = IrqTypes::default()
+    ///     .with(IrqType::Msi)
+    ///     .with(IrqType::MsiX);
+    /// ```
+    pub const fn with(mut self, irq_type: IrqType) -> Self {
+        self.0 |= irq_type.as_raw();
+        self
+    }
+
+    /// Get the raw flags value
+    const fn as_raw(self) -> u32 {
+        self.0
+    }
+}
+
 /// An adapter for the registration of PCI drivers.
 pub struct Adapter<T: Driver>(T);
 
@@ -516,6 +570,76 @@ pub fn pci_class(&self) -> Class {
     }
 }
 
+/// Represents an allocated IRQ vector for a specific PCI device.
+///
+/// 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)]
+pub struct IrqVector<'a> {
+    dev: &'a Device<Bound>,
+    index: u32,
+}
+
+impl<'a> IrqVector<'a> {
+    /// Creates a new `IrqVector` for the given device and index.
+    ///
+    /// # Safety
+    ///
+    /// - `index` must be a valid IRQ vector index for `dev`.
+    unsafe fn new(dev: &'a Device<Bound>, index: u32) -> Self {
+        Self { dev, index }
+    }
+
+    /// Returns the raw vector index.
+    fn index(&self) -> u32 {
+        self.index
+    }
+}
+
+/// Represents an IRQ vector allocation for a PCI device.
+///
+/// This type ensures that IRQ vectors are properly allocated and freed by
+/// tying the allocation to the lifetime of this registration object.
+struct IrqVectorRegistration {
+    dev: ARef<Device>,
+}
+
+impl IrqVectorRegistration {
+    /// Allocate IRQ vectors for the given PCI device.
+    ///
+    /// Returns the registration object and a range of valid IRQ vectors.
+    fn new<'a>(
+        dev: &'a Device<Bound>,
+        min_vecs: u32,
+        max_vecs: u32,
+        irq_types: IrqTypes,
+    ) -> Result<(Self, 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 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: Vectors are 0-based, so valid indices are [0, count-1].
+        // pci_alloc_irq_vectors guarantees count >= min_vecs > 0, so count - 1 is valid.
+        let range = unsafe { IrqVector::new(dev, 0)..=IrqVector::new(dev, count - 1) };
+
+        Ok((Self { dev: dev.into() }, range))
+    }
+}
+
+impl Drop for IrqVectorRegistration {
+    fn drop(&mut self) {
+        // SAFETY: `self.dev` is a valid ARef to a `struct pci_dev` that has successfully
+        // allocated IRQ vectors.
+        unsafe { bindings::pci_free_irq_vectors(self.dev.as_raw()) };
+    }
+}
+
 impl Device<device::Bound> {
     /// Mapps an entire PCI-BAR after performing a region-request on it. I/O operation bound checks
     /// can be performed on compile time for offsets (plus the requested type size) < SIZE.
@@ -536,10 +660,15 @@ pub fn iomap_region<'a>(
         self.iomap_region_sized::<0>(bar, name)
     }
 
-    /// Returns an [`IrqRequest`] for the IRQ vector at the given index, if any.
-    pub fn irq_vector(&self, index: u32) -> Result<IrqRequest<'_>> {
+    /// Returns an [`IrqRequest`] for the given IRQ vector.
+    pub fn irq_vector(&self, vector: IrqVector<'_>) -> Result<IrqRequest<'_>> {
+        // Verify that the vector belongs to this device
+        if !core::ptr::eq(vector.dev.as_raw(), self.as_raw()) {
+            return Err(EINVAL);
+        }
+
         // SAFETY: `self.as_raw` returns a valid pointer to a `struct pci_dev`.
-        let irq = unsafe { crate::bindings::pci_irq_vector(self.as_raw(), index) };
+        let irq = unsafe { crate::bindings::pci_irq_vector(self.as_raw(), vector.index()) };
         if irq < 0 {
             return Err(crate::error::Error::from_errno(irq));
         }
@@ -547,35 +676,79 @@ pub fn irq_vector(&self, index: u32) -> Result<IrqRequest<'_>> {
         Ok(unsafe { IrqRequest::new(self.as_ref(), irq as u32) })
     }
 
-    /// Returns a [`kernel::irq::Registration`] for the IRQ vector at the given
-    /// index.
+    /// Returns a [`kernel::irq::Registration`] for the given IRQ vector.
     pub fn request_irq<'a, T: crate::irq::Handler + 'static>(
         &'a self,
-        index: u32,
+        vector: IrqVector<'_>,
         flags: irq::Flags,
         name: &'static CStr,
         handler: impl PinInit<T, Error> + 'a,
     ) -> Result<impl PinInit<irq::Registration<T>, Error> + 'a> {
-        let request = self.irq_vector(index)?;
+        let request = self.irq_vector(vector)?;
 
         Ok(irq::Registration::<T>::new(request, flags, name, handler))
     }
 
-    /// Returns a [`kernel::irq::ThreadedRegistration`] for the IRQ vector at
-    /// the given index.
+    /// Returns a [`kernel::irq::ThreadedRegistration`] for the given IRQ vector.
     pub fn request_threaded_irq<'a, T: crate::irq::ThreadedHandler + 'static>(
         &'a self,
-        index: u32,
+        vector: IrqVector<'_>,
         flags: irq::Flags,
         name: &'static CStr,
         handler: impl PinInit<T, Error> + 'a,
     ) -> Result<impl PinInit<irq::ThreadedRegistration<T>, Error> + 'a> {
-        let request = self.irq_vector(index)?;
+        let request = self.irq_vector(vector)?;
 
         Ok(irq::ThreadedRegistration::<T>::new(
             request, flags, name, handler,
         ))
     }
+
+    /// Allocate IRQ vectors for this PCI device with automatic cleanup.
+    ///
+    /// Allocates between `min_vecs` and `max_vecs` interrupt vectors for the device.
+    /// The allocation will use MSI-X, MSI, or legacy 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 legacy interrupts.
+    ///
+    /// The allocated vectors are automatically freed when the device is unbound, using the
+    /// devres (device resource management) system.
+    ///
+    /// # Arguments
+    ///
+    /// * `min_vecs` - Minimum number of vectors required
+    /// * `max_vecs` - Maximum number of vectors to allocate
+    /// * `irq_types` - Types of interrupts that can be used
+    ///
+    /// # Returns
+    ///
+    /// Returns a range of IRQ vectors that were successfully allocated, or an error if the
+    /// allocation fails or cannot meet the minimum requirement.
+    ///
+    /// # Examples
+    ///
+    /// ```ignore
+    /// // Allocate using any available interrupt type in the order mentioned above.
+    /// let vectors = dev.alloc_irq_vectors(1, 32, IrqTypes::all())?;
+    ///
+    /// // Allocate MSI or MSI-X only (no legacy interrupts)
+    /// let msi_only = IrqTypes::default()
+    ///     .with(IrqType::Msi)
+    ///     .with(IrqType::MsiX);
+    /// let vectors = dev.alloc_irq_vectors(4, 16, msi_only)?;
+    /// ```
+    pub fn alloc_irq_vectors(
+        &self,
+        min_vecs: u32,
+        max_vecs: u32,
+        irq_types: IrqTypes,
+    ) -> Result<RangeInclusive<IrqVector<'_>>> {
+        let (irq_vecs, range) = IrqVectorRegistration::new(self, min_vecs, max_vecs, irq_types)?;
+
+        devres::register(self.as_ref(), irq_vecs, GFP_KERNEL)?;
+
+        Ok(range)
+    }
 }
 
 impl Device<device::Core> {
-- 
2.34.1

Re: [PATCH v2] rust: pci: Allocate and manage PCI interrupt vectors

[Bjorn Helgaas]

On Thu, Oct 02, 2025 at 02:39:12PM -0400, Joel Fernandes wrote:
> Add support to PCI rust module to allocate, free and manage IRQ vectors.
> Integrate with devres for managing the allocated resources.

> +/// IRQ type flags for PCI interrupt allocation.
> +#[derive(Debug, Clone, Copy)]
> +pub enum IrqType {
> +    /// Legacy INTx interrupts
> +    Legacy,

FWIW, when I can, I try to use "INTx" instead of "legacy" because
"INTx" has a specific meaning and is used in the PCIe specs, while
"legacy" by itself has no intrinsic meaning.

> +    /// Message Signaled Interrupts (MSI)
> +    Msi,
> +    /// Extended Message Signaled Interrupts (MSI-X)
> +    MsiX,
> +}

Re: [PATCH v2] rust: pci: Allocate and manage PCI interrupt vectors

[Joel Fernandes]

On 10/3/2025 1:33 PM, Bjorn Helgaas wrote:
> On Thu, Oct 02, 2025 at 02:39:12PM -0400, Joel Fernandes wrote:
>> Add support to PCI rust module to allocate, free and manage IRQ vectors.
>> Integrate with devres for managing the allocated resources.
> 
>> +/// IRQ type flags for PCI interrupt allocation.
>> +#[derive(Debug, Clone, Copy)]
>> +pub enum IrqType {
>> +    /// Legacy INTx interrupts
>> +    Legacy,
> 
> FWIW, when I can, I try to use "INTx" instead of "legacy" because
> "INTx" has a specific meaning and is used in the PCIe specs, while
> "legacy" by itself has no intrinsic meaning.
> 
Sure, I will avoid using the word legacy for naming. thanks,

 - Joel

Re: [PATCH v2] rust: pci: Allocate and manage PCI interrupt vectors

[Danilo Krummrich]

On Thu Oct 2, 2025 at 8:39 PM CEST, Joel Fernandes wrote:
> Add support to PCI rust module to allocate, free and manage IRQ vectors.
> Integrate with devres for managing the allocated resources.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> Previous patch was here:
> https://lore.kernel.org/all/20250910035415.381753-1-joelagnelf@nvidia.com/
>
>  rust/kernel/pci.rs | 199 ++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 186 insertions(+), 13 deletions(-)
>
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 78271bf88cea..f97a6a36cf5e 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -6,8 +6,9 @@
>  
>  use crate::{
>      bindings, container_of, device,
> +    device::Bound,
>      device_id::{RawDeviceId, RawDeviceIdIndex},
> -    devres::Devres,
> +    devres::{self, Devres},
>      driver,
>      error::{from_result, to_result, Result},
>      io::{Io, IoRaw},
> @@ -19,7 +20,7 @@
>  };
>  use core::{
>      marker::PhantomData,
> -    ops::Deref,
> +    ops::{Deref, RangeInclusive},
>      ptr::{addr_of_mut, NonNull},
>  };
>  use kernel::prelude::*;
> @@ -28,6 +29,59 @@
>  
>  pub use self::id::{Class, ClassMask, Vendor};
>  
> +/// IRQ type flags for PCI interrupt allocation.
> +#[derive(Debug, Clone, Copy)]
> +pub enum IrqType {
> +    /// Legacy INTx interrupts
> +    Legacy,

Like Bjorn said, I'd go with INTx too, also given that the C define is
PCI_IRQ_INTX.

> +    /// Message Signaled Interrupts (MSI)
> +    Msi,
> +    /// Extended Message Signaled Interrupts (MSI-X)
> +    MsiX,
> +}
> +
> +impl IrqType {
> +    /// Convert to the corresponding kernel flags

Please end with a period, here and in multiple other places.

> +    const fn as_raw(self) -> u32 {
> +        match self {
> +            IrqType::Legacy => bindings::PCI_IRQ_INTX,
> +            IrqType::Msi => bindings::PCI_IRQ_MSI,
> +            IrqType::MsiX => bindings::PCI_IRQ_MSIX,
> +        }
> +    }
> +}
> +
> +/// Set of IRQ types that can be used for PCI interrupt allocation.
> +#[derive(Debug, Clone, Copy, Default)]
> +pub struct IrqTypes(u32);
> +
> +impl IrqTypes {
> +    /// Create a set containing all IRQ types (MSI-X, MSI, and Legacy)
> +    pub const fn all() -> Self {
> +        Self(bindings::PCI_IRQ_ALL_TYPES)
> +    }
> +
> +    /// Build a set of IRQ types
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```ignore
> +    /// // Create a set with only MSI and MSI-X (no legacy interrupts)
> +    /// let msi_only = IrqTypes::default()
> +    ///     .with(IrqType::Msi)
> +    ///     .with(IrqType::MsiX);
> +    /// ```
> +    pub const fn with(mut self, irq_type: IrqType) -> Self {
> +        self.0 |= irq_type.as_raw();
> +        self

NIT: I'd probably write this as:
	
	Self(self.0 | irq_type.as_raw())

> +    }
> +
> +    /// Get the raw flags value
> +    const fn as_raw(self) -> u32 {
> +        self.0
> +    }
> +}
> +
>  /// An adapter for the registration of PCI drivers.
>  pub struct Adapter<T: Driver>(T);
>  
> @@ -516,6 +570,76 @@ pub fn pci_class(&self) -> Class {
>      }
>  }
>  
> +/// Represents an allocated IRQ vector for a specific PCI device.
> +///
> +/// 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)]
> +pub struct IrqVector<'a> {
> +    dev: &'a Device<Bound>,
> +    index: u32,
> +}
> +
> +impl<'a> IrqVector<'a> {
> +    /// Creates a new `IrqVector` for the given device and index.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `index` must be a valid IRQ vector index for `dev`.
> +    unsafe fn new(dev: &'a Device<Bound>, index: u32) -> Self {
> +        Self { dev, index }
> +    }
> +
> +    /// Returns the raw vector index.
> +    fn index(&self) -> u32 {
> +        self.index
> +    }
> +}
> +
> +/// Represents an IRQ vector allocation for a PCI device.
> +///
> +/// This type ensures that IRQ vectors are properly allocated and freed by
> +/// tying the allocation to the lifetime of this registration object.
> +struct IrqVectorRegistration {
> +    dev: ARef<Device>,
> +}
> +
> +impl IrqVectorRegistration {
> +    /// Allocate IRQ vectors for the given PCI device.
> +    ///
> +    /// Returns the registration object and a range of valid IRQ vectors.
> +    fn new<'a>(
> +        dev: &'a Device<Bound>,
> +        min_vecs: u32,
> +        max_vecs: u32,
> +        irq_types: IrqTypes,
> +    ) -> Result<(Self, 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 parameters and returns error codes.

"all other parameters"?

Please also format multiple statements in a safety comment as list.

> +        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: Vectors are 0-based, so valid indices are [0, count-1].
> +        // pci_alloc_irq_vectors guarantees count >= min_vecs > 0, so count - 1 is valid.

This is a justification why the range makes sense (which makes sense to keep as
a separate comment), but it doesn't justify the safety requirement of
IrqVector::new().

> +        let range = unsafe { IrqVector::new(dev, 0)..=IrqVector::new(dev, count - 1) };
> +
> +        Ok((Self { dev: dev.into() }, range))
> +    }
> +}
> +
> +impl Drop for IrqVectorRegistration {
> +    fn drop(&mut self) {
> +        // SAFETY: `self.dev` is a valid ARef to a `struct pci_dev` that has successfully
> +        // allocated IRQ vectors.

The "successfully allocated IRQ vectors" part should be a type invariant.

NIT: s/ARef/`ARef`/

> +        unsafe { bindings::pci_free_irq_vectors(self.dev.as_raw()) };
> +    }
> +}

<snip>

> +    /// Allocate IRQ vectors for this PCI device with automatic cleanup.
> +    ///
> +    /// Allocates between `min_vecs` and `max_vecs` interrupt vectors for the device.
> +    /// The allocation will use MSI-X, MSI, or legacy 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 legacy interrupts.
> +    ///
> +    /// The allocated vectors are automatically freed when the device is unbound, using the
> +    /// devres (device resource management) system.
> +    ///
> +    /// # Arguments
> +    ///
> +    /// * `min_vecs` - Minimum number of vectors required
> +    /// * `max_vecs` - Maximum number of vectors to allocate
> +    /// * `irq_types` - Types of interrupts that can be used
> +    ///
> +    /// # Returns
> +    ///
> +    /// Returns a range of IRQ vectors that were successfully allocated, or an error if the
> +    /// allocation fails or cannot meet the minimum requirement.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```ignore
> +    /// // Allocate using any available interrupt type in the order mentioned above.
> +    /// let vectors = dev.alloc_irq_vectors(1, 32, IrqTypes::all())?;
> +    ///
> +    /// // Allocate MSI or MSI-X only (no legacy interrupts)
> +    /// let msi_only = IrqTypes::default()
> +    ///     .with(IrqType::Msi)
> +    ///     .with(IrqType::MsiX);
> +    /// let vectors = dev.alloc_irq_vectors(4, 16, msi_only)?;
> +    /// ```
> +    pub fn alloc_irq_vectors(
> +        &self,
> +        min_vecs: u32,
> +        max_vecs: u32,
> +        irq_types: IrqTypes,
> +    ) -> Result<RangeInclusive<IrqVector<'_>>> {
> +        let (irq_vecs, range) = IrqVectorRegistration::new(self, min_vecs, max_vecs, irq_types)?;
> +
> +        devres::register(self.as_ref(), irq_vecs, GFP_KERNEL)?;

If we move the call to devres::register() into IrqVectorRegistration::new()
(which I'd call IrqVectorRegistration::register() then) we can enforce the
guarantee that an IrqVectorRegistration must not out-live the device / driver
binding internally.

> +        Ok(range)
> +    }
>  }

Re: [PATCH v2] rust: pci: Allocate and manage PCI interrupt vectors

[Joel Fernandes]

On 10/5/2025 8:56 AM, Danilo Krummrich wrote:
>> +        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: Vectors are 0-based, so valid indices are [0, count-1].
>> +        // pci_alloc_irq_vectors guarantees count >= min_vecs > 0, so count - 1 is valid.
>> This is a justification why the range makes sense (which makes sense to keep as
> a separate comment), but it doesn't justify the safety requirement of
> IrqVector::new().

Is the following better? Or did you have some other reasoning you want me to
mention? The safety comes from the fact that both start/end vector indices and
everything in between are valid.

// 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 device `dev`.
// - Vector indices are contiguous, so all vectors in [0, count-1] are valid.

Thanks.

Re: [PATCH v2] rust: pci: Allocate and manage PCI interrupt vectors

[Joel Fernandes]

Hi Danilo,

On 10/5/2025 8:56 AM, Danilo Krummrich wrote:
> 
>> +    /// Allocate IRQ vectors for this PCI device with automatic cleanup.
>> +    ///
>> +    /// Allocates between `min_vecs` and `max_vecs` interrupt vectors for the device.
>> +    /// The allocation will use MSI-X, MSI, or legacy 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 legacy interrupts.
>> +    ///
>> +    /// The allocated vectors are automatically freed when the device is unbound, using the
>> +    /// devres (device resource management) system.
>> +    ///
>> +    /// # Arguments
>> +    ///
>> +    /// * `min_vecs` - Minimum number of vectors required
>> +    /// * `max_vecs` - Maximum number of vectors to allocate
>> +    /// * `irq_types` - Types of interrupts that can be used
>> +    ///
>> +    /// # Returns
>> +    ///
>> +    /// Returns a range of IRQ vectors that were successfully allocated, or an error if the
>> +    /// allocation fails or cannot meet the minimum requirement.
>> +    ///
>> +    /// # Examples
>> +    ///
>> +    /// ```ignore
>> +    /// // Allocate using any available interrupt type in the order mentioned above.
>> +    /// let vectors = dev.alloc_irq_vectors(1, 32, IrqTypes::all())?;
>> +    ///
>> +    /// // Allocate MSI or MSI-X only (no legacy interrupts)
>> +    /// let msi_only = IrqTypes::default()
>> +    ///     .with(IrqType::Msi)
>> +    ///     .with(IrqType::MsiX);
>> +    /// let vectors = dev.alloc_irq_vectors(4, 16, msi_only)?;
>> +    /// ```
>> +    pub fn alloc_irq_vectors(
>> +        &self,
>> +        min_vecs: u32,
>> +        max_vecs: u32,
>> +        irq_types: IrqTypes,
>> +    ) -> Result<RangeInclusive<IrqVector<'_>>> {
>> +        let (irq_vecs, range) = IrqVectorRegistration::new(self, min_vecs, max_vecs, irq_types)?;
>> +
>> +        devres::register(self.as_ref(), irq_vecs, GFP_KERNEL)?;
>> If we move the call to devres::register() into IrqVectorRegistration::new()
> (which I'd call IrqVectorRegistration::register() then) we can enforce the
> guarantee that an IrqVectorRegistration must not out-live the device / driver
> binding internally.

Great idea, so paraphrasing for myself, your point is with the above code,
someone could theoretically do:

  1. Call new() directly on IrqVectorRegistration (bypassing alloc_irq_vectors()).
  2. Forget to call devres::register().
  3. Store the IrqVectorRegistration somewhere.
  4. Device gets unbound.
  5. Later when IrqVectorRegistration::drop() runs, it tries to free vectors on
a device that's gone.

Is that right?

So a better approach as you mentioned, is to do the devres registration during
the construction of the IrqVectorRegistration, so there's no way to do one
without the other. Did I get that right? Anyway great point and I have made this
change, thanks!

 - Joel

Re: [PATCH v2] rust: pci: Allocate and manage PCI interrupt vectors

[Danilo Krummrich]

On Wed Oct 8, 2025 at 8:45 PM CEST, Joel Fernandes wrote:
> Great idea, so paraphrasing for myself, your point is with the above code,
> someone could theoretically do:
>
>   1. Call new() directly on IrqVectorRegistration (bypassing alloc_irq_vectors()).
>   2. Forget to call devres::register().
>   3. Store the IrqVectorRegistration somewhere.
>   4. Device gets unbound.
>   5. Later when IrqVectorRegistration::drop() runs, it tries to free vectors on
> a device that's gone.
>
> Is that right?

Correct -- however, it's less about this could actually happen, since it's not a
public type. But it safes you writing invariants, unsafe calls, makes the code
cleaner and more self-contained.

> So a better approach as you mentioned, is to do the devres registration during
> the construction of the IrqVectorRegistration, so there's no way to do one
> without the other. Did I get that right? Anyway great point and I have made this
> change, thanks!

Great, thanks!