Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] SysMem silently breaks the implicit endianness and memory ordering contracts of the Io trait, making generic I/O abstraction unsafe on big-endian architectures and weakly-ordered CPUs. --
commit f9c9f4f480b4daad1545ae67d8a19099030dc2fd Author: Laura Nao <[email protected]> rust: io: add I/O backend for system memory with volatile access This commit adds a SysMem backend for the Io trait, intended for kernel virtual address ranges. It uses volatile accessors to provide access to shared memory, allowing alternative mapping layers to devices. > diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs > index 62643b23e9042..78c6ae2a0db82 100644 > --- a/rust/kernel/io.rs > +++ b/rust/kernel/io.rs [ ... ] > +macro_rules! impl_sysmem_io_capable { > + ($ty:ty) => { > + impl IoCapable<$ty> for SysMemBackend { > + #[inline] > + fn io_read(view: SysMem<'_, $ty>) -> $ty { > + // SAFETY: > + // - Per type invariant, `ptr` is valid and aligned. > + // - Using read_volatile() here so that race with hardware > is well-defined. > + // - Using read_volatile() here is not sound if it races > with other CPU per Rust > + // rules, but this is allowed per LKMM. > + // - The macro is only used on primitives so all bit > patterns are valid. > + unsafe { view.ptr.read_volatile() } > + } > + > + #[inline] > + fn io_write(view: SysMem<'_, $ty>, value: $ty) { > + // SAFETY: > + // - Per type invariant, `ptr` is valid and aligned. > + // - Using write_volatile() here so that race with hardware > is well-defined. > + // - Using write_volatile() here is not sound if it races > with other CPU per Rust > + // rules, but this is allowed per LKMM. > + unsafe { view.ptr.write_volatile(value) } > + } [Severity: High] Do these volatile accessors break the implicit endianness and memory ordering contracts established for the Io trait? The MmioBackend implements io_write and io_read using writel and readl, which perform little-endian byte-swapping and include memory barriers. In contrast, SysMemBackend uses native-endian write_volatile and read_volatile without memory barriers. If a driver uses the Io trait generically, could this difference lead to data corruption or DMA race conditions on big-endian or weakly-ordered architectures? For instance, if a driver manually byte-swaps to little-endian, MmioBackend would double-swap it, whereas SysMemBackend would write the manually swapped data. > + } > + }; > +} [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=14
