libxl: pvshim: Provide first-class config settings to enable shim mode

This is API-compatible because old callers are supposed to call
libxl_*_init to initialise the struct; and the updated function clears
these members.

It is ABI-compatible because the new fields make this member of the
guest type union larger but only within the existing size of that
union.

Unfortunately it is not easy to backport because it depends on the PVH
domain type.  Attempts to avoid use of the PVH domain type involved
working with two views of the configuration: the "underlying" domain
type and the "visible" type (and corresponding config info).  Also
there are different sets of config settings for PV and PVH, which
callers would have to know to set.

And, unfortunately, it will not be possible, with this approach, to
enable the shim by default for all libxl callers.  (Although it could
perhaps be done in xl.)

For now, our config defaults are:
 * if enabled, path is "xen-shim" in the xen firmware directory
 * if enabled, cmdline is the one we are currently debugging with

The debugging arguments will be rationalised in a moment.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: pvshim, not pvhshim
    works with type "pvh", not type "pv"

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 097e54be634a7cc5c5c7fc0e051982f08f018e39..baa23a7f1f01f1627171a0470ee13c337bfab6ac 100644 (file)
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1100,6 +1100,14 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
  */
 #define LIBXL_HAVE_MEMKB_64BITS 1
 
+/*
+ * LIBXL_HAVE_PV_SHIM
+ *
+ * If this is defined, libxl_domain_build_info's pvh type information
+ * contains members pvshim, pvshim_path, pvshim_cmdline.
+ */
+#define LIBXL_HAVE_PV_SHIM 1
+
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index ceb5e520cbc752ac5f3571c6e9351f720ad9f053..2c70e83fd4d1ef852ba8d4f32e95d29610a155b4 100644 (file)
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -385,6 +385,18 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         }
         break;
     case LIBXL_DOMAIN_TYPE_PVH:
+        libxl_defbool_setdefault(&b_info->u.pvh.pvshim, false);
+        if (libxl_defbool_val(b_info->u.pvh.pvshim)) {
+            if (!b_info->u.pvh.pvshim_path)
+                b_info->u.pvh.pvshim_path =
+                    libxl__sprintf(NOGC, "%s/%s",
+                                   libxl__xenfirmwaredir_path(),
+                                   PVSHIM_BASENAME);
+            if (!b_info->u.pvh.pvshim_cmdline)
+                b_info->u.pvh.pvshim_cmdline =
+                    libxl__strdup(NOGC, PVSHIM_CMDLINE);
+        }
+
         break;
     default:
         LOG(ERROR, "invalid domain type %s in create info",
@@ -493,6 +505,9 @@ int libxl__domain_build(libxl__gc *gc,
 
         break;
     case LIBXL_DOMAIN_TYPE_PVH:
+        state->shim_path = info->u.pvh.pvshim_path;
+        state->shim_cmdline = info->u.pvh.pvshim_cmdline;
+
         ret = libxl__build_hvm(gc, domid, d_config, state);
         if (ret)
             goto out;

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 8c0a7e27bdb3132a1553ea6314aed5f2af93e89d..3f91fc5072f84429390ffb4482cc9ae49d080c23 100644 (file)
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -980,22 +980,51 @@ static int libxl__domain_firmware(libxl__gc *gc,
 
     if (state->pv_kernel.path != NULL &&
         info->type == LIBXL_DOMAIN_TYPE_PVH) {
-        /* Try to load a kernel instead of the firmware. */
-        if (state->pv_kernel.mapped) {
-            rc = xc_dom_kernel_mem(dom, state->pv_kernel.data,
-                                   state->pv_kernel.size);
+
+        if (state->shim_path) {
+            rc = xc_dom_kernel_file(dom, state->shim_path);
             if (rc) {
-                LOGE(ERROR, "xc_dom_kernel_mem failed");
+                LOGE(ERROR, "xc_dom_kernel_file failed");
                 goto out;
             }
+
+            /* We've loaded the shim, so load the kernel as a secondary module */
+            if (state->pv_kernel.mapped) {
+                LOG(WARN, "xc_dom_module_mem, cmdline %s",
+                    state->pv_cmdline);
+                rc = xc_dom_module_mem(dom, state->pv_kernel.data,
+                                       state->pv_kernel.size, state->pv_cmdline);
+                if (rc) {
+                    LOGE(ERROR, "xc_dom_kernel_mem failed");
+                    goto out;
+                }
+            } else {
+                LOG(WARN, "xc_dom_module_file, path %s cmdline %s",
+                    state->pv_kernel.path, state->pv_cmdline);
+                rc = xc_dom_module_file(dom, state->pv_kernel.path, state->pv_cmdline);
+                if (rc) {
+                    LOGE(ERROR, "xc_dom_kernel_file failed");
+                    goto out;
+                }
+            }
         } else {
-            rc = xc_dom_kernel_file(dom, state->pv_kernel.path);
-            if (rc) {
-                LOGE(ERROR, "xc_dom_kernel_file failed");
-                goto out;
+            /* No shim, so load the kernel directly */
+            if (state->pv_kernel.mapped) {
+                rc = xc_dom_kernel_mem(dom, state->pv_kernel.data,
+                                       state->pv_kernel.size);
+                if (rc) {
+                    LOGE(ERROR, "xc_dom_kernel_mem failed");
+                    goto out;
+                }
+            } else {
+                rc = xc_dom_kernel_file(dom, state->pv_kernel.path);
+                if (rc) {
+                    LOGE(ERROR, "xc_dom_kernel_file failed");
+                    goto out;
+                }
             }
         }
-
+        
         if (state->pv_ramdisk.path && strlen(state->pv_ramdisk.path)) {
             if (state->pv_ramdisk.mapped) {
                 rc = xc_dom_module_mem(dom, state->pv_ramdisk.data,
@@ -1109,8 +1138,14 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
 
     xc_dom_loginit(ctx->xch);
 
+    /* 
+     * If PVH and we have a shim override, use the shim cmdline.
+     * If PVH and no shim override, use the pv cmdline.
+     * If not PVH, use info->cmdline.
+     */
     dom = xc_dom_allocate(ctx->xch, info->type == LIBXL_DOMAIN_TYPE_PVH ?
-                          state->pv_cmdline : info->cmdline, NULL);
+                          (state->shim_path ? state->shim_cmdline : state->pv_cmdline) :
+                          info->cmdline, NULL);
     if (!dom) {
         LOGE(ERROR, "xc_dom_allocate failed");
         rc = ERROR_NOMEM;

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 4460d21a6957d09012744c06ccc1446111651da7..66edc0796609186745032e66f0daae390ab30129 100644 (file)
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -116,6 +116,8 @@
 #define TAP_DEVICE_SUFFIX "-emu"
 #define DOMID_XS_PATH "domid"
 #define INVALID_DOMID ~0
+#define PVSHIM_BASENAME "xen-shim"
+#define PVSHIM_CMDLINE "pv-shim console=xen,pv sched=null loglvl=all guest_loglvl=all apic_verbosity=debug e820-verbose"
 
 /* Size macros. */
 #define __AC(X,Y)   (X##Y)
@@ -1126,6 +1128,8 @@ typedef struct {
 
     libxl__file_reference pv_kernel;
     libxl__file_reference pv_ramdisk;
+    const char * shim_path;
+    const char * shim_cmdline;
     const char * pv_cmdline;
     bool pvh_enabled;
 

diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 70ce42944cec3b3270fc73e848070c3a99e95ec0..512de2b70e0d6fe5f867ae209dbe224d1a091c5b 100644 (file)
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -566,6 +566,9 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                        ("timer_mode",       libxl_timer_mode),
                                        ("nested_hvm",       libxl_defbool),
                                        ("apic",             libxl_defbool),
+                                       ("pvshim",           libxl_defbool),
+                                       ("pvshim_path",      string),
+                                       ("pvshim_cmdline",   string),
                                        ])),
                  ("invalid", None),
                  ], keyvar_init_val = "LIBXL_DOMAIN_TYPE_INVALID")),