2018-02-16 19:24:01

by Ashok Raj

[permalink] [raw]
Subject: [PATCH] x86/microcode: Check microcode revision before updating sibling threads

After updating microcode on one of the threads in the core, the
thread sibling automatically gets the update since the microcode
resources are shared. Check the ucode revision on the cpu before
performing a ucode update.

Signed-off-by: Ashok Raj <[email protected]>
Cc: X86 ML <[email protected]>
Cc: LKML <[email protected]>
---
arch/x86/kernel/cpu/microcode/intel.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 09b95a7..5802c2f 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -786,11 +786,21 @@ static enum ucode_state apply_microcode_intel(int cpu)

uci = ucode_cpu_info + cpu;
mc = uci->mc;
+
if (!mc) {
/* Look for a newer patch in our cache: */
mc = find_patch(uci);
if (!mc)
return UCODE_NFOUND;
+ } else {
+ rev = intel_get_microcode_revision();
+ /*
+ * Its possible the microcode got udpated
+ * because its sibling update was done earlier.
+ * Skip the udpate in that case.
+ */
+ if (rev == mc->hdr.rev)
+ goto done;
}

/* write microcode via MSR 0x79 */
@@ -813,6 +823,7 @@ static enum ucode_state apply_microcode_intel(int cpu)
prev_rev = rev;
}

+done:
c = &cpu_data(cpu);

uci->cpu_sig.rev = rev;
--
2.7.4



2018-02-16 19:24:32

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] x86/microcode: Check microcode revision before updating sibling threads

On Fri, Feb 16, 2018 at 10:46:48AM -0800, Ashok Raj wrote:
> After updating microcode on one of the threads in the core, the
> thread sibling automatically gets the update since the microcode
> resources are shared. Check the ucode revision on the cpu before
> performing a ucode update.
>
> Signed-off-by: Ashok Raj <[email protected]>
> Cc: X86 ML <[email protected]>
> Cc: LKML <[email protected]>
> ---
> arch/x86/kernel/cpu/microcode/intel.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 09b95a7..5802c2f 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -786,11 +786,21 @@ static enum ucode_state apply_microcode_intel(int cpu)
>
> uci = ucode_cpu_info + cpu;
> mc = uci->mc;
> +
> if (!mc) {
> /* Look for a newer patch in our cache: */
> mc = find_patch(uci);
> if (!mc)
> return UCODE_NFOUND;
> + } else {
> + rev = intel_get_microcode_revision();
> + /*
> + * Its possible the microcode got udpated
> + * because its sibling update was done earlier.
> + * Skip the udpate in that case.
> + */
> + if (rev == mc->hdr.rev)
> + goto done;
> }

Make that like the AMD version:

/*
* Its possible the microcode got udpated
* because its sibling update was done earlier.
* Skip the udpate in that case.
*/
rev = intel_get_microcode_revision();
if (rev >= mc->hdr.rev) {
uci->cpu_sig.rev = rev;
c->microcode = rev;

return UCODE_OK;
}

and make the subject prefix "x86/microcode/intel:" for your next
submission.

Thx.

--
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--

Subject: Re: [PATCH] x86/microcode: Check microcode revision before updating sibling threads

On Fri, 16 Feb 2018, Ashok Raj wrote:
> After updating microcode on one of the threads in the core, the
> thread sibling automatically gets the update since the microcode
> resources are shared. Check the ucode revision on the cpu before
> performing a ucode update.

I wonder when this broke, because it used to do that just fine.

I even had to explain to a couple Debian users that no, nothing was
broken and yes, while it looked like the kernel was skipping some
"CPUs", it was actually updating all of them...

--
Henrique Holschuh

2018-02-17 14:22:39

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] x86/microcode: Check microcode revision before updating sibling threads

On Sat, Feb 17, 2018 at 11:42:48AM -0200, Henrique de Moraes Holschuh wrote:
> I wonder when this broke,

It didn't break - we're just printing the update only once.

--
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--

Subject: Re: [PATCH] x86/microcode: Check microcode revision before updating sibling threads

On Sat, 17 Feb 2018, Borislav Petkov wrote:
> On Sat, Feb 17, 2018 at 11:42:48AM -0200, Henrique de Moraes Holschuh wrote:
> > I wonder when this broke,
>
> It didn't break - we're just printing the update only once.

It was before that change, when the microcode subsystem still printed a
message for every CPU that got updated or early-updated.

It was fixed to properly refresh the microcode revision in the percpu
area right before deciding whether it should update that cpu or not, and
thus it would skip the second thread on Intel cores because it would
notice it was already up-to-date.

Maybe it was just the late loader, though... I'd need to do some git
log digging.

--
Henrique Holschuh