Re: [PATCH 06/14] EDAC/amd64: Add get_mc_regs() into pvt->ops

Yazen Ghannam
2022-04-04T21:39:09Z

On Thu, Mar 31, 2022 at 05:49:49PM +0530, Chatradhi, Naveen Krishna wrote:

...

> > > +static void read_top_mem_registers(struct amd64_pvt *pvt)
> > > +{
> > > + u64 msr_val;
> > > +
> > > + /*
> > > + * Retrieve TOP_MEM and TOP_MEM2; no masking off of reserved bits since
> > > + * those are Read-As-Zero.
> > > + */
> > > + rdmsrl(MSR_K8_TOP_MEM1, pvt->top_mem);
> > > + edac_dbg(0, " TOP_MEM: 0x%016llx\n", pvt->top_mem);
> > > +
> > > + /* Check first whether TOP_MEM2 is enabled: */
> > > + rdmsrl(MSR_AMD64_SYSCFG, msr_val);
> > > + if (msr_val & BIT(21)) {
> > > + rdmsrl(MSR_K8_TOP_MEM2, pvt->top_mem2);
> > > + edac_dbg(0, " TOP_MEM2: 0x%016llx\n", pvt->top_mem2);
> > > + } else {
> > > + edac_dbg(0, " TOP_MEM2 disabled\n");
> > > + }
> > These two values are not used by any code within this module. They are only
> > used in debug print statements and debug sysfs entries. I think this code
> > should just be removed. An expert user who wants to know TOM and TOM2 can use
> > another method, like msr-tools, rather than recompile a kernel with
> > CONFIG_EDAC_DEBUG, etc.
> Make sense, do you think some users have developed scripts to parse the EDAC
> debug logs ?

I'm not aware of any users of this. But this isn't a robust interface like an
ABI.

Thanks,
Yazen