x86/microcode: intel: correctly handle negative revisions

From: Henrique de Moraes Holschuh
Date: Thu Mar 24 2011 - 22:10:25 EST


As per the Intel SDM vol 3A, microcode revisions are signed 32-bit
numbers. The code was handling them as unsigned int in some places and as
an int in other places.

As per the clarification posted by Burt Triplett from the Intel BITS
project, negative microcode revisions are used internally at Intel and
should always get loaded. Also, they should not be overriden unless we
can somehow differentiate "automated" loading from "forced" loading (which
we cannot at this time). Burt says the SDM will be updated with this
information eventually.

The code should:

1. Ignore attempts to load a zero-revision microcode (that value is
reserved for the CPU to signal that it is running with the factory
microcode, and must not be present in a normal microcode update);

2. Always load negative revision microcodes, to help Intel's engineers;

3. Avoid upgrading from a BIOS-loaded negative revision microcode to
a normal microcode, to not get in the way of Intel's engineers.

4. Upgrade from revision 0 (no updates loaded in CPU) to any revision.

It was already doing some of that, but I don't feel like trying to track
down exactly how the old code with its mix of signed/unsigned handling of
revisions would behave in each of the above cases.

Signed-off-by: Henrique de Moraes Holschuh <hmh@xxxxxxxxxx>
LKML-Reference: <4D87E2CD.6020306@xxxxxxxxxxxxxxx>
Cc: Tigran Aivazian <tigran@xxxxxxxxxxxxxxxxxxxx>
Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
Cc: Burt Triplett <burt@xxxxxxxxxxxxxxx>

diff --git a/arch/x86/kernel/microcode_intel.c b/arch/x86/kernel/microcode_intel.c
index 1a1b606..92b7cfe 100644
--- a/arch/x86/kernel/microcode_intel.c
+++ b/arch/x86/kernel/microcode_intel.c
@@ -89,7 +89,7 @@ MODULE_LICENSE("GPL");

struct microcode_header_intel {
unsigned int hdrver;
- unsigned int rev;
+ int rev;
unsigned int date;
unsigned int sig;
unsigned int cksum;
@@ -178,10 +178,15 @@ static inline int update_match_cpu(struct cpu_signature *csig, int sig, int pf)
return (!sigmatch(sig, csig->sig, pf, csig->pf)) ? 0 : 1;
}

+/*
+ * rev < 0 : devel/test microcode, always install
+ * rev = 0 : (cpu: no microcode installed. data file: illegal)
+ * rev > 0 : normal microcode, avoid downgrading
+ */
static inline int
update_match_revision(struct microcode_header_intel *mc_header, int rev)
{
- return (mc_header->rev <= rev) ? 0 : 1;
+ return !!(mc_header->rev < 0 || (mc_header->rev > rev && rev >= 0));
}

static int microcode_sanity_check(void *mc)
@@ -298,7 +303,7 @@ static int apply_microcode(int cpu)
{
struct microcode_intel *mc_intel;
struct ucode_cpu_info *uci;
- unsigned int val[2];
+ int val[2];
int cpu_num;

cpu_num = raw_smp_processor_id();
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/