| Message ID | 1432132539-6194-1-git-send-email-liang.z.li@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Correct Gleb's email address. Liang > -----Original Message----- > From: Li, Liang Z > Sent: Wednesday, May 20, 2015 10:36 PM > To: kvm@vger.kernel.org; linux-kernel@vger.kernel.org > Cc: gleb@kernel.or; pbonzini@redhat.com; tglx@linutronix.de; > mingo@redhat.com; hpa@zytor.com; x86@kernel.org; Zhang, Yang Z; Li, > Liang Z > Subject: [PATCH] kvm/fpu: Enable eager restore kvm FPU for MPX > > The MPX feature requires eager KVM FPU restore support. We have verified > that MPX cannot work correctly with the current lazy KVM FPU restore > mechanism. Eager KVM FPU restore should be enabled if the MPX feature is > exposed to VM. > > Signed-off-by: Liang Li <liang.z.li@intel.com> > --- > arch/x86/kvm/vmx.c | 2 ++ > arch/x86/kvm/x86.c | 3 ++- > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index > f7b6168..e2cccbe 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -8445,6 +8445,8 @@ static struct kvm_vcpu *vmx_create_vcpu(struct > kvm *kvm, unsigned int id) > goto free_vmcs; > } > > + if (vmx_mpx_supported()) > + vmx_fpu_activate(&vmx->vcpu); > return &vmx->vcpu; > > free_vmcs: > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 5f38188..5993f5f > 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -7060,7 +7060,8 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu) > fpu_save_init(&vcpu->arch.guest_fpu); > __kernel_fpu_end(); > ++vcpu->stat.fpu_reload; > - kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); > + if (!kvm_x86_ops->mpx_supported()) > + kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); > trace_kvm_fpu(0); > } > > -- > 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Li, Liang Z wrote on 2015-05-20: > The MPX feature requires eager KVM FPU restore support. We have > verified that MPX cannot work correctly with the current lazy KVM FPU > restore mechanism. Eager KVM FPU restore should be enabled if the MPX > feature is exposed to VM. > > Signed-off-by: Liang Li <liang.z.li@intel.com> > --- > arch/x86/kvm/vmx.c | 2 ++ > arch/x86/kvm/x86.c | 3 ++- > 2 files changed, 4 insertions(+), 1 deletion(-) > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index > f7b6168..e2cccbe 100644 --- a/arch/x86/kvm/vmx.c +++ > b/arch/x86/kvm/vmx.c @@ -8445,6 +8445,8 @@ static struct kvm_vcpu > *vmx_create_vcpu(struct kvm *kvm, unsigned int id) > goto free_vmcs; > } > + if (vmx_mpx_supported()) Is it better to use guest_cpuid_has_mpx() instead of vmx_mpx_supported()? > + vmx_fpu_activate(&vmx->vcpu); > return &vmx->vcpu; > > free_vmcs: > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index > 5f38188..5993f5f 100644 --- a/arch/x86/kvm/x86.c +++ > b/arch/x86/kvm/x86.c @@ -7060,7 +7060,8 @@ void kvm_put_guest_fpu(struct > kvm_vcpu *vcpu) > fpu_save_init(&vcpu->arch.guest_fpu); > __kernel_fpu_end(); > ++vcpu->stat.fpu_reload; > - kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); > + if (!kvm_x86_ops->mpx_supported()) > + kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); > trace_kvm_fpu(0); > } Best regards, Yang -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20/05/2015 07:20, Zhang, Yang Z wrote: > Li, Liang Z wrote on 2015-05-20: >> The MPX feature requires eager KVM FPU restore support. We have >> verified that MPX cannot work correctly with the current lazy KVM FPU >> restore mechanism. Eager KVM FPU restore should be enabled if the MPX >> feature is exposed to VM. >> >> Signed-off-by: Liang Li <liang.z.li@intel.com> >> --- >> arch/x86/kvm/vmx.c | 2 ++ >> arch/x86/kvm/x86.c | 3 ++- >> 2 files changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index f7b6168..e2cccbe 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -8445,6 +8445,8 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id) >> goto free_vmcs; >> } >> >> + if (vmx_mpx_supported()) >> + vmx_fpu_activate(&vmx->vcpu); >> return &vmx->vcpu; >> >> free_vmcs: > > Is it better to use guest_cpuid_has_mpx() instead of vmx_mpx_supported()? CPUID hasn't been set yet, so I think it is okay to key it on vmx_mpx_supported(). It will be deactivated soon afterwards. Or even do it unconditionally; just make sure to add a comment about it. >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 5f38188..5993f5f >> 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -7060,7 +7060,8 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu) >> fpu_save_init(&vcpu->arch.guest_fpu); >> __kernel_fpu_end(); >> ++vcpu->stat.fpu_reload; >> - kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); >> + if (!kvm_x86_ops->mpx_supported()) >> + kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); >> trace_kvm_fpu(0); >> } This is a hotter path. Here it's definitely better to avoid the call to kvm_x86_ops->mpx_supported(). Especially because, with MPX enabled, you would call this on every userspace exit. Yang's suggestion of using CPUID is actually more valuable here. You could add a new field eager_fpu in kvm->arch and update it in kvm_update_cpuid. Thanks, Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Paolo Bonzini wrote on 2015-05-20: > > > On 20/05/2015 07:20, Zhang, Yang Z wrote: >> Li, Liang Z wrote on 2015-05-20: >>> The MPX feature requires eager KVM FPU restore support. We have >>> verified that MPX cannot work correctly with the current lazy KVM >>> FPU restore mechanism. Eager KVM FPU restore should be enabled if >>> the MPX feature is exposed to VM. >>> >>> Signed-off-by: Liang Li <liang.z.li@intel.com> >>> --- >>> arch/x86/kvm/vmx.c | 2 ++ >>> arch/x86/kvm/x86.c | 3 ++- >>> 2 files changed, 4 insertions(+), 1 deletion(-) >>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index >>> f7b6168..e2cccbe 100644 >>> --- a/arch/x86/kvm/vmx.c >>> +++ b/arch/x86/kvm/vmx.c >>> @@ -8445,6 +8445,8 @@ static struct kvm_vcpu >>> *vmx_create_vcpu(struct > kvm *kvm, unsigned int id) >>> goto free_vmcs; >>> } >>> + if (vmx_mpx_supported()) >>> + vmx_fpu_activate(&vmx->vcpu); >>> return &vmx->vcpu; >>> >>> free_vmcs: >> >> Is it better to use guest_cpuid_has_mpx() instead of vmx_mpx_supported()? > > CPUID hasn't been set yet, so I think it is okay to key it on > vmx_mpx_supported(). It will be deactivated soon afterwards. > > Or even do it unconditionally; just make sure to add a comment about it. Correct! Unconditionally would be acceptable. > >>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index >>> 5f38188..5993f5f >>> 100644 >>> --- a/arch/x86/kvm/x86.c >>> +++ b/arch/x86/kvm/x86.c >>> @@ -7060,7 +7060,8 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu) >>> fpu_save_init(&vcpu->arch.guest_fpu); >>> __kernel_fpu_end(); >>> ++vcpu->stat.fpu_reload; >>> - kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); >>> + if (!kvm_x86_ops->mpx_supported()) >>> + kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); >>> trace_kvm_fpu(0); >>> } > > This is a hotter path. Here it's definitely better to avoid the call > to kvm_x86_ops->mpx_supported(). Especially because, with MPX > enabled, you would call this on every userspace exit. > > Yang's suggestion of using CPUID is actually more valuable here. You > could add a new field eager_fpu in kvm->arch and update it in kvm_update_cpuid. Good suggestion! > > Thanks, > > Paolo Best regards, Yang -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Is it better to use guest_cpuid_has_mpx() instead of > vmx_mpx_supported()? > > CPUID hasn't been set yet, so I think it is okay to key it on > vmx_mpx_supported(). It will be deactivated soon afterwards. > > Or even do it unconditionally; just make sure to add a comment about it. > > >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index > >> 5f38188..5993f5f > >> 100644 > >> --- a/arch/x86/kvm/x86.c > >> +++ b/arch/x86/kvm/x86.c > >> @@ -7060,7 +7060,8 @@ void kvm_put_guest_fpu(struct kvm_vcpu > *vcpu) > >> fpu_save_init(&vcpu->arch.guest_fpu); > >> __kernel_fpu_end(); > >> ++vcpu->stat.fpu_reload; > >> - kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); > >> + if (!kvm_x86_ops->mpx_supported()) > >> + kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); > >> trace_kvm_fpu(0); > >> } > > This is a hotter path. Here it's definitely better to avoid the call to > kvm_x86_ops->mpx_supported(). Especially because, with MPX enabled, > you would call this on every userspace exit. > > Yang's suggestion of using CPUID is actually more valuable here. You could > add a new field eager_fpu in kvm->arch and update it in kvm_update_cpuid. Thanks for your comments. I will change the code according to your suggestion. > Thanks, > > Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index f7b6168..e2cccbe 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -8445,6 +8445,8 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id) goto free_vmcs; } + if (vmx_mpx_supported()) + vmx_fpu_activate(&vmx->vcpu); return &vmx->vcpu; free_vmcs: diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 5f38188..5993f5f 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -7060,7 +7060,8 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu) fpu_save_init(&vcpu->arch.guest_fpu); __kernel_fpu_end(); ++vcpu->stat.fpu_reload; - kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); + if (!kvm_x86_ops->mpx_supported()) + kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); trace_kvm_fpu(0); }
The MPX feature requires eager KVM FPU restore support. We have verified that MPX cannot work correctly with the current lazy KVM FPU restore mechanism. Eager KVM FPU restore should be enabled if the MPX feature is exposed to VM. Signed-off-by: Liang Li <liang.z.li@intel.com> --- arch/x86/kvm/vmx.c | 2 ++ arch/x86/kvm/x86.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-)