* [PATCH] powerpc: Adjust config HOTPLUG_CPU dependency @ 2023-11-22 9:23 Vishal Chourasia 2023-11-22 9:33 ` Aneesh Kumar K V 0 siblings, 1 reply; 6+ messages in thread From: Vishal Chourasia @ 2023-11-22 9:23 UTC (permalink / raw) To: linuxppc-dev; +Cc: Aneesh Kumar K V, Vishal Chourasia, Srikar Dronamraju Changed HOTPLUG_CPU dependency to SMP and either ARCH_HIBERNATION_POSSIBLE or ARCH_SUSPEND_POSSIBLE, aligning with systems' suspend/hibernation capabilities. This update links CPU hotplugging more logically with platforms' capabilities. Other changes include 1. configs ARCH_HIBERNATION_POSSIBLE and ARCH_SUSPEND_POSSIBLE are now selected only if required platform dependencies are met. 2. Defaults for configs ARCH_HIBERNATION_POSSIBLE and ARCH_SUSPEND_POSSIBLE has been set to 'N' Reported-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> Suggested-by: Aneesh Kumar K V <aneesh.kumar@linux.ibm.com> Suggested-by: Michael Ellerman <mpe@ellerman.id.au> Signed-off-by: Vishal Chourasia <vishalc@linux.ibm.com> v1: https://lore.kernel.org/all/20231114082046.6018-1-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 | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 6f105ee4f3cf..6e7e9af2f0c1 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -155,6 +155,8 @@ config PPC select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST select ARCH_HAS_UACCESS_FLUSHCACHE select ARCH_HAS_UBSAN_SANITIZE_ALL + select ARCH_HIBERNATION_POSSIBLE if (PPC_PSERIES || PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) + select ARCH_SUSPEND_POSSIBLE if (ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES || 44x || 40x) select ARCH_HAVE_NMI_SAFE_CMPXCHG select ARCH_KEEP_MEMBLOCK select ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE if PPC_RADIX_MMU @@ -380,14 +382,10 @@ 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 - depends on ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || \ - (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES \ - || 44x || 40x + def_bool n config ARCH_SUSPEND_NONZERO_CPU def_bool y @@ -568,8 +566,7 @@ config ARCH_USING_PATCHABLE_FUNCTION_ENTRY config HOTPLUG_CPU bool "Support for enabling/disabling CPUs" - depends on SMP && (PPC_PSERIES || \ - PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) + depends on SMP && (ARCH_HIBERNATION_POSSIBLE || ARCH_SUSPEND_POSSIBLE) help Say Y here to be able to disable and re-enable individual CPUs at runtime on SMP machines. -- 2.41.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: Adjust config HOTPLUG_CPU dependency 2023-11-22 9:23 [PATCH] powerpc: Adjust config HOTPLUG_CPU dependency Vishal Chourasia @ 2023-11-22 9:33 ` Aneesh Kumar K V 2023-11-22 9:51 ` Vishal Chourasia 2023-11-22 10:10 ` [PATCH v3] " Vishal Chourasia 0 siblings, 2 replies; 6+ messages in thread From: Aneesh Kumar K V @ 2023-11-22 9:33 UTC (permalink / raw) To: Vishal Chourasia, linuxppc-dev; +Cc: Aneesh Kumar K V, Srikar Dronamraju On 11/22/23 2:53 PM, Vishal Chourasia wrote: > Changed HOTPLUG_CPU dependency to SMP and either ARCH_HIBERNATION_POSSIBLE or > ARCH_SUSPEND_POSSIBLE, aligning with systems' suspend/hibernation capabilities. > This update links CPU hotplugging more logically with platforms' capabilities. > > Other changes include > > 1. configs ARCH_HIBERNATION_POSSIBLE and ARCH_SUSPEND_POSSIBLE are now selected > only if required platform dependencies are met. > > 2. Defaults for configs ARCH_HIBERNATION_POSSIBLE and > ARCH_SUSPEND_POSSIBLE has been set to 'N' > > Reported-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> > Suggested-by: Aneesh Kumar K V <aneesh.kumar@linux.ibm.com> > Suggested-by: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Vishal Chourasia <vishalc@linux.ibm.com> > > v1: https://lore.kernel.org/all/20231114082046.6018-1-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 | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index 6f105ee4f3cf..6e7e9af2f0c1 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -155,6 +155,8 @@ config PPC > select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST > select ARCH_HAS_UACCESS_FLUSHCACHE > select ARCH_HAS_UBSAN_SANITIZE_ALL > + select ARCH_HIBERNATION_POSSIBLE if (PPC_PSERIES || PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) > + select ARCH_SUSPEND_POSSIBLE if (ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES || 44x || 40x) We should keep that sorted as explained in comment around that. > select ARCH_HAVE_NMI_SAFE_CMPXCHG > select ARCH_KEEP_MEMBLOCK > select ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE if PPC_RADIX_MMU > @@ -380,14 +382,10 @@ 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 We should be able to keep this config ARCH_HIBERNATION_POSSIBLE bool That is how we have rest of the ARCH_* config. I am not sure we need to explicitly say "def_bool n" > > config ARCH_SUSPEND_POSSIBLE > - def_bool y > - depends on ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || \ > - (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES \ > - || 44x || 40x > + def_bool n > > config ARCH_SUSPEND_NONZERO_CPU > def_bool y > @@ -568,8 +566,7 @@ config ARCH_USING_PATCHABLE_FUNCTION_ENTRY > > config HOTPLUG_CPU > bool "Support for enabling/disabling CPUs" > - depends on SMP && (PPC_PSERIES || \ > - PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) > + depends on SMP && (ARCH_HIBERNATION_POSSIBLE || ARCH_SUSPEND_POSSIBLE) > help > Say Y here to be able to disable and re-enable individual > CPUs at runtime on SMP machines. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: Adjust config HOTPLUG_CPU dependency 2023-11-22 9:33 ` Aneesh Kumar K V @ 2023-11-22 9:51 ` Vishal Chourasia 2023-11-22 10:10 ` [PATCH v3] " Vishal Chourasia 1 sibling, 0 replies; 6+ messages in thread From: Vishal Chourasia @ 2023-11-22 9:51 UTC (permalink / raw) To: Aneesh Kumar K V, linuxppc-dev; +Cc: Aneesh Kumar K V, Srikar Dronamraju On 22/11/23 3:03 pm, Aneesh Kumar K V wrote: > On 11/22/23 2:53 PM, Vishal Chourasia wrote: >> Changed HOTPLUG_CPU dependency to SMP and either ARCH_HIBERNATION_POSSIBLE or >> ARCH_SUSPEND_POSSIBLE, aligning with systems' suspend/hibernation capabilities. >> This update links CPU hotplugging more logically with platforms' capabilities. >> >> Other changes include >> >> 1. configs ARCH_HIBERNATION_POSSIBLE and ARCH_SUSPEND_POSSIBLE are now selected >> only if required platform dependencies are met. >> >> 2. Defaults for configs ARCH_HIBERNATION_POSSIBLE and >> ARCH_SUSPEND_POSSIBLE has been set to 'N' >> >> Reported-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> >> Suggested-by: Aneesh Kumar K V <aneesh.kumar@linux.ibm.com> >> Suggested-by: Michael Ellerman <mpe@ellerman.id.au> >> Signed-off-by: Vishal Chourasia <vishalc@linux.ibm.com> >> >> v1: https://lore.kernel.org/all/20231114082046.6018-1-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 | 13 +++++-------- >> 1 file changed, 5 insertions(+), 8 deletions(-) >> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >> index 6f105ee4f3cf..6e7e9af2f0c1 100644 >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -155,6 +155,8 @@ config PPC >> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST >> select ARCH_HAS_UACCESS_FLUSHCACHE >> select ARCH_HAS_UBSAN_SANITIZE_ALL >> + select ARCH_HIBERNATION_POSSIBLE if (PPC_PSERIES || PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) >> + select ARCH_SUSPEND_POSSIBLE if (ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES || 44x || 40x) > We should keep that sorted as explained in comment around that. Sure. I will send out another patch > >> select ARCH_HAVE_NMI_SAFE_CMPXCHG >> select ARCH_KEEP_MEMBLOCK >> select ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE if PPC_RADIX_MMU >> @@ -380,14 +382,10 @@ 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 > > We should be able to keep this > config ARCH_HIBERNATION_POSSIBLE > bool > > > That is how we have rest of the ARCH_* config. I am not sure we need to explicitly say "def_bool n" I believed it improves readability, that all. I can revert it back. > >> >> config ARCH_SUSPEND_POSSIBLE >> - def_bool y >> - depends on ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || \ >> - (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES \ >> - || 44x || 40x >> + def_bool n >> >> config ARCH_SUSPEND_NONZERO_CPU >> def_bool y >> @@ -568,8 +566,7 @@ config ARCH_USING_PATCHABLE_FUNCTION_ENTRY >> >> config HOTPLUG_CPU >> bool "Support for enabling/disabling CPUs" >> - depends on SMP && (PPC_PSERIES || \ >> - PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) >> + depends on SMP && (ARCH_HIBERNATION_POSSIBLE || ARCH_SUSPEND_POSSIBLE) >> help >> Say Y here to be able to disable and re-enable individual >> CPUs at runtime on SMP machines. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3] powerpc: Adjust config HOTPLUG_CPU dependency 2023-11-22 9:33 ` Aneesh Kumar K V 2023-11-22 9:51 ` Vishal Chourasia @ 2023-11-22 10:10 ` Vishal Chourasia 2023-11-24 2:39 ` Michael Ellerman 1 sibling, 1 reply; 6+ messages in thread From: Vishal Chourasia @ 2023-11-22 10:10 UTC (permalink / raw) To: aneesh.kumar; +Cc: aneesh.kumar, vishalc, linuxppc-dev, srikar Changed HOTPLUG_CPU dependency to SMP and either ARCH_HIBERNATION_POSSIBLE or ARCH_SUSPEND_POSSIBLE, aligning with systems' suspend/hibernation capabilities. This update link CPU hotplugging more logically with platforms' capabilities. configs ARCH_HIBERNATION_POSSIBLE and ARCH_SUSPEND_POSSIBLE are now selected only if required platform dependencies are met. Reported-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> Suggested-by: Aneesh Kumar K V <aneesh.kumar@linux.ibm.com> Suggested-by: Michael Ellerman <mpe@ellerman.id.au> Signed-off-by: Vishal Chourasia <vishalc@linux.ibm.com> v2: https://lore.kernel.org/all/20231122092303.223719-1-vishalc@linux.ibm.com v1: https://lore.kernel.org/all/20231114082046.6018-1-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 | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 6f105ee4f3cf..87c8134da3da 100644 --- a/arch/powerpc/Kconfig +++ b/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 @@ -166,6 +167,7 @@ config PPC select ARCH_STACKWALK select ARCH_SUPPORTS_ATOMIC_RMW select ARCH_SUPPORTS_DEBUG_PAGEALLOC if PPC_BOOK3S || PPC_8xx || 40x + select ARCH_SUSPEND_POSSIBLE if (ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES || 44x || 40x) select ARCH_USE_BUILTIN_BSWAP select ARCH_USE_CMPXCHG_LOCKREF if PPC64 select ARCH_USE_MEMTEST @@ -381,13 +383,9 @@ config DEFAULT_UIMAGE config ARCH_HIBERNATION_POSSIBLE bool - default y config ARCH_SUSPEND_POSSIBLE - def_bool y - depends on ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || \ - (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES \ - || 44x || 40x + bool config ARCH_SUSPEND_NONZERO_CPU def_bool y @@ -568,8 +566,7 @@ config ARCH_USING_PATCHABLE_FUNCTION_ENTRY config HOTPLUG_CPU bool "Support for enabling/disabling CPUs" - depends on SMP && (PPC_PSERIES || \ - PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) + depends on SMP && (ARCH_HIBERNATION_POSSIBLE || ARCH_SUSPEND_POSSIBLE) help Say Y here to be able to disable and re-enable individual CPUs at runtime on SMP machines. -- 2.41.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] powerpc: Adjust config HOTPLUG_CPU dependency 2023-11-22 10:10 ` [PATCH v3] " Vishal Chourasia @ 2023-11-24 2:39 ` Michael Ellerman 2023-11-24 21:42 ` Vishal Chourasia 0 siblings, 1 reply; 6+ messages in thread From: Michael Ellerman @ 2023-11-24 2:39 UTC (permalink / raw) To: Vishal Chourasia, aneesh.kumar Cc: aneesh.kumar, vishalc, linuxppc-dev, srikar Hi Vishal, I think our wires got crossed here somewhere :) Vishal Chourasia <vishalc@linux.ibm.com> writes: > Changed HOTPLUG_CPU dependency to SMP and either ARCH_HIBERNATION_POSSIBLE or > ARCH_SUSPEND_POSSIBLE, aligning with systems' suspend/hibernation capabilities. > This update link CPU hotplugging more logically with platforms' capabilities. > > configs ARCH_HIBERNATION_POSSIBLE and ARCH_SUSPEND_POSSIBLE are now selected > only if required platform dependencies are met. > > Reported-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> > Suggested-by: Aneesh Kumar K V <aneesh.kumar@linux.ibm.com> > Suggested-by: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Vishal Chourasia <vishalc@linux.ibm.com> > > v2: https://lore.kernel.org/all/20231122092303.223719-1-vishalc@linux.ibm.com > v1: https://lore.kernel.org/all/20231114082046.6018-1-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 | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index 6f105ee4f3cf..87c8134da3da 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -166,6 +167,7 @@ config PPC > select ARCH_STACKWALK > select ARCH_SUPPORTS_ATOMIC_RMW > select ARCH_SUPPORTS_DEBUG_PAGEALLOC if PPC_BOOK3S || PPC_8xx || 40x > + select ARCH_SUSPEND_POSSIBLE if (ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES || 44x || 40x) I know Aneesh suggested moving symbols to under PPC, but I think this is too big and complicated to be under PPC. > @@ -381,13 +383,9 @@ config DEFAULT_UIMAGE > > config ARCH_HIBERNATION_POSSIBLE > bool > - default y > config ARCH_SUSPEND_POSSIBLE > - def_bool y > - depends on ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || \ > - (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES \ > - || 44x || 40x > + bool > > config ARCH_SUSPEND_NONZERO_CPU > def_bool y > @@ -568,8 +566,7 @@ config ARCH_USING_PATCHABLE_FUNCTION_ENTRY > > config HOTPLUG_CPU > bool "Support for enabling/disabling CPUs" > - depends on SMP && (PPC_PSERIES || \ > - PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) > + depends on SMP && (ARCH_HIBERNATION_POSSIBLE || ARCH_SUSPEND_POSSIBLE) It's good to fix these warnings, but IMHO the result is that the dependencies are now backward. HOTPLUG_CPU should retain its original dependency list. It's easier to reason directly about "what platforms support CPU hotplug?", oh it's pseries/powernv/powermac/85xx, because they implement cpu_disable(). If there's some dependency from suspend/hibernate on CPU hotplug, then those symbols (suspend/hibernate) should depend on something to do with CPU hotplug. Can you try the patch below? Though, going back to your original reproduction case, that kernel is configured for Book3S 64, but with no platforms enabled, which is a non-sensical configuration (it can't boot on any actual machines). So possibly the real root cause is that, and we should find some way to block creating a config that has no platforms enabled. cheers diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 6f105ee4f3cf..9fe656a17017 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -380,11 +380,12 @@ 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 !SMP || HAVE_HOTPLUG_CPU config ARCH_SUSPEND_POSSIBLE def_bool y + depends on !SMP || HAVE_HOTPLUG_CPU depends on ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || \ (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES \ || 44x || 40x @@ -566,10 +567,14 @@ config ARCH_USING_PATCHABLE_FUNCTION_ENTRY def_bool $(success,$(srctree)/arch/powerpc/tools/gcc-check-fpatchable-function-entry.sh $(CC) -mlittle-endian) if PPC64 && CPU_LITTLE_ENDIAN def_bool $(success,$(srctree)/arch/powerpc/tools/gcc-check-fpatchable-function-entry.sh $(CC) -mbig-endian) if PPC64 && CPU_BIG_ENDIAN +config HAVE_HOTPLUG_CPU + def_bool y + depends on SMP + depends on PPC_PSERIES || PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE + config HOTPLUG_CPU bool "Support for enabling/disabling CPUs" - depends on SMP && (PPC_PSERIES || \ - PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) + depends on HAVE_HOTPLUG_CPU help Say Y here to be able to disable and re-enable individual CPUs at runtime on SMP machines. ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] powerpc: Adjust config HOTPLUG_CPU dependency 2023-11-24 2:39 ` Michael Ellerman @ 2023-11-24 21:42 ` Vishal Chourasia 0 siblings, 0 replies; 6+ messages in thread From: Vishal Chourasia @ 2023-11-24 21:42 UTC (permalink / raw) To: Michael Ellerman, aneesh.kumar; +Cc: aneesh.kumar, linuxppc-dev, srikar On 24/11/23 8:09 am, Michael Ellerman wrote: > Hi Vishal, > > I think our wires got crossed here somewhere :) > > Vishal Chourasia <vishalc@linux.ibm.com> writes: >> Changed HOTPLUG_CPU dependency to SMP and either ARCH_HIBERNATION_POSSIBLE or >> ARCH_SUSPEND_POSSIBLE, aligning with systems' suspend/hibernation capabilities. >> This update link CPU hotplugging more logically with platforms' capabilities. >> >> configs ARCH_HIBERNATION_POSSIBLE and ARCH_SUSPEND_POSSIBLE are now selected >> only if required platform dependencies are met. >> >> Reported-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> >> Suggested-by: Aneesh Kumar K V <aneesh.kumar@linux.ibm.com> >> Suggested-by: Michael Ellerman <mpe@ellerman.id.au> >> Signed-off-by: Vishal Chourasia <vishalc@linux.ibm.com> >> >> v2: https://lore.kernel.org/all/20231122092303.223719-1-vishalc@linux.ibm.com >> v1: https://lore.kernel.org/all/20231114082046.6018-1-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 | 11 ++++------- >> 1 file changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >> index 6f105ee4f3cf..87c8134da3da 100644 >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -166,6 +167,7 @@ config PPC >> select ARCH_STACKWALK >> select ARCH_SUPPORTS_ATOMIC_RMW >> select ARCH_SUPPORTS_DEBUG_PAGEALLOC if PPC_BOOK3S || PPC_8xx || 40x >> + select ARCH_SUSPEND_POSSIBLE if (ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES || 44x || 40x) > > I know Aneesh suggested moving symbols to under PPC, but I think this is > too big and complicated to be under PPC. > >> @@ -381,13 +383,9 @@ config DEFAULT_UIMAGE >> >> config ARCH_HIBERNATION_POSSIBLE >> bool >> - default y >> config ARCH_SUSPEND_POSSIBLE >> - def_bool y >> - depends on ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || \ >> - (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES \ >> - || 44x || 40x >> + bool >> >> config ARCH_SUSPEND_NONZERO_CPU >> def_bool y >> @@ -568,8 +566,7 @@ config ARCH_USING_PATCHABLE_FUNCTION_ENTRY >> >> config HOTPLUG_CPU >> bool "Support for enabling/disabling CPUs" >> - depends on SMP && (PPC_PSERIES || \ >> - PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) >> + depends on SMP && (ARCH_HIBERNATION_POSSIBLE || ARCH_SUSPEND_POSSIBLE) > > It's good to fix these warnings, but IMHO the result is that the > dependencies are now backward. > > HOTPLUG_CPU should retain its original dependency list. It's easier to > reason directly about "what platforms support CPU hotplug?", oh it's > pseries/powernv/powermac/85xx, because they implement cpu_disable(). > > If there's some dependency from suspend/hibernate on CPU hotplug, then > those symbols (suspend/hibernate) should depend on something to do with > CPU hotplug. > > Can you try the patch below? > > Though, going back to your original reproduction case, that kernel is > configured for Book3S 64, but with no platforms enabled, which is a > non-sensical configuration (it can't boot on any actual machines). So > possibly the real root cause is that, and we should find some way to > block creating a config that has no platforms enabled. > > cheers > > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index 6f105ee4f3cf..9fe656a17017 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -380,11 +380,12 @@ 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 !SMP || HAVE_HOTPLUG_CPU > > config ARCH_SUSPEND_POSSIBLE > def_bool y > + depends on !SMP || HAVE_HOTPLUG_CPU > depends on ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || \ > (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES \ > || 44x || 40x > @@ -566,10 +567,14 @@ config ARCH_USING_PATCHABLE_FUNCTION_ENTRY > def_bool $(success,$(srctree)/arch/powerpc/tools/gcc-check-fpatchable-function-entry.sh $(CC) -mlittle-endian) if PPC64 && CPU_LITTLE_ENDIAN > def_bool $(success,$(srctree)/arch/powerpc/tools/gcc-check-fpatchable-function-entry.sh $(CC) -mbig-endian) if PPC64 && CPU_BIG_ENDIAN > > +config HAVE_HOTPLUG_CPU > + def_bool y > + depends on SMP > + depends on PPC_PSERIES || PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE > + > config HOTPLUG_CPU > bool "Support for enabling/disabling CPUs" > - depends on SMP && (PPC_PSERIES || \ > - PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) > + depends on HAVE_HOTPLUG_CPU > help > Say Y here to be able to disable and re-enable individual > CPUs at runtime on SMP machines. > Tried and it works. No warnings to be seen. But, consider this scenario SMP [=y] CONFIG_PPC64 [=y] PPC_BOOK3S_32 [=y] CONFIG_CPU_BIG_ENDIAN [=y] PPC_MPC52xx [=y] PPC_EFIKA [=y] With above config setting in place, config hotplug_cpu will be set to 'N' based on your patch. And, If we consider current way of things, config hotplug_cpu will be set to 'Y' Below patch fixes the warning by making arch_hibernation_possible depend upon all of the necessary platforms and extends the dependency of hotplug_cpu on arch_suspend_possible diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 6f105ee4f3cf..13e32494bbf1 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -380,8 +380,8 @@ 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 @@ -566,10 +566,14 @@ config ARCH_USING_PATCHABLE_FUNCTION_ENTRY def_bool $(success,$(srctree)/arch/powerpc/tools/gcc-check-fpatchable-function-entry.sh $(CC) -mlittle-endian) if PPC64 && CPU_LITTLE_ENDIAN def_bool $(success,$(srctree)/arch/powerpc/tools/gcc-check-fpatchable-function-entry.sh $(CC) -mbig-endian) if PPC64 && CPU_BIG_ENDIAN +config HAVE_HOTPLUG_CPU + def_bool y + depends on SMP + depends on ARCH_HIBERNATION_POSSIBLE || ARCH_SUSPEND_POSSIBLE + config HOTPLUG_CPU bool "Support for enabling/disabling CPUs" - depends on SMP && (PPC_PSERIES || \ - PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) + depends on HAVE_HOTPLUG_CPU help Say Y here to be able to disable and re-enable individual CPUs at runtime on SMP machines. What is the "correct value" of config HOTPLUG_CPU (Y/N) given the config settings (from above)? I am finding it hard to justify N over Y. Can you explain a bit more? Your patch produces N, where as the patch i have provided in this mail would sets it to Y. does cpu hotplug code path calls into suspend code logic for any platforms upon which arch_suspend_possible config depends.. ex. PPC_EFIKA, etc. ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-11-24 21:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-11-22 9:23 [PATCH] powerpc: Adjust config HOTPLUG_CPU dependency Vishal Chourasia 2023-11-22 9:33 ` Aneesh Kumar K V 2023-11-22 9:51 ` Vishal Chourasia 2023-11-22 10:10 ` [PATCH v3] " Vishal Chourasia 2023-11-24 2:39 ` Michael Ellerman 2023-11-24 21:42 ` Vishal Chourasia
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).