[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1f6956aa-5fa4-404f-bce4-3ddf87c50114@suse.com>
Date: Tue, 3 Jun 2025 15:36:36 +0300
From: Nikolay Borisov <nik.borisov@...e.com>
To: Chao Gao <chao.gao@...el.com>, linux-coco@...ts.linux.dev,
x86@...nel.org, kvm@...r.kernel.org
Cc: seanjc@...gle.com, pbonzini@...hat.com, eddie.dong@...el.com,
kirill.shutemov@...el.com, dave.hansen@...el.com, dan.j.williams@...el.com,
kai.huang@...el.com, isaku.yamahata@...el.com, elena.reshetova@...el.com,
rick.p.edgecombe@...el.com, Farrah Chen <farrah.chen@...el.com>,
Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
"H. Peter Anvin" <hpa@...or.com>,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 12/20] x86/virt/seamldr: Shut down the current TDX
module
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@...el.com>
> Tested-by: Farrah Chen <farrah.chen@...el.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>
Powered by blists - more mailing lists