Re: [PATCH 01/74] x86/cpu/vfm: Add/initialize x86_vfm field to struct cpuinfo_x86

[Tony Luck]

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