Skip to contentSkip to conversation

TDX host kernel support

PATCH Superseded Kai Huang 4 participants 51 messages Mar 13, 2022 – Mar 29, 2022

Subsystems (4)

v2 of 15 Superseded Latest is v15 →

lore
Share & tools
lore

Kai Huang

author
#0 lore raw

v2 cover TDX host kernel support

Hi,

Intel Trusted Domain Extensions (TDX) protects guest VMs from malicious host and certain physical attacks. This series provides support for initializing TDX in the host kernel. KVM support for TDX is being developed separately[1].

This series is based on Kirill's TDX guest series[2]. The reason is host side SEAMCALL implementation can share TDCALL's implementation which is implemented in TDX guest series.

You can also find this series in below repo in github:

https://github.com/intel/tdx/tree/host-upstream

The code has been tested on couple of TDX-capable machines. I would consider it as ready for review (from overall design to detail implementations). It would be highly appreciated if anyone can help to review this series. For Intel folks, I would appreciate acks if the patches look good to you.

Thanks in advance.

Changelog history:

RFC (v1):

https://lore.kernel.org/all/e0ff030a49b252d91c789a89c303bb4206f85e3d.1646007267.git.kai.huang@intel.com/T/

- RFC (v1) -> v2:
- Rebased to Kirill's latest TDX guest code.
- Fixed two issues that are related to finding all RAM memory regions

    based on e820.

- Minor improvement on comments and commit messages.

== Background ==

Intel Trusted Domain Extensions (TDX) protects guest VMs from malicious host and certain physical attacks. To support TDX, a new CPU mode called Secure Arbitration Mode (SEAM) is added to Intel processors.

SEAM is an extension to the existing VMX architecture. It defines a new VMX root operation (SEAM VMX root) and a new VMX non-root operation (SEAM VMX non-root).

SEAM VMX root operation is designed to host a CPU-attested, software module called the 'TDX module' which implements functions to manage crypto protected VMs called Trust Domains (TD). SEAM VMX root is also designed to host a CPU-attested, software module called the 'Intel Persistent SEAMLDR (Intel P-SEAMLDR)' to load and update the TDX module.

Host kernel transits to either the P-SEAMLDR or the TDX module via a new SEAMCALL instruction. SEAMCALLs 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 modules.

TDX leverages Intel Multi-Key Total Memory Encryption (MKTME) to crypto protect TD guests. TDX reserves part of MKTME KeyID space as TDX private KeyIDs, which can only be used by software runs in SEAM. The physical address bits for encoding TDX private KeyID are treated as reserved bits when not in SEAM operation. The partitioning of MKTME KeyIDs and TDX private KeyIDs is configured by BIOS.

Before being able to manage TD guests, the TDX module must be loaded and properly initialized using SEAMCALLs defined by TDX architecture. This series assumes both the P-SEAMLDR and the TDX module are loaded by BIOS before the kernel boots.

There's no CPUID or MSR to detect either the P-SEAMLDR or the TDX module. Instead, detecting them can be done by using P-SEAMLDR's SEAMLDR.INFO SEAMCALL to detect P-SEAMLDR. The success of this SEAMCALL means the P-SEAMLDR is loaded. The P-SEAMLDR information returned by this SEAMCALL further tells whether TDX module is loaded.

The TDX module is initialized in multiple steps:

1) Global initialization;
2) Logical-CPU scope initialization;
3) Enumerate the TDX module capabilities;
4) Configure the TDX module about usable memory ranges and global KeyID information;
5) Package-scope configuration for the global KeyID;
6) Initialize TDX metadata for usable memory ranges based on 4).

Step 2) requires calling some SEAMCALL on all "BIOS-enabled" (in MADT table) logical cpus, otherwise step 4) will fail. Step 5) requires calling SEAMCALL on at least one cpu on all packages.

TDX module can also be shut down at any time during module's lifetime, by calling SEAMCALL on all "BIOS-enabled" logical cpus.

== Design Considerations ==

1. Lazy TDX module initialization on-demand by caller

None of the steps in the TDX module initialization process must be done during kernel boot. This series doesn't initialize TDX at boot time, but instead, provides two functions to allow caller to detect and initialize TDX on demand:

        if (tdx_detect())
                goto no_tdx;
        if (tdx_init())
                goto no_tdx;

This approach has below pros:

1) Initializing the TDX module requires to reserve ~1/256th system RAM as metadata. Enabling TDX on demand allows only to consume this memory when TDX is truly needed (i.e. when KVM wants to create TD guests).

2) Both detecting and initializing the TDX module require calling SEAMCALL. However, SEAMCALL requires CPU being already in VMX operation (VMXON has been done). So far, KVM is the only user of TDX, and it already handles VMXON/VMXOFF. Therefore, letting KVM to initialize TDX on-demand avoids handling VMXON/VMXOFF (which is not that trivial) in core-kernel. Also, in long term, likely a reference based VMXON/VMXOFF approach is needed since more kernel components will need to handle VMXON/VMXONFF.

3) It is more flexible to support "TDX module runtime update" (not in this series). After updating to the new module at runtime, kernel needs to go through the initialization process again. For the new module, it's possible the metadata allocated for the old module cannot be reused for the new module, and needs to be re-allocated again.

2. Kernel policy on TDX memory

Host kernel is responsible for choosing which memory regions can be used as TDX memory, and configuring those memory regions to the TDX module by using an array of "TD Memory Regions" (TDMR), which is a data structure defined by TDX architecture.

The first generation of TDX essentially guarantees that all system RAM memory regions (excluding the memory below 1MB) can be used as TDX memory. To avoid having to modify the page allocator to distinguish TDX and non-TDX allocation, this series chooses to use all system RAM as TDX memory.

E820 table is used to find all system RAM entries. Following e820__memblock_setup(), both E820_TYPE_RAM and E820_TYPE_RESERVED_KERN types are treated as TDX memory, and contiguous ranges in the same NUMA node are merged together (similar to memblock_add()) before trimming the non-page-aligned part.

X86 Legacy PMEMs (E820_TYPE_PRAM) also unconditionally treated as TDX memory as underneath they are RAM and can be potentially used as TD guest memory.

Memblock is not used to find all RAM regions as: 1) it is gone after kernel boots; 2) it doesn't have legacy PMEM.

3. Memory hotplug

The first generation of TDX architecturally doesn't support memory hotplug. And the first generation of TDX-capable platforms don't support physical memory hotplug. Since it physically cannot happen, this series doesn't add any check in ACPI memory hotplug code path to disable it.

A special case of memory hotplug is adding NVDIMM as system RAM using kmem driver. However the first generation of TDX-capable platforms cannot enable TDX and NVDIMM simultaneously, so in practice this cannot happen either.

Another case is admin can use 'memmap' kernel command line to create legacy PMEMs and use them as TD guest memory, or theoretically, can use kmem driver to add them as system RAM. To avoid having to change memory hotplug code to prevent this from happening, this series always include legacy PMEMs when constructing TDMRs so they are also TDX memory.

4. CPU hotplug

The first generation of TDX architecturally doesn't support ACPI CPU hotplug. All logical cpus are enabled by BIOS in MADT table. Also, the first generation of TDX-capable platforms don't support ACPI CPU hotplug either. Since this physically cannot happen, this series doesn't add any check in ACPI CPU hotplug code path to disable it.

Also, only TDX module initialization requires all BIOS-enabled cpus are online. After the initialization, any logical cpu can be brought down and brought up to online again later. Therefore this series doesn't change logical CPU hotplug either.

5. TDX interaction with kexec()

If TDX is ever enabled and/or used to run any TD guests, the cachelines of TDX private memory, including PAMTs, used by TDX module need to be flushed before transiting to the new kernel otherwise they may silently corrupt the new kernel. Similar to SME, this series flushes cache in stop_this_cpu().

The TDX module can be initialized only once during its lifetime. The first generation of TDX doesn't have interface to reset TDX module to uninitialized state so it can be initialized again.

This implies:

- If the old kernel fails to initialize TDX, the new kernel cannot use TDX too unless the new kernel fixes the bug which leads to initialization failure in the old kernel and can resume from where the old kernel stops. This requires certain coordination between the two kernels.

- If the old kernel has initialized TDX successfully, the new kernel may be able to use TDX if the two kernels have the exactly same configurations on the TDX module. It further requires the new kernel to reserve the TDX metadata pages (allocated by the old kernel) in its page allocator. It also requires coordination between the two kernels. Furthermore, if kexec() is done when there are active TD guests running, the new kernel cannot use TDX because it's extremely hard for the old kernel to pass all TDX private pages to the new kernel.

Given that, this series doesn't support TDX after kexec() (except the old kernel doesn't attempt to initialize TDX at all).

And this series doesn't shut down TDX module but leaves it open during kexec(). It is because shutting down TDX module requires CPU being in VMX operation but there's no guarantee of this during kexec(). Leaving the TDX module open is not the best case, but it is OK since the new kernel won't be able to use TDX anyway (therefore TDX module won't run at all).

[1] https://lore.kernel.org/lkml/772b20e270b3451aea9714260f2c40ddcc4afe80.1646422845.git.isaku.yamahata@intel.com/T/
[2] https://github.com/intel/tdx/tree/guest-upstream

Kai Huang (21):
x86/virt/tdx: Detect SEAM
x86/virt/tdx: Detect TDX private KeyIDs
x86/virt/tdx: Implement the SEAMCALL base function x86/virt/tdx: Add skeleton for detecting and initializing TDX on
demand
x86/virt/tdx: Detect P-SEAMLDR and TDX module x86/virt/tdx: Shut down TDX module in case of error x86/virt/tdx: Do TDX module global initialization x86/virt/tdx: Do logical-cpu scope TDX module initialization x86/virt/tdx: Get information about TDX module and convertible memory x86/virt/tdx: Add placeholder to coveret all system RAM as TDX memory x86/virt/tdx: Choose to use all system RAM as TDX memory x86/virt/tdx: Create TDMRs to cover all system RAM x86/virt/tdx: Allocate and set up PAMTs for TDMRs x86/virt/tdx: Set up reserved areas for all TDMRs x86/virt/tdx: Reserve TDX module global KeyID x86/virt/tdx: Configure TDX module with TDMRs and global KeyID x86/virt/tdx: Configure global KeyID on all packages x86/virt/tdx: Initialize all TDMRs
x86: Flush cache of TDX private memory during kexec() x86/virt/tdx: Add kernel command line to opt-in TDX host support Documentation/x86: Add documentation for TDX host support

 .../admin-guide/kernel-parameters.txt         |    6 +
 Documentation/x86/index.rst                   |    1 +
 Documentation/x86/tdx_host.rst                |  300 +++
 arch/x86/Kconfig                              |   14 +
 arch/x86/Makefile                             |    2 +
 arch/x86/include/asm/tdx.h                    |   15 +
 arch/x86/kernel/cpu/intel.c                   |    3 +
 arch/x86/kernel/process.c                     |   15 +-
 arch/x86/virt/Makefile                        |    2 +
 arch/x86/virt/vmx/Makefile                    |    2 +
 arch/x86/virt/vmx/seamcall.S                  |   52 +
 arch/x86/virt/vmx/tdx.c                       | 1711 +++++++++++++++++
 arch/x86/virt/vmx/tdx.h                       |  137 ++
 13 files changed, 2259 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/x86/tdx_host.rst
 create mode 100644 arch/x86/virt/Makefile
 create mode 100644 arch/x86/virt/vmx/Makefile
 create mode 100644 arch/x86/virt/vmx/seamcall.S
 create mode 100644 arch/x86/virt/vmx/tdx.c
 create mode 100644 arch/x86/virt/vmx/tdx.h
Signature · 1 line
2.35.1
author
#1 lore raw

PATCH v2 1/21 x86/virt/tdx: Detect SEAM

Intel Trusted Domain Extensions (TDX) protects guest VMs from malicious host and certain physical attacks. To support TDX, a new CPU mode called Secure Arbitration Mode (SEAM) is added to Intel processors.

SEAM is an extension to the VMX architecture to define a new VMX root operation (SEAM VMX root) and a new VMX non-root operation (SEAM VMX non-root). SEAM VMX root operation is designed to host a CPU-attested software module called the 'TDX module' which implements functions to manage crypto protected VMs called Trust Domains (TD). It is also designed to additionally host a CPU-attested software module called the 'Intel Persistent SEAMLDR (Intel P-SEAMLDR)' to load and update the TDX module.

Software modules in SEAM VMX root run in a memory region defined by the SEAM range register (SEAMRR). So the first thing of detecting Intel TDX is to detect the validity of SEAMRR.

The presence of SEAMRR is reported via a new SEAMRR bit (15) of the IA32_MTRRCAP MSR. The SEAMRR range registers consist of a pair of MSRs:

IA32_SEAMRR_PHYS_BASE and IA32_SEAMRR_PHYS_MASK

BIOS is expected to configure SEAMRR with the same value across all cores. In case of BIOS misconfiguration, detect and compare SEAMRR on all cpus.

To start to support TDX, create a new arch/x86/virt/vmx/ for non-KVM host kernel virtualization support for Intel platforms, and create a new tdx.c under it for TDX host kernel support.

TDX also leverages Intel Multi-Key Total Memory Encryption (MKTME) to crypto protect TD guests. Part of MKTME KeyIDs are reserved as "TDX private KeyID" or "TDX KeyIDs" for short. Similar to detecting SEAMRR, detecting TDX private KeyIDs also needs to be done on all cpus to detect any BIOS misconfiguration.

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.

Also add a new Kconfig option CONFIG_INTEL_TDX_HOST to opt-in TDX host kernel support (to distinguish with TDX guest kernel support).

---
 arch/x86/Kconfig            |  12 +++++
 arch/x86/Makefile           |   2 +
 arch/x86/include/asm/tdx.h  |   9 ++++
 arch/x86/kernel/cpu/intel.c |   3 ++
 arch/x86/virt/Makefile      |   2 +
 arch/x86/virt/vmx/Makefile  |   2 +
 arch/x86/virt/vmx/tdx.c     | 102 ++++++++++++++++++++++++++++++++++++
 7 files changed, 132 insertions(+)
 create mode 100644 arch/x86/virt/Makefile
 create mode 100644 arch/x86/virt/vmx/Makefile
 create mode 100644 arch/x86/virt/vmx/tdx.c
arch/x86/Kconfig +12 -0
index fb2706f7f04a..f4c5481cca46 100644
@@ -1956,6 +1956,18 @@ config X86_SGX
 
 	  If unsure, say N.
 
+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
+	  host and certain physical attacks.  This option enables necessary TDX
+	  support in host kernel to run protected VMs.
+
+	  If unsure, say N.
+
 config EFI
 	bool "EFI runtime service support"
 	depends on ACPI
arch/x86/Makefile +2 -0
index e84cdd409b64..83a6a5a2e244 100644
@@ -238,6 +238,8 @@ head-y += arch/x86/kernel/platform-quirks.o
 
 libs-y  += arch/x86/lib/
 
+core-y += arch/x86/virt/
+
 # drivers-y are linked after core-y
 drivers-$(CONFIG_MATH_EMULATION) += arch/x86/math-emu/
 drivers-$(CONFIG_PCI)            += arch/x86/pci/
arch/x86/include/asm/tdx.h +9 -0
index 6a97d42b0de9..605d87ab580e 100644
@@ -11,6 +11,8 @@
 
 #ifndef __ASSEMBLY__
 
+#include <asm/processor.h>
+
 /*
  * Used to gather the output registers values of the TDCALL and SEAMCALL
  * instructions when requesting services from the TDX module.
@@ -78,5 +80,12 @@ static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1,
 	return -ENODEV;
 }
 #endif /* CONFIG_INTEL_TDX_GUEST && CONFIG_KVM_GUEST */
+
+#ifdef CONFIG_INTEL_TDX_HOST
+void tdx_detect_cpu(struct cpuinfo_x86 *c);
+#else
+static inline void tdx_detect_cpu(struct cpuinfo_x86 *c) { }
+#endif /* CONFIG_INTEL_TDX_HOST */
+
 #endif /* !__ASSEMBLY__ */
 #endif /* _ASM_X86_TDX_H */
arch/x86/kernel/cpu/intel.c +3 -0
index 8321c43554a1..b142a640fb8e 100644
@@ -26,6 +26,7 @@
 #include <asm/resctrl.h>
 #include <asm/numa.h>
 #include <asm/thermal.h>
+#include <asm/tdx.h>
 
 #ifdef CONFIG_X86_64
 #include <linux/topology.h>
@@ -715,6 +716,8 @@ static void init_intel(struct cpuinfo_x86 *c)
 	if (cpu_has(c, X86_FEATURE_TME))
 		detect_tme(c);
 
+	tdx_detect_cpu(c);
+
 	init_intel_misc_features(c);
 
 	if (tsx_ctrl_state == TSX_CTRL_ENABLE)
arch/x86/virt/Makefile new file +2 -0
new file mode 100644
index 000000000000..1e36502cd738
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-y	+= vmx/
arch/x86/virt/vmx/Makefile new file +2 -0
new file mode 100644
index 000000000000..1bd688684716
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_INTEL_TDX_HOST)	+= tdx.o
arch/x86/virt/vmx/tdx.c new file +102 -0

102 lines not on this page · raw message

Signature · 1 line
2.35.1
#2 lore raw

RE: [PATCH v2 01/21] x86/virt/tdx: Detect SEAM

arch/x86/kernel/cpu/intel.c static void init_intel(struct cpuinfo_x86 *c) · line 720

Quoted 9 lines

From: Kai Huang <[email protected]>
Sent: Sunday, March 13, 2022 6:50 PM

arch/x86/kernel/cpu/intel.c +2 -0
@@ -715,6 +716,8 @@ static void init_intel(struct cpuinfo_x86 *c)
 	if (cpu_has(c, X86_FEATURE_TME))
 		detect_tme(c);
 
+	tdx_detect_cpu(c);
+

TDX is not reported as a x86 feature. and the majority of detection and initialization have been conducted on demand in this series (as explained in patch04). Why is SEAM (and latter keyid) so different to be detected at early boot phase?

Thanks
Kevin

author
#3 lore raw

Re: [PATCH v2 01/21] x86/virt/tdx: Detect SEAM

Quoted 15 lines · Kevin

arch/x86/kernel/cpu/intel.c static void init_intel(struct cpuinfo_x86 *c) · line 720

Quoted 9 lines

From: Kai Huang <[email protected]>
Sent: Sunday, March 13, 2022 6:50 PM

arch/x86/kernel/cpu/intel.c +2 -0
@@ -715,6 +716,8 @@ static void init_intel(struct cpuinfo_x86 *c)
       if (cpu_has(c, X86_FEATURE_TME))
               detect_tme(c);
 
+     tdx_detect_cpu(c);
+

TDX is not reported as a x86 feature. and the majority of detection and initialization have been conducted on demand in this series (as explained in patch04). Why is SEAM (and latter keyid) so different to be detected at early boot phase?

Quoted 2 lines

Thanks
Kevin

Hi Kevin,

Sorry for late reply. I was out last week.

SEAMRR and TDX KeyIDs are configured by BIOS and they are static during machine's runtime. On the other hand, TDX module can be updated and reinitialized at runtime (not supported in this series but will be supported in the future). Theoretically, even P-SEAMLDR can be updated at runtime (although I think unlikely to be supported in Linux). Therefore I think detecting SEAMRR and TDX KeyIDs at boot fits better.

It is also more flexible from some other perspectives I think: 1) There was a request to add X86_FEATURE_SEAM bit and expose it to /proc/cpuinfo. I didn't add it because I didn't think the use case was solid. But in case someone has some use case in the future we can add it, and detecting SEAMRR during boot fits this more. 2) There was a request to expose TDX KeyID info via sysfs so userspace can know how many TDs can be created. It's not done in this series but it will be done at some time in the future. Detecting KeyIDs at boot allows this info being able to be exposed via sysfs at early stage, providing more flexibility to userspace.

At last, currently in this series the patch to handle kexec checks whether SEAMRR and TDX KeyIDs are enabled and then flush cache (of course this is open to discussion). Detecting them at boot fits better I think.

Signature · 2 lines
Thanks,
-Kai
#4 lore raw

RE: [PATCH v2 01/21] x86/virt/tdx: Detect SEAM

Quoted 26 lines

From: Huang, Kai <[email protected]>
Sent: Monday, March 28, 2022 11:55 AM

Quoted 15 lines · Kevin

arch/x86/kernel/cpu/intel.c static void init_intel(struct cpuinfo_x86 *c) · line 720

Quoted 9 lines

From: Kai Huang <[email protected]>
Sent: Sunday, March 13, 2022 6:50 PM

arch/x86/kernel/cpu/intel.c +2 -0
@@ -715,6 +716,8 @@ static void init_intel(struct cpuinfo_x86 *c)
       if (cpu_has(c, X86_FEATURE_TME))
               detect_tme(c);
 
+     tdx_detect_cpu(c);
+

TDX is not reported as a x86 feature. and the majority of detection and initialization have been conducted on demand in this series (as explained in patch04). Why is SEAM (and latter keyid) so different to be detected at early boot phase?

Quoted 2 lines

Thanks
Kevin

Hi Kevin,

Sorry for late reply. I was out last week.

Quoted 8 lines

SEAMRR and TDX KeyIDs are configured by BIOS and they are static during machine's runtime. On the other hand, TDX module can be updated and reinitialized at runtime (not supported in this series but will be supported in the future). Theoretically, even P-SEAMLDR can be updated at runtime (although
I think unlikely to be supported in Linux). Therefore I think detecting SEAMRR
and TDX KeyIDs at boot fits better.

If those info are static it's perfectly fine to detect them until they are required... and following are not solid cases (e.g. just exposing SEAM alone doesn't tell the availability of TDX) but let's also hear the opinions from others.

Quoted 25 lines

It is also more flexible from some other perspectives I think: 1) There was a request to add X86_FEATURE_SEAM bit and expose it to /proc/cpuinfo. I didn't add
it because I didn't think the use case was solid. But in case someone has some
use case in the future we can add it, and detecting SEAMRR during boot fits this
more. 2) There was a request to expose TDX KeyID info via sysfs so userspace can
know how many TDs can be created. It's not done in this series but it will be done at some time in the future. Detecting KeyIDs at boot allows this info being
able to be exposed via sysfs at early stage, providing more flexibility to userspace.

At last, currently in this series the patch to handle kexec checks whether SEAMRR and TDX KeyIDs are enabled and then flush cache (of course this is open
to discussion). Detecting them at boot fits better I think.

--
Thanks,
-Kai

#5 lore raw

Re: [PATCH v2 01/21] x86/virt/tdx: Detect SEAM

Quoted 12 lines · Kevin
Quoted 11 lines

From: Huang, Kai <[email protected]>
Sent: Monday, March 28, 2022 11:55 AM

SEAMRR and TDX KeyIDs are configured by BIOS and they are static during machine's runtime. On the other hand, TDX module can be updated and reinitialized at runtime (not supported in this series but will be supported in the future). Theoretically, even P-SEAMLDR can be updated at runtime (although
I think unlikely to be supported in Linux). Therefore I think detecting SEAMRR
and TDX KeyIDs at boot fits better.

Quoted 4 lines

If those info are static it's perfectly fine to detect them until they are required... and following are not solid cases (e.g. just exposing SEAM alone doesn't tell the availability of TDX) but let's also hear the opinions from others.

One use case is cloud use case. If TDX module is initialized dynamically at runtime, cloud management system wants to know if the physical machine is capable of TDX in addition to if TDX module is initialized. Also how many TDs can be run on the machine even when TDX module is not initialized yet. The management system will schedule TDs based on those information.

Signature · 1 line
Isaku Yamahata <[email protected]>
author
#6 lore raw

Re: [PATCH v2 01/21] x86/virt/tdx: Detect SEAM

Quoted 20 lines · Isaku Yamahata
Quoted 12 lines · Kevin
Quoted 11 lines

From: Huang, Kai <[email protected]>
Sent: Monday, March 28, 2022 11:55 AM

SEAMRR and TDX KeyIDs are configured by BIOS and they are static during machine's runtime. On the other hand, TDX module can be updated and reinitialized at runtime (not supported in this series but will be supported in the future). Theoretically, even P-SEAMLDR can be updated at runtime (although
I think unlikely to be supported in Linux). Therefore I think detecting SEAMRR
and TDX KeyIDs at boot fits better.

Quoted 4 lines

If those info are static it's perfectly fine to detect them until they are required... and following are not solid cases (e.g. just exposing SEAM alone doesn't tell the availability of TDX) but let's also hear the opinions from others.

Quoted 5 lines

One use case is cloud use case. If TDX module is initialized dynamically at runtime, cloud management system wants to know if the physical machine is capable of TDX in addition to if TDX module is initialized. Also how many TDs can be run on the machine even when TDX module is not initialized yet. The management system will schedule TDs based on those information.

Thanks Isaku. I'll keep current way for now.

Signature · 2 lines
Thanks,
-Kai
author
#7 lore raw

PATCH v2 2/21 x86/virt/tdx: Detect TDX private KeyIDs

In reply to Kai Huang

Pre-TDX Intel hardware has support for a memory encryption architecture called MKTME. The memory encryption hardware underpinning MKTME is also used for Intel TDX. TDX ends up "stealing" some of the physical address space from the MKTME architecture for crypto protection to VMs.

A new MSR (IA32_MKTME_KEYID_PARTITIONING) helps to enumerate how MKTME- enumerated "KeyID" space is distributed between TDX and legacy MKTME. KeyIDs reserved for TDX are called 'TDX private KeyIDs' or 'TDX KeyIDs' for short.

The new MSR is per package and BIOS is responsible for partitioning MKTME KeyIDs and TDX KeyIDs consistently among all packages.

Detect TDX private KeyIDs as a preparation to initialize TDX. Similar to detecting SEAMRR, detect on all cpus to detect any potential BIOS misconfiguration among packages.

---
 arch/x86/virt/vmx/tdx.c | 72 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)
arch/x86/virt/vmx/tdx.c +72 -0
index 03f35c75f439..ba2210001ea8 100644
@@ -29,9 +29,28 @@
 #define SEAMRR_ENABLED_BITS	\
 	(SEAMRR_PHYS_MASK_ENABLED | SEAMRR_PHYS_MASK_LOCKED)
 
+/*
+ * Intel Trusted Domain CPU Architecture Extension spec:
+ *
+ * IA32_MKTME_KEYID_PARTIONING:
+ *
+ *   Bit [31:0]: number of MKTME KeyIDs.
+ *   Bit [63:32]: number of TDX private KeyIDs.
+ *
+ * TDX private KeyIDs start after the last MKTME KeyID.
+ */
+#define MSR_IA32_MKTME_KEYID_PARTITIONING	0x00000087
+
+#define TDX_KEYID_START(_keyid_part)	\
+		((u32)(((_keyid_part) & 0xffffffffull) + 1))
+#define TDX_KEYID_NUM(_keyid_part)	((u32)((_keyid_part) >> 32))
+
 /* 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 bool __seamrr_enabled(void)
 {
 	return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;
@@ -96,7 +115,60 @@ static void detect_seam(struct cpuinfo_x86 *c)
 		detect_seam_ap(c);
 }
 
+static void detect_tdx_keyids_bsp(struct cpuinfo_x86 *c)
+{
+	u64 keyid_part;
+
+	/* TDX is built on MKTME, which is based on TME */
+	if (!boot_cpu_has(X86_FEATURE_TME))
+		return;
+
+	if (rdmsrl_safe(MSR_IA32_MKTME_KEYID_PARTITIONING, &keyid_part))
+		return;
+
+	/* If MSR value is 0, TDX is not enabled by BIOS. */
+	if (!keyid_part)
+		return;
+
+	tdx_keyid_num = TDX_KEYID_NUM(keyid_part);
+	if (!tdx_keyid_num)
+		return;
+
+	tdx_keyid_start = TDX_KEYID_START(keyid_part);
+}
+
+static void detect_tdx_keyids_ap(struct cpuinfo_x86 *c)
+{
+	u64 keyid_part;
+
+	/*
+	 * Don't bother to detect this AP if TDX KeyIDs are
+	 * not detected or cleared after earlier detections.
+	 */
+	if (!tdx_keyid_num)
+		return;
+
+	rdmsrl(MSR_IA32_MKTME_KEYID_PARTITIONING, keyid_part);
+
+	if ((tdx_keyid_start == TDX_KEYID_START(keyid_part)) &&
+			(tdx_keyid_num == TDX_KEYID_NUM(keyid_part)))
+		return;
+
+	pr_err("Inconsistent TDX KeyID configuration among packages by BIOS\n");
+	tdx_keyid_start = 0;
+	tdx_keyid_num = 0;
+}
+
+static void detect_tdx_keyids(struct cpuinfo_x86 *c)
+{
+	if (c == &boot_cpu_data)
+		detect_tdx_keyids_bsp(c);
+	else
+		detect_tdx_keyids_ap(c);
+}
+
 void tdx_detect_cpu(struct cpuinfo_x86 *c)
 {
 	detect_seam(c);
+	detect_tdx_keyids(c);
 }
Signature · 1 line
2.35.1
author
#8 lore raw

PATCH v2 3/21 x86/virt/tdx: Implement the SEAMCALL base function

In reply to 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 isolate 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.

Host kernel transits to either the P-SEAMLDR or the TDX module via the new SEAMCALL instruction. SEAMCALLs 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.

SEAMCALLs use an ABI different from the x86-64 system-v ABI. Instead, they share the same ABI with the TDCALL. %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 SEAMCALLs.

Implement a C function __seamcall() to do SEAMCALL using the assembly macro used by __tdx_module_call() (the implementation of TDCALL). 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/Makefile   |  2 +-
 arch/x86/virt/vmx/seamcall.S | 52 ++++++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx.h      | 11 ++++++++
 3 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/virt/vmx/seamcall.S
 create mode 100644 arch/x86/virt/vmx/tdx.h
arch/x86/virt/vmx/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/seamcall.S new file +52 -0

52 lines not on this page · raw message

arch/x86/virt/vmx/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
Signature · 1 line
2.35.1
#9 lore raw

RE: [PATCH v2 03/21] x86/virt/tdx: Implement the SEAMCALL base function

Quoted 9 lines

From: Kai Huang <[email protected]>
Sent: Sunday, March 13, 2022 6:50 PM

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 isolate from legacy VMX root and VMX
non-root mode.

s/isolate/are isolated/

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

Host kernel transits to either the P-SEAMLDR or the TDX module via the new SEAMCALL instruction. SEAMCALLs 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

"SEAMCALLs are ... functions ... around the new SEAMCALL instruction"

This is confusing. Probably just:

"SEAMCALL functions are defined and handled by the P-SEAMLDR and the TDX module"

Quoted 15 lines

host kernel to the SEAM software.

SEAMCALLs use an ABI different from the x86-64 system-v ABI. Instead, they share the same ABI with the TDCALL. %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 SEAMCALLs.

Implement a C function __seamcall() to do SEAMCALL using the assembly macro used by __tdx_module_call() (the implementation of TDCALL). 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.

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

SEAMCALL is TDX specific, is it? If yes, there is no need to have both TDX and SEAMCALL in one macro, i.e. above can be SEAMCALL_VMFAILINVALID.

Thanks
Kevin

author
#10 lore raw

Re: [PATCH v2 03/21] x86/virt/tdx: Implement the SEAMCALL base function

Quoted 11 lines · Kevin
Quoted 9 lines

From: Kai Huang <[email protected]>
Sent: Sunday, March 13, 2022 6:50 PM

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 isolate from legacy VMX root and VMX
non-root mode.

s/isolate/are isolated/

OK thanks.

Quoted 8 lines
Quoted 8 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 8 lines
Quoted 4 lines

Host kernel transits to either the P-SEAMLDR or the TDX module via the new SEAMCALL instruction. SEAMCALLs 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

"SEAMCALLs are ... functions ... around the new SEAMCALL instruction"

This is confusing. Probably just:

May I ask why is it confusing?

Quoted 19 lines

"SEAMCALL functions are defined and handled by the P-SEAMLDR and the TDX module"

Quoted 15 lines

host kernel to the SEAM software.

SEAMCALLs use an ABI different from the x86-64 system-v ABI. Instead, they share the same ABI with the TDCALL. %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 SEAMCALLs.

Implement a C function __seamcall() to do SEAMCALL using the assembly macro used by __tdx_module_call() (the implementation of TDCALL). 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.

Quoted 8 lines
Quoted 5 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.

SEAMCALL is TDX specific, is it? If yes, there is no need to have both TDX and SEAMCALL in one macro, i.e. above can be SEAMCALL_VMFAILINVALID.

This is defined in TDX guest series. I just use it.

https://lore.kernel.org/lkml/[email protected]/T/#md0b1aa563bd003ab625de159612a0d07e3ded7cb

Signature · 2 lines
Thanks,
-Kai
#11 lore raw

RE: [PATCH v2 03/21] x86/virt/tdx: Implement the SEAMCALL base function

Quoted 17 lines

From: Huang, Kai <[email protected]>
Sent: Monday, March 28, 2022 9:42 AM

Quoted 4 lines
Quoted 4 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-

Quoted 2 lines
Quoted 2 lines

root.
SEAM VMX root is also used to host another CPU-attested software

module

Quoted 5 lines
Quoted 5 lines

(called the 'P-SEAMLDR') to load and update the TDX module.

Host kernel transits to either the P-SEAMLDR or the TDX module via the new SEAMCALL instruction. SEAMCALLs are host-side interface functions defined by the P-SEAMLDR and the TDX module around the new

Quoted 8 lines

SEAMCALL

Quoted 5 lines
Quoted 1 line

instruction. They are similar to a hypercall, except they are made by

"SEAMCALLs are ... functions ... around the new SEAMCALL instruction"

This is confusing. Probably just:

May I ask why is it confusing?

SEAMCALL is an instruction. One of its arguments carries the function number.

author
#12 lore raw

Re: [PATCH v2 03/21] x86/virt/tdx: Implement the SEAMCALL base function

Quoted 20 lines · Kevin
Quoted 20 lines

From: Huang, Kai <[email protected]>
Sent: Monday, March 28, 2022 9:42 AM

Quoted 4 lines
Quoted 4 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-

Quoted 2 lines
Quoted 2 lines

root.
SEAM VMX root is also used to host another CPU-attested software

module

Quoted 5 lines
Quoted 5 lines

(called the 'P-SEAMLDR') to load and update the TDX module.

Host kernel transits to either the P-SEAMLDR or the TDX module via the new SEAMCALL instruction. SEAMCALLs are host-side interface functions defined by the P-SEAMLDR and the TDX module around the new

SEAMCALL

Quoted 2 lines
Quoted 1 line

instruction. They are similar to a hypercall, except they are made by

Quoted 9 lines
Quoted 5 lines
Quoted 3 lines

"SEAMCALLs are ... functions ... around the new SEAMCALL instruction"

This is confusing. Probably just:

May I ask why is it confusing?

SEAMCALL is an instruction. One of its arguments carries the function number.

To confirm, are you saying the word "SEAMCALLs" is confusing, and we should use "SEAMCALL leaf functions" instead?

Signature · 2 lines
Thanks,
-Kai
author
#13 lore raw

PATCH v2 4/21 x86/virt/tdx: Add skeleton for detecting and initializing TDX on demand

In reply to 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 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.

SEAMCALLs used in the above steps (including shutting down the TDX module) require SEAMRR being enabled and CPU being already in VMX operation (VMXON has been done), otherwise it generates #UD. So far only KVM handles entering/leaving VMX operation (VMXON/VMXOFF). Handling of VMXON/VMXOFF could be added to this implementation, for instance, right around the SEAMCALL. But in long term, more kernel components are likely to use VMXON/VMXOFF, so a reference based approach to enter/leave VMX operation may be needed. KVM so far is the only user of TDX, therefore this implementation chooses to not handle VMXON/VMXOFF in tdx_detect() and tdx_init() but requires caller to guarantee that.

---
 arch/x86/include/asm/tdx.h |   4 +
 arch/x86/virt/vmx/tdx.c    | 222 +++++++++++++++++++++++++++++++++++++
 2 files changed, 226 insertions(+)
arch/x86/include/asm/tdx.h +4 -0
index 605d87ab580e..b526d41c4bbf 100644
@@ -83,8 +83,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.c +222 -0

243 lines not on this page · raw message

Signature · 1 line
2.35.1
#14 lore raw

RE: [PATCH v2 04/21] x86/virt/tdx: Add skeleton for detecting and initializing TDX on demand

Quoted 8 lines

From: Kai Huang <[email protected]>
Sent: Sunday, March 13, 2022 6:50 PM

arch/x86/virt/vmx/tdx.c +6 -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

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

Quoted 10 lines
arch/x86/virt/vmx/tdx.c +10 -0
+	 * 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 WARN()
+	 * if not the case.
+	 */
+	if (WARN_ON_ONCE(!cpumask_equal(&cpus_booted_once_mask,
+					cpu_present_mask)))
+		return false;
+

cpu_present_mask doesn't always represent BIOS-enabled CPUs as it can be further restricted by 'nr_cpus' and 'possible_cpus'. From this angle above check doesn't capture all misconfigured boot options which is incompatible with TDX. Then is such partial check still useful or better to just document those restrictions and let TDX module capture any violation later as what you explained in __init_tdx()?

Thanks
Kevin

author
#15 lore raw

Re: [PATCH v2 04/21] x86/virt/tdx: Add skeleton for detecting and initializing TDX on demand

Quoted 26 lines · Kevin
Quoted 8 lines

From: Kai Huang <[email protected]>
Sent: Sunday, March 13, 2022 6:50 PM

arch/x86/virt/vmx/tdx.c +6 -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

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

Quoted 10 lines
arch/x86/virt/vmx/tdx.c +10 -0
+      * 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 WARN()
+      * if not the case.
+      */
+     if (WARN_ON_ONCE(!cpumask_equal(&cpus_booted_once_mask,
+                                     cpu_present_mask)))
+             return false;
+

cpu_present_mask doesn't always represent BIOS-enabled CPUs as it can be further restricted by 'nr_cpus' and 'possible_cpus'. From this angle above check doesn't capture all misconfigured boot options which is incompatible with TDX. Then is such partial check still useful or better to just document those restrictions and let TDX module capture any violation later as what you explained in __init_tdx()?

Quoted 2 lines

Thanks
Kevin

The purpose of checking cpus_booted_once_mask aganist cpu_present_mask is NOT to check whether all BIOS-enabled CPUs are brought up at least once. Instead, the purpose is to check whether all cpus that kernel can use are brought up at least once (TDX-capable machine doesn't support ACPI CPU hotplug and all cpus are marked as enabled in MADT table, therefore cpu_present_mask is used instead of cpu_possible_mask). This is used to make sure SEAMRR has been detected on all cpus that kernel can use.

Checking whether "all BIOS-enabled cpus are up" is not done here (neither in this series as we discussed it seems there's no appropriate variable to represent it). And we just let TDH.SYS.CONFIG to fail if TDH.SYS.LP.INIT is not done on all BIOS-enabled CPUs.

Signature · 2 lines
Thanks,
-Kai
#16 lore raw

RE: [PATCH v2 04/21] x86/virt/tdx: Add skeleton for detecting and initializing TDX on demand

Quoted 47 lines

From: Huang, Kai <[email protected]>
Sent: Monday, March 28, 2022 9:58 AM

Quoted 26 lines · Kevin
Quoted 8 lines

From: Kai Huang <[email protected]>
Sent: Sunday, March 13, 2022 6:50 PM

arch/x86/virt/vmx/tdx.c +6 -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

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

Quoted 10 lines
arch/x86/virt/vmx/tdx.c +10 -0
+      * 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 WARN()
+      * if not the case.
+      */
+     if (WARN_ON_ONCE(!cpumask_equal(&cpus_booted_once_mask,
+                                     cpu_present_mask)))
+             return false;
+

cpu_present_mask doesn't always represent BIOS-enabled CPUs as it can be further restricted by 'nr_cpus' and 'possible_cpus'. From this angle above check doesn't capture all misconfigured boot options which is incompatible with TDX. Then is such partial check still useful or better to just document those restrictions and let TDX module capture any violation later as what you explained in __init_tdx()?

Quoted 2 lines

Thanks
Kevin

The purpose of checking cpus_booted_once_mask aganist cpu_present_mask is NOT to
check whether all BIOS-enabled CPUs are brought up at least once. Instead, the
purpose is to check whether all cpus that kernel can use are brought up at least
once (TDX-capable machine doesn't support ACPI CPU hotplug and all cpus are
marked as enabled in MADT table, therefore cpu_present_mask is used instead of
cpu_possible_mask). This is used to make sure SEAMRR has been detected on all
cpus that kernel can use.

Quoted 5 lines

Checking whether "all BIOS-enabled cpus are up" is not done here (neither in this series as we discussed it seems there's no appropriate variable to represent it). And we just let TDH.SYS.CONFIG to fail if TDH.SYS.LP.INIT is not done on all BIOS-enabled CPUs.

TDX requires all BIOS-enabled CPUs to execute certain SEAMCALLs for TDX initialization.

cpu_present_mask does not always represent BIOS-enabled CPUs due to those boot options. Then why do we care whether CPUs in this mask (if only representing a subset of BIOS-enabled CPUs) are at least brought up once? It will fail at TDH.SYS.CONFIG anyway.

btw your comment said that 'maxcpus' is basically an invalid mode due to MCE broadcase problem. I didn't find any code to block it when MCE is enabled, thus wonder the rationale behind and whether that rationale can be brought to this series (i.e. no check against those conflicting boot options and just let SEAMCALL itself to detect and fail).

@Thomas, any guidance here?

Thanks
Kevin

author
#17 lore raw

Re: [PATCH v2 04/21] x86/virt/tdx: Add skeleton for detecting and initializing TDX on demand

Quoted 5 lines

cpu_present_mask does not always represent BIOS-enabled CPUs due to those boot options. Then why do we care whether CPUs in this mask (if only representing a subset of BIOS-enabled CPUs) are at least brought up once? It will fail at TDH.SYS.CONFIG anyway.

As I said, this is used to make sure SEAMRR has been detected on all cpus, so that any BIOS misconfiguration on SEAMRR has been detected. Otherwise, seamrr_enabled() may not be reliable (theoretically).

Alternatively, I think we can also add check to disable TDX when 'maxcpus' has been specified, but I think the current way is better.

Quoted 4 lines

btw your comment said that 'maxcpus' is basically an invalid mode due to MCE broadcase problem. I didn't find any code to block it when MCE is enabled,

Please see below comment in cpu_smt_allowed():

static inline bool cpu_smt_allowed(unsigned int cpu)

{
	...
        /*
         * On x86 it's required to boot all logical CPUs at least once so
         * that the init code can get a chance to set CR4.MCE on each
         * CPU. Otherwise, a broadcasted MCE observing CR4.MCE=0b on any
         * core will shutdown the machine.
         */
	 return !cpumask_test_cpu(cpu, &cpus_booted_once_mask);
}
Quoted 8 lines

thus wonder the rationale behind and whether that
rationale can be brought to this series (i.e. no check against those conflicting boot options and just let SEAMCALL itself to detect and fail).

@Thomas, any guidance here?

Thanks
Kevin

#18 lore raw

RE: [PATCH v2 04/21] x86/virt/tdx: Add skeleton for detecting and initializing TDX on demand

Quoted 11 lines

From: Huang, Kai <[email protected]>
Sent: Monday, March 28, 2022 5:24 PM

Quoted 5 lines

cpu_present_mask does not always represent BIOS-enabled CPUs due to those boot options. Then why do we care whether CPUs in this mask (if only representing a subset of BIOS-enabled CPUs) are at least brought up once? It will fail at TDH.SYS.CONFIG anyway.

As I said, this is used to make sure SEAMRR has been detected on all cpus, so that any BIOS misconfiguration on SEAMRR has been detected. Otherwise, seamrr_enabled() may not be reliable (theoretically).

*all cpus* is questionable.

Say BIOS enabled 8 CPUs: [0 - 7]

cpu_present_map covers [0 - 5], due to nr_cpus=6

You compared cpus_booted_once_mask to cpu_present_mask so if maxcpus is set to a number < nr_cpus SEAMRR is considered disabled because you cannot verify CPUs between [max_cpus, nr_cpus). If following the same rationale then you also need a proper way to detect the case where nr_cpus < BIOS enabled number i.e. when you cannot verify SEAMRR on CPUs between [nr_cpus, 7]. otherwise this check is just incomplete.

But the entire check is actually unnecessary. You just need to verify SEAMRR and do TDX cpu init on online CPUs. Any gap between online ones and BIOS enabled ones will be caught by the TDX module at TDH.SYS.CONFIG point.

Quoted 13 lines

Alternatively, I think we can also add check to disable TDX when 'maxcpus' has
been specified, but I think the current way is better.

Quoted 4 lines

btw your comment said that 'maxcpus' is basically an invalid mode due to MCE broadcase problem. I didn't find any code to block it when MCE is enabled,

Please see below comment in cpu_smt_allowed():

static inline bool cpu_smt_allowed(unsigned int cpu)

Quoted 10 lines
{
	...
        /*
         * On x86 it's required to boot all logical CPUs at least once so
         * that the init code can get a chance to set CR4.MCE on each
         * CPU. Otherwise, a broadcasted MCE observing CR4.MCE=0b on any
         * core will shutdown the machine.
         */
	 return !cpumask_test_cpu(cpu, &cpus_booted_once_mask);
}

I saw that code. My point is more about your statement that maxcpus is almost invalid due to above situation then why didn't we do anything to document such restriction or throw out a warning when it's misconfigured...

Quoted 9 lines
Quoted 9 lines

thus wonder the rationale behind and whether that
rationale can be brought to this series (i.e. no check against those conflicting boot options and just let SEAMCALL itself to detect and fail).

@Thomas, any guidance here?

Thanks
Kevin

author
#19 lore raw

Re: [PATCH v2 04/21] x86/virt/tdx: Add skeleton for detecting and initializing TDX on demand

Quoted 15 lines · Kevin
Quoted 8 lines

From: Huang, Kai <[email protected]>
Sent: Monday, March 28, 2022 5:24 PM

Quoted 5 lines

cpu_present_mask does not always represent BIOS-enabled CPUs due to those boot options. Then why do we care whether CPUs in this mask (if only representing a subset of BIOS-enabled CPUs) are at least brought up once? It will fail at TDH.SYS.CONFIG anyway.

Quoted 4 lines

As I said, this is used to make sure SEAMRR has been detected on all cpus, so
that any BIOS misconfiguration on SEAMRR has been detected. Otherwise, seamrr_enabled() may not be reliable (theoretically).

*all cpus* is questionable.

Quoted 7 lines

Say BIOS enabled 8 CPUs: [0 - 7]

cpu_present_map covers [0 - 5], due to nr_cpus=6

You compared cpus_booted_once_mask to cpu_present_mask so if maxcpus is set to a number < nr_cpus SEAMRR is considered disabled because you cannot verify CPUs between [max_cpus, nr_cpus).

SEAMRR is not considered as disabled in this case, at least in my intention. My understanding on the spec is if SEAMRR is configured as enabled on one core (the SEAMRR MSRs are core-scope), the SEAMCALL instruction can work on that core. It is TDX's requirement that some SEAMCALL needs to be done on all BIOS-enabled CPUs to finish TDX initialization, but not SEAM's.

From this perspective, if we forget TDX at this moment but talk about SEAM alone, it might make sense to not just treat SEAMRR as disabled if kernel usable cpus are limited by 'nr_cpus'. The chance that BIOS misconfigured SEAMRR is really rare. If kernel can detect potential BIOS misconfiguration, it should do it. Otherwise, perhaps it's more reasonable not to just treat SEAM as disabled.

Quoted 8 lines

If following the same
rationale then you also need a proper way to detect the case where nr_cpus < BIOS enabled number i.e. when you cannot verify SEAMRR on CPUs between [nr_cpus, 7]. otherwise this check is just incomplete.

But the entire check is actually unnecessary. You just need to verify SEAMRR and do TDX cpu init on online CPUs. Any gap between online ones and BIOS enabled ones will be caught by the TDX module at TDH.SYS.CONFIG point.

This is equivalent to not having the paranoid check in seamrr_enabled(). By detecting SEAMRR in identify_cpu(), the detection has already been done for any online cpu.

Quoted 25 lines
Quoted 13 lines

Alternatively, I think we can also add check to disable TDX when 'maxcpus' has
been specified, but I think the current way is better.

Quoted 4 lines

btw your comment said that 'maxcpus' is basically an invalid mode due to MCE broadcase problem. I didn't find any code to block it when MCE is enabled,

Please see below comment in cpu_smt_allowed():

Quoted 11 lines

static inline bool cpu_smt_allowed(unsigned int cpu)
{
      ...
        /*
         * On x86 it's required to boot all logical CPUs at least once so
         * that the init code can get a chance to set CR4.MCE on each
         * CPU. Otherwise, a broadcasted MCE observing CR4.MCE=0b on any
         * core will shutdown the machine.
         */
       return !cpumask_test_cpu(cpu, &cpus_booted_once_mask); }

Quoted 4 lines

I saw that code. My point is more about your statement that maxcpus is almost invalid due to above situation then why didn't we do anything to document such restriction or throw out a warning when it's misconfigured...

The sentence "'maxcpus' is an invalid operation mode due to the MCE broadcast problem" was from Thomas. Perhaps I should not just put it into the comment.

Also, Thomas suggested:

"you should have a paranoia check which checks for the maxcpus command line parameter and if it's there and smaller than the number of present CPUs then you just refuse to enable TDX.

Alternatively you just have a separate cpumask tdx_availabe_mask and keep track of the CPUs which have been checked. When TDX is initialized you then can do:

if (!cpumask_equal(cpu_present_mask, tdx_available_mask))
return;
"

I found we can just use cpus_booted_once_mask, instead of tdx_available_mask, so I used the second way. And instead of putting the check when initializing TDX, I put to seamrr_enabled() since I guess it's more reasonable to be here as the logic is to make sure SEAMRR has been detected on all cpus.

Hi Thomas,

If you see this, sorry for quoting your words here. Just want to have a better discussion. And appreciate if you can have some guidance here.

Quoted 11 lines
Quoted 10 lines
Quoted 10 lines

thus wonder the rationale behind and whether that
rationale can be brought to this series (i.e. no check against those conflicting boot options and just let SEAMCALL itself to detect and fail).

@Thomas, any guidance here?

Thanks
Kevin

#20 lore raw

RE: [PATCH v2 04/21] x86/virt/tdx: Add skeleton for detecting and initializing TDX on demand

Quoted 23 lines

From: Huang, Kai <[email protected]>
Sent: Tuesday, March 29, 2022 6:55 AM

Quoted 6 lines · Kevin
Quoted 6 lines

From: Huang, Kai <[email protected]>
Sent: Monday, March 28, 2022 5:24 PM

Quoted 4 lines

cpu_present_mask does not always represent BIOS-enabled CPUs due to those boot options. Then why do we care whether CPUs in this mask (if only representing a subset of BIOS-enabled CPUs) are at least

brought

Quoted 5 lines
Quoted 5 lines
Quoted 1 line

up once? It will fail at TDH.SYS.CONFIG anyway.

As I said, this is used to make sure SEAMRR has been detected on all cpus, so
that any BIOS misconfiguration on SEAMRR has been detected.

Otherwise,

Quoted 6 lines
Quoted 1 line

seamrr_enabled() may not be reliable (theoretically).

*all cpus* is questionable.

Say BIOS enabled 8 CPUs: [0 - 7]

Quoted 8 lines
Quoted 3 lines

cpu_present_map covers [0 - 5], due to nr_cpus=6

You compared cpus_booted_once_mask to cpu_present_mask so if

maxcpus

Quoted 2 lines

is set to a number < nr_cpus SEAMRR is considered disabled because you cannot verify CPUs between [max_cpus, nr_cpus).

SEAMRR is not considered as disabled in this case, at least in my intention.

the function is called seamrr_enabled(), and false is returned if above check is not passed. So it is the intention from the code.

Quoted 9 lines

My
understanding on the spec is if SEAMRR is configured as enabled on one core (the
SEAMRR MSRs are core-scope), the SEAMCALL instruction can work on that core. It
is TDX's requirement that some SEAMCALL needs to be done on all BIOS- enabled
CPUs to finish TDX initialization, but not SEAM's.

Quoted 7 lines

From this perspective, if we forget TDX at this moment but talk about SEAM alone, it might make sense to not just treat SEAMRR as disabled if kernel usable
cpus are limited by 'nr_cpus'. The chance that BIOS misconfigured SEAMRR is really rare. If kernel can detect potential BIOS misconfiguration, it should do it. Otherwise, perhaps it's more reasonable not to just treat SEAM as disabled.

My problem is just that you didn't adopt consistency policy for CPUs in [maxcpus, nr_cpus) and CPUs in [nr_cpus, nr_bios_enabled_cpus). This is the only trouble to me no matter what policy you want to pursue...

Quoted 69 lines
Quoted 4 lines

If following the same
rationale then you also need a proper way to detect the case where

nr_cpus

Quoted 4 lines

< BIOS enabled number i.e. when you cannot verify SEAMRR on CPUs between [nr_cpus, 7]. otherwise this check is just incomplete.

But the entire check is actually unnecessary. You just need to verify

SEAMRR

Quoted 2 lines

and do TDX cpu init on online CPUs. Any gap between online ones and BIOS enabled ones will be caught by the TDX module at TDH.SYS.CONFIG point.

This is equivalent to not having the paranoid check in seamrr_enabled(). By detecting SEAMRR in identify_cpu(), the detection has already been done for any
online cpu.

Quoted 3 lines
Quoted 3 lines

Alternatively, I think we can also add check to disable TDX when

'maxcpus'

Quoted 10 lines
Quoted 10 lines

has
been specified, but I think the current way is better.

Quoted 4 lines

btw your comment said that 'maxcpus' is basically an invalid mode due to MCE broadcase problem. I didn't find any code to block it when MCE is enabled,

Please see below comment in cpu_smt_allowed():

Quoted 7 lines
Quoted 7 lines

static inline bool cpu_smt_allowed(unsigned int cpu)
{
      ...
        /*
         * On x86 it's required to boot all logical CPUs at least once so
         * that the init code can get a chance to set CR4.MCE on each
         * CPU. Otherwise, a broadcasted MCE observing CR4.MCE=0b on

any

Quoted 9 lines
Quoted 4 lines

         * core will shutdown the machine.
         */
       return !cpumask_test_cpu(cpu, &cpus_booted_once_mask); }

I saw that code. My point is more about your statement that maxcpus is almost invalid due to above situation then why didn't we do anything to document such restriction or throw out a warning when it's misconfigured...

The sentence "'maxcpus' is an invalid operation mode due to the MCE broadcast
problem" was from Thomas. Perhaps I should not just put it into the comment.

Also, Thomas suggested:

"you should have a paranoia check which checks for the maxcpus command line parameter and if it's there and smaller than the number of present CPUs then you just refuse to enable TDX.

Alternatively you just have a separate cpumask tdx_availabe_mask and keep track of the CPUs which have been checked. When TDX is initialized you then can do:

if (!cpumask_equal(cpu_present_mask, tdx_available_mask))
return;
"

Quoted 5 lines

I found we can just use cpus_booted_once_mask, instead of tdx_available_mask, so
I used the second way. And instead of putting the check when initializing TDX, I put to seamrr_enabled() since I guess it's more reasonable to be here as the logic is to make sure SEAMRR has been detected on all cpus.

I'm not sure whether Thomas's comment just takes maxcpus as an example which should be extended to other boot options like nr_cpus or he really only cares about maxcpus.

Quoted 17 lines

Hi Thomas,

If you see this, sorry for quoting your words here. Just want to have a better discussion. And appreciate if you can have some guidance here.

Quoted 11 lines
Quoted 10 lines
Quoted 10 lines

thus wonder the rationale behind and whether that
rationale can be brought to this series (i.e. no check against those conflicting boot options and just let SEAMCALL itself to detect and fail).

@Thomas, any guidance here?

Thanks
Kevin

author
#21 lore raw

Re: [PATCH v2 04/21] x86/virt/tdx: Add skeleton for detecting and initializing TDX on demand

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

*all cpus* is questionable.

Quoted 8 lines
Quoted 8 lines
Quoted 5 lines

Say BIOS enabled 8 CPUs: [0 - 7]

cpu_present_map covers [0 - 5], due to nr_cpus=6

You compared cpus_booted_once_mask to cpu_present_mask so if

maxcpus

Quoted 2 lines

is set to a number < nr_cpus SEAMRR is considered disabled because you cannot verify CPUs between [max_cpus, nr_cpus).

Sorry my bad. We can verify CPUs between [max_cpus, nr_cpus). When any cpu within that range becomes online, the detection code is run. The paranoid check in seamrr_enabled() is used to check whether all cpus within [max_cpus, nr_cpus) (if there's any -- cpus within [0, max_cpus) are up during boot) have been brought up at least once.

Quoted 5 lines
Quoted 2 lines

SEAMRR is not considered as disabled in this case, at least in my intention.

the function is called seamrr_enabled(), and false is returned if above check is not passed. So it is the intention from the code.

The false is returned if something error is discovered among cpus [0 - present_cpus]. It returns true even if we cannot verify [present_cpus, bios_enabled_cpus).

Quoted 19 lines
Quoted 10 lines

My
understanding on the spec is if SEAMRR is configured as enabled on one core (the
SEAMRR MSRs are core-scope), the SEAMCALL instruction can work on that core. It
is TDX's requirement that some SEAMCALL needs to be done on all BIOS- enabled
CPUs to finish TDX initialization, but not SEAM's.

Quoted 8 lines

From this perspective, if we forget TDX at this moment but talk about SEAM alone, it might make sense to not just treat SEAMRR as disabled if kernel usable
cpus are limited by 'nr_cpus'. The chance that BIOS misconfigured SEAMRR is really rare. If kernel can detect potential BIOS misconfiguration, it should do
it. Otherwise, perhaps it's more reasonable not to just treat SEAM as disabled.

Quoted 3 lines

My problem is just that you didn't adopt consistency policy for CPUs in [maxcpus, nr_cpus) and CPUs in [nr_cpus, nr_bios_enabled_cpus). This is the only trouble to me no matter what policy you want to pursue...

Let's separate SEAMRR detection and TDX initialization. The paranoid check is only for SEAM detection, but not for TDX initialization. As I said, it is TDX's requirement that some SEAMCALL must be run on all bios-enabled cpus, but not SEAM's.

Signature · 2 lines
Thanks,
-Kai
author
#22 lore raw

Re: [PATCH v2 04/21] x86/virt/tdx: Add skeleton for detecting and initializing TDX on demand

Quoted 24 lines · Kai Huang
Quoted 11 lines
Quoted 11 lines
Quoted 8 lines

*all cpus* is questionable.

Say BIOS enabled 8 CPUs: [0 - 7]

cpu_present_map covers [0 - 5], due to nr_cpus=6

You compared cpus_booted_once_mask to cpu_present_mask so if

maxcpus

Quoted 2 lines

is set to a number < nr_cpus SEAMRR is considered disabled because you cannot verify CPUs between [max_cpus, nr_cpus).

Sorry my bad. We can verify CPUs between [max_cpus, nr_cpus). When any cpu within that range becomes online, the detection code is run. The paranoid check
in seamrr_enabled() is used to check whether all cpus within [max_cpus, nr_cpus)
(if there's any -- cpus within [0, max_cpus) are up during boot) have been brought up at least once.

Quoted 4 lines
Quoted 3 lines

SEAMRR is not considered as disabled in this case, at least in my intention.

Quoted 6 lines
Quoted 2 lines

the function is called seamrr_enabled(), and false is returned if above check is not passed. So it is the intention from the code.

The false is returned if something error is discovered among cpus [0 - present_cpus]. It returns true even if we cannot verify [present_cpus, bios_enabled_cpus).

Sorry typo. Not "something error is discovered among cpus [0 - present_cpus)", but "when there's any cpu within [0 - present_cpus) hasn't been brought up once".

Quoted 33 lines
Quoted 22 lines
Quoted 12 lines

My
understanding on the spec is if SEAMRR is configured as enabled on one core
(the
SEAMRR MSRs are core-scope), the SEAMCALL instruction can work on that core. It
is TDX's requirement that some SEAMCALL needs to be done on all BIOS- enabled
CPUs to finish TDX initialization, but not SEAM's.

Quoted 9 lines

From this perspective, if we forget TDX at this moment but talk about SEAM alone, it might make sense to not just treat SEAMRR as disabled if kernel usable
cpus are limited by 'nr_cpus'. The chance that BIOS misconfigured SEAMRR is
really rare. If kernel can detect potential BIOS misconfiguration, it should do
it. Otherwise, perhaps it's more reasonable not to just treat SEAM as disabled.

Quoted 3 lines

My problem is just that you didn't adopt consistency policy for CPUs in [maxcpus, nr_cpus) and CPUs in [nr_cpus, nr_bios_enabled_cpus). This is the only trouble to me no matter what policy you want to pursue...

Let's separate SEAMRR detection and TDX initialization. The paranoid check is only for SEAM detection, but not for TDX initialization. As I said, it is TDX's
requirement that some SEAMCALL must be run on all bios-enabled cpus, but not SEAM's.

author
#23 lore raw

PATCH v2 5/21 x86/virt/tdx: Detect P-SEAMLDR and TDX module

In reply to Kai Huang

The P-SEAMLDR (persistent SEAM loader) is the first software module that runs in SEAM VMX root, responsible for loading and updating the TDX module. Both the P-SEAMLDR and the TDX module are expected to be loaded before host kernel boots.

There is not CPUID or MSR to detect whether the P-SEAMLDR or the TDX module has been loaded. SEAMCALL instruction fails with VMfailInvalid when the target SEAM software module is not loaded, so SEAMCALL can be used to detect whether the P-SEAMLDR and the TDX module are loaded.

Detect the P-SEAMLDR and the TDX module by calling SEAMLDR.INFO SEAMCALL to get the P-SEAMLDR information. If the SEAMCALL succeeds, the P-SEAMLDR information further tells whether the TDX module is loaded or not.

Also add a wrapper of __seamcall() to make SEAMCALL to the P-SEAMLDR and the TDX module with additional defensive check on SEAMRR and CR4.VMXE, since both detecting and initializing TDX module require the caller of TDX to handle VMXON.

---
 arch/x86/virt/vmx/tdx.c | 175 +++++++++++++++++++++++++++++++++++++++-
 arch/x86/virt/vmx/tdx.h |  31 +++++++
 2 files changed, 205 insertions(+), 1 deletion(-)
arch/x86/virt/vmx/tdx.c +174 -1
index a3517e221578..b04f792f1e65 100644
@@ -15,7 +15,9 @@
 #include <asm/msr.h>
 #include <asm/cpufeature.h>
 #include <asm/cpufeatures.h>
+#include <asm/virtext.h>
 #include <asm/tdx.h>
+#include "tdx.h"
 
 /* Support Intel Secure Arbitration Mode Range Registers (SEAMRR) */
 #define MTRR_CAP_SEAMRR			BIT(15)
@@ -74,6 +76,8 @@ static enum tdx_module_status_t tdx_module_status;
 /* Prevent concurrent attempts on TDX detection and initialization */
 static DEFINE_MUTEX(tdx_module_lock);
 
+static struct p_seamldr_info p_seamldr_info;
+
 static bool __seamrr_enabled(void)
 {
 	return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;
@@ -229,6 +233,160 @@ static bool tdx_keyid_sufficient(void)
 	return tdx_keyid_num >= 2;
 }
 
+/*
+ * All error codes of both the P-SEAMLDR and the TDX module SEAMCALLs
+ * have bit 63 set if SEAMCALL fails.
+ */
+#define SEAMCALL_LEAF_ERROR(_ret)	((_ret) & BIT_ULL(63))
+
+/**
+ * seamcall - make SEAMCALL to the P-SEAMLDR or the TDX module with
+ *	      additional check on SEAMRR and CR4.VMXE
+ *
+ * @fn:			SEAMCALL leaf number.
+ * @rcx:		Input operand RCX.
+ * @rdx:		Input operand RDX.
+ * @r8:			Input operand R8.
+ * @r9:			Input operand R9.
+ * @seamcall_ret:	SEAMCALL completion status (can be NULL).
+ * @out:		Additional output operands (can be NULL).
+ *
+ * Wrapper of __seamcall() to make SEAMCALL to the P-SEAMLDR or the TDX
+ * module with additional defensive check on SEAMRR and CR4.VMXE.  Caller
+ * to make sure SEAMRR is enabled and CPU is already in VMX operation
+ * before calling this function.
+ *
+ * Unlike __seamcall(), it returns kernel error code instead of SEAMCALL
+ * completion status, which is returned via @seamcall_ret if desired.
+ *
+ * Return:
+ *
+ * * -ENODEV:	SEAMCALL failed with VMfailInvalid, or SEAMRR is not enabled.
+ * * -EPERM:	CR4.VMXE is not enabled
+ * * -EFAULT:	SEAMCALL failed
+ * * -0:	SEAMCALL succeeded
+ */
+static int seamcall(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
+		    u64 *seamcall_ret, struct tdx_module_output *out)
+{
+	u64 ret;
+
+	if (WARN_ON_ONCE(!seamrr_enabled()))
+		return -ENODEV;
+
+	/*
+	 * SEAMCALL instruction requires CPU being already in VMX
+	 * operation (VMXON has been done), otherwise it causes #UD.
+	 * Sanity check whether CR4.VMXE has been enabled.
+	 *
+	 * Note VMX being enabled in CR4 doesn't mean CPU is already
+	 * in VMX operation, but unfortunately there's no way to do
+	 * such check.  However in practice enabling CR4.VMXE and
+	 * doing VMXON are done together (for now) so in practice it
+	 * checks whether VMXON has been done.
+	 *
+	 * Preemption is disabled during the CR4.VMXE check and the
+	 * actual SEAMCALL so VMX doesn't get disabled by other threads
+	 * due to scheduling.
+	 */
+	preempt_disable();
+	if (WARN_ON_ONCE(!cpu_vmx_enabled())) {
+		preempt_enable_no_resched();
+		return -EPERM;
+	}
+
+	ret = __seamcall(fn, rcx, rdx, r8, r9, out);
+
+	preempt_enable_no_resched();
+
+	/*
+	 * Convert SEAMCALL error code to kernel error code:
+	 *  - -ENODEV:	VMfailInvalid
+	 *  - -EFAULT:	SEAMCALL failed
+	 *  - 0:	SEAMCALL was successful
+	 */
+	if (ret == TDX_SEAMCALL_VMFAILINVALID)
+		return -ENODEV;
+
+	/* Save the completion status if caller wants to use it */
+	if (seamcall_ret)
+		*seamcall_ret = ret;
+
+	/*
+	 * TDX module SEAMCALLs may also return non-zero completion
+	 * status codes but w/o bit 63 set.  Those codes are treated
+	 * as additional information/warning while the SEAMCALL is
+	 * treated as completed successfully.  Return 0 in this case.
+	 * Caller can use @seamcall_ret to get the additional code
+	 * when it is desired.
+	 */
+	if (SEAMCALL_LEAF_ERROR(ret)) {
+		pr_err("SEAMCALL leaf %llu failed: 0x%llx\n", fn, ret);
+		return -EFAULT;
+	}
+
+	return 0;
+}
+
+static inline bool p_seamldr_ready(void)
+{
+	return !!p_seamldr_info.p_seamldr_ready;
+}
+
+static inline bool tdx_module_ready(void)
+{
+	/*
+	 * SEAMLDR_INFO.SEAM_READY indicates whether TDX module
+	 * is (loaded and) ready for SEAMCALL.
+	 */
+	return p_seamldr_ready() && !!p_seamldr_info.seam_ready;
+}
+
+/*
+ * Detect whether the P-SEAMLDR has been loaded by calling SEAMLDR.INFO
+ * SEAMCALL to get the P-SEAMLDR information, which further tells whether
+ * the TDX module has been loaded and ready for SEAMCALL.  Caller to make
+ * sure only calling this function when CPU is already in VMX operation.
+ */
+static int detect_p_seamldr(void)
+{
+	int ret;
+
+	/*
+	 * SEAMCALL fails with VMfailInvalid when SEAM software is not
+	 * loaded, in which case seamcall() returns -ENODEV.  Use this
+	 * to detect the P-SEAMLDR.
+	 *
+	 * Note the P-SEAMLDR SEAMCALL also fails with VMfailInvalid when
+	 * the P-SEAMLDR is already busy with another SEAMCALL.  But this
+	 * won't happen here as this function is only called once.
+	 */
+	ret = seamcall(P_SEAMCALL_SEAMLDR_INFO, __pa(&p_seamldr_info),
+			0, 0, 0, NULL, NULL);
+	if (ret) {
+		if (ret == -ENODEV)
+			pr_info("P-SEAMLDR is not loaded.\n");
+		else
+			pr_info("Failed to detect P-SEAMLDR.\n");
+
+		return ret;
+	}
+
+	/*
+	 * If SEAMLDR.INFO was successful, it must be ready for SEAMCALL.
+	 * Otherwise it's either kernel or firmware bug.
+	 */
+	if (WARN_ON_ONCE(!p_seamldr_ready()))
+		return -ENODEV;
+
+	pr_info("P-SEAMLDR: version 0x%x, vendor_id: 0x%x, build_date: %u, build_num %u, major %u, minor %u\n",
+		p_seamldr_info.version, p_seamldr_info.vendor_id,
+		p_seamldr_info.build_date, p_seamldr_info.build_num,
+		p_seamldr_info.major, p_seamldr_info.minor);
+
+	return 0;
+}
+
 static int __tdx_detect(void)
 {
 	/* The TDX module is not loaded if SEAMRR is disabled */
@@ -247,7 +405,22 @@ static int __tdx_detect(void)
 		goto no_tdx_module;
 	}
 
-	/* Return -ENODEV until the TDX module is detected */
+	/*
+	 * For simplicity any error during detect_p_seamldr() marks
+	 * TDX module as not loaded.
+	 */
+	if (detect_p_seamldr())
+		goto no_tdx_module;
+
+	if (!tdx_module_ready()) {
+		pr_info("TDX module is not loaded.\n");
+		goto no_tdx_module;
+	}
+
+	pr_info("TDX module detected.\n");
+	tdx_module_status = TDX_MODULE_LOADED;
+	return 0;
+
 no_tdx_module:
 	tdx_module_status = TDX_MODULE_NONE;
 	return -ENODEV;
arch/x86/virt/vmx/tdx.h +31 -0
index 9d5b6f554c20..6990c93198b3 100644
@@ -3,6 +3,37 @@
 #define _X86_VIRT_TDX_H
 
 #include <linux/types.h>
+#include <linux/compiler.h>
+
+/*
+ * TDX architectural data structures
+ */
+
+#define P_SEAMLDR_INFO_ALIGNMENT	256
+
+struct p_seamldr_info {
+	u32	version;
+	u32	attributes;
+	u32	vendor_id;
+	u32	build_date;
+	u16	build_num;
+	u16	minor;
+	u16	major;
+	u8	reserved0[2];
+	u32	acm_x2apicid;
+	u8	reserved1[4];
+	u8	seaminfo[128];
+	u8	seam_ready;
+	u8	seam_debug;
+	u8	p_seamldr_ready;
+	u8	reserved2[88];
+} __packed __aligned(P_SEAMLDR_INFO_ALIGNMENT);
+
+/*
+ * P-SEAMLDR SEAMCALL leaf function
+ */
+#define P_SEAMLDR_SEAMCALL_BASE		BIT_ULL(63)
+#define P_SEAMCALL_SEAMLDR_INFO		(P_SEAMLDR_SEAMCALL_BASE | 0x0)
 
 struct tdx_module_output;
 u64 __seamcall(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
Signature · 1 line
2.35.1
author
#24 lore raw

PATCH v2 6/21 x86/virt/tdx: Shut down TDX module in case of error

In reply to Kai Huang

TDX supports shutting down the TDX module at any time during its lifetime. After TDX module is shut down, no further SEAMCALL can be made on any logical cpu.

Shut down the TDX module in case of any error happened during the initialization process. It's pointless to leave the TDX module in some middle state.

Shutting down the TDX module requires calling TDH.SYS.LP.SHUTDOWN on all BIOS-enabled cpus, and the SEMACALL can run concurrently on different cpus. Implement a mechanism to run SEAMCALL concurrently on all online cpus. Logical-cpu scope initialization will use it too.

---
 arch/x86/virt/vmx/tdx.c | 40 +++++++++++++++++++++++++++++++++++++++-
 arch/x86/virt/vmx/tdx.h |  5 +++++
 2 files changed, 44 insertions(+), 1 deletion(-)
arch/x86/virt/vmx/tdx.c +39 -1
index b04f792f1e65..d87af534db51 100644
@@ -11,6 +11,8 @@
 #include <linux/cpumask.h>
 #include <linux/mutex.h>
 #include <linux/cpu.h>
+#include <linux/smp.h>
+#include <linux/atomic.h>
 #include <asm/msr-index.h>
 #include <asm/msr.h>
 #include <asm/cpufeature.h>
@@ -328,6 +330,39 @@ static int seamcall(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
 	return 0;
 }
 
+/* Data structure to make SEAMCALL on multiple CPUs concurrently */
+struct seamcall_ctx {
+	u64 fn;
+	u64 rcx;
+	u64 rdx;
+	u64 r8;
+	u64 r9;
+	atomic_t err;
+	u64 seamcall_ret;
+	struct tdx_module_output out;
+};
+
+static void seamcall_smp_call_function(void *data)
+{
+	struct seamcall_ctx *sc = data;
+	int ret;
+
+	ret = seamcall(sc->fn, sc->rcx, sc->rdx, sc->r8, sc->r9,
+			&sc->seamcall_ret, &sc->out);
+	if (ret)
+		atomic_set(&sc->err, ret);
+}
+
+/*
+ * Call the SEAMCALL on all online cpus concurrently.
+ * Return error if SEAMCALL fails on any cpu.
+ */
+static int seamcall_on_each_cpu(struct seamcall_ctx *sc)
+{
+	on_each_cpu(seamcall_smp_call_function, sc, true);
+	return atomic_read(&sc->err);
+}
+
 static inline bool p_seamldr_ready(void)
 {
 	return !!p_seamldr_info.p_seamldr_ready;
@@ -437,7 +472,10 @@ static int init_tdx_module(void)
 
 static void shutdown_tdx_module(void)
 {
-	/* TODO: Shut down the TDX module */
+	struct seamcall_ctx sc = { .fn = TDH_SYS_LP_SHUTDOWN };
+
+	seamcall_on_each_cpu(&sc);
+
 	tdx_module_status = TDX_MODULE_SHUTDOWN;
 }
 
arch/x86/virt/vmx/tdx.h +5 -0
index 6990c93198b3..dcc1f6dfe378 100644
@@ -35,6 +35,11 @@ struct p_seamldr_info {
 #define P_SEAMLDR_SEAMCALL_BASE		BIT_ULL(63)
 #define P_SEAMCALL_SEAMLDR_INFO		(P_SEAMLDR_SEAMCALL_BASE | 0x0)
 
+/*
+ * TDX module SEAMCALL leaf functions
+ */
+#define TDH_SYS_LP_SHUTDOWN	44
+
 struct tdx_module_output;
 u64 __seamcall(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
 	       struct tdx_module_output *out);
Signature · 1 line
2.35.1
author
#25 lore raw

PATCH v2 7/21 x86/virt/tdx: Do TDX module global initialization

In reply to Kai Huang

Do the TDX module global initialization which requires calling TDH.SYS.INIT once on any logical cpu.

---
 arch/x86/virt/vmx/tdx.c | 11 ++++++++++-
 arch/x86/virt/vmx/tdx.h |  1 +
 2 files changed, 11 insertions(+), 1 deletion(-)
arch/x86/virt/vmx/tdx.c +10 -1
index d87af534db51..45e7404b5d81 100644
@@ -463,11 +463,20 @@ static int __tdx_detect(void)
 
 static int init_tdx_module(void)
 {
+	int ret;
+
+	/* TDX module global initialization */
+	ret = seamcall(TDH_SYS_INIT, 0, 0, 0, 0, NULL, NULL);
+	if (ret)
+		goto out;
+
 	/*
 	 * Return -EFAULT until all steps of TDX module
 	 * initialization are done.
 	 */
-	return -EFAULT;
+	ret = -EFAULT;
+out:
+	return ret;
 }
 
 static void shutdown_tdx_module(void)
arch/x86/virt/vmx/tdx.h +1 -0
index dcc1f6dfe378..f0983b1936d8 100644
@@ -38,6 +38,7 @@ struct p_seamldr_info {
 /*
  * TDX module SEAMCALL leaf functions
  */
+#define TDH_SYS_INIT		33
 #define TDH_SYS_LP_SHUTDOWN	44
 
 struct tdx_module_output;
Signature · 1 line
2.35.1
author
#26 lore raw

PATCH v2 8/21 x86/virt/tdx: Do logical-cpu scope TDX module initialization

In reply to Kai Huang

Logical-cpu scope initialization requires calling TDH.SYS.LP.INIT on all BIOS-enabled cpus, otherwise the TDH.SYS.CONFIG SEAMCALL will fail. TDH.SYS.LP.INIT can be called concurrently on all cpus.

Following global initialization, do the logical-cpu scope initialization by calling TDH.SYS.LP.INIT on all online cpus. Whether all BIOS-enabled cpus are online is not checked here for simplicity. The user of TDX should guarantee all BIOS-enabled cpus are online.

---
 arch/x86/virt/vmx/tdx.c | 12 ++++++++++++
 arch/x86/virt/vmx/tdx.h |  1 +
 2 files changed, 13 insertions(+)
arch/x86/virt/vmx/tdx.c +12 -0
index 45e7404b5d81..4b0c285d844b 100644
@@ -461,6 +461,13 @@ static int __tdx_detect(void)
 	return -ENODEV;
 }
 
+static int tdx_module_init_cpus(void)
+{
+	struct seamcall_ctx sc = { .fn = TDH_SYS_LP_INIT };
+
+	return seamcall_on_each_cpu(&sc);
+}
+
 static int init_tdx_module(void)
 {
 	int ret;
@@ -470,6 +477,11 @@ static int init_tdx_module(void)
 	if (ret)
 		goto out;
 
+	/* Logical-cpu scope initialization */
+	ret = tdx_module_init_cpus();
+	if (ret)
+		goto out;
+
 	/*
 	 * Return -EFAULT until all steps of TDX module
 	 * initialization are done.
arch/x86/virt/vmx/tdx.h +1 -0
index f0983b1936d8..b8cfdd6e12f3 100644
@@ -39,6 +39,7 @@ struct p_seamldr_info {
  * TDX module SEAMCALL leaf functions
  */
 #define TDH_SYS_INIT		33
+#define TDH_SYS_LP_INIT		35
 #define TDH_SYS_LP_SHUTDOWN	44
 
 struct tdx_module_output;
Signature · 1 line
2.35.1
author
#27 lore raw

PATCH v2 9/21 x86/virt/tdx: Get information about TDX module and convertible memory

In reply to Kai Huang

TDX provides increased levels of memory confidentiality and integrity. This requires special hardware support for features like memory encryption and storage of memory integrity checksums. Not all memory satisfies these requirements.

As a result, TDX introduced the concept of a "Convertible Memory Region" (CMR). During boot, the firmware builds a list of all of the memory ranges which can provide the TDX security guarantees. The list of these ranges, along with TDX module information, is available to the kernel by querying the TDX module via TDH.SYS.INFO SEAMCALL.

Host kernel can choose whether or not to use all convertible memory regions as TDX memory. Before TDX module is ready to create any TD guests, all TDX memory regions that host kernel intends to use must be configured to the TDX module, using specific data structures defined by TDX architecture. Constructing those structures requires information of both TDX module and the Convertible Memory Regions. Call TDH.SYS.INFO to get this information as preparation to construct those structures.

---
 arch/x86/virt/vmx/tdx.c | 127 ++++++++++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx.h |  61 +++++++++++++++++++
 2 files changed, 188 insertions(+)
arch/x86/virt/vmx/tdx.c +127 -0
index 4b0c285d844b..eb585bc5edda 100644
@@ -80,6 +80,11 @@ static DEFINE_MUTEX(tdx_module_lock);
 
 static struct p_seamldr_info p_seamldr_info;
 
+/* Base address of CMR array needs to be 512 bytes aligned. */
+static struct cmr_info tdx_cmr_array[MAX_CMRS] __aligned(CMR_INFO_ARRAY_ALIGNMENT);
+static int tdx_cmr_num;
+static struct tdsysinfo_struct tdx_sysinfo;
+
 static bool __seamrr_enabled(void)
 {
 	return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;
@@ -468,6 +473,123 @@ static int tdx_module_init_cpus(void)
 	return seamcall_on_each_cpu(&sc);
 }
 
+static inline bool cmr_valid(struct cmr_info *cmr)
+{
+	return !!cmr->size;
+}
+
+static void print_cmrs(struct cmr_info *cmr_array, int cmr_num,
+		       const char *name)
+{
+	int i;
+
+	for (i = 0; i < cmr_num; i++) {
+		struct cmr_info *cmr = &cmr_array[i];
+
+		pr_info("%s : [0x%llx, 0x%llx)\n", name,
+				cmr->base, cmr->base + cmr->size);
+	}
+}
+
+static int sanitize_cmrs(struct cmr_info *cmr_array, int cmr_num)
+{
+	int i, j;
+
+	/*
+	 * Intel TDX module spec, 20.7.3 CMR_INFO:
+	 *
+	 *   TDH.SYS.INFO leaf function returns a MAX_CMRS (32) entry
+	 *   array of CMR_INFO entries. The CMRs are sorted from the
+	 *   lowest base address to the highest base address, and they
+	 *   are non-overlapping.
+	 *
+	 * This implies that BIOS may generate invalid empty entries
+	 * if total CMRs are less than 32.  Skip them manually.
+	 */
+	for (i = 0; i < cmr_num; i++) {
+		struct cmr_info *cmr = &cmr_array[i];
+		struct cmr_info *prev_cmr = NULL;
+
+		/* Skip further invalid CMRs */
+		if (!cmr_valid(cmr))
+			break;
+
+		if (i > 0)
+			prev_cmr = &cmr_array[i - 1];
+
+		/*
+		 * It is a TDX firmware bug if CMRs are not
+		 * in address ascending order.
+		 */
+		if (prev_cmr && ((prev_cmr->base + prev_cmr->size) >
+					cmr->base)) {
+			pr_err("Firmware bug: CMRs not in address ascending order.\n");
+			return -EFAULT;
+		}
+	}
+
+	/*
+	 * Also a sane BIOS should never generate invalid CMR(s) between
+	 * two valid CMRs.  Sanity check this and simply return error in
+	 * this case.
+	 */
+	for (j = i; j < cmr_num; j++)
+		if (cmr_valid(&cmr_array[j])) {
+			pr_err("Firmware bug: invalid CMR(s) among valid CMRs.\n");
+			return -EFAULT;
+		}
+
+	/*
+	 * Trim all tail invalid empty CMRs.  BIOS should generate at
+	 * least one valid CMR, otherwise it's a TDX firmware bug.
+	 */
+	tdx_cmr_num = i;
+	if (!tdx_cmr_num) {
+		pr_err("Firmware bug: No valid CMR.\n");
+		return -EFAULT;
+	}
+
+	/* Print kernel sanitized CMRs */
+	print_cmrs(tdx_cmr_array, tdx_cmr_num, "Kernel-sanitized-CMR");
+
+	return 0;
+}
+
+static int tdx_get_sysinfo(void)
+{
+	struct tdx_module_output out;
+	u64 tdsysinfo_sz, cmr_num;
+	int ret;
+
+	BUILD_BUG_ON(sizeof(struct tdsysinfo_struct) != TDSYSINFO_STRUCT_SIZE);
+
+	ret = seamcall(TDH_SYS_INFO, __pa(&tdx_sysinfo), TDSYSINFO_STRUCT_SIZE,
+			__pa(tdx_cmr_array), MAX_CMRS, NULL, &out);
+	if (ret)
+		return ret;
+
+	/*
+	 * If TDH.SYS.CONFIG succeeds, RDX contains the actual bytes
+	 * written to @tdx_sysinfo and R9 contains the actual entries
+	 * written to @tdx_cmr_array.  Sanity check them.
+	 */
+	tdsysinfo_sz = out.rdx;
+	cmr_num = out.r9;
+	if (WARN_ON_ONCE((tdsysinfo_sz > sizeof(tdx_sysinfo)) || !tdsysinfo_sz ||
+				(cmr_num > MAX_CMRS) || !cmr_num))
+		return -EFAULT;
+
+	pr_info("TDX module: vendor_id 0x%x, major_version %u, minor_version %u, build_date %u, build_num %u",
+		tdx_sysinfo.vendor_id, tdx_sysinfo.major_version,
+		tdx_sysinfo.minor_version, tdx_sysinfo.build_date,
+		tdx_sysinfo.build_num);
+
+	/* Print BIOS provided CMRs */
+	print_cmrs(tdx_cmr_array, cmr_num, "BIOS-CMR");
+
+	return sanitize_cmrs(tdx_cmr_array, cmr_num);
+}
+
 static int init_tdx_module(void)
 {
 	int ret;
@@ -482,6 +604,11 @@ static int init_tdx_module(void)
 	if (ret)
 		goto out;
 
+	/* Get TDX module information and CMRs */
+	ret = tdx_get_sysinfo();
+	if (ret)
+		goto out;
+
 	/*
 	 * Return -EFAULT until all steps of TDX module
 	 * initialization are done.
arch/x86/virt/vmx/tdx.h +61 -0

73 lines not on this page · raw message

Signature · 1 line
2.35.1
#28 lore raw

Re: [PATCH v2 09/21] x86/virt/tdx: Get information about TDX module and convertible memory

Quoted 100 lines · Kai Huang

TDX provides increased levels of memory confidentiality and integrity. This requires special hardware support for features like memory encryption and storage of memory integrity checksums. Not all memory satisfies these requirements.

As a result, TDX introduced the concept of a "Convertible Memory Region" (CMR). During boot, the firmware builds a list of all of the memory ranges which can provide the TDX security guarantees. The list of these ranges, along with TDX module information, is available to the kernel by querying the TDX module via TDH.SYS.INFO SEAMCALL.

Host kernel can choose whether or not to use all convertible memory regions as TDX memory. Before TDX module is ready to create any TD guests, all TDX memory regions that host kernel intends to use must be configured to the TDX module, using specific data structures defined by TDX architecture. Constructing those structures requires information of both TDX module and the Convertible Memory Regions. Call TDH.SYS.INFO to get this information as preparation to construct those structures.

---
 arch/x86/virt/vmx/tdx.c | 127 ++++++++++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx.h |  61 +++++++++++++++++++
 2 files changed, 188 insertions(+)
arch/x86/virt/vmx/tdx.c +60 -0
index 4b0c285d844b..eb585bc5edda 100644
@@ -80,6 +80,11 @@ static DEFINE_MUTEX(tdx_module_lock);
 
 static struct p_seamldr_info p_seamldr_info;
 
+/* Base address of CMR array needs to be 512 bytes aligned. */
+static struct cmr_info tdx_cmr_array[MAX_CMRS] __aligned(CMR_INFO_ARRAY_ALIGNMENT);
+static int tdx_cmr_num;
+static struct tdsysinfo_struct tdx_sysinfo;
+
 static bool __seamrr_enabled(void)
 {
 	return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;
@@ -468,6 +473,123 @@ static int tdx_module_init_cpus(void)
 	return seamcall_on_each_cpu(&sc);
 }
 
+static inline bool cmr_valid(struct cmr_info *cmr)
+{
+	return !!cmr->size;
+}
+
+static void print_cmrs(struct cmr_info *cmr_array, int cmr_num,
+		       const char *name)
+{
+	int i;
+
+	for (i = 0; i < cmr_num; i++) {
+		struct cmr_info *cmr = &cmr_array[i];
+
+		pr_info("%s : [0x%llx, 0x%llx)\n", name,
+				cmr->base, cmr->base + cmr->size);
+	}
+}
+
+static int sanitize_cmrs(struct cmr_info *cmr_array, int cmr_num)
+{
+	int i, j;
+
+	/*
+	 * Intel TDX module spec, 20.7.3 CMR_INFO:
+	 *
+	 *   TDH.SYS.INFO leaf function returns a MAX_CMRS (32) entry
+	 *   array of CMR_INFO entries. The CMRs are sorted from the
+	 *   lowest base address to the highest base address, and they
+	 *   are non-overlapping.
+	 *
+	 * This implies that BIOS may generate invalid empty entries
+	 * if total CMRs are less than 32.  Skip them manually.
+	 */
+	for (i = 0; i < cmr_num; i++) {
+		struct cmr_info *cmr = &cmr_array[i];
+		struct cmr_info *prev_cmr = NULL;
+
+		/* Skip further invalid CMRs */
+		if (!cmr_valid(cmr))
+			break;
+
+		if (i > 0)
+			prev_cmr = &cmr_array[i - 1];
+
+		/*
+		 * It is a TDX firmware bug if CMRs are not
+		 * in address ascending order.
+		 */
+		if (prev_cmr && ((prev_cmr->base + prev_cmr->size) >
+					cmr->base)) {
+			pr_err("Firmware bug: CMRs not in address ascending order.\n");
+			return -EFAULT;
+		}
+	}
+

arch/x86/virt/vmx/tdx.c static int tdx_module_init_cpus(void) · line 540

Quoted 10 lines
arch/x86/virt/vmx/tdx.c +10 -0
+	/*
+	 * Also a sane BIOS should never generate invalid CMR(s) between
+	 * two valid CMRs.  Sanity check this and simply return error in
+	 * this case.
+	 */
+	for (j = i; j < cmr_num; j++)
+		if (cmr_valid(&cmr_array[j])) {
+			pr_err("Firmware bug: invalid CMR(s) among valid CMRs.\n");
+			return -EFAULT;
+		}

This check doesn't make sense because above i-for loop has break.

arch/x86/virt/vmx/tdx.c static int tdx_module_init_cpus(void) · line 550

Quoted 10 lines
arch/x86/virt/vmx/tdx.c +10 -0
+
+	/*
+	 * Trim all tail invalid empty CMRs.  BIOS should generate at
+	 * least one valid CMR, otherwise it's a TDX firmware bug.
+	 */
+	tdx_cmr_num = i;
+	if (!tdx_cmr_num) {
+		pr_err("Firmware bug: No valid CMR.\n");
+		return -EFAULT;
+	}

Something strange.
Probably we'd like to check it by decrementing.
for (i = cmr_num; i >= 0; i--)
if (!cmr_valid()) // if last invalid cmr
tdx_cmr_num
// more check. overlapping

Signature · 1 line
Isaku Yamahata <[email protected]>
author
#29 lore raw

Re: [PATCH v2 09/21] x86/virt/tdx: Get information about TDX module and convertible memory

Quoted 43 lines
Quoted 43 lines
arch/x86/virt/vmx/tdx.c +42 -0
+
+static int sanitize_cmrs(struct cmr_info *cmr_array, int cmr_num)
+{
+	int i, j;
+
+	/*
+	 * Intel TDX module spec, 20.7.3 CMR_INFO:
+	 *
+	 *   TDH.SYS.INFO leaf function returns a MAX_CMRS (32) entry
+	 *   array of CMR_INFO entries. The CMRs are sorted from the
+	 *   lowest base address to the highest base address, and they
+	 *   are non-overlapping.
+	 *
+	 * This implies that BIOS may generate invalid empty entries
+	 * if total CMRs are less than 32.  Skip them manually.
+	 */
+	for (i = 0; i < cmr_num; i++) {
+		struct cmr_info *cmr = &cmr_array[i];
+		struct cmr_info *prev_cmr = NULL;
+
+		/* Skip further invalid CMRs */
+		if (!cmr_valid(cmr))
+			break;
+
+		if (i > 0)
+			prev_cmr = &cmr_array[i - 1];
+
+		/*
+		 * It is a TDX firmware bug if CMRs are not
+		 * in address ascending order.
+		 */
+		if (prev_cmr && ((prev_cmr->base + prev_cmr->size) >
+					cmr->base)) {
+			pr_err("Firmware bug: CMRs not in address ascending order.\n");
+			return -EFAULT;
+		}
+	}
+
+	/*
+	 * Also a sane BIOS should never generate invalid CMR(s) between
+	 * two valid CMRs.  Sanity check this and simply return error in
+	 * this case.
Quoted 8 lines

arch/x86/virt/vmx/tdx.c static int tdx_module_init_cpus(void) · line 540

Quoted 6 lines
arch/x86/virt/vmx/tdx.c +6 -0
+	 */
+	for (j = i; j < cmr_num; j++)
+		if (cmr_valid(&cmr_array[j])) {
+			pr_err("Firmware bug: invalid CMR(s) among valid CMRs.\n");
+			return -EFAULT;
+		}

This check doesn't make sense because above i-for loop has break.

The break in above i-for loop will hit at the first invalid CMR entry. Yes "j = i" will make double check on this invalid CMR entry, but it should have no problem. Or we can change to "j = i + 1" to skip the first invalid CMR entry.

Does this make sense?

Quoted 10 lines
Quoted 10 lines
arch/x86/virt/vmx/tdx.c +9 -0
+
+	/*
+	 * Trim all tail invalid empty CMRs.  BIOS should generate at
+	 * least one valid CMR, otherwise it's a TDX firmware bug.
+	 */
+	tdx_cmr_num = i;
+	if (!tdx_cmr_num) {
+		pr_err("Firmware bug: No valid CMR.\n");
+		return -EFAULT;
Quoted 9 lines

Review

Quoted 1 line
(quoted diff) +1 -0
+	}

Something strange.
Probably we'd like to check it by decrementing.
for (i = cmr_num; i >= 0; i--)
if (!cmr_valid()) // if last invalid cmr
tdx_cmr_num
// more check. overlapping

I don't know how does this look strange to you? As replied above, "i" is the index to the first invalid CMR entry (or cmr_num, which is array size), so "tdx_cmr_num = i" initializes the total valid CMR number, correct?

#30 lore raw

Re: [PATCH v2 09/21] x86/virt/tdx: Get information about TDX module and convertible memory

Quoted 52 lines · Kai Huang
Quoted 43 lines
Quoted 43 lines
arch/x86/virt/vmx/tdx.c +42 -0
+
+static int sanitize_cmrs(struct cmr_info *cmr_array, int cmr_num)
+{
+	int i, j;
+
+	/*
+	 * Intel TDX module spec, 20.7.3 CMR_INFO:
+	 *
+	 *   TDH.SYS.INFO leaf function returns a MAX_CMRS (32) entry
+	 *   array of CMR_INFO entries. The CMRs are sorted from the
+	 *   lowest base address to the highest base address, and they
+	 *   are non-overlapping.
+	 *
+	 * This implies that BIOS may generate invalid empty entries
+	 * if total CMRs are less than 32.  Skip them manually.
+	 */
+	for (i = 0; i < cmr_num; i++) {
+		struct cmr_info *cmr = &cmr_array[i];
+		struct cmr_info *prev_cmr = NULL;
+
+		/* Skip further invalid CMRs */
+		if (!cmr_valid(cmr))
+			break;
+
+		if (i > 0)
+			prev_cmr = &cmr_array[i - 1];
+
+		/*
+		 * It is a TDX firmware bug if CMRs are not
+		 * in address ascending order.
+		 */
+		if (prev_cmr && ((prev_cmr->base + prev_cmr->size) >
+					cmr->base)) {
+			pr_err("Firmware bug: CMRs not in address ascending order.\n");
+			return -EFAULT;
+		}
+	}
+
+	/*
+	 * Also a sane BIOS should never generate invalid CMR(s) between
+	 * two valid CMRs.  Sanity check this and simply return error in
+	 * this case.
Quoted 8 lines

arch/x86/virt/vmx/tdx.c static int tdx_module_init_cpus(void) · line 540

Quoted 6 lines
arch/x86/virt/vmx/tdx.c +6 -0
+	 */
+	for (j = i; j < cmr_num; j++)
+		if (cmr_valid(&cmr_array[j])) {
+			pr_err("Firmware bug: invalid CMR(s) among valid CMRs.\n");
+			return -EFAULT;
+		}

This check doesn't make sense because above i-for loop has break.

Quoted 5 lines

The break in above i-for loop will hit at the first invalid CMR entry. Yes "j = i" will make double check on this invalid CMR entry, but it should have no problem. Or we can change to "j = i + 1" to skip the first invalid CMR entry.

Does this make sense?

It makes sense. Somehow I missed j = i. I scratch my review.

Signature · 1 line
Isaku Yamahata <[email protected]>
#31 lore raw

Re: [PATCH v2 09/21] x86/virt/tdx: Get information about TDX module and convertible memory

Quoted 11 lines · Isaku Yamahata
Quoted 11 lines
Quoted 11 lines

arch/x86/virt/vmx/tdx.c static int tdx_module_init_cpus(void) · line 540

Quoted 10 lines
arch/x86/virt/vmx/tdx.c +10 -0
+	/*
+	 * Also a sane BIOS should never generate invalid CMR(s) between
+	 * two valid CMRs.  Sanity check this and simply return error in
+	 * this case.
+	 */
+	for (j = i; j < cmr_num; j++)
+		if (cmr_valid(&cmr_array[j])) {
+			pr_err("Firmware bug: invalid CMR(s) among valid CMRs.\n");
+			return -EFAULT;
+		}

This check doesn't make sense because above i-for loop has break.

Quoted 6 lines
Quoted 5 lines

The break in above i-for loop will hit at the first invalid CMR entry. Yes "j = i" will make double check on this invalid CMR entry, but it should have no problem. Or we can change to "j = i + 1" to skip the first invalid CMR entry.

Does this make sense?

It makes sense. Somehow I missed j = i. I scratch my review.

You can also take it as something you might want to refactor, add comments, or work on better variable names. If it confused one person, it will confuse more in the future.

author
#32 lore raw

Re: [PATCH v2 09/21] x86/virt/tdx: Get information about TDX module and convertible memory

Quoted 16 lines · Dave Hansen
Quoted 15 lines · Isaku Yamahata
Quoted 15 lines
Quoted 11 lines

arch/x86/virt/vmx/tdx.c static int tdx_module_init_cpus(void) · line 540

Quoted 10 lines
arch/x86/virt/vmx/tdx.c +10 -0
+	/*
+	 * Also a sane BIOS should never generate invalid CMR(s) between
+	 * two valid CMRs.  Sanity check this and simply return error in
+	 * this case.
+	 */
+	for (j = i; j < cmr_num; j++)
+		if (cmr_valid(&cmr_array[j])) {
+			pr_err("Firmware bug: invalid CMR(s) among valid CMRs.\n");
+			return -EFAULT;
+		}

This check doesn't make sense because above i-for loop has break.

The break in above i-for loop will hit at the first invalid CMR entry. Yes "j = i" will make double check on this invalid CMR entry, but it should have no problem. Or we can change to "j = i + 1" to skip the first invalid CMR entry.

Quoted 6 lines
Quoted 2 lines
Quoted 1 line

Does this make sense?

It makes sense. Somehow I missed j = i. I scratch my review.

You can also take it as something you might want to refactor, add comments, or work on better variable names. If it confused one person, it will confuse more in the future.

Hi Dave,

OK I'll think over whether I can improve. Thanks for advice.

Btw if you have time, could you help to review this series? Or could you take a look at whether the overall design is OK, for instance, the design limitations described in the cover letter?

Signature · 2 lines
Thanks,
-Kai
#33 lore raw

Re: [PATCH v2 09/21] x86/virt/tdx: Get information about TDX module and convertible memory

Quoted 3 lines · Kai Huang

Btw if you have time, could you help to review this series? Or could you take a look at whether the overall design is OK, for instance, the design limitations described in the cover letter?

Kai, it's on my list, but it's a long list.

If you want to help, there are at least two other *big* TDX patch sets out there that need eyeballs:

Quoted 1 line

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

and

Quoted 1 line

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

author
#34 lore raw

Re: [PATCH v2 09/21] x86/virt/tdx: Get information about TDX module and convertible memory

Quoted 5 lines · Dave Hansen
Quoted 3 lines · Kai Huang

Btw if you have time, could you help to review this series? Or could you take a look at whether the overall design is OK, for instance, the design limitations described in the cover letter?

Quoted 11 lines

Kai, it's on my list, but it's a long list.

If you want to help, there are at least two other *big* TDX patch sets out there that need eyeballs:

Quoted 1 line

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

and

Quoted 1 line

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

Yes I am aware of them. I'll comment if I think that can help to speed up. Thanks for the info!

Signature · 2 lines
Thanks,
-Kai
author
#35 lore raw

PATCH v2 10/21 x86/virt/tdx: Add placeholder to coveret all system RAM as TDX memory

In reply to Kai Huang

TDX provides increased levels of memory confidentiality and integrity. This requires special hardware support for features like memory encryption and storage of memory integrity checksums. Not all memory satisfies these requirements.

As a result, TDX introduced the concept of a "Convertible Memory Region" (CMR). During boot, the firmware builds a list of all of the memory ranges which can provide the TDX security guarantees. The list of these ranges, along with TDX module information, is available to the kernel by querying the TDX module.

In order to provide crypto protection to TD guests, the TDX architecture also needs additional metadata to record things like which TD guest "owns" a given page of memory. This metadata essentially serves as the 'struct page' for the TDX module. The space for this metadata is not reserved by the hardware upfront and must be allocated by the kernel and given to the TDX module.

Since this metadata consumes space, the VMM can choose whether or not to allocate it for a given area of convertible memory. If it chooses not to, the memory cannot receive TDX protections and can not be used by TDX guests as private memory.

For every memory region that the VMM wants to use as TDX memory, it sets up a "TD Memory Region" (TDMR). Each TDMR represents a physically contiguous convertible range and must also have its own physically contiguous metadata table, referred to as a Physical Address Metadata Table (PAMT), to track status for each page in the TDMR range.

Unlike a CMR, each TDMR requires 1G granularity and alignment. To support physical RAM areas that don't meet those strict requirements, each TDMR permits a number of internal "reserved areas" which can be placed over memory holes. If PAMT metadata is placed within a TDMR it must be covered by one of these reserved areas.

Let's summarize the concepts:

CMR - Firmware-enumerated physical ranges that support TDX. CMRs are
4K aligned.
TDMR - Physical address range which is chosen by the kernel to support
TDX. 1G granularity and alignment required. Each TDMR has reserved areas where TDX memory holes and overlapping PAMTs can be put into.
PAMT - Physically contiguous TDX metadata. One table for each page size
per TDMR. Roughly 1/256th of TDMR in size. 256G TDMR = ~1G PAMT.

As one step of initializing the TDX module, the memory regions that TDX module can use must be configured to the TDX module via an array of TDMRs.

Constructing TDMRs to build the TDX memory consists below steps:

1) Create TDMRs to cover all memory regions that TDX module can use;
2) Allocate and set up PAMT for each TDMR;
3) Set up reserved areas for each TDMR.

Add a placeholder right after getting TDX module and CMRs information to construct TDMRs to do the above steps, as the preparation to configure the TDX module. Always free TDMRs at the end of the initialization (no matter successful or not), as TDMRs are only used during the initialization.

---
 arch/x86/virt/vmx/tdx.c | 47 +++++++++++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx.h | 23 ++++++++++++++++++++
 2 files changed, 70 insertions(+)
arch/x86/virt/vmx/tdx.c +47 -0
index eb585bc5edda..1571bf192dde 100644
@@ -13,6 +13,7 @@
 #include <linux/cpu.h>
 #include <linux/smp.h>
 #include <linux/atomic.h>
+#include <linux/slab.h>
 #include <asm/msr-index.h>
 #include <asm/msr.h>
 #include <asm/cpufeature.h>
@@ -590,8 +591,29 @@ static int tdx_get_sysinfo(void)
 	return sanitize_cmrs(tdx_cmr_array, cmr_num);
 }
 
+static void free_tdmrs(struct tdmr_info **tdmr_array, int tdmr_num)
+{
+	int i;
+
+	for (i = 0; i < tdmr_num; i++) {
+		struct tdmr_info *tdmr = tdmr_array[i];
+
+		/* kfree() works with NULL */
+		kfree(tdmr);
+		tdmr_array[i] = NULL;
+	}
+}
+
+static int construct_tdmrs(struct tdmr_info **tdmr_array, int *tdmr_num)
+{
+	/* Return -EFAULT until constructing TDMRs is done */
+	return -EFAULT;
+}
+
 static int init_tdx_module(void)
 {
+	struct tdmr_info **tdmr_array;
+	int tdmr_num;
 	int ret;
 
 	/* TDX module global initialization */
@@ -609,11 +631,36 @@ static int init_tdx_module(void)
 	if (ret)
 		goto out;
 
+	/*
+	 * Prepare enough space to hold pointers of TDMRs (TDMR_INFO).
+	 * TDX requires TDMR_INFO being 512 aligned.  Each TDMR is
+	 * allocated individually within construct_tdmrs() to meet
+	 * this requirement.
+	 */
+	tdmr_array = kcalloc(tdx_sysinfo.max_tdmrs, sizeof(struct tdmr_info *),
+			GFP_KERNEL);
+	if (!tdmr_array) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	/* Construct TDMRs to build TDX memory */
+	ret = construct_tdmrs(tdmr_array, &tdmr_num);
+	if (ret)
+		goto out_free_tdmrs;
+
 	/*
 	 * Return -EFAULT until all steps of TDX module
 	 * initialization are done.
 	 */
 	ret = -EFAULT;
+out_free_tdmrs:
+	/*
+	 * TDMRs are only used during initializing TDX module.  Always
+	 * free them no matter the initialization was successful or not.
+	 */
+	free_tdmrs(tdmr_array, tdmr_num);
+	kfree(tdmr_array);
 out:
 	return ret;
 }
arch/x86/virt/vmx/tdx.h +23 -0
index 2f21c45df6ac..05bf9fe6bd00 100644
@@ -89,6 +89,29 @@ struct tdsysinfo_struct {
 	};
 } __packed __aligned(TDSYSINFO_STRUCT_ALIGNMENT);
 
+struct tdmr_reserved_area {
+	u64 offset;
+	u64 size;
+} __packed;
+
+#define TDMR_INFO_ALIGNMENT	512
+
+struct tdmr_info {
+	u64 base;
+	u64 size;
+	u64 pamt_1g_base;
+	u64 pamt_1g_size;
+	u64 pamt_2m_base;
+	u64 pamt_2m_size;
+	u64 pamt_4k_base;
+	u64 pamt_4k_size;
+	/*
+	 * Actual number of reserved areas depends on
+	 * 'struct tdsysinfo_struct'::max_reserved_per_tdmr.
+	 */
+	struct tdmr_reserved_area reserved_areas[0];
+} __packed __aligned(TDMR_INFO_ALIGNMENT);
+
 /*
  * P-SEAMLDR SEAMCALL leaf function
  */
Signature · 1 line
2.35.1
author
#36 lore raw

PATCH v2 11/21 x86/virt/tdx: Choose to use all system RAM as TDX memory

In reply to Kai Huang

As one step of initializing the TDX module, the memory regions that the TDX module can use must be configured to it via an array of 'TD Memory Regions' (TDMR). The kernel is responsible for choosing which memory regions to be used as TDX memory and building the array of TDMRs to cover those memory regions.

The first generation of TDX-capable platforms basically guarantees all system RAM regions during machine boot are Convertible Memory Regions (excluding the memory below 1MB) and can be used by TDX. The memory pages allocated to TD guests can be any pages managed by the page allocator. To avoid having to modify the page allocator to distinguish TDX and non-TDX memory allocation, adopt a simple policy to use all system RAM regions as TDX memory. The low 1MB pages are excluded from TDX memory since they are not in CMRs in some platforms (those pages are reserved at boot time and won't be managed by page allocator anyway).

This policy could be revised later if future TDX generations break the guarantee or when the size of the metadata (~1/256th of the size of the TDX usable memory) becomes a concern. At that time a CMR-aware page allocator may be necessary.

Also, on the first generation of TDX-capable machine, the system RAM ranges discovered during boot time are all memory regions that kernel can use during its runtime. This is because the first generation of TDX architecturally doesn't support ACPI memory hotplug (CMRs are generated during machine boot and are static during machine's runtime). Also, the first generation of TDX-capable platform doesn't support TDX and ACPI memory hotplug at the same time on a single machine. Another case of memory hotplug is user may use NVDIMM as system RAM via kmem driver. But the first generation of TDX-capable machine doesn't support TDX and NVDIMM simultaneously, therefore in practice it cannot happen. One special case is user may use 'memmap' kernel command line to reserve part of system RAM as x86 legacy PMEMs, and user can theoretically add them as system RAM via kmem driver. This can be resolved by always treating legacy PMEMs as TDX memory.

Implement a helper to loop over all RAM entries in e820 table to find all system RAM ranges, as a preparation to covert all of them to TDX memory. Use 'e820_table', rather than 'e820_table_firmware' to honor 'mem' and 'memmap' command lines. Following e820__memblock_setup(), both E820_TYPE_RAM and E820_TYPE_RESERVED_KERN types are treated as TDX memory, and contiguous ranges in the same NUMA node are merged together.

One difference is, as mentioned above, x86 legacy PMEMs (E820_TYPE_PRAM) are also always treated as TDX memory. They are underneath RAM, and they could be used as TD guest memory. Always including them as TDX memory also avoids having to modify memory hotplug code to handle adding them as system RAM via kmem driver.

To begin with, sanity check all memory regions found in e820 are fully covered by any CMR and can be used as TDX memory.

---
 arch/x86/Kconfig        |   1 +
 arch/x86/virt/vmx/tdx.c | 228 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 228 insertions(+), 1 deletion(-)
arch/x86/Kconfig +1 -0
index f4c5481cca46..2d3f983e3582 100644
@@ -1961,6 +1961,7 @@ config INTEL_TDX_HOST
 	default n
 	depends on CPU_SUP_INTEL
 	depends on X86_64
+	select NUMA_KEEP_MEMINFO if NUMA
 	help
 	  Intel Trust Domain Extensions (TDX) protects guest VMs from malicious
 	  host and certain physical attacks.  This option enables necessary TDX
arch/x86/virt/vmx/tdx.c +227 -1

252 lines not on this page · raw message

Signature · 1 line
2.35.1
author
#37 lore raw

PATCH v2 12/21 x86/virt/tdx: Create TDMRs to cover all system RAM

In reply to Kai Huang

The kernel configures TDX usable memory regions to the TDX module via an array of "TD Memory Region" (TDMR). Each TDMR entry (TDMR_INFO) contains the information of the base/size of a memory region, the base/size of the associated Physical Address Metadata Table (PAMT) and a list of reserved areas in the region.

Create a number of TDMRs according to the verified e820 RAM entries. As the first step only set up the base/size information for each TDMR.

TDMR must be 1G aligned and the size must be in 1G granularity. This implies that one TDMR could cover multiple e820 RAM entries. If a RAM entry spans the 1GB boundary and the former part is already covered by the previous TDMR, just create a new TDMR for the latter part.

TDX only supports a limited number of TDMRs (currently 64). Abort the TDMR construction process when the number of TDMRs exceeds this limitation.

---
 arch/x86/virt/vmx/tdx.c | 138 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 138 insertions(+)
arch/x86/virt/vmx/tdx.c +138 -0
index e5206599f558..1939b64d23e8 100644
@@ -54,6 +54,18 @@
 		((u32)(((_keyid_part) & 0xffffffffull) + 1))
 #define TDX_KEYID_NUM(_keyid_part)	((u32)((_keyid_part) >> 32))
 
+/* TDMR must be 1gb aligned */
+#define TDMR_ALIGNMENT		BIT_ULL(30)
+#define TDMR_PFN_ALIGNMENT	(TDMR_ALIGNMENT >> PAGE_SHIFT)
+
+/* Align up and down the address to TDMR boundary */
+#define TDMR_ALIGN_DOWN(_addr)	ALIGN_DOWN((_addr), TDMR_ALIGNMENT)
+#define TDMR_ALIGN_UP(_addr)	ALIGN((_addr), TDMR_ALIGNMENT)
+
+/* TDMR's start and end address */
+#define TDMR_START(_tdmr)	((_tdmr)->base)
+#define TDMR_END(_tdmr)		((_tdmr)->base + (_tdmr)->size)
+
 /*
  * TDX module status during initialization
  */
@@ -809,6 +821,44 @@ static int e820_check_against_cmrs(void)
 	return 0;
 }
 
+/* The starting offset of reserved areas within TDMR_INFO */
+#define TDMR_RSVD_START		64
+
+static struct tdmr_info *__alloc_tdmr(void)
+{
+	int tdmr_sz;
+
+	/*
+	 * TDMR_INFO's actual size depends on maximum number of reserved
+	 * areas that one TDMR supports.
+	 */
+	tdmr_sz = TDMR_RSVD_START + tdx_sysinfo.max_reserved_per_tdmr *
+		sizeof(struct tdmr_reserved_area);
+
+	/*
+	 * TDX requires TDMR_INFO to be 512 aligned.  Always align up
+	 * TDMR_INFO size to 512 so the memory allocated via kzalloc()
+	 * can meet the alignment requirement.
+	 */
+	tdmr_sz = ALIGN(tdmr_sz, TDMR_INFO_ALIGNMENT);
+
+	return kzalloc(tdmr_sz, GFP_KERNEL);
+}
+
+/* Create a new TDMR at given index in the TDMR array */
+static struct tdmr_info *alloc_tdmr(struct tdmr_info **tdmr_array, int idx)
+{
+	struct tdmr_info *tdmr;
+
+	if (WARN_ON_ONCE(tdmr_array[idx]))
+		return NULL;
+
+	tdmr = __alloc_tdmr();
+	tdmr_array[idx] = tdmr;
+
+	return tdmr;
+}
+
 static void free_tdmrs(struct tdmr_info **tdmr_array, int tdmr_num)
 {
 	int i;
@@ -822,6 +872,89 @@ static void free_tdmrs(struct tdmr_info **tdmr_array, int tdmr_num)
 	}
 }
 
+/*
+ * Create TDMRs to cover all RAM entries in e820_table.  The created
+ * TDMRs are saved to @tdmr_array and @tdmr_num is set to the actual
+ * number of TDMRs.  All entries in @tdmr_array must be initially NULL.
+ */
+static int create_tdmrs(struct tdmr_info **tdmr_array, int *tdmr_num)
+{
+	struct tdmr_info *tdmr;
+	u64 start, end;
+	int i, tdmr_idx;
+	int ret = 0;
+
+	tdmr_idx = 0;
+	tdmr = alloc_tdmr(tdmr_array, 0);
+	if (!tdmr)
+		return -ENOMEM;
+	/*
+	 * Loop over all RAM entries in e820 and create TDMRs to cover
+	 * them.  To keep it simple, always try to use one TDMR to cover
+	 * one RAM entry.
+	 */
+	e820_for_each_mem(e820_table, i, start, end) {
+		start = TDMR_ALIGN_DOWN(start);
+		end = TDMR_ALIGN_UP(end);
+
+		/*
+		 * If the current TDMR's size hasn't been initialized, it
+		 * is a new allocated TDMR to cover the new RAM entry.
+		 * Otherwise the current TDMR already covers the previous
+		 * RAM entry.  In the latter case, check whether the
+		 * current RAM entry has been fully or partially covered
+		 * by the current TDMR, since TDMR is 1G aligned.
+		 */
+		if (tdmr->size) {
+			/*
+			 * Loop to next RAM entry if the current entry
+			 * is already fully covered by the current TDMR.
+			 */
+			if (end <= TDMR_END(tdmr))
+				continue;
+
+			/*
+			 * If part of current RAM entry has already been
+			 * covered by current TDMR, skip the already
+			 * covered part.
+			 */
+			if (start < TDMR_END(tdmr))
+				start = TDMR_END(tdmr);
+
+			/*
+			 * Create a new TDMR to cover the current RAM
+			 * entry, or the remaining part of it.
+			 */
+			tdmr_idx++;
+			if (tdmr_idx >= tdx_sysinfo.max_tdmrs) {
+				ret = -E2BIG;
+				goto err;
+			}
+			tdmr = alloc_tdmr(tdmr_array, tdmr_idx);
+			if (!tdmr) {
+				ret = -ENOMEM;
+				goto err;
+			}
+		}
+
+		tdmr->base = start;
+		tdmr->size = end - start;
+	}
+
+	/* @tdmr_idx is always the index of last valid TDMR. */
+	*tdmr_num = tdmr_idx + 1;
+
+	return 0;
+err:
+	/*
+	 * Clean up already allocated TDMRs in case of error.  @tdmr_idx
+	 * indicates the last TDMR that wasn't created successfully,
+	 * therefore only needs to free @tdmr_idx TDMRs.
+	 */
+	free_tdmrs(tdmr_array, tdmr_idx);
+	return ret;
+}
+
 static int construct_tdmrs(struct tdmr_info **tdmr_array, int *tdmr_num)
 {
 	int ret;
@@ -830,8 +963,13 @@ static int construct_tdmrs(struct tdmr_info **tdmr_array, int *tdmr_num)
 	if (ret)
 		goto err;
 
+	ret = create_tdmrs(tdmr_array, tdmr_num);
+	if (ret)
+		goto err;
+
 	/* Return -EFAULT until constructing TDMRs is done */
 	ret = -EFAULT;
+	free_tdmrs(tdmr_array, *tdmr_num);
 err:
 	return ret;
 }
Signature · 1 line
2.35.1
author
#38 lore raw

PATCH v2 13/21 x86/virt/tdx: Allocate and set up PAMTs for TDMRs

In reply to Kai Huang

In order to provide crypto protection to guests, the TDX module uses additional metadata to record things like which guest "owns" a given page of memory. This metadata, referred as Physical Address Metadata Table (PAMT), essentially serves as the 'struct page' for the TDX module. PAMTs are not reserved by hardware upfront. They must be allocated by the kernel and then given to the TDX module.

TDX supports 3 page sizes: 4K, 2M, and 1G. Each "TD Memory Region" (TDMR) has 3 PAMTs to track the 3 supported page sizes respectively. Each PAMT must be a physically contiguous area from the Convertible Memory Regions (CMR). However, the PAMTs which track pages in one TDMR do not need to reside within that TDMR but can be anywhere in CMRs. If one PAMT overlaps with any TDMR, the overlapping part must be reported as a reserved area in that particular TDMR.

Use alloc_contig_pages() since PAMT must be a physically contiguous area and it may be potentially large (~1/256th of the size of the given TDMR).

The current version of TDX supports at most 16 reserved areas per TDMR to cover both PAMTs and potential memory holes within the TDMR. If many PAMTs are allocated within a single TDMR, 16 reserved areas may not be sufficient to cover all of them.

Adopt the following policies when allocating PAMTs for a given TDMR:

- Allocate three PAMTs of the TDMR in one contiguous chunk to minimize the total number of reserved areas consumed for PAMTs.
- Try to first allocate PAMT from the local node of the TDMR for better NUMA locality.

---
 arch/x86/Kconfig        |   1 +
 arch/x86/virt/vmx/tdx.c | 167 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 167 insertions(+), 1 deletion(-)
arch/x86/Kconfig +1 -0
index 2d3f983e3582..193ba089d353 100644
@@ -1962,6 +1962,7 @@ config INTEL_TDX_HOST
 	depends on CPU_SUP_INTEL
 	depends on X86_64
 	select NUMA_KEEP_MEMINFO if NUMA
+	depends on CONTIG_ALLOC
 	help
 	  Intel Trust Domain Extensions (TDX) protects guest VMs from malicious
 	  host and certain physical attacks.  This option enables necessary TDX
arch/x86/virt/vmx/tdx.c +166 -1

205 lines not on this page · raw message

Signature · 1 line
2.35.1
#39 lore raw

Re: [PATCH v2 13/21] x86/virt/tdx: Allocate and set up PAMTs for TDMRs

Quoted 30 lines · Kai Huang
arch/x86/virt/vmx/tdx.c +11 -0
index 1939b64d23e8..c58c99b94c72 100644
@@ -21,6 +21,7 @@
 #include <asm/cpufeatures.h>
 #include <asm/virtext.h>
 #include <asm/e820/api.h>
+#include <asm/pgtable.h>
 #include <asm/tdx.h>
 #include "tdx.h"
 
@@ -66,6 +67,16 @@
 #define TDMR_START(_tdmr)	((_tdmr)->base)
 #define TDMR_END(_tdmr)		((_tdmr)->base + (_tdmr)->size)
 
+/* Page sizes supported by TDX */
+enum tdx_page_sz {
+	TDX_PG_4K = 0,
+	TDX_PG_2M,
+	TDX_PG_1G,
+	TDX_PG_MAX,
+};
+
+#define TDX_HPAGE_SHIFT	9
+
 /*
  * TDX module status during initialization
  */

arch/x86/virt/vmx/tdx.c static int create_tdmrs(struct tdmr_info **tdmr_array, int *tdmr_num) · line 907

Quoted 5 lines
arch/x86/virt/vmx/tdx.c +1 -1
@@ -893,7 +904,7 @@ static int create_tdmrs(struct tdmr_info **tdmr_array, int *tdmr_num)
 	 * them.  To keep it simple, always try to use one TDMR to cover
 	 * one RAM entry.
 	 */
-	e820_for_each_mem(e820_table, i, start, end) {
+	e820_for_each_mem(i, start, end) {

This patch doesn't change e820_for_each_mem. This hunk should go into the previous patch?

thansk,

Signature · 1 line
Isaku Yamahata <[email protected]>
author
#40 lore raw

Re: [PATCH v2 13/21] x86/virt/tdx: Allocate and set up PAMTs for TDMRs

Quoted 42 lines · Isaku Yamahata
Quoted 30 lines · Kai Huang
arch/x86/virt/vmx/tdx.c +11 -0
index 1939b64d23e8..c58c99b94c72 100644
@@ -21,6 +21,7 @@
 #include <asm/cpufeatures.h>
 #include <asm/virtext.h>
 #include <asm/e820/api.h>
+#include <asm/pgtable.h>
 #include <asm/tdx.h>
 #include "tdx.h"
 
@@ -66,6 +67,16 @@
 #define TDMR_START(_tdmr)	((_tdmr)->base)
 #define TDMR_END(_tdmr)		((_tdmr)->base + (_tdmr)->size)
 
+/* Page sizes supported by TDX */
+enum tdx_page_sz {
+	TDX_PG_4K = 0,
+	TDX_PG_2M,
+	TDX_PG_1G,
+	TDX_PG_MAX,
+};
+
+#define TDX_HPAGE_SHIFT	9
+
 /*
  * TDX module status during initialization
  */

arch/x86/virt/vmx/tdx.c static int create_tdmrs(struct tdmr_info **tdmr_array, int *tdmr_num) · line 907

Quoted 5 lines
arch/x86/virt/vmx/tdx.c +1 -1
@@ -893,7 +904,7 @@ static int create_tdmrs(struct tdmr_info **tdmr_array, int *tdmr_num)
 	 * them.  To keep it simple, always try to use one TDMR to cover
 	 * one RAM entry.
 	 */
-	e820_for_each_mem(e820_table, i, start, end) {
+	e820_for_each_mem(i, start, end) {

This patch doesn't change e820_for_each_mem. This hunk should go into the previous patch?

Quoted 2 lines

thansk,

Exactly. My mistake. Thanks for catching.

Signature · 2 lines
Thanks,
-Kai
author
#41 lore raw

PATCH v2 14/21 x86/virt/tdx: Set up reserved areas for all TDMRs

In reply to Kai Huang

As the last step of constructing TDMRs, create reserved area information for the memory region holes in each TDMR. If any PAMT (or part of it) resides within a particular TDMR, also it as reserved.

All reserved areas in each TDMR must be in address ascending order, required by TDX architecture.

---
 arch/x86/virt/vmx/tdx.c | 148 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 146 insertions(+), 2 deletions(-)
arch/x86/virt/vmx/tdx.c +146 -2
index c58c99b94c72..f6345af29414 100644
@@ -15,6 +15,7 @@
 #include <linux/atomic.h>
 #include <linux/slab.h>
 #include <linux/math.h>
+#include <linux/sort.h>
 #include <asm/msr-index.h>
 #include <asm/msr.h>
 #include <asm/cpufeature.h>
@@ -1108,6 +1109,145 @@ static int tdmrs_setup_pamt_all(struct tdmr_info **tdmr_array, int tdmr_num)
 	return -ENOMEM;
 }
 
+static int tdmr_add_rsvd_area(struct tdmr_info *tdmr, int *p_idx,
+			      u64 addr, u64 size)
+{
+	struct tdmr_reserved_area *rsvd_areas = tdmr->reserved_areas;
+	int idx = *p_idx;
+
+	/* Reserved area must be 4K aligned in offset and size */
+	if (WARN_ON(addr & ~PAGE_MASK || size & ~PAGE_MASK))
+		return -EINVAL;
+
+	/* Cannot exceed maximum reserved areas supported by TDX */
+	if (idx >= tdx_sysinfo.max_reserved_per_tdmr)
+		return -E2BIG;
+
+	rsvd_areas[idx].offset = addr - tdmr->base;
+	rsvd_areas[idx].size = size;
+
+	*p_idx = idx + 1;
+
+	return 0;
+}
+
+/* Compare function called by sort() for TDMR reserved areas */
+static int rsvd_area_cmp_func(const void *a, const void *b)
+{
+	struct tdmr_reserved_area *r1 = (struct tdmr_reserved_area *)a;
+	struct tdmr_reserved_area *r2 = (struct tdmr_reserved_area *)b;
+
+	if (r1->offset + r1->size <= r2->offset)
+		return -1;
+	if (r1->offset >= r2->offset + r2->size)
+		return 1;
+
+	/* Reserved areas cannot overlap.  Caller should guarantee. */
+	WARN_ON(1);
+	return -1;
+}
+
+/* Set up reserved areas for a TDMR, including memory holes and PAMTs */
+static int tdmr_setup_rsvd_areas(struct tdmr_info *tdmr,
+				     struct tdmr_info **tdmr_array,
+				     int tdmr_num)
+{
+	u64 start, end, prev_end;
+	int rsvd_idx, i, ret = 0;
+
+	/* Mark holes between e820 RAM entries as reserved */
+	rsvd_idx = 0;
+	prev_end = TDMR_START(tdmr);
+	e820_for_each_mem(i, start, end) {
+		/* Break if this entry is after the TDMR */
+		if (start >= TDMR_END(tdmr))
+			break;
+
+		/* Exclude entries before this TDMR */
+		if (end < TDMR_START(tdmr))
+			continue;
+
+		/*
+		 * Skip if no hole exists before this entry. "<=" is
+		 * used because one e820 entry might span two TDMRs.
+		 * In that case the start address of this entry is
+		 * smaller then the start address of the second TDMR.
+		 */
+		if (start <= prev_end) {
+			prev_end = end;
+			continue;
+		}
+
+		/* Add the hole before this e820 entry */
+		ret = tdmr_add_rsvd_area(tdmr, &rsvd_idx, prev_end,
+				start - prev_end);
+		if (ret)
+			return ret;
+
+		prev_end = end;
+	}
+
+	/* Add the hole after the last RAM entry if it exists. */
+	if (prev_end < TDMR_END(tdmr)) {
+		ret = tdmr_add_rsvd_area(tdmr, &rsvd_idx, prev_end,
+				TDMR_END(tdmr) - prev_end);
+		if (ret)
+			return ret;
+	}
+
+	/*
+	 * Walk over all TDMRs to find out whether any PAMT falls into
+	 * the given TDMR. If yes, mark it as reserved too.
+	 */
+	for (i = 0; i < tdmr_num; i++) {
+		struct tdmr_info *tmp = tdmr_array[i];
+		u64 pamt_start, pamt_end;
+
+		pamt_start = tmp->pamt_4k_base;
+		pamt_end = pamt_start + tmp->pamt_4k_size +
+			tmp->pamt_2m_size + tmp->pamt_1g_size;
+
+		/* Skip PAMTs outside of the given TDMR */
+		if ((pamt_end <= TDMR_START(tdmr)) ||
+				(pamt_start >= TDMR_END(tdmr)))
+			continue;
+
+		/* Only mark the part within the TDMR as reserved */
+		if (pamt_start < TDMR_START(tdmr))
+			pamt_start = TDMR_START(tdmr);
+		if (pamt_end > TDMR_END(tdmr))
+			pamt_end = TDMR_END(tdmr);
+
+		ret = tdmr_add_rsvd_area(tdmr, &rsvd_idx, pamt_start,
+				pamt_end - pamt_start);
+		if (ret)
+			return ret;
+	}
+
+	/* TDX requires reserved areas listed in address ascending order */
+	sort(tdmr->reserved_areas, rsvd_idx, sizeof(struct tdmr_reserved_area),
+			rsvd_area_cmp_func, NULL);
+
+	return 0;
+}
+
+static int tdmrs_setup_rsvd_areas_all(struct tdmr_info **tdmr_array,
+				      int tdmr_num)
+{
+	int i;
+
+	for (i = 0; i < tdmr_num; i++) {
+		int ret;
+
+		ret = tdmr_setup_rsvd_areas(tdmr_array[i], tdmr_array,
+				tdmr_num);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int construct_tdmrs(struct tdmr_info **tdmr_array, int *tdmr_num)
 {
 	int ret;
@@ -1124,8 +1264,12 @@ static int construct_tdmrs(struct tdmr_info **tdmr_array, int *tdmr_num)
 	if (ret)
 		goto err_free_tdmrs;
 
-	/* Return -EFAULT until constructing TDMRs is done */
-	ret = -EFAULT;
+	ret = tdmrs_setup_rsvd_areas_all(tdmr_array, *tdmr_num);
+	if (ret)
+		goto err_free_pamts;
+
+	return 0;
+err_free_pamts:
 	tdmrs_free_pamt_all(tdmr_array, *tdmr_num);
 err_free_tdmrs:
 	free_tdmrs(tdmr_array, *tdmr_num);
Signature · 1 line
2.35.1
author
#42 lore raw

PATCH v2 15/21 x86/virt/tdx: Reserve TDX module global KeyID

In reply to Kai Huang

TDX module initialization requires to use one TDX private KeyID as the global KeyID to crypto protect TDX metadata. The global KeyID is configured to the TDX module along with TDMRs.

Just reserve the first TDX private KeyID as the global KeyID.

---
 arch/x86/virt/vmx/tdx.c | 9 +++++++++
 1 file changed, 9 insertions(+)
arch/x86/virt/vmx/tdx.c +9 -0
index f6345af29414..f70eee7df0c1 100644
@@ -112,6 +112,9 @@ static struct cmr_info tdx_cmr_array[MAX_CMRS] __aligned(CMR_INFO_ARRAY_ALIGNMEN
 static int tdx_cmr_num;
 static struct tdsysinfo_struct tdx_sysinfo;
 
+/* TDX global KeyID to protect TDX metadata */
+static u32 tdx_global_keyid;
+
 static bool __seamrr_enabled(void)
 {
 	return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;
@@ -1316,6 +1319,12 @@ static int init_tdx_module(void)
 	if (ret)
 		goto out_free_tdmrs;
 
+	/*
+	 * Reserve the first TDX KeyID as global KeyID to protect
+	 * TDX module metadata.
+	 */
+	tdx_global_keyid = tdx_keyid_start;
+
 	/*
 	 * Return -EFAULT until all steps of TDX module
 	 * initialization are done.
Signature · 1 line
2.35.1
author
#43 lore raw

PATCH v2 16/21 x86/virt/tdx: Configure TDX module with TDMRs and global KeyID

In reply to Kai Huang

After the TDX usable memory regions are constructed in an array of TDMRs and the global KeyID is reserved, configure them to the TDX module. The configuration is done via TDH.SYS.CONFIG, which is one call and can be done on any logical cpu.

---
 arch/x86/virt/vmx/tdx.c | 42 +++++++++++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx.h |  2 ++
 2 files changed, 44 insertions(+)
arch/x86/virt/vmx/tdx.c +42 -0
index f70eee7df0c1..e03dc3e420db 100644
@@ -1280,6 +1280,42 @@ static int construct_tdmrs(struct tdmr_info **tdmr_array, int *tdmr_num)
 	return ret;
 }
 
+static int config_tdx_module(struct tdmr_info **tdmr_array, int tdmr_num,
+			     u64 global_keyid)
+{
+	u64 *tdmr_pa_array;
+	int i, array_sz;
+	int ret;
+
+	/*
+	 * TDMR_INFO entries are configured to the TDX module via an
+	 * array of the physical address of each TDMR_INFO.  TDX requires
+	 * the array itself must be 512 aligned.  Round up the array size
+	 * to 512 aligned so the buffer allocated by kzalloc() meets the
+	 * alignment requirement.
+	 */
+	array_sz = ALIGN(tdmr_num * sizeof(u64), TDMR_INFO_PA_ARRAY_ALIGNMENT);
+	tdmr_pa_array = kzalloc(array_sz, GFP_KERNEL);
+	if (!tdmr_pa_array)
+		return -ENOMEM;
+
+	for (i = 0; i < tdmr_num; i++)
+		tdmr_pa_array[i] = __pa(tdmr_array[i]);
+
+	/*
+	 * TDH.SYS.CONFIG fails when TDH.SYS.LP.INIT is not done on all
+	 * BIOS-enabled cpus.  tdx_init() only disables CPU hotplug but
+	 * doesn't do early check whether all BIOS-enabled cpus are
+	 * online, so TDH.SYS.CONFIG can fail here.
+	 */
+	ret = seamcall(TDH_SYS_CONFIG, __pa(tdmr_pa_array), tdmr_num,
+				global_keyid, 0, NULL, NULL);
+	/* Free the array as it is not required any more. */
+	kfree(tdmr_pa_array);
+
+	return ret;
+}
+
 static int init_tdx_module(void)
 {
 	struct tdmr_info **tdmr_array;
@@ -1325,11 +1361,17 @@ static int init_tdx_module(void)
 	 */
 	tdx_global_keyid = tdx_keyid_start;
 
+	/* Config the TDX module with TDMRs and global KeyID */
+	ret = config_tdx_module(tdmr_array, tdmr_num, tdx_global_keyid);
+	if (ret)
+		goto out_free_pamts;
+
 	/*
 	 * Return -EFAULT until all steps of TDX module
 	 * initialization are done.
 	 */
 	ret = -EFAULT;
+out_free_pamts:
 	/*
 	 * Free PAMTs allocated in construct_tdmrs() when TDX module
 	 * initialization fails.
arch/x86/virt/vmx/tdx.h +2 -0
index 05bf9fe6bd00..d8e2800397af 100644
@@ -95,6 +95,7 @@ struct tdmr_reserved_area {
 } __packed;
 
 #define TDMR_INFO_ALIGNMENT	512
+#define TDMR_INFO_PA_ARRAY_ALIGNMENT	512
 
 struct tdmr_info {
 	u64 base;
@@ -125,6 +126,7 @@ struct tdmr_info {
 #define TDH_SYS_INIT		33
 #define TDH_SYS_LP_INIT		35
 #define TDH_SYS_LP_SHUTDOWN	44
+#define TDH_SYS_CONFIG		45
 
 struct tdx_module_output;
 u64 __seamcall(u64 fn, u64 rcx, u64 rdx, u64 r8, u64 r9,
Signature · 1 line
2.35.1
author
#44 lore raw

PATCH v2 17/21 x86/virt/tdx: Configure global KeyID on all packages

In reply to Kai Huang

Before TDX module can use the global KeyID to access TDX metadata, the key of the global KeyID must be configured on all physical packages via TDH.SYS.KEY.CONFIG. This SEAMCALL cannot run concurrently on different cpus since it exclusively acquires the TDX module.

Implement a helper to run SEAMCALL on one (any) cpu for all packages in serialized way, and run TDH.SYS.KEY.CONFIG on all packages using the helper.

The TDX module uses the global KeyID to initialize its metadata (PAMTs). Before TDX module can do that, all cachelines of PAMTs must be flushed. Otherwise, they may silently corrupt the PAMTs later initialized by the TDX module.

Use wbinvd to flush cache as PAMTs can be potentially large (~1/256th of system RAM).

Flush cache before configuring the global KeyID on all packages, as suggested by TDX specification. In practice, the current generation of TDX doesn't use the global KeyID in TDH.SYS.KEY.CONFIG. Therefore in practice flushing cache can be done after configuring the global KeyID is done on all packages. But the future generation of TDX may change this behaviour, so just follow TDX specification's suggestion to flush cache before configuring the global KeyID on all packages.

---
 arch/x86/virt/vmx/tdx.c | 94 ++++++++++++++++++++++++++++++++++++++++-
 arch/x86/virt/vmx/tdx.h |  1 +
 2 files changed, 94 insertions(+), 1 deletion(-)
arch/x86/virt/vmx/tdx.c +93 -1
index e03dc3e420db..39b1b7d0417d 100644
@@ -23,6 +23,7 @@
 #include <asm/virtext.h>
 #include <asm/e820/api.h>
 #include <asm/pgtable.h>
+#include <asm/smp.h>
 #include <asm/tdx.h>
 #include "tdx.h"
 
@@ -398,6 +399,47 @@ static int seamcall_on_each_cpu(struct seamcall_ctx *sc)
 	return atomic_read(&sc->err);
 }
 
+/*
+ * Call the SEAMCALL on one (any) cpu for each physical package in
+ * serialized way.  Note for serialized calls 'seamcall_ctx::err'
+ * doesn't have to be atomic, but for simplicity just reuse it
+ * instead of adding a new one.
+ *
+ * Return -ENXIO if IPI SEAMCALL wasn't run on any cpu, or -EFAULT
+ * when SEAMCALL fails, or -EPERM when the cpu where SEAMCALL runs
+ * on is not in VMX operation.  In case of -EFAULT, the error code
+ * of SEAMCALL is in 'struct seamcall_ctx::seamcall_ret'.
+ */
+static int seamcall_on_each_package_serialized(struct seamcall_ctx *sc)
+{
+	cpumask_var_t packages;
+	int cpu, ret;
+
+	if (!zalloc_cpumask_var(&packages, GFP_KERNEL))
+		return -ENOMEM;
+
+	for_each_online_cpu(cpu) {
+		if (cpumask_test_and_set_cpu(topology_physical_package_id(cpu),
+					packages))
+			continue;
+
+		ret = smp_call_function_single(cpu, seamcall_smp_call_function,
+				sc, true);
+		if (ret)
+			return ret;
+
+		/*
+		 * Doesn't have to use atomic_read(), but it doesn't
+		 * hurt either.
+		 */
+		ret = atomic_read(&sc->err);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static inline bool p_seamldr_ready(void)
 {
 	return !!p_seamldr_info.p_seamldr_ready;
@@ -1316,6 +1358,18 @@ static int config_tdx_module(struct tdmr_info **tdmr_array, int tdmr_num,
 	return ret;
 }
 
+static int config_global_keyid(u64 global_keyid)
+{
+	struct seamcall_ctx sc = { .fn = TDH_SYS_KEY_CONFIG };
+
+	/*
+	 * TDH.SYS.KEY.CONFIG may fail with entropy error (which is
+	 * a recoverable error).  Assume this is exceedingly rare and
+	 * just return error if encountered instead of retrying.
+	 */
+	return seamcall_on_each_package_serialized(&sc);
+}
+
 static int init_tdx_module(void)
 {
 	struct tdmr_info **tdmr_array;
@@ -1366,6 +1420,37 @@ static int init_tdx_module(void)
 	if (ret)
 		goto out_free_pamts;
 
+	/*
+	 * The same physical address associated with different KeyIDs
+	 * has separate cachelines.  Before using the new KeyID to access
+	 * some memory, the cachelines associated with the old KeyID must
+	 * be flushed, otherwise they may later silently corrupt the data
+	 * written with the new KeyID.  After cachelines associated with
+	 * the old KeyID are flushed, CPU speculative fetch using the old
+	 * KeyID is OK since the prefetched cachelines won't be consumed
+	 * by CPU core.
+	 *
+	 * TDX module initializes PAMTs using the global KeyID to crypto
+	 * protect them from malicious host.  Before that, the PAMTs are
+	 * used by kernel (with KeyID 0) and the cachelines associated
+	 * with the PAMTs must be flushed.  Given PAMTs are potentially
+	 * large (~1/256th of system RAM), just use WBINVD on all cpus to
+	 * flush the cache.
+	 *
+	 * In practice, the current generation of TDX doesn't use the
+	 * global KeyID in TDH.SYS.KEY.CONFIG.  Therefore in practice,
+	 * the cachelines can be flushed after configuring the global
+	 * KeyID on all pkgs is done.  But the future generation of TDX
+	 * may change this, so just follow the suggestion of TDX spec to
+	 * flush cache before TDH.SYS.KEY.CONFIG.
+	 */
+	wbinvd_on_all_cpus();
+
+	/* Config the key of global KeyID on all packages */
+	ret = config_global_keyid(tdx_global_keyid);
+	if (ret)
+		goto out_free_pamts;
+
 	/*
 	 * Return -EFAULT until all steps of TDX module
 	 * initialization are done.
@@ -1376,8 +1461,15 @@ static int init_tdx_module(void)
 	 * Free PAMTs allocated in construct_tdmrs() when TDX module
 	 * initialization fails.
 	 */
-	if (ret)
+	if (ret) {
+		/*
+		 * Part of PAMTs may already have been initialized by
+		 * TDX module.  Flush cache before returning them back
+		 * to kernel.
+		 */
+		wbinvd_on_all_cpus();
 		tdmrs_free_pamt_all(tdmr_array, tdmr_num);
+	}
 out_free_tdmrs:
 	/*
 	 * TDMRs are only used during initializing TDX module.  Always
arch/x86/virt/vmx/tdx.h +1 -0
index d8e2800397af..bba8cabea4bb 100644
@@ -122,6 +122,7 @@ struct tdmr_info {
 /*
  * TDX module SEAMCALL leaf functions
  */
+#define TDH_SYS_KEY_CONFIG	31
 #define TDH_SYS_INFO		32
 #define TDH_SYS_INIT		33
 #define TDH_SYS_LP_INIT		35
Signature · 1 line
2.35.1
#45 lore raw

Re: [PATCH v2 17/21] x86/virt/tdx: Configure global KeyID on all packages

Quoted 24 lines · Kai Huang
arch/x86/virt/vmx/tdx.c +9 -0
index e03dc3e420db..39b1b7d0417d 100644
@@ -23,6 +23,7 @@
 #include <asm/virtext.h>
 #include <asm/e820/api.h>
 #include <asm/pgtable.h>
+#include <asm/smp.h>
 #include <asm/tdx.h>
 #include "tdx.h"
 
@@ -398,6 +399,47 @@ static int seamcall_on_each_cpu(struct seamcall_ctx *sc)
 	return atomic_read(&sc->err);
 }
 
+/*
+ * Call the SEAMCALL on one (any) cpu for each physical package in
+ * serialized way.  Note for serialized calls 'seamcall_ctx::err'
+ * doesn't have to be atomic, but for simplicity just reuse it
+ * instead of adding a new one.
+ *
+ * Return -ENXIO if IPI SEAMCALL wasn't run on any cpu, or -EFAULT
+ * when SEAMCALL fails, or -EPERM when the cpu where SEAMCALL runs

arch/x86/virt/vmx/tdx.c static int seamcall_on_each_cpu(struct seamcall_ctx *sc) · line 419

Quoted 10 lines
arch/x86/virt/vmx/tdx.c +10 -0
+ * on is not in VMX operation.  In case of -EFAULT, the error code
+ * of SEAMCALL is in 'struct seamcall_ctx::seamcall_ret'.
+ */
+static int seamcall_on_each_package_serialized(struct seamcall_ctx *sc)
+{
+	cpumask_var_t packages;
+	int cpu, ret;
+
+	if (!zalloc_cpumask_var(&packages, GFP_KERNEL))
+		return -ENOMEM;

Memory leak. This should be freed before returning.

Quoted 26 lines
arch/x86/virt/vmx/tdx.c +22 -0
+	for_each_online_cpu(cpu) {
+		if (cpumask_test_and_set_cpu(topology_physical_package_id(cpu),
+					packages))
+			continue;
+
+		ret = smp_call_function_single(cpu, seamcall_smp_call_function,
+				sc, true);
+		if (ret)
+			return ret;
+
+		/*
+		 * Doesn't have to use atomic_read(), but it doesn't
+		 * hurt either.
+		 */
+		ret = atomic_read(&sc->err);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static inline bool p_seamldr_ready(void)
 {
 	return !!p_seamldr_info.p_seamldr_ready;

arch/x86/virt/vmx/tdx.c static int config_tdx_module(struct tdmr_info **tdmr_array, int tdmr_num, · line 1361

Quoted 4 lines
arch/x86/virt/vmx/tdx.c +1 -0
@@ -1316,6 +1358,18 @@ static int config_tdx_module(struct tdmr_info **tdmr_array, int tdmr_num,
 	return ret;
 }
 
+static int config_global_keyid(u64 global_keyid)

global_keyid argument isn't used. Is global variable tdx_global_keyid used?

Quoted 11 lines
arch/x86/virt/vmx/tdx.c +11 -0
+{
+	struct seamcall_ctx sc = { .fn = TDH_SYS_KEY_CONFIG };
+
+	/*
+	 * TDH.SYS.KEY.CONFIG may fail with entropy error (which is
+	 * a recoverable error).  Assume this is exceedingly rare and
+	 * just return error if encountered instead of retrying.
+	 */
+	return seamcall_on_each_package_serialized(&sc);
+}
+
Signature · 1 line
Isaku Yamahata <[email protected]>
author
#46 lore raw

Re: [PATCH v2 17/21] x86/virt/tdx: Configure global KeyID on all packages

Quoted 75 lines · On Thu, 2022-03-24 at 11:18 -0700, [email protected] wrote:
Quoted 24 lines · Kai Huang
arch/x86/virt/vmx/tdx.c +9 -0
index e03dc3e420db..39b1b7d0417d 100644
@@ -23,6 +23,7 @@
 #include <asm/virtext.h>
 #include <asm/e820/api.h>
 #include <asm/pgtable.h>
+#include <asm/smp.h>
 #include <asm/tdx.h>
 #include "tdx.h"
 
@@ -398,6 +399,47 @@ static int seamcall_on_each_cpu(struct seamcall_ctx *sc)
 	return atomic_read(&sc->err);
 }
 
+/*
+ * Call the SEAMCALL on one (any) cpu for each physical package in
+ * serialized way.  Note for serialized calls 'seamcall_ctx::err'
+ * doesn't have to be atomic, but for simplicity just reuse it
+ * instead of adding a new one.
+ *
+ * Return -ENXIO if IPI SEAMCALL wasn't run on any cpu, or -EFAULT
+ * when SEAMCALL fails, or -EPERM when the cpu where SEAMCALL runs

arch/x86/virt/vmx/tdx.c static int seamcall_on_each_cpu(struct seamcall_ctx *sc) · line 419

Quoted 10 lines
arch/x86/virt/vmx/tdx.c +10 -0
+ * on is not in VMX operation.  In case of -EFAULT, the error code
+ * of SEAMCALL is in 'struct seamcall_ctx::seamcall_ret'.
+ */
+static int seamcall_on_each_package_serialized(struct seamcall_ctx *sc)
+{
+	cpumask_var_t packages;
+	int cpu, ret;
+
+	if (!zalloc_cpumask_var(&packages, GFP_KERNEL))
+		return -ENOMEM;

Memory leak. This should be freed before returning.

Quoted 26 lines
arch/x86/virt/vmx/tdx.c +22 -0
+	for_each_online_cpu(cpu) {
+		if (cpumask_test_and_set_cpu(topology_physical_package_id(cpu),
+					packages))
+			continue;
+
+		ret = smp_call_function_single(cpu, seamcall_smp_call_function,
+				sc, true);
+		if (ret)
+			return ret;
+
+		/*
+		 * Doesn't have to use atomic_read(), but it doesn't
+		 * hurt either.
+		 */
+		ret = atomic_read(&sc->err);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static inline bool p_seamldr_ready(void)
 {
 	return !!p_seamldr_info.p_seamldr_ready;

arch/x86/virt/vmx/tdx.c static int config_tdx_module(struct tdmr_info **tdmr_array, int tdmr_num, · line 1361

Quoted 4 lines
arch/x86/virt/vmx/tdx.c +1 -0
@@ -1316,6 +1358,18 @@ static int config_tdx_module(struct tdmr_info **tdmr_array, int tdmr_num,
 	return ret;
 }
 
+static int config_global_keyid(u64 global_keyid)

global_keyid argument isn't used. Is global variable tdx_global_keyid used?

Quoted 11 lines

arch/x86/virt/vmx/tdx.c static int config_tdx_module(struct tdmr_info **tdmr_array, int tdmr_num, · line 1372

Quoted 11 lines
arch/x86/virt/vmx/tdx.c +11 -0
+{
+	struct seamcall_ctx sc = { .fn = TDH_SYS_KEY_CONFIG };
+
+	/*
+	 * TDH.SYS.KEY.CONFIG may fail with entropy error (which is
+	 * a recoverable error).  Assume this is exceedingly rare and
+	 * just return error if encountered instead of retrying.
+	 */
+	return seamcall_on_each_package_serialized(&sc);
+}
+

Both will be fixed. Thanks!

Signature · 2 lines
Thanks,
-Kai
author
#47 lore raw

PATCH v2 18/21 x86/virt/tdx: Initialize all TDMRs

In reply to Kai Huang

Initialize TDMRs via TDH.SYS.TDMR.INIT as the last step to complete the TDX initialization.

All TDMRs need to be initialized using TDH.SYS.TDMR.INIT SEAMCALL before the TDX memory can be used to run any TD guest. The SEAMCALL internally uses the global KeyID to initialize PAMTs in order to crypto protect them from malicious host kernel. TDH.SYS.TDMR.INIT can be done any cpu.

The time of initializing TDMR is proportional to the size of the TDMR. To avoid long latency caused in one SEAMCALL, TDH.SYS.TDMR.INIT only initializes an (implementation-specific) subset of PAMT entries of one TDMR in one invocation. The caller is responsible for calling TDH.SYS.TDMR.INIT iteratively until all PAMT entries of the requested TDMR are initialized.

Current implementation initializes TDMRs one by one. It takes ~100ms on a 2-socket machine with 2.2GHz CPUs and 64GB memory when the system is idle. Each TDH.SYS.TDMR.INIT takes ~7us on average.

TDX does allow different TDMRs to be initialized concurrently on multiple CPUs. This parallel scheme could be introduced later when the total initialization time becomes a real concern, e.g. on a platform with a much bigger memory size.

---
 arch/x86/virt/vmx/tdx.c | 75 ++++++++++++++++++++++++++++++++++++++---
 arch/x86/virt/vmx/tdx.h |  1 +
 2 files changed, 71 insertions(+), 5 deletions(-)
arch/x86/virt/vmx/tdx.c +70 -5
index 39b1b7d0417d..f2b9c98191ed 100644
@@ -1370,6 +1370,65 @@ static int config_global_keyid(u64 global_keyid)
 	return seamcall_on_each_package_serialized(&sc);
 }
 
+/* Initialize one TDMR */
+static int init_tdmr(struct tdmr_info *tdmr)
+{
+	u64 next;
+
+	/*
+	 * Initializing PAMT entries might be time-consuming (in
+	 * proportion to the size of the requested TDMR).  To avoid long
+	 * latency in one SEAMCALL, TDH.SYS.TDMR.INIT only initializes
+	 * an (implementation-defined) subset of PAMT entries in one
+	 * invocation.
+	 *
+	 * Call TDH.SYS.TDMR.INIT iteratively until all PAMT entries
+	 * of the requested TDMR are initialized (if next-to-initialize
+	 * address matches the end address of the TDMR).
+	 */
+	do {
+		struct tdx_module_output out;
+		int ret;
+
+		ret = seamcall(TDH_SYS_TDMR_INIT, tdmr->base, 0, 0, 0,
+				NULL, &out);
+		if (ret)
+			return ret;
+		/*
+		 * RDX contains 'next-to-initialize' address if
+		 * TDH.SYS.TDMR.INT succeeded.
+		 */
+		next = out.rdx;
+		if (need_resched())
+			cond_resched();
+	} while (next < tdmr->base + tdmr->size);
+
+	return 0;
+}
+
+/* Initialize all TDMRs */
+static int init_tdmrs(struct tdmr_info **tdmr_array, int tdmr_num)
+{
+	int i;
+
+	/*
+	 * Initialize TDMRs one-by-one for simplicity, though the TDX
+	 * architecture does allow different TDMRs to be initialized in
+	 * parallel on multiple CPUs.  Parallel initialization could
+	 * be added later when the time spent in the serialized scheme
+	 * becomes a real concern.
+	 */
+	for (i = 0; i < tdmr_num; i++) {
+		int ret;
+
+		ret = init_tdmr(tdmr_array[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int init_tdx_module(void)
 {
 	struct tdmr_info **tdmr_array;
@@ -1451,11 +1510,12 @@ static int init_tdx_module(void)
 	if (ret)
 		goto out_free_pamts;
 
-	/*
-	 * Return -EFAULT until all steps of TDX module
-	 * initialization are done.
-	 */
-	ret = -EFAULT;
+	/* Initialize TDMRs to complete the TDX module initialization */
+	ret = init_tdmrs(tdmr_array, tdmr_num);
+	if (ret)
+		goto out_free_pamts;
+
+	tdx_module_status = TDX_MODULE_INITIALIZED;
 out_free_pamts:
 	/*
 	 * Free PAMTs allocated in construct_tdmrs() when TDX module
@@ -1478,6 +1538,11 @@ static int init_tdx_module(void)
 	free_tdmrs(tdmr_array, tdmr_num);
 	kfree(tdmr_array);
 out:
+	if (ret)
+		pr_info("Failed to initialize TDX module.\n");
+	else
+		pr_info("TDX module initialized.\n");
+
 	return ret;
 }
 
arch/x86/virt/vmx/tdx.h +1 -0
index bba8cabea4bb..212f83374c0a 100644
@@ -126,6 +126,7 @@ struct tdmr_info {
 #define TDH_SYS_INFO		32
 #define TDH_SYS_INIT		33
 #define TDH_SYS_LP_INIT		35
+#define TDH_SYS_TDMR_INIT	36
 #define TDH_SYS_LP_SHUTDOWN	44
 #define TDH_SYS_CONFIG		45
 
Signature · 1 line
2.35.1
author
#48 lore raw

PATCH v2 19/21 x86: Flush cache of TDX private memory during kexec()

In reply to Kai Huang

If TDX is ever enabled and/or used to run any TD guests, the cachelines of TDX private memory, including PAMTs, used by TDX module need to be flushed before transiting to the new kernel otherwise they may silently corrupt the new kernel.

TDX module can only be initialized once during its lifetime. TDX does not have interface to reset TDX module to an uninitialized state so it could be initialized again. If the old kernel has enabled TDX, the new kernel won't be able to use TDX again. Therefore, ideally the old kernel should shut down the TDX module if it is ever initialized so that no SEAMCALLs can be made to it again.

However, shutting down the TDX module requires calling SEAMCALL, which requires cpu being in VMX operation (VMXON has been done). Currently, only KVM does entering/leaving VMX operation, so there's no guarantee that all cpus are in VMX operation during kexec(). Therefore, this implementation doesn't shut down the TDX module, but only does cache flush and just leave the TDX module open.

And it's fine to leave the module open. If the new kernel wants to use TDX, it needs to go through the initialization process, and it will fail at the first SEAMCALL due to the TDX module is not in the uninitialized state. If the new kernel doesn't want to use TDX, then the TDX module won't run at all.

Following the implementation of SME support, use wbinvd() to flush cache in stop_this_cpu(). Introduce a new function platform_has_tdx() to only check whether the platform is TDX-capable and do wbinvd() when it is true. platform_has_tdx() returns true when SEAMRR is enabled and there are enough TDX private KeyIDs to run at least one TD guest (both of which are detected at boot time). TDX is enabled on demand at runtime and it has a state machine with mutex to protect multiple callers to initialize TDX in parallel. Getting TDX module state needs to hold the mutex but stop_this_cpu() runs in interrupt context, so just check whether platform supports TDX and flush cache.

---
 arch/x86/include/asm/tdx.h |  2 ++
 arch/x86/kernel/process.c  | 15 ++++++++++++++-
 arch/x86/virt/vmx/tdx.c    | 14 ++++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)
arch/x86/include/asm/tdx.h +2 -0
index b526d41c4bbf..24f2b7e8b280 100644
@@ -85,10 +85,12 @@ static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1,
 void tdx_detect_cpu(struct cpuinfo_x86 *c);
 int tdx_detect(void);
 int tdx_init(void);
+bool platform_has_tdx(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; }
+static inline bool platform_has_tdx(void) { return false; }
 #endif /* CONFIG_INTEL_TDX_HOST */
 
 #endif /* !__ASSEMBLY__ */
arch/x86/kernel/process.c +14 -1
index 71aa12082370..bf3d1c9cb00c 100644
@@ -766,8 +766,21 @@ void stop_this_cpu(void *dummy)
 	 * without the encryption bit, they don't race each other when flushed
 	 * and potentially end up with the wrong entry being committed to
 	 * memory.
+	 *
+	 * In case of kexec, similar to SME, if TDX is ever enabled, the
+	 * cachelines of TDX private memory (including PAMTs) used by TDX
+	 * module need to be flushed before transiting to the new kernel,
+	 * otherwise they may silently corrupt the new kernel.
+	 *
+	 * Note TDX is enabled on demand at runtime, and enabling TDX has a
+	 * state machine protected with a mutex to prevent concurrent calls
+	 * from multiple callers.  Holding the mutex is required to get the
+	 * TDX enabling status, but this function runs in interrupt context.
+	 * So to make it simple, always flush cache when platform supports
+	 * TDX (detected at boot time), regardless whether TDX is truly
+	 * enabled by kernel.
 	 */
-	if (boot_cpu_has(X86_FEATURE_SME))
+	if (boot_cpu_has(X86_FEATURE_SME) || platform_has_tdx())
 		native_wbinvd();
 	for (;;) {
 		/*
arch/x86/virt/vmx/tdx.c +14 -0
index f2b9c98191ed..d9ad8dc7111e 100644
@@ -1681,3 +1681,17 @@ int tdx_init(void)
 	return ret;
 }
 EXPORT_SYMBOL_GPL(tdx_init);
+
+/**
+ * platform_has_tdx - Whether platform supports TDX
+ *
+ * Check whether platform supports TDX (i.e. TDX is enabled in BIOS),
+ * regardless whether TDX is truly enabled by kernel.
+ *
+ * Return true if SEAMRR is enabled, and there are sufficient TDX private
+ * KeyIDs to run TD guests.
+ */
+bool platform_has_tdx(void)
+{
+	return seamrr_enabled() && tdx_keyid_sufficient();
+}
Signature · 1 line
2.35.1
author
#49 lore raw

PATCH v2 20/21 x86/virt/tdx: Add kernel command line to opt-in TDX host support

In reply to Kai Huang

Enabling TDX consumes additional memory (used by TDX as metadata) and additional initialization time. Introduce a kernel command line to allow to opt-in TDX host kernel support when user truly wants to use TDX.

---
 Documentation/admin-guide/kernel-parameters.txt |  6 ++++++
 arch/x86/virt/vmx/tdx.c                         | 14 ++++++++++++++
 2 files changed, 20 insertions(+)
Documentation/admin-guide/kernel-parameters.txt +6 -0
index f5a27f067db9..9f85cafd0c2d 100644
@@ -5707,6 +5707,12 @@
 
 	tdfx=		[HW,DRM]
 
+	tdx_host=	[X86-64, TDX]
+			Format: {on|off}
+			on: Enable TDX host kernel support
+			off: Disable TDX host kernel support
+			Default is off.
+
 	test_suspend=	[SUSPEND][,N]
 			Specify "mem" (for Suspend-to-RAM) or "standby" (for
 			standby suspend) or "freeze" (for suspend type freeze)
arch/x86/virt/vmx/tdx.c +14 -0
index d9ad8dc7111e..2022f9c019b8 100644
@@ -116,6 +116,16 @@ static struct tdsysinfo_struct tdx_sysinfo;
 /* TDX global KeyID to protect TDX metadata */
 static u32 tdx_global_keyid;
 
+static bool enable_tdx_host;
+
+static int __init tdx_host_setup(char *s)
+{
+	if (!strcmp(s, "on"))
+		enable_tdx_host = true;
+	return 1;
+}
+__setup("tdx_host=", tdx_host_setup);
+
 static bool __seamrr_enabled(void)
 {
 	return (seamrr_mask & SEAMRR_ENABLED_BITS) == SEAMRR_ENABLED_BITS;
@@ -501,6 +511,10 @@ static int detect_p_seamldr(void)
 
 static int __tdx_detect(void)
 {
+	/* Disabled by kernel command line */
+	if (!enable_tdx_host)
+		goto no_tdx_module;
+
 	/* The TDX module is not loaded if SEAMRR is disabled */
 	if (!seamrr_enabled()) {
 		pr_info("SEAMRR not enabled.\n");
Signature · 1 line
2.35.1
author
#50 lore raw

PATCH v2 21/21 Documentation/x86: Add documentation for TDX host support

In reply to Kai Huang

Add <Documentation/x86/tdx_host.rst> for TDX host support.

---
 Documentation/x86/index.rst    |   1 +
 Documentation/x86/tdx_host.rst | 300 +++++++++++++++++++++++++++++++++
 2 files changed, 301 insertions(+)
 create mode 100644 Documentation/x86/tdx_host.rst
Documentation/x86/index.rst +1 -0
index 382e53ca850a..145fc251fbfc 100644
@@ -25,6 +25,7 @@ x86-specific Documentation
    intel_txt
    amd-memory-encryption
    tdx
+   tdx_host
    pti
    mds
    microcode
Documentation/x86/tdx_host.rst new file +300 -0

300 lines not on this page · raw message

Signature · 1 line
2.35.1