Re: [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems

["Chang S. Bae" <chang.seok.bae@intel.com>]

On 9/2/2026 6:51 PM, Borislav Petkov wrote:
> On Tue, Sep 01, 2026 at 11:16:26PM +0000, Chang S. Bae wrote:
>> +static bool is_loading_denied(struct cpu_signature *sig, u32 rev)
> 
> This naming is not better, sorry.
> 
> Is loading denied means the loading in general is denied because <raisin> or
> are you trying to check whether this particular revision should not be loaded?
> 
> I think it is latter.
> 
> So you wanna say
> 
> 	revision_blacklisted()
> 
> or so.

Maybe revision_banned()?

> 
>> +{
>> +	u32 vfm = IFM(x86_family(sig->sig), x86_model(sig->sig));
>> +
>> +	/*
>> +	 * Revision 0x1000405 contains prerequisite changes for subsequent
>> +	 * microcode updates on Granite Rapids systems. Updates directly from
>> +	 * an older revision to this or a newer one can result in #MC (GNR98).
>> +	 *
>> +	 * This dependency can be indicated from the minimum revision field.
>> +	 * However, revision 0x1000423 has an incorrect minimum revision in its
> 
> So you lost me here: 0x1000423 is not tested anywhere - just mentioned here.
> 
> So what I understand is: usually, dependencies like that can be expressed with
> minrev but *in addition* to the current issue, patch 0x1000423 has minrev
> wrong so that dependency cannot be upheld there either.
> 
> But then why even mention it if you're not going to test it?
> 
> Why do we care about GNR101 at all?
> 

My original intention was to explain why rejection is need for the late 
loading path. There is a minrev check, but broken with that case.

But to preserve the legacy behavior, the minrev check isn't enforced by 
default. So what's the point of mentioning? Yes, if we need to ban 
particular revisions, we need to do it everywhere.

So mentioning minrev is just distracting readers as long as rejection is 
based on revision number in the first place. Let me remove minrev 
wording from this patch.

>> +	 * header (GNR101).
>> +	 *
>> +	 * Prevent loading 0x1000405 or later unless the CPU has already been
>> +	 * updated to 0x1000405 or later.
>> +	 */
>> +	if (vfm == INTEL_GRANITERAPIDS_X &&
>> +	    x86_stepping(sig->sig) == 1 &&
>> +	    sig->pf & 0x95 &&
>> +	    sig->rev < 0x1000405 &&
>> +	    rev >= 0x1000405) {
> 
> You don't really need to test rev here - it is enough that sig->rev is
> < 0x1000405 - that already makes you susceptible and then you can check rev
> inside the { }.

Yeah, I'd like to simplify like that, though. Intel repository looks to 
have three published revisions before 0x1000405. So there can be a valid 
update between those revisions.

So still need to allow:
   old_rev < 0x1000405 -> new_rev < 0x1000405

while rejecting:
   old_rev < 0x1000405 -> new_rev >= 0x1000405

> 
>> +		if (rev == 0x1000405)
>> +			pr_err_once("Erratum GNR98: 0x1000405 is not loadable.\n");
>> +		else
>> +			pr_err_once("Erratum GNR98: 0x1000405 is required before 0x%x.\n", rev);
>> +		pr_err_once("Please update the system BIOS or firmware.\n");
> 
> This is useless most of the time because client won't usually get BIOS
> updates. You can tell people they should update their microcode packages
> instead. That's where we can really help.
> 

I see.

I think anyone seeing this message has probably already tried updating 
microcode. With `GNR98` in the message, they would still need to decode 
it by looking up the erratum and its workaround if want to follow up on it:

   Workaround: None identified. Software should avoid updating MCU
   versions earlier than 0x1000405 to 0x1000405 or later until the system
   is Firmware Interface Table (FIT) loaded with UEFI FW/BIOS to
   0x1000405 or later.

Then there isn't much point of the additional message, I suppose. I'll 
remove it.

>>   
>> -	if (is_blacklisted(cpu))
>> +	if (is_late_loading_denied(cpu))
> 
> Aaaah, you wanna be politically correct and can't use "blacklisted" anymore.
> 
> Well, you're not introducing new usage so you don't have to touch old usage.
> And "is denied" does not express the situation properly. Try a better one.
Okay. I'll leave the existing "is_blacklisted(cpu)" as it-is since it 
already exists and probably descriptive enough. Also assuming it will 
remain distinctive from the new one, e.g. revision_banned(...).

Thanks,
Chang