> On 15 Feb 2022, at 10:56, Juergen Gross <jgr...@suse.com> wrote:
>
> On 15.02.22 11:15, 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_arch_domainconfig public interface so the
>> XEN_DOMCTL_INTERFACE_VERSION version is bumped.
>> Update documentation about the property.
>> Signed-off-by: Luca Fancellu <luca.fance...@arm.com>
>> ---
>> docs/misc/arm/device-tree/booting.txt | 5 +++++
>> xen/arch/arm/domain.c | 6 ++++++
>> xen/arch/arm/domain_build.c | 9 ++++++++-
>> xen/arch/x86/domain.c | 6 ++++++
>> xen/common/domain.c | 5 ++++-
>> xen/include/public/arch-arm.h | 2 ++
>> xen/include/public/domctl.h | 2 +-
>> xen/include/xen/domain.h | 3 +++
>> 8 files changed, 35 insertions(+), 3 deletions(-)
>> diff --git a/docs/misc/arm/device-tree/booting.txt
>> b/docs/misc/arm/device-tree/booting.txt
>> index 71895663a4de..0f1f210fa449 100644
>> --- a/docs/misc/arm/device-tree/booting.txt
>> +++ b/docs/misc/arm/device-tree/booting.txt
>> @@ -182,6 +182,11 @@ with the following properties:
>> Both #address-cells and #size-cells need to be specified because
>> both sub-nodes (described shortly) have reg properties.
>> +- 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.c b/xen/arch/arm/domain.c
>> index 92a6c509e5c5..be350b28b588 100644
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -788,6 +788,12 @@ fail:
>> return rc;
>> }
>> +unsigned int
>> +arch_get_domain_cpupool_id(const struct xen_domctl_createdomain *config)
>> +{
>> + return config->arch.cpupool_id;
>> +}
>> +
>
> I don't see why this should be arch specific.
>
>> void arch_domain_destroy(struct domain *d)
>> {
>> /* IOMMU page table is shared with P2M, always call
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 6931c022a2e8..4f239e756775 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -3015,7 +3015,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)
>> @@ -3053,6 +3054,12 @@ void __init create_domUs(void)
>> GUEST_VPL011_SPI - 32 + 1);
>> }
>> + /* Get the optional property domain-cpupool */
>> + cpupool_node = dt_parse_phandle(node, "domain-cpupool", 0);
>> + if ( cpupool_node )
>> + dt_property_read_u32(cpupool_node, "cpupool-id",
>> + &d_cfg.arch.cpupool_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/arch/x86/domain.c b/xen/arch/x86/domain.c
>> index ef1812dc1402..3e3cf88c9c82 100644
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -880,6 +880,12 @@ int arch_domain_create(struct domain *d,
>> return rc;
>> }
>> +unsigned int
>> +arch_get_domain_cpupool_id(const struct xen_domctl_createdomain *config)
>> +{
>> + return 0;
>> +}
>> +
>> void arch_domain_destroy(struct domain *d)
>> {
>> if ( is_hvm_domain(d) )
>> diff --git a/xen/common/domain.c b/xen/common/domain.c
>> index 2048ebad86ff..d42ca8292025 100644
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -665,6 +665,8 @@ struct domain *domain_create(domid_t domid,
>> if ( !is_idle_domain(d) )
>> {
>> + unsigned int domain_cpupool_id;
>> +
>> watchdog_domain_init(d);
>> init_status |= INIT_watchdog;
>> @@ -698,7 +700,8 @@ struct domain *domain_create(domid_t domid,
>> if ( !d->pbuf )
>> goto fail;
>> - if ( (err = sched_init_domain(d, 0)) != 0 )
>> + domain_cpupool_id = arch_get_domain_cpupool_id(config);
>> + if ( (err = sched_init_domain(d, domain_cpupool_id)) != 0 )
>> goto fail;
>> if ( (err = late_hwdom_init(d)) != 0 )
>> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
>> index 94b31511ddea..2c5d1ea7f01a 100644
>> --- a/xen/include/public/arch-arm.h
>> +++ b/xen/include/public/arch-arm.h
>> @@ -321,6 +321,8 @@ struct xen_arch_domainconfig {
>> uint16_t tee_type;
>> /* IN */
>> uint32_t nr_spis;
>> + /* IN */
>> + unsigned int cpupool_id;
>
> As said above: why is this arch specific? Moving it to the common part
> would enable libxl to get rid of having to call xc_cpupool_movedomain()
> in libxl__domain_make().
I’ve put it in arch because it’s only modified by the arm code, but if you
think it’s ok
to have it in struct xen_domctl_createdomain, I don’t see any problem.
My knowledge of the tool stack is limited so I didn’t know about the advantages
of that change.
Cheers,
Luca
>
>
> Juergen
> <OpenPGP_0xB0DE9DD628BF132F.asc>
