Re: [PATCH v5 4/6] xen/cpupool: Create different cpupools at boot time
Hi Luca,
On 07/04/2022 10:52, Luca Fancellu wrote:
+void __init btcpupools_dtb_parse(void)
+{
+const struct dt_device_node *chosen, *node;
+
+chosen = dt_find_node_by_path("/chosen");
+if ( !chosen )
+return;
Aside when using ACPI, the chosen node should always be there. So I think we
should throw/print an error if chosen is not present.
When you say error, do you mean like a panic or just a printk XENLOG_ERR and
return?
You seem to use panic() below. So I would also use panic() here as this
shouldn't be expected.
Cheers,
--
Julien Grall
Re: [PATCH v5 4/6] xen/cpupool: Create different cpupools at boot time
> On 7 Apr 2022, at 09:58, Julien Grall wrote:
>
> Hi Luca,
>
> On 05/04/2022 09:57, 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.
>
> How will this work for ACPI? Note that I am not suggesting to add suport
> right now. However, we probably want to clarify that this is not yet
> supported.
Ok I will add a note saying ACPI is not supported in v6
>
> [...]
>
>> diff --git a/docs/misc/arm/device-tree/cpupools.txt
>> b/docs/misc/arm/device-tree/cpupools.txt
>> new file mode 100644
>> index ..5dac2b1384e0
>> --- /dev/null
>> +++ b/docs/misc/arm/device-tree/cpupools.txt
>> @@ -0,0 +1,136 @@
>> +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.
>
> How about ACPI?
Same as above
>
>> +
>> +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 string having the name of a Xen scheduler. Check the
>> sched=<...>
>> +boot argument for allowed values.
>
> I would clarify what would be the scheduler if cpupool-sched is not specified.
>
> Also, I would give a pointer to xen-command-line.pandoc so it is easier to
> know where 'sched' is described.
Good point, I think the info got missed through the series, I’ll add and put
the link.
>
> [...]
>
>> +void __init btcpupools_dtb_parse(void)
>> +{
>> +const struct dt_device_node *chosen, *node;
>> +
>> +chosen = dt_find_node_by_path("/chosen");
>> +if ( !chosen )
>> +return;
> Aside when using ACPI, the chosen node should always be there. So I think we
> should throw/print an error if chosen is not present.
When you say error, do you mean like a panic or just a printk XENLOG_ERR and
return?
>
> Also, I would check that we haven't booted using ACPI rather than relying on
> dt_find_node_by_path("/chosen") to return NULL.
Ok I will add a check on acpi_disabled to return if it is false.
Cheers,
Luca
>
> Cheers,
>
> --
> Julien Grall
Re: [PATCH v5 4/6] xen/cpupool: Create different cpupools at boot time
Hi Luca,
On 05/04/2022 09:57, 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.
How will this work for ACPI? Note that I am not suggesting to add suport
right now. However, we probably want to clarify that this is not yet
supported.
[...]
diff --git a/docs/misc/arm/device-tree/cpupools.txt
b/docs/misc/arm/device-tree/cpupools.txt
new file mode 100644
index ..5dac2b1384e0
--- /dev/null
+++ b/docs/misc/arm/device-tree/cpupools.txt
@@ -0,0 +1,136 @@
+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.
How about ACPI?
+
+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 string having the name of a Xen scheduler. Check the sched=<...>
+boot argument for allowed values.
I would clarify what would be the scheduler if cpupool-sched is not
specified.
Also, I would give a pointer to xen-command-line.pandoc so it is easier
to know where 'sched' is described.
[...]
+void __init btcpupools_dtb_parse(void)
+{
+const struct dt_device_node *chosen, *node;
+
+chosen = dt_find_node_by_path("/chosen");
+if ( !chosen )
+return;
Aside when using ACPI, the chosen node should always be there. So I
think we should throw/print an error if chosen is not present.
Also, I would check that we haven't booted using ACPI rather than
relying on dt_find_node_by_path("/chosen") to return NULL.
Cheers,
--
Julien Grall
Re: [PATCH v5 4/6] xen/cpupool: Create different cpupools at boot time
On 05.04.22 10:57, 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 Reviewed-by: Juergen Gross # xen/common/sched OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v5 4/6] xen/cpupool: Create different cpupools at boot time
On Tue, 5 Apr 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
Looks good to me now:
Reviewed-by: Stefano Stabellini
I think we only need Juergen to take one last look at the series and it
should be good to go.
> ---
> Changes in v5:
> - Fixed wrong variable name, swapped schedulers, add scheduler info
> in the printk (Stefano)
> - introduce assert in cpupool_init and btcpupools_get_cpupool_id to
> harden the code
> Changes in v4:
> - modify Makefile to put in *.init.o, fixed stubs and macro (Jan)
> - fixed docs, fix brakets (Stefano)
> - keep cpu0 in Pool-0 (Julien)
> - moved printk from btcpupools_allocate_pools to
> btcpupools_get_cpupool_id
> - Add to docs constraint about cpu0 and Pool-0
> Changes in v3:
> - Add newline to cpupools.txt and removed "default n" from Kconfig (Jan)
> - Fixed comment, moved defines, used global cpu_online_map, use
> HAS_DEVICE_TREE instead of ARM and place arch specific code in header
> (Juergen)
> - Fix brakets, x86 code only panic, get rid of scheduler dt node, don't
> save pool pointer and look for it from the pool list (Stefano)
> - Changed data structures to allow modification to the code.
> 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 | 136 +
> xen/arch/arm/include/asm/smp.h | 3 +
> xen/common/Kconfig | 7 +
> xen/common/Makefile| 1 +
> xen/common/boot_cpupools.c | 203 +
> xen/common/sched/cpupool.c | 12 +-
> xen/include/xen/sched.h| 14 ++
> 7 files changed, 375 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 ..5dac2b1384e0
> --- /dev/null
> +++ b/docs/misc/arm/device-tree/cpupools.txt
> @@ -0,0 +1,136 @@
> +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 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.
> +
> +The boot cpu must be assigned to Pool-0, so the cpupool containing that core
> +will become Pool-0 automatically.
> +
> +
> +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";
> +
[PATCH v5 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 v5:
- Fixed wrong variable name, swapped schedulers, add scheduler info
in the printk (Stefano)
- introduce assert in cpupool_init and btcpupools_get_cpupool_id to
harden the code
Changes in v4:
- modify Makefile to put in *.init.o, fixed stubs and macro (Jan)
- fixed docs, fix brakets (Stefano)
- keep cpu0 in Pool-0 (Julien)
- moved printk from btcpupools_allocate_pools to
btcpupools_get_cpupool_id
- Add to docs constraint about cpu0 and Pool-0
Changes in v3:
- Add newline to cpupools.txt and removed "default n" from Kconfig (Jan)
- Fixed comment, moved defines, used global cpu_online_map, use
HAS_DEVICE_TREE instead of ARM and place arch specific code in header
(Juergen)
- Fix brakets, x86 code only panic, get rid of scheduler dt node, don't
save pool pointer and look for it from the pool list (Stefano)
- Changed data structures to allow modification to the code.
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 | 136 +
xen/arch/arm/include/asm/smp.h | 3 +
xen/common/Kconfig | 7 +
xen/common/Makefile| 1 +
xen/common/boot_cpupools.c | 203 +
xen/common/sched/cpupool.c | 12 +-
xen/include/xen/sched.h| 14 ++
7 files changed, 375 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 ..5dac2b1384e0
--- /dev/null
+++ b/docs/misc/arm/device-tree/cpupools.txt
@@ -0,0 +1,136 @@
+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 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.
+
+The boot cpu must be assigned to Pool-0, so the cpupool containing that core
+will become Pool-0 automatically.
+
+
+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 {
+
+cpupool_a {
+compatible =
