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 <[email protected]>
---
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
Correct Gleb's email address.
Liang
> -----Original Message-----
> From: Li, Liang Z
> Sent: Wednesday, May 20, 2015 10:36 PM
> To: [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; 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 <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
>> ---
>> 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
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 <[email protected]>
>>> ---
>>> 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
> > 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