[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-changelog] [xen master] tools/libxl: Reposition build_pre() logic between architectures



commit 1e9bc407cf0732654916ca4311ca1972495d5cbe
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Fri Dec 20 17:13:41 2019 +0000
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Sat Jan 25 18:43:32 2020 +0000

    tools/libxl: Reposition build_pre() logic between architectures
    
    The call to xc_domain_disable_migrate() is made only from x86, while its
    handling in Xen is common.  Move it to the libxl__build_pre().
    
    hvm_set_conf_params(), hvm_set_viridian_features(),
    hvm_set_mca_capabilities(), and the altp2m logic is all in common code 
(parts
    ifdef'd) but despite this, is all actually x86 specific, as least as 
currently
    implemented in Xen.  Some concepts (nested virt, altp2m) are common in
    principle, but need their interface changing to be part of domain_create, 
and
    are not expecting to survive in their current HVM_PARAM form.
    
    Move it all into x86 specific code, and fold all of the xc_hvm_param_set()
    calls together into hvm_set_conf_params() in a far more coherent way.
    
    Finally - ensure that all hypercalls have their return values checked.
    
    No practical change in constructed domains.  Fewer useless hypercalls now to
    construct an ARM guest.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Acked-by: Wei Liu <wl@xxxxxxx>
---
 tools/libxl/libxl_dom.c | 183 ++----------------------------------------------
 tools/libxl/libxl_x86.c | 181 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 185 insertions(+), 179 deletions(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index cdb294ab8d..573c63692b 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -243,149 +243,6 @@ static int numa_place_domain(libxl__gc *gc, uint32_t 
domid,
     return rc;
 }
 
-static unsigned long timer_mode(const libxl_domain_build_info *info)
-{
-    const libxl_timer_mode mode = info->timer_mode;
-    assert(mode >= LIBXL_TIMER_MODE_DELAY_FOR_MISSED_TICKS &&
-           mode <= LIBXL_TIMER_MODE_ONE_MISSED_TICK_PENDING);
-    return ((unsigned long)mode);
-}
-
-#if defined(__i386__) || defined(__x86_64__)
-static int hvm_set_viridian_features(libxl__gc *gc, uint32_t domid,
-                                     libxl_domain_build_info *const info)
-{
-    libxl_bitmap enlightenments;
-    libxl_viridian_enlightenment v;
-    uint64_t mask = 0;
-
-    libxl_bitmap_init(&enlightenments);
-    libxl_bitmap_alloc(CTX, &enlightenments,
-                       LIBXL_BUILDINFO_HVM_VIRIDIAN_ENABLE_DISABLE_WIDTH);
-
-    if (libxl_defbool_val(info->u.hvm.viridian)) {
-        /* Enable defaults */
-        libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_BASE);
-        libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_FREQ);
-        libxl_bitmap_set(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_TIME_REF_COUNT);
-        libxl_bitmap_set(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_APIC_ASSIST);
-        libxl_bitmap_set(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_CRASH_CTL);
-    }
-
-    libxl_for_each_set_bit(v, info->u.hvm.viridian_enable) {
-        if (libxl_bitmap_test(&info->u.hvm.viridian_disable, v)) {
-            LOG(ERROR, "%s group both enabled and disabled",
-                libxl_viridian_enlightenment_to_string(v));
-            goto err;
-        }
-        if (libxl_viridian_enlightenment_to_string(v)) /* check validity */
-            libxl_bitmap_set(&enlightenments, v);
-    }
-
-    libxl_for_each_set_bit(v, info->u.hvm.viridian_disable)
-        if (libxl_viridian_enlightenment_to_string(v)) /* check validity */
-            libxl_bitmap_reset(&enlightenments, v);
-
-    /* The base set is a pre-requisite for all others */
-    if (!libxl_bitmap_is_empty(&enlightenments) &&
-        !libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_BASE)) {
-        LOG(ERROR, "base group not enabled");
-        goto err;
-    }
-
-    libxl_for_each_set_bit(v, enlightenments)
-        LOG(DETAIL, "%s group enabled", 
libxl_viridian_enlightenment_to_string(v));
-
-    if (libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_BASE)) 
{
-        mask |= HVMPV_base_freq;
-
-        if (!libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_FREQ))
-            mask |= HVMPV_no_freq;
-    }
-
-    if (libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_TIME_REF_COUNT))
-        mask |= HVMPV_time_ref_count;
-
-    if (libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_REFERENCE_TSC))
-        mask |= HVMPV_reference_tsc;
-
-    if (libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_HCALL_REMOTE_TLB_FLUSH))
-        mask |= HVMPV_hcall_remote_tlb_flush;
-
-    if (libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_APIC_ASSIST))
-        mask |= HVMPV_apic_assist;
-
-    if (libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_CRASH_CTL))
-        mask |= HVMPV_crash_ctl;
-
-    if (libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_SYNIC))
-        mask |= HVMPV_synic;
-
-    if (libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_STIMER))
-        mask |= HVMPV_time_ref_count | HVMPV_synic | HVMPV_stimer;
-
-    if (libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_HCALL_IPI))
-        mask |= HVMPV_hcall_ipi;
-
-    if (mask != 0 &&
-        xc_hvm_param_set(CTX->xch,
-                         domid,
-                         HVM_PARAM_VIRIDIAN,
-                         mask) != 0) {
-        LOGE(ERROR, "Couldn't set viridian feature mask (0x%"PRIx64")", mask);
-        goto err;
-    }
-
-    libxl_bitmap_dispose(&enlightenments);
-    return 0;
-
-err:
-    libxl_bitmap_dispose(&enlightenments);
-    return ERROR_FAIL;
-}
-
-static int hvm_set_mca_capabilities(libxl__gc *gc, uint32_t domid,
-                                    libxl_domain_build_info *const info)
-{
-    unsigned long caps = info->u.hvm.mca_caps;
-
-    if (!caps)
-        return 0;
-
-    return xc_hvm_param_set(CTX->xch, domid, HVM_PARAM_MCA_CAP, caps);
-}
-#endif
-
-static void hvm_set_conf_params(xc_interface *handle, uint32_t domid,
-                                libxl_domain_build_info *const info)
-{
-    switch(info->type) {
-    case LIBXL_DOMAIN_TYPE_PVH:
-        xc_hvm_param_set(handle, domid, HVM_PARAM_PAE_ENABLED, true);
-        xc_hvm_param_set(handle, domid, HVM_PARAM_TIMER_MODE,
-                         timer_mode(info));
-        xc_hvm_param_set(handle, domid, HVM_PARAM_NESTEDHVM,
-                         libxl_defbool_val(info->nested_hvm));
-        break;
-    case LIBXL_DOMAIN_TYPE_HVM:
-        xc_hvm_param_set(handle, domid, HVM_PARAM_PAE_ENABLED,
-                         libxl_defbool_val(info->u.hvm.pae));
-#if defined(__i386__) || defined(__x86_64__)
-        xc_hvm_param_set(handle, domid, HVM_PARAM_HPET_ENABLED,
-                         libxl_defbool_val(info->u.hvm.hpet));
-#endif
-        xc_hvm_param_set(handle, domid, HVM_PARAM_TIMER_MODE,
-                         timer_mode(info));
-        xc_hvm_param_set(handle, domid, HVM_PARAM_VPT_ALIGN,
-                         libxl_defbool_val(info->u.hvm.vpt_align));
-        xc_hvm_param_set(handle, domid, HVM_PARAM_NESTEDHVM,
-                         libxl_defbool_val(info->nested_hvm));
-        break;
-    default:
-        abort();
-    }
-}
-
 int libxl__build_pre(libxl__gc *gc, uint32_t domid,
               libxl_domain_config *d_config, libxl__domain_build_state *state)
 {
@@ -400,6 +257,12 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
         return ERROR_FAIL;
     }
 
+    if (libxl_defbool_val(d_config->b_info.disable_migrate) &&
+        xc_domain_disable_migrate(ctx->xch, domid) != 0) {
+        LOG(ERROR, "Couldn't set nomigrate");
+        return ERROR_FAIL;
+    }
+
     /*
      * Check if the domain has any CPU or node affinity already. If not, try
      * to build up the latter via automatic NUMA placement. In fact, in case
@@ -522,40 +385,6 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
     state->store_port = xc_evtchn_alloc_unbound(ctx->xch, domid, 
state->store_domid);
     state->console_port = xc_evtchn_alloc_unbound(ctx->xch, domid, 
state->console_domid);
 
-    if (info->type != LIBXL_DOMAIN_TYPE_PV)
-        hvm_set_conf_params(ctx->xch, domid, info);
-
-#if defined(__i386__) || defined(__x86_64__)
-    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
-        rc = hvm_set_viridian_features(gc, domid, info);
-        if (rc)
-            return rc;
-
-        rc = hvm_set_mca_capabilities(gc, domid, info);
-        if (rc)
-            return rc;
-    }
-#endif
-
-    /* Alternate p2m support on x86 is available only for PVH/HVM guests. */
-    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
-        /* The config parameter "altp2m" replaces the parameter "altp2mhvm". 
For
-         * legacy reasons, both parameters are accepted on x86 HVM guests.
-         *
-         * If the legacy field info->u.hvm.altp2m is set, activate altp2m.
-         * Otherwise set altp2m based on the field info->altp2m. */
-        if (info->altp2m == LIBXL_ALTP2M_MODE_DISABLED &&
-            libxl_defbool_val(info->u.hvm.altp2m))
-            xc_hvm_param_set(ctx->xch, domid, HVM_PARAM_ALTP2M,
-                             libxl_defbool_val(info->u.hvm.altp2m));
-        else
-            xc_hvm_param_set(ctx->xch, domid, HVM_PARAM_ALTP2M,
-                             info->altp2m);
-    } else if (info->type == LIBXL_DOMAIN_TYPE_PVH) {
-        xc_hvm_param_set(ctx->xch, domid, HVM_PARAM_ALTP2M,
-                         info->altp2m);
-    }
-
     rc = libxl__arch_domain_create(gc, d_config, domid);
 
     return rc;
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index 8b804537ba..1cae0e2b26 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -285,14 +285,193 @@ static int libxl__e820_alloc(libxl__gc *gc, uint32_t 
domid,
     return 0;
 }
 
+static unsigned long timer_mode(const libxl_domain_build_info *info)
+{
+    const libxl_timer_mode mode = info->timer_mode;
+    assert(mode >= LIBXL_TIMER_MODE_DELAY_FOR_MISSED_TICKS &&
+           mode <= LIBXL_TIMER_MODE_ONE_MISSED_TICK_PENDING);
+    return ((unsigned long)mode);
+}
+
+static int hvm_set_viridian_features(libxl__gc *gc, uint32_t domid,
+                                     const libxl_domain_build_info *info)
+{
+    libxl_bitmap enlightenments;
+    libxl_viridian_enlightenment v;
+    uint64_t mask = 0;
+
+    libxl_bitmap_init(&enlightenments);
+    libxl_bitmap_alloc(CTX, &enlightenments,
+                       LIBXL_BUILDINFO_HVM_VIRIDIAN_ENABLE_DISABLE_WIDTH);
+
+    if (libxl_defbool_val(info->u.hvm.viridian)) {
+        /* Enable defaults */
+        libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_BASE);
+        libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_FREQ);
+        libxl_bitmap_set(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_TIME_REF_COUNT);
+        libxl_bitmap_set(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_APIC_ASSIST);
+        libxl_bitmap_set(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_CRASH_CTL);
+    }
+
+    libxl_for_each_set_bit(v, info->u.hvm.viridian_enable) {
+        if (libxl_bitmap_test(&info->u.hvm.viridian_disable, v)) {
+            LOG(ERROR, "%s group both enabled and disabled",
+                libxl_viridian_enlightenment_to_string(v));
+            goto err;
+        }
+        if (libxl_viridian_enlightenment_to_string(v)) /* check validity */
+            libxl_bitmap_set(&enlightenments, v);
+    }
+
+    libxl_for_each_set_bit(v, info->u.hvm.viridian_disable)
+        if (libxl_viridian_enlightenment_to_string(v)) /* check validity */
+            libxl_bitmap_reset(&enlightenments, v);
+
+    /* The base set is a pre-requisite for all others */
+    if (!libxl_bitmap_is_empty(&enlightenments) &&
+        !libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_BASE)) {
+        LOG(ERROR, "base group not enabled");
+        goto err;
+    }
+
+    libxl_for_each_set_bit(v, enlightenments)
+        LOG(DETAIL, "%s group enabled", 
libxl_viridian_enlightenment_to_string(v));
+
+    if (libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_BASE)) 
{
+        mask |= HVMPV_base_freq;
+
+        if (!libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_FREQ))
+            mask |= HVMPV_no_freq;
+    }
+
+    if (libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_TIME_REF_COUNT))
+        mask |= HVMPV_time_ref_count;
+
+    if (libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_REFERENCE_TSC))
+        mask |= HVMPV_reference_tsc;
+
+    if (libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_HCALL_REMOTE_TLB_FLUSH))
+        mask |= HVMPV_hcall_remote_tlb_flush;
+
+    if (libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_APIC_ASSIST))
+        mask |= HVMPV_apic_assist;
+
+    if (libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_CRASH_CTL))
+        mask |= HVMPV_crash_ctl;
+
+    if (libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_SYNIC))
+        mask |= HVMPV_synic;
+
+    if (libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_STIMER))
+        mask |= HVMPV_time_ref_count | HVMPV_synic | HVMPV_stimer;
+
+    if (libxl_bitmap_test(&enlightenments, 
LIBXL_VIRIDIAN_ENLIGHTENMENT_HCALL_IPI))
+        mask |= HVMPV_hcall_ipi;
+
+    if (mask != 0 &&
+        xc_hvm_param_set(CTX->xch,
+                         domid,
+                         HVM_PARAM_VIRIDIAN,
+                         mask) != 0) {
+        LOGE(ERROR, "Couldn't set viridian feature mask (0x%"PRIx64")", mask);
+        goto err;
+    }
+
+    libxl_bitmap_dispose(&enlightenments);
+    return 0;
+
+err:
+    libxl_bitmap_dispose(&enlightenments);
+    return ERROR_FAIL;
+}
+
+static int hvm_set_conf_params(libxl__gc *gc, uint32_t domid,
+                               const libxl_domain_build_info *info)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    xc_interface *xch = ctx->xch;
+    int ret = ERROR_FAIL;
+    bool pae = true, altp2m = info->altp2m;
+
+    switch(info->type) {
+    case LIBXL_DOMAIN_TYPE_HVM:
+        pae = libxl_defbool_val(info->u.hvm.pae);
+
+        /* The config parameter "altp2m" replaces the parameter "altp2mhvm". 
For
+         * legacy reasons, both parameters are accepted on x86 HVM guests.
+         *
+         * If the legacy field info->u.hvm.altp2m is set, activate altp2m.
+         * Otherwise set altp2m based on the field info->altp2m. */
+        if (info->altp2m == LIBXL_ALTP2M_MODE_DISABLED &&
+            libxl_defbool_val(info->u.hvm.altp2m))
+            altp2m = libxl_defbool_val(info->u.hvm.altp2m);
+
+        if (xc_hvm_param_set(xch, domid, HVM_PARAM_HPET_ENABLED,
+                             libxl_defbool_val(info->u.hvm.hpet))) {
+            LOG(ERROR, "Couldn't set HVM_PARAM_HPET_ENABLED");
+            goto out;
+        }
+        if (xc_hvm_param_set(xch, domid, HVM_PARAM_VPT_ALIGN,
+                             libxl_defbool_val(info->u.hvm.vpt_align))) {
+            LOG(ERROR, "Couldn't set HVM_PARAM_VPT_ALIGN");
+            goto out;
+        }
+        if (info->u.hvm.mca_caps &&
+            xc_hvm_param_set(CTX->xch, domid, HVM_PARAM_MCA_CAP,
+                             info->u.hvm.mca_caps)) {
+            LOG(ERROR, "Couldn't set HVM_PARAM_MCA_CAP");
+            goto out;
+        }
+
+        /* Fallthrough */
+    case LIBXL_DOMAIN_TYPE_PVH:
+        if (xc_hvm_param_set(xch, domid, HVM_PARAM_PAE_ENABLED, pae)) {
+            LOG(ERROR, "Couldn't set HVM_PARAM_PAE_ENABLED");
+            goto out;
+        }
+        if (xc_hvm_param_set(xch, domid, HVM_PARAM_TIMER_MODE,
+                             timer_mode(info))) {
+            LOG(ERROR, "Couldn't set HVM_PARAM_TIMER_MODE");
+            goto out;
+        }
+        if (xc_hvm_param_set(xch, domid, HVM_PARAM_NESTEDHVM,
+                             libxl_defbool_val(info->nested_hvm))) {
+            LOG(ERROR, "Couldn't set HVM_PARAM_NESTEDHVM");
+            goto out;
+        }
+        if (xc_hvm_param_set(xch, domid, HVM_PARAM_ALTP2M, altp2m)) {
+            LOG(ERROR, "Couldn't set HVM_PARAM_ALTP2M");
+            goto out;
+        }
+        break;
+
+    default:
+        abort();
+    }
+
+    ret = 0;
+
+ out:
+    return ret;
+}
+
 int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
         uint32_t domid)
 {
+    const libxl_domain_build_info *info = &d_config->b_info;
     int ret = 0;
     int tsc_mode;
     uint32_t rtc_timeoffset;
     libxl_ctx *ctx = libxl__gc_owner(gc);
 
+    if (info->type != LIBXL_DOMAIN_TYPE_PV &&
+        (ret = hvm_set_conf_params(gc, domid, info)) != 0)
+        goto out;
+
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM &&
+        (ret = hvm_set_viridian_features(gc, domid, info)) != 0)
+        goto out;
+
     if (d_config->b_info.type == LIBXL_DOMAIN_TYPE_PV)
         xc_domain_set_memmap_limit(ctx->xch, domid,
                                    (d_config->b_info.max_memkb +
@@ -322,8 +501,6 @@ int libxl__arch_domain_create(libxl__gc *gc, 
libxl_domain_config *d_config,
         goto out;
     }
 
-    if (libxl_defbool_val(d_config->b_info.disable_migrate))
-        xc_domain_disable_migrate(ctx->xch, domid);
     rtc_timeoffset = d_config->b_info.rtc_timeoffset;
     if (libxl_defbool_val(d_config->b_info.localtime)) {
         time_t t;
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.