| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aKObRk0Ze1_LVqWj@google.com>
Date: Mon, 18 Aug 2025 14:29:42 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: "Ahmed S. Darwish" <darwi@...utronix.de>
Cc: Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...hat.com>,
Dave Hansen <dave.hansen@...ux.intel.com>, Thomas Gleixner <tglx@...utronix.de>,
Andrew Cooper <andrew.cooper3@...rix.com>, "H. Peter Anvin" <hpa@...or.com>,
David Woodhouse <dwmw2@...radead.org>, Peter Zijlstra <peterz@...radead.org>,
Sohil Mehta <sohil.mehta@...el.com>, John Ogness <john.ogness@...utronix.de>, x86@...nel.org,
x86-cpuid@...ts.linux.dev, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 07/34] x86/cpuid: Introduce a centralized CPUID data model
On Mon, Aug 18, 2025, Ahmed S. Darwish wrote:
> Hi,
>
> On Mon, 18 Aug 2025, Sean Christopherson wrote:
> >
> > Why not? Like C structs, there can only be one variable sized array, i.e. there
> > can't be multiple "n" subleafs. If the concern is calling __cpuid_subleaf_n()
> > with i < start, then I don't see how embedding start in the structure name helps
> > in any way, since 'i' isn't a compile-time constant and so needs to be checked at
> > runtime no matter what.
> >
>
> Hmmm...
>
> I can indeed find the offset of the dynamic 'leaf_0x.._n' subleaf storage
> by CPP tokenization, since the CPUID table will be in the form:
>
> struct leaf_0x0_0 leaf_0x0_0[1];
> struct leaf_0x1_0 leaf_0x1_0[1];
> struct leaf_0x2_0 leaf_0x2_0[1];
> struct leaf_0x4_0 leaf_0x4_0[8];
> struct leaf_0xd_0 leaf_0xd_0[1];
> struct leaf_0xd_1 leaf_0xd_1[1];
> struct leaf_0xd_n leaf_0xd_n[62];
> struct leaf_0x16_0 leaf_0x16_0[1];
> struct leaf_0x80000000_0 leaf_0x80000000_0[1];
> struct leaf_0x80000005_0 leaf_0x80000005_0[1];
> struct leaf_0x80000006_0 leaf_0x80000006_0[1];
> struct leaf_0x8000001d_0 leaf_0x8000001d_0[8];
>
> I was also kinda worried about the different subleaf semantics:
>
> struct leaf_0x4_n => starts from subleaf 0
> struct leaf_0xd_n => starts from subleaf 2
>
> But, hopefully it would be clear when looking at the generated header in
> total.
>
> Still: for the API you're proposing, I'll need to generate an error for
> things like:
>
> cpuid_subleaf_n(c, 0xd, 0);
> cpuid_subleaf_n(c, 0xd, 1);
>
> which could not have happened in the API I proposed earlier. But... I
> can let the XML files generate some symbols in the form:
>
> LEAF_0x4_n_MIN_SUBLEAF 0
> LEAF_0xd_n_MIN_SUBLEAF 2
>
> and generate an error (once) if the passed subleaf is less than the
> minimum. I can also generate that error (once) at compile-time if the
> given subleaf was a compile-time constant.
Eh, if the kernel can flag "cpuid_subleaf_n(c, 0xd, 1)" at compile time, then yay.
But I don't think it's worth going too far out of the way to detect, and it's most
definitely not worth bleeding the lower bound into the APIs.
E.g. it's not really any different than doing:
cpuid_subleaf_n(c, 0xd, 2, 64);
and AFAIT the original code wouldn't flag that at compile time.
> Maybe there's a cleaner way for detecting this subleaf lower-bound error,
Not sure "cleaner" is the right word, but if you really want to add compile-time
sanity checks, you could put the actual definitions in a dedicated header file
that's included multiple times without any #ifdef guards. Once to define
"struct cpuid_leaves", and a second time to define global metadata for each leaf,
e.g. the first/last subleaf in a dynamic range.
---
arch/x86/include/asm/cpuid/api.h | 11 ++++---
arch/x86/include/asm/cpuid/leaf_defs.h | 12 +++++++
arch/x86/include/asm/cpuid/types.h | 43 +++++++++++++++-----------
3 files changed, 44 insertions(+), 22 deletions(-)
create mode 100644 arch/x86/include/asm/cpuid/leaf_defs.h
diff --git a/arch/x86/include/asm/cpuid/api.h b/arch/x86/include/asm/cpuid/api.h
index d4e50e394e0b..3be741b9b461 100644
--- a/arch/x86/include/asm/cpuid/api.h
+++ b/arch/x86/include/asm/cpuid/api.h
@@ -393,7 +393,7 @@ static inline u32 cpuid_base_hypervisor(const char *sig, u32 leaves)
((struct cpuid_regs *)(cpuid_leaf(_cpuinfo, _leaf)))
#define __cpuid_assert_leaf_has_dynamic_subleaves(_cpuinfo, _leaf) \
- static_assert(ARRAY_SIZE((_cpuinfo)->cpuid.leaves.leaf_ ## _leaf ## _0) > 1);
+ static_assert(ARRAY_SIZE((_cpuinfo)->cpuid.leaves.leaf_ ## _leaf ## _n) > 1);
/**
* cpuid_subleaf_index() - Access parsed CPUID data at runtime subleaf index
@@ -415,7 +415,7 @@ static inline u32 cpuid_base_hypervisor(const char *sig, u32 leaves)
*
* Example usage::
*
- * const struct leaf_0x4_0 *l4;
+ * const struct leaf_0x4_n *l4;
*
* for (int i = 0; i < cpuid_subleaf_count(c, 0x4); i++) {
* l4 = cpuid_subleaf_index(c, 0x4, i);
@@ -432,7 +432,10 @@ static inline u32 cpuid_base_hypervisor(const char *sig, u32 leaves)
#define cpuid_subleaf_index(_cpuinfo, _leaf, _idx) \
({ \
__cpuid_assert_leaf_has_dynamic_subleaves(_cpuinfo, _leaf); \
- __cpuid_table_subleaf_idx(&(_cpuinfo)->cpuid, _leaf, 0, _idx); \
+ BUILD_BUG_ON(__builtin_constant_p(_idx) && \
+ ((_idx) < CPUID_LEAF_ ## _leaf ## _N_FIRST || \
+ (_idx) > CPUID_LEAF_ ## _leaf ## _N_LAST)); \
+ __cpuid_table_subleaf_idx(&(_cpuinfo)->cpuid, _leaf, n, _idx); \
})
/**
@@ -464,7 +467,7 @@ static inline u32 cpuid_base_hypervisor(const char *sig, u32 leaves)
#define cpuid_subleaf_count(_cpuinfo, _leaf) \
({ \
__cpuid_assert_leaf_has_dynamic_subleaves(_cpuinfo, _leaf); \
- __cpuid_leaves_subleaf_info(&(_cpuinfo)->cpuid.leaves, _leaf, 0).nr_entries; \
+ __cpuid_leaves_subleaf_info(&(_cpuinfo)->cpuid.leaves, _leaf, n).nr_entries; \
})
/*
diff --git a/arch/x86/include/asm/cpuid/leaf_defs.h b/arch/x86/include/asm/cpuid/leaf_defs.h
new file mode 100644
index 000000000000..bb3d744939c2
--- /dev/null
+++ b/arch/x86/include/asm/cpuid/leaf_defs.h
@@ -0,0 +1,12 @@
+CPUID_LEAF(0x0, 0)
+CPUID_LEAF(0x1, 0)
+CPUID_LEAF(0x2, 0)
+CPUID_LEAF_N(0x4, 0, 7)
+CPUID_LEAF(0xd, 0)
+CPUID_LEAF(0xd, 1)
+CPUID_LEAF_N(0xd, 2, 63)
+CPUID_LEAF(0x16, 0)
+CPUID_LEAF(0x80000000, 0)
+CPUID_LEAF(0x80000005, 0)
+CPUID_LEAF(0x80000006, 0)
+CPUID_LEAF_N(0x8000001d, 0, 7)
\ No newline at end of file
diff --git a/arch/x86/include/asm/cpuid/types.h b/arch/x86/include/asm/cpuid/types.h
index c044f7bc7137..2a0d30e13b71 100644
--- a/arch/x86/include/asm/cpuid/types.h
+++ b/arch/x86/include/asm/cpuid/types.h
@@ -153,7 +153,7 @@ struct leaf_query_info {
/**
* __CPUID_LEAF() - Define CPUID output storage and query info entry
- * @_name: Struct type name of the CPUID leaf/subleaf (e.g. 'leaf_0x4_0').
+ * @_name: Struct type name of the CPUID leaf/subleaf (e.g. 'leaf_0x4_n').
* Such types are defined at <cpuid/leaf_types.h>, and follow the
* format 'struct leaf_0xN_M', where 0xN is the leaf and M is the
* subleaf.
@@ -183,19 +183,19 @@ struct leaf_query_info {
*
* The example invocation for CPUID(0x4) storage::
*
- * __CPUID_LEAF(leaf_0x4_0, 8);
+ * __CPUID_LEAF(leaf_0x4_n, 8);
*
* generates storage entries in the form:
*
- * struct leaf_0x4_0 leaf_0x4_0[8];
- * struct leaf_query_info leaf_0x4_0_info;
+ * struct leaf_0x4_n leaf_0x4_n[8];
+ * struct leaf_query_info leaf_0x4_n_info;
*
- * where the 'leaf_0x4_0[8]' storage array can accommodate the output of
+ * where the 'leaf_0x4_n[8]' storage array can accommodate the output of
* CPUID(0x4) subleaves 0->7, since they all have the same output format.
*/
#define __CPUID_LEAF(_name, _count) \
struct _name _name[_count]; \
- struct leaf_query_info _name##_info
+ struct leaf_query_info _name##_info;
/**
* CPUID_LEAF() - Define a CPUID storage entry in 'struct cpuid_leaves'
@@ -205,23 +205,30 @@ struct leaf_query_info {
*
* Convenience wrapper around __CPUID_LEAF().
*/
-#define CPUID_LEAF(_leaf, _subleaf, _count) \
- __CPUID_LEAF(leaf_ ## _leaf ## _ ## _subleaf, _count)
+#define CPUID_LEAF(_leaf, _subleaf) \
+ __CPUID_LEAF(leaf_ ## _leaf ## _ ## _subleaf, 1)
+
+#define CPUID_LEAF_N(_leaf, _first, _last) \
+ __CPUID_LEAF(leaf_ ## _leaf ## _n, _last - _first + 1)
+
+
/*
* struct cpuid_leaves - Structured CPUID data repository
*/
struct cpuid_leaves {
- /* leaf subleaf count */
- CPUID_LEAF(0x0, 0, 1);
- CPUID_LEAF(0x1, 0, 1);
- CPUID_LEAF(0x2, 0, 1);
- CPUID_LEAF(0x4, 0, 8);
- CPUID_LEAF(0x16, 0, 1);
- CPUID_LEAF(0x80000000, 0, 1);
- CPUID_LEAF(0x80000005, 0, 1);
- CPUID_LEAF(0x80000006, 0, 1);
- CPUID_LEAF(0x8000001d, 0, 8);
+#include "leaf_defs.h"
+};
+
+#undef CPUID_LEAF
+#undef CPUID_LEAF_N
+#define CPUID_LEAF(_leaf, _subleaf)
+#define CPUID_LEAF_N(_leaf, _first, _last) \
+ CPUID_LEAF_ ## _leaf ## _N_FIRST = _first, \
+ CPUID_LEAF_ ## _leaf ## _N_LAST = _last,
+
+enum cpuid_dynamic_leaf_ranges {
+#include "leaf_defs.h"
};
/*
base-commit: 2f267fb52973ddf5949127628a4338d4d976ff11
--
Powered by blists - more mailing lists