Re: [PATCH] crypto: marvell/cesa - Avoid empty transfer descriptor

Arnd Bergmann
2025-05-21T09:59:10Z

On Wed, May 21, 2025, at 11:16, Herbert Xu wrote:
> On Wed, May 21, 2025 at 01:06:43PM +0800, Herbert Xu wrote:
>>
>> Can you please try this patch to see if it makes a difference?
>
> Actually, please try this one instead.
>

>
> memcpy(ahashdreq->cache, creq->cache, creq->cache_ptr);
> + arch_sync_dma_for_device(ahashdreq->cache_dma, creq->cache_ptr,
> DMA_TO_DEVICE);
>
> return mv_cesa_dma_add_data_transfer(chain,
> CESA_SA_DATA_SRAM_OFFSET,

I did not see the entire background of the discussion, but would
point out that this is not supposed to work at all:

- arch_sync_dma_for_device() is not callable from drivers,
on a streaming mapping one would instead have to call
dma_sync_single_for_device() with the correct device.

- in the mainline kernel, the memory comes from a coherent
dma pool, and the devices that the driver binds (Marvell
Armada 32-bit) do not have cache-coherent DMA, so the
buffer itself is in an uncached page and you must never
do cache management operations on it.

If the allocation is changed from a coherent dma pool to
a dma_alloc_noncoherent() call, then the
dma_sync_single_for_device() would indeed be the correct
interface for passing the buffer to the device, but for
a coherent buffer, a simple 'dma_wmb()' as you suggested
earlier would be the right interface.

Depending on the size of the data, the streaming mapping
would likely be significantly faster than the coherent
dma pool object.

Arnd