2023-01-21 02:21:48

by Kechen Lu

[permalink] [raw]
Subject: [RFC PATCH v6 1/6] KVM: x86: only allow exits disable before vCPUs created

From: Sean Christopherson <[email protected]>

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.

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

Signed-off-by: Sean Christopherson <[email protected]>
Signed-off-by: Kechen Lu <[email protected]>
Cc: [email protected]
---
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 9807b05a1b57..fb0fcc566d5a 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -7087,6 +7087,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 da4bbd043a7b..c8ae9c4f9f08 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6227,6 +6227,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;
@@ -6237,6 +6241,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.34.1


2023-01-21 07:37:13

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [RFC PATCH v6 1/6] KVM: x86: only allow exits disable before vCPUs created

On Sat, Jan 21, 2023 at 02:07:33AM +0000, Kechen Lu wrote:
> From: Sean Christopherson <[email protected]>
>
> 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.
>
> Fixes: 4d5422cea3b6 ("KVM: X86: Provide a capability to disable MWAIT intercepts")
>
> Signed-off-by: Sean Christopherson <[email protected]>

Nit, no blank line between fixes and signed-off-by please.

And an RFC on v6? An RFC usually means "I don't think this is correct
so do not take it". How can you do that for 6 versions? And know that
no one will take an RFC series for that reason (or at least I will
not...)

thanks,

greg k-h

2023-01-22 02:46:40

by Kechen Lu

[permalink] [raw]
Subject: RE: [RFC PATCH v6 1/6] KVM: x86: only allow exits disable before vCPUs created

Hi Greg,

> -----Original Message-----
> From: Greg KH <[email protected]>
> Sent: Friday, January 20, 2023 11:28 PM
> To: Kechen Lu <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]
> Subject: Re: [RFC PATCH v6 1/6] KVM: x86: only allow exits disable before
> vCPUs created
>
> External email: Use caution opening links or attachments
>
>
> On Sat, Jan 21, 2023 at 02:07:33AM +0000, Kechen Lu wrote:
> > From: Sean Christopherson <[email protected]>
> >
> > 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.
> >
> > Fixes: 4d5422cea3b6 ("KVM: X86: Provide a capability to disable MWAIT
> > intercepts")
> >
> > Signed-off-by: Sean Christopherson <[email protected]>
>
> Nit, no blank line between fixes and signed-off-by please.

Ack.

>
> And an RFC on v6? An RFC usually means "I don't think this is correct so do
> not take it". How can you do that for 6 versions? And know that no one will
> take an RFC series for that reason (or at least I will
> not...)

Thanks for correcting this, this is my bad. The v2 to v4 revisions, there are big changes
on the following patches after this prerequisite patch, so I still "RFC" for the design.
But I should drop the "RFC" starting from v5, there are already consensus on the v5 design
options

Best Regards,
Kechen

>
> thanks,
>
> greg k-h