[PATCH] powerpc: Restrict ARCH_HIBERNATION_POSSIBLE to supported configurations
Michael Ellerman
mpe at ellerman.id.au
Wed Nov 22 16:05:34 AEDT 2023
Vishal Chourasia <vishalc at linux.ibm.com> writes:
> On 17/11/23 4:52 am, Michael Ellerman wrote:
>> Vishal Chourasia <vishalc at 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 <vishalc at 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 <vishalc at 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
More information about the Linuxppc-dev
mailing list