Re: [PATCH 2/2] rust: dma: add CoherentHandle for DMA allocations without kernel mapping

From: Gary Guo

Date: Wed Mar 25 2026 - 09:34:29 EST


On Tue Mar 24, 2026 at 10:20 PM GMT, Danilo Krummrich wrote:
> On Tue Mar 24, 2026 at 9:10 PM CET, Gary Guo wrote:
>> On Sat Mar 21, 2026 at 5:27 PM GMT, Danilo Krummrich wrote:
>>> Add CoherentHandle, an opaque DMA allocation type for buffers that are
>>> only ever accessed by hardware. Unlike Coherent<T>, it does not provide
>>> CPU access to the allocated memory.
>>>
>>> CoherentHandle implicitly sets DMA_ATTR_NO_KERNEL_MAPPING and stores the
>>> value returned by dma_alloc_attrs() as an opaque handle
>>> (NonNull<c_void>) rather than a typed pointer, since with this flag the
>>> C API returns an opaque cookie (e.g. struct page *), not a CPU pointer
>>> to the allocated memory.
>>>
>>> Only the DMA bus address is exposed to drivers; the opaque handle is
>>> used solely to free the allocation on drop.
>>>
>>> This commit is for reference only; there is currently no in-tree user.
>>
>> Thinking about this a bit more, instead of creating new separate types, we can
>> probably just have multiple flavour/kind of `Coherent`, and we default to the
>> most common one.
>>
>> Something like
>>
>> pub struct Normal;
>> pub struct NoMapping;
>>
>> struct Coherent<T: KnonwSize + ?Sized, Kind = Normal> {
>> ...
>> }
>>
>> Where for the common functions, they're implemented on `Coherent<T, Kind>` while
>> the ones that require CPU mapping have `impl Coherent<T>`.
>>
>> The benefit is that there's no code duplication.
>
> This was my first thought as well.
>
> But I found it a bit odd that users would still have to define T, then I thought
> we could have a type alias, e.g.
>
> type CoherentHandle = Coherent<[u8], NoMapping>;
>
> In this case the constructor would be CoherentHandle::zeroed_slice(), which I
> find a bit odd as well.
>
> Alternatively, we could have a const SIZE generic on CoherentHandle wrapping
> Coherent<[u8; SIZE], NoMapping>.
>
> Then we'd get CoherentHandle::<SIZE>::zeroed() (which I find much better), but
> we would still have Coherent<T, NoMapping> publically available, which I still
> find a bit odd.

This only gives you fixed size, though. If you want the same type that supports
both a fixed size and dynamic size, then a generic that is either array/slice is
the way to go.

>
> And then I thought, it's probably not worth the additional complexity and a
> separate CoherentHandle type is probably good enough.
>
>> Also, you could also implement `Io` but leave out all `IoCapable` impls, so
>> you may do I/O projections on these and be able to get offseted DMA addresses
>> without having to do manual computation.
>
> This is a reason for having Coherent<T, NoMapping>, but I'm not sure it is an
> improvement in the first place.
>
> If the memory is not mapped on the CPU side it is probably just an opaque
> buffer. And I'm not sure there is a huge advantage in defining a structure
> representing any relevant offsets and then do an I/O projection compared to just
> having constants or an enum defining those offsets.
>
> I also think there's not a huge difference in terms of robustness. In the former
> case, if you mess up the offsets, the buffer size will be wrong (unless multiple
> offsets are wrong and compensate each other) and the device will not properly
> work on runtime.
>
> In the latter case, you either also pass a wrong offset to the device, or you
> get a compile-time error if the offset is out of bounds (which is probably even
> better).
>
> Also, consider the case where the offset into the opaque buffer is dynamic, then
> a type and I/O projections won't help either.

You still can use `io_project!(handle, [start..end]?)` to do the bounds
checking. But as you said, the benefit might be minimal.

Best,
Gary