2024-03-28 16:42:55

by Luck, Tony

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

Refactor struct cpuinfo_x86 so that the vendor, family, and model
fields are overlayed in a union with a 32-bit field that combines
all three (together with a one byte reserved field in the upper
byte).

This will make it easy, cheap, and reliable to check all three
values at once.

Signed-off-by: Tony Luck <[email protected]>
---
arch/x86/include/asm/processor.h | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 811548f131f4..87115e5d884f 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -108,9 +108,15 @@ struct cpuinfo_topology {
};

struct cpuinfo_x86 {
- __u8 x86; /* CPU family */
- __u8 x86_vendor; /* CPU vendor */
- __u8 x86_model;
+ union {
+ struct {
+ __u8 x86_vendor; /* CPU vendor */
+ __u8 x86; /* CPU family */
+ __u8 x86_model;
+ __u8 x86_reserved;
+ };
+ __u32 x86_vfm; /* combined vendor, family, model */
+ };
__u8 x86_stepping;
#ifdef CONFIG_X86_64
/* Number of 4K pages in DTLB/ITLB combined(in pages): */
--
2.44.0



2024-03-28 16:53:08

by Borislav Petkov

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

On Thu, Mar 28, 2024 at 05:48:11PM +0100, Borislav Petkov wrote:
> > + struct {
> > + __u8 x86_vendor; /* CPU vendor */

Looking at this more - you don't really need the vendor to be part of
this as the CPUID leaf ranges should *actually* be disjunct. Actually...

--
Regards/Gruss,
Boris.

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

2024-03-28 16:57:18

by Luck, Tony

[permalink] [raw]
Subject: RE: [PATCH 01/74] x86/cpu/vfm: Add/initialize x86_vfm field to struct cpuinfo_x86

>> __u8 x86_stepping;
>
> Why are you leaving out stepping?

There are only a handful of places where stepping is significant in CPU model
specific checks (mostly errata and side-channel security stuff). I didn't want to
have to invent new #define names for each stepping. E.g. there are about
three separate steppings of INTEL_SKYLAKE_X out in the wild. It would
be messy to have three #defines and add them in all the stepping independent
code paths.

> And since we want to simplify all this, why aren't we replacing all
> f/m/s checks by using the whole CPUID(1).EAX u32 instead?
>
> Then the macros need to build that CPUID leaf simply.

I could make the raw format of the #define values be CPUID(1).EAX
with the stepping masked out. But then I'd need to add a new field to
the structure instead of overlaying with the vendor/family/model
fields.

-Tony

2024-03-28 17:05:39

by Luck, Tony

[permalink] [raw]
Subject: RE: [PATCH 01/74] x86/cpu/vfm: Add/initialize x86_vfm field to struct cpuinfo_x86

> Looking at this more - you don't really need the vendor to be part of
> this as the CPUID leaf ranges should *actually* be disjunct. Actually...

It's essentially free to do this, and it makes the code more robust. It
becomes impossible to check model without also checking vendor
and family at the same time.

-Tony

2024-03-28 17:08:48

by Borislav Petkov

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

On Thu, Mar 28, 2024 at 04:56:37PM +0000, Luck, Tony wrote:
> I could make the raw format of the #define values be CPUID(1).EAX
> with the stepping masked out. But then I'd need to add a new field to
> the structure instead of overlaying with the vendor/family/model
> fields.

Yes, that would be better. And if you're going to replace our f/m/s
checking with something better, then it better handle the stepping just
like the rest. How it is used now doesn't mean a whole lot for the
future.

And if it is not too important for most checks, you can mask it out with
macros.

--
Regards/Gruss,
Boris.

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

2024-03-28 18:32:55

by Luck, Tony

[permalink] [raw]
Subject: RE: [PATCH 01/74] x86/cpu/vfm: Add/initialize x86_vfm field to struct cpuinfo_x86

> And frankly, the x86 vendor is a Linux thing so I wouldn't mind if
> checks are
>
> if (c->x86_vendor == X86_VENDOR... &&
> c->cpuid_1_eax == MACRO_BUILD_CPUID_1_EAX(fam, model, stepping))

That hurts my eyes compared to:

if (c->x86_vfm == INTEL_DOUGLASCOVE)

>
> Anyway, using CPUID(1).EAX is just a suggestion so that we don't have to
> invent our own format and convert to and fro.

All the conversion is at compile time to generate the values for the CPU model
name #defines.

> And that would be advantageous when we convert to dealing with
> CPUID(1).EAX values everywhere and we compare them straightaway. We'd
> need macros only when we need only some data elements from that leaf.

It's a humungous amount more code churn. Most of the changes in this set were
achieved with some simple sed scripts (followed by hand massage to adjust TABs
to make things pretty because lengths of strings are different).

Take a look at a few of the patches that implement this change and consider
how they would look based on a CPUID(1).EAX value.

> And since that leaf's layout is commonly known, the conversion errors
> should be at a minimum...
>
> I'd say.

I don't think the format is really that big an issue. Including stepping in the
format adds complexity to a thousand places these checks are made while
only being useful in a few dozen.

-Tony

2024-03-28 18:52:03

by Borislav Petkov

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

On Thu, Mar 28, 2024 at 05:00:59PM +0000, Luck, Tony wrote:
> It's essentially free to do this, and it makes the code more robust. It
> becomes impossible to check model without also checking vendor
> and family at the same time.

I see that but if we want to use CPUID(1).EAX, there's no vendor in
there.

And frankly, the x86 vendor is a Linux thing so I wouldn't mind if
checks are

if (c->x86_vendor == X86_VENDOR... &&
c->cpuid_1_eax == MACRO_BUILD_CPUID_1_EAX(fam, model, stepping))

Anyway, using CPUID(1).EAX is just a suggestion so that we don't have to
invent our own format and convert to and fro.

And that would be advantageous when we convert to dealing with
CPUID(1).EAX values everywhere and we compare them straightaway. We'd
need macros only when we need only some data elements from that leaf.

And since that leaf's layout is commonly known, the conversion errors
should be at a minimum...

I'd say.

--
Regards/Gruss,
Boris.

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

2024-03-28 19:38:48

by Borislav Petkov

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

On Thu, Mar 28, 2024 at 09:37:44AM -0700, Tony Luck wrote:
> Refactor struct cpuinfo_x86 so that the vendor, family, and model
> fields are overlayed in a union with a 32-bit field that combines
> all three (together with a one byte reserved field in the upper
> byte).
>
> This will make it easy, cheap, and reliable to check all three
> values at once.
>
> Signed-off-by: Tony Luck <[email protected]>
> ---
> arch/x86/include/asm/processor.h | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 811548f131f4..87115e5d884f 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -108,9 +108,15 @@ struct cpuinfo_topology {
> };
>
> struct cpuinfo_x86 {
> - __u8 x86; /* CPU family */
> - __u8 x86_vendor; /* CPU vendor */
> - __u8 x86_model;
> + union {
> + struct {
> + __u8 x86_vendor; /* CPU vendor */
> + __u8 x86; /* CPU family */
> + __u8 x86_model;
> + __u8 x86_reserved;
> + };
> + __u32 x86_vfm; /* combined vendor, family, model */
> + };
> __u8 x86_stepping;

Why are you leaving out stepping?

And since we want to simplify all this, why aren't we replacing all
f/m/s checks by using the whole CPUID(1).EAX u32 instead?

Then the macros need to build that CPUID leaf simply.

--
Regards/Gruss,
Boris.

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

2024-03-28 21:20:15

by Luck, Tony

[permalink] [raw]
Subject: RE: [PATCH 01/74] x86/cpu/vfm: Add/initialize x86_vfm field to struct cpuinfo_x86

> I don't think the format is really that big an issue. Including stepping in the
> format adds complexity to a thousand places these checks are made while
> only being useful in a few dozen.

Stats to back that up:

$ git grep INTEL_FAM6 | wc -l
876

but some of those are the definitions of the model name macros:
$ git grep INTEL_FAM6 -- arch/x86/include/asm/intel-family.h | wc -l
82

Places using the X86_MATCH_INTEL macros don't show in above count:

$ git grep X86_MATCH_INTEL | wc -l
430

Places that use STEPPINGS:
$ git grep X86_MATCH_INTEL_FAM6_MODEL_STEPPINGS | wc -l
21

or STEPPINGS + FEATURE
$ git grep gg X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE | wc -l
6

$ git grep x86_stepping | wc -l
83

-Tony

2024-03-29 11:40:43

by Borislav Petkov

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

On Thu, Mar 28, 2024 at 06:32:35PM +0000, Luck, Tony wrote:
> I don't think the format is really that big an issue. Including stepping in the
> format adds complexity to a thousand places these checks are made while
> only being useful in a few dozen.

I've figured out what the problem is with steppings - ranges. If you
have a range of steppings which all belong to the same model, then you
have to complicate the checks by either masking out the stepping or use
the X86_STEPPING_ANY thing which forces you to use x86_match_cpu()
instead of a simple integer comparison.

And you should talk to your folks what their plan is for the new
families because if they do a range of model numbers which all belong to
the same CPU model like AMD does, then your simple comparison scheme
goes out the window because it can't really deal with ranges.

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.

So I don't think I can switch AMD to use that. It looks like the 'V' in
"VFM" could just as well turn into "I".

:-)

I'd say.

--
Regards/Gruss,
Boris.

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

2024-03-29 16:46:34

by Luck, Tony

[permalink] [raw]
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:
> On Thu, Mar 28, 2024 at 06:32:35PM +0000, Luck, Tony wrote:
> > I don't think the format is really that big an issue. Including stepping in the
> > format adds complexity to a thousand places these checks are made while
> > only being useful in a few dozen.
>
> I've figured out what the problem is with steppings - ranges. If you
> have a range of steppings which all belong to the same model, then you
> have to complicate the checks by either masking out the stepping or use
> the X86_STEPPING_ANY thing which forces you to use x86_match_cpu()
> instead of a simple integer comparison.

I think you are talking about a range of models that all belong to
the same family (rather than steppings in the same model).

> And you should talk to your folks what their plan is for the new
> families because if they do a range of model numbers which all belong to
> the same CPU model like AMD does, then your simple comparison scheme
> goes out the window because it can't really deal with ranges.

History of Intel model number allocations apparently looks like
we just throw a dart in the general area of a block of unused
model numbers :-) I will check with the relevent folks, but this
seems unlikely. There's more push (from the Linux community!) to
assign CPUID feature bits for stuff that goes more than 2-3
CPU generations. That leaves the stuff that is different almost
every time (perfomaance counters, power management, EDAC, etc.).

> 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

I'm glad I don't have to keep track of groups of hex numbers like that.

> 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.

My patch doesn't help with this, but doesn't prevent you from doing
a switch (c->x86_model). If that list of model number ranges shows
up more than twice you could add a helper that converts that list to
a #define AMD_ZEN2 to make the code clearer.

> So I don't think I can switch AMD to use that. It looks like the 'V' in
> "VFM" could just as well turn into "I".

Patch 3 includes:

#define IFM(_fam, _model) VFM_MAKE(X86_VENDOR_INTEL, _fam, _model)

as a helper to build model numbers in <asm/intel-family.h>
>
> :-)
>
> I'd say.

So keep the "V" in the common code. Maybe one of the other x86
vendors will want to have #define names for their CPU models
some day.

Thanks for digging into this.

-Tony

2024-03-29 17:24:29

by Borislav Petkov

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

On Fri, Mar 29, 2024 at 09:46:25AM -0700, Tony Luck wrote:
> I think you are talking about a range of models that all belong to
> the same family (rather than steppings in the same model).

Either. Depending on what you're tracking. If the majority of your
feature tests want to determine whether you're running on the same set
of hw features which belong to a model determined by a single or
multiple model numbers, then you need to track that.

On Intel you have a single model number determining that set of hw
features.

On AMD you have s range of model numbers and there can be differences
too.

Seldom we pay attention to steppings but it is not unheard of. We have
had an incremented stepping denoting a hw bug fix in the past.

> History of Intel model number allocations apparently looks like
> we just throw a dart in the general area of a block of unused
> model numbers :-)

Don't say that. The guy who's assigning the numbers and keeps track of
what he's given to which team, will be mad at you. :-P

> I'm glad I don't have to keep track of groups of hex numbers like that.

Depends on how you model it. Setting a X86_FEATURE_ZEN<n> for each works
like a charm.

> My patch doesn't help with this, but doesn't prevent you from doing
> a switch (c->x86_model). If that list of model number ranges shows
> up more than twice you could add a helper that converts that list to
> a #define AMD_ZEN2 to make the code clearer.

Haven't needed such stunts yet and I hope I won't ever.

> So keep the "V" in the common code. Maybe one of the other x86
> vendors will want to have #define names for their CPU models
> some day.

Right.

Thx.

--
Regards/Gruss,
Boris.

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

2024-04-01 18:19:36

by Luck, Tony

[permalink] [raw]
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

2024-04-01 18:24:24

by Luck, Tony

[permalink] [raw]
Subject: [PATCH v2 01/74] x86/cpu/vfm: Add/initialize x86_vfm field to struct cpuinfo_x86

Refactor struct cpuinfo_x86 so that the vendor, family, and model
fields are overlayed in a union with a 32-bit field that combines
all three (together with a one byte reserved field in the upper
byte).

This will make it easy, cheap, and reliable to check all three
values at once.

Signed-off-by: Tony Luck <[email protected]>
---
arch/x86/include/asm/processor.h | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 811548f131f4..4c5d166aa473 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -108,9 +108,15 @@ struct cpuinfo_topology {
};

struct cpuinfo_x86 {
- __u8 x86; /* CPU family */
- __u8 x86_vendor; /* CPU vendor */
- __u8 x86_model;
+ union {
+ struct {
+ __u8 x86_model;
+ __u8 x86; /* CPU family */
+ __u8 x86_vendor; /* CPU vendor */
+ __u8 x86_reserved;
+ };
+ __u32 x86_vfm; /* combined vendor, family, model */
+ };
__u8 x86_stepping;
#ifdef CONFIG_X86_64
/* Number of 4K pages in DTLB/ITLB combined(in pages): */
--
2.44.0


2024-04-07 10:54:34

by Borislav Petkov

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

On Mon, Apr 01, 2024 at 11:18:57AM -0700, Tony Luck wrote:
> That's more visually more compact, but maybe not any more readable.

Yap, I see it the same way.

> But you would have the *option* to do this.

Thanks, yeah, in case we ever get to cross that bridge...

--
Regards/Gruss,
Boris.

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

2024-04-08 16:21:30

by Luck, Tony

[permalink] [raw]
Subject: RE: [PATCH 01/74] x86/cpu/vfm: Add/initialize x86_vfm field to struct cpuinfo_x86

>> That's more visually more compact, but maybe not any more readable.
>
> Yap, I see it the same way.
>
> But you would have the *option* to do this.
>
> Thanks, yeah, in case we ever get to cross that bridge...

Boris,

So are parts 1-3 ready for TIP? If they get added I can start collecting
reviews for parts 4-72.

73 and 74 are the cleanup to delete bits that are no longer needed.

-Tony


2024-04-09 08:24:28

by Borislav Petkov

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

On Mon, Apr 08, 2024 at 04:20:36PM +0000, Luck, Tony wrote:
> So are parts 1-3 ready for TIP? If they get added I can start collecting
> reviews for parts 4-72.

I'd like for tglx to have a look at this first too.

Thx.

--
Regards/Gruss,
Boris.

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

2024-04-09 13:01:43

by Thomas Gleixner

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

On Mon, Apr 01 2024 at 11:23, Tony Luck wrote:

> Refactor struct cpuinfo_x86 so that the vendor, family, and model
> fields are overlayed in a union with a 32-bit field that combines
> all three (together with a one byte reserved field in the upper
> byte).
>
> This will make it easy, cheap, and reliable to check all three
> values at once.
>
> Signed-off-by: Tony Luck <[email protected]>

Reviewed-by: Thomas Gleixner <[email protected]>

> ---
> arch/x86/include/asm/processor.h | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 811548f131f4..4c5d166aa473 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -108,9 +108,15 @@ struct cpuinfo_topology {
> };
>
> struct cpuinfo_x86 {
> - __u8 x86; /* CPU family */
> - __u8 x86_vendor; /* CPU vendor */
> - __u8 x86_model;
> + union {
> + struct {
> + __u8 x86_model;
> + __u8 x86; /* CPU family */
> + __u8 x86_vendor; /* CPU vendor */
> + __u8 x86_reserved;
> + };
> + __u32 x86_vfm; /* combined vendor, family, model */
> + };
> __u8 x86_stepping;
> #ifdef CONFIG_X86_64
> /* Number of 4K pages in DTLB/ITLB combined(in pages): */

2024-04-16 18:16:16

by Luck, Tony

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

On Tue, Apr 09, 2024 at 10:22:17AM +0200, Borislav Petkov wrote:
> On Mon, Apr 08, 2024 at 04:20:36PM +0000, Luck, Tony wrote:
> > So are parts 1-3 ready for TIP? If they get added I can start collecting
> > reviews for parts 4-72.
>
> I'd like for tglx to have a look at this first too.

Boris,

Thomas gave his Reviewed-by for parts 1-3

Link: https://lore.kernel.org/all/871q7e7lfu.ffs@tglx/
Link: https://lore.kernel.org/all/87y19m66tu.ffs@tglx/
Link: https://lore.kernel.org/all/87v84q66sn.ffs@tglx/

Is there anyone else that needs a quick poke to look at these?

-Tony

2024-04-16 18:24:17

by Borislav Petkov

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

On Tue, Apr 16, 2024 at 11:16:05AM -0700, Tony Luck wrote:
> Thomas gave his Reviewed-by for parts 1-3
>
> Link: https://lore.kernel.org/all/871q7e7lfu.ffs@tglx/
> Link: https://lore.kernel.org/all/87y19m66tu.ffs@tglx/
> Link: https://lore.kernel.org/all/87v84q66sn.ffs@tglx/
>
> Is there anyone else that needs a quick poke to look at these?

I don't think so.

Aren't you going to send the whole set first though?

Or what is the merge strategy here?

Thx.

--
Regards/Gruss,
Boris.

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

2024-04-16 18:38:25

by Luck, Tony

[permalink] [raw]
Subject: RE: [PATCH 01/74] x86/cpu/vfm: Add/initialize x86_vfm field to struct cpuinfo_x86

>> Is there anyone else that needs a quick poke to look at these?
>
> I don't think so.
>
> Aren't you going to send the whole set first though?
>
> Or what is the merge strategy here?

I thought to get the first three into TIP, and thus into linux-next. Then I'd
start posting patches to the individual files to their respective maintainers.

Only about half are arch/x86. The rest are scattered around the tree.

But if you'd like to see the whole series in one big mail thread, I can
post it all.

-Tony

2024-04-16 19:59:17

by Borislav Petkov

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

On Tue, Apr 16, 2024 at 06:37:18PM +0000, Luck, Tony wrote:
> I thought to get the first three into TIP, and thus into linux-next. Then I'd
> start posting patches to the individual files to their respective maintainers.
>
> Only about half are arch/x86. The rest are scattered around the tree.

If you do that, people would have to merge the tip branch which has them
before they apply them in their tree.

Alternatively, we can take the arch/x86 parts and once 6.10 releases,
the other maintainers will have them in tree and thus not need the
cross-tree merges.

> But if you'd like to see the whole series in one big mail thread, I can
> post it all.

Or you can post the whole series and we can take it all through tip once
the other maintainers ack the respective patch for their area. Which
sounds like the simplest solution to me...

Thx.

--
Regards/Gruss,
Boris.

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

2024-04-16 21:45:59

by Luck, Tony

[permalink] [raw]
Subject: RE: [PATCH 01/74] x86/cpu/vfm: Add/initialize x86_vfm field to struct cpuinfo_x86

> Alternatively, we can take the arch/x86 parts and once 6.10 releases,
> the other maintainers will have them in tree and thus not need the
> cross-tree merges.

I did a bit of this. I moved all the arch/x86 patches up to immediately follow
the three prep patches. So you can pick through parts 0004..0039 and apply
any that look good to you (there is no ordering requirement among these).
Bounce anything that needs extra work back to me.

> Or you can post the whole series and we can take it all through tip once
> the other maintainers ack the respective patch for their area. Which
> sounds like the simplest solution to me...

Also a bit of this. Parts 0040..0072 are posted to whomever get_maintainer.pl
said might be interested. So may see some reviews coming in for these.

Parts 0073..0074 are the cleanup to delete the old macros. They can only
be applied after everything else has been merged.

-Tony

2024-04-17 19:11:40

by Sean Christopherson

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

On Tue, Apr 16, 2024, Tony Luck wrote:
> > Alternatively, we can take the arch/x86 parts and once 6.10 releases,
> > the other maintainers will have them in tree and thus not need the
> > cross-tree merges.
>
> I did a bit of this. I moved all the arch/x86 patches up to immediately follow
> the three prep patches. So you can pick through parts 0004..0039 and apply
> any that look good to you (there is no ordering requirement among these).
> Bounce anything that needs extra work back to me.

There are two KVM changes hiding in there, but they're obviously quite trivial
and can to through tip, I don't expect any conflicts.

2024-04-17 19:44:40

by Borislav Petkov

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

On Wed, Apr 17, 2024 at 12:02:21PM -0700, Sean Christopherson wrote:
> There are two KVM changes hiding in there, but they're obviously quite trivial
> and can to through tip, I don't expect any conflicts.

Yeah, the plan is to queue it all through tip as that would be a lot
less conflicts and cross-tree issues so you acking them is what I was
hoping to get.

So thanks. :-)

--
Regards/Gruss,
Boris.

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

2024-04-18 01:48:33

by Luck, Tony

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


> On Apr 17, 2024, at 12:43, Borislav Petkov <[email protected]> wrote:
>
> On Wed, Apr 17, 2024 at 12:02:21PM -0700, Sean Christopherson wrote:
>> There are two KVM changes hiding in there, but they're obviously quite trivial
>> and can to through tip, I don't expect any conflicts.
>
> Yeah, the plan is to queue it all through tip as that would be a lot
> less conflicts and cross-tree issues so you acking them is what I was
> hoping to get.
>
> So thanks. :-)

There’s general (justified) grumbling about
my auto-generated Subject lines. So I’ll make
a pass through the whole series to make the
initial tag match each subsystem convention.
Also make it generally more useful. I.e. like this
for most of the arch/x86 patches:

Subject: x86/cpu: Use new Intel CPU model #defines

Perhaps all the pieces with Reviewed or Acked
tags can go into tip tree next week?

-Tony