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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <69352bd044fdb_1b2e10033@dwillia2-mobl4.notmuch>
Date: Sat, 6 Dec 2025 23:25:04 -0800
From: <dan.j.williams@...el.com>
To: Sean Christopherson <seanjc@...gle.com>, Thomas Gleixner
	<tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov
	<bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>, <x86@...nel.org>,
	Kiryl Shutsemau <kas@...nel.org>, Sean Christopherson <seanjc@...gle.com>,
	Paolo Bonzini <pbonzini@...hat.com>
CC: <linux-kernel@...r.kernel.org>, <linux-coco@...ts.linux.dev>,
	<kvm@...r.kernel.org>, Chao Gao <chao.gao@...el.com>, Dan Williams
	<dan.j.williams@...el.com>
Subject: Re: [PATCH v2 3/7] KVM: x86/tdx: Do VMXON and TDX-Module
 initialization during subsys init

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@...gle.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@...el.com>