Re: [PATCH v2 05/30] xen/public: Export cpu featureset information in the public API

[Joao Martins <joao.m.martins@oracle.com>]

On 02/19/2016 05:55 PM, Andrew Cooper wrote:
> On 19/02/16 17:29, Joao Martins wrote:
>> On 02/05/2016 01:41 PM, Andrew Cooper wrote:
>>> For the featureset to be a useful object, it needs a stable interpretation, a
>>> property which is missing from the current hw_caps interface.
>>>
>>> Additionly, introduce TSC_ADJUST, SHA, PREFETCHWT1, ITSC, EFRO and CLZERO
>>> which will be used by later changes.
>>>
>>> To maintain compilation, FSCAPINTS is currently hardcoded at 9.  Future
>>> changes will change this to being dynamically generated.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Hey Andrew!
>>
>> There are a few word motions in this patch:
> 
> Indeed there are.  They are in aid of getting a new clean interface.
> 
>>
>> [current]   [this series]
>> word 0   ->  word 0
>> word 4   ->  word 1
>> word 1,6 ->  word 2,3
>> word 2   ->  word 4
>> word 7,8 ->  word 5,6
>>          ->  word 7   (new leaf not previously described)
>>          ->  word 8   (new leaf not previously described)
>> word 3   ->  word 9   (linux defined mapping)
>>
>> Since you're proposing the stabilization of physinfo.hw_caps
> 
> Stabilising of physinfo.hw_caps is a side effect, but it has shifted
> words in the past (c/s 4f4eec3, 6c421a1, 9c907c6).  It is not a stable
> interface from Xen, and cannot be relied upon.
> 
> It has also never had a published ABI.
> 
Thanks for the clarification! I thought that it was sort of a stable API because
it was exposed through libxl, but I got the wrong idea entirely.

>>  and given that this
>> is exposed on both sysctl and libxl (through libxl_hwcap) shouldn't its size
>> match the real one (boot_cpu_data.x86_capability) i.e. NCAPINTS ? Additionally I
>> see that libxl_hwcap is also hardcoded to 8 alongside struct xen_sysctl_physinfo
>> when it should be 10 ?
> 
> Hardcoding of the size in sysctl can be worked around. Fixing libxl is
> harder.
> 
> The synthetic leaves are internal and should not be exposed.
> 
>> libxl users could potentially make use of this hwcap field to see what features
>> the host CPU supports.
> 
> The purpose of the new featureset interface is to have stable object
> which can be used by higher level toolstacks.
> 
> This is done by pretending that hw_caps never existed, and replacing it
> wholesale with a bitmap, (specified as variable length and safe to
> zero-extend), with an ABI in the public header files detailing what each
> bit means.

Given that you introduce a new API for libxc (xc_get_cpu_featureset()) perhaps
an equivalent to libxl could also be added? That wat users of libxl could also
query about the host and guests supported features. I would be happy to produce
patches towards that.

Joao

> 
> ~Andrew
>