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

[Andrew Cooper <andrew.cooper3@citrix.com>]

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.

>  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.

~Andrew