Re: [PATCH v2 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type

["Danilo Krummrich" <dakr@kernel.org>]

On Wed Aug 12, 2026 at 6:26 PM CEST, Gary Guo wrote:
>>  /// 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>,
>
> The registration has a refence to the device so we don't need to keep both reg
> and dev?

In this patch dev is still needed for the TryInto impl, but a subsquent patch
does remove it in favor of IrqRequest.

>> +pub struct IrqVectorRegistration<'a> {
>> +    dev: &'a Device<Bound>,
>> +    count: NonZero<usize>,
>
> I wonder if it should be called "len" as I view this as a collection of IRQ
> vectors.

Either sounds good to me.

>> +    #[inline]
>> +    pub fn vector(&self, index: usize) -> Result<IrqVector<'_>> {
>> +        if index >= self.count.get() {
>> +            return Err(EINVAL);
>> +        }
>
> Given that the error is for out-of-bound access only, perhaps return `Option`
> like `get()` function of various containers?

It's not really a case a driver would handle other than just follow it up with
.ok_or(EINVAL)? anyways, so I'd like to keep that.

(Further consideration on a subsequent patch.)

>> @@ -256,7 +249,21 @@ 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 count = NonZero::new(ret as usize).ok_or(EINVAL)?;
>
> I don't think `ret` can ever be zero. `expect` or `new_unchecked()` perhaps?

Correct, but I don't see a reason to BUG_ON() for this. A WARN_ON() makes sense,
but I see this to be the job of the C API making the promise.

We could use new_unchecked(), but since this method is fallible already and not
a hot path, I don't think it's worth.