Credit2 has no buisness including asm/cpufeature.h or asm/processor.h.

This was caused by a bad original abstraction, and an even less wise attempt
to fix the build on my behalf.  It is also the sole reason why PPC and RISC-V
need cpufeature.h header.

Worst of all, cpu_data[cpu].x86_num_siblings doesn't even have the same
meaning between vendors on x86 CPUS.

Implement cpu_nr_siblings() locally in credit2.c, leaving behind a TODO.  Drop
the stub from each architecture.

Fixes: 8e2aa76dc167 ("xen: credit2: limit the max number of CPUs in a runqueue")
Fixes: ad33a573c009 ("xen/credit2: Fix build following c/s 8e2aa76dc (take 2)")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
CC: Shawn Anastasio <sanastasio@raptorengineering.com>
CC: Juergen Gross <jgross@suse.com>
---
 xen/arch/arm/include/asm/cpufeature.h   |  5 -----
 xen/arch/ppc/include/asm/cpufeature.h   | 10 ----------
 xen/arch/riscv/include/asm/cpufeature.h | 23 -----------------------
 xen/arch/x86/include/asm/processor.h    |  5 -----
 xen/common/sched/credit2.c              | 15 +++++++++++++--
 5 files changed, 13 insertions(+), 45 deletions(-)
 delete mode 100644 xen/arch/ppc/include/asm/cpufeature.h
 delete mode 100644 xen/arch/riscv/include/asm/cpufeature.h

diff --git a/xen/arch/arm/include/asm/cpufeature.h b/xen/arch/arm/include/asm/cpufeature.h
index 969e043f5bda..50297e53d90e 100644
--- a/xen/arch/arm/include/asm/cpufeature.h
+++ b/xen/arch/arm/include/asm/cpufeature.h
@@ -98,11 +98,6 @@ static inline bool cpus_have_cap(unsigned int num)
     return test_bit(num, cpu_hwcaps);
 }
 
-static inline int cpu_nr_siblings(unsigned int cpu)
-{
-    return 1;
-}
-
 /* System capability check for constant cap */
 #define cpus_have_const_cap(num) ({                 \
         register_t __ret;                           \
diff --git a/xen/arch/ppc/include/asm/cpufeature.h b/xen/arch/ppc/include/asm/cpufeature.h
deleted file mode 100644
index 1c5946512bc5..000000000000
--- a/xen/arch/ppc/include/asm/cpufeature.h
+++ /dev/null
@@ -1,10 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_CPUFEATURE_H__
-#define __ASM_PPC_CPUFEATURE_H__
-
-static inline int cpu_nr_siblings(unsigned int cpu)
-{
-    return 1;
-}
-
-#endif /* __ASM_PPC_CPUFEATURE_H__ */
diff --git a/xen/arch/riscv/include/asm/cpufeature.h b/xen/arch/riscv/include/asm/cpufeature.h
deleted file mode 100644
index 41a792b0b273..000000000000
--- a/xen/arch/riscv/include/asm/cpufeature.h
+++ /dev/null
@@ -1,23 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef ASM__RISCV__CPUFEATURE_H
-#define ASM__RISCV__CPUFEATURE_H
-
-#ifndef __ASSEMBLY__
-
-static inline unsigned int cpu_nr_siblings(unsigned int cpu)
-{
-    return 1;
-}
-
-#endif /* __ASSEMBLY__ */
-
-#endif /* ASM__RISCV__CPUFEATURE_H */
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h
index 877651212273..d247ef8dd226 100644
--- a/xen/arch/x86/include/asm/processor.h
+++ b/xen/arch/x86/include/asm/processor.h
@@ -118,11 +118,6 @@ extern void init_intel_cacheinfo(struct cpuinfo_x86 *c);
 
 unsigned int apicid_to_socket(unsigned int apicid);
 
-static inline int cpu_nr_siblings(unsigned int cpu)
-{
-    return cpu_data[cpu].x86_num_siblings;
-}
-
 /* Some CPUID calls want 'count' to be placed in ecx */
 static inline void cpuid_count(
     unsigned int op,
diff --git a/xen/common/sched/credit2.c b/xen/common/sched/credit2.c
index 76a273d4f6d5..4b2ef034ca84 100644
--- a/xen/common/sched/credit2.c
+++ b/xen/common/sched/credit2.c
@@ -26,8 +26,6 @@
 #include <xen/trace.h>
 #include <xen/cpu.h>
 #include <xen/keyhandler.h>
-#include <asm/cpufeature.h>
-#include <asm/processor.h>
 
 #include "private.h"
 
@@ -35,6 +33,19 @@
 /* #define d2printk printk */
 #define d2printk(x...)
 
+/*
+ * TODO: Abstract this properly, and figure out what Credit2 wants to do with
+ *       the fact that x86_num_siblings doesn't even have the same meaning
+ *       between x86 vendors.
+ */
+static unsigned int cpu_nr_siblings(unsigned int cpu)
+{
+#ifdef CONFIG_X86
+    return cpu_data[cpu].x86_num_siblings;
+#else
+    return 1;
+#endif
+}
 
 /*
  * Credit2 tracing events ("only" 512 available!). Check
-- 
2.39.5

Hi Andrew,

On 12/18/24 6:49 AM, Andrew Cooper wrote:
> Credit2 has no buisness including asm/cpufeature.h or asm/processor.h.
> 
> This was caused by a bad original abstraction, and an even less wise attempt
> to fix the build on my behalf.  It is also the sole reason why PPC and RISC-V
> need cpufeature.h header.
> 
> Worst of all, cpu_data[cpu].x86_num_siblings doesn't even have the same
> meaning between vendors on x86 CPUS.
> 
> Implement cpu_nr_siblings() locally in credit2.c, leaving behind a TODO.  Drop
> the stub from each architecture.
> 
> Fixes: 8e2aa76dc167 ("xen: credit2: limit the max number of CPUs in a runqueue")
> Fixes: ad33a573c009 ("xen/credit2: Fix build following c/s 8e2aa76dc (take 2)")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>

Thanks,
Shawn

On 12/18/24 1:49 PM, Andrew Cooper wrote:
> Credit2 has no buisness including asm/cpufeature.h or asm/processor.h.
>
> This was caused by a bad original abstraction, and an even less wise attempt
> to fix the build on my behalf.  It is also the sole reason why PPC and RISC-V
> need cpufeature.h header.
>
> Worst of all, cpu_data[cpu].x86_num_siblings doesn't even have the same
> meaning between vendors on x86 CPUS.
>
> Implement cpu_nr_siblings() locally in credit2.c, leaving behind a TODO.  Drop
> the stub from each architecture.
>
> Fixes: 8e2aa76dc167 ("xen: credit2: limit the max number of CPUs in a runqueue")
> Fixes: ad33a573c009 ("xen/credit2: Fix build following c/s 8e2aa76dc (take 2)")
> Signed-off-by: Andrew Cooper<andrew.cooper3@citrix.com>

Reviewed-By: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii

> ---
> CC: Jan Beulich<JBeulich@suse.com>
> CC: Roger Pau Monné<roger.pau@citrix.com>
> CC: Stefano Stabellini<sstabellini@kernel.org>
> CC: Julien Grall<julien@xen.org>
> CC: Volodymyr Babchuk<Volodymyr_Babchuk@epam.com>
> CC: Bertrand Marquis<bertrand.marquis@arm.com>
> CC: Michal Orzel<michal.orzel@amd.com>
> CC: Oleksii Kurochko<oleksii.kurochko@gmail.com>
> CC: Shawn Anastasio<sanastasio@raptorengineering.com>
> CC: Juergen Gross<jgross@suse.com>
> ---
>   xen/arch/arm/include/asm/cpufeature.h   |  5 -----
>   xen/arch/ppc/include/asm/cpufeature.h   | 10 ----------
>   xen/arch/riscv/include/asm/cpufeature.h | 23 -----------------------
>   xen/arch/x86/include/asm/processor.h    |  5 -----
>   xen/common/sched/credit2.c              | 15 +++++++++++++--
>   5 files changed, 13 insertions(+), 45 deletions(-)
>   delete mode 100644 xen/arch/ppc/include/asm/cpufeature.h
>   delete mode 100644 xen/arch/riscv/include/asm/cpufeature.h
>
> diff --git a/xen/arch/arm/include/asm/cpufeature.h b/xen/arch/arm/include/asm/cpufeature.h
> index 969e043f5bda..50297e53d90e 100644
> --- a/xen/arch/arm/include/asm/cpufeature.h
> +++ b/xen/arch/arm/include/asm/cpufeature.h
> @@ -98,11 +98,6 @@ static inline bool cpus_have_cap(unsigned int num)
>       return test_bit(num, cpu_hwcaps);
>   }
>   
> -static inline int cpu_nr_siblings(unsigned int cpu)
> -{
> -    return 1;
> -}
> -
>   /* System capability check for constant cap */
>   #define cpus_have_const_cap(num) ({                 \
>           register_t __ret;                           \
> diff --git a/xen/arch/ppc/include/asm/cpufeature.h b/xen/arch/ppc/include/asm/cpufeature.h
> deleted file mode 100644
> index 1c5946512bc5..000000000000
> --- a/xen/arch/ppc/include/asm/cpufeature.h
> +++ /dev/null
> @@ -1,10 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -#ifndef __ASM_PPC_CPUFEATURE_H__
> -#define __ASM_PPC_CPUFEATURE_H__
> -
> -static inline int cpu_nr_siblings(unsigned int cpu)
> -{
> -    return 1;
> -}
> -
> -#endif /* __ASM_PPC_CPUFEATURE_H__ */
> diff --git a/xen/arch/riscv/include/asm/cpufeature.h b/xen/arch/riscv/include/asm/cpufeature.h
> deleted file mode 100644
> index 41a792b0b273..000000000000
> --- a/xen/arch/riscv/include/asm/cpufeature.h
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -#ifndef ASM__RISCV__CPUFEATURE_H
> -#define ASM__RISCV__CPUFEATURE_H
> -
> -#ifndef __ASSEMBLY__
> -
> -static inline unsigned int cpu_nr_siblings(unsigned int cpu)
> -{
> -    return 1;
> -}
> -
> -#endif /* __ASSEMBLY__ */
> -
> -#endif /* ASM__RISCV__CPUFEATURE_H */
> -
> -/*
> - * Local variables:
> - * mode: C
> - * c-file-style: "BSD"
> - * c-basic-offset: 4
> - * indent-tabs-mode: nil
> - * End:
> - */
> diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h
> index 877651212273..d247ef8dd226 100644
> --- a/xen/arch/x86/include/asm/processor.h
> +++ b/xen/arch/x86/include/asm/processor.h
> @@ -118,11 +118,6 @@ extern void init_intel_cacheinfo(struct cpuinfo_x86 *c);
>   
>   unsigned int apicid_to_socket(unsigned int apicid);
>   
> -static inline int cpu_nr_siblings(unsigned int cpu)
> -{
> -    return cpu_data[cpu].x86_num_siblings;
> -}
> -
>   /* Some CPUID calls want 'count' to be placed in ecx */
>   static inline void cpuid_count(
>       unsigned int op,
> diff --git a/xen/common/sched/credit2.c b/xen/common/sched/credit2.c
> index 76a273d4f6d5..4b2ef034ca84 100644
> --- a/xen/common/sched/credit2.c
> +++ b/xen/common/sched/credit2.c
> @@ -26,8 +26,6 @@
>   #include <xen/trace.h>
>   #include <xen/cpu.h>
>   #include <xen/keyhandler.h>
> -#include <asm/cpufeature.h>
> -#include <asm/processor.h>
>   
>   #include "private.h"
>   
> @@ -35,6 +33,19 @@
>   /* #define d2printk printk */
>   #define d2printk(x...)
>   
> +/*
> + * TODO: Abstract this properly, and figure out what Credit2 wants to do with
> + *       the fact that x86_num_siblings doesn't even have the same meaning
> + *       between x86 vendors.
> + */
> +static unsigned int cpu_nr_siblings(unsigned int cpu)
> +{
> +#ifdef CONFIG_X86
> +    return cpu_data[cpu].x86_num_siblings;
> +#else
> +    return 1;
> +#endif
> +}
>   
>   /*
>    * Credit2 tracing events ("only" 512 available!). Check

On 18.12.24 13:49, Andrew Cooper wrote:
> Credit2 has no buisness including asm/cpufeature.h or asm/processor.h.
> 
> This was caused by a bad original abstraction, and an even less wise attempt
> to fix the build on my behalf.  It is also the sole reason why PPC and RISC-V
> need cpufeature.h header.
> 
> Worst of all, cpu_data[cpu].x86_num_siblings doesn't even have the same
> meaning between vendors on x86 CPUS.
> 
> Implement cpu_nr_siblings() locally in credit2.c, leaving behind a TODO.  Drop
> the stub from each architecture.
> 
> Fixes: 8e2aa76dc167 ("xen: credit2: limit the max number of CPUs in a runqueue")
> Fixes: ad33a573c009 ("xen/credit2: Fix build following c/s 8e2aa76dc (take 2)")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen