| Message ID | 20220506120012.32326-7-luca.fancellu@arm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Boot time cpupools | expand |
On 06.05.22 14:00, Luca Fancellu wrote: > Introduce domain-cpupool property of a xen,domain device tree node, > that specifies the cpupool device tree handle of a xen,cpupool node > that identifies a cpupool created at boot time where the guest will > be assigned on creation. > > Add member to the xen_domctl_createdomain public interface so the > XEN_DOMCTL_INTERFACE_VERSION version is bumped. > > Add public function to retrieve a pool id from the device tree > cpupool node. > > Update documentation about the property. > > Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> Reviewed-by: Juergen Gross <jgross@suse.com> # non-arm parts Juergen
On 06/05/2022 13:00, Luca Fancellu wrote: > Introduce domain-cpupool property of a xen,domain device tree node, > that specifies the cpupool device tree handle of a xen,cpupool node > that identifies a cpupool created at boot time where the guest will > be assigned on creation. > > Add member to the xen_domctl_createdomain public interface so the > XEN_DOMCTL_INTERFACE_VERSION version is bumped. > > Add public function to retrieve a pool id from the device tree > cpupool node. > > Update documentation about the property. > > Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> This has broken the Ocaml bindings, and is conceptually wrong. The cpupool to use is a property of the vcpu, not the domain. It isn't legitimately part of createdomain. ~Andrew
On 17.05.22 15:01, Andrew Cooper wrote: > On 06/05/2022 13:00, Luca Fancellu wrote: >> Introduce domain-cpupool property of a xen,domain device tree node, >> that specifies the cpupool device tree handle of a xen,cpupool node >> that identifies a cpupool created at boot time where the guest will >> be assigned on creation. >> >> Add member to the xen_domctl_createdomain public interface so the >> XEN_DOMCTL_INTERFACE_VERSION version is bumped. >> >> Add public function to retrieve a pool id from the device tree >> cpupool node. >> >> Update documentation about the property. >> >> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> >> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > This has broken the Ocaml bindings, and is conceptually wrong. > > The cpupool to use is a property of the vcpu, not the domain. It isn't > legitimately part of createdomain. What? All vcpus of a domain are always in the same cpupool. There is no operation "move vcpu to cpupool", but "move domain to cpupool". So your claim that the cpupool wouldn't be a property of the domain is IMO wrong. Juergen
On 17/05/2022 14:01, Andrew Cooper wrote: > On 06/05/2022 13:00, Luca Fancellu wrote: >> Introduce domain-cpupool property of a xen,domain device tree node, >> that specifies the cpupool device tree handle of a xen,cpupool node >> that identifies a cpupool created at boot time where the guest will >> be assigned on creation. >> >> Add member to the xen_domctl_createdomain public interface so the >> XEN_DOMCTL_INTERFACE_VERSION version is bumped. >> >> Add public function to retrieve a pool id from the device tree >> cpupool node. >> >> Update documentation about the property. >> >> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> >> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > This has broken the Ocaml bindings, and is conceptually wrong. And it lets the toolstack pass in CPUPOOLID_NONE at which point Xen will fall over a NULL pointer. ~Andrew
Hi Andrew, > On 17 May 2022, at 14:01, Andrew Cooper <Andrew.Cooper3@citrix.com> wrote: > > On 06/05/2022 13:00, Luca Fancellu wrote: >> Introduce domain-cpupool property of a xen,domain device tree node, >> that specifies the cpupool device tree handle of a xen,cpupool node >> that identifies a cpupool created at boot time where the guest will >> be assigned on creation. >> >> Add member to the xen_domctl_createdomain public interface so the >> XEN_DOMCTL_INTERFACE_VERSION version is bumped. >> >> Add public function to retrieve a pool id from the device tree >> cpupool node. >> >> Update documentation about the property. >> >> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> >> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > This has broken the Ocaml bindings, and is conceptually wrong. Ok, can you tell me what to do to update the Ocaml bindings? Do we have some resources somewhere? > > The cpupool to use is a property of the vcpu, not the domain. It isn't > legitimately part of createdomain. I agree with Juergen on that, could you explain it better for me to understand your point? > > ~Andrew
diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt index a94125394e35..7b4a29a2c293 100644 --- a/docs/misc/arm/device-tree/booting.txt +++ b/docs/misc/arm/device-tree/booting.txt @@ -188,6 +188,11 @@ with the following properties: An empty property to request the memory of the domain to be direct-map (guest physical address == physical address). +- domain-cpupool + + Optional. Handle to a xen,cpupool device tree node that identifies the + cpupool where the guest will be started at boot. + Under the "xen,domain" compatible node, one or more sub-nodes are present for the DomU kernel and ramdisk. diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 5df5c8ffb8ba..aa777741bdd0 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -3174,7 +3174,8 @@ static int __init construct_domU(struct domain *d, void __init create_domUs(void) { struct dt_device_node *node; - const struct dt_device_node *chosen = dt_find_node_by_path("/chosen"); + const struct dt_device_node *cpupool_node, + *chosen = dt_find_node_by_path("/chosen"); BUG_ON(chosen == NULL); dt_for_each_child_node(chosen, node) @@ -3243,6 +3244,17 @@ void __init create_domUs(void) vpl011_virq - 32 + 1); } + /* Get the optional property domain-cpupool */ + cpupool_node = dt_parse_phandle(node, "domain-cpupool", 0); + if ( cpupool_node ) + { + int pool_id = btcpupools_get_domain_pool_id(cpupool_node); + if ( pool_id < 0 ) + panic("Error getting cpupool id from domain-cpupool (%d)\n", + pool_id); + d_cfg.cpupool_id = pool_id; + } + /* * The variable max_init_domid is initialized with zero, so here it's * very important to use the pre-increment operator to call diff --git a/xen/common/domain.c b/xen/common/domain.c index 8d2c2a989708..7570eae91a24 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -697,7 +697,7 @@ struct domain *domain_create(domid_t domid, if ( !d->pbuf ) goto fail; - if ( (err = sched_init_domain(d, 0)) != 0 ) + if ( (err = sched_init_domain(d, config->cpupool_id)) != 0 ) goto fail; if ( (err = late_hwdom_init(d)) != 0 ) diff --git a/xen/common/sched/boot-cpupool.c b/xen/common/sched/boot-cpupool.c index 9429a5025fc4..240bae4cebb8 100644 --- a/xen/common/sched/boot-cpupool.c +++ b/xen/common/sched/boot-cpupool.c @@ -22,6 +22,8 @@ static unsigned int __initdata next_pool_id; #define BTCPUPOOLS_DT_NODE_NO_REG (-1) #define BTCPUPOOLS_DT_NODE_NO_LOG_CPU (-2) +#define BTCPUPOOLS_DT_WRONG_NODE (-3) +#define BTCPUPOOLS_DT_CORRUPTED_NODE (-4) static int __init get_logical_cpu_from_hw_id(unsigned int hwid) { @@ -56,6 +58,28 @@ get_logical_cpu_from_cpu_node(const struct dt_device_node *cpu_node) return cpu_num; } +int __init btcpupools_get_domain_pool_id(const struct dt_device_node *node) +{ + const struct dt_device_node *phandle_node; + int cpu_num; + + if ( !dt_device_is_compatible(node, "xen,cpupool") ) + return BTCPUPOOLS_DT_WRONG_NODE; + /* + * Get first cpu listed in the cpupool, from its reg it's possible to + * retrieve the cpupool id. + */ + phandle_node = dt_parse_phandle(node, "cpupool-cpus", 0); + if ( !phandle_node ) + return BTCPUPOOLS_DT_CORRUPTED_NODE; + + cpu_num = get_logical_cpu_from_cpu_node(phandle_node); + if ( cpu_num < 0 ) + return cpu_num; + + return pool_cpu_map[cpu_num]; +} + static int __init check_and_get_sched_id(const char* scheduler_name) { int sched_id = sched_get_id_by_name(scheduler_name); diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index b85e6170b0aa..84e75829b980 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -38,7 +38,7 @@ #include "hvm/save.h" #include "memory.h" -#define XEN_DOMCTL_INTERFACE_VERSION 0x00000014 +#define XEN_DOMCTL_INTERFACE_VERSION 0x00000015 /* * NB. xen_domctl.domain is an IN/OUT parameter for this operation. @@ -106,6 +106,9 @@ struct xen_domctl_createdomain { /* Per-vCPU buffer size in bytes. 0 to disable. */ uint32_t vmtrace_size; + /* CPU pool to use; specify 0 or a specific existing pool */ + uint32_t cpupool_id; + struct xen_arch_domainconfig arch; }; diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 74b3aae10b94..32d2a6294b6d 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -1188,6 +1188,7 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi); void btcpupools_allocate_pools(void); unsigned int btcpupools_get_cpupool_id(unsigned int cpu); void btcpupools_dtb_parse(void); +int btcpupools_get_domain_pool_id(const struct dt_device_node *node); #else /* !CONFIG_BOOT_TIME_CPUPOOLS */ static inline void btcpupools_allocate_pools(void) {} @@ -1196,6 +1197,14 @@ static inline unsigned int btcpupools_get_cpupool_id(unsigned int cpu) { return 0; } +#ifdef CONFIG_HAS_DEVICE_TREE +static inline int +btcpupools_get_domain_pool_id(const struct dt_device_node *node) +{ + return 0; +} +#endif + #endif #endif /* __SCHED_H__ */