Re: [PATCH v5 01/12] x86/paravirt: Move halt paravirt calls under CONFIG_PARAVIRT

From: Kuppuswamy, Sathyanarayanan
Date: Tue Aug 17 2021 - 09:17:01 EST

On 8/17/21 5:50 AM, Juergen Gross wrote:
On 12.08.21 09:18, Borislav Petkov wrote:
On Wed, Aug 04, 2021 at 11:13:18AM -0700, Kuppuswamy Sathyanarayanan wrote:
From: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>

CONFIG_PARAVIRT_XXL is mainly defined/used by XEN PV guests. For
other VM guest types, features supported under CONFIG_PARAVIRT
are self sufficient. CONFIG_PARAVIRT mainly provides support for
TLB flush operations and time related operations.

For TDX guest as well, paravirt calls under CONFIG_PARVIRT meets
most of its requirement except the need of HLT and SAFE_HLT
paravirt calls, which is currently defined under

Since enabling CONFIG_PARAVIRT_XXL is too bloated for TDX guest
like platforms, move HLT and SAFE_HLT paravirt calls under

Moving HLT and SAFE_HLT paravirt calls are not fatal and should not
break any functionality for current users of CONFIG_PARAVIRT.

Co-developed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
Reviewed-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
Reviewed-by: Tony Luck <tony.luck@xxxxxxxxx>

You need to do this before sending your patches:

./scripts/ /tmp/tdx.01
Thomas Gleixner <tglx@xxxxxxxxxxxxx> (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),commit_signer:1/6=17%)
Ingo Molnar <mingo@xxxxxxxxxx> (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
Borislav Petkov <bp@xxxxxxxxx> (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),commit_signer:6/6=100%)
x86@xxxxxxxxxx (maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
"H. Peter Anvin" <hpa@xxxxxxxxx> (reviewer:X86 ARCHITECTURE (32-BIT AND 64-BIT))
Juergen Gross <jgross@xxxxxxxx> (supporter:PARAVIRT_OPS INTERFACE,commit_signer:5/6=83%,authored:5/6=83%,added_lines:15/16=94%,removed_lines:38/39=97%)
Deep Shah <sdeep@xxxxxxxxxx> (supporter:PARAVIRT_OPS INTERFACE)
"VMware, Inc." <pv-drivers@xxxxxxxxxx> (supporter:PARAVIRT_OPS INTERFACE)

and CC also the supporters - I'm pretty sure at least Juergen would like
to be kept up-to-date on pv changes. I'll CC him and the others now and
leave in the whole diff but make sure you do that in the future please.

  arch/x86/include/asm/irqflags.h       | 40 +++++++++++++++------------
  arch/x86/include/asm/paravirt.h       | 20 +++++++-------
  arch/x86/include/asm/paravirt_types.h |  3 +-
  arch/x86/kernel/paravirt.c            |  4 ++-
  4 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
index c5ce9845c999..f3bb33b1715d 100644
--- a/arch/x86/include/asm/irqflags.h
+++ b/arch/x86/include/asm/irqflags.h
@@ -59,6 +59,28 @@ static inline __cpuidle void native_halt(void)
+#ifndef __ASSEMBLY__
+ * Used in the idle loop; sti takes one instruction cycle
+ * to complete:
+ */
+static inline __cpuidle void arch_safe_halt(void)
+    native_safe_halt();
+ * Used when interrupts are already enabled or to
+ * shutdown the processor:
+ */
+static inline __cpuidle void halt(void)
+    native_halt();
+#endif /* __ASSEMBLY__ */
+#endif /* CONFIG_PARAVIRT */
  #include <asm/paravirt.h>

Did you test this with CONFIG_PARAVIRT enabled and CONFIG_PARAVIRT_XXL

I'm asking because in this case I don't see where halt() and
arch_safe_halt() would be defined in case someone is including
asm/irqflags.h and not asm/paravirt.h.

We have tested both cases and did not hit any issues.


diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 04cafc057bed..124e0f6c5d1c 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -283,9 +283,11 @@ struct paravirt_patch_template pv_ops = {
      .irq.save_fl        = __PV_IS_CALLEE_SAVE(native_save_fl),
      .irq.irq_disable    = __PV_IS_CALLEE_SAVE(native_irq_disable),
      .irq.irq_enable        = __PV_IS_CALLEE_SAVE(native_irq_enable),
+    /* Irq HLT ops. */

What's that comment for?

I agree, please drop it.

Yes. I will drop it.


Sathyanarayanan Kuppuswamy
Linux Kernel Developer