On 11.03.22 09:56, Luca Fancellu wrote:
Hi Juergen,

Thanks for your review

On 11 Mar 2022, at 08:09, Juergen Gross <jgr...@suse.com> 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 <luca.fance...@arm.com>
---
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 000000000000..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 cpus assigned (created by Xen with all the not
+  assigned cpus a53_3 and a53_4).
+
+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
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 64439438891c..dc9eed31682f 100644
--- 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
+       help
+         Creates cpupools during boot time and assigns cpus to them. Cpupools
+         options can be specified in the device tree.
+
  config ALTERNATIVE_CALL
        bool
  diff --git a/xen/common/Makefile b/xen/common/Makefile
index dc8d3a13f5b8..c5949785ab28 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -1,5 +1,6 @@
  obj-$(CONFIG_ARGO) += argo.o
  obj-y += bitmap.o
+obj-$(CONFIG_BOOT_TIME_CPUPOOLS) += boot_cpupools.o
  obj-$(CONFIG_HYPFS_CONFIG) += config_data.o
  obj-$(CONFIG_CORE_PARKING) += core_parking.o
  obj-y += cpu.o
diff --git a/xen/common/boot_cpupools.c b/xen/common/boot_cpupools.c
new file mode 100644
index 000000000000..e8529a902d21
--- /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 <xen/sched.h>
+
+#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?

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.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature

Reply via email to