Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] dma-pool: cc_make_shared() runs after arch_dma_prep_coherent() --
commit daae4fcc15a00bc83c4081ef4b27786a7f224537 Author: Aneesh Kumar K.V (Arm) <[email protected]> dma-pool: Allocate CoCo atomic pools using CoCo shared memory allocator This commit updates atomic_pool_expand() to allocate confidential computing shared atomic DMA pools using the cc_shared layout and transition helpers (cc_shared_calc_layout(), cc_make_shared(), and cc_make_private()), enforcing shared granule size alignment and minimum allocation order. > diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c > index 70b7f64b17ab5..da487c3f9dc61 100644 > --- a/kernel/dma/pool.c > +++ b/kernel/dma/pool.c [ ... ] > @@ -126,8 +142,7 @@ static int atomic_pool_expand(struct dma_gen_pool > *dma_pool, size_t pool_size, > * shrink so no re-encryption occurs in dma_direct_free(). > */ > if (dma_pool->cc_shared) { > - ret = set_memory_decrypted((unsigned long)page_to_virt(page), > - 1 << order); > + ret = cc_make_shared(page_to_virt(page), pool_size); [Severity: High] Does calling cc_make_shared(page_to_virt(page), pool_size) after arch_dma_prep_coherent() and dma_common_contiguous_remap() leave dirty cache lines on the cacheable linear mapping alias when CONFIG_DMA_DIRECT_REMAP is enabled? In atomic_pool_expand(), arch_dma_prep_coherent() and the vmap remapping run first: kernel/dma/pool.c:atomic_pool_expand() { ... arch_dma_prep_coherent(page, pool_size); #ifdef CONFIG_DMA_DIRECT_REMAP 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; #else addr = page_to_virt(page); #endif if (dma_pool->cc_shared) { ret = cc_make_shared(page_to_virt(page), pool_size); ... } Because set_memory_decrypted() (in commit 0a47431deefc ("mm: Zero memory during shared memory transitions")) zeroes the range through the cacheable linear map address passed to it: cc_make_shared() -> set_memory_decrypted() -> realm_set_memory_decrypted() / pkvm_set_memory_decrypted() -> memset((void *)addr, 0, (size_t)numpages << PAGE_SHIFT); performing this transition after arch_dma_prep_coherent() has already cleaned the linear map alias dirties the CPU data cache for page_to_virt(page) with zeros without a subsequent cache clean (and on Arm CCA, cleans the protected physical address alias before the RIPAS transition rather than the shared physical address alias after zeroing). Those dirty cache lines on the linear mapping can later be evicted to the Point of Coherency and overwrite live DMA buffers accessed via the non-cacheable vmap alias at addr. In contrast, dma_direct_alloc() transitions the pages before calling arch_dma_prep_coherent() and dma_common_contiguous_remap(). Should atomic_pool_expand() similarly call cc_make_shared() before arch_dma_prep_coherent() and dma_common_contiguous_remap()? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
