Skip to contentSkip to conversation

dma-mapping: Use DMA_ATTR_CC_SHARED through direct, pool and swiotlb paths

PATCH Superseded Aneesh Kumar K.V (Arm) 4 participants 19 messages Apr 27 – May 12

Subsystems (7)

v3 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

v3 cover dma-mapping: Use DMA_ATTR_CC_SHARED through direct, pool and swiotlb paths

This series propagates DMA_ATTR_CC_SHARED through the dma-direct, dma-pool, and swiotlb paths so that encrypted and decrypted DMA buffers are handled consistently.

Today, the direct DMA path mostly relies on force_dma_unencrypted() for shared/decrypted buffer handling. This series consolidates the force_dma_unencrypted() checks in the top-level functions and ensures that the remaining DMA interfaces use DMA attributes to make the correct decisions.

The series:
- moves swiotlb-backed allocations out of __dma_direct_alloc_pages(),
- propagates DMA_ATTR_CC_SHARED through the dma-direct alloc/free paths
- teaches the atomic DMA pools to track encrypted versus decrypted state
- tracks swiotlb pool encryption state and enforces strict pool selection
- centralizes encrypted/decrypted pgprot handling in dma_pgprot() using DMA attributes
- passes DMA attributes down to dma_capable() so capability checks can validate whether the selected DMA address encoding matches DMA_ATTR_CC_SHARED
- makes dma_direct_map_phys() choose the DMA address encoding from DMA_ATTR_CC_SHARED and fall back to swiotlb when a shared DMA request cannot use the direct mapping, which lets arm64 and x86 CCA guests stop relying on SWIOTLB_FORCE for DMA mappings
- use the selected swiotlb pool state to derive the returned DMA address.

Changes from v2:
https://lore.kernel.org/all/[email protected]
* pass attrs to dma_capable() and update direct, swiotlb, Xen swiotlb, and x86 GART paths so the capability checks see the DMA address attr value DMA_ATTR_CC_SHARED.
* rework dma_direct_map_phys() so DMA_ATTR_CC_SHARED selects phys_to_dma_unencrypted() while the default path uses phys_to_dma_encrypted(), with swiotlb fallback when the requested shared/private state cannot be satisfied by a direct DMA address.
* stop relying on SWIOTLB_FORCE for arm64 and x86 CC guest DMA mappings; swiotlb is still enabled there, but shared mappings is now selected through the generic dma_direct_map_phys()/dma_capable() decision instead of a global force-bounce flag.

Changes from v1:
https://lore.kernel.org/all/[email protected]
* rebased to latest kernel (change from DMA_ATTR_CC_DECRYPTED -> DMA_ATTR_CC_SHARED)
* update the alloc path so DMA_ATTR_CC_SHARED is not a caller-visible attribute.

Cc Robin Murphy <[email protected]>
Cc Marek Szyprowski <[email protected]>
Cc Will Deacon <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Steven Price <[email protected]>
Cc Suzuki K Poulose <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Jiri Pirko <[email protected]>
Cc Jason Gunthorpe <[email protected]>
Cc Mostafa Saleh <[email protected]>
Cc Petr Tesarik <[email protected]>
Cc Alexey Kardashevskiy <[email protected]>
Cc Dan Williams <[email protected]>
Cc Xu Yilun <[email protected]>

Aneesh Kumar K.V (Arm) (9):
dma-direct: swiotlb: handle swiotlb alloc/free outside
__dma_direct_alloc_pages
dma-direct: use DMA_ATTR_CC_SHARED in alloc/free paths
dma-pool: track decrypted atomic pools and select them via attrs
dma: swiotlb: track pool encryption state and honor DMA_ATTR_CC_SHARED
dma-mapping: make dma_pgprot() honor DMA_ATTR_CC_SHARED
dma-direct: pass attrs to dma_capable() for DMA_ATTR_CC_SHARED checks
dma-direct: make dma_direct_map_phys() honor DMA_ATTR_CC_SHARED
dma-direct: set decrypted flag for remapped DMA allocations
dma-direct: select DMA address encoding from DMA_ATTR_CC_SHARED

 arch/arm64/mm/init.c          |   4 +-
 arch/x86/kernel/amd_gart_64.c |  30 +++---
 arch/x86/kernel/pci-dma.c     |   4 +-
 drivers/iommu/dma-iommu.c     |   2 +-
 drivers/xen/swiotlb-xen.c     |   9 +-
 include/linux/dma-direct.h    |  19 +++-
 include/linux/dma-map-ops.h   |   2 +-
 include/linux/swiotlb.h       |   7 +-
 kernel/dma/direct.c           | 189 ++++++++++++++++++++++++++--------
 kernel/dma/direct.h           |  32 +++---
 kernel/dma/mapping.c          |  16 ++-
 kernel/dma/pool.c             | 154 +++++++++++++++++----------
 kernel/dma/swiotlb.c          |  93 +++++++++++++----
 13 files changed, 393 insertions(+), 168 deletions(-)

base-commit: dd6c438c3e64a5ff0b5d7e78f7f9be547803ef1b

Signature · 1 line
2.43.0
#1 lore raw

PATCH v3 1/9 dma-direct: swiotlb: handle swiotlb alloc/free outside __dma_direct_alloc_pages

Move swiotlb allocation out of __dma_direct_alloc_pages() and handle it in dma_direct_alloc() / dma_direct_alloc_pages().

This is needed for follow-up changes that simplify the handling of memory encryption/decryption based on the DMA attribute flags.

swiotlb backing pages are already mapped decrypted by swiotlb_update_mem_attributes() and rmem_swiotlb_device_init(), so dma-direct should not call dma_set_decrypted() on allocation nor dma_set_encrypted() on free for swiotlb-backed memory.

Update alloc/free paths to detect swiotlb-backed pages and skip encrypt/decrypt transitions for those paths. Keep the existing highmem rejection in dma_direct_alloc_pages() for swiotlb allocations.

Only for "restricted-dma-pool", we currently set for_alloc = true, while rmem_swiotlb_device_init() decrypts the whole pool up front. This pool is typically used together with "shared-dma-pool", where the shared region is accessed after remap/ioremap and the returned address is suitable for decrypted memory access. So existing code paths remain valid.

---
 kernel/dma/direct.c | 44 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 7 deletions(-)
kernel/dma/direct.c +37 -7
index ec887f443741..b958f150718a 100644
@@ -125,9 +125,6 @@ static struct page *__dma_direct_alloc_pages(struct device *dev, size_t size,
 
 	WARN_ON_ONCE(!PAGE_ALIGNED(size));
 
-	if (is_swiotlb_for_alloc(dev))
-		return dma_direct_alloc_swiotlb(dev, size);
-
 	gfp |= dma_direct_optimal_gfp_mask(dev, &phys_limit);
 	page = dma_alloc_contiguous(dev, size, gfp);
 	if (page) {
@@ -204,6 +201,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 		dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
 {
 	bool remap = false, set_uncached = false;
+	bool mark_mem_decrypt = true;
 	struct page *page;
 	void *ret;
 
@@ -250,11 +248,21 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 	    dma_direct_use_pool(dev, gfp))
 		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
 
+	if (is_swiotlb_for_alloc(dev)) {
+		page = dma_direct_alloc_swiotlb(dev, size);
+		if (page) {
+			mark_mem_decrypt = false;
+			goto setup_page;
+		}
+		return NULL;
+	}
+
 	/* we always manually zero the memory once we are done */
 	page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO, true);
 	if (!page)
 		return NULL;
 
+setup_page:
 	/*
 	 * dma_alloc_contiguous can return highmem pages depending on a
 	 * combination the cma= arguments and per-arch setup.  These need to be
@@ -281,7 +289,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 			goto out_free_pages;
 	} else {
 		ret = page_address(page);
-		if (dma_set_decrypted(dev, ret, size))
+		if (mark_mem_decrypt && dma_set_decrypted(dev, ret, size))
 			goto out_leak_pages;
 	}
 
@@ -298,7 +306,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 	return ret;
 
 out_encrypt_pages:
-	if (dma_set_encrypted(dev, page_address(page), size))
+	if (mark_mem_decrypt && dma_set_encrypted(dev, page_address(page), size))
 		return NULL;
 out_free_pages:
 	__dma_direct_free_pages(dev, page, size);
@@ -310,6 +318,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 void dma_direct_free(struct device *dev, size_t size,
 		void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
 {
+	bool mark_mem_encrypted = true;
 	unsigned int page_order = get_order(size);
 
 	if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
@@ -338,12 +347,15 @@ void dma_direct_free(struct device *dev, size_t size,
 	    dma_free_from_pool(dev, cpu_addr, PAGE_ALIGN(size)))
 		return;
 
+	if (swiotlb_find_pool(dev, dma_to_phys(dev, dma_addr)))
+		mark_mem_encrypted = false;
+
 	if (is_vmalloc_addr(cpu_addr)) {
 		vunmap(cpu_addr);
 	} else {
 		if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_CLEAR_UNCACHED))
 			arch_dma_clear_uncached(cpu_addr, size);
-		if (dma_set_encrypted(dev, cpu_addr, size))
+		if (mark_mem_encrypted && dma_set_encrypted(dev, cpu_addr, size))
 			return;
 	}
 
@@ -359,6 +371,19 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
 	if (force_dma_unencrypted(dev) && dma_direct_use_pool(dev, gfp))
 		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
 
+	if (is_swiotlb_for_alloc(dev)) {
+		page = dma_direct_alloc_swiotlb(dev, size);
+		if (!page)
+			return NULL;
+
+		if (PageHighMem(page)) {
+			swiotlb_free(dev, page, size);
+			return NULL;
+		}
+		ret = page_address(page);
+		goto setup_page;
+	}
+
 	page = __dma_direct_alloc_pages(dev, size, gfp, false);
 	if (!page)
 		return NULL;
@@ -366,6 +391,7 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
 	ret = page_address(page);
 	if (dma_set_decrypted(dev, ret, size))
 		goto out_leak_pages;
+setup_page:
 	memset(ret, 0, size);
 	*dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
 	return page;
@@ -378,13 +404,17 @@ void dma_direct_free_pages(struct device *dev, size_t size,
 		enum dma_data_direction dir)
 {
 	void *vaddr = page_address(page);
+	bool mark_mem_encrypted = true;
 
 	/* If cpu_addr is not from an atomic pool, dma_free_from_pool() fails */
 	if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
 	    dma_free_from_pool(dev, vaddr, size))
 		return;
 
-	if (dma_set_encrypted(dev, vaddr, size))
+	if (swiotlb_find_pool(dev, page_to_phys(page)))
+		mark_mem_encrypted = false;
+
+	if (mark_mem_encrypted && dma_set_encrypted(dev, vaddr, size))
 		return;
 	__dma_direct_free_pages(dev, page, size);
 }
Signature · 1 line
2.43.0
#2 lore raw

PATCH v3 2/9 dma-direct: use DMA_ATTR_CC_SHARED in alloc/free paths

In reply to Aneesh Kumar K.V (Arm)

Propagate force_dma_unencrypted() into DMA_ATTR_CC_SHARED in the dma-direct allocation path and use the attribute to drive the related decisions.

This updates dma_direct_alloc(), dma_direct_free(), and dma_direct_alloc_pages() to fold the forced unencrypted case into attrs.

---
 kernel/dma/direct.c | 44 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 8 deletions(-)
kernel/dma/direct.c +36 -8
index b958f150718a..0c2e1f8436ce 100644
@@ -201,16 +201,31 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 		dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
 {
 	bool remap = false, set_uncached = false;
-	bool mark_mem_decrypt = true;
+	bool mark_mem_decrypt = false;
 	struct page *page;
 	void *ret;
 
+	/*
+	 * DMA_ATTR_CC_SHARED is not a caller-visible dma_alloc_*()
+	 * attribute. The direct allocator uses it internally after it has
+	 * decided that the backing pages must be shared/decrypted, so the
+	 * rest of the allocation path can consistently select DMA addresses,
+	 * choose compatible pools and restore encryption on free.
+	 */
+	if (attrs & DMA_ATTR_CC_SHARED)
+		return NULL;
+
+	if (force_dma_unencrypted(dev)) {
+		attrs |= DMA_ATTR_CC_SHARED;
+		mark_mem_decrypt = true;
+	}
+
 	size = PAGE_ALIGN(size);
 	if (attrs & DMA_ATTR_NO_WARN)
 		gfp |= __GFP_NOWARN;
 
-	if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
-	    !force_dma_unencrypted(dev) && !is_swiotlb_for_alloc(dev))
+	if (((attrs & (DMA_ATTR_NO_KERNEL_MAPPING | DMA_ATTR_CC_SHARED)) ==
+	     DMA_ATTR_NO_KERNEL_MAPPING) && !is_swiotlb_for_alloc(dev))
 		return dma_direct_alloc_no_mapping(dev, size, dma_handle, gfp);
 
 	if (!dev_is_dma_coherent(dev)) {
@@ -244,7 +259,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 	 * Remapping or decrypting memory may block, allocate the memory from
 	 * the atomic pools instead if we aren't allowed block.
 	 */
-	if ((remap || force_dma_unencrypted(dev)) &&
+	if ((remap || (attrs & DMA_ATTR_CC_SHARED)) &&
 	    dma_direct_use_pool(dev, gfp))
 		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
 
@@ -318,11 +333,20 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 void dma_direct_free(struct device *dev, size_t size,
 		void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
 {
-	bool mark_mem_encrypted = true;
+	bool mark_mem_encrypted = false;
 	unsigned int page_order = get_order(size);
 
-	if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
-	    !force_dma_unencrypted(dev) && !is_swiotlb_for_alloc(dev)) {
+	/*
+	 * if the device had requested for an unencrypted buffer,
+	 * convert it to encrypted on free
+	 */
+	if (force_dma_unencrypted(dev)) {
+		attrs |= DMA_ATTR_CC_SHARED;
+		mark_mem_encrypted = true;
+	}
+
+	if (((attrs & (DMA_ATTR_NO_KERNEL_MAPPING | DMA_ATTR_CC_SHARED)) ==
+	     DMA_ATTR_NO_KERNEL_MAPPING) && !is_swiotlb_for_alloc(dev)) {
 		/* cpu_addr is a struct page cookie, not a kernel address */
 		dma_free_contiguous(dev, cpu_addr, size);
 		return;
@@ -365,10 +389,14 @@ void dma_direct_free(struct device *dev, size_t size,
 struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
 		dma_addr_t *dma_handle, enum dma_data_direction dir, gfp_t gfp)
 {
+	unsigned long attrs = 0;
 	struct page *page;
 	void *ret;
 
-	if (force_dma_unencrypted(dev) && dma_direct_use_pool(dev, gfp))
+	if (force_dma_unencrypted(dev))
+		attrs |= DMA_ATTR_CC_SHARED;
+
+	if ((attrs & DMA_ATTR_CC_SHARED) && dma_direct_use_pool(dev, gfp))
 		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
 
 	if (is_swiotlb_for_alloc(dev)) {
Signature · 1 line
2.43.0
maintainer
#3 lore raw

Re: [PATCH v3 2/9] dma-direct: use DMA_ATTR_CC_SHARED in alloc/free paths

Quoted 4 lines · Aneesh Kumar K.V (Arm)
kernel/dma/direct.c +0 -0
@@ -365,10 +389,14 @@ void dma_direct_free(struct device *dev, size_t size,
 struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
 		dma_addr_t *dma_handle, enum dma_data_direction dir, gfp_t gfp)
 {

kernel/dma/direct.c void dma_direct_free(struct device *dev, size_t size, · line 400

Quoted 10 lines
kernel/dma/direct.c +5 -1
+	unsigned long attrs = 0;
 	struct page *page;
 	void *ret;
 
-	if (force_dma_unencrypted(dev) && dma_direct_use_pool(dev, gfp))
+	if (force_dma_unencrypted(dev))
+		attrs |= DMA_ATTR_CC_SHARED;
+
+	if ((attrs & DMA_ATTR_CC_SHARED) && dma_direct_use_pool(dev, gfp))
 		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);

I was looking at Sashiko's reports and it noticed the wrong type returned here. Not something your patch introduces but I think it should be fixed rather than continue to propagate it. It's been around since 5.10, commit 5b138c534fda ("dma-direct: factor out a dma_direct_alloc_from_pool helper"). This code path isn't used much I guess.

Signature · 1 line
Catalin
#4 lore raw

Re: [PATCH v3 2/9] dma-direct: use DMA_ATTR_CC_SHARED in alloc/free paths

Quoted 9 lines · Catalin Marinas
Quoted 8 lines · Aneesh Kumar K.V (Arm)
kernel/dma/direct.c +1 -0
@@ -365,10 +389,14 @@ void dma_direct_free(struct device *dev, size_t size,
 struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
 		dma_addr_t *dma_handle, enum dma_data_direction dir, gfp_t gfp)
 {
+	unsigned long attrs = 0;
 	struct page *page;
 	void *ret;
 
Quoted 14 lines

kernel/dma/direct.c void dma_direct_free(struct device *dev, size_t size, · line 400

Quoted 6 lines
kernel/dma/direct.c +4 -1
-	if (force_dma_unencrypted(dev) && dma_direct_use_pool(dev, gfp))
+	if (force_dma_unencrypted(dev))
+		attrs |= DMA_ATTR_CC_SHARED;
+
+	if ((attrs & DMA_ATTR_CC_SHARED) && dma_direct_use_pool(dev, gfp))
 		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);

I was looking at Sashiko's reports and it noticed the wrong type returned here. Not something your patch introduces but I think it should be fixed rather than continue to propagate it. It's been around since 5.10, commit 5b138c534fda ("dma-direct: factor out a dma_direct_alloc_from_pool helper"). This code path isn't used much I guess.

I can add this change as one of the patch

modified kernel/dma/direct.c
@@ -165,24 +165,24 @@
return !gfpflags_allow_blocking(gfp) && !is_swiotlb_for_alloc(dev);
}

-static void *dma_direct_alloc_from_pool(struct device *dev, size_t size,
- dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
+static struct page *dma_direct_alloc_from_pool(struct device *dev, size_t size,
+ dma_addr_t *dma_handle, void **cpu_addr, gfp_t gfp,
+ unsigned long attrs)
{
struct page *page;
u64 phys_limit;
- void *ret;

if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_DMA_COHERENT_POOL)))
return NULL;

gfp |= dma_direct_optimal_gfp_mask(dev, &phys_limit);
- page = dma_alloc_from_pool(dev, size, &ret, gfp, attrs,
- dma_coherent_ok);
+ page = dma_alloc_from_pool(dev, size, cpu_addr, gfp, attrs,
+ dma_coherent_ok);
if (!page)
return NULL;
*dma_handle = phys_to_dma_direct(dev, page_to_phys(page),
!!(attrs & DMA_ATTR_CC_SHARED));
- return ret;
+ return page;
}

static void *dma_direct_alloc_no_mapping(struct device *dev, size_t size,
@@ -212,7 +212,7 @@
bool mark_mem_decrypt = false;
bool allow_highmem = true;
struct page *page;
- void *ret;
+ void *cpu_addr;

/*
* DMA_ATTR_CC_SHARED is not a caller-visible dma_alloc_*()
@@ -278,9 +278,12 @@
* the atomic pools instead if we aren't allowed block. */
if ((remap || (attrs & DMA_ATTR_CC_SHARED)) &&
- dma_direct_use_pool(dev, gfp))
- return dma_direct_alloc_from_pool(dev, size, dma_handle,
- gfp, attrs);
+ dma_direct_use_pool(dev, gfp)) {
+ page = dma_direct_alloc_from_pool(dev, size,
+ dma_handle, &cpu_addr,
+ gfp, attrs);
+ return page ? cpu_addr : NULL;
+ }

if (is_swiotlb_for_alloc(dev)) {
page = dma_direct_alloc_swiotlb(dev, size, attrs);
@@ -318,12 +321,12 @@
arch_dma_prep_coherent(page, size);

/* create a coherent mapping */
- ret = dma_common_contiguous_remap(page, size, prot,
+ cpu_addr = dma_common_contiguous_remap(page, size, prot,
__builtin_return_address(0));
- if (!ret)
+ if (!cpu_addr)
goto out_free_pages;
} else {
- ret = page_address(page);
+ cpu_addr = page_address(page);
}

if (mark_mem_decrypt) {
@@ -334,18 +337,18 @@
goto out_leak_pages;
}

- memset(ret, 0, size);
+ memset(cpu_addr, 0, size);

if (set_uncached) {
arch_dma_prep_coherent(page, size);
- ret = arch_dma_set_uncached(ret, size);
- if (IS_ERR(ret))
+ cpu_addr = arch_dma_set_uncached(cpu_addr, size);
+ if (IS_ERR(cpu_addr))
goto out_encrypt_pages;
}

*dma_handle = phys_to_dma_direct(dev, page_to_phys(page),
!!(attrs & DMA_ATTR_CC_SHARED));
- return ret;
+ return cpu_addr;

out_encrypt_pages:
if (mark_mem_decrypt && dma_set_encrypted(dev, page_address(page), size))
@@ -427,14 +430,14 @@
{
unsigned long attrs = 0;
struct page *page;
- void *ret;
+ void *cpu_addr;

if (force_dma_unencrypted(dev))
attrs |= DMA_ATTR_CC_SHARED;

if ((attrs & DMA_ATTR_CC_SHARED) && dma_direct_use_pool(dev, gfp))
return dma_direct_alloc_from_pool(dev, size, dma_handle,
- gfp, attrs);
+ &cpu_addr, gfp, attrs);

if (is_swiotlb_for_alloc(dev)) {
page = dma_direct_alloc_swiotlb(dev, size, attrs);
@@ -445,7 +448,7 @@
swiotlb_free(dev, page, size); return NULL;
}
- ret = page_address(page);
+ cpu_addr = page_address(page);
goto setup_page;
}

@@ -453,11 +456,11 @@
if (!page)
return NULL;

- ret = page_address(page);
- if ((attrs & DMA_ATTR_CC_SHARED) && dma_set_decrypted(dev, ret, size))
+ cpu_addr = page_address(page);
+ if ((attrs & DMA_ATTR_CC_SHARED) && dma_set_decrypted(dev, cpu_addr, size))
goto out_leak_pages;
setup_page:
- memset(ret, 0, size);
+ memset(cpu_addr, 0, size);
*dma_handle = phys_to_dma_direct(dev, page_to_phys(page),
!!(attrs & DMA_ATTR_CC_SHARED));
return page;

#5 lore raw

PATCH v3 3/9 dma-pool: track decrypted atomic pools and select them via attrs

In reply to Aneesh Kumar K.V (Arm)

Teach the atomic DMA pool code to distinguish between encrypted and decrypted pools, and make pool allocation select the matching pool based on DMA attributes.

Introduce a dma_gen_pool wrapper that records whether a pool is decrypted, initialize that state when the atomic pools are created, and use it when expanding and resizing the pools. Update dma_alloc_from_pool() to take attrs and skip pools whose encrypted/decrypted state does not match DMA_ATTR_CC_SHARED. Update dma_free_from_pool() accordingly.

Also pass DMA_ATTR_CC_SHARED from the swiotlb atomic allocation path so decrypted swiotlb allocations are taken from the correct atomic pool.

---
 drivers/iommu/dma-iommu.c   |   2 +-
 include/linux/dma-map-ops.h |   2 +-
 kernel/dma/direct.c         |  11 ++-
 kernel/dma/pool.c           | 154 +++++++++++++++++++++++-------------
 kernel/dma/swiotlb.c        |   2 +
 5 files changed, 109 insertions(+), 62 deletions(-)
drivers/iommu/dma-iommu.c +1 -1
index 54d96e847f16..c2595bee3d41 100644
@@ -1673,7 +1673,7 @@ void *iommu_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
 	if (IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
 	    !gfpflags_allow_blocking(gfp) && !coherent)
 		page = dma_alloc_from_pool(dev, PAGE_ALIGN(size), &cpu_addr,
-					       gfp, NULL);
+					   gfp, attrs, NULL);
 	else
 		cpu_addr = iommu_dma_alloc_pages(dev, size, &page, gfp, attrs);
 	if (!cpu_addr)
include/linux/dma-map-ops.h +1 -1
index 6a1832a73cad..696b2c3a2305 100644
@@ -212,7 +212,7 @@ void *dma_common_pages_remap(struct page **pages, size_t size, pgprot_t prot,
 void dma_common_free_remap(void *cpu_addr, size_t size);
 
 struct page *dma_alloc_from_pool(struct device *dev, size_t size,
-		void **cpu_addr, gfp_t flags,
+		void **cpu_addr, gfp_t flags, unsigned long attrs,
 		bool (*phys_addr_ok)(struct device *, phys_addr_t, size_t));
 bool dma_free_from_pool(struct device *dev, void *start, size_t size);
 
kernel/dma/direct.c +7 -4
index 0c2e1f8436ce..707d410b3669 100644
@@ -162,7 +162,7 @@ static bool dma_direct_use_pool(struct device *dev, gfp_t gfp)
 }
 
 static void *dma_direct_alloc_from_pool(struct device *dev, size_t size,
-		dma_addr_t *dma_handle, gfp_t gfp)
+		dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
 {
 	struct page *page;
 	u64 phys_limit;
@@ -172,7 +172,8 @@ static void *dma_direct_alloc_from_pool(struct device *dev, size_t size,
 		return NULL;
 
 	gfp |= dma_direct_optimal_gfp_mask(dev, &phys_limit);
-	page = dma_alloc_from_pool(dev, size, &ret, gfp, dma_coherent_ok);
+	page = dma_alloc_from_pool(dev, size, &ret, gfp, attrs,
+				  dma_coherent_ok);
 	if (!page)
 		return NULL;
 	*dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
@@ -261,7 +262,8 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 	 */
 	if ((remap || (attrs & DMA_ATTR_CC_SHARED)) &&
 	    dma_direct_use_pool(dev, gfp))
-		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
+		return dma_direct_alloc_from_pool(dev, size, dma_handle,
+					  gfp, attrs);
 
 	if (is_swiotlb_for_alloc(dev)) {
 		page = dma_direct_alloc_swiotlb(dev, size);
@@ -397,7 +399,8 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
 		attrs |= DMA_ATTR_CC_SHARED;
 
 	if ((attrs & DMA_ATTR_CC_SHARED) && dma_direct_use_pool(dev, gfp))
-		return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
+		return dma_direct_alloc_from_pool(dev, size, dma_handle,
+					  gfp, attrs);
 
 	if (is_swiotlb_for_alloc(dev)) {
 		page = dma_direct_alloc_swiotlb(dev, size);
kernel/dma/pool.c +98 -56

253 lines not on this page · raw message

kernel/dma/swiotlb.c +2 -0
index 1abd3e6146f4..b215b274a873 100644
@@ -623,7 +623,9 @@ static struct page *swiotlb_alloc_tlb(struct device *dev, size_t bytes,
 		if (!IS_ENABLED(CONFIG_DMA_COHERENT_POOL))
 			return NULL;
 
+		/* considered decrypted by default */
 		return dma_alloc_from_pool(dev, bytes, &vaddr, gfp,
+					   DMA_ATTR_CC_SHARED,
 					   dma_coherent_ok);
 	}
 
Signature · 1 line
2.43.0
#6 lore raw

PATCH v3 4/9 dma: swiotlb: track pool encryption state and honor DMA_ATTR_CC_SHARED

In reply to Aneesh Kumar K.V (Arm)

Teach swiotlb to distinguish between encrypted and decrypted bounce buffer pools, and make allocation and mapping paths select a pool whose state matches the requested DMA attributes.

Add a decrypted flag to io_tlb_mem, initialize it for the default and restricted pools, and propagate DMA_ATTR_CC_SHARED into swiotlb pool allocation. Reject swiotlb alloc/map requests when the selected pool does not match the required encrypted/decrypted state.

Also return DMA addresses with the matching phys_to_dma_{encrypted, unencrypted} helper so the DMA address encoding stays consistent with the chosen pool.

---
 include/linux/dma-direct.h | 10 +++++
 include/linux/swiotlb.h    |  7 ++-
 kernel/dma/direct.c        | 14 ++++--
 kernel/dma/swiotlb.c       | 89 ++++++++++++++++++++++++++++++--------
 4 files changed, 95 insertions(+), 25 deletions(-)
include/linux/dma-direct.h +10 -0
index c249912456f9..94fad4e7c11e 100644
@@ -77,6 +77,10 @@ static inline dma_addr_t dma_range_map_max(const struct bus_dma_region *map)
 #ifndef phys_to_dma_unencrypted
 #define phys_to_dma_unencrypted		phys_to_dma
 #endif
+
+#ifndef phys_to_dma_encrypted
+#define phys_to_dma_encrypted		phys_to_dma
+#endif
 #else
 static inline dma_addr_t __phys_to_dma(struct device *dev, phys_addr_t paddr)
 {
@@ -90,6 +94,12 @@ static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
 {
 	return dma_addr_unencrypted(__phys_to_dma(dev, paddr));
 }
+
+static inline dma_addr_t phys_to_dma_encrypted(struct device *dev,
+		phys_addr_t paddr)
+{
+	return dma_addr_encrypted(__phys_to_dma(dev, paddr));
+}
 /*
  * If memory encryption is supported, phys_to_dma will set the memory encryption
  * bit in the DMA address, and dma_to_phys will clear it.
include/linux/swiotlb.h +5 -2
index 3dae0f592063..382753ba3f06 100644
@@ -111,6 +111,7 @@ struct io_tlb_mem {
 	struct dentry *debugfs;
 	bool force_bounce;
 	bool for_alloc;
+	bool decrypted;
 #ifdef CONFIG_SWIOTLB_DYNAMIC
 	bool can_grow;
 	u64 phys_limit;
@@ -282,7 +283,8 @@ static inline void swiotlb_sync_single_for_cpu(struct device *dev,
 extern void swiotlb_print_info(void);
 
 #ifdef CONFIG_DMA_RESTRICTED_POOL
-struct page *swiotlb_alloc(struct device *dev, size_t size);
+struct page *swiotlb_alloc(struct device *dev, size_t size,
+		unsigned long attrs);
 bool swiotlb_free(struct device *dev, struct page *page, size_t size);
 
 static inline bool is_swiotlb_for_alloc(struct device *dev)
@@ -290,7 +292,8 @@ static inline bool is_swiotlb_for_alloc(struct device *dev)
 	return dev->dma_io_tlb_mem->for_alloc;
 }
 #else
-static inline struct page *swiotlb_alloc(struct device *dev, size_t size)
+static inline struct page *swiotlb_alloc(struct device *dev, size_t size,
+		unsigned long attrs)
 {
 	return NULL;
 }
kernel/dma/direct.c +10 -4
index 707d410b3669..9373b9dcfb7b 100644
@@ -104,9 +104,10 @@ static void __dma_direct_free_pages(struct device *dev, struct page *page,
 	dma_free_contiguous(dev, page, size);
 }
 
-static struct page *dma_direct_alloc_swiotlb(struct device *dev, size_t size)
+static struct page *dma_direct_alloc_swiotlb(struct device *dev, size_t size,
+		unsigned long attrs)
 {
-	struct page *page = swiotlb_alloc(dev, size);
+	struct page *page = swiotlb_alloc(dev, size, attrs);
 
 	if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
 		swiotlb_free(dev, page, size);
@@ -266,8 +267,12 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 					  gfp, attrs);
 
 	if (is_swiotlb_for_alloc(dev)) {
-		page = dma_direct_alloc_swiotlb(dev, size);
+		page = dma_direct_alloc_swiotlb(dev, size, attrs);
 		if (page) {
+			/*
+			 * swiotlb allocations comes from pool already marked
+			 * decrypted
+			 */
 			mark_mem_decrypt = false;
 			goto setup_page;
 		}
@@ -374,6 +379,7 @@ void dma_direct_free(struct device *dev, size_t size,
 		return;
 
 	if (swiotlb_find_pool(dev, dma_to_phys(dev, dma_addr)))
+		/* Swiotlb doesn't need a page attribute update on free */
 		mark_mem_encrypted = false;
 
 	if (is_vmalloc_addr(cpu_addr)) {
@@ -403,7 +409,7 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
 					  gfp, attrs);
 
 	if (is_swiotlb_for_alloc(dev)) {
-		page = dma_direct_alloc_swiotlb(dev, size);
+		page = dma_direct_alloc_swiotlb(dev, size, attrs);
 		if (!page)
 			return NULL;
 
kernel/dma/swiotlb.c +70 -19

214 lines not on this page · raw message

Signature · 1 line
2.43.0
maintainer
#7 lore raw

Re: [PATCH v3 4/9] dma: swiotlb: track pool encryption state and honor DMA_ATTR_CC_SHARED

kernel/dma/swiotlb.c phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, · line 1438

Quoted 11 lines · Aneesh Kumar K.V (Arm)
kernel/dma/swiotlb.c +7 -0
@@ -1408,6 +1429,17 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
 	if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
 		pr_warn_once("Memory encryption is active and system is using DMA bounce buffers\n");
 
+	/*
+	 * if we are trying to swiotlb map a decrypted paddr or the paddr is encrypted
+	 * but the device is forcing decryption, use decrypted io_tlb_mem
+	 */
+	if ((attrs & DMA_ATTR_CC_SHARED) ||
+	    (!(attrs & DMA_ATTR_CC_SHARED) && force_dma_unencrypted(dev)))
+		require_decrypted = true;

Nit: just this should do:

	if ((attrs & DMA_ATTR_CC_SHARED) || force_dma_unencrypted(dev))

kernel/dma/swiotlb.c phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, · line 1441

Quoted 2 lines
kernel/dma/swiotlb.c +2 -0
+	if (require_decrypted != mem->decrypted)
+		return (phys_addr_t)DMA_MAPPING_ERROR;

I wonder whether io_tlb_mem should store the attrs that were used when created (just DMA_ATTR_CC_SHARED for now) and use that to check here. In patch 7, this hunk in swiotlb_map() confused me:

	if (dev->dma_io_tlb_mem->decrypted) {
		dma_addr = phys_to_dma_unencrypted(dev, swiotlb_addr);
		attrs |= DMA_ATTR_CC_SHARED;
	} else {
		dma_addr = phys_to_dma_encrypted(dev, swiotlb_addr);
	}

as I thought we'd not update the attributes on the streaming API path. But what you meant here is for dma_capable() to be checked against the device with the actual io_tlb_mem attributes.

Anyway, the new swiotlb_tbl_map_single() rejects kmalloc-minalign bouncing if the device is private while the bounce buffer is shared. Unlikely we'll need such bouncing if the devices are coherent but it's good as a safety check.

Signature · 1 line
Catalin
#8 lore raw

Re: [PATCH v3 4/9] dma: swiotlb: track pool encryption state and honor DMA_ATTR_CC_SHARED

Quoted 15 lines · Catalin Marinas

kernel/dma/swiotlb.c phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, · line 1438

Quoted 11 lines · Aneesh Kumar K.V (Arm)
kernel/dma/swiotlb.c +7 -0
@@ -1408,6 +1429,17 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr,
 	if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
 		pr_warn_once("Memory encryption is active and system is using DMA bounce buffers\n");
 
+	/*
+	 * if we are trying to swiotlb map a decrypted paddr or the paddr is encrypted
+	 * but the device is forcing decryption, use decrypted io_tlb_mem
+	 */
+	if ((attrs & DMA_ATTR_CC_SHARED) ||
+	    (!(attrs & DMA_ATTR_CC_SHARED) && force_dma_unencrypted(dev)))
+		require_decrypted = true;

Nit: just this should do:

Quoted 2 lines
	if ((attrs & DMA_ATTR_CC_SHARED) || force_dma_unencrypted(dev))

I will update this in the next version.

Quoted 7 lines

kernel/dma/swiotlb.c phys_addr_t swiotlb_tbl_map_single(struct device *dev, phys_addr_t orig_addr, · line 1441

Quoted 2 lines
kernel/dma/swiotlb.c +2 -0
+	if (require_decrypted != mem->decrypted)
+		return (phys_addr_t)DMA_MAPPING_ERROR;

I wonder whether io_tlb_mem should store the attrs that were used when created (just DMA_ATTR_CC_SHARED for now) and use that to check here. In patch 7, this hunk in swiotlb_map() confused me:

We already added io_tlb_mem->decrypted. Are you suggesting that this should instead be io_tlb_mem->attrs and use DMA_ATTR_CC_SHARED? Do we foresee the need to use any other attributes (other than shared) with respect to io_tlb_mem?

Quoted 8 lines
	if (dev->dma_io_tlb_mem->decrypted) {
		dma_addr = phys_to_dma_unencrypted(dev, swiotlb_addr);
		attrs |= DMA_ATTR_CC_SHARED;
	} else {
		dma_addr = phys_to_dma_encrypted(dev, swiotlb_addr);
	}
Quoted 4 lines

as I thought we'd not update the attributes on the streaming API path. But what you meant here is for dma_capable() to be checked against the device with the actual io_tlb_mem attributes.

if we allocated/mapped from a decrypted iot_tlb_mem, we should return an unencrypted dma_addr_t.

Quoted 5 lines

Anyway, the new swiotlb_tbl_map_single() rejects kmalloc-minalign bouncing if the device is private while the bounce buffer is shared. Unlikely we'll need such bouncing if the devices are coherent but it's good as a safety check.

-aneesh

#9 lore raw

PATCH v3 5/9 dma-mapping: make dma_pgprot() honor DMA_ATTR_CC_SHARED

In reply to Aneesh Kumar K.V (Arm)

Fold encrypted/decrypted pgprot selection into dma_pgprot() so callers do not need to adjust the page protection separately.

Update dma_pgprot() to apply pgprot_decrypted() when DMA_ATTR_CC_SHARED is set and pgprot_encrypted() otherwise Convert the dma-direct allocation and mmap paths to pass DMA_ATTR_CC_SHARED instead of open-coding force_dma_unencrypted() handling around dma_pgprot().

---
 kernel/dma/direct.c  |  8 +++-----
 kernel/dma/mapping.c | 16 ++++++++++++----
 2 files changed, 15 insertions(+), 9 deletions(-)
kernel/dma/direct.c +3 -5
index 9373b9dcfb7b..a4aa7e1df2bb 100644
@@ -298,9 +298,6 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 	if (remap) {
 		pgprot_t prot = dma_pgprot(dev, PAGE_KERNEL, attrs);
 
-		if (force_dma_unencrypted(dev))
-			prot = pgprot_decrypted(prot);
-
 		/* remove any dirty cache lines on the kernel alias */
 		arch_dma_prep_coherent(page, size);
 
@@ -603,9 +600,10 @@ int dma_direct_mmap(struct device *dev, struct vm_area_struct *vma,
 	unsigned long pfn = PHYS_PFN(dma_to_phys(dev, dma_addr));
 	int ret = -ENXIO;
 
-	vma->vm_page_prot = dma_pgprot(dev, vma->vm_page_prot, attrs);
 	if (force_dma_unencrypted(dev))
-		vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
+		attrs |= DMA_ATTR_CC_SHARED;
+
+	vma->vm_page_prot = dma_pgprot(dev, vma->vm_page_prot, attrs);
 
 	if (dma_mmap_from_dev_coherent(dev, vma, cpu_addr, size, &ret))
 		return ret;
kernel/dma/mapping.c +12 -4
index 23ed8eb9233e..44f715f3aa2d 100644
@@ -543,13 +543,21 @@ EXPORT_SYMBOL(dma_get_sgtable_attrs);
  */
 pgprot_t dma_pgprot(struct device *dev, pgprot_t prot, unsigned long attrs)
 {
+	pgprot_t dma_prot;
+
 	if (dev_is_dma_coherent(dev))
-		return prot;
+		dma_prot = prot;
 #ifdef CONFIG_ARCH_HAS_DMA_WRITE_COMBINE
-	if (attrs & DMA_ATTR_WRITE_COMBINE)
-		return pgprot_writecombine(prot);
+	else if (attrs & DMA_ATTR_WRITE_COMBINE)
+		dma_prot = pgprot_writecombine(prot);
 #endif
-	return pgprot_dmacoherent(prot);
+	else
+		dma_prot = pgprot_dmacoherent(prot);
+
+	if (attrs & DMA_ATTR_CC_SHARED)
+		return pgprot_decrypted(dma_prot);
+	else
+		return pgprot_encrypted(dma_prot);
 }
 #endif /* CONFIG_MMU */
 
Signature · 1 line
2.43.0
#10 lore raw

PATCH v3 6/9 dma-direct: pass attrs to dma_capable() for DMA_ATTR_CC_SHARED checks

In reply to Aneesh Kumar K.V (Arm)

Teach dma_capable() about DMA_ATTR_CC_SHARED so the capability check can reject encrypted DMA addresses for devices that require unencrypted/shared DMA.

Also propagate DMA_ATTR_CC_SHARED in swiotlb_map() when the selected SWIOTLB pool is decrypted so the capability check sees the correct DMA address attribute.

---
 arch/x86/kernel/amd_gart_64.c | 30 ++++++++++++++++--------------
 drivers/xen/swiotlb-xen.c     |  9 ++++++---
 include/linux/dma-direct.h    |  9 ++++++++-
 kernel/dma/direct.h           |  6 +++---
 kernel/dma/swiotlb.c          |  8 +++++---
 5 files changed, 38 insertions(+), 24 deletions(-)
arch/x86/kernel/amd_gart_64.c +16 -14
index e8000a56732e..b5f1f031d45b 100644
@@ -180,22 +180,23 @@ static void iommu_full(struct device *dev, size_t size, int dir)
 }
 
 static inline int
-need_iommu(struct device *dev, unsigned long addr, size_t size)
+need_iommu(struct device *dev, unsigned long addr, size_t size, unsigned long attrs)
 {
-	return force_iommu || !dma_capable(dev, addr, size, true);
+	return force_iommu || !dma_capable(dev, addr, size, true, attrs);
 }
 
 static inline int
-nonforced_iommu(struct device *dev, unsigned long addr, size_t size)
+nonforced_iommu(struct device *dev, unsigned long addr, size_t size,
+		unsigned long attrs)
 {
-	return !dma_capable(dev, addr, size, true);
+	return !dma_capable(dev, addr, size, true, attrs);
 }
 
 /* Map a single continuous physical area into the IOMMU.
  * Caller needs to check if the iommu is needed and flush.
  */
 static dma_addr_t dma_map_area(struct device *dev, dma_addr_t phys_mem,
-				size_t size, int dir, unsigned long align_mask)
+		size_t size, int dir, unsigned long align_mask, unsigned long attrs)
 {
 	unsigned long npages = iommu_num_pages(phys_mem, size, PAGE_SIZE);
 	unsigned long iommu_page;
@@ -206,7 +207,7 @@ static dma_addr_t dma_map_area(struct device *dev, dma_addr_t phys_mem,
 
 	iommu_page = alloc_iommu(dev, npages, align_mask);
 	if (iommu_page == -1) {
-		if (!nonforced_iommu(dev, phys_mem, size))
+		if (!nonforced_iommu(dev, phys_mem, size, attrs))
 			return phys_mem;
 		if (panic_on_overflow)
 			panic("dma_map_area overflow %lu bytes\n", size);
@@ -231,10 +232,10 @@ static dma_addr_t gart_map_phys(struct device *dev, phys_addr_t paddr,
 	if (unlikely(attrs & DMA_ATTR_MMIO))
 		return DMA_MAPPING_ERROR;
 
-	if (!need_iommu(dev, paddr, size))
+	if (!need_iommu(dev, paddr, size, attrs))
 		return paddr;
 
-	bus = dma_map_area(dev, paddr, size, dir, 0);
+	bus = dma_map_area(dev, paddr, size, dir, 0, attrs);
 	flush_gart();
 
 	return bus;
@@ -289,7 +290,7 @@ static void gart_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
 
 /* Fallback for dma_map_sg in case of overflow */
 static int dma_map_sg_nonforce(struct device *dev, struct scatterlist *sg,
-			       int nents, int dir)
+		int nents, int dir, unsigned long attrs)
 {
 	struct scatterlist *s;
 	int i;
@@ -301,8 +302,8 @@ static int dma_map_sg_nonforce(struct device *dev, struct scatterlist *sg,
 	for_each_sg(sg, s, nents, i) {
 		unsigned long addr = sg_phys(s);
 
-		if (nonforced_iommu(dev, addr, s->length)) {
-			addr = dma_map_area(dev, addr, s->length, dir, 0);
+		if (nonforced_iommu(dev, addr, s->length, attrs)) {
+			addr = dma_map_area(dev, addr, s->length, dir, 0, attrs);
 			if (addr == DMA_MAPPING_ERROR) {
 				if (i > 0)
 					gart_unmap_sg(dev, sg, i, dir, 0);
@@ -401,7 +402,7 @@ static int gart_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 		s->dma_address = addr;
 		BUG_ON(s->length == 0);
 
-		nextneed = need_iommu(dev, addr, s->length);
+		nextneed = need_iommu(dev, addr, s->length, attrs);
 
 		/* Handle the previous not yet processed entries */
 		if (i > start) {
@@ -449,7 +450,7 @@ static int gart_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 
 	/* When it was forced or merged try again in a dumb way */
 	if (force_iommu || iommu_merge) {
-		out = dma_map_sg_nonforce(dev, sg, nents, dir);
+		out = dma_map_sg_nonforce(dev, sg, nents, dir, attrs);
 		if (out > 0)
 			return out;
 	}
@@ -473,7 +474,8 @@ gart_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_addr,
 		return vaddr;
 
 	*dma_addr = dma_map_area(dev, virt_to_phys(vaddr), size,
-			DMA_BIDIRECTIONAL, (1UL << get_order(size)) - 1);
+				 DMA_BIDIRECTIONAL,
+				 (1UL << get_order(size)) - 1, attrs);
 	flush_gart();
 	if (unlikely(*dma_addr == DMA_MAPPING_ERROR))
 		goto out_free;
drivers/xen/swiotlb-xen.c +6 -3
index 2cbf2b588f5b..7584c6f42141 100644
@@ -212,7 +212,7 @@ static dma_addr_t xen_swiotlb_map_phys(struct device *dev, phys_addr_t phys,
 	BUG_ON(dir == DMA_NONE);
 
 	if (attrs & DMA_ATTR_MMIO) {
-		if (unlikely(!dma_capable(dev, phys, size, false))) {
+		if (unlikely(!dma_capable(dev, phys, size, false, attrs))) {
 			dev_err_once(
 				dev,
 				"DMA addr %pa+%zu overflow (mask %llx, bus limit %llx).\n",
@@ -231,7 +231,7 @@ static dma_addr_t xen_swiotlb_map_phys(struct device *dev, phys_addr_t phys,
 	 * we can safely return the device addr and not worry about bounce
 	 * buffering it.
 	 */
-	if (dma_capable(dev, dev_addr, size, true) &&
+	if (dma_capable(dev, dev_addr, size, true, attrs) &&
 	    !dma_kmalloc_needs_bounce(dev, size, dir) &&
 	    !range_straddles_page_boundary(phys, size) &&
 		!xen_arch_need_swiotlb(dev, phys, dev_addr) &&
@@ -250,10 +250,13 @@ static dma_addr_t xen_swiotlb_map_phys(struct device *dev, phys_addr_t phys,
 	phys = map;
 	dev_addr = xen_phys_to_dma(dev, map);
 
+	if (dev->dma_io_tlb_mem->decrypted)
+		attrs |= DMA_ATTR_CC_SHARED;
+
 	/*
 	 * Ensure that the address returned is DMA'ble
 	 */
-	if (unlikely(!dma_capable(dev, dev_addr, size, true))) {
+	if (unlikely(!dma_capable(dev, dev_addr, size, true, attrs))) {
 		__swiotlb_tbl_unmap_single(dev, map, size, dir,
 				attrs | DMA_ATTR_SKIP_CPU_SYNC,
 				swiotlb_find_pool(dev, map));
include/linux/dma-direct.h +8 -1
index 94fad4e7c11e..1281d91050a4 100644
@@ -135,12 +135,19 @@ static inline bool force_dma_unencrypted(struct device *dev)
 #endif /* CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED */
 
 static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size,
-		bool is_ram)
+		bool is_ram, unsigned long attrs)
 {
 	dma_addr_t end = addr + size - 1;
 
 	if (addr == DMA_MAPPING_ERROR)
 		return false;
+	/*
+	 * if phys addr attribute is encrypted but the
+	 * device is forcing an encrypted dma addr
+	 */
+	if (!(attrs & DMA_ATTR_CC_SHARED) && force_dma_unencrypted(dev))
+		return false;
+
 	if (is_ram && !IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT) &&
 	    min(addr, end) < phys_to_dma(dev, PFN_PHYS(min_low_pfn)))
 		return false;
kernel/dma/direct.h +3 -3
index 7140c208c123..e05dc7649366 100644
@@ -101,15 +101,15 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
 
 	if (attrs & DMA_ATTR_MMIO) {
 		dma_addr = phys;
-		if (unlikely(!dma_capable(dev, dma_addr, size, false)))
+		if (unlikely(!dma_capable(dev, dma_addr, size, false, attrs)))
 			goto err_overflow;
 	} else if (attrs & DMA_ATTR_CC_SHARED) {
 		dma_addr = phys_to_dma_unencrypted(dev, phys);
-		if (unlikely(!dma_capable(dev, dma_addr, size, false)))
+		if (unlikely(!dma_capable(dev, dma_addr, size, false, attrs)))
 			goto err_overflow;
 	} else {
 		dma_addr = phys_to_dma(dev, phys);
-		if (unlikely(!dma_capable(dev, dma_addr, size, true)) ||
+		if (unlikely(!dma_capable(dev, dma_addr, size, true, attrs)) ||
 		    dma_kmalloc_needs_bounce(dev, size, dir)) {
 			if (is_swiotlb_active(dev) &&
 			    !(attrs & DMA_ATTR_REQUIRE_COHERENT))
kernel/dma/swiotlb.c +5 -3
index dad351988daf..0d3ae9a0130a 100644
@@ -1640,12 +1640,14 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t paddr, size_t size,
 	/*
 	 * Use the allocated io_tlb_mem encryption type to determine dma addr.
 	 */
-	if (dev->dma_io_tlb_mem->decrypted)
+	if (dev->dma_io_tlb_mem->decrypted) {
 		dma_addr = phys_to_dma_unencrypted(dev, swiotlb_addr);
-	else
+		attrs |= DMA_ATTR_CC_SHARED;
+	} else {
 		dma_addr = phys_to_dma_encrypted(dev, swiotlb_addr);
+	}
 
-	if (unlikely(!dma_capable(dev, dma_addr, size, true))) {
+	if (unlikely(!dma_capable(dev, dma_addr, size, true, attrs))) {
 		__swiotlb_tbl_unmap_single(dev, swiotlb_addr, size, dir,
 			attrs | DMA_ATTR_SKIP_CPU_SYNC,
 			swiotlb_find_pool(dev, swiotlb_addr));
Signature · 1 line
2.43.0
#11 lore raw

PATCH v3 7/9 dma-direct: make dma_direct_map_phys() honor DMA_ATTR_CC_SHARED

In reply to Aneesh Kumar K.V (Arm)

Teach dma_direct_map_phys() to select the DMA address encoding based on DMA_ATTR_CC_SHARED.

Use phys_to_dma_unencrypted() for decrypted mappings and phys_to_dma_encrypted() otherwise. If a device requires unencrypted DMA but the source physical address is still encrypted, force the mapping through swiotlb so the DMA address and backing memory attributes remain consistent.

Update the arm64 and x86 CC-guest setup to not use swiotlb force option

---
 arch/arm64/mm/init.c      |  4 +---
 arch/x86/kernel/pci-dma.c |  4 +---
 kernel/dma/direct.c       |  4 +++-
 kernel/dma/direct.h       | 30 +++++++++++++-----------------
 4 files changed, 18 insertions(+), 24 deletions(-)
arch/arm64/mm/init.c +1 -3
index 97987f850a33..acf67c7064db 100644
@@ -338,10 +338,8 @@ void __init arch_mm_preinit(void)
 	unsigned int flags = SWIOTLB_VERBOSE;
 	bool swiotlb = max_pfn > PFN_DOWN(arm64_dma_phys_limit);
 
-	if (is_realm_world()) {
+	if (is_realm_world())
 		swiotlb = true;
-		flags |= SWIOTLB_FORCE;
-	}
 
 	if (IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) && !swiotlb) {
 		/*
arch/x86/kernel/pci-dma.c +1 -3
index 6267363e0189..75cf8f6ae8cd 100644
@@ -59,10 +59,8 @@ static void __init pci_swiotlb_detect(void)
 	 * bounce buffers as the hypervisor can't access arbitrary VM memory
 	 * that is not explicitly shared with it.
 	 */
-	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
+	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
 		x86_swiotlb_enable = true;
-		x86_swiotlb_flags |= SWIOTLB_FORCE;
-	}
 }
 #else
 static inline void __init pci_swiotlb_detect(void)
kernel/dma/direct.c +3 -1
index a4aa7e1df2bb..7d51dd93513d 100644
@@ -691,8 +691,10 @@ size_t dma_direct_max_mapping_size(struct device *dev)
 {
 	/* If SWIOTLB is active, use its maximum mapping size */
 	if (is_swiotlb_active(dev) &&
-	    (dma_addressing_limited(dev) || is_swiotlb_force_bounce(dev)))
+	    (dma_addressing_limited(dev) || is_swiotlb_force_bounce(dev) ||
+	     force_dma_unencrypted(dev)))
 		return swiotlb_max_mapping_size(dev);
+
 	return SIZE_MAX;
 }
 
kernel/dma/direct.h +13 -17
index e05dc7649366..af035dc935ab 100644
@@ -89,36 +89,32 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
 	dma_addr_t dma_addr;
 
 	if (is_swiotlb_force_bounce(dev)) {
-		if (!(attrs & DMA_ATTR_CC_SHARED)) {
-			if (attrs & (DMA_ATTR_MMIO | DMA_ATTR_REQUIRE_COHERENT))
-				return DMA_MAPPING_ERROR;
+		if (attrs & (DMA_ATTR_MMIO | DMA_ATTR_REQUIRE_COHERENT))
+			return DMA_MAPPING_ERROR;
 
-			return swiotlb_map(dev, phys, size, dir, attrs);
-		}
-	} else if (attrs & DMA_ATTR_CC_SHARED) {
-		return DMA_MAPPING_ERROR;
+		return swiotlb_map(dev, phys, size, dir, attrs);
 	}
 
 	if (attrs & DMA_ATTR_MMIO) {
 		dma_addr = phys;
 		if (unlikely(!dma_capable(dev, dma_addr, size, false, attrs)))
 			goto err_overflow;
+		goto dma_mapped;
 	} else if (attrs & DMA_ATTR_CC_SHARED) {
 		dma_addr = phys_to_dma_unencrypted(dev, phys);
-		if (unlikely(!dma_capable(dev, dma_addr, size, false, attrs)))
-			goto err_overflow;
 	} else {
-		dma_addr = phys_to_dma(dev, phys);
-		if (unlikely(!dma_capable(dev, dma_addr, size, true, attrs)) ||
-		    dma_kmalloc_needs_bounce(dev, size, dir)) {
-			if (is_swiotlb_active(dev) &&
-			    !(attrs & DMA_ATTR_REQUIRE_COHERENT))
-				return swiotlb_map(dev, phys, size, dir, attrs);
+		dma_addr = phys_to_dma_encrypted(dev, phys);
+	}
 
-			goto err_overflow;
-		}
+	if (unlikely(!dma_capable(dev, dma_addr, size, true, attrs)) ||
+	    dma_kmalloc_needs_bounce(dev, size, dir)) {
+		if (is_swiotlb_active(dev) &&
+		    !(attrs & DMA_ATTR_REQUIRE_COHERENT))
+			return swiotlb_map(dev, phys, size, dir, attrs);
+		goto err_overflow;
 	}
 
+dma_mapped:
 	if (!dev_is_dma_coherent(dev) &&
 	    !(attrs & (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_MMIO))) {
 		arch_sync_dma_for_device(phys, size, dir);
Signature · 1 line
2.43.0
#12 lore raw

PATCH v3 8/9 dma-direct: set decrypted flag for remapped DMA allocations

In reply to Aneesh Kumar K.V (Arm)

Devices that are DMA non-coherent and require a remap were skipping dma_set_decrypted(), leaving DMA buffers encrypted even when the device requires unencrypted access. Move the call after the if (remap) branch so that both the direct and remapped allocation paths correctly mark the allocation as decrypted (or fail cleanly) before use.

Fix dma_direct_alloc() and dma_direct_free() to apply set_memory_*() to the linear-map alias of the backing pages instead of the remapped CPU address. Also disallow highmem pages for DMA_ATTR_CC_SHARED, because highmem buffers do not provide a usable linear-map address.

Fixes f3c962226dbe ("dma-direct: clean up the remapping checks in dma_direct_alloc")
---
 kernel/dma/direct.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)
kernel/dma/direct.c +27 -3
index 7d51dd93513d..f874be2d85c2 100644
@@ -204,6 +204,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 {
 	bool remap = false, set_uncached = false;
 	bool mark_mem_decrypt = false;
+	bool allow_highmem = true;
 	struct page *page;
 	void *ret;
 
@@ -222,6 +223,15 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 		mark_mem_decrypt = true;
 	}
 
+	if (attrs & DMA_ATTR_CC_SHARED)
+		/*
+		 * Unencrypted/shared DMA requires a linear-mapped buffer
+		 * address to look up the PFN and set architecture-required PFN
+		 * attributes. This is not possible with HighMem. Avoid HighMem
+		 * allocation.
+		 */
+		allow_highmem = false;
+
 	size = PAGE_ALIGN(size);
 	if (attrs & DMA_ATTR_NO_WARN)
 		gfp |= __GFP_NOWARN;
@@ -280,7 +290,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 	}
 
 	/* we always manually zero the memory once we are done */
-	page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO, true);
+	page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO, allow_highmem);
 	if (!page)
 		return NULL;
 
@@ -308,7 +318,13 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 			goto out_free_pages;
 	} else {
 		ret = page_address(page);
-		if (mark_mem_decrypt && dma_set_decrypted(dev, ret, size))
+	}
+
+	if (mark_mem_decrypt) {
+		void *lm_addr;
+
+		lm_addr = page_address(page);
+		if (set_memory_decrypted((unsigned long)lm_addr, PFN_UP(size)))
 			goto out_leak_pages;
 	}
 
@@ -384,8 +400,16 @@ void dma_direct_free(struct device *dev, size_t size,
 	} else {
 		if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_CLEAR_UNCACHED))
 			arch_dma_clear_uncached(cpu_addr, size);
-		if (mark_mem_encrypted && dma_set_encrypted(dev, cpu_addr, size))
+	}
+
+	if (mark_mem_encrypted) {
+		void *lm_addr;
+
+		lm_addr = phys_to_virt(dma_to_phys(dev, dma_addr));
+		if (set_memory_encrypted((unsigned long)lm_addr, PFN_UP(size))) {
+			pr_warn_ratelimited("leaking DMA memory that can't be re-encrypted\n");
 			return;
+		}
 	}
 
 	__dma_direct_free_pages(dev, dma_direct_to_page(dev, dma_addr), size);
Signature · 1 line
2.43.0
#13 lore raw

PATCH v3 9/9 dma-direct: select DMA address encoding from DMA_ATTR_CC_SHARED

In reply to Aneesh Kumar K.V (Arm)

Make the dma-direct helpers derive the DMA address encoding from DMA_ATTR_CC_SHARED instead of implicitly relying on force_dma_unencrypted() inside phys_to_dma_direct()

Pass an explicit unencrypted/decrypted state into phys_to_dma_direct(), make the alloc paths return DMA addresses that match the requested buffer encryption state. Also only call dma_set_decrypted() DMA_ATTR_CC_SHARED is actually set.

---
 kernel/dma/direct.c | 48 ++++++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 18 deletions(-)
kernel/dma/direct.c +30 -18
index f874be2d85c2..cafa6b5ed1c6 100644
@@ -24,11 +24,11 @@
 u64 zone_dma_limit __ro_after_init = DMA_BIT_MASK(24);
 
 static inline dma_addr_t phys_to_dma_direct(struct device *dev,
-		phys_addr_t phys)
+		phys_addr_t phys, bool unencrypted)
 {
-	if (force_dma_unencrypted(dev))
+	if (unencrypted)
 		return phys_to_dma_unencrypted(dev, phys);
-	return phys_to_dma(dev, phys);
+	return phys_to_dma_encrypted(dev, phys);
 }
 
 static inline struct page *dma_direct_to_page(struct device *dev,
@@ -39,8 +39,9 @@ static inline struct page *dma_direct_to_page(struct device *dev,
 
 u64 dma_direct_get_required_mask(struct device *dev)
 {
+	bool require_decrypted = force_dma_unencrypted(dev);
 	phys_addr_t phys = (phys_addr_t)(max_pfn - 1) << PAGE_SHIFT;
-	u64 max_dma = phys_to_dma_direct(dev, phys);
+	u64 max_dma = phys_to_dma_direct(dev, phys, require_decrypted);
 
 	return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
 }
@@ -69,7 +70,8 @@ static gfp_t dma_direct_optimal_gfp_mask(struct device *dev, u64 *phys_limit)
 
 bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
 {
-	dma_addr_t dma_addr = phys_to_dma_direct(dev, phys);
+	bool require_decrypted = force_dma_unencrypted(dev);
+	dma_addr_t dma_addr = phys_to_dma_direct(dev, phys, require_decrypted);
 
 	if (dma_addr == DMA_MAPPING_ERROR)
 		return false;
@@ -79,17 +81,18 @@ bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
 
 static int dma_set_decrypted(struct device *dev, void *vaddr, size_t size)
 {
-	if (!force_dma_unencrypted(dev))
-		return 0;
-	return set_memory_decrypted((unsigned long)vaddr, PFN_UP(size));
+	int ret;
+
+	ret = set_memory_decrypted((unsigned long)vaddr, PFN_UP(size));
+	if (ret)
+		pr_warn_ratelimited("leaking DMA memory that can't be decrypted\n");
+	return ret;
 }
 
 static int dma_set_encrypted(struct device *dev, void *vaddr, size_t size)
 {
 	int ret;
 
-	if (!force_dma_unencrypted(dev))
-		return 0;
 	ret = set_memory_encrypted((unsigned long)vaddr, PFN_UP(size));
 	if (ret)
 		pr_warn_ratelimited("leaking DMA memory that can't be re-encrypted\n");
@@ -177,7 +180,8 @@ static void *dma_direct_alloc_from_pool(struct device *dev, size_t size,
 				  dma_coherent_ok);
 	if (!page)
 		return NULL;
-	*dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
+	*dma_handle = phys_to_dma_direct(dev, page_to_phys(page),
+					 !!(attrs & DMA_ATTR_CC_SHARED));
 	return ret;
 }
 
@@ -193,9 +197,11 @@ static void *dma_direct_alloc_no_mapping(struct device *dev, size_t size,
 	/* remove any dirty cache lines on the kernel alias */
 	if (!PageHighMem(page))
 		arch_dma_prep_coherent(page, size);
-
-	/* return the page pointer as the opaque cookie */
-	*dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
+	/*
+	 * return the page pointer as the opaque cookie.
+	 * Never used for unencrypted allocation
+	 */
+	*dma_handle = phys_to_dma_encrypted(dev, page_to_phys(page));
 	return page;
 }
 
@@ -337,7 +343,8 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 			goto out_encrypt_pages;
 	}
 
-	*dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
+	*dma_handle = phys_to_dma_direct(dev, page_to_phys(page),
+					 !!(attrs & DMA_ATTR_CC_SHARED));
 	return ret;
 
 out_encrypt_pages:
@@ -447,11 +454,12 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
 		return NULL;
 
 	ret = page_address(page);
-	if (dma_set_decrypted(dev, ret, size))
+	if ((attrs & DMA_ATTR_CC_SHARED) && dma_set_decrypted(dev, ret, size))
 		goto out_leak_pages;
 setup_page:
 	memset(ret, 0, size);
-	*dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
+	*dma_handle = phys_to_dma_direct(dev, page_to_phys(page),
+					 !!(attrs & DMA_ATTR_CC_SHARED));
 	return page;
 out_leak_pages:
 	return NULL;
@@ -461,8 +469,12 @@ void dma_direct_free_pages(struct device *dev, size_t size,
 		struct page *page, dma_addr_t dma_addr,
 		enum dma_data_direction dir)
 {
+	/*
+	 * if the device had requested for an unencrypted buffer,
+	 * convert it to encrypted on free
+	 */
+	bool mark_mem_encrypted =  force_dma_unencrypted(dev);
 	void *vaddr = page_address(page);
-	bool mark_mem_encrypted = true;
 
 	/* If cpu_addr is not from an atomic pool, dma_free_from_pool() fails */
 	if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
Signature · 1 line
2.43.0
maintainer
#14 lore raw
In reply to Aneesh Kumar K.V (Arm)
Quoted 3 lines · Aneesh Kumar K.V (Arm)

This series propagates DMA_ATTR_CC_SHARED through the dma-direct, dma-pool, and swiotlb paths so that encrypted and decrypted DMA buffers are handled consistently.

I think this series makes sense, using DMA_ATTR_CC_SHARED throughout the DMA API, either for alloc or for streaming to decide/check what bouncing does. Sashiko has a few interesting reports, it probably breaks s390 as well (it might be similar to the pKVM case).

I don't think it addresses earlier Mostafa's issues with pKVM, although I'd rather base additional pKVM related fixes on top of this series. With pKVM, cc_platform_has(CC_ATTR_MEM_ENCRYPT) returns false, as does force_dma_unencrypted(). I think we should update protected guests to return true for these if they need shared buffers (the whole decrypted/shared terminology is messy but in most places it just means buffer not private to the protected guest, whether encryption is available or not).

That said, does CC_ATTR_GUEST_MEM_ENCRYPT actually make more sense than CC_ATTR_MEM_ENCRYPT throughout this series? We'd need to change arm64 realms as well to use this one.

Signature · 1 line
Catalin
#15 lore raw
Quoted 3 lines · Catalin Marinas

That said, does CC_ATTR_GUEST_MEM_ENCRYPT actually make more sense than CC_ATTR_MEM_ENCRYPT throughout this series? We'd need to change arm64 realms as well to use this one.

It is often quite confusing in the DMA API and iommu what is guest logic and what is host logic, so this does sound nice. AFAIK none of this forced bouncing, T=1 or CC_SHARED logic applies to host in the DMA API.

Jason

#16 lore raw
In reply to Catalin Marinas
Quoted 9 lines · Catalin Marinas
Quoted 3 lines · Aneesh Kumar K.V (Arm)

This series propagates DMA_ATTR_CC_SHARED through the dma-direct, dma-pool, and swiotlb paths so that encrypted and decrypted DMA buffers are handled consistently.

I think this series makes sense, using DMA_ATTR_CC_SHARED throughout the DMA API, either for alloc or for streaming to decide/check what bouncing does. Sashiko has a few interesting reports, it probably breaks s390 as well (it might be similar to the pKVM case).

I have this series on my review list, I believe there is an overlap with my series, I can rebase mine on top of this if that makes sense, I will probably wait for a new version to address the current comments and Sashiko notes.

Thanks,
Mostafa

Quoted 16 lines

I don't think it addresses earlier Mostafa's issues with pKVM, although I'd rather base additional pKVM related fixes on top of this series. With pKVM, cc_platform_has(CC_ATTR_MEM_ENCRYPT) returns false, as does force_dma_unencrypted(). I think we should update protected guests to return true for these if they need shared buffers (the whole decrypted/shared terminology is messy but in most places it just means buffer not private to the protected guest, whether encryption is available or not).

That said, does CC_ATTR_GUEST_MEM_ENCRYPT actually make more sense than CC_ATTR_MEM_ENCRYPT throughout this series? We'd need to change arm64 realms as well to use this one.

Signature · 1 line
Catalin
#17 lore raw
Quoted 11 lines · Mostafa Saleh
Quoted 9 lines · Catalin Marinas
Quoted 3 lines · Aneesh Kumar K.V (Arm)

This series propagates DMA_ATTR_CC_SHARED through the dma-direct, dma-pool, and swiotlb paths so that encrypted and decrypted DMA buffers are handled consistently.

I think this series makes sense, using DMA_ATTR_CC_SHARED throughout the DMA API, either for alloc or for streaming to decide/check what bouncing does. Sashiko has a few interesting reports, it probably breaks s390 as well (it might be similar to the pKVM case).

Quoted 4 lines

I have this series on my review list, I believe there is an overlap with my series, I can rebase mine on top of this if that makes sense, I will probably wait for a new version to address the current comments and Sashiko notes.

I've been wondering about the two series as well.. I think this is a big help to your project, but it would be nice to know??

Jason

#18 lore raw
In reply to Catalin Marinas
Quoted 10 lines · Catalin Marinas
Quoted 3 lines · Aneesh Kumar K.V (Arm)

This series propagates DMA_ATTR_CC_SHARED through the dma-direct, dma-pool, and swiotlb paths so that encrypted and decrypted DMA buffers are handled consistently.

I think this series makes sense, using DMA_ATTR_CC_SHARED throughout the DMA API, either for alloc or for streaming to decide/check what bouncing does. Sashiko has a few interesting reports, it probably breaks s390 as well (it might be similar to the pKVM case).

I will address Shahiko’s review comments in the next revision.

With respect to s390/powerpc, I can drop SWIOTLB_FORCE from this series. However, I was not sure whether we would get enough testing for that soon. Both architectures are similar to x86/arm in forcing dma unencrypted.

powerpc:
static inline bool force_dma_unencrypted(struct device *dev)

{
	return is_secure_guest();
}

s390:
/* are we a protected virtualization guest? */
bool force_dma_unencrypted(struct device *dev)

{
	return is_prot_virt_guest();
}
Quoted 10 lines

I don't think it addresses earlier Mostafa's issues with pKVM, although I'd rather base additional pKVM related fixes on top of this series. With pKVM, cc_platform_has(CC_ATTR_MEM_ENCRYPT) returns false, as does force_dma_unencrypted(). I think we should update protected guests to return true for these if they need shared buffers (the whole decrypted/shared terminology is messy but in most places it just means buffer not private to the protected guest, whether encryption is available or not).

Quoted 4 lines

That said, does CC_ATTR_GUEST_MEM_ENCRYPT actually make more sense than CC_ATTR_MEM_ENCRYPT throughout this series? We'd need to change arm64 realms as well to use this one.

x86 memory encryption can use swiotlb boucing even on the host right?

+ if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
+ io_tlb_default_mem.decrypted = true;
+ set_memory_decrypted((unsigned long)mem->vaddr, bytes >> PAGE_SHIFT);
+ } else {
+ io_tlb_default_mem.decrypted = false;
+ }
+

-aneesh