Xen devel - Nov 2010 - [Patch] Adding back CPUID support for Xsave

Hi, Keir,

This patch adds back the reverted CPUID support for XSAVE feature. Can
you have a review?

Signed-off-by: Shan Haitao <haitao.shan@intel.com>

Shan Haitao


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Please use this one instead. I forget to remove a local change to Config.mk.

Shan Haitao

2010/11/9 Haitao Shan <maillists.shan@gmail.com>:
> Hi, Keir,
>
> This patch adds back the reverted CPUID support for XSAVE feature. Can
> you have a review?
>
> Signed-off-by: Shan Haitao <haitao.shan@intel.com>
>
> Shan Haitao
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Thanks, this looks better. I have some further questions however.

You appear to only be exposing XSAVE to PV guests -- Why is that?

Also, the patch passes the AVX feature unconditionally through to HVM guests
-- firstly, why only visible HVM guests; and secondly, should it not be
conditional on XSAVE support in the hypervisor (don''t we need XSAVE to
be
able to save/restore AVX state)?

 -- Keir

On 09/11/2010 02:31, "Haitao Shan" <maillists.shan@gmail.com>
wrote:
> Hi, Keir,
> 
> This patch adds back the reverted CPUID support for XSAVE feature. Can
> you have a review?
> 
> Signed-off-by: Shan Haitao <haitao.shan@intel.com>
> 
> Shan Haitao
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

I remember cpuid exposure is covered by hvm_cpuid(), which is done by
former (long ago) patches.
Probably we may clean that a bit? So that both PV and HVM have the
same mechanism.

I have considered AVX exposure. My concern is: if AVX is considered to
be relying on XSAVE(of course, it is the truth), will we need to add
future features one by one since they are likely all dependent on
XSAVE.
Or could we just let it be, since even they see it, they can not use it, anyway.

Shan Haitao

2010/11/9 Keir Fraser <keir@xen.org>:
> Thanks, this looks better. I have some further questions however.
>
> You appear to only be exposing XSAVE to PV guests -- Why is that?
>
> Also, the patch passes the AVX feature unconditionally through to HVM
guests
> -- firstly, why only visible HVM guests; and secondly, should it not be
> conditional on XSAVE support in the hypervisor (don''t we need
XSAVE to be
> able to save/restore AVX state)?
>
>  -- Keir
>
> On 09/11/2010 02:31, "Haitao Shan"
<maillists.shan@gmail.com> wrote:
>
>> Hi, Keir,
>>
>> This patch adds back the reverted CPUID support for XSAVE feature. Can
>> you have a review?
>>
>> Signed-off-by: Shan Haitao <haitao.shan@intel.com>
>>
>> Shan Haitao
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 09/11/2010 09:02, "Haitao Shan" <maillists.shan@gmail.com>
wrote:
> I remember cpuid exposure is covered by hvm_cpuid(), which is done by
> former (long ago) patches.
> Probably we may clean that a bit? So that both PV and HVM have the
> same mechanism.
Well they do have the same mechanism -- they both use domain_cpuid() in the
hypervisor, which is set up by the toolstack via functions in
xc_cpuid_x86.c.
> I have considered AVX exposure. My concern is: if AVX is considered to
> be relying on XSAVE(of course, it is the truth), will we need to add
> future features one by one since they are likely all dependent on
> XSAVE.
For HVM guests, we already add features one by one. It''s quite reckless
to
let guests see unknown future features by default. It''s pretty bad we
do
that for PV guests, really.
> Or could we just let it be, since even they see it, they can not use it,
> anyway.
So, HVM guests sees AVX, tries to use it, and what happens...? Does it
decide not to because it doesn''t see XSAVE feature? Note that
currently,
without manual domain config hacking, an HVM guest can *never* see XSAVE, so
automatically passing through the AVX feature seems a bit pointless!

You might be getting a bit confused by the fundamental policy difference we
now have between PV and HVM guest CPUIDs. By default, we pass through all
CPU features to PV guests (masking out only the ones known bad), while we
mask all CPU features to HVM guests (passing thru only the ones known good).
It''s like whitelisting versus blacklisting.

 -- Keir
> Shan Haitao
> 
> 2010/11/9 Keir Fraser <keir@xen.org>:
>> Thanks, this looks better. I have some further questions however.
>> 
>> You appear to only be exposing XSAVE to PV guests -- Why is that?
>> 
>> Also, the patch passes the AVX feature unconditionally through to HVM
guests
>> -- firstly, why only visible HVM guests; and secondly, should it not be
>> conditional on XSAVE support in the hypervisor (don''t we need
XSAVE to be
>> able to save/restore AVX state)?
>> 
>>  -- Keir
>> 
>> On 09/11/2010 02:31, "Haitao Shan"
<maillists.shan@gmail.com> wrote:
>> 
>>> Hi, Keir,
>>> 
>>> This patch adds back the reverted CPUID support for XSAVE feature.
Can
>>> you have a review?
>>> 
>>> Signed-off-by: Shan Haitao <haitao.shan@intel.com>
>>> 
>>> Shan Haitao
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xensource.com
>>> http://lists.xensource.com/xen-devel
>> 
>> 
>> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel