Re: [PATCH v7 07/16] rust: add `io::{Io, IoRaw}` base types

[Danilo Krummrich <dakr@kernel.org>]

On 1/16/25 11:31 AM, Fiona Behrens wrote:
> On 19 Dec 2024, at 18:04, Danilo Krummrich wrote:

>> +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() }
>> +    }
>> +
>> +    /// Returns the base address of this mapping.
>> +    #[inline]
>> +    pub fn addr(&self) -> usize {
>> +        self.0.addr()
>> +    }
>> +
>> +    /// 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
>> +        }
>> +    }
>> +
>> +    #[inline]
>> +    fn io_addr<U>(&self, offset: usize) -> Result<usize> {
>> +        if !Self::offset_valid::<U>(offset, self.maxsize()) {
>> +            return Err(EINVAL);
>> +        }
>> +
>> +        // Probably no need to check, since the safety requirements of `Self::new` guarantee that
>> +        // this can't overflow.
>> +        self.addr().checked_add(offset).ok_or(EINVAL)
>> +    }
>> +
>> +    #[inline]
>> +    fn io_addr_assert<U>(&self, offset: usize) -> usize {
>> +        build_assert!(Self::offset_valid::<U>(offset, SIZE));
>> +
>> +        self.addr() + offset
>> +    }
> 
> Currently reworking the portmem abstractions I wrote for the LED/SE10 driver.
> Right now I’m wondering if it would make sense to move the 3 functions above (`offset_valid`, `io_addr` and `io_addr_assert` into IoRaw),
> as I’m considering reusing the IoRaw in the portmem and then just offer the outb/outw/outl functions on a wraping type around IoRaw.
> For this I would also use the same functions to check bounds.

Sure, feel free to move them. I think I should have moved those functions to
IoRaw from the get-go.

- Danilo