diff mbox series

[3/3] x86/ucode: Drop MIS_UCODE and microcode_match_result

Message ID 20241108144252.315604-4-andrew.cooper3@citrix.com (mailing list archive)
State New
Headers show
Series x86/ucode: Untangle revision-relational logic | expand

Commit Message

Andrew Cooper Nov. 8, 2024, 2:42 p.m. UTC
All uses of MIS_UCODE, have been removed, leaving only a simple ordering
relation, and microcode_match_result being a stale name.

Drop the enum entirely, and use a simple int -1/0/1 scheme like other standard
ordering primitives in C.

Swap the order or parameters to compare_patch(), to reduce cognitive
complexity; all other logic operates the other way around.

No functional change.

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

I don't particular like keeping "result" as a variable name, but nothing
better comes to mind.
---
 xen/arch/x86/cpu/microcode/amd.c     | 10 ++++------
 xen/arch/x86/cpu/microcode/core.c    |  5 ++---
 xen/arch/x86/cpu/microcode/intel.c   |  9 ++++-----
 xen/arch/x86/cpu/microcode/private.h | 21 ++++++++++-----------
 4 files changed, 20 insertions(+), 25 deletions(-)
diff mbox series

Patch

diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 3861fec6565a..366c8c59e93a 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -170,8 +170,7 @@  static bool check_final_patch_levels(const struct cpu_signature *sig)
     return false;
 }
 
-static enum microcode_match_result compare_revisions(
-    uint32_t old_rev, uint32_t new_rev)
+static int compare_revisions(uint32_t old_rev, uint32_t new_rev)
 {
     if ( new_rev > old_rev )
         return NEW_UCODE;
@@ -199,8 +198,8 @@  static bool microcode_fits_cpu(const struct microcode_patch *patch)
     return equiv.id == patch->processor_rev_id;
 }
 
-static enum microcode_match_result cf_check compare_patch(
-    const struct microcode_patch *new, const struct microcode_patch *old)
+static int cf_check compare_patch(
+    const struct microcode_patch *old, const struct microcode_patch *new)
 {
     /* Both patches to compare are supposed to be applicable to local CPU. */
     ASSERT(microcode_fits_cpu(new));
@@ -212,11 +211,10 @@  static enum microcode_match_result cf_check compare_patch(
 static int cf_check apply_microcode(const struct microcode_patch *patch,
                                     unsigned int flags)
 {
-    int hw_err;
+    int hw_err, result;
     unsigned int cpu = smp_processor_id();
     struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);
     uint32_t rev, old_rev = sig->rev;
-    enum microcode_match_result result;
     bool ucode_force = flags & XENPF_UCODE_FORCE;
 
     if ( !microcode_fits_cpu(patch) )
diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 0cc5daa251e2..05d0d68d8158 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -470,8 +470,7 @@  struct ucode_buf {
 static long cf_check microcode_update_helper(void *data)
 {
     struct microcode_patch *patch = NULL;
-    enum microcode_match_result result;
-    int ret;
+    int ret, result;
     struct ucode_buf *buffer = data;
     unsigned int cpu, updated;
     struct patch_with_flags patch_with_flags;
@@ -527,7 +526,7 @@  static long cf_check microcode_update_helper(void *data)
     spin_lock(&microcode_mutex);
     if ( microcode_cache )
     {
-        result = alternative_call(ucode_ops.compare_patch, patch, microcode_cache);
+        result = alternative_call(ucode_ops.compare_patch, microcode_cache, patch);
 
         if ( result != NEW_UCODE &&
              !(ucode_force && (result == OLD_UCODE || result == SAME_UCODE)) )
diff --git a/xen/arch/x86/cpu/microcode/intel.c b/xen/arch/x86/cpu/microcode/intel.c
index 3f37792ab4b5..9616a5e9db4b 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -229,8 +229,7 @@  static int microcode_sanity_check(const struct microcode_patch *patch)
  * Production microcode has a positive revision.  Pre-production microcode has
  * a negative revision.
  */
-static enum microcode_match_result compare_revisions(
-    int32_t old_rev, int32_t new_rev)
+static int compare_revisions(int32_t old_rev, int32_t new_rev)
 {
     if ( new_rev > old_rev )
         return NEW_UCODE;
@@ -270,8 +269,8 @@  static bool microcode_fits_cpu(const struct microcode_patch *mc)
     return false;
 }
 
-static enum microcode_match_result cf_check compare_patch(
-    const struct microcode_patch *new, const struct microcode_patch *old)
+static int cf_check compare_patch(
+    const struct microcode_patch *old, const struct microcode_patch *new)
 {
     /*
      * Both patches to compare are supposed to be applicable to local CPU.
@@ -290,7 +289,7 @@  static int cf_check apply_microcode(const struct microcode_patch *patch,
     unsigned int cpu = smp_processor_id();
     struct cpu_signature *sig = &this_cpu(cpu_sig);
     uint32_t rev, old_rev = sig->rev;
-    enum microcode_match_result result;
+    int result;
     bool ucode_force = flags & XENPF_UCODE_FORCE;
 
     if ( !microcode_fits_cpu(patch) )
diff --git a/xen/arch/x86/cpu/microcode/private.h b/xen/arch/x86/cpu/microcode/private.h
index c9dd8ba066f9..957d4d4293d0 100644
--- a/xen/arch/x86/cpu/microcode/private.h
+++ b/xen/arch/x86/cpu/microcode/private.h
@@ -5,13 +5,6 @@ 
 
 #include <asm/microcode.h>
 
-enum microcode_match_result {
-    OLD_UCODE, /* signature matched, but revision id is older */
-    SAME_UCODE, /* signature matched, but revision id is the same */
-    NEW_UCODE, /* signature matched, but revision id is newer */
-    MIS_UCODE, /* signature mismatched */
-};
-
 /* Opaque.  Internals are vendor-specific. */
 struct microcode_patch;
 
@@ -54,11 +47,17 @@  struct microcode_ops {
                            unsigned int flags);
 
     /*
-     * Given two patches, are they both applicable to the current CPU, and is
-     * new a higher revision than old?
+     * Given a current patch, and a proposed new patch, order them based on revision.
+     *
+     * This operation is not necessarily symmetrical.  In some cases, a debug
+     * "new" patch will always considered to be newer, on the expectation that
+     * whomever is using debug patches knows exactly what they're doing.
      */
-    enum microcode_match_result (*compare_patch)(
-        const struct microcode_patch *new, const struct microcode_patch *old);
+#define OLD_UCODE  -1
+#define SAME_UCODE  0
+#define NEW_UCODE   1
+    int (*compare_patch)(const struct microcode_patch *old,
+                         const struct microcode_patch *new);
 
     /*
      * For Linux inird microcode compatibliity.