Re: [PATCH v7 3/6] rust: io: factor common I/O helpers into Io trait

["Alexandre Courbot" <acourbot@nvidia.com>]

On Tue Nov 25, 2025 at 11:09 PM JST, Alexandre Courbot wrote:
> On Wed Nov 19, 2025 at 8:21 PM JST, Zhi Wang wrote:
> <snip>
>> -impl<const SIZE: usize> Io<SIZE> {
>> -    /// Converts an `IoRaw` into an `Io` instance, providing the accessors to the MMIO mapping.
>> -    ///
>> -    /// # Safety
>> -    ///
>> -    /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of size
>> -    /// `maxsize`.
>> -    pub unsafe fn from_raw(raw: &IoRaw<SIZE>) -> &Self {
>> -        // SAFETY: `Io` is a transparent wrapper around `IoRaw`.
>> -        unsafe { &*core::ptr::from_ref(raw).cast() }
>> +/// Checks whether an access of type `U` at the given `offset`
>> +/// is valid within this region.
>> +#[inline]
>> +const fn offset_valid<U>(offset: usize, size: usize) -> bool {
>> +    let type_size = core::mem::size_of::<U>();
>> +    if let Some(end) = offset.checked_add(type_size) {
>> +        end <= size && offset % type_size == 0
>> +    } else {
>> +        false
>>      }
>> +}
>> +
>> +/// Represents a region of I/O space of a fixed size.
>> +///
>> +/// Provides common helpers for offset validation and address
>> +/// calculation on top of a base address and maximum size.
>> +///
>> +pub trait Io {
>> +    /// Minimum usable size of this region.
>> +    const MIN_SIZE: usize;
>
> This associated constant should probably be part of `IoInfallible` -
> otherwise what value should it take if some type only implement
> `IoFallible`?
>
>>  
>>      /// Returns the base address of this mapping.
>> -    #[inline]
>> -    pub fn addr(&self) -> usize {
>> -        self.0.addr()
>> -    }
>> +    fn addr(&self) -> usize;
>>  
>>      /// Returns the maximum size of this mapping.
>> -    #[inline]
>> -    pub fn maxsize(&self) -> usize {
>> -        self.0.maxsize()
>> -    }
>> -
>> -    #[inline]
>> -    const fn offset_valid<U>(offset: usize, size: usize) -> bool {
>> -        let type_size = core::mem::size_of::<U>();
>> -        if let Some(end) = offset.checked_add(type_size) {
>> -            end <= size && offset % type_size == 0
>> -        } else {
>> -            false
>> -        }
>> -    }
>> +    fn maxsize(&self) -> usize;
>>  
>> +    /// Returns the absolute I/O address for a given `offset`,
>> +    /// performing runtime bound checks.
>>      #[inline]
>>      fn io_addr<U>(&self, offset: usize) -> Result<usize> {
>> -        if !Self::offset_valid::<U>(offset, self.maxsize()) {
>> +        if !offset_valid::<U>(offset, self.maxsize()) {
>>              return Err(EINVAL);
>>          }
>
> Similarly I cannot find any context where `maxsize` and `io_addr` are
> used outside of `IoFallible`, hinting that these should be part of this
> trait.
>
>>  
>> @@ -239,50 +240,190 @@ fn io_addr<U>(&self, offset: usize) -> Result<usize> {
>>          self.addr().checked_add(offset).ok_or(EINVAL)
>>      }
>>  
>> +    /// Returns the absolute I/O address for a given `offset`,
>> +    /// performing compile-time bound checks.
>>      #[inline]
>>      fn io_addr_assert<U>(&self, offset: usize) -> usize {
>> -        build_assert!(Self::offset_valid::<U>(offset, SIZE));
>> +        build_assert!(offset_valid::<U>(offset, Self::MIN_SIZE));
>>  
>>          self.addr() + offset
>>      }
>
> ... and `io_addr_assert` is only used from `IoFallible`.
>
> So if my gut feeling is correct, we can disband `Io` entirely and only

... except that we can't due to `addr`, unless we find a better way to
provide this base. But even if we need to keep `Io`, the compile-time
and runtime members should be moved to their respective traits.