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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.11.15 at 14:10, <roger.pau@xxxxxxxxxx> wrote:
> El 20/11/15 a les 16.49, Jan Beulich ha escrit:
>>>>> 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?
> 
> If !fpu_initialized the fpu context save record is all zeroed out. I
> don't think it matters much (apart from saving a few CPU cycles). I can
> send a new version that doesn't save/restore the fpu context at all if
> !fpu_initialised.

I'd appreciate that (ideally with if(!fpu_initialised) memset(); else if ...).

>>> @@ -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?
> 
> AFAIK we agreed on adding this field to the tail and making it a
> uint32_t so that when new fields are added they can be detected by
> looking at the size of the structure:
> 
> http://marc.info/?l=xen-devel&m=144490321208291 

Admittedly it's a little implicit, but that mail has, in its quoting parts,

"... (and validate unused tail bits are zero, so they can be used for
something later on)"

going back to that intention of using just a single bit here afaict.

Jan


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