Re: [RFC PATCH 12/20] x86/virt/seamldr: Shut down the current TDX module

[Nikolay Borisov <nik.borisov@suse.com>]

On 5/23/25 12:52, Chao Gao wrote:
> TD-Preserving updates request shutting down the existing TDX module.
> During this shutdown, the module generates hand-off data, which captures
> the module's states essential for preserving running TDs. The new TDX
> module can utilize this hand-off data to establish its states.
> 
> Invoke the TDH_SYS_SHUTDOWN API on one CPU to perform the shutdown. This
> API requires a hand-off module version. Use the module's own hand-off
> version, as it is the highest version the module can produce and is more
> likely to be compatible with new modules.
> 
> Changes to tdx_global_metadata.{hc} are auto-generated by following the
> instructions detailed in [1], after adding the following section to the
> tdx.py script:
> 
>      "handoff": [
>         "MODULE_HV",
>      ],
> 
> Add a check to ensure that module_hv is guarded by the TDX module's
> support for TD-Preserving.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Tested-by: Farrah Chen <farrah.chen@intel.com>
> Link: https://lore.kernel.org/kvm/20250226181453.2311849-12-pbonzini@redhat.com/ [1]
> ---
>   arch/x86/include/asm/tdx_global_metadata.h  |  5 +++++
>   arch/x86/virt/vmx/tdx/seamldr.c             | 11 +++++++++++
>   arch/x86/virt/vmx/tdx/tdx.c                 | 18 ++++++++++++++++++
>   arch/x86/virt/vmx/tdx/tdx.h                 |  4 ++++
>   arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 13 +++++++++++++
>   5 files changed, 51 insertions(+)
> 
> diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
> index ce0370f4a5b9..a2011a3575ff 100644
> --- a/arch/x86/include/asm/tdx_global_metadata.h
> +++ b/arch/x86/include/asm/tdx_global_metadata.h
> @@ -40,12 +40,17 @@ struct tdx_sys_info_td_conf {
>   	u64 cpuid_config_values[128][2];
>   };
>   
> +struct tdx_sys_info_handoff {
> +	u16 module_hv;
> +};
> +
>   struct tdx_sys_info {
>   	struct tdx_sys_info_versions versions;
>   	struct tdx_sys_info_features features;
>   	struct tdx_sys_info_tdmr tdmr;
>   	struct tdx_sys_info_td_ctrl td_ctrl;
>   	struct tdx_sys_info_td_conf td_conf;
> +	struct tdx_sys_info_handoff handoff;
>   };
>   
>   #endif
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index 9d0d37a92bfd..11c0c5a93c32 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -241,6 +241,7 @@ static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
>   
>   enum tdp_state {
>   	TDP_START,
> +	TDP_SHUTDOWN,
>   	TDP_DONE,
>   };
>   
> @@ -281,8 +282,12 @@ static void ack_state(void)
>   static int do_seamldr_install_module(void *params)
>   {
>   	enum tdp_state newstate, curstate = TDP_START;
> +	int cpu = smp_processor_id();
> +	bool primary;
>   	int ret = 0;
>   
> +	primary = !!(cpumask_first(cpu_online_mask) == cpu);

nit: the !! is not needed here, as the check is clearly boolean.

 > +>   	do {
>   		/* Chill out and ensure we re-read tdp_data. */
>   		cpu_relax();
> @@ -291,6 +296,12 @@ static int do_seamldr_install_module(void *params)
>   		if (newstate != curstate) {
>   			curstate = newstate;
>   			switch (curstate) {
> +			case TDP_SHUTDOWN:
> +				if (!primary)
> +					break;
> +
> +				ret = tdx_module_shutdown();
> +				break;
>   			default:
>   				break;
>   			}
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 22ffc15b4299..fa6b3f1eb197 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -295,6 +295,11 @@ static int read_sys_metadata_field(u64 field_id, u64 *data)
>   	return 0;
>   }
>   
> +static bool tdx_has_td_preserving(void)
> +{
> +	return tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_TD_PRESERVING;
> +}
> +
>   #include "tdx_global_metadata.c"
>   
>   static int check_features(struct tdx_sys_info *sysinfo)
> @@ -1341,6 +1346,19 @@ int tdx_enable(void)
>   }
>   EXPORT_SYMBOL_GPL(tdx_enable);
>   
> +int tdx_module_shutdown(void)
> +{
> +	struct tdx_module_args args = {};
> +
> +	/*
> +	 * Shut down TDX module and prepare handoff data for the next TDX module.
> +	 * Following a successful TDH_SYS_SHUTDOWN, further TDX module APIs will
> +	 * fail.
> +	 */
> +	args.rcx = tdx_sysinfo.handoff.module_hv;
> +	return seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
> +}
> +
>   static bool is_pamt_page(unsigned long phys)
>   {
>   	struct tdmr_info_list *tdmr_list = &tdx_tdmr_list;
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index 48c0a850c621..3830dee4da91 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -48,6 +48,7 @@
>   #define TDH_PHYMEM_PAGE_WBINVD		41
>   #define TDH_VP_WR			43
>   #define TDH_SYS_CONFIG			45
> +#define TDH_SYS_SHUTDOWN		52
>   
>   /*
>    * SEAMCALL leaf:
> @@ -87,6 +88,7 @@ struct tdmr_info {
>   } __packed __aligned(TDMR_INFO_ALIGNMENT);
>   
>   /* Bit definitions of TDX_FEATURES0 metadata field */
> +#define TDX_FEATURES0_TD_PRESERVING	BIT(1)
>   #define TDX_FEATURES0_NO_RBP_MOD	BIT(18)
>   
>   /*
> @@ -122,4 +124,6 @@ struct tdmr_info_list {
>   
>   int seamldr_prerr(u64 fn, struct tdx_module_args *args);
>   
> +int tdx_module_shutdown(void);
> +
>   #endif
> diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> index 088e5bff4025..a17cbb82e6b8 100644
> --- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> +++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> @@ -100,6 +100,18 @@ static int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *sysinfo_td_conf
>   	return ret;
>   }
>   
> +static int get_tdx_sys_info_handoff(struct tdx_sys_info_handoff *sysinfo_handoff)
> +{
> +	int ret = 0;
> +	u64 val;
> +
> +	if (!ret && tdx_has_td_preserving() &&

nit: That first !ret is redundant since it's always true.

<snip>