2023-07-21 20:25:25

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH v4 10/19] x86/virt: KVM: Move VMXOFF helpers into KVM VMX

Now that VMX is disabled in emergencies via the virt callbacks, move the
VMXOFF helpers into KVM, the only remaining user.

No functional change intended.

Reviewed-by: Kai Huang <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/include/asm/virtext.h | 42 ----------------------------------
arch/x86/kvm/vmx/vmx.c | 29 ++++++++++++++++++++---
2 files changed, 26 insertions(+), 45 deletions(-)

diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h
index b1171a5ad452..a27801f2bc71 100644
--- a/arch/x86/include/asm/virtext.h
+++ b/arch/x86/include/asm/virtext.h
@@ -19,48 +19,6 @@
#include <asm/svm.h>
#include <asm/tlbflush.h>

-/*
- * VMX functions:
- */
-/**
- * cpu_vmxoff() - Disable VMX on the current CPU
- *
- * Disable VMX and clear CR4.VMXE (even if VMXOFF faults)
- *
- * Note, VMXOFF causes a #UD if the CPU is !post-VMXON, but it's impossible to
- * atomically track post-VMXON state, e.g. this may be called in NMI context.
- * Eat all faults as all other faults on VMXOFF faults are mode related, i.e.
- * faults are guaranteed to be due to the !post-VMXON check unless the CPU is
- * magically in RM, VM86, compat mode, or at CPL>0.
- */
-static inline int cpu_vmxoff(void)
-{
- asm_volatile_goto("1: vmxoff\n\t"
- _ASM_EXTABLE(1b, %l[fault])
- ::: "cc", "memory" : fault);
-
- cr4_clear_bits(X86_CR4_VMXE);
- return 0;
-
-fault:
- cr4_clear_bits(X86_CR4_VMXE);
- return -EIO;
-}
-
-static inline int cpu_vmx_enabled(void)
-{
- return __read_cr4() & X86_CR4_VMXE;
-}
-
-/** Disable VMX if it is enabled on the current CPU
- */
-static inline void __cpu_emergency_vmxoff(void)
-{
- if (cpu_vmx_enabled())
- cpu_vmxoff();
-}
-
-
/*
* SVM functions:
*/
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 71571cd9adbb..6f4fcd82fa6e 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -47,7 +47,6 @@
#include <asm/mshyperv.h>
#include <asm/mwait.h>
#include <asm/spec-ctrl.h>
-#include <asm/virtext.h>
#include <asm/vmx.h>

#include "capabilities.h"
@@ -744,6 +743,29 @@ static int vmx_set_guest_uret_msr(struct vcpu_vmx *vmx,
return ret;
}

+/*
+ * Disable VMX and clear CR4.VMXE (even if VMXOFF faults)
+ *
+ * Note, VMXOFF causes a #UD if the CPU is !post-VMXON, but it's impossible to
+ * atomically track post-VMXON state, e.g. this may be called in NMI context.
+ * Eat all faults as all other faults on VMXOFF faults are mode related, i.e.
+ * faults are guaranteed to be due to the !post-VMXON check unless the CPU is
+ * magically in RM, VM86, compat mode, or at CPL>0.
+ */
+static int kvm_cpu_vmxoff(void)
+{
+ asm_volatile_goto("1: vmxoff\n\t"
+ _ASM_EXTABLE(1b, %l[fault])
+ ::: "cc", "memory" : fault);
+
+ cr4_clear_bits(X86_CR4_VMXE);
+ return 0;
+
+fault:
+ cr4_clear_bits(X86_CR4_VMXE);
+ return -EIO;
+}
+
static void vmx_emergency_disable(void)
{
int cpu = raw_smp_processor_id();
@@ -753,7 +775,8 @@ static void vmx_emergency_disable(void)
loaded_vmcss_on_cpu_link)
vmcs_clear(v->vmcs);

- __cpu_emergency_vmxoff();
+ if (__read_cr4() & X86_CR4_VMXE)
+ kvm_cpu_vmxoff();
}

static void __loaded_vmcs_clear(void *arg)
@@ -2818,7 +2841,7 @@ static void vmx_hardware_disable(void)
{
vmclear_local_loaded_vmcss();

- if (cpu_vmxoff())
+ if (kvm_cpu_vmxoff())
kvm_spurious_fault();

hv_reset_evmcs();
--
2.41.0.487.g6d72f3e995-goog



2023-07-28 10:27:25

by Huang, Kai

[permalink] [raw]
Subject: Re: [PATCH v4 10/19] x86/virt: KVM: Move VMXOFF helpers into KVM VMX

On Fri, 2023-07-28 at 17:08 +0800, Xu, Yilun wrote:
> On 2023-07-21 at 13:18:50 -0700, Sean Christopherson wrote:
> > Now that VMX is disabled in emergencies via the virt callbacks, move the
> > VMXOFF helpers into KVM, the only remaining user.
>
> Not sure if it's too early to mention.
>
> Intel TDX Connect could be a future user, it is the TDX extension for
> device security.
>
> TDX uses SEAMCALL to interact with TDX Module, and SEAMCALL execution
> requires VMXON. This is also true for TDX Connect. But TDX Connect
> covers more controls out of KVM scope, like PCI IDE, SPDM, IOMMU.
> IOW, other driver modules may use SEAMCALLs and in turn use VMXON/OFF
> for TDX Connect.
>
> I'm wondering if then we should again move VMXON/OFF helpers back to
> virtext.h
>
> Or, could we just keep vmxoff unchanged now?
>

I'd say we should just proceed with Sean's this patch. Moving VMXON/VMXOFF out
from KVM needs additional things besides keeping the basic vmxon()/vmxoff()
functions at core-x86 in order to handle multiple callers from multiple kernel
components. And vmxon()/vmxoff() aren't necessary to be in virtext.h either,
depending on the implementation. Let's handle that when we need that in the
future.

2023-07-28 10:31:14

by Xu Yilun

[permalink] [raw]
Subject: Re: [PATCH v4 10/19] x86/virt: KVM: Move VMXOFF helpers into KVM VMX

On 2023-07-21 at 13:18:50 -0700, Sean Christopherson wrote:
> Now that VMX is disabled in emergencies via the virt callbacks, move the
> VMXOFF helpers into KVM, the only remaining user.

Not sure if it's too early to mention.

Intel TDX Connect could be a future user, it is the TDX extension for
device security.

TDX uses SEAMCALL to interact with TDX Module, and SEAMCALL execution
requires VMXON. This is also true for TDX Connect. But TDX Connect
covers more controls out of KVM scope, like PCI IDE, SPDM, IOMMU.
IOW, other driver modules may use SEAMCALLs and in turn use VMXON/OFF
for TDX Connect.

I'm wondering if then we should again move VMXON/OFF helpers back to
virtext.h

Or, could we just keep vmxoff unchanged now?

Thanks,
Yilun