lkml.org 
[lkml]   [2021]   [Dec]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 05/26] x86/tdx: Add HLT support for TDX guests (#VE approach)
On Tue, Dec 14, 2021 at 06:02:43PM +0300, Kirill A. Shutemov wrote:
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index e9ee8b526319..273e4266b2c1 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -46,6 +46,7 @@
> #include <asm/proto.h>
> #include <asm/frame.h>
> #include <asm/unwind.h>
> +#include <asm/tdx.h>
>
> #include "process.h"
>
> @@ -864,6 +865,12 @@ void select_idle_routine(const struct cpuinfo_x86 *c)
> if (x86_idle || boot_option_idle_override == IDLE_POLL)
> return;
>
> + if (cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) {
> + x86_idle = tdx_guest_idle;
> + pr_info("using TDX aware idle routine\n");
> + return;
> + }

Why isn't this part of the following if-else if-else if... noodle?

> +
> if (boot_cpu_has_bug(X86_BUG_AMD_E400)) {
> pr_info("using AMD E400 aware idle routine\n");
> x86_idle = amd_e400_idle;

This one starting here: ^^^^

> diff --git a/arch/x86/kernel/tdcall.S b/arch/x86/kernel/tdcall.S
> index ee52dde01b24..e19187048be8 100644
> --- a/arch/x86/kernel/tdcall.S
> +++ b/arch/x86/kernel/tdcall.S
> @@ -3,6 +3,7 @@
> #include <asm/asm.h>
> #include <asm/frame.h>
> #include <asm/unwind_hints.h>
> +#include <uapi/asm/vmx.h>
>
> #include <linux/linkage.h>
> #include <linux/bits.h>
> @@ -39,6 +40,13 @@
> */
> #define tdcall .byte 0x66,0x0f,0x01,0xcc
>
> +/*
> + * Used in the __tdx_hypercall() function to test R15 register content
> + * and optionally include the STI instruction before the TDCALL
> + * instruction (for EXIT_REASON_HLT case).

"Used in __tdx_hypercall() to determine whether to enable interrupts
before issuing TDCALL for the EXIT_REASON_HLT case."

Plain and simple.

> + */
> +#define do_sti 0x01

#define ENABLE_IRQS_BEFORE_HLT 0x01

and when you call it that, you don't even need the comment above it
because the name is self-explanatory.

> +
> /*
> * __tdx_module_call() - Used by TDX guests to request services from
> * the TDX module (does not include VMM services).
> @@ -231,6 +239,30 @@ 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, add
> + * support to conditionally call STI before TDCALL.

"add support"?

> + *
> + * Since STI instruction is only required for the idle case
> + * (a special case of EXIT_REASON_HLT), use the r15 register
> + * value to identify it. Since the R15 register is not used
> + * by the VMM as per EXIT_REASON_HLT ABI, re-use it in
> + * software to identify the STI case.
> + */
> + cmpl $EXIT_REASON_HLT, %r11d
> + jne skip_sti
> + cmpl $do_sti, %r15d
> + jne skip_sti
> + /* Set R15 register to 0, it is unused in EXIT_REASON_HLT case */
> + xor %r15, %r15
> + sti
> +skip_sti:

.Lskip_sti:

> tdcall
>
> /* Restore output pointer to R9 */
> diff --git a/arch/x86/kernel/tdx.c b/arch/x86/kernel/tdx.c
> index b6d0e45e6589..6749ca3b2e3d 100644
> --- a/arch/x86/kernel/tdx.c
> +++ b/arch/x86/kernel/tdx.c
> @@ -6,6 +6,7 @@
>
> #include <linux/cpufeature.h>
> #include <asm/tdx.h>
> +#include <asm/vmx.h>
>
> /* TDX Module Call Leaf IDs */
> #define TDX_GET_VEINFO 3

...

> +void __cpuidle tdx_guest_idle(void)
> +{
> + tdx_safe_halt();
> +}

That wrapper looks useless...

--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette

\
 
 \ /
  Last update: 2021-12-28 20:09    [W:0.376 / U:4.244 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site