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

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

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

>
> -aneesh