Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Strict `usize` alignment check in `Region::ptr_try_from_raw_parts_mut` 
breaks driver probes for valid 4-byte aligned MMIO mappings.
--

commit facc65605656f1d5585f26a25f200941e6ae7d73
Author: Gary Guo <[email protected]>

rust: io: generalize MmioRaw to pointer to arbitrary type

This commit generalizes MmioRaw to represent an untyped I/O region using
the dynamically-sized Region<SIZE> type. It renames the maxsize method
to size and implements Clone, Copy, Send, and Sync manually.

> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index d57df2a072a0e..c9533d3f003ba 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs

[ ... ]

> @@ -90,37 +90,67 @@ fn size(p: *const Self) -> usize {

[ ... ]

> +impl<const SIZE: usize> MmioRaw<Region<SIZE>> {
> +    /// Create a `MmioRaw` representing a I/O region with given size.
> +    ///
> +    /// The size is checked against the minimum size specified via const 
> generics.
> +    #[inline]
> +    pub fn new_region(addr: usize, size: usize) -> Result<Self> {
> +        Ok(Self {
> +            ptr: 
> Region::ptr_try_from_raw_parts_mut(core::ptr::without_provenance_mut(addr), 
> size)?,

[Severity: High]
Does this alignment check break driver probes for standard 4-byte aligned
MMIO mappings on 64-bit architectures?

Looking at Region::ptr_try_from_raw_parts_mut(), it enforces a strict
usize alignment on the base address:

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

On 64-bit systems, this forces an 8-byte alignment requirement.

However, PCI 32-bit BARs and SoC IP block MMIO regions frequently have
4-byte aligned physical base addresses. When mapped with ioremap() or
pci_iomap(), the resulting virtual address retains this 4-byte alignment
offset, which would fail the 8-byte alignment check here.

Consequently, standard hardware mapping operations in devres::IoMem::new(),
IoMem::ioremap(), and pci::Bar::new() will unconditionally fail at runtime
with -EINVAL during driver probe for these devices, making them unusable.

> +        })
>      }
> +}

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=5

Reply via email to