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

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

On Mon, Mar 23, 2026 at 12:38:48PM +0100, Jan Beulich wrote:
> On 23.03.2026 12:16, Roger Pau Monné wrote:
> > 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().
> 
> Hmm, right you are. Had I been asked, I would have confirmed that I checked
> the code past the "enable_cpu" label, but clearly I must not have, or I was
> blind at that time. Let me strip that out.
> 
> >> --- 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?
> 
> To be honest, I would put the question the other way around: Is it really
> okay to do it this late for APs (during boot also for the BSP [1])? Iirc
> an #MC prior to mcheck_init() is going to be deadly to the system. Moving
> it earlier may, however, be a more intrusive change.

We might want to at least add a note to document this asymmetric
initialization between the BSP and the APs at least?

I would be perfectly happy with moving this earlier, and it needs to
be consistent between the APs and the BSP.

Thanks, Roger.