[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <67200f7c12a55_bc69d29456@dwillia2-xfh.jf.intel.com.notmuch>
Date: Mon, 28 Oct 2024 15:26:04 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: Kai Huang <kai.huang@...el.com>, <dave.hansen@...el.com>,
<kirill.shutemov@...ux.intel.com>, <tglx@...utronix.de>, <bp@...en8.de>,
<peterz@...radead.org>, <mingo@...hat.com>, <hpa@...or.com>,
<dan.j.williams@...el.com>, <seanjc@...gle.com>, <pbonzini@...hat.com>
CC: <x86@...nel.org>, <linux-kernel@...r.kernel.org>, <kvm@...r.kernel.org>,
<rick.p.edgecombe@...el.com>, <isaku.yamahata@...el.com>,
<adrian.hunter@...el.com>, <nik.borisov@...e.com>, <kai.huang@...el.com>
Subject: Re: [PATCH v6 08/10] x86/virt/tdx: Reduce TDMR's reserved areas by
using CMRs to find memory holes
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@...el.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@...el.com>
Powered by blists - more mailing lists