On Tue, Apr 5, 2016 at 4:28 AM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > ... > > While running my acceptance tests, in one case I got one CPU whose xcr0 > had leaked into the host. This showed up as a SIGILL in strncasecmp's > AVX code, and a simple program confirmed it: > > $ cat xgetbv.c > #include <stdio.h> > int main(void) > { > unsigned xcr0_h, xcr0_l; > asm("xgetbv" : "=d"(xcr0_h), "=a"(xcr0_l) : "c"(0)); > printf("%08x:%08x\n", xcr0_h, xcr0_l); > } > $ gcc xgetbv.c -O2 > $ for i in `seq 0 55`; do echo $i `taskset -c $i ./a.out`; done|grep -v 007 > 19 00000000:00000003 > > I'm going to rerun the tests without this patch, as it seems the most > likely culprit, and leave it out of the pull request if they pass. Agreed this is a very likely culprit. I think I see one way the guest's xcr0 can leak into the host. I will do some testing an send another version. Thanks. > > Paolo > >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index e260ccb..8df1167 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -700,7 +700,6 @@ static int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr) >> if ((xcr0 & XFEATURE_MASK_AVX512) != XFEATURE_MASK_AVX512) >> return 1; >> } >> - kvm_put_guest_xcr0(vcpu); >> vcpu->arch.xcr0 = xcr0; >> >> if ((xcr0 ^ old_xcr0) & XFEATURE_MASK_EXTEND) >> @@ -6590,8 +6589,6 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) >> kvm_x86_ops->prepare_guest_switch(vcpu); >> if (vcpu->fpu_active) >> kvm_load_guest_fpu(vcpu); >> - kvm_load_guest_xcr0(vcpu); >> - >> vcpu->mode = IN_GUEST_MODE; >> >> srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx); >> @@ -6607,6 +6604,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) >> >> local_irq_disable(); >> >> + kvm_load_guest_xcr0(vcpu); >> + >> if (vcpu->mode == EXITING_GUEST_MODE || vcpu->requests >> || need_resched() || signal_pending(current)) { Here, after we've loaded the guest xcr0, if we enter this if statement, we return from vcpu_enter_guest with the guest's xcr0 still loaded. >> vcpu->mode = OUTSIDE_GUEST_MODE; >> @@ -6667,6 +6666,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) >> vcpu->mode = OUTSIDE_GUEST_MODE; >> smp_wmb(); >> >> + kvm_put_guest_xcr0(vcpu); >> + >> /* Interrupt is enabled by handle_external_intr() */ >> kvm_x86_ops->handle_external_intr(vcpu); >> >> @@ -7314,7 +7315,6 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu) >> * and assume host would use all available bits. >> * Guest xcr0 would be loaded later. >> */ >> - kvm_put_guest_xcr0(vcpu); >> vcpu->guest_fpu_loaded = 1; >> __kernel_fpu_begin(); >> __copy_kernel_to_fpregs(&vcpu->arch.guest_fpu.state); >> @@ -7323,8 +7323,6 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu) >> >> void kvm_put_guest_fpu(struct kvm_vcpu *vcpu) >> { >> - kvm_put_guest_xcr0(vcpu); >> - >> if (!vcpu->guest_fpu_loaded) { >> vcpu->fpu_counter = 0; >> return; >> -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html