Nouveau - Sep 2025 - [PATCH] rust: pci: add PCI interrupt allocation and management support

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)
> +    }
>  }

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)
> > +    }
> >  }