Re: [PATCH 2/5] rust: dma: add DMA addressing capabilities

["Alexandre Courbot" <acourbot@nvidia.com>]

On Fri Jul 11, 2025 at 4:45 AM JST, Danilo Krummrich wrote:
> Implement `dma_set_mask()`, `dma_set_coherent_mask()` and
> `dma_set_mask_and_coherent()` in the `dma::Device` trait.
>
> Those methods are used to set up the device's DMA addressing
> capabilities.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/helpers/dma.c |  5 +++
>  rust/kernel/dma.rs | 82 ++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 84 insertions(+), 3 deletions(-)
>
> diff --git a/rust/helpers/dma.c b/rust/helpers/dma.c
> index df8b8a77355a..6e741c197242 100644
> --- a/rust/helpers/dma.c
> +++ b/rust/helpers/dma.c
> @@ -14,3 +14,8 @@ void rust_helper_dma_free_attrs(struct device *dev, size_t size, void *cpu_addr,
>  {
>  	dma_free_attrs(dev, size, cpu_addr, dma_handle, attrs);
>  }
> +
> +int rust_helper_dma_set_mask_and_coherent(struct device *dev, u64 mask)
> +{
> +	return dma_set_mask_and_coherent(dev, mask);
> +}
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index f0af23d08e8d..4b27b8279941 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -6,9 +6,9 @@
>  
>  use crate::{
>      bindings, build_assert, device,
> -    device::Bound,
> +    device::{Bound, Core},
>      error::code::*,
> -    error::Result,
> +    error::{to_result, Result},
>      transmute::{AsBytes, FromBytes},
>      types::ARef,
>  };
> @@ -18,7 +18,83 @@
>  /// The [`dma::Device`](Device) trait should be implemented by bus specific device representations,
>  /// where the underlying bus is DMA capable, such as [`pci::Device`](::kernel::pci::Device) or
>  /// [`platform::Device`](::kernel::platform::Device).
> -pub trait Device: AsRef<device::Device<Core>> {}
> +pub trait Device: AsRef<device::Device<Core>> {
> +    /// Set up the device's DMA streaming addressing capabilities.
> +    ///
> +    /// This method is usually called once from `probe()` as soon as the device capabilities are
> +    /// known.
> +    ///
> +    /// # Safety
> +    ///
> +    /// This method must not be called concurrently with any DMA allocation or mapping primitives,
> +    /// such as [`CoherentAllocation::alloc_attrs`].

I'm a bit confused by the use of "concurrently" in this sentence. Do you
mean that it must be called *before* any DMA allocation of mapping
primitives? In this case, wouldn't it be clearer to make the order
explicit?

> +    unsafe fn dma_set_mask(&self, mask: u64) -> Result {

Do we want to allow any u64 as a valid mask? If not, shall we restrict
the accepted values by taking either the parameter to give to
`dma_bit_mask`, or a bit range (similarly to Daniel's bitmask series
[1], which it might make sense to leverage)?

[1]
https://lore.kernel.org/rust-for-linux/20250714-topics-tyr-genmask2-v9-1-9e6422cbadb6@collabora.com/