Re: [PATCH v6 08/10] x86/virt/tdx: Reduce TDMR's reserved areas by using CMRs to find memory holes

[Dan Williams <dan.j.williams@intel.com>]

Kai Huang wrote:
> A TDX module initialization failure was reported on a Emerald Rapids
> platform [*]:
> 
>   virt/tdx: initialization failed: TDMR [0x0, 0x80000000): reserved areas exhausted.
>   virt/tdx: module initialization failed (-28)
> 
> As part of initializing the TDX module, the kernel informs the TDX
> module of all "TDX-usable memory regions" using an array of TDX defined
> structure "TD Memory Region" (TDMR).  Each TDMR must be in 1GB aligned
> and in 1GB granularity, and all "non-TDX-usable memory holes" within a
> given TDMR are marked as "reserved areas".  The TDX module reports a
> maximum number of reserved areas that can be supported per TDMR (16).
> 
> The kernel builds the "TDX-usable memory regions" based on memblocks
> (which reflects e820), and uses this list to find all "reserved areas"
> for each TDMR.
> 
> It turns out that the kernel's view of memory holes is too fine grained
> and sometimes exceeds the number of holes that the TDX module can track
> per TDMR [1], resulting in the above failure.
> 
> Thankfully the module also lists memory that is potentially convertible
> in a list of "Convertible Memory Regions" (CMRs).  That coarser grained
> CMR list tends to track usable memory in the memory map even if it might
> be reserved for host usage like 'ACPI data' [2].
> 
> Use that list to relax what the kernel considers unusable memory.  If it
> falls in a CMR no need to instantiate a hole, and rely on the fact that
> kernel will keep what it considers 'reserved' out of the page allocator.
[..]
> Link: https://github.com/canonical/tdx/issues/135 [*]
> Fixes: dde3b60d572c ("x86/virt/tdx: Designate reserved areas for all TDMRs")
> Signed-off-by: Kai Huang <kai.huang@intel.com>
> ---
>  arch/x86/virt/vmx/tdx/tdx.c | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index e81bdcfc20bf..9acb12c75e9b 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -747,29 +747,28 @@ static int tdmr_add_rsvd_area(struct tdmr_info *tdmr, int *p_idx, u64 addr,
>  }
>  
>  /*
> - * Go through @tmb_list to find holes between memory areas.  If any of
> + * Go through all CMRs in @sysinfo_cmr to find memory holes.  If any of
>   * those holes fall within @tdmr, set up a TDMR reserved area to cover
>   * the hole.
>   */
> -static int tdmr_populate_rsvd_holes(struct list_head *tmb_list,
> +static int tdmr_populate_rsvd_holes(struct tdx_sys_info_cmr *sysinfo_cmr,
>  				    struct tdmr_info *tdmr,
>  				    int *rsvd_idx,
>  				    u16 max_reserved_per_tdmr)
>  {
> -	struct tdx_memblock *tmb;
>  	u64 prev_end;
> -	int ret;
> +	int i, ret;
>  
>  	/*
>  	 * Start looking for reserved blocks at the
>  	 * beginning of the TDMR.
>  	 */
>  	prev_end = tdmr->base;
> -	list_for_each_entry(tmb, tmb_list, list) {
> +	for (i = 0; i < sysinfo_cmr->num_cmrs; i++) {
>  		u64 start, end;
>  
> -		start = PFN_PHYS(tmb->start_pfn);
> -		end   = PFN_PHYS(tmb->end_pfn);
> +		start = sysinfo_cmr->cmr_base[i];
> +		end   = start + sysinfo_cmr->cmr_size[i];

This had me go check the inclusive vs exclusive range comparisons. Even
though it is not in this patch I think tdmr_populate_rsvd_holes() needs
this fixup:

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 4e2b2e2ac9f9..b5026edf1eeb 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -776,7 +776,7 @@ static int tdmr_populate_rsvd_holes(struct list_head *tmb_list,
                        break;
 
                /* Exclude regions before this TDMR */
-               if (end < tdmr->base)
+               if (end <= tdmr->base)
                        continue;
 
                /*

...because a CMR that ends at tdmr->base is still "before" the TDMR.

As that's a separate fixup you can add for this patch.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>