Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 18 Jan 2023 18:31:10 +0000
From:   Yazen Ghannam <yazen.ghannam@amd.com>
To:     Ingo Molnar <mingo@kernel.org>
Cc:     Avadhut Naik <avadnaik@amd.com>, linux-edac@vger.kernel.org,
        x86@kernel.org, linux-kernel@vger.kernel.org, bp@alien8.de,
        tony.luck@intel.com
Subject: Re: [PATCH v1 3/3] x86/MCE/AMD: Handle reassigned bit definitions
 for CS SMCA
Message-ID: <Y8g67l4GkCEwSOg9@yaz-fattaah>
References: <20230116191102.4226-1-avadnaik@amd.com>
 <20230116191102.4226-4-avadnaik@amd.com>
 <Y8Zo+hj8qxYpZdAl@gmail.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y8Zo+hj8qxYpZdAl@gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?TItCboSrgWBCnvBcQhNA3l4J3Fa98HfsgZ0qbWLWfspAmVbocP9OChX1PGKa?=
 =?us-ascii?Q?1uwgLc/d7Q23C1SovuyouB2nE1tOa3nueP08vwkBopqSlgaCB+xOX/MoDiN2?=
 =?us-ascii?Q?+mr4qilWI3qzdJLeWEuue84KTgDZto+SEIOkXK8lHgKkyQ+i/edaGnLyTHid?=
 =?us-ascii?Q?wXUuauZ/rHpUcbryd3reY7/cEStqxoosUJFQJ+Hqnc/CfW0MYO6ZluHGZEt4?=
 =?us-ascii?Q?fGgzaAAR77gGpnC/85LuWolWZZyExo8HV6FalONgAUyk7Cp282MPBxujjPuW?=
 =?us-ascii?Q?xTp/HJuEZrrUIQkVkP6l+gZmW6Bh2TYWf/IuLIPX536Wkf/0fE6xkY+1BCvV?=
 =?us-ascii?Q?PA0uRbDuiLFLBb1PdSuQmhUdzrLpYe6+nUJ5mW+DxfVrVCEN21mqLu83AgoU?=
 =?us-ascii?Q?F9R1rX86X/J8RQSs2EIYiAwDFiYTz8gjjo3uwBT7jOTfIw0DXFwODXEC9F02?=
 =?us-ascii?Q?kbNWB7yoF6V5bFymKicBikrqVkh6REfRsaiJloCR7ScYbLIT0cVK+c4Pfn7d?=
 =?us-ascii?Q?Azaf/5GewEKp7O2dVD2euZ0NowxeIkJvDrIrgh0yBewZQFDN7DYxK3phZSGK?=
 =?us-ascii?Q?tpaOMxC9ViqCM488LZHAC17Hk8sJNIbW/wONzdrEsSYFGYPo13xakvKadeOG?=
 =?us-ascii?Q?zg3yfoHz4K1i+XxKgSq5eApG8RXzsMB8+5X1H2d/pRUBeiV6S8sob20tHEry?=
 =?us-ascii?Q?OBAJu5cBy1A57jt7DiDgdI5OrluKigsZD0mcC36xRNxmcS/B5uhU4gPqPpfT?=
 =?us-ascii?Q?4I6olwkA2/vOMTfwqKl+ahXpAnNMKS9sYipsXoRiWFFXxJndPlCtnnMR7NPN?=
 =?us-ascii?Q?hCjkn3vjOKPEy8rplr+AzhZFpZZCeKpTgCAer+2eoU/fXhJ5AfX+cp3/xpZL?=
 =?us-ascii?Q?VvSzcWWSRIDwFFhJajVMtaezFDRLzTZexQo3TQ7Hn3gkwn74EzAdzbUx+SPG?=
 =?us-ascii?Q?wZ+/lTuFkNuhUSPJ658knAzc/TRt26bCoziWCAjCMr5MaIq4QKbTGwG0NkeI?=
 =?us-ascii?Q?O3d83SjrpvAS2mbXOuzrXv1ki9Wc6hxx253omTjoWZ61EBdAIcGr2K/V7klH?=
 =?us-ascii?Q?lp34OGBVh8OwRKisGCFeXW4dpMWavsV6gjP2jw1i0puEEzz3PKJqdaq5jgqM?=
 =?us-ascii?Q?5gN6x2ElHy6fl52/jXAUOwH7aQBn55VzINM2GZ6/423y/0V9kqgjaVmdgcoZ?=
 =?us-ascii?Q?BEPMaFWHrppixWGr4uHZmOVXkiXbUAQns95kdm5lH572oazt63vwF1neY9IX?=
 =?us-ascii?Q?K9Gyv8RbqcrMkVFoeey1wJ7ss66nPDawEE3gLnf/iPqQ1mqKbF8iLcEn8HQ7?=
 =?us-ascii?Q?UNo9QbdK4k0uC0nVoFQyh03AOVnZcXTnCFEjOFJsWITXBhV2Zu9x2vNqqGos?=
 =?us-ascii?Q?2u/D8ZkvJ//2kJq79eb6HhVvPpCvEnTWCSml2Nt4H49dSkTF82thiAA6nTDD?=
 =?us-ascii?Q?Rj0kQd9ar90yM+aFDt9CYZ65OPUAgS1TQPjRslJzOvEKjpMIzYp4tGhfLSmf?=
 =?us-ascii?Q?lTYh5Ezk2FeezrLFKW9KaP60yNcLIxln/m9EHMTv8/MSpM/3W50mn1Gp9Cuy?=
 =?us-ascii?Q?P/wEtPRupzhnD0tX0CU31kBWYmXLERcm57R0dVz9?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9c646ad3-6f58-46e4-2f82-08daf9823375
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3108.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Jan 2023 18:31:23.3072
 (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: hiksaGsTV3dbJ6+ucaC486vZkaRlwjpPhaXyIaQRoVz3OG4ZYCdCwzJhHqqqTo69V6903yAXDclsqGEB4e4zGQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB7793
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Jan 17, 2023 at 10:23:09AM +0100, Ingo Molnar wrote:
> 
> * Avadhut Naik <avadnaik@amd.com> wrote:
> 
> > @@ -178,6 +178,8 @@ static const struct smca_hwid smca_hwid_mcatypes[] = {
> >  	{ SMCA_CS,	 HWID_MCATYPE(0x2E, 0x0)	},
> >  	{ SMCA_PIE,	 HWID_MCATYPE(0x2E, 0x1)	},
> >  	{ SMCA_CS_V2,	 HWID_MCATYPE(0x2E, 0x2)	},
> > +	/* Software defined SMCA bank type to handle erratum 1384*/
> > +	{ SMCA_CS_V2_QUIRK, HWID_MCATYPE(0x0, 0x1)  },
> >  
> >  	/* Unified Memory Controller MCA type */
> >  	{ SMCA_UMC,	 HWID_MCATYPE(0x96, 0x0)	},
> > @@ -259,6 +261,17 @@ static inline void fixup_hwid(unsigned int *hwid_mcatype)
> >  
> >  	if (c->x86 == 0x19) {
> >  		switch (c->x86_model) {
> > +		/*
> > +		 * Per Genoa's revision guide, erratum 1384, some SMCA Extended
> > +		 * Error Codes and SMCA Control bits are incorrect for SMCA CS
> > +		 * bank type.
> > +		 */
> > +		case 0x10 ... 0x1F:
> > +		case 0x60 ... 0x7B:
> > +		case 0xA0 ... 0xAF:
> > +			if (*hwid_mcatype == HWID_MCATYPE(0x2E, 0x2))
> > +				*hwid_mcatype = HWID_MCATYPE(0x0, 0x1);
> 
> Why are we open-coding these types?
> 
> Why not use smca_hwid_mcatypes[SMCA_CS_V2], etc.?
> 
> > +			if (*hwid_mcatype == HWID_MCATYPE(0x2E, 0x2))
> > +				*hwid_mcatype = HWID_MCATYPE(0x0, 0x1);
> 
> Ditto.
>

This code runs before matching on a struct with the enums. It seems simplest
to fixup the hardware-provided value before setting things up rather than
changing things later.

Current code flow:
1) Read HWID/McaType values from hardware.
2) Loop through known types and match on the HWID_MCATYPE() tuple.
3) Continue setup based on matched type and its Linux enum.

New code flow:
1) Read HWID/McaType values from hardware.
--> Fixup HWID/McaType values due to any known hardware quirks.
2) Loop through known types and match on the HWID_MCATYPE() tuple.
3) Continue setup based on matched type and its Linux enum.

What do you think?

Also, a further update (maybe you're alluding to?) is get rid of the struct
smca_hwid and just define an enum with "NAME = HWID_MCATYPE(XXX, YYY)".

The struct smca_hwid had another field that was removed, so it seems
unnecessary at the moment.

Thanks,
Yazen