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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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@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.

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.


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