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

[Aneesh Kumar K V <aneesh.kumar@linux.ibm.com>]

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


-aneesh