| Message ID | 20231114082046.6018-1-vishalc@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc: Restrict ARCH_HIBERNATION_POSSIBLE to supported configurations | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 23 jobs. |
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? -aneesh
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
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
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
Vishal Chourasia <vishalc@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@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. 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
On 17/11/23 4:52 am, Michael Ellerman wrote: > Vishal Chourasia <vishalc@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@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. > 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 Verified. Will send out another version. > > cheers
On 17/11/23 4:52 am, Michael Ellerman wrote: > Vishal Chourasia <vishalc@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@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 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. > > cheers
Vishal Chourasia <vishalc@linux.ibm.com> writes: > On 17/11/23 4:52 am, Michael Ellerman wrote: >> Vishal Chourasia <vishalc@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@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 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
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
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(-)