* [PATCH v7 1/3] x86/paravirt: Move halt paravirt calls under CONFIG_PARAVIRT
2025-02-28 1:44 [PATCH v7 0/3] x86/tdx: Fix HLT logic execution for TDX VMs Vishal Annapurve
@ 2025-02-28 1:44 ` Vishal Annapurve
2025-03-26 8:02 ` [tip: x86/tdx] " tip-bot2 for Kirill A. Shutemov
2025-02-28 1:44 ` [PATCH v7 2/3] x86/tdx: Fix arch_safe_halt() execution for TDX VMs Vishal Annapurve
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Vishal Annapurve @ 2025-02-28 1:44 UTC (permalink / raw)
To: dave.hansen, kirill.shutemov, jgross, ajay.kaher, ak, tony.luck,
thomas.lendacky
Cc: tglx, mingo, bp, hpa, pbonzini, seanjc, kai.huang, chao.p.peng,
isaku.yamahata, sathyanarayanan.kuppuswamy, erdemaktas,
ackerleytng, jxgao, sagis, afranji, kees, jikos, peterz, x86,
linux-kernel, linux-coco, virtualization,
bcm-kernel-feedback-list, stable, Vishal Annapurve
From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
CONFIG_PARAVIRT_XXL is mainly defined/used by XEN PV guests. For
other VM guest types, features supported under CONFIG_PARAVIRT
are self sufficient. CONFIG_PARAVIRT mainly provides support for
TLB flush operations and time related operations.
For TDX guest as well, paravirt calls under CONFIG_PARVIRT meets
most of its requirement except the need of HLT and SAFE_HLT
paravirt calls, which is currently defined under
CONFIG_PARAVIRT_XXL.
Since enabling CONFIG_PARAVIRT_XXL is too bloated for TDX guest
like platforms, move HLT and SAFE_HLT paravirt calls under
CONFIG_PARAVIRT.
Moving HLT and SAFE_HLT paravirt calls are not fatal and should not
break any functionality for current users of CONFIG_PARAVIRT.
Cc: stable@vger.kernel.org
Fixes: bfe6ed0c6727 ("x86/tdx: Add HLT support for TDX guests")
Co-developed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reviewed-by: Andi Kleen <ak@linux.intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Vishal Annapurve <vannapurve@google.com>
---
arch/x86/include/asm/irqflags.h | 40 +++++++++++++++------------
arch/x86/include/asm/paravirt.h | 20 +++++++-------
arch/x86/include/asm/paravirt_types.h | 3 +-
arch/x86/kernel/paravirt.c | 14 ++++++----
4 files changed, 41 insertions(+), 36 deletions(-)
diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
index cf7fc2b8e3ce..1c2db11a2c3c 100644
--- a/arch/x86/include/asm/irqflags.h
+++ b/arch/x86/include/asm/irqflags.h
@@ -76,6 +76,28 @@ static __always_inline void native_local_irq_restore(unsigned long flags)
#endif
+#ifndef CONFIG_PARAVIRT
+#ifndef __ASSEMBLY__
+/*
+ * Used in the idle loop; sti takes one instruction cycle
+ * to complete:
+ */
+static __always_inline void arch_safe_halt(void)
+{
+ native_safe_halt();
+}
+
+/*
+ * Used when interrupts are already enabled or to
+ * shutdown the processor:
+ */
+static __always_inline void halt(void)
+{
+ native_halt();
+}
+#endif /* __ASSEMBLY__ */
+#endif /* CONFIG_PARAVIRT */
+
#ifdef CONFIG_PARAVIRT_XXL
#include <asm/paravirt.h>
#else
@@ -97,24 +119,6 @@ static __always_inline void arch_local_irq_enable(void)
native_irq_enable();
}
-/*
- * Used in the idle loop; sti takes one instruction cycle
- * to complete:
- */
-static __always_inline void arch_safe_halt(void)
-{
- native_safe_halt();
-}
-
-/*
- * Used when interrupts are already enabled or to
- * shutdown the processor:
- */
-static __always_inline void halt(void)
-{
- native_halt();
-}
-
/*
* For spinlocks, etc:
*/
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 041aff51eb50..29e7331a0c98 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -107,6 +107,16 @@ static inline void notify_page_enc_status_changed(unsigned long pfn,
PVOP_VCALL3(mmu.notify_page_enc_status_changed, pfn, npages, enc);
}
+static __always_inline void arch_safe_halt(void)
+{
+ PVOP_VCALL0(irq.safe_halt);
+}
+
+static inline void halt(void)
+{
+ PVOP_VCALL0(irq.halt);
+}
+
#ifdef CONFIG_PARAVIRT_XXL
static inline void load_sp0(unsigned long sp0)
{
@@ -170,16 +180,6 @@ static inline void __write_cr4(unsigned long x)
PVOP_VCALL1(cpu.write_cr4, x);
}
-static __always_inline void arch_safe_halt(void)
-{
- PVOP_VCALL0(irq.safe_halt);
-}
-
-static inline void halt(void)
-{
- PVOP_VCALL0(irq.halt);
-}
-
static inline u64 paravirt_read_msr(unsigned msr)
{
return PVOP_CALL1(u64, cpu.read_msr, msr);
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index fea56b04f436..abccfccc2e3f 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -120,10 +120,9 @@ struct pv_irq_ops {
struct paravirt_callee_save save_fl;
struct paravirt_callee_save irq_disable;
struct paravirt_callee_save irq_enable;
-
+#endif
void (*safe_halt)(void);
void (*halt)(void);
-#endif
} __no_randomize_layout;
struct pv_mmu_ops {
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 1ccaa3397a67..c5bb980b8a67 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -110,6 +110,11 @@ int paravirt_disable_iospace(void)
return request_resource(&ioport_resource, &reserve_ioports);
}
+static noinstr void pv_native_safe_halt(void)
+{
+ native_safe_halt();
+}
+
#ifdef CONFIG_PARAVIRT_XXL
static noinstr void pv_native_write_cr2(unsigned long val)
{
@@ -125,11 +130,6 @@ static noinstr void pv_native_set_debugreg(int regno, unsigned long val)
{
native_set_debugreg(regno, val);
}
-
-static noinstr void pv_native_safe_halt(void)
-{
- native_safe_halt();
-}
#endif
struct pv_info pv_info = {
@@ -186,9 +186,11 @@ struct paravirt_patch_template pv_ops = {
.irq.save_fl = __PV_IS_CALLEE_SAVE(pv_native_save_fl),
.irq.irq_disable = __PV_IS_CALLEE_SAVE(pv_native_irq_disable),
.irq.irq_enable = __PV_IS_CALLEE_SAVE(pv_native_irq_enable),
+#endif /* CONFIG_PARAVIRT_XXL */
+
+ /* Irq HLT ops. */
.irq.safe_halt = pv_native_safe_halt,
.irq.halt = native_halt,
-#endif /* CONFIG_PARAVIRT_XXL */
/* Mmu ops. */
.mmu.flush_tlb_user = native_flush_tlb_local,
--
2.48.1.711.g2feabab25a-goog
^ permalink raw reply [flat|nested] 9+ messages in thread* [tip: x86/tdx] x86/paravirt: Move halt paravirt calls under CONFIG_PARAVIRT
2025-02-28 1:44 ` [PATCH v7 1/3] x86/paravirt: Move halt paravirt calls under CONFIG_PARAVIRT Vishal Annapurve
@ 2025-03-26 8:02 ` tip-bot2 for Kirill A. Shutemov
0 siblings, 0 replies; 9+ messages in thread
From: tip-bot2 for Kirill A. Shutemov @ 2025-03-26 8:02 UTC (permalink / raw)
To: linux-tip-commits
Cc: Kuppuswamy Sathyanarayanan, Kirill A. Shutemov, Vishal Annapurve,
Ingo Molnar, Andi Kleen, Tony Luck, Juergen Gross, Ryan Afranji,
Andy Lutomirski, Brian Gerst, H. Peter Anvin, Linus Torvalds,
Josh Poimboeuf, stable, x86, linux-kernel
The following commit has been merged into the x86/tdx branch of tip:
Commit-ID: 22cc5ca5de52bbfc36a7d4a55323f91fb4492264
Gitweb: https://git.kernel.org/tip/22cc5ca5de52bbfc36a7d4a55323f91fb4492264
Author: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
AuthorDate: Fri, 28 Feb 2025 01:44:14
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 26 Mar 2025 08:48:18 +01:00
x86/paravirt: Move halt paravirt calls under CONFIG_PARAVIRT
CONFIG_PARAVIRT_XXL is mainly defined/used by XEN PV guests. For
other VM guest types, features supported under CONFIG_PARAVIRT
are self sufficient. CONFIG_PARAVIRT mainly provides support for
TLB flush operations and time related operations.
For TDX guest as well, paravirt calls under CONFIG_PARVIRT meets
most of its requirement except the need of HLT and SAFE_HLT
paravirt calls, which is currently defined under
CONFIG_PARAVIRT_XXL.
Since enabling CONFIG_PARAVIRT_XXL is too bloated for TDX guest
like platforms, move HLT and SAFE_HLT paravirt calls under
CONFIG_PARAVIRT.
Moving HLT and SAFE_HLT paravirt calls are not fatal and should not
break any functionality for current users of CONFIG_PARAVIRT.
Fixes: bfe6ed0c6727 ("x86/tdx: Add HLT support for TDX guests")
Co-developed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Vishal Annapurve <vannapurve@google.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Andi Kleen <ak@linux.intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Tested-by: Ryan Afranji <afranji@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: stable@kernel.org
Link: https://lore.kernel.org/r/20250228014416.3925664-2-vannapurve@google.com
---
arch/x86/include/asm/irqflags.h | 40 ++++++++++++++------------
arch/x86/include/asm/paravirt.h | 20 ++++++-------
arch/x86/include/asm/paravirt_types.h | 3 +--
arch/x86/kernel/paravirt.c | 14 +++++----
4 files changed, 41 insertions(+), 36 deletions(-)
diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
index abb8374..9a9b21b 100644
--- a/arch/x86/include/asm/irqflags.h
+++ b/arch/x86/include/asm/irqflags.h
@@ -76,6 +76,28 @@ static __always_inline void native_local_irq_restore(unsigned long flags)
#endif
+#ifndef CONFIG_PARAVIRT
+#ifndef __ASSEMBLY__
+/*
+ * Used in the idle loop; sti takes one instruction cycle
+ * to complete:
+ */
+static __always_inline void arch_safe_halt(void)
+{
+ native_safe_halt();
+}
+
+/*
+ * Used when interrupts are already enabled or to
+ * shutdown the processor:
+ */
+static __always_inline void halt(void)
+{
+ native_halt();
+}
+#endif /* __ASSEMBLY__ */
+#endif /* CONFIG_PARAVIRT */
+
#ifdef CONFIG_PARAVIRT_XXL
#include <asm/paravirt.h>
#else
@@ -98,24 +120,6 @@ static __always_inline void arch_local_irq_enable(void)
}
/*
- * Used in the idle loop; sti takes one instruction cycle
- * to complete:
- */
-static __always_inline void arch_safe_halt(void)
-{
- native_safe_halt();
-}
-
-/*
- * Used when interrupts are already enabled or to
- * shutdown the processor:
- */
-static __always_inline void halt(void)
-{
- native_halt();
-}
-
-/*
* For spinlocks, etc:
*/
static __always_inline unsigned long arch_local_irq_save(void)
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index bed346b..c4c2319 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -102,6 +102,16 @@ static inline void notify_page_enc_status_changed(unsigned long pfn,
PVOP_VCALL3(mmu.notify_page_enc_status_changed, pfn, npages, enc);
}
+static __always_inline void arch_safe_halt(void)
+{
+ PVOP_VCALL0(irq.safe_halt);
+}
+
+static inline void halt(void)
+{
+ PVOP_VCALL0(irq.halt);
+}
+
#ifdef CONFIG_PARAVIRT_XXL
static inline void load_sp0(unsigned long sp0)
{
@@ -165,16 +175,6 @@ static inline void __write_cr4(unsigned long x)
PVOP_VCALL1(cpu.write_cr4, x);
}
-static __always_inline void arch_safe_halt(void)
-{
- PVOP_VCALL0(irq.safe_halt);
-}
-
-static inline void halt(void)
-{
- PVOP_VCALL0(irq.halt);
-}
-
static inline u64 paravirt_read_msr(unsigned msr)
{
return PVOP_CALL1(u64, cpu.read_msr, msr);
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 6291202..631c306 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -120,10 +120,9 @@ struct pv_irq_ops {
struct paravirt_callee_save save_fl;
struct paravirt_callee_save irq_disable;
struct paravirt_callee_save irq_enable;
-
+#endif
void (*safe_halt)(void);
void (*halt)(void);
-#endif
} __no_randomize_layout;
struct pv_mmu_ops {
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 9792563..1ccd05d 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -75,6 +75,11 @@ void paravirt_set_sched_clock(u64 (*func)(void))
static_call_update(pv_sched_clock, func);
}
+static noinstr void pv_native_safe_halt(void)
+{
+ native_safe_halt();
+}
+
#ifdef CONFIG_PARAVIRT_XXL
static noinstr void pv_native_write_cr2(unsigned long val)
{
@@ -100,11 +105,6 @@ static noinstr void pv_native_set_debugreg(int regno, unsigned long val)
{
native_set_debugreg(regno, val);
}
-
-static noinstr void pv_native_safe_halt(void)
-{
- native_safe_halt();
-}
#endif
struct pv_info pv_info = {
@@ -161,9 +161,11 @@ struct paravirt_patch_template pv_ops = {
.irq.save_fl = __PV_IS_CALLEE_SAVE(pv_native_save_fl),
.irq.irq_disable = __PV_IS_CALLEE_SAVE(pv_native_irq_disable),
.irq.irq_enable = __PV_IS_CALLEE_SAVE(pv_native_irq_enable),
+#endif /* CONFIG_PARAVIRT_XXL */
+
+ /* Irq HLT ops. */
.irq.safe_halt = pv_native_safe_halt,
.irq.halt = native_halt,
-#endif /* CONFIG_PARAVIRT_XXL */
/* Mmu ops. */
.mmu.flush_tlb_user = native_flush_tlb_local,
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v7 2/3] x86/tdx: Fix arch_safe_halt() execution for TDX VMs
2025-02-28 1:44 [PATCH v7 0/3] x86/tdx: Fix HLT logic execution for TDX VMs Vishal Annapurve
2025-02-28 1:44 ` [PATCH v7 1/3] x86/paravirt: Move halt paravirt calls under CONFIG_PARAVIRT Vishal Annapurve
@ 2025-02-28 1:44 ` Vishal Annapurve
2025-03-26 8:02 ` [tip: x86/tdx] " tip-bot2 for Vishal Annapurve
2025-02-28 1:44 ` [PATCH v7 3/3] x86/tdx: Emit warning if IRQs are enabled during HLT #VE handling Vishal Annapurve
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Vishal Annapurve @ 2025-02-28 1:44 UTC (permalink / raw)
To: dave.hansen, kirill.shutemov, jgross, ajay.kaher, ak, tony.luck,
thomas.lendacky
Cc: tglx, mingo, bp, hpa, pbonzini, seanjc, kai.huang, chao.p.peng,
isaku.yamahata, sathyanarayanan.kuppuswamy, erdemaktas,
ackerleytng, jxgao, sagis, afranji, kees, jikos, peterz, x86,
linux-kernel, linux-coco, virtualization,
bcm-kernel-feedback-list, Vishal Annapurve, stable
Direct HLT instruction execution causes #VEs for TDX VMs which is routed
to hypervisor via TDCALL. If HLT is executed in STI-shadow, resulting #VE
handler will enable interrupts before TDCALL is routed to hypervisor
leading to missed wakeup events, as current TDX spec doesn't expose
interruptibility state information to allow #VE handler to selectively
enable interrupts.
Commit bfe6ed0c6727 ("x86/tdx: Add HLT support for TDX guests")
prevented the idle routines from executing HLT instruction in STI-shadow.
But it missed the paravirt routine which can be reached via this path
as an example:
kvm_wait() =>
safe_halt() =>
raw_safe_halt() =>
arch_safe_halt() =>
irq.safe_halt() =>
pv_native_safe_halt()
To reliably handle arch_safe_halt() for TDX VMs, introduce explicit
dependency on CONFIG_PARAVIRT and override paravirt halt()/safe_halt()
routines with TDX-safe versions that execute direct TDCALL and needed
interrupt flag updates. Executing direct TDCALL brings in additional
benefit of avoiding HLT related #VEs altogether.
Cc: stable@vger.kernel.org
Fixes: bfe6ed0c6727 ("x86/tdx: Add HLT support for TDX guests")
Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Vishal Annapurve <vannapurve@google.com>
---
arch/x86/Kconfig | 1 +
arch/x86/coco/tdx/tdx.c | 26 +++++++++++++++++++++++++-
arch/x86/include/asm/tdx.h | 4 ++--
arch/x86/kernel/process.c | 2 +-
4 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index be2c311f5118..933c046e8966 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -902,6 +902,7 @@ config INTEL_TDX_GUEST
depends on X86_64 && CPU_SUP_INTEL
depends on X86_X2APIC
depends on EFI_STUB
+ depends on PARAVIRT
select ARCH_HAS_CC_PLATFORM
select X86_MEM_ENCRYPT
select X86_MCE
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 32809a06dab4..6aad910d119d 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -14,6 +14,7 @@
#include <asm/ia32.h>
#include <asm/insn.h>
#include <asm/insn-eval.h>
+#include <asm/paravirt_types.h>
#include <asm/pgtable.h>
#include <asm/set_memory.h>
#include <asm/traps.h>
@@ -398,7 +399,7 @@ static int handle_halt(struct ve_info *ve)
return ve_instr_len(ve);
}
-void __cpuidle tdx_safe_halt(void)
+void __cpuidle tdx_halt(void)
{
const bool irq_disabled = false;
@@ -409,6 +410,16 @@ void __cpuidle tdx_safe_halt(void)
WARN_ONCE(1, "HLT instruction emulation failed\n");
}
+static void __cpuidle tdx_safe_halt(void)
+{
+ tdx_halt();
+ /*
+ * "__cpuidle" section doesn't support instrumentation, so stick
+ * with raw_* variant that avoids tracing hooks.
+ */
+ raw_local_irq_enable();
+}
+
static int read_msr(struct pt_regs *regs, struct ve_info *ve)
{
struct tdx_module_args args = {
@@ -1109,6 +1120,19 @@ void __init tdx_early_init(void)
x86_platform.guest.enc_kexec_begin = tdx_kexec_begin;
x86_platform.guest.enc_kexec_finish = tdx_kexec_finish;
+ /*
+ * Avoid "sti;hlt" execution in TDX guests as HLT induces a #VE that
+ * will enable interrupts before HLT TDCALL invocation if executed
+ * in STI-shadow, possibly resulting in missed wakeup events.
+ *
+ * Modify all possible HLT execution paths to use TDX specific routines
+ * that directly execute TDCALL and toggle the interrupt state as
+ * needed after TDCALL completion. This also reduces HLT related #VEs
+ * in addition to having a reliable halt logic execution.
+ */
+ pv_ops.irq.safe_halt = tdx_safe_halt;
+ pv_ops.irq.halt = tdx_halt;
+
/*
* TDX intercepts the RDMSR to read the X2APIC ID in the parallel
* bringup low level code. That raises #VE which cannot be handled
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index b4b16dafd55e..40f9a97371a9 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -58,7 +58,7 @@ void tdx_get_ve_info(struct ve_info *ve);
bool tdx_handle_virt_exception(struct pt_regs *regs, struct ve_info *ve);
-void tdx_safe_halt(void);
+void tdx_halt(void);
bool tdx_early_handle_ve(struct pt_regs *regs);
@@ -72,7 +72,7 @@ void __init tdx_dump_td_ctls(u64 td_ctls);
#else
static inline void tdx_early_init(void) { };
-static inline void tdx_safe_halt(void) { };
+static inline void tdx_halt(void) { };
static inline bool tdx_early_handle_ve(struct pt_regs *regs) { return false; }
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 6da6769d7254..d11956a178df 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -934,7 +934,7 @@ void __init select_idle_routine(void)
static_call_update(x86_idle, mwait_idle);
} else if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) {
pr_info("using TDX aware idle routine\n");
- static_call_update(x86_idle, tdx_safe_halt);
+ static_call_update(x86_idle, tdx_halt);
} else {
static_call_update(x86_idle, default_idle);
}
--
2.48.1.711.g2feabab25a-goog
^ permalink raw reply [flat|nested] 9+ messages in thread* [tip: x86/tdx] x86/tdx: Fix arch_safe_halt() execution for TDX VMs
2025-02-28 1:44 ` [PATCH v7 2/3] x86/tdx: Fix arch_safe_halt() execution for TDX VMs Vishal Annapurve
@ 2025-03-26 8:02 ` tip-bot2 for Vishal Annapurve
0 siblings, 0 replies; 9+ messages in thread
From: tip-bot2 for Vishal Annapurve @ 2025-03-26 8:02 UTC (permalink / raw)
To: linux-tip-commits
Cc: Vishal Annapurve, Ingo Molnar, Kirill A. Shutemov, Ryan Afranji,
Andy Lutomirski, Brian Gerst, Juergen Gross, H. Peter Anvin,
Linus Torvalds, Josh Poimboeuf, stable, x86, linux-kernel
The following commit has been merged into the x86/tdx branch of tip:
Commit-ID: 9f98a4f4e7216dbe366010b4cdcab6b220f229c4
Gitweb: https://git.kernel.org/tip/9f98a4f4e7216dbe366010b4cdcab6b220f229c4
Author: Vishal Annapurve <vannapurve@google.com>
AuthorDate: Fri, 28 Feb 2025 01:44:15
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 26 Mar 2025 08:51:20 +01:00
x86/tdx: Fix arch_safe_halt() execution for TDX VMs
Direct HLT instruction execution causes #VEs for TDX VMs which is routed
to hypervisor via TDCALL. If HLT is executed in STI-shadow, resulting #VE
handler will enable interrupts before TDCALL is routed to hypervisor
leading to missed wakeup events, as current TDX spec doesn't expose
interruptibility state information to allow #VE handler to selectively
enable interrupts.
Commit bfe6ed0c6727 ("x86/tdx: Add HLT support for TDX guests")
prevented the idle routines from executing HLT instruction in STI-shadow.
But it missed the paravirt routine which can be reached via this path
as an example:
kvm_wait() =>
safe_halt() =>
raw_safe_halt() =>
arch_safe_halt() =>
irq.safe_halt() =>
pv_native_safe_halt()
To reliably handle arch_safe_halt() for TDX VMs, introduce explicit
dependency on CONFIG_PARAVIRT and override paravirt halt()/safe_halt()
routines with TDX-safe versions that execute direct TDCALL and needed
interrupt flag updates. Executing direct TDCALL brings in additional
benefit of avoiding HLT related #VEs altogether.
As tested by Ryan Afranji:
"Tested with the specjbb2015 benchmark. It has heavy lock contention which leads
to many halt calls. TDX VMs suffered a poor score before this patchset.
Verified the major performance improvement with this patchset applied."
Fixes: bfe6ed0c6727 ("x86/tdx: Add HLT support for TDX guests")
Signed-off-by: Vishal Annapurve <vannapurve@google.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Tested-by: Ryan Afranji <afranji@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20250228014416.3925664-3-vannapurve@google.com
---
arch/x86/Kconfig | 1 +
arch/x86/coco/tdx/tdx.c | 26 +++++++++++++++++++++++++-
arch/x86/include/asm/tdx.h | 4 ++--
arch/x86/kernel/process.c | 2 +-
4 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 05b4eca..f614c05 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -878,6 +878,7 @@ config INTEL_TDX_GUEST
depends on X86_64 && CPU_SUP_INTEL
depends on X86_X2APIC
depends on EFI_STUB
+ depends on PARAVIRT
select ARCH_HAS_CC_PLATFORM
select X86_MEM_ENCRYPT
select X86_MCE
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 7772b01..aa0eb40 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -14,6 +14,7 @@
#include <asm/ia32.h>
#include <asm/insn.h>
#include <asm/insn-eval.h>
+#include <asm/paravirt_types.h>
#include <asm/pgtable.h>
#include <asm/set_memory.h>
#include <asm/traps.h>
@@ -398,7 +399,7 @@ static int handle_halt(struct ve_info *ve)
return ve_instr_len(ve);
}
-void __cpuidle tdx_safe_halt(void)
+void __cpuidle tdx_halt(void)
{
const bool irq_disabled = false;
@@ -409,6 +410,16 @@ void __cpuidle tdx_safe_halt(void)
WARN_ONCE(1, "HLT instruction emulation failed\n");
}
+static void __cpuidle tdx_safe_halt(void)
+{
+ tdx_halt();
+ /*
+ * "__cpuidle" section doesn't support instrumentation, so stick
+ * with raw_* variant that avoids tracing hooks.
+ */
+ raw_local_irq_enable();
+}
+
static int read_msr(struct pt_regs *regs, struct ve_info *ve)
{
struct tdx_module_args args = {
@@ -1110,6 +1121,19 @@ void __init tdx_early_init(void)
x86_platform.guest.enc_kexec_finish = tdx_kexec_finish;
/*
+ * Avoid "sti;hlt" execution in TDX guests as HLT induces a #VE that
+ * will enable interrupts before HLT TDCALL invocation if executed
+ * in STI-shadow, possibly resulting in missed wakeup events.
+ *
+ * Modify all possible HLT execution paths to use TDX specific routines
+ * that directly execute TDCALL and toggle the interrupt state as
+ * needed after TDCALL completion. This also reduces HLT related #VEs
+ * in addition to having a reliable halt logic execution.
+ */
+ pv_ops.irq.safe_halt = tdx_safe_halt;
+ pv_ops.irq.halt = tdx_halt;
+
+ /*
* TDX intercepts the RDMSR to read the X2APIC ID in the parallel
* bringup low level code. That raises #VE which cannot be handled
* there.
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 65394aa..4a1922e 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -58,7 +58,7 @@ void tdx_get_ve_info(struct ve_info *ve);
bool tdx_handle_virt_exception(struct pt_regs *regs, struct ve_info *ve);
-void tdx_safe_halt(void);
+void tdx_halt(void);
bool tdx_early_handle_ve(struct pt_regs *regs);
@@ -72,7 +72,7 @@ void __init tdx_dump_td_ctls(u64 td_ctls);
#else
static inline void tdx_early_init(void) { };
-static inline void tdx_safe_halt(void) { };
+static inline void tdx_halt(void) { };
static inline bool tdx_early_handle_ve(struct pt_regs *regs) { return false; }
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 91f6ff6..962c3ce 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -939,7 +939,7 @@ void __init select_idle_routine(void)
static_call_update(x86_idle, mwait_idle);
} else if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) {
pr_info("using TDX aware idle routine\n");
- static_call_update(x86_idle, tdx_safe_halt);
+ static_call_update(x86_idle, tdx_halt);
} else {
static_call_update(x86_idle, default_idle);
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v7 3/3] x86/tdx: Emit warning if IRQs are enabled during HLT #VE handling
2025-02-28 1:44 [PATCH v7 0/3] x86/tdx: Fix HLT logic execution for TDX VMs Vishal Annapurve
2025-02-28 1:44 ` [PATCH v7 1/3] x86/paravirt: Move halt paravirt calls under CONFIG_PARAVIRT Vishal Annapurve
2025-02-28 1:44 ` [PATCH v7 2/3] x86/tdx: Fix arch_safe_halt() execution for TDX VMs Vishal Annapurve
@ 2025-02-28 1:44 ` Vishal Annapurve
2025-03-26 8:02 ` [tip: x86/tdx] " tip-bot2 for Vishal Annapurve
2025-03-22 1:08 ` [PATCH v7 0/3] x86/tdx: Fix HLT logic execution for TDX VMs Vishal Annapurve
2025-03-24 18:53 ` Ryan Afranji
4 siblings, 1 reply; 9+ messages in thread
From: Vishal Annapurve @ 2025-02-28 1:44 UTC (permalink / raw)
To: dave.hansen, kirill.shutemov, jgross, ajay.kaher, ak, tony.luck,
thomas.lendacky
Cc: tglx, mingo, bp, hpa, pbonzini, seanjc, kai.huang, chao.p.peng,
isaku.yamahata, sathyanarayanan.kuppuswamy, erdemaktas,
ackerleytng, jxgao, sagis, afranji, kees, jikos, peterz, x86,
linux-kernel, linux-coco, virtualization,
bcm-kernel-feedback-list, Vishal Annapurve
Direct HLT instruction execution causes #VEs for TDX VMs which is routed
to hypervisor via TDCALL. safe_halt() routines execute HLT in STI-shadow
so IRQs need to remain disabled until the TDCALL to ensure that pending
IRQs are correctly treated as wake events.
Emit warning and fail emulation if IRQs are enabled during HLT #VE handling
to avoid running into scenarios where IRQ wake events are lost resulting in
indefinite HLT execution times.
Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Vishal Annapurve <vannapurve@google.com>
---
arch/x86/coco/tdx/tdx.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 6aad910d119d..a97ddc6a52c3 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -393,6 +393,14 @@ static int handle_halt(struct ve_info *ve)
{
const bool irq_disabled = irqs_disabled();
+ /*
+ * HLT with IRQs enabled is unsafe, as an IRQ that is intended to be a
+ * wake event may be consumed before requesting HLT emulation, leaving
+ * the vCPU blocking indefinitely.
+ */
+ if (WARN_ONCE(!irq_disabled, "HLT emulation with IRQs enabled"))
+ return -EIO;
+
if (__halt(irq_disabled))
return -EIO;
--
2.48.1.711.g2feabab25a-goog
^ permalink raw reply [flat|nested] 9+ messages in thread* [tip: x86/tdx] x86/tdx: Emit warning if IRQs are enabled during HLT #VE handling
2025-02-28 1:44 ` [PATCH v7 3/3] x86/tdx: Emit warning if IRQs are enabled during HLT #VE handling Vishal Annapurve
@ 2025-03-26 8:02 ` tip-bot2 for Vishal Annapurve
0 siblings, 0 replies; 9+ messages in thread
From: tip-bot2 for Vishal Annapurve @ 2025-03-26 8:02 UTC (permalink / raw)
To: linux-tip-commits
Cc: Vishal Annapurve, Ingo Molnar, Kirill A. Shutemov, Ryan Afranji,
Andy Lutomirski, Brian Gerst, Juergen Gross, H. Peter Anvin,
Linus Torvalds, Josh Poimboeuf, x86, linux-kernel
The following commit has been merged into the x86/tdx branch of tip:
Commit-ID: e8f45927ee5d99fa52f14205a2c7ac3820c64457
Gitweb: https://git.kernel.org/tip/e8f45927ee5d99fa52f14205a2c7ac3820c64457
Author: Vishal Annapurve <vannapurve@google.com>
AuthorDate: Fri, 28 Feb 2025 01:44:16
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 26 Mar 2025 08:52:10 +01:00
x86/tdx: Emit warning if IRQs are enabled during HLT #VE handling
Direct HLT instruction execution causes #VEs for TDX VMs which is routed
to hypervisor via TDCALL. safe_halt() routines execute HLT in STI-shadow
so IRQs need to remain disabled until the TDCALL to ensure that pending
IRQs are correctly treated as wake events.
Emit warning and fail emulation if IRQs are enabled during HLT #VE handling
to avoid running into scenarios where IRQ wake events are lost resulting in
indefinite HLT execution times.
Signed-off-by: Vishal Annapurve <vannapurve@google.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Tested-by: Ryan Afranji <afranji@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Link: https://lore.kernel.org/r/20250228014416.3925664-4-vannapurve@google.com
---
arch/x86/coco/tdx/tdx.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index aa0eb40..edab6d6 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -393,6 +393,14 @@ static int handle_halt(struct ve_info *ve)
{
const bool irq_disabled = irqs_disabled();
+ /*
+ * HLT with IRQs enabled is unsafe, as an IRQ that is intended to be a
+ * wake event may be consumed before requesting HLT emulation, leaving
+ * the vCPU blocking indefinitely.
+ */
+ if (WARN_ONCE(!irq_disabled, "HLT emulation with IRQs enabled"))
+ return -EIO;
+
if (__halt(irq_disabled))
return -EIO;
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v7 0/3] x86/tdx: Fix HLT logic execution for TDX VMs
2025-02-28 1:44 [PATCH v7 0/3] x86/tdx: Fix HLT logic execution for TDX VMs Vishal Annapurve
` (2 preceding siblings ...)
2025-02-28 1:44 ` [PATCH v7 3/3] x86/tdx: Emit warning if IRQs are enabled during HLT #VE handling Vishal Annapurve
@ 2025-03-22 1:08 ` Vishal Annapurve
2025-03-24 18:53 ` Ryan Afranji
4 siblings, 0 replies; 9+ messages in thread
From: Vishal Annapurve @ 2025-03-22 1:08 UTC (permalink / raw)
To: dave.hansen, kirill.shutemov, jgross, ajay.kaher, ak, tony.luck,
thomas.lendacky
Cc: tglx, mingo, bp, hpa, pbonzini, seanjc, kai.huang, chao.p.peng,
isaku.yamahata, sathyanarayanan.kuppuswamy, erdemaktas,
ackerleytng, jxgao, sagis, afranji, kees, jikos, peterz, x86,
linux-kernel, linux-coco, virtualization,
bcm-kernel-feedback-list
On Thu, Feb 27, 2025 at 5:44 PM Vishal Annapurve <vannapurve@google.com> wrote:
>
> Direct HLT instruction execution causes #VEs for TDX VMs which is routed
> to hypervisor via TDCALL. safe_halt() routines execute HLT in STI-shadow
> so IRQs need to remain disabled until the TDCALL to ensure that pending
> IRQs are correctly treated as wake events. As per current TDX spec, HLT
> #VE handler doesn't have access to interruptibility state to selectively
> enable interrupts, it ends up enabling interrupts during #VE handling
> before the TDCALL is executed.
>
> Commit bfe6ed0c6727 ("x86/tdx: Add HLT support for TDX guests")
> effectively solved this issue for idle routines by defining TDX specific
> idle routine which directly invokes TDCALL while keeping interrupts
> disabled, but missed handling arch_safe_halt(). This series intends to fix
> arch_safe_halt() execution for TDX VMs.
>
> Changes introduced by the series include:
> - Move *halt() variants outside CONFIG_PARAVIRT_XXL and under
> CONFIG_PARAVIRT [1].
> - Add explicit dependency on CONFIG_PARAVIRT for TDX VMs.
> - Route "sti; hlt" sequences via tdx_safe_halt() for reliability.
> - Route "hlt" sequences via tdx_halt() to avoid unnecessary #VEs.
> - Warn and fail emulation if HLT #VE emulation executes with interrupts
> enabled.
>
> Changes since v6:
> 1) Addressed Kirills's comments.
> 2) Fixed a build failure.
>
> v6: https://lore.kernel.org/lkml/20250225004704.603652-1-vannapurve@google.com/
>
> Kirill A. Shutemov (1):
> x86/paravirt: Move halt paravirt calls under CONFIG_PARAVIRT
>
> Vishal Annapurve (2):
> x86/tdx: Fix arch_safe_halt() execution for TDX VMs
> x86/tdx: Emit warning if IRQs are enabled during HLT #VE handling
>
> arch/x86/Kconfig | 1 +
> arch/x86/coco/tdx/tdx.c | 34 ++++++++++++++++++++++-
> arch/x86/include/asm/irqflags.h | 40 +++++++++++++++------------
> arch/x86/include/asm/paravirt.h | 20 +++++++-------
> arch/x86/include/asm/paravirt_types.h | 3 +-
> arch/x86/include/asm/tdx.h | 4 +--
> arch/x86/kernel/paravirt.c | 14 ++++++----
> arch/x86/kernel/process.c | 2 +-
> 8 files changed, 78 insertions(+), 40 deletions(-)
>
> --
> 2.48.1.711.g2feabab25a-goog
>
Just a ping for review of this series if anything more needs to be discussed.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v7 0/3] x86/tdx: Fix HLT logic execution for TDX VMs
2025-02-28 1:44 [PATCH v7 0/3] x86/tdx: Fix HLT logic execution for TDX VMs Vishal Annapurve
` (3 preceding siblings ...)
2025-03-22 1:08 ` [PATCH v7 0/3] x86/tdx: Fix HLT logic execution for TDX VMs Vishal Annapurve
@ 2025-03-24 18:53 ` Ryan Afranji
4 siblings, 0 replies; 9+ messages in thread
From: Ryan Afranji @ 2025-03-24 18:53 UTC (permalink / raw)
To: vannapurve
Cc: ackerleytng, afranji, ajay.kaher, ak, bcm-kernel-feedback-list,
bp, chao.p.peng, dave.hansen, erdemaktas, hpa, isaku.yamahata,
jgross, jikos, jxgao, kai.huang, kees, kirill.shutemov,
linux-coco, linux-kernel, mingo, pbonzini, peterz, sagis,
sathyanarayanan.kuppuswamy, seanjc, tglx, thomas.lendacky,
tony.luck, virtualization, x86
Tested with the specjbb2015 benchmark. It has heavy lock contention which leads
to many halt calls. TDX VMs suffered a poor score before this patchset.
Verified the major performance improvement with this patchset applied.
Tested-by: Ryan Afranji <afranji@google.com>
^ permalink raw reply [flat|nested] 9+ messages in thread