Re: [PATCH v3] x86/ucode: Work around Granite Rapids erraturm GNR98

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

On Tue, Sep 08, 2026 at 06:15:25PM +0100, Andrew Cooper wrote:
> Block loads which are known to hang the system.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger@xxxxxxxxxxxxxx>
> CC: Teddy Astie <teddy.astie@xxxxxxxxxx>
> 
> A more complete solution is in the works, but it's taken 4 months to get this
> much published...
> 
> v3:
>  * Double XENLOG_WARNING
> 
> v2:
>  * Correct the sign of the cpu_sig->rev check.
>  * Expand the comment to explain why we are not following what GNR98 says.
> ---
>  xen/arch/x86/cpu/microcode/intel.c | 40 ++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/xen/arch/x86/cpu/microcode/intel.c 
> b/xen/arch/x86/cpu/microcode/intel.c
> index c45b00c6b033..86cbfb798160 100644
> --- a/xen/arch/x86/cpu/microcode/intel.c
> +++ b/xen/arch/x86/cpu/microcode/intel.c
> @@ -27,6 +27,7 @@
>  #include <xen/string.h>
>  #include <xen/xmalloc.h>
>  
> +#include <asm/intel-family.h>
>  #include <asm/msr.h>
>  #include <asm/processor.h>
>  #include <asm/system.h>
> @@ -273,6 +274,44 @@ static bool microcode_fits_cpu(const struct 
> microcode_patch *mc)
>      return false;
>  }
>  
> +static bool microcode_safe_to_load(const struct microcode_patch *mc)
> +{
> +    struct cpu_signature *cpu_sig = &this_cpu(cpu_sig);

I think this could be const?  Or are there further changes expected
that will modify the signature?

> +
> +    /*
> +     * Treat pre-production as always safe - anyone using pre-production
> +     * microcode knows what they are doing, and can keep any resulting 
> pieces.
> +     */
> +    if ( (int)cpu_sig->rev < 0 || mc->rev < 0 )
> +        return true;
> +
> +    /*
> +     * GNR98 states that Granite Rapids systems hang when loading new ucode 
> on
> +     * sufficiently old firmware.  GNR101 retroactively states that one ucode
> +     * had an incorrect minimum revision field, in light of discovering 
> GNR98.
> +     *
> +     * Both are incomplete statements of the problem.
> +     *
> +     * At the time of writing (August 2026), the believed safe sequence is:
> +     *   0x01000370 -> [0x01000380...0x010003f3] -> 0x01000405 -> any later
> +     *
> +     * Disallow known-unsafe loads while permitting believed-safe loads.  For
> +     * GNR, this allows multi-hop loading to get up to the latest.
> +     */
> +    if ( boot_cpu_data.vfm == INTEL_GRANITERAPIDS_X &&
> +         boot_cpu_data.stepping == 1 && (cpu_sig->pf & 0x95) &&
> +         ((cpu_sig->rev < 0x01000380 && mc->rev >= 0x01000405) ||
> +          (cpu_sig->rev < 0x01000405 && mc->rev >  0x01000405)) )

Given the logic, I don't think the CPU needs to strictly be in version
0x01000370 to update to the [0x01000380...0x010003f3] range?

Maybe you want to replace 0x01000370 with "any previous" to match the
semantics used in the tail of the sequence with "any later".

In any case:

Reviewed-by: Roger Pau Monné <roger@xxxxxxxxxxxxxx>

Thanks, Roger.