Re: [Xen-devel] [PATCH v3 3/4] xen/hvm: introduce a fpu_uninitialised field to the CPU save record

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.11.15 at 17:37, <roger.pau@xxxxxxxxxx> wrote:
> @@ -2091,7 +2092,8 @@ static int hvm_load_cpu_ctxt(struct domain *d, 
> hvm_domain_context_t *h)
>          struct xsave_struct *xsave_area = v->arch.xsave_area;
>  
>          memcpy(v->arch.xsave_area, ctxt.fpu_regs, sizeof(ctxt.fpu_regs));
> -        xsave_area->xsave_hdr.xstate_bv = XSTATE_FP_SSE;
> +        xsave_area->xsave_hdr.xstate_bv = ctxt.fpu_initialised ?
> +                                                    XSTATE_FP_SSE : 0;
>      }
>      else
>          memcpy(v->arch.fpu_ctxt, ctxt.fpu_regs, sizeof(ctxt.fpu_regs));

Question is - are the memcpy()s here really meaningful/valid
when !ctxt.fpu_initialized? I.e. shouldn't this code rather be
skipped instead of getting modified?

> @@ -157,6 +159,8 @@ struct hvm_hw_cpu {
>      };
>      /* error code for pending event */
>      uint32_t error_code;
> +    /* is fpu initialised? */
> +    uint32_t fpu_initialised;

A whole uint32_t for just one bit? Didn't we talk about making this
new field a flags one, consuming just one bit from it?

> @@ -266,6 +270,7 @@ struct hvm_hw_cpu_compat {
>      };
>      /* error code for pending event */
>      uint32_t error_code;
> +    /*uint32_t fpu_initialised; COMPAT */
>  };

I think this is misleading - the compat structure never has this field.

Jan


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