Message-ID: <CAGtprH_oR44Vx9Z0cfxvq5-QbyLmy_+Gn3tWm3wzHPmC1nC0eg@mail.gmail.com>
Date: Thu, 30 Oct 2025 10:01:41 -0700
From: Vishal Annapurve <vannapurve@...gle.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: dan.j.williams@...el.com, Erdem Aktas <erdemaktas@...gle.com>, 
	Dave Hansen <dave.hansen@...el.com>, Chao Gao <chao.gao@...el.com>, 
	Elena Reshetova <elena.reshetova@...el.com>, 
	"linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "x86@...nel.org" <x86@...nel.org>, 
	Reinette Chatre <reinette.chatre@...el.com>, Ira Weiny <ira.weiny@...el.com>, 
	Kai Huang <kai.huang@...el.com>, "yilun.xu@...ux.intel.com" <yilun.xu@...ux.intel.com>, 
	"sagis@...gle.com" <sagis@...gle.com>, "paulmck@...nel.org" <paulmck@...nel.org>, 
	"nik.borisov@...e.com" <nik.borisov@...e.com>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, "H. Peter Anvin" <hpa@...or.com>, 
	Ingo Molnar <mingo@...hat.com>, "Kirill A. Shutemov" <kas@...nel.org>, Paolo Bonzini <pbonzini@...hat.com>, 
	Rick P Edgecombe <rick.p.edgecombe@...el.com>, Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v2 00/21] Runtime TDX Module update support

On Wed, Oct 29, 2025 at 6:48 AM Sean Christopherson <seanjc@...gle.com> wrote:
>
> On Tue, Oct 28, 2025, dan.j.williams@...el.com wrote:
> > Sean Christopherson wrote:
> > [..]
> > > > IMO, It is something userspace should decide, kernel's job is to
> > > > provide the necessary interface about it.
> > >
> > > I disagree, I don't think userspace should even get the option.  IMO, not setting
> > > AVOID_COMPAT_SENSITIVE is all kinds of crazy.
> >
> > Do see Table 4.4: "Comparison of Update Incompatibility Detection and/or
> > Avoidance Methods" from the latest base architecture specification [1].
> > It lists out the pros and cons of not setting AVOID_COMPAT_SENSITIVE.
> > This thread has only argued the merits of "None" and "Avoid updates
> > during update- sensitive times". It has not discussed "Detect
> > incompatibility after update", but let us not do that.
>
> But we already are discussing that, because the "None" option is just punting
> "Detect incompatibility after update" to something other than the VMM.  Doing
> literally nothing isn't an option.  The fact that it's even listed in the table,
> not to mention has "Simplest." listed as a pro, makes me question whether or not
> the authors actually understand how software built around the TDX-Module is used
> in practice.
>
> If an update causes a TD build to fail, or to generate the wrong measurement, or
> whatever "Failures due to incompatibilities" means, *something* eventually needs
> to take action.  Doing nothing is certainly the simplest option for the hypervisor
> and VMM, but when looking at the entire stack/ecosystem, it's the most complex
> option as it bleeds the damage into multiple, potentially-unknown components of
> the stack.  Either that, or I'm grossly misunderstanding what "Failures" means.
>
> That section also states:
>
>   Future TDX Module versions may have different or additional update-sensitive cases.
>
> Which means that from an ABI perspective, "Avoid updates during update-sensitive
> times" is the _ONLY_ viable option.  My read of that is that future TDX-Modules
> can effectively change the failure modes for a existing KVM ioctls.  That is an
> ABI change and will break userspace, e.g. if userspace is sane and expects certain
> operations to succeed.

A reference patch we tested for "Avoid updates during update-sensitive
times" and one caveat was that
/sys/devices/virtual/tdx/tdx_tsm/version was not available post update
failure until a subsequent successful update:

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index e00650b83f08..96ae7c679e4e 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -22,6 +22,7 @@
 #define TDX_FEATURES0_NO_RBP_MOD               BIT_ULL(18)
 #define TDX_FEATURES0_CLFLUSH_BEFORE_ALLOC     BIT_ULL(23)
 #define TDX_FEATURES0_DYNAMIC_PAMT             BIT_ULL(36)
+#define TDX_FEATURES0_UPDATE_COMPATIBILITY     BIT_ULL(47)

 #ifndef __ASSEMBLY__

@@ -129,6 +130,11 @@ static inline bool
tdx_supports_dynamic_pamt(const struct tdx_sys_info *sysinfo)
        return sysinfo->features.tdx_features0 & TDX_FEATURES0_DYNAMIC_PAMT;
 }

+static inline bool tdx_supports_update_compatibility(const struct
tdx_sys_info *sysinfo)
+{
+       return sysinfo->features.tdx_features0 &
TDX_FEATURES0_UPDATE_COMPATIBILITY;
+}
+
 int tdx_guest_keyid_alloc(void);
 u32 tdx_get_nr_guest_keyids(void);
 void tdx_guest_keyid_free(unsigned int keyid);
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index f6199f8ce411..95deb1146a79 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1523,6 +1523,10 @@ int tdx_module_shutdown(void)
         * fail.
         */
        args.rcx = tdx_sysinfo.handoff.module_hv;
+
+       if (tdx_supports_update_compatibility(&tdx_sysinfo))
+               args.rcx |= TDX_SYS_SHUTDOWN_AVOID_COMPAT_SENSITIVE;
+
        ret = seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
        if (!ret)
                tdx_module_reset_state();
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 0cd9140620f9..772c714de2bc 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -94,6 +94,8 @@ struct tdmr_info {
 /* Bit definitions of TDX_FEATURES0 metadata field */
 #define TDX_FEATURES0_TD_PRESERVING    BIT(1)

+#define TDX_SYS_SHUTDOWN_AVOID_COMPAT_SENSITIVE                BIT(16)
+
 /*
  * Do not put any hardware-defined TDX structure representations below
  * this comment!

Powered by blists - more mailing lists