Move __mcheck_cpu_init_generic() after __mcheck_cpu_init_prepare_banks()
so that MCA is enabled after the first MCA polling event.

This brings the MCA init flow closer to what is described in the x86 docs.

The AMD PPRs say
  "The operating system must initialize the MCA_CONFIG registers prior
  to initialization of the MCA_CTL registers.

  The MCA_CTL registers must be initialized prior to enabling the error
  reporting banks in MCG_CTL".

However, the Intel SDM "Machine-Check Initialization Pseudocode" says
MCG_CTL first then MCi_CTL.

But both agree that CR4.MCE should be set last.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
---

Notes:
    Link:
    https://lore.kernel.org/r/52a37afe-c41b-4f20-bbdc-bddc3ae26260@suse.com
    
    v4->v5:
    * New in v5.

 arch/x86/kernel/cpu/mce/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 0326fbb83adc..9cbf9e8c8060 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -2272,9 +2272,9 @@ void mcheck_cpu_init(struct cpuinfo_x86 *c)
 
 	mca_cfg.initialized = 1;
 
-	__mcheck_cpu_init_generic();
 	__mcheck_cpu_init_vendor(c);
 	__mcheck_cpu_init_prepare_banks();
+	__mcheck_cpu_init_generic();
 	__mcheck_cpu_setup_timer();
 }
 
@@ -2440,9 +2440,9 @@ static void mce_syscore_shutdown(void)
  */
 static void mce_syscore_resume(void)
 {
-	__mcheck_cpu_init_generic();
 	__mcheck_cpu_init_vendor(raw_cpu_ptr(&cpu_info));
 	__mcheck_cpu_init_prepare_banks();
+	__mcheck_cpu_init_generic();
 }
 
 static struct syscore_ops mce_syscore_ops = {
@@ -2459,8 +2459,8 @@ static void mce_cpu_restart(void *data)
 {
 	if (!mce_available(raw_cpu_ptr(&cpu_info)))
 		return;
-	__mcheck_cpu_init_generic();
 	__mcheck_cpu_init_prepare_banks();
+	__mcheck_cpu_init_generic();
 	__mcheck_cpu_init_timer();
 }
 

-- 
2.51.0

On Mon, Aug 25, 2025 at 05:33:04PM +0000, Yazen Ghannam wrote:
> Move __mcheck_cpu_init_generic() after __mcheck_cpu_init_prepare_banks()
> so that MCA is enabled after the first MCA polling event.
> 
> This brings the MCA init flow closer to what is described in the x86 docs.
> 
> The AMD PPRs say
>   "The operating system must initialize the MCA_CONFIG registers prior
>   to initialization of the MCA_CTL registers.
> 
>   The MCA_CTL registers must be initialized prior to enabling the error
>   reporting banks in MCG_CTL".
> 
> However, the Intel SDM "Machine-Check Initialization Pseudocode" says
> MCG_CTL first then MCi_CTL.
> 
> But both agree that CR4.MCE should be set last.
> 
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> ---
> 
> Notes:
>     Link:
>     https://lore.kernel.org/r/52a37afe-c41b-4f20-bbdc-bddc3ae26260@suse.com
>     
>     v4->v5:
>     * New in v5.
> 
>  arch/x86/kernel/cpu/mce/core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
> index 0326fbb83adc..9cbf9e8c8060 100644
> --- a/arch/x86/kernel/cpu/mce/core.c
> +++ b/arch/x86/kernel/cpu/mce/core.c
> @@ -2272,9 +2272,9 @@ void mcheck_cpu_init(struct cpuinfo_x86 *c)
>  
>  	mca_cfg.initialized = 1;
>  
> -	__mcheck_cpu_init_generic();
>  	__mcheck_cpu_init_vendor(c);
>  	__mcheck_cpu_init_prepare_banks();
> +	__mcheck_cpu_init_generic();

With that flow we have now:

	1. MCA_CTL
	2. CR4.MCE
	3. MCG_CTL

So this is nothing like in the commit message above and the MC*_CTL flow is
not what the SDM suggests and CR4.MCE is not last.

So what's the point even here?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

On Mon, Sep 01, 2025 at 07:07:41PM +0200, Borislav Petkov wrote:
> On Mon, Aug 25, 2025 at 05:33:04PM +0000, Yazen Ghannam wrote:
> > Move __mcheck_cpu_init_generic() after __mcheck_cpu_init_prepare_banks()
> > so that MCA is enabled after the first MCA polling event.
> > 
> > This brings the MCA init flow closer to what is described in the x86 docs.
> > 
> > The AMD PPRs say
> >   "The operating system must initialize the MCA_CONFIG registers prior
> >   to initialization of the MCA_CTL registers.
> > 
> >   The MCA_CTL registers must be initialized prior to enabling the error
> >   reporting banks in MCG_CTL".
> > 
> > However, the Intel SDM "Machine-Check Initialization Pseudocode" says
> > MCG_CTL first then MCi_CTL.
> > 
> > But both agree that CR4.MCE should be set last.
> > 
> > Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> > ---
> > 
> > Notes:
> >     Link:
> >     https://lore.kernel.org/r/52a37afe-c41b-4f20-bbdc-bddc3ae26260@suse.com
> >     
> >     v4->v5:
> >     * New in v5.
> > 
> >  arch/x86/kernel/cpu/mce/core.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
 > index 0326fbb83adc..9cbf9e8c8060 100644
> > --- a/arch/x86/kernel/cpu/mce/core.c
> > +++ b/arch/x86/kernel/cpu/mce/core.c
> > @@ -2272,9 +2272,9 @@ void mcheck_cpu_init(struct cpuinfo_x86 *c)
> >  
> >  	mca_cfg.initialized = 1;
> >  
> > -	__mcheck_cpu_init_generic();
> >  	__mcheck_cpu_init_vendor(c);
> >  	__mcheck_cpu_init_prepare_banks();
> > +	__mcheck_cpu_init_generic();
> 
> With that flow we have now:
> 
> 	1. MCA_CTL
> 	2. CR4.MCE
> 	3. MCG_CTL
> 
> So this is nothing like in the commit message above and the MC*_CTL flow is
> not what the SDM suggests and CR4.MCE is not last.
> 
> So what's the point even here?
> 

The main point is to initialize MCA after polling for leftover errors.

You're right that this change doesn't bring the code in sync with the
docs. I'll work on it more.

Moving CR4.MCE last should be okay, but deciding when to do MCG_CTL
needs some more thought. Maybe we can have an early call for Intel and
a late call for AMD?

Thanks,
Yazen

On Tue, Sep 02, 2025 at 09:30:15AM -0400, Yazen Ghannam wrote:
> Moving CR4.MCE last should be okay, but deciding when to do MCG_CTL
> needs some more thought. Maybe we can have an early call for Intel and
> a late call for AMD?

I'd say move only the CR4 write and leave everything else as-is. It has worked
fine until now. Unless someone comes hands-a-waving that we need to fix it all
of a sudden...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

On Tue, Sep 02, 2025 at 06:26:57PM +0200, Borislav Petkov wrote:
> On Tue, Sep 02, 2025 at 09:30:15AM -0400, Yazen Ghannam wrote:
> > Moving CR4.MCE last should be okay, but deciding when to do MCG_CTL
> > needs some more thought. Maybe we can have an early call for Intel and
> > a late call for AMD?
> 
> I'd say move only the CR4 write and leave everything else as-is. It has worked
> fine until now. Unless someone comes hands-a-waving that we need to fix it all
> of a sudden...
> 

Okay, will do.

Thanks,
Yazen