Hi Sean,
> -----Original Message-----
> From: Sean Christopherson <seanjc@xxxxxxxxxx>
> Sent: Monday, January 10, 2022 10:50 AM
> To: Kechen Lu <kechenl@xxxxxxxxxx>
> Cc: kvm@xxxxxxxxxxxxxxx; pbonzini@xxxxxxxxxx; wanpengli@xxxxxxxxxxx;
> vkuznets@xxxxxxxxxx; mst@xxxxxxxxxx; Somdutta Roy
> <somduttar@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx, 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@xxxxxxxxxx>
>
> > Signed-off-by: Kechen Lu <kechenl@xxxxxxxxxx>
> > ---
> > 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
> >
