[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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 000000000000..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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.