[v7] x86/tdx: Fix HLT logic execution for TDX VMs

[PATCH v7 0/3] x86/tdx: Fix HLT logic execution for TDX VMs

Posted by Vishal Annapurve 9 months, 3 weeks ago

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

Re: [PATCH v7 0/3] x86/tdx: Fix HLT logic execution for TDX VMs

Posted by Ryan Afranji 8 months, 3 weeks ago

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>

Re: [PATCH v7 0/3] x86/tdx: Fix HLT logic execution for TDX VMs

Posted by Vishal Annapurve 8 months, 4 weeks ago

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.