* Memory clearing in swiotlb_update_mem_attributes() @ 2022-01-04 22:49 Kirill A. Shutemov 2022-01-05 14:06 ` Tom Lendacky via iommu 0 siblings, 1 reply; 5+ messages in thread From: Kirill A. Shutemov @ 2022-01-04 22:49 UTC (permalink / raw) To: Tom Lendacky; +Cc: Christoph Hellwig, iommu 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? -- Kirill A. Shutemov _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Memory clearing in swiotlb_update_mem_attributes() 2022-01-04 22:49 Memory clearing in swiotlb_update_mem_attributes() Kirill A. Shutemov @ 2022-01-05 14:06 ` Tom Lendacky via iommu 2022-01-05 14:12 ` Christoph Hellwig 2022-01-10 15:58 ` Tom Lendacky via iommu 0 siblings, 2 replies; 5+ messages in thread From: Tom Lendacky via iommu @ 2022-01-05 14:06 UTC (permalink / raw) To: Kirill A. Shutemov; +Cc: Christoph Hellwig, iommu 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. If I get some time over the next couple of days, I can also try and test to see what happens. Thanks, Tom > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Memory clearing in swiotlb_update_mem_attributes() 2022-01-05 14:06 ` Tom Lendacky via iommu @ 2022-01-05 14:12 ` Christoph Hellwig 2022-01-05 15:58 ` Kirill A. Shutemov 2022-01-10 15:58 ` Tom Lendacky via iommu 1 sibling, 1 reply; 5+ messages in thread From: Christoph Hellwig @ 2022-01-05 14:12 UTC (permalink / raw) To: Tom Lendacky; +Cc: Christoph Hellwig, Kirill A. Shutemov, iommu 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. _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Memory clearing in swiotlb_update_mem_attributes() 2022-01-05 14:12 ` Christoph Hellwig @ 2022-01-05 15:58 ` Kirill A. Shutemov 0 siblings, 0 replies; 5+ messages in thread From: Kirill A. Shutemov @ 2022-01-05 15:58 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Tom Lendacky, iommu 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: Memory clearing in swiotlb_update_mem_attributes() 2022-01-05 14:06 ` Tom Lendacky via iommu 2022-01-05 14:12 ` Christoph Hellwig @ 2022-01-10 15:58 ` Tom Lendacky via iommu 1 sibling, 0 replies; 5+ messages in thread From: Tom Lendacky via iommu @ 2022-01-10 15:58 UTC (permalink / raw) To: Kirill A. Shutemov; +Cc: Christoph Hellwig, iommu On 1/5/22 8:06 AM, 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. > > If I get some time over the next couple of days, I can also try and test > to see what happens. FWIW, I removed the memset() and it doesn't appear to cause any issues. My tests weren't extensive (boot, kernel build, netperf/iperf), though. Thanks, Tom > > Thanks, > Tom > >> _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-10 15:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-04 22:49 Memory clearing in swiotlb_update_mem_attributes() Kirill A. Shutemov 2022-01-05 14:06 ` Tom Lendacky via iommu 2022-01-05 14:12 ` Christoph Hellwig 2022-01-05 15:58 ` Kirill A. Shutemov 2022-01-10 15:58 ` Tom Lendacky via iommu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.