[V1 PATCH 0/5]: PVH xen tools: tools changes to create PVH guest

[V1 PATCH 0/5]: PVH xen tools: tools changes to create PVH guest

[Mukesh Rathor]

Ian C, 

Please find the tools changes for PVH. I got everything just the way
you wanted :)... please review. These were built on unstable c/s:
8a7769b4453168e23e8935a85e9a875ef5117253.

A quick intro for the uninformed, these are patches for tools changes
to support creation of a PVH guest. A PVH guest is a PV guest that
runs in an HVM container. Note, at present HAP is required, and also
only a 64bit PV can run in PVH mode. A PVH guest is created by 
specifying pvh=1 in the config file. 

thanks,
Mukesh

[V1 PATCH 1/5] PVH xen tools: Elf and header change.

[Mukesh Rathor]

 Add XENFEAT_hvm_callback_vector to elf_xen_feature_names
 so we can ensure the kernel supports all features requird for 
 PVH mode. Note, hvm callback is required for PVH. 
 Also, introduce _XEN_DOMINF_pvh_guest.

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
 xen/common/libelf/libelf-dominfo.c |    1 +
 xen/include/public/domctl.h        |    3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c
index e79b339..25dd892 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -27,6 +27,7 @@ static const char *const elf_xen_feature_names[] = {
     [XENFEAT_auto_translated_physmap] = "auto_translated_physmap",
     [XENFEAT_supervisor_mode_kernel] = "supervisor_mode_kernel",
     [XENFEAT_pae_pgdir_above_4gb] = "pae_pgdir_above_4gb",
+    [XENFEAT_hvm_callback_vector] = "hvm_callback_vector",
     [XENFEAT_dom0] = "dom0"
 };
 static const unsigned elf_xen_features =
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 4c5b2bb..6b1aa11 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -89,6 +89,9 @@ struct xen_domctl_getdomaininfo {
  /* Being debugged.  */
 #define _XEN_DOMINF_debugged  6
 #define XEN_DOMINF_debugged   (1U<<_XEN_DOMINF_debugged)
+/* domain is PVH */
+#define _XEN_DOMINF_pvh_guest 7
+#define XEN_DOMINF_pvh_guest   (1U<<_XEN_DOMINF_pvh_guest)
  /* XEN_DOMINF_shutdown guest-supplied code.  */
 #define XEN_DOMINF_shutdownmask 255
 #define XEN_DOMINF_shutdownshift 16
-- 
1.7.2.3

[V1 PATCH 2/5] PVH xen tools: libxc changes to build a PVH guest.

[Mukesh Rathor]

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
 tools/libxc/xc_dom.h      |    1 +
 tools/libxc/xc_dom_core.c |    8 ++++
 tools/libxc/xc_dom_x86.c  |   83 ++++++++++++++++++++++++++++-----------------
 3 files changed, 61 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..6ebe7ec 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -766,6 +766,14 @@ int xc_dom_parse_image(struct xc_dom_image *dom)
         goto err;
     }
 
+    if ( dom->pvh_enabled )
+    {
+        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..5b2d61d 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) )
@@ -629,12 +630,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 +637,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 +756,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 +832,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 +902,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 +915,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

[V1 PATCH 3/5] PVH xen tools: libxl changes to create a PVH guest.

[Mukesh Rathor]

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
 tools/libxl/libxl_create.c   |   12 ++++++++++++
 tools/libxl/libxl_dom.c      |    5 ++++-
 tools/libxl/libxl_internal.h |    1 +
 tools/libxl/libxl_types.idl  |    1 +
 tools/libxl/libxl_x86.c      |    4 +++-
 tools/libxl/xl_cmdimpl.c     |    1 +
 6 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 0c32d0b..0dfc4b7 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, false);
     }
 
     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) ) {
+            LIBXL__LOG(ctx, LIBXL__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..d84313f 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,
@@ -400,7 +402,8 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
         LOGE(ERROR, "xc_dom_boot_image failed");
         goto out;
     }
-    if ( (ret = xc_dom_gnttab_init(dom)) != 0 ) {
+    /* PVH sets up its own grant during boot via hvm mechanisms */
+    if ( !dom->pvh_enabled && (ret = xc_dom_gnttab_init(dom)) != 0 ) {
         LOGE(ERROR, "xc_dom_gnttab_init failed");
         goto out;
     }
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 85341a0..9919786 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -248,6 +248,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

[V1 PATCH 4/5] PVH xen tools: xenstored changes

[Mukesh Rathor]

 Allow map_interface in xenstored to revert to the old
 xc_map_foreign_range if xc_gnttab_map_grant_ref is unable
 to map the mfn because the guest is auto translate.

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
 tools/xenstore/xenstored_domain.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index bf83d58..10c23a1 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -168,13 +168,15 @@ static int readchn(struct connection *conn, void *data, unsigned int len)
 static void *map_interface(domid_t domid, unsigned long mfn)
 {
 	if (*xcg_handle != NULL) {
-		/* this is the preferred method */
-		return xc_gnttab_map_grant_ref(*xcg_handle, domid,
+                void *addr;
+                /* this is the preferred method */
+                addr = xc_gnttab_map_grant_ref(*xcg_handle, domid,
 			GNTTAB_RESERVED_XENSTORE, PROT_READ|PROT_WRITE);
-	} else {
-		return xc_map_foreign_range(*xc_handle, domid,
-			getpagesize(), PROT_READ|PROT_WRITE, mfn);
+                if (addr)
+                        return addr;
 	}
+	return xc_map_foreign_range(*xc_handle, domid,
+		        getpagesize(), PROT_READ|PROT_WRITE, mfn);
 }
 
 static void unmap_interface(void *interface)
-- 
1.7.2.3

[V1 PATCH 5/5] PVH xen tools: misc changes

[Mukesh Rathor]

  - Introduce PVH guest type for gdbsx so it can be used to
    debug a PVH guest.
  - xl man page change.

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
 docs/man/xl.cfg.pod.5             |    3 +++
 tools/debugger/gdbsx/xg/xg_main.c |    4 +++-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 08d6cc4..4c925c9 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 guest in an HVM container. Default is 0.
+
 =back
 
 =head2 Fully-virtualised (HVM) Guest Specific Options
diff --git a/tools/debugger/gdbsx/xg/xg_main.c b/tools/debugger/gdbsx/xg/xg_main.c
index 64c7484..5736b86 100644
--- a/tools/debugger/gdbsx/xg/xg_main.c
+++ b/tools/debugger/gdbsx/xg/xg_main.c
@@ -81,6 +81,7 @@ int xgtrc_on = 0;
 struct xen_domctl domctl;         /* just use a global domctl */
 
 static int     _hvm_guest;        /* hvm guest? 32bit HVMs have 64bit context */
+static int     _pvh_guest;        /* PV guest in HVM container */
 static domid_t _dom_id;           /* guest domid */
 static int     _max_vcpu_id;      /* thus max_vcpu_id+1 VCPUs */
 static int     _dom0_fd;          /* fd of /dev/privcmd */
@@ -309,6 +310,7 @@ xg_attach(int domid, int guest_bitness)
 
     _max_vcpu_id = domctl.u.getdomaininfo.max_vcpu_id;
     _hvm_guest = (domctl.u.getdomaininfo.flags & XEN_DOMINF_hvm_guest);
+    _pvh_guest = (domctl.u.getdomaininfo.flags & XEN_DOMINF_pvh_guest);
     return _max_vcpu_id;
 }
 
@@ -369,7 +371,7 @@ _change_TF(vcpuid_t which_vcpu, int guest_bitness, int setit)
     int sz = sizeof(anyc);
 
     /* first try the MTF for hvm guest. otherwise do manually */
-    if (_hvm_guest) {
+    if (_hvm_guest || _pvh_guest) {
         domctl.u.debug_op.vcpu = which_vcpu;
         domctl.u.debug_op.op = setit ? XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON :
                                        XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF;
-- 
1.7.2.3

Re: [V1 PATCH 1/5] PVH xen tools: Elf and header change.

[Jan Beulich]

 >>> On 05.09.13 at 03:32, Mukesh Rathor <mukesh.rathor@oracle.com> wrote:
> Add XENFEAT_hvm_callback_vector to elf_xen_feature_names
>  so we can ensure the kernel supports all features requird for 
>  PVH mode. Note, hvm callback is required for PVH. 
>  Also, introduce _XEN_DOMINF_pvh_guest.

I suppose you lumped these together just because they're both
outside the tools tree? The domctl change clearly belongs in the
hypervisor series. Whether the libelf change belongs in the tools
or hypervisor series you know. In any event, no matter how
small the constituent parts, these changes continue your model
of randomly mixing changes together rather than structuring
things in a logical way. Once we get to see George's rework of
your hypervisor series, it'll become clear whether allowing the
change here to be left as is would be just one more exception
we make for your stuff to finally make it in, or whether we can
rather get something committed that's well structured.

Jan

> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> ---
>  xen/common/libelf/libelf-dominfo.c |    1 +
>  xen/include/public/domctl.h        |    3 +++
>  2 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/xen/common/libelf/libelf-dominfo.c 
> b/xen/common/libelf/libelf-dominfo.c
> index e79b339..25dd892 100644
> --- a/xen/common/libelf/libelf-dominfo.c
> +++ b/xen/common/libelf/libelf-dominfo.c
> @@ -27,6 +27,7 @@ static const char *const elf_xen_feature_names[] = {
>      [XENFEAT_auto_translated_physmap] = "auto_translated_physmap",
>      [XENFEAT_supervisor_mode_kernel] = "supervisor_mode_kernel",
>      [XENFEAT_pae_pgdir_above_4gb] = "pae_pgdir_above_4gb",
> +    [XENFEAT_hvm_callback_vector] = "hvm_callback_vector",
>      [XENFEAT_dom0] = "dom0"
>  };
>  static const unsigned elf_xen_features =
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 4c5b2bb..6b1aa11 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -89,6 +89,9 @@ struct xen_domctl_getdomaininfo {
>   /* Being debugged.  */
>  #define _XEN_DOMINF_debugged  6
>  #define XEN_DOMINF_debugged   (1U<<_XEN_DOMINF_debugged)
> +/* domain is PVH */
> +#define _XEN_DOMINF_pvh_guest 7
> +#define XEN_DOMINF_pvh_guest   (1U<<_XEN_DOMINF_pvh_guest)
>   /* XEN_DOMINF_shutdown guest-supplied code.  */
>  #define XEN_DOMINF_shutdownmask 255
>  #define XEN_DOMINF_shutdownshift 16
> -- 
> 1.7.2.3

Re: [V1 PATCH 3/5] PVH xen tools: libxl changes to create a PVH guest.

[Ian Campbell]

On Wed, 2013-09-04 at 18:32 -0700, Mukesh Rathor wrote:
> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> ---
>  tools/libxl/libxl_create.c   |   12 ++++++++++++
>  tools/libxl/libxl_dom.c      |    5 ++++-
>  tools/libxl/libxl_internal.h |    1 +
>  tools/libxl/libxl_types.idl  |    1 +
>  tools/libxl/libxl_x86.c      |    4 +++-
>  tools/libxl/xl_cmdimpl.c     |    1 +

Please also modify libxl.h to add a suitable LIBXL_HAVE_xxx #define as
described in the big comment about API compatibility.

>  6 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 0c32d0b..0dfc4b7 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, false);

Make this:
	   libxl_defbool_setdefault(&c_info->hap, libxl_defbool_val(c_info->pvh);
so that HAP defaults on if PVH is on.

>      }
>  
>      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) ) {
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "HAP must be on for PVH");

Please use LOG(ERROR, "...") in new code.

> +            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..d84313f 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,
> @@ -400,7 +402,8 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
>          LOGE(ERROR, "xc_dom_boot_image failed");
>          goto out;
>      }
> -    if ( (ret = xc_dom_gnttab_init(dom)) != 0 ) {
> +    /* PVH sets up its own grant during boot via hvm mechanisms */
> +    if ( !dom->pvh_enabled && (ret = xc_dom_gnttab_init(dom)) != 0 ) {

I think xc_dom_gnttab_init should silently return if dom->pvh_enabled.

>          LOGE(ERROR, "xc_dom_gnttab_init failed");
>          goto out;
>      }
> 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 85341a0..9919786 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -248,6 +248,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)) {

Re: [V1 PATCH 4/5] PVH xen tools: xenstored changes

[Ian Campbell]

On Wed, 2013-09-04 at 18:32 -0700, Mukesh Rathor wrote:
>  Allow map_interface in xenstored to revert to the old
>  xc_map_foreign_range if xc_gnttab_map_grant_ref is unable
>  to map the mfn because the guest is auto translate.

Please can you describe here when/why this might be the case.

Also you change is messing up the indentation. xenstored uses hard tabs.

> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> ---
>  tools/xenstore/xenstored_domain.c |   12 +++++++-----
>  1 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
> index bf83d58..10c23a1 100644
> --- a/tools/xenstore/xenstored_domain.c
> +++ b/tools/xenstore/xenstored_domain.c
> @@ -168,13 +168,15 @@ static int readchn(struct connection *conn, void *data, unsigned int len)
>  static void *map_interface(domid_t domid, unsigned long mfn)
>  {
>  	if (*xcg_handle != NULL) {
> -		/* this is the preferred method */
> -		return xc_gnttab_map_grant_ref(*xcg_handle, domid,
> +                void *addr;
> +                /* this is the preferred method */
> +                addr = xc_gnttab_map_grant_ref(*xcg_handle, domid,
>  			GNTTAB_RESERVED_XENSTORE, PROT_READ|PROT_WRITE);
> -	} else {
> -		return xc_map_foreign_range(*xc_handle, domid,
> -			getpagesize(), PROT_READ|PROT_WRITE, mfn);
> +                if (addr)
> +                        return addr;
>  	}
> +	return xc_map_foreign_range(*xc_handle, domid,
> +		        getpagesize(), PROT_READ|PROT_WRITE, mfn);
>  }
>  
>  static void unmap_interface(void *interface)

Re: [V1 PATCH 5/5] PVH xen tools: misc changes

[Ian Campbell]

On Wed, 2013-09-04 at 18:32 -0700, Mukesh Rathor wrote:
>   - Introduce PVH guest type for gdbsx so it can be used to
>     debug a PVH guest.
>   - xl man page change.
> 
> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> ---
>  docs/man/xl.cfg.pod.5             |    3 +++

This belongs in the patch to xl which introduces the option.

Which then solves the problem of you arbitrarily grouping unrelated
changes into a single patch...

>  tools/debugger/gdbsx/xg/xg_main.c |    4 +++-
>  2 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 08d6cc4..4c925c9 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 guest in an HVM container. Default is 0.
> +
>  =back
>  
>  =head2 Fully-virtualised (HVM) Guest Specific Options
> diff --git a/tools/debugger/gdbsx/xg/xg_main.c b/tools/debugger/gdbsx/xg/xg_main.c
> index 64c7484..5736b86 100644
> --- a/tools/debugger/gdbsx/xg/xg_main.c
> +++ b/tools/debugger/gdbsx/xg/xg_main.c
> @@ -81,6 +81,7 @@ int xgtrc_on = 0;
>  struct xen_domctl domctl;         /* just use a global domctl */
>  
>  static int     _hvm_guest;        /* hvm guest? 32bit HVMs have 64bit context */
> +static int     _pvh_guest;        /* PV guest in HVM container */
>  static domid_t _dom_id;           /* guest domid */
>  static int     _max_vcpu_id;      /* thus max_vcpu_id+1 VCPUs */
>  static int     _dom0_fd;          /* fd of /dev/privcmd */
> @@ -309,6 +310,7 @@ xg_attach(int domid, int guest_bitness)
>  
>      _max_vcpu_id = domctl.u.getdomaininfo.max_vcpu_id;
>      _hvm_guest = (domctl.u.getdomaininfo.flags & XEN_DOMINF_hvm_guest);
> +    _pvh_guest = (domctl.u.getdomaininfo.flags & XEN_DOMINF_pvh_guest);
>      return _max_vcpu_id;
>  }
>  
> @@ -369,7 +371,7 @@ _change_TF(vcpuid_t which_vcpu, int guest_bitness, int setit)
>      int sz = sizeof(anyc);
>  
>      /* first try the MTF for hvm guest. otherwise do manually */
> -    if (_hvm_guest) {
> +    if (_hvm_guest || _pvh_guest) {
>          domctl.u.debug_op.vcpu = which_vcpu;
>          domctl.u.debug_op.op = setit ? XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON :
>                                         XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF;

Re: [V1 PATCH 2/5] PVH xen tools: libxc changes to build a PVH guest.

[Ian Campbell]

On Wed, 2013-09-04 at 18:32 -0700, Mukesh Rathor wrote:
> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> ---
>  tools/libxc/xc_dom.h      |    1 +
>  tools/libxc/xc_dom_core.c |    8 ++++
>  tools/libxc/xc_dom_x86.c  |   83 ++++++++++++++++++++++++++++-----------------
>  3 files changed, 61 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..6ebe7ec 100644
> --- a/tools/libxc/xc_dom_core.c
> +++ b/tools/libxc/xc_dom_core.c
> @@ -766,6 +766,14 @@ int xc_dom_parse_image(struct xc_dom_image *dom)
>          goto err;
>      }
>  
> +    if ( dom->pvh_enabled )
> +    {
> +        char *pvh_features = "writable_descriptor_tables|"

const char or just inline in the call please.

> +                             "auto_translated_physmap|supervisor_mode_kernel|"
> +                             "hvm_callback_vector";
> +        elf_xen_parse_features(pvh_features, dom->f_requested, NULL);

I suppose this doesn't nobble anything which the user passed in which
was previously parsed into dom->f_requested?

In any case this is now internal to the libxc API so we can change it to
fix it as we please.

> +    }
> +
>      /* 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..5b2d61d 100644
> --- a/tools/libxc/xc_dom_x86.c
> +++ b/tools/libxc/xc_dom_x86.c

I don't seem to see anything which would reject PVH on x86_32 at the
moment?

> @@ -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) )
> @@ -629,12 +630,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 +637,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 +756,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 +832,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 +902,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 +915,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;
>      }
>  

Re: [V1 PATCH 2/5] PVH xen tools: libxc changes to build a PVH guest.

[Mukesh Rathor]

On Thu, 5 Sep 2013 11:22:05 +0100
Ian Campbell <Ian.Campbell@citrix.com> wrote:

> On Wed, 2013-09-04 at 18:32 -0700, Mukesh Rathor wrote:
> > Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
...
> >      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..5b2d61d 100644
> > --- a/tools/libxc/xc_dom_x86.c
> > +++ b/tools/libxc/xc_dom_x86.c
> 
> I don't seem to see anything which would reject PVH on x86_32 at the
> moment?

32bit kernel wouldn't have the elf features so it would be rejected. 
Also, the hypervisor has checks for 32bit guest, and will fail one of
the hcalls used to build the guest.

thanks
Mukesh

Re: [V1 PATCH 3/5] PVH xen tools: libxl changes to create a PVH guest.

[Mukesh Rathor]

On Thu, 5 Sep 2013 11:03:01 +0100
Ian Campbell <Ian.Campbell@citrix.com> wrote:

> On Wed, 2013-09-04 at 18:32 -0700, Mukesh Rathor wrote:
> > Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> > ---
> >  tools/libxl/libxl_create.c   |   12 ++++++++++++
> >  tools/libxl/libxl_dom.c      |    5 ++++-
> >  tools/libxl/libxl_internal.h |    1 +
> >  tools/libxl/libxl_types.idl  |    1 +
> >  tools/libxl/libxl_x86.c      |    4 +++-
> >  tools/libxl/xl_cmdimpl.c     |    1 +
> 
> Please also modify libxl.h to add a suitable LIBXL_HAVE_xxx #define as
> described in the big comment about API compatibility.

Ah, not sure whey LIBXL_API_VERSION can't be used in this case. But, 
anyways, maybe something like:

/*
 * LIBXL_HAVE_PVH_SUPPORT or LIBXL_HAVE_PVH_EXTENSIONS or 
 *     LIBXL_HAVE_PVH_FEATURE.... 
 *
 * If this is defined, the library supports PVH extensions for building a
 * PV guest to run in HVM container. This is done by adding pvh=1 in the
 * config file.
 */
#define LIBXL_HAVE_PVH_SUPPORT 1


thanks
Mukesh

Re: [V1 PATCH 2/5] PVH xen tools: libxc changes to build a PVH guest.

[Ian Campbell]

On Thu, 2013-09-05 at 18:12 -0700, Mukesh Rathor wrote:
> On Thu, 5 Sep 2013 11:22:05 +0100
> Ian Campbell <Ian.Campbell@citrix.com> wrote:
> 
> > On Wed, 2013-09-04 at 18:32 -0700, Mukesh Rathor wrote:
> > > Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> ...
> > >      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..5b2d61d 100644
> > > --- a/tools/libxc/xc_dom_x86.c
> > > +++ b/tools/libxc/xc_dom_x86.c
> > 
> > I don't seem to see anything which would reject PVH on x86_32 at the
> > moment?
> 
> 32bit kernel wouldn't have the elf features so it would be rejected.

A RHEL5 i686 kernel-xen might actual implement all of these features, I
think they did at one point?

> Also, the hypervisor has checks for 32bit guest, and will fail one of
> the hcalls used to build the guest.

Great. Will the error message be comprehensible? It would be a one line
check and error message in vcpu_x86_32 if not.

Ian.

Re: [V1 PATCH 3/5] PVH xen tools: libxl changes to create a PVH guest.

[Ian Campbell]

On Thu, 2013-09-05 at 18:23 -0700, Mukesh Rathor wrote:
> On Thu, 5 Sep 2013 11:03:01 +0100
> Ian Campbell <Ian.Campbell@citrix.com> wrote:
> 
> > On Wed, 2013-09-04 at 18:32 -0700, Mukesh Rathor wrote:
> > > Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> > > ---
> > >  tools/libxl/libxl_create.c   |   12 ++++++++++++
> > >  tools/libxl/libxl_dom.c      |    5 ++++-
> > >  tools/libxl/libxl_internal.h |    1 +
> > >  tools/libxl/libxl_types.idl  |    1 +
> > >  tools/libxl/libxl_x86.c      |    4 +++-
> > >  tools/libxl/xl_cmdimpl.c     |    1 +
> > 
> > Please also modify libxl.h to add a suitable LIBXL_HAVE_xxx #define as
> > described in the big comment about API compatibility.
> 
> Ah, not sure whey LIBXL_API_VERSION can't be used in this case.

They are somewhat orthogonal mechanisms.

>  But, 
> anyways, maybe something like:
> 
> /*
>  * LIBXL_HAVE_PVH_SUPPORT or LIBXL_HAVE_PVH_EXTENSIONS or 
>  *     LIBXL_HAVE_PVH_FEATURE.... 
>  *
>  * If this is defined, the library supports PVH extensions for building a
>  * PV guest to run in HVM container. This is done by adding pvh=1 in the
>  * config file.
>  */
> #define LIBXL_HAVE_PVH_SUPPORT 1

We have a bunch of LIBXL_HAVE_BUILDINFO_FOO, how about
LIBXL_HAVE_CREATEINFO_PVH ? (assuming I'm remembering correctly that
this is where you added the field...)

Ian.

Re: [V1 PATCH 4/5] PVH xen tools: xenstored changes

[Mukesh Rathor]

On Thu, 5 Sep 2013 11:04:53 +0100
Ian Campbell <Ian.Campbell@citrix.com> wrote:

> On Wed, 2013-09-04 at 18:32 -0700, Mukesh Rathor wrote:
> >  Allow map_interface in xenstored to revert to the old
> >  xc_map_foreign_range if xc_gnttab_map_grant_ref is unable
> >  to map the mfn because the guest is auto translate.
> 
> Please can you describe here when/why this might be the case.

http://lists.xen.org/archives/html/xen-devel/2012-07/msg00130.html:
In Steffano's words, 

"xenstored is going to try to open the grant table interface
(xc_gnttab_open, implemented via gntdev), if that fails it should fall
back to xc_map_foreign_range. That should work out of the box, or at
least it does for my "hybrid" ARM dom0."

I'll put it in the commit message.

thanks
mukesh

Re: [V1 PATCH 2/5] PVH xen tools: libxc changes to build a PVH guest.

[Mukesh Rathor]

On Fri, 6 Sep 2013 10:29:02 +0100
Ian Campbell <Ian.Campbell@citrix.com> wrote:

> On Thu, 2013-09-05 at 18:12 -0700, Mukesh Rathor wrote:
> > On Thu, 5 Sep 2013 11:22:05 +0100
> > Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > 
> > > On Wed, 2013-09-04 at 18:32 -0700, Mukesh Rathor wrote:
> > > > Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> > ...
> > > >      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..5b2d61d 100644
> > > > --- a/tools/libxc/xc_dom_x86.c
> > > > +++ b/tools/libxc/xc_dom_x86.c
> > > 
> > > I don't seem to see anything which would reject PVH on x86_32 at
> > > the moment?
> > 
> > 32bit kernel wouldn't have the elf features so it would be rejected.
> 
> A RHEL5 i686 kernel-xen might actual implement all of these features,
> I think they did at one point?

Already discovered that, all except hvm callback vector.

> > Also, the hypervisor has checks for 32bit guest, and will fail one
> > of the hcalls used to build the guest.
> 
> Great. Will the error message be comprehensible? It would be a one
> line check and error message in vcpu_x86_32 if not.

Ok, added anyways.

thanks
mukesh