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

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

Keir Fraser wrote:
> On 01/09/2010 02:53, "Weidong Han" <weidong.han@intel.com> wrote:
>
>   
>>> Performance overhead of this fix? Is there no other lazy save technique that
>>> can work?
>>>   
>>>       
>> I think the cost of set_xcr0 which just changes some bits in XCR0
>> register should be little.  I don't have any optimization for it now.
>>     
>
> I obviously don't mean that. What about the increased cost of XSAVE and
> XRSTOR s/r'ing more state unconditionally? At least it is conditional on
> v->fpu_dirtied I suppose?
>   
One possible optimization is that only save/restore legacy states (FPU 
and SSE) for guests which don't enable XSAVE.

Both xsave() and xrstor are invoked only when v->fpu_dirtied.
>   
>>>> Patch 3/3. Enable guest AVX
>>>> This patch enables Intel(R) Advanced Vector Extension (AVX) for guest.
>>>>     
>>>>         
>>> If we enable this but don't implement save/restore then don't guests lose
>>> state across s/r with unpredictable results?
>>>   
>>>       
>> 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.
>
> 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?
>   
I'm afraid XSAVE related stuff cannot be move out into 
libxc/xc_cpuid_x86.c completely? At least Xen needs to control 
X86_CR4_OSXSAVE for guests which don't support XSAVE. I will have a look 
at it.

Regards,
Weidong
>  -- Keir
>
>   
>> Regards,
>> Weidong
>>     
>>>
>>>   
>>>       
>
>
>