On Mon, Aug 25, 2025 at 08:19:38PM +0200, Borislav Petkov wrote: > On Mon, Aug 25, 2025 at 05:33:00PM +0000, Yazen Ghannam wrote: > > The MCx_MISC0[BlkPtr] field was used on legacy systems to hold a > > register offset for the next MCx_MISC* register. In this way, an > > implementation-specific number of registers can be discovered at > > runtime. > > > > The MCAX/SMCA register space simplifies this by always including > > the MCx_MISC[1-4] registers. The MCx_MISC0[BlkPtr] field is used to > > indicate (true/false) whether any MCx_MISC[1-4] registers are present. > > > > Currently, MCx_MISC0[BlkPtr] is checked early and cached to be used > > during sysfs init later. This is unnecessary as the MCx_MISC0 register > > is read again later anyway. > > > > Remove the smca_banks_map variable as it is effectively redundant, and > > use a direct register/bit check instead. > > > > Reviewed-by: Qiuxu Zhuo <qiuxu.zhuo@xxxxxxxxx> > > Tested-by: Tony Luck <tony.luck@xxxxxxxxx> > > Reviewed-by: Tony Luck <tony.luck@xxxxxxxxx> > > Signed-off-by: Yazen Ghannam <yazen.ghannam@xxxxxxx> > > --- > > > > Notes: > > Link: > > https://lore.kernel.org/r/20250624-wip-mca-updates-v4-7-236dd74f645f@xxxxxxx > > > > v4->v5: > > * Keep MCx_MISC0[BlkPtr] check to be compliant with uarch. > > I'm not sure I understand what that means...? I completely removed the check below in previous revisions. But I put it back to make sure we follow the microarchitecture guidelines, i.e. the procedure(s) in documentation (APM, PPR, etc.). if (!(low & MASK_BLKPTR_LO)) return 0; > > Anyway, some more cleanup ontop: > > --- > diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c > index 580682af432d..7e36bc0d0e6c 100644 > --- a/arch/x86/kernel/cpu/mce/amd.c > +++ b/arch/x86/kernel/cpu/mce/amd.c > @@ -498,17 +498,6 @@ static void deferred_error_interrupt_enable(struct cpuinfo_x86 *c) > wrmsr(MSR_CU_DEF_ERR, low, high); > } > > -static u32 smca_get_block_address(unsigned int bank, unsigned int block, u32 low) > -{ > - if (!block) > - return MSR_AMD64_SMCA_MCx_MISC(bank); > - > - if (!(low & MASK_BLKPTR_LO)) > - return 0; > - > - return MSR_AMD64_SMCA_MCx_MISCy(bank, block - 1); > -} > - > static u32 get_block_address(u32 current_addr, u32 low, u32 high, > unsigned int bank, unsigned int block, > unsigned int cpu) > @@ -518,8 +507,15 @@ static u32 get_block_address(u32 current_addr, u32 low, u32 high, > if ((bank >= per_cpu(mce_num_banks, cpu)) || (block >= NR_BLOCKS)) > return addr; > > - if (mce_flags.smca) > - return smca_get_block_address(bank, block, low); > + if (mce_flags.smca) { > + if (!block) > + return MSR_AMD64_SMCA_MCx_MISC(bank); > + > + if (!(low & MASK_BLKPTR_LO)) > + return 0; > + > + return MSR_AMD64_SMCA_MCx_MISCy(bank, block - 1); > + } > > /* Fall back to method we used for older processors: */ > switch (block) { > > > -- Looks good to me. Thanks, Yazen
