2023-09-30 20:28:31

by Yi Sun

[permalink] [raw]
Subject: [PATCH v3] x86/tdx: Dump TDX version During the TD Bootup

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 following the TDX Module ABI
specification, because there are significant differences between different
versions of TDX, as mentioned in the "Intel® TDX Module Incompatibilities
between v1.0 and v1.5" reference. Here are a few examples:

MSR Name Index Reason
----------------------------------------------
IA32_UARCH_MISC_CTL 0x1B01 From v1.5
IA32_ARCH_CAPABILITIES 0x010A Changed in v1.5
IA32_TSX_CTRL 0x0122 Changed in v1.5

CPUID Leaf Sub-leaf Reason
---------------------------------------
0x7 2 From v1.5
0x22 0 From v1.5
0x23 0~3 From v1.5
0x80000007 0 From v1.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 <[email protected]>
Signed-off-by: Dongcheng Yan <[email protected]>
Signed-off-by: Yi Sun <[email protected]>

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 1d6b863c42b0..052376d521d1 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -37,6 +37,24 @@

#define TDREPORT_SUBTYPE_0 0

+/*
+ * TDX metadata base field id, used by TDCALL TDG.SYS.RD
+ * See TDX ABI Spec section 3.3.2.3 Global Metadata Fields
+ */
+#define TDX_SYS_VENDOR_ID_FID 0x0800000200000000ULL
+#define TDX_SYS_MINOR_FID 0x0800000100000003ULL
+#define TDX_SYS_MAJOR_FID 0x0800000100000004ULL
+#define TDX_VENDOR_INTEL 0x8086
+
+/*
+ * The global-scope metadata field via TDG.SYS.RD TDCALL
+ */
+struct tdg_sys_info {
+ u32 vendor_id;
+ u16 major_version;
+ u16 minor_version;
+};
+
/* Called from __tdx_hypercall() for unrecoverable failure */
noinstr void __tdx_hypercall_failed(void)
{
@@ -757,10 +775,54 @@ 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(struct tdg_sys_info *td_sys)
+{
+ struct tdx_module_output out;
+ u64 ret;
+
+ if (!td_sys)
+ return -EINVAL;
+
+ 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->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->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->minor_version = (u16)out.r8;
+
+ return 0;
+
+ /* TDX 1.0 does not have the TDCALL TDG.SYS.RD */
+version_1_0:
+ td_sys->vendor_id = TDX_VENDOR_INTEL;
+ td_sys->major_version = 1;
+ td_sys->minor_version = 0;
+
+ return 0;
+}
+
void __init tdx_early_init(void)
{
u64 cc_mask;
u32 eax, sig[3];
+ struct tdg_sys_info td_sys_info;

cpuid_count(TDX_CPUID_LEAF_ID, 0, &eax, &sig[0], &sig[2], &sig[1]);

@@ -820,5 +882,9 @@ void __init tdx_early_init(void)
*/
x86_cpuinit.parallel_bringup = false;

- pr_info("Guest detected\n");
+ tdg_get_sysinfo(&td_sys_info);
+
+ 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..10ecb5dece84 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
--
2.34.1


2023-10-02 14:38:44

by Kirill A. Shutemov

[permalink] [raw]
Subject: Re: [PATCH v3] x86/tdx: Dump TDX version During the TD Bootup

On Sun, Oct 01, 2023 at 12:11:10AM +0800, 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 following the TDX Module ABI
> specification, because there are significant differences between different
> versions of TDX, as mentioned in the "Intel? TDX Module Incompatibilities
> between v1.0 and v1.5" reference. Here are a few examples:
>
> MSR Name Index Reason
> ----------------------------------------------
> IA32_UARCH_MISC_CTL 0x1B01 From v1.5
> IA32_ARCH_CAPABILITIES 0x010A Changed in v1.5
> IA32_TSX_CTRL 0x0122 Changed in v1.5
>
> CPUID Leaf Sub-leaf Reason
> ---------------------------------------
> 0x7 2 From v1.5
> 0x22 0 From v1.5
> 0x23 0~3 From v1.5
> 0x80000007 0 From v1.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 <[email protected]>
> Signed-off-by: Dongcheng Yan <[email protected]>
> Signed-off-by: Yi Sun <[email protected]>

Looks good to me.

Reviewed-by: Kirill A. Shutemov <[email protected]>

But you need to send it to x86 maintainers. I've CCed x86@, but you might
need to resend it properly to maintainers.

--
Kiryl Shutsemau / Kirill A. Shutemov

2023-10-02 19:57:36

by Kai Huang

[permalink] [raw]
Subject: Re: [PATCH v3] x86/tdx: Dump TDX version During the TD Bootup

On Mon, 2023-10-02 at 13:38 +0300, [email protected] wrote:
> On Sun, Oct 01, 2023 at 12:11:10AM +0800, 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 following the TDX Module ABI
> > specification, because there are significant differences between different
> > versions of TDX, as mentioned in the "Intel® TDX Module Incompatibilities
> > between v1.0 and v1.5" reference. Here are a few examples:
> >
> > MSR Name Index Reason
> > ----------------------------------------------
> > IA32_UARCH_MISC_CTL 0x1B01 From v1.5
> > IA32_ARCH_CAPABILITIES 0x010A Changed in v1.5
> > IA32_TSX_CTRL 0x0122 Changed in v1.5
> >
> > CPUID Leaf Sub-leaf Reason
> > ---------------------------------------
> > 0x7 2 From v1.5
> > 0x22 0 From v1.5
> > 0x23 0~3 From v1.5
> > 0x80000007 0 From v1.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 <[email protected]>
> > Signed-off-by: Dongcheng Yan <[email protected]>
> > Signed-off-by: Yi Sun <[email protected]>
>
> Looks good to me.
>
> Reviewed-by: Kirill A. Shutemov <[email protected]>
>
> But you need to send it to x86 maintainers. I've CCed x86@, but you might
> need to resend it properly to maintainers.
>

I looked at the patch on lore.kernel.org:

https://lore.kernel.org/all/[email protected]/T/

It seems it hasn't been rebased to the latest tip/x86/tdx yet, which has patches
to unify TDCALL/TDVMCALL assembly.

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



On 9/30/2023 9:11 AM, 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 following the TDX Module ABI
> specification, because there are significant differences between different
> versions of TDX, as mentioned in the "Intel® TDX Module Incompatibilities
> between v1.0 and v1.5" reference. Here are a few examples:
>
> MSR Name Index Reason
> ----------------------------------------------
> IA32_UARCH_MISC_CTL 0x1B01 From v1.5
> IA32_ARCH_CAPABILITIES 0x010A Changed in v1.5
> IA32_TSX_CTRL 0x0122 Changed in v1.5
>
> CPUID Leaf Sub-leaf Reason
> ---------------------------------------
> 0x7 2 From v1.5
> 0x22 0 From v1.5
> 0x23 0~3 From v1.5
> 0x80000007 0 From v1.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 <[email protected]>
> Signed-off-by: Dongcheng Yan <[email protected]>
> Signed-off-by: Yi Sun <[email protected]>
>
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 1d6b863c42b0..052376d521d1 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -37,6 +37,24 @@
>
> #define TDREPORT_SUBTYPE_0 0
>
> +/*
> + * TDX metadata base field id, used by TDCALL TDG.SYS.RD
> + * See TDX ABI Spec section 3.3.2.3 Global Metadata Fields
> + */
> +#define TDX_SYS_VENDOR_ID_FID 0x0800000200000000ULL
> +#define TDX_SYS_MINOR_FID 0x0800000100000003ULL
> +#define TDX_SYS_MAJOR_FID 0x0800000100000004ULL
> +#define TDX_VENDOR_INTEL 0x8086
> +
> +/*
> + * The global-scope metadata field via TDG.SYS.RD TDCALL
> + */
> +struct tdg_sys_info {
> + u32 vendor_id;
> + u16 major_version;
> + u16 minor_version;
> +};
> +
> /* Called from __tdx_hypercall() for unrecoverable failure */
> noinstr void __tdx_hypercall_failed(void)
> {
> @@ -757,10 +775,54 @@ 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(struct tdg_sys_info *td_sys)
> +{
> + struct tdx_module_output out;
> + u64 ret;
> +
> + if (!td_sys)
> + return -EINVAL;
> +
> + 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;

For this failure case, do you want to reset tdg_sys_info to some value like zero
or some constants to specify unknown?

> +
> + td_sys->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->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->minor_version = (u16)out.r8;
> +
> + return 0;
> +
> + /* TDX 1.0 does not have the TDCALL TDG.SYS.RD */
> +version_1_0:
> + td_sys->vendor_id = TDX_VENDOR_INTEL;
> + td_sys->major_version = 1;
> + td_sys->minor_version = 0;
> +
> + return 0;
> +}
> +
> void __init tdx_early_init(void)
> {
> u64 cc_mask;
> u32 eax, sig[3];
> + struct tdg_sys_info td_sys_info;
>
> cpuid_count(TDX_CPUID_LEAF_ID, 0, &eax, &sig[0], &sig[2], &sig[1]);
>
> @@ -820,5 +882,9 @@ void __init tdx_early_init(void)
> */
> x86_cpuinit.parallel_bringup = false;
>
> - pr_info("Guest detected\n");
> + tdg_get_sysinfo(&td_sys_info);

Why not check the return value before dumping the info?

> +
> + 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..10ecb5dece84 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

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

2023-10-03 13:57:09

by Yi Sun

[permalink] [raw]
Subject: Re: [PATCH v3] x86/tdx: Dump TDX version During the TD Bootup

On 03.10.2023 06:26, Kuppuswamy Sathyanarayanan wrote:
>
>
>On 9/30/2023 9:11 AM, Yi Sun wrote:
>> +static int tdg_get_sysinfo(struct tdg_sys_info *td_sys)
>> +{
>> + struct tdx_module_output out;
>> + u64 ret;
>> +
>> + if (!td_sys)
>> + return -EINVAL;
>> +
>> + 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;
>
>For this failure case, do you want to reset tdg_sys_info to some value like zero
>or some constants to specify unknown?
Yes, that would be better for this case.

>> +
>> + td_sys->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->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->minor_version = (u16)out.r8;
>> +
>> + return 0;
>> +
>> + /* TDX 1.0 does not have the TDCALL TDG.SYS.RD */
>> +version_1_0:
>> + td_sys->vendor_id = TDX_VENDOR_INTEL;
>> + td_sys->major_version = 1;
>> + td_sys->minor_version = 0;
>> +
>> + return 0;
>> +}
>> +
>> void __init tdx_early_init(void)
>> {
>> u64 cc_mask;
>> u32 eax, sig[3];
>> + struct tdg_sys_info td_sys_info;
>>
>> cpuid_count(TDX_CPUID_LEAF_ID, 0, &eax, &sig[0], &sig[2], &sig[1]);
>>
>> @@ -820,5 +882,9 @@ void __init tdx_early_init(void)
>> */
>> x86_cpuinit.parallel_bringup = false;
>>
>> - pr_info("Guest detected\n");
>> + tdg_get_sysinfo(&td_sys_info);
>
>Why not check the return value before dumping the info?
>
I overlooked that. I will add it in the next version.
Thanks Sathya for your comments.

Thanks
--Yi Sun

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



On 10/3/2023 6:56 AM, Yi Sun wrote:
> On 03.10.2023 06:26, Kuppuswamy Sathyanarayanan wrote:
>>
>>
>> On 9/30/2023 9:11 AM, Yi Sun wrote:
>>> +static int tdg_get_sysinfo(struct tdg_sys_info *td_sys)
>>> +{
>>> +    struct tdx_module_output out;
>>> +    u64 ret;
>>> +
>>> +    if (!td_sys)
>>> +        return -EINVAL;
>>> +
>>> +    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;
>>
>> For this failure case, do you want to reset tdg_sys_info to some value like zero
>> or some constants to specify unknown?
> Yes, that would be better for this case.
>
>>> +
>>> +    td_sys->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->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->minor_version = (u16)out.r8;
>>> +
>>> +    return 0;
>>> +
>>> +    /* TDX 1.0 does not have the TDCALL TDG.SYS.RD */
>>> +version_1_0:
>>> +    td_sys->vendor_id = TDX_VENDOR_INTEL;
>>> +    td_sys->major_version = 1;
>>> +    td_sys->minor_version = 0;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>  void __init tdx_early_init(void)
>>>  {
>>>      u64 cc_mask;
>>>      u32 eax, sig[3];
>>> +    struct tdg_sys_info td_sys_info;
>>>
>>>      cpuid_count(TDX_CPUID_LEAF_ID, 0, &eax, &sig[0], &sig[2],  &sig[1]);
>>>
>>> @@ -820,5 +882,9 @@ void __init tdx_early_init(void)
>>>       */
>>>      x86_cpuinit.parallel_bringup = false;
>>>
>>> -    pr_info("Guest detected\n");
>>> +    tdg_get_sysinfo(&td_sys_info);
>>
>> Why not check the return value before dumping the info?
>>
> I overlooked that. I will add it in the next version.
> Thanks Sathya for your comments.

if you plan to zero out the td_sys_info value in error case, I think it
is better to make the tdg_get_sysinfo as void

>
> Thanks
>      --Yi Sun

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer