Re: [PATCH 06/10] dma: Use free_decrypted_pages()

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Mon, 2023-10-23 at 18:22 +0100, Robin Murphy wrote:
> > > 
> > > If something's needed in the fallback path here, what about the
> > > cma_release() paths?
> > 
> > You mean inside cma_release(). If so, unfortunately I think it
> > won't
> > fit great because there are callers that are never dealing with
> > shared
> > memory (huge tlb). The reset-to-private operation does extra work
> > that
> > would be nice to avoid when possible.
> > 
> > The cases I thought exhibited the issue were the two calls sites of
> > dma_set_decrypted(). Playing around with it, I was thinking it
> > might be
> > easier to just fix those to open code leaking the pages on
> > dma_set_decrypted() error. In which case it won't have the re-
> > encrypt
> > problem.
> > 
> > It make's it less fool proof, but more efficient. And
> > free_decrypted_pages() doesn't fit great anyway, as pointed out by
> > Christoph.
> 
> My point is that in dma_direct_alloc(), we get some memory either 
> straight from the page allocator *or* from a CMA area, then call 
> set_memory_decrypted() on it. If the problem is that 
> set_memory_decrypted() can fail and require cleanup, then logically
> if 
> that cleanup is necessary for the dma_free_contiguous()-
> >__free_pages() 
> call, then surely it must also be necessary for the 
> dma_free_contiguous()->cma_release()->free_contig_range()-
> >__free_page() 
> calls.

Oh, I see you are saying the patch misses that case. Yes, makes sense.

Sorry for the confusion. In trying to fix the callers, I waded through
a lot of area's that I didn't have much expertise in and probably
should have marked the whole thing RFC.