[PATCH v5 0/5] TDX host: kexec() support

[PATCH v5 0/5] TDX host: kexec() support

[Kai Huang]

Currently kexec() support and TDX host are muturally exclusive in the
Kconfig.  This series adds the TDX host kexec support so that they can
work together and can be enabled at the same time in the Kconfig.

Hi maintainers,

This series aims to go through the tip tree, but I also CC'ed Sean/Paolo
due to when KVM TDX comes to play a KVM patch [*] is needed to complete
the kexec support for TDX.  Also copy Dan for TDX connect.

Thanks for your time!

=== More information ===

If the kernel has ever enabled TDX, part of system memory remains TDX
private memory when kexec happens.  E.g., the PAMT (Physical Address
Metadata Table) pages used by the TDX module to track each TDX memory
page's state are never freed once the TDX module is initialized.  TDX
guests also have guest private memory and secure-EPT pages.

Similar to AMD SME, to support kexec the kernel needs to flush dirty
cachelines for TDX private memory before booting to the second kernel.
Also, the kernel needs to reset TDX private memory to normal (using
MOVDIR64B) before booting to the second kernel when the platform has
"partial write machine check" erratum, otherwise the second kernel may
see unexpected machine check.

The majority code change in this series handles "resetting TDX private
memory" (flushing cache part is relatively straightforward).  Due to
currently the kernel doesn't have a unified way to tell whether a given
page is TDX private or not, this series chooses to only reset PAMT in
the core-kernel kexec code, but requires the in-kernel TDX users (e.g.,
KVM to reset the TDX private pages that they manage (see [*]).

Other options are also mentioned in the changelog of patch:

  x86/kexec: Reset TDX private memory on platforms with TDX erratum

..which also contains more information about the above TDX erratum.

This series also covers crash kexec, but no special handling is needed
for crash kexec:

1) kdump kernel uses reserved memory from the first kernel, but the
   reserved memory will never be used as TDX memory.
2) /proc/vmcore in the kdump kernel will only be used for read, but read
   itself won't poison TDX private memory thus won't cause unexpected
   machine check (only "partial write" will).


v4 -> v5:
 - Rebase to tip/master.
 - Remove the TDX-specific callback due to no need to reset TDX private
   memory for crash kexec.
 - Add a new patch to make module status immutable in reboot notifier
   (split from v1) in order to use module status to tell the presence of
   TDX private memory.
 - Minor changelog updates, trivial comments improvements.
 - Add Tom's Reviewed-by tag.

 v4: https://lore.kernel.org/all/cover.1713439632.git.kai.huang@intel.com/

v3 -> v4:
 - Updated changelog and comments of patch 1/2 per comments from
   Kirill and Tom (see specific patch for details).

 v3: https://lore.kernel.org/linux-kernel/cover.1712493366.git.kai.huang@intel.com/

v2 -> v3:
 - Change to only do WBINVD for bare-metal, as Kirill/Tom pointed out
   WBINVD in TDX guests and SEV-ES/SEV-SNP guests triggers #VE.

 v2: https://lore.kernel.org/linux-kernel/cover.1710811610.git.kai.huang@intel.com/

v1 -> v2:
 - Do unconditional WBINVD during kexec() -- Boris
 - Change to cover crash kexec() -- Rick
 - Add a new patch (last one) to add a mechanism to reset all TDX private
   pages due to having to cover crash kexec().
 - Other code improvements  -- Dave
 - Rebase to latest tip/master.

 v1: https://lore.kernel.org/linux-kernel/cover.1706698706.git.kai.huang@intel.com/

[*]: https://github.com/intel/tdx/commit/513e24d7913457ba87b6f25644d02fbed0848f21


Kai Huang (5):
  x86/kexec: do unconditional WBINVD for bare-metal in stop_this_cpu()
  x86/kexec: do unconditional WBINVD for bare-metal in relocate_kernel()
  x86/virt/tdx: Make module initializatiton state immutable in reboot
    notifier
  x86/kexec: Reset TDX private memory on platforms with TDX erratum
  x86/virt/tdx: Remove the !KEXEC_CORE dependency

 arch/x86/Kconfig                     |  1 -
 arch/x86/include/asm/kexec.h         |  2 +-
 arch/x86/include/asm/tdx.h           |  2 +
 arch/x86/kernel/machine_kexec_64.c   | 29 +++++++++--
 arch/x86/kernel/process.c            | 19 ++++---
 arch/x86/kernel/relocate_kernel_64.S | 19 +++++--
 arch/x86/virt/vmx/tdx/tdx.c          | 78 ++++++++++++++++++++++++++++
 7 files changed, 129 insertions(+), 21 deletions(-)


base-commit: b8c7cbc324dc17b9e42379b42603613580bec2d8
-- 
2.45.2

[PATCH v5 1/5] x86/kexec: do unconditional WBINVD for bare-metal in stop_this_cpu()

[Kai Huang]

TL;DR:

Change to do unconditional WBINVD in stop_this_cpu() for bare metal
to cover kexec support for both AMD SME and Intel TDX, despite there
_was_ some issue preventing from doing so but now has it got fixed.

Long version:

Both AMD SME and Intel TDX can leave caches in an incoherent state due
to memory encryption, which can lead to silent memory corruption during
kexec.  To address this issue, it is necessary to flush the caches
before jumping to the second kernel.

Currently, the kernel only performs WBINVD in stop_this_cpu() when SME
is supported by hardware.  To support TDX, instead of adding one more
vendor-specific check, it is proposed to perform unconditional WBINVD.
Kexec() is a slow path, and the additional WBINVD is acceptable for the
sake of simplicity and maintainability.

It is important to note that WBINVD should only be done for bare-metal
scenarios, as TDX guests and SEV-ES/SEV-SNP guests may not handle the
unexpected exception (#VE or #VC) caused by WBINVD.

Note:

Historically, there _was_ an issue preventing doing unconditional WBINVD
but that has been fixed.

When SME kexec() support was initially added in commit

  bba4ed011a52: ("x86/mm, kexec: Allow kexec to be used with SME")

WBINVD was done unconditionally.  However since then some issues were
reported that different Intel systems would hang or reset due to that
commit.

To try to fix, a later commit

  f23d74f6c66c: ("x86/mm: Rework wbinvd, hlt operation in stop_this_cpu()")

then changed to only do WBINVD when hardware supports SME.

While this commit made the reported issues go away, it didn't pinpoint
the root cause.  Also, it forgot to handle a corner case[*], which
resulted in the reveal of the root cause and the final fix by commit

  1f5e7eb7868e: ("x86/smp: Make stop_other_cpus() more robust")

See [1][2] for more information.

Further testing of doing unconditional WBINVD based on the above fix on
the problematic machines (that issues were originally reported)
confirmed the issues couldn't be reproduced.

See [3][4] for more information.

Therefore, it is safe to do unconditional WBINVD for bare-metal now.

[*] The commit didn't check whether the CPUID leaf is available or not.
Making unsupported CPUID leaf on Intel returns garbage resulting in
unintended WBINVD which caused some issue (followed by the analysis and
the reveal of the final root cause).  The corner case was independently
fixed by commit

  9b040453d444: ("x86/smp: Dont access non-existing CPUID leaf")

[1]: https://lore.kernel.org/lkml/CALu+AoQKmeixJdkO07t7BtttN7v3RM4_aBKi642bQ3fTBbSAVg@mail.gmail.com/T/#m300f3f9790850b5daa20a71abcc200ae8d94a12a
[2]: https://lore.kernel.org/lkml/CALu+AoQKmeixJdkO07t7BtttN7v3RM4_aBKi642bQ3fTBbSAVg@mail.gmail.com/T/#ma7263a7765483db0dabdeef62a1110940e634846
[3]: https://lore.kernel.org/lkml/CALu+AoQKmeixJdkO07t7BtttN7v3RM4_aBKi642bQ3fTBbSAVg@mail.gmail.com/T/#mc043191f2ff860d649c8466775dc61ac1e0ae320
[4]: https://lore.kernel.org/lkml/CALu+AoQKmeixJdkO07t7BtttN7v3RM4_aBKi642bQ3fTBbSAVg@mail.gmail.com/T/#md23f1a8f6afcc59fa2b0ac1967f18e418e24347c

Signed-off-by: Kai Huang <kai.huang@intel.com>
Suggested-by: Borislav Petkov <bp@alien8.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Dave Young <dyoung@redhat.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
---

v4 -> v5:
 - Add Tom's tag

v3 -> v4:
 - Update part of changelog based on Kirill's version (with minor tweak).
 - Use "exception (#VE or #VC)" for TDX and SEV-ES/SEV-SNP in changelog
   and comments.  (Kirill, Tom)
 - Point out "WBINVD is not necessary for TDX and SEV-ES/SEV-SNP guests"
   in the comment.  (Tom)

v2 -> v3:
 - Change to only do WBINVD for bare metal

---
 arch/x86/kernel/process.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 1b3d417cd6c4..134c8d99758e 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -810,18 +810,17 @@ void __noreturn stop_this_cpu(void *dummy)
 	mcheck_cpu_clear(c);
 
 	/*
-	 * Use wbinvd on processors that support SME. This provides support
-	 * for performing a successful kexec when going from SME inactive
-	 * to SME active (or vice-versa). The cache must be cleared so that
-	 * if there are entries with the same physical address, both with and
-	 * without the encryption bit, they don't race each other when flushed
-	 * and potentially end up with the wrong entry being committed to
-	 * memory.
+	 * The kernel could leave caches in incoherent state on SME/TDX
+	 * capable platforms.  Flush cache to avoid silent memory
+	 * corruption for these platforms.
 	 *
-	 * Test the CPUID bit directly because the machine might've cleared
-	 * X86_FEATURE_SME due to cmdline options.
+	 * stop_this_cpu() isn't a fast path, just do WBINVD for bare-metal
+	 * to cover both SME and TDX.  It isn't necessary to perform WBINVD
+	 * in a guest and performing one could result in an exception (#VE
+	 * or #VC) for a TDX or SEV-ES/SEV-SNP guest that the guest may
+	 * not be able to handle (e.g., TDX guest panics if it sees #VE).
 	 */
-	if (c->extended_cpuid_level >= 0x8000001f && (cpuid_eax(0x8000001f) & BIT(0)))
+	if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
 		native_wbinvd();
 
 	/*
-- 
2.45.2

[PATCH v5 2/5] x86/kexec: do unconditional WBINVD for bare-metal in relocate_kernel()

[Kai Huang]

Both SME and TDX can leave caches in incoherent state due to memory
encryption.  During kexec, the caches must be flushed before jumping to
the second kernel to avoid silent memory corruption to the second kernel.

During kexec, the WBINVD in stop_this_cpu() flushes caches for all
remote cpus when they are being stopped.  For SME, the WBINVD in
relocate_kernel() flushes the cache for the last running cpu (which is
executing the kexec).

Similarly, to support kexec for TDX host, after stopping all remote cpus
with cache flushed, the kernel needs to flush cache for the last running
cpu.

Use the existing WBINVD in relocate_kernel() to cover TDX host as well.

However, instead of sprinkling around vendor-specific checks, just do
unconditional WBINVD to cover both SME and TDX.  Kexec is not a fast path
so having one additional WBINVD for platforms w/o SME/TDX is acceptable.

But only do WBINVD for bare-metal because TDX guests and SEV-ES/SEV-SNP
guests will get unexpected (and yet unnecessary) exception (#VE or #VC)
which the kernel is unable to handle at this stage.

Signed-off-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Dave Young <dyoung@redhat.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
---

v4 -> v5:
 - Add Tom's tag

v3 -> v4:
 - Use "exception (#VE or #VC)" for TDX and SEV-ES/SEV-SNP in changelog
   and comments.  (Kirill, Tom)
 - "Save the bare_metal" -> "Save the bare_metal flag" (Tom)
 - Point out "WBINVD is not necessary for TDX and SEV-ES/SEV-SNP guests"
   in the comment.  (Tom)

v2 -> v3:
 - Change to only do WBINVD for bare metal

---
 arch/x86/include/asm/kexec.h         |  2 +-
 arch/x86/kernel/machine_kexec_64.c   |  2 +-
 arch/x86/kernel/relocate_kernel_64.S | 19 +++++++++++++++----
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
index ae5482a2f0ca..b3429c70847d 100644
--- a/arch/x86/include/asm/kexec.h
+++ b/arch/x86/include/asm/kexec.h
@@ -128,7 +128,7 @@ relocate_kernel(unsigned long indirection_page,
 		unsigned long page_list,
 		unsigned long start_address,
 		unsigned int preserve_context,
-		unsigned int host_mem_enc_active);
+		unsigned int bare_metal);
 #endif
 
 #define ARCH_HAS_KIMAGE_ARCH
diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 9c9ac606893e..07ca9d3361a3 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -392,7 +392,7 @@ void machine_kexec(struct kimage *image)
 				       (unsigned long)page_list,
 				       image->start,
 				       image->preserve_context,
-				       host_mem_enc_active);
+				       !boot_cpu_has(X86_FEATURE_HYPERVISOR));
 
 #ifdef CONFIG_KEXEC_JUMP
 	if (image->preserve_context)
diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
index 042c9a0334e9..a1a8a79d6b78 100644
--- a/arch/x86/kernel/relocate_kernel_64.S
+++ b/arch/x86/kernel/relocate_kernel_64.S
@@ -52,7 +52,7 @@ SYM_CODE_START_NOALIGN(relocate_kernel)
 	 * %rsi page_list
 	 * %rdx start address
 	 * %rcx preserve_context
-	 * %r8  host_mem_enc_active
+	 * %r8  bare_metal
 	 */
 
 	/* Save the CPU context, used for jumping back */
@@ -80,7 +80,7 @@ SYM_CODE_START_NOALIGN(relocate_kernel)
 	pushq $0
 	popfq
 
-	/* Save SME active flag */
+	/* Save the bare_metal flag */
 	movq	%r8, %r12
 
 	/*
@@ -161,9 +161,20 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
 	movq	%r9, %cr3
 
 	/*
-	 * If SME is active, there could be old encrypted cache line
+	 * The kernel could leave caches in incoherent state on SME/TDX
+	 * capable platforms.  Just do unconditional WBINVD to avoid
+	 * silent memory corruption to the new kernel for these platforms.
+	 *
+	 * For SME, need to flush cache here before copying the kernel.
+	 * When it is active, there could be old encrypted cache line
 	 * entries that will conflict with the now unencrypted memory
-	 * used by kexec. Flush the caches before copying the kernel.
+	 * used by kexec.
+	 *
+	 * Do WBINVD for bare-metal only to cover both SME and TDX.  It
+	 * isn't necessary to perform a WBINVD in a guest and performing
+	 * one could result in an exception (#VE or #VC) for a TDX or
+	 * SEV-ES/SEV-SNP guest that can crash the guest since, at this
+	 * stage, the kernel has torn down the IDT.
 	 */
 	testq	%r12, %r12
 	jz .Lsme_off
-- 
2.45.2

Re: [PATCH v5 2/5] x86/kexec: do unconditional WBINVD for bare-metal in relocate_kernel()

[Huang, Kai]

>  
>  #define ARCH_HAS_KIMAGE_ARCH
> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> index 9c9ac606893e..07ca9d3361a3 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -392,7 +392,7 @@ void machine_kexec(struct kimage *image)
>  				       (unsigned long)page_list,
>  				       image->start,
>  				       image->preserve_context,
> -				       host_mem_enc_active);
> +				       !boot_cpu_has(X86_FEATURE_HYPERVISOR));
>  
> 

LKP reported below warning:

All warnings (new ones prefixed by >>):

   arch/x86/kernel/machine_kexec_64.c: In function 'machine_kexec':
>> arch/x86/kernel/machine_kexec_64.c:325:22: warning: variable
'host_mem_enc_active' set but not used [-Wunused-but-set-variable]
     325 |         unsigned int host_mem_enc_active;
         |                      ^~~~~~~~~~~~~~~~~~~

This is due to while rebasing I didn't pay enough attention to the recent code
from commit 

  93c1800b3799f ("x86/kexec: Fix bug with call depth tracking")

which introduced the host_mem_enc_active variable in order to avoid
cc_platform_has() function call after load_segments() to resolve a problem
when call depth tracking is on.

A 100% safe way is to replace 

	host_mem_enc_active = cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT);

... with

	bare_metal = !boot_cpu_has(X86_FEATURE_HYPERVISOR);

but I think we can just remove that variable and directly use

	!boot_cpu_has(X86_FEATURE_HYPERVISOR)

as the last argument of calling the relocate_kernel(), because AFAICT the
above X86_FEATURE_HYPERVISOR bit test will always generate inline code thus
there will be no additional CALL/RET.

The incremental diff will be:

--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -331,16 +331,9 @@ static void kexec_save_processor_start(struct kimage
*image)
 void machine_kexec(struct kimage *image)
 {
        unsigned long page_list[PAGES_NR];
-       unsigned int host_mem_enc_active;
        int save_ftrace_enabled;
        void *control_page;
 
-       /*
-        * This must be done before load_segments() since if call depth
tracking
-        * is used then GS must be valid to make any function calls.
-        */
-       host_mem_enc_active = cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT);
-

Am I missing anything?

I'll send out a new version with the above and put some explanation to the
changelog if I don't see any other feedback on the rest TDX patches in the
coming days.  Thanks!

Re: [PATCH v5 2/5] x86/kexec: do unconditional WBINVD for bare-metal in relocate_kernel()

[Borislav Petkov]

On Fri, Aug 16, 2024 at 12:29:18AM +1200, Kai Huang wrote:
> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
> index 9c9ac606893e..07ca9d3361a3 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -392,7 +392,7 @@ void machine_kexec(struct kimage *image)
>  				       (unsigned long)page_list,
>  				       image->start,
>  				       image->preserve_context,
> -				       host_mem_enc_active);
> +				       !boot_cpu_has(X86_FEATURE_HYPERVISOR));

Everytime you feel the need to check a X86_FEATURE_ flag, make sure you use
cpu_feature_enabled().

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v5 2/5] x86/kexec: do unconditional WBINVD for bare-metal in relocate_kernel()

[Huang, Kai]

On 5/09/2024 3:30 am, Borislav Petkov wrote:
> On Fri, Aug 16, 2024 at 12:29:18AM +1200, Kai Huang wrote:
>> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
>> index 9c9ac606893e..07ca9d3361a3 100644
>> --- a/arch/x86/kernel/machine_kexec_64.c
>> +++ b/arch/x86/kernel/machine_kexec_64.c
>> @@ -392,7 +392,7 @@ void machine_kexec(struct kimage *image)
>>   				       (unsigned long)page_list,
>>   				       image->start,
>>   				       image->preserve_context,
>> -				       host_mem_enc_active);
>> +				       !boot_cpu_has(X86_FEATURE_HYPERVISOR));
> 
> Everytime you feel the need to check a X86_FEATURE_ flag, make sure you use
> cpu_feature_enabled().
> 

Thanks for review.  Yeah will do.

[PATCH v5 3/5] x86/virt/tdx: Make module initializatiton state immutable in reboot notifier

[Kai Huang]

If the kernel has ever enabled TDX, part of system memory remains TDX
private memory when kexec happens.  E.g., the PAMT (Physical Address
Metadata Table) pages used by the TDX module to track each TDX memory
page's state are never freed once the TDX module is initialized.

In kexec, the kernel will need to convert all TDX private pages back to
normal when the platform has the TDX "partial write machine check"
erratum.  Such conversion will need to be done after stopping all remote
CPUs so that no more TDX activity can possibly happen.

Register a reboot notifier to make the TDX module initialization state
immutable during the preparation phase of kexec, so that the kernel can
later use module state to determine whether it is possible for the
system to have any TDX private page.  Otherwise, the remote CPU could be
stopped when it is in the middle of module initialization and the module
state wouldn't be able to reflect this.

Specifically, upon receiving the reboot notifier, stop further module
initialization if the kernel hasn't enabled TDX yet.  If there's any
other thread trying to initialize TDX module, wait until the ongoing
module initialization to finish.

The reboot notifier is triggered when the kernel goes to reboot, kexec,
halt or shutdown.  In any case, there's no need to allow the kernel to
continue to initialize the TDX module anyway (if not done yet).

Signed-off-by: Kai Huang <kai.huang@intel.com>
---

v4 -> v5:
 - New patch to split the 'tdx_rebooting' around reboot notifier (Dave).

---
 arch/x86/virt/vmx/tdx/tdx.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 4e2b2e2ac9f9..c33417fe4086 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -27,6 +27,7 @@
 #include <linux/log2.h>
 #include <linux/acpi.h>
 #include <linux/suspend.h>
+#include <linux/reboot.h>
 #include <asm/page.h>
 #include <asm/special_insns.h>
 #include <asm/msr-index.h>
@@ -52,6 +53,8 @@ static DEFINE_MUTEX(tdx_module_lock);
 /* All TDX-usable memory regions.  Protected by mem_hotplug_lock. */
 static LIST_HEAD(tdx_memlist);
 
+static bool tdx_rebooting;
+
 typedef void (*sc_err_func_t)(u64 fn, u64 err, struct tdx_module_args *args);
 
 static inline void seamcall_err(u64 fn, u64 err, struct tdx_module_args *args)
@@ -1185,6 +1188,9 @@ static int __tdx_enable(void)
 {
 	int ret;
 
+	if (tdx_rebooting)
+		return -EINVAL;
+
 	ret = init_tdx_module();
 	if (ret) {
 		pr_err("module initialization failed (%d)\n", ret);
@@ -1418,6 +1424,21 @@ static struct notifier_block tdx_memory_nb = {
 	.notifier_call = tdx_memory_notifier,
 };
 
+static int tdx_reboot_notifier(struct notifier_block *nb, unsigned long mode,
+			       void *unused)
+{
+	/* Wait for ongoing TDX initialization to finish */
+	mutex_lock(&tdx_module_lock);
+	tdx_rebooting = true;
+	mutex_unlock(&tdx_module_lock);
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block tdx_reboot_nb = {
+	.notifier_call = tdx_reboot_notifier,
+};
+
 static void __init check_tdx_erratum(void)
 {
 	/*
@@ -1472,6 +1493,14 @@ void __init tdx_init(void)
 		return;
 	}
 
+	err = register_reboot_notifier(&tdx_reboot_nb);
+	if (err) {
+		pr_err("initialization failed: register_reboot_notifier() failed (%d)\n",
+				err);
+		unregister_memory_notifier(&tdx_memory_nb);
+		return;
+	}
+
 #if defined(CONFIG_ACPI) && defined(CONFIG_SUSPEND)
 	pr_info("Disable ACPI S3. Turn off TDX in the BIOS to use ACPI S3.\n");
 	acpi_suspend_lowlevel = NULL;
-- 
2.45.2

[PATCH v5 4/5] x86/kexec: Reset TDX private memory on platforms with TDX erratum

[Kai Huang]

TL;DR:

On the platforms with TDX "partial write machine check" erratum, during
kexec, convert TDX private memory back to normal before jumping to the
second kernel to avoid the second kernel seeing potential unexpected
machine check.

Long version:

The first few generations of TDX hardware have an erratum.  A partial
write to a TDX private memory cacheline will silently "poison" the
line.  Subsequent reads will consume the poison and generate a machine
check.  According to the TDX hardware spec, neither of these things
should have happened.

== Background ==

Virtually all kernel memory accesses operations happen in full
cachelines.  In practice, writing a "byte" of memory usually reads a 64
byte cacheline of memory, modifies it, then writes the whole line back.
Those operations do not trigger this problem.

This problem is triggered by "partial" writes where a write transaction
of less than cacheline lands at the memory controller.  The CPU does
these via non-temporal write instructions (like MOVNTI), or through
UC/WC memory mappings.  The issue can also be triggered away from the
CPU by devices doing partial writes via DMA.

== Problem ==

A fast warm reset doesn't reset TDX private memory.  Kexec() can also
boot into the new kernel directly.  Thus if the old kernel has left any
TDX private pages on the platform with this erratum, the new kernel
might get unexpected machine check.

Note that w/o this erratum any kernel read/write on TDX private memory
should never cause machine check, thus it's OK for the old kernel to
leave TDX private pages as is.

Also note only the normal kexec needs to worry about this problem, but
not the crash kexec: 1) The kdump kernel only uses the special memory
reserved by the first kernel, and the reserved memory can never be used
by TDX in the first kernel; 2) The /proc/vmcore, which reflects the
first (crashed) kernel's memory, is only for read.  The read will never
"poison" TDX memory thus cause unexpected machine check (only partial
write does).

== Solution ==

In short, with this erratum, the kernel needs to explicitly convert all
TDX private pages back to normal (using MOVDIR64B to reset these pages)
to give the new kernel a clean slate after kexec().

The BIOS is also expected to disable fast warm reset as a workaround to
this erratum, thus this implementation doesn't try to reset TDX private
memory for the normal reboot case in the kernel but depends on the BIOS
to enable the workaround.

Reset TDX private pages in machine_kexec() so that: 1) all remote cpus
are stopped with cache flushed and there's no more TDX activity; 2) no
memory reset overhead for the normal reboot case since the BIOS is
expected to turn on the workaround.

There are different types of TDX private pages.  The TDX module itself
uses PAMT (Physical Address Metadata Table) to track each TDX memory
page's state.  TDX guests also have guest private memory and secure-EPT
pages.

It would be ideal to reset all types of TDX private memory once for all
in machine_kexec(), but there are practical problems to do so:

1) There's no existing infrastructure to track TDX private pages;
2) It's not feasible to query the TDX module about page type, because
   VMX, which making SEAMCALL requires, has already been disabled;
3) Even if it is feasible to query the TDX module, the result may not be
   accurate.  E.g., the remote CPU could be stopped right before the
   MOVDIR64B.

One temporary solution is to blindly convert all memory pages, but it's
problematic to do so too, because not all pages are mapped as writable
in the direct mapping.  It can be done by switching to the identical
mapping created for kexec(), or a new page table, but the complication
is overkill.

Therefore, rather than doing something dramatic, only reset PAMT pages
in machine_kexec().  All the in-kernel TDX users (e.g., KVM) need to
reset TDX private pages that they manage before the machine_kexec() by
registering either the reboot notifier or the syscore shutdown ops.

Signed-off-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---

v4 -> v5:
 - Remove the TDX-specific notifier, since there's no need to handle
   crash kexec specially.
 - Minor update to changelog and comments.

v3 -> v4:
 - No change

v2 -> v3:
 - No change

v1 -> v2:
 - Remove using reboot notifier to stop TDX module as it doesn't
   cover crash kexec.  Change to use a variable with barrier instead.
   (Rick)
 - Introduce kexec_save_processor_start() to make code better, and
   make the comment around calling site of tdx_reset_memory() more
   concise. (Dave)
 - Mention cache for all other cpus have been flushed around
   native_wbinvd() in tdx_reset_memory(). (Dave)
 - Remove the extended alternaties discussion from the comment, but leave
   it in the changelog. Point out what does current code do and point out
   risk. (Dave)


---
 arch/x86/include/asm/tdx.h         |  2 ++
 arch/x86/kernel/machine_kexec_64.c | 27 +++++++++++++---
 arch/x86/virt/vmx/tdx/tdx.c        | 49 ++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index eba178996d84..ed3ac9a8a079 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -116,11 +116,13 @@ static inline u64 sc_retry(sc_func_t func, u64 fn,
 int tdx_cpu_enable(void);
 int tdx_enable(void);
 const char *tdx_dump_mce_info(struct mce *m);
+void tdx_reset_memory(void);
 #else
 static inline void tdx_init(void) { }
 static inline int tdx_cpu_enable(void) { return -ENODEV; }
 static inline int tdx_enable(void)  { return -ENODEV; }
 static inline const char *tdx_dump_mce_info(struct mce *m) { return NULL; }
+static inline void tdx_reset_memory(void) { }
 #endif	/* CONFIG_INTEL_TDX_HOST */
 
 #endif /* !__ASSEMBLY__ */
diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 07ca9d3361a3..87da1b96ad67 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -29,6 +29,7 @@
 #include <asm/set_memory.h>
 #include <asm/cpu.h>
 #include <asm/efi.h>
+#include <asm/tdx.h>
 
 #ifdef CONFIG_ACPI
 /*
@@ -315,6 +316,14 @@ void machine_kexec_cleanup(struct kimage *image)
 	free_transition_pgtable(image);
 }
 
+static void kexec_save_processor_start(struct kimage *image)
+{
+#ifdef CONFIG_KEXEC_JUMP
+	if (image->preserve_context)
+		save_processor_state();
+#endif
+}
+
 /*
  * Do not allocate memory (or fail in any way) in machine_kexec().
  * We are past the point of no return, committed to rebooting now.
@@ -332,10 +341,20 @@ void machine_kexec(struct kimage *image)
 	 */
 	host_mem_enc_active = cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT);
 
-#ifdef CONFIG_KEXEC_JUMP
-	if (image->preserve_context)
-		save_processor_state();
-#endif
+	kexec_save_processor_start(image);
+
+	/*
+	 * Convert TDX private memory back to normal (when needed) to
+	 * avoid the second kernel potentially seeing unexpected machine
+	 * check.
+	 *
+	 * However skip this when preserve_context is on.  By reaching
+	 * here, TDX (if ever got enabled by the kernel) has survived
+	 * from the suspend when preserve_context is on, and it can
+	 * continue to work after jumping back from the second kernel.
+	 */
+	if (!image->preserve_context)
+		tdx_reset_memory();
 
 	save_ftrace_enabled = __ftrace_enabled_save();
 
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index c33417fe4086..a69a65f57616 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1518,3 +1518,52 @@ void __init tdx_init(void)
 
 	check_tdx_erratum();
 }
+
+void tdx_reset_memory(void)
+{
+	if (!boot_cpu_has(X86_FEATURE_TDX_HOST_PLATFORM))
+		return;
+
+	/*
+	 * Kernel read/write to TDX private memory doesn't cause
+	 * machine check on hardware w/o this erratum.
+	 */
+	if (!boot_cpu_has_bug(X86_BUG_TDX_PW_MCE))
+		return;
+
+	/*
+	 * Converting TDX private pages back to normal must be done
+	 * after all remote cpus have been stopped so that no more
+	 * TDX activity can happen and caches have been flushed.
+	 */
+	WARN_ON_ONCE(num_online_cpus() != 1);
+
+	/*
+	 * The system can only have TDX private memory after the TDX
+	 * module has been initialized.  tdx_reboot_notifier() has made
+	 * sure @tdx_module_status reflects the module initialization
+	 * status correctly and is immutable by now thus can be read
+	 * w/o holding lock.
+	 */
+	if (tdx_module_status != TDX_MODULE_INITIALIZED)
+		return;
+
+	/*
+	 * All remote cpus have been stopped, and their caches have
+	 * been flushed in stop_this_cpu().  Now flush cache for the
+	 * last running cpu _before_ converting TDX private pages.
+	 */
+	native_wbinvd();
+
+	/*
+	 * It's ideal to cover all types of TDX private pages here, but
+	 * currently there's no unified way to tell whether a given page
+	 * is TDX private page or not.
+	 *
+	 * Only convert PAMT here.  All in-kernel TDX users (e.g., KVM)
+	 * are responsible for converting TDX private pages that are
+	 * managed by them by either registering reboot notifier or
+	 * shutdown syscore ops.
+	 */
+	tdmrs_reset_pamt_all(&tdx_tdmr_list);
+}
-- 
2.45.2

[PATCH v5 5/5] x86/virt/tdx: Remove the !KEXEC_CORE dependency

[Kai Huang]

Now TDX host can work with kexec().  Remove the !KEXEC_CORE dependency.

Signed-off-by: Kai Huang <kai.huang@intel.com>
---
 arch/x86/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d422247b2882..2da088202969 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1979,7 +1979,6 @@ config INTEL_TDX_HOST
 	depends on X86_X2APIC
 	select ARCH_KEEP_MEMBLOCK
 	depends on CONTIG_ALLOC
-	depends on !KEXEC_CORE
 	depends on X86_MCE
 	help
 	  Intel Trust Domain Extensions (TDX) protects guest VMs from malicious
-- 
2.45.2

[PATCH v5 0/5] TDX host: kexec() support

[Sagi Shahar]

> Currently kexec() support and TDX host are muturally exclusive in the
> Kconfig.  This series adds the TDX host kexec support so that they can
> work together and can be enabled at the same time in the Kconfig.

I tried testing the kexec functionality and noticed that the TDX module
fails initialization on the second kernel so you can't actually kexec
between 2 kernels that enable TDX. Is that the expected behavior? Are
there future patches to enable that functionality?

Re: [PATCH v5 0/5] TDX host: kexec() support

[Huang, Kai]

On 20/08/2024 9:21 am, Sagi Shahar wrote:
>> Currently kexec() support and TDX host are muturally exclusive in the
>> Kconfig.  This series adds the TDX host kexec support so that they can
>> work together and can be enabled at the same time in the Kconfig.
> 
> I tried testing the kexec functionality and noticed that the TDX module
> fails initialization on the second kernel so you can't actually kexec
> between 2 kernels that enable TDX. Is that the expected behavior? Are
> there future patches to enable that functionality?
> 

Thanks for testing!

Yes this is the expected behaviour.  If the first kernel has enabled 
TDX, then the second kernel will fail to init TDX.  The reason the first 
SEAMCALL to initialize TDX module in the second kernel will fail due to 
module having been initialized.

However if the first kernel has not enabled TDX, the second kernel is 
able to enable it.

Re: [PATCH v5 0/5] TDX host: kexec() support

[Sagi Shahar]

On Mon, Aug 19, 2024 at 5:16 PM Huang, Kai <kai.huang@intel.com> wrote:
>
>
>
> On 20/08/2024 9:21 am, Sagi Shahar wrote:
> >> Currently kexec() support and TDX host are muturally exclusive in the
> >> Kconfig.  This series adds the TDX host kexec support so that they can
> >> work together and can be enabled at the same time in the Kconfig.
> >
> > I tried testing the kexec functionality and noticed that the TDX module
> > fails initialization on the second kernel so you can't actually kexec
> > between 2 kernels that enable TDX. Is that the expected behavior? Are
> > there future patches to enable that functionality?
> >
>
> Thanks for testing!
>
> Yes this is the expected behaviour.  If the first kernel has enabled
> TDX, then the second kernel will fail to init TDX.  The reason the first
> SEAMCALL to initialize TDX module in the second kernel will fail due to
> module having been initialized.
>
> However if the first kernel has not enabled TDX, the second kernel is
> able to enable it.

Are there any plans to support both kernels being able to enable TDX
in the future? Either by changes to KVM or the TDX module?

Re: [PATCH v5 0/5] TDX host: kexec() support

[Huang, Kai]

On 20/08/2024 10:28 am, Sagi Shahar wrote:
> On Mon, Aug 19, 2024 at 5:16 PM Huang, Kai <kai.huang@intel.com> wrote:
>>
>>
>>
>> On 20/08/2024 9:21 am, Sagi Shahar wrote:
>>>> Currently kexec() support and TDX host are muturally exclusive in the
>>>> Kconfig.  This series adds the TDX host kexec support so that they can
>>>> work together and can be enabled at the same time in the Kconfig.
>>>
>>> I tried testing the kexec functionality and noticed that the TDX module
>>> fails initialization on the second kernel so you can't actually kexec
>>> between 2 kernels that enable TDX. Is that the expected behavior? Are
>>> there future patches to enable that functionality?
>>>
>>
>> Thanks for testing!
>>
>> Yes this is the expected behaviour.  If the first kernel has enabled
>> TDX, then the second kernel will fail to init TDX.  The reason the first
>> SEAMCALL to initialize TDX module in the second kernel will fail due to
>> module having been initialized.
>>
>> However if the first kernel has not enabled TDX, the second kernel is
>> able to enable it.
> 
> Are there any plans to support both kernels being able to enable TDX
> in the future? Either by changes to KVM or the TDX module?

AFAICT we haven't received such requirement so far.  Let me double check 
internally and get back here.

Btw, if we want to do this purely from software, changing KVM isn't the 
right thing to do.  We need to somehow pass key data structures managing 
TDX module to the second kernel, e.g., module status, locations of 
PAMTs.  And the second kernel needs to be modified to understand those, 
which means some old (second) kernels with TDX support may not be able 
to support this even if we add this to the kernel.

Re: [PATCH v5 0/5] TDX host: kexec() support

[Sagi Shahar]

On Mon, Aug 19, 2024 at 5:44 PM Huang, Kai <kai.huang@intel.com> wrote:
>
>
>
> On 20/08/2024 10:28 am, Sagi Shahar wrote:
> > On Mon, Aug 19, 2024 at 5:16 PM Huang, Kai <kai.huang@intel.com> wrote:
> >>
> >>
> >>
> >> On 20/08/2024 9:21 am, Sagi Shahar wrote:
> >>>> Currently kexec() support and TDX host are muturally exclusive in the
> >>>> Kconfig.  This series adds the TDX host kexec support so that they can
> >>>> work together and can be enabled at the same time in the Kconfig.
> >>>
> >>> I tried testing the kexec functionality and noticed that the TDX module
> >>> fails initialization on the second kernel so you can't actually kexec
> >>> between 2 kernels that enable TDX. Is that the expected behavior? Are
> >>> there future patches to enable that functionality?
> >>>
> >>
> >> Thanks for testing!
> >>
> >> Yes this is the expected behaviour.  If the first kernel has enabled
> >> TDX, then the second kernel will fail to init TDX.  The reason the first
> >> SEAMCALL to initialize TDX module in the second kernel will fail due to
> >> module having been initialized.
> >>
> >> However if the first kernel has not enabled TDX, the second kernel is
> >> able to enable it.
> >
> > Are there any plans to support both kernels being able to enable TDX
> > in the future? Either by changes to KVM or the TDX module?
>
> AFAICT we haven't received such requirement so far.  Let me double check
> internally and get back here.
>
> Btw, if we want to do this purely from software, changing KVM isn't the
> right thing to do.  We need to somehow pass key data structures managing
> TDX module to the second kernel, e.g., module status, locations of
> PAMTs.  And the second kernel needs to be modified to understand those,
> which means some old (second) kernels with TDX support may not be able
> to support this even if we add this to the kernel.

Would it be possible to tear down the tdx module and re-initialize it
on the next kernel? I don't think there's a requirement for the tdx
module data structures to remain intact during kexec but it could be
useful if tdx can be enabled on the new kernel.

Re: [PATCH v5 0/5] TDX host: kexec() support

[Huang, Kai]

On Fri, 2024-08-23 at 11:15 -0500, Sagi Shahar wrote:
> On Mon, Aug 19, 2024 at 5:44 PM Huang, Kai <kai.huang@intel.com> wrote:
> > 
> > 
> > 
> > On 20/08/2024 10:28 am, Sagi Shahar wrote:
> > > On Mon, Aug 19, 2024 at 5:16 PM Huang, Kai <kai.huang@intel.com> wrote:
> > > > 
> > > > 
> > > > 
> > > > On 20/08/2024 9:21 am, Sagi Shahar wrote:
> > > > > > Currently kexec() support and TDX host are muturally exclusive in the
> > > > > > Kconfig.  This series adds the TDX host kexec support so that they can
> > > > > > work together and can be enabled at the same time in the Kconfig.
> > > > > 
> > > > > I tried testing the kexec functionality and noticed that the TDX module
> > > > > fails initialization on the second kernel so you can't actually kexec
> > > > > between 2 kernels that enable TDX. Is that the expected behavior? Are
> > > > > there future patches to enable that functionality?
> > > > > 
> > > > 
> > > > Thanks for testing!
> > > > 
> > > > Yes this is the expected behaviour.  If the first kernel has enabled
> > > > TDX, then the second kernel will fail to init TDX.  The reason the first
> > > > SEAMCALL to initialize TDX module in the second kernel will fail due to
> > > > module having been initialized.
> > > > 
> > > > However if the first kernel has not enabled TDX, the second kernel is
> > > > able to enable it.
> > > 
> > > Are there any plans to support both kernels being able to enable TDX
> > > in the future? Either by changes to KVM or the TDX module?
> > 
> > AFAICT we haven't received such requirement so far.  Let me double check
> > internally and get back here.
> > 
> > Btw, if we want to do this purely from software, changing KVM isn't the
> > right thing to do.  We need to somehow pass key data structures managing
> > TDX module to the second kernel, e.g., module status, locations of
> > PAMTs.  And the second kernel needs to be modified to understand those,
> > which means some old (second) kernels with TDX support may not be able
> > to support this even if we add this to the kernel.
> 
> Would it be possible to tear down the tdx module and re-initialize it
> on the next kernel? I don't think there's a requirement for the tdx
> module data structures to remain intact during kexec but it could be
> useful if tdx can be enabled on the new kernel.

We discussed this internally.  The TDX module cannot be re-initialized after
torn down.  However the new kernel can reload the (same) TDX module and re-
initialize it (the P-SEAMLDR supports reload or runtime update TDX module).

However our primary focus is to enable baseline TDX support in upstream.  For
TDX host kexec, at this stage we focus on: 1) enable both TDX and Kexec in the
Kconfig; 2) allow normal kexec and kdump to work when TDX is enabled.  Making
the second kernel be able to use TDX is next-step plan.

May I ask is there any real use case that requires the second kernel to be
able to use TDX at this stage?

Re: [PATCH v5 0/5] TDX host: kexec() support

[Sagi Shahar]

On Sat, Aug 24, 2024 at 4:31 AM Huang, Kai <kai.huang@intel.com> wrote:
>
> On Fri, 2024-08-23 at 11:15 -0500, Sagi Shahar wrote:
> > On Mon, Aug 19, 2024 at 5:44 PM Huang, Kai <kai.huang@intel.com> wrote:
> > >
> > >
> > >
> > > On 20/08/2024 10:28 am, Sagi Shahar wrote:
> > > > On Mon, Aug 19, 2024 at 5:16 PM Huang, Kai <kai.huang@intel.com> wrote:
> > > > >
> > > > >
> > > > >
> > > > > On 20/08/2024 9:21 am, Sagi Shahar wrote:
> > > > > > > Currently kexec() support and TDX host are muturally exclusive in the
> > > > > > > Kconfig.  This series adds the TDX host kexec support so that they can
> > > > > > > work together and can be enabled at the same time in the Kconfig.
> > > > > >
> > > > > > I tried testing the kexec functionality and noticed that the TDX module
> > > > > > fails initialization on the second kernel so you can't actually kexec
> > > > > > between 2 kernels that enable TDX. Is that the expected behavior? Are
> > > > > > there future patches to enable that functionality?
> > > > > >
> > > > >
> > > > > Thanks for testing!
> > > > >
> > > > > Yes this is the expected behaviour.  If the first kernel has enabled
> > > > > TDX, then the second kernel will fail to init TDX.  The reason the first
> > > > > SEAMCALL to initialize TDX module in the second kernel will fail due to
> > > > > module having been initialized.
> > > > >
> > > > > However if the first kernel has not enabled TDX, the second kernel is
> > > > > able to enable it.
> > > >
> > > > Are there any plans to support both kernels being able to enable TDX
> > > > in the future? Either by changes to KVM or the TDX module?
> > >
> > > AFAICT we haven't received such requirement so far.  Let me double check
> > > internally and get back here.
> > >
> > > Btw, if we want to do this purely from software, changing KVM isn't the
> > > right thing to do.  We need to somehow pass key data structures managing
> > > TDX module to the second kernel, e.g., module status, locations of
> > > PAMTs.  And the second kernel needs to be modified to understand those,
> > > which means some old (second) kernels with TDX support may not be able
> > > to support this even if we add this to the kernel.
> >
> > Would it be possible to tear down the tdx module and re-initialize it
> > on the next kernel? I don't think there's a requirement for the tdx
> > module data structures to remain intact during kexec but it could be
> > useful if tdx can be enabled on the new kernel.
>
> We discussed this internally.  The TDX module cannot be re-initialized after
> torn down.  However the new kernel can reload the (same) TDX module and re-
> initialize it (the P-SEAMLDR supports reload or runtime update TDX module).
>
> However our primary focus is to enable baseline TDX support in upstream.  For
> TDX host kexec, at this stage we focus on: 1) enable both TDX and Kexec in the
> Kconfig; 2) allow normal kexec and kdump to work when TDX is enabled.  Making
> the second kernel be able to use TDX is next-step plan.
>
> May I ask is there any real use case that requires the second kernel to be
> able to use TDX at this stage?

[Again in plaintext]

Right now I don't think we have production requirements for kexec at
all besides kdump support. Kexec from TDX enabled kernel to a non-TDX
kernel definitely doesn't have a production requirement.

It would be nice to be able to kexec to a TDX enabled kernel to speed
up the development cycle instead of waiting for a full reboot but
that's not a high priority at the moment.

Re: [PATCH v5 0/5] TDX host: kexec() support

[Huang, Kai]

On Mon, 2024-08-26 at 14:22 -0500, Sagi Shahar wrote:
> On Sat, Aug 24, 2024 at 4:31 AM Huang, Kai <kai.huang@intel.com> wrote:
> > 
> > On Fri, 2024-08-23 at 11:15 -0500, Sagi Shahar wrote:
> > > On Mon, Aug 19, 2024 at 5:44 PM Huang, Kai <kai.huang@intel.com> wrote:
> > > > 
> > > > 
> > > > 
> > > > On 20/08/2024 10:28 am, Sagi Shahar wrote:
> > > > > On Mon, Aug 19, 2024 at 5:16 PM Huang, Kai <kai.huang@intel.com> wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On 20/08/2024 9:21 am, Sagi Shahar wrote:
> > > > > > > > Currently kexec() support and TDX host are muturally exclusive in the
> > > > > > > > Kconfig.  This series adds the TDX host kexec support so that they can
> > > > > > > > work together and can be enabled at the same time in the Kconfig.
> > > > > > > 
> > > > > > > I tried testing the kexec functionality and noticed that the TDX module
> > > > > > > fails initialization on the second kernel so you can't actually kexec
> > > > > > > between 2 kernels that enable TDX. Is that the expected behavior? Are
> > > > > > > there future patches to enable that functionality?
> > > > > > > 
> > > > > > 
> > > > > > Thanks for testing!
> > > > > > 
> > > > > > Yes this is the expected behaviour.  If the first kernel has enabled
> > > > > > TDX, then the second kernel will fail to init TDX.  The reason the first
> > > > > > SEAMCALL to initialize TDX module in the second kernel will fail due to
> > > > > > module having been initialized.
> > > > > > 
> > > > > > However if the first kernel has not enabled TDX, the second kernel is
> > > > > > able to enable it.
> > > > > 
> > > > > Are there any plans to support both kernels being able to enable TDX
> > > > > in the future? Either by changes to KVM or the TDX module?
> > > > 
> > > > AFAICT we haven't received such requirement so far.  Let me double check
> > > > internally and get back here.
> > > > 
> > > > Btw, if we want to do this purely from software, changing KVM isn't the
> > > > right thing to do.  We need to somehow pass key data structures managing
> > > > TDX module to the second kernel, e.g., module status, locations of
> > > > PAMTs.  And the second kernel needs to be modified to understand those,
> > > > which means some old (second) kernels with TDX support may not be able
> > > > to support this even if we add this to the kernel.
> > > 
> > > Would it be possible to tear down the tdx module and re-initialize it
> > > on the next kernel? I don't think there's a requirement for the tdx
> > > module data structures to remain intact during kexec but it could be
> > > useful if tdx can be enabled on the new kernel.
> > 
> > We discussed this internally.  The TDX module cannot be re-initialized after
> > torn down.  However the new kernel can reload the (same) TDX module and re-
> > initialize it (the P-SEAMLDR supports reload or runtime update TDX module).
> > 
> > However our primary focus is to enable baseline TDX support in upstream.  For
> > TDX host kexec, at this stage we focus on: 1) enable both TDX and Kexec in the
> > Kconfig; 2) allow normal kexec and kdump to work when TDX is enabled.  Making
> > the second kernel be able to use TDX is next-step plan.
> > 
> > May I ask is there any real use case that requires the second kernel to be
> > able to use TDX at this stage?
> 
> [Again in plaintext]
> 
> Right now I don't think we have production requirements for kexec at
> all besides kdump support. Kexec from TDX enabled kernel to a non-TDX
> kernel definitely doesn't have a production requirement.
> 
> It would be nice to be able to kexec to a TDX enabled kernel to speed
> up the development cycle instead of waiting for a full reboot but
> that's not a high priority at the moment.

Yeah agreed.  But then let's do this as a future work after baseline TDX
support is done.