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

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

On 05/01/16 15:17, Ian Campbell wrote:
> On Wed, 2015-12-16 at 21:24 +0000, Andrew Cooper wrote:
>> It is able to reports the current featuresets; both the static masks and
>> dynamic featuresets from Xen, or to decode an arbitrary featureset into
>> `/proc/cpuinfo` style strings.
> More than adding a utility does this not also arrange for a whole bunch of
> new functionality to be compiled into libxc? That's worth mentioning here.

It is almost all just data.  Only a single (trivial) function at
present.  But yes - it is worth mentioning.

>
> And doesn't it do so in a non-library namespaced way (e.g.
> with calculate_featuresets() and decode_featureset() being exposed by the
> library)?

calculate_featuresets() is hypervisor-only code and not included by this
change, and decode_featureset() is a local function to the utility.

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

I was also looking to avoid extraneous namespacing in Xen.  I am open to
suggestions here, but the only way to get at the definitions is via
cpuid-private.h

>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> ---
>>  .gitignore                         |   1 +
>>  tools/libxc/Makefile               |   6 +
>>  tools/misc/Makefile                |   6 +
>>  tools/misc/xen-cpuid.c             | 392
>> +++++++++++++++++++++++++++++++++++++
>>  xen/arch/x86/cpuid/cpuid-private.h |   9 +-
>>  5 files changed, 413 insertions(+), 1 deletion(-)
>>  create mode 100644 tools/misc/xen-cpuid.c
>>
>> diff --git a/.gitignore b/.gitignore
>> index 9ead7c4..63944b5 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -167,6 +167,7 @@ tools/misc/cpuperf/cpuperf-perfcntr
>>  tools/misc/cpuperf/cpuperf-xen
>>  tools/misc/xc_shadow
>>  tools/misc/xen_cpuperf
>> +tools/misc/xen-cpuid
>>  tools/misc/xen-detect
>>  tools/misc/xen-tmem-list-parse
>>  tools/misc/xenperf
>> diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile
>> index a0f899b..83547e1 100644
>> --- a/tools/libxc/Makefile
>> +++ b/tools/libxc/Makefile
>> @@ -79,6 +79,12 @@ GUEST_SRCS-y += $(ELF_SRCS-y)
>>  $(patsubst %.c,%.o,$(ELF_SRCS-y)): CFLAGS += -Wno-pointer-sign
>>  $(patsubst %.c,%.opic,$(ELF_SRCS-y)): CFLAGS += -Wno-pointer-sign
>>  
>> +ifeq ($(CONFIG_X86),y)
>> +vpath %.c ../../xen/arch/x86/cpuid
>> +CFLAGS += -I../../xen/arch/x86/cpuid
>> +CTRL_SRCS-y += cpuid.c
>> +endif
>> +
>>  # new domain builder
>>  GUEST_SRCS-y                 += xc_dom_core.c xc_dom_boot.c
>>  GUEST_SRCS-y                 += xc_dom_elfloader.c
>> diff --git a/tools/misc/Makefile b/tools/misc/Makefile
>> index c4490f3..eb7649d 100644
>> --- a/tools/misc/Makefile
>> +++ b/tools/misc/Makefile
>> @@ -9,6 +9,7 @@ CFLAGS += $(CFLAGS_xeninclude)
>>  CFLAGS += $(CFLAGS_libxenstore)
>>  
>>  # Everything to be installed in regular bin/
>> +INSTALL_BIN-$(CONFIG_X86)      += xen-cpuid
>>  INSTALL_BIN-$(CONFIG_X86)      += xen-detect
>>  INSTALL_BIN                    += xencons
>>  INSTALL_BIN                    += xencov_split
>> @@ -67,6 +68,11 @@ clean:
>>  .PHONY: distclean
>>  distclean: clean
>>  
>> +xen-cpuid.o: CFLAGS += -I$(XEN_ROOT)/xen/arch/x86/cpuid
>> +xen-cpuid.o: CFLAGS += -I$(XEN_ROOT)/tools/libxc
>> +xen-cpuid: xen-cpuid.o
>> +	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
>> +
>>  xen-hvmctx: xen-hvmctx.o
>>  	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
>>  
>> diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
>> new file mode 100644
>> index 0000000..e0cd6bb
>> --- /dev/null
>> +++ b/tools/misc/xen-cpuid.c
>> @@ -0,0 +1,392 @@
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <err.h>
>> +#include <getopt.h>
>> +#include <string.h>
>> +
>> +#include <xenctrl.h>
>> +#include "cpuid-private.h"
>> +
>> +static uint32_t nr_features = XEN_NR_FEATURESET_ENTRIES;
>> +
>> +static const char *str_1d[32] =
> If these are to be Xen's names could they come from the common cpuid
> location such that other users of these interfaces (including Xen itself)
> can use consistent names?

Currently there are no uses of these strings in the hypervisor.  The
reason they are here is to avoid linking the stringtables.

However, I can definitely see about getting them automatically generated
from a single source of information.

>
> (and then naturally be part of the autogeneration which has been discussed)
>> + option_error:
>> +            printf("Usage: %s [ info | detail | <featureset>* ]\n", argv[0]);
> What format does <featureset> take?

: or - delimited 32bit hex strings.

Some sample outputs look like:

[root@idol ~]# xen-cpuid
nr_features: 9
                      KEY 1d       1c       e1d      e1c      Da1     
7b0      7c0      e7d      e8b     

Static sets:
Known                    
ffeffbff:fffef7ff:efdbfbff:2469bfff:0000000f:21dcffbb:00000001:00000100:00000001
Inverted                 
00000000:00000000:00000000:00000000:00000000:00002000:00000000:00000000:00000000
PV Mask                  
07c9cbf5:fef83203:e3d9cbf5:042189e1:00000007:21cc0b39:00000001:00000000:00000000
HVM Shadow Mask          
07cbfbff:fffa3223:efdbfbff:04218df5:0000000f:21dc0fbb:00000001:00000000:00000000
HVM Hap Mask             
07cbfbff:fffa3223:efdbfbff:04218df5:0000000f:21dc0fbb:00000001:00000000:00000000

Dynamic sets:
Host                     
bfebfbff:0000e43d:20100800:00000001:00000000:00002000:00000000:00000000:00000000
PV                       
07c9cbf5:80002001:20100800:00000001:00000000:00000000:00000000:00000000:00000000
HVM                      
07cbfbff:80002021:20100800:00000001:00000000:00000000:00000000:00000000:00000000
Raw                      
bfebfbff:0000e43d:20100800:00000001:00000000:00002000:00000000:00000000:00000000
[root@idol ~]# xen-cpuid
07cbfbff:80002021:20100800:00000001:00000000:00000000:00000000:00000000:00000000
Raw                      
07cbfbff:80002021:20100800:00000001:00000000:00000000:00000000:00000000:00000000
  [00] 0x00000001.edx     fpu vme de pse tsc msr pae mce cx8 apic
sysenter mtrr pge mca cmov pat pse36 clflsh acpi mmx fxsr sse sse2
  [01] 0x00000001.ecx     sse3 vmx cx16 hyper
  [02] 0x80000001.edx     syscall nx lm
  [03] 0x80000001.ecx     lahf_lm
  [04] 0x0000000d:1.eax 
  [05] 0x00000007:0.ebx 
  [06] 0x00000007:0.ecx 
  [07] 0x80000007.edx   
  [08] 0x80000008.ebx   

~Andrew