Re: [PATCH v6 7/9] dma-buf: system_heap: Enforce shared-granule alignment for cc-shared buffers

["Christian König" <christian.koenig@amd.com>]

On 9/18/26 17:16, Catalin Marinas wrote:
> First, none of the DMA-BUF maintainers have been cc'ed. It might be fine
> for an RFC but this series got to version 6. We need their feedback.

Yeah, thanks for doing this.
> On Fri, Sep 04, 2026 at 04:04:50PM +0530, Aneesh Kumar K.V (Arm) wrote:
>> The system heap can allocate buffers that are decrypted and shared with the
>> host. For confidential-computing guests, those shared buffers must cover
>> whole shared-buffer granule; otherwise a userspace mmap of the dma-buf may
>> expose only part of a host-managed granule and allow unintended access to
>> adjacent private memory.
>>
>> Require cc-shared system-heap allocations to have a size aligned to
>> mem_cc_shared_granule_size(), and allocate pages at least as large as the
>> required granule. Keep the allocation bounded by the existing heap orders,
>> but fall back to an exact minimum-order allocation when the required
>> granule is not one of the preferred heap orders.

Uff, I don't think we can do that.

That is massively platform specific behavior in a non platform specific code.

When the architecture can only expose unencrypted memory in larger chunks than the only real alternative is to use bounce buffers and those are incompatible with DMA-buf.

Additional to that you can trivially circumvent this with a memfd and udmabuf, so you most likely don't win anything with that.

Regards,
Christian.

>>
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> ---
>>  drivers/dma-buf/heaps/system_heap.c | 50 +++++++++++++++++++++++------
>>  1 file changed, 41 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
>> index c8959eadc71d..9cbfcebe2088 100644
>> --- a/drivers/dma-buf/heaps/system_heap.c
>> +++ b/drivers/dma-buf/heaps/system_heap.c
>> @@ -55,7 +55,6 @@ struct dma_heap_attachment {
>>  #define HIGH_ORDER_GFP  (((GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN \
>>  				| __GFP_NORETRY) & ~__GFP_RECLAIM) \
>>  				| __GFP_COMP)
>> -static gfp_t order_flags[] = {HIGH_ORDER_GFP, HIGH_ORDER_GFP, LOW_ORDER_GFP};
>>  /*
>>   * The selection of the orders used for allocation (1MB, 64K, 4K) is designed
>>   * to match with the sizes often found in IOMMUs. Using order 4 pages instead
>> @@ -375,26 +374,44 @@ static const struct dma_buf_ops system_heap_buf_ops = {
>>  	.release = system_heap_dma_buf_release,
>>  };
>>  
>> +static struct page *system_heap_alloc_order(unsigned int order)
>> +{
>> +	gfp_t flags = order ? HIGH_ORDER_GFP : LOW_ORDER_GFP;
>> +
>> +	if (mem_accounting)
>> +		flags |= __GFP_ACCOUNT;
>> +
>> +	return alloc_pages(flags, order);
>> +}
>> +
>>  static struct page *alloc_largest_available(unsigned long size,
>> -					    unsigned int max_order)
>> +					    unsigned int max_order,
>> +					    unsigned int min_order)
>>  {
>>  	struct page *page;
>>  	int i;
>> -	gfp_t flags;
>>  
>>  	for (i = 0; i < NUM_ORDERS; i++) {
>>  		if (size <  (PAGE_SIZE << orders[i]))
>>  			continue;
>> -		if (max_order < orders[i])
>> +
>> +		if (max_order < orders[i] || orders[i] < min_order)
>>  			continue;
>> -		flags = order_flags[i];
>> -		if (mem_accounting)
>> -			flags |= __GFP_ACCOUNT;
>> -		page = alloc_pages(flags, orders[i]);
>> +
>> +		page = system_heap_alloc_order(orders[i]);
>>  		if (!page)
>>  			continue;
>>  		return page;
>>  	}
>> +	/*
>> +	 * The required minimum order might not be one of the preferred heap
>> +	 * orders. Allocate exactly min_order when it does not exceed the
>> +	 * remaining size.
>> +	 */
>> +	if (min_order && min_order <= max_order &&
>> +	    size >= (PAGE_SIZE << min_order))
>> +		return system_heap_alloc_order(min_order);
>> +
>>  	return NULL;
>>  }
>>  
>> @@ -409,6 +426,8 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
>>  	unsigned int max_order = orders[0];
>>  	struct system_heap_priv *priv = dma_heap_get_drvdata(heap);
>>  	bool cc_shared = priv->cc_shared;
>> +	unsigned int min_order = 0;
>> +	size_t cc_granule_size;
>>  	struct dma_buf *dmabuf;
>>  	struct sg_table *table;
>>  	struct scatterlist *sg;
>> @@ -425,6 +444,18 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
>>  	buffer->heap = heap;
>>  	buffer->len = len;
>>  	buffer->cc_shared = cc_shared;
>> +	if (cc_shared_buffer(buffer)) {
>> +		cc_granule_size = mem_cc_shared_granule_size();
>> +		if (!IS_ALIGNED(len, cc_granule_size)) {
>> +			ret = -EINVAL;
>> +			goto free_buffer;
>> +		}
>> +		min_order = get_order(cc_granule_size);
>> +		if (min_order > max_order) {
>> +			ret = -EINVAL;
>> +			goto free_buffer;
>> +		}
>> +	}
>>  
>>  	INIT_LIST_HEAD(&pages);
> 
> You need to get a proper base so that Sashiko can review this series. I
> ran it through Claude and the above is wrong to go to free_buffer which
> walks over the pages list that has not been initialised yet.
> 
> Another issue, I'm just copying it verbatim:
> 
> drivers/dma-buf/heaps/system_heap.c:379 (patch 7/9)
> system_heap_alloc_order() picks "order ? HIGH_ORDER_GFP :
> LOW_ORDER_GFP".  HIGH_ORDER_GFP clears __GFP_RECLAIM and sets
> __GFP_NORETRY, which was fine while orders 8/4 always had order 0 as a
> fallback.  The new min_order fallback at line 411 has no smaller order
> to fall back to - it is the mandatory minimum.  On a 16K-page host with
> a 4K-page guest, min_order = get_order(SZ_16K) = 2, which is not in
> orders[] = {8, 4, 0}, and order 0 is filtered out by "orders[i] <
> min_order".  Every tail allocation below 64K therefore goes through the
> non-reclaiming fallback and returns -ENOMEM under pressure that ordinary
> reclaim would have cleared.  Note any fix must keep __GFP_COMP:
> compound_order()/page_size() are used on the result
> (system_heap_set_page_decrypted(), the free paths and sg_set_page()).
>