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.
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).
What actually breaks with the existing definitions?
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
> ---
> CC: Jan Beulich <jbeul...@suse.com>
> CC: Ian Campbell <ian.campb...@citrix.com>
> CC: Ian Jackson <ian.jack...@eu.citrix.com>
> CC: Wei Liu <wei.l...@citrix.com>
>
> 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.
> 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?
> 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.
> +
> + # Expected duplicate symbols. Discard
> + if name in ('3DNOW_ALT', ):
> + continue
OOI how does this arise?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
