This changes ResourceSize to use the resource_size_t typedef (currently ResourceSize is defined as phys_addr_t), and moves ResourceSize to kernel::io and defines PhysAddr next to it. Any usage of ResourceSize or bindings::phys_addr_t that references a physical address is updated to use the new PhysAddr typedef.
I included some cc stable annotations because I think it is useful to backport this to v6.18. This is to make backporting drivers to the 6.18 LTS easier as we will not have to worry about changing imports when backporting.
Signed-off-by: Alice Ryhl aliceryhl@google.com --- Changes in v2: - Fix build error in last patch. - Add cc stable. - Link to v1: https://lore.kernel.org/r/20251106-resource-phys-typedefs-v1-0-0c0edc7301ce@...
--- Alice Ryhl (4): rust: io: define ResourceSize as resource_size_t rust: io: move ResourceSize to top-level io module rust: scatterlist: import ResourceSize from kernel::io rust: io: add typedef for phys_addr_t
rust/kernel/devres.rs | 18 +++++++++++++++--- rust/kernel/io.rs | 26 +++++++++++++++++++++++--- rust/kernel/io/resource.rs | 13 ++++++------- rust/kernel/scatterlist.rs | 2 +- 4 files changed, 45 insertions(+), 14 deletions(-) --- base-commit: 211ddde0823f1442e4ad052a2f30f050145ccada change-id: 20251106-resource-phys-typedefs-6db37927d159
Best regards,
These typedefs are always equivalent so this should not change anything, but the code makes a lot more sense like this.
Cc: stable@vger.kernel.org # for v6.18 Signed-off-by: Alice Ryhl aliceryhl@google.com --- rust/kernel/io/resource.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rust/kernel/io/resource.rs b/rust/kernel/io/resource.rs index bea3ee0ed87b51816e2afb5a8a36fedee60d0e06..11b6bb9678b4e36603cc26fa2d6552c0a7e8276c 100644 --- a/rust/kernel/io/resource.rs +++ b/rust/kernel/io/resource.rs @@ -16,7 +16,7 @@ /// /// This is a type alias to either `u32` or `u64` depending on the config option /// `CONFIG_PHYS_ADDR_T_64BIT`, and it can be a u64 even on 32-bit architectures. -pub type ResourceSize = bindings::phys_addr_t; +pub type ResourceSize = bindings::resource_size_t;
/// A region allocated from a parent [`Resource`]. ///
On Wed, Nov 12, 2025 at 10:49 AM Alice Ryhl aliceryhl@google.com wrote:
These typedefs are always equivalent so this should not change anything, but the code makes a lot more sense like this.
Cc: stable@vger.kernel.org # for v6.18
From the cover letter thread, if we treat this as a fix, then this should be:
Cc: stable@vger.kernel.org Fixes: 493fc33ec252 ("rust: io: add resource abstraction")
(Replying here as well in case it helps automated tooling to pick it up)
Cheers, Miguel
Resource sizes are a general concept for dealing with physical addresses, and not specific to the Resource type, which is just one way to access physical addresses. Thus, move the typedef to the io module.
Still keep a re-export under resource. This avoids this commit from being a flag-day, but I also think it's a useful re-export in general so that you can import
use kernel::io::resource::{Resource, ResourceSize};
instead of having to write
use kernel::io::{ resource::Resource, ResourceSize, };
in the specific cases where you need ResourceSize because you are using the Resource type. Therefore I think it makes sense to keep this re-export indefinitely and it is *not* intended as a temporary re-export for migration purposes.
Cc: stable@vger.kernel.org # for v6.18 Signed-off-by: Alice Ryhl aliceryhl@google.com --- rust/kernel/io.rs | 6 ++++++ rust/kernel/io/resource.rs | 6 +----- 2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs index ee182b0b5452dfcc9891d46cc6cd84d0cf7cdae7..6465ea94e85d689aef1f9031a4a5cc9505b9af6e 100644 --- a/rust/kernel/io.rs +++ b/rust/kernel/io.rs @@ -13,6 +13,12 @@
pub use resource::Resource;
+/// Resource Size type. +/// +/// This is a type alias to either `u32` or `u64` depending on the config option +/// `CONFIG_PHYS_ADDR_T_64BIT`, and it can be a u64 even on 32-bit architectures. +pub type ResourceSize = bindings::resource_size_t; + /// 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 index 11b6bb9678b4e36603cc26fa2d6552c0a7e8276c..7fed41fc20307fa7ce230da4b7841743631c965e 100644 --- a/rust/kernel/io/resource.rs +++ b/rust/kernel/io/resource.rs @@ -12,11 +12,7 @@ use crate::str::{CStr, CString}; use crate::types::Opaque;
-/// Resource Size type. -/// -/// This is a type alias to either `u32` or `u64` depending on the config option -/// `CONFIG_PHYS_ADDR_T_64BIT`, and it can be a u64 even on 32-bit architectures. -pub type ResourceSize = bindings::resource_size_t; +pub use super::ResourceSize;
/// A region allocated from a parent [`Resource`]. ///
The C typedef phys_addr_t is missing an analogue in Rust, meaning that we end up using bindings::phys_addr_t or ResourceSize as a replacement in various places throughout the kernel. Fix that by introducing a new typedef on the Rust side. Place it next to the existing ResourceSize typedef since they're quite related to each other.
Cc: stable@vger.kernel.org # for v6.18 Signed-off-by: Alice Ryhl aliceryhl@google.com --- rust/kernel/devres.rs | 18 +++++++++++++++--- rust/kernel/io.rs | 20 +++++++++++++++++--- rust/kernel/io/resource.rs | 9 ++++++--- 3 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 10a6a17898541d4a0f01af41f68e3ab3d6bb0aca..e01e0d36702d38e9530a2fa4be884c935c6b5d0c 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -52,8 +52,20 @@ struct Inner<T: Send> { /// # Examples /// /// ```no_run -/// # use kernel::{bindings, device::{Bound, Device}, devres::Devres, io::{Io, IoRaw}}; -/// # use core::ops::Deref; +/// use kernel::{ +/// bindings, +/// device::{ +/// Bound, +/// Device, +/// }, +/// devres::Devres, +/// io::{ +/// Io, +/// IoRaw, +/// PhysAddr, +/// }, +/// }; +/// use core::ops::Deref; /// /// // See also [`pci::Bar`] for a real example. /// struct IoMem<const SIZE: usize>(IoRaw<SIZE>); @@ -66,7 +78,7 @@ struct Inner<T: Send> { /// unsafe fn new(paddr: usize) -> Result<Self>{ /// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is /// // valid for `ioremap`. -/// let addr = unsafe { bindings::ioremap(paddr as bindings::phys_addr_t, SIZE) }; +/// let addr = unsafe { bindings::ioremap(paddr as PhysAddr, SIZE) }; /// if addr.is_null() { /// return Err(ENOMEM); /// } diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs index 6465ea94e85d689aef1f9031a4a5cc9505b9af6e..56a435eb14e3a1ce72dd58b88cbf296041f1703e 100644 --- a/rust/kernel/io.rs +++ b/rust/kernel/io.rs @@ -13,6 +13,12 @@
pub use resource::Resource;
+/// 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; + /// Resource Size type. /// /// This is a type alias to either `u32` or `u64` depending on the config option @@ -68,8 +74,16 @@ pub fn maxsize(&self) -> usize { /// # Examples /// /// ```no_run -/// # use kernel::{bindings, ffi::c_void, io::{Io, IoRaw}}; -/// # use core::ops::Deref; +/// use kernel::{ +/// bindings, +/// ffi::c_void, +/// io::{ +/// Io, +/// IoRaw, +/// PhysAddr, +/// }, +/// }; +/// use core::ops::Deref; /// /// // See also [`pci::Bar`] for a real example. /// struct IoMem<const SIZE: usize>(IoRaw<SIZE>); @@ -82,7 +96,7 @@ pub fn maxsize(&self) -> usize { /// unsafe fn new(paddr: usize) -> Result<Self>{ /// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is /// // valid for `ioremap`. -/// let addr = unsafe { bindings::ioremap(paddr as bindings::phys_addr_t, SIZE) }; +/// let addr = unsafe { bindings::ioremap(paddr as PhysAddr, SIZE) }; /// if addr.is_null() { /// return Err(ENOMEM); /// } diff --git a/rust/kernel/io/resource.rs b/rust/kernel/io/resource.rs index 7fed41fc20307fa7ce230da4b7841743631c965e..0e86ee9c98d840b44b8f8953f83d31612fdaea46 100644 --- a/rust/kernel/io/resource.rs +++ b/rust/kernel/io/resource.rs @@ -12,7 +12,10 @@ use crate::str::{CStr, CString}; use crate::types::Opaque;
-pub use super::ResourceSize; +pub use super::{ + PhysAddr, + ResourceSize, // +};
/// A region allocated from a parent [`Resource`]. /// @@ -93,7 +96,7 @@ impl Resource { /// the region, or a part of it, is already in use. pub fn request_region( &self, - start: ResourceSize, + start: PhysAddr, size: ResourceSize, name: CString, flags: Flags, @@ -127,7 +130,7 @@ pub fn size(&self) -> ResourceSize { }
/// Returns the start address of the resource. - pub fn start(&self) -> ResourceSize { + pub fn start(&self) -> PhysAddr { let inner = self.0.get(); // SAFETY: Safe as per the invariants of `Resource`. unsafe { (*inner).start }
On Wed, Nov 12, 2025 at 10:49 AM Alice Ryhl aliceryhl@google.com wrote:
This changes ResourceSize to use the resource_size_t typedef (currently ResourceSize is defined as phys_addr_t), and moves ResourceSize to kernel::io and defines PhysAddr next to it. Any usage of ResourceSize or bindings::phys_addr_t that references a physical address is updated to use the new PhysAddr typedef.
Should we have these as actual types instead of aliases? i.e. same discussion as for `Offset`.
If there is a change of these getting mixed up, then I think we should just pay that price (not necessarily now, of course).
I included some cc stable annotations because I think it is useful to backport this to v6.18. This is to make backporting drivers to the 6.18 LTS easier as we will not have to worry about changing imports when backporting.
For context, will those drivers be backported upstream too?
i.e. we have sometimes backported bits to simplify further backporting elsewhere, which is fine and up to the stable team of course, but I am not sure if using Option 1 (i.e. the Cc tag) may be a bit confusing in the log, i.e. Option 2 or 3 offer a better chance to give a reason.
Thanks!
Cheers, Miguel
On Wed, Nov 12, 2025 at 11:12:32AM +0100, Miguel Ojeda wrote:
On Wed, Nov 12, 2025 at 10:49 AM Alice Ryhl aliceryhl@google.com wrote:
This changes ResourceSize to use the resource_size_t typedef (currently ResourceSize is defined as phys_addr_t), and moves ResourceSize to kernel::io and defines PhysAddr next to it. Any usage of ResourceSize or bindings::phys_addr_t that references a physical address is updated to use the new PhysAddr typedef.
Should we have these as actual types instead of aliases? i.e. same discussion as for `Offset`.
If there is a change of these getting mixed up, then I think we should just pay that price (not necessarily now, of course).
Maybe later. Right now I think it's more trouble than it's worth.
I included some cc stable annotations because I think it is useful to backport this to v6.18. This is to make backporting drivers to the 6.18 LTS easier as we will not have to worry about changing imports when backporting.
For context, will those drivers be backported upstream too?
I could imagine cases where a normal fix gets backported upstream and benefits from this, but I mainly thought it was useful for backports that happen downstream.
i.e. we have sometimes backported bits to simplify further backporting elsewhere, which is fine and up to the stable team of course, but I am not sure if using Option 1 (i.e. the Cc tag) may be a bit confusing in the log, i.e. Option 2 or 3 offer a better chance to give a reason.
Using a different option makes sense to me.
Alice
On Wed, Nov 12, 2025 at 10:24:45AM +0000, Alice Ryhl wrote:
On Wed, Nov 12, 2025 at 11:12:32AM +0100, Miguel Ojeda wrote:
On Wed, Nov 12, 2025 at 10:49 AM Alice Ryhl aliceryhl@google.com wrote:
This changes ResourceSize to use the resource_size_t typedef (currently ResourceSize is defined as phys_addr_t), and moves ResourceSize to kernel::io and defines PhysAddr next to it. Any usage of ResourceSize or bindings::phys_addr_t that references a physical address is updated to use the new PhysAddr typedef.
Should we have these as actual types instead of aliases? i.e. same discussion as for `Offset`.
If there is a change of these getting mixed up, then I think we should just pay that price (not necessarily now, of course).
Maybe later. Right now I think it's more trouble than it's worth.
I included some cc stable annotations because I think it is useful to backport this to v6.18. This is to make backporting drivers to the 6.18 LTS easier as we will not have to worry about changing imports when backporting.
For context, will those drivers be backported upstream too?
I could imagine cases where a normal fix gets backported upstream and benefits from this, but I mainly thought it was useful for backports that happen downstream.
i.e. we have sometimes backported bits to simplify further backporting elsewhere, which is fine and up to the stable team of course, but I am not sure if using Option 1 (i.e. the Cc tag) may be a bit confusing in the log, i.e. Option 2 or 3 offer a better chance to give a reason.
Using a different option makes sense to me.
On the other hand, I think that the first patch qualifies as an actual fix.
Alice
On Wed, Nov 12, 2025 at 11:43 AM Alice Ryhl aliceryhl@google.com wrote:
On the other hand, I think that the first patch qualifies as an actual fix.
Yes -- for that one we should add a Fixes instead:
Fixes: 493fc33ec252 ("rust: io: add resource abstraction")
Which means it may go to 6.17 as well and the "# for 6.18" part is not needed in the other tag.
Cheers, Miguel
On Wed, Nov 12, 2025 at 11:24 AM Alice Ryhl aliceryhl@google.com wrote:
Maybe later. Right now I think it's more trouble than it's worth.
In case it may inspire a newcomer, I filled:
https://github.com/Rust-for-Linux/linux/issues/1203 https://github.com/Rust-for-Linux/linux/issues/1204
Cheers, Miguel
linux-stable-mirror@lists.linaro.org
-
Alice Ryhl -
Miguel Ojeda