|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3.5 3/4] tools/xen-cpuid: Use automatically generated feature names
On Mon, May 20, 2024 at 04:20:37PM +0100, Andrew Cooper wrote:
> On 20/05/2024 4:07 pm, Roger Pau Monné wrote:
> > On Mon, May 20, 2024 at 03:33:59PM +0100, Andrew Cooper wrote:
> >> From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >>
> >> Have gen-cpuid.py write out INIT_FEATURE_VAL_TO_NAME, derived from the same
> >> data source as INIT_FEATURE_NAME_TO_VAL, although both aliases of common_1d
> >> are needed.
> >>
> >> In xen-cpuid.c, sanity check at build time that leaf_info[] and
> >> feature_names[] are of sensible length.
> >>
> >> As dump_leaf() rendered missing names as numbers, always dump leaves even
> >> if
> >> we don't have the leaf name. This conversion was argumably missed in
> >> commit
> >> 59afdb8a81d6 ("tools/misc: Tweak reserved bit handling for xen-cpuid").
> >>
> >> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>
> Thanks.
>
> >
> > Just one question below.
> >
> >> ---
> >> CC: Jan Beulich <JBeulich@xxxxxxxx>
> >> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >>
> >> Differences in names are:
> >>
> >> sysenter -> sep
> >> tm -> tm1
> >> ds-cpl -> dscpl
> >> est -> eist
> >> sse41 -> sse4-1
> >> sse42 -> sse4-2
> >> movebe -> movbe
> >> tsc-dl -> tsc-deadline
> >> rdrnd -> rdrand
> >> hyper -> hypervisor
> >> mmx+ -> mmext
> >> fxsr+ -> ffxsr
> >> pg1g -> page1gb
> >> 3dnow+ -> 3dnowext
> >> cmp -> cmp-legacy
> >> cr8d -> cr8-legacy
> >> lzcnt -> abm
> >> msse -> misalignsse
> >> 3dnowpf -> 3dnowprefetch
> >> nodeid -> nodeid-msr
> >> dbx -> dbext
> >> tsc-adj -> tsc-adjust
> >> fdp-exn -> fdp-excp-only
> >> deffp -> no-fpu-sel
> >> <24> -> bld
> >> ppin -> amd-ppin
> >> lfence+ -> lfence-dispatch
> >> ppin -> intel-ppin
> >> energy-ctrl -> energy-filtering
> >>
> >> Apparently BLD missed the update to xen-cpuid.c. It appears to be the only
> >> one. Several of the + names would be nice to keep as were, but doing so
> >> isn't
> >> nice in gen-cpuid. Any changes would alter the {dom0-}cpuid= cmdline
> >> options,
> >> but we intentionally don't list them, so I'm not worried.
> >>
> >> Thoughts?
> >>
> >> v3:
> >> * Rework somewhat.
> >> * Insert aliases of common_1d.
> >>
> >> v4:
> >> * Pad at the gen stage. I don't like this, but I'm clearly outvoted on
> >> the matter.
> >> ---
> >> tools/misc/xen-cpuid.c | 16 ++++++++--------
> >> xen/tools/gen-cpuid.py | 29 +++++++++++++++++++++++++++++
> >> 2 files changed, 37 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
> >> index 6ee835b22949..51009683da1b 100644
> >> --- a/tools/misc/xen-cpuid.c
> >> +++ b/tools/misc/xen-cpuid.c
> >> @@ -11,6 +11,7 @@
> >> #include <xenguest.h>
> >>
> >> #include <xen-tools/common-macros.h>
> >> +#include <xen/lib/x86/cpuid-autogen.h>
> >>
> >> static uint32_t nr_features;
> >>
> >> @@ -291,6 +292,8 @@ static const struct {
> >>
> >> #define COL_ALIGN "24"
> >>
> >> +static const char *const feature_names[] = INIT_FEATURE_VAL_TO_NAME;
> >> +
> >> static const char *const fs_names[] = {
> >> [XEN_SYSCTL_cpu_featureset_raw] = "Raw",
> >> [XEN_SYSCTL_cpu_featureset_host] = "Host",
> >> @@ -304,12 +307,6 @@ static void dump_leaf(uint32_t leaf, const char
> >> *const *strs)
> >> {
> >> unsigned i;
> >>
> >> - if ( !strs )
> >> - {
> >> - printf(" ???");
> >> - return;
> >> - }
> >> -
> >> for ( i = 0; i < 32; ++i )
> >> if ( leaf & (1u << i) )
> >> {
> >> @@ -327,6 +324,10 @@ static void decode_featureset(const uint32_t
> >> *features,
> >> {
> >> unsigned int i;
> >>
> >> + /* If this trips, you probably need to extend leaf_info[] above. */
> >> + BUILD_BUG_ON(ARRAY_SIZE(leaf_info) != FEATURESET_NR_ENTRIES);
> >> + BUILD_BUG_ON(ARRAY_SIZE(feature_names) != FEATURESET_NR_ENTRIES * 32);
> >> +
> >> printf("%-"COL_ALIGN"s ", name);
> >> for ( i = 0; i < length; ++i )
> >> printf("%08x%c", features[i],
> >> @@ -338,8 +339,7 @@ static void decode_featureset(const uint32_t *features,
> >> for ( i = 0; i < length && i < ARRAY_SIZE(leaf_info); ++i )
> >> {
> >> printf(" [%02u] %-"COL_ALIGN"s", i, leaf_info[i].name ?:
> >> "<UNKNOWN>");
> >> - if ( leaf_info[i].name )
> >> - dump_leaf(features[i], leaf_info[i].strs);
> >> + dump_leaf(features[i], &feature_names[i * 32]);
> >> printf("\n");
> >> }
> >> }
> >> diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
> >> index 79d7f5c8e1c9..601eec608983 100755
> >> --- a/xen/tools/gen-cpuid.py
> >> +++ b/xen/tools/gen-cpuid.py
> >> @@ -470,6 +470,35 @@ def write_results(state):
> >> state.output.write(
> >> """}
> >>
> >> +""")
> >> +
> >> + state.output.write(
> >> +"""
> >> +#define INIT_FEATURE_VAL_TO_NAME { \\
> >> +""")
> >> +
> >> + for name, bit in sorted(state.values.items()):
> >> + state.output.write(
> >> + ' [%s] = "%s",\\\n' % (bit, name)
> >> + )
> >> +
> >> + # Add the other alias for 1d/e1d common bits. 64 is the
> >> difference
> >> + # between 1d and e1d.
> >> + if bit in state.common_1d:
> >> + state.output.write(
> >> + ' [%s] = "%s",\\\n' % (64 + bit, name)
> >> + )
> >> +
> >> + # Pad to an exact multiple of FEATURESET_SIZE if necessary
> >> + pad_feat = state.nr_entries * 32 - 1
> >> + if not state.names.get(pad_feat):
> >> + state.output.write(
> >> + ' [%s] = NULL,\\\n' % (pad_feat, )
> > One likely stupid question, but since my understanding of Python is
> > very limited, why do you add the comma after pad_feat? There's no
> > other parameter to print.
>
> It's a common python gotcha with %.
>
> >>> a = (1, 2)
> >>> "%s" % a
> Traceback (most recent call last):
> File "<stdin>", line 1, in <module>
> TypeError: not all arguments converted during string formatting
> >>> "%s" % (a, )
> '(1, 2)'
Right, but just using:
>>> "%s" % (a)
Should still be fine?
Thanks, Roger.
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |