On Wed, Jan 29, 2025 at 11:25:25PM +0000, Vishal Annapurve wrote:
Direct HLT instruction execution causes #VEs for TDX VMs which is routed to hypervisor via tdvmcall. This process renders HLT instruction execution inatomic, so any preceding instructions like STI/MOV SS will end up enabling interrupts before the HLT instruction is routed to the hypervisor. This creates scenarios where interrupts could land during HLT instruction emulation without aborting halt operation leading to idefinite halt wait times.
Commit bfe6ed0c6727 ("x86/tdx: Add HLT support for TDX guests") already upgraded x86_idle() to invoke tdvmcall to avoid such scenarios, but it didn't cover pv_native_safe_halt() which can be invoked using raw_safe_halt() from call sites like acpi_safe_halt().
raw_safe_halt() also returns with interrupts enabled so upgrade tdx_safe_halt() to enable interrupts by default and ensure that paravirt safe_halt() executions invoke tdx_safe_halt(). Earlier x86_idle() is now handled via tdx_idle() which simply invokes tdvmcall while preserving irq state.
To avoid future call sites which cause HLT instruction emulation with irqs enabled, add a warn and fail the HLT instruction emulation.
Cc: stable@vger.kernel.org Fixes: bfe6ed0c6727 ("x86/tdx: Add HLT support for TDX guests") Signed-off-by: Vishal Annapurve vannapurve@google.com
Changes since V1:
- Addressed comments from Dave H
- Comment regarding adding a check for TDX VMs in halt path is not resolved in v2, would like feedback around better place to do so, maybe in pv_native_safe_halt (?).
- Added a new version of tdx_safe_halt() that will enable interrupts.
- Previous tdx_safe_halt() implementation is moved to newly introduced
tdx_idle().
V1: https://lore.kernel.org/lkml/Z5l6L3Hen9_Y3SGC@google.com/T/
arch/x86/coco/tdx/tdx.c | 23 ++++++++++++++++++++++- arch/x86/include/asm/tdx.h | 2 +- arch/x86/kernel/process.c | 2 +- 3 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c index 0d9b090b4880..cc2a637dca15 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> @@ -380,13 +381,18 @@ static int handle_halt(struct ve_info *ve) { const bool irq_disabled = irqs_disabled();
- if (!irq_disabled) {
WARN_ONCE(1, "HLT instruction emulation unsafe with irqs enabled\n");return -EIO;- }
I think it is worth to putting this into a separate patch and not backport. The rest of the patch is bugfix and this doesn't belong.
Otherwise, looks good to me:
Reviewed-by: Kirill A. Shutemov kirill.shutemov@linux.intel.com@linux.intel.com>