[PATCH] smp: Add bootcpus parameter to boot subset of CPUs

[PATCH] smp: Add bootcpus parameter to boot subset of CPUs

[Elliot Berman]

In a heterogeneous multiprocessor system, specifying the 'maxcpus'
parameter on kernel command line does not provide sufficient control
over which CPUs are brought online at kernel boot time, since CPUs may
have nonuniform performance characteristics. Thus, add bootcpus kernel
parameter to control which CPUs should be brought online during kernel
boot. When both maxcpus and bootcpus is set, the more restrictive of the
two are booted.

Signed-off-by: Elliot Berman <eberman@codeaurora.org>
---
 Documentation/admin-guide/kernel-parameters.txt |  8 +++++++
 include/linux/cpu.h                             |  2 +-
 kernel/cpu.c                                    |  4 ++--
 kernel/smp.c                                    | 28 +++++++++++++++++++++++--
 4 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 65d047f..ea31af3 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -449,6 +449,14 @@
 
 			See Documentation/admin-guide/bootconfig.rst
 
+	bootcpus=	[SMP]  List of processors that an SMP kernel
+			will bring up during bootup. Similar to maxcpus, except
+			as a cpu list as described above. The more restrictive
+			of maxcpus and bootcpus applies. If bootcpus=1-3 and
+			maxcpus=2, only processors 1 and 2 are booted. As with
+			maxcpus, you can bring up other plugged cpu by executing
+			"echo 1 > /sys/devices/system/cpu/cpuX/online"
+
 	bert_disable	[ACPI]
 			Disable BERT OS support on buggy BIOSes.
 
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 8aa84c0..4146f71 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -95,7 +95,7 @@ void notify_cpu_starting(unsigned int cpu);
 extern void cpu_maps_update_begin(void);
 extern void cpu_maps_update_done(void);
 int bringup_hibernate_cpu(unsigned int sleep_cpu);
-void bringup_nonboot_cpus(unsigned int setup_max_cpus);
+void bringup_nonboot_cpus(unsigned int setup_max_cpus, cpumask_var_t boot_cpus);
 
 #else	/* CONFIG_SMP */
 #define cpuhp_tasks_frozen	0
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 6ff2578..71f626b 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1328,14 +1328,14 @@ int bringup_hibernate_cpu(unsigned int sleep_cpu)
 	return 0;
 }
 
-void bringup_nonboot_cpus(unsigned int setup_max_cpus)
+void bringup_nonboot_cpus(unsigned int setup_max_cpus, cpumask_var_t boot_cpus)
 {
 	unsigned int cpu;
 
 	for_each_present_cpu(cpu) {
 		if (num_online_cpus() >= setup_max_cpus)
 			break;
-		if (!cpu_online(cpu))
+		if (!cpu_online(cpu) && cpumask_test_cpu(cpu, boot_cpus))
 			cpu_up(cpu, CPUHP_ONLINE);
 	}
 }
diff --git a/kernel/smp.c b/kernel/smp.c
index 4d17501..727e003 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -738,7 +738,7 @@ EXPORT_SYMBOL(smp_call_function);
 /* Setup configured maximum number of CPUs to activate */
 unsigned int setup_max_cpus = NR_CPUS;
 EXPORT_SYMBOL(setup_max_cpus);
-
+static cpumask_var_t boot_cpus;
 
 /*
  * Setup routine for controlling SMP activation
@@ -787,6 +787,27 @@ static int __init maxcpus(char *str)
 
 early_param("maxcpus", maxcpus);
 
+static int __init bootcpus(char *str)
+{
+	alloc_bootmem_cpumask_var(&boot_cpus);
+	if (cpulist_parse(str, boot_cpus) < 0) {
+		pr_warn("incorrect bootcpus mask\n");
+		return -EINVAL;
+	}
+	cpumask_set_cpu(smp_processor_id(), boot_cpus);
+	return 0;
+}
+
+early_param("bootcpus", bootcpus);
+
+static void __init boot_cpus_init(void)
+{
+	if (!cpumask_available(boot_cpus))
+		zalloc_cpumask_var(&boot_cpus, GFP_NOWAIT);
+	if (cpumask_empty(boot_cpus))
+		cpumask_setall(boot_cpus);
+}
+
 /* Setup number of possible processor ids */
 unsigned int nr_cpu_ids __read_mostly = NR_CPUS;
 EXPORT_SYMBOL(nr_cpu_ids);
@@ -804,10 +825,13 @@ void __init smp_init(void)
 
 	idle_threads_init();
 	cpuhp_threads_init();
+	boot_cpus_init();
 
 	pr_info("Bringing up secondary CPUs ...\n");
 
-	bringup_nonboot_cpus(setup_max_cpus);
+	bringup_nonboot_cpus(setup_max_cpus, boot_cpus);
+
+	free_bootmem_cpumask_var(boot_cpus);
 
 	num_nodes = num_online_nodes();
 	num_cpus  = num_online_cpus();
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] smp: Add bootcpus parameter to boot subset of CPUs

[Peter Zijlstra]

On Thu, Oct 22, 2020 at 03:04:03PM -0700, Elliot Berman wrote:
> In a heterogeneous multiprocessor system, specifying the 'maxcpus'
> parameter on kernel command line does not provide sufficient control
> over which CPUs are brought online at kernel boot time, since CPUs may
> have nonuniform performance characteristics. Thus, add bootcpus kernel
> parameter to control which CPUs should be brought online during kernel
> boot. When both maxcpus and bootcpus is set, the more restrictive of the
> two are booted.

Why would one wish to do so? Having the ability for abilities sake is no
good.

Re: [PATCH] smp: Add bootcpus parameter to boot subset of CPUs

[Thomas Gleixner]

On Thu, Oct 22 2020 at 15:04, Elliot Berman wrote:
> In a heterogeneous multiprocessor system, specifying the 'maxcpus'
> parameter on kernel command line does not provide sufficient control
> over which CPUs are brought online at kernel boot time, since CPUs may
> have nonuniform performance characteristics. Thus, add bootcpus kernel
> parameter to control which CPUs should be brought online during kernel
> boot. When both maxcpus and bootcpus is set, the more restrictive of the
> two are booted.

What for? 'maxcpus' is a debug hack at best and outright dangerous on
certain architectures. Why do we need more of that? Just let the machine
boot and offline the CPUs from user space.

Thanks,

        tglx

Re: [PATCH] smp: Add bootcpus parameter to boot subset of CPUs

[Qais Yousef]

Hi Elliot

On 10/22/20 15:04, Elliot Berman wrote:
> In a heterogeneous multiprocessor system, specifying the 'maxcpus'
> parameter on kernel command line does not provide sufficient control
> over which CPUs are brought online at kernel boot time, since CPUs may
> have nonuniform performance characteristics. Thus, add bootcpus kernel

Are you seeing boot time issues on big.LITTLE systems that's why you need this
extra config? If so it'll be good to elaborate on the problem. Scheduler should
place the heavy tasks on bigs. It'll be good to understand what's failing in
here as this might not be addressing the root cause.

Thanks

--
Qais Yousef

> parameter to control which CPUs should be brought online during kernel
> boot. When both maxcpus and bootcpus is set, the more restrictive of the
> two are booted.

Re: [PATCH] smp: Add bootcpus parameter to boot subset of CPUs

[psodagud]

On 2020-10-23 14:59, Thomas Gleixner wrote:
> On Thu, Oct 22 2020 at 15:04, Elliot Berman wrote:
>> In a heterogeneous multiprocessor system, specifying the 'maxcpus'
>> parameter on kernel command line does not provide sufficient control
>> over which CPUs are brought online at kernel boot time, since CPUs may
>> have nonuniform performance characteristics. Thus, add bootcpus kernel
>> parameter to control which CPUs should be brought online during kernel
>> boot. When both maxcpus and bootcpus is set, the more restrictive of 
>> the
>> two are booted.
> 
> What for? 'maxcpus' is a debug hack at best and outright dangerous on
> certain architectures. Why do we need more of that? Just let the 
> machine
> boot and offline the CPUs from user space.

Hi Thomas and Peter,

Based on my understanding with maxcpus option provides, maximum no of 
CPUs are brough up during the device boot up. There is a different case, 
in which we want to restrict which CPUs to be brough up.
On a system with 8 cpus, if we set maxcpus as 3, cpu0, cpu1, and cpu2 
are brough up during the bootup.  For example, if we want to bring 
core0, core3 and core4 current maxcpu(as 3) setting would not help us.
On some platform we want the flexibility on which CPUs to bring up 
during the device bootup. bootcpus command line is helping to bring 
specific CPUs and these patches are working downstream.

> 
> Thanks,
> 
>         tglx

Re: [PATCH] smp: Add bootcpus parameter to boot subset of CPUs

[Peter Zijlstra]

On Mon, Oct 26, 2020 at 10:08:47AM -0700, psodagud@codeaurora.org wrote:
> On 2020-10-23 14:59, Thomas Gleixner wrote:
> > On Thu, Oct 22 2020 at 15:04, Elliot Berman wrote:
> > > In a heterogeneous multiprocessor system, specifying the 'maxcpus'
> > > parameter on kernel command line does not provide sufficient control
> > > over which CPUs are brought online at kernel boot time, since CPUs may
> > > have nonuniform performance characteristics. Thus, add bootcpus kernel
> > > parameter to control which CPUs should be brought online during kernel
> > > boot. When both maxcpus and bootcpus is set, the more restrictive of
> > > the
> > > two are booted.
> > 
> > What for? 'maxcpus' is a debug hack at best and outright dangerous on
> > certain architectures. Why do we need more of that? Just let the machine
> > boot and offline the CPUs from user space.
> 
> Hi Thomas and Peter,
> 
> Based on my understanding with maxcpus option provides, maximum no of CPUs
> are brough up during the device boot up. There is a different case, in which
> we want to restrict which CPUs to be brough up.
> On a system with 8 cpus, if we set maxcpus as 3, cpu0, cpu1, and cpu2 are
> brough up during the bootup.  For example, if we want to bring core0, core3
> and core4 current maxcpu(as 3) setting would not help us.
> On some platform we want the flexibility on which CPUs to bring up during
> the device bootup. bootcpus command line is helping to bring specific CPUs
> and these patches are working downstream.

That's a lot of words, but exactly 0 on _WHY_ you would want to do that.

Re: [PATCH] smp: Add bootcpus parameter to boot subset of CPUs

[Thomas Gleixner]

On Mon, Oct 26 2020 at 10:08, psodagud wrote:
> Based on my understanding with maxcpus option provides, maximum no of 
> CPUs are brough up during the device boot up. There is a different case, 
> in which we want to restrict which CPUs to be brough up.

Again: What for? Why?

> On a system with 8 cpus, if we set maxcpus as 3, cpu0, cpu1, and cpu2 
> are brough up during the bootup.  For example, if we want to bring 
> core0, core3 and core4 current maxcpu(as 3) setting would not help us.
> On some platform we want the flexibility on which CPUs to bring up 
> during the device bootup. bootcpus command line is helping to bring 
> specific CPUs and these patches are working downstream.

A lot of patches work downstream by some definition of work. But that
does not make an argument to bring them upstream.

Thanks,

        tglx

Re: [PATCH] smp: Add bootcpus parameter to boot subset of CPUs

[Elliot Berman]

On 10/26/2020 10:12 AM, Peter Zijlstra wrote:
> On Mon, Oct 26, 2020 at 10:08:47AM -0700, psodagud@codeaurora.org wrote:
>> On 2020-10-23 14:59, Thomas Gleixner wrote:
>>> On Thu, Oct 22 2020 at 15:04, Elliot Berman wrote:
>>>> In a heterogeneous multiprocessor system, specifying the 'maxcpus'
>>>> parameter on kernel command line does not provide sufficient control
>>>> over which CPUs are brought online at kernel boot time, since CPUs may
>>>> have nonuniform performance characteristics. Thus, add bootcpus kernel
>>>> parameter to control which CPUs should be brought online during kernel
>>>> boot. When both maxcpus and bootcpus is set, the more restrictive of
>>>> the
>>>> two are booted.
>>>
>>> What for? 'maxcpus' is a debug hack at best and outright dangerous on
>>> certain architectures. Why do we need more of that? Just let the machine
>>> boot and offline the CPUs from user space.
>>
>> Hi Thomas and Peter,
>>
>> Based on my understanding with maxcpus option provides, maximum no of CPUs
>> are brough up during the device boot up. There is a different case, in which
>> we want to restrict which CPUs to be brough up.
>> On a system with 8 cpus, if we set maxcpus as 3, cpu0, cpu1, and cpu2 are
>> brough up during the bootup.  For example, if we want to bring core0, core3
>> and core4 current maxcpu(as 3) setting would not help us.
>> On some platform we want the flexibility on which CPUs to bring up during
>> the device bootup. bootcpus command line is helping to bring specific CPUs
>> and these patches are working downstream.
> 
> That's a lot of words, but exactly 0 on _WHY_ you would want to do that.
> 

We find the ability to limit the number of cpus brought online at bootup 
useful, and to possibly later enable those cores. One use case is when 
device is undergoing initial testing is to use bootcpus to limit bootup 
to only a couple cores and later bring up the other cores for a 
controlled stress test. A core brought up during boot is also running 
device initialization. Besides being useful for SoC vendor bringup which 
typically occurs downstream, this particular use case could be exercised 
by developer of upstream support for a SoC when initial CPU settings are 
being determined.

Another use case is if user wishes to limit bootup only to the smaller 
or bigger cores. maxcpus= is not sufficient here to ensure that only 
those cores are booted since it limits only to the first N cores, which 
may not be the desired small or big cores. User may want to bring up 
only the smaller cores during bootup for thermal reasons. For instance, 
device may be later sufficiently charged such that boot up of the bigger 
cores is now permissible. Relying on thermal drivers to later take care 
of putting core into lower power idle may not occur until much later in 
boot (for instance, if the governor is a module).

Re: [PATCH] smp: Add bootcpus parameter to boot subset of CPUs

[Qais Yousef]

Hi Elliot

+ Sudeep

On 10/27/20 10:06, Elliot Berman wrote:
> 
> On 10/26/2020 10:12 AM, Peter Zijlstra wrote:
> > On Mon, Oct 26, 2020 at 10:08:47AM -0700, psodagud@codeaurora.org wrote:
> > > On 2020-10-23 14:59, Thomas Gleixner wrote:
> > > > On Thu, Oct 22 2020 at 15:04, Elliot Berman wrote:
> > > > > In a heterogeneous multiprocessor system, specifying the 'maxcpus'
> > > > > parameter on kernel command line does not provide sufficient control
> > > > > over which CPUs are brought online at kernel boot time, since CPUs may
> > > > > have nonuniform performance characteristics. Thus, add bootcpus kernel
> > > > > parameter to control which CPUs should be brought online during kernel
> > > > > boot. When both maxcpus and bootcpus is set, the more restrictive of
> > > > > the
> > > > > two are booted.
> > > > 
> > > > What for? 'maxcpus' is a debug hack at best and outright dangerous on
> > > > certain architectures. Why do we need more of that? Just let the machine
> > > > boot and offline the CPUs from user space.
> > > 
> > > Hi Thomas and Peter,
> > > 
> > > Based on my understanding with maxcpus option provides, maximum no of CPUs
> > > are brough up during the device boot up. There is a different case, in which
> > > we want to restrict which CPUs to be brough up.
> > > On a system with 8 cpus, if we set maxcpus as 3, cpu0, cpu1, and cpu2 are
> > > brough up during the bootup.  For example, if we want to bring core0, core3
> > > and core4 current maxcpu(as 3) setting would not help us.
> > > On some platform we want the flexibility on which CPUs to bring up during
> > > the device bootup. bootcpus command line is helping to bring specific CPUs
> > > and these patches are working downstream.
> > 
> > That's a lot of words, but exactly 0 on _WHY_ you would want to do that.
> > 
> 
> We find the ability to limit the number of cpus brought online at bootup
> useful, and to possibly later enable those cores. One use case is when
> device is undergoing initial testing is to use bootcpus to limit bootup to
> only a couple cores and later bring up the other cores for a controlled
> stress test. A core brought up during boot is also running device
> initialization. Besides being useful for SoC vendor bringup which typically
> occurs downstream, this particular use case could be exercised by developer
> of upstream support for a SoC when initial CPU settings are being
> determined.
> 
> Another use case is if user wishes to limit bootup only to the smaller or
> bigger cores. maxcpus= is not sufficient here to ensure that only those
> cores are booted since it limits only to the first N cores, which may not be
> the desired small or big cores. User may want to bring up only the smaller
> cores during bootup for thermal reasons. For instance, device may be later
> sufficiently charged such that boot up of the bigger cores is now
> permissible. Relying on thermal drivers to later take care of putting core
> into lower power idle may not occur until much later in boot (for instance,
> if the governor is a module).

I would have thought that FW/SCP would have the power to block booting up the
CPUs if it deemed that to be unsafe.

Any thoughts Sudeep?

Thanks

--
Qais Yousef

Re: [PATCH] smp: Add bootcpus parameter to boot subset of CPUs

[Sudeep Holla]

On Wed, Oct 28, 2020 at 02:55:16PM +0000, Qais Yousef wrote:
> Hi Elliot
> 
> + Sudeep
> 
> On 10/27/20 10:06, Elliot Berman wrote:
> > 
> > On 10/26/2020 10:12 AM, Peter Zijlstra wrote:
> > > On Mon, Oct 26, 2020 at 10:08:47AM -0700, psodagud@codeaurora.org wrote:
> > > > On 2020-10-23 14:59, Thomas Gleixner wrote:
> > > > > On Thu, Oct 22 2020 at 15:04, Elliot Berman wrote:
> > > > > > In a heterogeneous multiprocessor system, specifying the 'maxcpus'
> > > > > > parameter on kernel command line does not provide sufficient control
> > > > > > over which CPUs are brought online at kernel boot time, since CPUs may
> > > > > > have nonuniform performance characteristics. Thus, add bootcpus kernel
> > > > > > parameter to control which CPUs should be brought online during kernel
> > > > > > boot. When both maxcpus and bootcpus is set, the more restrictive of
> > > > > > the
> > > > > > two are booted.
> > > > > 
> > > > > What for? 'maxcpus' is a debug hack at best and outright dangerous on
> > > > > certain architectures. Why do we need more of that? Just let the machine
> > > > > boot and offline the CPUs from user space.

Completely agreed, this is what I have suggested people in the past.

> > > >
> > > > Hi Thomas and Peter,
> > > >
> > > > Based on my understanding with maxcpus option provides, maximum no of CPUs
> > > > are brough up during the device boot up. There is a different case, in which
> > > > we want to restrict which CPUs to be brough up.
> > > > On a system with 8 cpus, if we set maxcpus as 3, cpu0, cpu1, and cpu2 are
> > > > brough up during the bootup.  For example, if we want to bring core0, core3
> > > > and core4 current maxcpu(as 3) setting would not help us.
> > > > On some platform we want the flexibility on which CPUs to bring up during
> > > > the device bootup. bootcpus command line is helping to bring specific CPUs
> > > > and these patches are working downstream.
> > >

Either offline "unwanted" CPUs from user space. If that is not possible
for whatever thermal reasons, we need to check if we can disable them in
the DT like ACPI does. IIUC, it is not supported for some reasons I need
to recall/check, can't remember that now. If that is not possible, make
those nodes disappear in the bootloader ?

> > > That's a lot of words, but exactly 0 on _WHY_ you would want to do that.
> > >
> >
> > We find the ability to limit the number of cpus brought online at bootup
> > useful, and to possibly later enable those cores. One use case is when
> > device is undergoing initial testing is to use bootcpus to limit bootup to
> > only a couple cores and later bring up the other cores for a controlled
> > stress test. A core brought up during boot is also running device
> > initialization. Besides being useful for SoC vendor bringup which typically
> > occurs downstream, this particular use case could be exercised by developer
> > of upstream support for a SoC when initial CPU settings are being
> > determined.
> >

Why not try single core instead of couple of core and add the needed ones
for the user-space ?

> > Another use case is if user wishes to limit bootup only to the smaller or
> > bigger cores. maxcpus= is not sufficient here to ensure that only those
> > cores are booted since it limits only to the first N cores, which may not be
> > the desired small or big cores. User may want to bring up only the smaller
> > cores during bootup for thermal reasons. For instance, device may be later
> > sufficiently charged such that boot up of the bigger cores is now
> > permissible. Relying on thermal drivers to later take care of putting core
> > into lower power idle may not occur until much later in boot (for instance,
> > if the governor is a module).
>
> I would have thought that FW/SCP would have the power to block booting up the
> CPUs if it deemed that to be unsafe.
>

I think it is more like *desire* to run with whatever battery life is left
rather than *unsafe* to bring up the core.

Also not sure if we can put such battery life related policies in the
firmware. If there is a thermal constraint, I am sure f/w will and must
refuse to boot the core. I doubt if we are talking about that here. It is
more a policy to extract max out of the battery life left, at-least the way
I see this issue. I may not have full context here, sorry.

-- 
Regards,
Sudeep

Re: [PATCH] smp: Add bootcpus parameter to boot subset of CPUs

[Sudeep Holla]

On Fri, Oct 23, 2020 at 7:24 AM Elliot Berman <eberman@codeaurora.org> wrote:
>
> In a heterogeneous multiprocessor system, specifying the 'maxcpus'
> parameter on kernel command line does not provide sufficient control
> over which CPUs are brought online at kernel boot time, since CPUs may
> have nonuniform performance characteristics. Thus, add bootcpus kernel
> parameter to control which CPUs should be brought online during kernel
> boot. When both maxcpus and bootcpus is set, the more restrictive of the
> two are booted.
>
> Signed-off-by: Elliot Berman <eberman@codeaurora.org>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  8 +++++++
>  include/linux/cpu.h                             |  2 +-
>  kernel/cpu.c                                    |  4 ++--
>  kernel/smp.c                                    | 28 +++++++++++++++++++++++--
>  4 files changed, 37 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 65d047f..ea31af3 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -449,6 +449,14 @@
>
>                         See Documentation/admin-guide/bootconfig.rst
>
> +       bootcpus=       [SMP]  List of processors that an SMP kernel
> +                       will bring up during bootup. Similar to maxcpus, except
> +                       as a cpu list as described above. The more restrictive
> +                       of maxcpus and bootcpus applies. If bootcpus=1-3 and
> +                       maxcpus=2, only processors 1 and 2 are booted. As with
> +                       maxcpus, you can bring up other plugged cpu by executing
> +                       "echo 1 > /sys/devices/system/cpu/cpuX/online"
> +

There is a fundamental assumption here that the user of this cmdline
understands how
the logical cpu numbers are allocated for the physical cpus. Based on
the discussion
I understand that you want to boot specific physical cpus for whatever
reasons and
here you want to specify the logical cpu numbers for them. So NACK for
that concept
alone irrespective of whether this concept as a whole is acceptable or not.

-- 
Regards,
Sudeep

Re: [PATCH] smp: Add bootcpus parameter to boot subset of CPUs

[Elliot Berman]

On 10/28/2020 8:15 AM, Sudeep Holla wrote:
>>>>> Hi Thomas and Peter,
>>>>>
>>>>> Based on my understanding with maxcpus option provides, maximum no of CPUs
>>>>> are brough up during the device boot up. There is a different case, in which
>>>>> we want to restrict which CPUs to be brough up.
>>>>> On a system with 8 cpus, if we set maxcpus as 3, cpu0, cpu1, and cpu2 are
>>>>> brough up during the bootup.  For example, if we want to bring core0, core3
>>>>> and core4 current maxcpu(as 3) setting would not help us.
>>>>> On some platform we want the flexibility on which CPUs to bring up during
>>>>> the device bootup. bootcpus command line is helping to bring specific CPUs
>>>>> and these patches are working downstream.
>>>>
> 
> Either offline "unwanted" CPUs from user space. If that is not possible
> for whatever thermal reasons, we need to check if we can disable them in
> the DT like ACPI does. IIUC, it is not supported for some reasons I need
> to recall/check, can't remember that now. If that is not possible, make
> those nodes disappear in the bootloader ?
> 

If I disappear the cpu nodes in bootloader, then I can't later online 
these cpus back up when policy permits. In our experience, there is a 
performance hit of ~100ms to modify any devicetree node in bootloader, 
which is significant on a commercial device wanting to disable bootup of 
certain cores for thermal.

>>>> That's a lot of words, but exactly 0 on _WHY_ you would want to do that.
>>>>
>>>
>>> We find the ability to limit the number of cpus brought online at bootup
>>> useful, and to possibly later enable those cores. One use case is when
>>> device is undergoing initial testing is to use bootcpus to limit bootup to
>>> only a couple cores and later bring up the other cores for a controlled
>>> stress test. A core brought up during boot is also running device
>>> initialization. Besides being useful for SoC vendor bringup which typically
>>> occurs downstream, this particular use case could be exercised by developer
>>> of upstream support for a SoC when initial CPU settings are being
>>> determined.
>>>
> 
> Why not try single core instead of couple of core and add the needed ones
> for the user-space ?

In some instances, we have seen that further debugging is needed from 
firmware or hardware teams. In these instances, we wanted device to 
still be able to do SMP boot, but with a few cores disabled.

In the case where commercial device is using feature for thermal, device 
should boot multiple small cores. Booting only one core means we would 
not be able to use all possible cores to maximum extent possible in this 
thermal case.

>>> Another use case is if user wishes to limit bootup only to the smaller or
>>> bigger cores. maxcpus= is not sufficient here to ensure that only those
>>> cores are booted since it limits only to the first N cores, which may not be
>>> the desired small or big cores. User may want to bring up only the smaller
>>> cores during bootup for thermal reasons. For instance, device may be later
>>> sufficiently charged such that boot up of the bigger cores is now
>>> permissible. Relying on thermal drivers to later take care of putting core
>>> into lower power idle may not occur until much later in boot (for instance,
>>> if the governor is a module).
>>
>> I would have thought that FW/SCP would have the power to block booting up the
>> CPUs if it deemed that to be unsafe.
>>
> 
> I think it is more like *desire* to run with whatever battery life is left
> rather than *unsafe* to bring up the core.
> 
> Also not sure if we can put such battery life related policies in the
> firmware. If there is a thermal constraint, I am sure f/w will and must
> refuse to boot the core. I doubt if we are talking about that here. It is
> more a policy to extract max out of the battery life left, at-least the way
> I see this issue. I may not have full context here, sorry.

This is correct, FW here does not actually prevent core from starting 
since it is not a significant enough thermal issue.

Re: [PATCH] smp: Add bootcpus parameter to boot subset of CPUs

[Sudeep Holla]

On Thu, Oct 29, 2020 at 02:37:06PM -0700, Elliot Berman wrote:
> On 10/28/2020 8:15 AM, Sudeep Holla wrote:
> > > > > > Hi Thomas and Peter,
> > > > > >
> > > > > > Based on my understanding with maxcpus option provides, maximum no of CPUs
> > > > > > are brough up during the device boot up. There is a different case, in which
> > > > > > we want to restrict which CPUs to be brough up.
> > > > > > On a system with 8 cpus, if we set maxcpus as 3, cpu0, cpu1, and cpu2 are
> > > > > > brough up during the bootup.  For example, if we want to bring core0, core3
> > > > > > and core4 current maxcpu(as 3) setting would not help us.
> > > > > > On some platform we want the flexibility on which CPUs to bring up during
> > > > > > the device bootup. bootcpus command line is helping to bring specific CPUs
> > > > > > and these patches are working downstream.
> > > > >
> >
> > Either offline "unwanted" CPUs from user space. If that is not possible
> > for whatever thermal reasons, we need to check if we can disable them in
> > the DT like ACPI does. IIUC, it is not supported for some reasons I need
> > to recall/check, can't remember that now. If that is not possible, make
> > those nodes disappear in the bootloader ?
> >
>
> If I disappear the cpu nodes in bootloader, then I can't later online these
> cpus back up when policy permits.

No I meant, to have policy in bootloader and manage device nodes based on
what you need in that boot.

> In our experience, there is a performance hit of ~100ms to modify any
> devicetree node in bootloader, which is significant on a commercial
> device wanting to disable bootup of certain cores for thermal.
>

I bet that performance hit is nowhere close to what you may have when you
offline a bunch of big cores, so I dismiss that, sorry.

> > > > > That's a lot of words, but exactly 0 on _WHY_ you would want to do that.
> > > > >
> > > >
> > > > We find the ability to limit the number of cpus brought online at bootup
> > > > useful, and to possibly later enable those cores. One use case is when
> > > > device is undergoing initial testing is to use bootcpus to limit bootup to
> > > > only a couple cores and later bring up the other cores for a controlled
> > > > stress test. A core brought up during boot is also running device
> > > > initialization. Besides being useful for SoC vendor bringup which typically
> > > > occurs downstream, this particular use case could be exercised by developer
> > > > of upstream support for a SoC when initial CPU settings are being
> > > > determined.
> > > >
> >
> > Why not try single core instead of couple of core and add the needed ones
> > for the user-space ?
>
> In some instances, we have seen that further debugging is needed from
> firmware or hardware teams. In these instances, we wanted device to still be
> able to do SMP boot, but with a few cores disabled.
>

Still manageable with different configuration in the bootloader.

> In the case where commercial device is using feature for thermal, device
> should boot multiple small cores. Booting only one core means we would not
> be able to use all possible cores to maximum extent possible in this thermal
> case.
>


I understood that point. But you haven't responded on my logical vs physical
number argument. I am clearly NACKing this patch as is for just usage of
logical CPU IDs in the command line while your intention is to control
the physical CPUs. So once again, NACK for that reason.

> > > > Another use case is if user wishes to limit bootup only to the smaller or
> > > > bigger cores. maxcpus= is not sufficient here to ensure that only those
> > > > cores are booted since it limits only to the first N cores, which may not be
> > > > the desired small or big cores. User may want to bring up only the smaller
> > > > cores during bootup for thermal reasons. For instance, device may be later
> > > > sufficiently charged such that boot up of the bigger cores is now
> > > > permissible. Relying on thermal drivers to later take care of putting core
> > > > into lower power idle may not occur until much later in boot (for instance,
> > > > if the governor is a module).
> > >
> > > I would have thought that FW/SCP would have the power to block booting up the
> > > CPUs if it deemed that to be unsafe.
> > >
> >
> > I think it is more like *desire* to run with whatever battery life is left
> > rather than *unsafe* to bring up the core.
> >
> > Also not sure if we can put such battery life related policies in the
> > firmware. If there is a thermal constraint, I am sure f/w will and must
> > refuse to boot the core. I doubt if we are talking about that here. It is
> > more a policy to extract max out of the battery life left, at-least the way
> > I see this issue. I may not have full context here, sorry.
>
> This is correct, FW here does not actually prevent core from starting since
> it is not a significant enough thermal issue.

Thanks for confirming.

--
Regards,
Sudeep

Re: [PATCH] smp: Add bootcpus parameter to boot subset of CPUs

[Thomas Gleixner]

On Fri, Oct 30 2020 at 17:45, Sudeep Holla wrote:
> On Thu, Oct 29, 2020 at 02:37:06PM -0700, Elliot Berman wrote:
>> In the case where commercial device is using feature for thermal, device
>> should boot multiple small cores. Booting only one core means we would not
>> be able to use all possible cores to maximum extent possible in this thermal
>> case.
>
> I understood that point. But you haven't responded on my logical vs physical
> number argument. I am clearly NACKing this patch as is for just usage of
> logical CPU IDs in the command line while your intention is to control
> the physical CPUs. So once again, NACK for that reason.

Correct. And no, we are not going to add a command line option to select
physical CPU ids.

There are two ways to solve that:

  1) The firmware can tell the kernel whether a CPU should be brought up
     or not, e.g. by failing the bootup request.

  2) The kernel has a way to figure out the current thermal and/or power
     budget early in the boot process and sorts out which and how many
     CPUs fit into that limit.

Thanks,

        tglx