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

[Sohil Mehta <sohil.mehta@intel.com>]

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 ...

> This dependency logically fits the minimum revision requirement.
> 
> The minimum revision check, however, currently applies only to the late
> loading path, since the dependency was primarily intended for OS-visible
> changes. The early loading path is therefore still vulnerable to this
> issue.
> 
> Furthermore, one of the subsequent revisions does not correctly specify
> the minimum revision, so unfortunately the late loading cannot rely on
> that check either in this case.
> 
> 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?

> Apply this blocking to both early- and late-loading paths.
> 

> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 1142183c950c..c502138fd8e9 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -309,6 +309,38 @@ static void save_microcode_patch(struct microcode_intel *patch)
>  		pr_err("Unable to allocate microcode memory size: %u\n", size);
>  }
>  
> +static bool is_loading_denied(struct cpu_signature *sig, u32 rev)
> +{
> +	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
> +	 * 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) {

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?


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

						   ^ revision

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?

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.


> +		pr_err_once("Please update the system BIOS or firmware.\n");
> +		return true;
> +	}
> +
> +	return false;
> +}
> +