Vishal Chourasia <vish...@linux.ibm.com> writes: > On 17/11/23 4:52 am, Michael Ellerman wrote: >> Vishal Chourasia <vish...@linux.ibm.com> writes: >>> On 15/11/23 5:46 pm, Aneesh Kumar K V wrote: >>>> On 11/15/23 5:23 PM, Vishal Chourasia wrote: >>>>> On 15/11/23 1:39 pm, Aneesh Kumar K.V wrote: >>>>>> Vishal Chourasia <vish...@linux.ibm.com> writes: >>>>>> >>>>>>> This patch modifies the ARCH_HIBERNATION_POSSIBLE option to ensure that >>>>>>> it >>>>>>> correctly depends on these PowerPC configurations being enabled. As a >>>>>>> result, >>>>>>> it prevents the HOTPLUG_CPU from being selected when the required >>>>>>> dependencies >>>>>>> are not satisfied. >>>>>>> >>>>>>> This change aligns the dependency tree with the expected hardware >>>>>>> support for >>>>>>> CPU hot-plugging under PowerPC architectures, ensuring that the kernel >>>>>>> configuration steps do not lead to inconsistent states. >>>>>>> >>>>>>> Signed-off-by: Vishal Chourasia <vish...@linux.ibm.com> >>>>>>> --- >>>>>>> During the configuration process with 'make randconfig' followed by >>>>>>> 'make olddefconfig', we observed a warning indicating an unmet direct >>>>>>> dependency for the HOTPLUG_CPU option. The dependency in question >>>>>>> relates to >>>>>>> various PowerPC configurations (PPC_PSERIES, PPC_PMAC, PPC_POWERNV, >>>>>>> FSL_SOC_BOOKE) which were not enabled, yet the HOTPLUG_CPU was >>>>>>> beingDuring the configuration process with 'make randconfig' followed by >>>>>>> 'make olddefconfig', we observed a warning indicating an unmet direct >>>>>>> dependency for the HOTPLUG_CPU option. The dependency in question >>>>>>> relates to >>>>>>> various PowerPC configurations (PPC_PSERIES, PPC_PMAC, PPC_POWERNV, >>>>>>> FSL_SOC_BOOKE) which were not enabled, yet the HOTPLUG_CPU was being >>>>>>> erroneously selected due to an implicit assumption by the PM_SLEEP_SMP >>>>>>> option. >>>>>>> This misalignment in dependencies could potentially lead to >>>>>>> inconsistent kernel >>>>>>> configuration states, especially when considering the necessary hardware >>>>>>> support for CPU hot-plugging on PowerPC platforms. The patch aims to >>>>>>> correct >>>>>>> this by ensuring that ARCH_HIBERNATION_POSSIBLE is contingent upon the >>>>>>> appropriate PowerPC configurations being active. >>>>>>> >>>>>>> steps to reproduce (before applying the patch): >>>>>>> >>>>>>> Run 'make pseries_le_defconfig' >>>>>>> Run 'make menuconfig' >>>>>>> Enable hibernation [ Kernel options -> Hibernation (aka 'suspend to >>>>>>> disk') ] >>>>>>> Disable [ Platform support -> IBM PowerNV (Non-Virtualized) platform >>>>>>> support ] >>>>>>> Disable [ Platform support -> IBM pSeries & new (POWER5-based) iSeries ] >>>>>>> Enable SMP [ Processor support -> Symmetric multi-processing support ] >>>>>>> Save the config >>>>>>> Run 'make olddefconfig' >>>>>>> erroneously selected due to an implicit assumption by the PM_SLEEP_SMP >>>>>>> option. >>>>>>> This misalignment in dependencies could potentially lead to >>>>>>> inconsistent kernel >>>>>>> configuration states, especially when considering the necessary hardware >>>>>>> support for CPU hot-plugging on PowerPC platforms. The patch aims to >>>>>>> correct >>>>>>> this by ensuring that ARCH_HIBERNATION_POSSIBLE is contingent upon the >>>>>>> appropriate PowerPC configurations being active. >>>>>>> >>>>>>> steps to reproduce (before applying the patch): >>>>>>> >>>>>>> Run 'make pseries_le_defconfig' >>>>>>> Run 'make menuconfig' >>>>>>> Enable hibernation [ Kernel options -> Hibernation (aka 'suspend to >>>>>>> disk') ] >>>>>>> Disable [ Platform support -> IBM PowerNV (Non-Virtualized) platform >>>>>>> support ] >>>>>>> Disable [ Platform support -> IBM pSeries & new (POWER5-based) iSeries ] >>>>>>> Enable SMP [ Processor support -> Symmetric multi-processing support ] >>>>>>> Save the config >>>>>>> Run 'make olddefconfig' >>>>>>> >>>>>>> arch/powerpc/Kconfig | 5 +++-- >>>>>>> 1 file changed, 3 insertions(+), 2 deletions(-) >>>>>>> >>>>>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >>>>>>> index 6f105ee4f3cf..bf99ff9869f6 100644 >>>>>>> --- a/arch/powerpc/Kconfig >>>>>>> +++ b/arch/powerpc/Kconfig >>>>>>> @@ -380,8 +380,9 @@ config DEFAULT_UIMAGE >>>>>>> Used to allow a board to specify it wants a uImage built by >>>>>>> default >>>>>>> >>>>>>> config ARCH_HIBERNATION_POSSIBLE >>>>>>> - bool >>>>>>> - default y >>>>>>> + def_bool y >>>>>>> + depends on PPC_PSERIES || \ >>>>>>> + PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE >>>>>>> >>>>>>> config ARCH_SUSPEND_POSSIBLE >>>>>>> def_bool y >>>>>>> >>>>>> I am wondering whether it should be switched to using select from >>>>>> config PPC? >>>>>> >>>>>> selecting ARCH_HIBERNATION_POSSIBLE based on value of config PPC >>>>>> will not guarantee config PPC_PSERIES being set >>>>>> >>>>>> PPC_PSERIES can be set to N, even when config PPC is set. >>> I understand what you meant before. Having ARCH_HIBERNATION_POSSIBLE under >>> config PPC makes more sense. >>>>>> grep -A 5 -i "config ppc_pseries" arch/powerpc/platforms/pseries/Kconfig >>>>>> config PPC_PSERIES >>>>>> depends on PPC64 && PPC_BOOK3S >>>>>> bool "IBM pSeries & new (POWER5-based) iSeries" >>>>>> select HAVE_PCSPKR_PLATFORM >>>>>> select MPIC >>>>>> select OF_DYNAMIC >>>>>> >>>> modified arch/powerpc/Kconfig >>>> @@ -156,6 +156,7 @@ config PPC >>>> select ARCH_HAS_UACCESS_FLUSHCACHE >>>> select ARCH_HAS_UBSAN_SANITIZE_ALL >>>> select ARCH_HAVE_NMI_SAFE_CMPXCHG >>>> + select ARCH_HIBERNATION_POSSIBLE if (PPC_PSERIES || PPC_PMAC || >>>> PPC_POWERNV || FSL_SOC_BOOKE) >>>> select ARCH_KEEP_MEMBLOCK >>>> select ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE if PPC_RADIX_MMU >>>> select ARCH_MIGHT_HAVE_PC_PARPORT >>> Though, even with these changes I was able to reproduce same warnings. >>> (using steps from above) >>> It's because one can enable HIBERNATION manually. >> But how? You shouldn't be able to enable it manually, it depends on >> ARCH_HIBERNATION_POSSIBLE which shouldn't be enabled. >> >> For the above to work you also need to make it default n, eg: >> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >> index 6f105ee4f3cf..dd2a9b938188 100644 >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -380,8 +380,7 @@ config DEFAULT_UIMAGE >> Used to allow a board to specify it wants a uImage built by default >> >> config ARCH_HIBERNATION_POSSIBLE >> - bool >> - default y >> + def_bool n >> >> config ARCH_SUSPEND_POSSIBLE >> def_bool y > > Ran make randconfig bunch of times. > > # make KCONFIG_SEED=0x97C94A3C randconfig > make[1]: Entering directory '' > GEN Makefile > KCONFIG_SEED=0x97C94A3C > > WARNING: unmet direct dependencies detected for HOTPLUG_CPU > Depends on [n]: SMP [=y] && (PPC_PSERIES [=n] || PPC_PMAC [=n] || > PPC_POWERNV [=n] || FSL_SOC_BOOKE [=n]) > Selected by [y]: > - PM_SLEEP_SMP [=y] && SMP [=y] && (ARCH_SUSPEND_POSSIBLE [=y] > || ARCH_HIBERNATION_POSSIBLE [=n]) && > PM_SLEEP [=y] > # > # configuration written to .config > # > make[1]: Leaving directory '' > > As per my understanding, > > "Depends on" clause of the config HOTPLUG_CPU as CPU hotplugging is only > available for SMP systems and is only available for following power platforms > (PPC_PSERIES || PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) > > Also, config HOTPLUG_CPU is being selected by config PM_SLEEP_SMP > config PM_SLEEP_SMP > def_bool y > depends on SMP > depends on ARCH_SUSPEND_POSSIBLE || ARCH_HIBERNATION_POSSIBLE > depends on PM_SLEEP > select HOTPLUG_CPU <----- Here > > Power management functionality depends and vary upon arch (and in powerpc > case, platforms) > supporting suspend/hibernation > > There are some platforms which support suspend and hence ARCH_SUSPEND_POSSIBLE > is set, leading to PM_SLEEP_SMP being set, and ultimately, config HOTPLUG_CPU > gets set. > But, CPU hotplug is not supported for such platform and hence the conflict.
Yeah. We need to restrict ARCH_SUSPEND_POSSIBLE in a similar way to ARCH_HIBERNATION_POSSIBLE. cheers
