Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
On 15.03.22 09:40, Luca Fancellu wrote: Hmm, this will fail the ASSERT(spin_is_locked(&cpupool_lock)) in __cpupool_find_by_id(). I think you need to use cpupool_get_by_id() and cpupool_put() by making them globally visible (move their prototypes to sched.h). Hi Juergen, Yes I was thinking more to a __init wrapper that takes the lock and calls __cpupool_find_by_id, this to avoid exporting cpupool_put outside since we would be the only user. But I would like your opinion on that. I'd be fine with that. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
> Hmm, this will fail the ASSERT(spin_is_locked(&cpupool_lock)) in > __cpupool_find_by_id(). > > I think you need to use cpupool_get_by_id() and cpupool_put() by making them > globally visible (move their prototypes to sched.h). Hi Juergen, Yes I was thinking more to a __init wrapper that takes the lock and calls __cpupool_find_by_id, this to avoid exporting cpupool_put outside since we would be the only user. But I would like your opinion on that. Cheers, Luca > > > Juergen >
Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
On 12.03.22 01:10, Stefano Stabellini wrote:
On Fri, 11 Mar 2022, Luca Fancellu wrote:
On Thu, 10 Mar 2022, Luca Fancellu wrote:
Introduce a way to create different cpupools at boot time, this is
particularly useful on ARM big.LITTLE system where there might be the
need to have different cpupools for each type of core, but also
systems using NUMA can have different cpu pools for each node.
The feature on arm relies on a specification of the cpupools from the
device tree to build pools and assign cpus to them.
Documentation is created to explain the feature.
Signed-off-by: Luca Fancellu
---
Changes in v2:
- Move feature to common code (Juergen)
- Try to decouple dtb parse and cpupool creation to allow
more way to specify cpupools (for example command line)
- Created standalone dt node for the scheduler so it can
be used in future work to set scheduler specific
parameters
- Use only auto generated ids for cpupools
---
docs/misc/arm/device-tree/cpupools.txt | 156 ++
xen/common/Kconfig | 8 +
xen/common/Makefile| 1 +
xen/common/boot_cpupools.c | 212 +
xen/common/sched/cpupool.c | 6 +-
xen/include/xen/sched.h| 19 +++
6 files changed, 401 insertions(+), 1 deletion(-)
create mode 100644 docs/misc/arm/device-tree/cpupools.txt
create mode 100644 xen/common/boot_cpupools.c
diff --git a/docs/misc/arm/device-tree/cpupools.txt
b/docs/misc/arm/device-tree/cpupools.txt
new file mode 100644
index ..d5a82ed0d45a
--- /dev/null
+++ b/docs/misc/arm/device-tree/cpupools.txt
@@ -0,0 +1,156 @@
+Boot time cpupools
+==
+
+When BOOT_TIME_CPUPOOLS is enabled in the Xen configuration, it is possible to
+create cpupools during boot phase by specifying them in the device tree.
+
+Cpupools specification nodes shall be direct childs of /chosen node.
+Each cpupool node contains the following properties:
+
+- compatible (mandatory)
+
+Must always include the compatiblity string: "xen,cpupool".
+
+- cpupool-cpus (mandatory)
+
+Must be a list of device tree phandle to nodes describing cpus (e.g. having
+device_type = "cpu"), it can't be empty.
+
+- cpupool-sched (optional)
+
+Must be a device tree phandle to a node having "xen,scheduler" compatible
+(description below), it has no effect when the cpupool refers to the
cpupool
+number zero, in that case the default Xen scheduler is selected
(sched=<...>
+boot argument).
This is *a lot* better.
The device tree part is nice. I have only one question left on it: why
do we need a separate scheduler node? Could the "cpupool-sched" property
be a simple string with the scheduler name?
E.g.:
cpupool_a {
compatible = "xen,cpupool";
cpupool-cpus = <&a53_1 &a53_2>;
};
cpupool_b {
compatible = "xen,cpupool";
cpupool-cpus = <&a72_1 &a72_2>;
cpupool-sched = "null";
};
To me, it doesn't look like these new "scheduler specification nodes"
bring any benefits. I would just get rid of them.
From a comment of Juergen on the second patch I thought someone sees the need
to
have a way to set scheduling parameters:
“you are allowing to use another scheduler,
but what if someone wants to set non-standard scheduling parameters
(e.g. another time slice)?”
So I thought I could introduce a scheduler specification node that could in the
future be
extended and used to set scheduling parameter.
If it is something that is not needed, I will get rid of it.
I think you should get rid of it because it doesn't help. For instance,
if two cpupools want to use the same scheduler but with different
parameters we would end up with two different scheduler nodes.
Instead, in the future we could have one or more sched-params properties
as needed in the cpupools node to specify scheduler parameters.
+A scheduler specification node is a device tree node that contains the
following
+properties:
+
+- compatible (mandatory)
+
+Must always include the compatiblity string: "xen,scheduler".
+
+- sched-name (mandatory)
+
+Must be a string having the name of a Xen scheduler, check the sched=<...>
+boot argument for allowed values.
+
+
+Constraints
+===
+
+If no cpupools are specified, all cpus will be assigned to one cpupool
+implicitly created (Pool-0).
+
+If cpupools node are specified, but not every cpu brought up by Xen is
assigned,
+all the not assigned cpu will be assigned to an additional cpupool.
+
+If a cpu is assigned to a cpupool, but it's not brought up correctly, Xen will
+stop.
+
+
+Examples
+
+
+A system having two types of core, the following device tree specification will
+instruct Xen to have two cpupools:
+
+- The cpupool with id 0 will have 4 cpus assigned.
+- The cpupool with id 1 will have 2 cpus assigned.
+
+The following example can work only if hmp-unsafe=1 is passed to Xen boot
+arguments, otherwise not all cores will
Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
On Fri, 11 Mar 2022, Luca Fancellu wrote:
> > On Thu, 10 Mar 2022, Luca Fancellu wrote:
> >> Introduce a way to create different cpupools at boot time, this is
> >> particularly useful on ARM big.LITTLE system where there might be the
> >> need to have different cpupools for each type of core, but also
> >> systems using NUMA can have different cpu pools for each node.
> >>
> >> The feature on arm relies on a specification of the cpupools from the
> >> device tree to build pools and assign cpus to them.
> >>
> >> Documentation is created to explain the feature.
> >>
> >> Signed-off-by: Luca Fancellu
> >> ---
> >> Changes in v2:
> >> - Move feature to common code (Juergen)
> >> - Try to decouple dtb parse and cpupool creation to allow
> >> more way to specify cpupools (for example command line)
> >> - Created standalone dt node for the scheduler so it can
> >> be used in future work to set scheduler specific
> >> parameters
> >> - Use only auto generated ids for cpupools
> >> ---
> >> docs/misc/arm/device-tree/cpupools.txt | 156 ++
> >> xen/common/Kconfig | 8 +
> >> xen/common/Makefile| 1 +
> >> xen/common/boot_cpupools.c | 212 +
> >> xen/common/sched/cpupool.c | 6 +-
> >> xen/include/xen/sched.h| 19 +++
> >> 6 files changed, 401 insertions(+), 1 deletion(-)
> >> create mode 100644 docs/misc/arm/device-tree/cpupools.txt
> >> create mode 100644 xen/common/boot_cpupools.c
> >>
> >> diff --git a/docs/misc/arm/device-tree/cpupools.txt
> >> b/docs/misc/arm/device-tree/cpupools.txt
> >> new file mode 100644
> >> index ..d5a82ed0d45a
> >> --- /dev/null
> >> +++ b/docs/misc/arm/device-tree/cpupools.txt
> >> @@ -0,0 +1,156 @@
> >> +Boot time cpupools
> >> +==
> >> +
> >> +When BOOT_TIME_CPUPOOLS is enabled in the Xen configuration, it is
> >> possible to
> >> +create cpupools during boot phase by specifying them in the device tree.
> >> +
> >> +Cpupools specification nodes shall be direct childs of /chosen node.
> >> +Each cpupool node contains the following properties:
> >> +
> >> +- compatible (mandatory)
> >> +
> >> +Must always include the compatiblity string: "xen,cpupool".
> >> +
> >> +- cpupool-cpus (mandatory)
> >> +
> >> +Must be a list of device tree phandle to nodes describing cpus (e.g.
> >> having
> >> +device_type = "cpu"), it can't be empty.
> >> +
> >> +- cpupool-sched (optional)
> >> +
> >> +Must be a device tree phandle to a node having "xen,scheduler"
> >> compatible
> >> +(description below), it has no effect when the cpupool refers to the
> >> cpupool
> >> +number zero, in that case the default Xen scheduler is selected
> >> (sched=<...>
> >> +boot argument).
> >
> > This is *a lot* better.
> >
> > The device tree part is nice. I have only one question left on it: why
> > do we need a separate scheduler node? Could the "cpupool-sched" property
> > be a simple string with the scheduler name?
> >
> > E.g.:
> >
> >cpupool_a {
> >compatible = "xen,cpupool";
> >cpupool-cpus = <&a53_1 &a53_2>;
> >};
> >cpupool_b {
> >compatible = "xen,cpupool";
> >cpupool-cpus = <&a72_1 &a72_2>;
> >cpupool-sched = "null";
> >};
> >
> >
> > To me, it doesn't look like these new "scheduler specification nodes"
> > bring any benefits. I would just get rid of them.
>
> From a comment of Juergen on the second patch I thought someone sees the need
> to
> have a way to set scheduling parameters:
>
> “you are allowing to use another scheduler,
> but what if someone wants to set non-standard scheduling parameters
> (e.g. another time slice)?”
>
> So I thought I could introduce a scheduler specification node that could in
> the future be
> extended and used to set scheduling parameter.
>
> If it is something that is not needed, I will get rid of it.
I think you should get rid of it because it doesn't help. For instance,
if two cpupools want to use the same scheduler but with different
parameters we would end up with two different scheduler nodes.
Instead, in the future we could have one or more sched-params properties
as needed in the cpupools node to specify scheduler parameters.
> >> +A scheduler specification node is a device tree node that contains the
> >> following
> >> +properties:
> >> +
> >> +- compatible (mandatory)
> >> +
> >> +Must always include the compatiblity string: "xen,scheduler".
> >> +
> >> +- sched-name (mandatory)
> >> +
> >> +Must be a string having the name of a Xen scheduler, check the
> >> sched=<...>
> >> +boot argument for allowed values.
> >> +
> >> +
> >> +Constraints
> >> +===
> >> +
> >> +If no cpupools are specified, all cpus will be assigned to one cpupool
> >> +implicitly created (Pool-0).
> >> +
> >> +If cpupools node are specified, but not every cpu brought up by Xen is
> >> assigned,
> >> +all the not
Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
> On 11 Mar 2022, at 14:11, Luca Fancellu wrote:
>
> Hi Stefano,
>
>> On 11 Mar 2022, at 03:57, Stefano Stabellini wrote:
>>
>> On Thu, 10 Mar 2022, Luca Fancellu wrote:
>>> Introduce a way to create different cpupools at boot time, this is
>>> particularly useful on ARM big.LITTLE system where there might be the
>>> need to have different cpupools for each type of core, but also
>>> systems using NUMA can have different cpu pools for each node.
>>>
>>> The feature on arm relies on a specification of the cpupools from the
>>> device tree to build pools and assign cpus to them.
>>>
>>> Documentation is created to explain the feature.
>>>
>>> Signed-off-by: Luca Fancellu
>>> ---
>>> Changes in v2:
>>> - Move feature to common code (Juergen)
>>> - Try to decouple dtb parse and cpupool creation to allow
>>> more way to specify cpupools (for example command line)
>>> - Created standalone dt node for the scheduler so it can
>>> be used in future work to set scheduler specific
>>> parameters
>>> - Use only auto generated ids for cpupools
>>> ---
>>> docs/misc/arm/device-tree/cpupools.txt | 156 ++
>>> xen/common/Kconfig | 8 +
>>> xen/common/Makefile| 1 +
>>> xen/common/boot_cpupools.c | 212 +
>>> xen/common/sched/cpupool.c | 6 +-
>>> xen/include/xen/sched.h| 19 +++
>>> 6 files changed, 401 insertions(+), 1 deletion(-)
>>> create mode 100644 docs/misc/arm/device-tree/cpupools.txt
>>> create mode 100644 xen/common/boot_cpupools.c
>>>
>>> diff --git a/docs/misc/arm/device-tree/cpupools.txt
>>> b/docs/misc/arm/device-tree/cpupools.txt
>>> new file mode 100644
>>> index ..d5a82ed0d45a
>>> --- /dev/null
>>> +++ b/docs/misc/arm/device-tree/cpupools.txt
>>> @@ -0,0 +1,156 @@
>>> +Boot time cpupools
>>> +==
>>> +
>>> +When BOOT_TIME_CPUPOOLS is enabled in the Xen configuration, it is
>>> possible to
>>> +create cpupools during boot phase by specifying them in the device tree.
>>> +
>>> +Cpupools specification nodes shall be direct childs of /chosen node.
>>> +Each cpupool node contains the following properties:
>>> +
>>> +- compatible (mandatory)
>>> +
>>> +Must always include the compatiblity string: "xen,cpupool".
>>> +
>>> +- cpupool-cpus (mandatory)
>>> +
>>> +Must be a list of device tree phandle to nodes describing cpus (e.g.
>>> having
>>> +device_type = "cpu"), it can't be empty.
>>> +
>>> +- cpupool-sched (optional)
>>> +
>>> +Must be a device tree phandle to a node having "xen,scheduler"
>>> compatible
>>> +(description below), it has no effect when the cpupool refers to the
>>> cpupool
>>> +number zero, in that case the default Xen scheduler is selected
>>> (sched=<...>
>>> +boot argument).
>>
>> This is *a lot* better.
>>
>> The device tree part is nice. I have only one question left on it: why
>> do we need a separate scheduler node? Could the "cpupool-sched" property
>> be a simple string with the scheduler name?
>>
>> E.g.:
>>
>> cpupool_a {
>> compatible = "xen,cpupool";
>> cpupool-cpus = <&a53_1 &a53_2>;
>> };
>> cpupool_b {
>> compatible = "xen,cpupool";
>> cpupool-cpus = <&a72_1 &a72_2>;
>> cpupool-sched = "null";
>> };
>>
>>
>> To me, it doesn't look like these new "scheduler specification nodes"
>> bring any benefits. I would just get rid of them.
>
> From a comment of Juergen on the second patch I thought someone sees the need
> to
> have a way to set scheduling parameters:
>
> “you are allowing to use another scheduler,
> but what if someone wants to set non-standard scheduling parameters
> (e.g. another time slice)?”
>
> So I thought I could introduce a scheduler specification node that could in
> the future be
> extended and used to set scheduling parameter.
>
> If it is something that is not needed, I will get rid of it.
>
>>
>>
>>> +A scheduler specification node is a device tree node that contains the
>>> following
>>> +properties:
>>> +
>>> +- compatible (mandatory)
>>> +
>>> +Must always include the compatiblity string: "xen,scheduler".
>>> +
>>> +- sched-name (mandatory)
>>> +
>>> +Must be a string having the name of a Xen scheduler, check the
>>> sched=<...>
>>> +boot argument for allowed values.
>>> +
>>> +
>>> +Constraints
>>> +===
>>> +
>>> +If no cpupools are specified, all cpus will be assigned to one cpupool
>>> +implicitly created (Pool-0).
>>> +
>>> +If cpupools node are specified, but not every cpu brought up by Xen is
>>> assigned,
>>> +all the not assigned cpu will be assigned to an additional cpupool.
>>> +
>>> +If a cpu is assigned to a cpupool, but it's not brought up correctly, Xen
>>> will
>>> +stop.
>>> +
>>> +
>>> +Examples
>>> +
>>> +
>>> +A system having two types of core, the following device tree specification
>>> will
>>> +instruct Xen to have two cpupools:
>>> +
>>> +- The cpupool with
Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
Hi Stefano,
> On 11 Mar 2022, at 03:57, Stefano Stabellini wrote:
>
> On Thu, 10 Mar 2022, Luca Fancellu wrote:
>> Introduce a way to create different cpupools at boot time, this is
>> particularly useful on ARM big.LITTLE system where there might be the
>> need to have different cpupools for each type of core, but also
>> systems using NUMA can have different cpu pools for each node.
>>
>> The feature on arm relies on a specification of the cpupools from the
>> device tree to build pools and assign cpus to them.
>>
>> Documentation is created to explain the feature.
>>
>> Signed-off-by: Luca Fancellu
>> ---
>> Changes in v2:
>> - Move feature to common code (Juergen)
>> - Try to decouple dtb parse and cpupool creation to allow
>> more way to specify cpupools (for example command line)
>> - Created standalone dt node for the scheduler so it can
>> be used in future work to set scheduler specific
>> parameters
>> - Use only auto generated ids for cpupools
>> ---
>> docs/misc/arm/device-tree/cpupools.txt | 156 ++
>> xen/common/Kconfig | 8 +
>> xen/common/Makefile| 1 +
>> xen/common/boot_cpupools.c | 212 +
>> xen/common/sched/cpupool.c | 6 +-
>> xen/include/xen/sched.h| 19 +++
>> 6 files changed, 401 insertions(+), 1 deletion(-)
>> create mode 100644 docs/misc/arm/device-tree/cpupools.txt
>> create mode 100644 xen/common/boot_cpupools.c
>>
>> diff --git a/docs/misc/arm/device-tree/cpupools.txt
>> b/docs/misc/arm/device-tree/cpupools.txt
>> new file mode 100644
>> index ..d5a82ed0d45a
>> --- /dev/null
>> +++ b/docs/misc/arm/device-tree/cpupools.txt
>> @@ -0,0 +1,156 @@
>> +Boot time cpupools
>> +==
>> +
>> +When BOOT_TIME_CPUPOOLS is enabled in the Xen configuration, it is possible
>> to
>> +create cpupools during boot phase by specifying them in the device tree.
>> +
>> +Cpupools specification nodes shall be direct childs of /chosen node.
>> +Each cpupool node contains the following properties:
>> +
>> +- compatible (mandatory)
>> +
>> +Must always include the compatiblity string: "xen,cpupool".
>> +
>> +- cpupool-cpus (mandatory)
>> +
>> +Must be a list of device tree phandle to nodes describing cpus (e.g.
>> having
>> +device_type = "cpu"), it can't be empty.
>> +
>> +- cpupool-sched (optional)
>> +
>> +Must be a device tree phandle to a node having "xen,scheduler"
>> compatible
>> +(description below), it has no effect when the cpupool refers to the
>> cpupool
>> +number zero, in that case the default Xen scheduler is selected
>> (sched=<...>
>> +boot argument).
>
> This is *a lot* better.
>
> The device tree part is nice. I have only one question left on it: why
> do we need a separate scheduler node? Could the "cpupool-sched" property
> be a simple string with the scheduler name?
>
> E.g.:
>
>cpupool_a {
>compatible = "xen,cpupool";
>cpupool-cpus = <&a53_1 &a53_2>;
>};
>cpupool_b {
>compatible = "xen,cpupool";
>cpupool-cpus = <&a72_1 &a72_2>;
>cpupool-sched = "null";
>};
>
>
> To me, it doesn't look like these new "scheduler specification nodes"
> bring any benefits. I would just get rid of them.
From a comment of Juergen on the second patch I thought someone sees the need to
have a way to set scheduling parameters:
“you are allowing to use another scheduler,
but what if someone wants to set non-standard scheduling parameters
(e.g. another time slice)?”
So I thought I could introduce a scheduler specification node that could in the
future be
extended and used to set scheduling parameter.
If it is something that is not needed, I will get rid of it.
>
>
>> +A scheduler specification node is a device tree node that contains the
>> following
>> +properties:
>> +
>> +- compatible (mandatory)
>> +
>> +Must always include the compatiblity string: "xen,scheduler".
>> +
>> +- sched-name (mandatory)
>> +
>> +Must be a string having the name of a Xen scheduler, check the
>> sched=<...>
>> +boot argument for allowed values.
>> +
>> +
>> +Constraints
>> +===
>> +
>> +If no cpupools are specified, all cpus will be assigned to one cpupool
>> +implicitly created (Pool-0).
>> +
>> +If cpupools node are specified, but not every cpu brought up by Xen is
>> assigned,
>> +all the not assigned cpu will be assigned to an additional cpupool.
>> +
>> +If a cpu is assigned to a cpupool, but it's not brought up correctly, Xen
>> will
>> +stop.
>> +
>> +
>> +Examples
>> +
>> +
>> +A system having two types of core, the following device tree specification
>> will
>> +instruct Xen to have two cpupools:
>> +
>> +- The cpupool with id 0 will have 4 cpus assigned.
>> +- The cpupool with id 1 will have 2 cpus assigned.
>> +
>> +The following example can work only if hmp-unsafe=1 is passed to Xen boot
>> +arguments, otherwise not all cores w
Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
On 11.03.2022 12:29, Luca Fancellu wrote:
>> On 11 Mar 2022, at 10:18, Juergen Gross wrote:
>> On 11.03.22 10:46, Jan Beulich wrote:
>>> On 11.03.2022 10:29, Juergen Gross wrote:
On 11.03.22 09:56, Luca Fancellu wrote:
>> On 11 Mar 2022, at 08:09, Juergen Gross wrote:
>> On 10.03.22 18:10, Luca Fancellu wrote:
>>> --- /dev/null
>>> +++ b/xen/common/boot_cpupools.c
>>> @@ -0,0 +1,212 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * xen/common/boot_cpupools.c
>>> + *
>>> + * Code to create cpupools at boot time for arm architecture.
>>
>> Please drop the arm reference here.
>>
>>> + *
>>> + * Copyright (C) 2022 Arm Ltd.
>>> + */
>>> +
>>> +#include
>>> +
>>> +#define BTCPUPOOLS_DT_NODE_NO_REG (-1)
>>> +#define BTCPUPOOLS_DT_NODE_NO_LOG_CPU (-2)
>>
>> Move those inside the #ifdef below, please
>>
>>> +
>>> +struct pool_map {
>>> +int pool_id;
>>> +int sched_id;
>>> +struct cpupool *pool;
>>> +};
>>> +
>>> +static struct pool_map __initdata pool_cpu_map[NR_CPUS] =
>>> +{ [0 ... NR_CPUS-1] = {.pool_id = -1, .sched_id = -1, .pool =
>>> NULL} };
>>> +static unsigned int __initdata next_pool_id;
>>> +
>>> +#ifdef CONFIG_ARM
>>
>> Shouldn't this be CONFIG_HAS_DEVICE_TREE?
>
> Yes, the only problem is that in get_logical_cpu_from_hw_id I use the arm
> specific
> cpu_logical_map(…), so what do you think it’s the better way here?
> Do you think I should have everything under CONFIG_HAS_DEVICE_TREE
> and get_logical_cpu_from_hw_id under CONFIG_ARM like in this way below?
Hmm, what is the hwid used for on Arm? I guess this could be similar
to the x86 acpi-id?
>>> Since there's going to be only one of DT or ACPI, if anything this could
>>> be the APIC ID and then ...
So I'd rather put get_logical_cpu_from_hw_id() into Arm specific code
and add a related x86 function to x86 code. Depending on the answer to
above question this could either be get_cpu_id(), or maybe an identity
function.
>>> ... a lookup loop over x86_cpu_to_apicid[] (I thought we had a function
>>> doing so, but right now I can't find one).
>>
>> It is the second half of get_cpu_id().
>
> I was going to say, maybe I can do something like this:
>
> #ifdef CONFIG_ARM
> #define hwid_from_logical_cpu_id(x) cpu_logical_map(x)
> #elif defined(CONFIG_X86)
> #define hwid_from_logical_cpu_id(x) x86_cpu_to_apicid(x)
> #else
> #define hwid_from_logical_cpu_id(x) (-1)
> #end
>
> static int __init get_logical_cpu_from_hw_id(unsigned int hwid)
> {
> unsigned int i;
>
> for ( i = 0; i < nr_cpu_ids; i++ )
> if ( hwid_from_logical_cpu_id(i) == hwid )
> return i;
>
> return -1;
> }
>
> Do you think it is acceptable?
Why not, if even on Arm you have to use a loop. As Jürgen said, this
likely wants to move to some header file. Whether the names are
suitable for an arch abstraction I'm not sure, but I also have no
immediate alternative suggestion.
> I see the current get_cpu_id(…) from x86 code is starting from an acpi id to
> lookup the apicid and then it is looking for the logical cpu number.
> In the x86 context, eventually, the reg property of a cpu node would hold an
> Acpi id or an apicid? I would have say the last one but I’m not sure now.
Without ACPI it can't sensibly be an ACPI ID. The most logical thing
to expect would be an APIC ID, but then it's all up to whoever specifies
what DT is to supply.
Jan
Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
On 11.03.22 12:29, Luca Fancellu wrote:
On 11 Mar 2022, at 10:18, Juergen Gross wrote:
On 11.03.22 10:46, Jan Beulich wrote:
On 11.03.2022 10:29, Juergen Gross wrote:
On 11.03.22 09:56, Luca Fancellu wrote:
On 11 Mar 2022, at 08:09, Juergen Gross wrote:
On 10.03.22 18:10, Luca Fancellu wrote:
--- /dev/null
+++ b/xen/common/boot_cpupools.c
@@ -0,0 +1,212 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * xen/common/boot_cpupools.c
+ *
+ * Code to create cpupools at boot time for arm architecture.
Please drop the arm reference here.
+ *
+ * Copyright (C) 2022 Arm Ltd.
+ */
+
+#include
+
+#define BTCPUPOOLS_DT_NODE_NO_REG (-1)
+#define BTCPUPOOLS_DT_NODE_NO_LOG_CPU (-2)
Move those inside the #ifdef below, please
+
+struct pool_map {
+int pool_id;
+int sched_id;
+struct cpupool *pool;
+};
+
+static struct pool_map __initdata pool_cpu_map[NR_CPUS] =
+{ [0 ... NR_CPUS-1] = {.pool_id = -1, .sched_id = -1, .pool = NULL} };
+static unsigned int __initdata next_pool_id;
+
+#ifdef CONFIG_ARM
Shouldn't this be CONFIG_HAS_DEVICE_TREE?
Yes, the only problem is that in get_logical_cpu_from_hw_id I use the arm
specific
cpu_logical_map(…), so what do you think it’s the better way here?
Do you think I should have everything under CONFIG_HAS_DEVICE_TREE
and get_logical_cpu_from_hw_id under CONFIG_ARM like in this way below?
Hmm, what is the hwid used for on Arm? I guess this could be similar
to the x86 acpi-id?
Since there's going to be only one of DT or ACPI, if anything this could
be the APIC ID and then ...
So I'd rather put get_logical_cpu_from_hw_id() into Arm specific code
and add a related x86 function to x86 code. Depending on the answer to
above question this could either be get_cpu_id(), or maybe an identity
function.
... a lookup loop over x86_cpu_to_apicid[] (I thought we had a function
doing so, but right now I can't find one).
It is the second half of get_cpu_id().
I was going to say, maybe I can do something like this:
#ifdef CONFIG_ARM
#define hwid_from_logical_cpu_id(x) cpu_logical_map(x)
#elif defined(CONFIG_X86)
#define hwid_from_logical_cpu_id(x) x86_cpu_to_apicid(x)
#else
#define hwid_from_logical_cpu_id(x) (-1)
#end
static int __init get_logical_cpu_from_hw_id(unsigned int hwid)
{
unsigned int i;
for ( i = 0; i < nr_cpu_ids; i++ )
if ( hwid_from_logical_cpu_id(i) == hwid )
return i;
return -1;
}
Do you think it is acceptable?
I'd rather have this abstraction in some header, but this is
something the related maintainers should decide.
I see the current get_cpu_id(…) from x86 code is starting from an acpi id to
lookup the apicid and then it is looking for the logical cpu number.
In the x86 context, eventually, the reg property of a cpu node would hold an
Acpi id or an apicid? I would have say the last one but I’m not sure now.
According to Jan ACPI and device tree are mutually exclusive, so the
apicid is probably the correct answer.
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
> On 11 Mar 2022, at 10:18, Juergen Gross wrote:
>
> On 11.03.22 10:46, Jan Beulich wrote:
>> On 11.03.2022 10:29, Juergen Gross wrote:
>>> On 11.03.22 09:56, Luca Fancellu wrote:
> On 11 Mar 2022, at 08:09, Juergen Gross wrote:
> On 10.03.22 18:10, Luca Fancellu wrote:
>> --- /dev/null
>> +++ b/xen/common/boot_cpupools.c
>> @@ -0,0 +1,212 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * xen/common/boot_cpupools.c
>> + *
>> + * Code to create cpupools at boot time for arm architecture.
>
> Please drop the arm reference here.
>
>> + *
>> + * Copyright (C) 2022 Arm Ltd.
>> + */
>> +
>> +#include
>> +
>> +#define BTCPUPOOLS_DT_NODE_NO_REG (-1)
>> +#define BTCPUPOOLS_DT_NODE_NO_LOG_CPU (-2)
>
> Move those inside the #ifdef below, please
>
>> +
>> +struct pool_map {
>> +int pool_id;
>> +int sched_id;
>> +struct cpupool *pool;
>> +};
>> +
>> +static struct pool_map __initdata pool_cpu_map[NR_CPUS] =
>> +{ [0 ... NR_CPUS-1] = {.pool_id = -1, .sched_id = -1, .pool = NULL}
>> };
>> +static unsigned int __initdata next_pool_id;
>> +
>> +#ifdef CONFIG_ARM
>
> Shouldn't this be CONFIG_HAS_DEVICE_TREE?
Yes, the only problem is that in get_logical_cpu_from_hw_id I use the arm
specific
cpu_logical_map(…), so what do you think it’s the better way here?
Do you think I should have everything under CONFIG_HAS_DEVICE_TREE
and get_logical_cpu_from_hw_id under CONFIG_ARM like in this way below?
>>>
>>> Hmm, what is the hwid used for on Arm? I guess this could be similar
>>> to the x86 acpi-id?
>> Since there's going to be only one of DT or ACPI, if anything this could
>> be the APIC ID and then ...
>>> So I'd rather put get_logical_cpu_from_hw_id() into Arm specific code
>>> and add a related x86 function to x86 code. Depending on the answer to
>>> above question this could either be get_cpu_id(), or maybe an identity
>>> function.
>> ... a lookup loop over x86_cpu_to_apicid[] (I thought we had a function
>> doing so, but right now I can't find one).
>
> It is the second half of get_cpu_id().
I was going to say, maybe I can do something like this:
#ifdef CONFIG_ARM
#define hwid_from_logical_cpu_id(x) cpu_logical_map(x)
#elif defined(CONFIG_X86)
#define hwid_from_logical_cpu_id(x) x86_cpu_to_apicid(x)
#else
#define hwid_from_logical_cpu_id(x) (-1)
#end
static int __init get_logical_cpu_from_hw_id(unsigned int hwid)
{
unsigned int i;
for ( i = 0; i < nr_cpu_ids; i++ )
if ( hwid_from_logical_cpu_id(i) == hwid )
return i;
return -1;
}
Do you think it is acceptable?
I see the current get_cpu_id(…) from x86 code is starting from an acpi id to
lookup the apicid and then it is looking for the logical cpu number.
In the x86 context, eventually, the reg property of a cpu node would hold an
Acpi id or an apicid? I would have say the last one but I’m not sure now.
Cheers,
Luca
>
>
> Juergen
>
Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
On 11.03.22 10:46, Jan Beulich wrote:
On 11.03.2022 10:29, Juergen Gross wrote:
On 11.03.22 09:56, Luca Fancellu wrote:
On 11 Mar 2022, at 08:09, Juergen Gross wrote:
On 10.03.22 18:10, Luca Fancellu wrote:
--- /dev/null
+++ b/xen/common/boot_cpupools.c
@@ -0,0 +1,212 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * xen/common/boot_cpupools.c
+ *
+ * Code to create cpupools at boot time for arm architecture.
Please drop the arm reference here.
+ *
+ * Copyright (C) 2022 Arm Ltd.
+ */
+
+#include
+
+#define BTCPUPOOLS_DT_NODE_NO_REG (-1)
+#define BTCPUPOOLS_DT_NODE_NO_LOG_CPU (-2)
Move those inside the #ifdef below, please
+
+struct pool_map {
+int pool_id;
+int sched_id;
+struct cpupool *pool;
+};
+
+static struct pool_map __initdata pool_cpu_map[NR_CPUS] =
+{ [0 ... NR_CPUS-1] = {.pool_id = -1, .sched_id = -1, .pool = NULL} };
+static unsigned int __initdata next_pool_id;
+
+#ifdef CONFIG_ARM
Shouldn't this be CONFIG_HAS_DEVICE_TREE?
Yes, the only problem is that in get_logical_cpu_from_hw_id I use the arm
specific
cpu_logical_map(…), so what do you think it’s the better way here?
Do you think I should have everything under CONFIG_HAS_DEVICE_TREE
and get_logical_cpu_from_hw_id under CONFIG_ARM like in this way below?
Hmm, what is the hwid used for on Arm? I guess this could be similar
to the x86 acpi-id?
Since there's going to be only one of DT or ACPI, if anything this could
be the APIC ID and then ...
So I'd rather put get_logical_cpu_from_hw_id() into Arm specific code
and add a related x86 function to x86 code. Depending on the answer to
above question this could either be get_cpu_id(), or maybe an identity
function.
... a lookup loop over x86_cpu_to_apicid[] (I thought we had a function
doing so, but right now I can't find one).
It is the second half of get_cpu_id().
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
On 11.03.2022 10:29, Juergen Gross wrote:
> On 11.03.22 09:56, Luca Fancellu wrote:
>>> On 11 Mar 2022, at 08:09, Juergen Gross wrote:
>>> On 10.03.22 18:10, Luca Fancellu wrote:
--- /dev/null
+++ b/xen/common/boot_cpupools.c
@@ -0,0 +1,212 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * xen/common/boot_cpupools.c
+ *
+ * Code to create cpupools at boot time for arm architecture.
>>>
>>> Please drop the arm reference here.
>>>
+ *
+ * Copyright (C) 2022 Arm Ltd.
+ */
+
+#include
+
+#define BTCPUPOOLS_DT_NODE_NO_REG (-1)
+#define BTCPUPOOLS_DT_NODE_NO_LOG_CPU (-2)
>>>
>>> Move those inside the #ifdef below, please
>>>
+
+struct pool_map {
+int pool_id;
+int sched_id;
+struct cpupool *pool;
+};
+
+static struct pool_map __initdata pool_cpu_map[NR_CPUS] =
+{ [0 ... NR_CPUS-1] = {.pool_id = -1, .sched_id = -1, .pool = NULL} };
+static unsigned int __initdata next_pool_id;
+
+#ifdef CONFIG_ARM
>>>
>>> Shouldn't this be CONFIG_HAS_DEVICE_TREE?
>>
>> Yes, the only problem is that in get_logical_cpu_from_hw_id I use the arm
>> specific
>> cpu_logical_map(…), so what do you think it’s the better way here?
>> Do you think I should have everything under CONFIG_HAS_DEVICE_TREE
>> and get_logical_cpu_from_hw_id under CONFIG_ARM like in this way below?
>
> Hmm, what is the hwid used for on Arm? I guess this could be similar
> to the x86 acpi-id?
Since there's going to be only one of DT or ACPI, if anything this could
be the APIC ID and then ...
> So I'd rather put get_logical_cpu_from_hw_id() into Arm specific code
> and add a related x86 function to x86 code. Depending on the answer to
> above question this could either be get_cpu_id(), or maybe an identity
> function.
... a lookup loop over x86_cpu_to_apicid[] (I thought we had a function
doing so, but right now I can't find one).
Jan
Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
On 11.03.22 09:56, Luca Fancellu wrote:
Hi Juergen,
Thanks for your review
On 11 Mar 2022, at 08:09, Juergen Gross wrote:
On 10.03.22 18:10, Luca Fancellu wrote:
Introduce a way to create different cpupools at boot time, this is
particularly useful on ARM big.LITTLE system where there might be the
need to have different cpupools for each type of core, but also
systems using NUMA can have different cpu pools for each node.
The feature on arm relies on a specification of the cpupools from the
device tree to build pools and assign cpus to them.
Documentation is created to explain the feature.
Signed-off-by: Luca Fancellu
---
Changes in v2:
- Move feature to common code (Juergen)
- Try to decouple dtb parse and cpupool creation to allow
more way to specify cpupools (for example command line)
- Created standalone dt node for the scheduler so it can
be used in future work to set scheduler specific
parameters
- Use only auto generated ids for cpupools
---
docs/misc/arm/device-tree/cpupools.txt | 156 ++
xen/common/Kconfig | 8 +
xen/common/Makefile| 1 +
xen/common/boot_cpupools.c | 212 +
xen/common/sched/cpupool.c | 6 +-
xen/include/xen/sched.h| 19 +++
6 files changed, 401 insertions(+), 1 deletion(-)
create mode 100644 docs/misc/arm/device-tree/cpupools.txt
create mode 100644 xen/common/boot_cpupools.c
diff --git a/docs/misc/arm/device-tree/cpupools.txt
b/docs/misc/arm/device-tree/cpupools.txt
new file mode 100644
index ..d5a82ed0d45a
--- /dev/null
+++ b/docs/misc/arm/device-tree/cpupools.txt
@@ -0,0 +1,156 @@
+Boot time cpupools
+==
+
+When BOOT_TIME_CPUPOOLS is enabled in the Xen configuration, it is possible to
+create cpupools during boot phase by specifying them in the device tree.
+
+Cpupools specification nodes shall be direct childs of /chosen node.
+Each cpupool node contains the following properties:
+
+- compatible (mandatory)
+
+Must always include the compatiblity string: "xen,cpupool".
+
+- cpupool-cpus (mandatory)
+
+Must be a list of device tree phandle to nodes describing cpus (e.g. having
+device_type = "cpu"), it can't be empty.
+
+- cpupool-sched (optional)
+
+Must be a device tree phandle to a node having "xen,scheduler" compatible
+(description below), it has no effect when the cpupool refers to the
cpupool
+number zero, in that case the default Xen scheduler is selected
(sched=<...>
+boot argument).
+
+
+A scheduler specification node is a device tree node that contains the
following
+properties:
+
+- compatible (mandatory)
+
+Must always include the compatiblity string: "xen,scheduler".
+
+- sched-name (mandatory)
+
+Must be a string having the name of a Xen scheduler, check the sched=<...>
+boot argument for allowed values.
+
+
+Constraints
+===
+
+If no cpupools are specified, all cpus will be assigned to one cpupool
+implicitly created (Pool-0).
+
+If cpupools node are specified, but not every cpu brought up by Xen is
assigned,
+all the not assigned cpu will be assigned to an additional cpupool.
+
+If a cpu is assigned to a cpupool, but it's not brought up correctly, Xen will
+stop.
+
+
+Examples
+
+
+A system having two types of core, the following device tree specification will
+instruct Xen to have two cpupools:
+
+- The cpupool with id 0 will have 4 cpus assigned.
+- The cpupool with id 1 will have 2 cpus assigned.
+
+The following example can work only if hmp-unsafe=1 is passed to Xen boot
+arguments, otherwise not all cores will be brought up by Xen and the cpupool
+creation process will stop Xen.
+
+
+a72_1: cpu@0 {
+compatible = "arm,cortex-a72";
+reg = <0x0 0x0>;
+device_type = "cpu";
+[...]
+};
+
+a72_2: cpu@1 {
+compatible = "arm,cortex-a72";
+reg = <0x0 0x1>;
+device_type = "cpu";
+[...]
+};
+
+a53_1: cpu@100 {
+compatible = "arm,cortex-a53";
+reg = <0x0 0x100>;
+device_type = "cpu";
+[...]
+};
+
+a53_2: cpu@101 {
+compatible = "arm,cortex-a53";
+reg = <0x0 0x101>;
+device_type = "cpu";
+[...]
+};
+
+a53_3: cpu@102 {
+compatible = "arm,cortex-a53";
+reg = <0x0 0x102>;
+device_type = "cpu";
+[...]
+};
+
+a53_4: cpu@103 {
+compatible = "arm,cortex-a53";
+reg = <0x0 0x103>;
+device_type = "cpu";
+[...]
+};
+
+chosen {
+
+sched: sched_a {
+compatible = "xen,scheduler";
+sched-name = "credit2";
+};
+cpupool_a {
+compatible = "xen,cpupool";
+cpupool-cpus = <&a53_1 &a53_2 &a53_3 &a53_4>;
+};
+cpupool_b {
+compatible = "xen,cpupool";
+cpupool-cpus = <&a72_1 &a72_2>;
+cpupool-sched = <&sched>;
+};
+
+[...]
+
+};
+
+
+A system having the cpupools specification below w
Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
Hi Juergen,
Thanks for your review
> On 11 Mar 2022, at 08:09, Juergen Gross wrote:
>
> On 10.03.22 18:10, Luca Fancellu wrote:
>> Introduce a way to create different cpupools at boot time, this is
>> particularly useful on ARM big.LITTLE system where there might be the
>> need to have different cpupools for each type of core, but also
>> systems using NUMA can have different cpu pools for each node.
>> The feature on arm relies on a specification of the cpupools from the
>> device tree to build pools and assign cpus to them.
>> Documentation is created to explain the feature.
>> Signed-off-by: Luca Fancellu
>> ---
>> Changes in v2:
>> - Move feature to common code (Juergen)
>> - Try to decouple dtb parse and cpupool creation to allow
>> more way to specify cpupools (for example command line)
>> - Created standalone dt node for the scheduler so it can
>> be used in future work to set scheduler specific
>> parameters
>> - Use only auto generated ids for cpupools
>> ---
>> docs/misc/arm/device-tree/cpupools.txt | 156 ++
>> xen/common/Kconfig | 8 +
>> xen/common/Makefile| 1 +
>> xen/common/boot_cpupools.c | 212 +
>> xen/common/sched/cpupool.c | 6 +-
>> xen/include/xen/sched.h| 19 +++
>> 6 files changed, 401 insertions(+), 1 deletion(-)
>> create mode 100644 docs/misc/arm/device-tree/cpupools.txt
>> create mode 100644 xen/common/boot_cpupools.c
>> diff --git a/docs/misc/arm/device-tree/cpupools.txt
>> b/docs/misc/arm/device-tree/cpupools.txt
>> new file mode 100644
>> index ..d5a82ed0d45a
>> --- /dev/null
>> +++ b/docs/misc/arm/device-tree/cpupools.txt
>> @@ -0,0 +1,156 @@
>> +Boot time cpupools
>> +==
>> +
>> +When BOOT_TIME_CPUPOOLS is enabled in the Xen configuration, it is possible
>> to
>> +create cpupools during boot phase by specifying them in the device tree.
>> +
>> +Cpupools specification nodes shall be direct childs of /chosen node.
>> +Each cpupool node contains the following properties:
>> +
>> +- compatible (mandatory)
>> +
>> +Must always include the compatiblity string: "xen,cpupool".
>> +
>> +- cpupool-cpus (mandatory)
>> +
>> +Must be a list of device tree phandle to nodes describing cpus (e.g.
>> having
>> +device_type = "cpu"), it can't be empty.
>> +
>> +- cpupool-sched (optional)
>> +
>> +Must be a device tree phandle to a node having "xen,scheduler"
>> compatible
>> +(description below), it has no effect when the cpupool refers to the
>> cpupool
>> +number zero, in that case the default Xen scheduler is selected
>> (sched=<...>
>> +boot argument).
>> +
>> +
>> +A scheduler specification node is a device tree node that contains the
>> following
>> +properties:
>> +
>> +- compatible (mandatory)
>> +
>> +Must always include the compatiblity string: "xen,scheduler".
>> +
>> +- sched-name (mandatory)
>> +
>> +Must be a string having the name of a Xen scheduler, check the
>> sched=<...>
>> +boot argument for allowed values.
>> +
>> +
>> +Constraints
>> +===
>> +
>> +If no cpupools are specified, all cpus will be assigned to one cpupool
>> +implicitly created (Pool-0).
>> +
>> +If cpupools node are specified, but not every cpu brought up by Xen is
>> assigned,
>> +all the not assigned cpu will be assigned to an additional cpupool.
>> +
>> +If a cpu is assigned to a cpupool, but it's not brought up correctly, Xen
>> will
>> +stop.
>> +
>> +
>> +Examples
>> +
>> +
>> +A system having two types of core, the following device tree specification
>> will
>> +instruct Xen to have two cpupools:
>> +
>> +- The cpupool with id 0 will have 4 cpus assigned.
>> +- The cpupool with id 1 will have 2 cpus assigned.
>> +
>> +The following example can work only if hmp-unsafe=1 is passed to Xen boot
>> +arguments, otherwise not all cores will be brought up by Xen and the cpupool
>> +creation process will stop Xen.
>> +
>> +
>> +a72_1: cpu@0 {
>> +compatible = "arm,cortex-a72";
>> +reg = <0x0 0x0>;
>> +device_type = "cpu";
>> +[...]
>> +};
>> +
>> +a72_2: cpu@1 {
>> +compatible = "arm,cortex-a72";
>> +reg = <0x0 0x1>;
>> +device_type = "cpu";
>> +[...]
>> +};
>> +
>> +a53_1: cpu@100 {
>> +compatible = "arm,cortex-a53";
>> +reg = <0x0 0x100>;
>> +device_type = "cpu";
>> +[...]
>> +};
>> +
>> +a53_2: cpu@101 {
>> +compatible = "arm,cortex-a53";
>> +reg = <0x0 0x101>;
>> +device_type = "cpu";
>> +[...]
>> +};
>> +
>> +a53_3: cpu@102 {
>> +compatible = "arm,cortex-a53";
>> +reg = <0x0 0x102>;
>> +device_type = "cpu";
>> +[...]
>> +};
>> +
>> +a53_4: cpu@103 {
>> +compatible = "arm,cortex-a53";
>> +reg = <0x0 0x103>;
>> +device_type = "cpu";
>> +[...]
>> +};
>> +
>> +chosen {
>> +
>> +sched
Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
On 10.03.22 18:10, Luca Fancellu wrote:
Introduce a way to create different cpupools at boot time, this is
particularly useful on ARM big.LITTLE system where there might be the
need to have different cpupools for each type of core, but also
systems using NUMA can have different cpu pools for each node.
The feature on arm relies on a specification of the cpupools from the
device tree to build pools and assign cpus to them.
Documentation is created to explain the feature.
Signed-off-by: Luca Fancellu
---
Changes in v2:
- Move feature to common code (Juergen)
- Try to decouple dtb parse and cpupool creation to allow
more way to specify cpupools (for example command line)
- Created standalone dt node for the scheduler so it can
be used in future work to set scheduler specific
parameters
- Use only auto generated ids for cpupools
---
docs/misc/arm/device-tree/cpupools.txt | 156 ++
xen/common/Kconfig | 8 +
xen/common/Makefile| 1 +
xen/common/boot_cpupools.c | 212 +
xen/common/sched/cpupool.c | 6 +-
xen/include/xen/sched.h| 19 +++
6 files changed, 401 insertions(+), 1 deletion(-)
create mode 100644 docs/misc/arm/device-tree/cpupools.txt
create mode 100644 xen/common/boot_cpupools.c
diff --git a/docs/misc/arm/device-tree/cpupools.txt
b/docs/misc/arm/device-tree/cpupools.txt
new file mode 100644
index ..d5a82ed0d45a
--- /dev/null
+++ b/docs/misc/arm/device-tree/cpupools.txt
@@ -0,0 +1,156 @@
+Boot time cpupools
+==
+
+When BOOT_TIME_CPUPOOLS is enabled in the Xen configuration, it is possible to
+create cpupools during boot phase by specifying them in the device tree.
+
+Cpupools specification nodes shall be direct childs of /chosen node.
+Each cpupool node contains the following properties:
+
+- compatible (mandatory)
+
+Must always include the compatiblity string: "xen,cpupool".
+
+- cpupool-cpus (mandatory)
+
+Must be a list of device tree phandle to nodes describing cpus (e.g. having
+device_type = "cpu"), it can't be empty.
+
+- cpupool-sched (optional)
+
+Must be a device tree phandle to a node having "xen,scheduler" compatible
+(description below), it has no effect when the cpupool refers to the
cpupool
+number zero, in that case the default Xen scheduler is selected
(sched=<...>
+boot argument).
+
+
+A scheduler specification node is a device tree node that contains the
following
+properties:
+
+- compatible (mandatory)
+
+Must always include the compatiblity string: "xen,scheduler".
+
+- sched-name (mandatory)
+
+Must be a string having the name of a Xen scheduler, check the sched=<...>
+boot argument for allowed values.
+
+
+Constraints
+===
+
+If no cpupools are specified, all cpus will be assigned to one cpupool
+implicitly created (Pool-0).
+
+If cpupools node are specified, but not every cpu brought up by Xen is
assigned,
+all the not assigned cpu will be assigned to an additional cpupool.
+
+If a cpu is assigned to a cpupool, but it's not brought up correctly, Xen will
+stop.
+
+
+Examples
+
+
+A system having two types of core, the following device tree specification will
+instruct Xen to have two cpupools:
+
+- The cpupool with id 0 will have 4 cpus assigned.
+- The cpupool with id 1 will have 2 cpus assigned.
+
+The following example can work only if hmp-unsafe=1 is passed to Xen boot
+arguments, otherwise not all cores will be brought up by Xen and the cpupool
+creation process will stop Xen.
+
+
+a72_1: cpu@0 {
+compatible = "arm,cortex-a72";
+reg = <0x0 0x0>;
+device_type = "cpu";
+[...]
+};
+
+a72_2: cpu@1 {
+compatible = "arm,cortex-a72";
+reg = <0x0 0x1>;
+device_type = "cpu";
+[...]
+};
+
+a53_1: cpu@100 {
+compatible = "arm,cortex-a53";
+reg = <0x0 0x100>;
+device_type = "cpu";
+[...]
+};
+
+a53_2: cpu@101 {
+compatible = "arm,cortex-a53";
+reg = <0x0 0x101>;
+device_type = "cpu";
+[...]
+};
+
+a53_3: cpu@102 {
+compatible = "arm,cortex-a53";
+reg = <0x0 0x102>;
+device_type = "cpu";
+[...]
+};
+
+a53_4: cpu@103 {
+compatible = "arm,cortex-a53";
+reg = <0x0 0x103>;
+device_type = "cpu";
+[...]
+};
+
+chosen {
+
+sched: sched_a {
+compatible = "xen,scheduler";
+sched-name = "credit2";
+};
+cpupool_a {
+compatible = "xen,cpupool";
+cpupool-cpus = <&a53_1 &a53_2 &a53_3 &a53_4>;
+};
+cpupool_b {
+compatible = "xen,cpupool";
+cpupool-cpus = <&a72_1 &a72_2>;
+cpupool-sched = <&sched>;
+};
+
+[...]
+
+};
+
+
+A system having the cpupools specification below will instruct Xen to have
three
+cpupools:
+
+- The cpupool Pool-0 will have 2 cpus assigned.
+- The cpupool Pool-1 will hav
Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
On 10.03.2022 18:10, Luca Fancellu wrote:
> +chosen {
> +
> +sched: sched_a {
> +compatible = "xen,scheduler";
> +sched-name = "null";
> +};
> +cpupool_a {
> +compatible = "xen,cpupool";
> +cpupool-cpus = <&a53_1 &a53_2>;
> +};
> +cpupool_b {
> +compatible = "xen,cpupool";
> +cpupool-cpus = <&a72_1 &a72_2>;
> +cpupool-sched = <&sched>;
> +};
> +
> +[...]
> +
> +};
> \ No newline at end of file
Only seeing this in context of where I wanted to actually comment on.
Please fix.
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -22,6 +22,14 @@ config GRANT_TABLE
>
> If unsure, say Y.
>
> +config BOOT_TIME_CPUPOOLS
> + bool "Create cpupools at boot time"
> + depends on HAS_DEVICE_TREE
> + default n
Nit: Please omit this line - the default is N anyway unless specified
otherwise explicitly.
Jan
Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
On Thu, 10 Mar 2022, Luca Fancellu wrote:
> Introduce a way to create different cpupools at boot time, this is
> particularly useful on ARM big.LITTLE system where there might be the
> need to have different cpupools for each type of core, but also
> systems using NUMA can have different cpu pools for each node.
>
> The feature on arm relies on a specification of the cpupools from the
> device tree to build pools and assign cpus to them.
>
> Documentation is created to explain the feature.
>
> Signed-off-by: Luca Fancellu
> ---
> Changes in v2:
> - Move feature to common code (Juergen)
> - Try to decouple dtb parse and cpupool creation to allow
> more way to specify cpupools (for example command line)
> - Created standalone dt node for the scheduler so it can
> be used in future work to set scheduler specific
> parameters
> - Use only auto generated ids for cpupools
> ---
> docs/misc/arm/device-tree/cpupools.txt | 156 ++
> xen/common/Kconfig | 8 +
> xen/common/Makefile| 1 +
> xen/common/boot_cpupools.c | 212 +
> xen/common/sched/cpupool.c | 6 +-
> xen/include/xen/sched.h| 19 +++
> 6 files changed, 401 insertions(+), 1 deletion(-)
> create mode 100644 docs/misc/arm/device-tree/cpupools.txt
> create mode 100644 xen/common/boot_cpupools.c
>
> diff --git a/docs/misc/arm/device-tree/cpupools.txt
> b/docs/misc/arm/device-tree/cpupools.txt
> new file mode 100644
> index ..d5a82ed0d45a
> --- /dev/null
> +++ b/docs/misc/arm/device-tree/cpupools.txt
> @@ -0,0 +1,156 @@
> +Boot time cpupools
> +==
> +
> +When BOOT_TIME_CPUPOOLS is enabled in the Xen configuration, it is possible
> to
> +create cpupools during boot phase by specifying them in the device tree.
> +
> +Cpupools specification nodes shall be direct childs of /chosen node.
> +Each cpupool node contains the following properties:
> +
> +- compatible (mandatory)
> +
> +Must always include the compatiblity string: "xen,cpupool".
> +
> +- cpupool-cpus (mandatory)
> +
> +Must be a list of device tree phandle to nodes describing cpus (e.g.
> having
> +device_type = "cpu"), it can't be empty.
> +
> +- cpupool-sched (optional)
> +
> +Must be a device tree phandle to a node having "xen,scheduler" compatible
> +(description below), it has no effect when the cpupool refers to the
> cpupool
> +number zero, in that case the default Xen scheduler is selected
> (sched=<...>
> +boot argument).
This is *a lot* better.
The device tree part is nice. I have only one question left on it: why
do we need a separate scheduler node? Could the "cpupool-sched" property
be a simple string with the scheduler name?
E.g.:
cpupool_a {
compatible = "xen,cpupool";
cpupool-cpus = <&a53_1 &a53_2>;
};
cpupool_b {
compatible = "xen,cpupool";
cpupool-cpus = <&a72_1 &a72_2>;
cpupool-sched = "null";
};
To me, it doesn't look like these new "scheduler specification nodes"
bring any benefits. I would just get rid of them.
> +A scheduler specification node is a device tree node that contains the
> following
> +properties:
> +
> +- compatible (mandatory)
> +
> +Must always include the compatiblity string: "xen,scheduler".
> +
> +- sched-name (mandatory)
> +
> +Must be a string having the name of a Xen scheduler, check the
> sched=<...>
> +boot argument for allowed values.
> +
> +
> +Constraints
> +===
> +
> +If no cpupools are specified, all cpus will be assigned to one cpupool
> +implicitly created (Pool-0).
> +
> +If cpupools node are specified, but not every cpu brought up by Xen is
> assigned,
> +all the not assigned cpu will be assigned to an additional cpupool.
> +
> +If a cpu is assigned to a cpupool, but it's not brought up correctly, Xen
> will
> +stop.
> +
> +
> +Examples
> +
> +
> +A system having two types of core, the following device tree specification
> will
> +instruct Xen to have two cpupools:
> +
> +- The cpupool with id 0 will have 4 cpus assigned.
> +- The cpupool with id 1 will have 2 cpus assigned.
> +
> +The following example can work only if hmp-unsafe=1 is passed to Xen boot
> +arguments, otherwise not all cores will be brought up by Xen and the cpupool
> +creation process will stop Xen.
> +
> +
> +a72_1: cpu@0 {
> +compatible = "arm,cortex-a72";
> +reg = <0x0 0x0>;
> +device_type = "cpu";
> +[...]
> +};
> +
> +a72_2: cpu@1 {
> +compatible = "arm,cortex-a72";
> +reg = <0x0 0x1>;
> +device_type = "cpu";
> +[...]
> +};
> +
> +a53_1: cpu@100 {
> +compatible = "arm,cortex-a53";
> +reg = <0x0 0x100>;
> +device_type = "cpu";
> +[...]
> +};
> +
> +a53_2: cpu@101 {
> +compatible = "arm,cortex-a53";
> +reg = <0x0 0x101>;
> +device_type = "cpu";
> +[...]
> +};
>
[PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time
Introduce a way to create different cpupools at boot time, this is
particularly useful on ARM big.LITTLE system where there might be the
need to have different cpupools for each type of core, but also
systems using NUMA can have different cpu pools for each node.
The feature on arm relies on a specification of the cpupools from the
device tree to build pools and assign cpus to them.
Documentation is created to explain the feature.
Signed-off-by: Luca Fancellu
---
Changes in v2:
- Move feature to common code (Juergen)
- Try to decouple dtb parse and cpupool creation to allow
more way to specify cpupools (for example command line)
- Created standalone dt node for the scheduler so it can
be used in future work to set scheduler specific
parameters
- Use only auto generated ids for cpupools
---
docs/misc/arm/device-tree/cpupools.txt | 156 ++
xen/common/Kconfig | 8 +
xen/common/Makefile| 1 +
xen/common/boot_cpupools.c | 212 +
xen/common/sched/cpupool.c | 6 +-
xen/include/xen/sched.h| 19 +++
6 files changed, 401 insertions(+), 1 deletion(-)
create mode 100644 docs/misc/arm/device-tree/cpupools.txt
create mode 100644 xen/common/boot_cpupools.c
diff --git a/docs/misc/arm/device-tree/cpupools.txt
b/docs/misc/arm/device-tree/cpupools.txt
new file mode 100644
index ..d5a82ed0d45a
--- /dev/null
+++ b/docs/misc/arm/device-tree/cpupools.txt
@@ -0,0 +1,156 @@
+Boot time cpupools
+==
+
+When BOOT_TIME_CPUPOOLS is enabled in the Xen configuration, it is possible to
+create cpupools during boot phase by specifying them in the device tree.
+
+Cpupools specification nodes shall be direct childs of /chosen node.
+Each cpupool node contains the following properties:
+
+- compatible (mandatory)
+
+Must always include the compatiblity string: "xen,cpupool".
+
+- cpupool-cpus (mandatory)
+
+Must be a list of device tree phandle to nodes describing cpus (e.g. having
+device_type = "cpu"), it can't be empty.
+
+- cpupool-sched (optional)
+
+Must be a device tree phandle to a node having "xen,scheduler" compatible
+(description below), it has no effect when the cpupool refers to the
cpupool
+number zero, in that case the default Xen scheduler is selected
(sched=<...>
+boot argument).
+
+
+A scheduler specification node is a device tree node that contains the
following
+properties:
+
+- compatible (mandatory)
+
+Must always include the compatiblity string: "xen,scheduler".
+
+- sched-name (mandatory)
+
+Must be a string having the name of a Xen scheduler, check the sched=<...>
+boot argument for allowed values.
+
+
+Constraints
+===
+
+If no cpupools are specified, all cpus will be assigned to one cpupool
+implicitly created (Pool-0).
+
+If cpupools node are specified, but not every cpu brought up by Xen is
assigned,
+all the not assigned cpu will be assigned to an additional cpupool.
+
+If a cpu is assigned to a cpupool, but it's not brought up correctly, Xen will
+stop.
+
+
+Examples
+
+
+A system having two types of core, the following device tree specification will
+instruct Xen to have two cpupools:
+
+- The cpupool with id 0 will have 4 cpus assigned.
+- The cpupool with id 1 will have 2 cpus assigned.
+
+The following example can work only if hmp-unsafe=1 is passed to Xen boot
+arguments, otherwise not all cores will be brought up by Xen and the cpupool
+creation process will stop Xen.
+
+
+a72_1: cpu@0 {
+compatible = "arm,cortex-a72";
+reg = <0x0 0x0>;
+device_type = "cpu";
+[...]
+};
+
+a72_2: cpu@1 {
+compatible = "arm,cortex-a72";
+reg = <0x0 0x1>;
+device_type = "cpu";
+[...]
+};
+
+a53_1: cpu@100 {
+compatible = "arm,cortex-a53";
+reg = <0x0 0x100>;
+device_type = "cpu";
+[...]
+};
+
+a53_2: cpu@101 {
+compatible = "arm,cortex-a53";
+reg = <0x0 0x101>;
+device_type = "cpu";
+[...]
+};
+
+a53_3: cpu@102 {
+compatible = "arm,cortex-a53";
+reg = <0x0 0x102>;
+device_type = "cpu";
+[...]
+};
+
+a53_4: cpu@103 {
+compatible = "arm,cortex-a53";
+reg = <0x0 0x103>;
+device_type = "cpu";
+[...]
+};
+
+chosen {
+
+sched: sched_a {
+compatible = "xen,scheduler";
+sched-name = "credit2";
+};
+cpupool_a {
+compatible = "xen,cpupool";
+cpupool-cpus = <&a53_1 &a53_2 &a53_3 &a53_4>;
+};
+cpupool_b {
+compatible = "xen,cpupool";
+cpupool-cpus = <&a72_1 &a72_2>;
+cpupool-sched = <&sched>;
+};
+
+[...]
+
+};
+
+
+A system having the cpupools specification below will instruct Xen to have
three
+cpupools:
+
+- The cpupool Pool-0 will have 2 cpus assigned.
+- The cpupool Pool-1 will have 2 cpus assigned.
+- The cpupool Pool-2 will have 2 c
