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

[Dave Hansen]

On 9/8/26 15:32, 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.

This is a kinda wonky-looking patch but I don't know a nicer way to do
it, so:

Reviewed-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>

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

I honestly still prefer that this comment be more short and sweet. Does
this miss anything?

	/*
	 * Erratum GNR98 can cause #MC's if "jumping over"
	 * revision 0x1000405. Avoid the jumps.
	 */

Also, we don't need to put whole document titles or numbers in here.
They're fine, but if you can Google "erratum GNR98" and get the doc for
the first hit, what does it matter?

Leave the precision and long chit chat for the changelog, I say.

But we can edit this in place when it gets applied.

> +	if (vfm == INTEL_GRANITERAPIDS_X &&
> +	    x86_stepping(sig->sig) == 1 &&
> +	    sig->pf & 0x95 &&
> +	    sig->rev < 0x1000405 &&
> +	    rev >= 0x1000405) {
> +		if (rev == 0x1000405)
> +			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);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
>  /* Scan blob for microcode matching the boot CPUs family, model, stepping */
>  static __init struct microcode_intel *scan_microcode(void *data, size_t size,
>  						     struct ucode_cpu_info *uci,
> @@ -330,6 +356,9 @@ static __init struct microcode_intel *scan_microcode(void *data, size_t size,
>  		if (!intel_find_matching_signature(data, &uci->cpu_sig))
>  			continue;
>  
> +		if (revision_banned(&uci->cpu_sig, mc_header->rev))
> +			continue;
> +
>  		/*
>  		 * For saving the early microcode, find the matching revision which
>  		 * was loaded on the BSP.
> @@ -878,6 +907,9 @@ static enum ucode_state parse_microcode_blobs(int cpu, struct iov_iter *iter)
>  		if (!intel_find_matching_signature(mc, &uci->cpu_sig))
>  			continue;
>  
> +		if (revision_banned(&uci->cpu_sig, mc_header.rev))
> +			continue;
> +
>  		is_safe = ucode_validate_minrev(&mc_header);
>  		if (force_minrev && !is_safe)
>  			continue;