| Message ID | 20181003174207.26418-1-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: VMX: check flexpriority module param when setting virt apic mode | expand |
On Wed, Oct 3, 2018 at 10:42 AM, Sean Christopherson <sean.j.christopherson@intel.com> wrote: > Query the flexpriority module param instead of simply checking if > flexpriority is supported by the cpu when checking whether or not VMCS > fields need to be updated in response to the guest changing its > APIC_BASE MSR control bits. This avoids multiple unnecessary VMREADs > and a VMWRITE if flexpriority is disabled via its module param and the > cpu doesn't support X2APIC virtualization. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > > This is basically fixup for commit 76e97cc35522 ("KVM: VMX: check for > existence of secondary exec controls before accessing") in > kvm.git/queue. The aforementioned commit came from a series with a > prior patch that removed the module param, but the prior patch was > ultimately rejected resulting in this partially wrong code. > > arch/x86/kvm/vmx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index c7ae8ea87bc4..612fd17be635 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -10223,7 +10223,7 @@ static void vmx_set_virtual_apic_mode(struct kvm_vcpu *vcpu) > if (!lapic_in_kernel(vcpu)) > return; > > - if (!cpu_has_vmx_flexpriority() && > + if (!flexpriority_enabled && > !cpu_has_vmx_virtualize_x2apic_mode()) > return; I think it would be better if cpu_has_vmx_flexpriority() returned false when flexpriority_enabled is false.
On Wed, Oct 03, 2018 at 11:32:33AM -0700, Jim Mattson wrote: > On Wed, Oct 3, 2018 at 10:42 AM, Sean Christopherson > <sean.j.christopherson@intel.com> wrote: > > Query the flexpriority module param instead of simply checking if > > flexpriority is supported by the cpu when checking whether or not VMCS > > fields need to be updated in response to the guest changing its > > APIC_BASE MSR control bits. This avoids multiple unnecessary VMREADs > > and a VMWRITE if flexpriority is disabled via its module param and the > > cpu doesn't support X2APIC virtualization. > > > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > --- > > > > This is basically fixup for commit 76e97cc35522 ("KVM: VMX: check for > > existence of secondary exec controls before accessing") in > > kvm.git/queue. The aforementioned commit came from a series with a > > prior patch that removed the module param, but the prior patch was > > ultimately rejected resulting in this partially wrong code. > > > > arch/x86/kvm/vmx.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > index c7ae8ea87bc4..612fd17be635 100644 > > --- a/arch/x86/kvm/vmx.c > > +++ b/arch/x86/kvm/vmx.c > > @@ -10223,7 +10223,7 @@ static void vmx_set_virtual_apic_mode(struct kvm_vcpu *vcpu) > > if (!lapic_in_kernel(vcpu)) > > return; > > > > - if (!cpu_has_vmx_flexpriority() && > > + if (!flexpriority_enabled && > > !cpu_has_vmx_virtualize_x2apic_mode()) > > return; > > I think it would be better if cpu_has_vmx_flexpriority() returned > false when flexpriority_enabled is false. Ha, I had that all coded up but decided against it because the convention used by other module params is to not adjust cpu_has_*().
On 03/10/2018 20:39, Sean Christopherson wrote: >>> - if (!cpu_has_vmx_flexpriority() && >>> + if (!flexpriority_enabled && >>> !cpu_has_vmx_virtualize_x2apic_mode()) >>> return; >> I think it would be better if cpu_has_vmx_flexpriority() returned >> false when flexpriority_enabled is false. > Ha, I had that all coded up but decided against it because the > convention used by other module params is to not adjust cpu_has_*(). Yeah, for now let's stick to the minimum. It's a common idiom to adjust the variable instead of the cpu_has_*() because then the processor capabilities show up in /sys/kernel/module/kvm_intel/parameters. It requires more care but it's also more user friendly. Paolo
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index c7ae8ea87bc4..612fd17be635 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -10223,7 +10223,7 @@ static void vmx_set_virtual_apic_mode(struct kvm_vcpu *vcpu) if (!lapic_in_kernel(vcpu)) return; - if (!cpu_has_vmx_flexpriority() && + if (!flexpriority_enabled && !cpu_has_vmx_virtualize_x2apic_mode()) return;
Query the flexpriority module param instead of simply checking if flexpriority is supported by the cpu when checking whether or not VMCS fields need to be updated in response to the guest changing its APIC_BASE MSR control bits. This avoids multiple unnecessary VMREADs and a VMWRITE if flexpriority is disabled via its module param and the cpu doesn't support X2APIC virtualization. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- This is basically fixup for commit 76e97cc35522 ("KVM: VMX: check for existence of secondary exec controls before accessing") in kvm.git/queue. The aforementioned commit came from a series with a prior patch that removed the module param, but the prior patch was ultimately rejected resulting in this partially wrong code. arch/x86/kvm/vmx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)