rust/kernel/io.rs | 99 +++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 64 insertions(+), 35 deletions(-)
Move the helper functions `offset_valid`, `io_addr` and
`io_addr_asset` from `Io` to `IoRaw`. This allows `IoRaw` to be reused
if other abstractions with different write/read functions are
needed (e.g. `writeb` vs `iowrite` vs `outb`).
Signed-off-by: Fiona Behrens <me@kloenk.dev>
---
Changes in v2:
- make the io_addr functions private again
- reword the invariant description
- use the invariant (unchecked_add)
- Link to v1: https://lore.kernel.org/r/20250122-rust-io-offset-v1-1-914725ab55ed@kloenk.dev
---
rust/kernel/io.rs | 99 +++++++++++++++++++++++++++++++++++--------------------
1 file changed, 64 insertions(+), 35 deletions(-)
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index d4a73e52e3ee68f7b558749ed0108acde92ae5fe..c4dd8289705eacc93e285f4e25fb6262f605328c 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -15,6 +15,11 @@
/// Instead, the bus specific MMIO implementation must convert this raw representation into an `Io`
/// instance providing the actual memory accessors. Only by the conversion into an `Io` structure
/// any guarantees are given.
+///
+/// # Invariant
+///
+/// - `maxsize` is larger or equal to `SIZE`.
+/// - `addr + maxsize` is always smaller then [`usize::MAX`] so that it fits in virtual memory.
pub struct IoRaw<const SIZE: usize = 0> {
addr: usize,
maxsize: usize,
@@ -23,24 +28,76 @@ pub struct IoRaw<const SIZE: usize = 0> {
impl<const SIZE: usize> IoRaw<SIZE> {
/// Returns a new `IoRaw` instance on success, an error otherwise.
pub fn new(addr: usize, maxsize: usize) -> Result<Self> {
- if maxsize < SIZE {
+ if maxsize < SIZE || addr.checked_add(maxsize).is_none() {
return Err(EINVAL);
}
+ // INVARIANT: `maxsize` is at least `SIZE`, `addr + maxsize` is not wrapping.
Ok(Self { addr, maxsize })
}
/// Returns the base address of the MMIO region.
#[inline]
- pub fn addr(&self) -> usize {
+ pub const fn addr(&self) -> usize {
self.addr
}
/// Returns the maximum size of the MMIO region.
#[inline]
- pub fn maxsize(&self) -> usize {
+ pub const fn maxsize(&self) -> usize {
self.maxsize
}
+
+ /// Check if the offset plus the size of the type `U` fits in the bounds of `size`.
+ /// Also checks if the offset is aligned with the type size.
+ #[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
+ }
+ }
+
+ /// Check if the offset (plus the type size) is out of bounds.
+ ///
+ /// Runtime checked version of [`io_addr_assert`].
+ ///
+ /// See [`offset_valid`] for the performed offset check.
+ ///
+ /// # Errors
+ ///
+ /// Returns [`EINVAL`] if the type does not fit into [`IoRaw`] at the given offset.
+ ///
+ /// [`offset_valid`]: Self::offset_valid
+ /// [`io_addr_assert`]: Self::io_addr_assert
+ #[inline]
+ fn io_addr<U>(&self, offset: usize) -> Result<usize> {
+ if !Self::offset_valid::<U>(offset, self.maxsize()) {
+ return Err(EINVAL);
+ }
+
+ // SAFETY: by the invariant of `IoRaw` this cannot overflow.
+ Ok(unsafe { self.addr().unchecked_add(offset) })
+ }
+
+ /// Check at build time if the offset (plus the type size) is out of bounds.
+ ///
+ /// Compiletime checked version of [`io_addr`].
+ ///
+ /// See [`offset_valid`] for the performed offset check.
+ ///
+ ///
+ /// [`offset_valid`]: Self::offset_valid
+ /// [`io_addr`]: Self::io_addr
+ #[inline]
+ const fn io_addr_assert<U>(&self, offset: usize) -> usize {
+ build_assert!(Self::offset_valid::<U>(offset, SIZE));
+
+ // SAFETY: by the invariant of `IoRaw` this cannot overflow.
+ unsafe { self.addr().unchecked_add(offset) }
+ }
}
/// IO-mapped memory, starting at the base address @addr and spanning @maxlen bytes.
@@ -116,7 +173,7 @@ macro_rules! define_read {
$(#[$attr])*
#[inline]
pub fn $name(&self, offset: usize) -> $type_name {
- let addr = self.io_addr_assert::<$type_name>(offset);
+ let addr = self.0.io_addr_assert::<$type_name>(offset);
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
unsafe { bindings::$name(addr as _) }
@@ -128,7 +185,7 @@ pub fn $name(&self, offset: usize) -> $type_name {
/// out of bounds.
$(#[$attr])*
pub fn $try_name(&self, offset: usize) -> Result<$type_name> {
- let addr = self.io_addr::<$type_name>(offset)?;
+ let addr = self.0.io_addr::<$type_name>(offset)?;
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
Ok(unsafe { bindings::$name(addr as _) })
@@ -145,7 +202,7 @@ macro_rules! define_write {
$(#[$attr])*
#[inline]
pub fn $name(&self, value: $type_name, offset: usize) {
- let addr = self.io_addr_assert::<$type_name>(offset);
+ let addr = self.0.io_addr_assert::<$type_name>(offset);
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
unsafe { bindings::$name(value, addr as _, ) }
@@ -157,7 +214,7 @@ pub fn $name(&self, value: $type_name, offset: usize) {
/// out of bounds.
$(#[$attr])*
pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
- let addr = self.io_addr::<$type_name>(offset)?;
+ let addr = self.0.io_addr::<$type_name>(offset)?;
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
unsafe { bindings::$name(value, addr as _) }
@@ -190,34 +247,6 @@ 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
- }
- }
-
- #[inline]
- fn io_addr<U>(&self, offset: usize) -> Result<usize> {
- if !Self::offset_valid::<U>(offset, self.maxsize()) {
- return Err(EINVAL);
- }
-
- // Probably no need to check, since the safety requirements of `Self::new` guarantee that
- // this can't overflow.
- self.addr().checked_add(offset).ok_or(EINVAL)
- }
-
- #[inline]
- fn io_addr_assert<U>(&self, offset: usize) -> usize {
- build_assert!(Self::offset_valid::<U>(offset, SIZE));
-
- self.addr() + offset
- }
-
define_read!(readb, try_readb, u8);
define_read!(readw, try_readw, u16);
define_read!(readl, try_readl, u32);
---
base-commit: 01b3cb620815fc3feb90ee117d9445a5b608a9f7
change-id: 20250122-rust-io-offset-7b39b11e84ac
Best regards,
--
Fiona Behrens <me@kloenk.dev>On Tue, Jan 28, 2025 at 01:46:00PM +0100, Fiona Behrens wrote:
> Move the helper functions `offset_valid`, `io_addr` and
> `io_addr_asset` from `Io` to `IoRaw`. This allows `IoRaw` to be reused
> if other abstractions with different write/read functions are
> needed (e.g. `writeb` vs `iowrite` vs `outb`).
>
> Signed-off-by: Fiona Behrens <me@kloenk.dev>
> ---
> Changes in v2:
> - make the io_addr functions private again
> - reword the invariant description
> - use the invariant (unchecked_add)
> - Link to v1: https://lore.kernel.org/r/20250122-rust-io-offset-v1-1-914725ab55ed@kloenk.dev
> ---
> rust/kernel/io.rs | 99 +++++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 64 insertions(+), 35 deletions(-)
>
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index d4a73e52e3ee68f7b558749ed0108acde92ae5fe..c4dd8289705eacc93e285f4e25fb6262f605328c 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -15,6 +15,11 @@
> /// Instead, the bus specific MMIO implementation must convert this raw representation into an `Io`
> /// instance providing the actual memory accessors. Only by the conversion into an `Io` structure
> /// any guarantees are given.
> +///
> +/// # Invariant
As mentioned in v1, please put the addition of the invariant and the subsequent
switch to unchecked_add() into a separate patch.
Also note that unchecked_math is not stable before 1.79, but the minimum
supported version is 1.78 currently, hence you see Clippy complain.
So, I think we'd need to enable unchecked_math in order to use this, not sure
that's worth though. Plus, I'm not sure this makes the Clippy lint warning go
away by itself.
> +///
> +/// - `maxsize` is larger or equal to `SIZE`.
> +/// - `addr + maxsize` is always smaller then [`usize::MAX`] so that it fits in virtual memory.
My reply to this line from v1 was more meant as a question, because I wasn't
entirely sure if you want to say that this does never overflow, or if you want
to say that this must be within the bounds of some address space. Sorry for the
confusion.
I think this should be limited to saying that `addr + maxsize` does not overflow
and leave the rest to the `Io` type.
Please also note that you can't assume that `usize::MAX` is always equivalent to
the size of the virtual address space.
> pub struct IoRaw<const SIZE: usize = 0> {
> addr: usize,
> maxsize: usize,
> @@ -23,24 +28,76 @@ pub struct IoRaw<const SIZE: usize = 0> {
> impl<const SIZE: usize> IoRaw<SIZE> {
> /// Returns a new `IoRaw` instance on success, an error otherwise.
> pub fn new(addr: usize, maxsize: usize) -> Result<Self> {
> - if maxsize < SIZE {
> + if maxsize < SIZE || addr.checked_add(maxsize).is_none() {
> return Err(EINVAL);
> }
>
> + // INVARIANT: `maxsize` is at least `SIZE`, `addr + maxsize` is not wrapping.
> Ok(Self { addr, maxsize })
> }
>
> /// Returns the base address of the MMIO region.
> #[inline]
> - pub fn addr(&self) -> usize {
> + pub const fn addr(&self) -> usize {
> self.addr
> }
>
> /// Returns the maximum size of the MMIO region.
> #[inline]
> - pub fn maxsize(&self) -> usize {
> + pub const fn maxsize(&self) -> usize {
> self.maxsize
> }
> +
> + /// Check if the offset plus the size of the type `U` fits in the bounds of `size`.
> + /// Also checks if the offset is aligned with the type size.
I think it's good to add documentation even if not required. Please do so in a
separate patch though.
> + #[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
> + }
> + }
> +
> + /// Check if the offset (plus the type size) is out of bounds.
> + ///
> + /// Runtime checked version of [`io_addr_assert`].
> + ///
> + /// See [`offset_valid`] for the performed offset check.
> + ///
> + /// # Errors
> + ///
> + /// Returns [`EINVAL`] if the type does not fit into [`IoRaw`] at the given offset.
> + ///
> + /// [`offset_valid`]: Self::offset_valid
> + /// [`io_addr_assert`]: Self::io_addr_assert
> + #[inline]
> + fn io_addr<U>(&self, offset: usize) -> Result<usize> {
> + if !Self::offset_valid::<U>(offset, self.maxsize()) {
> + return Err(EINVAL);
> + }
> +
> + // SAFETY: by the invariant of `IoRaw` this cannot overflow.
> + Ok(unsafe { self.addr().unchecked_add(offset) })
> + }
> +
> + /// Check at build time if the offset (plus the type size) is out of bounds.
> + ///
> + /// Compiletime checked version of [`io_addr`].
> + ///
> + /// See [`offset_valid`] for the performed offset check.
> + ///
> + ///
> + /// [`offset_valid`]: Self::offset_valid
> + /// [`io_addr`]: Self::io_addr
> + #[inline]
> + const fn io_addr_assert<U>(&self, offset: usize) -> usize {
> + build_assert!(Self::offset_valid::<U>(offset, SIZE));
> +
> + // SAFETY: by the invariant of `IoRaw` this cannot overflow.
> + unsafe { self.addr().unchecked_add(offset) }
> + }
> }
>
> /// IO-mapped memory, starting at the base address @addr and spanning @maxlen bytes.
> @@ -116,7 +173,7 @@ macro_rules! define_read {
> $(#[$attr])*
> #[inline]
> pub fn $name(&self, offset: usize) -> $type_name {
> - let addr = self.io_addr_assert::<$type_name>(offset);
> + let addr = self.0.io_addr_assert::<$type_name>(offset);
>
> // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> unsafe { bindings::$name(addr as _) }
> @@ -128,7 +185,7 @@ pub fn $name(&self, offset: usize) -> $type_name {
> /// out of bounds.
> $(#[$attr])*
> pub fn $try_name(&self, offset: usize) -> Result<$type_name> {
> - let addr = self.io_addr::<$type_name>(offset)?;
> + let addr = self.0.io_addr::<$type_name>(offset)?;
>
> // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> Ok(unsafe { bindings::$name(addr as _) })
> @@ -145,7 +202,7 @@ macro_rules! define_write {
> $(#[$attr])*
> #[inline]
> pub fn $name(&self, value: $type_name, offset: usize) {
> - let addr = self.io_addr_assert::<$type_name>(offset);
> + let addr = self.0.io_addr_assert::<$type_name>(offset);
>
> // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> unsafe { bindings::$name(value, addr as _, ) }
> @@ -157,7 +214,7 @@ pub fn $name(&self, value: $type_name, offset: usize) {
> /// out of bounds.
> $(#[$attr])*
> pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
> - let addr = self.io_addr::<$type_name>(offset)?;
> + let addr = self.0.io_addr::<$type_name>(offset)?;
>
> // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> unsafe { bindings::$name(value, addr as _) }
> @@ -190,34 +247,6 @@ 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
> - }
> - }
> -
> - #[inline]
> - fn io_addr<U>(&self, offset: usize) -> Result<usize> {
> - if !Self::offset_valid::<U>(offset, self.maxsize()) {
> - return Err(EINVAL);
> - }
> -
> - // Probably no need to check, since the safety requirements of `Self::new` guarantee that
> - // this can't overflow.
> - self.addr().checked_add(offset).ok_or(EINVAL)
> - }
> -
> - #[inline]
> - fn io_addr_assert<U>(&self, offset: usize) -> usize {
> - build_assert!(Self::offset_valid::<U>(offset, SIZE));
> -
> - self.addr() + offset
> - }
> -
> define_read!(readb, try_readb, u8);
> define_read!(readw, try_readw, u16);
> define_read!(readl, try_readl, u32);
>
> ---
> base-commit: 01b3cb620815fc3feb90ee117d9445a5b608a9f7
> change-id: 20250122-rust-io-offset-7b39b11e84ac
>
> Best regards,
> --
> Fiona Behrens <me@kloenk.dev>
>
Hi Fiona,
kernel test robot noticed the following build errors:
[auto build test ERROR on 01b3cb620815fc3feb90ee117d9445a5b608a9f7]
url: https://github.com/intel-lab-lkp/linux/commits/Fiona-Behrens/rust-io-move-offset_valid-and-io_addr-_assert-to-IoRaw/20250128-204724
base: 01b3cb620815fc3feb90ee117d9445a5b608a9f7
patch link: https://lore.kernel.org/r/20250128-rust-io-offset-v2-1-222de7c96a34%40kloenk.dev
patch subject: [PATCH v2] rust: io: move offset_valid and io_addr(_assert) to IoRaw
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250130/202501302233.VecDQamx-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250130/202501302233.VecDQamx-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501302233.VecDQamx-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from arch/x86/kernel/asm-offsets.c:14:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:21:
In file included from include/linux/mm.h:2224:
include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
504 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
505 | item];
| ~~~~
include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
511 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
512 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
524 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
525 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.
***
*** Rust bindings generator 'bindgen' < 0.69.5 together with libclang >= 19.1
*** may not work due to a bug (https://github.com/rust-lang/rust-bindgen/pull/2824),
*** unless patched (like Debian's).
*** Your bindgen version: 0.65.1
*** Your libclang version: 19.1.3
***
***
*** Please see Documentation/rust/quick-start.rst for details
*** on how to set up the Rust support.
***
In file included from rust/helpers/helpers.c:10:
In file included from rust/helpers/blk.c:3:
In file included from include/linux/blk-mq.h:5:
In file included from include/linux/blkdev.h:9:
In file included from include/linux/blk_types.h:10:
In file included from include/linux/bvec.h:10:
In file included from include/linux/highmem.h:8:
In file included from include/linux/cacheflush.h:5:
In file included from arch/x86/include/asm/cacheflush.h:5:
In file included from include/linux/mm.h:2224:
include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
504 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
505 | item];
| ~~~~
include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
511 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
512 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
524 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
525 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.
clang diag: include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
clang diag: include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
clang diag: include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
clang diag: include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
clang diag: include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
clang diag: include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
clang diag: include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
clang diag: include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
clang diag: include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
>> error[E0658]: use of unstable library feature 'unchecked_math': niche optimization path
--> rust/kernel/io.rs:82:33
|
82 | Ok(unsafe { self.addr().unchecked_add(offset) })
| ^^^^^^^^^^^^^
|
= note: see issue #85122 <https://github.com/rust-lang/rust/issues/85122> for more information
= help: add `#![feature(unchecked_math)]` to the crate attributes to enable
= note: this compiler was built on 2024-04-29; consider upgrading it if it is out of date
--
>> error[E0658]: use of unstable library feature 'unchecked_math': niche optimization path
--> rust/kernel/io.rs:99:30
|
99 | unsafe { self.addr().unchecked_add(offset) }
| ^^^^^^^^^^^^^
|
= note: see issue #85122 <https://github.com/rust-lang/rust/issues/85122> for more information
= help: add `#![feature(unchecked_math)]` to the crate attributes to enable
= note: this compiler was built on 2024-04-29; consider upgrading it if it is out of date
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2025 Red Hat, Inc.