* RE: [PATCH] kvm/fpu: Enable eager restore kvm FPU for MPX
2015-05-20 14:35 [PATCH] kvm/fpu: Enable eager restore kvm FPU for MPX Liang Li
@ 2015-05-20 3:00 ` Li, Liang Z
2015-05-20 5:20 ` Zhang, Yang Z
1 sibling, 0 replies; 6+ messages in thread
From: Li, Liang Z @ 2015-05-20 3:00 UTC (permalink / raw)
To: kvm, linux-kernel; +Cc: pbonzini, tglx, mingo, hpa, x86, Zhang, Yang Z, gleb
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] kvm/fpu: Enable eager restore kvm FPU for MPX
2015-05-20 14:35 [PATCH] kvm/fpu: Enable eager restore kvm FPU for MPX Liang Li
2015-05-20 3:00 ` Li, Liang Z
@ 2015-05-20 5:20 ` Zhang, Yang Z
2015-05-20 6:41 ` Paolo Bonzini
1 sibling, 1 reply; 6+ messages in thread
From: Zhang, Yang Z @ 2015-05-20 5:20 UTC (permalink / raw)
To: Li, Liang Z, kvm, linux-kernel; +Cc: gleb, pbonzini, tglx, mingo, hpa, x86
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kvm/fpu: Enable eager restore kvm FPU for MPX
2015-05-20 5:20 ` Zhang, Yang Z
@ 2015-05-20 6:41 ` Paolo Bonzini
2015-05-20 6:46 ` Zhang, Yang Z
2015-05-20 7:14 ` Li, Liang Z
0 siblings, 2 replies; 6+ messages in thread
From: Paolo Bonzini @ 2015-05-20 6:41 UTC (permalink / raw)
To: Zhang, Yang Z, Li, Liang Z, kvm, linux-kernel; +Cc: gleb, tglx, mingo, hpa, x86
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] kvm/fpu: Enable eager restore kvm FPU for MPX
2015-05-20 6:41 ` Paolo Bonzini
@ 2015-05-20 6:46 ` Zhang, Yang Z
2015-05-20 7:14 ` Li, Liang Z
1 sibling, 0 replies; 6+ messages in thread
From: Zhang, Yang Z @ 2015-05-20 6:46 UTC (permalink / raw)
To: Paolo Bonzini, Li, Liang Z, kvm, linux-kernel; +Cc: gleb, tglx, mingo, hpa, x86
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] kvm/fpu: Enable eager restore kvm FPU for MPX
2015-05-20 6:41 ` Paolo Bonzini
2015-05-20 6:46 ` Zhang, Yang Z
@ 2015-05-20 7:14 ` Li, Liang Z
1 sibling, 0 replies; 6+ messages in thread
From: Li, Liang Z @ 2015-05-20 7:14 UTC (permalink / raw)
To: Paolo Bonzini, Zhang, Yang Z, kvm, linux-kernel
Cc: gleb, tglx, mingo, hpa, x86
> > 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] kvm/fpu: Enable eager restore kvm FPU for MPX
@ 2015-05-20 14:35 Liang Li
2015-05-20 3:00 ` Li, Liang Z
2015-05-20 5:20 ` Zhang, Yang Z
0 siblings, 2 replies; 6+ messages in thread
From: Liang Li @ 2015-05-20 14:35 UTC (permalink / raw)
To: kvm, linux-kernel
Cc: gleb, pbonzini, tglx, mingo, hpa, x86, yang.z.zhang, Liang Li
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-05-20 7:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-20 14:35 [PATCH] kvm/fpu: Enable eager restore kvm FPU for MPX Liang Li
2015-05-20 3:00 ` Li, Liang Z
2015-05-20 5:20 ` Zhang, Yang Z
2015-05-20 6:41 ` Paolo Bonzini
2015-05-20 6:46 ` Zhang, Yang Z
2015-05-20 7:14 ` Li, Liang Z
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.