Re: [PATCH RFC 12/31] tools: Utility for dealing with featuresets

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

On 05/01/16 17:37, Ian Campbell wrote:
> On Tue, 2016-01-05 at 17:13 +0000, Andrew Cooper wrote:
>> They are: SHARED_1d, known_features[], inverted_features[],
>> {pv,hvm_shadow,hvm_hap}_featuremask[], struct tagged_featureset,
>> deep_deps[], nr_deep_deps, deep_dep_features[] and lookup_deep_deps().
> Which ones of these are expected to be consumed by applications/utilities
> using libxc as well as shared between Xen and libxc?

libxc uses all the deep_deps, but only as an internal implementation
detail of xc_cpuid_apply_policy()

I do not expect anything from cpuid-private.h to be exposed in the libxc
API, although by the looks of it, the symbols are actually present in
libxenctrl.so

>
> You mention *_featuremask below.
>
>>>>> Granted there's lots of that sort of thing already, but should we
>>>>> really be
>>>>> making it worse?
>>>>>
>>>>> libelf avoids this by namespacing itself as a quasi-standalone
>>>>> library
>>>>> in
>>>>> both the tools and hypervisor contexts.
>>>> Nothing from the shared .c files is expected to be exposed in the
>>>> API. 
>>>> The guts of xc_cpuid_policy() end up using it, but that is an
>>>> implementation detail.
>>> Ah, so is the presence of the vpath stuff in libxc here spurious then?
>> No - this utility needs the generated data (specifically the static
>> *_featuremask[] bitmaps), so the vpath is needed.
> OK, so those which are used by this tool are therefore being added to the
> set of exported libxenctrl symbols and are not namespaced correctly.
>
> Also, it seems wrong for something in tools/misc to be delving into
> xen/blah and including "cpuid-private.h" (or if it does then it should
> build it's own cpuid.o, but I don't like that either). This is akin to the
> various inclusions of x?_private.h which we are trying to remove.
>
> If these symbols are to be consumed outside of libxc and Xen by apps
> linking against libxc then they need to be prototyped in xenctrl.h, I
> think, or the header needs to become public itself.
>
> How bad does providing xc_get_*_featuremask sound? How to cleanly expose
> the size of the various arrays consistently is the first annoying niggle
> which I thought of.

I will see whether I can make something like this happen.  It certainly
wouldn't be hard to get enough exposed in the libxc api for the debug
utility to be happy.

~Andrew