[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <66db90d269408_22a22948c@dwillia2-xfh.jf.intel.com.notmuch>
Date: Fri, 6 Sep 2024 16:31:30 -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>,
<chao.gao@...el.com>, <binbin.wu@...ux.intel.com>, <adrian.hunter@...el.com>,
<kai.huang@...el.com>
Subject: Re: [PATCH v3 7/8] 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)
>
[..]
I feel what I trimmed above is a lot of text just to say:
"build_tdx_memlist() tries to fulfill the TDX module requirement it be
told about holes in the TDMR space <insert pointer / reference to
requirement>. It turns out that the kernel's view of memory holes is too
fine grained and sometimes exceeds the number of holes (16) that the TDX
module can track per TDMR <insert problematic memory map>. Thankfully
the module also lists memory that is potentially convertible in a list
of 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'. 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."
...but don't spin the patch just to make the changelog more concise. It
took me reading a few times to pull out those salient details, that is,
if I understood it correctly?
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 0fb673dd43ed..fa335ab1ae92 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -331,6 +331,72 @@ static int get_tdx_sys_info_version(struct tdx_sys_info_version *sysinfo_version
> return ret;
> }
>
> +/* Update the @sysinfo_cmr->num_cmrs to trim tail empty CMRs */
> +static void trim_empty_tail_cmrs(struct tdx_sys_info_cmr *sysinfo_cmr)
> +{
> + int i;
> +
> + for (i = 0; i < sysinfo_cmr->num_cmrs; i++) {
> + u64 cmr_base = sysinfo_cmr->cmr_base[i];
> + u64 cmr_size = sysinfo_cmr->cmr_size[i];
> +
> + if (!cmr_size) {
> + WARN_ON_ONCE(cmr_base);
> + break;
> + }
> +
> + /* TDX architecture: CMR must be 4KB aligned */
> + WARN_ON_ONCE(!PAGE_ALIGNED(cmr_base) ||
> + !PAGE_ALIGNED(cmr_size));
Is it really required to let the TDX module taint and possibly crash the
kernel if it provides misaligned CMRs? Shouldn't these be validated
early and just turn off TDX support if the TDX module is this broken?
Powered by blists - more mailing lists