Re: [Xen-devel] [Patch] Adding back CPUID support for Xsave

[Keir Fraser <keir@xxxxxxx>]

On 09/11/2010 09:02, "Haitao Shan" <maillists.shan@xxxxxxxxx> 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@xxxxxxx>:
>> 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@xxxxxxxxx> 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@xxxxxxxxx>
>>> 
>>> Shan Haitao
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>>> http://lists.xensource.com/xen-devel
>> 
>> 
>> 



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