Re: [PATCH 2/4] rust: dma: generalize `dma_{read,write}` macro

["Benno Lossin" <lossin@kernel.org>]

On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
> +    (@parse [$dma:expr] [$($proj:tt)*] [, $val:expr]) => {
> +        let ptr = $crate::project_pointer!(
> +            mut $crate::dma::CoherentAllocation::as_mut_ptr(&$dma), $($proj)*
> +        );
> +        let val = $val;
> +        // SAFETY: pointer created by projection is within DMA region.
> +        unsafe { $crate::dma::CoherentAllocation::field_write(&$dma, ptr, val) }

This evaluates `$dma` for a second time (and also places it inside an
`unsafe` block).

> +    };

Missing surrounding `{}` to allow this in expression position?

> +    (@parse [$dma:expr] [$($proj:tt)*] [.$field:tt $($rest:tt)*]) => {
> +        $crate::dma_write!(@parse [$dma] [$($proj)* .$field] [$($rest)*])
> +    };
> +    (@parse [$dma:expr] [$($proj:tt)*] [[$index:expr]? $($rest:tt)*]) => {
> +        $crate::dma_write!(@parse [$dma] [$($proj)* [$index]?] [$($rest)*])
> +    };
> +    (@parse [$dma:expr] [$($proj:tt)*] [[$index:expr] $($rest:tt)*]) => {
> +        $crate::dma_write!(@parse [$dma] [$($proj)* [$index]] [$($rest)*])
> +    };
> +    ($dma:expr, $($rest:tt)*) => {
> +        $crate::dma_write!(@parse [$dma] [] [$($rest)*])
>      };

I'm wondering if this also works:
    
    ($dma:expr, $($(.$field:ident)? $([$index:expr])?)*, $val:expr) => {{
        let dma = &$dma;
        let ptr = $crate::project_pointer!(
            mut $crate::dma::CoherentAllocation::as_mut_ptr(dma),
            $($(.$field)? $([$index])?)*,
        );
        let val = $val;
        // SAFETY: pointer created by projection is within DMA region.
        unsafe { $crate::dma::CoherentAllocation::field_write(dma, ptr, val) }
    }}

>  }
> diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
> index 9c45851c876e..b772ada2c65c 100644
> --- a/samples/rust/rust_dma.rs
> +++ b/samples/rust/rust_dma.rs
> @@ -68,7 +68,7 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E
>                  CoherentAllocation::alloc_coherent(pdev.as_ref(), TEST_VALUES.len(), GFP_KERNEL)?;
>  
>              for (i, value) in TEST_VALUES.into_iter().enumerate() {
> -                kernel::dma_write!(ca[i] = MyStruct::new(value.0, value.1))?;
> +                kernel::dma_write!(ca, [i]?, MyStruct::new(value.0, value.1));
>              }
>  
>              let size = 4 * page::PAGE_SIZE;
> @@ -91,17 +91,17 @@ fn drop(self: Pin<&mut Self>) {
>          dev_info!(self.pdev, "Unload DMA test driver.\n");
>  
>          for (i, value) in TEST_VALUES.into_iter().enumerate() {
> -            let val0 = kernel::dma_read!(self.ca[i].h);
> -            let val1 = kernel::dma_read!(self.ca[i].b);
> -            assert!(val0.is_ok());
> -            assert!(val1.is_ok());
> +            let result = (|| -> Result<_> {
> +                let val0 = kernel::dma_read!(self.ca, [i]?.h);
> +                let val1 = kernel::dma_read!(self.ca, [i]?.b);
>  
> -            if let Ok(val0) = val0 {
>                  assert_eq!(val0, value.0);
> -            }
> -            if let Ok(val1) = val1 {
>                  assert_eq!(val1, value.1);
> -            }
> +
> +                Ok(())
> +            })();

I dislike that we have to reintroduce the budget-try block here. Ideally
we could add something like `try` at the beginning of the macro and then
automatically add the try block. Feel free to make that a future series.

Cheers,
Benno

> +
> +            assert!(result.is_ok());
>          }
>  
>          for (i, entry) in self.sgt.iter().enumerate() {