Re: [PATCH 1/3] x86/MCE, EDAC/mce_amd: Add support for new MCA_SYND{1,2} registers

From: Borislav Petkov <bp@alien8.de>
To: Tony Luck <tony.luck@intel.com>
Cc: Yazen Ghannam <yazen.ghannam@amd.com>,
	linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
	x86@kernel.org, Smita.KoralahalliChannabasappa@amd.com
Subject: Re: [PATCH 1/3] x86/MCE, EDAC/mce_amd: Add support for new MCA_SYND{1,2} registers
Date: Mon, 24 Oct 2022 22:30:30 +0200	[thread overview]
Message-ID: <Y1b15vnE/Pd1U4r8@zn.tnic> (raw)
In-Reply-To: <Y1a/lCVnlTMk8p75@agluck-desk3.sc.intel.com>

On Mon, Oct 24, 2022 at 09:38:44AM -0700, Tony Luck wrote:
> There are still a fair number of users of mcelog, so I think it needs
> to remain in its half-undead state a while longer.

That's not the question - the question is how should vendor-specific
info should be logged so that the struct mce record doesn't get blown
up or ends up containing unused fields on the other vendor.

I.e., how to keep it as small as possible and to share the space there
in the most compact way.

That vendor-specific "space" in there could be used by each vendor
differently. As in this case, Intel doesn't have MCA_SYND{1,2} u64
values. So they could be part of a vendor_info which gets interpreted
based on vendor.

When Intel wants to carry more info through struct mce to userspace,
it can reuse those 2 u64s which are vendor_info but interpret them
differently.

Which then begs the question, how should those get logged etc.

I guess a u8 vendor_info[VENDOR_INFO_SIZE] or so which we can extend
later if needed.

Perhaps prepend it with its length too:

	error_record {
		struct mce;
		unsigned int vendor_info_len;
		u8 vendor_info[vendor_info_len];
	};

For example.

Not saying this is how it should be done - this is just what is swirling
around in my head right now.

Hmm.

-- 
Regards/Gruss,
    Boris.

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