From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mukesh Rathor Subject: Re: [PATCH 08/18] PVH xen: tools changes to create PVH domain Date: Tue, 30 Jul 2013 16:47:16 -0700 Message-ID: <20130730164716.10969419@mantra.us.oracle.com> References: <1369445137-19755-1-git-send-email-mukesh.rathor@oracle.com> <1369445137-19755-9-git-send-email-mukesh.rathor@oracle.com> <1371049088.24512.450.camel@zakaz.uk.xensource.com> <20130614171437.49f55cea@mantra.us.oracle.com> <1371467494.23802.49.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1371467494.23802.49.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: "Xen-devel@lists.xensource.com" , Ian Jackson List-Id: xen-devel@lists.xenproject.org On Mon, 17 Jun 2013 12:11:34 +0100 Ian Campbell wrote: > On Fri, 2013-06-14 at 17:14 -0700, Mukesh Rathor wrote: .... > > > > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > > > > index b38d0a7..cefbf76 100644 > > > > --- a/tools/libxl/libxl_dom.c > > > > +++ b/tools/libxl/libxl_dom.c > > > > @@ -329,9 +329,23 @@ int libxl__build_pv(libxl__gc *gc, uint32_t > > > > domid, struct xc_dom_image *dom; > > > > int ret; > > > > int flags = 0; > > > > + int is_pvh = libxl_defbool_val(info->pvh); > > > > > > > > xc_dom_loginit(ctx->xch); > > > > > > > > + if (is_pvh) { > > > > + char *pv_feats = > > > > "writable_descriptor_tables|auto_translated_physmap" > > > > + > > > > "|supervisor_mode_kernel|hvm_callback_vector"; + > > > > + if (info->u.pv.features && info->u.pv.features[0] != > > > > '\0') > > > > + { > > > > + LOG(ERROR, "Didn't expect info->u.pv.features to > > > > contain string\n"); > > > > + LOG(ERROR, "String: %s\n", info->u.pv.features); > > > > + return ERROR_FAIL; > > > > + } > > > > + info->u.pv.features = strdup(pv_feats); > > > > > > What is this trying to achieve? I think the requirement for > > > certain features to be present if pvh is enabled needs to be > > > handled in the xc_dom library and not here. This field is (I > > > think) for the user to specify other features which they may wish > > > to require. > > > > I had asked for assitance on this long ago. But anyways, basically > > here I want to make sure the kernel has all those features because > > the user has asked a PVH guest must be created (by pvh=1 in vm.cfg > > file). Can you kindly advise the best way to do this? > > This should be done in xc_dom build stuff not in libxl. Basically > libxl should call xc_dom_foo with a kernel and pvh=yes (or > =ifpossible) and the builder is then responsible internally for > knowing which features are therefore required from the kernel. Alright, I'm still not able to figure this out. I was able to instrument libraries to figure what goes on for PV. But, I see for PV both dom->f_requested and dom->parms.f_required is null in xc_dom_parse_image(). Also, in the same function dom->parms.f_supported is checked, but I can't tell greping for f_supported where it's set! I am using xl and not xm, so don't care what xm/python is setting. I was expecting to see some features for PV set in those strings. It looks like elf_xen_parse_features sets the feature bits, but it's not being called for PV in xc_dom_allocate() because features parameter is null. So, that brings me back to setting the feature string somewhere before xc_dom_allocate() is called. I'm at a loss where to set it? The feature string should be set to following for pvh when xc_dom_allocate() is called: "writable_descriptor_tables|auto_translated_physmap" "|supervisor_mode_kernel|hvm_callback_vector and if the kernel elf doesn't provide those features, the create should fail. libxl__build_pv calls xc_dom_allocate(), but you don't want me to add it in libxl. I can just put the damn thing in xc_dom_allocate() if features is NULL, or re-malloc if feature is not NULL for pvh domain? thanks mukesh