Re: [PATCH v2 03/25] x86/virt/tdx: Read essential global metadata for KVM

[Xiaoyao Li <xiaoyao.li@intel.com>]

On 10/31/2024 3:00 AM, Rick Edgecombe wrote:
> From: Kai Huang <kai.huang@intel.com>
> 
> KVM needs two classes of global metadata to create and run TDX guests:
> 
>   - "TD Control Structures"
>   - "TD Configurability"
> 
> The first class contains the sizes of TDX guest per-VM and per-vCPU
> control structures.  KVM will need to use them to allocate enough space
> for those control structures.
> 
> The second class contains info which reports things like which features
> are configurable to TDX guest etc.  KVM will need to use them to
> properly configure TDX guests.
> 
> Read them for KVM TDX to use.
> 
> The code change is auto-generated by re-running the script in [1] after
> uncommenting the "td_conf" and "td_ctrl" part to regenerate the
> tdx_global_metadata.{hc} and update them to the existing ones in the
> kernel.
> 
>    #python tdx.py global_metadata.json tdx_global_metadata.h \
> 	tdx_global_metadata.c
> 
> The 'global_metadata.json' can be fetched from [2].
> 
> Link: https://lore.kernel.org/kvm/0853b155ec9aac09c594caa60914ed6ea4dc0a71.camel@intel.com/ [1]
> Link: https://cdrdv2.intel.com/v1/dl/getContent/795381 [2]
> Signed-off-by: Kai Huang <kai.huang@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> ---
> uAPI breakout v2:
>   - New patch
> ---
>   arch/x86/include/asm/tdx_global_metadata.h  | 19 +++++++++
>   arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 46 +++++++++++++++++++++
>   2 files changed, 65 insertions(+)
> 
> diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
> index fde370b855f1..206090c9952f 100644
> --- a/arch/x86/include/asm/tdx_global_metadata.h
> +++ b/arch/x86/include/asm/tdx_global_metadata.h
> @@ -32,11 +32,30 @@ struct tdx_sys_info_cmr {
>   	u64 cmr_size[32];
>   };
>   
> +struct tdx_sys_info_td_ctrl {
> +	u16 tdr_base_size;
> +	u16 tdcs_base_size;
> +	u16 tdvps_base_size;
> +};
> +
> +struct tdx_sys_info_td_conf {
> +	u64 attributes_fixed0;
> +	u64 attributes_fixed1;
> +	u64 xfam_fixed0;
> +	u64 xfam_fixed1;
> +	u16 num_cpuid_config;
> +	u16 max_vcpus_per_td;
> +	u64 cpuid_config_leaves[32];
> +	u64 cpuid_config_values[32][2];
> +};
> +
>   struct tdx_sys_info {
>   	struct tdx_sys_info_version version;
>   	struct tdx_sys_info_features features;
>   	struct tdx_sys_info_tdmr tdmr;
>   	struct tdx_sys_info_cmr cmr;
> +	struct tdx_sys_info_td_ctrl td_ctrl;
> +	struct tdx_sys_info_td_conf td_conf;
>   };
>   
>   #endif
> diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> index 2fe57e084453..44c2b3e079de 100644
> --- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> +++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> @@ -76,6 +76,50 @@ static int get_tdx_sys_info_cmr(struct tdx_sys_info_cmr *sysinfo_cmr)
>   	return ret;
>   }
>   
> +static int get_tdx_sys_info_td_ctrl(struct tdx_sys_info_td_ctrl *sysinfo_td_ctrl)
> +{
> +	int ret = 0;
> +	u64 val;
> +
> +	if (!ret && !(ret = read_sys_metadata_field(0x9800000100000000, &val)))
> +		sysinfo_td_ctrl->tdr_base_size = val;
> +	if (!ret && !(ret = read_sys_metadata_field(0x9800000100000100, &val)))
> +		sysinfo_td_ctrl->tdcs_base_size = val;
> +	if (!ret && !(ret = read_sys_metadata_field(0x9800000100000200, &val)))
> +		sysinfo_td_ctrl->tdvps_base_size = val;
> +
> +	return ret;
> +}
> +
> +static int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *sysinfo_td_conf)
> +{
> +	int ret = 0;
> +	u64 val;
> +	int i, j;
> +
> +	if (!ret && !(ret = read_sys_metadata_field(0x1900000300000000, &val)))
> +		sysinfo_td_conf->attributes_fixed0 = val;
> +	if (!ret && !(ret = read_sys_metadata_field(0x1900000300000001, &val)))
> +		sysinfo_td_conf->attributes_fixed1 = val;
> +	if (!ret && !(ret = read_sys_metadata_field(0x1900000300000002, &val)))
> +		sysinfo_td_conf->xfam_fixed0 = val;
> +	if (!ret && !(ret = read_sys_metadata_field(0x1900000300000003, &val)))
> +		sysinfo_td_conf->xfam_fixed1 = val;
> +	if (!ret && !(ret = read_sys_metadata_field(0x9900000100000004, &val)))
> +		sysinfo_td_conf->num_cpuid_config = val;
> +	if (!ret && !(ret = read_sys_metadata_field(0x9900000100000008, &val)))
> +		sysinfo_td_conf->max_vcpus_per_td = val;
> +	for (i = 0; i < sysinfo_td_conf->num_cpuid_config; i++)

It is not safe. We need to check

	sysinfo_td_conf->num_cpuid_config <= 32.

If the TDX module version is not matched with the json file that was 
used to generate the tdx_global_metadata.h, the num_cpuid_config 
reported by the actual TDX module might exceed 32 which causes 
out-of-bound array access.

> +		if (!ret && !(ret = read_sys_metadata_field(0x9900000300000400 + i, &val)))
> +			sysinfo_td_conf->cpuid_config_leaves[i] = val;
> +	for (i = 0; i < sysinfo_td_conf->num_cpuid_config; i++)
> +		for (j = 0; j < 2; j++)
> +			if (!ret && !(ret = read_sys_metadata_field(0x9900000300000500 + i * 2 + j, &val)))
> +				sysinfo_td_conf->cpuid_config_values[i][j] = val;
> +
> +	return ret;
> +}
> +
>   static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
>   {
>   	int ret = 0;
> @@ -84,6 +128,8 @@ static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
>   	ret = ret ?: get_tdx_sys_info_features(&sysinfo->features);
>   	ret = ret ?: get_tdx_sys_info_tdmr(&sysinfo->tdmr);
>   	ret = ret ?: get_tdx_sys_info_cmr(&sysinfo->cmr);
> +	ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
> +	ret = ret ?: get_tdx_sys_info_td_conf(&sysinfo->td_conf);
>   
>   	return ret;
>   }