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

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

[Ashok Raj]

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 <ashok.raj@intel.com>
Cc: X86 ML <x86@kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>
---
 arch/x86/kernel/cpu/microcode/intel.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 09b95a7..036d1db 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -776,7 +776,7 @@ static enum ucode_state apply_microcode_intel(int cpu)
 {
 	struct microcode_intel *mc;
 	struct ucode_cpu_info *uci;
-	struct cpuinfo_x86 *c;
+	struct cpuinfo_x86 *c = &cpu_data(cpu);
 	static int prev_rev;
 	u32 rev;
 
@@ -793,6 +793,18 @@ static enum ucode_state apply_microcode_intel(int cpu)
 			return UCODE_NFOUND;
 	}
 
+	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) {
+		uci->cpu_sig.rev = rev;
+		c->microcode = rev;
+		return UCODE_OK;
+	}
+
 	/* write microcode via MSR 0x79 */
 	wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc->bits);
 
@@ -813,8 +825,6 @@ static enum ucode_state apply_microcode_intel(int cpu)
 		prev_rev = rev;
 	}
 
-	c = &cpu_data(cpu);
-
 	uci->cpu_sig.rev = rev;
 	c->microcode = rev;
 
-- 
2.7.4

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

[Ingo Molnar]

* Ashok Raj <ashok.raj@intel.com> 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.

s/cpu/CPU

> 
> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> Cc: X86 ML <x86@kernel.org>
> Cc: LKML <linux-kernel@vger.kernel.org>
> ---
>  arch/x86/kernel/cpu/microcode/intel.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 09b95a7..036d1db 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -776,7 +776,7 @@ static enum ucode_state apply_microcode_intel(int cpu)
>  {
>  	struct microcode_intel *mc;
>  	struct ucode_cpu_info *uci;
> -	struct cpuinfo_x86 *c;
> +	struct cpuinfo_x86 *c = &cpu_data(cpu);
>  	static int prev_rev;
>  	u32 rev;
>  
> @@ -793,6 +793,18 @@ static enum ucode_state apply_microcode_intel(int cpu)
>  			return UCODE_NFOUND;
>  	}
>  
> +	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) {
> +		uci->cpu_sig.rev = rev;
> +		c->microcode = rev;
> +		return UCODE_OK;
> +	}

s/udpate
 /update

Also, more fundamentally, during microcode early testing, isn't it possible for 
internal iterations of the microcode to have the same revision, but be different?

This patch would prevent re-loading it - for a seemingly minimal benefit.

Thanks,

	Ingo

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

[Raj, Ashok]

Hi Ingo

On Sat, Feb 17, 2018 at 08:50:53AM +0100, Ingo Molnar wrote:
> 
> Also, more fundamentally, during microcode early testing, isn't it possible for 
> internal iterations of the microcode to have the same revision, but be different?

Atleast we don't do that here. Such debug microcode isn't OS loadable. 
Anything that comes out ready for OS loading, the version will keep moving.

> This patch would prevent re-loading it - for a seemingly minimal benefit.

Cheers,
Ashok

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

[Ingo Molnar]

* Raj, Ashok <ashok.raj@intel.com> wrote:

> Hi Ingo
> 
> On Sat, Feb 17, 2018 at 08:50:53AM +0100, Ingo Molnar wrote:
> > 
> > Also, more fundamentally, during microcode early testing, isn't it possible for 
> > internal iterations of the microcode to have the same revision, but be different?
> 
> Atleast we don't do that here. Such debug microcode isn't OS loadable. 
> Anything that comes out ready for OS loading, the version will keep moving.

Ok.

Thanks,

	Ingo