Re: Memory clearing in swiotlb_update_mem_attributes()

["Kirill A. Shutemov" <kirill@shutemov.name>]

On Wed, Jan 05, 2022 at 06:12:34AM -0800, Christoph Hellwig wrote:
> On Wed, Jan 05, 2022 at 08:06:10AM -0600, Tom Lendacky wrote:
> > On 1/4/22 4:49 PM, Kirill A. Shutemov wrote:
> > > Hi Tom,
> > > 
> > > For larger TDX VM, memset() after set_memory_decrypted() in
> > > swiotlb_update_mem_attributes() takes substantial portion of boot time.
> > > 
> > > It makes me wounder why do we need it there? Malicious VMM can mess with
> > > decrypted/shared buffer at any point and for normal use it will be
> > > populated with real data anyway.
> > > 
> > > Can we drop it?
> > 
> > Probably more a question for Christoph. Does SWIOTLB need to be initialized
> > to zeroes? If it does, then the memset after the set_memory_decrypted() is
> > required, otherwise it will appear as ciphertext to SWIOTLB.
> 
> While the traditional swiotlb initialization zeroes it I can't really
> see any reason why we would want to zero it.  If we really care about
> not leaking data to the device we'd need to zero the padding at mapping
> time.

Does the patch below look fine?

From ab1aa6abbdbba9e34ac7b86e5af1f9f488afaa07 Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Date: Wed, 5 Jan 2022 18:48:01 +0300
Subject: [PATCH] swiotlb: Do not zero buffer in set_memory_decrypted()

For larger TDX VM, memset() after set_memory_decrypted() in
swiotlb_update_mem_attributes() takes substantial portion of boot time.

Zeroing doesn't serve any functional purpose. Malicious VMM can mess
with decrypted/shared buffer at any point.

Remove the memset().

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 kernel/dma/swiotlb.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 8e840fbbed7c..4546c834accb 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -172,7 +172,6 @@ void __init swiotlb_update_mem_attributes(void)
 	vaddr = phys_to_virt(mem->start);
 	bytes = PAGE_ALIGN(mem->nslabs << IO_TLB_SHIFT);
 	set_memory_decrypted((unsigned long)vaddr, bytes >> PAGE_SHIFT);
-	memset(vaddr, 0, bytes);
 }
 
 static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
-- 
 Kirill A. Shutemov
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu