Re: [PATCH] powerpc: Restrict ARCH_HIBERNATION_POSSIBLE to supported configurations

[Vishal Chourasia <vishalc@linux.ibm.com>]

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@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@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 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'
>>>>
>>>>  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.

As these warnings were observed through make randconfig, there is still a chance that randconfig
may result in a permutation that may produce these warnings again.

>
> -aneesh