linux-kernel - Re: [PATCH v3 2/2] EDAC/amd64: Add new register offset support and related changes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 31 Jan 2022 22:43:37 +0000
From:   Yazen Ghannam <yazen.ghannam@....com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     linux-edac@...r.kernel.org, linux-kernel@...r.kernel.org,
        mchehab@...nel.org, tony.luck@...el.com, james.morse@....com,
        rric@...nel.org, Smita.KoralahalliChannabasappa@....com,
        william.roche@...cle.com
Subject: Re: [PATCH v3 2/2] EDAC/amd64: Add new register offset support and
 related changes

On Fri, Jan 21, 2022 at 01:25:21PM +0100, Borislav Petkov wrote:
> On Tue, Dec 28, 2021 at 08:06:15PM +0000, Yazen Ghannam wrote:
> > diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> > index 4db92c77276f..a299c361a904 100644
> > --- a/drivers/edac/amd64_edac.c
> > +++ b/drivers/edac/amd64_edac.c
> > @@ -15,6 +15,31 @@ static struct msr __percpu *msrs;
> >  
> >  static struct amd64_family_type *fam_type;
> >  
> > +/* Family flag helpers */
> > +static inline u64 get_addr_cfg(void)
> > +{
> > +	if (fam_type->flags.zn_regs_v2)
> > +		return UMCCH_ADDR_CFG_DDR5;
> > +
> > +	return UMCCH_ADDR_CFG;
> > +}
> > +
> > +static inline u64 get_addr_mask_sec(void)
> > +{
> > +	if (fam_type->flags.zn_regs_v2)
> > +		return UMCCH_ADDR_MASK_SEC_DDR5;
> > +
> > +	return UMCCH_ADDR_MASK_SEC;
> > +}
> > +
> > +static inline u64 get_dimm_cfg(void)
> > +{
> > +	if (fam_type->flags.zn_regs_v2)
> > +		return UMCCH_DIMM_CFG_DDR5;
> > +
> > +	return UMCCH_DIMM_CFG;
> > +}
> 
> Yeah, you can do it either this way and have a lot of small functions
> or you can do what I did with mca_msr_reg() which is a single mapping
> function you then use everywhere.
> 
> Your call.
>

Yes, I'll do this.
 
> > +
> >  /* Per-node stuff */
> >  static struct ecc_settings **ecc_stngs;
> >  
> > @@ -1429,8 +1454,10 @@ static void __dump_misc_regs_df(struct amd64_pvt *pvt)
> >  		edac_dbg(1, "UMC%d x16 DIMMs present: %s\n",
> >  				i, (umc->dimm_cfg & BIT(7)) ? "yes" : "no");
> >  
> > -		if (pvt->dram_type == MEM_LRDDR4) {
> > -			amd_smn_read(pvt->mc_node_id, umc_base + UMCCH_ADDR_CFG, &tmp);
> > +		if (pvt->dram_type == MEM_LRDDR4 || pvt->dram_type == MEM_LRDDR5) {
> 
> This still keeps the ->dram_type per pvt, which is per memory controller
> in amd64_edac nomenclature.
> 
> But AFAIR, we said last time that the DRAM type is per UMC now, as you
> do in the previous patch.
> 
> Which means, you either have to test umc->dimm_cfg to get the DRAM type
> here or push ->dram_type into the umc struct...
>

Yep, you're right. I'll cache the dram_type in the umc struct.

Thanks,
Yazen