* [PATCH] KVM: x86: remove vcpu's CPL check in host invoked vcpu's xcr set process
@ 2013-06-14 7:36 Zhanghaoyu (A)
2013-06-17 15:38 ` Paolo Bonzini
2013-06-18 8:00 ` Paolo Bonzini
0 siblings, 2 replies; 6+ messages in thread
From: Zhanghaoyu (A) @ 2013-06-14 7:36 UTC (permalink / raw)
To: KVM, Paolo Bonzini, mst@redhat.com
Cc: Luonengjun, Zanghongyong, Huangweidong (C), Wangrui (K)
__kvm_set_xcr function does the CPL check when set xcr. __kvm_set_xcr is called in two flows,
one is invoked by guest, call stack shown as below,
handle_xsetbv(or xsetbv_interception)
kvm_set_xcr
__kvm_set_xcr
the other one is invoked by host(QEMU), call stack shown as below,
kvm_arch_vcpu_ioctl
kvm_vcpu_ioctl_x86_set_xcrs
__kvm_set_xcr
The former does need the CPL check, but the latter does not.
Signed-off-by: Zhang Haoyu <haoyu.zhang@huawei.com>
---
arch/x86/kvm/x86.c | 5 +-
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 094b5d9..16ff44e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -582,8 +582,6 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
if (index != XCR_XFEATURE_ENABLED_MASK)
return 1;
xcr0 = xcr;
- if (kvm_x86_ops->get_cpl(vcpu) != 0)
- return 1;
if (!(xcr0 & XSTATE_FP))
return 1;
if ((xcr0 & XSTATE_YMM) && !(xcr0 & XSTATE_SSE))
@@ -597,7 +595,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
{
- if (__kvm_set_xcr(vcpu, index, xcr)) {
+ if ((kvm_x86_ops->get_cpl(vcpu) != 0) ||
+ __kvm_set_xcr(vcpu, index, xcr)) {
kvm_inject_gp(vcpu, 0);
return 1;
}
1.7.3.1.msysgit.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: remove vcpu's CPL check in host invoked vcpu's xcr set process
2013-06-14 7:36 [PATCH] KVM: x86: remove vcpu's CPL check in host invoked vcpu's xcr set process Zhanghaoyu (A)
@ 2013-06-17 15:38 ` Paolo Bonzini
2013-06-18 1:32 ` Zhanghaoyu (A)
2013-06-18 8:00 ` Paolo Bonzini
1 sibling, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2013-06-17 15:38 UTC (permalink / raw)
To: Zhanghaoyu (A)
Cc: KVM, mst@redhat.com, Luonengjun, Zanghongyong, Huangweidong (C),
Wangrui (K)
Il 14/06/2013 09:36, Zhanghaoyu (A) ha scritto:
> __kvm_set_xcr function does the CPL check when set xcr. __kvm_set_xcr is called in two flows,
> one is invoked by guest, call stack shown as below,
> handle_xsetbv(or xsetbv_interception)
> kvm_set_xcr
> __kvm_set_xcr
> the other one is invoked by host(QEMU), call stack shown as below,
> kvm_arch_vcpu_ioctl
> kvm_vcpu_ioctl_x86_set_xcrs
> __kvm_set_xcr
>
> The former does need the CPL check, but the latter does not.
>
> Signed-off-by: Zhang Haoyu <haoyu.zhang@huawei.com>
What does this patch fix? I suppose it is some kind of migration
problem since you mentioned QEMU, but I'd rather be sure. I can fix the
commit message myself when applying.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: remove vcpu's CPL check in host invoked vcpu's xcr set process
2013-06-17 15:38 ` Paolo Bonzini
@ 2013-06-18 1:32 ` Zhanghaoyu (A)
0 siblings, 0 replies; 6+ messages in thread
From: Zhanghaoyu (A) @ 2013-06-18 1:32 UTC (permalink / raw)
To: Paolo Bonzini
Cc: KVM, mst@redhat.com, Luonengjun, Zanghongyong, Huangweidong (C),
Wangrui (K)
>> __kvm_set_xcr function does the CPL check when set xcr. __kvm_set_xcr
>> is called in two flows, one is invoked by guest, call stack shown as
>> below, handle_xsetbv(or xsetbv_interception)
>> kvm_set_xcr
>> __kvm_set_xcr
>> the other one is invoked by host(QEMU), call stack shown as below,
>> kvm_arch_vcpu_ioctl
>> kvm_vcpu_ioctl_x86_set_xcrs
>> __kvm_set_xcr
>>
>> The former does need the CPL check, but the latter does not.
>>
>> Signed-off-by: Zhang Haoyu <haoyu.zhang@huawei.com>
>
>What does this patch fix? I suppose it is some kind of migration problem since you mentioned QEMU, but I'd rather be sure. I can fix the commit message myself when applying.
This patch tries to fix the problem of system_reset invoked by qemu monitor[system_reset] or virsh command[virsh reset domain].
QEMU will reset the domain on receiving the reset request from qemu monitor or libvirtd, the reset flow shown as below,
main_loop_should_exit
|- pause_all_vcpus
|- cpu_synchronize_all_states
|- qemu_system_reset
|-- cpu_synchronize_all_post_reset
|--- cpu_synchronize_post_reset
|---- kvm_cpu_synchronize_post_reset
|----- kvm_arch_put_registers
|------ kvm_put_xcrs
|------- kvm_vcpu_ioctl(CPU(cpu), KVM_SET_XCRS, &xcrs)
Above IOCTL syscall traps to kernel-space, KVM hypervisor then deal with the xcr set request,
kvm_arch_vcpu_ioctl
|- kvm_vcpu_ioctl_x86_set_xcrs
|-- __kvm_set_xcr
|--- if (kvm_x86_ops->get_cpl(vcpu) != 0)
return 1;
It's quite probably happened that vcpu's CPL is 3 while QEMU monitor doing the reset operation, which is a normal case, so the CPL check should be removed.
But __kvm_set_xcr is also called in below flow, which is invoked by guest,
handle_xsetbv(or xsetbv_interception)
|- kvm_set_xcr
|-- __kvm_set_xcr
On this case, vcpu's CPL check must be performed, if vcpu's CPL!=0, then #GP exception should be injected to VM.
Thanks,
Zhang Haoyu
>
>Thanks,
>
>Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: remove vcpu's CPL check in host invoked vcpu's xcr set process
2013-06-14 7:36 [PATCH] KVM: x86: remove vcpu's CPL check in host invoked vcpu's xcr set process Zhanghaoyu (A)
2013-06-17 15:38 ` Paolo Bonzini
@ 2013-06-18 8:00 ` Paolo Bonzini
2013-06-18 12:39 ` Zhanghaoyu (A)
1 sibling, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2013-06-18 8:00 UTC (permalink / raw)
To: Zhanghaoyu (A)
Cc: KVM, mst@redhat.com, Luonengjun, Zanghongyong, Huangweidong (C),
Wangrui (K)
Il 14/06/2013 09:36, Zhanghaoyu (A) ha scritto:
> __kvm_set_xcr function does the CPL check when set xcr. __kvm_set_xcr is called in two flows,
> one is invoked by guest, call stack shown as below,
> handle_xsetbv(or xsetbv_interception)
> kvm_set_xcr
> __kvm_set_xcr
> the other one is invoked by host(QEMU), call stack shown as below,
> kvm_arch_vcpu_ioctl
> kvm_vcpu_ioctl_x86_set_xcrs
> __kvm_set_xcr
>
> The former does need the CPL check, but the latter does not.
>
> Signed-off-by: Zhang Haoyu <haoyu.zhang@huawei.com>
> ---
> arch/x86/kvm/x86.c | 5 +-
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 094b5d9..16ff44e 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -582,8 +582,6 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
> if (index != XCR_XFEATURE_ENABLED_MASK)
> return 1;
> xcr0 = xcr;
> - if (kvm_x86_ops->get_cpl(vcpu) != 0)
> - return 1;
> if (!(xcr0 & XSTATE_FP))
> return 1;
> if ((xcr0 & XSTATE_YMM) && !(xcr0 & XSTATE_SSE))
> @@ -597,7 +595,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
>
> int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
> {
> - if (__kvm_set_xcr(vcpu, index, xcr)) {
> + if ((kvm_x86_ops->get_cpl(vcpu) != 0) ||
> + __kvm_set_xcr(vcpu, index, xcr)) {
> kvm_inject_gp(vcpu, 0);
> return 1;
> }
> 1.7.3.1.msysgit.0
>
Applied to master, but check your setup because I had to do so manually.
Your patch has 2 tabs + 1 space for each indentation level (??), the
file only has one tab.
This time the patch was pretty simple and may even be worth having in
3.9, but next time I'd ask you to resubmit.
Can you write a testcase for kvm-unit-tests?
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: remove vcpu's CPL check in host invoked vcpu's xcr set process
2013-06-18 8:00 ` Paolo Bonzini
@ 2013-06-18 12:39 ` Zhanghaoyu (A)
2013-06-18 13:34 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: Zhanghaoyu (A) @ 2013-06-18 12:39 UTC (permalink / raw)
To: Paolo Bonzini
Cc: KVM, mst@redhat.com, Luonengjun, Zanghongyong, Huangweidong (C),
Wangrui (K)
>> __kvm_set_xcr function does the CPL check when set xcr. __kvm_set_xcr
>> is called in two flows, one is invoked by guest, call stack shown as
>> below, handle_xsetbv(or xsetbv_interception)
>> kvm_set_xcr
>> __kvm_set_xcr
>> the other one is invoked by host(QEMU), call stack shown as below,
>> kvm_arch_vcpu_ioctl
>> kvm_vcpu_ioctl_x86_set_xcrs
>> __kvm_set_xcr
>>
>> The former does need the CPL check, but the latter does not.
>>
>> Signed-off-by: Zhang Haoyu <haoyu.zhang@huawei.com>
>> ---
>> arch/x86/kvm/x86.c | 5 +-
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index
>> 094b5d9..16ff44e 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -582,8 +582,6 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
>> if (index != XCR_XFEATURE_ENABLED_MASK)
>> return 1;
>> xcr0 = xcr;
>> - if (kvm_x86_ops->get_cpl(vcpu) != 0)
>> - return 1;
>> if (!(xcr0 & XSTATE_FP))
>> return 1;
>> if ((xcr0 & XSTATE_YMM) && !(xcr0 & XSTATE_SSE)) @@ -597,7 +595,8
>> @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
>>
>> int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr) {
>> - if (__kvm_set_xcr(vcpu, index, xcr)) {
>> + if ((kvm_x86_ops->get_cpl(vcpu) != 0) ||
>> + __kvm_set_xcr(vcpu, index, xcr)) {
>> kvm_inject_gp(vcpu, 0);
>> return 1;
>> }
>> 1.7.3.1.msysgit.0
>>
>
>Applied to master, but check your setup because I had to do so manually.
> Your patch has 2 tabs + 1 space for each indentation level (??), the file only has one tab.
>
>This time the patch was pretty simple and may even be worth having in 3.9, but next time I'd ask you to resubmit.
>
>Can you write a testcase for kvm-unit-tests?
>
>Paolo
So sorry for above bad format patch, this time the format is OK, but I'm not sure could the format be changed in transmission?
Signed-off-by: Zhang Haoyu <haoyu.zhang@huawei.com>
---
arch/x86/kvm/x86.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 094b5d9..aa7bda1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -582,8 +582,6 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
if (index != XCR_XFEATURE_ENABLED_MASK)
return 1;
xcr0 = xcr;
- if (kvm_x86_ops->get_cpl(vcpu) != 0)
- return 1;
if (!(xcr0 & XSTATE_FP))
return 1;
if ((xcr0 & XSTATE_YMM) && !(xcr0 & XSTATE_SSE))
@@ -597,7 +595,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
{
- if (__kvm_set_xcr(vcpu, index, xcr)) {
+ if ((kvm_x86_ops->get_cpl(vcpu) != 0) ||
+ __kvm_set_xcr(vcpu, index, xcr)) {
kvm_inject_gp(vcpu, 0);
return 1;
}
--
1.7.3.1.msysgit.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: remove vcpu's CPL check in host invoked vcpu's xcr set process
2013-06-18 12:39 ` Zhanghaoyu (A)
@ 2013-06-18 13:34 ` Paolo Bonzini
0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2013-06-18 13:34 UTC (permalink / raw)
To: Zhanghaoyu (A)
Cc: KVM, mst@redhat.com, Luonengjun, Zanghongyong, Huangweidong (C),
Wangrui (K)
Il 18/06/2013 14:39, Zhanghaoyu (A) ha scritto:
>> Applied to master, but check your setup because I had to do so manually.
>> Your patch has 2 tabs + 1 space for each indentation level (??), the file only has one tab.
>>
>> This time the patch was pretty simple and may even be worth having in 3.9, but next time I'd ask you to resubmit.
>>
>> Can you write a testcase for kvm-unit-tests?
>>
>> Paolo
>
> So sorry for above bad format patch, this time the format is OK, but
> I'm not sure could the format be changed in transmission?
This time it is okay.
Paolo
>
> Signed-off-by: Zhang Haoyu <haoyu.zhang@huawei.com>
> ---
> arch/x86/kvm/x86.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 094b5d9..aa7bda1 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -582,8 +582,6 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
> if (index != XCR_XFEATURE_ENABLED_MASK)
> return 1;
> xcr0 = xcr;
> - if (kvm_x86_ops->get_cpl(vcpu) != 0)
> - return 1;
> if (!(xcr0 & XSTATE_FP))
> return 1;
> if ((xcr0 & XSTATE_YMM) && !(xcr0 & XSTATE_SSE))
> @@ -597,7 +595,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
>
> int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
> {
> - if (__kvm_set_xcr(vcpu, index, xcr)) {
> + if ((kvm_x86_ops->get_cpl(vcpu) != 0) ||
> + __kvm_set_xcr(vcpu, index, xcr)) {
> kvm_inject_gp(vcpu, 0);
> return 1;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-18 13:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-14 7:36 [PATCH] KVM: x86: remove vcpu's CPL check in host invoked vcpu's xcr set process Zhanghaoyu (A)
2013-06-17 15:38 ` Paolo Bonzini
2013-06-18 1:32 ` Zhanghaoyu (A)
2013-06-18 8:00 ` Paolo Bonzini
2013-06-18 12:39 ` Zhanghaoyu (A)
2013-06-18 13:34 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).