TDX host kernel support

arch/x86/virt/vmx/tdx/tdx.c new file +44 -0

+/* BIOS must configure SEAMRR registers for all cores consistently */
+static u64 seamrr_base, seamrr_mask;
+
+static bool __seamrr_enabled(void)
+{
+	return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;
+}
+
+static void detect_seam_bsp(struct cpuinfo_x86 *c)
+{
+	u64 mtrrcap, base, mask;
+
+	/* SEAMRR is reported via MTRRcap */
+	if (!boot_cpu_has(X86_FEATURE_MTRR))
+		return;
+
+	rdmsrl(MSR_MTRRcap, mtrrcap);
+	if (!(mtrrcap & MTRR_CAP_SEAMRR))
+		return;
+
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_BASE, base);
+	if (!(base & SEAMRR_PHYS_BASE_CONFIGURED)) {
+		pr_info("SEAMRR base is not configured by BIOS\n");
+		return;
+	}
+
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_MASK, mask);
+	if ((mask & SEAMRR_ENABLED_BITS) != SEAMRR_ENABLED_BITS) {
+		pr_info("SEAMRR is not enabled by BIOS\n");
+		return;
+	}
+
+	seamrr_base = base;
+	seamrr_mask = mask;
+}
+
+static void detect_seam_ap(struct cpuinfo_x86 *c)
+{
+	u64 base, mask;
+
+	/*
+	 * Don't bother to detect this AP if SEAMRR is not
+	 * enabled after earlier detections.
+	 */

arch/x86/virt/vmx/tdx/tdx.c new file +10 -0

+	if (c == &boot_cpu_data)
+		detect_seam_bsp(c);
+	else
+		detect_seam_ap(c);
+}
+
+void tdx_detect_cpu(struct cpuinfo_x86 *c)
+{
+	detect_seam(c);
+}

Quoted 12 lines

arch/x86/virt/vmx/tdx/tdx.c new file +12 -0

+static void detect_seam_ap(struct cpuinfo_x86 *c)
+{
+	u64 base, mask;
+
+	/*
+	 * Don't bother to detect this AP if SEAMRR is not
+	 * enabled after earlier detections.
+	 */
+	if (!__seamrr_enabled())
+		return;
+
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_BASE, base);

arch/x86/virt/vmx/tdx/tdx.c line 85

Quoted 6 lines

arch/x86/virt/vmx/tdx/tdx.c new file +6 -0

+	rdmsrl(MSR_IA32_SEAMRR_PHYS_MASK, mask);
+
+	if (base == seamrr_base && mask == seamrr_mask)
+		return;
+
+	pr_err("Inconsistent SEAMRR configuration by BIOS\n");

Do we need to panic for SEAM config issue (for security)?

Quoted 4 lines

arch/x86/virt/vmx/tdx/tdx.c new file +4 -0

+	/* Mark SEAMRR as disabled. */
+	seamrr_base = 0;
+	seamrr_mask = 0
+}

arch/x86/virt/vmx/tdx/tdx.c line 92

Quoted 4 lines

arch/x86/virt/vmx/tdx/tdx.c new file +3 -0

+
+static void detect_seam(struct cpuinfo_x86 *c)
+{

why not do this check directly in tdx_detect_cpu()?

Quoted 11 lines

arch/x86/virt/vmx/tdx/tdx.c new file +10 -0

+	if (c == &boot_cpu_data)
+		detect_seam_bsp(c);
+	else
+		detect_seam_ap(c);
+}
+
+void tdx_detect_cpu(struct cpuinfo_x86 *c)
+{
+	detect_seam(c);
+}

arch/x86/virt/vmx/tdx/tdx.c new file +28 -0

new file mode 100644
index 000000000000..03f35c75f439
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright(c) 2022 Intel Corporation.
+ *
+ * Intel Trusted Domain Extensions (TDX) support
+ */
+
+#define pr_fmt(fmt)	"tdx: " fmt
+
+#include <linux/types.h>
+#include <linux/cpumask.h>
+#include <asm/msr-index.h>
+#include <asm/msr.h>
+#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
+#include <asm/tdx.h>
+
+/* Support Intel Secure Arbitration Mode Range Registers (SEAMRR) */
+#define MTRR_CAP_SEAMRR			BIT(15)
+
+/* Core-scope Intel SEAMRR base and mask registers. */
+#define MSR_IA32_SEAMRR_PHYS_BASE	0x00001400
+#define MSR_IA32_SEAMRR_PHYS_MASK	0x00001401
+
+#define SEAMRR_PHYS_BASE_CONFIGURED	BIT_ULL(3)
+#define SEAMRR_PHYS_MASK_ENABLED	BIT_ULL(11)
+#define SEAMRR_PHYS_MASK_LOCKED		BIT_ULL(10)
+

arch/x86/virt/vmx/tdx/tdx.c new file +17 -0

+static void detect_seam_bsp(struct cpuinfo_x86 *c)
+{
+	u64 mtrrcap, base, mask;
+
+	/* SEAMRR is reported via MTRRcap */
+	if (!boot_cpu_has(X86_FEATURE_MTRR))
+		return;
+
+	rdmsrl(MSR_MTRRcap, mtrrcap);
+	if (!(mtrrcap & MTRR_CAP_SEAMRR))
+		return;
+
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_BASE, base);
+	if (!(base & SEAMRR_PHYS_BASE_CONFIGURED)) {
+		pr_info("SEAMRR base is not configured by BIOS\n");
+		return;
+	}

arch/x86/virt/vmx/tdx/tdx.c new file +4 -0

+static void detect_seam_ap(struct cpuinfo_x86 *c)
+{
+	u64 base, mask;
+

arch/x86/virt/vmx/tdx/tdx.c new file +11 -0

+	if (base == seamrr_base && mask == seamrr_mask)
+		return;
+
+	pr_err("Inconsistent SEAMRR configuration by BIOS\n");
+	/* Mark SEAMRR as disabled. */
+	seamrr_base = 0;
+	seamrr_mask = 0;
+}
+
+static void detect_seam(struct cpuinfo_x86 *c)
+{

Quoted 7 lines

arch/x86/Kconfig +7 -0

+config INTEL_TDX_HOST
+	bool "Intel Trust Domain Extensions (TDX) host support"
+	default n
+	depends on CPU_SUP_INTEL
+	depends on X86_64
+	help
+	  Intel Trust Domain Extensions (TDX) protects guest VMs from

arch/x86/Kconfig config X86_SGX · line 1980

Quoted 6 lines

malicious

(quoted diff) +1 -0

+	  host and certain physical attacks.  This option enables necessary

TDX

arch/x86/Kconfig +3 -0

+	  support in host kernel to run protected VMs.
+
+	  If unsure, say N.

Nothing about KVM?

...

Quoted 39 lines

arch/x86/virt/vmx/tdx/tdx.c new file +33 -0

new file mode 100644
index 000000000000..03f35c75f439
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright(c) 2022 Intel Corporation.
+ *
+ * Intel Trusted Domain Extensions (TDX) support
+ */
+
+#define pr_fmt(fmt)	"tdx: " fmt
+
+#include <linux/types.h>
+#include <linux/cpumask.h>
+#include <asm/msr-index.h>
+#include <asm/msr.h>
+#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
+#include <asm/tdx.h>
+
+/* Support Intel Secure Arbitration Mode Range Registers (SEAMRR) */
+#define MTRR_CAP_SEAMRR			BIT(15)
+
+/* Core-scope Intel SEAMRR base and mask registers. */
+#define MSR_IA32_SEAMRR_PHYS_BASE	0x00001400
+#define MSR_IA32_SEAMRR_PHYS_MASK	0x00001401
+
+#define SEAMRR_PHYS_BASE_CONFIGURED	BIT_ULL(3)
+#define SEAMRR_PHYS_MASK_ENABLED	BIT_ULL(11)
+#define SEAMRR_PHYS_MASK_LOCKED		BIT_ULL(10)
+
+#define SEAMRR_ENABLED_BITS	\
+	(SEAMRR_PHYS_MASK_ENABLED | SEAMRR_PHYS_MASK_LOCKED)
+
+/* BIOS must configure SEAMRR registers for all cores consistently */
+static u64 seamrr_base, seamrr_mask;

arch/x86/virt/vmx/tdx/tdx.c line 38

Quoted 5 lines

arch/x86/virt/vmx/tdx/tdx.c new file +5 -0

+
+static bool __seamrr_enabled(void)
+{
+	return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;
+}

But there's no case where seamrr_mask is non-zero and where _seamrr_enabled(). Why bother checking the SEAMRR_ENABLED_BITS?

Quoted 18 lines

arch/x86/virt/vmx/tdx/tdx.c new file +17 -0

+static void detect_seam_bsp(struct cpuinfo_x86 *c)
+{
+	u64 mtrrcap, base, mask;
+
+	/* SEAMRR is reported via MTRRcap */
+	if (!boot_cpu_has(X86_FEATURE_MTRR))
+		return;
+
+	rdmsrl(MSR_MTRRcap, mtrrcap);
+	if (!(mtrrcap & MTRR_CAP_SEAMRR))
+		return;
+
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_BASE, base);
+	if (!(base & SEAMRR_PHYS_BASE_CONFIGURED)) {
+		pr_info("SEAMRR base is not configured by BIOS\n");
+		return;
+	}

arch/x86/virt/vmx/tdx/tdx.c line 66

Quoted 10 lines

arch/x86/virt/vmx/tdx/tdx.c new file +10 -0

+
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_MASK, mask);
+	if ((mask & SEAMRR_ENABLED_BITS) != SEAMRR_ENABLED_BITS) {
+		pr_info("SEAMRR is not enabled by BIOS\n");
+		return;
+	}
+
+	seamrr_base = base;
+	seamrr_mask = mask;
+}

Comment, please.

	/*
	 * Stash the boot CPU's MSR values so that AP values
	 * can can be checked for consistency.
	 */

Quoted 5 lines

arch/x86/virt/vmx/tdx/tdx.c new file +4 -0

+static void detect_seam_ap(struct cpuinfo_x86 *c)
+{
+	u64 base, mask;
+

arch/x86/virt/vmx/tdx/tdx.c line 81

Quoted 10 lines

arch/x86/virt/vmx/tdx/tdx.c new file +10 -0

+	/*
+	 * Don't bother to detect this AP if SEAMRR is not
+	 * enabled after earlier detections.
+	 */
+	if (!__seamrr_enabled())
+		return;
+
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_BASE, base);
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_MASK, mask);
+

This is the place for a comment about why the values have to be equal.

Quoted 17 lines

arch/x86/virt/vmx/tdx/tdx.c new file +16 -0

+	if (base == seamrr_base && mask == seamrr_mask)
+		return;
+
+	pr_err("Inconsistent SEAMRR configuration by BIOS\n");
+	/* Mark SEAMRR as disabled. */
+	seamrr_base = 0;
+	seamrr_mask = 0;
+}
+
+static void detect_seam(struct cpuinfo_x86 *c)
+{
+	if (c == &boot_cpu_data)
+		detect_seam_bsp(c);
+	else
+		detect_seam_ap(c);
+}

arch/x86/virt/vmx/tdx/tdx.c line 102

Quoted 5 lines

arch/x86/virt/vmx/tdx/tdx.c new file +5 -0

+
+void tdx_detect_cpu(struct cpuinfo_x86 *c)
+{
+	detect_seam(c);
+}

The extra function looks a bit silly here now. Maybe this gets filled out later, but it's goofy-looking here.

Hi Dave,

Thanks for review!

Quoted 7 lines · Dave Hansen

Quoted 7 lines

arch/x86/Kconfig +7 -0

+config INTEL_TDX_HOST
+	bool "Intel Trust Domain Extensions (TDX) host support"
+	default n
+	depends on CPU_SUP_INTEL
+	depends on X86_64
+	help
+	  Intel Trust Domain Extensions (TDX) protects guest VMs from

Quoted 8 lines

arch/x86/Kconfig config X86_SGX · line 1980

Quoted 6 lines

malicious

(quoted diff) +1 -0

+	  host and certain physical attacks.  This option enables necessary

TDX

arch/x86/Kconfig +3 -0

+	  support in host kernel to run protected VMs.
+
+	  If unsure, say N.

Nothing about KVM?

I'll add KVM into the context. How about below?

"Intel Trust Domain Extensions (TDX) protects guest VMs from malicious host and certain physical attacks. This option enables necessary TDX support in host kernel to allow KVM to run protected VMs called Trust Domains (TD)."

Quoted 39 lines

Quoted 39 lines

arch/x86/virt/vmx/tdx/tdx.c new file +33 -0

new file mode 100644
index 000000000000..03f35c75f439
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright(c) 2022 Intel Corporation.
+ *
+ * Intel Trusted Domain Extensions (TDX) support
+ */
+
+#define pr_fmt(fmt)	"tdx: " fmt
+
+#include <linux/types.h>
+#include <linux/cpumask.h>
+#include <asm/msr-index.h>
+#include <asm/msr.h>
+#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
+#include <asm/tdx.h>
+
+/* Support Intel Secure Arbitration Mode Range Registers (SEAMRR) */
+#define MTRR_CAP_SEAMRR			BIT(15)
+
+/* Core-scope Intel SEAMRR base and mask registers. */
+#define MSR_IA32_SEAMRR_PHYS_BASE	0x00001400
+#define MSR_IA32_SEAMRR_PHYS_MASK	0x00001401
+
+#define SEAMRR_PHYS_BASE_CONFIGURED	BIT_ULL(3)
+#define SEAMRR_PHYS_MASK_ENABLED	BIT_ULL(11)
+#define SEAMRR_PHYS_MASK_LOCKED		BIT_ULL(10)
+
+#define SEAMRR_ENABLED_BITS	\
+	(SEAMRR_PHYS_MASK_ENABLED | SEAMRR_PHYS_MASK_LOCKED)
+
+/* BIOS must configure SEAMRR registers for all cores consistently */
+static u64 seamrr_base, seamrr_mask;

Quoted 8 lines

arch/x86/virt/vmx/tdx/tdx.c line 38

Quoted 5 lines

arch/x86/virt/vmx/tdx/tdx.c new file +5 -0

+
+static bool __seamrr_enabled(void)
+{
+	return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;
+}

But there's no case where seamrr_mask is non-zero and where _seamrr_enabled(). Why bother checking the SEAMRR_ENABLED_BITS?

seamrr_mask will only be non-zero when SEAMRR is enabled by BIOS, otherwise it is 0. It will also be cleared when BIOS mis-configuration is detected on any AP. SEAMRR_ENABLED_BITS is used to check whether SEAMRR is enabled.

Quoted 8 lines

Quoted 8 lines

arch/x86/virt/vmx/tdx/tdx.c new file +8 -0

+static void detect_seam_ap(struct cpuinfo_x86 *c)
+{
+	u64 base, mask;
+
+	/*
+	 * Don't bother to detect this AP if SEAMRR is not
+	 * enabled after earlier detections.
+	 */

Quoted 8 lines

arch/x86/virt/vmx/tdx/tdx.c line 81

Quoted 6 lines

arch/x86/virt/vmx/tdx/tdx.c new file +6 -0

+	if (!__seamrr_enabled())
+		return;
+
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_BASE, base);
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_MASK, mask);
+

This is the place for a comment about why the values have to be equal.

I'll add below:

/* BIOS must configure SEAMRR consistently across all cores */

Quoted 16 lines

Quoted 16 lines

arch/x86/virt/vmx/tdx/tdx.c new file +16 -0

+	if (base == seamrr_base && mask == seamrr_mask)
+		return;
+
+	pr_err("Inconsistent SEAMRR configuration by BIOS\n");
+	/* Mark SEAMRR as disabled. */
+	seamrr_base = 0;
+	seamrr_mask = 0;
+}
+
+static void detect_seam(struct cpuinfo_x86 *c)
+{
+	if (c == &boot_cpu_data)
+		detect_seam_bsp(c);
+	else
+		detect_seam_ap(c);
+}

Quoted 8 lines

arch/x86/virt/vmx/tdx/tdx.c line 102

Quoted 5 lines

arch/x86/virt/vmx/tdx/tdx.c new file +5 -0

+
+void tdx_detect_cpu(struct cpuinfo_x86 *c)
+{
+	detect_seam(c);
+}

The extra function looks a bit silly here now. Maybe this gets filled out later, but it's goofy-looking here.

Thomas suggested to put all TDX detection related in one function call, so I added tdx_detect_cpu(). I'll move this to the next patch when detecting TDX KeyIDs.

Quoted 21 lines · Kai Huang

Hi Dave,

Thanks for review!

Quoted 15 lines · Dave Hansen

arch/x86/Kconfig config X86_SGX · line 1980

Quoted 13 lines

arch/x86/Kconfig +7 -0

+config INTEL_TDX_HOST
+	bool "Intel Trust Domain Extensions (TDX) host support"
+	default n
+	depends on CPU_SUP_INTEL
+	depends on X86_64
+	help
+	  Intel Trust Domain Extensions (TDX) protects guest VMs from

malicious

(quoted diff) +1 -0

+	  host and certain physical attacks.  This option enables necessary

TDX

arch/x86/Kconfig +3 -0

+	  support in host kernel to run protected VMs.
+
+	  If unsure, say N.

Nothing about KVM?

Quoted 6 lines

I'll add KVM into the context. How about below?

"Intel Trust Domain Extensions (TDX) protects guest VMs from malicious host and certain physical attacks. This option enables necessary TDX support in host kernel to allow KVM to run protected VMs called Trust Domains (TD)."

What about a dependency? Isn't this dead code without CONFIG_KVM=y/m?

Quoted 49 lines

Quoted 48 lines

Quoted 35 lines

arch/x86/virt/vmx/tdx/tdx.c new file +28 -0

new file mode 100644
index 000000000000..03f35c75f439
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright(c) 2022 Intel Corporation.
+ *
+ * Intel Trusted Domain Extensions (TDX) support
+ */
+
+#define pr_fmt(fmt)	"tdx: " fmt
+
+#include <linux/types.h>
+#include <linux/cpumask.h>
+#include <asm/msr-index.h>
+#include <asm/msr.h>
+#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
+#include <asm/tdx.h>
+
+/* Support Intel Secure Arbitration Mode Range Registers (SEAMRR) */
+#define MTRR_CAP_SEAMRR			BIT(15)
+
+/* Core-scope Intel SEAMRR base and mask registers. */
+#define MSR_IA32_SEAMRR_PHYS_BASE	0x00001400
+#define MSR_IA32_SEAMRR_PHYS_MASK	0x00001401
+
+#define SEAMRR_PHYS_BASE_CONFIGURED	BIT_ULL(3)
+#define SEAMRR_PHYS_MASK_ENABLED	BIT_ULL(11)
+#define SEAMRR_PHYS_MASK_LOCKED		BIT_ULL(10)
+

arch/x86/virt/vmx/tdx/tdx.c line 38

Quoted 10 lines

arch/x86/virt/vmx/tdx/tdx.c new file +10 -0

+#define SEAMRR_ENABLED_BITS	\
+	(SEAMRR_PHYS_MASK_ENABLED | SEAMRR_PHYS_MASK_LOCKED)
+
+/* BIOS must configure SEAMRR registers for all cores consistently */
+static u64 seamrr_base, seamrr_mask;
+
+static bool __seamrr_enabled(void)
+{
+	return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;
+}

But there's no case where seamrr_mask is non-zero and where _seamrr_enabled(). Why bother checking the SEAMRR_ENABLED_BITS?

Quoted 3 lines

seamrr_mask will only be non-zero when SEAMRR is enabled by BIOS, otherwise it is 0. It will also be cleared when BIOS mis-configuration is detected on any AP. SEAMRR_ENABLED_BITS is used to check whether SEAMRR is enabled.

The point is that this could be:

	return !!seamrr_mask;

Quoted 19 lines

Quoted 18 lines

Quoted 6 lines

arch/x86/virt/vmx/tdx/tdx.c new file +4 -0

+static void detect_seam_ap(struct cpuinfo_x86 *c)
+{
+	u64 base, mask;
+

arch/x86/virt/vmx/tdx/tdx.c line 81

Quoted 10 lines

arch/x86/virt/vmx/tdx/tdx.c new file +10 -0

+	/*
+	 * Don't bother to detect this AP if SEAMRR is not
+	 * enabled after earlier detections.
+	 */
+	if (!__seamrr_enabled())
+		return;
+
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_BASE, base);
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_MASK, mask);
+

This is the place for a comment about why the values have to be equal.

Quoted 3 lines

I'll add below:

/* BIOS must configure SEAMRR consistently across all cores */

What happens if the BIOS doesn't do this? What actually breaks? In other words, do we *NEED* error checking here?

Quoted 26 lines

Quoted 25 lines

Quoted 12 lines

arch/x86/virt/vmx/tdx/tdx.c new file +11 -0

+	if (base == seamrr_base && mask == seamrr_mask)
+		return;
+
+	pr_err("Inconsistent SEAMRR configuration by BIOS\n");
+	/* Mark SEAMRR as disabled. */
+	seamrr_base = 0;
+	seamrr_mask = 0;
+}
+
+static void detect_seam(struct cpuinfo_x86 *c)
+{

arch/x86/virt/vmx/tdx/tdx.c line 102

Quoted 10 lines

arch/x86/virt/vmx/tdx/tdx.c new file +10 -0

+	if (c == &boot_cpu_data)
+		detect_seam_bsp(c);
+	else
+		detect_seam_ap(c);
+}
+
+void tdx_detect_cpu(struct cpuinfo_x86 *c)
+{
+	detect_seam(c);
+}

The extra function looks a bit silly here now. Maybe this gets filled out later, but it's goofy-looking here.

Quoted 3 lines

Thomas suggested to put all TDX detection related in one function call, so I added tdx_detect_cpu(). I'll move this to the next patch when detecting TDX KeyIDs.

That's fine, or just add a comment or a changelog sentence about this being filled out later.

Quoted 29 lines · Dave Hansen

Quoted 27 lines · Kai Huang

Hi Dave,

Thanks for review!

Quoted 7 lines · Dave Hansen

Quoted 7 lines

arch/x86/Kconfig +7 -0

+config INTEL_TDX_HOST
+	bool "Intel Trust Domain Extensions (TDX) host support"
+	default n
+	depends on CPU_SUP_INTEL
+	depends on X86_64
+	help
+	  Intel Trust Domain Extensions (TDX) protects guest VMs from

Quoted 8 lines

arch/x86/Kconfig config X86_SGX · line 1980

Quoted 6 lines

malicious

(quoted diff) +1 -0

+	  host and certain physical attacks.  This option enables necessary

TDX

arch/x86/Kconfig +3 -0

+	  support in host kernel to run protected VMs.
+
+	  If unsure, say N.

Nothing about KVM?

I'll add KVM into the context. How about below?

"Intel Trust Domain Extensions (TDX) protects guest VMs from malicious host and certain physical attacks. This option enables necessary TDX support in host kernel to allow KVM to run protected VMs called Trust Domains (TD)."

Quoted 1 line

What about a dependency? Isn't this dead code without CONFIG_KVM=y/m?

Conceptually, KVM is one user of the TDX module, so it doesn't seem correct to make CONFIG_INTEL_TDX_HOST depend on CONFIG_KVM. But so far KVM is the only user of TDX, so in practice the code is dead w/o KVM.

What's your opinion?

Quoted 36 lines · Kai Huang

Quoted 22 lines · Dave Hansen

Quoted 21 lines · Kai Huang

Hi Dave,

Thanks for review!

Quoted 15 lines · Dave Hansen

arch/x86/Kconfig config X86_SGX · line 1980

Quoted 13 lines

arch/x86/Kconfig +7 -0

+config INTEL_TDX_HOST
+	bool "Intel Trust Domain Extensions (TDX) host support"
+	default n
+	depends on CPU_SUP_INTEL
+	depends on X86_64
+	help
+	  Intel Trust Domain Extensions (TDX) protects guest VMs from

malicious

(quoted diff) +1 -0

+	  host and certain physical attacks.  This option enables necessary

TDX

arch/x86/Kconfig +3 -0

+	  support in host kernel to run protected VMs.
+
+	  If unsure, say N.

Nothing about KVM?

Quoted 8 lines

Quoted 6 lines

I'll add KVM into the context. How about below?

"Intel Trust Domain Extensions (TDX) protects guest VMs from malicious host and certain physical attacks. This option enables necessary TDX support in host kernel to allow KVM to run protected VMs called Trust Domains (TD)."

What about a dependency? Isn't this dead code without CONFIG_KVM=y/m?

Conceptually, KVM is one user of the TDX module, so it doesn't seem correct to make CONFIG_INTEL_TDX_HOST depend on CONFIG_KVM. But so far KVM is the only user of TDX, so in practice the code is dead w/o KVM.

Quoted 1 line

What's your opinion?

Take a dependency on CONFIG_KVM_INTEL, there's already precedence for this specific case of a feature that can't possibly have an in-kernel user. See arch/x86/kernel/cpu/feat_ctl.c, which in the (very) unlikely event IA32_FEATURE_CONTROL is left unlocked by BIOS, will deliberately disable VMX if CONFIG_KVM_INTEL=n.

Quoted 1 line · Dave Hansen

What about a dependency? Isn't this dead code without CONFIG_KVM=y/m?

Conceptually, KVM is one user of the TDX module, so it doesn't seem correct to make CONFIG_INTEL_TDX_HOST depend on CONFIG_KVM. But so far KVM is the only user of TDX, so in practice the code is dead w/o KVM.

What's your opinion?

Quoted 12 lines

Quoted 11 lines

Quoted 7 lines

arch/x86/virt/vmx/tdx/tdx.c line 38

Quoted 4 lines

arch/x86/virt/vmx/tdx/tdx.c new file +4 -0

+static bool __seamrr_enabled(void)
+{
+	return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;
+}

But there's no case where seamrr_mask is non-zero and where _seamrr_enabled(). Why bother checking the SEAMRR_ENABLED_BITS?

seamrr_mask will only be non-zero when SEAMRR is enabled by BIOS, otherwise it is 0. It will also be cleared when BIOS mis-configuration is detected on any AP. SEAMRR_ENABLED_BITS is used to check whether SEAMRR is enabled.

Quoted 3 lines

The point is that this could be:

	return !!seamrr_mask;

The definition of this SEAMRR_MASK MSR defines "ENABLED" and "LOCKED" bits. Explicitly checking the two bits, instead of !!seamrr_mask roles out other incorrect configurations. For instance, we should not treat SEAMRR being enabled if we only have "ENABLED" bit set or "LOCKED" bit set.

Quoted 19 lines

Quoted 19 lines

Quoted 8 lines

Quoted 8 lines

arch/x86/virt/vmx/tdx/tdx.c new file +8 -0

+static void detect_seam_ap(struct cpuinfo_x86 *c)
+{
+	u64 base, mask;
+
+	/*
+	 * Don't bother to detect this AP if SEAMRR is not
+	 * enabled after earlier detections.
+	 */

Quoted 8 lines

arch/x86/virt/vmx/tdx/tdx.c line 81

Quoted 6 lines

arch/x86/virt/vmx/tdx/tdx.c new file +6 -0

+	if (!__seamrr_enabled())
+		return;
+
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_BASE, base);
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_MASK, mask);
+

This is the place for a comment about why the values have to be equal.

I'll add below:

Quoted 4 lines

Quoted 1 line

/* BIOS must configure SEAMRR consistently across all cores */

What happens if the BIOS doesn't do this? What actually breaks? In other words, do we *NEED* error checking here?

AFAICT the spec doesn't explicitly mention what will happen if BIOS doesn't configure them consistently among cores. But for safety I think it's better to detect.

Quoted 8 lines

Quoted 8 lines

Quoted 7 lines

arch/x86/virt/vmx/tdx/tdx.c line 102

Quoted 4 lines

arch/x86/virt/vmx/tdx/tdx.c new file +4 -0

+void tdx_detect_cpu(struct cpuinfo_x86 *c)
+{
+	detect_seam(c);
+}

The extra function looks a bit silly here now. Maybe this gets filled out later, but it's goofy-looking here.

Quoted 6 lines

Quoted 3 lines

Thomas suggested to put all TDX detection related in one function call, so I added tdx_detect_cpu(). I'll move this to the next patch when detecting TDX KeyIDs.

That's fine, or just add a comment or a changelog sentence about this being filled out later.

There's already one sentence in the changelog:

"......Add a function to detect all TDX preliminaries (SEAMRR, TDX private KeyIDs) for a given cpu when it is brought up. As the first step, detect the validity of SEAMRR."

Does this look good to you?

Quoted 8 lines · Kai Huang

Quoted 1 line · Dave Hansen

What about a dependency? Isn't this dead code without CONFIG_KVM=y/m?

Conceptually, KVM is one user of the TDX module, so it doesn't seem correct to make CONFIG_INTEL_TDX_HOST depend on CONFIG_KVM. But so far KVM is the only user of TDX, so in practice the code is dead w/o KVM.

What's your opinion?

You're stuck in some really weird fantasy world. Sure, we can dream up more than one user of the TDX module. But, in the real world, there's only one. Plus, code can have multiple dependencies!

	depends on FOO || BAR

This TDX cruft is dead code in today's real-world kernel without KVM. You should add a dependency.

Quoted 13 lines

Quoted 13 lines

Quoted 12 lines

Quoted 8 lines

arch/x86/virt/vmx/tdx/tdx.c line 38

Quoted 5 lines

arch/x86/virt/vmx/tdx/tdx.c new file +4 -0

+static bool __seamrr_enabled(void)
+{
+	return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;
+}

But there's no case where seamrr_mask is non-zero and where _seamrr_enabled(). Why bother checking the SEAMRR_ENABLED_BITS?

seamrr_mask will only be non-zero when SEAMRR is enabled by BIOS, otherwise it is 0. It will also be cleared when BIOS mis-configuration is detected on any AP. SEAMRR_ENABLED_BITS is used to check whether SEAMRR is enabled.

Quoted 8 lines

Quoted 3 lines

The point is that this could be:

	return !!seamrr_mask;

The definition of this SEAMRR_MASK MSR defines "ENABLED" and "LOCKED" bits. Explicitly checking the two bits, instead of !!seamrr_mask roles out other incorrect configurations. For instance, we should not treat SEAMRR being enabled if we only have "ENABLED" bit set or "LOCKED" bit set.

You're confusing two different things:
* The state of the variable
* The actual correct hardware state

The *VARIABLE* can't be non-zero and also denote that SEAMRR is enabled.
Does this *CODE* ever set ENABLED or LOCKED without each other?

Quoted 22 lines

Quoted 22 lines

Quoted 21 lines

Quoted 17 lines

Quoted 5 lines

arch/x86/virt/vmx/tdx/tdx.c new file +4 -0

+static void detect_seam_ap(struct cpuinfo_x86 *c)
+{
+	u64 base, mask;
+

arch/x86/virt/vmx/tdx/tdx.c line 81

Quoted 10 lines

arch/x86/virt/vmx/tdx/tdx.c new file +10 -0

+	/*
+	 * Don't bother to detect this AP if SEAMRR is not
+	 * enabled after earlier detections.
+	 */
+	if (!__seamrr_enabled())
+		return;
+
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_BASE, base);
+	rdmsrl(MSR_IA32_SEAMRR_PHYS_MASK, mask);
+

This is the place for a comment about why the values have to be equal.

I'll add below:

/* BIOS must configure SEAMRR consistently across all cores */

Quoted 6 lines

Quoted 2 lines

What happens if the BIOS doesn't do this? What actually breaks? In other words, do we *NEED* error checking here?

AFAICT the spec doesn't explicitly mention what will happen if BIOS doesn't configure them consistently among cores. But for safety I think it's better to detect.

Safety? Safety of what?

+ * Intel Trusted Domain CPU Architecture Extension spec:

Quoted 1 line · Kai Huang

+ * Intel Trusted Domain CPU Architecture Extension spec:

In TDX guest code, we have been using TDX as "Intel Trust Domain Extensions". It also aligns with spec. Maybe you should change your patch set to use the same.

arch/x86/virt/vmx/tdx/tdx.c static void detect_seam(struct cpuinfo_x86 *c) · line 173

Quoted 2 lines · Kai Huang

  	detect_seam(c);

arch/x86/virt/vmx/tdx/tdx.c +1 -0

+	detect_tdx_keyids(c);

Do you want to add some return value to detect_seam() and not proceed if it fails?

In case if this function is going to be extended by future patch set, maybe do the same for detect_tdx_keyids()?

SEAMCALL leaf functions use an ABI different from the x86-64 system-v ABI. Instead, they share the same ABI with the TDCALL leaf functions.

%rax is used to carry both the SEAMCALL leaf function number (input) and the completion status code (output). Additional GPRs (%rcx, %rdx, %r8->%r11) may be further used as both input and output operands in individual leaf functions.

Quoted 2 lines · Kai Huang

SEAMCALL leaf functions use an ABI different from the x86-64 system-v ABI. Instead, they share the same ABI with the TDCALL leaf functions.

TDCALL is a new term for this patch set. Maybe add some detail about it in ()?.

Quoted 1 line

Quoted 5 lines · Sathyanarayanan Kuppuswamy

Quoted 2 lines · Kai Huang

SEAMCALL leaf functions use an ABI different from the x86-64 system-v ABI. Instead, they share the same ABI with the TDCALL leaf functions.

Quoted 4 lines

TDCALL is a new term for this patch set. Maybe add some detail about it in ()?.

Quoted 1 line

TDCALL implementation is already in tip/tdx. This series will be rebased to it. I don't think we need to explain more about something that is already in the tip tree?

Quoted 11 lines · Kai Huang

Quoted 9 lines · Sathyanarayanan Kuppuswamy

Quoted 2 lines · Kai Huang

SEAMCALL leaf functions use an ABI different from the x86-64 system-v ABI. Instead, they share the same ABI with the TDCALL leaf functions.

TDCALL is a new term for this patch set. Maybe add some detail about it in ()?.

Quoted 1 line

Quoted 3 lines

TDCALL implementation is already in tip/tdx. This series will be rebased to it. I don't think we need to explain more about something that is already in the tip tree?

Since you have already expanded terms like TD,TDX and SEAM in this patch set, I thought you wanted to explain TDX terms to make it easy for new readers. So to keep it uniform, I have suggested adding some brief details about the TDCALL.

Secure Arbitration Mode (SEAM) is an extension of VMX architecture. It defines a new VMX root operation (SEAM VMX root) and a new VMX non-root operation (SEAM VMX non-root) which are isolated from legacy VMX root and VMX non-root mode.

A CPU-attested software module (called the 'TDX module') runs in SEAM VMX root to manage the crypto-protected VMs running in SEAM VMX non-root. SEAM VMX root is also used to host another CPU-attested software module (called the 'P-SEAMLDR') to load and update the TDX module.

Quoted 1 line

Host kernel transits to either the P-SEAMLDR or the TDX module via the

new SEAMCALL instruction. SEAMCALL leaf functions are host-side interface functions defined by the P-SEAMLDR and the TDX module around the new SEAMCALL instruction. They are similar to a hypercall, except they are made by host kernel to the SEAM software.

SEAMCALL leaf functions use an ABI different from the x86-64 system-v ABI. Instead, they share the same ABI with the TDCALL leaf functions. %rax is used to carry both the SEAMCALL leaf function number (input) and the completion status code (output). Additional GPRs (%rcx, %rdx, %r8->%r11) may be further used as both input and output operands in individual leaf functions.

Implement a C function __seamcall()

to do SEAMCALL leaf functions using
the assembly macro used by __tdx_module_call() (the implementation of TDCALL leaf functions). The only exception not covered here is TDENTER leaf function which takes all GPRs and XMM0-XMM15 as both input and output. The caller of TDENTER should implement its own logic to call TDENTER directly instead of using this function.

SEAMCALL instruction is essentially a VMExit from VMX root to SEAM VMX root, and it can fail with VMfailInvalid, for instance, when the SEAM software module is not loaded. The C function __seamcall() returns TDX_SEAMCALL_VMFAILINVALID, which doesn't conflict with any actual error code of SEAMCALLs, to uniquely represent this case.

arch/x86/virt/vmx/tdx/Makefile +1 -1

index 1bd688684716..fd577619620e 100644
@@ -1,2 +1,2 @@
 # SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_INTEL_TDX_HOST)	+= tdx.o
+obj-$(CONFIG_INTEL_TDX_HOST)	+= tdx.o seamcall.o

arch/x86/virt/vmx/tdx/seamcall.S new file +4 -0

new file mode 100644
index 000000000000..327961b2dd5a
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <linux/linkage.h>
+#include <asm/frame.h>
+

arch/x86/virt/vmx/tdx/seamcall.S new file +37 -0

+ *-------------------------------------------------------------------------
+ * SEAMCALL ABI:
+ *-------------------------------------------------------------------------
+ * Input Registers:
+ *
+ * RAX                 - SEAMCALL Leaf number.
+ * RCX,RDX,R8-R9       - SEAMCALL Leaf specific input registers.
+ *
+ * Output Registers:
+ *
+ * RAX                 - SEAMCALL completion status code.
+ * RCX,RDX,R8-R11      - SEAMCALL Leaf specific output registers.
+ *
+ *-------------------------------------------------------------------------
+ *
+ * __seamcall() function ABI:
+ *
+ * @fn  (RDI)          - SEAMCALL Leaf number, moved to RAX
+ * @rcx (RSI)          - Input parameter 1, moved to RCX
+ * @rdx (RDX)          - Input parameter 2, moved to RDX
+ * @r8  (RCX)          - Input parameter 3, moved to R8
+ * @r9  (R8)           - Input parameter 4, moved to R9
+ *
+ * @out (R9)           - struct tdx_module_output pointer
+ *			 stored temporarily in R12 (not
+ *			 used by the P-SEAMLDR or the TDX
+ *			 module). It can be NULL.
+ *
+ * Return (via RAX) the completion status of the SEAMCALL, or
+ * TDX_SEAMCALL_VMFAILINVALID.
+ */
+SYM_FUNC_START(__seamcall)
+	FRAME_BEGIN
+	TDX_MODULE_CALL host=1
+	FRAME_END
+	ret
+SYM_FUNC_END(__seamcall)

arch/x86/virt/vmx/tdx/tdx.h new file +11 -0

new file mode 100644
index 000000000000..9d5b6f554c20
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _X86_VIRT_TDX_H
+#define _X86_VIRT_TDX_H
+
+#include <linux/types.h>
+
+struct tdx_module_output;
+u64 __seamcall(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
+	       struct tdx_module_output *out);
+
+#endif

Quoted 4 lines · Kai Huang

Secure Arbitration Mode (SEAM) is an extension of VMX architecture. It defines a new VMX root operation (SEAM VMX root) and a new VMX non-root operation (SEAM VMX non-root) which are isolated from legacy VMX root and VMX non-root mode.

I feel like this is too much detail for an opening paragraph.

Quoted 9 lines

A CPU-attested software module (called the 'TDX module') runs in SEAM VMX root to manage the crypto-protected VMs running in SEAM VMX non-root. SEAM VMX root is also used to host another CPU-attested software module (called the 'P-SEAMLDR') to load and update the TDX module.

Quoted 1 line

Host kernel transits to either the P-SEAMLDR or the TDX module via the

new SEAMCALL instruction. SEAMCALL leaf functions are host-side interface functions defined by the P-SEAMLDR and the TDX module around the new SEAMCALL instruction. They are similar to a hypercall, except they are made by host kernel to the SEAM software.

I think you can get rid of about half of this changelog so farand make it more clear in the process with this:

TDX introduces a new CPU mode: Secure Arbitration Mode (SEAM). This mode runs only the TDX module itself or other code needed to load the TDX module.

The host kernel communicates with SEAM software via a new SEAMCALL instruction. This is conceptually similar to a guest->host hypercall, except it is made from the host to SEAM software instead.

This is a technical document, but you're writing too technically for my taste and focusing on the low-level details rather than the high-level concepts. What do I care that SEAM is two modes and what their names are at this juncture? Are those details necesarry to get me to understand what a SEAMCALL is or what this patch implements?

Quoted 9 lines

SEAMCALL leaf functions use an ABI different from the x86-64 system-v ABI. Instead, they share the same ABI with the TDCALL leaf functions. %rax is used to carry both the SEAMCALL leaf function number (input) and the completion status code (output). Additional GPRs (%rcx, %rdx, %r8->%r11) may be further used as both input and output operands in individual leaf functions.

Implement a C function __seamcall()

Your "C function" looks a bit like assembly to me.

Quoted 7 lines

to do SEAMCALL leaf functions using
the assembly macro used by __tdx_module_call() (the implementation of TDCALL leaf functions). The only exception not covered here is TDENTER leaf function which takes all GPRs and XMM0-XMM15 as both input and output. The caller of TDENTER should implement its own logic to call TDENTER directly instead of using this function.

I have no idea why this paragraph is here or what it is trying to tell me.

Quoted 6 lines

SEAMCALL instruction is essentially a VMExit from VMX root to SEAM VMX root, and it can fail with VMfailInvalid, for instance, when the SEAM software module is not loaded. The C function __seamcall() returns TDX_SEAMCALL_VMFAILINVALID, which doesn't conflict with any actual error code of SEAMCALLs, to uniquely represent this case.

Again, I'm lost. Why is this detail here? I don't even see TDX_SEAMCALL_VMFAILINVALID in the patch.

Quoted 19 lines

arch/x86/virt/vmx/tdx/Makefile +1 -1

index 1bd688684716..fd577619620e 100644
@@ -1,2 +1,2 @@
 # SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_INTEL_TDX_HOST)	+= tdx.o
+obj-$(CONFIG_INTEL_TDX_HOST)	+= tdx.o seamcall.o

arch/x86/virt/vmx/tdx/seamcall.S new file +4 -0

new file mode 100644
index 000000000000..327961b2dd5a
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <linux/linkage.h>
+#include <asm/frame.h>
+

arch/x86/virt/vmx/tdx/seamcall.S line 14

Quoted 10 lines

arch/x86/virt/vmx/tdx/seamcall.S new file +10 -0

+#include "tdxcall.S"
+
+/*
+ * __seamcall()  - Host-side interface functions to SEAM software module
+ *		   (the P-SEAMLDR or the TDX module)
+ *
+ * Transform function call register arguments into the SEAMCALL register
+ * ABI.  Return TDX_SEAMCALL_VMFAILINVALID, or the completion status of
+ * the SEAMCALL.  Additional output operands are saved in @out (if it is
+ * provided by caller).

This needs to say:

Returns TDX_SEAMCALL_VMFAILINVALID if the SEAMCALL itself fails.

The TDX module is essentially a CPU-attested software module running in the new Secure Arbitration Mode (SEAM) to protect VMs from malicious host and certain physical attacks. The TDX module implements the

functions to build, tear down and start execution of the protected VMs called Trusted Domains (TD). Before the TDX module can be used to

create and run TD guests, it must be loaded into the SEAM Range Register (SEAMRR) and properly initialized. The TDX module is expected to be loaded by BIOS before booting to the kernel, and the kernel is expected to detect and initialize it, using the SEAMCALLs defined by TDX architecture.

The TDX module can be initialized only once in its lifetime. Instead of always initializing it at boot time, this implementation chooses an on-demand approach to initialize TDX until there is a real need (e.g when requested by KVM). This avoids consuming the memory that must be allocated by kernel and given to the TDX module as metadata (~1/256th of

the TDX-usable memory), and also saves the time of initializing the TDX module (and the metadata) when TDX is not used at all. Initializing the TDX module at runtime on-demand also is more flexible to support TDX module runtime updating in the future (after updating the TDX module, it needs to be initialized again).

Introduce two placeholders tdx_detect() and tdx_init() to detect and initialize the TDX module on demand, with a state machine introduced to orchestrate the entire process (in case of multiple callers).

To start with, tdx_detect() checks SEAMRR and TDX private KeyIDs. The TDX module is reported as not loaded if either SEAMRR is not enabled, or there are no enough TDX private KeyIDs to create any TD guest. The TDX module itself requires one global TDX private KeyID to crypto protect its metadata.

And tdx_init() is currently empty. The TDX module will be initialized in multi-steps defined by the TDX architecture:

1) Global initialization;
2) Logical-CPU scope initialization;
3) Enumerate the TDX module capabilities and platform configuration;
4) Configure the TDX module about usable memory ranges and global

      KeyID information;

5) Package-scope configuration for the global KeyID;
6) Initialize usable memory ranges based on 4).

The TDX module can also be shut down at any time during its lifetime. In case of any error during the initialization process, shut down the module. It's pointless to leave the module in any intermediate state during the initialization.

SEAMCALL requires SEAMRR being enabled and CPU being already in VMX operation (VMXON has been done), otherwise it generates #UD. So far only KVM handles VMXON/VMXOFF. Choose to not handle VMXON/VMXOFF in tdx_detect() and tdx_init() but depend on the caller to guarantee that, since so far KVM is the only user of TDX. In the long term, more kernel components are likely to use VMXON/VMXOFF to support TDX (i.e. TDX module runtime update), so a reference-based approach to do VMXON/VMXOFF is likely needed.

Signed-off-by Kai Huang <[email protected]>

---
  arch/x86/include/asm/tdx.h  |   4 +
  arch/x86/virt/vmx/tdx/tdx.c | 222 ++++++++++++++++++++++++++++++++++++
  2 files changed, 226 insertions(+)

arch/x86/include/asm/tdx.h +4 -0

index 1f29813b1646..c8af2ba6bb8a 100644
@@ -92,8 +92,12 @@ static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1,
 
  #ifdef CONFIG_INTEL_TDX_HOST
  void tdx_detect_cpu(struct cpuinfo_x86 *c);
+int tdx_detect(void);
+int tdx_init(void);
  #else
  static inline void tdx_detect_cpu(struct cpuinfo_x86 *c) { }
+static inline int tdx_detect(void) { return -ENODEV; }
+static inline int tdx_init(void) { return -ENODEV; }
  #endif /* CONFIG_INTEL_TDX_HOST */
 
  #endif /* !__ASSEMBLY__ */

arch/x86/virt/vmx/tdx/tdx.c +8 -0

index ba2210001ea8..53093d4ad458 100644
@@ -9,6 +9,8 @@
 
  #include <linux/types.h>
  #include <linux/cpumask.h>
+#include <linux/mutex.h>
+#include <linux/cpu.h>
  #include <asm/msr-index.h>
  #include <asm/msr.h>
  #include <asm/cpufeature.h>

@@ -45,12 +47,33 @@
  		((u32)(((_keyid_part) & 0xffffffffull) + 1))
  #define TDX_KEYID_NUM(_keyid_part)	((u32)((_keyid_part) >> 32))
 
+/*
+ * TDX module status during initialization
+ */
+enum tdx_module_status_t {
+	/* TDX module status is unknown */
+	TDX_MODULE_UNKNOWN,

arch/x86/virt/vmx/tdx/tdx.c +2 -0

+
  static bool __seamrr_enabled(void)
  {
  	return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;

@@ -172,3 +195,202 @@ void tdx_detect_cpu(struct cpuinfo_x86 *c)
  	detect_seam(c);
  	detect_tdx_keyids(c);
  }
+

arch/x86/virt/vmx/tdx/tdx.c +25 -0

+	 * report SEAMRR as enabled in this case.
+	 */
+	if (!cpumask_equal(&cpus_booted_once_mask,
+					cpu_present_mask))
+		return false;
+
+	return __seamrr_enabled();
+}
+
+static bool tdx_keyid_sufficient(void)
+{
+	if (!cpumask_equal(&cpus_booted_once_mask,
+					cpu_present_mask))
+		return false;
+
+	/*
+	 * TDX requires at least two KeyIDs: one global KeyID to
+	 * protect the metadata of the TDX module and one or more
+	 * KeyIDs to run TD guests.
+	 */
+	return tdx_keyid_num >= 2;
+}
+
+static int __tdx_detect(void)
+{

arch/x86/virt/vmx/tdx/tdx.c +25 -0

+	if (!tdx_keyid_sufficient()) {
+		pr_info("Number of TDX private KeyIDs too small: %u.\n",
+				tdx_keyid_num);
+		goto no_tdx_module;
+	}
+
+	/* Return -ENODEV until the TDX module is detected */
+no_tdx_module:
+	tdx_module_status = TDX_MODULE_NONE;
+	return -ENODEV;
+}
+
+static int init_tdx_module(void)
+{
+	/*
+	 * Return -EFAULT until all steps of TDX module
+	 * initialization are done.
+	 */
+	return -EFAULT;
+}
+
+static void shutdown_tdx_module(void)
+{
+	/* TODO: Shut down the TDX module */
+	tdx_module_status = TDX_MODULE_SHUTDOWN;

arch/x86/virt/vmx/tdx/tdx.c +37 -0

+	 * the key of the global KeyID requires calling one SEAMCALL for
+	 * each package.  For simplicity, disable CPU hotplug in the whole
+	 * initialization process.
+	 *
+	 * It's perhaps better to check whether all BIOS-enabled cpus are
+	 * online before starting initializing, and return early if not.
+	 * But none of 'possible', 'present' and 'online' CPU masks
+	 * represents BIOS-enabled cpus.  For example, 'possible' mask is
+	 * impacted by 'nr_cpus' or 'possible_cpus' kernel command line.
+	 * Just let the SEAMCALL to fail if not all BIOS-enabled cpus are
+	 * online.
+	 */
+	cpus_read_lock();
+
+	ret = init_tdx_module();
+
+	/*
+	 * Shut down the TDX module in case of any error during the
+	 * initialization process.  It's meaningless to leave the TDX
+	 * module in any middle state of the initialization process.
+	 */
+	if (ret)
+		shutdown_tdx_module();
+
+	cpus_read_unlock();
+
+	return ret;
+}
+
+/**
+ * tdx_detect - Detect whether the TDX module has been loaded
+ *
+ * Detect whether the TDX module has been loaded and ready for
+ * initialization.  Only call this function when all cpus are
+ * already in VMX operation.
+ *
+ * This function can be called in parallel by multiple callers.

arch/x86/virt/vmx/tdx/tdx.c +21 -0

+{
+	int ret;
+
+	mutex_lock(&tdx_module_lock);
+
+	switch (tdx_module_status) {
+	case TDX_MODULE_UNKNOWN:
+		ret = __tdx_detect();
+		break;
+	case TDX_MODULE_NONE:
+		ret = -ENODEV;
+		break;
+	case TDX_MODULE_LOADED:
+	case TDX_MODULE_INITIALIZED:
+		ret = 0;
+		break;
+	case TDX_MODULE_SHUTDOWN:
+		ret = -EFAULT;
+		break;
+	default:
+		WARN_ON(1);

arch/x86/virt/vmx/tdx/tdx.c +5 -0

+ *
+ * Initialize the TDX module to make it ready to run TD guests.  This
+ * function should be called after tdx_detect() returns successful.
+ * Only call this function when all cpus are online and are in VMX
+ * operation.  CPU hotplug is temporarily disabled internally.

arch/x86/virt/vmx/tdx/tdx.c +25 -0

+int tdx_init(void)
+{
+	int ret;
+
+	mutex_lock(&tdx_module_lock);
+
+	switch (tdx_module_status) {
+	case TDX_MODULE_NONE:
+		ret = -ENODEV;
+		break;
+	case TDX_MODULE_LOADED:
 
+		ret = __tdx_init();
+		break;
+	case TDX_MODULE_INITIALIZED:
+		ret = 0;
+		break;
+	default:
+		ret = -EFAULT;
+		break;
+	}
+	mutex_unlock(&tdx_module_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tdx_init);

Quoted 3 lines · Kai Huang

The TDX module is essentially a CPU-attested software module running in the new Secure Arbitration Mode (SEAM) to protect VMs from malicious host and certain physical attacks. The TDX module implements the

/s/host/hosts

Quoted 3 lines

functions to build, tear down and start execution of the protected VMs called Trusted Domains (TD). Before the TDX module can be used to

/s/Trusted/Trust

Quoted 7 lines

create and run TD guests, it must be loaded into the SEAM Range Register (SEAMRR) and properly initialized. The TDX module is expected to be loaded by BIOS before booting to the kernel, and the kernel is expected to detect and initialize it, using the SEAMCALLs defined by TDX architecture.

Quoted 5 lines

The TDX module can be initialized only once in its lifetime. Instead of always initializing it at boot time, this implementation chooses an on-demand approach to initialize TDX until there is a real need (e.g when requested by KVM). This avoids consuming the memory that must be allocated by kernel and given to the TDX module as metadata (~1/256th of

allocated by the kernel

Quoted 94 lines

the TDX-usable memory), and also saves the time of initializing the TDX module (and the metadata) when TDX is not used at all. Initializing the TDX module at runtime on-demand also is more flexible to support TDX module runtime updating in the future (after updating the TDX module, it needs to be initialized again).

Introduce two placeholders tdx_detect() and tdx_init() to detect and initialize the TDX module on demand, with a state machine introduced to orchestrate the entire process (in case of multiple callers).

To start with, tdx_detect() checks SEAMRR and TDX private KeyIDs. The TDX module is reported as not loaded if either SEAMRR is not enabled, or there are no enough TDX private KeyIDs to create any TD guest. The TDX module itself requires one global TDX private KeyID to crypto protect its metadata.

And tdx_init() is currently empty. The TDX module will be initialized in multi-steps defined by the TDX architecture:

1) Global initialization;
2) Logical-CPU scope initialization;
3) Enumerate the TDX module capabilities and platform configuration;
4) Configure the TDX module about usable memory ranges and global

      KeyID information;

5) Package-scope configuration for the global KeyID;
6) Initialize usable memory ranges based on 4).

The TDX module can also be shut down at any time during its lifetime. In case of any error during the initialization process, shut down the module. It's pointless to leave the module in any intermediate state during the initialization.

SEAMCALL requires SEAMRR being enabled and CPU being already in VMX operation (VMXON has been done), otherwise it generates #UD. So far only KVM handles VMXON/VMXOFF. Choose to not handle VMXON/VMXOFF in tdx_detect() and tdx_init() but depend on the caller to guarantee that, since so far KVM is the only user of TDX. In the long term, more kernel components are likely to use VMXON/VMXOFF to support TDX (i.e. TDX module runtime update), so a reference-based approach to do VMXON/VMXOFF is likely needed.

Signed-off-by Kai Huang <[email protected]>

---
  arch/x86/include/asm/tdx.h  |   4 +
  arch/x86/virt/vmx/tdx/tdx.c | 222 ++++++++++++++++++++++++++++++++++++
  2 files changed, 226 insertions(+)

arch/x86/include/asm/tdx.h +4 -0

index 1f29813b1646..c8af2ba6bb8a 100644
@@ -92,8 +92,12 @@ static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1,
 
  #ifdef CONFIG_INTEL_TDX_HOST
  void tdx_detect_cpu(struct cpuinfo_x86 *c);
+int tdx_detect(void);
+int tdx_init(void);
  #else
  static inline void tdx_detect_cpu(struct cpuinfo_x86 *c) { }
+static inline int tdx_detect(void) { return -ENODEV; }
+static inline int tdx_init(void) { return -ENODEV; }
  #endif /* CONFIG_INTEL_TDX_HOST */
 
  #endif /* !__ASSEMBLY__ */

arch/x86/virt/vmx/tdx/tdx.c +14 -0

index ba2210001ea8..53093d4ad458 100644
@@ -9,6 +9,8 @@
 
  #include <linux/types.h>
  #include <linux/cpumask.h>
+#include <linux/mutex.h>
+#include <linux/cpu.h>
  #include <asm/msr-index.h>
  #include <asm/msr.h>
  #include <asm/cpufeature.h>

@@ -45,12 +47,33 @@
  		((u32)(((_keyid_part) & 0xffffffffull) + 1))
  #define TDX_KEYID_NUM(_keyid_part)	((u32)((_keyid_part) >> 32))
 
+/*
+ * TDX module status during initialization
+ */
+enum tdx_module_status_t {
+	/* TDX module status is unknown */
+	TDX_MODULE_UNKNOWN,
+	/* TDX module is not loaded */
+	TDX_MODULE_NONE,
+	/* TDX module is loaded, but not initialized */
+	TDX_MODULE_LOADED,
+	/* TDX module is fully initialized */
+	TDX_MODULE_INITIALIZED,

arch/x86/virt/vmx/tdx/tdx.c line 65

Quoted 4 lines

arch/x86/virt/vmx/tdx/tdx.c +4 -0

+	/* TDX module is shutdown due to error during initialization */
+	TDX_MODULE_SHUTDOWN,
+};
+

May be adding these states when you really need will make more sense. Currently this patch only uses SHUTDOWN and NONE states. Other state usage is not very clear.

Quoted 7 lines

arch/x86/virt/vmx/tdx/tdx.c +0 -0

  /* BIOS must configure SEAMRR registers for all cores consistently */
  static u64 seamrr_base, seamrr_mask;
 
  static u32 tdx_keyid_start;
  static u32 tdx_keyid_num;
 

arch/x86/virt/vmx/tdx/tdx.c line 75

Quoted 4 lines

arch/x86/virt/vmx/tdx/tdx.c +4 -0

+static enum tdx_module_status_t tdx_module_status;
+
+/* Prevent concurrent attempts on TDX detection and initialization */
+static DEFINE_MUTEX(tdx_module_lock);

Any possible concurrent usage models?

Quoted 14 lines

arch/x86/virt/vmx/tdx/tdx.c +6 -0

+
  static bool __seamrr_enabled(void)
  {
  	return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;

@@ -172,3 +195,202 @@ void tdx_detect_cpu(struct cpuinfo_x86 *c)
  	detect_seam(c);
  	detect_tdx_keyids(c);
  }
+
+static bool seamrr_enabled(void)
+{
+	/*
+	 * To detect any BIOS misconfiguration among cores, all logical

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 208

Quoted 6 lines

arch/x86/virt/vmx/tdx/tdx.c +6 -0

+	 * cpus must have been brought up at least once.  This is true
+	 * unless 'maxcpus' kernel command line is used to limit the
+	 * number of cpus to be brought up during boot time.  However
+	 * 'maxcpus' is basically an invalid operation mode due to the
+	 * MCE broadcast problem, and it should not be used on a TDX
+	 * capable machine.  Just do paranoid check here and do not

a paranoid check

Quoted 31 lines

arch/x86/virt/vmx/tdx/tdx.c +30 -0

+	 * report SEAMRR as enabled in this case.
+	 */
+	if (!cpumask_equal(&cpus_booted_once_mask,
+					cpu_present_mask))
+		return false;
+
+	return __seamrr_enabled();
+}
+
+static bool tdx_keyid_sufficient(void)
+{
+	if (!cpumask_equal(&cpus_booted_once_mask,
+					cpu_present_mask))
+		return false;
+
+	/*
+	 * TDX requires at least two KeyIDs: one global KeyID to
+	 * protect the metadata of the TDX module and one or more
+	 * KeyIDs to run TD guests.
+	 */
+	return tdx_keyid_num >= 2;
+}
+
+static int __tdx_detect(void)
+{
+	/* The TDX module is not loaded if SEAMRR is disabled */
+	if (!seamrr_enabled()) {
+		pr_info("SEAMRR not enabled.\n");
+		goto no_tdx_module;
+	}

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 243

Quoted 5 lines

arch/x86/virt/vmx/tdx/tdx.c +5 -0

+
+	/*
+	 * Also do not report the TDX module as loaded if there's
+	 * no enough TDX private KeyIDs to run any TD guests.
+	 */

You are not returning TDX_MODULE_LOADED under any current scenarios. So think above comment is not accurate.

Quoted 30 lines

arch/x86/virt/vmx/tdx/tdx.c +29 -0

+	if (!tdx_keyid_sufficient()) {
+		pr_info("Number of TDX private KeyIDs too small: %u.\n",
+				tdx_keyid_num);
+		goto no_tdx_module;
+	}
+
+	/* Return -ENODEV until the TDX module is detected */
+no_tdx_module:
+	tdx_module_status = TDX_MODULE_NONE;
+	return -ENODEV;
+}
+
+static int init_tdx_module(void)
+{
+	/*
+	 * Return -EFAULT until all steps of TDX module
+	 * initialization are done.
+	 */
+	return -EFAULT;
+}
+
+static void shutdown_tdx_module(void)
+{
+	/* TODO: Shut down the TDX module */
+	tdx_module_status = TDX_MODULE_SHUTDOWN;
+}
+
+static int __tdx_init(void)
+{

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 278

Quoted 6 lines

arch/x86/virt/vmx/tdx/tdx.c +6 -0

+	int ret;
+
+	/*
+	 * Logical-cpu scope initialization requires calling one SEAMCALL
+	 * on all logical cpus enabled by BIOS.  Shutting down the TDX
+	 * module also has such requirement.  Further more, configuring

such a requirement

Quoted 43 lines

arch/x86/virt/vmx/tdx/tdx.c +42 -0

+	 * the key of the global KeyID requires calling one SEAMCALL for
+	 * each package.  For simplicity, disable CPU hotplug in the whole
+	 * initialization process.
+	 *
+	 * It's perhaps better to check whether all BIOS-enabled cpus are
+	 * online before starting initializing, and return early if not.
+	 * But none of 'possible', 'present' and 'online' CPU masks
+	 * represents BIOS-enabled cpus.  For example, 'possible' mask is
+	 * impacted by 'nr_cpus' or 'possible_cpus' kernel command line.
+	 * Just let the SEAMCALL to fail if not all BIOS-enabled cpus are
+	 * online.
+	 */
+	cpus_read_lock();
+
+	ret = init_tdx_module();
+
+	/*
+	 * Shut down the TDX module in case of any error during the
+	 * initialization process.  It's meaningless to leave the TDX
+	 * module in any middle state of the initialization process.
+	 */
+	if (ret)
+		shutdown_tdx_module();
+
+	cpus_read_unlock();
+
+	return ret;
+}
+
+/**
+ * tdx_detect - Detect whether the TDX module has been loaded
+ *
+ * Detect whether the TDX module has been loaded and ready for
+ * initialization.  Only call this function when all cpus are
+ * already in VMX operation.
+ *
+ * This function can be called in parallel by multiple callers.
+ *
+ * Return:
+ *
+ * * -0:	The TDX module has been loaded and ready for
+ *		initialization.

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 325

Quoted 5 lines

arch/x86/virt/vmx/tdx/tdx.c +5 -0

+ * * -ENODEV:	The TDX module is not loaded.
+ * * -EPERM:	CPU is not in VMX operation.
+ * * -EFAULT:	Other internal fatal errors.
+ */
+int tdx_detect(void)

Will this function be used separately or always along with tdx_init()?

Quoted 28 lines

arch/x86/virt/vmx/tdx/tdx.c +27 -0

+{
+	int ret;
+
+	mutex_lock(&tdx_module_lock);
+
+	switch (tdx_module_status) {
+	case TDX_MODULE_UNKNOWN:
+		ret = __tdx_detect();
+		break;
+	case TDX_MODULE_NONE:
+		ret = -ENODEV;
+		break;
+	case TDX_MODULE_LOADED:
+	case TDX_MODULE_INITIALIZED:
+		ret = 0;
+		break;
+	case TDX_MODULE_SHUTDOWN:
+		ret = -EFAULT;
+		break;
+	default:
+		WARN_ON(1);
+		ret = -EFAULT;
+	}
+
+	mutex_unlock(&tdx_module_lock);
+	return ret;
+}

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 356

Quoted 4 lines

arch/x86/virt/vmx/tdx/tdx.c +4 -0

+EXPORT_SYMBOL_GPL(tdx_detect);
+
+/**
+ * tdx_init - Initialize the TDX module

If it for tdx module initialization, why not call it tdx_module_init()? If not, update the description appropriately.

Quoted 6 lines

arch/x86/virt/vmx/tdx/tdx.c +5 -0

+ *
+ * Initialize the TDX module to make it ready to run TD guests.  This
+ * function should be called after tdx_detect() returns successful.
+ * Only call this function when all cpus are online and are in VMX
+ * operation.  CPU hotplug is temporarily disabled internally.

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 371

Quoted 10 lines

arch/x86/virt/vmx/tdx/tdx.c +10 -0

+ *
+ * This function can be called in parallel by multiple callers.
+ *
+ * Return:
+ *
+ * * -0:	The TDX module has been successfully initialized.
+ * * -ENODEV:	The TDX module is not loaded.
+ * * -EPERM:	The CPU which does SEAMCALL is not in VMX operation.
+ * * -EFAULT:	Other internal fatal errors.
+ */

You return differnt error values just for debug prints or there are other uses for it?

Quoted 5 lines · Sathyanarayanan Kuppuswamy

Quoted 3 lines · Kai Huang

The TDX module is essentially a CPU-attested software module running in the new Secure Arbitration Mode (SEAM) to protect VMs from malicious host and certain physical attacks. The TDX module implements the

/s/host/hosts

I don't quite get. Could you explain why there are multiple hosts?

Quoted 7 lines · Sathyanarayanan Kuppuswamy

Quoted 3 lines · Kai Huang

The TDX module is essentially a CPU-attested software module running in the new Secure Arbitration Mode (SEAM) to protect VMs from malicious host and certain physical attacks. The TDX module implements the

/s/host/hosts

I don't quite get. Could you explain why there are multiple hosts?

Quoted 17 lines

Quoted 17 lines

arch/x86/virt/vmx/tdx/tdx.c +14 -0

+
+/**
+ * tdx_detect - Detect whether the TDX module has been loaded
+ *
+ * Detect whether the TDX module has been loaded and ready for
+ * initialization.  Only call this function when all cpus are
+ * already in VMX operation.
+ *
+ * This function can be called in parallel by multiple callers.
+ *
+ * Return:
+ *
+ * * -0:	The TDX module has been loaded and ready for
+ *		initialization.

Quoted 8 lines

Quoted 5 lines

+ * * -ENODEV: The TDX module is not loaded.
+ * * -EPERM: CPU is not in VMX operation.
+ * * -EFAULT: Other internal fatal errors.
+ */
+int tdx_detect(void)

Will this function be used separately or always along with tdx_init()?

The caller should first use tdx_detect() and then use tdx_init(). If caller only uses tdx_detect(), then TDX module won't be initialized (unless other caller does this). If caller calls tdx_init() before tdx_detect(), it will get error.

Quoted 24 lines

Quoted 15 lines

Quoted 15 lines

arch/x86/virt/vmx/tdx/tdx.c +14 -0

+
+/**
+ * tdx_detect - Detect whether the TDX module has been loaded
+ *
+ * Detect whether the TDX module has been loaded and ready for
+ * initialization.  Only call this function when all cpus are
+ * already in VMX operation.
+ *
+ * This function can be called in parallel by multiple callers.
+ *
+ * Return:
+ *
+ * * -0:	The TDX module has been loaded and ready for
+ *		initialization.

Quoted 8 lines

Quoted 5 lines

+ * * -ENODEV: The TDX module is not loaded.
+ * * -EPERM: CPU is not in VMX operation.
+ * * -EFAULT: Other internal fatal errors.
+ */
+int tdx_detect(void)

Will this function be used separately or always along with tdx_init()?

Quoted 5 lines

The caller should first use tdx_detect() and then use tdx_init(). If caller only uses tdx_detect(), then TDX module won't be initialized (unless other caller does this). If caller calls tdx_init() before tdx_detect(), it will get error.

I just checked your patch set to understand where you are using tdx_detect()/tdx_init(). But I did not find any callers. Did I miss it? or it is not used in your patch set?

The TDX module is essentially a CPU-attested software module running in the new Secure Arbitration Mode (SEAM) to protect VMs from malicious host and certain physical attacks. The TDX module implements the functions to build, tear down and start execution of the protected VMs called Trusted Domains (TD). Before the TDX module can be used to create and run TD guests, it must be loaded into the SEAM Range Register (SEAMRR) and properly initialized.

The TDX module is expected to be
loaded by BIOS before booting to the kernel, and the kernel is expected to detect and initialize it, using the SEAMCALLs defined by TDX architecture.

The TDX module can be initialized only once in its lifetime. Instead of always initializing it at boot time, this implementation chooses an on-demand approach to initialize TDX until there is a real need (e.g when requested by KVM). This avoids consuming the memory that must be allocated by kernel and given to the TDX module as metadata (~1/256th of the TDX-usable memory), and also saves the time of initializing the TDX module (and the metadata) when TDX is not used at all. Initializing the TDX module at runtime on-demand also is more flexible to support TDX module runtime updating in the future (after updating the TDX module, it needs to be initialized again).

Introduce two placeholders tdx_detect() and tdx_init() to detect and initialize the TDX module on demand, with a state machine introduced to orchestrate the entire process (in case of multiple callers).

To start with, tdx_detect() checks SEAMRR and TDX private KeyIDs. The TDX module is reported as not loaded if either SEAMRR is not enabled, or there are no enough TDX private KeyIDs to create any TD guest. The TDX module itself requires one global TDX private KeyID to crypto protect its metadata.

And tdx_init() is currently empty. The TDX module will be initialized in multi-steps defined by the TDX architecture:

1) Global initialization;
2) Logical-CPU scope initialization;
3) Enumerate the TDX module capabilities and platform configuration;
4) Configure the TDX module about usable memory ranges and global

     KeyID information;

5) Package-scope configuration for the global KeyID;
6) Initialize usable memory ranges based on 4).

The TDX module can also be shut down at any time during its lifetime. In case of any error during the initialization process, shut down the module. It's pointless to leave the module in any intermediate state during the initialization.

SEAMCALL requires SEAMRR being enabled and CPU being already in VMX operation (VMXON has been done), otherwise it generates #UD. So far only KVM handles VMXON/VMXOFF. Choose to not handle VMXON/VMXOFF in tdx_detect() and tdx_init() but depend on the caller to guarantee that, since so far KVM is the only user of TDX. In the long term, more kernel components are likely to use VMXON/VMXOFF to support TDX (i.e. TDX module runtime update), so a reference-based approach to do VMXON/VMXOFF is likely needed.

Signed-off-by Kai Huang <[email protected]>

---
 arch/x86/include/asm/tdx.h  |   4 +
 arch/x86/virt/vmx/tdx/tdx.c | 222 ++++++++++++++++++++++++++++++++++++
 2 files changed, 226 insertions(+)

arch/x86/include/asm/tdx.h +4 -0

index 1f29813b1646..c8af2ba6bb8a 100644
@@ -92,8 +92,12 @@ static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1,
 
 #ifdef CONFIG_INTEL_TDX_HOST
 void tdx_detect_cpu(struct cpuinfo_x86 *c);
+int tdx_detect(void);
+int tdx_init(void);
 #else
 static inline void tdx_detect_cpu(struct cpuinfo_x86 *c) { }
+static inline int tdx_detect(void) { return -ENODEV; }
+static inline int tdx_init(void) { return -ENODEV; }
 #endif /* CONFIG_INTEL_TDX_HOST */
 
 #endif /* !__ASSEMBLY__ */

arch/x86/virt/vmx/tdx/tdx.c +38 -0

index ba2210001ea8..53093d4ad458 100644
@@ -9,6 +9,8 @@
 
 #include <linux/types.h>
 #include <linux/cpumask.h>
+#include <linux/mutex.h>
+#include <linux/cpu.h>
 #include <asm/msr-index.h>
 #include <asm/msr.h>
 #include <asm/cpufeature.h>

@@ -45,12 +47,33 @@
 		((u32)(((_keyid_part) & 0xffffffffull) + 1))
 #define TDX_KEYID_NUM(_keyid_part)	((u32)((_keyid_part) >> 32))
 
+/*
+ * TDX module status during initialization
+ */
+enum tdx_module_status_t {
+	/* TDX module status is unknown */
+	TDX_MODULE_UNKNOWN,
+	/* TDX module is not loaded */
+	TDX_MODULE_NONE,
+	/* TDX module is loaded, but not initialized */
+	TDX_MODULE_LOADED,
+	/* TDX module is fully initialized */
+	TDX_MODULE_INITIALIZED,
+	/* TDX module is shutdown due to error during initialization */
+	TDX_MODULE_SHUTDOWN,
+};
+
 /* BIOS must configure SEAMRR registers for all cores consistently */
 static u64 seamrr_base, seamrr_mask;
 
 static u32 tdx_keyid_start;
 static u32 tdx_keyid_num;
 
+static enum tdx_module_status_t tdx_module_status;
+
+/* Prevent concurrent attempts on TDX detection and initialization */
+static DEFINE_MUTEX(tdx_module_lock);
+
 static bool __seamrr_enabled(void)
 {
 	return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;

@@ -172,3 +195,202 @@ void tdx_detect_cpu(struct cpuinfo_x86 *c)
 	detect_seam(c);
 	detect_tdx_keyids(c);
 }
+
+static bool seamrr_enabled(void)
+{
+	/*
+	 * To detect any BIOS misconfiguration among cores, all logical
+	 * cpus must have been brought up at least once.  This is true
+	 * unless 'maxcpus' kernel command line is used to limit the
+	 * number of cpus to be brought up during boot time.  However
+	 * 'maxcpus' is basically an invalid operation mode due to the
+	 * MCE broadcast problem, and it should not be used on a TDX
+	 * capable machine.  Just do paranoid check here and do not
+	 * report SEAMRR as enabled in this case.
+	 */
+	if (!cpumask_equal(&cpus_booted_once_mask,
+					cpu_present_mask))

arch/x86/virt/vmx/tdx/tdx.c +5 -0

+	/*
+	 * TDX requires at least two KeyIDs: one global KeyID to
+	 * protect the metadata of the TDX module and one or more
+	 * KeyIDs to run TD guests.
+	 */

arch/x86/virt/vmx/tdx/tdx.c +35 -0

+	/*
+	 * Also do not report the TDX module as loaded if there's
+	 * no enough TDX private KeyIDs to run any TD guests.
+	 */
+	if (!tdx_keyid_sufficient()) {
+		pr_info("Number of TDX private KeyIDs too small: %u.\n",
+				tdx_keyid_num);
+		goto no_tdx_module;
+	}
+
+	/* Return -ENODEV until the TDX module is detected */
+no_tdx_module:
+	tdx_module_status = TDX_MODULE_NONE;
+	return -ENODEV;
+}
+
+static int init_tdx_module(void)
+{
+	/*
+	 * Return -EFAULT until all steps of TDX module
+	 * initialization are done.
+	 */
+	return -EFAULT;
+}
+
+static void shutdown_tdx_module(void)
+{
+	/* TODO: Shut down the TDX module */
+	tdx_module_status = TDX_MODULE_SHUTDOWN;
+}
+
+static int __tdx_init(void)
+{
+	int ret;
+

arch/x86/virt/vmx/tdx/tdx.c +26 -0

+	 * But none of 'possible', 'present' and 'online' CPU masks
+	 * represents BIOS-enabled cpus.  For example, 'possible' mask is
+	 * impacted by 'nr_cpus' or 'possible_cpus' kernel command line.
+	 * Just let the SEAMCALL to fail if not all BIOS-enabled cpus are
+	 * online.
+	 */
+	cpus_read_lock();
+
+	ret = init_tdx_module();
+
+	/*
+	 * Shut down the TDX module in case of any error during the
+	 * initialization process.  It's meaningless to leave the TDX
+	 * module in any middle state of the initialization process.
+	 */
+	if (ret)
+		shutdown_tdx_module();
+
+	cpus_read_unlock();
+
+	return ret;
+}
+
+/**
+ * tdx_detect - Detect whether the TDX module has been loaded
+ *

arch/x86/virt/vmx/tdx/tdx.c +66 -0

+ * * -ENODEV:	The TDX module is not loaded.
+ * * -EPERM:	CPU is not in VMX operation.
+ * * -EFAULT:	Other internal fatal errors.
+ */
+int tdx_detect(void)
+{
+	int ret;
+
+	mutex_lock(&tdx_module_lock);
+
+	switch (tdx_module_status) {
+	case TDX_MODULE_UNKNOWN:
+		ret = __tdx_detect();
+		break;
+	case TDX_MODULE_NONE:
+		ret = -ENODEV;
+		break;
+	case TDX_MODULE_LOADED:
+	case TDX_MODULE_INITIALIZED:
+		ret = 0;
+		break;
+	case TDX_MODULE_SHUTDOWN:
+		ret = -EFAULT;
+		break;
+	default:
+		WARN_ON(1);
+		ret = -EFAULT;
+	}
+
+	mutex_unlock(&tdx_module_lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tdx_detect);
+
+/**
+ * tdx_init - Initialize the TDX module
+ *
+ * Initialize the TDX module to make it ready to run TD guests.  This
+ * function should be called after tdx_detect() returns successful.
+ * Only call this function when all cpus are online and are in VMX
+ * operation.  CPU hotplug is temporarily disabled internally.
+ *
+ * This function can be called in parallel by multiple callers.
+ *
+ * Return:
+ *
+ * * -0:	The TDX module has been successfully initialized.
+ * * -ENODEV:	The TDX module is not loaded.
+ * * -EPERM:	The CPU which does SEAMCALL is not in VMX operation.
+ * * -EFAULT:	Other internal fatal errors.
+ */
+int tdx_init(void)
+{
+	int ret;
+
+	mutex_lock(&tdx_module_lock);
+
+	switch (tdx_module_status) {
+	case TDX_MODULE_NONE:
+		ret = -ENODEV;
+		break;
+	case TDX_MODULE_LOADED:
+		ret = __tdx_init();
+		break;
+	case TDX_MODULE_INITIALIZED:
+		ret = 0;

Quoted 7 lines · Kai Huang

The TDX module is essentially a CPU-attested software module running in the new Secure Arbitration Mode (SEAM) to protect VMs from malicious host and certain physical attacks. The TDX module implements the functions to build, tear down and start execution of the protected VMs called Trusted Domains (TD). Before the TDX module can be used to create and run TD guests, it must be loaded into the SEAM Range Register (SEAMRR) and properly initialized.

The module isn't loaded into a register, right?

It's loaded into a memory area pointed to *by* the register.

Quoted 5 lines

The TDX module is expected to be
loaded by BIOS before booting to the kernel, and the kernel is expected to detect and initialize it, using the SEAMCALLs defined by TDX architecture.

Wait a sec... So, what was all this gobleygook about TDX module loading and SEAMRR's if the kernel just has the TDX module *handed* to it already loaded?

It looks to me like you wrote all of this before the TDX module was being loaded by the BIOS and neglected to go and update these changelogs.

Quoted 16 lines

The TDX module can be initialized only once in its lifetime. Instead of always initializing it at boot time, this implementation chooses an on-demand approach to initialize TDX until there is a real need (e.g when requested by KVM). This avoids consuming the memory that must be allocated by kernel and given to the TDX module as metadata (~1/256th of the TDX-usable memory), and also saves the time of initializing the TDX module (and the metadata) when TDX is not used at all. Initializing the TDX module at runtime on-demand also is more flexible to support TDX module runtime updating in the future (after updating the TDX module, it needs to be initialized again).

Introduce two placeholders tdx_detect() and tdx_init() to detect and initialize the TDX module on demand, with a state machine introduced to orchestrate the entire process (in case of multiple callers).

Quoted 5 lines

To start with, tdx_detect() checks SEAMRR and TDX private KeyIDs. The TDX module is reported as not loaded if either SEAMRR is not enabled, or there are no enough TDX private KeyIDs to create any TD guest. The TDX module itself requires one global TDX private KeyID to crypto protect its metadata.

This is stepping over the line into telling me what the code does instead of why.

Quoted 19 lines

arch/x86/virt/vmx/tdx/tdx.c +19 -0

+
+static bool seamrr_enabled(void)
+{
+	/*
+	 * To detect any BIOS misconfiguration among cores, all logical
+	 * cpus must have been brought up at least once.  This is true
+	 * unless 'maxcpus' kernel command line is used to limit the
+	 * number of cpus to be brought up during boot time.  However
+	 * 'maxcpus' is basically an invalid operation mode due to the
+	 * MCE broadcast problem, and it should not be used on a TDX
+	 * capable machine.  Just do paranoid check here and do not
+	 * report SEAMRR as enabled in this case.
+	 */
+	if (!cpumask_equal(&cpus_booted_once_mask,
+					cpu_present_mask))
+		return false;
+
+	return __seamrr_enabled();
+}

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 222

Quoted 6 lines

arch/x86/virt/vmx/tdx/tdx.c +6 -0

+
+static bool tdx_keyid_sufficient(void)
+{
+	if (!cpumask_equal(&cpus_booted_once_mask,
+					cpu_present_mask))
+		return false;

I'd move this cpumask_equal() to a helper.

Quoted 11 lines

arch/x86/virt/vmx/tdx/tdx.c +10 -0

+	/*
+	 * TDX requires at least two KeyIDs: one global KeyID to
+	 * protect the metadata of the TDX module and one or more
+	 * KeyIDs to run TD guests.
+	 */
+	return tdx_keyid_num >= 2;
+}
+
+static int __tdx_detect(void)
+{

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 238

Quoted 5 lines

arch/x86/virt/vmx/tdx/tdx.c +5 -0

+	/* The TDX module is not loaded if SEAMRR is disabled */
+	if (!seamrr_enabled()) {
+		pr_info("SEAMRR not enabled.\n");
+		goto no_tdx_module;
+	}

Why even bother with the SEAMRR stuff? It sounded like you can "ping" the module with SEAMCALL. Why not just use that directly?

Quoted 40 lines

arch/x86/virt/vmx/tdx/tdx.c +39 -0

+	/*
+	 * Also do not report the TDX module as loaded if there's
+	 * no enough TDX private KeyIDs to run any TD guests.
+	 */
+	if (!tdx_keyid_sufficient()) {
+		pr_info("Number of TDX private KeyIDs too small: %u.\n",
+				tdx_keyid_num);
+		goto no_tdx_module;
+	}
+
+	/* Return -ENODEV until the TDX module is detected */
+no_tdx_module:
+	tdx_module_status = TDX_MODULE_NONE;
+	return -ENODEV;
+}
+
+static int init_tdx_module(void)
+{
+	/*
+	 * Return -EFAULT until all steps of TDX module
+	 * initialization are done.
+	 */
+	return -EFAULT;
+}
+
+static void shutdown_tdx_module(void)
+{
+	/* TODO: Shut down the TDX module */
+	tdx_module_status = TDX_MODULE_SHUTDOWN;
+}
+
+static int __tdx_init(void)
+{
+	int ret;
+
+	/*
+	 * Logical-cpu scope initialization requires calling one SEAMCALL
+	 * on all logical cpus enabled by BIOS.  Shutting down the TDX
+	 * module also has such requirement.  Further more, configuring

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 284

Quoted 6 lines

arch/x86/virt/vmx/tdx/tdx.c +6 -0

+	 * the key of the global KeyID requires calling one SEAMCALL for
+	 * each package.  For simplicity, disable CPU hotplug in the whole
+	 * initialization process.
+	 *
+	 * It's perhaps better to check whether all BIOS-enabled cpus are
+	 * online before starting initializing, and return early if not.

But you did some of this cpumask checking above. Right?

Quoted 31 lines

arch/x86/virt/vmx/tdx/tdx.c +30 -0

+	 * But none of 'possible', 'present' and 'online' CPU masks
+	 * represents BIOS-enabled cpus.  For example, 'possible' mask is
+	 * impacted by 'nr_cpus' or 'possible_cpus' kernel command line.
+	 * Just let the SEAMCALL to fail if not all BIOS-enabled cpus are
+	 * online.
+	 */
+	cpus_read_lock();
+
+	ret = init_tdx_module();
+
+	/*
+	 * Shut down the TDX module in case of any error during the
+	 * initialization process.  It's meaningless to leave the TDX
+	 * module in any middle state of the initialization process.
+	 */
+	if (ret)
+		shutdown_tdx_module();
+
+	cpus_read_unlock();
+
+	return ret;
+}
+
+/**
+ * tdx_detect - Detect whether the TDX module has been loaded
+ *
+ * Detect whether the TDX module has been loaded and ready for
+ * initialization.  Only call this function when all cpus are
+ * already in VMX operation.
+ *

Quoted 6 lines

+ * This function can be called in parallel by multiple callers.
+ *
+ * Return:
+ *
+ * * -0: The TDX module has been loaded and ready for
+ * initialization.

"-0", eh?

Quoted 72 lines

+ * * -ENODEV: The TDX module is not loaded.
+ * * -EPERM: CPU is not in VMX operation.
+ * * -EFAULT: Other internal fatal errors.
+ */

arch/x86/virt/vmx/tdx/tdx.c +67 -0

+int tdx_detect(void)
+{
+	int ret;
+
+	mutex_lock(&tdx_module_lock);
+
+	switch (tdx_module_status) {
+	case TDX_MODULE_UNKNOWN:
+		ret = __tdx_detect();
+		break;
+	case TDX_MODULE_NONE:
+		ret = -ENODEV;
+		break;
+	case TDX_MODULE_LOADED:
+	case TDX_MODULE_INITIALIZED:
+		ret = 0;
+		break;
+	case TDX_MODULE_SHUTDOWN:
+		ret = -EFAULT;
+		break;
+	default:
+		WARN_ON(1);
+		ret = -EFAULT;
+	}
+
+	mutex_unlock(&tdx_module_lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tdx_detect);
+
+/**
+ * tdx_init - Initialize the TDX module
+ *
+ * Initialize the TDX module to make it ready to run TD guests.  This
+ * function should be called after tdx_detect() returns successful.
+ * Only call this function when all cpus are online and are in VMX
+ * operation.  CPU hotplug is temporarily disabled internally.
+ *
+ * This function can be called in parallel by multiple callers.
+ *
+ * Return:
+ *
+ * * -0:	The TDX module has been successfully initialized.
+ * * -ENODEV:	The TDX module is not loaded.
+ * * -EPERM:	The CPU which does SEAMCALL is not in VMX operation.
+ * * -EFAULT:	Other internal fatal errors.
+ */
+int tdx_init(void)
+{
+	int ret;
+
+	mutex_lock(&tdx_module_lock);
+
+	switch (tdx_module_status) {
+	case TDX_MODULE_NONE:
+		ret = -ENODEV;
+		break;
+	case TDX_MODULE_LOADED:
+		ret = __tdx_init();
+		break;
+	case TDX_MODULE_INITIALIZED:
+		ret = 0;
+		break;
+	default:
+		ret = -EFAULT;
+		break;
+	}

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 396

Quoted 5 lines

arch/x86/virt/vmx/tdx/tdx.c +5 -0

+	mutex_unlock(&tdx_module_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tdx_init);

Why does this need both a tdx_detect() and a tdx_init()? Shouldn't the interface from outside just be "get TDX up and running, please?"

Quoted 1 line · Dave Hansen

On 4/5/22 21:49, Kai Huang wrote:

Quoted 7 lines

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 222

Quoted 5 lines

arch/x86/virt/vmx/tdx/tdx.c +5 -0

+static bool tdx_keyid_sufficient(void)
+{
+	if (!cpumask_equal(&cpus_booted_once_mask,
+					cpu_present_mask))
+		return false;

I'd move this cpumask_equal() to a helper.

Sorry to double confirm, do you want something like:

static bool tdx_detected_on_all_cpus(void)
{
/*
* To detect any BIOS misconfiguration among cores, all logical
* cpus must have been brought up at least once. This is true
* unless 'maxcpus' kernel command line is used to limit the
* number of cpus to be brought up during boot time. However
* 'maxcpus' is basically an invalid operation mode due to the
* MCE broadcast problem, and it should not be used on a TDX
* capable machine. Just do paranoid check here and do not
* report SEAMRR as enabled in this case.
*/
return cpumask_equal(&cpus_booted_once_mask, cpu_present_mask);
}

static bool seamrr_enabled(void)

{
	if (!tdx_detected_on_all_cpus())
		return false;

	return __seamrr_enabled();
}

static bool tdx_keyid_sufficient()

{
	if (!tdx_detected_on_all_cpus())
		return false;

	...
}

Quoted 10 lines

Quoted 10 lines

arch/x86/virt/vmx/tdx/tdx.c +10 -0

+	/*
+	 * TDX requires at least two KeyIDs: one global KeyID to
+	 * protect the metadata of the TDX module and one or more
+	 * KeyIDs to run TD guests.
+	 */
+	return tdx_keyid_num >= 2;
+}
+
+static int __tdx_detect(void)
+{

Quoted 8 lines

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 238

Quoted 5 lines

arch/x86/virt/vmx/tdx/tdx.c +5 -0

+	/* The TDX module is not loaded if SEAMRR is disabled */
+	if (!seamrr_enabled()) {
+		pr_info("SEAMRR not enabled.\n");
+		goto no_tdx_module;
+	}

Why even bother with the SEAMRR stuff? It sounded like you can "ping" the module with SEAMCALL. Why not just use that directly?

SEAMCALL will cause #GP if SEAMRR is not enabled. We should check whether SEAMRR is enabled before making SEAMCALL.

Quoted 7 lines

Quoted 7 lines

arch/x86/virt/vmx/tdx/tdx.c +7 -0

+	/*
+	 * Also do not report the TDX module as loaded if there's
+	 * no enough TDX private KeyIDs to run any TD guests.
+	 */
+	if (!tdx_keyid_sufficient()) {
+		pr_info("Number of TDX private KeyIDs too small: %u.\n",
+				tdx_keyid_num);

Quoted 8 lines

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 254

Quoted 8 lines

arch/x86/virt/vmx/tdx/tdx.c +8 -0

+		goto no_tdx_module;
+	}
+
+	/* Return -ENODEV until the TDX module is detected */
+no_tdx_module:
+	tdx_module_status = TDX_MODULE_NONE;
+	return -ENODEV;
+}

Quoted 23 lines

Quoted 23 lines

arch/x86/virt/vmx/tdx/tdx.c +23 -0

+static int init_tdx_module(void)
+{
+	/*
+	 * Return -EFAULT until all steps of TDX module
+	 * initialization are done.
+	 */
+	return -EFAULT;
+}
+
+static void shutdown_tdx_module(void)
+{
+	/* TODO: Shut down the TDX module */
+	tdx_module_status = TDX_MODULE_SHUTDOWN;
+}
+
+static int __tdx_init(void)
+{
+	int ret;
+
+	/*
+	 * Logical-cpu scope initialization requires calling one SEAMCALL
+	 * on all logical cpus enabled by BIOS.  Shutting down the TDX
+	 * module also has such requirement.  Further more, configuring

Quoted 8 lines

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 284

Quoted 6 lines

arch/x86/virt/vmx/tdx/tdx.c +6 -0

+	 * the key of the global KeyID requires calling one SEAMCALL for
+	 * each package.  For simplicity, disable CPU hotplug in the whole
+	 * initialization process.
+	 *
+	 * It's perhaps better to check whether all BIOS-enabled cpus are
+	 * online before starting initializing, and return early if not.

But you did some of this cpumask checking above. Right?

Above check only guarantees SEAMRR/TDX KeyID has been detected on all presnet cpus. the 'present' cpumask doesn't equal to all BIOS-enabled CPUs.

Quoted 2 lines

Why does this need both a tdx_detect() and a tdx_init()? Shouldn't the interface from outside just be "get TDX up and running, please?"

We can have a single tdx_init(). However tdx_init() can be heavy, and having a separate non-heavy tdx_detect() may be useful if caller wants to separate "detecting the TDX module" and "initializing the TDX module", i.e. to do something in the middle.

However tdx_detect() basically only detects P-SEAMLDR. If we move P-SEAMLDR detection to tdx_init(), or we git rid of P-SEAMLDR completely, then we don't need tdx_detect() anymore. We can expose seamrr_enabled() and TDX KeyID variables or functions so caller can use them to see whether it should do TDX related staff and then call tdx_init().

Quoted 2 lines · Kai Huang

Quoted 1 line · Dave Hansen

On 4/5/22 21:49, Kai Huang wrote:

...

Quoted 10 lines

Quoted 7 lines

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 222

Quoted 5 lines

arch/x86/virt/vmx/tdx/tdx.c +5 -0

+static bool tdx_keyid_sufficient(void)
+{
+	if (!cpumask_equal(&cpus_booted_once_mask,
+					cpu_present_mask))
+		return false;

I'd move this cpumask_equal() to a helper.

Sorry to double confirm, do you want something like:

Quoted 14 lines

static bool tdx_detected_on_all_cpus(void)
{
/*
* To detect any BIOS misconfiguration among cores, all logical
* cpus must have been brought up at least once. This is true
* unless 'maxcpus' kernel command line is used to limit the
* number of cpus to be brought up during boot time. However
* 'maxcpus' is basically an invalid operation mode due to the
* MCE broadcast problem, and it should not be used on a TDX
* capable machine. Just do paranoid check here and do not
* report SEAMRR as enabled in this case.
*/
return cpumask_equal(&cpus_booted_once_mask, cpu_present_mask);
}

That's logically the right idea, but I hate the name since the actual test has nothing to do with TDX being detected. The comment is also rather verbose and rambling.

It should be named something like:

all_cpus_booted()

and with a comment like this:

/*
* To initialize TDX, the kernel needs to run some code on every
* present CPU. Detect cases where present CPUs have not been
* booted, like when maxcpus=N is used.
*/

Quoted 9 lines

static bool seamrr_enabled(void)

{
	if (!tdx_detected_on_all_cpus())
		return false;

	return __seamrr_enabled();
}

Quoted 7 lines

static bool tdx_keyid_sufficient()

{
	if (!tdx_detected_on_all_cpus())
		return false;

	...
}

Although, looking at those, it's *still* unclear why you need this. I assume it's because some later TDX SEAMCALL will fail if you get this wrong, and you want to be able to provide a better error message.

*BUT* this code doesn't actually provide halfway reasonable error messages. If someone uses maxcpus=99, then this code will report:

	pr_info("SEAMRR not enabled.\n");

right? That's bonkers.

Quoted 20 lines

Quoted 19 lines

Quoted 6 lines

arch/x86/virt/vmx/tdx/tdx.c +5 -0

+	/*
+	 * TDX requires at least two KeyIDs: one global KeyID to
+	 * protect the metadata of the TDX module and one or more
+	 * KeyIDs to run TD guests.
+	 */

arch/x86/virt/vmx/tdx/tdx.c void tdx_detect_cpu(struct cpuinfo_x86 *c) · line 238

Quoted 10 lines

arch/x86/virt/vmx/tdx/tdx.c +10 -0

+	return tdx_keyid_num >= 2;
+}
+
+static int __tdx_detect(void)
+{
+	/* The TDX module is not loaded if SEAMRR is disabled */
+	if (!seamrr_enabled()) {
+		pr_info("SEAMRR not enabled.\n");
+		goto no_tdx_module;
+	}

Why even bother with the SEAMRR stuff? It sounded like you can "ping" the module with SEAMCALL. Why not just use that directly?