2020-11-25 15:05:05

by Punit Agrawal

[permalink] [raw]
Subject: [RFC PATCH 3/4] x86/cpu: amd: Define processor families

So far, the AMD processor identifier (family, models, stepping) are
referred to by raw values making it easy to make mistakes. It is also
harder to read and maintain. Additionally, these values are also being
used in subsystems outside the arch code where not everybody maybe be
as familiar with the processor identifiers.

As a first step towards improving the status quo, add macros for the
AMD processor families and propagate them through the existing
cpu_device_id.h header used for this purpose.

Signed-off-by: Punit Agrawal <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: [email protected]
---
arch/x86/include/asm/amd-family.h | 18 ++++++++++++++++++
arch/x86/include/asm/cpu_device_id.h | 2 ++
2 files changed, 20 insertions(+)
create mode 100644 arch/x86/include/asm/amd-family.h

diff --git a/arch/x86/include/asm/amd-family.h b/arch/x86/include/asm/amd-family.h
new file mode 100644
index 000000000000..dff4d13b8e74
--- /dev/null
+++ b/arch/x86/include/asm/amd-family.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_AMD_FAMILY_H
+#define _ASM_X86_AMD_FAMILY_H
+
+#define AMD_FAM_K5 0x04
+#define AMD_FAM_K6 0x05
+#define AMD_FAM_K7 0x06
+#define AMD_FAM_K8 0x0F
+#define AMD_FAM_K10 0x10
+#define AMD_FAM_K8_K10_HYBRID 0x11
+#define AMD_FAM_LLANO 0x12
+#define AMD_FAM_BOBCAT 0x14
+#define AMD_FAM_BULLDOZER 0x15
+#define AMD_FAM_JAGUAR 0x16
+#define AMD_FAM_ZEN 0x17
+#define AMD_FAM_ZEN3 0x19
+
+#endif /* _ASM_X86_AMD_FAMILY_H */
diff --git a/arch/x86/include/asm/cpu_device_id.h b/arch/x86/include/asm/cpu_device_id.h
index eb8fcede9e3b..bbb48ba4c7ff 100644
--- a/arch/x86/include/asm/cpu_device_id.h
+++ b/arch/x86/include/asm/cpu_device_id.h
@@ -12,6 +12,8 @@
#include <linux/mod_devicetable.h>
/* Get the INTEL_FAM* model defines */
#include <asm/intel-family.h>
+/* Get the AMD model defines */
+#include <asm/amd-family.h>
/* And the X86_VENDOR_* ones */
#include <asm/processor.h>

--
2.29.2


2020-11-30 14:05:26

by Borislav Petkov

[permalink] [raw]
Subject: Re: [RFC PATCH 3/4] x86/cpu: amd: Define processor families

On Wed, Nov 25, 2020 at 11:48:46PM +0900, Punit Agrawal wrote:
> So far, the AMD processor identifier (family, models, stepping) are
> referred to by raw values making it easy to make mistakes. It is also
> harder to read and maintain. Additionally, these values are also being
> used in subsystems outside the arch code where not everybody maybe be
> as familiar with the processor identifiers.
>
> As a first step towards improving the status quo, add macros for the
> AMD processor families and propagate them through the existing
> cpu_device_id.h header used for this purpose.
>
> Signed-off-by: Punit Agrawal <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: [email protected]
> ---
> arch/x86/include/asm/amd-family.h | 18 ++++++++++++++++++
> arch/x86/include/asm/cpu_device_id.h | 2 ++
> 2 files changed, 20 insertions(+)
> create mode 100644 arch/x86/include/asm/amd-family.h
>
> diff --git a/arch/x86/include/asm/amd-family.h b/arch/x86/include/asm/amd-family.h
> new file mode 100644
> index 000000000000..dff4d13b8e74
> --- /dev/null
> +++ b/arch/x86/include/asm/amd-family.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_X86_AMD_FAMILY_H
> +#define _ASM_X86_AMD_FAMILY_H
> +
> +#define AMD_FAM_K5 0x04
> +#define AMD_FAM_K6 0x05
> +#define AMD_FAM_K7 0x06
> +#define AMD_FAM_K8 0x0F
> +#define AMD_FAM_K10 0x10

Fam 0x10 is Greyhound and a lot more core names. I'd let the AMD folks
on Cc say what they wanna call it but I don't think K10 was used
anywhere except outside of AMD. :)

> +#define AMD_FAM_K8_K10_HYBRID 0x11

That was called Griffin so AMD_FAM_GRIFFIN I guess. If not used anywhere
in the tree, no need to add the define. Same holds true for the rest of
the defines - add them only when they're used.

> +#define AMD_FAM_LLANO 0x12
> +#define AMD_FAM_BOBCAT 0x14
> +#define AMD_FAM_BULLDOZER 0x15
> +#define AMD_FAM_JAGUAR 0x16
> +#define AMD_FAM_ZEN 0x17

ZEN2 is also 0x17 but different models so this is where the family
matching scheme doesn't work - you'd need the models too.

0x18 is HYGON

> +#define AMD_FAM_ZEN3 0x19
> +
> +#endif /* _ASM_X86_AMD_FAMILY_H */
> diff --git a/arch/x86/include/asm/cpu_device_id.h b/arch/x86/include/asm/cpu_device_id.h
> index eb8fcede9e3b..bbb48ba4c7ff 100644
> --- a/arch/x86/include/asm/cpu_device_id.h
> +++ b/arch/x86/include/asm/cpu_device_id.h
> @@ -12,6 +12,8 @@
> #include <linux/mod_devicetable.h>
> /* Get the INTEL_FAM* model defines */
> #include <asm/intel-family.h>
> +/* Get the AMD model defines */
> +#include <asm/amd-family.h>
> /* And the X86_VENDOR_* ones */
> #include <asm/processor.h>
>
> --
> 2.29.2
>

--
Regards/Gruss,
Boris.

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

2020-12-02 14:16:48

by Punit Agrawal

[permalink] [raw]
Subject: Re: [RFC PATCH 3/4] x86/cpu: amd: Define processor families

Hi Boris,

Borislav Petkov <[email protected]> writes:

> On Wed, Nov 25, 2020 at 11:48:46PM +0900, Punit Agrawal wrote:
>> So far, the AMD processor identifier (family, models, stepping) are
>> referred to by raw values making it easy to make mistakes. It is also
>> harder to read and maintain. Additionally, these values are also being
>> used in subsystems outside the arch code where not everybody maybe be
>> as familiar with the processor identifiers.
>>
>> As a first step towards improving the status quo, add macros for the
>> AMD processor families and propagate them through the existing
>> cpu_device_id.h header used for this purpose.
>>
>> Signed-off-by: Punit Agrawal <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Borislav Petkov <[email protected]>
>> Cc: [email protected]
>> ---
>> arch/x86/include/asm/amd-family.h | 18 ++++++++++++++++++
>> arch/x86/include/asm/cpu_device_id.h | 2 ++
>> 2 files changed, 20 insertions(+)
>> create mode 100644 arch/x86/include/asm/amd-family.h
>>
>> diff --git a/arch/x86/include/asm/amd-family.h b/arch/x86/include/asm/amd-family.h
>> new file mode 100644
>> index 000000000000..dff4d13b8e74
>> --- /dev/null
>> +++ b/arch/x86/include/asm/amd-family.h
>> @@ -0,0 +1,18 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _ASM_X86_AMD_FAMILY_H
>> +#define _ASM_X86_AMD_FAMILY_H
>> +
>> +#define AMD_FAM_K5 0x04
>> +#define AMD_FAM_K6 0x05
>> +#define AMD_FAM_K7 0x06
>> +#define AMD_FAM_K8 0x0F
>> +#define AMD_FAM_K10 0x10
>
> Fam 0x10 is Greyhound and a lot more core names. I'd let the AMD folks
> on Cc say what they wanna call it but I don't think K10 was used
> anywhere except outside of AMD. :)

Didn't realize the core was internal only. There are a couple of
instances where the family does shows up arch/x86/kernel/cpu/amd.c so
atleast some of the patches did make it upstream.

>> +#define AMD_FAM_K8_K10_HYBRID 0x11
>
> That was called Griffin so AMD_FAM_GRIFFIN I guess. If not used anywhere
> in the tree, no need to add the define.

I haven't looked to deeply but there's one instance in
arch/x86/kernel/cpu/amd.c - though I wonder if that could be re-written
to rely on a hardware / firmware interface instead.

> Same holds true for the rest of the defines - add them only when
> they're used.

Makes sense - I will follow your suggestion in the next version.

>> +#define AMD_FAM_LLANO 0x12
>> +#define AMD_FAM_BOBCAT 0x14
>> +#define AMD_FAM_BULLDOZER 0x15
>> +#define AMD_FAM_JAGUAR 0x16
>> +#define AMD_FAM_ZEN 0x17
>
> ZEN2 is also 0x17 but different models so this is where the family
> matching scheme doesn't work - you'd need the models too.

Yes, I wasn't sure the best way to handle this so went with the earlier
generation name. I think for such cases, something that looks at the
cpuinfo_x86 structure and decides the family / generation is probably
the way to go.

> 0x18 is HYGON

I missed this one. I'll add it to the list.

Thanks for the review and your comments. I'll wait a bit to see if
there's any further feedback.

Cheers,
Punit

[...]

2020-12-02 17:03:48

by Borislav Petkov

[permalink] [raw]
Subject: Re: [RFC PATCH 3/4] x86/cpu: amd: Define processor families

On Wed, Dec 02, 2020 at 11:13:02PM +0900, Punit Agrawal wrote:
> Didn't realize the core was internal only.

F10h is not internal only - all I'm saying is that "K10" wasn't use
inside AMD AFAIR.

> Makes sense - I will follow your suggestion in the next version.

Well, I don't think it is worth the churn, TBH.

I'd prefer comments over the f/m/s checks which explain what is matched
much better than defines for family numbers which are inadequate when
one needs to match the model too, for one.

--
Regards/Gruss,
Boris.

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