Re: [RFC PATCH] dma: swiotlb: Size shared default pools for memory encryption

Catalin Marinas <[email protected]> writes:

>
> I also wonder whether we could address Will's pKVM request not to
> allocate a bounce buffer once pKVM guests will start reporting
> CC_ATTR_GUEST_MEM_ENCRYPT. Some simple heuristic: if a
> restricted-dma-pool is advertised in DT (it will end up in
> rmem_swiotlb_setup()), skip resizing the default swiotlb. It's not
> perfect but the bounce buffer can be overridden on the command line.

I have a follow-up cleanup patch where I switch the swiotlb
initialization reason to a flag value. With that, we now have
SWIOTLB_INIT_CC_SHARED.

The challenge is that it is still not clear when pKVM would want to use
an unencrypted swiotlb pool for DMA bouncing. I would expect pKVM to
have some additional condition based on which it chooses either the
restricted-dma-pool or the default swiotlb pool.

Using the presence of a restricted-dma-pool to decide whether the
default swiotlb pool should be unencrypted is one option, but I am not
sure that is the right abstraction.

modified   arch/arm64/mm/init.c
@@ -342,7 +342,7 @@ void __init arch_mm_preinit(void)
 	bool cc_guest = is_realm_world();
 
 	if (cc_guest)
-		swiotlb_mark_default_cc_shared();
+		flags |= SWIOTLB_INIT_CC_SHARED;
 	else if (max_pfn > PFN_DOWN(arm64_dma_phys_limit))
 		flags |= SWIOTLB_INIT_ADDRESSING_LIMIT;
 
modified   arch/powerpc/mm/mem.c
@@ -289,9 +289,8 @@ void __init arch_mm_preinit(void)
 #ifdef CONFIG_SWIOTLB
 	if (is_secure_guest()) {
 		/* The host can access DMA buffers only through the SWIOTLB. */
-		swiotlb_mark_default_cc_shared();
 		ppc_swiotlb_enable = 1;
-		ppc_swiotlb_flags |= SWIOTLB_ANY;
+		ppc_swiotlb_flags |= SWIOTLB_INIT_CC_SHARED | SWIOTLB_ANY;
 	}
 
 	/*
modified   arch/s390/mm/init.c
@@ -163,12 +163,10 @@ static void __init pv_init(void)
 	if (!is_prot_virt_guest())
 		return;
 
-	swiotlb_mark_default_cc_shared();
-
 	virtio_set_mem_acc_cb(virtio_require_restricted_mem_acc);
 
 	/* make sure bounce buffers are shared */
-	swiotlb_init(SWIOTLB_VERBOSE);
+	swiotlb_init(SWIOTLB_VERBOSE | SWIOTLB_INIT_CC_SHARED);
 	swiotlb_update_mem_attributes();
 }
 
modified   arch/x86/mm/mem_encrypt.c
@@ -14,6 +14,7 @@
 #include <linux/mem_encrypt.h>
 #include <linux/virtio_anchor.h>
 
+#include <asm/iommu.h>
 #include <asm/sev.h>
 
 /* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
@@ -109,7 +110,7 @@ void __init mem_encrypt_setup_arch(void)
 		snp_fixup_e820_tables();
 
 	if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
-		swiotlb_mark_default_cc_shared();
+		x86_swiotlb_flags |= SWIOTLB_INIT_CC_SHARED;
 
 	if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
 		return;
modified   include/linux/swiotlb.h
@@ -18,6 +18,8 @@ struct scatterlist;
 #define SWIOTLB_ANY	(1 << 1) /* allow any memory for the buffer */
 /* Initialize a default-sized pool for devices with limited DMA addressing. */
 #define SWIOTLB_INIT_ADDRESSING_LIMIT	(1 << 2)
+/* Initialize a shared default pool for confidential-computing systems. */
+#define SWIOTLB_INIT_CC_SHARED		(1 << 3)
 
 /*
  * Maximum allowable number of contiguous slabs to map,
@@ -191,7 +193,6 @@ void swiotlb_dev_init(struct device *dev);
 size_t swiotlb_max_mapping_size(struct device *dev);
 bool is_swiotlb_allocated(void);
 bool is_swiotlb_active(struct device *dev);
-void __init swiotlb_mark_default_cc_shared(void);
 void __init swiotlb_adjust_size(unsigned long size);
 phys_addr_t default_swiotlb_base(void);
 phys_addr_t default_swiotlb_limit(void);
@@ -231,10 +232,6 @@ static inline bool is_swiotlb_active(struct device *dev)
 	return false;
 }
 
-static inline void swiotlb_mark_default_cc_shared(void)
-{
-}
-
 static inline void swiotlb_adjust_size(unsigned long size)
 {
 }
modified   kernel/dma/swiotlb.c
@@ -373,11 +373,6 @@ static void __init *swiotlb_memblock_alloc(unsigned long nslabs,
 	return tlb;
 }
 
-void __init swiotlb_mark_default_cc_shared(void)
-{
-	io_tlb_default_mem.cc_shared = true;
-}
-
 static void __init swiotlb_adjust_cc_attributes(void)
 {
 	unsigned long size;
@@ -429,7 +424,7 @@ static bool __init swiotlb_should_init(unsigned int flags)
 	if (swiotlb_force_bounce)
 		return true;
 
-	if (io_tlb_default_mem.cc_shared)
+	if (flags & SWIOTLB_INIT_CC_SHARED)
 		return true;
 
 	return false;
@@ -451,6 +446,9 @@ void __init swiotlb_init_remap(unsigned int flags,
 	if (!swiotlb_should_init(flags))
 		return;
 
+	if (flags & SWIOTLB_INIT_CC_SHARED)
+		io_tlb_default_mem.cc_shared = true;
+
 	io_tlb_default_mem.force_bounce = swiotlb_force_bounce;
 
 #ifdef CONFIG_SWIOTLB_DYNAMIC