lkml.org 
[lkml]   [2022]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH v2 22/44] 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: Peter Zijlstra (Intel) <peterz@infradead.org>
    ---
    arch/x86/coco/tdx/tdcall.S | 13 -------------
    arch/x86/coco/tdx/tdx.c | 23 ++++-------------------
    arch/x86/include/asm/shared/tdx.h | 1 -
    3 files changed, 4 insertions(+), 33 deletions(-)

    --- a/arch/x86/coco/tdx/tdcall.S
    +++ b/arch/x86/coco/tdx/tdcall.S
    @@ -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

    /*
    --- a/arch/x86/coco/tdx/tdx.c
    +++ b/arch/x86/coco/tdx/tdx.c
    @@ -169,7 +169,7 @@ static int ve_instr_len(struct ve_info *
    }
    }

    -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,
    @@ -189,20 +189,14 @@ static u64 __cpuidle __halt(const bool i
    * 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);
    @@ -210,22 +204,13 @@ static int handle_halt(struct ve_info *v

    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)
    --- a/arch/x86/include/asm/shared/tdx.h
    +++ b/arch/x86/include/asm/shared/tdx.h
    @@ -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 "

    \
     
     \ /
      Last update: 2022-09-19 12:23    [W:4.097 / U:0.124 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site