| Message ID | 20240520143359.3376849-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Mon, May 20, 2024 at 03:33:59PM +0100, Andrew Cooper wrote: > From: Roger Pau Monné <roger.pau@citrix.com> > > 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@citrix.com> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Just one question below. > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > > 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. Thanks, Roger.
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@citrix.com> >> >> 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@citrix.com> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks. > > Just one question below. > >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> CC: Roger Pau Monné <roger.pau@citrix.com> >> >> 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)' You should always pass % a tuple, even a 1-element tuple, so it does the right thing when you're not sure of the type of the thing being printed. ~Andrew
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@citrix.com> > >> > >> 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@citrix.com> > >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > > Thanks. > > > > > Just one question below. > > > >> --- > >> CC: Jan Beulich <JBeulich@suse.com> > >> CC: Roger Pau Monné <roger.pau@citrix.com> > >> > >> 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.
On 20/05/2024 5:29 pm, Roger Pau Monné wrote: > 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@citrix.com> >>>> >>>> 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@citrix.com> >>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >>> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> >> Thanks. >> >>> Just one question below. >>> >>>> --- >>>> CC: Jan Beulich <JBeulich@suse.com> >>>> CC: Roger Pau Monné <roger.pau@citrix.com> >>>> >>>> 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? (a) is just a set of brackets around an expression. (a, ) is a 1-element tuple containing a as it's only element. ~Andrew
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, ) + ) + + state.output.write( +"""} + """) for idx, text in enumerate(state.bitfields):