[PATCH v4] x86/microcode/intel: Blacklist the specific BDW-EP for late loading

[PATCH v4] x86/microcode/intel: Blacklist the specific BDW-EP for late loading

[Jia Zhang]

Instead of blacklisting all types of Broadwell processor when running
a late loading, only BDW-EP (signature 0x406f1, aka family 6, model 79,
stepping 1) with the microcode version less than 0x0b000021 needs to
be blacklisted.

The erratum is documented in the the public documentation #334165 (See
the item BDF90 for details).

Signed-off-by: Jia Zhang <qianyue.zj@alibaba-inc.com>
---
 arch/x86/kernel/cpu/microcode/intel.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 8ccdca6..79cad85 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -910,8 +910,16 @@ static bool is_blacklisted(unsigned int cpu)
 {
 	struct cpuinfo_x86 *c = &cpu_data(cpu);
 
-	if (c->x86 == 6 && c->x86_model == INTEL_FAM6_BROADWELL_X) {
-		pr_err_once("late loading on model 79 is disabled.\n");
+	/*
+	 * The Broadwell-EP processor with the microcode version less
+	 * then 0x0b000021 may result in system hang when running a late
+	 * loading. This behavior is documented in item BDF90, #334165
+	 * (Intel Xeon Processor E7-8800/4800 v4 Product Family).
+	 */
+	if (c->x86 == 6 && c->x86_model == INTEL_FAM6_BROADWELL_X &&
+	    c->x86_mask == 0x01 && c->microcode < 0x0b000021) {
+		pr_err_once("late loading on cpu (sig 0x406f1) is disabled "
+			    "due to erratum causing system hang.\n");
 		return true;
 	}
 
-- 
1.8.3.1

Re: [PATCH v4] x86/microcode/intel: Blacklist the specific BDW-EP for late loading

[Luck, Tony]

When all the patch issues are ironed out consider this 

Acked-by: Tony Luck <tony.luck@intel.com>

I was planning to hunt down the erratum when I got back from vacation in the new year.

Sent from my iPhone

> On Dec 26, 2017, at 04:42, Jia Zhang <qianyue.zj@alibaba-inc.com> wrote:
> 
> Instead of blacklisting all types of Broadwell processor when running
> a late loading, only BDW-EP (signature 0x406f1, aka family 6, model 79,
> stepping 1) with the microcode version less than 0x0b000021 needs to
> be blacklisted.
> 
> The erratum is documented in the the public documentation #334165 (See
> the item BDF90 for details).
> 
> Signed-off-by: Jia Zhang <qianyue.zj@alibaba-inc.com>
> ---
> arch/x86/kernel/cpu/microcode/intel.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 8ccdca6..79cad85 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -910,8 +910,16 @@ static bool is_blacklisted(unsigned int cpu)
> {
>    struct cpuinfo_x86 *c = &cpu_data(cpu);
> 
> -    if (c->x86 == 6 && c->x86_model == INTEL_FAM6_BROADWELL_X) {
> -        pr_err_once("late loading on model 79 is disabled.\n");
> +    /*
> +     * The Broadwell-EP processor with the microcode version less
> +     * then 0x0b000021 may result in system hang when running a late
> +     * loading. This behavior is documented in item BDF90, #334165
> +     * (Intel Xeon Processor E7-8800/4800 v4 Product Family).
> +     */
> +    if (c->x86 == 6 && c->x86_model == INTEL_FAM6_BROADWELL_X &&
> +        c->x86_mask == 0x01 && c->microcode < 0x0b000021) {
> +        pr_err_once("late loading on cpu (sig 0x406f1) is disabled "
> +                "due to erratum causing system hang.\n");
>        return true;
>    }
> 
> -- 
> 1.8.3.1
> 

Re: [PATCH v4] x86/microcode/intel: Blacklist the specific BDW-EP for late loading

[Ingo Molnar]

* Jia Zhang <qianyue.zj@alibaba-inc.com> wrote:

> Instead of blacklisting all types of Broadwell processor when running
> a late loading, only BDW-EP (signature 0x406f1, aka family 6, model 79,
> stepping 1) with the microcode version less than 0x0b000021 needs to
> be blacklisted.
> 
> The erratum is documented in the the public documentation #334165 (See
> the item BDF90 for details).
> 
> Signed-off-by: Jia Zhang <qianyue.zj@alibaba-inc.com>
> ---
>  arch/x86/kernel/cpu/microcode/intel.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 8ccdca6..79cad85 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -910,8 +910,16 @@ static bool is_blacklisted(unsigned int cpu)
>  {
>  	struct cpuinfo_x86 *c = &cpu_data(cpu);
>  
> -	if (c->x86 == 6 && c->x86_model == INTEL_FAM6_BROADWELL_X) {
> -		pr_err_once("late loading on model 79 is disabled.\n");
> +	/*
> +	 * The Broadwell-EP processor with the microcode version less
> +	 * then 0x0b000021 may result in system hang when running a late
> +	 * loading. This behavior is documented in item BDF90, #334165
> +	 * (Intel Xeon Processor E7-8800/4800 v4 Product Family).
> +	 */
> +	if (c->x86 == 6 && c->x86_model == INTEL_FAM6_BROADWELL_X &&
> +	    c->x86_mask == 0x01 && c->microcode < 0x0b000021) {
> +		pr_err_once("late loading on cpu (sig 0x406f1) is disabled "
> +			    "due to erratum causing system hang.\n");

Please never break user-readable messages mid-sentence!

This should be something like:

		pr_err_once("Late loading of the CPU microcode (sig 0x406f1) is disabled due to Intel erratum BDF90 causing system hangs.\n");

(note the spelling and readability improvements as well)

Btw., what does 'sig 0x406f1' refer to?

Thanks,


	Ingo

Re: [PATCH v4] x86/microcode/intel: Blacklist the specific BDW-EP for late loading

[Jia Zhang]

在 2017/12/28 下午8:24, Ingo Molnar 写道:
> 
> * Jia Zhang <qianyue.zj@alibaba-inc.com> wrote:
> 
>> Instead of blacklisting all types of Broadwell processor when running
>> a late loading, only BDW-EP (signature 0x406f1, aka family 6, model 79,
>> stepping 1) with the microcode version less than 0x0b000021 needs to
>> be blacklisted.
>>
>> The erratum is documented in the the public documentation #334165 (See
>> the item BDF90 for details).
>>
>> Signed-off-by: Jia Zhang <qianyue.zj@alibaba-inc.com>
>> ---
>>  arch/x86/kernel/cpu/microcode/intel.c | 12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
>> index 8ccdca6..79cad85 100644
>> --- a/arch/x86/kernel/cpu/microcode/intel.c
>> +++ b/arch/x86/kernel/cpu/microcode/intel.c
>> @@ -910,8 +910,16 @@ static bool is_blacklisted(unsigned int cpu)
>>  {
>>  	struct cpuinfo_x86 *c = &cpu_data(cpu);
>>  
>> -	if (c->x86 == 6 && c->x86_model == INTEL_FAM6_BROADWELL_X) {
>> -		pr_err_once("late loading on model 79 is disabled.\n");
>> +	/*
>> +	 * The Broadwell-EP processor with the microcode version less
>> +	 * then 0x0b000021 may result in system hang when running a late
>> +	 * loading. This behavior is documented in item BDF90, #334165
>> +	 * (Intel Xeon Processor E7-8800/4800 v4 Product Family).
>> +	 */
>> +	if (c->x86 == 6 && c->x86_model == INTEL_FAM6_BROADWELL_X &&
>> +	    c->x86_mask == 0x01 && c->microcode < 0x0b000021) {
>> +		pr_err_once("late loading on cpu (sig 0x406f1) is disabled "
>> +			    "due to erratum causing system hang.\n");
> 
> Please never break user-readable messages mid-sentence!
> 
> This should be something like:
> 
> 		pr_err_once("Late loading of the CPU microcode (sig 0x406f1) is disabled due to Intel erratum BDF90 causing system hangs.\n");
> 
> (note the spelling and readability improvements as well)
> 
> Btw., what does 'sig 0x406f1' refer to?

It is so-called processor signature which can be used to identify a
model of x86 processor uniquely. It's the return value of cpuid
instruction with leaf 1(eax == 1).

Jia

> 
> Thanks,
> 
> 
> 	Ingo
> 

Re: [PATCH v4] x86/microcode/intel: Blacklist the specific BDW-EP for late loading

[Ingo Molnar]

* Jia Zhang <qianyue.zj@alibaba-inc.com> wrote:

> 
> 
> 在 2017/12/28 下午8:24, Ingo Molnar 写道:
> > 
> > * Jia Zhang <qianyue.zj@alibaba-inc.com> wrote:
> > 
> >> Instead of blacklisting all types of Broadwell processor when running
> >> a late loading, only BDW-EP (signature 0x406f1, aka family 6, model 79,
> >> stepping 1) with the microcode version less than 0x0b000021 needs to
> >> be blacklisted.
> >>
> >> The erratum is documented in the the public documentation #334165 (See
> >> the item BDF90 for details).
> >>
> >> Signed-off-by: Jia Zhang <qianyue.zj@alibaba-inc.com>
> >> ---
> >>  arch/x86/kernel/cpu/microcode/intel.c | 12 ++++++++++--
> >>  1 file changed, 10 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> >> index 8ccdca6..79cad85 100644
> >> --- a/arch/x86/kernel/cpu/microcode/intel.c
> >> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> >> @@ -910,8 +910,16 @@ static bool is_blacklisted(unsigned int cpu)
> >>  {
> >>  	struct cpuinfo_x86 *c = &cpu_data(cpu);
> >>  
> >> -	if (c->x86 == 6 && c->x86_model == INTEL_FAM6_BROADWELL_X) {
> >> -		pr_err_once("late loading on model 79 is disabled.\n");
> >> +	/*
> >> +	 * The Broadwell-EP processor with the microcode version less
> >> +	 * then 0x0b000021 may result in system hang when running a late
> >> +	 * loading. This behavior is documented in item BDF90, #334165
> >> +	 * (Intel Xeon Processor E7-8800/4800 v4 Product Family).
> >> +	 */
> >> +	if (c->x86 == 6 && c->x86_model == INTEL_FAM6_BROADWELL_X &&
> >> +	    c->x86_mask == 0x01 && c->microcode < 0x0b000021) {
> >> +		pr_err_once("late loading on cpu (sig 0x406f1) is disabled "
> >> +			    "due to erratum causing system hang.\n");
> > 
> > Please never break user-readable messages mid-sentence!
> > 
> > This should be something like:
> > 
> > 		pr_err_once("Late loading of the CPU microcode (sig 0x406f1) is disabled due to Intel erratum BDF90 causing system hangs.\n");
> > 
> > (note the spelling and readability improvements as well)
> > 
> > Btw., what does 'sig 0x406f1' refer to?
> 
> It is so-called processor signature which can be used to identify a
> model of x86 processor uniquely. It's the return value of cpuid
> instruction with leaf 1(eax == 1).

Ah, indeed, the (somewhat weird) encoding described in arch/x86/lib/cpu.c, which 
is essentially family+model+stepping encoded into a single integer, right?

That whole area needs a good cleanup to be less confusing (we refer to the CPU 
stepping as x86_stepping(), but the field is called ->x86_mask?), but in the 
meanwhile, let's please make it more obvious in user facing message what's 
happening.

Instead of using the microcode signature of the CPU model, please write out what's 
going on:

	pr_err_once("Not loading old microcode version: erratum BDF90 on Intel Broadwell-EP stepping 1 CPUs may cause system hangs.\n");

... and please also tell the user what to do about it:

	pr_err_once("Please update your microcode files.\n");

!!

Agreed?

Thanks,

	Ingo

Re: [PATCH v4] x86/microcode/intel: Blacklist the specific BDW-EP for late loading

[Jia Zhang]

在 2017/12/29 下午8:48, Ingo Molnar 写道:
> 
> * Jia Zhang <qianyue.zj@alibaba-inc.com> wrote:
> 
>>
>>
>> 在 2017/12/28 下午8:24, Ingo Molnar 写道:
>>>
>>> * Jia Zhang <qianyue.zj@alibaba-inc.com> wrote:
>>>
>>>> Instead of blacklisting all types of Broadwell processor when running
>>>> a late loading, only BDW-EP (signature 0x406f1, aka family 6, model 79,
>>>> stepping 1) with the microcode version less than 0x0b000021 needs to
>>>> be blacklisted.
>>>>
>>>> The erratum is documented in the the public documentation #334165 (See
>>>> the item BDF90 for details).
>>>>
>>>> Signed-off-by: Jia Zhang <qianyue.zj@alibaba-inc.com>
>>>> ---
>>>>  arch/x86/kernel/cpu/microcode/intel.c | 12 ++++++++++--
>>>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
>>>> index 8ccdca6..79cad85 100644
>>>> --- a/arch/x86/kernel/cpu/microcode/intel.c
>>>> +++ b/arch/x86/kernel/cpu/microcode/intel.c
>>>> @@ -910,8 +910,16 @@ static bool is_blacklisted(unsigned int cpu)
>>>>  {
>>>>  	struct cpuinfo_x86 *c = &cpu_data(cpu);
>>>>  
>>>> -	if (c->x86 == 6 && c->x86_model == INTEL_FAM6_BROADWELL_X) {
>>>> -		pr_err_once("late loading on model 79 is disabled.\n");
>>>> +	/*
>>>> +	 * The Broadwell-EP processor with the microcode version less
>>>> +	 * then 0x0b000021 may result in system hang when running a late
>>>> +	 * loading. This behavior is documented in item BDF90, #334165
>>>> +	 * (Intel Xeon Processor E7-8800/4800 v4 Product Family).
>>>> +	 */
>>>> +	if (c->x86 == 6 && c->x86_model == INTEL_FAM6_BROADWELL_X &&
>>>> +	    c->x86_mask == 0x01 && c->microcode < 0x0b000021) {
>>>> +		pr_err_once("late loading on cpu (sig 0x406f1) is disabled "
>>>> +			    "due to erratum causing system hang.\n");
>>>
>>> Please never break user-readable messages mid-sentence!
>>>
>>> This should be something like:
>>>
>>> 		pr_err_once("Late loading of the CPU microcode (sig 0x406f1) is disabled due to Intel erratum BDF90 causing system hangs.\n");
>>>
>>> (note the spelling and readability improvements as well)
>>>
>>> Btw., what does 'sig 0x406f1' refer to?
>>
>> It is so-called processor signature which can be used to identify a
>> model of x86 processor uniquely. It's the return value of cpuid
>> instruction with leaf 1(eax == 1).
> 
> Ah, indeed, the (somewhat weird) encoding described in arch/x86/lib/cpu.c, which 
> is essentially family+model+stepping encoded into a single integer, right?

Totally correct.

> 
> That whole area needs a good cleanup to be less confusing (we refer to the CPU 
> stepping as x86_stepping(), but the field is called ->x86_mask?), but in the 

Yes. This is a confusing name. I will send another patch to clean up it.

> meanwhile, let's please make it more obvious in user facing message what's 
> happening.
> 
> Instead of using the microcode signature of the CPU model, please write out what's 
> going on:
> 
> 	pr_err_once("Not loading old microcode version: erratum BDF90 on Intel Broadwell-EP stepping 1 CPUs may cause system hangs.\n");
> 
> ... and please also tell the user what to do about it:
> 
> 	pr_err_once("Please update your microcode files.\n");

Let me give a full background and we will have a best description for
this erratum clearly.

If current processor signature matches the problematic Broadwell-EP
model (0x406f1) *AND* current version of microcode is less than
0x0b000021, launching a microcode update in Linux runtime (or so-called
late loading) must be prohibited in order to prevent from system hang
due to the erratum. Namely, the end user has to make a BIOS update to
uprev the microcode. The code of microcode update loader in BIOS can
safely issus an microcode update without the concern about this erratum.
This is the so-called manner of early loading.

Thanks,
Jia

> 
> !!
> 
> Agreed?
> 
> Thanks,
> 
> 	Ingo
> 

Re: [PATCH v4] x86/microcode/intel: Blacklist the specific BDW-EP for late loading

[Borislav Petkov]

On Fri, Dec 29, 2017 at 09:17:34PM +0800, Jia Zhang wrote:
> Namely, the end user has to make a BIOS update to uprev the microcode.

Not quite: end user is dependent on the OEM to get a BIOS update.

What is meant with early loading in this context is, to put the
microcode in the initrd so that it gets loaded very early during the
*kernel* boot. And early-initrd-based loading is the default method we
do microcode loading with, anyway.

So the blacklisting is only for *late* loading, through the sysfs
trigger. Which is only there for historical reasons and for some
configurations.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH v4] x86/microcode/intel: Blacklist the specific BDW-EP for late loading

[Jia Zhang]

在 2017/12/29 下午9:44, Borislav Petkov 写道:
> On Fri, Dec 29, 2017 at 09:17:34PM +0800, Jia Zhang wrote:
>> Namely, the end user has to make a BIOS update to uprev the microcode.
> 
> Not quite: end user is dependent on the OEM to get a BIOS update.
> 
> What is meant with early loading in this context is, to put the
> microcode in the initrd so that it gets loaded very early during the
> *kernel* boot. And early-initrd-based loading is the default method we
> do microcode loading with, anyway.

Thanks for clarifying the terms. What I meant was the microcode update
has to be launched in BIOS rather than late loading in linux runtime.

But I'm not quite sure whether or not the early-initrd-based loading is
affected. I used a shared BDW-EP machine with a rev of microcode less
than 0x0b000021 and observed the late loading is prohibited. I attempted
to remove the is_blacklisted() and observed a system hang with the late
loading. Eventually I followed up the erratum and updated the microcode
through BIOS update.

It is impossible to downgrade the BIOS to recover the old rev of
microcode for me to have a test with early-initrd-based loading.

Anyway, is_blacklisted() initially was introduced by Borislav to
prohibit late loading only, so the early-initrd-based loading seems to
be OK.

Thanks,
Jia

> 
> So the blacklisting is only for *late* loading, through the sysfs
> trigger. Which is only there for historical reasons and for some
> configurations.
> 

Re: [PATCH v4] x86/microcode/intel: Blacklist the specific BDW-EP for late loading

[Borislav Petkov]

On Fri, Dec 29, 2017 at 10:12:34PM +0800, Jia Zhang wrote:
> Thanks for clarifying the terms. What I meant was the microcode update
> has to be launched in BIOS rather than late loading in linux runtime.

Microcode update on those machines can happen in two ways:

* loaded by the BIOS

OR

* loaded early by the kernel from the initrd.

  [ There's also the builtin microcode option which happens early too
    but I'm mentioning it here only for completeness sake - don't get
    confused by it. ]

> But I'm not quite sure whether or not the early-initrd-based loading is
> affected.

Apparently it isn't.

> I used a shared BDW-EP machine with a rev of microcode less than
> 0x0b000021 and observed the late loading is prohibited. I attempted
> to remove the is_blacklisted() and observed a system hang with the
> late loading. Eventually I followed up the erratum and updated the
> microcode through BIOS update.

Yap, you confirmed the erratum. Which is a good data point. :-)

> It is impossible to downgrade the BIOS to recover the old rev of
> microcode for me to have a test with early-initrd-based loading.

Some BIOSes allow that. Depends on the platform.

> Anyway, is_blacklisted() initially was introduced by Borislav to
> prohibit late loading only, so the early-initrd-based loading seems to
> be OK.

Yes, Borislav, that is me. :-)

And yes, the blacklisting is for late loading only. This has a greater
importance for old kernels where the initrd method is not supported yet
and we wanted to prevent machine hangs from microcode update. Thus this
solution which got backported.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--