[03/10] x86/gen-cpuid: Rework internal logic to ease future changes
diff mbox series

Message ID 20200226202221.6555-4-andrew.cooper3@citrix.com
State New
Headers show
Series
  • x86: Default vs Max policies
Related show

Commit Message

Andrew Cooper Feb. 26, 2020, 8:22 p.m. UTC
Better split the logic between parse/calculate/write.  Collect the feature
comment by their comment character, and perform the accumulation operations in
crunch_numbers().

Avoid rendering the featuresets to C uint32_t's in crunch_numbers(), and
instead do this in write_results().  Update format_uint32s() to call
featureset_to_uint32s() internally.

No functional change - the generated cpuid-autogen.h is identical.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/tools/gen-cpuid.py | 77 +++++++++++++++++++-------------------------------
 1 file changed, 29 insertions(+), 48 deletions(-)

Comments

Jan Beulich Feb. 27, 2020, 7:57 a.m. UTC | #1
On 26.02.2020 21:22, Andrew Cooper wrote:
> Better split the logic between parse/calculate/write.  Collect the feature
> comment by their comment character, and perform the accumulation operations in
> crunch_numbers().

Would you mind saying "character(s)" here, as there are items with
multiple of them?

> Avoid rendering the featuresets to C uint32_t's in crunch_numbers(), and
> instead do this in write_results().  Update format_uint32s() to call
> featureset_to_uint32s() internally.
> 
> No functional change - the generated cpuid-autogen.h is identical.

I notice the "special" field (or however such is called in Python)
goes away, in favor of using raw['!'] at the apparently sole
consuming site. I also notice the same isn't true for "pv", which
now could also be raw['A'] as it seems. If this is the case (i.e.
I'm not overlooking anything), could you say a word on the change
for "special" and/or the difference between "special" and "pv"?

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

With my limited Python skills merely
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan
Andrew Cooper Feb. 27, 2020, 10:08 a.m. UTC | #2
On 27/02/2020 07:57, Jan Beulich wrote:
> On 26.02.2020 21:22, Andrew Cooper wrote:
>> Better split the logic between parse/calculate/write.  Collect the feature
>> comment by their comment character, and perform the accumulation operations in
>> crunch_numbers().
> Would you mind saying "character(s)" here, as there are items with
> multiple of them?

Ok.

>
>> Avoid rendering the featuresets to C uint32_t's in crunch_numbers(), and
>> instead do this in write_results().  Update format_uint32s() to call
>> featureset_to_uint32s() internally.
>>
>> No functional change - the generated cpuid-autogen.h is identical.
> I notice the "special" field (or however such is called in Python)
> goes away, in favor of using raw['!'] at the apparently sole
> consuming site. I also notice the same isn't true for "pv", which
> now could also be raw['A'] as it seems. If this is the case (i.e.
> I'm not overlooking anything), could you say a word on the change
> for "special" and/or the difference between "special" and "pv"?

There is no point copying data just for the sake of copying data.

While we could drop state.pv (pv_def by the end of the series), that is
the only set it would be true for, and dropping it does interfere with
the derivation of raw_shadow (raw_shadow_def by the end of the series).

~Andrew

Patch
diff mbox series

diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
index 71ea78f4eb..99b2e7aeee 100755
--- a/xen/tools/gen-cpuid.py
+++ b/xen/tools/gen-cpuid.py
@@ -20,20 +20,21 @@  def __init__(self, input, output):
         # State parsed from input
         self.names = {}  # Value => Name mapping
         self.values = {} # Name => Value mapping
-        self.raw_special = set()
-        self.raw_pv = set()
-        self.raw_hvm_shadow = set()
-        self.raw_hvm_hap = set()
+        self.raw = {
+            '!': set(),
+            'A': set(), 'S': set(), 'H': set(),
+        }
 
         # State calculated
         self.nr_entries = 0 # Number of words in a featureset
         self.common_1d = 0 # Common features between 1d and e1d
-        self.known = [] # All known features
-        self.special = [] # Features with special semantics
-        self.pv = []
-        self.hvm_shadow = []
-        self.hvm_hap = []
+        self.pv = set() # PV features
+        self.hvm_shadow = set() # HVM shadow features
+        self.hvm_hap = set() # HVM HAP features
         self.bitfields = [] # Text to declare named bitfields in C
+        self.deep_deps = {} # { feature num => dependant features }
+        self.nr_deep_deps = 0 # Number of entries in deep_deps
+        self.deep_features = set() # featureset of keys in deep_deps
 
 def parse_definitions(state):
     """
@@ -81,20 +82,9 @@  def parse_definitions(state):
         state.values[name.lower().replace("_", "-")] = val
 
         for a in attr:
-
-            if a == "!":
-                state.raw_special.add(val)
-            elif a in "ASH":
-                if a == "A":
-                    state.raw_pv.add(val)
-                    state.raw_hvm_shadow.add(val)
-                    state.raw_hvm_hap.add(val)
-                elif attr == "S":
-                    state.raw_hvm_shadow.add(val)
-                    state.raw_hvm_hap.add(val)
-                elif attr == "H":
-                    state.raw_hvm_hap.add(val)
-            else:
+            try:
+                state.raw[a].add(val)
+            except KeyError:
                 raise Fail("Unrecognised attribute '%s' for %s" % (a, name))
 
     if len(state.names) == 0:
@@ -117,10 +107,11 @@  def featureset_to_uint32s(fs, nr):
     if len(words) < nr:
         words.extend([0] * (nr - len(words)))
 
-    return [ "0x%08xU" % x for x in words ]
+    return ("0x%08xU" % x for x in words)
 
-def format_uint32s(words, indent):
+def format_uint32s(state, featureset, indent):
     """ Format a list of uint32_t's suitable for a macro definition """
+    words = featureset_to_uint32s(featureset, state.nr_entries)
     spaces = " " * indent
     return spaces + (", \\\n" + spaces).join(words) + ", \\"
 
@@ -133,13 +124,11 @@  def crunch_numbers(state):
     # Features common between 1d and e1d.
     common_1d = (FPU, VME, DE, PSE, TSC, MSR, PAE, MCE, CX8, APIC,
                  MTRR, PGE, MCA, CMOV, PAT, PSE36, MMX, FXSR)
+    state.common_1d = common_1d
 
-    state.known = featureset_to_uint32s(state.names.keys(), nr_entries)
-    state.common_1d = featureset_to_uint32s(common_1d, 1)[0]
-    state.special = featureset_to_uint32s(state.raw_special, nr_entries)
-    state.pv = featureset_to_uint32s(state.raw_pv, nr_entries)
-    state.hvm_shadow = featureset_to_uint32s(state.raw_hvm_shadow, nr_entries)
-    state.hvm_hap = featureset_to_uint32s(state.raw_hvm_hap, nr_entries)
+    state.pv = state.raw['A']
+    state.hvm_shadow = state.pv | state.raw['S']
+    state.hvm_hap = state.hvm_shadow | state.raw['H']
 
     #
     # Feature dependency information.
@@ -317,17 +306,9 @@  def crunch_numbers(state):
 
         state.deep_deps[feat] = seen[1:]
 
-    state.deep_features = featureset_to_uint32s(deps.keys(), nr_entries)
+    state.deep_features = deps.keys()
     state.nr_deep_deps = len(state.deep_deps.keys())
 
-    try:
-        _tmp = state.deep_deps.iteritems()
-    except AttributeError:
-        _tmp = state.deep_deps.items()
-
-    for k, v in _tmp:
-        state.deep_deps[k] = featureset_to_uint32s(v, nr_entries)
-
     # Calculate the bitfield name declarations
     for word in range(nr_entries):
 
@@ -382,21 +363,21 @@  def write_results(state):
 
 #define INIT_DEEP_DEPS { \\
 """ % (state.nr_entries,
-       state.common_1d,
-       format_uint32s(state.known, 4),
-       format_uint32s(state.special, 4),
-       format_uint32s(state.pv, 4),
-       format_uint32s(state.hvm_shadow, 4),
-       format_uint32s(state.hvm_hap, 4),
+       next(featureset_to_uint32s(state.common_1d, 1)),
+       format_uint32s(state, state.names.keys(), 4),
+       format_uint32s(state, state.raw['!'], 4),
+       format_uint32s(state, state.pv, 4),
+       format_uint32s(state, state.hvm_shadow, 4),
+       format_uint32s(state, state.hvm_hap, 4),
        state.nr_deep_deps,
-       format_uint32s(state.deep_features, 4),
+       format_uint32s(state, state.deep_features, 4),
        ))
 
     for dep in sorted(state.deep_deps.keys()):
         state.output.write(
             "    { %#xU, /* %s */ { \\\n%s\n    }, }, \\\n"
             % (dep, state.names[dep],
-               format_uint32s(state.deep_deps[dep], 8)
+               format_uint32s(state, state.deep_deps[dep], 8)
            ))
 
     state.output.write(