[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] x86: consistently mask floating point exceptions


  • To: Jan Beulich <JBeulich@xxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxx>
  • From: Keir Fraser <keir@xxxxxxx>
  • Date: Wed, 16 Jan 2013 11:13:44 +0000
  • Delivery-date: Wed, 16 Jan 2013 11:14:13 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xen.org>
  • Thread-index: Ac3z2ovzPAwYKUAjR0msamaGXgeAuw==
  • Thread-topic: [Xen-devel] [PATCH] x86: consistently mask floating point exceptions

On 16/01/2013 10:51, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:

> c/s 23142:f5e8d152a565 resulted in v->arch.fpu_ctxt to point into the
> save area allocated for xsave/xrstor (when they're available). The way
> vcpu_restore_fpu_lazy() works (using fpu_init() for an uninitialized
> vCPU only when there's no xsave support) causes this to load whatever
> arch_set_info_guest() put there, irrespective of whether the i387 state
> was specified to be valid in the respective input structure.
> 
> Consequently, with a cleared (al zeroes) incoming FPU context, and with
> xsave available, one gets all exceptions unmasked (as opposed to to the
> legacy case, where FINIT and LDMXCSR get used, masking all exceptions).
> This causes e.g. para-virtualized NetWare to crash.
> 
> The behavior of arch_set_info_guest() is thus being made more hardware-
> like for the FPU portion of it: Considering it to be similar to INIT,
> it will leave untouched all floating point state now. An alternative
> would be to make the behavior RESET-like, forcing all state to known
> values, albeit - taking into account legacy behavior - not to precisely
> the values RESET would enforce (which masks only SSE exceptions, but
> not x87 ones); that would come closest to mimicing FINIT behavior in
> the xsave case. Another option would be to continue copying whatever
> was provided, but override (at least) FCW and MXCSR if VGCF_I387_VALID
> isn't set.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

This seems a sane and simple fix.

Acked-by: Keir Fraser <keir@xxxxxxx>

> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -748,7 +748,9 @@ int arch_set_info_guest(
>  
>      v->arch.vgc_flags = flags;
>  
> -    memcpy(v->arch.fpu_ctxt, &c.nat->fpu_ctxt, sizeof(c.nat->fpu_ctxt));
> +    if ( flags & VGCF_I387_VALID )
> +        memcpy(v->arch.fpu_ctxt, &c.nat->fpu_ctxt, sizeof(c.nat->fpu_ctxt));
> +
>      if ( !compat )
>      {
>          memcpy(&v->arch.user_regs, &c.nat->user_regs,
> sizeof(c.nat->user_regs));
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.