linux-kernel - Re: [PATCH v2 01/29] x86: treewide: Introduce x86_vendor_amd_or

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]

Message-ID: <20250317183421.GHZ9hrLZbBCBqOGXtL@fat_crate.local>
Date: Mon, 17 Mar 2025 19:34:21 +0100
From: Borislav Petkov <bp@...en8.de>
To: "H. Peter Anvin" <hpa@...or.com>
Cc: Dave Hansen <dave.hansen@...el.com>,
	Peter Zijlstra <peterz@...radead.org>,
	"Ahmed S. Darwish" <darwi@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Cooper <andrew.cooper3@...rix.com>,
	John Ogness <john.ogness@...utronix.de>, x86@...nel.org,
	x86-cpuid@...ts.linux.dev, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 01/29] x86: treewide: Introduce
 x86_vendor_amd_or_hygon()

On Mon, Mar 17, 2025 at 11:21:41AM -0700, H. Peter Anvin wrote:
> On March 17, 2025 11:11:11 AM PDT, Dave Hansen <dave.hansen@...el.com> wrote:
> >On 3/17/25 10:48, Peter Zijlstra wrote:
> >> On Mon, Mar 17, 2025 at 06:32:16PM +0100, Borislav Petkov wrote:
> >>> On Mon, Mar 17, 2025 at 05:47:17PM +0100, Ahmed S. Darwish wrote:
> >>>> The pattern to check if an x86 vendor is AMD or HYGON (or not both) is
> >>>> pretty common.  Introduce x86_vendor_amd_or_hygon() at <asm/processor.h>
> >>> So if we need to check "intel too", we do
> >>>
> >>> x86_vendor_amd_or_hygon_or_intel?
> >>>
> >>> Nah, this is silly.
> >> Would it make more sense to have a Zen1 feature and check that instead?
> >> 
> >> Because, IIRC Hygon is simply a Zen1 copy.
> >
> >Some of them can just go away, I think. This, for instance:
> >
> >        if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD &&
> >            boot_cpu_data.x86_vendor != X86_VENDOR_HYGON)
> >                return false;
> >
> >        if (cpu_feature_enabled(X86_FEATURE_ZEN))
> >                return false;
> >
> >Do we even need a vendor check if we're checking X86_FEATURE_ZEN? Is
> >someone setting X86_FEATURE_ZEN on Intel? ;)
> 
> Maybe having a vendor bitmask instead of (or as an alternative to) an enumeration wouldn't be a horrible idea. That way multiple arbitrary vendors can be tested with a single test instruction. It would be pretty crazy to have 32 x86 vendors, never mind 64, without a *dramatic* shift in the ecosystem (there are currently fewer than 32 registered RISC-V G implementations; that doesn't include the myriad of microcontroller class implementations that don't implement the full G [general purpose] feature set – but those are irrelevant for Linux.)

All y'all - you do realize that all that is a bit too much.

What's the problem with having a two-vendor test instead of a silly wrapper
around it?

Or is the need for such a wrapper so important that we absolutely need it?

Did that thing cause a bug somewhere I didn't catch or so and we absolutely
need a wrapper so that we don't forget one or the other?

Also, no one has Hygon hw so we can't just lump those together, strictly
speaking. Whoever lumps them, needs to test on both.

-- 
Regards/Gruss,
    Boris.

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