Message-Id: <E7B266B2-4CD6-4C20-9969-AC5BEA57C5A4@collabora.com>
Date: Tue, 26 Aug 2025 14:10:13 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: akpm@...ux-foundation.org,
 ojeda@...nel.org,
 alex.gaynor@...il.com,
 boqun.feng@...il.com,
 gary@...yguo.net,
 bjorn3_gh@...tonmail.com,
 lossin@...nel.org,
 a.hindborg@...nel.org,
 aliceryhl@...gle.com,
 tmgross@...ch.edu,
 abdiel.janulgue@...il.com,
 acourbot@...dia.com,
 jgg@...pe.ca,
 lyude@...hat.com,
 robin.murphy@....com,
 rust-for-linux@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/5] rust: dma: implement DataDirection



> On 25 Aug 2025, at 10:24, Danilo Krummrich <dakr@...nel.org> wrote:
> 
> Add the `DataDirection` struct, a newtype wrapper around the C
> `enum dma_data_direction`.
> 
> This provides a type-safe Rust interface for specifying the direction of
> DMA transfers.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@...gle.com>
> Reviewed-by: Alexandre Courbot <acourbot@...dia.com>
> Signed-off-by: Danilo Krummrich <dakr@...nel.org>
> ---
> rust/bindings/bindings_helper.h |  1 +
> rust/kernel/dma.rs              | 68 +++++++++++++++++++++++++++++++++
> 2 files changed, 69 insertions(+)
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 0e140e07758b..c2cc52ee9945 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -47,6 +47,7 @@
> #include <linux/cpumask.h>
> #include <linux/cred.h>
> #include <linux/device/faux.h>
> +#include <linux/dma-direction.h>
> #include <linux/dma-mapping.h>
> #include <linux/errname.h>
> #include <linux/ethtool.h>
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index 2bc8ab51ec28..27b25f041f32 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -244,6 +244,74 @@ pub mod attrs {
>     pub const DMA_ATTR_PRIVILEGED: Attrs = Attrs(bindings::DMA_ATTR_PRIVILEGED);
> }
> 
> +/// DMA data direction.
> +///
> +/// Corresponds to the C [`enum dma_data_direction`].
> +///
> +/// [`enum dma_data_direction`]: srctree/include/linux/dma-direction.h
> +#[derive(Copy, Clone, PartialEq, Eq, Debug)]
> +#[repr(u32)]
> +pub enum DataDirection {
> +    /// The DMA mapping is for bidirectional data transfer.
> +    ///
> +    /// This is used when the buffer can be both read from and written to by the device.
> +    /// The cache for the corresponding memory region is both flushed and invalidated.
> +    Bidirectional = Self::const_cast(bindings::dma_data_direction_DMA_BIDIRECTIONAL),
> +
> +    /// The DMA mapping is for data transfer from memory to the device (write).
> +    ///
> +    /// The CPU has prepared data in the buffer, and the device will read it.
> +    /// The cache for the corresponding memory region is flushed before device access.
> +    ToDevice = Self::const_cast(bindings::dma_data_direction_DMA_TO_DEVICE),
> +
> +    /// The DMA mapping is for data transfer from the device to memory (read).
> +    ///
> +    /// The device will write data into the buffer for the CPU to read.
> +    /// The cache for the corresponding memory region is invalidated before CPU access.
> +    FromDevice = Self::const_cast(bindings::dma_data_direction_DMA_FROM_DEVICE),
> +
> +    /// The DMA mapping is not for data transfer.
> +    ///
> +    /// This is primarily for debugging purposes. With this direction, the DMA mapping API
> +    /// will not perform any cache coherency operations.
> +    None = Self::const_cast(bindings::dma_data_direction_DMA_NONE),
> +}
> +
> +impl DataDirection {
> +    /// Casts the bindgen-generated enum type to a `u32` at compile time.
> +    ///
> +    /// This function will cause a compile-time error if the underlying value of the
> +    /// C enum is out of bounds for `u32`.
> +    const fn const_cast(val: bindings::dma_data_direction) -> u32 {
> +        // CAST: The C standard allows compilers to choose different integer types for enums.
> +        // To safely check the value, we cast it to a wide signed integer type (`i128`)
> +        // which can hold any standard C integer enum type without truncation.
> +        let wide_val = val as i128;
> +
> +        // Check if the value is outside the valid range for the target type `u32`.
> +        // CAST: `u32::MAX` is cast to `i128` to match the type of `wide_val` for the comparison.
> +        if wide_val < 0 || wide_val > u32::MAX as i128 {
> +            // Trigger a compile-time error in a const context.
> +            build_error!("C enum value is out of bounds for the target type `u32`.");
> +        }
> +
> +        // CAST: This cast is valid because the check above guarantees that `wide_val`
> +        // is within the representable range of `u32`.
> +        wide_val as u32
> +    }
> +}
> +
> +impl From<DataDirection> for bindings::dma_data_direction {
> +    /// Returns the raw representation of [`enum dma_data_direction`].
> +    fn from(direction: DataDirection) -> Self {
> +        // CAST: `direction as u32` gets the underlying representation of our `#[repr(u32)]` enum.
> +        // The subsequent cast to `Self` (the bindgen type) assumes the C enum is compatible
> +        // with the enum variants of `DataDirection`, which is a valid assumption given our
> +        // compile-time checks.
> +        direction as u32 as Self
> +    }
> +}
> +
> /// An abstraction of the `dma_alloc_coherent` API.
> ///
> /// This is an abstraction around the `dma_alloc_coherent` API which is used to allocate and map
> -- 
> 2.51.0
> 
> 

Reviewed-by: Daniel Almeida <daniel.almeida@...labora.com>

Powered by blists - more mailing lists