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.
