[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5D7935C3-8CAE-4821-8E31-A43169B8CB83@zytor.com>
Date: Tue, 18 Mar 2025 11:48:41 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: mingo@...nel.org, linux-kernel@...r.kernel.org
CC: Juergen Gross <jgross@...e.com>,
Stefano Stabellini <sstabellini@...nel.org>,
"Ahmed S . Darwish" <darwi@...utronix.de>,
Andrew Cooper <andrew.cooper3@...rix.com>,
John Ogness <john.ogness@...utronix.de>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Peter Zijlstra <peterz@...radead.org>, Borislav Petkov <bp@...en8.de>,
Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 4/5] x86/cpuid: Standardize on u32 in <asm/cpuid/api.h>
On March 17, 2025 3:30:38 PM PDT, mingo@...nel.org wrote:
>From: Ingo Molnar <mingo@...nel.org>
>
>Convert all uses of 'unsigned int' to 'u32' in <asm/cpuid/api.h>.
>
>This is how a lot of the call sites are doing it, and the two
>types are equivalent in the C sense - but 'u32' better expresses
>that these are expressions of an immutable hardware ABI.
>
>Signed-off-by: Ingo Molnar <mingo@...nel.org>
>Cc: Ahmed S. Darwish <darwi@...utronix.de>
>Cc: Andrew Cooper <andrew.cooper3@...rix.com>
>Cc: "H. Peter Anvin" <hpa@...or.com>
>Cc: John Ogness <john.ogness@...utronix.de>
>Cc: x86-cpuid@...ts.linux.dev
>Link: https://lore.kernel.org/r/20250317164745.4754-3-darwi@linutronix.de
>---
> arch/x86/include/asm/cpuid/api.h | 40 ++++++++++++++++++++--------------------
> 1 file changed, 20 insertions(+), 20 deletions(-)
>
>diff --git a/arch/x86/include/asm/cpuid/api.h b/arch/x86/include/asm/cpuid/api.h
>index f26926ba5289..356db1894588 100644
>--- a/arch/x86/include/asm/cpuid/api.h
>+++ b/arch/x86/include/asm/cpuid/api.h
>@@ -22,8 +22,8 @@ static inline bool have_cpuid_p(void)
> }
> #endif
>
>-static inline void native_cpuid(unsigned int *eax, unsigned int *ebx,
>- unsigned int *ecx, unsigned int *edx)
>+static inline void native_cpuid(u32 *eax, u32 *ebx,
>+ u32 *ecx, u32 *edx)
> {
> /* ecx is often an input as well as an output. */
> asm volatile("cpuid"
>@@ -36,9 +36,9 @@ static inline void native_cpuid(unsigned int *eax, unsigned int *ebx,
> }
>
> #define NATIVE_CPUID_REG(reg) \
>-static inline unsigned int native_cpuid_##reg(unsigned int op) \
>+static inline u32 native_cpuid_##reg(u32 op) \
> { \
>- unsigned int eax = op, ebx, ecx = 0, edx; \
>+ u32 eax = op, ebx, ecx = 0, edx; \
> \
> native_cpuid(&eax, &ebx, &ecx, &edx); \
> \
>@@ -65,9 +65,9 @@ NATIVE_CPUID_REG(edx)
> * Clear ECX since some CPUs (Cyrix MII) do not set or clear ECX
> * resulting in stale register contents being returned.
> */
>-static inline void cpuid(unsigned int op,
>- unsigned int *eax, unsigned int *ebx,
>- unsigned int *ecx, unsigned int *edx)
>+static inline void cpuid(u32 op,
>+ u32 *eax, u32 *ebx,
>+ u32 *ecx, u32 *edx)
> {
> *eax = op;
> *ecx = 0;
>@@ -75,9 +75,9 @@ static inline void cpuid(unsigned int op,
> }
>
> /* Some CPUID calls want 'count' to be placed in ECX */
>-static inline void cpuid_count(unsigned int op, int count,
>- unsigned int *eax, unsigned int *ebx,
>- unsigned int *ecx, unsigned int *edx)
>+static inline void cpuid_count(u32 op, int count,
>+ u32 *eax, u32 *ebx,
>+ u32 *ecx, u32 *edx)
> {
> *eax = op;
> *ecx = count;
>@@ -88,43 +88,43 @@ static inline void cpuid_count(unsigned int op, int count,
> * CPUID functions returning a single datum:
> */
>
>-static inline unsigned int cpuid_eax(unsigned int op)
>+static inline u32 cpuid_eax(u32 op)
> {
>- unsigned int eax, ebx, ecx, edx;
>+ u32 eax, ebx, ecx, edx;
>
> cpuid(op, &eax, &ebx, &ecx, &edx);
>
> return eax;
> }
>
>-static inline unsigned int cpuid_ebx(unsigned int op)
>+static inline u32 cpuid_ebx(u32 op)
> {
>- unsigned int eax, ebx, ecx, edx;
>+ u32 eax, ebx, ecx, edx;
>
> cpuid(op, &eax, &ebx, &ecx, &edx);
>
> return ebx;
> }
>
>-static inline unsigned int cpuid_ecx(unsigned int op)
>+static inline u32 cpuid_ecx(u32 op)
> {
>- unsigned int eax, ebx, ecx, edx;
>+ u32 eax, ebx, ecx, edx;
>
> cpuid(op, &eax, &ebx, &ecx, &edx);
>
> return ecx;
> }
>
>-static inline unsigned int cpuid_edx(unsigned int op)
>+static inline u32 cpuid_edx(u32 op)
> {
>- unsigned int eax, ebx, ecx, edx;
>+ u32 eax, ebx, ecx, edx;
>
> cpuid(op, &eax, &ebx, &ecx, &edx);
>
> return edx;
> }
>
>-static inline void __cpuid_read(unsigned int leaf, unsigned int subleaf, u32 *regs)
>+static inline void __cpuid_read(u32 leaf, u32 subleaf, u32 *regs)
> {
> regs[CPUID_EAX] = leaf;
> regs[CPUID_ECX] = subleaf;
>@@ -141,7 +141,7 @@ static inline void __cpuid_read(unsigned int leaf, unsigned int subleaf, u32 *re
> __cpuid_read(leaf, 0, (u32 *)(regs)); \
> }
>
>-static inline void __cpuid_read_reg(unsigned int leaf, unsigned int subleaf,
>+static inline void __cpuid_read_reg(u32 leaf, u32 subleaf,
> enum cpuid_regs_idx regidx, u32 *reg)
> {
> u32 regs[4];
So in addition to avoid the in/out pointer hack, I would like to point out that cpuid() is now *exactly* the same as cpuid_count with a count of 0 (which is probably a good idea anyway.)
One more thing is that we ought to be able to make cpuid a const function, allowing the compiler to elide multiple calls. (Slight warning for feature-enabling MSRs changing CPUID), but that would require changing the API to returning a structure, since a pure or const structure can't return values by reference.
Powered by blists - more mailing lists