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:11 AM, Sohil Mehta wrote:
> On 9/1/2026 4:16 PM, Chang S. Bae wrote:
>> Revision 0x1000405 contains internal microcode changes that are required
>> by subsequent revisions to avoid #MC during loading.
> 
> Before going to the solution space, can we add some more
> context/background here?
> 
> For example:
> 
> GNR revision 0x1000405 introduces a breaking change that causes a #MC
> when the OS updates the microcode from any version older than 0x1000405
> to a newer version. This is applicable to late load as well as early
> loading during boot.
> 
> A BIOS or firmware update is required to update the revision to
> 0x1000405 or newer before any OS updates can be safely loaded.
> This dependency ...

Thanks. I ended up taking Dave's version for V2.


>> Prevent loading 0x1000405 or later when the system has not yet been
>> updated to 0x1000405 or later.
> 
> This is a bit confusing. Should it say jumping from any anything older
> than 0x1000405 to anything newer?

I tweaked it in V2 to:
  "Prevent loading 0x1000405 or later unless the running revision is
   already at least 0x1000405."


>> +	if (vfm == INTEL_GRANITERAPIDS_X &&
>> +	    x86_stepping(sig->sig) == 1 &&
>> +	    sig->pf & 0x95 &&
>> +	    sig->rev < 0x1000405 &&
>> +	    rev >= 0x1000405) {
> 
> I don't think we have a helper that can be used here directly. Also,
> this is tagged for stable so adding a new one probably doesn't make sense.
> 
> But, 0x1000405 is repeated way too many times in this function :)
> At a minimum, can we add something like this?
> 
> #define GNR98_UCODE_MIN_REV 	0x1000405
> 
> Maybe add defines for the PF as well?


I saw exactly the same suggestion from AI, but I was not sure much value 
beyond more cosmetic. The revision is a one-off specific to the blocking 
check, so putting the revision value directly instead of a new define.


> 
> 
>> +		if (rev == 0x1000405)
>> +			pr_err_once("Erratum GNR98: 0x1000405 is not loadable.\n");
> 
> 						   ^ revision

Let me add `revision` in the message. Thanks.

> 
> With early load, his is probably one of the first messages that folks
> will see but it will only be displayed on platforms that have the issue
> until they update their microcode. Should we be more verbose here?
> 
> 
>> +		else
>> +			pr_err_once("Erratum GNR98: 0x1000405 is required before 0x%x.\n", rev);
> 
> "Erratum GNR98: revision 0x1000405 is required before revision 0x%x can
> be loaded. \n"
> 
> Should this be 0x1000405 *or later* ? We don't want common users to
> specifically try to find the revision 0x1000405 and load it, right?


For the error messages, `GNR98` needs to be decoded anyway, since the 
full context of the erratum is needed to understand for the next step. 
The message then is intended to point the erratum with a brief 
explanation. So I don't think it is too terse.

> 
> I am wondering what is the use of the if-else? Is that intended to guide
> the late loading users? I think for other users the second message would
> be confusing.

When the incoming revision is exactly 0x1000405, saying "revision 
0x1000405 is required" doesn't make any sense. For the later revisions, 
the second message can explain more directly too. So I split the two cases.

Thanks,
Chang