[RFC,v6,4/6] KVM: x86: Let userspace re-enable previously disabled exits

Message ID 20230121020738.2973-5-kechenl@nvidia.com
State New
Headers
Series KVM: x86: add per-vCPU exits disable capability |

Commit Message

Kechen Lu Jan. 21, 2023, 2:07 a.m. UTC
  From: Sean Christopherson <seanjc@google.com>

Add an OVERRIDE flag to KVM_CAP_X86_DISABLE_EXITS allow userspace to
re-enable exits and/or override previous settings.  There's no real use
case for the per-VM ioctl, but a future per-vCPU variant wants to let
userspace toggle interception while the vCPU is running; add the OVERRIDE
functionality now to provide consistent between the per-VM and
per-vCPU variants.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 Documentation/virt/kvm/api.rst |  5 +++++
 arch/x86/kvm/x86.c             | 32 ++++++++++++++++++++++++--------
 include/uapi/linux/kvm.h       |  4 +++-
 3 files changed, 32 insertions(+), 9 deletions(-)
  

Comments

Chao Gao Jan. 30, 2023, 6:19 a.m. UTC | #1
On Sat, Jan 21, 2023 at 02:07:36AM +0000, Kechen Lu wrote:
>From: Sean Christopherson <seanjc@google.com>
>
>Add an OVERRIDE flag to KVM_CAP_X86_DISABLE_EXITS allow userspace to
>re-enable exits and/or override previous settings.  There's no real use
>case for the per-VM ioctl, but a future per-vCPU variant wants to let
>userspace toggle interception while the vCPU is running; add the OVERRIDE
>functionality now to provide consistent between the per-VM and
>per-vCPU variants.
>
>Signed-off-by: Sean Christopherson <seanjc@google.com>

Kechen, add your signed-off-by as you are the submitter.

>---
> Documentation/virt/kvm/api.rst |  5 +++++
> arch/x86/kvm/x86.c             | 32 ++++++++++++++++++++++++--------
> include/uapi/linux/kvm.h       |  4 +++-
> 3 files changed, 32 insertions(+), 9 deletions(-)
>
>diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
>index fb0fcc566d5a..3850202942d0 100644
>--- a/Documentation/virt/kvm/api.rst
>+++ b/Documentation/virt/kvm/api.rst
>@@ -7095,6 +7095,7 @@ Valid bits in args[0] are::
>   #define KVM_X86_DISABLE_EXITS_HLT              (1 << 1)
>   #define KVM_X86_DISABLE_EXITS_PAUSE            (1 << 2)
>   #define KVM_X86_DISABLE_EXITS_CSTATE           (1 << 3)
>+  #define KVM_X86_DISABLE_EXITS_OVERRIDE         (1ull << 63)
> 
> Enabling this capability on a VM provides userspace with a way to no
> longer intercept some instructions for improved latency in some
>@@ -7103,6 +7104,10 @@ physical CPUs.  More bits can be added in the future; userspace can
> just pass the KVM_CHECK_EXTENSION result to KVM_ENABLE_CAP to disable
> all such vmexits.
> 
>+By default, this capability only disables exits.  To re-enable an exit, or to
>+override previous settings, userspace can set KVM_X86_DISABLE_EXITS_OVERRIDE,
>+in which case KVM will enable/disable according to the mask (a '1' == disable).
>+
> Do not enable KVM_FEATURE_PV_UNHALT if you disable HLT exits.
> 
> 7.14 KVM_CAP_S390_HPAGE_1M
>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index 60caa3fd40e5..3ea5f12536a0 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -5484,6 +5484,28 @@ static int kvm_vcpu_ioctl_device_attr(struct kvm_vcpu *vcpu,
> 	return r;
> }
> 
>+
>+#define kvm_ioctl_disable_exits(a, mask)				     \
>+({									     \

>+	if (!kvm_can_mwait_in_guest())                                       \
>+		(mask) &= KVM_X86_DISABLE_EXITS_MWAIT;                       \

This can be dropped or should be a WARN_ON_ONCE() because if kvm cannot
support mwait in guest (i.e., !kvm_can_mwait_in_guest()), attempts to
disable mwait exits are already treated as invalid requests in patch 3.

>+	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;	     \
>+		(a).pause_in_guest = (mask) & KVM_X86_DISABLE_EXITS_PAUSE;   \
>+		(a).cstate_in_guest = (mask) & KVM_X86_DISABLE_EXITS_CSTATE; \
>+	} else {							     \
>+		if ((mask) & KVM_X86_DISABLE_EXITS_MWAIT)		     \
>+			(a).mwait_in_guest = true;			     \
>+		if ((mask) & KVM_X86_DISABLE_EXITS_HLT)			     \
>+			(a).hlt_in_guest = true;			     \
>+		if ((mask) & KVM_X86_DISABLE_EXITS_PAUSE)		     \
>+			(a).pause_in_guest = true;			     \
>+		if ((mask) & KVM_X86_DISABLE_EXITS_CSTATE)		     \
>+			(a).cstate_in_guest = true;			     \
>+	}								     \
>+})
>+
> static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
> 				     struct kvm_enable_cap *cap)
> {
>@@ -6238,14 +6260,8 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
> 		if (kvm->created_vcpus)
> 			goto disable_exits_unlock;
> 
>-		if (cap->args[0] & KVM_X86_DISABLE_EXITS_MWAIT)
>-			kvm->arch.mwait_in_guest = true;
>-		if (cap->args[0] & KVM_X86_DISABLE_EXITS_HLT)
>-			kvm->arch.hlt_in_guest = true;
>-		if (cap->args[0] & KVM_X86_DISABLE_EXITS_PAUSE)
>-			kvm->arch.pause_in_guest = true;
>-		if (cap->args[0] & KVM_X86_DISABLE_EXITS_CSTATE)
>-			kvm->arch.cstate_in_guest = true;
>+		kvm_ioctl_disable_exits(kvm->arch, cap->args[0]);
>+
> 		r = 0;
> disable_exits_unlock:
> 		mutex_unlock(&kvm->lock);
>diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>index 55155e262646..876dcccbfff2 100644
>--- a/include/uapi/linux/kvm.h
>+++ b/include/uapi/linux/kvm.h
>@@ -823,10 +823,12 @@ struct kvm_ioeventfd {
> #define KVM_X86_DISABLE_EXITS_HLT            (1 << 1)
> #define KVM_X86_DISABLE_EXITS_PAUSE          (1 << 2)
> #define KVM_X86_DISABLE_EXITS_CSTATE         (1 << 3)
>+#define KVM_X86_DISABLE_EXITS_OVERRIDE	     (1ull << 63)
> #define KVM_X86_DISABLE_VALID_EXITS          (KVM_X86_DISABLE_EXITS_MWAIT | \
>                                               KVM_X86_DISABLE_EXITS_HLT | \
>                                               KVM_X86_DISABLE_EXITS_PAUSE | \
>-                                              KVM_X86_DISABLE_EXITS_CSTATE)
>+					      KVM_X86_DISABLE_EXITS_CSTATE | \
>+					      KVM_X86_DISABLE_EXITS_OVERRIDE)
> 
> /* for KVM_ENABLE_CAP */
> struct kvm_enable_cap {
>-- 
>2.34.1
>
  
Kechen Lu Jan. 30, 2023, 8:25 p.m. UTC | #2
Hi Chao,

> -----Original Message-----
> From: Chao Gao <chao.gao@intel.com>
> Sent: Sunday, January 29, 2023 10:19 PM
> To: Kechen Lu <kechenl@nvidia.com>
> Cc: kvm@vger.kernel.org; seanjc@google.com; pbonzini@redhat.com;
> zhi.wang.linux@gmail.com; shaoqin.huang@intel.com;
> vkuznets@redhat.com; linux-kernel@vger.kernel.org
> Subject: Re: [RFC PATCH v6 4/6] KVM: x86: Let userspace re-enable
> previously disabled exits
> 
> External email: Use caution opening links or attachments
> 
> 
> On Sat, Jan 21, 2023 at 02:07:36AM +0000, Kechen Lu wrote:
> >From: Sean Christopherson <seanjc@google.com>
> >
> >Add an OVERRIDE flag to KVM_CAP_X86_DISABLE_EXITS allow userspace to
> >re-enable exits and/or override previous settings.  There's no real use
> >case for the per-VM ioctl, but a future per-vCPU variant wants to let
> >userspace toggle interception while the vCPU is running; add the
> >OVERRIDE functionality now to provide consistent between the per-VM
> and
> >per-vCPU variants.
> >
> >Signed-off-by: Sean Christopherson <seanjc@google.com>
> 
> Kechen, add your signed-off-by as you are the submitter.
>
 
Ack! Forgot this.

> >---
> > Documentation/virt/kvm/api.rst |  5 +++++
> > arch/x86/kvm/x86.c             | 32 ++++++++++++++++++++++++--------
> > include/uapi/linux/kvm.h       |  4 +++-
> > 3 files changed, 32 insertions(+), 9 deletions(-)
> >
> >diff --git a/Documentation/virt/kvm/api.rst
> >b/Documentation/virt/kvm/api.rst index fb0fcc566d5a..3850202942d0
> >100644
> >--- a/Documentation/virt/kvm/api.rst
> >+++ b/Documentation/virt/kvm/api.rst
> >@@ -7095,6 +7095,7 @@ Valid bits in args[0] are::
> >   #define KVM_X86_DISABLE_EXITS_HLT              (1 << 1)
> >   #define KVM_X86_DISABLE_EXITS_PAUSE            (1 << 2)
> >   #define KVM_X86_DISABLE_EXITS_CSTATE           (1 << 3)
> >+  #define KVM_X86_DISABLE_EXITS_OVERRIDE         (1ull << 63)
> >
> > Enabling this capability on a VM provides userspace with a way to no
> >longer intercept some instructions for improved latency in some @@
> >-7103,6 +7104,10 @@ physical CPUs.  More bits can be added in the
> >future; userspace can  just pass the KVM_CHECK_EXTENSION result to
> >KVM_ENABLE_CAP to disable  all such vmexits.
> >
> >+By default, this capability only disables exits.  To re-enable an
> >+exit, or to override previous settings, userspace can set
> >+KVM_X86_DISABLE_EXITS_OVERRIDE, in which case KVM will
> enable/disable according to the mask (a '1' == disable).
> >+
> > Do not enable KVM_FEATURE_PV_UNHALT if you disable HLT exits.
> >
> > 7.14 KVM_CAP_S390_HPAGE_1M
> >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index
> >60caa3fd40e5..3ea5f12536a0 100644
> >--- a/arch/x86/kvm/x86.c
> >+++ b/arch/x86/kvm/x86.c
> >@@ -5484,6 +5484,28 @@ static int kvm_vcpu_ioctl_device_attr(struct
> kvm_vcpu *vcpu,
> >       return r;
> > }
> >
> >+
> >+#define kvm_ioctl_disable_exits(a, mask)                                   \
> >+({                                                                         \
> 
> >+      if (!kvm_can_mwait_in_guest())                                       \
> >+              (mask) &= KVM_X86_DISABLE_EXITS_MWAIT;                       \
> 
> This can be dropped or should be a WARN_ON_ONCE() because if kvm
> cannot support mwait in guest (i.e., !kvm_can_mwait_in_guest()), attempts
> to disable mwait exits are already treated as invalid requests in patch 3.

As last time Sean suggests adding this workaround in case some hypervisors 
apply without checking supported flags. Would prefer WARN_ON_ONCE(). 
Thanks!

BR,
Kechen

> 
> >+      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;       \
> >+              (a).pause_in_guest = (mask) & KVM_X86_DISABLE_EXITS_PAUSE;
> \
> >+              (a).cstate_in_guest = (mask) & KVM_X86_DISABLE_EXITS_CSTATE;
> \
> >+      } else {                                                             \
> >+              if ((mask) & KVM_X86_DISABLE_EXITS_MWAIT)                    \
> >+                      (a).mwait_in_guest = true;                           \
> >+              if ((mask) & KVM_X86_DISABLE_EXITS_HLT)                      \
> >+                      (a).hlt_in_guest = true;                             \
> >+              if ((mask) & KVM_X86_DISABLE_EXITS_PAUSE)                    \
> >+                      (a).pause_in_guest = true;                           \
> >+              if ((mask) & KVM_X86_DISABLE_EXITS_CSTATE)                   \
> >+                      (a).cstate_in_guest = true;                          \
> >+      }                                                                    \
> >+})
> >+
> > static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
> >                                    struct kvm_enable_cap *cap)  { @@
> >-6238,14 +6260,8 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
> >               if (kvm->created_vcpus)
> >                       goto disable_exits_unlock;
> >
> >-              if (cap->args[0] & KVM_X86_DISABLE_EXITS_MWAIT)
> >-                      kvm->arch.mwait_in_guest = true;
> >-              if (cap->args[0] & KVM_X86_DISABLE_EXITS_HLT)
> >-                      kvm->arch.hlt_in_guest = true;
> >-              if (cap->args[0] & KVM_X86_DISABLE_EXITS_PAUSE)
> >-                      kvm->arch.pause_in_guest = true;
> >-              if (cap->args[0] & KVM_X86_DISABLE_EXITS_CSTATE)
> >-                      kvm->arch.cstate_in_guest = true;
> >+              kvm_ioctl_disable_exits(kvm->arch, cap->args[0]);
> >+
> >               r = 0;
> > disable_exits_unlock:
> >               mutex_unlock(&kvm->lock); diff --git
> >a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index
> >55155e262646..876dcccbfff2 100644
> >--- a/include/uapi/linux/kvm.h
> >+++ b/include/uapi/linux/kvm.h
> >@@ -823,10 +823,12 @@ struct kvm_ioeventfd {
> > #define KVM_X86_DISABLE_EXITS_HLT            (1 << 1)
> > #define KVM_X86_DISABLE_EXITS_PAUSE          (1 << 2)
> > #define KVM_X86_DISABLE_EXITS_CSTATE         (1 << 3)
> >+#define KVM_X86_DISABLE_EXITS_OVERRIDE             (1ull << 63)
> > #define KVM_X86_DISABLE_VALID_EXITS
> (KVM_X86_DISABLE_EXITS_MWAIT | \
> >                                               KVM_X86_DISABLE_EXITS_HLT | \
> >                                               KVM_X86_DISABLE_EXITS_PAUSE | \
> >-                                              KVM_X86_DISABLE_EXITS_CSTATE)
> >+                                            KVM_X86_DISABLE_EXITS_CSTATE | \
> >+
> >+ KVM_X86_DISABLE_EXITS_OVERRIDE)
> >
> > /* for KVM_ENABLE_CAP */
> > struct kvm_enable_cap {
> >--
> >2.34.1
> >
  

Patch

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index fb0fcc566d5a..3850202942d0 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -7095,6 +7095,7 @@  Valid bits in args[0] are::
   #define KVM_X86_DISABLE_EXITS_HLT              (1 << 1)
   #define KVM_X86_DISABLE_EXITS_PAUSE            (1 << 2)
   #define KVM_X86_DISABLE_EXITS_CSTATE           (1 << 3)
+  #define KVM_X86_DISABLE_EXITS_OVERRIDE         (1ull << 63)
 
 Enabling this capability on a VM provides userspace with a way to no
 longer intercept some instructions for improved latency in some
@@ -7103,6 +7104,10 @@  physical CPUs.  More bits can be added in the future; userspace can
 just pass the KVM_CHECK_EXTENSION result to KVM_ENABLE_CAP to disable
 all such vmexits.
 
+By default, this capability only disables exits.  To re-enable an exit, or to
+override previous settings, userspace can set KVM_X86_DISABLE_EXITS_OVERRIDE,
+in which case KVM will enable/disable according to the mask (a '1' == disable).
+
 Do not enable KVM_FEATURE_PV_UNHALT if you disable HLT exits.
 
 7.14 KVM_CAP_S390_HPAGE_1M
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 60caa3fd40e5..3ea5f12536a0 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5484,6 +5484,28 @@  static int kvm_vcpu_ioctl_device_attr(struct kvm_vcpu *vcpu,
 	return r;
 }
 
+
+#define kvm_ioctl_disable_exits(a, mask)				     \
+({									     \
+	if (!kvm_can_mwait_in_guest())                                       \
+		(mask) &= KVM_X86_DISABLE_EXITS_MWAIT;                       \
+	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;	     \
+		(a).pause_in_guest = (mask) & KVM_X86_DISABLE_EXITS_PAUSE;   \
+		(a).cstate_in_guest = (mask) & KVM_X86_DISABLE_EXITS_CSTATE; \
+	} else {							     \
+		if ((mask) & KVM_X86_DISABLE_EXITS_MWAIT)		     \
+			(a).mwait_in_guest = true;			     \
+		if ((mask) & KVM_X86_DISABLE_EXITS_HLT)			     \
+			(a).hlt_in_guest = true;			     \
+		if ((mask) & KVM_X86_DISABLE_EXITS_PAUSE)		     \
+			(a).pause_in_guest = true;			     \
+		if ((mask) & KVM_X86_DISABLE_EXITS_CSTATE)		     \
+			(a).cstate_in_guest = true;			     \
+	}								     \
+})
+
 static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
 				     struct kvm_enable_cap *cap)
 {
@@ -6238,14 +6260,8 @@  int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 		if (kvm->created_vcpus)
 			goto disable_exits_unlock;
 
-		if (cap->args[0] & KVM_X86_DISABLE_EXITS_MWAIT)
-			kvm->arch.mwait_in_guest = true;
-		if (cap->args[0] & KVM_X86_DISABLE_EXITS_HLT)
-			kvm->arch.hlt_in_guest = true;
-		if (cap->args[0] & KVM_X86_DISABLE_EXITS_PAUSE)
-			kvm->arch.pause_in_guest = true;
-		if (cap->args[0] & KVM_X86_DISABLE_EXITS_CSTATE)
-			kvm->arch.cstate_in_guest = true;
+		kvm_ioctl_disable_exits(kvm->arch, cap->args[0]);
+
 		r = 0;
 disable_exits_unlock:
 		mutex_unlock(&kvm->lock);
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 55155e262646..876dcccbfff2 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -823,10 +823,12 @@  struct kvm_ioeventfd {
 #define KVM_X86_DISABLE_EXITS_HLT            (1 << 1)
 #define KVM_X86_DISABLE_EXITS_PAUSE          (1 << 2)
 #define KVM_X86_DISABLE_EXITS_CSTATE         (1 << 3)
+#define KVM_X86_DISABLE_EXITS_OVERRIDE	     (1ull << 63)
 #define KVM_X86_DISABLE_VALID_EXITS          (KVM_X86_DISABLE_EXITS_MWAIT | \
                                               KVM_X86_DISABLE_EXITS_HLT | \
                                               KVM_X86_DISABLE_EXITS_PAUSE | \
-                                              KVM_X86_DISABLE_EXITS_CSTATE)
+					      KVM_X86_DISABLE_EXITS_CSTATE | \
+					      KVM_X86_DISABLE_EXITS_OVERRIDE)
 
 /* for KVM_ENABLE_CAP */
 struct kvm_enable_cap {