Re: [PATCH v3 3/4] x86/mce, EDAC/mce_amd: Cache MCA_CONFIG[McaX] in struct mce_bank

From: "Koralahalli Channabasappa, Smita" <skoralah@amd.com>
To: Borislav Petkov <bp@alien8.de>,
	Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
Cc: x86@kernel.org, linux-edac@vger.kernel.org,
	linux-kernel@vger.kernel.org, Tony Luck <tony.luck@intel.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H . Peter Anvin" <hpa@zytor.com>,
	James Morse <james.morse@arm.com>,
	Robert Richter <rric@kernel.org>,
	Yazen Ghannam <yazen.ghannam@amd.com>
Subject: Re: [PATCH v3 3/4] x86/mce, EDAC/mce_amd: Cache MCA_CONFIG[McaX] in struct mce_bank
Date: Tue, 22 Feb 2022 14:47:44 -0600	[thread overview]
Message-ID: <66a6cc6e-55fa-45c4-1387-ff9d055eec23@amd.com> (raw)
In-Reply-To: <YhUC1wW0mxC/Dmpv@zn.tnic>

On 2/22/22 9:35 AM, Borislav Petkov wrote:
> On Fri, Feb 11, 2022 at 04:34:41PM -0600, Smita Koralahalli wrote:
>> Cache the value of MCA_CONFIG[McaX] in the existing mce_bank struct
>> similar to MCA_CONFIG[McaLsbInStatusSupported].
>>
>> This simplifies and eliminates the need to read MCA_CONFIG register each
>> time to check McaX.
> I don't see the point for this change, frankly.
>
> I doubt it is speed because those are not really hot paths.
>
> Code savings ain't either: 5 files changed, 36 insertions(+), 22 deletions(-)
>
> Having yet another exported function to modules if not really necessary
> doesn't make it prettier too.
>
> So if there's no point for it, you can simply drop it.
>
> Thx.
>
Hmm okay. The main thought to come up with this patch was of course speed.
The speed might not matter much when we are trying to configure registers
as in mce/amd.c.

But what do you think of severity? Will this make an impact when handling
panic severity levels? .. mce_severity_amd_smca().

I can drop this patch if it doesn't impact much.

Thanks,
Smita