Il 16/04/2013 04:30, Marcelo Tosatti ha scritto:
> 
> ** Untested **.
> 
> Emulation of xcr0 writes zero guest_xcr0_loaded variable so that
> subsequent VM-entry reloads CPU's xcr0 with guests xcr0 value.
> 
> However, this is incorrect because guest_xcr0_loaded variable is 
> read to decide whether to reload hosts xcr0.
> 
> In case the vcpu thread is scheduled out after the guest_xcr0_loaded = 0
> assignment, and scheduler decides to preload FPU:
> 
> switch_to
> {
>   __switch_to
>     __math_state_restore
>       restore_fpu_checking
>         fpu_restore_checking
>           if (use_xsave())
>               fpu_xrstor_checking
>               xrstor64 with CPU's xcr0 == guests xcr0
> 
> Fix by properly restoring hosts xcr0 during emulation of xcr0 writes.
> 
> Analyzed-by: Ulrich Obergfell <uober...@redhat.com>
> Signed-off-by: Marcelo Tosatti <mtosa...@redhat.com>
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 999d124..222926a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -555,6 +555,25 @@ void kvm_lmsw(struct kvm_vcpu *vcpu, unsigned long msw)
>  }
>  EXPORT_SYMBOL_GPL(kvm_lmsw);
>  
> +static void kvm_load_guest_xcr0(struct kvm_vcpu *vcpu)
> +{
> +     if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) &&
> +                     !vcpu->guest_xcr0_loaded) {
> +             /* kvm_set_xcr() also depends on this */
> +             xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
> +             vcpu->guest_xcr0_loaded = 1;
> +     }
> +}
> +
> +static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> +{
> +     if (vcpu->guest_xcr0_loaded) {
> +             if (vcpu->arch.xcr0 != host_xcr0)
> +                     xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
> +             vcpu->guest_xcr0_loaded = 0;
> +     }
> +}
> +
>  int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
>  {
>       u64 xcr0;

This is just code movement...

> @@ -571,8 +590,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 
> xcr)
>               return 1;
>       if (xcr0 & ~host_xcr0)
>               return 1;
> +     kvm_put_guest_xcr0(vcpu);
>       vcpu->arch.xcr0 = xcr0;
> -     vcpu->guest_xcr0_loaded = 0;
>       return 0;
>  }
>  

... and this is the bulk of the fix: never set guest_xcr0_loaded, always
go through kvm_load/put_guest_xcr0.

Pending test,

Reviewed-by: Paolo Bonzini <pbonz...@redhat.com>

Paolo

> @@ -5600,25 +5619,6 @@ static void inject_pending_event(struct kvm_vcpu *vcpu)
>       }
>  }
>  
> -static void kvm_load_guest_xcr0(struct kvm_vcpu *vcpu)
> -{
> -     if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) &&
> -                     !vcpu->guest_xcr0_loaded) {
> -             /* kvm_set_xcr() also depends on this */
> -             xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
> -             vcpu->guest_xcr0_loaded = 1;
> -     }
> -}
> -
> -static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> -{
> -     if (vcpu->guest_xcr0_loaded) {
> -             if (vcpu->arch.xcr0 != host_xcr0)
> -                     xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
> -             vcpu->guest_xcr0_loaded = 0;
> -     }
> -}
> -
>  static void process_nmi(struct kvm_vcpu *vcpu)
>  {
>       unsigned limit = 2;
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to