Re: [PATCH 0/4] XSAVE/XRSTOR fixes and enhancements

[Weidong Han <weidong.han@intel.com>]

Keir Fraser wrote:
> On 31/08/2010 03:59, "Han, Weidong" <weidong.han@intel.com> wrote:
>
>   
>> The patchset is as follows:
>> Patch 1/4: Fix frozen states
>> If a guest sets a state and dirties the state, but later temporarily clears
>> the state, and at this time this vcpu is scheduled out, then other vcpus may
>> corrupt the state before the vcpu is scheduled in again, thus the state cannot
>> be restored correctly. To solve this issue, this patch save/restore all states
>> unconditionally on vcpu context switch.
>>     
>
> I don't really understand, and the patch is non-obvious and large. Can it be
> broken up or explained more.
>   
The patch also includes some cleanups. I will split it.

>   
>> Patch 2/4: Enable XSAVE/XRSTOR live migration:
>> This patch enables save/restore xsave states for guests. Because xsave area
>> size is not fixed, this patch uses 4K Bytes for future extension.
>>     
>
> Breaks backward compatibility by changing size of vcpu_guest_context (part
> of the PV guest ABI). Totally unacceptable -- is this a nasty attempt to get
>   
yes, it cannot restore guest saved by old Xen. do you mean we should not 
change size of vcpu_guest_context in future?
> SR working for PV guests? I think you'll be needing a special XSAVE
> save/restore domctl for PV. However I think you're targeting HVM only for
> now so I'm not sure why this is at all needed.
>   
I'm targeting HVM only for now. Yes, it only needs to add a separate 
XSAVE s/r chunk for hvm guest, instead of changing above structures 
which breaks PV.
> Also expanding the size of the hvm_vcpu s/r structure similarly breaks
> compatibility with older saved images. You'll have to put XSAVE state in a
> separate s/r chunk with its own identifier tag. I'm not sure we currently
> support variable-length HVM s/r chunks but there's no reason not to and this
> XSAVE chunk would be a good time to implement that. Then you don't need to
> limit yourself to a static 4kB for all time.
>   
Yes, XSAVE needs variable-length s/r chunk.

Regards,
Weidong
>  -- Keir
>
>   
>> Patch 3/4. Enable guest AVX
>> This patch enables Intel(R) Advanced Vector Extension (AVX) for guest.
>>
>> Patch 4/4. Dom0: extend fpu_ctxt to consist with Xen hypervisor
>> fpu_ctxt must be consistent between Dom0 and Xen hypervisor. Otherwise, dom0
>> cannot boot up.
>>
>> Signed-off-by: Weidong Han <weidong.han@intel.com>
>>     
>
>
>