Vishal Chourasia [off-list ref] writes:

On 15/11/23 5:46 pm, Aneesh Kumar K V wrote:

quoted

On 11/15/23 5:23 PM, Vishal Chourasia wrote:

quoted

On 15/11/23 1:39 pm, Aneesh Kumar K.V wrote:

quoted

Vishal Chourasia [off-list ref] writes:

quoted

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

quoted

quoted

quoted

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

cheers