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

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

Keir Fraser wrote:
> On 01/09/2010 08:26, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:
>
>   
>>> Yes. As I said in another email, actually it already breaks hvm guests
>>> save/restore on platforms which supports XSAVE/XRSTOR.
>>>       
>> Wow, so the last couple of Xen releases are broken for the latest Intel
>> platforms unless you specify no-xsave. Handy to know I guess.
>>     
>
> Actually, hang on, Dexuan's patch only enabled XSAVE of FPU/SSE and didn't
> make any other new state visible. We already save/restore FPU/SSE of course,
> so I don't see why the code we already have is broken for HVM save/restore.
> It's the adding of new state that we don't s/r that would be broken --- like
> your patch to make AVX visible.
>
> Well that's how it was explained to me at the time. Was that in fact wrong
> and we are already broken for save/restore for some subtle unexplained
> reason?
>   

When XSAVE is enabled, it saves states to xsave_area in struct hvm_vcpu. 
But hvm_save_cpu_ctxt and hvm_load_cpu_ctxt don't save/restore FPU/SSE 
from xsave_area. So FPU/SSE states are incorrect after guest save/restore.

Regards,
Weidong
>  -- Keir
>
>   
>> Why is the feature flag stuff all stuffed in Xen itself rather than
>> xc_cpuid_x86.c, by the way? Shouldn't your change also be in the same place,
>> or (much preferably) all XSAVE related stuff be moved out into
>> libxc/xc_cpuid_x86.c?
>>     
>
>
>