[PATCH] rust: pci: add PCI interrupt allocation and management support

[PATCH] rust: pci: add PCI interrupt allocation and management support

[Joel Fernandes]

Add support for allocating and managing PCI interrupts (MSI-X, MSI, and
legacy) in the Rust PCI abstractions. This provides an interface for
drivers to allocate interrupt vectors and obtain their Linux IRQ
numbers.

Add the following methods to PCI device:
- alloc_irq_vectors() for allocating interrupts during probe
- irq_vector() for obtaining Linux IRQ numbers for each vector
- free_irq_vectors() for releasing interrupt resources during unbind

This is required for Nova's IRQ handling to allocate and manage
interrupts using PCI interrupt APIs.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 rust/kernel/pci.rs | 131 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 131 insertions(+)

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 887ee611b553..98c3a7a04e88 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -23,6 +23,59 @@
 };
 use kernel::prelude::*;
 
+/// 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 to_flags(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)
+    }
+
+    /// Add a single IRQ type to the set
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// // 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.to_flags();
+        self
+    }
+
+    /// Get the raw flags value
+    const fn raw(self) -> u32 {
+        self.0
+    }
+}
+
 /// An adapter for the registration of PCI drivers.
 pub struct Adapter<T: Driver>(T);
 
@@ -413,6 +466,16 @@ pub fn resource_len(&self, bar: u32) -> Result<bindings::resource_size_t> {
 }
 
 impl Device<device::Bound> {
+    /// Free all allocated IRQ vectors for this device.
+    ///
+    /// This should be called to release interrupt resources when they are no longer needed,
+    /// during driver unbind or removal.
+    pub fn free_irq_vectors(&self) {
+        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+        // `pci_free_irq_vectors` is safe to call even if no vectors are currently allocated.
+        unsafe { bindings::pci_free_irq_vectors(self.as_raw()) };
+    }
+
     /// 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.
     pub fn iomap_region_sized<'a, const SIZE: usize>(
@@ -445,6 +508,74 @@ pub fn set_master(&self) {
         // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
         unsafe { bindings::pci_set_master(self.as_raw()) };
     }
+
+    /// 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 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.
+    /// This is called during driver probe.
+    ///
+    /// # 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 the number of vectors successfully allocated, or an error if the allocation
+    /// fails or cannot meet the minimum requirement.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// // Allocate using any available interrupt type in the order mentioned above.
+    /// let nvecs = 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 nvecs = dev.alloc_irq_vectors(4, 16, msi_only)?;
+    /// ```
+    pub fn alloc_irq_vectors(
+        &self,
+        min_vecs: u32,
+        max_vecs: u32,
+        irq_types: IrqTypes,
+    ) -> Result<u32> {
+        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+        // `pci_alloc_irq_vectors` internally validates all parameters and returns error codes.
+        let ret = unsafe {
+            bindings::pci_alloc_irq_vectors(self.as_raw(), min_vecs, max_vecs, irq_types.raw())
+        };
+
+        to_result(ret)?;
+        Ok(ret as u32)
+    }
+
+    /// Get the Linux IRQ number for a specific vector.
+    ///
+    /// This is called during driver probe after successful IRQ allocation
+    /// to obtain the IRQ numbers for registering interrupt handlers.
+    ///
+    /// # Arguments
+    ///
+    /// * `vector` - The vector index (0-based)
+    ///
+    /// # Returns
+    ///
+    /// Returns the Linux IRQ number for the specified vector, or an error if the vector
+    /// index is invalid or no vectors are allocated.
+    pub fn irq_vector(&self, vector: u32) -> Result<u32> {
+        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
+        let irq = unsafe { bindings::pci_irq_vector(self.as_raw(), vector) };
+
+        to_result(irq)?;
+        Ok(irq as u32)
+    }
 }
 
 // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
-- 
2.34.1

Re: [PATCH] rust: pci: add PCI interrupt allocation and management support

[Danilo Krummrich]

On Wed Sep 10, 2025 at 5:54 AM CEST, Joel Fernandes wrote:
>  impl Device<device::Bound> {

The Bound context is not enough for some of the methods below, some of them
require the Core context, more below.

> +    /// Free all allocated IRQ vectors for this device.
> +    ///
> +    /// This should be called to release interrupt resources when they are no longer needed,
> +    /// during driver unbind or removal.
> +    pub fn free_irq_vectors(&self) {
> +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
> +        // `pci_free_irq_vectors` is safe to call even if no vectors are currently allocated.
> +        unsafe { bindings::pci_free_irq_vectors(self.as_raw()) };
> +    }

This requires the Core context, but we should not provide this method at all to
begin with; it puts the burden on drivers to remember calling this.

Instead, alloc_irq_vectors() should register a devres object with
devres::register(), so this gets called automatically when the device is
unbound.

Note that a cleanup through devres is not in conflict with the Core context
requirement.

> +    /// 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 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.
> +    /// This is called during driver probe.
> +    ///
> +    /// # 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 the number of vectors successfully allocated, or an error if the allocation
> +    /// fails or cannot meet the minimum requirement.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// // Allocate using any available interrupt type in the order mentioned above.
> +    /// let nvecs = 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 nvecs = dev.alloc_irq_vectors(4, 16, msi_only)?;
> +    /// ```
> +    pub fn alloc_irq_vectors(
> +        &self,
> +        min_vecs: u32,
> +        max_vecs: u32,
> +        irq_types: IrqTypes,
> +    ) -> Result<u32> {
> +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
> +        // `pci_alloc_irq_vectors` internally validates all parameters and returns error codes.
> +        let ret = unsafe {
> +            bindings::pci_alloc_irq_vectors(self.as_raw(), min_vecs, max_vecs, irq_types.raw())
> +        };
> +
> +        to_result(ret)?;
> +        Ok(ret as u32)
> +    }

This is only valid to be called from the Core context, as it modifies internal
fields of the inner struct device.

Also, it would be nice if it would return a new type that can serve as argument
for irq_vector(), such that we don't have to rely on random integers.

> +
> +    /// Get the Linux IRQ number for a specific vector.
> +    ///
> +    /// This is called during driver probe after successful IRQ allocation
> +    /// to obtain the IRQ numbers for registering interrupt handlers.
> +    ///
> +    /// # Arguments
> +    ///
> +    /// * `vector` - The vector index (0-based)
> +    ///
> +    /// # Returns
> +    ///
> +    /// Returns the Linux IRQ number for the specified vector, or an error if the vector
> +    /// index is invalid or no vectors are allocated.
> +    pub fn irq_vector(&self, vector: u32) -> Result<u32> {

This method is already staged for inclusion in v6.18 in driver-core-next. Please
make sure to base changes on top of the tree mentioned in the maintainers file,
driver-core in this case.

The signature of the existing method is:

	pub fn irq_vector(&self, index: u32) -> Result<IrqRequest<'_>>

We return an IrqRequest, which captures the IRQ number *and* the corresponding
device, such that you can't get the combination wrong.

Maybe it's worth looking at improving the index argument with a new type as
mentioned above.

> +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
> +        let irq = unsafe { bindings::pci_irq_vector(self.as_raw(), vector) };
> +
> +        to_result(irq)?;
> +        Ok(irq as u32)
> +    }
>  }

Re: [PATCH] rust: pci: add PCI interrupt allocation and management support

[Joel Fernandes]

On Wed, Sep 10, 2025 at 10:47:05AM +0200, Danilo Krummrich wrote:
> On Wed Sep 10, 2025 at 5:54 AM CEST, Joel Fernandes wrote:
> >  impl Device<device::Bound> {
> 
> The Bound context is not enough for some of the methods below, some of them
> require the Core context, more below.

Actually my patch already does that, the diff format creates confusion. Some
of the below methods (like alloc) are in fact added to the device::Core
context.

> > +    /// Free all allocated IRQ vectors for this device.
> > +    ///
> > +    /// This should be called to release interrupt resources when they are no longer needed,
> > +    /// during driver unbind or removal.
> > +    pub fn free_irq_vectors(&self) {
> > +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
> > +        // `pci_free_irq_vectors` is safe to call even if no vectors are currently allocated.
> > +        unsafe { bindings::pci_free_irq_vectors(self.as_raw()) };
> > +    }
> 
> This requires the Core context, but we should not provide this method at all to
> begin with; it puts the burden on drivers to remember calling this.
> Instead, alloc_irq_vectors() should register a devres object with
> devres::register(), so this gets called automatically when the device is
> unbound.

Great idea, thanks I will try this out.
> 
> Note that a cleanup through devres is not in conflict with the Core context
> requirement.

Got it.

> > +    /// 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 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.
> > +    /// This is called during driver probe.
> > +    ///
> > +    /// # 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 the number of vectors successfully allocated, or an error if the allocation
> > +    /// fails or cannot meet the minimum requirement.
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// ```
> > +    /// // Allocate using any available interrupt type in the order mentioned above.
> > +    /// let nvecs = 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 nvecs = dev.alloc_irq_vectors(4, 16, msi_only)?;
> > +    /// ```
> > +    pub fn alloc_irq_vectors(
> > +        &self,
> > +        min_vecs: u32,
> > +        max_vecs: u32,
> > +        irq_types: IrqTypes,
> > +    ) -> Result<u32> {
> > +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
> > +        // `pci_alloc_irq_vectors` internally validates all parameters and returns error codes.
> > +        let ret = unsafe {
> > +            bindings::pci_alloc_irq_vectors(self.as_raw(), min_vecs, max_vecs, irq_types.raw())
> > +        };
> > +
> > +        to_result(ret)?;
> > +        Ok(ret as u32)
> > +    }
> 
> This is only valid to be called from the Core context, as it modifies internal
> fields of the inner struct device.

It is called from core context, the diff format confuses.
> 
> Also, it would be nice if it would return a new type that can serve as argument
> for irq_vector(), such that we don't have to rely on random integers.

Makes sense, I will do that.

> > +
> > +    /// Get the Linux IRQ number for a specific vector.
> > +    ///
> > +    /// This is called during driver probe after successful IRQ allocation
> > +    /// to obtain the IRQ numbers for registering interrupt handlers.
> > +    ///
> > +    /// # Arguments
> > +    ///
> > +    /// * `vector` - The vector index (0-based)
> > +    ///
> > +    /// # Returns
> > +    ///
> > +    /// Returns the Linux IRQ number for the specified vector, or an error if the vector
> > +    /// index is invalid or no vectors are allocated.
> > +    pub fn irq_vector(&self, vector: u32) -> Result<u32> {
> 
> This method is already staged for inclusion in v6.18 in driver-core-next. Please
> make sure to base changes on top of the tree mentioned in the maintainers file,
> driver-core in this case.
> 
> The signature of the existing method is:
> 
> 	pub fn irq_vector(&self, index: u32) -> Result<IrqRequest<'_>>
> 
> We return an IrqRequest, which captures the IRQ number *and* the corresponding
> device, such that you can't get the combination wrong.
> 
> Maybe it's worth looking at improving the index argument with a new type as
> mentioned above.

Ah Ok, thanks for pointing this out. I will rebase and reuse this.

thanks,

 - Joel

> 
> > +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
> > +        let irq = unsafe { bindings::pci_irq_vector(self.as_raw(), vector) };
> > +
> > +        to_result(irq)?;
> > +        Ok(irq as u32)
> > +    }
> >  }

Re: [PATCH] rust: pci: add PCI interrupt allocation and management support

[Joel Fernandes]

On Wed, Sep 10, 2025 at 02:09:55PM -0400, Joel Fernandes wrote:
[...] 
> > > +    /// 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 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.
> > > +    /// This is called during driver probe.
> > > +    ///
> > > +    /// # 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 the number of vectors successfully allocated, or an error if the allocation
> > > +    /// fails or cannot meet the minimum requirement.
> > > +    ///
> > > +    /// # Examples
> > > +    ///
> > > +    /// ```
> > > +    /// // Allocate using any available interrupt type in the order mentioned above.
> > > +    /// let nvecs = 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 nvecs = dev.alloc_irq_vectors(4, 16, msi_only)?;
> > > +    /// ```
> > > +    pub fn alloc_irq_vectors(
> > > +        &self,
> > > +        min_vecs: u32,
> > > +        max_vecs: u32,
> > > +        irq_types: IrqTypes,
> > > +    ) -> Result<u32> {
> > > +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
> > > +        // `pci_alloc_irq_vectors` internally validates all parameters and returns error codes.
> > > +        let ret = unsafe {
> > > +            bindings::pci_alloc_irq_vectors(self.as_raw(), min_vecs, max_vecs, irq_types.raw())
> > > +        };
> > > +
> > > +        to_result(ret)?;
> > > +        Ok(ret as u32)
> > > +    }
> > 
> > This is only valid to be called from the Core context, as it modifies internal
> > fields of the inner struct device.
> 
> It is called from core context, the diff format confuses.
> > 
> > Also, it would be nice if it would return a new type that can serve as argument
> > for irq_vector(), such that we don't have to rely on random integers.
> 
> Makes sense, I will do that.
> 
By the way, the "ret" value returned by pci_alloc_irq_vectors() is the number
of vectors, not the vector index. So basically there are 3 numbers that mean
different things:
1. Number of vectors (as returned by alloc_irq_vectors).
2. Index of a vector (passed to pci_irq_vector).
3. The Linux IRQ number (passed to request_irq).

And your point is well taken, in fact even in current code there is
ambiguity: irq_vector() accepts a vector index, where as request_irq()
accepts a Linux IRQ number, which are different numbers. I can try to clean
that up as well but let me know if you had any other thoughts. In fact, I
think Device<device::Bound>::request_irq() pci should just accept IrqRequest?

thanks,

 - Joel

Re: [PATCH] rust: pci: add PCI interrupt allocation and management support

[Danilo Krummrich]

On Wed Sep 10, 2025 at 9:02 PM CEST, Joel Fernandes wrote:
> On Wed, Sep 10, 2025 at 02:09:55PM -0400, Joel Fernandes wrote:
> [...] 
>> > > +    /// 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 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.
>> > > +    /// This is called during driver probe.
>> > > +    ///
>> > > +    /// # 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 the number of vectors successfully allocated, or an error if the allocation
>> > > +    /// fails or cannot meet the minimum requirement.
>> > > +    ///
>> > > +    /// # Examples
>> > > +    ///
>> > > +    /// ```
>> > > +    /// // Allocate using any available interrupt type in the order mentioned above.
>> > > +    /// let nvecs = 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 nvecs = dev.alloc_irq_vectors(4, 16, msi_only)?;
>> > > +    /// ```
>> > > +    pub fn alloc_irq_vectors(
>> > > +        &self,
>> > > +        min_vecs: u32,
>> > > +        max_vecs: u32,
>> > > +        irq_types: IrqTypes,
>> > > +    ) -> Result<u32> {
>> > > +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
>> > > +        // `pci_alloc_irq_vectors` internally validates all parameters and returns error codes.
>> > > +        let ret = unsafe {
>> > > +            bindings::pci_alloc_irq_vectors(self.as_raw(), min_vecs, max_vecs, irq_types.raw())
>> > > +        };
>> > > +
>> > > +        to_result(ret)?;
>> > > +        Ok(ret as u32)
>> > > +    }
>> > 
>> > This is only valid to be called from the Core context, as it modifies internal
>> > fields of the inner struct device.
>> 
>> It is called from core context, the diff format confuses.
>> > 
>> > Also, it would be nice if it would return a new type that can serve as argument
>> > for irq_vector(), such that we don't have to rely on random integers.
>> 
>> Makes sense, I will do that.
>> 
> By the way, the "ret" value returned by pci_alloc_irq_vectors() is the number
> of vectors, not the vector index.

Sure, but the vector index passed to pci_irq_vector() must be in the range
defined by the return value of pci_alloc_irq_vectors().

I thought of e.g. Range<pci::IrqVector> as return value. This way you can easily
iterate it and prove that it's an allocated vector index.

> So basically there are 3 numbers that mean
> different things:
> 1. Number of vectors (as returned by alloc_irq_vectors).
> 2. Index of a vector (passed to pci_irq_vector).
> 3. The Linux IRQ number (passed to request_irq).
>
> And your point is well taken, in fact even in current code there is
> ambiguity: irq_vector() accepts a vector index, where as request_irq()
> accepts a Linux IRQ number, which are different numbers. I can try to clean
> that up as well but let me know if you had any other thoughts. In fact, I
> think Device<device::Bound>::request_irq() pci should just accept IrqRequest?

Currently, pci::Device::request_irq() takes an IRQ vector index and calls
irq_vector() internally to convert the vector index into an IRQ number.

I'd keep this semantics, but introduce a new type IrqVector rather than using
the raw integer. So, drivers would call

	// `irq_vecs` is of type `Range<pci::IrqVector>`.
	let irq_vecs = dev.alloc_irq_vectors(1, 1, pci::IrqTypes::ANY)?;
	let irq = KBox::pin_init(
	   dev.request_irq(irq_vecs.start, ...)?,
	)?;

Alternatively, to request all of them, if we have multiple, we can leverage
KBox::pin_slice(), which will land in v6.18 (see alloc-next or rust-next branch
in the Rust tree), so all irq::Registration objects can be stored in a single
allocation.

Re: [PATCH] rust: pci: add PCI interrupt allocation and management support

[Joel Fernandes]

On Mon, Sep 15, 2025 at 11:48:19AM +0200, Danilo Krummrich wrote:
> On Wed Sep 10, 2025 at 9:02 PM CEST, Joel Fernandes wrote:
> > On Wed, Sep 10, 2025 at 02:09:55PM -0400, Joel Fernandes wrote:
> > [...] 
> >> > > +    /// 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 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.
> >> > > +    /// This is called during driver probe.
> >> > > +    ///
> >> > > +    /// # 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 the number of vectors successfully allocated, or an error if the allocation
> >> > > +    /// fails or cannot meet the minimum requirement.
> >> > > +    ///
> >> > > +    /// # Examples
> >> > > +    ///
> >> > > +    /// ```
> >> > > +    /// // Allocate using any available interrupt type in the order mentioned above.
> >> > > +    /// let nvecs = 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 nvecs = dev.alloc_irq_vectors(4, 16, msi_only)?;
> >> > > +    /// ```
> >> > > +    pub fn alloc_irq_vectors(
> >> > > +        &self,
> >> > > +        min_vecs: u32,
> >> > > +        max_vecs: u32,
> >> > > +        irq_types: IrqTypes,
> >> > > +    ) -> Result<u32> {
> >> > > +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
> >> > > +        // `pci_alloc_irq_vectors` internally validates all parameters and returns error codes.
> >> > > +        let ret = unsafe {
> >> > > +            bindings::pci_alloc_irq_vectors(self.as_raw(), min_vecs, max_vecs, irq_types.raw())
> >> > > +        };
> >> > > +
> >> > > +        to_result(ret)?;
> >> > > +        Ok(ret as u32)
> >> > > +    }
> >> > 
> >> > This is only valid to be called from the Core context, as it modifies internal
> >> > fields of the inner struct device.
> >> 
> >> It is called from core context, the diff format confuses.
> >> > 
> >> > Also, it would be nice if it would return a new type that can serve as argument
> >> > for irq_vector(), such that we don't have to rely on random integers.
> >> 
> >> Makes sense, I will do that.
> >> 
> > By the way, the "ret" value returned by pci_alloc_irq_vectors() is the number
> > of vectors, not the vector index.
> 
> Sure, but the vector index passed to pci_irq_vector() must be in the range
> defined by the return value of pci_alloc_irq_vectors().
> 
> I thought of e.g. Range<pci::IrqVector> as return value. This way you can easily
> iterate it and prove that it's an allocated vector index.

Agreed, I will do it like this.

> > So basically there are 3 numbers that mean
> > different things:
> > 1. Number of vectors (as returned by alloc_irq_vectors).
> > 2. Index of a vector (passed to pci_irq_vector).
> > 3. The Linux IRQ number (passed to request_irq).
> >
> > And your point is well taken, in fact even in current code there is
> > ambiguity: irq_vector() accepts a vector index, where as request_irq()
> > accepts a Linux IRQ number, which are different numbers. I can try to clean
> > that up as well but let me know if you had any other thoughts. In fact, I
> > think Device<device::Bound>::request_irq() pci should just accept IrqRequest?
> 
> Currently, pci::Device::request_irq() takes an IRQ vector index and calls
> irq_vector() internally to convert the vector index into an IRQ number.
> 
> I'd keep this semantics, but introduce a new type IrqVector rather than using
> the raw integer. So, drivers would call
> 
> 	// `irq_vecs` is of type `Range<pci::IrqVector>`.
> 	let irq_vecs = dev.alloc_irq_vectors(1, 1, pci::IrqTypes::ANY)?;
> 	let irq = KBox::pin_init(
> 	   dev.request_irq(irq_vecs.start, ...)?,
> 	)?;

This sounds good to me. Thanks,

 - Joel

Re: [PATCH] rust: pci: add PCI interrupt allocation and management support

[kernel test robot]

Hi Joel,

kernel test robot noticed the following build errors:

[auto build test ERROR on pci/next]
[also build test ERROR on pci/for-linus drm-misc/drm-misc-next drm-tip/drm-tip linus/master v6.17-rc5]
[cannot apply to next-20250911]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Joel-Fernandes/rust-pci-add-PCI-interrupt-allocation-and-management-support/20250910-115528
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20250910035415.381753-1-joelagnelf%40nvidia.com
patch subject: [PATCH] rust: pci: add PCI interrupt allocation and management support
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250912/202509121404.668X5Vy6-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250912/202509121404.668X5Vy6-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509121404.668X5Vy6-lkp@intel.com/

All errors (new ones prefixed by >>):

>> error[E0425]: cannot find value `dev` in this scope
   --> rust/doctests_kernel_generated.rs:6968:13
   |
   6968 | let nvecs = dev.alloc_irq_vectors(1, 32, IrqTypes::all())?;
   |             ^^^ not found in this scope
--
>> error[E0433]: failed to resolve: use of undeclared type `IrqTypes`
   --> rust/doctests_kernel_generated.rs:6968:42
   |
   6968 | let nvecs = dev.alloc_irq_vectors(1, 32, IrqTypes::all())?;
   |                                          ^^^^^^^^ use of undeclared type `IrqTypes`
   |
   help: consider importing this struct
   |
   3    + use kernel::pci::IrqTypes;
   |
--
>> error[E0433]: failed to resolve: use of undeclared type `IrqTypes`
   --> rust/doctests_kernel_generated.rs:6971:16
   |
   6971 | let msi_only = IrqTypes::default()
   |                ^^^^^^^^ use of undeclared type `IrqTypes`
   |
   help: consider importing this struct
   |
   3    + use kernel::pci::IrqTypes;
   |
--
>> error[E0433]: failed to resolve: use of undeclared type `IrqType`
   --> rust/doctests_kernel_generated.rs:6972:11
   |
   6972 |     .with(IrqType::Msi)
   |           ^^^^^^^ use of undeclared type `IrqType`
   |
   help: consider importing this enum
   |
   3    + use kernel::pci::IrqType;
   |
--
>> error[E0433]: failed to resolve: use of undeclared type `IrqType`
   --> rust/doctests_kernel_generated.rs:6973:11
   |
   6973 |     .with(IrqType::MsiX);
   |           ^^^^^^^ use of undeclared type `IrqType`
   |
   help: consider importing this enum
   |
   3    + use kernel::pci::IrqType;
   |
--
>> error[E0425]: cannot find value `dev` in this scope
   --> rust/doctests_kernel_generated.rs:6974:13
   |
   6974 | let nvecs = dev.alloc_irq_vectors(4, 16, msi_only)?;
   |             ^^^ not found in this scope
--
>> error[E0433]: failed to resolve: use of undeclared type `IrqTypes`
   --> rust/doctests_kernel_generated.rs:7025:16
   |
   7025 | let msi_only = IrqTypes::default()
   |                ^^^^^^^^ use of undeclared type `IrqTypes`
   |
   help: consider importing this struct
   |
   3    + use kernel::pci::IrqTypes;
   |
--
>> error[E0433]: failed to resolve: use of undeclared type `IrqType`
   --> rust/doctests_kernel_generated.rs:7026:11
   |
   7026 |     .with(IrqType::Msi)
   |           ^^^^^^^ use of undeclared type `IrqType`
   |
   help: consider importing this enum
   |
   3    + use kernel::pci::IrqType;
   |
--
>> error[E0433]: failed to resolve: use of undeclared type `IrqType`
   --> rust/doctests_kernel_generated.rs:7027:11
   |
   7027 |     .with(IrqType::MsiX);
   |           ^^^^^^^ use of undeclared type `IrqType`
   |
   help: consider importing this enum
   |
   3    + use kernel::pci::IrqType;
   |

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki