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

Xu Yilun posted 31 patches 5 days, 23 hours ago
[PATCH v2 04/31] x86/virt/tdx: Support allocating contiguous pages for tdx_page_array
Posted by Xu Yilun 5 days, 23 hours ago
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@linux.intel.com>
---
 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)
 {
 	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)
-- 
2.25.1
Re: [PATCH v2 04/31] x86/virt/tdx: Support allocating contiguous pages for tdx_page_array
Posted by Nikolay Borisov 3 days, 1 hour ago

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@linux.intel.com>
> ---
>   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)

Re: [PATCH v2 04/31] x86/virt/tdx: Support allocating contiguous pages for tdx_page_array
Posted by Xu Yilun 2 days, 2 hours ago
> >   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.

Thank for the suggestion. There are also other concerns in this thread,
but I'll take this into consideration.