[Xen-devel] [PATCH v1.5 20/20] xen/domain: Allocate d->vcpu[] in arch_domain_create()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On the ARM side, audit config->max_vcpus after the vgic has been initialised,
at which point we have a real upper bound to test against.  This allows for
the removal of the vgic_max_vcpus() juggling to cope with the call from
evtchn_init() before the vgic settings are known.

For each arch's dom0's, drop the temporary max_vcpus parameter, and allocation
of dom0->vcpu.

With arch_domain_create() now in charge of auditing config->max_vcpus, the
per-arch domain_max_vcpus() can be dropped.  Finally, evtchn_init() can be
updated to allocate a poll mask suitable for the domain, rather than suitable
for the worst case setting.

From this point on, d->max_vcpus and d->vcpus[] are valid for any domain which
can be looked up by ID.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>

v2:
 * Retain vgic_max_vcpus() on the ARM side, but remove the NULL special case.
---
 xen/arch/arm/domain.c        | 11 +++++++++++
 xen/arch/arm/domain_build.c  |  8 +-------
 xen/arch/arm/setup.c         |  2 +-
 xen/arch/arm/vgic.c          | 11 +----------
 xen/arch/x86/dom0_build.c    |  8 +-------
 xen/arch/x86/domain.c        | 11 +++++++++++
 xen/arch/x86/setup.c         |  2 +-
 xen/common/domctl.c          | 14 --------------
 xen/common/event_channel.c   |  4 ++--
 xen/include/asm-arm/domain.h |  6 ------
 xen/include/asm-x86/domain.h |  2 --
 xen/include/xen/domain.h     |  2 +-
 12 files changed, 30 insertions(+), 51 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 57a2b8b..7abe766 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -646,6 +646,17 @@ int arch_domain_create(struct domain *d,
     if ( (rc = domain_vtimer_init(d, &config->arch)) != 0 )
         goto fail;
 
+    rc = -EINVAL;
+    /* On ARM, the number of VCPUs is limited by the type of GIC emulated. */
+    if ( (config->max_vcpus < 1) || (config->max_vcpus > vgic_max_vcpus(d)) )
+        goto fail;
+
+    rc = -ENOMEM;
+    d->vcpu = xzalloc_array(struct vcpu *, config->max_vcpus);
+    if ( !d->vcpu )
+        goto fail;
+    d->max_vcpus = config->max_vcpus;
+
     update_domain_wallclock_time(d);
 
     /*
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2e145d9..b13c47e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -76,14 +76,8 @@ unsigned int __init dom0_max_vcpus(void)
     return opt_dom0_max_vcpus;
 }
 
-struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0,
-                                     unsigned int max_vcpus)
+struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
 {
-    dom0->vcpu = xzalloc_array(struct vcpu *, max_vcpus);
-    if ( !dom0->vcpu )
-        return NULL;
-    dom0->max_vcpus = max_vcpus;
-
     return alloc_vcpu(dom0, 0, 0);
 }
 
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index be24f20..0ada4d5 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -859,7 +859,7 @@ void __init start_xen(unsigned long boot_phys_offset,
     dom0_cfg.max_vcpus = dom0_max_vcpus();
 
     dom0 = domain_create(0, &dom0_cfg);
-    if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0, dom0_cfg.max_vcpus) == NULL) )
+    if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
             panic("Error creating domain 0");
 
     dom0->is_privileged = 1;
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 3fafdd0..4d2b00c 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -669,16 +669,7 @@ void vgic_free_virq(struct domain *d, unsigned int virq)
 
 unsigned int vgic_max_vcpus(const struct domain *d)
 {
-    /*
-     * Since evtchn_init would call domain_max_vcpus for poll_mask
-     * allocation when the vgic_ops haven't been initialised yet,
-     * we return MAX_VIRT_CPUS if d->arch.vgic.handler is null.
-     */
-    if ( !d->arch.vgic.handler )
-        return MAX_VIRT_CPUS;
-    else
-        return min_t(unsigned int, MAX_VIRT_CPUS,
-                     d->arch.vgic.handler->max_vcpus);
+    return min_t(unsigned int, MAX_VIRT_CPUS, d->arch.vgic.handler->max_vcpus);
 }
 
 /*
diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index e82bc48..4c25789 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -200,17 +200,11 @@ unsigned int __init dom0_max_vcpus(void)
     return max_vcpus;
 }
 
-struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0,
-                                     unsigned int max_vcpus)
+struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
 {
     dom0->node_affinity = dom0_nodes;
     dom0->auto_node_affinity = !dom0_nr_pxms;
 
-    dom0->vcpu = xzalloc_array(struct vcpu *, max_vcpus);
-    if ( !dom0->vcpu )
-        return NULL;
-    dom0->max_vcpus = max_vcpus;
-
     return dom0_setup_vcpu(dom0, 0,
                            cpumask_last(&dom0_cpus) /* so it wraps around to 
first pcpu */);
 }
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index c4c34b4..90fe50a 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -504,6 +504,17 @@ int arch_domain_create(struct domain *d,
     HYPERVISOR_COMPAT_VIRT_START(d) =
         is_pv_domain(d) ? __HYPERVISOR_COMPAT_VIRT_START : ~0u;
 
+    if ( (config->max_vcpus < 1) ||
+         (config->max_vcpus >
+          (is_hvm_domain(d) ? HVM_MAX_VCPUS : MAX_VIRT_CPUS)) )
+        return -EINVAL;
+
+    rc = -ENOMEM;
+    d->vcpu = xzalloc_array(struct vcpu *, config->max_vcpus);
+    if ( !d->vcpu )
+        goto fail;
+    d->max_vcpus = config->max_vcpus;
+
     /* Need to determine if HAP is enabled before initialising paging */
     if ( is_hvm_domain(d) )
         d->arch.hvm_domain.hap_enabled =
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index b0e85b0..07e9893 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1649,7 +1649,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
     /* Create initial domain 0. */
     dom0 = domain_create(get_initial_domain_id(), &dom0_cfg);
-    if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0, dom0_cfg.max_vcpus) == NULL) )
+    if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
         panic("Error creating domain 0");
 
     if ( !pv_shim )
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 2f9d993..1dc19ea 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -547,23 +547,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
         op->domain = d->domain_id;
         copyback = true;
 
-        ret = -EINVAL;
-        if ( vcpus > domain_max_vcpus(d) )
-            goto createdomain_fail_late;
-
         ret = -ENOMEM;
         online = cpupool_domain_cpumask(d);
 
-        BUG_ON(d->vcpu);
-        BUG_ON(d->max_vcpus);
-
-        d->vcpu = xzalloc_array(struct vcpu *, vcpus);
-        /* Install vcpu array /then/ update max_vcpus. */
-        smp_wmb();
-        if ( !d->vcpu )
-            goto createdomain_fail_late;
-        d->max_vcpus = vcpus;
-
         cpu = cpumask_any(online);
         for ( i = 0; i < vcpus; ++i )
         {
diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 41cbbae..2e6f84b 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -1303,8 +1303,8 @@ int evtchn_init(struct domain *d, unsigned int max_port)
     evtchn_from_port(d, 0)->state = ECS_RESERVED;
 
 #if MAX_VIRT_CPUS > BITS_PER_LONG
-    d->poll_mask = xzalloc_array(unsigned long,
-                                 BITS_TO_LONGS(domain_max_vcpus(d)));
+    BUG_ON(d->max_vcpus == 0);
+    d->poll_mask = xzalloc_array(unsigned long, BITS_TO_LONGS(d->max_vcpus));
     if ( !d->poll_mask )
     {
         free_evtchn_bucket(d, d->evtchn);
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 7ba6528..2273b1b 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -208,12 +208,6 @@ void vcpu_show_execution_state(struct vcpu *);
 void vcpu_show_registers(const struct vcpu *);
 void vcpu_switch_to_aarch64_mode(struct vcpu *);
 
-/* On ARM, the number of VCPUs is limited by the type of GIC emulated. */
-static inline unsigned int domain_max_vcpus(const struct domain *d)
-{
-    return vgic_max_vcpus(d);
-}
-
 /*
  * Due to the restriction of GICv3, the number of vCPUs in AFF0 is
  * limited to 16, thus only the first 4 bits of AFF0 are legal. We will
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 47aadc2..9a21e0f 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -624,8 +624,6 @@ unsigned long pv_guest_cr4_fixup(const struct vcpu *, 
unsigned long guest_cr4);
              X86_CR4_OSXSAVE | X86_CR4_SMEP |               \
              X86_CR4_FSGSBASE | X86_CR4_SMAP))
 
-#define domain_max_vcpus(d) (is_hvm_domain(d) ? HVM_MAX_VCPUS : MAX_VIRT_CPUS)
-
 static inline struct vcpu_guest_context *alloc_vcpu_guest_context(void)
 {
     return vmalloc(sizeof(struct vcpu_guest_context));
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index dc022b4..3dcec06 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -17,7 +17,7 @@ struct vcpu *alloc_vcpu(
     struct domain *d, unsigned int vcpu_id, unsigned int cpu_id);
 
 unsigned int dom0_max_vcpus(void);
-struct vcpu *alloc_dom0_vcpu0(struct domain *dom0, unsigned int max_vcpus);
+struct vcpu *alloc_dom0_vcpu0(struct domain *dom0);
 
 int vcpu_reset(struct vcpu *);
 int vcpu_up(struct vcpu *v);
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel