Re: XSAVE save/restore shortcomings

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 30/08/13 11:11, Jan Beulich wrote:
>>>> On 30.08.13 at 11:55, "Jan Beulich" <JBeulich@suse.com> wrote:
>> Next both HVM and PV save code needed tweaking to not always save the
>> full state supported by the underlying hardware, but just the parts
>> that the guest actually used. Similarly the restore code should bail
>> not just on state being restored that the hardware cannot handle, but
>> also on inconsistent save state (inconsistent XCR0 settings or size of
>> saved state not in line with XCR0).
>>
>> And finally the PV extended context get/set code needs to use slightly
>> different logic than the HVM one, as here we can't just key off of
>> xsave_enabled() (i.e. avoid doing anything if a guest doesn't use
>> xsave) because the tools use this function to determine host
>> capabilities as well as read/write vCPU state. The set operation in
>> particular needs to be capable of cleanly dealing with input that
>> consists of only the xcr0 and xcr0_accum values (if they're both zero
>> then no further data is required).
> I'd like to make clear that the change presented is going to handle
> only the most trivial cases (where any new xsave state addition
> adds to the end of the save area). This is an effect of a more
> fundamental flaw in the original design (which the patch doesn't try
> to revise, as it's not clear to me yet what the best approach here is):
> While the XSAVE hardware specification allows for each piece to be
> individually savable/restorable, both PV and HVM save/restore
> assume a single monolithic blob. Which is already going to be a
> problem: AVX-512 as well as MPX conflict with LWP. And obviously
> it can't be excluded that we'll see CPUs supporting AVX-512 but not
> MPX as well as guests using the former but not the latter, and
> neither can be dealt with under the current design.
>
> I therefore think that we'll need to start over from scratch with
> how save/restore is to be implemented, breaking up the current
> monolithic block into individual pieces. But before working on a
> proposal, I'd first like to hear whether others can see better (and
> namely less intrusive) ways of dealing with the problem.
>
> Jan

Yikes that's a big patch.  I think I need to read it a few more times.

XenServer has xsave disabled by default, not least because support in
Xen was buggy until very recently.  Getting it working is on my todo
list (behind some other integration issues), but I admit I had not
realised that migration was this broken.  I will see about testing the
patch, particularly to do with migrating between one Xen with the fix
and one without, but I cant guarantee to get around to it any time soon.

I think that breaking the save/restore it into pieces is the only
tenable solution going forward.  I cant spot a less intrusive method,
but even if there is, hacking around this broken design now might be the
easy solution but will be more work in the future as new extensions
appear in the future.

~Andrew