| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <08956b4fe99619064a4424d500837807ba4c92e6.camel@posteo.de>
Date: Fri, 16 Jan 2026 13:57:32 +0000
From: Markus Probst <markus.probst@...teo.de>
To: Zhi Wang <zhiw@...dia.com>, rust-for-linux@...r.kernel.org,
linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: dakr@...nel.org, aliceryhl@...gle.com, bhelgaas@...gle.com,
kwilczynski@...nel.org, ojeda@...nel.org, alex.gaynor@...il.com,
boqun.feng@...il.com, gary@...yguo.net, bjorn3_gh@...tonmail.com,
lossin@...nel.org, a.hindborg@...nel.org, tmgross@...ch.edu,
helgaas@...nel.org, cjia@...dia.com, smitra@...dia.com, ankita@...dia.com,
aniketa@...dia.com, kwankhede@...dia.com, targupta@...dia.com,
acourbot@...dia.com, joelagnelf@...dia.com, jhubbard@...dia.com,
zhiwang@...nel.org, daniel.almeida@...labora.com
Subject: Re: [PATCH v9 2/5] rust: io: factor common I/O helpers into Io trait
On Thu, 2026-01-15 at 23:26 +0200, Zhi Wang wrote:
> The previous Io<SIZE> type combined both the generic I/O access helpers
> and MMIO implementation details in a single struct.
>
> To establish a cleaner layering between the I/O interface and its concrete
> backends, paving the way for supporting additional I/O mechanisms in the
> future, Io<SIZE> need to be factored.
>
> Factor the common helpers into new {Io, Io64} traits, and move the
> MMIO-specific logic into a dedicated Mmio<SIZE> type implementing that
> trait. Rename the IoRaw to MmioRaw and update the bus MMIO implementations
> to use MmioRaw.
>
> No functional change intended.
>
> Cc: Alexandre Courbot <acourbot@...dia.com>
> Cc: Alice Ryhl <aliceryhl@...gle.com>
> Cc: Bjorn Helgaas <helgaas@...nel.org>
> Cc: Danilo Krummrich <dakr@...nel.org>
> Cc: John Hubbard <jhubbard@...dia.com>
> Signed-off-by: Zhi Wang <zhiw@...dia.com>
> ---
> drivers/gpu/drm/tyr/regs.rs | 1 +
> drivers/gpu/nova-core/gsp/sequencer.rs | 5 +-
> drivers/gpu/nova-core/regs/macros.rs | 90 +++++---
> drivers/gpu/nova-core/vbios.rs | 1 +
> rust/kernel/devres.rs | 14 +-
> rust/kernel/io.rs | 291 +++++++++++++++++++------
> rust/kernel/io/mem.rs | 16 +-
> rust/kernel/io/poll.rs | 16 +-
> rust/kernel/pci/io.rs | 12 +-
> samples/rust/rust_driver_pci.rs | 2 +
> 10 files changed, 317 insertions(+), 131 deletions(-)
>
> +/// Represents a region of I/O space of a fixed size.
> +///
> +/// This is an abstract representation to be implemented by arbitrary I/O
> +/// backends (e.g. MMIO, I2C, etc.).
> +///
> +/// Provides common helpers for offset validation and address
> +/// calculation on top of a base address and maximum size.
> +pub trait IoBase {
> + /// Minimum usable size of this region.
> + const MIN_SIZE: usize;
>
> /// Returns the base address of this mapping.
> - #[inline]
> - pub fn addr(&self) -> usize {
> - self.0.addr()
> - }
> + fn addr(&self) -> usize;
>
> /// Returns the maximum size of this mapping.
> - #[inline]
> - pub fn maxsize(&self) -> usize {
> - self.0.maxsize()
> - }
> -
> - #[inline]
> - const fn offset_valid<U>(offset: usize, size: usize) -> bool {
> - let type_size = core::mem::size_of::<U>();
> - if let Some(end) = offset.checked_add(type_size) {
> - end <= size && offset % type_size == 0
> - } else {
> - false
> - }
> - }
> + fn maxsize(&self) -> usize;
>
> + /// Returns the absolute I/O address for a given `offset`,
> + /// performing runtime bound checks.
> #[inline]
> fn io_addr<U>(&self, offset: usize) -> Result<usize> {
> - if !Self::offset_valid::<U>(offset, self.maxsize()) {
> + if !offset_valid::<U>(offset, self.maxsize()) {
> return Err(EINVAL);
> }
>
> @@ -239,50 +242,198 @@ fn io_addr<U>(&self, offset: usize) -> Result<usize> {
> self.addr().checked_add(offset).ok_or(EINVAL)
> }
>
> + /// Returns the absolute I/O address for a given `offset`,
> + /// performing compile-time bound checks.
> #[inline]
> fn io_addr_assert<U>(&self, offset: usize) -> usize {
> - build_assert!(Self::offset_valid::<U>(offset, SIZE));
> + build_assert!(offset_valid::<U>(offset, Self::MIN_SIZE));
>
> self.addr() + offset
> }
> +}
> +
> +/// Types implementing this trait (e.g. MMIO BARs or PCI config regions)
> +/// can share the same IoKnownSize accessors.
> +///
> +/// In this context, "known size" represents the static guarantee regarding
> +/// the minimum accessible size requested from the I/O backend.
> +///
> +/// Note that the actual size of the resource provided by the hardware
> +/// (e.g., the physical BAR size) might be larger than this "known size".
> +pub trait IoKnownSize: IoBase {
> + /// Infallible 8-bit read with compile-time bounds check.
> + fn read8(&self, offset: usize) -> u8;
> +
> + /// Infallible 16-bit read with compile-time bounds check.
> + fn read16(&self, offset: usize) -> u16;
> +
> + /// Infallible 32-bit read with compile-time bounds check.
> + fn read32(&self, offset: usize) -> u32;
> +
> + /// Infallible 8-bit write with compile-time bounds check.
> + fn write8(&self, value: u8, offset: usize);
> +
> + /// Infallible 16-bit write with compile-time bounds check.
> + fn write16(&self, value: u16, offset: usize);
>
> - define_read!(read8, try_read8, readb -> u8);
> - define_read!(read16, try_read16, readw -> u16);
> - define_read!(read32, try_read32, readl -> u32);
> + /// Infallible 32-bit write with compile-time bounds check.
> + fn write32(&self, value: u32, offset: usize);
> +}
> +
> +/// Types implementing this trait (e.g. MMIO BARs or PCI config regions)
> +/// can share the same Io accessors.
> +///
> +/// This trait defines the accessors for performing runtime-checked
> +/// operations on an I/O region. Note that due to the runtime checks, these
> +/// operations may be more expensive than those provided by
> +/// [`IoKnownSize`].
> +///
> +/// Implement this trait when:
> +/// - The I/O backend cannot guarantee a minimum accessible size for the
> +/// region.
> +/// - The I/O backend wishes to return meaningful error codes when the
> +/// driver accesses the region.
> +pub trait Io: IoBase {
> + /// Fallible 8-bit read with runtime bounds check.
> + fn try_read8(&self, offset: usize) -> Result<u8>;
> +
> + /// Fallible 16-bit read with runtime bounds check.
> + fn try_read16(&self, offset: usize) -> Result<u16>;
> +
> + /// Fallible 32-bit read with runtime bounds check.
> + fn try_read32(&self, offset: usize) -> Result<u32>;
> +
> + /// Fallible 8-bit write with runtime bounds check.
> + fn try_write8(&self, value: u8, offset: usize) -> Result;
> +
> + /// Fallible 16-bit write with runtime bounds check.
> + fn try_write16(&self, value: u16, offset: usize) -> Result;
> +
> + /// Fallible 32-bit write with runtime bounds check.
> + fn try_write32(&self, value: u32, offset: usize) -> Result;
How would this work with I2C as a IO backend?
I2C by itself doesn't have 32-bit IO, which the trait requires.
There is
"byte data" - 8 bit
"word data" - 16 bit
"block data" - arbitrary number of bytes (in SMBus limited to 32 bytes.
Outside of SMBus it could be higher)
Note that I only require "byte data" for my led driver.
Thanks
- Markus Probst
Download attachment "signature.asc" of type "application/pgp-signature" (871 bytes)
Powered by blists - more mailing lists