On 2021-11-09 14:10, Christoph Hellwig wrote:
On Thu, Nov 04, 2021 at 12:35:41PM +0000, Robin Murphy wrote:
On 2021-10-21 10:06, Christoph Hellwig wrote:
When dma_set_decrypted fails for the remapping case in dma_direct_alloc
we also need to unmap the pages before freeing them.
Signed-off-by: Christoph Hellwig <[email protected]>
---
kernel/dma/direct.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index d4d54af31a341..2fef8dd401fe9 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -230,7 +230,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
if (!ret)
goto out_free_pages;
if (dma_set_decrypted(dev, ret, size))
I was going to say just stick the vunmap() in here to avoid adding yet more
messy conditionals, but one rabbit hole later... Given that the
dma_pgprot() we've just passed to dma_common_pages_remap() already adds in
pgprot_decrypted, why is this even here at all?
Good point. This combination is pretty much untested anyway as the
architectures that support memory encryption never remap, but yes
I think the best is if gets removed.
Right, I carried on looking out of curiosity, and of the two other
architectures, it would definitely be broken on PowerPC whose
set_memory_decrypted() assumes it can call virt_to_phys() on the given
VA, while s390 appears to pass the VA straight to its ultravisor call so
I'm not sure what the deal is there.
I suspect we might run into this properly on arm64, where some of the
various protected virtualisation prototypes are currently banking on
hooking into the memory encryption APIs, but even then I'd rather fix
said APIs properly such that vmap() with pgprot_decrypted() always ends
up doing the right thing - as it stands, having "generic" functionality
which may or may not actually do what you think it should is terrible.
Cheers,
Robin.
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
