KVM: x86: add per-vCPU exits disable capability

Since VMX and SVM both would never update the control bits if exits are disable after vCPUs are created, only allow setting exits disable flag before vCPU creation.

Signed-off-by Sean Christopherson <[email protected]>

Signed-off-by Kechen Lu <[email protected]>

---
 Documentation/virt/kvm/api.rst | 1 +
 arch/x86/kvm/x86.c             | 6 ++++++
 2 files changed, 7 insertions(+)

Documentation/virt/kvm/api.rst +1 -0

index aeeb071c7688..d1c50b95bbc1 100644
@@ -6581,6 +6581,7 @@ branch to guests' 0x200 interrupt vector.
 :Architectures: x86
 :Parameters: args[0] defines which exits are disabled
 :Returns: 0 on success, -EINVAL when args[0] contains invalid exits
+          or if any vCPU has already been created
 
 Valid bits in args[0] are::
 

arch/x86/kvm/x86.c +6 -0

index 0cf1082455df..37529c0c279d 100644
@@ -5764,6 +5764,10 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 		if (cap->args[0] & ~KVM_X86_DISABLE_VALID_EXITS)
 			break;
 
+		mutex_lock(&kvm->lock);
+		if (kvm->created_vcpus)
+			goto disable_exits_unlock;
+
 		if ((cap->args[0] & KVM_X86_DISABLE_EXITS_MWAIT) &&
 			kvm_can_mwait_in_guest())
 			kvm->arch.mwait_in_guest = true;

@@ -5774,6 +5778,8 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 		if (cap->args[0] & KVM_X86_DISABLE_EXITS_CSTATE)
 			kvm->arch.cstate_in_guest = true;
 		r = 0;
+disable_exits_unlock:
+		mutex_unlock(&kvm->lock);
 		break;
 	case KVM_CAP_MSR_PLATFORM_INFO:
 		kvm->arch.guest_can_read_msr_platform_info = cap->args[0];

Signature · 1 line

2.30.2

-----Original Message-----
From: Sean Christopherson <[email protected]>
Sent: Monday, January 10, 2022 10:50 AM
To: Kechen Lu <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Somdutta Roy <[email protected]>; [email protected]
Subject: Re: [RFC PATCH v2 1/3] KVM: x86: only allow exits disable before vCPUs created

External email: Use caution opening links or attachments

Quoted 4 lines · Kechen Lu

Since VMX and SVM both would never update the control bits if exits are disable after vCPUs are created, only allow setting exits disable flag before vCPU creation.

Quoted 1 line

Signed-off-by Sean Christopherson <[email protected]>

For this to carry my SOB, I should be attributed as the author, or add a
Co-developed-by: for me. I'm also totally ok with a Suggested-by: or Reported-by:

And we should at least have

Fixes 4d5422cea3b6 ("KVM: X86: Provide a capability to disable MWAIT

intercepts")

andy maybe Cc: [email protected], though I'm not entirely sure this is stable material as it could in theory do more harm than good if there's a busted userspace out there.

If this doesn't carry my SOB...

Reviewed-by Sean Christopherson <[email protected]>

Quoted 47 lines

Signed-off-by Kechen Lu <[email protected]>

---
 Documentation/virt/kvm/api.rst | 1 +
 arch/x86/kvm/x86.c             | 6 ++++++
 2 files changed, 7 insertions(+)

(quoted diff) +0 -0

b/Documentation/virt/kvm/api.rst index aeeb071c7688..d1c50b95bbc1 100644

Documentation/virt/kvm/api.rst +1 -0

@@ -6581,6 +6581,7 @@ branch to guests' 0x200 interrupt vector.
 :Architectures: x86
 :Parameters: args[0] defines which exits are disabled
 :Returns: 0 on success, -EINVAL when args[0] contains invalid exits
+          or if any vCPU has already been created
 
 Valid bits in args[0] are::
 

arch/x86/kvm/x86.c index renamed from arch/x86/kvm/x86.c +0 -0

0cf1082455df..37529c0c279d 100644

arch/x86/kvm/x86.c +7 -0

@@ -5764,6 +5764,10 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
              if (cap->args[0] & ~KVM_X86_DISABLE_VALID_EXITS)
                      break;
 
+             mutex_lock(&kvm->lock);
+             if (kvm->created_vcpus)
+                     goto disable_exits_unlock;
+
              if ((cap->args[0] & KVM_X86_DISABLE_EXITS_MWAIT) &&
                      kvm_can_mwait_in_guest())
                      kvm->arch.mwait_in_guest = true; @@ -5774,6
+5778,8 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
              if (cap->args[0] & KVM_X86_DISABLE_EXITS_CSTATE)
                      kvm->arch.cstate_in_guest = true;
              r = 0;
+disable_exits_unlock:
+             mutex_unlock(&kvm->lock);
              break;
      case KVM_CAP_MSR_PLATFORM_INFO:
              kvm->arch.guest_can_read_msr_platform_info =

cap->args[0];
--
2.30.2

For futher extensions on finer-grained control on per-vCPU exits disable control, and because VM-scoped restricted to set before vCPUs creation, runtime disabled exits flag check could be purely vCPU scope.

Suggested-by Sean Christopherson <[email protected]>

Signed-off-by Kechen Lu <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |  5 +++++
 arch/x86/kvm/cpuid.c            |  2 +-
 arch/x86/kvm/lapic.c            |  2 +-
 arch/x86/kvm/svm/svm.c          | 10 +++++-----
 arch/x86/kvm/vmx/vmx.c          | 16 ++++++++--------
 arch/x86/kvm/x86.c              |  6 +++++-
 arch/x86/kvm/x86.h              | 16 ++++++++--------
 7 files changed, 33 insertions(+), 24 deletions(-)

arch/x86/include/asm/kvm_host.h +5 -0

index 2164b9f4c7b0..edc5fca4d8c8 100644
@@ -908,6 +908,11 @@ struct kvm_vcpu_arch {
 #if IS_ENABLED(CONFIG_HYPERV)
 	hpa_t hv_root_tdp;
 #endif
+
+	bool mwait_in_guest;
+	bool hlt_in_guest;
+	bool pause_in_guest;
+	bool cstate_in_guest;
 };
 
 struct kvm_lpage_info {

arch/x86/kvm/cpuid.c +0 -0

index 07e9215e911d..6291e15710ba 100644

arch/x86/kvm/lapic.c +0 -0

 		    kvm_can_post_timer_interrupt(vcpu));
 }
 EXPORT_SYMBOL_GPL(kvm_can_use_hv_timer);

-----Original Message-----
From: Sean Christopherson <[email protected]>
Sent: Monday, January 10, 2022 11:36 AM
To: Kechen Lu <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Somdutta Roy <[email protected]>; [email protected]
Subject: Re: [RFC PATCH v2 2/3] KVM: x86: move ()_in_guest checking to vCPU scope

External email: Use caution opening links or attachments

The shortlog is weird, I get what you're going for with the "()", but it honestly looks like a typo :-) And add "power management" so that there's a bit more context in the shortlog? Maybe this?

KVM: x86: Move *_in_guest power management flags to vCPU scope

Quoted 4 lines

For futher extensions on finer-grained control on per-vCPU exits disable control, and because VM-scoped restricted to set before vCPUs creation, runtime disabled exits flag check could be purely vCPU scope.

State what the patch does, not what it "could" do. E.g.

Make the runtime disabled mwait/hlt/pause/cstate exits flags vCPU scope to allow finer-grained, per-vCPU control. The VM-scoped control is only allowed before vCPUs are created, thus preserving the existing behavior is a simple matter of snapshotting the flags at vCPU creation.

A few nits below that aren't even from this path, but otherwise,

Reviewed-by Sean Christopherson <[email protected]>

arch/x86/kvm/cpuid.c void kvm_update_cpuid_runtime(struct kvm_vcpu

Quoted 34 lines

Suggested-by Sean Christopherson <[email protected]>

Signed-off-by Kechen Lu <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |  5 +++++
 arch/x86/kvm/cpuid.c            |  2 +-
 arch/x86/kvm/lapic.c            |  2 +-
 arch/x86/kvm/svm/svm.c          | 10 +++++-----
 arch/x86/kvm/vmx/vmx.c          | 16 ++++++++--------
 arch/x86/kvm/x86.c              |  6 +++++-
 arch/x86/kvm/x86.h              | 16 ++++++++--------
 7 files changed, 33 insertions(+), 24 deletions(-)

(quoted diff) +0 -0

b/arch/x86/include/asm/kvm_host.h index 2164b9f4c7b0..edc5fca4d8c8 100644

arch/x86/include/asm/kvm_host.h +5 -0

@@ -908,6 +908,11 @@ struct kvm_vcpu_arch {  #if
 IS_ENABLED(CONFIG_HYPERV)
      hpa_t hv_root_tdp;
 #endif
+
+     bool mwait_in_guest;
+     bool hlt_in_guest;
+     bool pause_in_guest;
+     bool cstate_in_guest;
 };
 
 struct kvm_lpage_info {

arch/x86/kvm/cpuid.c index renamed from arch/x86/kvm/cpuid.c +0 -0

07e9215e911d..6291e15710ba 100644

arch/x86/kvm/cpuid.c +0 -0

@@ -177,7 +177,7 @@ void kvm_update_cpuid_runtime(struct kvm_vcpu

*vcpu)

arch/x86/kvm/cpuid.c void kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu) · line 181

Quoted 6 lines

arch/x86/kvm/cpuid.c +1 -1

              best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
 
      best = kvm_find_kvm_cpuid_features(vcpu);
-     if (kvm_hlt_in_guest(vcpu->kvm) && best &&
+     if (kvm_hlt_in_guest(vcpu) && best &&
              (best->eax & (1 << KVM_FEATURE_PV_UNHALT)))

Can you (or Paolo?) opportunistically align this? And maybe even shuffle the check on "best" to pair the !NULL check with the functional check? E.g.

        if (kvm_hlt_in_guest(vcpu) &&
            best && (best->eax & (1 << KVM_FEATURE_PV_UNHALT)))
                best->eax &= ~(1 << KVM_FEATURE_PV_UNHALT);

arch/x86/kvm/lapic.c static bool kvm_can_post_timer_interrupt(struct · line 121

Quoted 11 lines

arch/x86/kvm/cpuid.c +0 -0

              best->eax &= ~(1 << KVM_FEATURE_PV_UNHALT);
 

arch/x86/kvm/lapic.c index renamed from arch/x86/kvm/lapic.c +0 -0

f206fc35deff..effb994e6642 100644

arch/x86/kvm/lapic.c +1 -1

@@ -119,7 +119,7 @@ static bool kvm_can_post_timer_interrupt(struct
 kvm_vcpu *vcpu)  bool kvm_can_use_hv_timer(struct kvm_vcpu *vcpu)  {
      return kvm_x86_ops.set_hv_timer
-            && !(kvm_mwait_in_guest(vcpu->kvm) ||
+            && !(kvm_mwait_in_guest(vcpu) ||

And another opportunistic tweak? I'm been itching for an excuse to "fix" this particular helper for quite some time :-)

        return kvm_x86_ops.set_hv_timer &&
               !(kvm_mwait_in_guest(vcpu) ||

kvm_can_post_timer_interrupt(vcpu));

That overruns the 80 char soft limit, but IMO it's worth it in this case as the resulting code is more readable.

Quoted 4 lines

arch/x86/kvm/lapic.c +0 -0

                  kvm_can_post_timer_interrupt(vcpu));
 }
 EXPORT_SYMBOL_GPL(kvm_can_use_hv_timer);

Does your use case require toggling intercepts? Or is the configuration static? If it's static, then the easiest thing would be to follow the per-VM behavior so that there are no suprises. If toggling is required, then I think the best thing would be to add a prep patch to add an override flag to the per-VM ioctl, and then share code between the per-VM and per-vCPU paths for modifying the flags (attached as patch 0003).

If toggling is not required, then I still think it makes sense to add a macro to handle propagating the capability args to the arch flags.

-----Original Message-----
From: Sean Christopherson <[email protected]>
Sent: Monday, January 10, 2022 1:01 PM
To: Kechen Lu <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Somdutta Roy <[email protected]>; [email protected]
Subject: Re: [RFC PATCH v2 3/3] KVM: x86: add vCPU ioctl for HLT exits disable capability

External email: Use caution opening links or attachments

Quoted 1 line · Sean Christopherson

Does your use case require toggling intercepts? Or is the configuration

static?

Quoted 5 lines

If it's static, then the easiest thing would be to follow the per-VM behavior so that there are no suprises. If toggling is required, then I think the best thing would be to add a prep patch to add an override flag to the per-VM ioctl, and then share code between the per-VM and per-vCPU paths for modifying the flags (attached as patch 0003).

...

Quoted 2 lines

If toggling is not required, then I still think it makes sense to add a macro to handle propagating the capability args to the arch flags.

Almost forgot. Can you please add a selftests to verify whatever per-VM and per-vCPU behavior we end implementing? Thanks!

-----Original Message-----
From: Sean Christopherson <[email protected]>
Sent: Monday, January 10, 2022 12:56 PM
To: Kechen Lu <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Somdutta Roy <[email protected]>; [email protected]
Subject: Re: [RFC PATCH v2 3/3] KVM: x86: add vCPU ioctl for HLT exits disable capability

External email: Use caution opening links or attachments

arch/x86/kvm/x86.c static int kvm_vcpu_ioctl_enable_cap(struct

Quoted 5 lines · Kechen Lu

arch/x86/kvm/x86.c index renamed from arch/x86/kvm/x86.c +0 -0

d5d0d99b584e..d7b4a3e360bb 100644

arch/x86/kvm/x86.c +0 -0

@@ -5072,6 +5072,18 @@ static int kvm_vcpu_ioctl_enable_cap(struct

kvm_vcpu *vcpu,

arch/x86/kvm/x86.c static int kvm_vcpu_ioctl_enable_cap(struct · line 5078

Quoted 7 lines

arch/x86/kvm/x86.c +4 -0

                      kvm_update_pv_runtime(vcpu);
 
              return 0;
+
+     case KVM_CAP_X86_DISABLE_EXITS:
+             if (cap->args[0] && (cap->args[0] &
+                             ~KVM_X86_DISABLE_VALID_EXITS))

Bad alignment, but there's no need for the !0 in the first place, i.e.

                if (cap->args[0] & ~KVM_X86_DISABLE_VALID_EXITS)

but that's also incomplete as this path only supports toggling HLT, yet allows all flavors of KVM_X86_DISABLE_VALID_EXITS. Unless there's a good reason to not allow maniuplating the other exits, the proper fix is to just support everything.

arch/x86/kvm/x86.c static int kvm_vcpu_ioctl_enable_cap(struct · line 5082

Quoted 4 lines

arch/x86/kvm/x86.c +4 -0

+                     return -EINVAL;
+
+             vcpu->arch.hlt_in_guest = (cap->args[0] &
+                     KVM_X86_DISABLE_EXITS_HLT) ? true : false;

Hmm, this behavior diverges from the per-VM ioctl, which doesn't allow re- enabling a disabled exit. We can't change the per-VM behavior without breaking backwards compatibility, e.g. if userspace does:

if (allow_mwait)
kvm_vm_disable_exit(KVM_X86_DISABLE_EXITS_MWAIT)
if (allow_hlt)
kvm_vm_disable_exit(KVM_X86_DISABLE_EXITS_HLT)

then changing KVM behavior would result in MWAIT behavior intercepted when previously it would have been allowed. We have a userspace VMM that operates like this...

Does your use case require toggling intercepts? Or is the configuration static?
If it's static, then the easiest thing would be to follow the per-VM behavior so that there are no suprises. If toggling is required, then I think the best thing would be to add a prep patch to add an override flag to the per-VM ioctl, and then share code between the per-VM and per-vCPU paths for modifying the flags (attached as patch 0003).

Somewhat related, there's another bug of sorts that I think we can safely fix. KVM doesn't reject disabling of MWAIT exits when MWAIT isn't allowed in the guest, and instead ignores the bad input. Not a big deal, but fixing that means KVM doesn't need to check kvm_can_mwait_in_guest() when processing the args to update flags. If that breaks someone's userspace, the alternative would be to tweak the attached patch 0003 to introduce the OVERRIDE, e.g.

arch/x86/kvm/x86.c index renamed from arch/x86/kvm/x86.c +0 -0

f611a49ceba4..3bac756bab79 100644

arch/x86/kvm/x86.c +0 -0

        if ((mask) & KVM_X86_DISABLE_EXITS_OVERRIDE) {                       \
                (a).mwait_in_guest = (mask) & KVM_X86_DISABLE_EXITS_MWAIT;

\
                (a).hlt_in_guest = (mask) & KVM_X86_DISABLE_EXITS_HLT;       \

If toggling is not required, then I still think it makes sense to add a macro to handle propagating the capability args to the arch flags.

Summary
===========
Introduce support of vCPU-scoped ioctl with KVM_CAP_X86_DISABLE_EXITS cap for disabling exits to enable finer-grained VM exits disabling on per vCPU scales instead of whole guest. This patch series enabled the vCPU-scoped exits control on HLT VM-exits.

Motivation
============
In use cases like Windows guest running heavy CPU-bound workloads, disabling HLT VM-exits could mitigate host sched ctx switch overhead. Simply HLT disabling on all vCPUs could bring performance benefits, but if no pCPUs reserved for host threads, could happened to the forced preemption as host does not know the time to do the schedule for other host threads want to run. With this patch, we could only disable part of vCPUs HLT exits for one guest, this still keeps performance benefits, and also shows resiliency to host stressing workload running at the same time.

Performance and Testing
=========================
In the host stressing workload experiment with Windows guest heavy CPU-bound workloads, it shows good resiliency and having the ~3% performance improvement. E.g. Passmark running in a Windows guest with this patch disabling HLT exits on only half of vCPUs still showing 2.4% higher main score v/s baseline.

Tested everything on AMD machines.

v1->v2 (Sean Christopherson) :
- Add explicit restriction for VM-scoped exits disabling to be called before vCPUs creation (patch 1)
- Use vCPU ioctl instead of 64bit vCPU bitmask (patch 3), and make exits disable flags check purely for vCPU instead of VM (patch 2)

Best Regards,
Kechen

Kechen Lu (3):
KVM: x86: only allow exits disable before vCPUs created
KVM: x86: move ()_in_guest checking to vCPU scope
KVM: x86: add vCPU ioctl for HLT exits disable capability

 Documentation/virt/kvm/api.rst     |  4 +++-
 arch/x86/include/asm/kvm-x86-ops.h |  1 +
 arch/x86/include/asm/kvm_host.h    |  7 +++++++
 arch/x86/kvm/cpuid.c               |  2 +-
 arch/x86/kvm/lapic.c               |  2 +-
 arch/x86/kvm/svm/svm.c             | 20 +++++++++++++++-----
 arch/x86/kvm/vmx/vmx.c             | 26 ++++++++++++++++++--------
 arch/x86/kvm/x86.c                 | 24 +++++++++++++++++++++++-
 arch/x86/kvm/x86.h                 | 16 ++++++++--------
 9 files changed, 77 insertions(+), 25 deletions(-)

Signature · 1 line

2.30.2

-----Original Message-----
From: Michael S. Tsirkin <[email protected]>
Sent: Monday, January 10, 2022 1:18 PM
To: Kechen Lu <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Somdutta Roy <[email protected]>; [email protected]
Subject: Re: [RFC PATCH v2 0/3] KVM: x86: add per-vCPU exits disable capability

External email: Use caution opening links or attachments

Quoted 3 lines · Kechen Lu

Summary
===========
Introduce support of vCPU-scoped ioctl with

KVM_CAP_X86_DISABLE_EXITS

Quoted 24 lines

cap for disabling exits to enable finer-grained VM exits disabling on per vCPU scales instead of whole guest. This patch series enabled the vCPU-scoped exits control on HLT VM-exits.

Motivation
============
In use cases like Windows guest running heavy CPU-bound workloads, disabling HLT VM-exits could mitigate host sched ctx switch overhead. Simply HLT disabling on all vCPUs could bring performance benefits, but if no pCPUs reserved for host threads, could happened to the forced preemption as host does not know the time to do the schedule for other host threads want to run. With this patch, we could only disable part of vCPUs HLT exits for one guest, this still keeps performance benefits, and also shows resiliency to host stressing workload running at the same time.

Performance and Testing
=========================
In the host stressing workload experiment with Windows guest heavy CPU-bound workloads, it shows good resiliency and having the ~3% performance improvement. E.g. Passmark running in a Windows guest with this patch disabling HLT exits on only half of vCPUs still showing 2.4% higher main score v/s baseline.

Quoted 8 lines

Tested everything on AMD machines.

v1->v2 (Sean Christopherson) :
- Add explicit restriction for VM-scoped exits disabling to be called before vCPUs creation (patch 1)
- Use vCPU ioctl instead of 64bit vCPU bitmask (patch 3), and make exits disable flags check purely for vCPU instead of VM (patch 2)

This is still quite blunt and assumes a ton of configuration on the host exactly matching the workload within guest. Which seems a waste since guests actually have the smarts to know what's happening within them.

If you are going to allow guest to halt a vCPU, how about working on exposing mwait to guest cleanly instead?
The idea is to expose this in ACPI - linux guests ignore ACPI and go by CPUID but windows guests follow ACPI. Linux can be patched ;)

What we would have is a mirror of host ACPI states, such that lower states invoke HLT and exit, higher power states invoke mwait and wait within guest.

The nice thing with this approach is that it's already supported by the host kernel, so it's just a question of coding up ACPI.

Quoted 24 lines

Best Regards,
Kechen

Kechen Lu (3):
KVM: x86: only allow exits disable before vCPUs created
KVM: x86: move ()_in_guest checking to vCPU scope
KVM: x86: add vCPU ioctl for HLT exits disable capability

 Documentation/virt/kvm/api.rst     |  4 +++-
 arch/x86/include/asm/kvm-x86-ops.h |  1 +
 arch/x86/include/asm/kvm_host.h    |  7 +++++++
 arch/x86/kvm/cpuid.c               |  2 +-
 arch/x86/kvm/lapic.c               |  2 +-
 arch/x86/kvm/svm/svm.c             | 20 +++++++++++++++-----
 arch/x86/kvm/vmx/vmx.c             | 26 ++++++++++++++++++--------
 arch/x86/kvm/x86.c                 | 24 +++++++++++++++++++++++-
 arch/x86/kvm/x86.h                 | 16 ++++++++--------
 9 files changed, 77 insertions(+), 25 deletions(-)

--
2.30.2