* [PATCH] rust: io: use newtype for PhysAddr
@ 2026-05-03 10:30 x
2026-05-03 10:43 ` Greg Kroah-Hartman
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: x @ 2026-05-03 10:30 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J . Wysocki, Danilo Krummrich,
Miguel Ojeda, Daniel Almeida, Joerg Roedel, Will Deacon
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Robin Murphy,
driver-core, rust-for-linux, linux-kernel, iommu
From: Favilances Noir <favilances@proton.me>
From: Favilances Noir <favilances@proton.me>
Currently, `PhysAddr` is a type alias for `phys_addr_t`. This means
that physical addresses can be mixed with unrelated integer values
without Rust noticing.
Use a transparent newtype for `PhysAddr` instead, and update the
affected call sites to perform explicit raw conversions at the FFI
boundary.
This keeps the existing `phys_addr_t` representation while making
physical addresses distinct in Rust type checking.
Signed-off-by: Favilances Noir <favilances@proton.me>
---
rust/kernel/devres.rs | 8 +++++---
rust/kernel/io.rs | 30 ++++++++++++++++++++++++------
rust/kernel/io/mem.rs | 4 ++--
rust/kernel/io/resource.rs | 6 +++---
rust/kernel/iommu/pgtable.rs | 2 +-
5 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 9e5f93aed20c..beb3723d2519 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -85,10 +85,10 @@ struct Inner<T> {
/// ///
/// /// [`paddr`, `paddr` + `SIZE`) must be a valid MMIO region that is mappable into the CPUs
/// /// virtual address space.
-/// unsafe fn new(paddr: usize) -> Result<Self>{
+/// unsafe fn new(paddr: PhysAddr) -> Result<Self>{
/// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
/// // valid for `ioremap`.
-/// let addr = unsafe { bindings::ioremap(paddr as PhysAddr, SIZE) };
+/// let addr = unsafe { bindings::ioremap(paddr.into_raw(), SIZE) };
/// if addr.is_null() {
/// return Err(ENOMEM);
/// }
@@ -114,7 +114,9 @@ struct Inner<T> {
/// }
/// # fn no_run(dev: &Device<Bound>) -> Result<(), Error> {
/// // SAFETY: Invalid usage for example purposes.
-/// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
+/// let iomem = unsafe {
+/// IoMem::<{ core::mem::size_of::<u32>() }>::new(PhysAddr::from_raw(0xBAAAAAAD))?
+/// };
/// let devres = Devres::new(dev, iomem)?;
///
/// let res = devres.try_access().ok_or(ENXIO)?;
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index fcc7678fd9e3..2d55ee93caae 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -21,9 +21,25 @@
/// Physical address 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 PhysAddr = bindings::phys_addr_t;
+/// This wraps 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.
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct PhysAddr(bindings::phys_addr_t);
+
+impl PhysAddr {
+ /// Creates a physical address from the raw C type.
+ #[inline]
+ pub const fn from_raw(value: bindings::phys_addr_t) -> Self {
+ Self(value)
+ }
+
+ /// Turns this physical address into the raw C type.
+ #[inline]
+ pub const fn into_raw(self) -> bindings::phys_addr_t {
+ self.0
+ }
+}
/// Resource Size type.
///
@@ -101,10 +117,10 @@ pub fn maxsize(&self) -> usize {
/// ///
/// /// [`paddr`, `paddr` + `SIZE`) must be a valid MMIO region that is mappable into the CPUs
/// /// virtual address space.
-/// unsafe fn new(paddr: usize) -> Result<Self>{
+/// unsafe fn new(paddr: PhysAddr) -> Result<Self>{
/// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
/// // valid for `ioremap`.
-/// let addr = unsafe { bindings::ioremap(paddr as PhysAddr, SIZE) };
+/// let addr = unsafe { bindings::ioremap(paddr.into_raw(), SIZE) };
/// if addr.is_null() {
/// return Err(ENOMEM);
/// }
@@ -131,7 +147,9 @@ pub fn maxsize(&self) -> usize {
///
///# fn no_run() -> Result<(), Error> {
/// // SAFETY: Invalid usage for example purposes.
-/// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
+/// let iomem = unsafe {
+/// IoMem::<{ core::mem::size_of::<u32>() }>::new(PhysAddr::from_raw(0xBAAAAAAD))?
+/// };
/// iomem.write32(0x42, 0x0);
/// assert!(iomem.try_write32(0x42, 0x0).is_ok());
/// assert!(iomem.try_write32(0x42, 0x4).is_err());
diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs
index 7dc78d547f7a..f21f9038b297 100644
--- a/rust/kernel/io/mem.rs
+++ b/rust/kernel/io/mem.rs
@@ -254,12 +254,12 @@ fn ioremap(resource: &Resource) -> Result<Self> {
// 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) }
+ unsafe { bindings::ioremap_np(res_start.into_raw(), size) }
} 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) }
+ unsafe { bindings::ioremap(res_start.into_raw(), size) }
};
if addr.is_null() {
diff --git a/rust/kernel/io/resource.rs b/rust/kernel/io/resource.rs
index b7ac9faf141d..5dda1061db68 100644
--- a/rust/kernel/io/resource.rs
+++ b/rust/kernel/io/resource.rs
@@ -58,7 +58,7 @@ fn drop(&mut self) {
};
// SAFETY: Safe as per the invariant of `Region`.
- unsafe { release_fn(start, size) };
+ unsafe { release_fn(start.into_raw(), size) };
}
}
@@ -113,7 +113,7 @@ pub fn request_region(
let region = unsafe {
bindings::__request_region(
self.0.get(),
- start,
+ start.into_raw(),
size,
name.as_char_ptr(),
flags.0 as c_int,
@@ -137,7 +137,7 @@ pub fn size(&self) -> ResourceSize {
pub fn start(&self) -> PhysAddr {
let inner = self.0.get();
// SAFETY: Safe as per the invariants of `Resource`.
- unsafe { (*inner).start }
+ PhysAddr::from_raw(unsafe { (*inner).start })
}
/// Returns the name of the resource.
diff --git a/rust/kernel/iommu/pgtable.rs b/rust/kernel/iommu/pgtable.rs
index c88e38fd938a..ae724ca1e8ad 100644
--- a/rust/kernel/iommu/pgtable.rs
+++ b/rust/kernel/iommu/pgtable.rs
@@ -182,7 +182,7 @@ pub unsafe fn map_pages(
(map_pages)(
self.raw_ops(),
iova,
- paddr,
+ paddr.into_raw(),
pgsize,
pgcount,
prot as i32,
--
2.54.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] rust: io: use newtype for PhysAddr
2026-05-03 10:30 [PATCH] rust: io: use newtype for PhysAddr x
@ 2026-05-03 10:43 ` Greg Kroah-Hartman
2026-05-03 18:28 ` Onur Özkan
2026-05-04 11:14 ` [PATCH v2] " x
2 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2026-05-03 10:43 UTC (permalink / raw)
To: x
Cc: Rafael J . Wysocki, Danilo Krummrich, Miguel Ojeda,
Daniel Almeida, Joerg Roedel, Will Deacon, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Robin Murphy, driver-core, rust-for-linux,
linux-kernel, iommu
On Sun, May 03, 2026 at 01:30:50PM +0300, x@2005.tr wrote:
> From: Favilances Noir <favilances@proton.me>
>
> From: Favilances Noir <favilances@proton.me>
Odd, this showed up twice?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] rust: io: use newtype for PhysAddr
2026-05-03 10:30 [PATCH] rust: io: use newtype for PhysAddr x
2026-05-03 10:43 ` Greg Kroah-Hartman
@ 2026-05-03 18:28 ` Onur Özkan
2026-05-03 18:45 ` Danilo Krummrich
2026-05-04 11:14 ` [PATCH v2] " x
2 siblings, 1 reply; 7+ messages in thread
From: Onur Özkan @ 2026-05-03 18:28 UTC (permalink / raw)
To: x
Cc: Greg Kroah-Hartman, Rafael J . Wysocki, Danilo Krummrich,
Miguel Ojeda, Daniel Almeida, Joerg Roedel, Will Deacon,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Robin Murphy,
driver-core, rust-for-linux, linux-kernel, iommu
On Sun, 03 May 2026 13:30:50 +0300
x@2005.tr wrote:
> From: Favilances Noir <favilances@proton.me>
>
> From: Favilances Noir <favilances@proton.me>
>
> Currently, `PhysAddr` is a type alias for `phys_addr_t`. This means
> that physical addresses can be mixed with unrelated integer values
> without Rust noticing.
>
> Use a transparent newtype for `PhysAddr` instead, and update the
> affected call sites to perform explicit raw conversions at the FFI
> boundary.
>
> This keeps the existing `phys_addr_t` representation while making
> physical addresses distinct in Rust type checking.
>
> Signed-off-by: Favilances Noir <favilances@proton.me>
> ---
> rust/kernel/devres.rs | 8 +++++---
> rust/kernel/io.rs | 30 ++++++++++++++++++++++++------
> rust/kernel/io/mem.rs | 4 ++--
> rust/kernel/io/resource.rs | 6 +++---
> rust/kernel/iommu/pgtable.rs | 2 +-
> 5 files changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> index 9e5f93aed20c..beb3723d2519 100644
> --- a/rust/kernel/devres.rs
> +++ b/rust/kernel/devres.rs
> @@ -85,10 +85,10 @@ struct Inner<T> {
> /// ///
> /// /// [`paddr`, `paddr` + `SIZE`) must be a valid MMIO region that is mappable into the CPUs
> /// /// virtual address space.
> -/// unsafe fn new(paddr: usize) -> Result<Self>{
> +/// unsafe fn new(paddr: PhysAddr) -> Result<Self>{
> /// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
> /// // valid for `ioremap`.
> -/// let addr = unsafe { bindings::ioremap(paddr as PhysAddr, SIZE) };
> +/// let addr = unsafe { bindings::ioremap(paddr.into_raw(), SIZE) };
> /// if addr.is_null() {
> /// return Err(ENOMEM);
> /// }
> @@ -114,7 +114,9 @@ struct Inner<T> {
> /// }
> /// # fn no_run(dev: &Device<Bound>) -> Result<(), Error> {
> /// // SAFETY: Invalid usage for example purposes.
> -/// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
> +/// let iomem = unsafe {
> +/// IoMem::<{ core::mem::size_of::<u32>() }>::new(PhysAddr::from_raw(0xBAAAAAAD))?
> +/// };
> /// let devres = Devres::new(dev, iomem)?;
> ///
> /// let res = devres.try_access().ok_or(ENXIO)?;
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index fcc7678fd9e3..2d55ee93caae 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -21,9 +21,25 @@
>
> /// Physical address 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 PhysAddr = bindings::phys_addr_t;
> +/// This wraps 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.
> +#[repr(transparent)]
> +#[derive(Clone, Copy)]
Copy and Clone seem never used, please drop them (also see [1]).
[1]: https://rust-for-linux.zulipchat.com/#narrow/channel/509436-Nova/topic/clone.2Fcopy.20additions
Regards,
Onur
> +pub struct PhysAddr(bindings::phys_addr_t);
> +
> +impl PhysAddr {
> + /// Creates a physical address from the raw C type.
> + #[inline]
> + pub const fn from_raw(value: bindings::phys_addr_t) -> Self {
> + Self(value)
> + }
> +
> + /// Turns this physical address into the raw C type.
> + #[inline]
> + pub const fn into_raw(self) -> bindings::phys_addr_t {
> + self.0
> + }
> +}
>
> /// Resource Size type.
> ///
> @@ -101,10 +117,10 @@ pub fn maxsize(&self) -> usize {
> /// ///
> /// /// [`paddr`, `paddr` + `SIZE`) must be a valid MMIO region that is mappable into the CPUs
> /// /// virtual address space.
> -/// unsafe fn new(paddr: usize) -> Result<Self>{
> +/// unsafe fn new(paddr: PhysAddr) -> Result<Self>{
> /// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
> /// // valid for `ioremap`.
> -/// let addr = unsafe { bindings::ioremap(paddr as PhysAddr, SIZE) };
> +/// let addr = unsafe { bindings::ioremap(paddr.into_raw(), SIZE) };
> /// if addr.is_null() {
> /// return Err(ENOMEM);
> /// }
> @@ -131,7 +147,9 @@ pub fn maxsize(&self) -> usize {
> ///
> ///# fn no_run() -> Result<(), Error> {
> /// // SAFETY: Invalid usage for example purposes.
> -/// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
> +/// let iomem = unsafe {
> +/// IoMem::<{ core::mem::size_of::<u32>() }>::new(PhysAddr::from_raw(0xBAAAAAAD))?
> +/// };
> /// iomem.write32(0x42, 0x0);
> /// assert!(iomem.try_write32(0x42, 0x0).is_ok());
> /// assert!(iomem.try_write32(0x42, 0x4).is_err());
> diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs
> index 7dc78d547f7a..f21f9038b297 100644
> --- a/rust/kernel/io/mem.rs
> +++ b/rust/kernel/io/mem.rs
> @@ -254,12 +254,12 @@ fn ioremap(resource: &Resource) -> Result<Self> {
> // 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) }
> + unsafe { bindings::ioremap_np(res_start.into_raw(), size) }
> } 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) }
> + unsafe { bindings::ioremap(res_start.into_raw(), size) }
> };
>
> if addr.is_null() {
> diff --git a/rust/kernel/io/resource.rs b/rust/kernel/io/resource.rs
> index b7ac9faf141d..5dda1061db68 100644
> --- a/rust/kernel/io/resource.rs
> +++ b/rust/kernel/io/resource.rs
> @@ -58,7 +58,7 @@ fn drop(&mut self) {
> };
>
> // SAFETY: Safe as per the invariant of `Region`.
> - unsafe { release_fn(start, size) };
> + unsafe { release_fn(start.into_raw(), size) };
> }
> }
>
> @@ -113,7 +113,7 @@ pub fn request_region(
> let region = unsafe {
> bindings::__request_region(
> self.0.get(),
> - start,
> + start.into_raw(),
> size,
> name.as_char_ptr(),
> flags.0 as c_int,
> @@ -137,7 +137,7 @@ pub fn size(&self) -> ResourceSize {
> pub fn start(&self) -> PhysAddr {
> let inner = self.0.get();
> // SAFETY: Safe as per the invariants of `Resource`.
> - unsafe { (*inner).start }
> + PhysAddr::from_raw(unsafe { (*inner).start })
> }
>
> /// Returns the name of the resource.
> diff --git a/rust/kernel/iommu/pgtable.rs b/rust/kernel/iommu/pgtable.rs
> index c88e38fd938a..ae724ca1e8ad 100644
> --- a/rust/kernel/iommu/pgtable.rs
> +++ b/rust/kernel/iommu/pgtable.rs
> @@ -182,7 +182,7 @@ pub unsafe fn map_pages(
> (map_pages)(
> self.raw_ops(),
> iova,
> - paddr,
> + paddr.into_raw(),
> pgsize,
> pgcount,
> prot as i32,
> --
> 2.54.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] rust: io: use newtype for PhysAddr
2026-05-03 18:28 ` Onur Özkan
@ 2026-05-03 18:45 ` Danilo Krummrich
2026-05-03 18:52 ` Onur Özkan
0 siblings, 1 reply; 7+ messages in thread
From: Danilo Krummrich @ 2026-05-03 18:45 UTC (permalink / raw)
To: Onur Özkan
Cc: x, Greg Kroah-Hartman, Rafael J . Wysocki, Miguel Ojeda,
Daniel Almeida, Joerg Roedel, Will Deacon, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Robin Murphy, driver-core, rust-for-linux,
linux-kernel, iommu
On Sun May 3, 2026 at 8:28 PM CEST, Onur Özkan wrote:
> Copy and Clone seem never used, please drop them (also see [1]).
This is a transparent wrapper of a primitive, so passing by value is a very
natural and idiomatic thing to do, so we should keep them.
What you are referring to is more meant for larger types where a copy might
actually hurt performance.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] rust: io: use newtype for PhysAddr
2026-05-03 18:45 ` Danilo Krummrich
@ 2026-05-03 18:52 ` Onur Özkan
0 siblings, 0 replies; 7+ messages in thread
From: Onur Özkan @ 2026-05-03 18:52 UTC (permalink / raw)
To: Danilo Krummrich
Cc: x, Greg Kroah-Hartman, Rafael J . Wysocki, Miguel Ojeda,
Daniel Almeida, Joerg Roedel, Will Deacon, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Robin Murphy, driver-core, rust-for-linux,
linux-kernel, iommu
On Sun, 03 May 2026 20:45:50 +0200
Danilo Krummrich <dakr@kernel.org> wrote:
> On Sun May 3, 2026 at 8:28 PM CEST, Onur Özkan wrote:
> > Copy and Clone seem never used, please drop them (also see [1]).
>
> This is a transparent wrapper of a primitive, so passing by value is a very
> natural and idiomatic thing to do, so we should keep them.
Right, my bad. I didn't pay attention to the type itself.
>
> What you are referring to is more meant for larger types where a copy might
> actually hurt performance.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] rust: io: use newtype for PhysAddr
2026-05-03 10:30 [PATCH] rust: io: use newtype for PhysAddr x
2026-05-03 10:43 ` Greg Kroah-Hartman
2026-05-03 18:28 ` Onur Özkan
@ 2026-05-04 11:14 ` x
2026-05-04 22:04 ` Danilo Krummrich
2 siblings, 1 reply; 7+ messages in thread
From: x @ 2026-05-04 11:14 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J . Wysocki, Danilo Krummrich,
Miguel Ojeda, Daniel Almeida, Joerg Roedel, Will Deacon
Cc: Onur Özkan, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Robin Murphy, driver-core, rust-for-linux, linux-kernel, iommu
From: Favilances Noir <favilances@proton.me>
Currently, PhysAddr is a type alias for phys_addr_t. This means
that physical addresses can be mixed with unrelated integer values
without Rust noticing.
Use a transparent newtype for PhysAddr instead, and update the
affected call sites to perform explicit raw conversions at the FFI
boundary.
This keeps the existing phys_addr_t representation while making
physical addresses distinct in Rust type checking.
Drop the unused Clone and Copy derives from PhysAddr, as pointed out
in review.
Signed-off-by: Favilances Noir <favilances@proton.me>
---
rust/kernel/devres.rs | 8 +++++---
rust/kernel/io.rs | 29 +++++++++++++++++++++++------
rust/kernel/io/mem.rs | 4 ++--
rust/kernel/io/resource.rs | 6 +++---
rust/kernel/iommu/pgtable.rs | 2 +-
5 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 9e5f93aed20c..beb3723d2519 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -85,10 +85,10 @@ struct Inner<T> {
/// ///
/// /// [`paddr`, `paddr` + `SIZE`) must be a valid MMIO region that is mappable into the CPUs
/// /// virtual address space.
-/// unsafe fn new(paddr: usize) -> Result<Self>{
+/// unsafe fn new(paddr: PhysAddr) -> Result<Self>{
/// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
/// // valid for `ioremap`.
-/// let addr = unsafe { bindings::ioremap(paddr as PhysAddr, SIZE) };
+/// let addr = unsafe { bindings::ioremap(paddr.into_raw(), SIZE) };
/// if addr.is_null() {
/// return Err(ENOMEM);
/// }
@@ -114,7 +114,9 @@ struct Inner<T> {
/// }
/// # fn no_run(dev: &Device<Bound>) -> Result<(), Error> {
/// // SAFETY: Invalid usage for example purposes.
-/// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
+/// let iomem = unsafe {
+/// IoMem::<{ core::mem::size_of::<u32>() }>::new(PhysAddr::from_raw(0xBAAAAAAD))?
+/// };
/// let devres = Devres::new(dev, iomem)?;
///
/// let res = devres.try_access().ok_or(ENXIO)?;
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index fcc7678fd9e3..999fa285a6ec 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -21,9 +21,24 @@
/// Physical address 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 PhysAddr = bindings::phys_addr_t;
+/// This wraps 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.
+#[repr(transparent)]
+pub struct PhysAddr(bindings::phys_addr_t);
+
+impl PhysAddr {
+ /// Creates a physical address from the raw C type.
+ #[inline]
+ pub const fn from_raw(value: bindings::phys_addr_t) -> Self {
+ Self(value)
+ }
+
+ /// Turns this physical address into the raw C type.
+ #[inline]
+ pub const fn into_raw(self) -> bindings::phys_addr_t {
+ self.0
+ }
+}
/// Resource Size type.
///
@@ -101,10 +116,10 @@ pub fn maxsize(&self) -> usize {
/// ///
/// /// [`paddr`, `paddr` + `SIZE`) must be a valid MMIO region that is mappable into the CPUs
/// /// virtual address space.
-/// unsafe fn new(paddr: usize) -> Result<Self>{
+/// unsafe fn new(paddr: PhysAddr) -> Result<Self>{
/// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
/// // valid for `ioremap`.
-/// let addr = unsafe { bindings::ioremap(paddr as PhysAddr, SIZE) };
+/// let addr = unsafe { bindings::ioremap(paddr.into_raw(), SIZE) };
/// if addr.is_null() {
/// return Err(ENOMEM);
/// }
@@ -131,7 +146,9 @@ pub fn maxsize(&self) -> usize {
///
///# fn no_run() -> Result<(), Error> {
/// // SAFETY: Invalid usage for example purposes.
-/// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
+/// let iomem = unsafe {
+/// IoMem::<{ core::mem::size_of::<u32>() }>::new(PhysAddr::from_raw(0xBAAAAAAD))?
+/// };
/// iomem.write32(0x42, 0x0);
/// assert!(iomem.try_write32(0x42, 0x0).is_ok());
/// assert!(iomem.try_write32(0x42, 0x4).is_err());
diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs
index 7dc78d547f7a..f21f9038b297 100644
--- a/rust/kernel/io/mem.rs
+++ b/rust/kernel/io/mem.rs
@@ -254,12 +254,12 @@ fn ioremap(resource: &Resource) -> Result<Self> {
// 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) }
+ unsafe { bindings::ioremap_np(res_start.into_raw(), size) }
} 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) }
+ unsafe { bindings::ioremap(res_start.into_raw(), size) }
};
if addr.is_null() {
diff --git a/rust/kernel/io/resource.rs b/rust/kernel/io/resource.rs
index b7ac9faf141d..5dda1061db68 100644
--- a/rust/kernel/io/resource.rs
+++ b/rust/kernel/io/resource.rs
@@ -58,7 +58,7 @@ fn drop(&mut self) {
};
// SAFETY: Safe as per the invariant of `Region`.
- unsafe { release_fn(start, size) };
+ unsafe { release_fn(start.into_raw(), size) };
}
}
@@ -113,7 +113,7 @@ pub fn request_region(
let region = unsafe {
bindings::__request_region(
self.0.get(),
- start,
+ start.into_raw(),
size,
name.as_char_ptr(),
flags.0 as c_int,
@@ -137,7 +137,7 @@ pub fn size(&self) -> ResourceSize {
pub fn start(&self) -> PhysAddr {
let inner = self.0.get();
// SAFETY: Safe as per the invariants of `Resource`.
- unsafe { (*inner).start }
+ PhysAddr::from_raw(unsafe { (*inner).start })
}
/// Returns the name of the resource.
diff --git a/rust/kernel/iommu/pgtable.rs b/rust/kernel/iommu/pgtable.rs
index c88e38fd938a..ae724ca1e8ad 100644
--- a/rust/kernel/iommu/pgtable.rs
+++ b/rust/kernel/iommu/pgtable.rs
@@ -182,7 +182,7 @@ pub unsafe fn map_pages(
(map_pages)(
self.raw_ops(),
iova,
- paddr,
+ paddr.into_raw(),
pgsize,
pgcount,
prot as i32,
--
2.54.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] rust: io: use newtype for PhysAddr
2026-05-04 11:14 ` [PATCH v2] " x
@ 2026-05-04 22:04 ` Danilo Krummrich
0 siblings, 0 replies; 7+ messages in thread
From: Danilo Krummrich @ 2026-05-04 22:04 UTC (permalink / raw)
To: x
Cc: Greg Kroah-Hartman, Rafael J . Wysocki, Miguel Ojeda,
Daniel Almeida, Joerg Roedel, Will Deacon, Onur Özkan,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Robin Murphy,
driver-core, rust-for-linux, linux-kernel, iommu
Hi Favilances,
(I assume you are Favilances; asking since the mail came from just x@2005.tr.)
On Mon May 4, 2026 at 1:14 PM CEST, x wrote:
> From: Favilances Noir <favilances@proton.me>
>
> Currently, PhysAddr is a type alias for phys_addr_t. This means
> that physical addresses can be mixed with unrelated integer values
> without Rust noticing.
>
> Use a transparent newtype for PhysAddr instead, and update the
> affected call sites to perform explicit raw conversions at the FFI
> boundary.
>
> This keeps the existing phys_addr_t representation while making
> physical addresses distinct in Rust type checking.
>
> Drop the unused Clone and Copy derives from PhysAddr, as pointed out
> in review.
This should be part of the version change log and should go below the "---"
line.
Please also consider the subsequent replies to this feedback, which point out
that this should indeed have Clone and Copy (and probably also other derives
such as PartialEq).
For instance, consider the case where you call into_raw() because you need the
raw value but then still want to do something with the PhysAddr. Without Copy
you'd need to write something like:
let r = p.into_raw();
let p = PhysAddr::from_raw(r); // `r` implements `Copy`.
// Use `r` and `p`.
> Signed-off-by: Favilances Noir <favilances@proton.me>
<snip>
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index fcc7678fd9e3..999fa285a6ec 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -21,9 +21,24 @@
>
> /// Physical address 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 PhysAddr = bindings::phys_addr_t;
> +/// This wraps 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.
> +#[repr(transparent)]
> +pub struct PhysAddr(bindings::phys_addr_t);
> +
> +impl PhysAddr {
> + /// Creates a physical address from the raw C type.
> + #[inline]
> + pub const fn from_raw(value: bindings::phys_addr_t) -> Self {
> + Self(value)
> + }
> +
> + /// Turns this physical address into the raw C type.
> + #[inline]
> + pub const fn into_raw(self) -> bindings::phys_addr_t {
> + self.0
> + }
> +}
I like the explicit methods, but I can also see that this might become a bit
verbose; maybe consider adding From impls as well.
I think eventually we also want {Try}From impls for usize etc., but let's wait
until we have a user.
Maybe also consider a Debug impl that prints a hex value.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-05-04 22:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-03 10:30 [PATCH] rust: io: use newtype for PhysAddr x
2026-05-03 10:43 ` Greg Kroah-Hartman
2026-05-03 18:28 ` Onur Özkan
2026-05-03 18:45 ` Danilo Krummrich
2026-05-03 18:52 ` Onur Özkan
2026-05-04 11:14 ` [PATCH v2] " x
2026-05-04 22:04 ` Danilo Krummrich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox