Mukesh Rathor
2013-Sep-07 00:28 UTC
[V2 PATCH 0/5]: PVH xen tools: tools changes to create PVH guest
Please find V2 tools changes for PVH. These were built on unstable c/s: 8a7769b4453168e23e8935a85e9a875ef5117253. I''ve dropped the gdbsx and realeted domctl change for now. I''ll submit after the xen changes have gone in. 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
Mukesh Rathor
2013-Sep-07 00:28 UTC
[V2 PATCH 1/4] PVH xen tools: In Elf code, add hvm callback vector
Add XENFEAT_hvm_callback_vector to elf_xen_feature_names
so we can ensure the kernel supports all features required for
PVH mode when building a PVH domU here. Note, hvm callback is
required for PVH.
Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
xen/common/libelf/libelf-dominfo.c | 1 +
1 files changed, 1 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 --
1.7.2.3
Mukesh Rathor
2013-Sep-07 00:28 UTC
[V2 PATCH 2/4] PVH xen tools: libxc changes to build a PVH guest.
V2: Make pvh_features string const, and fail 32bit PVH guest creation.
Move PVH check to xc_dom_gnttab_init().
Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
tools/libxc/xc_dom.h | 1 +
tools/libxc/xc_dom_boot.c | 4 ++
tools/libxc/xc_dom_core.c | 9 ++++
tools/libxc/xc_dom_x86.c | 90 +++++++++++++++++++++++++++++---------------
4 files changed, 73 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_boot.c b/tools/libxc/xc_dom_boot.c
index d4d57b4..73032a1 100644
--- a/tools/libxc/xc_dom_boot.c
+++ b/tools/libxc/xc_dom_boot.c
@@ -416,6 +416,10 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t
domid,
int xc_dom_gnttab_init(struct xc_dom_image *dom)
{
+ /* PVH sets up its own grant during boot via hvm mechanisms */
+ if ( dom->pvh_enabled )
+ return 0;
+
if ( xc_dom_feature_translated(dom) ) {
return xc_dom_gnttab_hvm_seed(dom->xch, dom->guest_domid,
dom->console_pfn,
dom->xenstore_pfn,
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
Mukesh Rathor
2013-Sep-07 00:28 UTC
[V2 PATCH 3/4] PVH xen tools: libxl changes to create a PVH guest.
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.
Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
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..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/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 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
Allow map_interface in xenstored to revert to the
xc_map_foreign_range if xc_gnttab_map_grant_ref is unable
to map the mfn because grant is not yet setup for a PVH guest.
PVH sets it up during boot via HVM mechanism.
Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
tools/xenstore/xenstored_domain.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/tools/xenstore/xenstored_domain.c
b/tools/xenstore/xenstored_domain.c
index bf83d58..4a89831 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) {
+ void *addr;
/* this is the preferred method */
- return xc_gnttab_map_grant_ref(*xcg_handle, domid,
+ 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
Ian Campbell
2013-Sep-09 08:33 UTC
Re: [V2 PATCH 1/4] PVH xen tools: In Elf code, add hvm callback vector
On Fri, 2013-09-06 at 17:28 -0700, Mukesh Rathor wrote:> Add XENFEAT_hvm_callback_vector to elf_xen_feature_names > so we can ensure the kernel supports all features required for > PVH mode when building a PVH domU here. Note, hvm callback is > required for PVH. > > Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>> --- > xen/common/libelf/libelf-dominfo.c | 1 + > 1 files changed, 1 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 =
Ian Campbell
2013-Sep-09 08:42 UTC
Re: [V2 PATCH 2/4] PVH xen tools: libxc changes to build a PVH guest.
On Fri, 2013-09-06 at 17:28 -0700, Mukesh Rathor wrote:> V2: Make pvh_features string const, and fail 32bit PVH guest creation. > Move PVH check to xc_dom_gnttab_init().Please put these intra-version changelogs after the S-o-b and a "---" tag on a line by itself.> > Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com> > --- > tools/libxc/xc_dom.h | 1 + > tools/libxc/xc_dom_boot.c | 4 ++ > tools/libxc/xc_dom_core.c | 9 ++++ > tools/libxc/xc_dom_x86.c | 90 +++++++++++++++++++++++++++++--------------- > 4 files changed, 73 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_boot.c b/tools/libxc/xc_dom_boot.c > index d4d57b4..73032a1 100644 > --- a/tools/libxc/xc_dom_boot.c > +++ b/tools/libxc/xc_dom_boot.c > @@ -416,6 +416,10 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid, > > int xc_dom_gnttab_init(struct xc_dom_image *dom) > { > + /* PVH sets up its own grant during boot via hvm mechanisms */Last time I saw this I took this to mean that it uses the HVM init mechanisms in the toolstack, but from the context below I now see this isn''t correct and it actually defers this entirely to the guest. Why isn''t it ok to call something similar to xc_dom_gnttab_hvm_seed here? That functionality is there to support disaggregated xenstored by allowing us to preseed grant entries for the xenstore domain to access the domains xenstore ring. Likewise for the console ring. It''s failure to do this which necessitates your fourth patch, but even with that patch xenstore dmains will be broken. Note that this code fills in grant table entries but does not leave them mapped for the guest to access (it maps them temporarily and then unmaps), so calling xc_dom_gnttab_hvm_seed is not contrary to allowing the guest to setup its own grant during boot.> + if ( dom->pvh_enabled ) > + return 0; > + > if ( xc_dom_feature_translated(dom) ) { > return xc_dom_gnttab_hvm_seed(dom->xch, dom->guest_domid, > dom->console_pfn, dom->xenstore_pfn, > 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; > } >
Ian Campbell
2013-Sep-09 08:49 UTC
Re: [V2 PATCH 3/4] PVH xen tools: libxl changes to create a PVH guest.
On Fri, 2013-09-06 at 17:28 -0700, Mukesh Rathor wrote:> 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. > > Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com> > --- > 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..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.^ PV Will help stop people becoming confused and thinking this turns on HVM mode. Otherwise: Acked-by: Ian Campbell <ian.campbell@citrix.com>
Mukesh Rathor
2013-Sep-10 01:40 UTC
Re: [V2 PATCH 2/4] PVH xen tools: libxc changes to build a PVH guest.
On Mon, 9 Sep 2013 09:42:46 +0100 Ian Campbell <Ian.Campbell@citrix.com> wrote:> On Fri, 2013-09-06 at 17:28 -0700, Mukesh Rathor wrote: > > V2: Make pvh_features string const, and fail 32bit PVH guest > > creation. Move PVH check to xc_dom_gnttab_init(). > > Please put these intra-version changelogs after the S-o-b and a "---" > tag on a line by itself.Ok.> > --- 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_boot.c b/tools/libxc/xc_dom_boot.c > > index d4d57b4..73032a1 100644 > > --- a/tools/libxc/xc_dom_boot.c > > +++ b/tools/libxc/xc_dom_boot.c > > @@ -416,6 +416,10 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, > > domid_t domid, > > int xc_dom_gnttab_init(struct xc_dom_image *dom) > > { > > + /* PVH sets up its own grant during boot via hvm mechanisms */ > > Last time I saw this I took this to mean that it uses the HVM init > mechanisms in the toolstack, but from the context below I now see this > isn''t correct and it actually defers this entirely to the guest. > > Why isn''t it ok to call something similar to xc_dom_gnttab_hvm_seed > here? That functionality is there to support disaggregated xenstored > by allowing us to preseed grant entries for the xenstore domain to > access the domains xenstore ring. Likewise for the console ring. > > It''s failure to do this which necessitates your fourth patch, but even > with that patch xenstore dmains will be broken.Stefano had suggested keeping this change even if not needed, as it provides a good fallback mechanism. I can drop the patch, lmk.> Note that this code fills in grant table entries but does not leave > them mapped for the guest to access (it maps them temporarily and then > unmaps), so calling xc_dom_gnttab_hvm_seed is not contrary to allowing > the guest to setup its own grant during boot.Correct, looks like xc_dom_gnttab_hvm_seed() would work for PVH at first glance. thanks Mukesh
Mukesh Rathor
2013-Sep-10 01:40 UTC
Re: [V2 PATCH 3/4] PVH xen tools: libxl changes to create a PVH guest.
On Mon, 9 Sep 2013 09:49:39 +0100 Ian Campbell <Ian.Campbell@citrix.com> wrote:> On Fri, 2013-09-06 at 17:28 -0700, Mukesh Rathor wrote: > > 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. > > > > Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com> > > --- > > 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..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. > > ^ PV > > Will help stop people becoming confused and thinking this turns on HVM > mode.The change is under PV guest. HVM section os separate. But I can add it anyways. thanks Mukesh
Ian Campbell
2013-Sep-10 08:27 UTC
Re: [V2 PATCH 2/4] PVH xen tools: libxc changes to build a PVH guest.
On Mon, 2013-09-09 at 18:40 -0700, Mukesh Rathor wrote:> On Mon, 9 Sep 2013 09:42:46 +0100 > Ian Campbell <Ian.Campbell@citrix.com> wrote: > > > On Fri, 2013-09-06 at 17:28 -0700, Mukesh Rathor wrote: > > > V2: Make pvh_features string const, and fail 32bit PVH guest > > > creation. Move PVH check to xc_dom_gnttab_init(). > > > > Please put these intra-version changelogs after the S-o-b and a "---" > > tag on a line by itself. > > Ok. > > > > --- 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_boot.c b/tools/libxc/xc_dom_boot.c > > > index d4d57b4..73032a1 100644 > > > --- a/tools/libxc/xc_dom_boot.c > > > +++ b/tools/libxc/xc_dom_boot.c > > > @@ -416,6 +416,10 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, > > > domid_t domid, > > > int xc_dom_gnttab_init(struct xc_dom_image *dom) > > > { > > > + /* PVH sets up its own grant during boot via hvm mechanisms */ > > > > Last time I saw this I took this to mean that it uses the HVM init > > mechanisms in the toolstack, but from the context below I now see this > > isn''t correct and it actually defers this entirely to the guest. > > > > Why isn''t it ok to call something similar to xc_dom_gnttab_hvm_seed > > here? That functionality is there to support disaggregated xenstored > > by allowing us to preseed grant entries for the xenstore domain to > > access the domains xenstore ring. Likewise for the console ring. > > > > It''s failure to do this which necessitates your fourth patch, but even > > with that patch xenstore dmains will be broken. > > Stefano had suggested keeping this change even if not needed, as it > provides a good fallback mechanism. I can drop the patch, lmk.I''m in two minds. In principal xenstore and the tools are a matched pair so xenstore can rely on the tools to have done the setup. On the other hand the fallback is reasonably simple.> > > > Note that this code fills in grant table entries but does not leave > > them mapped for the guest to access (it maps them temporarily and then > > unmaps), so calling xc_dom_gnttab_hvm_seed is not contrary to allowing > > the guest to setup its own grant during boot. > > Correct, looks like xc_dom_gnttab_hvm_seed() would work for PVH at > first glance.Excellent. Ian.
Ian Campbell
2013-Sep-10 08:28 UTC
Re: [V2 PATCH 3/4] PVH xen tools: libxl changes to create a PVH guest.
On Mon, 2013-09-09 at 18:40 -0700, Mukesh Rathor wrote:> On Mon, 9 Sep 2013 09:49:39 +0100 > Ian Campbell <Ian.Campbell@citrix.com> wrote: > > > On Fri, 2013-09-06 at 17:28 -0700, Mukesh Rathor wrote: > > > 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. > > > > > > Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com> > > > --- > > > 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..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. > > > > ^ PV > > > > Will help stop people becoming confused and thinking this turns on HVM > > mode. > > The change is under PV guest. HVM section os separate.Right, those sections are pretty big, I worry that people will miss the heading and/or not read properly.> But I can addit anyways.Ta! Ian.