Re: [RFC PATCH 3/5] x86/kexec: Disable kexec/kdump on platforms with TDX partial write erratum

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Thu, 2025-03-13 at 23:57 +0000, Huang, Kai wrote:
> > So this will switch all of TDX to be default off then, unless the kernel
> > gets a
> > parameter set. 
> > 
> 
> Currently in KVM TDX is also default off.

Good point. It begs the question of how many command line options the user
should have to pass to enable TDX.

> 
> > In which case we could also just unlock the Kconfig with just one
> > small change. TDX and kexec would still mutually exclusive, but just at
> > runtime.
> 
> Yeah I am thinking this too, given the "keyID 0 integrity" thing are still on-
> going.

You mentioned offline that there used to be a command line option, but it was
removed after discussion with Dave. I went to look for it and only found this:
https://lore.kernel.org/lkml/7e63912a-895f-d3b3-3173-336beaa86d08@intel.com/

...where Dave just asks why it's needed. In the next version it's dropped.
Unless there is anything more, it doesn't seem like there was really any
objection.

> 
> > We should try to flag Paolo and see what he thinks.
> 
> I appreciate if you could help to do.
> 
> > 
> > Or is the proposal to only be default tdx_host=off on the errata platforms?
> > And
> > tdx_host=on otherwise?
> 
> The tricky thing is, naturally, we want to skip all the code in tdx_init() if
> tdx_host=off, because there's no reason to do those detection/initialization
> if
> we are not going to use TDX, e.g., we don't need to this one:
> 
> 	register_memory_notifier(&tdx_memory_nb);
> 
> .. that means the code of detecting erratum will be skipped too.
> 
> If we only to only make tdx_host=off as default for erratum platforms, then we
> need to do cleanup (e.g., to unregister the above memory notifier).

This is a strange point. The errata detection is not dependent on the earlier
code in TDX init. It couldn't just be moved?

> 
> This isn't nice and seems hacky.
> 
> I don't see making tdx_host=off as default has problem, anyway, as mentioned
> above TDX is off by default in KVM.

Yea, tdx_host=!errata as a default value makes it more complicated.


So I think the situation is we need at one kernel parameter. We already have one
for KVM, which controls the late initialization parts of TDX that we care about
here. So what about just using the existing one? I think we don't want two.

If KVM has not initialized TDX (based on its own TDX parameter), then kexec is
fine. It could work by exposing an interface for features to be exclusive with
TDX. Since real TDX module initialization happens late anyway. I don't know if
it's better than a kernel one, but I don't see adding a second one going well.


Very, very rough:

diff --git a/arch/x86/kernel/machine_kexec_64.c
b/arch/x86/kernel/machine_kexec_64.c
index a68f5a0a9f37..bfea4e78c577 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -315,6 +315,12 @@ int machine_kexec_prepare(struct kimage *image)
        result = init_pgtable(image, __pa(control_page));
        if (result)
                return result;
+
+       if (tdx_exclude_feature()) {
+               pr_info_once("Not allowed once TDX has been used.\n");
+               return -EOPNOTSUPP;
+       }
+
        kexec_va_control_page = (unsigned long)control_page;
        kexec_pa_table_page = (unsigned long)__pa(image->arch.pgd);
 
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index f5e2a937c1e7..9b1f42a1059c 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1215,6 +1215,21 @@ int tdx_enable(void)
 }
 EXPORT_SYMBOL_GPL(tdx_enable);
 
+bool tdx_exclude_feature(void)
+{
+       bool ret = false;
+
+       mutex_lock(&tdx_module_lock);
+       if (tdx_module_status == TDX_MODULE_INITIALIZED)
+               ret = true;
+       else
+               tdx_module_status = TDX_MODULE_EXCLUDED;
+       mutex_lock(&tdx_module_lock);
+
+       return ret;
+}
+
 static bool is_pamt_page(unsigned long phys)
 {
        struct tdmr_info_list *tdmr_list = &tdx_tdmr_list;