[PATCH] x86: consistently mask floating point exceptions

[PATCH] x86: consistently mask floating point exceptions

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1853 bytes --]

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@suse.com>

--- 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));




[-- Attachment #2: x86-fpu-context-conditional.patch --]
[-- Type: text/plain, Size: 1899 bytes --]

x86: consistently mask floating point exceptions

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@suse.com>

--- 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));

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH] x86: consistently mask floating point exceptions

[Keir Fraser]

On 16/01/2013 10:51, "Jan Beulich" <JBeulich@suse.com> 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@suse.com>

This seems a sane and simple fix.

Acked-by: Keir Fraser <keir@xen.org>

> --- 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@lists.xen.org
> http://lists.xen.org/xen-devel