Re: [PATCH v3 10/19] rust: io: remove `MmioOwned`

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly introduced `Region` type strictly requires 8-byte alignment on 64-bit platforms, causing `iomap()` to fail for perfectly valid 4-byte aligned MMIO devices.
--

commit 4db6d2e35660784e019ec935f99ababad3a399c3
Author: Gary Guo <gary@garyguo.net>

rust: io: remove `MmioOwned`

The Io trait is now easier to implement, allowing Bar and IoMem to implement
it directly. This change removes the MmioOwned wrapper struct.

> diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs
> index 8f6c257c5b8e6..d9b3189d09b46 100644
> --- a/rust/kernel/io/mem.rs
> +++ b/rust/kernel/io/mem.rs

[ ... ]

> @@ -290,11 +292,13 @@ fn drop(&mut self) {
>      }
>  }
>  
> -impl<const SIZE: usize> Deref for IoMem<'_, SIZE> {
> -    type Target = MmioOwned<SIZE>;
> +impl<'a, const SIZE: usize> Io<'a> for &'a IoMem<'_, SIZE> {
> +    type Backend = MmioBackend;
> +    type Target = super::Region<SIZE>;

[Severity: High]
Does this new Region target properly support 4-byte aligned MMIO devices on
64-bit platforms?

Looking at Region::ptr_try_from_raw_parts_mut() in rust/kernel/io.rs, it
strictly requires the base pointer to be naturally aligned to usize (8 bytes
on 64-bit platforms):

rust/kernel/io.rs:Region::ptr_try_from_raw_parts_mut() {
    ...
    if size < SIZE || base.align_offset(size_of::<usize>()) != 0 {
        return Err(EINVAL);
    }
    ...
}

When IoMem::ioremap() maps hardware resources into the kernel, it calls
MmioRaw::new_region(), which enforces this alignment requirement:

rust/kernel/io/mem.rs:IoMem::ioremap() {
    ...
    let io = MmioRaw::new_region(addr as usize, size)?;
    Ok(IoMem { dev, io })
}

Since ioremap() preserves the page offset of the physical address, mapping a
physical resource starting at an offset of 0x04 will cause
base.align_offset(8) != 0 to evaluate to true.

Will this cause the entire iomap() driver probe to predictably fail with
-EINVAL for perfectly valid 4-byte aligned MMIO devices?

> +
> +    #[inline]
> +    fn as_view(self) -> Mmio<'a, Self::Target> {
>          // SAFETY: Safe as by the invariant of `IoMem`.
> -        unsafe { MmioOwned::from_raw(&self.io) }
> +        unsafe { Mmio::from_raw(self.io) }
>      }
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608-io_projection-v3-0-c5cde13a5ec4@garyguo.net?part=10