Skip to contentSkip to conversation

x86/microcode: Handle negative microcode revisions

PATCH Superseded Andi Kleen 4 participants 4 messages Oct 20, 2018

Subsystems (2)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

PATCH 1/1 x86/microcode: Handle negative microcode revisions

From: Andi Kleen <[email protected]>

The Intel ucode revision space is unsigned. Inside Intel there are special microcodes that have the highest bit set, and they are considered to have a higher revision than any microcodes that don't have this bit set.

The function comparing the microcodes in the Linux driver compares u32 with int, which ends up being signed extended to long on 64bit systems. This results in these highest bit set microcodes not loading because their revision appears negative and smaller than the existing microcode.

Change the comparison to unsigned. With that the loading works as expected.

---
 arch/x86/kernel/cpu/microcode/intel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kernel/cpu/microcode/intel.c +1 -1
index 16936a24795c..e95cebdd5993 100644
@@ -93,7 +93,7 @@ static int find_matching_signature(void *mc, unsigned int csig, int cpf)
 /*
  * Returns 1 if update has been found, 0 otherwise.
  */
-static int has_newer_microcode(void *mc, unsigned int csig, int cpf, int new_rev)
+static int has_newer_microcode(void *mc, unsigned int csig, int cpf, unsigned new_rev)
 {
 	struct microcode_header_intel *mc_hdr = mc;
 
Signature · 1 line
2.17.1
maintainer
#1 lore raw
Quoted 3 lines · Andi Kleen

From: Andi Kleen <[email protected]>

The Intel ucode revision space is unsigned. Inside Intel there are special

s/ucode/microcode/g

Quoted 1 line

microcodes that have the highest bit set, and they are considered to have

s/microcodes/microcode revisions/g

Quoted 21 lines

a higher revision than any microcodes that don't have this bit set.

The function comparing the microcodes in the Linux driver compares u32 with int, which ends up being signed extended to long on 64bit systems. This results in these highest bit set microcodes not loading because their revision appears negative and smaller than the existing microcode.

Change the comparison to unsigned. With that the loading works as expected.

---
 arch/x86/kernel/cpu/microcode/intel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kernel/cpu/microcode/intel.c +0 -0
index 16936a24795c..e95cebdd5993 100644

arch/x86/kernel/cpu/microcode/intel.c static int find_matching_signature(void *mc, unsigned int csig, int cpf) · line 96

Quoted 5 lines
arch/x86/kernel/cpu/microcode/intel.c +1 -1
@@ -93,7 +93,7 @@ static int find_matching_signature(void *mc, unsigned int csig, int cpf)
 /*
  * Returns 1 if update has been found, 0 otherwise.
  */
-static int has_newer_microcode(void *mc, unsigned int csig, int cpf, int new_rev)
+static int has_newer_microcode(void *mc, unsigned int csig, int cpf, unsigned new_rev)

I'm gonna let you run checkpatch yourself to find out what the problem here is.

Signature · 4 lines
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
#2 lore raw
In reply to Andi Kleen

Andi,

Quoted 3 lines · Andi Kleen

Change the comparison to unsigned. With that the loading works as expected.

I assume that wants a fixes tag and needs to be backported to stable, right?

Thanks,

tglx

#3 lore raw
Quoted 9 lines · Thomas Gleixner

Andi,

Quoted 3 lines · Andi Kleen

Change the comparison to unsigned. With that the loading works as expected.

I assume that wants a fixes tag and needs to be backported to stable, right?

I think the bug was there since the original microcode loader, not sure I can dig out the right commit id for that (it might be in linux-historic)

Yes please cc stable for this.

-Andi