[PATCH v2 59/62] libxl: pvshim: Provide first-class config settings to enable shim mode

[Wei Liu <wei.liu2@citrix.com>]

From: Ian Jackson <Ian.Jackson@eu.citrix.com>

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"
---
 tools/libxl/libxl.h          |  8 +++++++
 tools/libxl/libxl_create.c   | 19 +++++++++++----
 tools/libxl/libxl_dom.c      | 57 +++++++++++++++++++++++++++++++++++---------
 tools/libxl/libxl_internal.h |  4 ++++
 tools/libxl/libxl_types.idl  |  5 +++-
 5 files changed, 77 insertions(+), 16 deletions(-)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 5e9aed739d..9632fd6d2f 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1101,6 +1101,14 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
  */
 #define LIBXL_HAVE_SET_PARAMETERS 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 f15fb215c2..63cdc33991 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -389,6 +389,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",
@@ -476,10 +488,6 @@ int libxl__domain_build(libxl__gc *gc,
 
         break;
     case LIBXL_DOMAIN_TYPE_PV:
-        ret = libxl__build_pv(gc, domid, info, state);
-        if (ret)
-            goto out;
-
         vments = libxl__calloc(gc, 11, sizeof(char *));
         i = 0;
         vments[i++] = "image/ostype";
@@ -499,6 +507,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 fbbdb9ec2f..b03386409f 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1025,22 +1025,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,
@@ -1154,8 +1183,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 bfa95d8619..2454efa621 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -118,6 +118,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)
@@ -1136,6 +1138,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;
 
     xen_vmemrange_t *vmemranges;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index a239324341..6d060edc0d 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -592,7 +592,10 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                       # Use host's E820 for PCI passthrough.
                                       ("e820_host", libxl_defbool),
                                       ])),
-                 ("pvh", None),
+                 ("pvh", Struct(None, [("pvshim", libxl_defbool),
+                                       ("pvshim_path", string),
+                                       ("pvshim_cmdline", string),
+                                       ])),
                  ("invalid", None),
                  ], keyvar_init_val = "LIBXL_DOMAIN_TYPE_INVALID")),
 
-- 
2.11.0


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