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] [thread-next>] [day] [month] [year] [list]

Message-ID: <YeqmMcOSpjgHwYFD@zn.tnic>
Date:   Fri, 21 Jan 2022 13:25:21 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Yazen Ghannam <yazen.ghannam@....com>
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 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.

> +
>  /* 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...

-- 
Regards/Gruss,
    Boris.

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