[PATCH 2/3] x86/ucode/intel: Fix handling of microcode revision

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Jan Beulich" <JBeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	"Juergen Gross" <jgross@suse.com>,
	"Igor Druzhinin" <igor.druzhinin@citrix.com>
Subject: [PATCH 2/3] x86/ucode/intel: Fix handling of microcode revision
Date: Mon, 26 Oct 2020 17:25:18 +0000	[thread overview]
Message-ID: <20201026172519.17881-3-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <20201026172519.17881-1-andrew.cooper3@citrix.com>

For Intel microcodes, the revision field is signed (as documented in the SDM)
and negative revisions are used for pre-production/test microcode (not
documented publicly anywhere I can spot).

Adjust the revision checking to match the algorithm presented here:

  https://software.intel.com/security-software-guidance/best-practices/microcode-update-guidance

This treats pre-production microcode as always applicable, but also production
microcode having higher precident than pre-production.  It is expected that
anyone using pre-production microcode knows what they are doing.

This is necessary to load production microcode on an SDP with pre-production
microcode embedded in firmware.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Juergen Gross <jgross@suse.com>
CC: Igor Druzhinin <igor.druzhinin@citrix.com>

"signed" is somewhat of a problem.  The actual numbers only make sense as
sign-magnitude, and not as twos-compliement, which is why the rule is "any
debug microcode goes".

The actual upgrade/downgrade rules are quite complicated, but in general, any
change is permitted so long as the Security Version Number (embedded inside
the patch) doesn't go backwards.

---
 xen/arch/x86/cpu/microcode/intel.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/intel.c b/xen/arch/x86/cpu/microcode/intel.c
index e1ccb5d232..5fa2821cdb 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -33,7 +33,7 @@
 
 struct microcode_patch {
     uint32_t hdrver;
-    uint32_t rev;
+    int32_t rev;
     uint16_t year;
     uint8_t  day;
     uint8_t  month;
@@ -222,12 +222,23 @@ static int microcode_sanity_check(const struct microcode_patch *patch)
     return 0;
 }
 
+/*
+ * Production microcode has a positive revision.  Pre-production microcode has
+ * a negative revision.
+ */
 static enum microcode_match_result compare_revisions(
-    uint32_t old_rev, uint32_t new_rev)
+    int32_t old_rev, int32_t new_rev)
 {
     if ( new_rev > old_rev )
         return NEW_UCODE;
 
+    /*
+     * Treat pre-production as always applicable - anyone using pre-production
+     * microcode knows what they are doing, and can keep any resulting pieces.
+     */
+    if ( new_rev < 0 )
+        return NEW_UCODE;
+
     return OLD_UCODE;
 }
 
-- 
2.11.0


