| Message ID | D3E216785288A145B7BC975F83A2ED103FEAC791@szxeml556-mbx.china.huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- 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
>> __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 -- 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
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 -- 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/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; }
__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(-) 1.7.3.1.msysgit.0 -- 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