| Message ID | 20201125144847.3920-4-punitagrawal@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add processor to the ignore PSD override list | expand |
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 <punitagrawal@gmail.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: x86@kernel.org > --- > 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
Hi Boris, Borislav Petkov <bp@alien8.de> 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 <punitagrawal@gmail.com> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: Borislav Petkov <bp@alien8.de> >> Cc: x86@kernel.org >> --- >> 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 [...]
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
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>
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 <punitagrawal@gmail.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: Borislav Petkov <bp@alien8.de> Cc: x86@kernel.org --- 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