* [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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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 2026-05-05 8:23 ` [PATCH v3] " x 2 siblings, 2 replies; 9+ 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] 9+ 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 2026-05-05 8:23 ` [PATCH v3] " x 1 sibling, 0 replies; 9+ 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] 9+ messages in thread
* [PATCH v3] rust: io: use newtype for PhysAddr 2026-05-04 11:14 ` [PATCH v2] " x 2026-05-04 22:04 ` Danilo Krummrich @ 2026-05-05 8:23 ` x 2026-05-05 18:28 ` Miguel Ojeda 1 sibling, 1 reply; 9+ messages in thread From: x @ 2026-05-05 8:23 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. Derive Clone, Copy, PartialEq, and Eq for PhysAddr, implement Debug printing physical addresses in hexadecimal, and add From conversions between PhysAddr and the raw phys_addr_t type for API ergonomics. A fallible TryFrom<usize> conversion can be added later if needed. Signed-off-by: Favilances Noir <favilances@proton.me> --- v3: - Re-add Clone and Copy derives as suggested - Add PartialEq and Eq - Add From impls for better ergonomics - Add Debug impl printing hexadecimal values - Keep explicit conversions at the FFI boundary - Thanks to Onur and Danilo for the review and suggestions rust/kernel/devres.rs | 8 +++--- rust/kernel/io.rs | 49 +++++++++++++++++++++++++++++++----- rust/kernel/io/mem.rs | 4 +-- rust/kernel/io/resource.rs | 6 ++--- rust/kernel/iommu/pgtable.rs | 2 +- 5 files changed, 54 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..e9e8944ddea6 100644 --- a/rust/kernel/io.rs +++ b/rust/kernel/io.rs @@ -6,6 +6,7 @@ use crate::{ bindings, + fmt, prelude::*, // }; @@ -21,9 +22,43 @@ /// 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, PartialEq, Eq)] +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 + } +} + +impl fmt::Debug for PhysAddr { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:#x}", self.0) + } +} + +impl From<bindings::phys_addr_t> for PhysAddr { + fn from(value: bindings::phys_addr_t) -> Self { + Self::from_raw(value) + } +} + +impl From<PhysAddr> for bindings::phys_addr_t { + fn from(value: PhysAddr) -> Self { + value.into_raw() + } +} /// Resource Size type. /// @@ -101,10 +136,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 +166,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] 9+ messages in thread
* Re: [PATCH v3] rust: io: use newtype for PhysAddr 2026-05-05 8:23 ` [PATCH v3] " x @ 2026-05-05 18:28 ` Miguel Ojeda 0 siblings, 0 replies; 9+ messages in thread From: Miguel Ojeda @ 2026-05-05 18:28 UTC (permalink / raw) To: x Cc: Greg Kroah-Hartman, Rafael J . Wysocki, Danilo Krummrich, 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 On Tue, May 5, 2026 at 10:24 AM <x@2005.tr> wrote: > > From: Favilances Noir <favilances@proton.me> It is great to see more strong types, thanks for this! From the other parallel patch: Suggested-by: Miguel Ojeda <ojeda@kernel.org> Link: https://github.com/Rust-for-Linux/linux/issues/1204 By the way, please try to fix the email From header -- it can be quite confusing to see "x" in the mailing list and other tooling... :) We also probably want a few more `#[inline]`s as Sashiko mentions. Cheers, Miguel ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-05-05 18:28 UTC | newest] Thread overview: 9+ 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 2026-05-05 8:23 ` [PATCH v3] " x 2026-05-05 18:28 ` Miguel Ojeda
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox