[PATCH v3 2/5] rust: dma: add type alias for bindings::dma_addr_t

Danilo Krummrich posted 5 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v3 2/5] rust: dma: add type alias for bindings::dma_addr_t
Posted by Danilo Krummrich 1 month, 1 week ago
Add a type alias for bindings::dma_addr_t (DmaAddress), such that we do
not have to access bindings directly.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/gpu/nova-core/falcon.rs |  4 ++--
 rust/kernel/dma.rs              | 18 ++++++++++++++----
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index 50437c67c14a..aa36ed8c04ed 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -4,8 +4,8 @@
 
 use core::ops::Deref;
 use hal::FalconHal;
-use kernel::bindings;
 use kernel::device;
+use kernel::dma::DmaAddress;
 use kernel::prelude::*;
 use kernel::time::Delta;
 use kernel::types::ARef;
@@ -443,7 +443,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
                 fw.dma_handle_with_offset(load_offsets.src_start as usize)?,
             ),
         };
-        if dma_start % bindings::dma_addr_t::from(DMA_LEN) > 0 {
+        if dma_start % DmaAddress::from(DMA_LEN) > 0 {
             dev_err!(
                 self.dev,
                 "DMA transfer start addresses must be a multiple of {}",
diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 27b25f041f32..b2a6282876da 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -13,6 +13,16 @@
     types::ARef,
 };
 
+/// DMA address type.
+///
+/// Represents a bus address used for Direct Memory Access (DMA) operations.
+///
+/// This is an alias of the kernel's `dma_addr_t`, which may be `u32` or `u64` depending on
+/// `CONFIG_ARCH_DMA_ADDR_T_64BIT`.
+///
+/// Note that this may be `u64` even on 32-bit architectures.
+pub type DmaAddress = bindings::dma_addr_t;
+
 /// Trait to be implemented by DMA capable bus devices.
 ///
 /// The [`dma::Device`](Device) trait should be implemented by bus specific device representations,
@@ -343,7 +353,7 @@ fn from(direction: DataDirection) -> Self {
 // entire `CoherentAllocation` including the allocated memory itself.
 pub struct CoherentAllocation<T: AsBytes + FromBytes> {
     dev: ARef<device::Device>,
-    dma_handle: bindings::dma_addr_t,
+    dma_handle: DmaAddress,
     count: usize,
     cpu_addr: *mut T,
     dma_attrs: Attrs,
@@ -444,7 +454,7 @@ pub fn start_ptr_mut(&mut self) -> *mut T {
 
     /// Returns a DMA handle which may be given to the device as the DMA address base of
     /// the region.
-    pub fn dma_handle(&self) -> bindings::dma_addr_t {
+    pub fn dma_handle(&self) -> DmaAddress {
         self.dma_handle
     }
 
@@ -452,13 +462,13 @@ pub fn dma_handle(&self) -> bindings::dma_addr_t {
     /// device as the DMA address base of the region.
     ///
     /// Returns `EINVAL` if `offset` is not within the bounds of the allocation.
-    pub fn dma_handle_with_offset(&self, offset: usize) -> Result<bindings::dma_addr_t> {
+    pub fn dma_handle_with_offset(&self, offset: usize) -> Result<DmaAddress> {
         if offset >= self.count {
             Err(EINVAL)
         } else {
             // INVARIANT: The type invariant of `Self` guarantees that `size_of::<T> * count` fits
             // into a `usize`, and `offset` is inferior to `count`.
-            Ok(self.dma_handle + (offset * core::mem::size_of::<T>()) as bindings::dma_addr_t)
+            Ok(self.dma_handle + (offset * core::mem::size_of::<T>()) as DmaAddress)
         }
     }
 
-- 
2.51.0
Re: [PATCH v3 2/5] rust: dma: add type alias for bindings::dma_addr_t
Posted by Daniel Almeida 1 month, 1 week ago

> On 25 Aug 2025, at 10:24, Danilo Krummrich <dakr@kernel.org> wrote:
> 
> Add a type alias for bindings::dma_addr_t (DmaAddress), such that we do
> not have to access bindings directly.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> drivers/gpu/nova-core/falcon.rs |  4 ++--
> rust/kernel/dma.rs              | 18 ++++++++++++++----
> 2 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
> index 50437c67c14a..aa36ed8c04ed 100644
> --- a/drivers/gpu/nova-core/falcon.rs
> +++ b/drivers/gpu/nova-core/falcon.rs
> @@ -4,8 +4,8 @@
> 
> use core::ops::Deref;
> use hal::FalconHal;
> -use kernel::bindings;
> use kernel::device;
> +use kernel::dma::DmaAddress;
> use kernel::prelude::*;
> use kernel::time::Delta;
> use kernel::types::ARef;
> @@ -443,7 +443,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>(
>                 fw.dma_handle_with_offset(load_offsets.src_start as usize)?,
>             ),
>         };
> -        if dma_start % bindings::dma_addr_t::from(DMA_LEN) > 0 {
> +        if dma_start % DmaAddress::from(DMA_LEN) > 0 {
>             dev_err!(
>                 self.dev,
>                 "DMA transfer start addresses must be a multiple of {}",
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index 27b25f041f32..b2a6282876da 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -13,6 +13,16 @@
>     types::ARef,
> };
> 
> +/// DMA address type.
> +///
> +/// Represents a bus address used for Direct Memory Access (DMA) operations.
> +///
> +/// This is an alias of the kernel's `dma_addr_t`, which may be `u32` or `u64` depending on
> +/// `CONFIG_ARCH_DMA_ADDR_T_64BIT`.
> +///
> +/// Note that this may be `u64` even on 32-bit architectures.
> +pub type DmaAddress = bindings::dma_addr_t;
> +
> /// Trait to be implemented by DMA capable bus devices.
> ///
> /// The [`dma::Device`](Device) trait should be implemented by bus specific device representations,
> @@ -343,7 +353,7 @@ fn from(direction: DataDirection) -> Self {
> // entire `CoherentAllocation` including the allocated memory itself.
> pub struct CoherentAllocation<T: AsBytes + FromBytes> {
>     dev: ARef<device::Device>,
> -    dma_handle: bindings::dma_addr_t,
> +    dma_handle: DmaAddress,
>     count: usize,
>     cpu_addr: *mut T,
>     dma_attrs: Attrs,
> @@ -444,7 +454,7 @@ pub fn start_ptr_mut(&mut self) -> *mut T {
> 
>     /// Returns a DMA handle which may be given to the device as the DMA address base of
>     /// the region.
> -    pub fn dma_handle(&self) -> bindings::dma_addr_t {
> +    pub fn dma_handle(&self) -> DmaAddress {
>         self.dma_handle
>     }
> 
> @@ -452,13 +462,13 @@ pub fn dma_handle(&self) -> bindings::dma_addr_t {
>     /// device as the DMA address base of the region.
>     ///
>     /// Returns `EINVAL` if `offset` is not within the bounds of the allocation.
> -    pub fn dma_handle_with_offset(&self, offset: usize) -> Result<bindings::dma_addr_t> {
> +    pub fn dma_handle_with_offset(&self, offset: usize) -> Result<DmaAddress> {
>         if offset >= self.count {
>             Err(EINVAL)
>         } else {
>             // INVARIANT: The type invariant of `Self` guarantees that `size_of::<T> * count` fits
>             // into a `usize`, and `offset` is inferior to `count`.
> -            Ok(self.dma_handle + (offset * core::mem::size_of::<T>()) as bindings::dma_addr_t)
> +            Ok(self.dma_handle + (offset * core::mem::size_of::<T>()) as DmaAddress)
>         }
>     }
> 
> -- 
> 2.51.0
> 

Hmm, I wonder if this shouldn’t be its own type, instead of an alias. This
will be handy if we want to enforce that a given address is, in fact, a bus
address.

In any case, this can be a separate patch. This one is good.


Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Re: [PATCH v3 2/5] rust: dma: add type alias for bindings::dma_addr_t
Posted by Danilo Krummrich 1 month, 1 week ago
On Tue Aug 26, 2025 at 7:15 PM CEST, Daniel Almeida wrote:
> Hmm, I wonder if this shouldn’t be its own type, instead of an alias. This
> will be handy if we want to enforce that a given address is, in fact, a bus
> address.

I'm not sure I understand the idea. How can a new type compared to a type alias
help to guarantee that a DMA address is also a bus address?

This depends on whether there is an IOMMU, etc.

> In any case, this can be a separate patch. This one is good.
>
>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Re: [PATCH v3 2/5] rust: dma: add type alias for bindings::dma_addr_t
Posted by Daniel Almeida 1 month, 1 week ago

> On 26 Aug 2025, at 14:33, Danilo Krummrich <dakr@kernel.org> wrote:
> 
> On Tue Aug 26, 2025 at 7:15 PM CEST, Daniel Almeida wrote:
>> Hmm, I wonder if this shouldn’t be its own type, instead of an alias. This
>> will be handy if we want to enforce that a given address is, in fact, a bus
>> address.
> 
> I'm not sure I understand the idea. How can a new type compared to a type alias
> help to guarantee that a DMA address is also a bus address?
> 
> This depends on whether there is an IOMMU, etc.


I was referring to the term "bus address" as used here [0].

My understanding has always been that a dma_addr_t is a bus address regardless
of whether the system has an iommu? If there's no IOMMU then we there's a 1:1
correspondence but this doesn't invalidate the use of the term? i.e.: it's still
an address that can be directly accessed by a hardware device.

In any case, this is a bit orthogonal to the point I was trying to make, my bad
for not expressing it better.

What I mean is, by using a separate type, i.e.:

DmaAddress(bindings::dma_addr_t)

we now ensure that one cannot pass a random u32/u64 value where a DmaAddress is
expected. It's a bit like the __iomem or __user C annotation, but actually
enforced by the type system.

In fact, this is something that was recently done in uaccess.rs, IIRC.


[0]: https://docs.kernel.org/core-api/dma-api-howto.html#cpu-and-dma-addresses