> On 6 Jul 2026, at 09:44, Gary Guo <[email protected]> wrote:
>
> The primitive read/write use case is covered by the `io_read!` and
> `io_write!` macro. The non-primitive use case was finicky; they should
> either be achieved using `CoherentBox` or `as_ref()/as_mut()` to assert the
> lack of concurrent access, or should be using memcpy-like APIs to express
> the non-atomic and tearable nature.
>
> Reviewed-by: Andreas Hindborg <[email protected]>
> Reviewed-by: Alexandre Courbot <[email protected]>
> Signed-off-by: Gary Guo <[email protected]>
> ---
> rust/kernel/dma.rs | 128 -----------------------------------------------
> samples/rust/rust_dma.rs | 11 ++--
> 2 files changed, 8 insertions(+), 131 deletions(-)
>
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index 1535bc6eec64..6e7ea3b72f2f 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -661,52 +661,6 @@ pub unsafe fn as_mut(&self) -> &mut T {
> // SAFETY: per safety requirement.
> unsafe { &mut *self.as_mut_ptr() }
> }
> -
> - /// Reads the value of `field` and ensures that its type is
> [`FromBytes`].
> - ///
> - /// # Safety
> - ///
> - /// This must be called from the [`dma_read`] macro which ensures that
> the `field` pointer is
> - /// validated beforehand.
> - ///
> - /// Public but hidden since it should only be used from [`dma_read`]
> macro.
> - #[doc(hidden)]
> - pub unsafe fn field_read<F: FromBytes>(&self, field: *const F) -> F {
> - // SAFETY:
> - // - By the safety requirements field is valid.
> - // - Using read_volatile() here is not sound as per the usual rules,
> the usage here is
> - // a special exception with the following notes in place. When
> dealing with a potential
> - // race from a hardware or code outside kernel (e.g. user-space
> program), we need that
> - // read on a valid memory is not UB. Currently read_volatile() is
> used for this, and the
> - // rationale behind is that it should generate the same code as
> READ_ONCE() which the
> - // kernel already relies on to avoid UB on data races. Note that the
> usage of
> - // read_volatile() is limited to this particular case, it cannot be
> used to prevent
> - // the UB caused by racing between two kernel functions nor do they
> provide atomicity.
> - unsafe { field.read_volatile() }
> - }
> -
> - /// Writes a value to `field` and ensures that its type is [`AsBytes`].
> - ///
> - /// # Safety
> - ///
> - /// This must be called from the [`dma_write`] macro which ensures that
> the `field` pointer is
> - /// validated beforehand.
> - ///
> - /// Public but hidden since it should only be used from [`dma_write`]
> macro.
> - #[doc(hidden)]
> - pub unsafe fn field_write<F: AsBytes>(&self, field: *mut F, val: F) {
> - // SAFETY:
> - // - By the safety requirements field is valid.
> - // - Using write_volatile() here is not sound as per the usual
> rules, the usage here is
> - // a special exception with the following notes in place. When
> dealing with a potential
> - // race from a hardware or code outside kernel (e.g. user-space
> program), we need that
> - // write on a valid memory is not UB. Currently write_volatile() is
> used for this, and the
> - // rationale behind is that it should generate the same code as
> WRITE_ONCE() which the
> - // kernel already relies on to avoid UB on data races. Note that the
> usage of
> - // write_volatile() is limited to this particular case, it cannot be
> used to prevent
> - // the UB caused by racing between two kernel functions nor do they
> provide atomicity.
> - unsafe { field.write_volatile(val) }
> - }
> }
>
> impl<T: AsBytes + FromBytes> Coherent<T> {
> @@ -1266,85 +1220,3 @@ fn as_view(self) -> CoherentView<'a, Self::Target> {
> }
> }
> }
> -
> -/// Reads a field of an item from an allocated region of structs.
> -///
> -/// The syntax is of the form `kernel::dma_read!(dma, proj)` where `dma` is
> an expression evaluating
> -/// to a [`Coherent`] and `proj` is a [projection
> specification](kernel::ptr::project!).
> -///
> -/// # Examples
> -///
> -/// ```
> -/// use kernel::device::Device;
> -/// use kernel::dma::{attrs::*, Coherent};
> -///
> -/// struct MyStruct { field: u32, }
> -///
> -/// // SAFETY: All bit patterns are acceptable values for `MyStruct`.
> -/// unsafe impl kernel::transmute::FromBytes for MyStruct{};
> -/// // SAFETY: Instances of `MyStruct` have no uninitialized portions.
> -/// unsafe impl kernel::transmute::AsBytes for MyStruct{};
> -///
> -/// # fn test(alloc: &kernel::dma::Coherent<[MyStruct]>) -> Result {
> -/// let whole = kernel::dma_read!(alloc, [try: 2]);
> -/// let field = kernel::dma_read!(alloc, [panic: 1].field);
> -/// # Ok::<(), Error>(()) }
> -/// ```
> -#[macro_export]
> -macro_rules! dma_read {
> - ($dma:expr, $($proj:tt)*) => {{
> - let dma = &$dma;
> - let ptr = $crate::ptr::project!(
> - $crate::dma::Coherent::as_ptr(dma), $($proj)*
> - );
> - // SAFETY: The pointer created by the projection is within the DMA
> region.
> - unsafe { $crate::dma::Coherent::field_read(dma, ptr) }
> - }};
> -}
> -
> -/// Writes to a field of an item from an allocated region of structs.
> -///
> -/// The syntax is of the form `kernel::dma_write!(dma, proj, val)` where
> `dma` is an expression
> -/// evaluating to a [`Coherent`], `proj` is a
> -/// [projection specification](kernel::ptr::project!), and `val` is the
> value to be written to the
> -/// projected location.
> -///
> -/// # Examples
> -///
> -/// ```
> -/// use kernel::device::Device;
> -/// use kernel::dma::{attrs::*, Coherent};
> -///
> -/// struct MyStruct { member: u32, }
> -///
> -/// // SAFETY: All bit patterns are acceptable values for `MyStruct`.
> -/// unsafe impl kernel::transmute::FromBytes for MyStruct{};
> -/// // SAFETY: Instances of `MyStruct` have no uninitialized portions.
> -/// unsafe impl kernel::transmute::AsBytes for MyStruct{};
> -///
> -/// # fn test(alloc: &kernel::dma::Coherent<[MyStruct]>) -> Result {
> -/// kernel::dma_write!(alloc, [try: 2].member, 0xf);
> -/// kernel::dma_write!(alloc, [panic: 1], MyStruct { member: 0xf });
> -/// # Ok::<(), Error>(()) }
> -/// ```
> -#[macro_export]
> -macro_rules! dma_write {
> - (@parse [$dma:expr] [$($proj:tt)*] [, $val:expr]) => {{
> - let dma = &$dma;
> - let ptr = $crate::ptr::project!(
> - mut $crate::dma::Coherent::as_mut_ptr(dma), $($proj)*
> - );
> - let val = $val;
> - // SAFETY: The pointer created by the projection is within the DMA
> region.
> - unsafe { $crate::dma::Coherent::field_write(dma, ptr, val) }
> - }};
> - (@parse [$dma:expr] [$($proj:tt)*] [.$field:tt $($rest:tt)*]) => {
> - $crate::dma_write!(@parse [$dma] [$($proj)* .$field] [$($rest)*])
> - };
> - (@parse [$dma:expr] [$($proj:tt)*] [[$flavor:ident: $index:expr]
> $($rest:tt)*]) => {
> - $crate::dma_write!(@parse [$dma] [$($proj)* [$flavor: $index]]
> [$($rest)*])
> - };
> - ($dma:expr, $($rest:tt)*) => {
> - $crate::dma_write!(@parse [$dma] [] [$($rest)*])
> - };
> -}
> diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
> index 5046b4628d0e..4af46e99d2dd 100644
> --- a/samples/rust/rust_dma.rs
> +++ b/samples/rust/rust_dma.rs
> @@ -12,6 +12,10 @@
> Device,
> DmaMask, //
> },
> + io::{
> + io_project,
> + io_read, //
> + },
> page, pci,
> prelude::*,
> scatterlist::{Owned, SGTable},
> @@ -77,7 +81,8 @@ fn probe<'bound>(
> Coherent::zeroed_slice(pdev.as_ref(), TEST_VALUES.len(),
> GFP_KERNEL)?;
>
> for (i, value) in TEST_VALUES.into_iter().enumerate() {
> - kernel::dma_write!(ca, [try: i], MyStruct::new(value.0,
> value.1));
> + // SAFETY: `ca` is not yet shared with device or other
> threads.
> + unsafe { *io_project!(ca, [panic: i]).as_mut() =
> MyStruct::new(value.0, value.1) };
> }
>
> let size = 4 * page::PAGE_SIZE;
> @@ -97,8 +102,8 @@ fn probe<'bound>(
> impl DmaSampleDriver {
> fn check_dma(&self) {
> for (i, value) in TEST_VALUES.into_iter().enumerate() {
> - let val0 = kernel::dma_read!(self.ca, [panic: i].h);
> - let val1 = kernel::dma_read!(self.ca, [panic: i].b);
> + let val0 = io_read!(self.ca, [panic: i].h);
> + let val1 = io_read!(self.ca, [panic: i].b);
>
> assert_eq!(val0, value.0);
> assert_eq!(val1, value.1);
>
> --
> 2.54.0
>
>
Reviewed-by: Daniel Almeida <[email protected]>
