Petr Tesarik <[email protected]> writes:
> On Thu, 4 Jun 2026 14:09:45 +0530
> "Aneesh Kumar K.V (Arm)" <[email protected]> wrote:
>
...
>> /*
>> * If memory encryption is supported, phys_to_dma will set the memory
>> encryption
>> * bit in the DMA address, and dma_to_phys will clear it.
>> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
>> index 29187cec90d8..4dcbf3931be1 100644
>> --- a/include/linux/swiotlb.h
>> +++ b/include/linux/swiotlb.h
>> @@ -81,6 +81,7 @@ struct io_tlb_pool {
>> struct list_head node;
>> struct rcu_head rcu;
>> bool transient;
>> + bool unencrypted;
>
> IIUC this is a copy of the unencrypted member in the corresponding
> struct io_tlb_mem. In other words, if pools are allocated dynamically,
> all pools must have the same encryption state, correct?
>
That is correct. The reason we have the unencrypted member in struct
io_tlb_pool is that we need it in swiotlb_dyn_free().
When freeing memory from an unencrypted pool, we need to convert the
memory back to private/encrypted
>
>> #endif
>> };
>>
>> @@ -111,6 +112,7 @@ struct io_tlb_mem {
>> struct dentry *debugfs;
>> bool force_bounce;
>> bool for_alloc;
>> + bool unencrypted;
>> #ifdef CONFIG_SWIOTLB_DYNAMIC
>> bool can_grow;
>> u64 phys_limit;
>> @@ -282,7 +284,8 @@ static inline void swiotlb_sync_single_for_cpu(struct
>> device *dev,
>> extern void swiotlb_print_info(void);
....
>> @@ -604,30 +621,26 @@ static struct page *alloc_dma_pages(gfp_t gfp, size_t
>> bytes, u64 phys_limit)
>> * @dev: Device for which a memory pool is allocated.
>> * @bytes: Size of the buffer.
>> * @phys_limit: Maximum allowed physical address of the buffer.
>> + * @attrs: DMA attributes for the allocation.
>> * @gfp: GFP flags for the allocation.
>> *
>> * Return: Allocated pages, or %NULL on allocation failure.
>> */
>> static struct page *swiotlb_alloc_tlb(struct device *dev, size_t bytes,
>> - u64 phys_limit, gfp_t gfp)
>> + u64 phys_limit, unsigned long attrs, gfp_t gfp)
>
> If my assumption above is correct, then I prefer to add a struct
> io_tlb_mem *mem parameter here and calculate the allocation attributes
> inside this function, so you don't have to repeat it in the callers.
>
Will switch to that. That is, we will use struct io_tlb_mem->unencrypted
to determine the pool attribute instead of using unsigned long attrs
>
>> {
>> struct page *page;
>> - unsigned long attrs = 0;
>>
>> /*
>> * Allocate from the atomic pools if memory is encrypted and
>> * the allocation is atomic, because decrypting may block.
>> */
>> - if (!gfpflags_allow_blocking(gfp) && dev && force_dma_unencrypted(dev))
>> {
>> + if (!gfpflags_allow_blocking(gfp) && (attrs & DMA_ATTR_CC_SHARED)) {
>
> You're removing the check that dev is non-NULL. This is fine, because
> the only call with dev == NULL is from swiotlb_dyn_alloc(), and that one
> uses GFP_KERNEL (i.e. allows blocking). However, if this is an intended
> optimization, I'd rather have it in a separate commit, with this
> explanation why it's OK to do it.
>
> The rest of the patch looks good to me.
>
I'll add that back.
-aneesh
