* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.