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

[Chang S. Bae]

On 9/8/2026 5:13 PM, Borislav Petkov wrote:
> 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...

... If they "jump over" to revision 0x1000405 or later, they result in 
#MC. Avoid it.

> >   
> > +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@xxxxxxxxxx/T/#u
>
> ...is_safe is much better than banned.

s/revision_banned/revision_is_safe/ in V3.

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

Will replace the above with Dave's.

> > +	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)) )

Let me double-check this with ucode folks, first. Then will take.

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

pr_err_once("Erratum GNR98: skipping revision 0x%x.\n", rev);

Yeah, overall I can hear "keep it simple & short" messages.

Thanks,
Chang