lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 30 Jan 2023 20:25:11 +0000
From:   Kechen Lu <kechenl@...dia.com>
To:     Chao Gao <chao.gao@...el.com>
CC:     "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "seanjc@...gle.com" <seanjc@...gle.com>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "zhi.wang.linux@...il.com" <zhi.wang.linux@...il.com>,
        "shaoqin.huang@...el.com" <shaoqin.huang@...el.com>,
        "vkuznets@...hat.com" <vkuznets@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH v6 4/6] KVM: x86: Let userspace re-enable previously
 disabled exits

Hi Chao,

> -----Original Message-----
> From: Chao Gao <chao.gao@...el.com>
> Sent: Sunday, January 29, 2023 10:19 PM
> To: Kechen Lu <kechenl@...dia.com>
> Cc: kvm@...r.kernel.org; seanjc@...gle.com; pbonzini@...hat.com;
> zhi.wang.linux@...il.com; shaoqin.huang@...el.com;
> vkuznets@...hat.com; linux-kernel@...r.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@...gle.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@...gle.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
> >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ