> On 7 Apr 2022, at 09:58, Julien Grall <jul...@xen.org> 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 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?
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
