diff mbox series

[v8,6/7] arm/dom0less: assign dom0less guests to cpupools

Message ID 20220506120012.32326-7-luca.fancellu@arm.com (mailing list archive)
State Accepted
Headers show
Series Boot time cpupools | expand

Commit Message

Luca Fancellu May 6, 2022, noon UTC
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>
---
Changes in v8:
- no changes
Changes in v7:
- Add comment for cpupool_id struct member. (Jan)
Changes in v6:
- no changes
Changes in v5:
- no changes
Changes in v4:
- no changes
- add R-by
Changes in v3:
- Use explicitely sized integer for struct xen_domctl_createdomain
  cpupool_id member. (Stefano)
- Changed code due to previous commit code changes
Changes in v2:
- Moved cpupool_id from arch specific to common part (Juergen)
- Implemented functions to retrieve the cpupool id from the
  cpupool dtb node.
---
 docs/misc/arm/device-tree/booting.txt |  5 +++++
 xen/arch/arm/domain_build.c           | 14 +++++++++++++-
 xen/common/domain.c                   |  2 +-
 xen/common/sched/boot-cpupool.c       | 24 ++++++++++++++++++++++++
 xen/include/public/domctl.h           |  5 ++++-
 xen/include/xen/sched.h               |  9 +++++++++
 6 files changed, 56 insertions(+), 3 deletions(-)

Comments

Juergen Gross May 6, 2022, 1:06 p.m. UTC | #1
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
Andrew Cooper May 17, 2022, 1:01 p.m. UTC | #2
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
Juergen Gross May 17, 2022, 1:08 p.m. UTC | #3
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
Andrew Cooper May 17, 2022, 1:10 p.m. UTC | #4
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
Luca Fancellu May 17, 2022, 1:12 p.m. UTC | #5
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 mbox series

Patch

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__ */