linux-kernel - Re: [PATCH 01/74] x86/cpu/vfm: Add/initialize x86_vfm field to struct cpuinfo

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: <Zgr6kT8oULbnmEXx@agluck-desk3>
Date: Mon, 1 Apr 2024 11:18:57 -0700
From: Tony Luck <tony.luck@...el.com>
To: Borislav Petkov <bp@...en8.de>
Cc: "x86@...nel.org" <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 01/74] x86/cpu/vfm: Add/initialize x86_vfm field to
 struct cpuinfo_x86

On Fri, Mar 29, 2024 at 12:40:07PM +0100, Borislav Petkov wrote:
> Because from looking at your set, I don't see a slick way to check
> whether a concrete f/m/s tuple belongs to a range without involved
> checking.
> 
> For example, models:
> 
>                 case 0x30 ... 0x4f:
>                 case 0x60 ... 0x7f:
>                 case 0x90 ... 0x91:
>                 case 0xa0 ... 0xaf:
> 
> are all Zen2. I could do a X86_MATCH_VF_MODEL_RANGE and we even had
> a patch like that at some point but it didn't go in. But even if I did
> that, I'd still need to do x86_match_cpu() instead of the current
> X86_FEATURE_ZEN* checks we're doing.

I realized the problem with ranges is the order I put the bits into the
x86_vfm field. If I swap around to put the vendor in high bits, family
in the middle, model in low bits like this:

struct cpuinfo_x86 {
        union {
                struct {
                        __u8    x86_model;
                        __u8    x86;            /* CPU family */
                        __u8    x86_vendor;     /* CPU vendor */
                        __u8    x86_reserved;
                };
                __u32           x86_vfm;        /* combined vendor, family, model */
        };

Then ranges of models within (or across) familiies can work.  E.g. the
AMD Zen generation checking could be changed from:


	/* Figure out Zen generations: */
	switch (c->x86) {
	case 0x17:
		switch (c->x86_model) {
		case 0x00 ... 0x2f:
		case 0x50 ... 0x5f:
			setup_force_cpu_cap(X86_FEATURE_ZEN1);
			break;
		case 0x30 ... 0x4f:
		case 0x60 ... 0x7f:
		case 0x90 ... 0x91:
		case 0xa0 ... 0xaf:
			setup_force_cpu_cap(X86_FEATURE_ZEN2);
			break;
		default:
			goto warn;
		}
		break;

	case 0x19:
		switch (c->x86_model) {
		case 0x00 ... 0x0f:
		case 0x20 ... 0x5f:
			setup_force_cpu_cap(X86_FEATURE_ZEN3);
			break;
		case 0x10 ... 0x1f:
		case 0x60 ... 0xaf:
			setup_force_cpu_cap(X86_FEATURE_ZEN4);
			break;
		default:
			goto warn;
		}
		break;

	case 0x1a:
		switch (c->x86_model) {
		case 0x00 ... 0x0f:
		case 0x20 ... 0x2f:
		case 0x40 ... 0x4f:
		case 0x70 ... 0x7f:
			setup_force_cpu_cap(X86_FEATURE_ZEN5);
			break;
		default:
			goto warn;
		}
		break;

	default:
		break;
	}

to:

	/* Figure out Zen generations: */
	switch (c->x86_vfm) {
	case AFM(0x17, 0x00) ... AFM(0x17, 0x2f):
	case AFM(0x17, 0x50) ... AFM(0x17, 0x5f):
		setup_force_cpu_cap(X86_FEATURE_ZEN1);
		break;
	case AFM(0x17, 0x30) ... AFM(0x17, 0x4f):
	case AFM(0x17, 0x60) ... AFM(0x17, 0x7f):
	case AFM(0x17, 0x90) ... AFM(0x17, 0x91):
	case AFM(0x17, 0xa0) ... AFM(0x17, 0xaf):
		setup_force_cpu_cap(X86_FEATURE_ZEN2);
		break;
	case AFM(0x19, 0x00) ... AFM(0x19, 0x0f):
	case AFM(0x19, 0x20) ... AFM(0x19, 0x5f):
		setup_force_cpu_cap(X86_FEATURE_ZEN3);
		break;
	case AFM(0x19, 0x10) ... AFM(0x19, 0x1f):
	case AFM(0x19, 0x60) ... AFM(0x19, 0xaf):
		setup_force_cpu_cap(X86_FEATURE_ZEN4);
		break;
	case AFM(0x1a, 0x00) ... AFM(0x1a, 0x0f):
	case AFM(0x1a, 0x20) ... AFM(0x1a, 0x2f):
	case AFM(0x1a, 0x40) ... AFM(0x1a, 0x4f):
	case AFM(0x1a, 0x70) ... AFM(0x1a, 0x7f):
		setup_force_cpu_cap(X86_FEATURE_ZEN5);
		break;
	default:
		goto warn;
	}


That's more visually more compact, but maybe not any more readable.
But you would have the *option* to do this.

I'll post V2 of parts 1 & 2 with the re-ordered fields. None of the rest
of the patches need to change.

-Tony