2023-10-12 13:46:29

by Yi Sun

[permalink] [raw]
Subject: [PATCH v5] 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.

Add function detect_tdx_version to fetch and dump the version of the
TDX, which is called during TD initialization. Obtain the info by calling
TDG.SYS.RD, 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.

Reviewed-by: Kirill A. Shutemov <[email protected]>
Co-developed-by: Dongcheng Yan <[email protected]>
Signed-off-by: Dongcheng Yan <[email protected]>
Signed-off-by: Yi Sun <[email protected]>
---
V1 -> V2:
- Move the defination of field IDs and the struct tdg_sys_info to tdx.c.
(Kuppuswamy Sathyanarayanan)

V2 -> V3:
- Move the allocation of struct tdg_sys_info on stack inside
tdx_early_init() and pass down to tdg_get_sysinfo() to fill.
(Kirill Shutemov)

V3 -> V4:
- Rebase the patch on top of the latest tip tree. (Huang, Kai)
- Change the return value of function tdg_get_sysinfo as void, and
zero out tdg_sys_info when error occurs. (Kuppuswamy Sathyanarayanan)

V4 -> V5:
- Print the version info inside the function detect_tdx_version, but
not tdx_early_init(). Remove the structure tdg_sys_info, but have 3
local variables instead. (Huang, Kai)
- Check for errors on each call to TDG.SYS.RD (Nikolay &
Sathyanarayanan)

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 3e6dbd2199cf..23b56952e132 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -37,6 +37,15 @@

#define TDREPORT_SUBTYPE_0 0

+/*
+ * TDX metadata base field id, used by TDCALL TDG.SYS.RD
+ * See TDX ABI Spec 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
+
/* Called from __tdx_hypercall() for unrecoverable failure */
noinstr void __noreturn __tdx_hypercall_failed(void)
{
@@ -800,6 +809,63 @@ static bool tdx_enc_status_change_finish(unsigned long vaddr, int numpages,
return true;
}

+/*
+ * Detect TDX Module version info from TDG.SYS.RD TDCALL
+ */
+static void detect_tdx_version(void)
+{
+ struct tdx_module_args args = {};
+ u32 vendor_id = TDX_VENDOR_INTEL;
+ u16 major_version = 0;
+ u16 minor_version = 0;
+ u64 ret;
+
+ /*
+ * TDCALL leaf TDG_SYS_RD
+ */
+ args.rdx = TDX_SYS_VENDOR_ID_FID;
+ ret = __tdcall_ret(TDG_SYS_RD, &args);
+ /*
+ * The TDCALL TDG.SYS.RD originates from TDX version 1.5.
+ * Treat TDCALL_INVALID_OPERAND error as TDX version 1.0.
+ */
+ if (TDCALL_RETURN_CODE(ret) == TDCALL_INVALID_OPERAND)
+ goto version_1_0;
+ if (ret) {
+ WARN(1, "TDX detection: TDG.SYS.RD(VENDOR_ID) error, return %llu\n",
+ ret);
+ return;
+ }
+ vendor_id = (u32)args.r8;
+
+ args.rdx = TDX_SYS_MAJOR_FID;
+ ret = __tdcall_ret(TDG_SYS_RD, &args);
+ if (ret) {
+ WARN(1, "TDX detection: TDG.SYS.RD(MAJOR) error, return %llu\n",
+ ret);
+ return;
+ }
+ major_version = (u16)args.r8;
+
+ args.rdx = TDX_SYS_MINOR_FID;
+ ret = __tdcall_ret(TDG_SYS_RD, &args);
+ if (ret) {
+ WARN(1, "TDX detection: TDG.SYS.RD(MINOR) error, return %llu\n",
+ ret);
+ return;
+ }
+ minor_version = (u16)args.r8;
+
+ pr_info("TDX detected. TDX version:%u.%u VendorID:%x\n",
+ major_version, minor_version, vendor_id);
+
+ return;
+
+ /* TDX 1.0 does not have the TDCALL TDG.SYS.RD */
+version_1_0:
+ pr_info("TDX detected. TDG.SYS.RD not available, assuming TDX version: 1.x (x<5)\n");
+}
+
void __init tdx_early_init(void)
{
struct tdx_module_args args = {
@@ -867,5 +933,5 @@ void __init tdx_early_init(void)
*/
x86_cpuinit.parallel_bringup = false;

- pr_info("Guest detected\n");
+ detect_tdx_version();
}
diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
index f74695dea217..1a0cacad5a0c 100644
--- a/arch/x86/include/asm/shared/tdx.h
+++ b/arch/x86/include/asm/shared/tdx.h
@@ -17,6 +17,7 @@
#define TDG_MR_REPORT 4
#define TDG_MEM_PAGE_ACCEPT 6
#define TDG_VM_WR 8
+#define TDG_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-12 15:44:41

by Dave Hansen

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

On 10/12/23 06:41, 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.

... and they will all have to pull this "essential" information out of
dmesg?

If this is essential, why stick it in dmesg where it can be overwritten?

> 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.

This is orthogonal to this patch. No applications or device drivers can
do anything with the result of these version queries.

> Add function detect_tdx_version to fetch and dump the version of the
> TDX, which is called during TD initialization. Obtain the info by calling
> TDG.SYS.RD, including the major and minor version numbers and vendor ID.

You don't need to rewrite the code in text form.

> 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.

I don't understand what this is trying to say.

> #define TDREPORT_SUBTYPE_0 0
>
> +/*
> + * TDX metadata base field id, used by TDCALL TDG.SYS.RD
> + * See TDX ABI Spec 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
> +
> /* Called from __tdx_hypercall() for unrecoverable failure */
> noinstr void __noreturn __tdx_hypercall_failed(void)
> {
> @@ -800,6 +809,63 @@ static bool tdx_enc_status_change_finish(unsigned long vaddr, int numpages,
> return true;
> }
>
> +/*
> + * Detect TDX Module version info from TDG.SYS.RD TDCALL
> + */
> +static void detect_tdx_version(void)
> +{
> + struct tdx_module_args args = {};
> + u32 vendor_id = TDX_VENDOR_INTEL;

What's the purpose of this assignment?

> + u16 major_version = 0;
> + u16 minor_version = 0;
> + u64 ret;
> +
> + /*
> + * TDCALL leaf TDG_SYS_RD
> + */
> + args.rdx = TDX_SYS_VENDOR_ID_FID;
> + ret = __tdcall_ret(TDG_SYS_RD, &args);
> + /*
> + * The TDCALL TDG.SYS.RD originates from TDX version 1.5.
> + * Treat TDCALL_INVALID_OPERAND error as TDX version 1.0.
> + */
> + if (TDCALL_RETURN_CODE(ret) == TDCALL_INVALID_OPERAND)
> + goto version_1_0;
> + if (ret) {
> + WARN(1, "TDX detection: TDG.SYS.RD(VENDOR_ID) error, return %llu\n",
> + ret);
> + return;
> + }

We do not need random warnings at every step of the way. Worst case,
make a version of tdcall that will spew an error in common code.

> + vendor_id = (u32)args.r8;

What's the purpose of the cast?

> + args.rdx = TDX_SYS_MAJOR_FID;
> + ret = __tdcall_ret(TDG_SYS_RD, &args);

Does args need to be re-zeroed between tdcalls?

> + if (ret) {
> + WARN(1, "TDX detection: TDG.SYS.RD(MAJOR) error, return %llu\n",
> + ret);
> + return;
> + }
> + major_version = (u16)args.r8;
> +
> + args.rdx = TDX_SYS_MINOR_FID;
> + ret = __tdcall_ret(TDG_SYS_RD, &args);
> + if (ret) {
> + WARN(1, "TDX detection: TDG.SYS.RD(MINOR) error, return %llu\n",
> + ret);
> + return;
> + }
> + minor_version = (u16)args.r8;
> +
> + pr_info("TDX detected. TDX version:%u.%u VendorID:%x\n",
> + major_version, minor_version, vendor_id);
> +
> + return;
> +
> + /* TDX 1.0 does not have the TDCALL TDG.SYS.RD */
> +version_1_0:
> + pr_info("TDX detected. TDG.SYS.RD not available, assuming TDX version: 1.x (x<5)\n");
> +}
> +
> void __init tdx_early_init(void)
> {
> struct tdx_module_args args = {
> @@ -867,5 +933,5 @@ void __init tdx_early_init(void)
> */
> x86_cpuinit.parallel_bringup = false;
>
> - pr_info("Guest detected\n");
> + detect_tdx_version();
> }
> diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> index f74695dea217..1a0cacad5a0c 100644
> --- a/arch/x86/include/asm/shared/tdx.h
> +++ b/arch/x86/include/asm/shared/tdx.h
> @@ -17,6 +17,7 @@
> #define TDG_MR_REPORT 4
> #define TDG_MEM_PAGE_ACCEPT 6
> #define TDG_VM_WR 8
> +#define TDG_SYS_RD 11

*This* is the place to document that TDG_SYS_RD is not available on old
TDX modules.

2023-10-13 11:18:31

by Nikolay Borisov

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



On 12.10.23 г. 16:41 ч., 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.
>
> Add function detect_tdx_version to fetch and dump the version of the
> TDX, which is called during TD initialization. Obtain the info by calling
> TDG.SYS.RD, 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.
>
> Reviewed-by: Kirill A. Shutemov <[email protected]>
> Co-developed-by: Dongcheng Yan <[email protected]>
> Signed-off-by: Dongcheng Yan <[email protected]>
> Signed-off-by: Yi Sun <[email protected]>

Reviewed-by: Nikolay Borisov <[email protected]>


<rant>

Intel really outdid themselves in creating the most horrendous document
to read/comprehend aka tdx's 1.5 ABI ...

</rant>