Jason Gunthorpe <[email protected]> writes:
> On Wed, Jul 01, 2026 at 11:19:15AM +0530, Aneesh Kumar K.V (Arm) wrote:
>> @@ -114,14 +120,17 @@ static int atomic_pool_expand(struct gen_pool *pool,
>> size_t pool_size,
>> * Memory in the atomic DMA pools must be unencrypted, the pools do not
>> * shrink so no re-encryption occurs in dma_direct_free().
>> */
>> - ret = set_memory_decrypted((unsigned long)page_to_virt(page),
>> - 1 << order);
>> - if (ret) {
>> - leak_pages = true;
>> - goto remove_mapping;
>> + if (dma_pool->cc_shared) {
>> + ret = set_memory_decrypted((unsigned long)page_to_virt(page),
>> + 1 << order);
>> + if (ret) {
>> + leak_pages = true;
>> + goto remove_mapping;
>> + }
>> }
>
> This makes the memory_decrypted conditional, but it doesn't change
> the lines a few above:
>
> addr = dma_common_contiguous_remap(page, pool_size,
> pgprot_decrypted(pgprot_dmacoherent(PAGE_KERNEL)),
> ^^^^^^^^^^^^
> __builtin_return_address(0));
> if (!addr)
> goto free_page;
>
> It is wrong to pass pgprot_decrypted() to the arch code if
> set_memory_decrypted() was not called.
>
> Also it looks at some point the nature of the atomic pool has become
> confused. Originally it was just to allocate atomic memory that had
> been vmap'd outside the atomic context (to set the non-coherent
> pgprot), so every caller was expecting non-cached memory.
>
> Then it was reused to also allocate CC shared memory outside the
> atomic context. That was fine for x86 that doesn't use DMA_REMAP but
> on ARM64 it now means all atomic pool CC memory is uncached? That
> doesn't seem to make any sense...
>
> I suppose along the lines of this patch the solution is to add a
> noncoherent property to the pool so we can select the correct
> combination:
>
> noncoherent !SHARED = vmap pgprot_noncached
> !noncoherent SHARED= vmap pgprot_decrypted + set_memory_decrypted
> noncoherent SHARED = (probably unrealistic in real systems)
> !noncoherent !SHARED = normal __dma_direct_alloc_pages()
>
> But I don't view this as that important, the CC hypervisor is probably
> going to use the S2 page table to force cachable on all system memory
> so the non-cached pgprot is a NOP, but the extra vmap is wasteful and
> it is confusing.. So maybe a little fixme is all that is needed here.
>
Something like?
modified kernel/dma/direct.c
@@ -260,6 +260,9 @@ 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.
+ * FIXME!! With CONFIG_DMA_DIRECT_REMAP, the pool is also mapped as
+ * DMA-coherent (non-cacheable). We may want to create a separate pool
+ * dedicated to CC_SHARED atomic allocations.
*/
if ((remap || (attrs & __DMA_ATTR_ALLOC_CC_SHARED)) &&
dma_direct_use_pool(dev, gfp)) {
modified kernel/dma/pool.c
@@ -88,6 +88,7 @@ static int atomic_pool_expand(struct dma_gen_pool *dma_pool,
size_t pool_size,
unsigned int order;
struct page *page = NULL;
bool leak_pages = false;
+ pgprot_t prot;
void *addr;
int ret = -ENOMEM;
unsigned int min_encrypt_order =
get_order(mem_cc_shared_granule_size());
@@ -110,8 +111,12 @@ static int atomic_pool_expand(struct dma_gen_pool
*dma_pool, size_t pool_size,
arch_dma_prep_coherent(page, pool_size);
#ifdef CONFIG_DMA_DIRECT_REMAP
- addr = dma_common_contiguous_remap(page, pool_size,
- pgprot_decrypted(pgprot_dmacoherent(PAGE_KERNEL)),
+ if (dma_pool->cc_shared)
+ prot = pgprot_decrypted(pgprot_dmacoherent(PAGE_KERNEL));
+ else
+ prot = pgprot_dmacoherent(PAGE_KERNEL);
+
+ addr = dma_common_contiguous_remap(page, pool_size, prot,
__builtin_return_address(0));
if (!addr)
goto free_page;
