Received-SPF: pass (google.com: domain of linux-kernel+bounces-199713-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Mon, 3 Jun 2024 16:01:22 -0500
From: John Allen <john.allen@amd.com>
To: Borislav Petkov <bp@alien8.de>
Cc: linux-edac@vger.kernel.org, tony.luck@intel.com, yazen.ghannam@amd.com,
	linux-kernel@vger.kernel.org, avadhut.naik@amd.com,
	muralidhara.mk@amd.com
Subject: Re: [PATCH v4 4/4] RAS/AMD/ATL: Implement DF 4.5 NP2 denormalization
Message-ID: <Zl4vIkNNzP+cKSYF@AUS-L1-JOHALLEN.amd.com>
References: <20240506154605.71814-1-john.allen@amd.com>
 <20240506154605.71814-5-john.allen@amd.com>
 <20240528102344.GGZlWwsJzibnQfuNYW@fat_crate.local>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240528102344.GGZlWwsJzibnQfuNYW@fat_crate.local>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?FXTkJNXvrWpnOtA6HJBOePe+vcirlDSpbfIBhJkmx43+/4cZ4KWVmcfma7CH?=
 =?us-ascii?Q?R2U5BH2shS0wbq3DkVyxGubi5fhOgOQKRw0d8585YWf6jJyIClY0B/7YuqxA?=
 =?us-ascii?Q?ltrJyhHksn5QRIMhPLODfh+Y3al1xLg+BBrcJxjwdzTgKgyp50diDm+1gojW?=
 =?us-ascii?Q?/3lezSieeVlOOf7qootethqLTSDbF4V7TZlgqDUNzY1v+ja2kKM5eL12Imo9?=
 =?us-ascii?Q?JWwg8ZfC9lEvtKCI+mP5auj6WB7H0l0Mkes6l6/VVPvxdJlw5P5pqtS+jpaE?=
 =?us-ascii?Q?TadMS4o/cmwwlVIDcnxiRxDXvE+G8RRrKLusJjm6c7wbT8OPDcLlcOvUlvQc?=
 =?us-ascii?Q?RKnEukclx95PNj5UqslNXfQjvrkSeUp/fM4pfJ+9Tx3FrOy7VCPJr+YNsG24?=
 =?us-ascii?Q?CSAKVoSJigKXxpow74p5nMaAfaMTnUHRew8KLjmNe053juMjPqLKnZIFPgf9?=
 =?us-ascii?Q?V/shMwc4b0UR3Yw0xIpEApuPeB/sCOWozNAUKOMSHUEA17JuX+1zfJoesQqy?=
 =?us-ascii?Q?igIksKofdaEaw5PXqK31ophcTW2StcB5vuj9hmqUGobD/NxuAfgOh5wRxzcs?=
 =?us-ascii?Q?bDOM5UfncInw/7cpPMIlg62cM0W/NEhWE++nfTDGPGC9WEq36tuOEaYuDlR5?=
 =?us-ascii?Q?3mmTttLn1Too1XSgkQr6dkQOYRTtycSDlhEdrzI0QXnxx+O7O+jskUZuf61/?=
 =?us-ascii?Q?fen10QBgT8yttEoyYNkNxMYIzPeAtOSHn94pU7VhQQkV3yJU17JjuKV6x4jL?=
 =?us-ascii?Q?mJoZ1awpWpep7EeSv8yVxh0GaK57yFuz9P5WjDo5BwBr5R5LpJzBA8O7no63?=
 =?us-ascii?Q?8wOPVYXJ13AfyV1fTvPk75sZcAnXHMvifKfRjbcvKJdr8b9kmD3nfLznYCAS?=
 =?us-ascii?Q?12Ik1WfS952DZ+VK7UspTYy4AFyn680bVtvxSgR/hsZ6RdoY5p49cOtRvB1t?=
 =?us-ascii?Q?iOVz/iCF1gSYhPxg5rEA+RcCEus6mJaq6mV7sO3a7ow8RFY6+J16nRaYMu3g?=
 =?us-ascii?Q?kIr8Aayf2+E9OMC2Q4WMcT1oKK+IqKBve47sQBDmZbCQVlXnciuppUPH76yJ?=
 =?us-ascii?Q?hu/q54OrWTrY2q3TAKbEtPhjnrBfApjwMYjqnJRaRulCo/ZM9UIFSUWTrREp?=
 =?us-ascii?Q?YZ3N+uR9o3vewLNKHQW0j9GzOMnDwsYb9xI7cYpT6PxMXQEnw5Pl/0tRCh69?=
 =?us-ascii?Q?tO/4YkkPMFJ0GzB78HNxMX+ZZnE41+lx/hA7ggo9RGLAAVrTk99dZbEyIrHp?=
 =?us-ascii?Q?NjKPGxMEW6KvBflHGnkeuwxJ8cgvzKTImIYAxEn3t/LpXhEz8yQqNjv3fDWP?=
 =?us-ascii?Q?Rq83Yw59Wd0Og1+/xyez5tpK2GT8dm28/nNffao9d3yL9S5of+c0/V+pDbah?=
 =?us-ascii?Q?YtxhPn2CEj48YQB52l71UjqxWh8PeMalt1XYUbQW+r9y3bxWR4ezf1iePO9U?=
 =?us-ascii?Q?DAGauGvED3ZRk4d9QAWUaVBVGYKnAtZJHPr7LCZdSqTyxsyXUeGtktgrWMst?=
 =?us-ascii?Q?hH0dk0TdFftUT4bXaj8uCTu/XhCrU79kquoZ6Uigvpf0PvKLB0xEJuZMJK0R?=
 =?us-ascii?Q?3d3mWzHlPMkXuVpqkIbY+okWLyBYw5nHY0mG/hFi?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 91e4e40d-abf4-42ac-ce55-08dc84105549
X-MS-Exchange-CrossTenant-AuthSource: BL1PR12MB5995.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Jun 2024 21:01:27.0261
 (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: Cgbk6fSrhdGiEMZcdriMmUxwR1kt00qIdjtkgfcoL5LI9fAUcfYimdcQ72/L/ri/NCoU5w2uXFHNZitsKAmLMA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR12MB7751

On Tue, May 28, 2024 at 12:23:44PM +0200, Borislav Petkov wrote:
> On Mon, May 06, 2024 at 03:46:05PM +0000, John Allen wrote:
> > +	switch (ctx->map.intlv_mode) {
> > +	case DF4p5_NPS0_24CHAN_1K_HASH:
> > +		cs_id = FIELD_GET(GENMASK_ULL(63, 13), denorm_ctx->current_spa) << 3;
> > +		cs_id %= denorm_ctx->mod_value;
> > +		cs_id <<= 2;
> > +		cs_id |= (hash_pa9 | (hash_pa12 << 1));
> > +		cs_id |= hash_pa8 << df_cfg.socket_id_shift;
> > +		break;
> 
> Newlines after those "break"s pls.
> 
> > +	case DF4p5_NPS0_24CHAN_2K_HASH:
> > +		cs_id = FIELD_GET(GENMASK_ULL(63, 14), denorm_ctx->current_spa) << 4;
> > +		cs_id %= denorm_ctx->mod_value;
> > +		cs_id <<= 2;
> > +		cs_id |= (hash_pa12 | (hash_pa13 << 1));
> > +		cs_id |= hash_pa8 << df_cfg.socket_id_shift;
> > +		break;
> 
> ...
> 
> > +static u64 normalize_addr_df4p5_np2(struct addr_ctx *ctx, struct df4p5_denorm_ctx *denorm_ctx,
> > +				    u64 addr)
> > +{
> > +	u64 temp_addr_a = 0, temp_addr_b = 0;
> > +
> > +	switch (ctx->map.intlv_mode) {
> > +	case DF4p5_NPS0_24CHAN_1K_HASH:
> > +	case DF4p5_NPS1_12CHAN_1K_HASH:
> > +	case DF4p5_NPS2_6CHAN_1K_HASH:
> > +	case DF4p5_NPS4_3CHAN_1K_HASH:
> > +	case DF4p5_NPS1_10CHAN_1K_HASH:
> > +	case DF4p5_NPS2_5CHAN_1K_HASH:
> > +		temp_addr_a = FIELD_GET(GENMASK_ULL(11, 10), addr) << 8;
> > +		break;
> 
> Also linebreaks pls.
> 
> Please change this in all your patches.
> 
> > diff --git a/drivers/ras/amd/atl/internal.h b/drivers/ras/amd/atl/internal.h
> > index 05b870fcb24e..946e36c053c5 100644
> > --- a/drivers/ras/amd/atl/internal.h
> > +++ b/drivers/ras/amd/atl/internal.h
> > @@ -34,6 +34,8 @@
> >  #define DF_DRAM_BASE_LIMIT_LSB		28
> >  #define MI300_DRAM_LIMIT_LSB		20
> >  
> > +#define INVALID_SPA ~0ULL
> 
> No lazy definitions pls:
> 
> drivers/ras/amd/atl/internal.h:37:#define INVALID_SPA ~0ULL
> drivers/ras/amd/fmpm.c:119:#define INVALID_SPA  ~0ULL
> 
> Unify them.

Is there a particular existing header that you suggest we pull this into
or just create a new one under drivers/ras/amd?

Thanks,
John

> 
> > +
> >  enum df_revisions {
> >  	UNKNOWN,
> >  	DF2,
> > @@ -90,6 +92,44 @@ enum intlv_modes {
> >  	DF4p5_NPS1_10CHAN_2K_HASH	= 0x49,
> >  };
> >  
> > +struct df4p5_denorm_ctx {
> > +	/* perm_shift: Indicates the number of "lost" bits. This will be 1, 2, or 3. */
> 
> No need to repeat the variable name in the comment.
> 
> ...
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette