Mailing List Archive

PVH: domain creations...
Hi guys,

Good news, I got everything working on my refreshed tree. So, now time
to clean up the patch, look at the fixme's/TBDs to see what needs to be
done in phase I and what can be deferred. Lets start with domain
creation:

I currently have following for domain creation:
dom0: dom0pvh in the command line makes it PVH mode. IMO, we should leave
this as is until PVH is default.

domU: vm.cfg has line: is_pvh = 1. The tools then parse this and set
is_pvh fields in libxl_domain_create_info{} and
libxl_domain_build_info{}. I also introduced XEN_DOMCTL_CDF_pvh_guest
flag to send down the info to xen during domU creation.


1. I suppose in XEN_DOMCTL_createdomain in xen: I could just see if
it's PV guest with HAP, ie,
if (~XEN_DOMCTL_CDF_hvm_guest && XEN_DOMCTL_CDF_hap)
then domcr_flags |= DOMCRF_pvh;

This would result in elimination of XEN_DOMCTL_CDF_pvh_guest flag.
What do you think?

2. I suppose I could undo flags in libxl_domain_create_info and
libxl_domain_build_info, and just call xc_dom_feature_translated()
and assume it's PVH if it returns true for PV?


Also, I realized SIF_IS_PVH is probably not needed and I'm trying to
see if I can remove it.

Attaching user mode change diffs.

thanks,
Mukesh


diff -r 8b0762504037 tools/libxc/xc_dom_x86.c
--- a/tools/libxc/xc_dom_x86.c Wed Oct 31 16:08:55 2012 -0700
+++ b/tools/libxc/xc_dom_x86.c Tue Nov 20 15:58:21 2012 -0800
@@ -386,7 +386,8 @@ static int setup_pgtables_x86_64(struct
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->flags & SIF_IS_PVH) &&
+ (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) )
@@ -716,7 +717,7 @@ int arch_setup_meminit(struct xc_dom_ima
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->flags & SIF_IS_PVH) )
{
dom->shadow_enabled = 1;
rc = x86_shadow(dom->xch, dom->guest_domid);
@@ -830,7 +831,7 @@ int arch_setup_bootlate(struct xc_dom_im
}

/* Map grant table frames into guest physmap. */
- for ( i = 0; ; i++ )
+ for ( i = 0; !(dom->flags & SIF_IS_PVH); i++ )
{
rc = xc_domain_add_to_physmap(dom->xch, dom->guest_domid,
XENMAPSPACE_grant_table,
diff -r 8b0762504037 tools/libxc/xc_domain_restore.c
--- a/tools/libxc/xc_domain_restore.c Wed Oct 31 16:08:55 2012 -0700
+++ b/tools/libxc/xc_domain_restore.c Tue Nov 20 15:58:21 2012 -0800
@@ -1933,15 +1933,15 @@ int xc_domain_restore(xc_interface *xch,
munmap(start_info, PAGE_SIZE);
}
/* Uncanonicalise each GDT frame number. */
- if ( GET_FIELD(ctxt, gdt_ents) > 8192 )
+ if ( GET_FIELD(ctxt, u.pv.gdt_ents) > 8192 )
{
ERROR("GDT entry count out of range");
goto out;
}

- for ( j = 0; (512*j) < GET_FIELD(ctxt, gdt_ents); j++ )
+ for ( j = 0; (512*j) < GET_FIELD(ctxt, u.pv.gdt_ents); j++ )
{
- pfn = GET_FIELD(ctxt, gdt_frames[j]);
+ pfn = GET_FIELD(ctxt, u.pv.gdt_frames[j]);
if ( (pfn >= dinfo->p2m_size) ||
(pfn_type[pfn] != XEN_DOMCTL_PFINFO_NOTAB) )
{
@@ -1949,7 +1949,7 @@ int xc_domain_restore(xc_interface *xch,
j, (unsigned long)pfn);
goto out;
}
- SET_FIELD(ctxt, gdt_frames[j], ctx->p2m[pfn]);
+ SET_FIELD(ctxt, u.pv.gdt_frames[j], ctx->p2m[pfn]);
}
/* Uncanonicalise the page table base pointer. */
pfn = UNFOLD_CR3(GET_FIELD(ctxt, ctrlreg[3]));
diff -r 8b0762504037 tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c Wed Oct 31 16:08:55 2012 -0700
+++ b/tools/libxc/xc_domain_save.c Tue Nov 20 15:58:21 2012 -0800
@@ -1889,15 +1889,15 @@ int xc_domain_save(xc_interface *xch, in
}

/* Canonicalise each GDT frame number. */
- for ( j = 0; (512*j) < GET_FIELD(&ctxt, gdt_ents); j++ )
+ for ( j = 0; (512*j) < GET_FIELD(&ctxt, u.pv.gdt_ents); j++ )
{
- mfn = GET_FIELD(&ctxt, gdt_frames[j]);
+ mfn = GET_FIELD(&ctxt, u.pv.gdt_frames[j]);
if ( !MFN_IS_IN_PSEUDOPHYS_MAP(mfn) )
{
ERROR("GDT frame is not in range of pseudophys map");
goto out;
}
- SET_FIELD(&ctxt, gdt_frames[j], mfn_to_pfn(mfn));
+ SET_FIELD(&ctxt, u.pv.gdt_frames[j], mfn_to_pfn(mfn));
}

/* Canonicalise the page table base pointer. */
diff -r 8b0762504037 tools/libxl/Makefile
--- a/tools/libxl/Makefile Wed Oct 31 16:08:55 2012 -0700
+++ b/tools/libxl/Makefile Tue Nov 20 15:58:21 2012 -0800
@@ -13,7 +13,7 @@ XLUMINOR = 1

CFLAGS += -Werror -Wno-format-zero-length -Wmissing-declarations \
-Wno-declaration-after-statement -Wformat-nonliteral
-CFLAGS += -I. -fPIC
+CFLAGS += -I. -fPIC -O0

ifeq ($(CONFIG_Linux),y)
LIBUUID_LIBS += -luuid
diff -r 8b0762504037 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Wed Oct 31 16:08:55 2012 -0700
+++ b/tools/libxl/libxl_create.c Tue Nov 20 15:58:21 2012 -0800
@@ -409,6 +409,8 @@ int libxl__domain_make(libxl__gc *gc, li
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->ci_pvh) ) {
+ flags |= (XEN_DOMCTL_CDF_pvh_guest | XEN_DOMCTL_CDF_hap);
}
*domid = -1;

diff -r 8b0762504037 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c Wed Oct 31 16:08:55 2012 -0700
+++ b/tools/libxl/libxl_dom.c Tue Nov 20 15:58:21 2012 -0800
@@ -270,7 +270,8 @@ int libxl__build_pre(libxl__gc *gc, uint
if (rtc_timeoffset)
xc_domain_set_time_offset(ctx->xch, domid, rtc_timeoffset);

- if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
+ if (info->type == LIBXL_DOMAIN_TYPE_HVM ||
+ libxl_defbool_val(info->bi_pvh) ) {
unsigned long shadow;
shadow = (info->shadow_memkb + 1023) / 1024;
xc_shadow_control(ctx->xch, domid, XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION, NULL, 0, &shadow, 0, NULL);
@@ -368,9 +369,15 @@ int libxl__build_pv(libxl__gc *gc, uint3
struct xc_dom_image *dom;
int ret;
int flags = 0;
+ int is_pvh = libxl_defbool_val(info->bi_pvh);

xc_dom_loginit(ctx->xch);

+ if (is_pvh) {
+ printf("info..features:%s\n", info->u.pv.features);
+ info->u.pv.features = strdup("auto_translated_physmap");
+ }
+
dom = xc_dom_allocate(ctx->xch, state->pv_cmdline, info->u.pv.features);
if (!dom) {
LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xc_dom_allocate failed");
@@ -408,6 +415,7 @@ int libxl__build_pv(libxl__gc *gc, uint3
}
}

+ flags |= is_pvh ? SIF_IS_PVH : 0;
dom->flags = flags;
dom->console_evtchn = state->console_port;
dom->console_domid = state->console_domid;
@@ -438,7 +446,8 @@ int libxl__build_pv(libxl__gc *gc, uint3
LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_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 ( !is_pvh && (ret = xc_dom_gnttab_init(dom)) != 0 ) {
LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xc_dom_gnttab_init failed");
goto out;
}
diff -r 8b0762504037 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Wed Oct 31 16:08:55 2012 -0700
+++ b/tools/libxl/libxl_types.idl Tue Nov 20 15:58:21 2012 -0800
@@ -243,6 +243,7 @@ libxl_domain_create_info = Struct("domai
("platformdata", libxl_key_value_list),
("poolid", uint32),
("run_hotplug_scripts",libxl_defbool),
+ ("ci_pvh", libxl_defbool),
], dir=DIR_IN)

MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
@@ -339,6 +340,7 @@ libxl_domain_build_info = Struct("domain
])),
("invalid", Struct(None, [])),
], keyvar_init_val = "LIBXL_DOMAIN_TYPE_INVALID")),
+ ("bi_pvh", libxl_defbool),
], dir=DIR_IN
)

diff -r 8b0762504037 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Wed Oct 31 16:08:55 2012 -0700
+++ b/tools/libxl/xl_cmdimpl.c Tue Nov 20 15:58:21 2012 -0800
@@ -615,8 +615,18 @@ static void parse_config_data(const char
!strncmp(buf, "hvm", strlen(buf)))
c_info->type = LIBXL_DOMAIN_TYPE_HVM;

+ libxl_defbool_setdefault(&c_info->ci_pvh, false);
+ libxl_defbool_setdefault(&c_info->hap, false);
+ xlu_cfg_get_defbool(config, "pvh", &c_info->ci_pvh, 0);
xlu_cfg_get_defbool(config, "hap", &c_info->hap, 0);

+ if (libxl_defbool_val(c_info->ci_pvh) &&
+ !libxl_defbool_val(c_info->hap)) {
+
+ fprintf(stderr, "hap is required for PVH domain\n");
+ exit(1);
+ }
+
if (xlu_cfg_replace_string (config, "name", &c_info->name, 0)) {
fprintf(stderr, "Domain name must be specified.\n");
exit(1);
@@ -916,6 +926,7 @@ static void parse_config_data(const char

b_info->u.pv.cmdline = cmdline;
xlu_cfg_replace_string (config, "ramdisk", &b_info->u.pv.ramdisk, 0);
+ libxl_defbool_set(&b_info->bi_pvh, libxl_defbool_val(c_info->ci_pvh));
break;
}
default:

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: PVH: domain creations... [ In reply to ]
>>> On 21.11.12 at 03:26, Mukesh Rathor <mukesh.rathor@oracle.com> wrote:
> 1. I suppose in XEN_DOMCTL_createdomain in xen: I could just see if
> it's PV guest with HAP, ie,
> if (~XEN_DOMCTL_CDF_hvm_guest && XEN_DOMCTL_CDF_hap)
> then domcr_flags |= DOMCRF_pvh;
>
> This would result in elimination of XEN_DOMCTL_CDF_pvh_guest flag.
> What do you think?

Sounds plausible to me.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: PVH: domain creations... [ In reply to ]
On Wed, 2012-11-21 at 02:26 +0000, Mukesh Rathor wrote:
> Hi guys,
>
> Good news, I got everything working on my refreshed tree. So, now time
> to clean up the patch, look at the fixme's/TBDs to see what needs to be
> done in phase I and what can be deferred. Lets start with domain
> creation:
>
> I currently have following for domain creation:
> dom0: dom0pvh in the command line makes it PVH mode. IMO, we should leave
> this as is until PVH is default.
>
> domU: vm.cfg has line: is_pvh = 1. The tools then parse this and set
> is_pvh fields in libxl_domain_create_info{} and
> libxl_domain_build_info{}. I also introduced XEN_DOMCTL_CDF_pvh_guest
> flag to send down the info to xen during domU creation.
>
>
> 1. I suppose in XEN_DOMCTL_createdomain in xen: I could just see if
> it's PV guest with HAP, ie,
> if (~XEN_DOMCTL_CDF_hvm_guest && XEN_DOMCTL_CDF_hap)
> then domcr_flags |= DOMCRF_pvh;
>
> This would result in elimination of XEN_DOMCTL_CDF_pvh_guest flag.
> What do you think?

Seems to make sense. And this interface is a hyp<=>tools one so we can
always fix it or change it if necessary.

(one thought for the future would be to include the kernels feature
flags in struct xen_domctl_createdomain, for example)

> 2. I suppose I could undo flags in libxl_domain_create_info and
> libxl_domain_build_info, and just call xc_dom_feature_translated()
> and assume it's PVH if it returns true for PV?

I think you actually want both, i.e. libxl/libxc should provide a
sensible default (perhaps based on xc_dom_feature_translated as you
suggest) but you also want a way for the user to forcibly enable or
disable it in the guest config (to workaround bugs, allow comparative
benchmarks etc).

> Also, I realized SIF_IS_PVH is probably not needed and I'm trying to
> see if I can remove it.

Cool.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel