linux-kernel - Re: [PATCH 1/3] x86/MCE, EDAC/mce_amd: Add support for new MCA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 24 Oct 2022 23:23:31 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     "Luck, Tony" <tony.luck@...el.com>
Cc:     Yazen Ghannam <yazen.ghannam@....com>,
        "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "Smita.KoralahalliChannabasappa@....com" 
        <Smita.KoralahalliChannabasappa@....com>
Subject: Re: [PATCH 1/3] x86/MCE, EDAC/mce_amd: Add support for new
 MCA_SYND{1,2} registers

On Mon, Oct 24, 2022 at 09:08:54PM +0000, Luck, Tony wrote:
> We already have:
> 
>         __u8  cpuvendor;        /* Kernel's X86_VENDOR enum */
> 
> So the mce structure contains which vendor created it.
> 
> > I guess a u8 vendor_info[VENDOR_INFO_SIZE] or so which we can extend
> > later if needed.
> 
> Extending is hard because we already boxed in the two AMD specific fields
> with some generic fields that follow (ppin & microcode).
> 
> But we could change the current form to be something like:
> 
> 	union {
> 		struct vendor_info {
> 			__u64	vendor_info[2];
> 		};
> 		struct vendor_amd_info {
> 			__u64 synd;             /* MCA_SYND MSR: only valid on SMCA systems */
> 			__u64 ipid;             /* MCA_IPID MSR: only valid on SMCA systems */
> 		};
> 	};
> 
> to make it clear that these 16 bytes are up for grabs to be re-interpreted based on
> the value of "cpuvendor" (and possibly also "cpuid" if a vendor wants different data
> logged for different models).

Yes, there's that. But this thread contains a patch which wants to add
two more fields.

So the above you're proposing we could do if you wanna reuse that info
on Intel.

But again, there's this other question how to add a *new* vendor_info
at the *end* of the structure which can be shared too *and* which could
potentially be enlarged.

And, if we do

	struct mce;
	vendor_info;
	<field shared by the two vendors>

then we're boxing in that vendor_info again and we cannot enlarge it
either.

That's why I'm proposing this prepended length in front of the
vendor_info field so that it can be extended properly in the future
and new shared members can also be added and this whole scheme can be
forward-compatible, so to speak, without having to cast anything in
stone.

-- 
Regards/Gruss,
    Boris.

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