Commit e80a48ba authored by Peter Zijlstra's avatar Peter Zijlstra Committed by Ingo Molnar
Browse files

x86/tdx: Remove TDX_HCALL_ISSUE_STI 



Now that arch_cpu_idle() is expected to return with IRQs disabled,
avoid the useless STI/CLI dance.

Per the specs this is supposed to work, but nobody has yet relied up
this behaviour so broken implementations are possible.

Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
Tested-by: default avatarTony Lindgren <tony@atomide.com>
Tested-by: default avatarUlf Hansson <ulf.hansson@linaro.org>
Acked-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: default avatarFrederic Weisbecker <frederic@kernel.org>
Link: https://lore.kernel.org/r/20230112195540.682137572@infradead.org
parent 89b30987

arch/x86/coco/tdx/tdcall.S

+0 −13
Original line number Diff line number Diff line
@@ -139,19 +139,6 @@ SYM_FUNC_START(__tdx_hypercall) 


	movl $TDVMCALL_EXPOSE_REGS_MASK, %ecx



	/*

	 * For the idle loop STI needs to be called directly before the TDCALL

	 * that enters idle (EXIT_REASON_HLT case). STI instruction enables

	 * interrupts only one instruction later. If there is a window between

	 * STI and the instruction that emulates the HALT state, there is a

	 * chance for interrupts to happen in this window, which can delay the

	 * HLT operation indefinitely. Since this is the not the desired

	 * result, conditionally call STI before TDCALL.

	 */

	testq $TDX_HCALL_ISSUE_STI, %rsi

	jz .Lskip_sti

	sti

.Lskip_sti:

	tdcall



	/*

arch/x86/coco/tdx/tdx.c

+4 −19
Original line number Diff line number Diff line
@@ -220,7 +220,7 @@ static int ve_instr_len(struct ve_info *ve) 
	}

}



static u64 __cpuidle __halt(const bool irq_disabled, const bool do_sti)

static u64 __cpuidle __halt(const bool irq_disabled)

{

	struct tdx_hypercall_args args = {

		.r10 = TDX_HYPERCALL_STANDARD,
@@ -240,20 +240,14 @@ static u64 __cpuidle __halt(const bool irq_disabled, const bool do_sti) 
	 * can keep the vCPU in virtual HLT, even if an IRQ is

	 * pending, without hanging/breaking the guest.

	 */

	return __tdx_hypercall(&args, do_sti ? TDX_HCALL_ISSUE_STI : 0);

	return __tdx_hypercall(&args, 0);

}



static int handle_halt(struct ve_info *ve)

{

	/*

	 * Since non safe halt is mainly used in CPU offlining

	 * and the guest will always stay in the halt state, don't

	 * call the STI instruction (set do_sti as false).

	 */

	const bool irq_disabled = irqs_disabled();

	const bool do_sti = false;



	if (__halt(irq_disabled, do_sti))

	if (__halt(irq_disabled))

		return -EIO;



	return ve_instr_len(ve);
@@ -261,22 +255,13 @@ static int handle_halt(struct ve_info *ve) 


void __cpuidle tdx_safe_halt(void)

{

	 /*

	  * For do_sti=true case, __tdx_hypercall() function enables

	  * interrupts using the STI instruction before the TDCALL. So

	  * set irq_disabled as false.

	  */

	const bool irq_disabled = false;

	const bool do_sti = true;



	/*

	 * Use WARN_ONCE() to report the failure.

	 */

	if (__halt(irq_disabled, do_sti))

	if (__halt(irq_disabled))

		WARN_ONCE(1, "HLT instruction emulation failed\n");



	/* XXX I can't make sense of what @do_sti actually does */

	raw_local_irq_disable();

}



static int read_msr(struct pt_regs *regs, struct ve_info *ve)

arch/x86/include/asm/shared/tdx.h

+0 −1
Original line number Diff line number Diff line
@@ -8,7 +8,6 @@ 
#define TDX_HYPERCALL_STANDARD  0



#define TDX_HCALL_HAS_OUTPUT	BIT(0)

#define TDX_HCALL_ISSUE_STI	BIT(1)



#define TDX_CPUID_LEAF_ID	0x21

#define TDX_IDENT		"IntelTDX    "