Messages in this thread |  | | Date | Thu, 4 Apr 2024 11:04:49 +0200 | | Subject | Re: [PATCH 1/4] x86/cpu: Add and use new CPUID region helper | | From | Jürgen Groß <> |
| |
On 03.04.24 17:35, Dave Hansen wrote: > From: Dave Hansen <dave.hansen@linux.intel.com> > > There are some (before now) unwritten rules about CPUID "regions". > Basically, there is a 32-bit address space of CPUID leaves. The > top 16 bits address a "region" and the first leaf in a region > is special. > > The kernel only has a few spots that care about this, but it's > rather hard to make sense of the code as is. > > Add a helper that explains regions. Use it where applicable. > > Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com> > Reviewed-by: Kai Huang <kai.huang@intel.com> > Cc: Juergen Gross <jgross@suse.com> > > -- > > changes from v1: > * Fix typo comment and whitespace gunk noted by Ingo > > --- > > b/arch/x86/include/asm/cpuid.h | 59 ++++++++++++++++++++++++++++++++++++++ > b/arch/x86/kernel/cpu/common.c | 13 +++----- > b/arch/x86/kernel/cpu/transmeta.c | 9 +---- > b/arch/x86/xen/enlighten_pv.c | 9 +---- > 4 files changed, 69 insertions(+), 21 deletions(-) > > diff -puN arch/x86/include/asm/cpuid.h~cpuid-regions arch/x86/include/asm/cpuid.h > --- a/arch/x86/include/asm/cpuid.h~cpuid-regions 2024-04-02 15:22:58.838912075 -0700 > +++ b/arch/x86/include/asm/cpuid.h 2024-04-02 15:22:58.846912085 -0700 > @@ -168,4 +168,63 @@ static inline uint32_t hypervisor_cpuid_ > return 0; > } > > +/* > + * By convention, CPUID is broken up into regions which each > + * have 2^16 leaves. EAX in the first leaf of each valid > + * region returns the maximum valid leaf in that region. > + * > + * The regions can be thought of as being vendor-specific > + * areas of CPUID, but that's imprecise because everybody > + * implements the "Intel" region and Intel implements the > + * AMD region. There are a few well-known regions: > + * - Intel (0x0000) > + * - AMD (0x8000) > + * - Transmeta (0x8086) > + * - Centaur (0xC000) > + * > + * Consider a CPU where the maximum leaf in the Transmeta > + * region is 2. On such a CPU, leaf 0x80860000 would contain: > + * EAX==0x80860002. > + * region-^^^^ > + * max leaf-^^^^ > + */ > +static inline u32 cpuid_region_max_leaf(u16 region) > +{ > + u32 eax = cpuid_eax(region << 16); > + > + /* > + * An unsupported region may return data from the last > + * "basic" leaf, which is essentially garbage. Avoid > + * mistaking basic leaf data for region data. > + * > + * Note: this is not perfect. It is theoretically > + * possible for the last basic leaf to _resemble_ a > + * valid first leaf from a region that doesn't exist. > + * But Intel at least seems to pad out the basic region > + * with 0's, possibly to avoid this. > + */ > + if ((eax >> 16) != region) > + return 0; > + > + return eax; > +} > + > +/* Returns true if the leaf exists and @value was populated */ > +static inline bool get_cpuid_region_leaf(u32 leaf, enum cpuid_regs_idx reg, > + u32 *value) > +{ > + u16 region = leaf >> 16; > + u32 regs[4]; > + > + if (cpuid_region_max_leaf(region) < leaf) > + return false; > + > + cpuid(leaf, ®s[CPUID_EAX], ®s[CPUID_EBX], > + ®s[CPUID_ECX], ®s[CPUID_EDX]); > + > + *value = regs[reg]; > + > + return true; > +} > + > #endif /* _ASM_X86_CPUID_H */ > diff -puN arch/x86/kernel/cpu/common.c~cpuid-regions arch/x86/kernel/cpu/common.c > --- a/arch/x86/kernel/cpu/common.c~cpuid-regions 2024-04-02 15:22:58.842912079 -0700 > +++ b/arch/x86/kernel/cpu/common.c 2024-04-02 15:22:58.846912085 -0700 > @@ -1049,16 +1049,13 @@ void get_cpu_cap(struct cpuinfo_x86 *c) > } > > /* AMD-defined flags: level 0x80000001 */ > - eax = cpuid_eax(0x80000000); > - c->extended_cpuid_level = eax; > + c->extended_cpuid_level = cpuid_region_max_leaf(0x8000); > > - if ((eax & 0xffff0000) == 0x80000000) { > - if (eax >= 0x80000001) { > - cpuid(0x80000001, &eax, &ebx, &ecx, &edx); > + if (c->extended_cpuid_level >= 0x80000001) { > + cpuid(0x80000001, &eax, &ebx, &ecx, &edx); > > - c->x86_capability[CPUID_8000_0001_ECX] = ecx; > - c->x86_capability[CPUID_8000_0001_EDX] = edx; > - } > + c->x86_capability[CPUID_8000_0001_ECX] = ecx; > + c->x86_capability[CPUID_8000_0001_EDX] = edx; > } > > if (c->extended_cpuid_level >= 0x80000007) { > diff -puN arch/x86/kernel/cpu/transmeta.c~cpuid-regions arch/x86/kernel/cpu/transmeta.c > --- a/arch/x86/kernel/cpu/transmeta.c~cpuid-regions 2024-04-02 15:22:58.842912079 -0700 > +++ b/arch/x86/kernel/cpu/transmeta.c 2024-04-02 15:22:58.846912085 -0700 > @@ -9,14 +9,9 @@ > > static void early_init_transmeta(struct cpuinfo_x86 *c) > { > - u32 xlvl; > - > /* Transmeta-defined flags: level 0x80860001 */ > - xlvl = cpuid_eax(0x80860000); > - if ((xlvl & 0xffff0000) == 0x80860000) { > - if (xlvl >= 0x80860001) > - c->x86_capability[CPUID_8086_0001_EDX] = cpuid_edx(0x80860001); > - } > + get_cpuid_region_leaf(0x80860001, CPUID_EDX, > + &c->x86_capability[CPUID_8086_0001_EDX]); > } > > static void init_transmeta(struct cpuinfo_x86 *c) > diff -puN arch/x86/xen/enlighten_pv.c~cpuid-regions arch/x86/xen/enlighten_pv.c > --- a/arch/x86/xen/enlighten_pv.c~cpuid-regions 2024-04-02 15:22:58.842912079 -0700 > +++ b/arch/x86/xen/enlighten_pv.c 2024-04-03 08:34:28.221534043 -0700 > @@ -141,16 +141,13 @@ static void __init xen_set_mtrr_data(voi > }; > unsigned int reg; > unsigned long mask; > - uint32_t eax, width; > + uint32_t width; > static struct mtrr_var_range var[MTRR_MAX_VAR_RANGES] __initdata; > > /* Get physical address width (only 64-bit cpus supported). */ > width = 36; > - eax = cpuid_eax(0x80000000); > - if ((eax >> 16) == 0x8000 && eax >= 0x80000008) { > - eax = cpuid_eax(0x80000008); > - width = eax & 0xff; > - } > + /* Will overwrite 'width' if available in CPUID: */ > + get_cpuid_region_leaf(0x80000008, CPUID_EAX, &width);
Aren't you missing "width &= 0xff;" here? get_cpuid_region_leaf() doesn't truncate the returned value, which has the linear address width at bits 8-15.
Juergen
|  |