Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 31 Jan 2022 22:43:37 +0000
From:   Yazen Ghannam <yazen.ghannam@amd.com>
To:     Borislav Petkov <bp@alien8.de>
Cc:     linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
        mchehab@kernel.org, tony.luck@intel.com, james.morse@arm.com,
        rric@kernel.org, Smita.KoralahalliChannabasappa@amd.com,
        william.roche@oracle.com
Subject: Re: [PATCH v3 2/2] EDAC/amd64: Add new register offset support and
 related changes
Message-ID: <YfhmGaLQWaFM1V0n@yaz-ubuntu>
References: <20211228200615.412999-1-yazen.ghannam@amd.com>
 <20211228200615.412999-3-yazen.ghannam@amd.com>
 <YeqmMcOSpjgHwYFD@zn.tnic>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YeqmMcOSpjgHwYFD@zn.tnic>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?ht3I51iDXB43Kmd+Admtxy2Rt1kDm72VSIW7br6OE1QlnXWXiLVmztcAWM0L?=
 =?us-ascii?Q?GphuRKTJRE56QZ2BJmjbeQPcBGhbUeflLy19gTold9by3rM/punsVfUnSD3m?=
 =?us-ascii?Q?5WAR7I68zv1ghLSK0IFlO0WQKI8bAyxDfoD5nEFupBGa/Ut4c57dsJ1xeutB?=
 =?us-ascii?Q?+twCbg0ik/YxXgskVhdO2K8tFNx9thRVl/5bShM/olKCGjZYWz4k1IbZEDCB?=
 =?us-ascii?Q?SlthC6ZpL21jDl+y4GKN8mEF7TsF5kR0m8lRvBZKpqeMt4/ln2ZHZu0x/2kx?=
 =?us-ascii?Q?SLXjKlRCkGZIRgHQE+dwf3WaYfLvH4GdWESBpRH4XjQbQZS2prn3zxptiSrQ?=
 =?us-ascii?Q?c1EgUoNCOlPF2JhIqdJIA7FIkDUbc1l/boZQIgpoBgkGTBN2fK+9ychbzBjD?=
 =?us-ascii?Q?Vkwt0AY+BS8NxOVgpd598Mg50C8ZL3rWSFn4IQhg/9Z4iHD6tK+8+z+PaQsm?=
 =?us-ascii?Q?PoJ2IkZByKt/oVAZlmPUIa+SgqhNM45hgw9kKZnMfqIbnksKIHyHxoTmpM8m?=
 =?us-ascii?Q?Tnsrx+9AiSmBOpYk/vtUiY0IaCh7hiLJM6bScW1LHUX7bLTHHMu4vL2owwT+?=
 =?us-ascii?Q?MmDlq5fXB5uY/ePgl/oQiH7oW5NoIb3S5CICjxW6Ed1IKCuluZbhevnyxuDc?=
 =?us-ascii?Q?jrm9OEoTp1HRmEoul+8sCGteqytS4QZDWgY/jwB4KkHP5Amm/XINpmoXZbYp?=
 =?us-ascii?Q?o6I5es7C8rKQoL6W9cIsUhKkZ8InQmL8Ev9QBUObFPZ9cahVFq1EiD5VfKo0?=
 =?us-ascii?Q?LLMYome2RrbbDwWmC7IuP6ICEiio/asvZJU/D9HQ6GmIS3a/GV/wT4/g576L?=
 =?us-ascii?Q?8SFlFYnZDiPQA02qk4ufdU1nbXhjf+Cx3IBFpmPYQc3dTVSWCzgm7bJSILCJ?=
 =?us-ascii?Q?zB40/c3RSuh9PmAoVno5BOLwqHt5ZKOUzyOh7rjpF6w3O0LVEWBVe3NUB+A/?=
 =?us-ascii?Q?7DALZ7VAkG3jBX48fCmvCEDfPoxjHPpRBOzLMknTnTDg0cujdu/6wt6IOqCn?=
 =?us-ascii?Q?Mt3V7rssCqYgK4gvg+v+s0DBqeFjSfPhZidSMnURcCo21zw6jkWcXJHl7/Sk?=
 =?us-ascii?Q?jKUQOyeD9qg2FlX7RTmmzRVIdsigQ/luEtY82EmxJD2nOfZECCG7PDDwunAz?=
 =?us-ascii?Q?5TRs20nSQqFQ0BCdNKPfi3tYoCjsv7uuCENXTqeae7sEzpg31S9PfVv5xwep?=
 =?us-ascii?Q?GcvAUJ+qA9B4Eno+KY0DfJ4XDSXd8KUxAbu+w5omEH5vlm/xHThYJ+LQdBYm?=
 =?us-ascii?Q?lO59Yqkseu2VACIDDUqZ2S6/5ShcuJ1LTnH7FUMrZZXYorjNoC1X01XMCORo?=
 =?us-ascii?Q?IfjQF826O07Tt6oSpeV77YJnzX95ynGo7KEiV8UV4zYmGdPD+0z+JJH9fGB9?=
 =?us-ascii?Q?gIEtI0SCThrxu8YUP88P/ICub13loRsBRb0VX2tVbMaM9gLZOtLHPzH7SBIW?=
 =?us-ascii?Q?ai5sjAVww++5HDXOoDamfYceMSqk1GFPA8zS58HfzxgKOpX3WNsFhPNlr8Tf?=
 =?us-ascii?Q?OCXdmLbCHsi4+aLLnR4BtpJYVtA/SBBnNFyQrfEwOVuPZ3maRvih/aYMLz+s?=
 =?us-ascii?Q?QpepZRIBG7/UiDJwIZ/JHtYaUr3RF4yi5b4cW5uNgLhHCBxVV6cOJRgMq834?=
 =?us-ascii?Q?0mwvNYYtI0QpKr+y0x1vyBY=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2afe76d7-5db2-435c-7678-08d9e50b214c
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3108.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jan 2022 22:43:41.9601
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 2FLHctE8KUA+J+mDG6oEChYBD97AE+XaWiSXvvLJABC4clEto2wWCQrG4I4md8/804ewtULOw5cIe1JOtVjBDg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR12MB1325
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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