Re: [Xen-devel] [V2 PATCH 2/4] PVH xen tools: libxc changes to build a PVH guest.

[Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>]

On Mon, 9 Sep 2013 09:42:46 +0100
Ian Campbell <Ian.Campbell@xxxxxxxxxx> 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

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