Re: [PATCH] x86/S3: restore MCE (APs) and add MTRR (BSP) init

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Mar 04, 2026 at 02:39:01PM +0100, Jan Beulich wrote:
> MCE init for APs was broken when CPU feature re-checking was added. MTRR
> (re)init for the BSP looks to never have been there on the resume path.

I'm not sure the statement about MTRR init is correct, AFAICT
mtrr_aps_sync_end() will also re-init the MTRRs on the BSP, and hence
the added mtrr_ap_init() seems to duplicate what's already done in
mtrr_aps_sync_end().

> Fixes: bb502a8ca592 ("x86: check feature flags after resume")
> Reported-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> Sadly we need to go by CPU number (zero vs non-zero) here. See the call
> site of recheck_cpu_features() in enter_state().
> 
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -642,16 +642,21 @@ void identify_cpu(struct cpuinfo_x86 *c)
>                              smp_processor_id());
>       }
>  
> -     if (system_state == SYS_STATE_resume)
> -             return;
> +     if (system_state == SYS_STATE_resume) {
> +             unsigned int cpu = smp_processor_id();
>  
> +             if (cpu)
> +                     mcheck_init(&cpu_data[cpu], false);
> +             else /* Yes, the BSP needs to use the AP function here. */
> +                     mtrr_ap_init();

For symmetry with the BSP path, is it really needed to init MCE so
early for the BSP by calling it directly in enter_state(), or could it
also be done here?

Thanks, Roger.