Re: [PATCH 8/9] x86/mce/amd: Enable interrupt vectors once per-CPU on SMCA systems

[Yazen Ghannam <yazen.ghannam@amd.com>]

On Tue, Jun 04, 2024 at 05:46:35PM +0200, Borislav Petkov wrote:
> On Thu, May 23, 2024 at 10:56:40AM -0500, Yazen Ghannam wrote:
> >  static bool thresholding_irq_en;
> >  static DEFINE_PER_CPU_READ_MOSTLY(mce_banks_t, mce_thr_intr_banks);
> >  static DEFINE_PER_CPU_READ_MOSTLY(mce_banks_t, mce_dfr_intr_banks);
> > +static DEFINE_PER_CPU_READ_MOSTLY(bool, smca_thr_intr_enabled);
> > +static DEFINE_PER_CPU_READ_MOSTLY(bool, smca_dfr_intr_enabled);
> 
> So before you add those, we already have:
> 
> static DEFINE_PER_CPU_READ_MOSTLY(struct smca_bank[MAX_NR_BANKS], smca_banks);
> static DEFINE_PER_CPU_READ_MOSTLY(u8[N_SMCA_BANK_TYPES], smca_bank_counts);
> static DEFINE_PER_CPU(struct threshold_bank **, threshold_banks);
> static DEFINE_PER_CPU(u64, bank_map);
> static DEFINE_PER_CPU(u64, smca_misc_banks_map);
> 
> Please think of a proper struct which collects all that info in the
> smallest possible format and unify everything.
> 
> It is a mess currently.
> 

Agreed. I actually want to remove almost all of those. You can see the
goal here: https://github.com/AMDESE/linux/tree/wip-mca

Of course, this is out-of-date. I'll collect any new variables into a
struct so we (hopefully) don't repeat history. :P

> > +/*
> > + * Enable the APIC LVT interrupt vectors once per-CPU. This should be done before hardware is
> > + * ready to send interrupts.
> > + *
> > + * Individual error sources are enabled later during per-bank init.
> > + */
> > +static void smca_enable_interrupt_vectors(struct cpuinfo_x86 *c)
> > +{
> > +	u8 thr_offset, dfr_offset;
> > +	u64 mca_intr_cfg;
> > +
> > +	if (!mce_flags.smca || !mce_flags.succor)
> > +		return;
> > +
> > +	if (c == &boot_cpu_data) {
> > +		mce_threshold_vector		= amd_threshold_interrupt;
> > +		deferred_error_int_vector	= amd_deferred_error_interrupt;
> > +	}
> 
> Nah, this should be done differently: you define a function
> cpu_mca_init() which you call from early_identify_cpu(). In it, you do
> the proper checks and assign those two vectors above. That in
> a pre-patch.
> 
> Then, the rest becomes per-CPU code which you simply run in
> mce_amd_feature_init(), dilligently, one thing after the other.
> 
> And then you don't need smca_{dfr,thr}_intr_enabled anymore because you
> know that after having run setup_APIC_eilvt().
> 
> IOW, mce_amd_feature_init() does *all* per-CPU MCA init on AMD and it is
> all concentrated in one place and not spread around.
> 
> I think this should be a much better cleanup.
>

Okay, will work on it.

I have a couple of other "init cleanup" patches from a previous
discussion. I'll fold those into this set.

Thanks,
Yazen