* [PATCH v13 1/3] rust: io: add resource abstraction
2025-07-11 22:32 [PATCH v13 0/3] rust: platform: add Io support Daniel Almeida
@ 2025-07-11 22:32 ` Daniel Almeida
2025-07-15 8:03 ` Alice Ryhl
2025-07-11 22:32 ` [PATCH v13 2/3] rust: io: mem: add a generic iomem abstraction Daniel Almeida
2025-07-11 22:32 ` [PATCH v13 3/3] rust: platform: add resource accessors Daniel Almeida
2 siblings, 1 reply; 10+ messages in thread
From: Daniel Almeida @ 2025-07-11 22:32 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Rafael J. Wysocki
Cc: linux-kernel, rust-for-linux, Fiona Behrens, Daniel Almeida
In preparation for ioremap support, add a Rust abstraction for struct
resource.
A future commit will introduce the Rust API to ioremap a resource from a
platform device. The current abstraction, therefore, adds only the
minimum API needed to get that done.
Co-developed-by: Fiona Behrens <me@kloenk.dev>
Signed-off-by: Fiona Behrens <me@kloenk.dev>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
rust/bindings/bindings_helper.h | 1 +
rust/helpers/io.c | 36 +++++++
rust/kernel/io.rs | 4 +
rust/kernel/io/resource.rs | 204 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 245 insertions(+)
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 7e8f2285064797d5bbac5583990ff823b76c6bdc..5f795e60e889b9fc887013743c81b1cf92a52adb 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -53,6 +53,7 @@
#include <linux/file.h>
#include <linux/firmware.h>
#include <linux/fs.h>
+#include <linux/ioport.h>
#include <linux/jiffies.h>
#include <linux/jump_label.h>
#include <linux/mdio.h>
diff --git a/rust/helpers/io.c b/rust/helpers/io.c
index 15ea187c5466256effd07efe6f6995a1dd95a967..404776cf6717c8570c7600a24712ce6e4623d3c6 100644
--- a/rust/helpers/io.c
+++ b/rust/helpers/io.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/io.h>
+#include <linux/ioport.h>
void __iomem *rust_helper_ioremap(phys_addr_t offset, size_t size)
{
@@ -99,3 +100,38 @@ void rust_helper_writeq_relaxed(u64 value, void __iomem *addr)
writeq_relaxed(value, addr);
}
#endif
+
+resource_size_t rust_helper_resource_size(struct resource *res)
+{
+ return resource_size(res);
+}
+
+struct resource *rust_helper_request_mem_region(resource_size_t start,
+ resource_size_t n,
+ const char *name)
+{
+ return request_mem_region(start, n, name);
+}
+
+void rust_helper_release_mem_region(resource_size_t start, resource_size_t n)
+{
+ release_mem_region(start, n);
+}
+
+struct resource *rust_helper_request_region(resource_size_t start,
+ resource_size_t n, const char *name)
+{
+ return request_region(start, n, name);
+}
+
+struct resource *rust_helper_request_muxed_region(resource_size_t start,
+ resource_size_t n,
+ const char *name)
+{
+ return request_muxed_region(start, n, name);
+}
+
+void rust_helper_release_region(resource_size_t start, resource_size_t n)
+{
+ release_region(start, n);
+}
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 72d80a6f131e3e826ecd9d2c3bcf54e89aa60cc3..7b70d5b5477e57d6d0f24bcd26bd8b0071721bc0 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -7,6 +7,10 @@
use crate::error::{code::EINVAL, Result};
use crate::{bindings, build_assert};
+pub mod resource;
+
+pub use resource::Resource;
+
/// Raw representation of an MMIO region.
///
/// By itself, the existence of an instance of this structure does not provide any guarantees that
diff --git a/rust/kernel/io/resource.rs b/rust/kernel/io/resource.rs
new file mode 100644
index 0000000000000000000000000000000000000000..b20b2df3d65dfc8c14881c440ea901da0273b1bb
--- /dev/null
+++ b/rust/kernel/io/resource.rs
@@ -0,0 +1,204 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Abstractions for [system
+//! resources](https://docs.kernel.org/core-api/kernel-api.html#resources-management).
+//!
+//! C header: [`include/linux/ioport.h`](srctree/include/linux/ioport.h)
+
+use core::ops::Deref;
+use core::ptr::NonNull;
+
+use crate::prelude::*;
+use crate::str::CStr;
+use crate::types::Opaque;
+
+/// Resource Size type.
+///
+/// This is a type alias to either `u32` or `u64` depending on the config option
+/// `CONFIG_PHYS_ADDR_T_64BIT`, and it can be a u64 even on 32-bit architectures.
+pub type ResourceSize = bindings::phys_addr_t;
+
+/// A region allocated from a parent [`Resource`].
+///
+/// # Invariants
+///
+/// - `self.0` points to a valid `bindings::resource` that was obtained through
+/// `bindings::__request_region`.
+pub struct Region(NonNull<bindings::resource>);
+
+impl Deref for Region {
+ type Target = Resource;
+
+ fn deref(&self) -> &Self::Target {
+ // SAFETY: Safe as per the invariant of `Region`.
+ unsafe { Resource::from_raw(self.0.as_ptr()) }
+ }
+}
+
+impl Drop for Region {
+ fn drop(&mut self) {
+ let (flags, start, size) = {
+ let res = &**self;
+ (res.flags(), res.start(), res.size())
+ };
+
+ let release_fn = if flags.contains(Flags::IORESOURCE_MEM) {
+ bindings::release_mem_region
+ } else {
+ bindings::release_region
+ };
+
+ // SAFETY: Safe as per the invariant of `Region`.
+ unsafe { release_fn(start, size) };
+ }
+}
+
+// SAFETY: `Region` only holds a pointer to a C `struct resource`, which is safe to be used from
+// any thread.
+unsafe impl Send for Region {}
+
+// SAFETY: `Region` only holds a pointer to a C `struct resource`, references to which are
+// safe to be used from any thread.
+unsafe impl Sync for Region {}
+
+/// A resource abstraction.
+///
+/// # Invariants
+///
+/// [`Resource`] is a transparent wrapper around a valid `bindings::resource`.
+#[repr(transparent)]
+pub struct Resource(Opaque<bindings::resource>);
+
+impl Resource {
+ /// Creates a reference to a [`Resource`] from a valid pointer.
+ ///
+ /// # Safety
+ ///
+ /// The caller must ensure that for the duration of 'a, the pointer will
+ /// point at a valid `bindings::resource`.
+ ///
+ /// The caller must also ensure that the [`Resource`] is only accessed via the
+ /// returned reference for the duration of 'a.
+ pub(crate) const unsafe fn from_raw<'a>(ptr: *mut bindings::resource) -> &'a Self {
+ // SAFETY: Self is a transparent wrapper around `Opaque<bindings::resource>`.
+ unsafe { &*ptr.cast() }
+ }
+
+ /// Requests a resource region.
+ ///
+ /// Exclusive access will be given and the region will be marked as busy.
+ /// Further calls to [`Self::request_region`] will return [`None`] if
+ /// the region, or a part of it, is already in use.
+ pub fn request_region(
+ &self,
+ start: ResourceSize,
+ size: ResourceSize,
+ name: &'static CStr,
+ flags: Flags,
+ ) -> Option<Region> {
+ // SAFETY:
+ // - Safe as per the invariant of `Resource`.
+ // - `__request_region` will store a reference to the name, but that is
+ // safe as the name is 'static.
+ let region = unsafe {
+ bindings::__request_region(
+ self.0.get(),
+ start,
+ size,
+ name.as_char_ptr(),
+ flags.0 as c_int,
+ )
+ };
+
+ Some(Region(NonNull::new(region)?))
+ }
+
+ /// Returns the size of the resource.
+ pub fn size(&self) -> ResourceSize {
+ let inner = self.0.get();
+ // SAFETY: safe as per the invariants of `Resource`.
+ unsafe { bindings::resource_size(inner) }
+ }
+
+ /// Returns the start address of the resource.
+ pub fn start(&self) -> u64 {
+ let inner = self.0.get();
+ // SAFETY: safe as per the invariants of `Resource`.
+ unsafe { (*inner).start }
+ }
+
+ /// Returns the name of the resource.
+ pub fn name(&self) -> &'static CStr {
+ let inner = self.0.get();
+ // SAFETY: safe as per the invariants of `Resource`
+ unsafe { CStr::from_char_ptr((*inner).name) }
+ }
+
+ /// Returns the flags associated with the resource.
+ pub fn flags(&self) -> Flags {
+ let inner = self.0.get();
+ // SAFETY: safe as per the invariants of `Resource`
+ let flags = unsafe { (*inner).flags };
+
+ Flags(flags)
+ }
+}
+
+// SAFETY: `Resource` only holds a pointer to a C `struct resource`, which is
+// safe to be used from any thread.
+unsafe impl Send for Resource {}
+
+// SAFETY: `Resource` only holds a pointer to a C `struct resource`, references
+// to which are safe to be used from any thread.
+unsafe impl Sync for Resource {}
+
+/// Resource flags as stored in the C `struct resource::flags` field.
+///
+/// They can be combined with the operators `|`, `&`, and `!`.
+///
+/// Values can be used from the [`flags`] module.
+#[derive(Clone, Copy, PartialEq)]
+pub struct Flags(c_ulong);
+
+impl Flags {
+ /// Check whether `flags` is contained in `self`.
+ pub fn contains(self, flags: Flags) -> bool {
+ (self & flags) == flags
+ }
+}
+
+impl core::ops::BitOr for Flags {
+ type Output = Self;
+ fn bitor(self, rhs: Self) -> Self::Output {
+ Self(self.0 | rhs.0)
+ }
+}
+
+impl core::ops::BitAnd for Flags {
+ type Output = Self;
+ fn bitand(self, rhs: Self) -> Self::Output {
+ Self(self.0 & rhs.0)
+ }
+}
+
+impl core::ops::Not for Flags {
+ type Output = Self;
+ fn not(self) -> Self::Output {
+ Self(!self.0)
+ }
+}
+
+impl Flags {
+ /// PCI/ISA I/O ports.
+ pub const IORESOURCE_IO: Flags = Flags(bindings::IORESOURCE_IO as c_ulong);
+
+ /// Resource is software muxed.
+ pub const IORESOURCE_MUXED: Flags = Flags(bindings::IORESOURCE_MUXED as c_ulong);
+
+ /// Resource represents a memory region.
+ pub const IORESOURCE_MEM: Flags = Flags(bindings::IORESOURCE_MEM as c_ulong);
+
+ /// Resource represents a memory region that must be ioremaped using `ioremap_np`.
+ pub const IORESOURCE_MEM_NONPOSTED: Flags =
+ Flags(bindings::IORESOURCE_MEM_NONPOSTED as c_ulong);
+}
--
2.50.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v13 1/3] rust: io: add resource abstraction
2025-07-11 22:32 ` [PATCH v13 1/3] rust: io: add resource abstraction Daniel Almeida
@ 2025-07-15 8:03 ` Alice Ryhl
2025-07-16 16:52 ` Daniel Almeida
0 siblings, 1 reply; 10+ messages in thread
From: Alice Ryhl @ 2025-07-15 8:03 UTC (permalink / raw)
To: Daniel Almeida
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Rafael J. Wysocki, linux-kernel, rust-for-linux, Fiona Behrens
On Fri, Jul 11, 2025 at 07:32:27PM -0300, Daniel Almeida wrote:
> In preparation for ioremap support, add a Rust abstraction for struct
> resource.
>
> A future commit will introduce the Rust API to ioremap a resource from a
> platform device. The current abstraction, therefore, adds only the
> minimum API needed to get that done.
>
> Co-developed-by: Fiona Behrens <me@kloenk.dev>
> Signed-off-by: Fiona Behrens <me@kloenk.dev>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
Some nits below, but overall LGTM. With those fixed:
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> + /// Requests a resource region.
> + ///
> + /// Exclusive access will be given and the region will be marked as busy.
> + /// Further calls to [`Self::request_region`] will return [`None`] if
> + /// the region, or a part of it, is already in use.
> + pub fn request_region(
> + &self,
> + start: ResourceSize,
> + size: ResourceSize,
> + name: &'static CStr,
> + flags: Flags,
> + ) -> Option<Region> {
> + // SAFETY:
> + // - Safe as per the invariant of `Resource`.
> + // - `__request_region` will store a reference to the name, but that is
> + // safe as the name is 'static.
> + let region = unsafe {
> + bindings::__request_region(
> + self.0.get(),
> + start,
> + size,
> + name.as_char_ptr(),
> + flags.0 as c_int,
> + )
> + };
> +
> + Some(Region(NonNull::new(region)?))
> + }
> +
> + /// Returns the size of the resource.
> + pub fn size(&self) -> ResourceSize {
> + let inner = self.0.get();
> + // SAFETY: safe as per the invariants of `Resource`.
> + unsafe { bindings::resource_size(inner) }
> + }
> +
> + /// Returns the start address of the resource.
> + pub fn start(&self) -> u64 {
This should be a ResourceSize, right? You use the ResourceSize type for
the `start` when calling `request_region`.
Or ... should we have another PhysAddr typedef that we can use here?
> + let inner = self.0.get();
> + // SAFETY: safe as per the invariants of `Resource`.
> + unsafe { (*inner).start }
> + }
> +
> + /// Returns the name of the resource.
> + pub fn name(&self) -> &'static CStr {
> + let inner = self.0.get();
> + // SAFETY: safe as per the invariants of `Resource`
> + unsafe { CStr::from_char_ptr((*inner).name) }
This is 'static? I would like this safety comment to explicitly say that
the string always lives forever no matter what resource you call this
on.
Alice
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v13 1/3] rust: io: add resource abstraction
2025-07-15 8:03 ` Alice Ryhl
@ 2025-07-16 16:52 ` Daniel Almeida
2025-07-16 17:04 ` Alice Ryhl
2025-07-16 17:10 ` Danilo Krummrich
0 siblings, 2 replies; 10+ messages in thread
From: Daniel Almeida @ 2025-07-16 16:52 UTC (permalink / raw)
To: Alice Ryhl
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Rafael J. Wysocki, linux-kernel, rust-for-linux, Fiona Behrens
Hi Alice,
>
>> + let inner = self.0.get();
>> + // SAFETY: safe as per the invariants of `Resource`.
>> + unsafe { (*inner).start }
>> + }
>> +
>> + /// Returns the name of the resource.
>> + pub fn name(&self) -> &'static CStr {
>> + let inner = self.0.get();
>> + // SAFETY: safe as per the invariants of `Resource`
>> + unsafe { CStr::from_char_ptr((*inner).name) }
>
> This is 'static? I would like this safety comment to explicitly say that
> the string always lives forever no matter what resource you call this
> on.
>
> Alice
>
Actually, we have a bit of a problem here.
First, there appears to be no guarantee that a `Resource` has a valid name.
In fact:
#define DEFINE_RES_NAMED(_start, _size, _name, _flags) \
DEFINE_RES_NAMED_DESC(_start, _size, _name, _flags, IORES_DESC_NONE)
#define DEFINE_RES(_start, _size, _flags) \
DEFINE_RES_NAMED(_start, _size, NULL, _flags)
#define DEFINE_RES_IO_NAMED(_start, _size, _name) \
DEFINE_RES_NAMED((_start), (_size), (_name), IORESOURCE_IO)
#define DEFINE_RES_IO(_start, _size) \
DEFINE_RES_IO_NAMED((_start), (_size), NULL)
The non _NAMED version of these macros will assign a NULL pointer, so we can't
derive a CStr from that at all.
On top of that, although some call sites do use static names, i.e.:
struct resource ioport_resource = {
.name = "PCI IO",
.start = 0,
.end = IO_SPACE_LIMIT,
.flags = IORESOURCE_IO,
};
EXPORT_SYMBOL(ioport_resource);
struct resource iomem_resource = {
.name = "PCI mem",
.start = 0,
.end = -1,
.flags = IORESOURCE_MEM,
};
EXPORT_SYMBOL(iomem_resource);
static struct resource busn_resource = {
.name = "PCI busn",
.start = 0,
.end = 255,
.flags = IORESOURCE_BUS,
};
Some appear to use other, smaller lifetimes, like the one below:
struct pnp_resource *pnp_add_resource(struct pnp_dev *dev,
struct resource *res)
{
struct pnp_resource *pnp_res;
pnp_res = pnp_new_resource(dev);
if (!pnp_res) {
dev_err(&dev->dev, "can't add resource %pR\n", res);
return NULL;
}
pnp_res->res = *res;
pnp_res->res.name = dev->name;
I guess the easiest solution is to drop 'static in order to account for the
above, and change the signature to return Option<&CStr> instead.
We can also change Region to own the name, and pass name by value here:
pub fn request_region(
&self,
start: ResourceSize,
size: ResourceSize,
name: &'static CStr <------
Thoughts?
— Daniel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v13 1/3] rust: io: add resource abstraction
2025-07-16 16:52 ` Daniel Almeida
@ 2025-07-16 17:04 ` Alice Ryhl
2025-07-16 17:10 ` Danilo Krummrich
1 sibling, 0 replies; 10+ messages in thread
From: Alice Ryhl @ 2025-07-16 17:04 UTC (permalink / raw)
To: Daniel Almeida
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Rafael J. Wysocki, linux-kernel, rust-for-linux, Fiona Behrens
On Wed, Jul 16, 2025 at 6:53 PM Daniel Almeida
<daniel.almeida@collabora.com> wrote:
>
> Hi Alice,
>
> >
> >> + let inner = self.0.get();
> >> + // SAFETY: safe as per the invariants of `Resource`.
> >> + unsafe { (*inner).start }
> >> + }
> >> +
> >> + /// Returns the name of the resource.
> >> + pub fn name(&self) -> &'static CStr {
> >> + let inner = self.0.get();
> >> + // SAFETY: safe as per the invariants of `Resource`
> >> + unsafe { CStr::from_char_ptr((*inner).name) }
> >
> > This is 'static? I would like this safety comment to explicitly say that
> > the string always lives forever no matter what resource you call this
> > on.
> >
> > Alice
> >
>
> Actually, we have a bit of a problem here.
>
> First, there appears to be no guarantee that a `Resource` has a valid name.
>
> In fact:
>
> #define DEFINE_RES_NAMED(_start, _size, _name, _flags) \
> DEFINE_RES_NAMED_DESC(_start, _size, _name, _flags, IORES_DESC_NONE)
> #define DEFINE_RES(_start, _size, _flags) \
> DEFINE_RES_NAMED(_start, _size, NULL, _flags)
>
> #define DEFINE_RES_IO_NAMED(_start, _size, _name) \
> DEFINE_RES_NAMED((_start), (_size), (_name), IORESOURCE_IO)
> #define DEFINE_RES_IO(_start, _size) \
> DEFINE_RES_IO_NAMED((_start), (_size), NULL)
>
> The non _NAMED version of these macros will assign a NULL pointer, so we can't
> derive a CStr from that at all.
>
> On top of that, although some call sites do use static names, i.e.:
>
> struct resource ioport_resource = {
> .name = "PCI IO",
> .start = 0,
> .end = IO_SPACE_LIMIT,
> .flags = IORESOURCE_IO,
> };
> EXPORT_SYMBOL(ioport_resource);
>
> struct resource iomem_resource = {
> .name = "PCI mem",
> .start = 0,
> .end = -1,
> .flags = IORESOURCE_MEM,
> };
> EXPORT_SYMBOL(iomem_resource);
>
> static struct resource busn_resource = {
> .name = "PCI busn",
> .start = 0,
> .end = 255,
> .flags = IORESOURCE_BUS,
> };
>
> Some appear to use other, smaller lifetimes, like the one below:
>
> struct pnp_resource *pnp_add_resource(struct pnp_dev *dev,
> struct resource *res)
> {
> struct pnp_resource *pnp_res;
>
> pnp_res = pnp_new_resource(dev);
> if (!pnp_res) {
> dev_err(&dev->dev, "can't add resource %pR\n", res);
> return NULL;
> }
>
> pnp_res->res = *res;
> pnp_res->res.name = dev->name;
>
>
> I guess the easiest solution is to drop 'static in order to account for the
> above, and change the signature to return Option<&CStr> instead.
Using Option<&CStr> sounds good to me.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v13 1/3] rust: io: add resource abstraction
2025-07-16 16:52 ` Daniel Almeida
2025-07-16 17:04 ` Alice Ryhl
@ 2025-07-16 17:10 ` Danilo Krummrich
1 sibling, 0 replies; 10+ messages in thread
From: Danilo Krummrich @ 2025-07-16 17:10 UTC (permalink / raw)
To: Daniel Almeida
Cc: Alice Ryhl, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel,
rust-for-linux, Fiona Behrens
On Wed Jul 16, 2025 at 6:52 PM CEST, Daniel Almeida wrote:
> I guess the easiest solution is to drop 'static in order to account for the
> above, and change the signature to return Option<&CStr> instead.
I think you have to do both, this...
> We can also change Region to own the name, and pass name by value here:
...and this.
Otherwise you'd need to ensure that the Resource you create the Region from
out-lives the Region, which I think we can't.
> pub fn request_region(
> &self,
> start: ResourceSize,
> size: ResourceSize,
> name: &'static CStr <------
>
>
> Thoughts?
>
> — Daniel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v13 2/3] rust: io: mem: add a generic iomem abstraction
2025-07-11 22:32 [PATCH v13 0/3] rust: platform: add Io support Daniel Almeida
2025-07-11 22:32 ` [PATCH v13 1/3] rust: io: add resource abstraction Daniel Almeida
@ 2025-07-11 22:32 ` Daniel Almeida
2025-07-15 8:12 ` Alice Ryhl
2025-07-11 22:32 ` [PATCH v13 3/3] rust: platform: add resource accessors Daniel Almeida
2 siblings, 1 reply; 10+ messages in thread
From: Daniel Almeida @ 2025-07-11 22:32 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Rafael J. Wysocki
Cc: linux-kernel, rust-for-linux, Daniel Almeida
Add a generic iomem abstraction to safely read and write ioremapped
regions. This abstraction requires a previously acquired IoRequest
instance. This makes it so that both the resource and the device match,
or, in other words, that the resource is indeed a valid resource for a
given bound device.
A subsequent patch will add the ability to retrieve IoRequest instances
from platform devices.
The reads and writes are done through IoRaw, and are thus checked either
at compile-time, if the size of the region is known at that point, or at
runtime otherwise.
Non-exclusive access to the underlying memory region is made possible to
cater to cases where overlapped regions are unavoidable.
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
rust/helpers/io.c | 5 +
rust/kernel/io.rs | 1 +
rust/kernel/io/mem.rs | 271 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 277 insertions(+)
diff --git a/rust/helpers/io.c b/rust/helpers/io.c
index 404776cf6717c8570c7600a24712ce6e4623d3c6..c475913c69e647b1042e8e7d66b9148d892947a1 100644
--- a/rust/helpers/io.c
+++ b/rust/helpers/io.c
@@ -8,6 +8,11 @@ void __iomem *rust_helper_ioremap(phys_addr_t offset, size_t size)
return ioremap(offset, size);
}
+void __iomem *rust_helper_ioremap_np(phys_addr_t offset, size_t size)
+{
+ return ioremap_np(offset, size);
+}
+
void rust_helper_iounmap(void __iomem *addr)
{
iounmap(addr);
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 7b70d5b5477e57d6d0f24bcd26bd8b0071721bc0..b7fc759f8b5d3c3ac6f33f5a66e9f619c58b7405 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -7,6 +7,7 @@
use crate::error::{code::EINVAL, Result};
use crate::{bindings, build_assert};
+pub mod mem;
pub mod resource;
pub use resource::Resource;
diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs
new file mode 100644
index 0000000000000000000000000000000000000000..fc2d1f2715a004d34075eb6e1438d69f94cf4ba3
--- /dev/null
+++ b/rust/kernel/io/mem.rs
@@ -0,0 +1,271 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Generic memory-mapped IO.
+
+use core::ops::Deref;
+
+use crate::device::Bound;
+use crate::device::Device;
+use crate::devres::Devres;
+use crate::io;
+use crate::io::resource::Region;
+use crate::io::resource::Resource;
+use crate::io::Io;
+use crate::io::IoRaw;
+use crate::prelude::*;
+
+/// An IO request for a specific device and resource.
+pub struct IoRequest<'a> {
+ device: &'a Device<Bound>,
+ resource: &'a Resource,
+}
+
+impl<'a> IoRequest<'a> {
+ /// Creates a new [`IoRequest`] instance.
+ ///
+ /// # Safety
+ ///
+ /// Callers must ensure that `resource` is valid for `device` during the
+ /// lifetime `'a`.
+ pub(crate) unsafe fn new(device: &'a Device<Bound>, resource: &'a Resource) -> Self {
+ IoRequest { device, resource }
+ }
+
+ /// Maps an [`IoRequest`] where the size is known at compile time.
+ ///
+ /// This uses the [`ioremap()`] C API.
+ ///
+ /// [`ioremap()`]: https://docs.kernel.org/driver-api/device-io.html#getting-access-to-the-device
+ ///
+ /// # Examples
+ ///
+ /// The following example uses a [`platform::Device`] for illustration
+ /// purposes.
+ ///
+ /// ```no_run
+ /// use kernel::{bindings, c_str, platform, of, device::Core};
+ /// struct SampleDriver;
+ ///
+ /// impl platform::Driver for SampleDriver {
+ /// # type IdInfo = ();
+ /// # const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
+ ///
+ /// fn probe(
+ /// pdev: &platform::Device<Core>,
+ /// info: Option<&Self::IdInfo>,
+ /// ) -> Result<Pin<KBox<Self>>> {
+ /// let offset = 0; // Some offset.
+ ///
+ /// // If the size is known at compile time, use [`Self::iomap_sized`].
+ /// //
+ /// // No runtime checks will apply when reading and writing.
+ /// let request = pdev.request_io_by_index(0).ok_or(ENODEV)?;
+ /// let iomem = request.iomap_sized::<42>();
+ /// let iomem = KBox::pin_init(iomem, GFP_KERNEL)?;
+ ///
+ /// let io = iomem.access(pdev.as_ref())?;
+ ///
+ /// // Read and write a 32-bit value at `offset`.
+ /// let data = io.read32_relaxed(offset);
+ ///
+ /// io.write32_relaxed(data, offset);
+ ///
+ /// # Ok(KBox::new(SampleDriver, GFP_KERNEL)?.into())
+ /// }
+ /// }
+ /// ```
+ pub fn iomap_sized<const SIZE: usize>(self) -> impl PinInit<Devres<IoMem<SIZE>>, Error> + 'a {
+ IoMem::new(self)
+ }
+
+ /// Same as [`Self::iomap_sized`] but with exclusive access to the
+ /// underlying region.
+ ///
+ /// This uses the [`ioremap()`] C API.
+ ///
+ /// [`ioremap()`]: https://docs.kernel.org/driver-api/device-io.html#getting-access-to-the-device
+ pub fn iomap_exclusive_sized<const SIZE: usize>(
+ self,
+ ) -> impl PinInit<Devres<ExclusiveIoMem<SIZE>>, Error> + 'a {
+ ExclusiveIoMem::new(self)
+ }
+
+ /// Maps an [`IoRequest`] where the size is not known at compile time,
+ ///
+ /// This uses the [`ioremap()`] C API.
+ ///
+ /// [`ioremap()`]: https://docs.kernel.org/driver-api/device-io.html#getting-access-to-the-device
+ ///
+ /// # Examples
+ ///
+ /// The following example uses a [`platform::Device`] for illustration
+ /// purposes.
+ ///
+ /// ```no_run
+ /// use kernel::{bindings, c_str, platform, of, device::Core};
+ /// struct SampleDriver;
+ ///
+ /// impl platform::Driver for SampleDriver {
+ /// # type IdInfo = ();
+ /// # const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
+ ///
+ /// fn probe(
+ /// pdev: &platform::Device<Core>,
+ /// info: Option<&Self::IdInfo>,
+ /// ) -> Result<Pin<KBox<Self>>> {
+ /// let offset = 0; // Some offset.
+ ///
+ /// // Unlike [`Self::iomap_sized`], here the size of the memory region
+ /// // is not known at compile time, so only the `try_read*` and `try_write*`
+ /// // family of functions should be used, leading to runtime checks on every
+ /// // access.
+ /// let request = pdev.request_io_by_index(0).ok_or(ENODEV)?;
+ /// let iomem = request.iomap();
+ /// let iomem = KBox::pin_init(iomem, GFP_KERNEL)?;
+ ///
+ /// let io = iomem.access(pdev.as_ref())?;
+ ///
+ /// let data = io.try_read32_relaxed(offset)?;
+ ///
+ /// io.try_write32_relaxed(data, offset)?;
+ ///
+ /// # Ok(KBox::new(SampleDriver, GFP_KERNEL)?.into())
+ /// }
+ /// }
+ /// ```
+ pub fn iomap(self) -> impl PinInit<Devres<IoMem<0>>, Error> + 'a {
+ Self::iomap_sized::<0>(self)
+ }
+
+ /// Same as [`Self::iomap`] but with exclusive access to the underlying
+ /// region.
+ pub fn iomap_exclusive(self) -> impl PinInit<Devres<ExclusiveIoMem<0>>, Error> + 'a {
+ Self::iomap_exclusive_sized::<0>(self)
+ }
+}
+
+/// An exclusive memory-mapped IO region.
+///
+/// # Invariants
+///
+/// - [`ExclusiveIoMem`] has exclusive access to the underlying [`IoMem`].
+pub struct ExclusiveIoMem<const SIZE: usize> {
+ /// The underlying `IoMem` instance.
+ iomem: IoMem<SIZE>,
+
+ /// The region abstraction. This represents exclusive access to the
+ /// range represented by the underlying `iomem`.
+ ///
+ /// This field is needed for ownership of the region.
+ _region: Region,
+}
+
+impl<const SIZE: usize> ExclusiveIoMem<SIZE> {
+ /// Creates a new `ExclusiveIoMem` instance.
+ fn ioremap(resource: &Resource) -> Result<Self> {
+ let start = resource.start();
+ let size = resource.size();
+ let name = resource.name();
+
+ let region = resource
+ .request_region(start, size, name, io::resource::Flags::IORESOURCE_MEM)
+ .ok_or(EBUSY)?;
+
+ let iomem = IoMem::ioremap(resource)?;
+
+ let iomem = ExclusiveIoMem {
+ iomem,
+ _region: region,
+ };
+
+ Ok(iomem)
+ }
+
+ /// Creates a new `ExclusiveIoMem` instance from a previously acquired [`IoRequest`].
+ pub fn new<'a>(io_request: IoRequest<'a>) -> impl PinInit<Devres<Self>, Error> + 'a {
+ let dev = io_request.device;
+ let res = io_request.resource;
+
+ Devres::new(dev, Self::ioremap(res))
+ }
+}
+
+impl<const SIZE: usize> Deref for ExclusiveIoMem<SIZE> {
+ type Target = Io<SIZE>;
+
+ fn deref(&self) -> &Self::Target {
+ &self.iomem
+ }
+}
+
+/// A generic memory-mapped IO region.
+///
+/// Accesses to the underlying region is checked either at compile time, if the
+/// region's size is known at that point, or at runtime otherwise.
+///
+/// # Invariants
+///
+/// [`IoMem`] always holds an [`IoRaw`] instance that holds a valid pointer to the
+/// start of the I/O memory mapped region.
+pub struct IoMem<const SIZE: usize = 0> {
+ io: IoRaw<SIZE>,
+}
+
+impl<const SIZE: usize> IoMem<SIZE> {
+ fn ioremap(resource: &Resource) -> Result<Self> {
+ let size = resource.size();
+ if size == 0 {
+ return Err(EINVAL);
+ }
+
+ let res_start = resource.start();
+
+ let addr = if resource
+ .flags()
+ .contains(io::resource::Flags::IORESOURCE_MEM_NONPOSTED)
+ {
+ // SAFETY:
+ // - `res_start` and `size` are read from a presumably valid `struct resource`.
+ // - `size` is known not to be zero at this point.
+ unsafe { bindings::ioremap_np(res_start, size.try_into()?) }
+ } else {
+ // SAFETY:
+ // - `res_start` and `size` are read from a presumably valid `struct resource`.
+ // - `size` is known not to be zero at this point.
+ unsafe { bindings::ioremap(res_start, size.try_into()?) }
+ };
+
+ if addr.is_null() {
+ return Err(ENOMEM);
+ }
+
+ let io = IoRaw::new(addr as usize, size.try_into()?)?;
+ let io = IoMem { io };
+
+ Ok(io)
+ }
+
+ /// Creates a new `IoMem` instance from a previously acquired [`IoRequest`].
+ pub fn new<'a>(io_request: IoRequest<'a>) -> impl PinInit<Devres<Self>, Error> + 'a {
+ let dev = io_request.device;
+ let res = io_request.resource;
+
+ Devres::new(dev, Self::ioremap(res))
+ }
+}
+
+impl<const SIZE: usize> Drop for IoMem<SIZE> {
+ fn drop(&mut self) {
+ // SAFETY: Safe as by the invariant of `Io`.
+ unsafe { bindings::iounmap(self.io.addr() as *mut c_void) }
+ }
+}
+
+impl<const SIZE: usize> Deref for IoMem<SIZE> {
+ type Target = Io<SIZE>;
+
+ fn deref(&self) -> &Self::Target {
+ // SAFETY: Safe as by the invariant of `IoMem`.
+ unsafe { Io::from_raw(&self.io) }
+ }
+}
--
2.50.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v13 2/3] rust: io: mem: add a generic iomem abstraction
2025-07-11 22:32 ` [PATCH v13 2/3] rust: io: mem: add a generic iomem abstraction Daniel Almeida
@ 2025-07-15 8:12 ` Alice Ryhl
2025-07-15 8:20 ` Danilo Krummrich
0 siblings, 1 reply; 10+ messages in thread
From: Alice Ryhl @ 2025-07-15 8:12 UTC (permalink / raw)
To: Daniel Almeida
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Rafael J. Wysocki, linux-kernel, rust-for-linux
On Fri, Jul 11, 2025 at 07:32:28PM -0300, Daniel Almeida wrote:
> Add a generic iomem abstraction to safely read and write ioremapped
> regions. This abstraction requires a previously acquired IoRequest
> instance. This makes it so that both the resource and the device match,
> or, in other words, that the resource is indeed a valid resource for a
> given bound device.
>
> A subsequent patch will add the ability to retrieve IoRequest instances
> from platform devices.
>
> The reads and writes are done through IoRaw, and are thus checked either
> at compile-time, if the size of the region is known at that point, or at
> runtime otherwise.
>
> Non-exclusive access to the underlying memory region is made possible to
> cater to cases where overlapped regions are unavoidable.
>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> +impl<const SIZE: usize> IoMem<SIZE> {
> + fn ioremap(resource: &Resource) -> Result<Self> {
> + let size = resource.size();
> + if size == 0 {
> + return Err(EINVAL);
> + }
> +
> + let res_start = resource.start();
> +
> + let addr = if resource
> + .flags()
> + .contains(io::resource::Flags::IORESOURCE_MEM_NONPOSTED)
> + {
> + // SAFETY:
> + // - `res_start` and `size` are read from a presumably valid `struct resource`.
> + // - `size` is known not to be zero at this point.
> + unsafe { bindings::ioremap_np(res_start, size.try_into()?) }
> + } else {
> + // SAFETY:
> + // - `res_start` and `size` are read from a presumably valid `struct resource`.
> + // - `size` is known not to be zero at this point.
> + unsafe { bindings::ioremap(res_start, size.try_into()?) }
I thought a bit more about this, and I think it's fine for these sizes to
be converted with try_into()?.
> + };
> +
> + if addr.is_null() {
> + return Err(ENOMEM);
> + }
> +
> + let io = IoRaw::new(addr as usize, size.try_into()?)?;
Though may we could avoid converting it twice?
Alice
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v13 2/3] rust: io: mem: add a generic iomem abstraction
2025-07-15 8:12 ` Alice Ryhl
@ 2025-07-15 8:20 ` Danilo Krummrich
0 siblings, 0 replies; 10+ messages in thread
From: Danilo Krummrich @ 2025-07-15 8:20 UTC (permalink / raw)
To: Alice Ryhl
Cc: Daniel Almeida, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel,
rust-for-linux
On Tue Jul 15, 2025 at 10:12 AM CEST, Alice Ryhl wrote:
>> + {
>> + // SAFETY:
>> + // - `res_start` and `size` are read from a presumably valid `struct resource`.
>> + // - `size` is known not to be zero at this point.
>> + unsafe { bindings::ioremap_np(res_start, size.try_into()?) }
>> + } else {
>> + // SAFETY:
>> + // - `res_start` and `size` are read from a presumably valid `struct resource`.
>> + // - `size` is known not to be zero at this point.
>> + unsafe { bindings::ioremap(res_start, size.try_into()?) }
>
> I thought a bit more about this, and I think it's fine for these sizes to
> be converted with try_into()?.
Fine for me too, but we should add a comment with a TODO to get this straight
in the future.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v13 3/3] rust: platform: add resource accessors
2025-07-11 22:32 [PATCH v13 0/3] rust: platform: add Io support Daniel Almeida
2025-07-11 22:32 ` [PATCH v13 1/3] rust: io: add resource abstraction Daniel Almeida
2025-07-11 22:32 ` [PATCH v13 2/3] rust: io: mem: add a generic iomem abstraction Daniel Almeida
@ 2025-07-11 22:32 ` Daniel Almeida
2 siblings, 0 replies; 10+ messages in thread
From: Daniel Almeida @ 2025-07-11 22:32 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Rafael J. Wysocki
Cc: linux-kernel, rust-for-linux, Daniel Almeida
The previous patches have added the abstractions for Resources and the
ability to map them and therefore read and write the underlying memory .
The only thing missing to make this accessible for platform devices is
to provide accessors that return instances of IoRequest<'a>. These
ensure that the resource are valid only for the lifetime of the platform
device, and that the platform device is in the Bound state.
Therefore, add these accessors. Also make it possible to retrieve
resources from platform devices in Rust using either a name or an index.
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
rust/kernel/platform.rs | 60 ++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 59 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 86f9d73c64b38ffe067be329a77b2fc04564c7fe..57f9964bb736cf5749ec3954def03c0d02873642 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -5,8 +5,11 @@
//! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h)
use crate::{
- acpi, bindings, container_of, device, driver,
+ acpi, bindings, container_of,
+ device::{self, Bound},
+ driver,
error::{to_result, Result},
+ io::{mem::IoRequest, Resource},
of,
prelude::*,
str::CStr,
@@ -211,6 +214,61 @@ impl<Ctx: device::DeviceContext> Device<Ctx> {
fn as_raw(&self) -> *mut bindings::platform_device {
self.0.get()
}
+
+ /// Returns the resource at `index`, if any.
+ pub fn resource_by_index(&self, index: u32) -> Option<&Resource> {
+ // SAFETY: `self.as_raw()` returns a valid pointer to a `struct platform_device`.
+ let resource = unsafe {
+ bindings::platform_get_resource(self.as_raw(), bindings::IORESOURCE_MEM, index)
+ };
+
+ if resource.is_null() {
+ return None;
+ }
+
+ // SAFETY: `resource` is a valid pointer to a `struct resource` as
+ // returned by `platform_get_resource`.
+ Some(unsafe { Resource::from_raw(resource) })
+ }
+
+ /// Returns the resource with a given `name`, if any.
+ pub fn resource_by_name(&self, name: &CStr) -> Option<&Resource> {
+ // SAFETY: `self.as_raw()` returns a valid pointer to a `struct
+ // platform_device` and `name` points to a valid C string.
+ let resource = unsafe {
+ bindings::platform_get_resource_byname(
+ self.as_raw(),
+ bindings::IORESOURCE_MEM,
+ name.as_char_ptr(),
+ )
+ };
+
+ if resource.is_null() {
+ return None;
+ }
+
+ // SAFETY: `resource` is a valid pointer to a `struct resource` as
+ // returned by `platform_get_resource`.
+ Some(unsafe { Resource::from_raw(resource) })
+ }
+}
+
+impl Device<Bound> {
+ /// Returns an `IoRequest` for the resource at `index`, if any.
+ pub fn io_request_by_index(&self, index: u32) -> Option<IoRequest<'_>> {
+ // SAFETY: `resource` is a valid resource for `&self` during the
+ // lifetime of the `IoRequest`.
+ self.resource_by_index(index)
+ .map(|resource| unsafe { IoRequest::new(self.as_ref(), resource) })
+ }
+
+ /// Returns an `IoRequest` for the resource with a given `name`, if any.
+ pub fn io_request_by_name(&self, name: &CStr) -> Option<IoRequest<'_>> {
+ // SAFETY: `resource` is a valid resource for `&self` during the
+ // lifetime of the `IoRequest`.
+ self.resource_by_name(name)
+ .map(|resource| unsafe { IoRequest::new(self.as_ref(), resource) })
+ }
}
// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
--
2.50.0
^ permalink raw reply related [flat|nested] 10+ messages in thread