[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC 15/31] x86: Generate deep dependencies of x86 features
On 05/01/16 16:03, Ian Campbell wrote: > On Wed, 2015-12-16 at 21:24 +0000, Andrew Cooper wrote: >> Some features depend on other features. Working out and maintaining the >> exact >> dependency tree is complicated, so it is expressed in script form instead. >> >> `gen-feature-deps.py` parses 'xen/include/public/arch-x86/featureset.h' (To >> obtain some literal names conforming to the API), contains some single-step >> dependency information, performs some number crunching, and writes autogen.c >> to make the results of the number crunching available. > In libxl we prepend a _ to autogenerated file names, may as well do the > same here I guess. > >> In this case, it writes out deep dependency infomarion, to allow featureset > "information" >> code to find all eventual features cleared in a dependency chain. >> >> To be able to compile for userspace, libxc's bitmap macros are made more >> generic (to match Xen's), and accept a void * instead of unsigned long *. > Can we do this in a separate patch please. Will do (for all points) > > IIRC void * arithmetic is a gcc (and presumably clang) extension, which we > can specify for Xen itself, but I'm not sure about libxc (cf: recent > discussions about building stuff for Windows). It isn't void* arithmetic. Simply taking a void * to facilitate different underlying types for a bitmap. > > What actually breaks with the existing definitions? Xen uses unsigned int[] for bitmaps, while libxc uses unsigned long[]. The compiler objects to the mix of pointers to different sized. For a bitmap however it really doesn't matter. At the end of the pointer is a contiguous block of bits. > >> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> >> --- >> CC: Jan Beulich <JBeulich@xxxxxxxx> >> CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx> >> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> >> CC: Wei Liu <wei.liu2@xxxxxxxxxx> >> >> TODO: The generation of autogen.c now means that libxc needs to be compiled >> after the hypervisor, as the vpath doesn't convey the generation information. >> I need to find a way to fix this. > I'd be inclined to do the autogeneration twice with the output being > outside the shared directory or gaining a (tools|xen)_ prefix. > > It's a bit wasteful to do it twice but anything else would add an > undesirable linkage between tools and hypervisor build I think. I'm certain > we don't want to commit the generated files. I will see how much of this I can get into the automatically generated part. The problem is that there is a header file and secondary .c file which are not automatically generated, but needed in both places. I definitely do not want to duplicate those. > >> diff --git a/xen/arch/x86/cpuid/cpuid-private.h >> b/xen/arch/x86/cpuid/cpuid-private.h >> index 1c92ee4..438f5d2 100644 >> --- a/xen/arch/x86/cpuid/cpuid-private.h >> +++ b/xen/arch/x86/cpuid/cpuid-private.h >> @@ -64,6 +64,24 @@ extern const uint32_t >> pv_featuremask[XEN_NR_FEATURESET_ENTRIES]; >> extern const uint32_t hvm_shadow_featuremask[XEN_NR_FEATURESET_ENTRIES]; >> extern const uint32_t hvm_hap_featuremask[XEN_NR_FEATURESET_ENTRIES]; >> >> +/* A featureset with a tag. */ >> +struct tagged_featureset >> +{ >> + uint32_t tag; >> + uint32_t fs[XEN_NR_FEATURESET_ENTRIES]; >> +}; >> + >> +/* Sparse feature matrix identifying all features which depend on a >> feature. */ >> +extern const struct tagged_featureset deep_deps[]; > This'll be XEN_NR_FEATURESET_ENTRIES^2 entries? How big is that getting > OOI? It is a sparse matrix, not an n^2 matrix. It only has entries for features which sit in a dependency tree, which is vastly fewer than all known features. Currently, it is an array of 9 entries. > >> diff --git a/xen/arch/x86/cpuid/gen-feature-deps.py >> b/xen/arch/x86/cpuid/gen-feature-deps.py >> new file mode 100755 >> index 0000000..f0ecbba >> --- /dev/null >> +++ b/xen/arch/x86/cpuid/gen-feature-deps.py >> @@ -0,0 +1,152 @@ >> +#!/usr/bin/env python >> +import sys, os, os.path as path, re >> + >> +names = {} >> + >> +with open(path.join(os.environ["XEN_ROOT"], >> + "xen/include/public/arch-x86/featureset.h")) as f: > I'd weakly prefer all the input and output paths to come from the command > line. > > I have a feeling you are planning to redo a bunch of this (or if not then > Jan was requesting something more structured which may or may not cause big > changes here), so I'll leave the actual review for now. Yeah - there have been a number of suggestions, so v2 is going to look very different. > >> + >> + # Expected duplicate symbols. Discard >> + if name in ('3DNOW_ALT', ): >> + continue > OOI how does this arise? Bug in older AMD processors. See the first and final hunks of c/s d8ba3a9 ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |