Re: [PATCH v3 2/2] rust: dma: use pointer projection infra for `dma_{read, write}` macro

[Benno Lossin]

On Mon Mar 2, 2026 at 2:02 PM CET, Gary Guo wrote:
> From: Gary Guo <gary@xxxxxxxxxxx>
>
> Current `dma_read!`, `dma_write!` macros also use a custom
> `addr_of!()`-based implementation for projecting pointers, which has
> soundness issue as it relies on absence of `Deref` implementation on types.
> It also has a soundness issue where it does not protect against unaligned
> fields (when `#[repr(packed)]` is used) so it can generate misaligned
> accesses.
>
> This commit migrates them to use the general pointer projection
> infrastructure, which handles these cases correctly.
>
> As part of migration, the macro is updated to have an improved surface
> syntax. The current macro have
>
>     dma_read!(a.b.c[d].e.f)
>
> to mean `a.b.c` is a DMA coherent allocation and it should project into it
> with `[d].e.f` and do a read, which is confusing as it makes the indexing
> operator integral to the macro (so it will break if you have an array of
> `CoherentAllocation`, for example).
>
> This also is problematic as we would like to generalize
> `CoherentAllocation` from just slices to arbitrary types.
>
> Make the macro expects `dma_read!(path.to.dma, .path.inside.dma)` as the
> canonical syntax. The index operator is no longer special and is just one
> type of projection (in additional to field projection). Similarly, make
> `dma_write!(path.to.dma, .path.inside.dma, value)` become the canonical
> syntax for writing.
>
> Another issue of the current macro is that it is always fallible. This
> makes sense with existing design of `CoherentAllocation`, but once we
> support fixed size arrays with `CoherentAllocation`, it is desirable to
> have the ability to perform infallible indexing as well, e.g. doing a `[0]`
> index of `[Foo; 2]` is okay and can be checked at build-time, so forcing
> falliblity is non-ideal. To capture this, the macro is changed to use
> `[idx]` as infallible projection and `[idx]?` as fallible index projection
> (those syntax are part of the general projection infra). A benefit of this
> is that while individual indexing operation may fail, the overall
> read/write operation is not fallible.
>
> Fixes: ad2907b4e308 ("rust: add dma coherent allocator abstraction")
> Signed-off-by: Gary Guo <gary@xxxxxxxxxxx>

Reviewed-by: Benno Lossin <lossin@xxxxxxxxxx>

Cheers,
Benno

> ---
>  drivers/gpu/nova-core/gsp.rs      |  14 ++--
>  drivers/gpu/nova-core/gsp/boot.rs |   2 +-
>  drivers/gpu/nova-core/gsp/cmdq.rs |  10 ++-
>  rust/kernel/dma.rs                | 114 +++++++++++++-----------------
>  samples/rust/rust_dma.rs          |  30 ++++----
>  5 files changed, 81 insertions(+), 89 deletions(-)