Re: [PATCH v3 7/8] x86/virt/tdx: Reduce TDMR's reserved areas by using CMRs to find memory holes

[Dan Williams]

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?