Re: [PATCH v6 06/20] dma: swiotlb: track pool encryption state and honor DMA_ATTR_CC_SHARED
From: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
Date: 2026-06-10 08:46:40
Also in:
linux-arm-kernel, linux-coco, linux-iommu, linux-s390, lkml
Petr Tesarik [off-list ref] writes:
On Thu, 4 Jun 2026 14:09:45 +0530 "Aneesh Kumar K.V (Arm)" [off-list ref] wrote:
...
quoted
/* * 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.diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h index 29187cec90d8..4dcbf3931be1 100644 --- a/include/linux/swiotlb.h +++ b/include/linux/swiotlb.h@@ -81,6 +81,7 @@ struct io_tlb_pool { struct list_head node; struct rcu_head rcu; bool transient; + bool unencrypted;IIUC this is a copy of the unencrypted member in the corresponding struct io_tlb_mem. In other words, if pools are allocated dynamically, all pools must have the same encryption state, correct?
That is correct. The reason we have the unencrypted member in struct io_tlb_pool is that we need it in swiotlb_dyn_free(). When freeing memory from an unencrypted pool, we need to convert the memory back to private/encrypted
quoted
#endif };@@ -111,6 +112,7 @@ struct io_tlb_mem { struct dentry *debugfs; bool force_bounce; bool for_alloc; + bool unencrypted; #ifdef CONFIG_SWIOTLB_DYNAMIC bool can_grow; u64 phys_limit;@@ -282,7 +284,8 @@ static inline void swiotlb_sync_single_for_cpu(struct device *dev, extern void swiotlb_print_info(void);
....
quoted
@@ -604,30 +621,26 @@ static struct page *alloc_dma_pages(gfp_t gfp, size_t bytes, u64 phys_limit) * @dev: Device for which a memory pool is allocated. * @bytes: Size of the buffer. * @phys_limit: Maximum allowed physical address of the buffer. + * @attrs: DMA attributes for the allocation. * @gfp: GFP flags for the allocation. * * Return: Allocated pages, or %NULL on allocation failure. */ static struct page *swiotlb_alloc_tlb(struct device *dev, size_t bytes, - u64 phys_limit, gfp_t gfp) + u64 phys_limit, unsigned long attrs, gfp_t gfp)If my assumption above is correct, then I prefer to add a struct io_tlb_mem *mem parameter here and calculate the allocation attributes inside this function, so you don't have to repeat it in the callers.
Will switch to that. That is, we will use struct io_tlb_mem->unencrypted to determine the pool attribute instead of using unsigned long attrs
quoted
{ struct page *page; - unsigned long attrs = 0; /* * Allocate from the atomic pools if memory is encrypted and * the allocation is atomic, because decrypting may block. */ - if (!gfpflags_allow_blocking(gfp) && dev && force_dma_unencrypted(dev)) { + if (!gfpflags_allow_blocking(gfp) && (attrs & DMA_ATTR_CC_SHARED)) {You're removing the check that dev is non-NULL. This is fine, because the only call with dev == NULL is from swiotlb_dyn_alloc(), and that one uses GFP_KERNEL (i.e. allows blocking). However, if this is an intended optimization, I'd rather have it in a separate commit, with this explanation why it's OK to do it. The rest of the patch looks good to me.
I'll add that back. -aneesh