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

[Teddy Astie <teddy.astie@xxxxxxxxxx>]

Le 03/09/2026 à 00:14, Andrew Cooper a écrit :
> 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...
> ---
>   xen/arch/x86/cpu/microcode/intel.c | 31 ++++++++++++++++++++++++++++++
>   1 file changed, 31 insertions(+)
>
> diff --git a/xen/arch/x86/cpu/microcode/intel.c 
> b/xen/arch/x86/cpu/microcode/intel.c
> index c45b00c6b033..2160befd3197 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,35 @@ 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);
> +
> +    /*
> +     * Treat pre-production as always safe - anyone using pre-production
> +     * microcode knows what they are doing, and can keep any resulting pieces.
> +     */
> +    if ( cpu_sig->rev < 0 || mc->rev < 0 )
cpu_sig->rev can't be negative as it's unsigned int.
I guess that's something to fix in our struct cpu_signature and how we 
get it. Though the structure is shared with AMD, and I don't know if it 
also uses "negative" revisions for non-production ucode (PPR doesn't 
tell anything in that regard).
> +        return true;
> +
> +    /*
> +     * GNR98.  Granite Rapids systems hang when loading new ucode on
> +     * sufficiently old firmware.
> +     */
> +    if ( boot_cpu_data.vfm == INTEL_GRANITERAPIDS_X &&
> +         boot_cpu_data.stepping == 1 && (cpu_sig->pf & 0x95) &&
> +         cpu_sig->rev < 0x01000405 &&
> +         mc->rev      > 0x01000405 )
> +    {
> +        printk_once(XENLOG_WARNING
> +                    "microcode: Granite Rapids erratum GNR98 detected.  Skipping 
> ucode 0x%08x\n"
> +                    "microcode: Firmware update recommended\n", mc->rev);
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>   static int cf_check intel_compare(
>       const struct microcode_patch *old, const struct microcode_patch *new)
>   {
> @@ -365,6 +395,7 @@ static struct microcode_patch *cf_check intel_ucode_parse(
>            * one with higher revision.
>            */
>           if ( microcode_fits_cpu(mc) &&
> +             microcode_safe_to_load(mc) &&
>                (!saved || compare_revisions(saved->rev, mc->rev) == NEW_UCODE) )
>               saved = mc;
>   
The rest looks good to me; as my concern is unrelated to change itself 
(and doesn't affect production microcode anyway).
Reviewed-by: Teddy Astie <teddy.astie@xxxxxxxxxx>

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature