Re: [PATCH v2] x86/microcode/intel: Reject problematic loading on Granite Rapids systems

[Borislav Petkov <bp@alien8.de>]

On Tue, Sep 08, 2026 at 10:32:09PM +0000, Chang S. Bae wrote:
> Microcode updates can usually jump revisions. However, there is an
> erratum on Granite Rapids systems. If they "jump over" to revision
> 0x1000405 or later, they result in #MC.
>
> Prevent loading 0x1000405 or later unless the running revision is already
> at least 0x1000405. Apply this blocking to both early- and late-loading
> paths.

People have got to stop explaining the patch in the commit message. That
should be obvious from the diff. If you have to explain it then there is
something very non-obvious here which I don't see it...

> Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
> Cc: <stable@vger.kernel.org>
> ---
> V1 -> V2:
> * Cut the code comments and print messages (Boris)
> * Rename the new function and keep the old function as it-is (Boris)
> * Rewrote the changelog (Dave)
> * Add `revision` in the error messages (Sohil)
> ---
>  arch/x86/kernel/cpu/microcode/intel.c | 32 +++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 1142183c950c..61ad280497e9 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -309,6 +309,32 @@ static void save_microcode_patch(struct microcode_intel *patch)
>  		pr_err("Unable to allocate microcode memory size: %u\n", size);
>  }
>  
> +static bool revision_banned(struct cpu_signature *sig, u32 rev)

I like Andy's naming:

https://lore.kernel.org/xen-devel/20260908171525.3196765-1-andrew.cooper3@citrix.com/T/#u

...is_safe is much better than 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. This is
> +	 * documented item GNR98, #835486 (Intel Xeon 6900/6700/6500-Series
> +	 * Processors with P-Cores).
> +	 */

What dhansen said - keep this short'n'sweet.

> +	if (vfm == INTEL_GRANITERAPIDS_X &&
> +	    x86_stepping(sig->sig) == 1 &&
> +	    sig->pf & 0x95 &&
> +	    sig->rev < 0x1000405 &&
> +	    rev >= 0x1000405) {
> +		if (rev == 0x1000405)

I also like Andy's testing of the patch revs:

+         ((cpu_sig->rev < 0x01000380 && mc->rev >= 0x01000405) ||
+          (cpu_sig->rev < 0x01000405 && mc->rev >  0x01000405)) )

> +			pr_err_once("Erratum GNR98: revision 0x1000405 is not loadable.\n");
> +		else
> +			pr_err_once("Erratum GNR98: revision 0x1000405 is required before 0x%x.\n", rev);

And you don't need those semi-identical strings here.

> +		return true;
> +	}
> +
> +	return false;
> +}

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette