Mostafa Saleh <[email protected]> writes:
> On Tue, May 12, 2026 at 02:33:56PM +0530, Aneesh Kumar K.V (Arm) wrote:
>> Move swiotlb allocation out of __dma_direct_alloc_pages() and handle it in
>> dma_direct_alloc() / dma_direct_alloc_pages().
>>
>> This is needed for follow-up changes that simplify the handling of
>> memory encryption/decryption based on the DMA attribute flags.
>>
>> swiotlb backing pages are already mapped decrypted by
>> swiotlb_update_mem_attributes() and rmem_swiotlb_device_init(), so
>> dma-direct should not call dma_set_decrypted() on allocation nor
>> dma_set_encrypted() on free for swiotlb-backed memory.
>>
>> Update alloc/free paths to detect swiotlb-backed pages and skip
>> encrypt/decrypt transitions for those paths. Keep the existing highmem
>> rejection in dma_direct_alloc_pages() for swiotlb allocations.
>>
>> Only for "restricted-dma-pool", we currently set `for_alloc = true`, while
>> rmem_swiotlb_device_init() decrypts the whole pool up front. This pool is
>> typically used together with "shared-dma-pool", where the shared region is
>> accessed after remap/ioremap and the returned address is suitable for
>> decrypted memory access. So existing code paths remain valid.
>>
>> Signed-off-by: Aneesh Kumar K.V (Arm) <[email protected]>
>> ---
>> kernel/dma/direct.c | 44 +++++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 37 insertions(+), 7 deletions(-)
>>
>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>> index ec887f443741..b958f150718a 100644
>> --- a/kernel/dma/direct.c
>> +++ b/kernel/dma/direct.c
>> @@ -125,9 +125,6 @@ static struct page *__dma_direct_alloc_pages(struct
>> device *dev, size_t size,
>>
>> WARN_ON_ONCE(!PAGE_ALIGNED(size));
>>
>> - if (is_swiotlb_for_alloc(dev))
>> - return dma_direct_alloc_swiotlb(dev, size);
>> -
>> gfp |= dma_direct_optimal_gfp_mask(dev, &phys_limit);
>> page = dma_alloc_contiguous(dev, size, gfp);
>> if (page) {
>> @@ -204,6 +201,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
>> {
>> bool remap = false, set_uncached = false;
>> + bool mark_mem_decrypt = true;
>> struct page *page;
>> void *ret;
>>
>> @@ -250,11 +248,21 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> dma_direct_use_pool(dev, gfp))
>> return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>>
>> + if (is_swiotlb_for_alloc(dev)) {
>> + page = dma_direct_alloc_swiotlb(dev, size);
>> + if (page) {
>> + mark_mem_decrypt = false;
>> + goto setup_page;
>> + }
>> + return NULL;
>> + }
>> +
>> /* we always manually zero the memory once we are done */
>> page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO, true);
>> if (!page)
>> return NULL;
>>
>> +setup_page:
>> /*
>> * dma_alloc_contiguous can return highmem pages depending on a
>> * combination the cma= arguments and per-arch setup. These need to be
>> @@ -281,7 +289,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> goto out_free_pages;
>> } else {
>> ret = page_address(page);
>> - if (dma_set_decrypted(dev, ret, size))
>> + if (mark_mem_decrypt && dma_set_decrypted(dev, ret, size))
>
> I am ok with that approach, but Jason was mentioning we shouldn’t
> special case swiotlb and make the allocator return the memory state
> (similar to the dma_page [1]) . I am also OK if you want to merge that
> part of my series with is.
>
> [1]
> https://lore.kernel.org/linux-iommu/[email protected]/
>
I was not sure whether we need dma_page. As shown in this series, we can
simplify the allocation and free paths without adding new abstractions
like dma_page.
>
>> goto out_leak_pages;
>> }
>>
>> @@ -298,7 +306,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> return ret;
>>
>> out_encrypt_pages:
>> - if (dma_set_encrypted(dev, page_address(page), size))
>> + if (mark_mem_decrypt && dma_set_encrypted(dev, page_address(page),
>> size))
>> return NULL;
>> out_free_pages:
>> __dma_direct_free_pages(dev, page, size);
>> @@ -310,6 +318,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>> void dma_direct_free(struct device *dev, size_t size,
>> void *cpu_addr, dma_addr_t dma_addr, unsigned long attrs)
>> {
>> + bool mark_mem_encrypted = true;
>> unsigned int page_order = get_order(size);
>>
>> if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
>> @@ -338,12 +347,15 @@ void dma_direct_free(struct device *dev, size_t size,
>> dma_free_from_pool(dev, cpu_addr, PAGE_ALIGN(size)))
>> return;
>>
>> + if (swiotlb_find_pool(dev, dma_to_phys(dev, dma_addr)))
>> + mark_mem_encrypted = false;
>> +
>> if (is_vmalloc_addr(cpu_addr)) {
>> vunmap(cpu_addr);
>> } else {
>> if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_CLEAR_UNCACHED))
>> arch_dma_clear_uncached(cpu_addr, size);
>> - if (dma_set_encrypted(dev, cpu_addr, size))
>> + if (mark_mem_encrypted && dma_set_encrypted(dev, cpu_addr,
>> size))
>> return;
>> }
>>
>> @@ -359,6 +371,19 @@ struct page *dma_direct_alloc_pages(struct device *dev,
>> size_t size,
>> if (force_dma_unencrypted(dev) && dma_direct_use_pool(dev, gfp))
>> return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
>>
>> + if (is_swiotlb_for_alloc(dev)) {
>> + page = dma_direct_alloc_swiotlb(dev, size);
>> + if (!page)
>> + return NULL;
>> +
>> + if (PageHighMem(page)) {
>
> My understanding is that rmem_swiotlb_device_init() asserts that there
> is no PageHighMem()? Also a similar check doesn’t exist in
> dma_direct_alloc().
>
The reason I added that HighMem check is that __dma_direct_alloc_pages()
already has that check.
page = dma_alloc_contiguous(dev, size, gfp);
if (page) {
if (dma_coherent_ok(dev, page_to_phys(page), size) &&
(allow_highmem || !PageHighMem(page)))
return page;
dma_free_contiguous(dev, page, size);
}
I understand that the current usage of swiotlb alloc is restricted to
restricted memory, and it will not return HighMem pages. I will drop
this hunk from the patch.
-aneesh
