[PATCH] x86: don't include processor.h from system.h

Jan Beulich posted 1 patch 1 year, 1 month ago
Failed in applying to current master (apply log)
[PATCH] x86: don't include processor.h from system.h
Posted by Jan Beulich 1 year, 1 month ago
processor.h in particular pulls in xen/smp.h, which is overly heavy for
a supposedly pretty fundamental header like system.h. To keep things
building, move the declarations of struct cpuinfo_x86 and boot_cpu_data
to asm/cpufeature.h (which arguably also is where they belong). In the
course of the move switch away from using fixed-width types and convert
plain "int" to "unsigned int" for the two x86_cache_* fields.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -19,6 +19,30 @@
 #ifndef __ASSEMBLY__
 #include <xen/bitops.h>
 
+struct cpuinfo_x86 {
+    unsigned char x86;                 /* CPU family */
+    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 extended_cpuid_level; /* Maximum supported CPUID extended level */
+    unsigned int x86_capability[NCAPINTS];
+    char x86_vendor_id[16];
+    char x86_model_id[64];
+    unsigned int x86_cache_size;       /* in KB - valid only when supported */
+    unsigned int x86_cache_alignment;  /* In bytes */
+    unsigned int x86_max_cores;        /* cpuid returned max cores value */
+    unsigned int booted_cores;         /* number of cores as seen by OS */
+    unsigned int x86_num_siblings;     /* cpuid logical cpus per chip value */
+    unsigned int apicid;
+    unsigned int phys_proc_id;         /* package ID of each logical CPU */
+    unsigned int cpu_core_id;          /* core ID of each logical CPU */
+    unsigned int compute_unit_id;      /* AMD compute unit ID of each logical CPU */
+    unsigned short x86_clflush_size;
+} __cacheline_aligned;
+
+extern struct cpuinfo_x86 boot_cpu_data;
+
 #define cpu_has(c, bit)		test_bit(bit, (c)->x86_capability)
 #define boot_cpu_has(bit)	test_bit(bit, boot_cpu_data.x86_capability)
 
--- a/xen/arch/x86/include/asm/processor.h
+++ b/xen/arch/x86/include/asm/processor.h
@@ -118,34 +118,6 @@ struct x86_cpu_id {
     const void *driver_data;
 };
 
-struct cpuinfo_x86 {
-    uint8_t x86;            /* CPU family */
-    uint8_t x86_vendor;     /* CPU vendor */
-    uint8_t x86_model;
-    uint8_t x86_mask;
-    int  cpuid_level;    /* Maximum supported CPUID level, -1=no CPUID */
-    uint32_t extended_cpuid_level; /* Maximum supported CPUID extended level */
-    unsigned int x86_capability[NCAPINTS];
-    char x86_vendor_id[16];
-    char x86_model_id[64];
-    int  x86_cache_size; /* in KB - valid for CPUS which support this call  */
-    int  x86_cache_alignment;    /* In bytes */
-    uint32_t x86_max_cores;   /* cpuid returned max cores value */
-    uint32_t booted_cores;    /* number of cores as seen by OS */
-    uint32_t x86_num_siblings; /* cpuid logical cpus per chip value */
-    uint32_t apicid;
-    uint32_t phys_proc_id;    /* package ID of each logical CPU */
-    uint32_t cpu_core_id;     /* core ID of each logical CPU */
-    uint32_t compute_unit_id; /* AMD compute unit ID of each logical CPU */
-    unsigned short x86_clflush_size;
-} __cacheline_aligned;
-
-/*
- * capabilities of CPUs
- */
-
-extern struct cpuinfo_x86 boot_cpu_data;
-
 extern struct cpuinfo_x86 cpu_data[];
 #define current_cpu_data cpu_data[smp_processor_id()]
 
--- a/xen/arch/x86/include/asm/system.h
+++ b/xen/arch/x86/include/asm/system.h
@@ -3,7 +3,8 @@
 
 #include <xen/lib.h>
 #include <xen/bitops.h>
-#include <asm/processor.h>
+#include <asm/cpufeature.h>
+#include <asm/x86-defns.h>
 
 static inline void wbinvd(void)
 {
Re: [PATCH] x86: don't include processor.h from system.h
Posted by Andrew Cooper 1 year, 1 month ago
On 16/03/2023 10:09 am, Jan Beulich wrote:
> processor.h in particular pulls in xen/smp.h, which is overly heavy for
> a supposedly pretty fundamental header like system.h. To keep things
> building, move the declarations of struct cpuinfo_x86 and boot_cpu_data
> to asm/cpufeature.h (which arguably also is where they belong). In the
> course of the move switch away from using fixed-width types and convert
> plain "int" to "unsigned int" for the two x86_cache_* fields.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

I was genuinely thinking of starting to pull the other cpuid() stuff out
of processor.h in order to start tackling this problem.

IMO system.h is still overly large, but this is certainly a step in the
right direction.