Re: [PATCH v2 04/31] x86/virt/tdx: Support allocating contiguous pages for tdx_page_array

[Nikolay Borisov]

On 27.03.26 г. 18:01 ч., Xu Yilun wrote:
> The current tdx_page_array implementation allocates scattered order-0
> pages. However, some TDX Module operations benefit from contiguous
> physical memory. E.g. Enabling TDX Module Extensions (an optional TDX
> feature) requires ~50MB memory and never returns. Such allocation
> would at worst cause ~25GB permanently fragmented memory if each
> allocated page is from a different 2M region.
>
> Support allocating contiguous pages for tdx_page_array by making the
> allocation method configurable. Change the tdx_page_array_alloc() to
> accept a custom allocation function pointer and a context parameter.
> Wrap the specific allocation into a tdx_page_array_alloc_contig()
> helper.
>
> The foreseeable caller will allocate ~50MB memory with this helper,
> exceeding the maximum HPAs (512) a root page can hold, the typical usage
> will be:
>
>   - struct tdx_page_array *array = tdx_page_array_alloc_contig(nr_pages);
>   - for each 512-page bulk
>     - tdx_page_array_populate(array, offset);
>     - seamcall(TDH_XXX_ADD, array, ...);
>
> The configurable allocation method would also benefit more
> tdx_page_array usages. TDX Module may require more specific memory
> layouts encoded in the root page. Will introduce them in following
> patches.
>
> Signed-off-by: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
> ---
>   arch/x86/virt/vmx/tdx/tdx.c | 42 +++++++++++++++++++++++++++++++++----
>   1 file changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index a3021e7e2490..6c4ed80e8e5a 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -289,7 +289,8 @@ static void tdx_free_pages_bulk(unsigned int nr_pages, struct page **pages)
>   		__free_page(pages[i]);
>   }
>   
> -static int tdx_alloc_pages_bulk(unsigned int nr_pages, struct page **pages)
> +static int tdx_alloc_pages_bulk(unsigned int nr_pages, struct page **pages,
> +				void *data)
>   {
>   	unsigned int filled, done = 0;
>   
> @@ -326,7 +327,10 @@ void tdx_page_array_free(struct tdx_page_array *array)
>   EXPORT_SYMBOL_GPL(tdx_page_array_free);
>   
>   static struct tdx_page_array *
> -tdx_page_array_alloc(unsigned int nr_pages)
> +tdx_page_array_alloc(unsigned int nr_pages,
> +		     int (*alloc_fn)(unsigned int nr_pages,
> +				     struct page **pages, void *data),
> +		     void *data)

This interface seems cumbersome, since you will always have separate 
allocation paths:

Contig, Bulk and Iommu mt pages let's just keep them separate. I.e the 
flow should be:

1. Do common allocation by calling tdx_page_array_alloc (you aren't 
passing the alloc function) you just get a bare-bones tdx_page_array struct

2. Do the specific allocation in either :

tdx_page_array_create - for the bulk case
tdx_page_array_alloc_contig - for the contig case
tdx_page_array_create_iommu_mt - for the iommu case. Here you can open 
code tdx_alloc_pages_iommu_mt.

And keep the specific clearly separate in each function.

>   {
>   	struct tdx_page_array *array = NULL;
>   	struct page **pages = NULL;
> @@ -348,7 +352,7 @@ tdx_page_array_alloc(unsigned int nr_pages)
>   	if (!pages)
>   		goto out_free;
>   
> -	ret = tdx_alloc_pages_bulk(nr_pages, pages);
> +	ret = alloc_fn(nr_pages, pages, data);
>   	if (ret)
>   		goto out_free;
>   
> @@ -388,7 +392,7 @@ struct tdx_page_array *tdx_page_array_create(unsigned int nr_pages)
>   	if (nr_pages > TDX_PAGE_ARRAY_MAX_NENTS)
>   		return NULL;
>   
> -	array = tdx_page_array_alloc(nr_pages);
> +	array = tdx_page_array_alloc(nr_pages, tdx_alloc_pages_bulk, NULL);
>   	if (!array)
>   		return NULL;
>   
> @@ -521,6 +525,36 @@ int tdx_page_array_ctrl_release(struct tdx_page_array *array,
>   }
>   EXPORT_SYMBOL_GPL(tdx_page_array_ctrl_release);
>   
> +static int tdx_alloc_pages_contig(unsigned int nr_pages, struct page **pages,
> +				  void *data)
> +{
> +	struct page *page;
> +	int i;
> +
> +	page = alloc_contig_pages(nr_pages, GFP_KERNEL, numa_mem_id(),
> +				  &node_online_map);
> +	if (!page)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < nr_pages; i++)
> +		pages[i] = page + i;
> +
> +	return 0;
> +}
> +
> +/*
> + * For holding large number of contiguous pages, usually larger than
> + * TDX_PAGE_ARRAY_MAX_NENTS (512).
> + *
> + * Similar to tdx_page_array_alloc(), after allocating with this
> + * function, call tdx_page_array_populate() to populate the tdx_page_array.
> + */
> +static __maybe_unused struct tdx_page_array *
> +tdx_page_array_alloc_contig(unsigned int nr_pages)
> +{
> +	return tdx_page_array_alloc(nr_pages, tdx_alloc_pages_contig, NULL);
> +}
> +
>   #define HPA_LIST_INFO_FIRST_ENTRY	GENMASK_U64(11, 3)
>   #define HPA_LIST_INFO_PFN		GENMASK_U64(51, 12)
>   #define HPA_LIST_INFO_LAST_ENTRY	GENMASK_U64(63, 55)