[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 4/5] xen/cpupool: Create different cpupools at boot time
On 16.02.22 14:01, Luca Fancellu wrote: On 16 Feb 2022, at 12:55, Juergen Gross <jgross@xxxxxxxx> wrote: On 16.02.22 13:10, Luca Fancellu wrote:On 16 Feb 2022, at 02:45, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote: On Tue, 15 Feb 2022, Luca Fancellu wrote:Introduce an architecture specific 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.fancellu@xxxxxxx> --- docs/misc/arm/device-tree/cpupools.txt | 118 +++++++++++++++++++++++++ xen/arch/arm/Kconfig | 9 ++ xen/arch/arm/Makefile | 1 + xen/arch/arm/cpupool.c | 118 +++++++++++++++++++++++++ xen/common/sched/cpupool.c | 4 +- xen/include/xen/sched.h | 11 +++ 6 files changed, 260 insertions(+), 1 deletion(-) create mode 100644 docs/misc/arm/device-tree/cpupools.txt create mode 100644 xen/arch/arm/cpupool.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..7298b6394332 --- /dev/null +++ b/docs/misc/arm/device-tree/cpupools.txt @@ -0,0 +1,118 @@ +Boot time cpupools +================== + +On arm, 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-id (mandatory) + + Must be a positive integer number.Hi Stefano, Thank you for your review,Why is cpupool-id mandatory? It looks like it could be generated by Xen. Or is it actually better to have the user specify it anyway?Yes at first I thought to automatically generate that, however I needed a structure to map the id to the cpupool DT node. Here my doubt was about the size of the structure, because the user could even specify a cpupool for each cpu. I could allocate It dynamically and free it after domUs creation in setup_xen. What do you think could be the right way? Or the dom0less guest could specify the id, but I like it more when using a phandle to the Xen,cpupool node.+- 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, it has no effect when + used in conjunction of a cpupool-id equal to zero, in that case the + default Xen scheduler is selected (sched=<...> boot argument).I don't get why cpupool-id == 0 should trigger a special cpupool-sched behavior.Cpupool with id 0 is embedded in Xen, it has its own special case handling in cpupool_create that is giving it the default scheduler. I thought it was better to leave it as it was, however the cpupool0 scheduler can be modified using sched= boot args as it was before.+Constraints +=========== + +The cpupool with id zero is implicitly created even if not specified, that pool +must have at least one cpu assigned, otherwise Xen will stop. + +Every cpu brought up by Xen will be assigned to the cpupool with id zero if it's +not assigned to any other cpupool. + +If a cpu is assigned to a cpupool, but it's not brought up correctly, Xen will +stop.Thank you for documenting the constraints, but why do we have them? Imagine a user specifying 3 cpu pools and imagine the cpupool-id is optional and missing. We could take care of the cpupool-id generation in Xen and we could also assign the default scheduler everywhere cpupool-sched is not specified. Maybe I am missing something?Yes we could make the cpupool-id optional, my doubts are in the fist comment above. Whenever the cpupool-sched is not specified, the current behaviour is to use the default scheduler.Does cpupool0 has to exist? I guess the answer could be yes, but if it is specified as id of one of the pools we are fine, otherwise it could be automatically generated by Xen.Yes cpupool0 needs to exists, however it is still generated by Xen regardless of the DT specifications. In fact you could not specify in the DT any xen,cpupool compatible node with the cpupool-id == 0 and Xen will generate the cpupool0 anyway (Xen internals are tied with the existence of a cpupool0).In any case, I don't think that cpupool0 has to have the default scheduler?Ok I think I can create a function to assign a scheduler to the cpupool0 after its creation, I would need to test it to be sure I don’t find something strange.My suggestion would be: - make cpupool-id optional - assign automatically cpupool-ids starting from 0 - respect cpupool-ids chosen by the userOk, it would start from 1 because cpupool0 always exists- if some CPUs are left out (not specified in any pool) add an extra cpupool - the extra cpupool doesn't have to be cpupool-id == 0, it could be cpupool-id == n - the extra cpupool uses the default schedulerI gave all the unassigned cpus to cpupool0 to reflect the current behaviour, so that a user that doesn’t specify any xen,cpupool node ends up in a system reflecting the current behaviour as the feature is not enabled. However I can say, if no xen,cpupool nodes are found then assign cpus to cpupool0, else assign them to a new cpupool and...If the user created cpupools in device tree covering all CPUs and also specified all cpupool-ids everywhere, and none of them are 0 (no cpupool in the system is cpupool0) then panic. (Assuming that cpupool0 is required.)… panic if cpupool0 has no cpus.Today cpu 0 is always member of cpupool0, and changing that might be hard.Oh, are you sure? I did some test in the past for this serie using a Juno board, giving to cpupool0 only a72 cores and the a53 cores in another cpupool, my Juno firmware configuration makes Xen having the boot cpu (cpu 0) to be one of the a53 and it was working fine. But it was long time ago so I would need to try it again. Maybe on Arm the restrictions are less problematic, but I wouldn't bet that all operations (like moving cpus between cpupools, cpu hotplug, destroying cpupools, shutdown of the host, ...) are working in a sane way. Juergen Attachment:
OpenPGP_0xB0DE9DD628BF132F.asc Attachment:
OpenPGP_signature
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |