[PATCH 1/4] x86/gen-cpuid: Minor cleanup

Andrew Cooper posted 4 patches 6 months, 2 weeks ago
[PATCH 1/4] x86/gen-cpuid: Minor cleanup
Posted by Andrew Cooper 6 months, 2 weeks ago
Rename INIT_FEATURE_NAMES to INIT_FEATURE_NAME_TO_VAL as we're about to gain a
inverse mapping of the same thing.

Use dict.items() unconditionally.  iteritems() is a marginal perf optimsiation
for Python2 only, and simply not worth the effort on a script this small.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>

v3:
 * New
---
 tools/libs/light/libxl_cpuid.c | 2 +-
 xen/arch/x86/cpu-policy.c      | 2 +-
 xen/tools/gen-cpuid.py         | 9 ++-------
 3 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/tools/libs/light/libxl_cpuid.c b/tools/libs/light/libxl_cpuid.c
index ce4f3c7095ba..063fe86eb72f 100644
--- a/tools/libs/light/libxl_cpuid.c
+++ b/tools/libs/light/libxl_cpuid.c
@@ -296,7 +296,7 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list *policy, const char* str)
 
         {NULL, 0, NA, CPUID_REG_INV, 0, 0}
     };
-    static const struct feature_name features[] = INIT_FEATURE_NAMES;
+    static const struct feature_name features[] = INIT_FEATURE_NAME_TO_VAL;
     /*
      * NB: if we switch to using a cpu_policy derived object instead of a
      * libxl_cpuid_policy_list we could get rid of the featureset -> cpuid leaf
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 99871b8e0e05..b96f4ee55cc4 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -43,7 +43,7 @@ static const uint32_t deep_features[] = INIT_DEEP_FEATURES;
 static const struct feature_name {
     const char *name;
     unsigned int bit;
-} feature_names[] __initconstrel = INIT_FEATURE_NAMES;
+} feature_names[] __initconstrel = INIT_FEATURE_NAME_TO_VAL;
 
 /*
  * Parse a list of cpuid feature names -> bool, calling the callback for any
diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
index 380b9d973a67..79d7f5c8e1c9 100755
--- a/xen/tools/gen-cpuid.py
+++ b/xen/tools/gen-cpuid.py
@@ -459,15 +459,10 @@ def write_results(state):
     state.output.write(
 """}
 
-#define INIT_FEATURE_NAMES { \\
+#define INIT_FEATURE_NAME_TO_VAL { \\
 """)
 
-    try:
-        _tmp = state.values.iteritems()
-    except AttributeError:
-        _tmp = state.values.items()
-
-    for name, bit in sorted(_tmp):
+    for name, bit in sorted(state.values.items()):
         state.output.write(
             '    { "%s", %sU },\\\n' % (name, bit)
             )
-- 
2.30.2


Re: [PATCH 1/4] x86/gen-cpuid: Minor cleanup
Posted by Roger Pau Monné 6 months, 2 weeks ago
On Fri, May 10, 2024 at 11:39:59PM +0100, Andrew Cooper wrote:
> Rename INIT_FEATURE_NAMES to INIT_FEATURE_NAME_TO_VAL as we're about to gain a
> inverse mapping of the same thing.
> 
> Use dict.items() unconditionally.  iteritems() is a marginal perf optimsiation
> for Python2 only, and simply not worth the effort on a script this small.

My understanding is that what used to be iteritems() in Python 2 is
the behavior of items() in Python 3 (return a generator instead of a
copy of the dictionary list).

> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

Re: [PATCH 1/4] x86/gen-cpuid: Minor cleanup
Posted by Andrew Cooper 6 months, 1 week ago
On 14/05/2024 8:14 am, Roger Pau Monné wrote:
> On Fri, May 10, 2024 at 11:39:59PM +0100, Andrew Cooper wrote:
>> Rename INIT_FEATURE_NAMES to INIT_FEATURE_NAME_TO_VAL as we're about to gain a
>> inverse mapping of the same thing.
>>
>> Use dict.items() unconditionally.  iteritems() is a marginal perf optimsiation
>> for Python2 only, and simply not worth the effort on a script this small.
> My understanding is that what used to be iteritems() in Python 2 is
> the behavior of items() in Python 3 (return a generator instead of a
> copy of the dictionary list).

Yes-ish.  They're actually now view() objects following the official
stabilisation of the internal format, which are more-efficient-still in
the common case.

>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

~Andrew