Re: [PATCH v6 04/20] dma-pool: track decrypted atomic pools and select them via attrs
From: Jason Gunthorpe <jgg@ziepe.ca>
Date: 2026-06-10 16:41:56
Also in:
linux-arm-kernel, linux-coco, linux-iommu, linux-s390, lkml
On Wed, Jun 10, 2026 at 01:37:26PM +0530, Aneesh Kumar K.V wrote:
Jason Gunthorpe [off-list ref] writes:quoted
On Thu, Jun 04, 2026 at 02:09:43PM +0530, Aneesh Kumar K.V (Arm) wrote:quoted
struct page *dma_alloc_from_pool(struct device *dev, size_t size, - void **cpu_addr, gfp_t gfp, + void **cpu_addr, gfp_t gfp, unsigned long attrs, bool (*phys_addr_ok)(struct device *, phys_addr_t, size_t)) { - struct gen_pool *pool = NULL; + struct dma_gen_pool *dma_pool = NULL; struct page *page; bool pool_found = false; - while ((pool = dma_guess_pool(pool, gfp))) { + while ((dma_pool = dma_guess_pool(dma_pool, gfp))) { + + if (dma_pool->unencrypted != !!(attrs & DMA_ATTR_CC_SHARED)) + continue;I don't think you should be overloading DMA_ATTR_CC_SHARED like this. /* * 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; } It is fine to have a bit inside the attrs that is only used by the internal logic, but it needs to have a clearer name __DMA_ATTR_REQUIRE_CC_SHARED perhaps.Are you suggesting adding another attribute in addition to DMA_ATTR_CC_SHARED? Is the idea that __DMA_ATTR_REQUIRE_CC_SHARED would be used in the allocation path to request a CC_SHARED allocation, while DMA_ATTR_CC_SHARED would be used in the mapping path to describe the attribute of the address?
Yeah, it is a thought at least Maybe a comment is good enough. I just find it hard to follow when we have this dual usage. Like the code above for dma_pool->unencrypted is completely wrong if it is an "attribute of an address". Easy to cut & paste that into the wrong context. Especially if you move things up higher.. having the alloc set both CC_SHARED and REQUIRE_CC_SHARED or maybe ALLOC_CC_SHARED would make it clearer that the alloc code lives under that callchain Jason