Re: [PATCH] core-parking: fix build with gcc12 and NR_CPUS=1

[Julien Grall <julien@xxxxxxx>]

Hi,

On 09/09/2022 13:14, Jan Beulich wrote:
> On 09.09.2022 13:00, Julien Grall wrote:
> > On 09/09/2022 11:18, Jan Beulich wrote:
> > > Gcc12 takes issue with core_parking_remove()'s
> > >
> > >       for ( ; i < cur_idle_nums; ++i )
> > >           core_parking_cpunum[i] = core_parking_cpunum[i + 1];
> > >
> > > complaining that the right hand side array access is past the bounds of
> > > 1. Clearly the compiler can't know that cur_idle_nums can only ever be
> > > zero in this case (as the sole CPU cannot be parked).
> > >
> > > Beyond addressing the immediate issue also adjust core_parking_init():
> > > There's no point registering any policy when there's no CPU to park.
> > > Since this still doesn't result in the compiler spotting that
> > > core_parking_policy is never written (and hence is continuously NULL),
> > > also amend core_parking_helper() to avoid eventual similar issues there
> > > (minimizing generated code at the same time).
> > Given that CONFIG_NR_CPUS is a build time option. Wouldn't it be better
> > to set CONFIG_CORE_PARKING=n and provide dummy helper for any function
> > that may be called unconditionally?
> That might be an option, yes; not sure whether that's really better. It's
> likely a more intrusive change ...
I quickly try to implement it (see below) and the result is IHMO a lot 
nicer and make clear the code is not necessary on uni-processor.
This is only compile tested.

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 6a7825f4ba3c..f9a3daccdc92 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -10,7 +10,7 @@ config X86
        select ALTERNATIVE_CALL
        select ARCH_MAP_DOMAIN_PAGE
        select ARCH_SUPPORTS_INT128
-       select CORE_PARKING
+       select CORE_PARKING if NR_CPUS > 1
        select HAS_ALTERNATIVE
        select HAS_COMPAT
        select HAS_CPUFREQ
diff --git a/xen/arch/x86/include/asm/smp.h b/xen/arch/x86/include/asm/smp.h
index 41a3b6a0dadf..7baca00be182 100644
--- a/xen/arch/x86/include/asm/smp.h
+++ b/xen/arch/x86/include/asm/smp.h
@@ -61,7 +61,15 @@ long cf_check cpu_up_helper(void *data);
 long cf_check cpu_down_helper(void *data);

 long cf_check core_parking_helper(void *data);
+
+#ifdef CONFIG_CORE_PARKING
 bool core_parking_remove(unsigned int cpu);
+#else
+static inline bool core_parking_remove(unsigned int cpu)
+{
+    return false;
+}
+#endif
 uint32_t get_cur_idle_nums(void);

 /*
diff --git a/xen/arch/x86/platform_hypercall.c 
b/xen/arch/x86/platform_hypercall.c
index a7341dc3d7d3..5d13fac41bd4 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -718,6 +718,7 @@ ret_t do_platform_op(
                       op->u.mem_add.pxm);
         break;

+#ifdef CONFIG_CORE_PARKING
     case XENPF_core_parking:
     {
         uint32_t idle_nums;
@@ -743,6 +744,7 @@ ret_t do_platform_op(
         }
     }
     break;
+#endif

     case XENPF_resource_op:
     {

Cheers,

--
Julien Grall