[v5] x86/cpu: Rework topology evaluation

[patch v5 01/19] x86/cpu: Provide cpuid_read() et al.

Posted by Thomas Gleixner 1 year, 11 months ago

From: Thomas Gleixner <tglx@linutronix.de>

Provide a few helper functions to read CPUID leafs or individual registers
into a data structure without requiring unions.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Juergen Gross <jgross@suse.com>
Tested-by: Sohil Mehta <sohil.mehta@intel.com>
Tested-by: Michael Kelley <mhklinux@outlook.com>
Tested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Tested-by: Zhang Rui <rui.zhang@intel.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>


---
 arch/x86/include/asm/cpuid.h |   36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)
---
--- a/arch/x86/include/asm/cpuid.h
+++ b/arch/x86/include/asm/cpuid.h
@@ -127,6 +127,42 @@ static inline unsigned int cpuid_edx(uns
 	return edx;
 }
 
+static inline void __cpuid_read(unsigned int leaf, unsigned int subleaf, u32 *regs)
+{
+	regs[CPUID_EAX] = leaf;
+	regs[CPUID_ECX] = subleaf;
+	__cpuid(regs, regs + 1, regs + 2, regs + 3);
+}
+
+#define cpuid_subleaf(leaf, subleaf, regs) {		\
+	BUILD_BUG_ON(sizeof(*(regs)) != 16);		\
+	__cpuid_read(leaf, subleaf, (u32 *)(regs));	\
+}
+
+#define cpuid_leaf(leaf, regs) {			\
+	BUILD_BUG_ON(sizeof(*(regs)) != 16);		\
+	__cpuid_read(leaf, 0, (u32 *)(regs));		\
+}
+
+static inline void __cpuid_read_reg(unsigned int leaf, unsigned int subleaf,
+				    enum cpuid_regs_idx regidx, u32 *reg)
+{
+	u32 regs[4];
+
+	__cpuid_read(leaf, subleaf, regs);
+	*reg = regs[regidx];
+}
+
+#define cpuid_subleaf_reg(leaf, subleaf, regidx, reg) {		\
+	BUILD_BUG_ON(sizeof(*(reg)) != 4);			\
+	__cpuid_read_reg(leaf, subleaf, regidx, (u32 *)(reg));	\
+}
+
+#define cpuid_leaf_reg(leaf, regidx, reg) {			\
+	BUILD_BUG_ON(sizeof(*(reg)) != 4);			\
+	__cpuid_read_reg(leaf, 0, regidx, (u32 *)(reg));	\
+}
+
 static __always_inline bool cpuid_function_is_indexed(u32 function)
 {
 	switch (function) {

Re: [patch v5 01/19] x86/cpu: Provide cpuid_read() et al.

Posted by Borislav Petkov 1 year, 11 months ago

On Tue, Jan 23, 2024 at 01:53:30PM +0100, Thomas Gleixner wrote:
> +static inline void __cpuid_read(unsigned int leaf, unsigned int subleaf, u32 *regs)
> +{
> +	regs[CPUID_EAX] = leaf;
> +	regs[CPUID_ECX] = subleaf;
> +	__cpuid(regs, regs + 1, regs + 2, regs + 3);

You have defines for the regs - might as well use them:

	__cpuid(regs, regs + CPUID_EBX, regs + CPUID_ECX, regs + CPUID_EDX);

> +}
> +
> +#define cpuid_subleaf(leaf, subleaf, regs) {		\
> +	BUILD_BUG_ON(sizeof(*(regs)) != 16);		\
> +	__cpuid_read(leaf, subleaf, (u32 *)(regs));	\
> +}
> +
> +#define cpuid_leaf(leaf, regs) {			\
> +	BUILD_BUG_ON(sizeof(*(regs)) != 16);		\
> +	__cpuid_read(leaf, 0, (u32 *)(regs));		\
> +}
> +
> +static inline void __cpuid_read_reg(unsigned int leaf, unsigned int subleaf,
> +				    enum cpuid_regs_idx regidx, u32 *reg)
> +{
> +	u32 regs[4];
> +
> +	__cpuid_read(leaf, subleaf, regs);
> +	*reg = regs[regidx];

Why not do

	return regs[regidx];

instead?

-- 
Regards/Gruss,
    Boris.

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

Re: [patch v5 01/19] x86/cpu: Provide cpuid_read() et al.

Posted by Borislav Petkov 1 year, 11 months ago

On Wed, Jan 24, 2024 at 01:25:12PM +0100, Borislav Petkov wrote:
> > +static inline void __cpuid_read_reg(unsigned int leaf, unsigned int subleaf,
> > +				    enum cpuid_regs_idx regidx, u32 *reg)
> > +{
> > +	u32 regs[4];
> > +
> > +	__cpuid_read(leaf, subleaf, regs);
> > +	*reg = regs[regidx];
> 
> Why not do
> 
> 	return regs[regidx];
> 
> instead?

Or do you really want to be able to use anonymous structs with bitfields
in them and then convert them to a u32 * when passing in to
cpuid_leaf_reg() etc in order to save yourself all the masking and
shifting and read out the bitfields directly?

I'm looking at the parse_topology() use case.

Looks like it...

-- 
Regards/Gruss,
    Boris.

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

Re: [patch v5 01/19] x86/cpu: Provide cpuid_read() et al.

Posted by Thomas Gleixner 1 year, 10 months ago

On Wed, Jan 24 2024 at 21:02, Borislav Petkov wrote:
> On Wed, Jan 24, 2024 at 01:25:12PM +0100, Borislav Petkov wrote:
>> > +static inline void __cpuid_read_reg(unsigned int leaf, unsigned int subleaf,
>> > +				    enum cpuid_regs_idx regidx, u32 *reg)
>> > +{
>> > +	u32 regs[4];
>> > +
>> > +	__cpuid_read(leaf, subleaf, regs);
>> > +	*reg = regs[regidx];
>> 
>> Why not do
>> 
>> 	return regs[regidx];
>> 
>> instead?
>
> Or do you really want to be able to use anonymous structs with bitfields
> in them and then convert them to a u32 * when passing in to
> cpuid_leaf_reg() etc in order to save yourself all the masking and
> shifting and read out the bitfields directly?
>
> I'm looking at the parse_topology() use case.
>
> Looks like it...

Yes, that's the idea.