KVM TDX basic feature support

From: Chao Gao <[email protected]>

This partially reverts commit b99040853738 ("KVM: Pass kvm_init()'s opaque param to additional arch funcs") remove opaque from kvm_arch_check_processor_compat because no one uses this opaque now. Address conflicts for ARM (due to file movement) and manually handle RISC-V which comes after the commit. The change about kvm_arch_hardware_setup() in original commit are still needed so they are not reverted.

The current implementation enables hardware (e.g. enable VMX on all CPUs), arch-specific initialization for VM creation,

and disables hardware (in
x86, disable VMX on all CPUs) for last VM destruction.

TDX requires its initialization on loading KVM module with VMX enabled on all available CPUs. It needs to enable/disable hardware on module initialization. To reuse the same logic, one way is to pass around the

unused opaque argument, another way is to remove the unused opaque argument. This patch is a preparation for the latter by removing the argument

Signed-off-by Chao Gao <[email protected]>

Reviewed-by Sean Christopherson <[email protected]>

Reviewed-by Suzuki K Poulose <[email protected]>

Acked-by Anup Patel <[email protected]>

Acked-by Claudio Imbrenda <[email protected]>

Link https://lore.kernel.org/r/[email protected] open here

Signed-off-by Isaku Yamahata <[email protected]>

---
 arch/arm64/kvm/arm.c       |  2 +-
 arch/mips/kvm/mips.c       |  2 +-
 arch/powerpc/kvm/powerpc.c |  2 +-
 arch/riscv/kvm/main.c      |  2 +-
 arch/s390/kvm/kvm-s390.c   |  2 +-
 arch/x86/kvm/x86.c         |  2 +-
 include/linux/kvm_host.h   |  2 +-
 virt/kvm/kvm_main.c        | 16 +++-------------
 8 files changed, 10 insertions(+), 20 deletions(-)

arch/arm64/kvm/arm.c +1 -1

index 83a7f61354d3..c551ca587f16 100644
@@ -68,7 +68,7 @@ int kvm_arch_hardware_setup(void *opaque)
 	return 0;
 }
 
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
 {
 	return 0;
 }

arch/mips/kvm/mips.c +1 -1

index a25e0b73ee70..092d09fb6a7e 100644
@@ -140,7 +140,7 @@ int kvm_arch_hardware_setup(void *opaque)
 	return 0;
 }
 
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
 {
 	return 0;
 }

arch/powerpc/kvm/powerpc.c +1 -1

index 191992fcb2c2..ca8ef51092c6 100644
@@ -446,7 +446,7 @@ int kvm_arch_hardware_setup(void *opaque)
 	return 0;
 }
 
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
 {
 	return kvmppc_core_check_processor_compat();
 }

arch/riscv/kvm/main.c +1 -1

index 1549205fe5fe..f8d6372d208f 100644
@@ -20,7 +20,7 @@ long kvm_arch_dev_ioctl(struct file *filp,
 	return -EINVAL;
 }
 
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
 {
 	return 0;
 }

arch/s390/kvm/kvm-s390.c +1 -1

index edfd4bbd0cba..e26d4dd85668 100644
@@ -254,7 +254,7 @@ int kvm_arch_hardware_enable(void)
 	return 0;
 }
 
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
 {
 	return 0;
 }

arch/x86/kvm/x86.c +1 -1

index f5ff9b28e119..e533cce7a70b 100644
@@ -11990,7 +11990,7 @@ void kvm_arch_hardware_unsetup(void)
 	static_call(kvm_x86_hardware_unsetup)();
 }
 
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
 {
 	struct cpuinfo_x86 *c = &cpu_data(smp_processor_id());
 

include/linux/kvm_host.h +1 -1

index 1c480b1821e1..9643b8eadefe 100644
@@ -1438,7 +1438,7 @@ int kvm_arch_hardware_enable(void);
 void kvm_arch_hardware_disable(void);
 int kvm_arch_hardware_setup(void *opaque);
 void kvm_arch_hardware_unsetup(void);
-int kvm_arch_check_processor_compat(void *opaque);
+int kvm_arch_check_processor_compat(void);
 int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu);
 bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu);
 int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu);

virt/kvm/kvm_main.c +3 -13

index da263c370d00..0f5767e5ae45 100644
@@ -5793,22 +5793,14 @@ void kvm_unregister_perf_callbacks(void)
 }
 #endif
 
-struct kvm_cpu_compat_check {
-	void *opaque;
-	int *ret;
-};
-
-static void check_processor_compat(void *data)
+static void check_processor_compat(void *rtn)
 {
-	struct kvm_cpu_compat_check *c = data;
-
-	*c->ret = kvm_arch_check_processor_compat(c->opaque);
+	*(int *)rtn = kvm_arch_check_processor_compat();
 }
 
 int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
 		  struct module *module)
 {
-	struct kvm_cpu_compat_check c;
 	int r;
 	int cpu;
 

@@ -5836,10 +5828,8 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
 	if (r < 0)
 		goto out_free_1;
 
-	c.ret = &r;
-	c.opaque = opaque;
 	for_each_online_cpu(cpu) {
-		smp_call_function_single(cpu, check_processor_compat, &c, 1);
+		smp_call_function_single(cpu, check_processor_compat, &r, 1);
 		if (r < 0)
 			goto out_free_2;
 	}

Quoted 9 lines · On Sun, 2022-08-07 at 15:00 -0700, [email protected] wrote:

From: Chao Gao <[email protected]>

This partially reverts commit b99040853738 ("KVM: Pass kvm_init()'s opaque param to additional arch funcs") remove opaque from kvm_arch_check_processor_compat because no one uses this opaque now. Address conflicts for ARM (due to file movement) and manually handle RISC-V which comes after the commit. The change about kvm_arch_hardware_setup() in original commit are still needed so they are not reverted.

Quoted 3 lines

The current implementation enables hardware (e.g. enable VMX on all CPUs), arch-specific initialization for VM creation,

I guess you need to point out _first_ VM?

Quoted 7 lines

and disables hardware (in
x86, disable VMX on all CPUs) for last VM destruction.

TDX requires its initialization on loading KVM module with VMX enabled on all available CPUs. It needs to enable/disable hardware on module initialization. To reuse the same logic, one way is to pass around the

To reuse the same logic for what? I think you need to be specific (and focus) on why we need this patch: we will opportunistically move CPU compatibility check to hardware_enable_nolock(), which doesn't take any argument, and this patch is a preparation to do that.

Quoted 3 lines

unused opaque argument, another way is to remove the unused opaque argument. This patch is a preparation for the latter by removing the argument

So how about replacing the last two paragraphs with:

"
Initializing TDX will be done during module loading time, and in order to do that hardware_enable_all() will be done during module loading time too, as initializing TDX requires all cpus being in VMX operation. As a result, CPU compatibility check will be opportunistically moved to hardware_enable_nolock(), which doesn't take any argument. Instead of passing 'opaque' around to hardware_enable_nolock() and hardware_enable_all(), just remove the unused 'opaque' argument from kvm_arch_check_processor_compat().
"

Or even simpler:

"
To support TDX, hardware_enable_all() will be done during module loading time. As a result, CPU compatibility check will be opportunistically moved to hardware_enable_nolock(), which doesn't take any argument. Instead of passing 'opaque' around to hardware_enable_nolock() and hardware_enable_all(), just remove the unused 'opaque' argument from kvm_arch_check_processor_compat(). "

With changelog updated:

From: Isaku Yamahata <[email protected]>

TDX module requires its initialization. It requires VMX to be enabled. Although there are several options of when to initialize it, the choice is the initialization time of the KVM kernel module. There is no usable arch-specific hook for the TDX module to utilize during the KVM kernel module initialization. The code doesn't enable/disable hardware (VMX in TDX case) during the kernel module initialization. Add a hook for enabling hardware, arch-specific initialization, and disabling hardware during KVM kernel module initialization to make a room for TDX module initialization. The current KVM enables hardware when the first VM is created and disables hardware when the last VM is destroyed. When no VM is running, hardware is disabled. To follow these semantics, the kernel module initialization needs to disable hardware. Opportunistically refactor the code to enable/disable hardware.

Add hadware_enable_all() and hardware_disable_all() to kvm_init() and introduce a new arch-specific callback function, kvm_arch_post_hardware_enable_setup, for arch to do arch-specific initialization that requires hardware_enable_all(). Opportunistically, move kvm_arch_check_processor_compat() to to hardware_enabled_nolock(). TDX module initialization code will go into kvm_arch_post_hardware_enable_setup().

This patch reorders some function calls as below from (*) (**) (A) and (B) to (A) (B) and (*). Here (A) and (B) depends on (*), but not (**). By code inspection, only mips and VMX has the code of (*). No other

arch has empty (*). So refactor mips and VMX and eliminate the necessity hook for (*) instead of adding an unused hook.

Before this patch:
- Arch module initialization
- kvm_init()
- kvm_arch_init()
- kvm_arch_check_processor_compat() on each CPUs
- post-arch-specific initialization -- (*): (A) and (B) depends on this
- post-arch-specific initialization -- (**): no dependency to (A) and (B)

- When creating/deleting the first/last VM
- kvm_arch_hardware_enable() on each CPUs -- (A)
- kvm_arch_hardware_disable() on each CPUs -- (B)

After this patch:
- Arch module initialization
- kvm_init()
- kvm_arch_init()
- arch-specific initialization -- (*)
- kvm_arch_check_processor_compat() on each CPUs
- kvm_arch_hardware_enable() on each CPUs -- (A)

- kvm_arch_hardware_disable() on each CPUs -- (B)
- post-arch-specific initialization -- (**)

- When creating/deleting the first/last VM (no logic change)
- kvm_arch_hardware_enable() on each CPUs -- (A)
- kvm_arch_hardware_disable() on each CPUs -- (B)

From: Isaku Yamahata <[email protected]>

TDX module requires its initialization. It requires VMX to be enabled. Although there are several options of when to initialize it, the choice is the initialization time of the KVM kernel module. There is no usable arch-specific hook for the TDX module to utilize during the KVM kernel module initialization. The code doesn't enable/disable hardware (VMX in TDX case) during the kernel module initialization. Add a hook for enabling hardware, arch-specific initialization, and disabling hardware during KVM kernel module initialization to make a room for TDX module initialization.

The
current KVM enables hardware when the first VM is created and disables hardware when the last VM is destroyed. When no VM is running, hardware is disabled. To follow these semantics, the kernel module initialization needs to disable hardware. Opportunistically refactor the code to enable/disable hardware.

Add hadware_enable_all() and hardware_disable_all() to kvm_init() and introduce a new arch-specific callback function, kvm_arch_post_hardware_enable_setup, for arch to do arch-specific initialization that requires hardware_enable_all(). Opportunistically, move kvm_arch_check_processor_compat() to to hardware_enabled_nolock(). TDX module initialization code will go into kvm_arch_post_hardware_enable_setup().

This patch reorders some function calls as below from (*) (**) (A) and (B) to (A) (B) and (*). Here (A) and (B) depends on (*), but not (**). By code inspection, only mips and VMX has the code of (*). No other arch has empty (*). So refactor mips and VMX and eliminate the necessity hook for (*) instead of adding an unused hook.

Before this patch:
- Arch module initialization
- kvm_init()
- kvm_arch_init()
- kvm_arch_check_processor_compat() on each CPUs
- post-arch-specific initialization -- (*): (A) and (B) depends on this
- post-arch-specific initialization -- (**): no dependency to (A) and (B)

- When creating/deleting the first/last VM
- kvm_arch_hardware_enable() on each CPUs -- (A)
- kvm_arch_hardware_disable() on each CPUs -- (B)

After this patch:
- Arch module initialization
- kvm_init()
- kvm_arch_init()
- arch-specific initialization -- (*)
- kvm_arch_check_processor_compat() on each CPUs
- kvm_arch_hardware_enable() on each CPUs -- (A)
- kvm_arch_hardware_disable() on each CPUs -- (B)
- post-arch-specific initialization -- (**)

- When creating/deleting the first/last VM (no logic change)
- kvm_arch_hardware_enable() on each CPUs -- (A)
- kvm_arch_hardware_disable() on each CPUs -- (B)

Code inspection result:
As long as I inspected, I found only mips and VMX have non-empty (*) or non-empty (A) or (B).
x86: tested on a real machine
mips: compile test only
powerpc, s390, arm, riscv: code inspection only

- arch/mips/kvm/mips.c
module init function, kvm_mips_init(), does some initialization after kvm_init(). Compile test only.

- arch/x86/kvm/x86.c
- uses vm_list which is statically initialized.
- static_call(kvm_x86_hardware_enable)();
- SVM: (*) and (**) are empty.
- VMX: initialize percpu variable loaded_vmcss_on_cpu that VMXON uses.

- arch/powerpc/kvm/powerpc.c
kvm_arch_hardware_enable/disable() are nop

- arch/s390/kvm/kvm-s390.c
kvm_arch_hardware_enable/disable() are nop

- arch/arm64/kvm/arm.c
module init function, arm_init(), calls only kvm_init(). (*) and (**) are empty

- arch/riscv/kvm/main.c
module init function, riscv_kvm_init(), calls only kvm_init(). (*) and (**) are empty

Co-developed-by Sean Christopherson <[email protected]>

Signed-off-by Sean Christopherson <[email protected]>

Signed-off-by Isaku Yamahata <[email protected]>

---
 arch/mips/kvm/mips.c     | 16 +++++++++++-----
 arch/x86/kvm/vmx/vmx.c   | 23 +++++++++++++++++++----
 include/linux/kvm_host.h |  1 +
 virt/kvm/kvm_main.c      | 31 ++++++++++++++++++++++++-------
 4 files changed, 55 insertions(+), 16 deletions(-)

arch/mips/kvm/mips.c +0 -0

index 092d09fb6a7e..fd7339cff57c 100644

arch/mips/kvm/mips.c +7 -5

 	ret = kvm_mips_entry_setup();
-	if (ret)
-		return ret;
-
-	ret = kvm_init(NULL, sizeof(struct kvm_vcpu), 0, THIS_MODULE);
-
 	if (ret)
 		return ret;
 

@@ -1656,6 +1655,13 @@ static int __init kvm_mips_init(void)
 
 	register_die_notifier(&kvm_mips_csr_die_notifier);
 
+	ret = kvm_init(NULL, sizeof(struct kvm_vcpu), 0, THIS_MODULE);
+
+	if (ret) {
+		unregister_die_notifier(&kvm_mips_csr_die_notifier);
+		return ret;
+	}
+
 	return 0;
 }
 

arch/x86/kvm/vmx/vmx.c +19 -4

index c5637a4c1c43..4d7e1073984d 100644
@@ -8433,6 +8433,23 @@ static void vmx_exit(void)
 }
 module_exit(vmx_exit);
 
+/*
+ * Early initialization before kvm_init() so that vmx_hardware_enable/disable()
+ * can work.
+ */
+static void __init vmx_init_early(void)
+{
+	int cpu;
+
+	/*
+	 * vmx_hardware_disable() accesses loaded_vmcss_on_cpu list.
+	 * Initialize the variable before kvm_init() that calls
+	 * vmx_hardware_enable/disable().
+	 */
+	for_each_possible_cpu(cpu)
+		INIT_LIST_HEAD(&per_cpu(loaded_vmcss_on_cpu, cpu));
+}
+
 static int __init vmx_init(void)
 {
 	int r, cpu;

@@ -8470,6 +8487,7 @@ static int __init vmx_init(void)
 	}
 #endif
 
+	vmx_init_early();
 	r = kvm_init(&vmx_init_ops, sizeof(struct vcpu_vmx),
 		     __alignof__(struct vcpu_vmx), THIS_MODULE);
 	if (r)

@@ -8490,11 +8508,8 @@ static int __init vmx_init(void)
 
 	vmx_setup_fb_clear_ctrl();
 
-	for_each_possible_cpu(cpu) {
-		INIT_LIST_HEAD(&per_cpu(loaded_vmcss_on_cpu, cpu));
-
+	for_each_possible_cpu(cpu)
 		pi_init_cpu(cpu);
-	}
 
 #ifdef CONFIG_KEXEC_CORE
 	rcu_assign_pointer(crash_vmclear_loaded_vmcss,

include/linux/kvm_host.h +1 -0

index 9643b8eadefe..63d4e3ce3e53 100644
@@ -1437,6 +1437,7 @@ static inline void kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu) {}
 int kvm_arch_hardware_enable(void);
 void kvm_arch_hardware_disable(void);
 int kvm_arch_hardware_setup(void *opaque);
+int kvm_arch_post_hardware_enable_setup(void *opaque);
 void kvm_arch_hardware_unsetup(void);
 int kvm_arch_check_processor_compat(void);
 int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu);

virt/kvm/kvm_main.c +13 -7

index 0f5767e5ae45..a43fdbfaede2 100644
@@ -4995,8 +4995,13 @@ static void hardware_enable_nolock(void *junk)
 
 	cpumask_set_cpu(cpu, cpus_hardware_enabled);
 
+	r = kvm_arch_check_processor_compat();
+	if (r)
+		goto out;
+
 	r = kvm_arch_hardware_enable();
 
+out:
 	if (r) {
 		cpumask_clear_cpu(cpu, cpus_hardware_enabled);
 		atomic_inc(&hardware_enable_failed);

@@ -5793,9 +5798,9 @@ void kvm_unregister_perf_callbacks(void)
 }
 #endif
 
-static void check_processor_compat(void *rtn)
+__weak int kvm_arch_post_hardware_enable_setup(void *opaque)
 {
-	*(int *)rtn = kvm_arch_check_processor_compat();
+	return 0;
 }
 
 int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,

@@ -5828,11 +5833,23 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
 	if (r < 0)
 		goto out_free_1;
 
-	for_each_online_cpu(cpu) {
-		smp_call_function_single(cpu, check_processor_compat, &r, 1);
-		if (r < 0)
-			goto out_free_2;
-	}
+	/* hardware_enable_nolock() checks CPU compatibility on each CPUs. */
+	r = hardware_enable_all();
+	if (r)
+		goto out_free_2;
+	/*
+	 * Arch specific initialization that requires to enable virtualization

First of all, I think the patch title can be improved. "refactor CPU compatibility check on module initialization" isn't the purpose of this patch. It is just a bonus. The title should reflect the main purpose (or behaviour) of this patch:

KVM: Temporarily enable hardware on all cpus during module loading time

Quoted 6 lines

virt/kvm/kvm_main.c +6 -0

+	/* hardware_enable_nolock() checks CPU compatibility on each CPUs. */
+	r = hardware_enable_all();
+	if (r)
+		goto out_free_2;
+	/*
+	 * Arch specific initialization that requires to enable virtualization

virt/kvm/kvm_main.c int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align, · line 5851

Quoted 10 lines

virt/kvm/kvm_main.c +10 -0

+	 * feature.  e.g. TDX module initialization requires VMXON on all
+	 * present CPUs.
+	 */
+	kvm_arch_post_hardware_enable_setup(opaque);
+	/*
+	 * Make hardware disabled after the KVM module initialization.  KVM
+	 * enables hardware when the first KVM VM is created and disables
+	 * hardware when the last KVM VM is destroyed.  When no KVM VM is
+	 * running, hardware is disabled.  Keep that semantics.
+	 */

Except the first sentence, the remaining sentences are more like changelog material. Perhaps just say something below to be more specific on the purpose:

/*
* Disable hardware on all cpus so that out-of-tree drivers which
* also use hardware-assisted virtualization (such as virtualbox
* kernel module) can still be loaded when KVM is loaded. */

virt/kvm/kvm_main.c int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align, · line 5855

Quoted 4 lines

virt/kvm/kvm_main.c +1 -0

+	hardware_disable_all();
 
 	r = cpuhp_setup_state_nocalls(CPUHP_AP_KVM_STARTING, "kvm/cpu:starting",
 				      kvm_starting_cpu, kvm_dying_cpu);

+Will (for arm crud)

Quoted 6 lines · Kai

First of all, I think the patch title can be improved. "refactor CPU compatibility check on module initialization" isn't the purpose of this patch. It is just a bonus. The title should reflect the main purpose (or behaviour) of this patch:

KVM: Temporarily enable hardware on all cpus during module loading time

...

Quoted 26 lines

Quoted 6 lines

virt/kvm/kvm_main.c +6 -0

+	/* hardware_enable_nolock() checks CPU compatibility on each CPUs. */
+	r = hardware_enable_all();
+	if (r)
+		goto out_free_2;
+	/*
+	 * Arch specific initialization that requires to enable virtualization

virt/kvm/kvm_main.c int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align, · line 5851

Quoted 10 lines

virt/kvm/kvm_main.c +10 -0

+	 * feature.  e.g. TDX module initialization requires VMXON on all
+	 * present CPUs.
+	 */
+	kvm_arch_post_hardware_enable_setup(opaque);
+	/*
+	 * Make hardware disabled after the KVM module initialization.  KVM
+	 * enables hardware when the first KVM VM is created and disables
+	 * hardware when the last KVM VM is destroyed.  When no KVM VM is
+	 * running, hardware is disabled.  Keep that semantics.
+	 */

Except the first sentence, the remaining sentences are more like changelog material. Perhaps just say something below to be more specific on the purpose:

/*
* Disable hardware on all cpus so that out-of-tree drivers which
* also use hardware-assisted virtualization (such as virtualbox
* kernel module) can still be loaded when KVM is loaded. */

Quoted 4 lines

virt/kvm/kvm_main.c int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align, · line 5855

Quoted 4 lines

virt/kvm/kvm_main.c +1 -0

+	hardware_disable_all();
 
 	r = cpuhp_setup_state_nocalls(CPUHP_AP_KVM_STARTING, "kvm/cpu:starting",
 				      kvm_starting_cpu, kvm_dying_cpu);

I've been poking at the "hardware enable" code this week for other reasons, and have come to the conclusion that the current implementation is a mess.

x86 overloads "hardware enable" to do three different things:

1. actually enable hardware
2. snapshot per-CPU MSR value for user-return MSRs
3. handle unstable TSC _for existing VMs_ on suspend+resume and/or CPU hotplug

#2 and #3 have nothing to do with enabling hardware, kvm_arch_hardware_enable() just so happens to be called in a superset of what is needed for dealing with unstable TSCs, and AFAICT the user-return MSRs is simply a historical wart. The user-return MSRs code is subtly very, very nasty, as it means that KVM snaphots MSRs from IRQ context, e.g. if an out-of-tree module is running VMs, the IRQ can interrupt the _guest_ and cause KVM to snapshot guest registers. VMX and SVM kinda sorta guard against this by refusing to load if VMX/SVM are already enabled, but it's not foolproof.

Eww, and #3 is broken. If CPU (un)hotplug collides with kvm_destroy_vm() or kvm_create_vm(), kvm_arch_hardware_enable() could explode due to vm_list being modified while it's being walked.

Of course, that path is broken for other reasons too, e.g. needs to prevent CPUs from going on/off-line when KVM is enabling hardware.
https://lore.kernel.org/all/[email protected]

arm64 is also quite evil and circumvents KVM's hardware enabling logic to some extent. kvm_arch_init() => init_subsystems() unconditionally enables hardware, and for pKVM _leaves_ hardware enabled. And then hyp_init_cpu_pm_notifier() disables/enables hardware across lower power enter+exit, except if pKVM is enabled. The icing on the cake is "disabling" hardware doesn't even do anything (AFAICT) if the kernel is running at EL2 (which I think is nVHE + not-pKVM?).

PPC apparently didn't want to be left out of the party, and despite having a nop for kvm_arch_hardware_disable(), it does its own "is KVM enabled" tracking (see kvm_hv_vm_(de)activated()). At least PPC gets the cpus_read_(un)lock() stuff right...

MIPS doesn't appear to have any shenanigans, but kvm_vz_hardware_enable() appears to be a "heavy" operation, i.e. ideally not something that should be done spuriously.

s390 and PPC are the only sane architectures and don't require explicit enabling of virtualization.

At a glance, arm64 won't explode, but enabling hardware _twice_ during kvm_init() is all kinds of gross.

Another wart that we can clean up is the cpus_hardware_enabled mask. I don't see any reason KVM needs to use a global mask, a per-cpu variable a la kvm_arm_hardware_enabled would do just fine.

OMG, and there's another bug lurking (I need to stop looking at this code). Commit 5f6de5cbebee ("KVM: Prevent module exit until all VMs are freed") added an error path that can cause VM creation to fail _after_ it has been added to the list, but doesn't unwind _any_ of the stuff done by kvm_arch_post_init_vm() and beyond.

Rather than trying to rework common KVM to fit all the architectures random needs, I think we should instead overhaul the entire mess. And we should do that ASAP ahead of TDX, though obviously with an eye toward not sucking for TDX.

Not 100% thought out at this point, but I think we can do:

1. Have x86 snapshot per-CPU user-return MRS on first use (trivial to do by adding a flag to struct kvm_user_return_msrs, as user_return_msrs is already per-CPU).

2. Drop kvm_count_lock and instead protect kvm_usage_count with kvm_lock and

      cpu_read_lock().

3. Provide arch hooks that are invoked for "power management" operations (including CPU hotplug and host reboot, hence the quotes). Note, there's both a platform- wide PM notifier and a per-CPU notifier...

4. Rename kvm_arch_post_init_vm() to e.g. kvm_arch_add_vm(), call it under kvm_lock, and pass in kvm_usage_count.

5a. Drop cpus_hardware_enabled and drop the common hardware enable/disable code.

or

5b. Expose kvm_hardware_enable_all() and/or kvm_hardware_enable() so that archs
don't need to implement their own error handling and per-CPU flags.

I.e. give each architecture hooks to handle possible transition points, but otherwise let arch code decide when and how to do hardware enabling/disabling.

I'm very tempted to vote for (5a); x86 is the only architecture has an error path in kvm_arch_hardware_enable(), and trying to get common code to play nice with arm's kvm_arm_hardware_enabled logic is probably going to be weird.

E.g. if we can get the back half kvm_create_vm() to look like the below, then arch code can enable hardware during kvm_arch_add_vm() if the existing count is zero without generic KVM needing to worry about when hardware needs to be enabled and disabled.

	r = kvm_arch_init_vm(kvm, type);
	if (r)
		goto out_err_no_arch_destroy_vm;

	r = kvm_init_mmu_notifier(kvm);
	if (r)
		goto out_err_no_mmu_notifier;

	/*
	 * When the fd passed to this ioctl() is opened it pins the module,
	 * but try_module_get() also prevents getting a reference if the module
	 * is in MODULE_STATE_GOING (e.g. if someone ran "rmmod --wait").
	 */

	if (!try_module_get(kvm_chardev_ops.owner)) {
		r = -ENODEV;
		goto out_err;
	}

	mutex_lock(&kvm_lock);
	cpus_read_lock();
	r = kvm_arch_add_vm(kvm, kvm_usage_count);

	if (r)
		goto out_final;
	kvm_usage_count++;
	list_add(&kvm->vm_list, &vm_list);
	cpus_read_unlock();
	mutex_unlock(&kvm_lock);

	if (r)
		goto out_put_module;

	preempt_notifier_inc();
	kvm_init_pm_notifier(kvm);

	return kvm;

out_final:

	cpus_read_unlock();
	mutex_unlock(&kvm_lock);
	module_put(kvm_chardev_ops.owner);
out_err_no_put_module:
#if defined(CONFIG_MMU_NOTIFIER) && defined(KVM_ARCH_WANT_MMU_NOTIFIER)
	if (kvm->mmu_notifier.ops)
		mmu_notifier_unregister(&kvm->mmu_notifier, current->mm);
#endif
out_err_no_mmu_notifier:
	kvm_arch_destroy_vm(kvm);

Quoted 2 lines · Sean Christopherson

I've been poking at the "hardware enable" code this week for other reasons, and have come to the conclusion that the current implementation is a mess.

Thanks for the lengthy reply :)

First of all, to clarify, I guess by "current implementation" you mean the current upstream KVM code, but not this particular patch? :)

Quoted 3 lines

Of course, that path is broken for other reasons too, e.g. needs to prevent CPUs from going on/off-line when KVM is enabling hardware.
https://lore.kernel.org/all/[email protected]

If I read correctly, the problem described in above link seems only to be true after we move CPUHP_AP_KVM_STARTING from STARTING section to ONLINE section, but this hasn't been done yet in the current upstream KVM. Currently, CPUHP_AP_KVM_STARTING is still in STARTING section so it is guaranteed it has been executed before start_secondary sets itself to online cpu mask.

Btw I saw v4 of Chao's patchset was sent Feb this year. It seems that series indeed improved CPU compatibility check and hotplug handling. Any reason that series wasn't merged?

Also agreed that kvm_lock should be used. But I am not sure whether cpus_read_lock() is needed (whether CPU hotplug should be prevented). In current KVM, we don't do CPU compatibility check for hotplug CPU anyway, so when KVM does CPU compatibility check using for_each_online_cpu(), if CPU hotplug (hot-removal) happens, the worst case is we lose compatibility check on that CPU.

Or perhaps I am missing something?

Quoted 3 lines

	mutex_lock(&kvm_lock);
	cpus_read_lock();
	r = kvm_arch_add_vm(kvm, kvm_usage_count);

Holding cpus_read_lock() here implies CPU hotplug cannot happen during kvm_arch_add_vm(). This needs a justification/comment to explain why.

Also, assuming we have a justification, since (based on your description above) arch _may_ choose to enable hardware within it, but it is not a _must_. So maybe remove cpus_read_lock() here and let kvm_arch_add_vm() to decide whether to use it?

Quoted 8 lines · Kai

Quoted 2 lines · Sean Christopherson

I've been poking at the "hardware enable" code this week for other reasons, and have come to the conclusion that the current implementation is a mess.

Thanks for the lengthy reply :)

First of all, to clarify, I guess by "current implementation" you mean the current upstream KVM code, but not this particular patch? :)

Yeah, upstream code.

Quoted 9 lines

Quoted 3 lines

Of course, that path is broken for other reasons too, e.g. needs to prevent CPUs from going on/off-line when KVM is enabling hardware.
https://lore.kernel.org/all/[email protected]

If I read correctly, the problem described in above link seems only to be true after we move CPUHP_AP_KVM_STARTING from STARTING section to ONLINE section, but this hasn't been done yet in the current upstream KVM. Currently, CPUHP_AP_KVM_STARTING is still in STARTING section so it is guaranteed it has been executed before start_secondary sets itself to online cpu mask.

The lurking issue is that for_each_online_cpu() can against hotplug, i.e. every instance of for_each_online_cpu() in KVM is buggy (at least on the x86 side, I can't tell at a glance whether or not arm pKVM's usage is safe).

https://lore.kernel.org/all/87bl20aa72.ffs@tglx

Quoted 4 lines

Btw I saw v4 of Chao's patchset was sent Feb this year. It seems that series indeed improved CPU compatibility check and hotplug handling. Any reason that series wasn't merged?

AFAIK it was just a lack of reviews/acks for the non-KVM patches.

Quoted 7 lines

Also agreed that kvm_lock should be used. But I am not sure whether cpus_read_lock() is needed (whether CPU hotplug should be prevented). In current KVM, we don't do CPU compatibility check for hotplug CPU anyway, so when KVM does CPU compatibility check using for_each_online_cpu(), if CPU hotplug (hot-removal) happens, the worst case is we lose compatibility check on that CPU.

Quoted 1 line

Or perhaps I am missing something?

On a hot-add of an incompatible CPU, KVM would potentially skip the compatibility check and try to enable hardware on an incompatible/broken CPU.

Another possible bug is the checking of hv_get_vp_assist_page(); hot-adding a CPU that failed to allocate the VP assist page while vmx_init() is checking online CPUs could result in a NULL pointer deref due to KVM not rejecting the CPU as it should.

Quoted 14 lines

3. Provide arch hooks that are invoked for "power management" operations (including CPU hotplug and host reboot, hence the quotes). Note, there's both a platform- wide PM notifier and a per-CPU notifier...

4. Rename kvm_arch_post_init_vm() to e.g. kvm_arch_add_vm(), call it under kvm_lock, and pass in kvm_usage_count.

5a. Drop cpus_hardware_enabled and drop the common hardware enable/disable code.

or

5b. Expose kvm_hardware_enable_all() and/or kvm_hardware_enable() so that archs
don't need to implement their own error handling and per-CPU flags.

Quoted 7 lines

I.e. give each architecture hooks to handle possible transition points, but otherwise let arch code decide when and how to do hardware enabling/disabling.

I'm very tempted to vote for (5a); x86 is the only architecture has an error path in kvm_arch_hardware_enable(), and trying to get common code to play nice with arm's kvm_arm_hardware_enabled logic is probably going to be weird.

Quoted 18 lines

E.g. if we can get the back half kvm_create_vm() to look like the below, then arch code can enable hardware during kvm_arch_add_vm() if the existing count is zero without generic KVM needing to worry about when hardware needs to be enabled and disabled.

	r = kvm_arch_init_vm(kvm, type);
	if (r)
		goto out_err_no_arch_destroy_vm;

	r = kvm_init_mmu_notifier(kvm);
	if (r)
		goto out_err_no_mmu_notifier;

	/*
	 * When the fd passed to this ioctl() is opened it pins the module,
	 * but try_module_get() also prevents getting a reference if the module
	 * is in MODULE_STATE_GOING (e.g. if someone ran "rmmod --wait").
	 */

Quoted 8 lines

	if (!try_module_get(kvm_chardev_ops.owner)) {
		r = -ENODEV;
		goto out_err;
	}

	mutex_lock(&kvm_lock);
	cpus_read_lock();
	r = kvm_arch_add_vm(kvm, kvm_usage_count);

Holding cpus_read_lock() here implies CPU hotplug cannot happen during kvm_arch_add_vm(). This needs a justification/comment to explain why.

Also, assuming we have a justification, since (based on your description above) arch _may_ choose to enable hardware within it, but it is not a _must_. So maybe remove cpus_read_lock() here and let kvm_arch_add_vm() to decide whether to use it?

+Will (for arm crud)

arm64 is also quite evil and circumvents KVM's hardware enabling logic to some extent. kvm_arch_init() => init_subsystems() unconditionally enables hardware, and for pKVM _leaves_ hardware enabled. And then hyp_init_cpu_pm_notifier() disables/enables hardware across lower power enter+exit, except if pKVM is enabled. The icing on the cake is "disabling" hardware doesn't even do anything (AFAICT) if the kernel is running at EL2 (which I think is nVHE + not-pKVM?).

Sean,

Quoted 2 lines · Sean Christopherson

+Will (for arm crud)

When it comes to KVM/arm64, I'd appreciate if you could Cc me.

Quoted 8 lines

arm64 is also quite evil and circumvents KVM's hardware enabling logic to some extent. kvm_arch_init() => init_subsystems() unconditionally enables hardware, and for pKVM _leaves_ hardware enabled. And then hyp_init_cpu_pm_notifier() disables/enables hardware across lower power enter+exit, except if pKVM is enabled. The icing on the cake is "disabling" hardware doesn't even do anything (AFAICT) if the kernel is running at EL2 (which I think is nVHE + not-pKVM?).

In the cases where disabling doesn't do anything (which are the exact opposite of the cases you describe), that's because there is absolutely *nothing* to do:

- If VHE, the kernel is the bloody hypervisor: disable virtualisation, kill the kernel.

- if pKVM, the kernel is basically a guest, and has no business touching anything at all.

So much the 'evil' behaviour.

From: Sean Christopherson <[email protected]>

KVM accesses Virtual Machine Control Structure (VMCS) with VMX instructions to operate on VM. TDX defines its data structure and TDX SEAMCALL APIs for VMM to operate on Trust Domain (TD) instead.

Trust Domain Virtual Processor State (TDVPS) is the root control structure of a TD VCPU. It helps the TDX module control the operation of the VCPU, and holds the VCPU state while the VCPU is not running. TDVPS is opaque to software and DMA access, accessible only by using the TDX module interface functions (such as TDH.VP.RD, TDH.VP.WR ,..). TDVPS includes TD VMCS, and TD VMCS auxiliary structures, such as virtual APIC page, virtualization exception information, etc. TDVPS is composed of Trust Domain Virtual Processor Root (TDVPR) which is the root page of TDVPS and Trust Domain Virtual Processor eXtension (TDVPX) pages which extend TDVPR to help provide enough physical space for the logical TDVPS structure.

Also, we have a new structure, Trust Domain Control Structure (TDCS) is the main control structure of a guest TD, and encrypted (using the guest TD's ephemeral private key). At a high level, TDCS holds information for controlling TD operation as a whole, execution, EPTP, MSR bitmaps, etc. KVM needs to set it up. Note that MSR bitmaps are held as part of TDCS (unlike VMX) because they are meant to have the same value for all VCPUs of the same TD. TDCS is a multi-page logical structure composed of multiple Trust Domain Control Extension (TDCX) physical pages. Trust Domain Root (TDR) is the root control structure of a guest TD and is encrypted using the TDX global private key. It holds a minimal set of state variables that enable guest TD control even during times when the TD's private key is not known, or when the TD's key management state does not permit access to memory encrypted using the TD's private key.

The following shows the relationship between those structures.

         TDR--> TDCS                     per-TD
          |       \--> TDCX
          \
           \--> TDVPS                    per-TD VCPU
                  \--> TDVPR and TDVPX

The existing global struct kvm_x86_ops already defines an interface which fits with TDX. But kvm_x86_ops is system-wide, not per-VM structure. To allow VMX to coexist with TDs, the kvm_x86_ops callbacks will have wrappers "if (tdx) tdx_op() else vmx_op()" to switch VMX or TDX at run time.

To split the runtime switch, the VMX implementation, and the TDX implementation, add main.c, and move out the vmx_x86_ops hooks in preparation for adding TDX, which can coexist with VMX, i.e. KVM can run both VMs and TDs. Use 'vt' for the naming scheme as a nod to VT-x and as a concatenation of VmxTdx.

The current code looks as follows.
In vmx.c
static vmx_op() { ... }

   static struct kvm_x86_ops vmx_x86_ops = {
         .op = vmx_op,

initialization code

The eventually converted code will look like
In vmx.c, keep the VMX operations.
vmx_op() { ... }
VMX initialization
In tdx.c, define the TDX operations.
tdx_op() { ... }
TDX initialization
In x86_ops.h, declare the VMX and TDX operations.

   vmx_op();
   tdx_op();

In main.c, define common wrappers for VMX and VMX.

   static vt_ops() { if (tdx) tdx_ops() else vmx_ops() }
   static struct kvm_x86_ops vt_x86_ops = {
         .op = vt_op,
   initialization to call VMX and TDX initialization

Opportunistically, fix the name inconsistency from vmx_create_vcpu() and vmx_free_vcpu() to vmx_vcpu_create() and vxm_vcpu_free().

Co-developed-by Xiaoyao Li <[email protected]>

Signed-off-by Xiaoyao Li <[email protected]>

Signed-off-by Sean Christopherson <[email protected]>

Signed-off-by Isaku Yamahata <[email protected]>

Reviewed-by Paolo Bonzini <[email protected]>

From: Sean Christopherson <[email protected]>

KVM accesses Virtual Machine Control Structure (VMCS) with VMX instructions to operate on VM. TDX defines its data structure and TDX SEAMCALL APIs for

VMM to operate on Trust Domain (TD) instead.

Trust Domain Virtual Processor State (TDVPS) is the root control structure of a TD VCPU. It helps the TDX module control the operation of the VCPU, and holds the VCPU state while the VCPU is not running. TDVPS is opaque to software and DMA access, accessible only by using the TDX module interface functions (such as TDH.VP.RD, TDH.VP.WR ,..). TDVPS includes TD VMCS, and TD VMCS auxiliary structures, such as virtual APIC page, virtualization exception information, etc. TDVPS is composed of Trust Domain Virtual Processor Root (TDVPR) which is the root page of TDVPS and Trust Domain Virtual Processor eXtension (TDVPX) pages which extend TDVPR to help provide enough physical space for the logical TDVPS structure.

Also, we have a new structure, Trust Domain Control Structure (TDCS) is the main control structure of a guest TD, and encrypted (using the guest TD's ephemeral private key). At a high level, TDCS holds information for controlling TD operation as a whole, execution, EPTP, MSR bitmaps, etc. KVM needs to set it up. Note that MSR bitmaps are held as part of TDCS (unlike VMX) because they are meant to have the same value for all VCPUs of the same TD. TDCS is a multi-page logical structure composed of multiple Trust Domain Control Extension (TDCX) physical pages. Trust Domain Root (TDR) is the root control structure of a guest TD and is encrypted using the TDX global private key. It holds a minimal set of state variables that enable guest TD control even during times when the TD's private key is not known, or when the TD's key management state does not permit access to memory encrypted using the TD's private key.

The following shows the relationship between those structures.

        TDR--> TDCS                     per-TD
         |       \--> TDCX
         \
          \--> TDVPS                    per-TD VCPU
                 \--> TDVPR and TDVPX

The existing global struct kvm_x86_ops already defines an interface which fits with TDX. But kvm_x86_ops is system-wide, not per-VM structure. To allow VMX to coexist with TDs, the kvm_x86_ops callbacks will have wrappers "if (tdx) tdx_op() else vmx_op()" to switch VMX or TDX at run time.

To split the runtime switch, the VMX implementation, and the TDX implementation, add main.c, and move out the vmx_x86_ops hooks in preparation for adding TDX, which can coexist with VMX, i.e. KVM can run both VMs and TDs. Use 'vt' for the naming scheme as a nod to VT-x and as a concatenation of VmxTdx.

The current code looks as follows.
In vmx.c
static vmx_op() { ... }

  static struct kvm_x86_ops vmx_x86_ops = {
        .op = vmx_op,

initialization code

The eventually converted code will look like In vmx.c, keep the VMX operations.
vmx_op() { ... }
VMX initialization
In tdx.c, define the TDX operations.
tdx_op() { ... }
TDX initialization
In x86_ops.h, declare the VMX and TDX operations.
vmx_op();
tdx_op();
In main.c, define common wrappers for VMX and VMX.
static vt_ops() { if (tdx) tdx_ops() else vmx_ops() }

  static struct kvm_x86_ops vt_x86_ops = {
        .op = vt_op,

initialization to call VMX and TDX initialization

Opportunistically, fix the name inconsistency from vmx_create_vcpu() and vmx_free_vcpu() to vmx_vcpu_create() and vxm_vcpu_free().

Co-developed-by Xiaoyao Li <[email protected]>

Signed-off-by Xiaoyao Li <[email protected]>

Signed-off-by Sean Christopherson <[email protected]>

Signed-off-by Isaku Yamahata <[email protected]>

Reviewed-by Paolo Bonzini <[email protected]>

---
 arch/x86/kvm/Makefile      |   2 +-
 arch/x86/kvm/vmx/main.c    | 155 ++++++++++++++++
 arch/x86/kvm/vmx/vmx.c     | 363 +++++++++++--------------------------
 arch/x86/kvm/vmx/x86_ops.h | 125 +++++++++++++
 4 files changed, 386 insertions(+), 259 deletions(-)
 create mode 100644 arch/x86/kvm/vmx/main.c
 create mode 100644 arch/x86/kvm/vmx/x86_ops.h

From: Isaku Yamahata <[email protected]>

TDX KVM needs system-wide information about the TDX module, struct tdsysinfo_struct. Add a helper function tdx_get_sysinfo() to return it instead of KVM getting it with various error checks. Move out the struct definition about it to common place arch/x86/include/asm/tdx.h.

Signed-off-by Isaku Yamahata <[email protected]>

---
 arch/x86/include/asm/tdx.h  | 55 +++++++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx/tdx.c | 20 +++++++++++---
 arch/x86/virt/vmx/tdx/tdx.h | 52 -----------------------------------
 3 files changed, 71 insertions(+), 56 deletions(-)

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

index 801f6e10b2db..dfea0dd71bc1 100644
@@ -89,11 +89,66 @@ static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1,
 #endif /* CONFIG_INTEL_TDX_GUEST && CONFIG_KVM_GUEST */
 
 #ifdef CONFIG_INTEL_TDX_HOST
+struct tdx_cpuid_config {
+	u32	leaf;
+	u32	sub_leaf;
+	u32	eax;
+	u32	ebx;
+	u32	ecx;
+	u32	edx;
+} __packed;
+
+#define TDSYSINFO_STRUCT_SIZE		1024
+#define TDSYSINFO_STRUCT_ALIGNMENT	1024
+
+struct tdsysinfo_struct {
+	/* TDX-SEAM Module Info */
+	u32	attributes;
+	u32	vendor_id;
+	u32	build_date;
+	u16	build_num;
+	u16	minor_version;
+	u16	major_version;
+	u8	reserved0[14];
+	/* Memory Info */
+	u16	max_tdmrs;
+	u16	max_reserved_per_tdmr;
+	u16	pamt_entry_size;
+	u8	reserved1[10];
+	/* Control Struct Info */
+	u16	tdcs_base_size;
+	u8	reserved2[2];
+	u16	tdvps_base_size;
+	u8	tdvps_xfam_dependent_size;
+	u8	reserved3[9];
+	/* TD Capabilities */
+	u64	attributes_fixed0;
+	u64	attributes_fixed1;
+	u64	xfam_fixed0;
+	u64	xfam_fixed1;
+	u8	reserved4[32];
+	u32	num_cpuid_config;
+	/*
+	 * The actual number of CPUID_CONFIG depends on above
+	 * 'num_cpuid_config'.  The size of 'struct tdsysinfo_struct'
+	 * is 1024B defined by TDX architecture.  Use a union with
+	 * specific padding to make 'sizeof(struct tdsysinfo_struct)'
+	 * equal to 1024.
+	 */
+	union {
+		struct tdx_cpuid_config	cpuid_configs[0];
+		u8			reserved5[892];
+	};
+} __packed __aligned(TDSYSINFO_STRUCT_ALIGNMENT);
+
 bool platform_tdx_enabled(void);
 int tdx_init(void);
+const struct tdsysinfo_struct *tdx_get_sysinfo(void);
 #else	/* !CONFIG_INTEL_TDX_HOST */
 static inline bool platform_tdx_enabled(void) { return false; }
 static inline int tdx_init(void)  { return -ENODEV; }
+struct tdsysinfo_struct;
+static inline const struct tdsysinfo_struct *tdx_get_sysinfo(void) { return NULL; }
 #endif	/* CONFIG_INTEL_TDX_HOST */
 
 #endif /* !__ASSEMBLY__ */

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

index 787b26de8f53..b9567a2217df 100644
@@ -354,9 +354,9 @@ static int check_cmrs(struct cmr_info *cmr_array, int *actual_cmr_num)
 	return 0;
 }
 
-static int tdx_get_sysinfo(struct tdsysinfo_struct *tdsysinfo,
-			   struct cmr_info *cmr_array,
-			   int *actual_cmr_num)
+static int __tdx_get_sysinfo(struct tdsysinfo_struct *tdsysinfo,
+			     struct cmr_info *cmr_array,
+			     int *actual_cmr_num)
 {
 	struct tdx_module_output out;
 	u64 ret;

@@ -383,6 +383,18 @@ static int tdx_get_sysinfo(struct tdsysinfo_struct *tdsysinfo,
 	return check_cmrs(cmr_array, actual_cmr_num);
 }
 
+const struct tdsysinfo_struct *tdx_get_sysinfo(void)
+{
+       const struct tdsysinfo_struct *r = NULL;
+
+       mutex_lock(&tdx_module_lock);
+       if (tdx_module_status == TDX_MODULE_INITIALIZED)
+	       r = &tdx_sysinfo;
+       mutex_unlock(&tdx_module_lock);
+       return r;
+}
+EXPORT_SYMBOL_GPL(tdx_get_sysinfo);
+
 /*
  * Skip the memory region below 1MB.  Return true if the entire
  * region is skipped.  Otherwise, the updated range is returned.

@@ -1106,7 +1118,7 @@ static int init_tdx_module(void)
 	if (ret)
 		goto out;
 
-	ret = tdx_get_sysinfo(&tdx_sysinfo, tdx_cmr_array, &tdx_cmr_num);
+	ret = __tdx_get_sysinfo(&tdx_sysinfo, tdx_cmr_array, &tdx_cmr_num);
 	if (ret)
 		goto out;
 

arch/x86/virt/vmx/tdx/tdx.h +0 -53

index e0309558be13..c08e4ee2d0bf 100644
@@ -65,58 +65,6 @@ struct cmr_info {
 #define MAX_CMRS			32
 #define CMR_INFO_ARRAY_ALIGNMENT	512
 
-struct cpuid_config {
-	u32	leaf;
-	u32	sub_leaf;
-	u32	eax;
-	u32	ebx;
-	u32	ecx;
-	u32	edx;
-} __packed;
-
-#define TDSYSINFO_STRUCT_SIZE		1024
-#define TDSYSINFO_STRUCT_ALIGNMENT	1024
-
-struct tdsysinfo_struct {
-	/* TDX-SEAM Module Info */
-	u32	attributes;
-	u32	vendor_id;
-	u32	build_date;
-	u16	build_num;
-	u16	minor_version;
-	u16	major_version;
-	u8	reserved0[14];
-	/* Memory Info */
-	u16	max_tdmrs;
-	u16	max_reserved_per_tdmr;
-	u16	pamt_entry_size;
-	u8	reserved1[10];
-	/* Control Struct Info */
-	u16	tdcs_base_size;
-	u8	reserved2[2];
-	u16	tdvps_base_size;
-	u8	tdvps_xfam_dependent_size;
-	u8	reserved3[9];
-	/* TD Capabilities */
-	u64	attributes_fixed0;
-	u64	attributes_fixed1;
-	u64	xfam_fixed0;
-	u64	xfam_fixed1;
-	u8	reserved4[32];
-	u32	num_cpuid_config;
-	/*
-	 * The actual number of CPUID_CONFIG depends on above
-	 * 'num_cpuid_config'.  The size of 'struct tdsysinfo_struct'
-	 * is 1024B defined by TDX architecture.  Use a union with
-	 * specific padding to make 'sizeof(struct tdsysinfo_struct)'
-	 * equal to 1024.
-	 */
-	union {
-		struct cpuid_config	cpuid_configs[0];
-		u8			reserved5[892];
-	};
-} __packed __aligned(TDSYSINFO_STRUCT_ALIGNMENT);
-
 struct tdmr_reserved_area {
 	u64 offset;
 	u64 size;
--

2.25.1

From: Isaku Yamahata <[email protected]>

TDX requires several initialization steps for KVM to create guest TDs. Detect CPU feature, enable VMX (TDX is based on VMX), detect the TDX module availability, and initialize it. This patch implements those steps.

There are several options on when to initialize the TDX module. A.) kernel module loading time, B.) the first guest TD creation time. A.) was chosen. With B.), a user may hit an error of the TDX initialization when trying to create the first guest TD. The machine that fails to initialize the TDX module can't boot any guest TD further. Such failure is undesirable and a surprise because the user expects that the machine can accommodate guest TD, but actually not. So A.) is better than B.).

Introduce a module parameter, enable_tdx, to explicitly enable TDX KVM support. It's off by default to keep same behavior for those who don't use TDX. Implement hardware_setup method to detect TDX feature of CPU. Because TDX requires all present CPUs to enable VMX (VMXON). The x86 specific kvm_arch_post_hardware_enable_setup overrides the existing weak symbol of kvm_arch_post_hardware_enable_setup which is called at the KVM module initialization.

Suggested-by Sean Christopherson <[email protected]>

Signed-off-by Isaku Yamahata <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/Makefile           |  1 +
 arch/x86/kvm/vmx/main.c         | 29 ++++++++++-
 arch/x86/kvm/vmx/tdx.c          | 89 +++++++++++++++++++++++++++++++++
 arch/x86/kvm/vmx/tdx.h          |  4 ++
 arch/x86/kvm/vmx/x86_ops.h      |  6 +++
 arch/x86/kvm/x86.c              |  8 +++
 arch/x86/virt/vmx/tdx/tdx.c     |  1 +
 8 files changed, 138 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/kvm/vmx/tdx.c

arch/x86/include/asm/kvm_host.h +1 -0

index 3d000f060077..f432ad32515c 100644
@@ -1659,6 +1659,7 @@ struct kvm_x86_init_ops {
 	int (*cpu_has_kvm_support)(void);
 	int (*disabled_by_bios)(void);
 	int (*hardware_setup)(void);
+	int (*post_hardware_enable_setup)(void);
 	unsigned int (*handle_intel_pt_intr)(void);
 
 	struct kvm_x86_ops *runtime_ops;

arch/x86/kvm/Makefile +1 -0

index ee4d0999f20f..e2c05195cb95 100644
@@ -24,6 +24,7 @@ kvm-$(CONFIG_KVM_XEN)	+= xen.o
 kvm-intel-y		+= vmx/vmx.o vmx/vmenter.o vmx/pmu_intel.o vmx/vmcs12.o \
 			   vmx/evmcs.o vmx/nested.o vmx/posted_intr.o vmx/main.o
 kvm-intel-$(CONFIG_X86_SGX_KVM)	+= vmx/sgx.o
+kvm-intel-$(CONFIG_INTEL_TDX_HOST)	+= vmx/tdx.o
 
 kvm-amd-y		+= svm/svm.o svm/vmenter.o svm/pmu.o svm/nested.o svm/avic.o svm/sev.o
 

arch/x86/kvm/vmx/main.c +28 -1

index a0252cc0b48d..ac788af17d92 100644
@@ -7,6 +7,32 @@
 #include "pmu.h"
 #include "tdx.h"
 
+static bool __read_mostly enable_tdx = IS_ENABLED(CONFIG_INTEL_TDX_HOST);
+module_param_named(tdx, enable_tdx, bool, 0444);
+
+static __init int vt_hardware_setup(void)
+{
+	int ret;
+
+	ret = vmx_hardware_setup();
+	if (ret)
+		return ret;
+
+	enable_tdx = enable_tdx && !tdx_hardware_setup(&vt_x86_ops);
+
+	return 0;
+}
+
+static int __init vt_post_hardware_enable_setup(void)
+{
+	enable_tdx = enable_tdx && !tdx_module_setup();
+	/*
+	 * Even if it failed to initialize TDX module, conventional VMX is
+	 * available.  Keep VMX usable.
+	 */
+	return 0;
+}
+
 struct kvm_x86_ops vt_x86_ops __initdata = {
 	.name = "kvm_intel",
 

@@ -148,7 +174,8 @@ struct kvm_x86_ops vt_x86_ops __initdata = {
 struct kvm_x86_init_ops vt_init_ops __initdata = {
 	.cpu_has_kvm_support = vmx_cpu_has_kvm_support,
 	.disabled_by_bios = vmx_disabled_by_bios,
-	.hardware_setup = vmx_hardware_setup,
+	.hardware_setup = vt_hardware_setup,
+	.post_hardware_enable_setup = vt_post_hardware_enable_setup,
 	.handle_intel_pt_intr = NULL,
 
 	.runtime_ops = &vt_x86_ops,

arch/x86/kvm/vmx/tdx.c new file +63 -0

new file mode 100644
index 000000000000..e9a17f3666de
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/cpu.h>
+
+#include <asm/tdx.h>
+
+#include "capabilities.h"
+#include "x86_ops.h"
+#include "tdx.h"
+
+#undef pr_fmt
+#define pr_fmt(fmt) "tdx: " fmt
+
+#define TDX_MAX_NR_CPUID_CONFIGS					\
+	((sizeof(struct tdsysinfo_struct) -				\
+		offsetof(struct tdsysinfo_struct, cpuid_configs))	\
+		/ sizeof(struct tdx_cpuid_config))
+
+struct tdx_capabilities {
+	u8 tdcs_nr_pages;
+	u8 tdvpx_nr_pages;
+
+	u64 attrs_fixed0;
+	u64 attrs_fixed1;
+	u64 xfam_fixed0;
+	u64 xfam_fixed1;
+
+	u32 nr_cpuid_configs;
+	struct tdx_cpuid_config cpuid_configs[TDX_MAX_NR_CPUID_CONFIGS];
+};
+
+/* Capabilities of KVM + the TDX module. */
+static struct tdx_capabilities tdx_caps;
+
+int __init tdx_module_setup(void)
+{
+	const struct tdsysinfo_struct *tdsysinfo;
+	int ret = 0;
+
+	BUILD_BUG_ON(sizeof(*tdsysinfo) != 1024);
+	BUILD_BUG_ON(TDX_MAX_NR_CPUID_CONFIGS != 37);
+
+	ret = tdx_init();
+	if (ret) {
+		pr_info("Failed to initialize TDX module.\n");
+		return ret;
+	}
+
+	tdsysinfo = tdx_get_sysinfo();
+	if (tdsysinfo->num_cpuid_config > TDX_MAX_NR_CPUID_CONFIGS)
+		return -EIO;
+
+	tdx_caps = (struct tdx_capabilities) {
+		.tdcs_nr_pages = tdsysinfo->tdcs_base_size / PAGE_SIZE,
+		/*
+		 * TDVPS = TDVPR(4K page) + TDVPX(multiple 4K pages).
+		 * -1 for TDVPR.
+		 */
+		.tdvpx_nr_pages = tdsysinfo->tdvps_base_size / PAGE_SIZE - 1,
+		.attrs_fixed0 = tdsysinfo->attributes_fixed0,
+		.attrs_fixed1 = tdsysinfo->attributes_fixed1,
+		.xfam_fixed0 =	tdsysinfo->xfam_fixed0,
+		.xfam_fixed1 = tdsysinfo->xfam_fixed1,
+		.nr_cpuid_configs = tdsysinfo->num_cpuid_config,

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

+
+	return 0;
+}

arch/x86/kvm/vmx/tdx.h +4 -0

index 060bf48ec3d6..54d7a26ed9ee 100644
@@ -3,6 +3,8 @@
 #define __KVM_X86_TDX_H
 
 #ifdef CONFIG_INTEL_TDX_HOST
+int tdx_module_setup(void);
+
 struct kvm_tdx {
 	struct kvm kvm;
 	/* TDX specific members follow. */

@@ -37,6 +39,8 @@ static inline struct vcpu_tdx *to_tdx(struct kvm_vcpu *vcpu)
 	return container_of(vcpu, struct vcpu_tdx, vcpu);
 }
 #else
+static inline int tdx_module_setup(void) { return -ENODEV; };
+
 struct kvm_tdx {
 	struct kvm kvm;
 };

arch/x86/kvm/vmx/x86_ops.h +6 -0

index 90a8c6824833..f318a6258a24 100644
@@ -128,4 +128,10 @@ void vmx_cancel_hv_timer(struct kvm_vcpu *vcpu);
 #endif
 void vmx_setup_mce(struct kvm_vcpu *vcpu);
 
+#ifdef CONFIG_INTEL_TDX_HOST
+int __init tdx_hardware_setup(struct kvm_x86_ops *x86_ops);
+#else
+static inline int tdx_hardware_setup(struct kvm_x86_ops *x86_ops) { return 0; }
+#endif
+
 #endif /* __KVM_X86_VMX_X86_OPS_H */

arch/x86/kvm/x86.c +8 -0

index e533cce7a70b..32a2ef718112 100644
@@ -11983,6 +11983,14 @@ int kvm_arch_hardware_setup(void *opaque)
 	return 0;
 }
 
+int kvm_arch_post_hardware_enable_setup(void *opaque)
+{
+	struct kvm_x86_init_ops *ops = opaque;
+	if (ops->post_hardware_enable_setup)
+		return ops->post_hardware_enable_setup();
+	return 0;
+}
+
 void kvm_arch_hardware_unsetup(void)
 {
 	kvm_unregister_perf_callbacks();

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

index b9567a2217df..918e79159bbf 100644
@@ -1283,6 +1283,7 @@ bool platform_tdx_enabled(void)
 {
 	return tdx_keyid_num >= 2;
 }
+EXPORT_SYMBOL_GPL(platform_tdx_enabled);
 
 /**
  * tdx_init - Initialize the TDX module

Quoted 30 lines

arch/x86/kvm/vmx/tdx.c new file +30 -0

+int __init tdx_module_setup(void)
+{
+	const struct tdsysinfo_struct *tdsysinfo;
+	int ret = 0;
+
+	BUILD_BUG_ON(sizeof(*tdsysinfo) != 1024);
+	BUILD_BUG_ON(TDX_MAX_NR_CPUID_CONFIGS != 37);
+
+	ret = tdx_init();
+	if (ret) {
+		pr_info("Failed to initialize TDX module.\n");
+		return ret;
+	}
+
+	tdsysinfo = tdx_get_sysinfo();
+	if (tdsysinfo->num_cpuid_config > TDX_MAX_NR_CPUID_CONFIGS)
+		return -EIO;
+
+	tdx_caps = (struct tdx_capabilities) {
+		.tdcs_nr_pages = tdsysinfo->tdcs_base_size / PAGE_SIZE,
+		/*
+		 * TDVPS = TDVPR(4K page) + TDVPX(multiple 4K pages).
+		 * -1 for TDVPR.
+		 */
+		.tdvpx_nr_pages = tdsysinfo->tdvps_base_size / PAGE_SIZE - 1,
+		.attrs_fixed0 = tdsysinfo->attributes_fixed0,
+		.attrs_fixed1 = tdsysinfo->attributes_fixed1,
+		.xfam_fixed0 =	tdsysinfo->xfam_fixed0,
+		.xfam_fixed1 = tdsysinfo->xfam_fixed1,
+		.nr_cpuid_configs = tdsysinfo->num_cpuid_config,

arch/x86/kvm/vmx/tdx.c line 73

Quoted 10 lines

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

+	};
+	if (!memcpy(tdx_caps.cpuid_configs, tdsysinfo->cpuid_configs,
+			tdsysinfo->num_cpuid_config *
+			sizeof(struct tdx_cpuid_config)))
+		return -EIO;
+
+	return 0;
+}
+
+int __init tdx_hardware_setup(struct kvm_x86_ops *x86_ops)

Function argument isn't used.

arch/x86/kvm/vmx/tdx.c line 83

Quoted 10 lines

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

+{
+	if (!enable_ept) {
+		pr_warn("Cannot enable TDX with EPT disabled\n");
+		return -EINVAL;
+	}
+
+	if (!platform_tdx_enabled()) {
+		pr_warn("Cannot enable TDX on TDX disabled platform\n");
+		return -ENODEV;
+	}

I am not sure whether this is necessary, as tdx_init() will call it internally anyway.

arch/x86/kvm/vmx/tdx.c line 86

Quoted 3 lines

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

+
+	pr_info("kvm: TDX is supported. x86 phys bits %d\n",
+		boot_cpu_data.x86_phys_bits);

Is it a little early to say "TDX is supported" before tdx_init() is called?

I don't think the whole tdx_hardware_setup() is even necessary? Looks nothing is serious here anyway, and all staff can be done in tdx_module_setup().

From: Isaku Yamahata <[email protected]>

TDX requires several initialization steps for KVM to create guest TDs. Detect CPU feature, enable VMX (TDX is based on VMX), detect the TDX module availability, and initialize it. This patch implements those steps.

There are several options on when to initialize the TDX module. A.) kernel module loading time, B.) the first guest TD creation time. A.) was chosen. With B.), a user may hit an error of the TDX initialization when trying to create the first guest TD. The machine that fails to initialize the TDX module can't boot any guest TD further. Such failure is undesirable and a surprise because the user expects that the machine can accommodate guest TD, but actually not. So A.) is better than B.).

Introduce a module parameter, enable_tdx, to explicitly enable TDX KVM support. It's off by default to keep same behavior for those who don't use TDX. Implement hardware_setup method to detect TDX feature of CPU. Because TDX requires all present CPUs to enable VMX (VMXON). The x86 specific kvm_arch_post_hardware_enable_setup overrides the existing weak symbol of kvm_arch_post_hardware_enable_setup which is called at the KVM module initialization.

Suggested-by Sean Christopherson <[email protected]>

Signed-off-by Isaku Yamahata <[email protected]>

---
  arch/x86/include/asm/kvm_host.h |  1 +
  arch/x86/kvm/Makefile           |  1 +
  arch/x86/kvm/vmx/main.c         | 29 ++++++++++-
  arch/x86/kvm/vmx/tdx.c          | 89 +++++++++++++++++++++++++++++++++
  arch/x86/kvm/vmx/tdx.h          |  4 ++
  arch/x86/kvm/vmx/x86_ops.h      |  6 +++
  arch/x86/kvm/x86.c              |  8 +++
  arch/x86/virt/vmx/tdx/tdx.c     |  1 +
  8 files changed, 138 insertions(+), 1 deletion(-)

create mode 100644 arch/x86/kvm/vmx/tdx.c

arch/x86/include/asm/kvm_host.h +1 -0

index 3d000f060077..f432ad32515c 100644
@@ -1659,6 +1659,7 @@ struct kvm_x86_init_ops {
  	int (*cpu_has_kvm_support)(void);
  	int (*disabled_by_bios)(void);
  	int (*hardware_setup)(void);
+	int (*post_hardware_enable_setup)(void);
  	unsigned int (*handle_intel_pt_intr)(void);
 
  	struct kvm_x86_ops *runtime_ops;

arch/x86/kvm/Makefile +1 -0

index ee4d0999f20f..e2c05195cb95 100644
@@ -24,6 +24,7 @@ kvm-$(CONFIG_KVM_XEN)	+= xen.o
  kvm-intel-y		+= vmx/vmx.o vmx/vmenter.o vmx/pmu_intel.o vmx/vmcs12.o \
  			   vmx/evmcs.o vmx/nested.o vmx/posted_intr.o vmx/main.o
  kvm-intel-$(CONFIG_X86_SGX_KVM)	+= vmx/sgx.o
+kvm-intel-$(CONFIG_INTEL_TDX_HOST)	+= vmx/tdx.o
 
  kvm-amd-y		+= svm/svm.o svm/vmenter.o svm/pmu.o svm/nested.o svm/avic.o svm/sev.o
 

arch/x86/kvm/vmx/main.c +0 -0

index a0252cc0b48d..ac788af17d92 100644

arch/x86/kvm/vmx/main.c +27 -1

+module_param_named(tdx, enable_tdx, bool, 0444);
+
+static __init int vt_hardware_setup(void)
+{
+	int ret;
+
+	ret = vmx_hardware_setup();
+	if (ret)
+		return ret;
+
+	enable_tdx = enable_tdx && !tdx_hardware_setup(&vt_x86_ops);
+
+	return 0;
+}
+
+static int __init vt_post_hardware_enable_setup(void)
+{
+	enable_tdx = enable_tdx && !tdx_module_setup();
+	/*
+	 * Even if it failed to initialize TDX module, conventional VMX is
+	 * available.  Keep VMX usable.
+	 */
+	return 0;
+}
+
  struct kvm_x86_ops vt_x86_ops __initdata = {
  	.name = "kvm_intel",
 

@@ -148,7 +174,8 @@ struct kvm_x86_ops vt_x86_ops __initdata = {
  struct kvm_x86_init_ops vt_init_ops __initdata = {
  	.cpu_has_kvm_support = vmx_cpu_has_kvm_support,
  	.disabled_by_bios = vmx_disabled_by_bios,
-	.hardware_setup = vmx_hardware_setup,
+	.hardware_setup = vt_hardware_setup,
+	.post_hardware_enable_setup = vt_post_hardware_enable_setup,
  	.handle_intel_pt_intr = NULL,
 
  	.runtime_ops = &vt_x86_ops,

arch/x86/kvm/vmx/tdx.c new file +89 -0

new file mode 100644
index 000000000000..e9a17f3666de
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/cpu.h>
+
+#include <asm/tdx.h>
+
+#include "capabilities.h"
+#include "x86_ops.h"
+#include "tdx.h"
+
+#undef pr_fmt
+#define pr_fmt(fmt) "tdx: " fmt
+
+#define TDX_MAX_NR_CPUID_CONFIGS					\
+	((sizeof(struct tdsysinfo_struct) -				\
+		offsetof(struct tdsysinfo_struct, cpuid_configs))	\
+		/ sizeof(struct tdx_cpuid_config))
+
+struct tdx_capabilities {
+	u8 tdcs_nr_pages;
+	u8 tdvpx_nr_pages;
+
+	u64 attrs_fixed0;
+	u64 attrs_fixed1;
+	u64 xfam_fixed0;
+	u64 xfam_fixed1;
+
+	u32 nr_cpuid_configs;
+	struct tdx_cpuid_config cpuid_configs[TDX_MAX_NR_CPUID_CONFIGS];
+};
+
+/* Capabilities of KVM + the TDX module. */
+static struct tdx_capabilities tdx_caps;
+
+int __init tdx_module_setup(void)
+{
+	const struct tdsysinfo_struct *tdsysinfo;
+	int ret = 0;
+
+	BUILD_BUG_ON(sizeof(*tdsysinfo) != 1024);
+	BUILD_BUG_ON(TDX_MAX_NR_CPUID_CONFIGS != 37);
+
+	ret = tdx_init();
+	if (ret) {
+		pr_info("Failed to initialize TDX module.\n");
+		return ret;
+	}
+
+	tdsysinfo = tdx_get_sysinfo();
+	if (tdsysinfo->num_cpuid_config > TDX_MAX_NR_CPUID_CONFIGS)
+		return -EIO;
+
+	tdx_caps = (struct tdx_capabilities) {
+		.tdcs_nr_pages = tdsysinfo->tdcs_base_size / PAGE_SIZE,
+		/*
+		 * TDVPS = TDVPR(4K page) + TDVPX(multiple 4K pages).
+		 * -1 for TDVPR.
+		 */
+		.tdvpx_nr_pages = tdsysinfo->tdvps_base_size / PAGE_SIZE - 1,
+		.attrs_fixed0 = tdsysinfo->attributes_fixed0,
+		.attrs_fixed1 = tdsysinfo->attributes_fixed1,
+		.xfam_fixed0 =	tdsysinfo->xfam_fixed0,
+		.xfam_fixed1 = tdsysinfo->xfam_fixed1,
+		.nr_cpuid_configs = tdsysinfo->num_cpuid_config,
+	};
+	if (!memcpy(tdx_caps.cpuid_configs, tdsysinfo->cpuid_configs,
+			tdsysinfo->num_cpuid_config *
+			sizeof(struct tdx_cpuid_config)))
+		return -EIO;
+
+	return 0;
+}
+
+int __init tdx_hardware_setup(struct kvm_x86_ops *x86_ops)
+{
+	if (!enable_ept) {
+		pr_warn("Cannot enable TDX with EPT disabled\n");
+		return -EINVAL;
+	}
+
+	if (!platform_tdx_enabled()) {
+		pr_warn("Cannot enable TDX on TDX disabled platform\n");
+		return -ENODEV;
+	}
+
+	pr_info("kvm: TDX is supported. x86 phys bits %d\n",
+		boot_cpu_data.x86_phys_bits);
+
+	return 0;
+}

arch/x86/kvm/vmx/tdx.h +4 -0

index 060bf48ec3d6..54d7a26ed9ee 100644
@@ -3,6 +3,8 @@
  #define __KVM_X86_TDX_H
 
  #ifdef CONFIG_INTEL_TDX_HOST
+int tdx_module_setup(void);
+
  struct kvm_tdx {
  	struct kvm kvm;
  	/* TDX specific members follow. */

@@ -37,6 +39,8 @@ static inline struct vcpu_tdx *to_tdx(struct kvm_vcpu *vcpu)
  	return container_of(vcpu, struct vcpu_tdx, vcpu);
  }
  #else
+static inline int tdx_module_setup(void) { return -ENODEV; };
+
  struct kvm_tdx {
  	struct kvm kvm;
  };

arch/x86/kvm/vmx/x86_ops.h +6 -0

index 90a8c6824833..f318a6258a24 100644
@@ -128,4 +128,10 @@ void vmx_cancel_hv_timer(struct kvm_vcpu *vcpu);
  #endif
  void vmx_setup_mce(struct kvm_vcpu *vcpu);
 
+#ifdef CONFIG_INTEL_TDX_HOST
+int __init tdx_hardware_setup(struct kvm_x86_ops *x86_ops);
+#else
+static inline int tdx_hardware_setup(struct kvm_x86_ops *x86_ops) { return 0; }
+#endif
+
  #endif /* __KVM_X86_VMX_X86_OPS_H */

arch/x86/kvm/x86.c +8 -0

index e533cce7a70b..32a2ef718112 100644
@@ -11983,6 +11983,14 @@ int kvm_arch_hardware_setup(void *opaque)
  	return 0;
  }
 
+int kvm_arch_post_hardware_enable_setup(void *opaque)
+{
+	struct kvm_x86_init_ops *ops = opaque;
+	if (ops->post_hardware_enable_setup)
+		return ops->post_hardware_enable_setup();
+	return 0;
+}
+
  void kvm_arch_hardware_unsetup(void)
  {
  	kvm_unregister_perf_callbacks();

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

index b9567a2217df..918e79159bbf 100644
@@ -1283,6 +1283,7 @@ bool platform_tdx_enabled(void)
  {
  	return tdx_keyid_num >= 2;
  }
+EXPORT_SYMBOL_GPL(platform_tdx_enabled);
 
  /**
   * tdx_init - Initialize the TDX module

Quoted 62 lines · On 2022/8/8 6:00, [email protected] wrote:

From: Isaku Yamahata <[email protected]>

TDX requires several initialization steps for KVM to create guest TDs. Detect CPU feature, enable VMX (TDX is based on VMX), detect the TDX module availability, and initialize it. This patch implements those steps.

There are several options on when to initialize the TDX module. A.) kernel module loading time, B.) the first guest TD creation time. A.) was chosen. With B.), a user may hit an error of the TDX initialization when trying to create the first guest TD. The machine that fails to initialize the TDX module can't boot any guest TD further. Such failure is undesirable and a surprise because the user expects that the machine can accommodate guest TD, but actually not. So A.) is better than B.).

Introduce a module parameter, enable_tdx, to explicitly enable TDX KVM support. It's off by default to keep same behavior for those who don't use TDX. Implement hardware_setup method to detect TDX feature of CPU. Because TDX requires all present CPUs to enable VMX (VMXON). The x86 specific kvm_arch_post_hardware_enable_setup overrides the existing weak symbol of kvm_arch_post_hardware_enable_setup which is called at the KVM module initialization.

Suggested-by Sean Christopherson <[email protected]>

Signed-off-by Isaku Yamahata <[email protected]>

---
  arch/x86/include/asm/kvm_host.h |  1 +
  arch/x86/kvm/Makefile           |  1 +
  arch/x86/kvm/vmx/main.c         | 29 ++++++++++-
  arch/x86/kvm/vmx/tdx.c          | 89 +++++++++++++++++++++++++++++++++
  arch/x86/kvm/vmx/tdx.h          |  4 ++
  arch/x86/kvm/vmx/x86_ops.h      |  6 +++
  arch/x86/kvm/x86.c              |  8 +++
  arch/x86/virt/vmx/tdx/tdx.c     |  1 +
  8 files changed, 138 insertions(+), 1 deletion(-)

create mode 100644 arch/x86/kvm/vmx/tdx.c

arch/x86/include/asm/kvm_host.h +1 -0

index 3d000f060077..f432ad32515c 100644
@@ -1659,6 +1659,7 @@ struct kvm_x86_init_ops {
  	int (*cpu_has_kvm_support)(void);
  	int (*disabled_by_bios)(void);
  	int (*hardware_setup)(void);
+	int (*post_hardware_enable_setup)(void);
  	unsigned int (*handle_intel_pt_intr)(void);
  	struct kvm_x86_ops *runtime_ops;

arch/x86/kvm/Makefile +1 -0

index ee4d0999f20f..e2c05195cb95 100644
@@ -24,6 +24,7 @@ kvm-$(CONFIG_KVM_XEN)	+= xen.o
  kvm-intel-y		+= vmx/vmx.o vmx/vmenter.o vmx/pmu_intel.o vmx/vmcs12.o \
  			   vmx/evmcs.o vmx/nested.o vmx/posted_intr.o vmx/main.o
  kvm-intel-$(CONFIG_X86_SGX_KVM)	+= vmx/sgx.o
+kvm-intel-$(CONFIG_INTEL_TDX_HOST)	+= vmx/tdx.o
  kvm-amd-y		+= svm/svm.o svm/vmenter.o svm/pmu.o svm/nested.o svm/avic.o svm/sev.o

arch/x86/kvm/vmx/main.c +0 -0

index a0252cc0b48d..ac788af17d92 100644

arch/x86/kvm/vmx/main.c line 9

Quoted 3 lines

arch/x86/kvm/vmx/main.c +1 -0

@@ -7,6 +7,32 @@
  #include "pmu.h"
  #include "tdx.h"
+static bool __read_mostly enable_tdx = IS_ENABLED(CONFIG_INTEL_TDX_HOST);

So, if CONFIG_INTEL_TDX_HOST is opt-in in kconfig, the code will try to enable TDX.

The behavior seems a bit different from what you mentioned in the commit message about the option to "explicitly enable TDX KVM support".