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 <[email protected]> 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/[email protected]?part=10
