* [PATCH v8 1/3] rust: io: add resource abstraction
2025-05-09 20:29 [PATCH v8 0/3] rust: platform: add Io support Daniel Almeida
@ 2025-05-09 20:29 ` Daniel Almeida
2025-05-15 15:53 ` Danilo Krummrich
2025-05-09 20:29 ` [PATCH v8 2/3] rust: io: mem: add a generic iomem abstraction Daniel Almeida
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Daniel Almeida @ 2025-05-09 20:29 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, Andrew Morton, Andy Shevchenko,
Ilpo Järvinen, Bjorn Helgaas, Mika Westerberg, Ying Huang
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 | 2 +
rust/kernel/io/resource.rs | 222 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 261 insertions(+)
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index ab37e1d35c70d52e69b754bf855bc19911d156d8..b0c1a7ec3ef6f64834dc0e8c07931b7d40c975a9 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -19,6 +19,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 4c2401ccd72078af4e6901f4cd95f9070522f396..939ab38ca61dac4cf884677a20edc760094d5812 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, volatile 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..1da447078633e058000953a581b59d3ed5eb0e69 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -7,6 +7,8 @@
use crate::error::{code::EINVAL, Result};
use crate::{bindings, build_assert};
+pub mod 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..3eb5c8ef585078398551fe8189fd96c1b6c1eeff
--- /dev/null
+++ b/rust/kernel/io/resource.rs
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Abstraction for system resources.
+//!
+//! C header: [`include/linux/ioport.h`](srctree/include/linux/ioport.h)
+
+use core::ops::Deref;
+use core::ptr::NonNull;
+
+use crate::str::CStr;
+use crate::types::Opaque;
+
+#[cfg(CONFIG_HAS_IOPORT)]
+/// Returns a reference to the global `ioport_resource` variable.
+pub fn ioport_resource() -> &'static Resource {
+ // SAFETY: `bindings::ioport_resoure` has global lifetime and is of type Resource.
+ unsafe { Resource::from_ptr(core::ptr::addr_of_mut!(bindings::ioport_resource)) }
+}
+
+#[cfg(CONFIG_HAS_IOMEM)]
+/// Returns a reference to the global `iomem_resource` variable.
+pub fn iomem_resource() -> &'static Resource {
+ // SAFETY: `bindings::iomem_resoure` has global lifetime and is of type Resource.
+ unsafe { Resource::from_ptr(core::ptr::addr_of_mut!(bindings::iomem_resource)) }
+}
+
+/// Resource Size type.
+///
+/// This is a type alias to `u64` depending on the config option
+/// `CONFIG_PHYS_ADDR_T_64BIT`.
+#[cfg(CONFIG_PHYS_ADDR_T_64BIT)]
+pub type ResourceSize = u64;
+
+/// Resource Size type.
+///
+/// This is a type alias to `u32` depending on the config option
+/// `CONFIG_PHYS_ADDR_T_64BIT`.
+#[cfg(not(CONFIG_PHYS_ADDR_T_64BIT))]
+pub type ResourceSize = u32;
+
+/// A region allocated from a parent resource.
+///
+/// # Invariants
+///
+/// - `self.0` points to a valid `bindings::resource` that was obtained through
+/// `__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_ptr(self.0.as_ptr()) }
+ }
+}
+
+impl Drop for Region {
+ fn drop(&mut self) {
+ // SAFETY: Safe as per the invariant of `Region`
+ let res = unsafe { Resource::from_ptr(self.0.as_ptr()) };
+ let flags = res.flags();
+
+ 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(res.start(), res.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_ptr<'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 `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`
+ let region = unsafe {
+ bindings::__request_region(
+ self.0.get(),
+ start,
+ size,
+ name.as_char_ptr(),
+ flags.0 as i32,
+ )
+ };
+
+ 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 thead.
+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 thead.
+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(usize);
+
+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)
+ }
+}
+
+/// Resource flags as stored in the `struct resource::flags` field.
+pub mod flags {
+ use super::Flags;
+
+ /// PCI/ISA I/O ports.
+ pub const IORESOURCE_IO: Flags = Flags(bindings::IORESOURCE_IO as usize);
+
+ /// Resource is software muxed.
+ pub const IORESOURCE_MUXED: Flags = Flags(bindings::IORESOURCE_MUXED as usize);
+
+ /// Resource represents a memory region.
+ pub const IORESOURCE_MEM: Flags = Flags(bindings::IORESOURCE_MEM as usize);
+
+ /// Resource represents a memory region that must be ioremaped using `ioremap_np`.
+ pub const IORESOURCE_MEM_NONPOSTED: Flags = Flags(bindings::IORESOURCE_MEM_NONPOSTED as usize);
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v8 1/3] rust: io: add resource abstraction
2025-05-09 20:29 ` [PATCH v8 1/3] rust: io: add resource abstraction Daniel Almeida
@ 2025-05-15 15:53 ` Danilo Krummrich
0 siblings, 0 replies; 11+ messages in thread
From: Danilo Krummrich @ 2025-05-15 15:53 UTC (permalink / raw)
To: Daniel Almeida
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Greg Kroah-Hartman, Rafael J. Wysocki,
Andrew Morton, Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
Mika Westerberg, Ying Huang, linux-kernel, rust-for-linux,
Fiona Behrens
On Fri, May 09, 2025 at 05:29:46PM -0300, Daniel Almeida wrote:
> +#[cfg(CONFIG_HAS_IOPORT)]
> +/// Returns a reference to the global `ioport_resource` variable.
> +pub fn ioport_resource() -> &'static Resource {
> + // SAFETY: `bindings::ioport_resoure` has global lifetime and is of type Resource.
> + unsafe { Resource::from_ptr(core::ptr::addr_of_mut!(bindings::ioport_resource)) }
Should be possible to use `&raw mut`.
> +}
> +
> +#[cfg(CONFIG_HAS_IOMEM)]
> +/// Returns a reference to the global `iomem_resource` variable.
> +pub fn iomem_resource() -> &'static Resource {
> + // SAFETY: `bindings::iomem_resoure` has global lifetime and is of type Resource.
> + unsafe { Resource::from_ptr(core::ptr::addr_of_mut!(bindings::iomem_resource)) }
Same here.
> +/// 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_ptr<'a>(ptr: *mut bindings::resource) -> &'a Self {
Throughout the tree, functions with this signature are usually called as_ref().
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v8 2/3] rust: io: mem: add a generic iomem abstraction
2025-05-09 20:29 [PATCH v8 0/3] rust: platform: add Io support Daniel Almeida
2025-05-09 20:29 ` [PATCH v8 1/3] rust: io: add resource abstraction Daniel Almeida
@ 2025-05-09 20:29 ` Daniel Almeida
2025-05-15 16:01 ` Danilo Krummrich
2025-05-09 20:29 ` [PATCH v8 3/3] rust: platform: allow ioremap of platform resources Daniel Almeida
2025-05-15 15:49 ` [PATCH v8 0/3] rust: platform: add Io support Danilo Krummrich
3 siblings, 1 reply; 11+ messages in thread
From: Daniel Almeida @ 2025-05-09 20:29 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, Andrew Morton, Andy Shevchenko,
Ilpo Järvinen, Bjorn Helgaas, Mika Westerberg, Ying Huang
Cc: linux-kernel, rust-for-linux, Daniel Almeida
Add a generic iomem abstraction to safely read and write ioremapped
regions.
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 | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 147 insertions(+)
diff --git a/rust/helpers/io.c b/rust/helpers/io.c
index 939ab38ca61dac4cf884677a20edc760094d5812..4fbd70ab60f64155bef835a43b3c64d441aee7bf 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(volatile void __iomem *addr)
{
iounmap(addr);
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 1da447078633e058000953a581b59d3ed5eb0e69..0249aedd8c6ce435b6b00137be0895a206956bca 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;
/// Raw representation of an MMIO region.
diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs
new file mode 100644
index 0000000000000000000000000000000000000000..3e7ef8b6f0ca8b5b195a94c7ed0f94a9e6c72944
--- /dev/null
+++ b/rust/kernel/io/mem.rs
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Generic memory-mapped IO.
+
+use core::ops::Deref;
+
+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 exclusive memory-mapped IO region.
+///
+/// # Invariants
+///
+/// - [`ExclusiveIoMem`] has exclusive access to the underlying `iomem`.
+pub struct ExclusiveIoMem<const SIZE: usize> {
+ /// The region abstraction. This represents exclusive access to the
+ /// range represented by the underlying `iomem`.
+ ///
+ /// It's placed first to ensure that the region is released before it is
+ /// unmapped as a result of the drop order.
+ ///
+ /// This field is needed for ownership of the region.
+ _region: Region,
+ /// The underlying `IoMem` instance.
+ iomem: IoMem<SIZE>,
+}
+
+impl<const SIZE: usize> ExclusiveIoMem<SIZE> {
+ /// Creates a new `ExclusiveIoMem` instance.
+ pub(crate) fn ioremap(resource: &Resource) -> Result<Self> {
+ let iomem = IoMem::ioremap(resource)?;
+
+ 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 = ExclusiveIoMem {
+ iomem,
+ _region: region,
+ };
+
+ Ok(iomem)
+ }
+
+ pub(crate) fn new(resource: &Resource, device: &Device) -> Result<Devres<Self>> {
+ let iomem = Self::ioremap(resource)?;
+ let devres = Devres::new(device, iomem, GFP_KERNEL)?;
+
+ Ok(devres)
+ }
+}
+
+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 as usize) }
+ } 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 as usize) }
+ };
+
+ if addr.is_null() {
+ return Err(ENOMEM);
+ }
+
+ let io = IoRaw::new(addr as usize, size as usize)?;
+ let io = IoMem { io };
+
+ Ok(io)
+ }
+
+ /// Creates a new `IoMem` instance.
+ pub(crate) fn new(resource: &Resource, device: &Device) -> Result<Devres<Self>> {
+ let io = Self::ioremap(resource)?;
+ let devres = Devres::new(device, io, GFP_KERNEL)?;
+
+ Ok(devres)
+ }
+}
+
+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 core::ffi::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.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v8 2/3] rust: io: mem: add a generic iomem abstraction
2025-05-09 20:29 ` [PATCH v8 2/3] rust: io: mem: add a generic iomem abstraction Daniel Almeida
@ 2025-05-15 16:01 ` Danilo Krummrich
0 siblings, 0 replies; 11+ messages in thread
From: Danilo Krummrich @ 2025-05-15 16:01 UTC (permalink / raw)
To: Daniel Almeida
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Greg Kroah-Hartman, Rafael J. Wysocki,
Andrew Morton, Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
Mika Westerberg, Ying Huang, linux-kernel, rust-for-linux
On Fri, May 09, 2025 at 05:29:47PM -0300, Daniel Almeida wrote:
> +impl<const SIZE: usize> ExclusiveIoMem<SIZE> {
> + /// Creates a new `ExclusiveIoMem` instance.
> + pub(crate) fn ioremap(resource: &Resource) -> Result<Self> {
> + let iomem = IoMem::ioremap(resource)?;
> +
> + 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 = ExclusiveIoMem {
> + iomem,
> + _region: region,
> + };
> +
> + Ok(iomem)
> + }
> +
> + pub(crate) fn new(resource: &Resource, device: &Device) -> Result<Devres<Self>> {
> + let iomem = Self::ioremap(resource)?;
> + let devres = Devres::new(device, iomem, GFP_KERNEL)?;
Here and in IoMem, Devres::new() requires a &Device<Bound>.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v8 3/3] rust: platform: allow ioremap of platform resources
2025-05-09 20:29 [PATCH v8 0/3] rust: platform: add Io support Daniel Almeida
2025-05-09 20:29 ` [PATCH v8 1/3] rust: io: add resource abstraction Daniel Almeida
2025-05-09 20:29 ` [PATCH v8 2/3] rust: io: mem: add a generic iomem abstraction Daniel Almeida
@ 2025-05-09 20:29 ` Daniel Almeida
2025-05-15 16:23 ` Danilo Krummrich
2025-05-16 8:39 ` Danilo Krummrich
2025-05-15 15:49 ` [PATCH v8 0/3] rust: platform: add Io support Danilo Krummrich
3 siblings, 2 replies; 11+ messages in thread
From: Daniel Almeida @ 2025-05-09 20:29 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, Andrew Morton, Andy Shevchenko,
Ilpo Järvinen, Bjorn Helgaas, Mika Westerberg, Ying Huang
Cc: linux-kernel, rust-for-linux, Daniel Almeida
The preceding patches added support for resources, and for a general
IoMem abstraction, but thus far there is no way to access said IoMem
from drivers, as its creation is unsafe and depends on a resource that
must be acquired from some device first.
Now, allow the ioremap of platform resources themselves, thereby making
the IoMem available to platform drivers.
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
rust/kernel/platform.rs | 128 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 127 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 4917cb34e2fe8027d3d861e90de51de85f006735..5c550fc6d429ffc541a17fb5f8a1c2eb4476b560 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -5,8 +5,14 @@
//! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h)
use crate::{
- bindings, device, driver,
+ bindings, device,
+ devres::Devres,
+ driver,
error::{to_result, Result},
+ io::{
+ mem::{ExclusiveIoMem, IoMem},
+ resource::Resource,
+ },
of,
prelude::*,
str::CStr,
@@ -223,6 +229,126 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
}
}
+impl Device<device::Core> {
+ /// Maps a platform resource through ioremap() where the size is known at
+ /// compile time.
+ ///
+ /// # Examples
+ ///
+ /// ```no_run
+ /// # use kernel::{bindings, c_str, platform};
+ /// # use kernel::device::Core;
+ ///
+ ///
+ /// fn probe(pdev: &mut platform::Device<Core>, /* ... */) -> Result<()> {
+ /// let offset = 0; // Some offset.
+ ///
+ /// // If the size is known at compile time, use `ioremap_resource_sized`.
+ /// // No runtime checks will apply when reading and writing.
+ /// let resource = pdev.resource(0).ok_or(ENODEV)?;
+ /// let iomem = pdev.ioremap_resource_sized::<42>(&resource)?;
+ ///
+ /// // Read and write a 32-bit value at `offset`. Calling `try_access()` on
+ /// // the `Devres` makes sure that the resource is still valid.
+ /// let data = iomem.try_access().ok_or(ENODEV)?.read32_relaxed(offset);
+ ///
+ /// iomem.try_access().ok_or(ENODEV)?.write32_relaxed(data, offset);
+ ///
+ /// # Ok::<(), Error>(())
+ /// }
+ /// ```
+ pub fn ioremap_resource_sized<const SIZE: usize>(
+ &self,
+ resource: &Resource,
+ ) -> Result<Devres<IoMem<SIZE>>> {
+ IoMem::new(resource, self.as_ref())
+ }
+
+ /// Same as [`Self::ioremap_resource_sized`] but with exclusive access to the
+ /// underlying region.
+ pub fn ioremap_resource_exclusive_sized<const SIZE: usize>(
+ &self,
+ resource: &Resource,
+ ) -> Result<Devres<ExclusiveIoMem<SIZE>>> {
+ ExclusiveIoMem::new(resource, self.as_ref())
+ }
+
+ /// Maps a platform resource through ioremap().
+ ///
+ /// # Examples
+ ///
+ /// ```no_run
+ /// # use kernel::{bindings, c_str, platform};
+ /// # use kernel::device::Core;
+ ///
+ /// fn probe(pdev: &mut platform::Device<Core>, /* ... */) -> Result<()> {
+ /// let offset = 0; // Some offset.
+ ///
+ /// // Unlike `ioremap_resource_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 resource = pdev.resource(0).ok_or(ENODEV)?;
+ /// let iomem = pdev.ioremap_resource(&resource)?;
+ ///
+ /// let data = iomem.try_access().ok_or(ENODEV)?.try_read32_relaxed(offset)?;
+ ///
+ /// iomem.try_access().ok_or(ENODEV)?.try_write32_relaxed(data, offset)?;
+ ///
+ /// # Ok::<(), Error>(())
+ /// }
+ /// ```
+ pub fn ioremap_resource(&self, resource: &Resource) -> Result<Devres<IoMem<0>>> {
+ self.ioremap_resource_sized::<0>(resource)
+ }
+
+ /// Same as [`Self::ioremap_resource`] but with exclusive access to the underlying
+ /// region.
+ pub fn ioremap_resource_exclusive(
+ &self,
+ resource: &Resource,
+ ) -> Result<Devres<ExclusiveIoMem<0>>> {
+ self.ioremap_resource_exclusive_sized::<0>(resource)
+ }
+
+ /// Returns the resource at `index`, if any.
+ pub fn resource(&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_ptr(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_ptr(resource) })
+ }
+}
+
impl AsRef<device::Device> for Device {
fn as_ref(&self) -> &device::Device {
// SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v8 3/3] rust: platform: allow ioremap of platform resources
2025-05-09 20:29 ` [PATCH v8 3/3] rust: platform: allow ioremap of platform resources Daniel Almeida
@ 2025-05-15 16:23 ` Danilo Krummrich
2025-05-28 17:29 ` Daniel Almeida
2025-05-16 8:39 ` Danilo Krummrich
1 sibling, 1 reply; 11+ messages in thread
From: Danilo Krummrich @ 2025-05-15 16:23 UTC (permalink / raw)
To: Daniel Almeida
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Greg Kroah-Hartman, Rafael J. Wysocki,
Andrew Morton, Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
Mika Westerberg, Ying Huang, linux-kernel, rust-for-linux
On Fri, May 09, 2025 at 05:29:48PM -0300, Daniel Almeida wrote:
> +impl Device<device::Core> {
> + /// Maps a platform resource through ioremap() where the size is known at
> + /// compile time.
> + ///
> + /// # Examples
> + ///
> + /// ```no_run
> + /// # use kernel::{bindings, c_str, platform};
> + /// # use kernel::device::Core;
> + ///
> + ///
> + /// fn probe(pdev: &mut platform::Device<Core>, /* ... */) -> Result<()> {
Should be &platform::Device<Core> (i.e. not mutable). You should also be able to
just use `Result` as return type. Though, it would probably be better to use the
real probe() function here, i.e.
# struct Driver;
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 `ioremap_resource_sized`.
> + /// // No runtime checks will apply when reading and writing.
> + /// let resource = pdev.resource(0).ok_or(ENODEV)?;
> + /// let iomem = pdev.ioremap_resource_sized::<42>(&resource)?;
> + ///
> + /// // Read and write a 32-bit value at `offset`. Calling `try_access()` on
> + /// // the `Devres` makes sure that the resource is still valid.
> + /// let data = iomem.try_access().ok_or(ENODEV)?.read32_relaxed(offset);
> + ///
> + /// iomem.try_access().ok_or(ENODEV)?.write32_relaxed(data, offset);
Since this won't land for v6.16, can you please use Devres::access() [1]
instead? I.e.
let iomem = pdev.ioremap_resource_sized::<42>(&resource)?;
let io = Devres::access(pdev.as_ref())?;
let data = io.read32_relaxed(offset);
io.write32_relaxed(data, offset);
Devres::access() is in nova-next and lands in v6.16.
[1] https://gitlab.freedesktop.org/drm/nova/-/commit/f301cb978c068faa8fcd630be2cb317a2d0ec063
> + ///
> + /// # Ok::<(), Error>(())
> + /// }
> + /// ```
> + pub fn ioremap_resource_sized<const SIZE: usize>(
> + &self,
> + resource: &Resource,
> + ) -> Result<Devres<IoMem<SIZE>>> {
> + IoMem::new(resource, self.as_ref())
> + }
> +
> + /// Same as [`Self::ioremap_resource_sized`] but with exclusive access to the
> + /// underlying region.
> + pub fn ioremap_resource_exclusive_sized<const SIZE: usize>(
> + &self,
> + resource: &Resource,
> + ) -> Result<Devres<ExclusiveIoMem<SIZE>>> {
> + ExclusiveIoMem::new(resource, self.as_ref())
> + }
> +
> + /// Maps a platform resource through ioremap().
> + ///
> + /// # Examples
> + ///
> + /// ```no_run
> + /// # use kernel::{bindings, c_str, platform};
> + /// # use kernel::device::Core;
> + ///
> + /// fn probe(pdev: &mut platform::Device<Core>, /* ... */) -> Result<()> {
> + /// let offset = 0; // Some offset.
> + ///
> + /// // Unlike `ioremap_resource_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 resource = pdev.resource(0).ok_or(ENODEV)?;
> + /// let iomem = pdev.ioremap_resource(&resource)?;
> + ///
> + /// let data = iomem.try_access().ok_or(ENODEV)?.try_read32_relaxed(offset)?;
> + ///
> + /// iomem.try_access().ok_or(ENODEV)?.try_write32_relaxed(data, offset)?;
> + ///
> + /// # Ok::<(), Error>(())
> + /// }
Same as above.
> + /// ```
> + pub fn ioremap_resource(&self, resource: &Resource) -> Result<Devres<IoMem<0>>> {
> + self.ioremap_resource_sized::<0>(resource)
> + }
> +
> + /// Same as [`Self::ioremap_resource`] but with exclusive access to the underlying
> + /// region.
> + pub fn ioremap_resource_exclusive(
> + &self,
> + resource: &Resource,
> + ) -> Result<Devres<ExclusiveIoMem<0>>> {
> + self.ioremap_resource_exclusive_sized::<0>(resource)
> + }
> +
> + /// Returns the resource at `index`, if any.
> + pub fn resource(&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_ptr(resource) })
> + }
> +
> + /// Returns the resource with a given `name`, if any.
> + pub fn resource_by_name(&self, name: &CStr) -> Option<&Resource> {
This method should be a separate patch, no? Also, I think this one can go into
the `impl<Ctx: device::DeviceContext> Device<Ctx>` block, since it should be
valid to call from any device context.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v8 3/3] rust: platform: allow ioremap of platform resources
2025-05-15 16:23 ` Danilo Krummrich
@ 2025-05-28 17:29 ` Daniel Almeida
2025-05-28 18:06 ` Danilo Krummrich
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Almeida @ 2025-05-28 17:29 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Greg Kroah-Hartman, Rafael J. Wysocki,
Andrew Morton, Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
Mika Westerberg, Ying Huang, linux-kernel, rust-for-linux
Hi Danilo,
[…]
>
>> + /// let offset = 0; // Some offset.
>> + ///
>> + /// // If the size is known at compile time, use `ioremap_resource_sized`.
>> + /// // No runtime checks will apply when reading and writing.
>> + /// let resource = pdev.resource(0).ok_or(ENODEV)?;
>> + /// let iomem = pdev.ioremap_resource_sized::<42>(&resource)?;
>> + ///
>> + /// // Read and write a 32-bit value at `offset`. Calling `try_access()` on
>> + /// // the `Devres` makes sure that the resource is still valid.
>> + /// let data = iomem.try_access().ok_or(ENODEV)?.read32_relaxed(offset);
>> + ///
>> + /// iomem.try_access().ok_or(ENODEV)?.write32_relaxed(data, offset);
>
> Since this won't land for v6.16, can you please use Devres::access() [1]
> instead? I.e.
>
> let iomem = pdev.ioremap_resource_sized::<42>(&resource)?;
> let io = Devres::access(pdev.as_ref())?;
>
> let data = io.read32_relaxed(offset);
> io.write32_relaxed(data, offset);
>
> Devres::access() is in nova-next and lands in v6.16.
>
> [1] https://gitlab.freedesktop.org/drm/nova/-/commit/f301cb978c068faa8fcd630be2cb317a2d0ec063
Devres:access takes &Device<Bound>, but the argument in probe() is
&Device<Core>.
Are these two types supposed to convert between them? I see no explicit
function to do so.
— Daniel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 3/3] rust: platform: allow ioremap of platform resources
2025-05-28 17:29 ` Daniel Almeida
@ 2025-05-28 18:06 ` Danilo Krummrich
0 siblings, 0 replies; 11+ messages in thread
From: Danilo Krummrich @ 2025-05-28 18:06 UTC (permalink / raw)
To: Daniel Almeida
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Greg Kroah-Hartman, Rafael J. Wysocki,
Andrew Morton, Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
Mika Westerberg, Ying Huang, linux-kernel, rust-for-linux
On Wed, May 28, 2025 at 02:29:44PM -0300, Daniel Almeida wrote:
> Hi Danilo,
>
> […]
>
> >
> >> + /// let offset = 0; // Some offset.
> >> + ///
> >> + /// // If the size is known at compile time, use `ioremap_resource_sized`.
> >> + /// // No runtime checks will apply when reading and writing.
> >> + /// let resource = pdev.resource(0).ok_or(ENODEV)?;
> >> + /// let iomem = pdev.ioremap_resource_sized::<42>(&resource)?;
> >> + ///
> >> + /// // Read and write a 32-bit value at `offset`. Calling `try_access()` on
> >> + /// // the `Devres` makes sure that the resource is still valid.
> >> + /// let data = iomem.try_access().ok_or(ENODEV)?.read32_relaxed(offset);
> >> + ///
> >> + /// iomem.try_access().ok_or(ENODEV)?.write32_relaxed(data, offset);
> >
> > Since this won't land for v6.16, can you please use Devres::access() [1]
> > instead? I.e.
> >
> > let iomem = pdev.ioremap_resource_sized::<42>(&resource)?;
> > let io = Devres::access(pdev.as_ref())?;
> >
> > let data = io.read32_relaxed(offset);
> > io.write32_relaxed(data, offset);
> >
> > Devres::access() is in nova-next and lands in v6.16.
> >
> > [1] https://gitlab.freedesktop.org/drm/nova/-/commit/f301cb978c068faa8fcd630be2cb317a2d0ec063
>
> Devres:access takes &Device<Bound>, but the argument in probe() is
> &Device<Core>.
>
> Are these two types supposed to convert between them? I see no explicit
> function to do so.
Yes, it comes from impl_device_context_deref!() [1], which, as the name implies,
implements the corresponding Deref traits.
Device dereference in the following way:
&Device<Core> -> &Device<Bound> -> &Device (i.e. &Device<Normal>)
You can just pass in the &Device<Core>, it will work.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git/tree/rust/kernel/device.rs?h=driver-core-next#n284
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 3/3] rust: platform: allow ioremap of platform resources
2025-05-09 20:29 ` [PATCH v8 3/3] rust: platform: allow ioremap of platform resources Daniel Almeida
2025-05-15 16:23 ` Danilo Krummrich
@ 2025-05-16 8:39 ` Danilo Krummrich
1 sibling, 0 replies; 11+ messages in thread
From: Danilo Krummrich @ 2025-05-16 8:39 UTC (permalink / raw)
To: Daniel Almeida
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Greg Kroah-Hartman, Rafael J. Wysocki,
Andrew Morton, Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
Mika Westerberg, Ying Huang, linux-kernel, rust-for-linux
On Fri, May 09, 2025 at 05:29:48PM -0300, Daniel Almeida wrote:
> +impl Device<device::Core> {
For the ioremap_*() ones, `impl Device<device::Bound>` should be enough.
Also, PCI names those just iomap_*(), maybe we should align those names for
consistency.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 0/3] rust: platform: add Io support
2025-05-09 20:29 [PATCH v8 0/3] rust: platform: add Io support Daniel Almeida
` (2 preceding siblings ...)
2025-05-09 20:29 ` [PATCH v8 3/3] rust: platform: allow ioremap of platform resources Daniel Almeida
@ 2025-05-15 15:49 ` Danilo Krummrich
3 siblings, 0 replies; 11+ messages in thread
From: Danilo Krummrich @ 2025-05-15 15:49 UTC (permalink / raw)
To: Daniel Almeida
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Greg Kroah-Hartman, Rafael J. Wysocki,
Andrew Morton, Andy Shevchenko, Ilpo Järvinen, Bjorn Helgaas,
Mika Westerberg, Ying Huang, linux-kernel, rust-for-linux,
Fiona Behrens
On Fri, May 09, 2025 at 05:29:45PM -0300, Daniel Almeida wrote:
> Changes in v8:
> - Rebased on driver-core-next
Something went wrong I suppose. The patch series does not apply on top of
current driver-core-next and from the patches I can see that if it's based on
it, it must be driver-core-next from at least before 19th April.
^ permalink raw reply [flat|nested] 11+ messages in thread