Re: [PATCH crypto 2/2] crypto: safexcel - Map AEAD buffers with accurate DMA directions

[Ralf Lici <ralf@mandelbit.com>]

Hi Herbert,

On Fri, 16 Oct 2026 20:13:30 +1100, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Tue, Sep 15, 2026 at 01:32:12PM +0200, Ralf Lici wrote:
> >
> > @@ -748,8 +833,17 @@ static int safexcel_send_req(struct crypto_async_request *base, int ring,
> >  			return -EINVAL;
> >  		}
> >  		if (sreq->nr_src > 0) {
> > -			src_mapped = dma_map_sg(priv->dev, src, sreq->nr_src,
> > -						DMA_BIDIRECTIONAL);
> > +			if (ctx->aead)
> > +				src_mapped =
> > +					safexcel_map_aead(priv->dev, src,
> > +							  sreq->nr_src,
> > +							  sreq->output_offset,
> > +							  sreq->output_len,
> > +							  true);
> > +			else
> > +				src_mapped = dma_map_sg(priv->dev, src,
> > +							sreq->nr_src,
> > +							DMA_BIDIRECTIONAL);
>
> This path doesn't actualy matter because BIDIRECTIONAL is safe
> even if you use it unnecessarily.
>
> > @@ -775,8 +869,17 @@ static int safexcel_send_req(struct crypto_async_request *base, int ring,
> >  		}
> >  
> >  		if (sreq->nr_dst > 0) {
> > -			dst_mapped = dma_map_sg(priv->dev, dst, sreq->nr_dst,
> > -						DMA_FROM_DEVICE);
>
> So this is the really buggy path.  Why don't you just change it to
> BIDRECTIONAL, perhaps with a comment referring to the assoc data?
>

I tried that simplification on the MT7986 before respinning, but
unfortunately mapping the complete AEAD destination DMA_BIDIRECTIONAL
does not fix the corruption.

The relevant ovpn layout is physically:

    [ AAD ][ tag ][ payload ]

and is described to the AEAD implementation in logical order as:

    [ AAD ][ payload ][ tag ]

The entries do not overlap in bytes, but a small AAD entry and an
adjacent output entry can share a cache line. When mapped
DMA_BIDIRECTIONAL on this non-coherent system, the AAD entry is bounced.
After the accelerator has written the output to RAM, unmapping the AAD
copies its unchanged bytes back through the CPU. This dirties the CPU's
stale copy of the cache line corresponding to the memory where part of
the accelerator output now resides. Cache maintenance, while unmapping
the adjacent output entry, can then clean that line and overwrite the
output with stale pre-crypto bytes.

This also affects the in-place path. Although an in-place request uses
the same scatterlist as source and destination, its AAD entry remains
input-only. Mapping the whole list DMA_BIDIRECTIONAL therefore gives
that entry an unnecessary and harmful copyback.

So I don't think the source/in-place handling can be dropped on this
platform. I can add the physical/logical layout and the simplified A/B
result to the next cover letter to make the reason for the finer-grained
mapping clearer.

Thanks!

-- 
Ralf Lici
Mandelbit Srl