Subject: Re: [PATCH 2/2] x86/microcode/intel: Extend BDW late-loading with
 platform id and LLC check
To: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
References: <1515995003-113069-1-git-send-email-zhang.jia@linux.alibaba.com>
 <1515995003-113069-3-git-send-email-zhang.jia@linux.alibaba.com>
 <20180115114851.npyeo4uztf5ww33o@khazad-dum.debian.net>
Cc: tony.luck@intel.com, bp@alien8.de, mingo@redhat.com, hpa@zytor.com,
        tglx@linutronix.de, x86@kernel.org, linux-kernel@vger.kernel.org
From: Jia Zhang <zhang.jia@linux.alibaba.com>
Message-ID: <a57d531f-526a-46cd-a65a-95edb2706629@linux.alibaba.com>
Date: Mon, 15 Jan 2018 21:07:15 +0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0)
 Gecko/20100101 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <20180115114851.npyeo4uztf5ww33o@khazad-dum.debian.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org


在 2018/1/15 下午7:48, Henrique de Moraes Holschuh 写道:
> On Mon, 15 Jan 2018, Jia Zhang wrote:
>> For more details, see erratum BDF90 in document #334165 (Intel Xeon
>> Processor E7-8800/4800 v4 Product Family Specification Update) from
>> September 2017.
> 
> For the record, this erratum may well affect some E5v4 as well.
> Anything with a LLC/core ratio >= 2.5 is potentially affected as far as
> I could tell when I took a serious look at it months ago (based only on
> crash reports and public information).
> 
> It would be safer to just blacklist by sig == 0x406f1, revision <
> 0x0b00021, and LLC/core ratio >= 2.5, ignoring platform IDs.
> 
>>  	/*
>>  	 * Late loading on model 79 with microcode revision less than 0x0b000021
>> -	 * may result in a system hang. This behavior is documented in item
>> -	 * BDF90, #334165 (Intel Xeon Processor E7-8800/4800 v4 Product Family).
>> +	 * and LLC size per core bigger than 2.5MB may result in a system hang.
>> +	 * 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 &&
>> +	    llc_size_per_core(c) > 2621440 &&
>> +	    c->platform_id == 0xef &&
>>  	    c->microcode < 0x0b000021) {
>>  		pr_err_once("Erratum BDF90: late loading with revision < 0x0b000021 (0x%x) disabled.\n", c->microcode);
>>  		pr_err_once("Please consider either early loading through initrd/built-in or a potential BIOS update.\n");
> 
> The c->platform_id test looks wrong.  The processor will only have a
> single bit set, it is the microcode update that has more than a single
> bit set.
> 
> And do you really want 0xef?  That is everyhing the public available
> microcode updates can be applied to in the first place, so even a
> corrected test would be useless (it would always match) unless you
> actually expect to find never-seen-in-the-wild platform mask 0x10?

Yes you are right. That platform id is used in microcode update for
matching compare. I will drop platform id in v2.

Jia

>