2024-04-03 15:35:26

by Dave Hansen

Subject: [PATCH 1/4] x86/cpu: Add and use new CPUID region helper

From: Dave Hansen <[email protected]>

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 <[email protected]>
Reviewed-by: Kai Huang <[email protected]>
Cc: Juergen Gross <[email protected]>

--

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, &regs[CPUID_EAX], &regs[CPUID_EBX],
+ &regs[CPUID_ECX], &regs[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);

for (reg = 0; reg < MTRR_MAX_VAR_RANGES; reg++) {
op.u.read_memtype.reg = reg;
_

2024-04-04 09:05:03

by Juergen Gross

Subject: Re: [PATCH 1/4] x86/cpu: Add and use new CPUID region helper

On 03.04.24 17:35, Dave Hansen wrote:
> From: Dave Hansen <[email protected]>
>
> 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 <[email protected]>
> Reviewed-by: Kai Huang <[email protected]>
> Cc: Juergen Gross <[email protected]>
>
> --
>
> 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, &regs[CPUID_EAX], &regs[CPUID_EBX],
> + &regs[CPUID_ECX], &regs[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

2024-04-08 16:25:45

by Borislav Petkov

Subject: Re: [PATCH 1/4] x86/cpu: Add and use new CPUID region helper

On Thu, Apr 04, 2024 at 11:04:49AM +0200, Jürgen Groß wrote:
> 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.

Yeah.

Other than that, the intention looks ok as it documents what that code
magic was.

Thx.

--
Regards/Gruss,
Boris.

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