Since as a bug workaround (likely inapplicable to any 64-bit CPUs, but
it probably doesn't hurt to keep this) we clear %ecx on input anyway,
we can as well fall back to cpuid_count(). This allows getting rid of
four risky casts and makes things type-safe. The latter aspect requires
two type adjustments elsewhere. While adjusting init_intel_cacheinfo(),
convert three other local variables there as well. For the struct
cpuinfo_x86 change it is relevant to note that no 64-bit CPU comes
without CPUID support, and hence cpuid_level is never set to -1; the
comment there was simply stale.
No functional change intended, yet of course generated code isn't
identical.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
It may also be interesting to see whether this addresses issues Coverity
recently spotted (in error), in spec-ctrl.c:print_details(). Andrew
considers cpuid() being (just) a macro as a possible reason.
I'm willing to switch to an inline function, but I chose the macro route
because this way it's less code (and hence less redundancy) overall,
without any loss of functionality.
--- a/xen/arch/x86/cpu/intel_cacheinfo.c
+++ b/xen/arch/x86/cpu/intel_cacheinfo.c
@@ -172,8 +172,7 @@ void init_intel_cacheinfo(struct cpuinfo
c->x86_vendor != X86_VENDOR_SHANGHAI)
{
/* supports eax=2 call */
- int i, j, n;
- int regs[4];
+ unsigned int i, j, n, regs[4];
unsigned char *dp = (unsigned char *)regs;
int only_trace = 0;
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -23,7 +23,7 @@ struct cpuinfo_x86 {
unsigned char x86_vendor; /* CPU vendor */
unsigned char x86_model;
unsigned char x86_mask;
- int cpuid_level; /* Maximum supported CPUID level, -1=no CPUID */
+ unsigned int cpuid_level; /* Maximum supported CPUID level */
unsigned int extended_cpuid_level; /* Maximum supported CPUID extended level */
unsigned int x86_capability[NCAPINTS];
char x86_vendor_id[16];
--- a/xen/arch/x86/include/asm/processor.h
+++ b/xen/arch/x86/include/asm/processor.h
@@ -131,13 +131,8 @@ static inline int cpu_nr_siblings(unsign
* clear %ecx since some cpus (Cyrix MII) do not set or clear %ecx
* resulting in stale register contents being returned.
*/
-#define cpuid(_op,_eax,_ebx,_ecx,_edx) \
- asm volatile ( "cpuid" \
- : "=a" (*(int *)(_eax)), \
- "=b" (*(int *)(_ebx)), \
- "=c" (*(int *)(_ecx)), \
- "=d" (*(int *)(_edx)) \
- : "0" (_op), "2" (0) )
+#define cpuid(op, eax, ebx, ecx, edx) \
+ cpuid_count(op, 0, eax, ebx, ecx, edx)
/* Some CPUID calls want 'count' to be placed in ecx */
static inline void cpuid_count(
On 09/08/2023 1:29 pm, Jan Beulich wrote: > Since as a bug workaround (likely inapplicable to any 64-bit CPUs, but > it probably doesn't hurt to keep this) we clear %ecx on input anyway, > we can as well fall back to cpuid_count(). This allows getting rid of > four risky casts and makes things type-safe. The latter aspect requires > two type adjustments elsewhere. While adjusting init_intel_cacheinfo(), > convert three other local variables there as well. For the struct > cpuinfo_x86 change it is relevant to note that no 64-bit CPU comes > without CPUID support, and hence cpuid_level is never set to -1; the > comment there was simply stale. > > No functional change intended, yet of course generated code isn't > identical. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Hmm, far less invasive than when I last tried this, but that was years ago. > --- a/xen/arch/x86/include/asm/processor.h > +++ b/xen/arch/x86/include/asm/processor.h > @@ -131,13 +131,8 @@ static inline int cpu_nr_siblings(unsign > * clear %ecx since some cpus (Cyrix MII) do not set or clear %ecx > * resulting in stale register contents being returned. > */ > -#define cpuid(_op,_eax,_ebx,_ecx,_edx) \ > - asm volatile ( "cpuid" \ > - : "=a" (*(int *)(_eax)), \ > - "=b" (*(int *)(_ebx)), \ > - "=c" (*(int *)(_ecx)), \ > - "=d" (*(int *)(_edx)) \ > - : "0" (_op), "2" (0) ) > +#define cpuid(op, eax, ebx, ecx, edx) \ > + cpuid_count(op, 0, eax, ebx, ecx, edx) Can we swap op for leaf seeing as both are changing? Preferably with that changed, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
© 2016 - 2024 Red Hat, Inc.