On Tue, Oct 22, 2024 at 11:33 PM Danilo Krummrich <dakr@xxxxxxxxxx> wrote: > > I/O memory is typically either mapped through direct calls to ioremap() > or subsystem / bus specific ones such as pci_iomap(). > > Even though subsystem / bus specific functions to map I/O memory are > based on ioremap() / iounmap() it is not desirable to re-implement them > in Rust. > > Instead, implement a base type for I/O mapped memory, which generically > provides the corresponding accessors, such as `Io::readb` or > `Io:try_readb`. > > `Io` supports an optional const generic, such that a driver can indicate > the minimal expected and required size of the mapping at compile time. > Correspondingly, calls to the 'non-try' accessors, support compile time > checks of the I/O memory offset to read / write, while the 'try' > accessors, provide boundary checks on runtime. And using zero works because the user then statically knows that zero bytes are available ... ? > `Io` is meant to be embedded into a structure (e.g. pci::Bar or > io::IoMem) which creates the actual I/O memory mapping and initializes > `Io` accordingly. > > To ensure that I/O mapped memory can't out-live the device it may be > bound to, subsystems should embedd the corresponding I/O memory type > (e.g. pci::Bar) into a `Devres` container, such that it gets revoked > once the device is unbound. I wonder if `Io` should be a reference type instead. That is: struct Io<'a, const SIZE: usize> { addr: usize, maxsize: usize, _lifetime: PhantomData<&'a ()>, } and then the constructor requires "addr must be valid I/O mapped memory for maxsize bytes for the duration of 'a". And instead of embedding it in another struct, the other struct creates an `Io` on each access? > Co-developed-by: Philipp Stanner <pstanner@xxxxxxxxxx> > Signed-off-by: Philipp Stanner <pstanner@xxxxxxxxxx> > Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx> [...] > diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs > new file mode 100644 > index 000000000000..750af938f83e > --- /dev/null > +++ b/rust/kernel/io.rs > @@ -0,0 +1,234 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Memory-mapped IO. > +//! > +//! C header: [`include/asm-generic/io.h`](srctree/include/asm-generic/io.h) > + > +use crate::error::{code::EINVAL, Result}; > +use crate::{bindings, build_assert}; > + > +/// IO-mapped memory, starting at the base address @addr and spanning @maxlen bytes. > +/// > +/// The creator (usually a subsystem / bus such as PCI) is responsible for creating the > +/// mapping, performing an additional region request etc. > +/// > +/// # Invariant > +/// > +/// `addr` is the start and `maxsize` the length of valid I/O mapped memory region of size > +/// `maxsize`. Do you not also need an invariant that `SIZE <= maxsize`? Alice