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

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

On 18/11/15 10:51, Roger Pau Monnà wrote:
> El 17/11/15 a les 19.02, Andrew Cooper ha escrit:
>> On 17/11/15 18:44, Roger Pau Monne wrote:
>>> Introduce a new filed to signal if the FPU has been initialised or not. Xen
>> field
>>> needs this new filed in order to know whether to set the FPU as initialised
>>> or not during restore of CPU context. Previously Xen always wrongly assumed
>>> the FPU was initialised on restore.
>>> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
>>> Cc: Jan Beulich <jbeulich@xxxxxxxx>
>>> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> ---
>>> Changes since v1:
>>>  - Don't add yet another compat structure, new fields should always be added
>>>    to the end of the existing structure and offsetof should be used to
>>>    compare sizes.
>>>  - Leave the previous compat structure as-is, since the field was not added
>>>    to the end we cannot remove it and use offsetof in this case.
>> How can this work?
>> Making it zeroextended means that any short record will be padded with
>> zeroes.  As a result, the compat checking logic is skipped.
>> (This HVM_SAVE_* infrastructure is truly horrifying code which should
>> never have been accepted.  I think I have correctly followed what it is
>> doing, but I could be mistaken.)
> No, you are completely right, it was an oversight on my side. So neither
> hvm_load_entry or hvm_load_entry_zeroextend will do what I want/need.
> The options I see so far are:
>  - Make hvm_hw_cpu_compat hvm_hw_cpu minus the fpu_initalised field
> (this means dropping support for Xen pre-3.4).

Sorry - not an option.

>  - Rewrite part of the hvm_load_entry_zeroextend so that it will load
> records with "len < expected len" and still call the fixup function.
> TBH the mess with the hvm_hw_cpu_compat structure is very bad. Adding
> the msr_tsc_aux in the middle of the structure makes all the compat
> handling much more complicated that what it needs to be.

The inclusion of msr_tsc_aux in its current location was a very
regrettable mistake.

As for the problem at hand, I don't see what was wrong with v1.

Fundamentally, we have three different variations of the same structure;
two of which require special compat handling.  Pretending otherwise is
just silly.


Xen-devel mailing list



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