| Message ID | 20211221090449.15337-2-kechenl@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: add per-vCPU exits disable capability | expand |
On Tue, Dec 21, 2021, Kechen Lu wrote: > 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 <seanjc@google.com> 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: stable@vger.kernel.org, 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 <seanjc@google.com> > Signed-off-by: Kechen Lu <kechenl@nvidia.com> > --- > Documentation/virt/kvm/api.rst | 1 + > arch/x86/kvm/x86.c | 6 ++++++ > 2 files changed, 7 insertions(+) > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst > index aeeb071c7688..d1c50b95bbc1 100644 > --- a/Documentation/virt/kvm/api.rst > +++ b/Documentation/virt/kvm/api.rst > @@ -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:: > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 0cf1082455df..37529c0c279d 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -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 >
Hi Sean, > -----Original Message----- > From: Sean Christopherson <seanjc@google.com> > Sent: Monday, January 10, 2022 10:50 AM > To: Kechen Lu <kechenl@nvidia.com> > Cc: kvm@vger.kernel.org; pbonzini@redhat.com; wanpengli@tencent.com; > vkuznets@redhat.com; mst@redhat.com; Somdutta Roy > <somduttar@nvidia.com>; linux-kernel@vger.kernel.org > Subject: Re: [RFC PATCH v2 1/3] KVM: x86: only allow exits disable before > vCPUs created > > External email: Use caution opening links or attachments > > > On Tue, Dec 21, 2021, Kechen Lu wrote: > > 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 <seanjc@google.com> > > 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: > My apologies for putting incorrect SOB format :P Will fix it! > And we should at least have > > Fixes: 4d5422cea3b6 ("KVM: X86: Provide a capability to disable MWAIT > intercepts") > Ack! Will mention it in the description. > andy maybe Cc: stable@vger.kernel.org, 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. > I see, will cc stable mailing list. IMO with this patch, incorrect behavior from userspace only cause the set flag "ineffective", not sure if this breaks some userspace seriously. Best Regards, Kechen > If this doesn't carry my SOB... > > Reviewed-by: Sean Christopherson <seanjc@google.com> > > > Signed-off-by: Kechen Lu <kechenl@nvidia.com> > > --- > > Documentation/virt/kvm/api.rst | 1 + > > arch/x86/kvm/x86.c | 6 ++++++ > > 2 files changed, 7 insertions(+) > > > > diff --git a/Documentation/virt/kvm/api.rst > > b/Documentation/virt/kvm/api.rst index aeeb071c7688..d1c50b95bbc1 > > 100644 > > --- a/Documentation/virt/kvm/api.rst > > +++ b/Documentation/virt/kvm/api.rst > > @@ -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:: > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index > > 0cf1082455df..37529c0c279d 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -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 > >
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index aeeb071c7688..d1c50b95bbc1 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -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:: diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 0cf1082455df..37529c0c279d 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -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];