Xen devel - Sep 2013 - [V4 PATCH 0/2]: PVH xen tools: tools changes to create PVH guest

Please find V4 tools changes for PVH. These were built on unstable c/s:
546ba2f17008387cf9821df46e6dac04f0883a9b. Comment logs fixed with
''---''.

thanks
Mukesh

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
V2: Make pvh_features string const, and fail 32bit PVH guest creation.
    Move PVH check to xc_dom_gnttab_init().
V3: let xc_dom_gnttab_hvm_seed be called for PVH.
---
 tools/libxc/xc_dom.h      |    1 +
 tools/libxc/xc_dom_core.c |    9 ++++
 tools/libxc/xc_dom_x86.c  |   90 +++++++++++++++++++++++++++++---------------
 3 files changed, 69 insertions(+), 31 deletions(-)

diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
index 86e23ee..5168bcd 100644
--- a/tools/libxc/xc_dom.h
+++ b/tools/libxc/xc_dom.h
@@ -130,6 +130,7 @@ struct xc_dom_image {
     domid_t console_domid;
     domid_t xenstore_domid;
     xen_pfn_t shared_info_mfn;
+    int pvh_enabled;
 
     xc_interface *xch;
     domid_t guest_domid;
diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
index 0f367f6..faa7e0f 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -766,6 +766,15 @@ int xc_dom_parse_image(struct xc_dom_image *dom)
         goto err;
     }
 
+    if ( dom->pvh_enabled )
+    {
+        const char *pvh_features = "writable_descriptor_tables|"
+                                   "auto_translated_physmap|"
+                                   "supervisor_mode_kernel|"
+                                   "hvm_callback_vector";
+        elf_xen_parse_features(pvh_features, dom->f_requested, NULL);
+    }
+
     /* check features */
     for ( i = 0; i < XENFEAT_NR_SUBMAPS; i++ )
     {
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index 126c0f8..fcc9d87 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -415,7 +415,8 @@ static int setup_pgtables_x86_64(struct xc_dom_image *dom)
         pgpfn = (addr - dom->parms.virt_base) >> PAGE_SHIFT_X86;
         l1tab[l1off]              pfn_to_paddr(xc_dom_p2m_guest(dom, pgpfn)) |
L1_PROT;
-        if ( (addr >= dom->pgtables_seg.vstart) && 
+        if ( (!dom->pvh_enabled)                &&
+             (addr >= dom->pgtables_seg.vstart) &&
              (addr < dom->pgtables_seg.vend) )
             l1tab[l1off] &= ~_PAGE_RW; /* page tables are r/o */
         if ( l1off == (L1_PAGETABLE_ENTRIES_X86_64 - 1) )
@@ -587,6 +588,13 @@ static int vcpu_x86_32(struct xc_dom_image *dom, void *ptr)
 
     DOMPRINTF_CALLED(dom->xch);
 
+    if ( dom->pvh_enabled )
+    {
+        xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
+                     "%s: PVH not supported for 32bit guests.",
__FUNCTION__);
+        return -1;
+    }
+
     /* clear everything */
     memset(ctxt, 0, sizeof(*ctxt));
 
@@ -629,12 +637,6 @@ static int vcpu_x86_64(struct xc_dom_image *dom, void *ptr)
     /* clear everything */
     memset(ctxt, 0, sizeof(*ctxt));
 
-    ctxt->user_regs.ds = FLAT_KERNEL_DS_X86_64;
-    ctxt->user_regs.es = FLAT_KERNEL_DS_X86_64;
-    ctxt->user_regs.fs = FLAT_KERNEL_DS_X86_64;
-    ctxt->user_regs.gs = FLAT_KERNEL_DS_X86_64;
-    ctxt->user_regs.ss = FLAT_KERNEL_SS_X86_64;
-    ctxt->user_regs.cs = FLAT_KERNEL_CS_X86_64;
     ctxt->user_regs.rip = dom->parms.virt_entry;
     ctxt->user_regs.rsp          dom->parms.virt_base +
(dom->bootstack_pfn + 1) * PAGE_SIZE_X86;
@@ -642,15 +644,25 @@ static int vcpu_x86_64(struct xc_dom_image *dom, void
*ptr)
         dom->parms.virt_base + (dom->start_info_pfn) * PAGE_SIZE_X86;
     ctxt->user_regs.rflags = 1 << 9; /* Interrupt Enable */
 
-    ctxt->kernel_ss = ctxt->user_regs.ss;
-    ctxt->kernel_sp = ctxt->user_regs.esp;
-
     ctxt->flags = VGCF_in_kernel_X86_64 | VGCF_online_X86_64;
     cr3_pfn = xc_dom_p2m_guest(dom, dom->pgtables_seg.pfn);
     ctxt->ctrlreg[3] = xen_pfn_to_cr3_x86_64(cr3_pfn);
     DOMPRINTF("%s: cr3: pfn 0x%" PRIpfn " mfn 0x%" PRIpfn
"",
               __FUNCTION__, dom->pgtables_seg.pfn, cr3_pfn);
 
+    if ( dom->pvh_enabled )
+        return 0;
+
+    ctxt->user_regs.ds = FLAT_KERNEL_DS_X86_64;
+    ctxt->user_regs.es = FLAT_KERNEL_DS_X86_64;
+    ctxt->user_regs.fs = FLAT_KERNEL_DS_X86_64;
+    ctxt->user_regs.gs = FLAT_KERNEL_DS_X86_64;
+    ctxt->user_regs.ss = FLAT_KERNEL_SS_X86_64;
+    ctxt->user_regs.cs = FLAT_KERNEL_CS_X86_64;
+
+    ctxt->kernel_ss = ctxt->user_regs.ss;
+    ctxt->kernel_sp = ctxt->user_regs.esp;
+
     return 0;
 }
 
@@ -751,7 +763,7 @@ int arch_setup_meminit(struct xc_dom_image *dom)
     rc = x86_compat(dom->xch, dom->guest_domid, dom->guest_type);
     if ( rc )
         return rc;
-    if ( xc_dom_feature_translated(dom) )
+    if ( xc_dom_feature_translated(dom) && !dom->pvh_enabled )
     {
         dom->shadow_enabled = 1;
         rc = x86_shadow(dom->xch, dom->guest_domid);
@@ -827,6 +839,38 @@ int arch_setup_bootearly(struct xc_dom_image *dom)
     return 0;
 }
 
+/*
+ * Map grant table frames into guest physmap. PVH manages grant during boot
+ * via HVM mechanisms.
+ */
+static int map_grant_table_frames(struct xc_dom_image *dom)
+{
+    int i, rc;
+
+    if ( dom->pvh_enabled )
+        return 0;
+
+    for ( i = 0; ; i++ )
+    {
+        rc = xc_domain_add_to_physmap(dom->xch, dom->guest_domid,
+                                      XENMAPSPACE_grant_table,
+                                      i, dom->total_pages + i);
+        if ( rc != 0 )
+        {
+            if ( (i > 0) && (errno == EINVAL) )
+            {
+                DOMPRINTF("%s: %d grant tables mapped", __FUNCTION__,
i);
+                break;
+            }
+            xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
+                         "%s: mapping grant tables failed "
"(pfn=0x%" PRIpfn
+                         ", rc=%d)", __FUNCTION__,
dom->total_pages + i, rc);
+            return rc;
+        }
+    }
+    return 0;
+}
+
 int arch_setup_bootlate(struct xc_dom_image *dom)
 {
     static const struct {
@@ -865,7 +909,6 @@ int arch_setup_bootlate(struct xc_dom_image *dom)
     else
     {
         /* paravirtualized guest with auto-translation */
-        int i;
 
         /* Map shared info frame into guest physmap. */
         rc = xc_domain_add_to_physmap(dom->xch, dom->guest_domid,
@@ -879,25 +922,10 @@ int arch_setup_bootlate(struct xc_dom_image *dom)
             return rc;
         }
 
-        /* Map grant table frames into guest physmap. */
-        for ( i = 0; ; i++ )
-        {
-            rc = xc_domain_add_to_physmap(dom->xch, dom->guest_domid,
-                                          XENMAPSPACE_grant_table,
-                                          i, dom->total_pages + i);
-            if ( rc != 0 )
-            {
-                if ( (i > 0) && (errno == EINVAL) )
-                {
-                    DOMPRINTF("%s: %d grant tables mapped",
__FUNCTION__, i);
-                    break;
-                }
-                xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
-                             "%s: mapping grant tables failed "
"(pfn=0x%"
-                             PRIpfn ", rc=%d)", __FUNCTION__,
dom->total_pages + i, rc);
-                return rc;
-            }
-        }
+        rc = map_grant_table_frames(dom);
+        if ( rc != 0 )
+            return rc;
+
         shinfo = dom->shared_info_pfn;
     }
 
-- 
1.7.2.3

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
V2: set c_info->hap to default value of pvh, and use LOG(ERROR...).
    Move PVH check to xc_dom_gnttab_init.
    Move xl doc change here.
---
 docs/man/xl.cfg.pod.5        |    3 +++
 tools/libxl/libxl.h          |    6 ++++++
 tools/libxl/libxl_create.c   |   12 ++++++++++++
 tools/libxl/libxl_dom.c      |    2 ++
 tools/libxl/libxl_internal.h |    1 +
 tools/libxl/libxl_types.idl  |    1 +
 tools/libxl/libxl_x86.c      |    4 +++-
 tools/libxl/xl_cmdimpl.c     |    1 +
 8 files changed, 29 insertions(+), 1 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 08d6cc4..65d69f8 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -634,6 +634,9 @@ if your particular guest kernel does not require this
behaviour then
 it is safe to allow this to be enabled but you may wish to disable it
 anyway.
 
+=item B<pvh=BOOLEAN>
+Selects whether to run this PV guest in an HVM container. Default is 0.
+
 =back
 
 =head2 Fully-virtualised (HVM) Guest Specific Options
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index be19bf5..f0b0701 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -344,6 +344,12 @@
  */
 #define LIBXL_HAVE_DOMINFO_OUTSTANDING_MEMKB 1
 
+/*
+ * LIBXL_HAVE_CREATEINFO_PVH
+ * If this is defined, then libxl supports creation of a PVH guest.
+ */
+#define LIBXL_HAVE_CREATEINFO_PVH 1
+
 /* Functions annotated with LIBXL_EXTERNAL_CALLERS_ONLY may not be
  * called from within libxl itself. Callers outside libxl, who
  * do not #include libxl_internal.h, are fine. */
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 0c32d0b..6d2101e 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -33,6 +33,9 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
     if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) {
         libxl_defbool_setdefault(&c_info->hap, true);
         libxl_defbool_setdefault(&c_info->oos, true);
+    } else {
+        libxl_defbool_setdefault(&c_info->pvh, false);
+        libxl_defbool_setdefault(&c_info->hap,
libxl_defbool_val(c_info->pvh));
     }
 
     libxl_defbool_setdefault(&c_info->run_hotplug_scripts, true);
@@ -346,6 +349,8 @@ int libxl__domain_build(libxl__gc *gc,
 
         break;
     case LIBXL_DOMAIN_TYPE_PV:
+        state->pvh_enabled = libxl_defbool_val(d_config->c_info.pvh);
+
         ret = libxl__build_pv(gc, domid, info, state);
         if (ret)
             goto out;
@@ -405,6 +410,13 @@ int libxl__domain_make(libxl__gc *gc,
libxl_domain_create_info *info,
         flags |= XEN_DOMCTL_CDF_hvm_guest;
         flags |= libxl_defbool_val(info->hap) ? XEN_DOMCTL_CDF_hap : 0;
         flags |= libxl_defbool_val(info->oos) ? 0 : XEN_DOMCTL_CDF_oos_off;
+    } else if ( libxl_defbool_val(info->pvh) ) {
+        if ( !libxl_defbool_val(info->hap) ) {
+            LOG(ERROR, "HAP must be on for PVH");
+            rc = ERROR_INVAL;
+            goto out;
+        }
+        flags |= XEN_DOMCTL_CDF_hap;
     }
     *domid = -1;
 
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 6e2252a..89e919e 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -338,6 +338,8 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
         return ERROR_FAIL;
     }
 
+    dom->pvh_enabled = state->pvh_enabled;
+
     LOG(DEBUG, "pv kernel mapped %d path %s\n",
state->pv_kernel.mapped, state->pv_kernel.path);
     if (state->pv_kernel.mapped) {
         ret = xc_dom_kernel_mem(dom,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index f051d91..441ec66 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -886,6 +886,7 @@ typedef struct {
     libxl__file_reference pv_kernel;
     libxl__file_reference pv_ramdisk;
     const char * pv_cmdline;
+    bool pvh_enabled;
 } libxl__domain_build_state;
 
 _hidden int libxl__build_pre(libxl__gc *gc, uint32_t domid,
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 10f95f4..d4d5bb7 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -255,6 +255,7 @@ libxl_domain_create_info =
Struct("domain_create_info",[
     ("platformdata", libxl_key_value_list),
     ("poolid",       uint32),
     ("run_hotplug_scripts",libxl_defbool),
+    ("pvh",          libxl_defbool),
     ], dir=DIR_IN)
 
 MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index a78c91d..87a8110 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -290,7 +290,9 @@ int libxl__arch_domain_create(libxl__gc *gc,
libxl_domain_config *d_config,
     if (rtc_timeoffset)
         xc_domain_set_time_offset(ctx->xch, domid, rtc_timeoffset);
 
-    if (d_config->b_info.type == LIBXL_DOMAIN_TYPE_HVM) {
+    if (d_config->b_info.type == LIBXL_DOMAIN_TYPE_HVM ||
+        libxl_defbool_val(d_config->c_info.pvh)) {
+
         unsigned long shadow;
         shadow = (d_config->b_info.shadow_memkb + 1023) / 1024;
         xc_shadow_control(ctx->xch, domid,
XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION, NULL, 0, &shadow, 0, NULL);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 884f050..2ee68a9 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -610,6 +610,7 @@ static void parse_config_data(const char *config_source,
         !strncmp(buf, "hvm", strlen(buf)))
         c_info->type = LIBXL_DOMAIN_TYPE_HVM;
 
+    xlu_cfg_get_defbool(config, "pvh", &c_info->pvh, 0);
     xlu_cfg_get_defbool(config, "hap", &c_info->hap, 0);
 
     if (xlu_cfg_replace_string (config, "name", &c_info->name,
0)) {
-- 
1.7.2.3

On Wed, 2013-09-11 at 16:22 -0700, Mukesh Rathor wrote:
> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

On Wed, 2013-09-11 at 16:22 -0700, Mukesh Rathor wrote:
> Please find V4 tools changes for PVH. These were built on unstable c/s:
> 546ba2f17008387cf9821df46e6dac04f0883a9b. Comment logs fixed with
''---''.
These are both now acked but I don''t think it makes much sense to
actually apply them until the h/v side is ready (cc-ing some set of
people who I think are involved in that).

Perhaps you could append this to the h/v series and whoever applies that
can just hoover up the lot? 

Or just ping me when the appropriate time comes.

Ian.

>>> On 12.09.13 at 10:38, Ian Campbell <Ian.Campbell@citrix.com>
wrote:
> On Wed, 2013-09-11 at 16:22 -0700, Mukesh Rathor wrote:
>> Please find V4 tools changes for PVH. These were built on unstable c/s:
>> 546ba2f17008387cf9821df46e6dac04f0883a9b. Comment logs fixed with
''---''.
> 
> These are both now acked but I don''t think it makes much sense to
> actually apply them until the h/v side is ready (cc-ing some set of
> people who I think are involved in that).
> 
> Perhaps you could append this to the h/v series and whoever applies that
> can just hoover up the lot? 
That would be fine with me. Still waiting for George''s reworked
series though.

Jan

On Thu, 12 Sep 2013 09:38:14 +0100
Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Wed, 2013-09-11 at 16:22 -0700, Mukesh Rathor wrote:
> > Please find V4 tools changes for PVH. These were built on unstable
> > c/s: 546ba2f17008387cf9821df46e6dac04f0883a9b. Comment logs fixed
> > with ''---''.
> 
> These are both now acked but I don''t think it makes much sense to
> actually apply them until the h/v side is ready (cc-ing some set of
> people who I think are involved in that).
> 
> Perhaps you could append this to the h/v series and whoever applies
> that can just hoover up the lot? 
FWIW, applying them would be harmless in the sense that guest
creation will fail but will not crash xen or anything...

thanks
Mukesh

On Thu, 2013-09-12 at 17:23 -0700, Mukesh Rathor wrote:
> On Thu, 12 Sep 2013 09:38:14 +0100
> Ian Campbell <Ian.Campbell@citrix.com> wrote:
> 
> > On Wed, 2013-09-11 at 16:22 -0700, Mukesh Rathor wrote:
> > > Please find V4 tools changes for PVH. These were built on
unstable
> > > c/s: 546ba2f17008387cf9821df46e6dac04f0883a9b. Comment logs fixed
> > > with ''---''.
> > 
> > These are both now acked but I don''t think it makes much
sense to
> > actually apply them until the h/v side is ready (cc-ing some set of
> > people who I think are involved in that).
> > 
> > Perhaps you could append this to the h/v series and whoever applies
> > that can just hoover up the lot? 
> 
> FWIW, applying them would be harmless in the sense that guest
> creation will fail but will not crash xen or anything...
It would expose and options (and the associated docs) for things which
don''t actually work.

Jan''s fine with these being appended to the h/v series, so lets do
that.

Ian.