* Re: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32
2026-09-01 10:58 ` Alexandre Courbot
@ 2026-09-01 11:08 ` Danilo Krummrich
2026-09-01 13:32 ` Alexandre Courbot
2026-09-01 11:47 ` ai qubits
2026-09-01 12:42 ` Miguel Ojeda
2 siblings, 1 reply; 15+ messages in thread
From: Danilo Krummrich @ 2026-09-01 11:08 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Sophon Zhang via B4 Relay, aiqubits, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Daniel Almeida, Tamir Duberstein, Onur Özkan,
linux-pci, rust-for-linux, linux-kernel
On Tue Sep 1, 2026 at 12:58 PM CEST, Alexandre Courbot wrote:
> That makes me wonder, shouldn't we make `index` take a `u32` directly?
> If that's what the C API expects, it does make sense to align to it
> instead of forcing users to make a potential unneeded conversion if they
> already have a u32.
I intentionally did not do this, as the common type for an index is usize. Thus,
I do not expect anyone to already have a u32, but to already have a usize, e.g.
from some iterator.
The fact that the C API did pick unsigned int as index type is an implementation
detail the abstraction should bother with.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32
2026-09-01 11:08 ` Danilo Krummrich
@ 2026-09-01 13:32 ` Alexandre Courbot
2026-09-01 13:36 ` Danilo Krummrich
2026-09-01 13:48 ` Gary Guo
0 siblings, 2 replies; 15+ messages in thread
From: Alexandre Courbot @ 2026-09-01 13:32 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Sophon Zhang via B4 Relay, aiqubits, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Daniel Almeida, Tamir Duberstein, Onur Özkan,
linux-pci, rust-for-linux, linux-kernel
On Tue Sep 1, 2026 at 8:08 PM JST, Danilo Krummrich wrote:
> On Tue Sep 1, 2026 at 12:58 PM CEST, Alexandre Courbot wrote:
>> That makes me wonder, shouldn't we make `index` take a `u32` directly?
>> If that's what the C API expects, it does make sense to align to it
>> instead of forcing users to make a potential unneeded conversion if they
>> already have a u32.
>
> I intentionally did not do this, as the common type for an index is usize. Thus,
> I do not expect anyone to already have a u32, but to already have a usize, e.g.
> from some iterator.
>
> The fact that the C API did pick unsigned int as index type is an implementation
> detail the abstraction should bother with.
Thanks for the clarification (and Miguel for elaborating - I wasn't
completely aware of it!). In that case this patch looks correct to me.
Note that there is another `as` right after, in the same method:
if irq < 0 {
return Err(Error::from_errno(irq));
}
// SAFETY: `irq` is a valid IRQ number for `self.dev`, resolved from this registration.
Ok(unsafe { IrqVector::new(IrqRequest::new(self.dev.as_ref(), irq as u32), self) })
We could get rid of it by replacing the `if irq < 0` test with:
let irq = u32::try_from(irq).map_err(|_| Error::from_errno(irq))?;
Sophon, if you feel like doing it I think this could improve the patch
further.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32
2026-09-01 13:32 ` Alexandre Courbot
@ 2026-09-01 13:36 ` Danilo Krummrich
2026-09-01 13:52 ` Alexandre Courbot
2026-09-01 13:48 ` Gary Guo
1 sibling, 1 reply; 15+ messages in thread
From: Danilo Krummrich @ 2026-09-01 13:36 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Sophon Zhang via B4 Relay, aiqubits, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Daniel Almeida, Tamir Duberstein, Onur Özkan,
linux-pci, rust-for-linux, linux-kernel
On 9/1/26 3:32 PM, Alexandre Courbot wrote:
> On Tue Sep 1, 2026 at 8:08 PM JST, Danilo Krummrich wrote:
>> On Tue Sep 1, 2026 at 12:58 PM CEST, Alexandre Courbot wrote:
>>> That makes me wonder, shouldn't we make `index` take a `u32` directly?
>>> If that's what the C API expects, it does make sense to align to it
>>> instead of forcing users to make a potential unneeded conversion if they
>>> already have a u32.
>>
>> I intentionally did not do this, as the common type for an index is usize. Thus,
>> I do not expect anyone to already have a u32, but to already have a usize, e.g.
>> from some iterator.
>>
>> The fact that the C API did pick unsigned int as index type is an implementation
>> detail the abstraction should bother with.
>
> Thanks for the clarification (and Miguel for elaborating - I wasn't
> completely aware of it!). In that case this patch looks correct to me.
>
> Note that there is another `as` right after, in the same method:
>
> if irq < 0 {
> return Err(Error::from_errno(irq));
> }
>
> // SAFETY: `irq` is a valid IRQ number for `self.dev`, resolved from this registration.
> Ok(unsafe { IrqVector::new(IrqRequest::new(self.dev.as_ref(), irq as u32), self) })
>
>
> We could get rid of it by replacing the `if irq < 0` test with:
>
> let irq = u32::try_from(irq).map_err(|_| Error::from_errno(irq))?;
That's a good suggestion!
> Sophon, if you feel like doing it I think this could improve the patch
> further.
Let's please do that as a separate patch though.
Thanks,
Danilo
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32
2026-09-01 13:36 ` Danilo Krummrich
@ 2026-09-01 13:52 ` Alexandre Courbot
0 siblings, 0 replies; 15+ messages in thread
From: Alexandre Courbot @ 2026-09-01 13:52 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Sophon Zhang via B4 Relay, aiqubits, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Daniel Almeida, Tamir Duberstein, Onur Özkan,
linux-pci, rust-for-linux, linux-kernel
On Tue Sep 1, 2026 at 10:36 PM JST, Danilo Krummrich wrote:
> On 9/1/26 3:32 PM, Alexandre Courbot wrote:
>> On Tue Sep 1, 2026 at 8:08 PM JST, Danilo Krummrich wrote:
>>> On Tue Sep 1, 2026 at 12:58 PM CEST, Alexandre Courbot wrote:
>>>> That makes me wonder, shouldn't we make `index` take a `u32` directly?
>>>> If that's what the C API expects, it does make sense to align to it
>>>> instead of forcing users to make a potential unneeded conversion if they
>>>> already have a u32.
>>>
>>> I intentionally did not do this, as the common type for an index is usize. Thus,
>>> I do not expect anyone to already have a u32, but to already have a usize, e.g.
>>> from some iterator.
>>>
>>> The fact that the C API did pick unsigned int as index type is an implementation
>>> detail the abstraction should bother with.
>>
>> Thanks for the clarification (and Miguel for elaborating - I wasn't
>> completely aware of it!). In that case this patch looks correct to me.
>>
>> Note that there is another `as` right after, in the same method:
>>
>> if irq < 0 {
>> return Err(Error::from_errno(irq));
>> }
>>
>> // SAFETY: `irq` is a valid IRQ number for `self.dev`, resolved from this registration.
>> Ok(unsafe { IrqVector::new(IrqRequest::new(self.dev.as_ref(), irq as u32), self) })
>>
>>
>> We could get rid of it by replacing the `if irq < 0` test with:
>>
>> let irq = u32::try_from(irq).map_err(|_| Error::from_errno(irq))?;
>
> That's a good suggestion!
I expect this pattern to be common, so how about we add a variant of
`error::to_result` that returns the value as a `u32`? I.e.
pub fn to_result_value(err: crate::ffi::c_int) -> Result<u32>
>
>> Sophon, if you feel like doing it I think this could improve the patch
>> further.
>
> Let's please do that as a separate patch though.
In this case, the current patch is
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32
2026-09-01 13:32 ` Alexandre Courbot
2026-09-01 13:36 ` Danilo Krummrich
@ 2026-09-01 13:48 ` Gary Guo
2026-09-01 14:08 ` Alexandre Courbot
1 sibling, 1 reply; 15+ messages in thread
From: Gary Guo @ 2026-09-01 13:48 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich
Cc: Sophon Zhang via B4 Relay, aiqubits, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Daniel Almeida, Tamir Duberstein, Onur Özkan,
linux-pci, rust-for-linux, linux-kernel
On Tue Sep 1, 2026 at 2:32 PM BST, Alexandre Courbot wrote:
> On Tue Sep 1, 2026 at 8:08 PM JST, Danilo Krummrich wrote:
>> On Tue Sep 1, 2026 at 12:58 PM CEST, Alexandre Courbot wrote:
>>> That makes me wonder, shouldn't we make `index` take a `u32` directly?
>>> If that's what the C API expects, it does make sense to align to it
>>> instead of forcing users to make a potential unneeded conversion if they
>>> already have a u32.
>>
>> I intentionally did not do this, as the common type for an index is usize. Thus,
>> I do not expect anyone to already have a u32, but to already have a usize, e.g.
>> from some iterator.
>>
>> The fact that the C API did pick unsigned int as index type is an implementation
>> detail the abstraction should bother with.
>
> Thanks for the clarification (and Miguel for elaborating - I wasn't
> completely aware of it!). In that case this patch looks correct to me.
>
> Note that there is another `as` right after, in the same method:
>
> if irq < 0 {
> return Err(Error::from_errno(irq));
> }
>
> // SAFETY: `irq` is a valid IRQ number for `self.dev`, resolved from this registration.
> Ok(unsafe { IrqVector::new(IrqRequest::new(self.dev.as_ref(), irq as u32), self) })
>
>
> We could get rid of it by replacing the `if irq < 0` test with:
>
> let irq = u32::try_from(irq).map_err(|_| Error::from_errno(irq))?;
I wonder if we can just change kernel::error::to_result to return `Result<u32>`
instead and have the cast there?
Best,
Gary
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32
2026-09-01 13:48 ` Gary Guo
@ 2026-09-01 14:08 ` Alexandre Courbot
2026-09-01 15:58 ` 回复: " ai qubits
0 siblings, 1 reply; 15+ messages in thread
From: Alexandre Courbot @ 2026-09-01 14:08 UTC (permalink / raw)
To: Gary Guo
Cc: Danilo Krummrich, Sophon Zhang via B4 Relay, aiqubits,
Bjorn Helgaas, Krzysztof Wilczyński, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Daniel Almeida, Tamir Duberstein,
Onur Özkan, linux-pci, rust-for-linux, linux-kernel
On Tue Sep 1, 2026 at 10:48 PM JST, Gary Guo wrote:
> On Tue Sep 1, 2026 at 2:32 PM BST, Alexandre Courbot wrote:
>> On Tue Sep 1, 2026 at 8:08 PM JST, Danilo Krummrich wrote:
>>> On Tue Sep 1, 2026 at 12:58 PM CEST, Alexandre Courbot wrote:
>>>> That makes me wonder, shouldn't we make `index` take a `u32` directly?
>>>> If that's what the C API expects, it does make sense to align to it
>>>> instead of forcing users to make a potential unneeded conversion if they
>>>> already have a u32.
>>>
>>> I intentionally did not do this, as the common type for an index is usize. Thus,
>>> I do not expect anyone to already have a u32, but to already have a usize, e.g.
>>> from some iterator.
>>>
>>> The fact that the C API did pick unsigned int as index type is an implementation
>>> detail the abstraction should bother with.
>>
>> Thanks for the clarification (and Miguel for elaborating - I wasn't
>> completely aware of it!). In that case this patch looks correct to me.
>>
>> Note that there is another `as` right after, in the same method:
>>
>> if irq < 0 {
>> return Err(Error::from_errno(irq));
>> }
>>
>> // SAFETY: `irq` is a valid IRQ number for `self.dev`, resolved from this registration.
>> Ok(unsafe { IrqVector::new(IrqRequest::new(self.dev.as_ref(), irq as u32), self) })
>>
>>
>> We could get rid of it by replacing the `if irq < 0` test with:
>>
>> let irq = u32::try_from(irq).map_err(|_| Error::from_errno(irq))?;
>
> I wonder if we can just change kernel::error::to_result to return `Result<u32>`
> instead and have the cast there?
Looks like our messages crossed [1]. :)
Converting `to_result` would require quite a bit of work to update all
the callers, but maybe we can introduce a new variant indeed.
But this makes me think of another step we can take to harden
`to_result`: it should probably warn if the non-error value if not `0`,
as that would indicate the caller needs to consider it.
[1] https://lore.kernel.org/all/DL412XWBP4Y2.K1TH9NELBKPR@nvidia.com/
^ permalink raw reply [flat|nested] 15+ messages in thread* 回复: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32
2026-09-01 14:08 ` Alexandre Courbot
@ 2026-09-01 15:58 ` ai qubits
0 siblings, 0 replies; 15+ messages in thread
From: ai qubits @ 2026-09-01 15:58 UTC (permalink / raw)
To: Alexandre Courbot, Gary Guo
Cc: Danilo Krummrich, Sophon Zhang via B4 Relay, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Daniel Almeida, Tamir Duberstein, Onur Özkan,
linux-pci@vger.kernel.org, rust-for-linux@vger.kernel.org,
linux-kernel@vger.kernel.org
I agree with Alexandre on this point.
I will keep v4 unchanged and handle the
return-value conversion helper separately.
Best regards,
Sophon
^ permalink raw reply [flat|nested] 15+ messages in thread
* 回复: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32
2026-09-01 10:58 ` Alexandre Courbot
2026-09-01 11:08 ` Danilo Krummrich
@ 2026-09-01 11:47 ` ai qubits
2026-09-01 12:06 ` Gary Guo
2026-09-01 12:42 ` Miguel Ojeda
2 siblings, 1 reply; 15+ messages in thread
From: ai qubits @ 2026-09-01 11:47 UTC (permalink / raw)
To: Alexandre Courbot, Sophon Zhang via B4 Relay
Cc: Danilo Krummrich, Bjorn Helgaas, Krzysztof Wilczyński,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Daniel Almeida, Tamir Duberstein, Onur Özkan,
linux-pci@vger.kernel.org, rust-for-linux@vger.kernel.org,
linux-kernel@vger.kernel.org
Thanks for the question, Alexandre, and for the clarification, Danilo.
I agree that usize is the more appropriate type for an index-oriented
Rust API. Callers are likely to obtain indices from ranges, iterators, or
other Rust indexing operations, which naturally use usize.
The unsigned int expected by pci_irq_vector() is an implementation detail
of the C interface, so the necessary validation and conversion should
remain inside the Rust abstraction rather than being exposed to callers.
I will therefore keep IrqVectorRegistration::index() accepting usize and
handle the C API boundary internally.
Best regards,
Sophon
________________________________________
发件人: Alexandre Courbot <acourbot@nvidia.com>
发送时间: 2026年9月1日 18:58
收件人: Sophon Zhang via B4 Relay
抄送: aiqubits@hotmail.com; Danilo Krummrich; Bjorn Helgaas; Krzysztof Wilczyński; Miguel Ojeda; Boqun Feng; Gary Guo; Björn Roy Baron; Benno Lossin; Andreas Hindborg; Alice Ryhl; Trevor Gross; Daniel Almeida; Tamir Duberstein; Onur Özkan; linux-pci@vger.kernel.org; rust-for-linux@vger.kernel.org; linux-kernel@vger.kernel.org
主题: Re: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32
On Tue Sep 1, 2026 at 2:09 AM JST, Sophon Zhang via B4 Relay wrote:
> From: Sophon Zhang <aiqubits@hotmail.com>
>
> IrqVectorRegistration::index() accepts a usize, but pci_irq_vector()
> takes an unsigned int. On 64-bit architectures, casting an index larger
> than u32::MAX wraps it before the PCI core can validate it. In
> particular, u32::MAX + 1 becomes zero and can resolve to the first
> allocated vector.
>
> Use a checked conversion and return EINVAL when the index cannot be
> represented by the C API.
>
> Fixes: 2fb7755b0a7e ("rust: pci: resolve IRQ in index() and embed IrqRequest in IrqVector")
> Signed-off-by: Sophon Zhang <aiqubits@hotmail.com>
> ---
> Prevent 64-bit Rust IRQ vector indices from wrapping when they cross the
> PCI C API boundary.
> ---
> Changes in v4:
> - Drop the explicit length check in favor of PCI core range validation.
> - Use the existing TryFromIntError-to-Error conversion directly.
> - Keep commit trailers adjacent and narrow the description to truncation.
> - Link to v3: https://patch.msgid.link/20260831-fix-pci-irq-vector-index-truncation-v3-1-a2103084d20e@hotmail.com
>
> Changes in v3:
> - Check the index against the allocated vector count before entering the C API.
> - Keep the checked usize-to-u32 conversion and document the C-side warning.
> - Link to v2: https://patch.msgid.link/20260831-fix-pci-irq-vector-index-truncation-v2-1-4030ea7746a9@hotmail.com
>
> Changes in v2:
> - No code changes.
> - Link to v1: https://patch.msgid.link/20260831-fix-pci-irq-vector-index-truncation-v1-1-d63217d99b67@hotmail.com
>
> Testing:
> - make rustfmtcheck
> - Not build- or hardware-tested; bindgen is unavailable in the test environment.
> ---
> rust/kernel/pci/irq.rs | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index 6741046ec1c0..22e2cdf82a21 100644
> --- a/rust/kernel/pci/irq.rs
> +++ b/rust/kernel/pci/irq.rs
> @@ -151,8 +151,10 @@ pub fn irq_type(&self) -> IrqType {
> /// [`Self::len()`].
> #[inline]
> pub fn index(&self, index: usize) -> Result<IrqVector<'_>> {
> + let index = u32::try_from(index)?;
> +
> // SAFETY: `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`.
> - let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), index as u32) };
> + let irq = unsafe { bindings::pci_irq_vector(self.dev.as_raw(), index) };
> if irq < 0 {
> return Err(Error::from_errno(irq));
> }
That makes me wonder, shouldn't we make `index` take a `u32` directly?
If that's what the C API expects, it does make sense to align to it
instead of forcing users to make a potential unneeded conversion if they
already have a u32.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: 回复: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32
2026-09-01 11:47 ` ai qubits
@ 2026-09-01 12:06 ` Gary Guo
2026-09-01 12:35 ` 回复: " ai qubits
0 siblings, 1 reply; 15+ messages in thread
From: Gary Guo @ 2026-09-01 12:06 UTC (permalink / raw)
To: ai qubits, Alexandre Courbot, Sophon Zhang via B4 Relay
Cc: Danilo Krummrich, Bjorn Helgaas, Krzysztof Wilczyński,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Daniel Almeida, Tamir Duberstein, Onur Özkan,
linux-pci@vger.kernel.org, rust-for-linux@vger.kernel.org,
linux-kernel@vger.kernel.org
On Tue Sep 1, 2026 at 12:47 PM BST, ai qubits wrote:
> Thanks for the question, Alexandre, and for the clarification, Danilo.
>
> I agree that usize is the more appropriate type for an index-oriented
> Rust API. Callers are likely to obtain indices from ranges, iterators, or
> other Rust indexing operations, which naturally use usize.
>
> The unsigned int expected by pci_irq_vector() is an implementation detail
> of the C interface, so the necessary validation and conversion should
> remain inside the Rust abstraction rather than being exposed to callers.
>
> I will therefore keep IrqVectorRegistration::index() accepting usize and
> handle the C API boundary internally.
>
> Best regards,
> Sophon
Hi Sophon,
Please avoid top-posting. See
https://subspace.kernel.org/etiquette.html#do-not-top-post-when-replying (and
rest of that page, too).
Also, your email contains byte order marks (FEFF). Not sure what clients are you
using, but please pick a proper one for plain text mails.
Thanks,
Gary
^ permalink raw reply [flat|nested] 15+ messages in thread
* 回复: 回复: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32
2026-09-01 12:06 ` Gary Guo
@ 2026-09-01 12:35 ` ai qubits
0 siblings, 0 replies; 15+ messages in thread
From: ai qubits @ 2026-09-01 12:35 UTC (permalink / raw)
To: Gary Guo, Sophon Zhang via B4 Relay
Cc: Alexandre Courbot, Danilo Krummrich, Bjorn Helgaas,
Krzysztof Wilczyński, Miguel Ojeda, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Daniel Almeida, Tamir Duberstein, Onur Özkan,
linux-pci@vger.kernel.org, rust-for-linux@vger.kernel.org,
linux-kernel@vger.kernel.org
Thanks for pointing this out, Gary. I will optimize these email responses in future emails.
Best regards,
Sophon
________________________________________
发件人: Gary Guo <gary@garyguo.net>
发送时间: 2026年9月1日 20:06
收件人: ai qubits; Alexandre Courbot; Sophon Zhang via B4 Relay
抄送: Danilo Krummrich; Bjorn Helgaas; Krzysztof Wilczyński; Miguel Ojeda; Boqun Feng; Gary Guo; Björn Roy Baron; Benno Lossin; Andreas Hindborg; Alice Ryhl; Trevor Gross; Daniel Almeida; Tamir Duberstein; Onur Özkan; linux-pci@vger.kernel.org; rust-for-linux@vger.kernel.org; linux-kernel@vger.kernel.org
主题: Re: 回复: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32
On Tue Sep 1, 2026 at 12:47 PM BST, ai qubits wrote:
> Thanks for the question, Alexandre, and for the clarification, Danilo.
>
> I agree that usize is the more appropriate type for an index-oriented
> Rust API. Callers are likely to obtain indices from ranges, iterators, or
> other Rust indexing operations, which naturally use usize.
>
> The unsigned int expected by pci_irq_vector() is an implementation detail
> of the C interface, so the necessary validation and conversion should
> remain inside the Rust abstraction rather than being exposed to callers.
>
> I will therefore keep IrqVectorRegistration::index() accepting usize and
> handle the C API boundary internally.
>
> Best regards,
> Sophon
Hi Sophon,
Please avoid top-posting. See
https://subspace.kernel.org/etiquette.html#do-not-top-post-when-replying (and
rest of that page, too).
Also, your email contains byte order marks (FEFF). Not sure what clients are you
using, but please pick a proper one for plain text mails.
Thanks,
Gary
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4] rust: pci: reject IRQ vector indices that do not fit in u32
2026-09-01 10:58 ` Alexandre Courbot
2026-09-01 11:08 ` Danilo Krummrich
2026-09-01 11:47 ` ai qubits
@ 2026-09-01 12:42 ` Miguel Ojeda
2 siblings, 0 replies; 15+ messages in thread
From: Miguel Ojeda @ 2026-09-01 12:42 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Sophon Zhang via B4 Relay, aiqubits, Danilo Krummrich,
Bjorn Helgaas, Krzysztof Wilczyński, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Onur Özkan, linux-pci, rust-for-linux,
linux-kernel
On Tue, Sep 1, 2026 at 12:58 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> That makes me wonder, shouldn't we make `index` take a `u32` directly?
> If that's what the C API expects, it does make sense to align to it
> instead of forcing users to make a potential unneeded conversion if they
> already have a u32.
In general, Rust users shouldn't be dealing with C APIs, so if they
have a `u32` then it usually is because we exposed it from somewhere
else (e.g. we returned it to them) or because that is generally the
right underlying type, in which case it may make sense to align
everything.
But even in those cases, it may have made sense to define a Rust
newtype or similar instead.
So other than exceptional cases, the types that C APIs use shouldn't
drive the decisions on the Rust signatures, since they shouldn't be
seen by the Rust users to begin with.
That definitely introduces some friction on our side, but it does give
us a lot of freedom defining APIs the best way we can, which is a
major advantage, i.e. since we have to provide these abstractions, it
is a good time to be able to clean old decisions and improve on them
using whatever tools Rust give us.
Otherwise, one could also argue we should be passing the underlying
types in general, e.g. even pointers.
(Of course, you know this, I am just elaborating; and obviously there
may be cases it may make sense to just use the underlying type since
it is something that has never changed or is fixed due to some
"standard" etc.).
Cheers,
Miguel
^ permalink raw reply [flat|nested] 15+ messages in thread