Re: [PATCH v2 3/7] KVM: x86/tdx: Do VMXON and TDX-Module initialization during subsys init

dan.j.williams
2025-12-07T07:25:12Z

Sean Christopherson wrote:
> Now that VMXON can be done without bouncing through KVM, do TDX-Module
> initialization during subsys init (specifically before module_init() so
> that it runs before KVM when both are built-in). Aside from the obvious
> benefits of separating core TDX code from KVM, this will allow tagging a
> pile of TDX functions and globals as being __init and __ro_after_init.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> Documentation/arch/x86/tdx.rst | 26 -----
> arch/x86/include/asm/tdx.h | 4 -
> arch/x86/kvm/vmx/tdx.c | 169 ++++++--------------------------
> arch/x86/virt/vmx/tdx/tdx.c | 170 ++++++++++++++++++---------------
> arch/x86/virt/vmx/tdx/tdx.h | 8 --
> 5 files changed, 124 insertions(+), 253 deletions(-)

yes!

>
> @@ -3304,17 +3304,7 @@ int tdx_gmem_max_mapping_level(struct kvm *kvm, kvm_pfn_t pfn, bool is_private)
>
> static int tdx_online_cpu(unsigned int cpu)
> {
> - unsigned long flags;
> - int r;
> -
> - /* Sanity check CPU is already in post-VMXON */
> - WARN_ON_ONCE(!(cr4_read_shadow() & X86_CR4_VMXE));
> -
> - local_irq_save(flags);
> - r = tdx_cpu_enable();
> - local_irq_restore(flags);
> -
> - return r;
> + return 0;
> }

Given this routine now has nothing to do...

> + * TDX-specific cpuhp callback to disallow offlining the last CPU in a
> + * packing while KVM is running one or more TDs. Reclaiming HKIDs
> + * requires doing PAGE.WBINVD on every package, i.e. offlining all CPUs
> + * of a package would prevent reclaiming the HKID.
> */
> + r = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "kvm/cpu/tdx:online",
> + tdx_online_cpu, tdx_offline_cpu);

...the @startup param can be NULL. That also saves some grep pain no
more multiple implementations of a "tdx_online_cpu".

Along those lines, should tdx_offline_cpu() become
kvm_tdx_offline_cpu()?

[..]
> /*
> * Add a memory region as a TDX memory block. The caller must make sure
> @@ -1156,67 +1194,50 @@ static int init_tdx_module(void)
> goto out_put_tdxmem;
> }
>
> -static int __tdx_enable(void)
> +static int tdx_enable(void)

Almost commented about this being able to be __init now, but then I see
you have a combo patch for that later.

With or without the additional tdx_{on,off}line_cpu fixups:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>