From: Aneesh Kumar K.V <[email protected]> Sent: Monday, June 1, 2026 11:05 PM > > Michael Kelley <[email protected]> writes: > > > From: Aneesh Kumar K.V (Arm) <[email protected]>Sent: Thursday, May > > 21, 2026 9:28 PM > >> > >> Teach the atomic DMA pool code to distinguish between encrypted and > >> unencrypted 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 > >> unencrypted, 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 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. > >> > >> Tested-by: Jiri Pirko <[email protected]> > >> Reviewed-by: Mostafa Saleh <[email protected]> > >> Signed-off-by: Aneesh Kumar K.V (Arm) <[email protected]> > >> --- > >> drivers/iommu/dma-iommu.c | 2 +- > >> include/linux/dma-map-ops.h | 2 +- > >> kernel/dma/direct.c | 11 ++- > >> kernel/dma/pool.c | 167 +++++++++++++++++++++++------------- > >> kernel/dma/swiotlb.c | 7 +- > >> 5 files changed, 123 insertions(+), 66 deletions(-) > >> > > > > [snip] > > > >> +static __init struct dma_gen_pool *__dma_atomic_pool_init(struct > >> dma_gen_pool *dma_pool, > >> + size_t pool_size, gfp_t gfp) > >> { > >> - struct gen_pool *pool; > >> int ret; > >> > >> - pool = gen_pool_create(PAGE_SHIFT, NUMA_NO_NODE); > >> - if (!pool) > >> + dma_pool->pool = gen_pool_create(PAGE_SHIFT, NUMA_NO_NODE); > >> + if (!dma_pool->pool) > >> return NULL; > >> > >> - gen_pool_set_algo(pool, gen_pool_first_fit_order_align, NULL); > >> + gen_pool_set_algo(dma_pool->pool, gen_pool_first_fit_order_align, NULL); > >> + > >> + /* if platform is using memory encryption atomic pools are by default > >> decrypted. */ > >> + if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) > >> + dma_pool->unencrypted = true; > >> + else > >> + dma_pool->unencrypted = false; > > > > I'm curious about the name of the "unencrypted" field in struct > > dma_gen_pool, > > and similarly in Patch 7 of the series for the swiotlb struct io_tlb_pool > > and > > struct io_tlb_mem. Up through v3 of this series, you used "decrypted", but > > starting in v4 switched to "unencrypted". > > > > To me, the above "if" statement has some cognitive dissonance in that if > > CC_ATTR_MEM_ENCRYPT is false (i.e., a normal VM), "unencrypted" is set > > to false. But I think of memory in a normal VM as "unencrypted" since it > > was never encrypted. A similar "if" statement occurs in your swiotlb > > changes. > > > > Two related concepts are captured by the field: > > 1) Is some action needed to put the memory into the unencrypted state, > > and to remove it from that state? This applies when assigning memory to the > > pool, or freeing the memory in the pool. > > 2) Is the memory currently in the unencrypted state? This applies when > > allocating memory from the pool to a caller. > > > > It's hard to capture all that in a short field name. But I think I prefer > > "decrypted" > > over "unencrypted". The former implies that some action was taken. It's a > > little easier to think of a normal VM as *not* having decrypted memory. The > > memory was never encrypted in the first place, so no decryption action was > > taken. > > > > Throughout the kernel, "decrypted" occurs much more frequently than > > "unencrypted". We have set_memory_encrypted() and set_memory_decrypted() > > that are "take action" names. But we also have force_dma_unencrypted(), > > phys_to_dma_unencrypted(), and dma_addr_unencrypted(). So it's a bit > > of a mess. > > > > > > But maybe there's more background here that led to the change > > between your v3 and v4. > > > > Michael > > The current APIs, phys_to_dma_unencrypted() and dma_addr_unencrypted(), > are the reason I changed the pool attribute name from decrypted to > unencrypted. The rationale was that nobody actually decrypted the > memory; the memory was already in an unencrypted state. > > In other words, the DMA pool did not contain encrypted content that was > later decrypted. Rather, the DMA pool itself was in an unencrypted > state. > > IMHO, set_memory_decrypted()/set_memory_encrypted() is the right naming > because those APIs describe an operation that transitions memory between > states. In contrast, the pool attribute describes the state of the > memory itself, which is why I used unencrypted rather than decrypted. >
Except that in a normal VM, the "unencrypted" pool attribute does *not* describe the state of the memory itself. In a normal VM, the memory is unencrypted, but the "unencrypted" pool attribute is false. That contradiction is the essence of my concern. Michael
