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.
