Re: [PATCH v1] x86/tdx: Dump TDX version During the TD Bootup

[Kuppuswamy Sathyanarayanan]

Hi,

On 9/21/2023 8:06 PM, Yi Sun wrote:
> It is essential for TD users to be aware of the vendor and version of
> the current TDX. Additionally, they can reference the TDX version when
> reporting bugs or issues.
> 
> Furthermore, the applications or device drivers running in TD can achieve
> enhanced reliability and flexibility by adhering to the TDX Module ABI
> specification, as there are significant differences between various
> versions of TDX.

You can list some examples for where it can be used. For example,
verifyreport module call only exist in TDX 1.5 and it should not be
allowed for  version < 1.5. 
> 
> During TD initialization, the TDX version info can be obtained by calling
> TDG.SYS.RD. This will fetch the current version of TDX, including the major
> and minor version numbers and vendor ID.
> 
> The TDCALL TDG.SYS.RD originates from TDX version 1.5. If the error
> TDCALL_INVALID_OPERAND occurs, it should be treated as TDX version 1.0.
> 
> Co-developed-by: Dongcheng Yan <dongcheng.yan@xxxxxxxxx>
> Signed-off-by: Dongcheng Yan <dongcheng.yan@xxxxxxxxx>
> Signed-off-by: Yi Sun <yi.sun@xxxxxxxxx>
> 
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 1d6b863c42b0..acf298ebbfd8 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -37,6 +37,9 @@
>  
>  #define TDREPORT_SUBTYPE_0	0
>  
> +/* Caches TDX Module global-scope metadata field via TDG.SYS.RD TDCALL */
> +static struct tdg_sys_info td_sys_info;

I think it is __ro_after_init

> +
>  /* Called from __tdx_hypercall() for unrecoverable failure */
>  noinstr void __tdx_hypercall_failed(void)
>  {
> @@ -757,6 +760,46 @@ static bool tdx_enc_status_change_finish(unsigned long vaddr, int numpages,
>  	return true;
>  }
>  
> +/*
> + * Parse the tdx module version info from the global-scope metadata fields.
> + */
> +static int tdg_get_sysinfo(void)
> +{
> +	struct tdx_module_output out;
> +	u64 ret;
> +
> +	ret = __tdx_module_call(TDX_SYS_RD, 0, TDX_SYS_VENDOR_ID_FID, 0, 0,
> +				&out);
> +	if (TDCALL_RETURN_CODE(ret) == TDCALL_INVALID_OPERAND)
> +		goto version_1_0;
> +	else if (ret)
> +		return ret;
> +
> +	td_sys_info.vendor_id = (u32)(out).r8;
> +
> +	ret = __tdx_module_call(TDX_SYS_RD, 0, TDX_SYS_MAJOR_FID, 0, 0, &out);
> +	if (ret)
> +		return ret;
> +
> +	td_sys_info.major_version = (u16)(out).r8;
> +
> +	ret = __tdx_module_call(TDX_SYS_RD, 0, TDX_SYS_MINOR_FID, 0, 0, &out);
> +	if (ret)
> +		return ret;
> +
> +	td_sys_info.minor_version = (u16)(out).r8;
> +
> +	return 0;
> +
> +	/* TDX 1.0 does not have the TDCALL TDG.SYS.RD */
> +version_1_0:
> +	td_sys_info.vendor_id = 0x8086;

macro TDX_VENDOR_INTEL?

> +	td_sys_info.major_version = 1;
> +	td_sys_info.minor_version = 0;
> +
> +	return 0;
> +}
> +
>  void __init tdx_early_init(void)
>  {
>  	u64 cc_mask;
> @@ -820,5 +863,9 @@ void __init tdx_early_init(void)
>  	 */
>  	x86_cpuinit.parallel_bringup = false;
>  
> -	pr_info("Guest detected\n");
> +	tdg_get_sysinfo();
> +
> +	pr_info("Guest detected. TDX version:%u.%u VendorID: %x\n",
> +		td_sys_info.major_version, td_sys_info.minor_version,
> +		td_sys_info.vendor_id);
>  }
> diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> index 7513b3bb69b7..20ab96114970 100644
> --- a/arch/x86/include/asm/shared/tdx.h
> +++ b/arch/x86/include/asm/shared/tdx.h
> @@ -16,6 +16,7 @@
>  #define TDX_GET_REPORT			4
>  #define TDX_ACCEPT_PAGE			6
>  #define TDX_WR				8
> +#define TDX_SYS_RD			11
>  
>  /* TDCS fields. To be used by TDG.VM.WR and TDG.VM.RD module calls */
>  #define TDCS_NOTIFY_ENABLES		0x9100000000000010
> @@ -26,6 +27,11 @@
>  
>  #ifndef __ASSEMBLY__
>  
> +/* TDX metadata base field id, used by TDCALL TDG.SYS.RD */

IMO, you can add some spec section reference for it.

> +#define TDX_SYS_VENDOR_ID_FID		0x0800000200000000ULL
> +#define TDX_SYS_MINOR_FID		0x0800000100000003ULL
> +#define TDX_SYS_MAJOR_FID		0x0800000100000004ULL
> +

Since you have no usage outside tdx.c, you can move it there.

>  /*
>   * Used in __tdx_hypercall() to pass down and get back registers' values of
>   * the TDCALL instruction when requesting services from the VMM.
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 603e6d1e9d4a..108409f24d59 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -40,6 +40,12 @@ struct ve_info {
>  
>  #ifdef CONFIG_INTEL_TDX_GUEST
>  
> +struct tdg_sys_info {
> +	u32 vendor_id;
> +	u16 major_version;
> +	u16 minor_version;
> +};
> +

same as above. You can move it to tdx.c

>  void __init tdx_early_init(void);
>  
>  void tdx_get_ve_info(struct ve_info *ve);

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer