[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to seed grant table



On Fri, Aug 04, 2017 at 01:26:21PM +0100, Wei Liu wrote:
> On Wed, Aug 02, 2017 at 10:59:49AM +0100, Paul Durrant wrote:
> > A previous patch added support for priv-mapping guest resources directly
> > (rather than having to foreign-map, which requires P2M modification for
> > HVM guests).
> > 
> > This patch makes use of the new API to seed the guest grant table unless
> > the underlying infrastructure (i.e. privcmd) doesn't support it, in which
> > case the old scheme is used.
> > 
> 
> The code mostly looks fine.
> 
> What's the benefit of doing this?

Also, I see changed signature of xc_dom_gnttab_seed (it got is_hvm
parameter). Wei, what is the policy for backward incompatible libxc API
changes?

> > NOTE: The call to xc_dom_gnttab_hvm_seed() in hvm_build_set_params() was
> >       actually unnecessary, as the grant table has already been seeded
> >       by a prior call to xc_dom_gnttab_init() made by libxl__build_dom().
> > 
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > ---
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> 
> BTW Marek needs to be CC on changes to Python bindings. I've done that
> for you.

For Python bits:
Acked-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>

> > ---
> >  tools/libxc/include/xc_dom.h        |   8 +--
> >  tools/libxc/xc_dom_boot.c           | 102 
> > ++++++++++++++++++++++++++++--------
> >  tools/libxc/xc_sr_restore_x86_hvm.c |  10 ++--
> >  tools/libxc/xc_sr_restore_x86_pv.c  |   2 +-
> >  tools/libxl/libxl_dom.c             |   1 -
> >  tools/python/xen/lowlevel/xc/xc.c   |   6 +--
> >  6 files changed, 92 insertions(+), 37 deletions(-)
> > 
> > diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> > index ce47058c41..d6ca0a8680 100644
> > --- a/tools/libxc/include/xc_dom.h
> > +++ b/tools/libxc/include/xc_dom.h
> > @@ -323,12 +323,8 @@ void *xc_dom_boot_domU_map(struct xc_dom_image *dom, 
> > xen_pfn_t pfn,
> >  int xc_dom_boot_image(struct xc_dom_image *dom);
> >  int xc_dom_compat_check(struct xc_dom_image *dom);
> >  int xc_dom_gnttab_init(struct xc_dom_image *dom);
> > -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > -                           xen_pfn_t console_gmfn,
> > -                           xen_pfn_t xenstore_gmfn,
> > -                           domid_t console_domid,
> > -                           domid_t xenstore_domid);
> > -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> > +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> > +                       bool is_hvm,
> >                         xen_pfn_t console_gmfn,
> >                         xen_pfn_t xenstore_gmfn,
> >                         domid_t console_domid,
> > diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
> > index c3b44dd399..fc3174ad7e 100644
> > --- a/tools/libxc/xc_dom_boot.c
> > +++ b/tools/libxc/xc_dom_boot.c
> > @@ -280,11 +280,11 @@ static xen_pfn_t xc_dom_gnttab_setup(xc_interface 
> > *xch, domid_t domid)
> >      return gmfn;
> >  }
> >  
> > -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> > -                       xen_pfn_t console_gmfn,
> > -                       xen_pfn_t xenstore_gmfn,
> > -                       domid_t console_domid,
> > -                       domid_t xenstore_domid)
> > +static int compat_gnttab_seed(xc_interface *xch, domid_t domid,
> > +                              xen_pfn_t console_gmfn,
> > +                              xen_pfn_t xenstore_gmfn,
> > +                              domid_t console_domid,
> > +                              domid_t xenstore_domid)
> >  {
> >  
> >      xen_pfn_t gnttab_gmfn;
> > @@ -337,11 +337,11 @@ int xc_dom_gnttab_seed(xc_interface *xch, domid_t 
> > domid,
> >      return 0;
> >  }
> >  
> > -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > -                           xen_pfn_t console_gpfn,
> > -                           xen_pfn_t xenstore_gpfn,
> > -                           domid_t console_domid,
> > -                           domid_t xenstore_domid)
> > +static int compat_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > +                                  xen_pfn_t console_gpfn,
> > +                                  xen_pfn_t xenstore_gpfn,
> > +                                  domid_t console_domid,
> > +                                  domid_t xenstore_domid)
> >  {
> >      int rc;
> >      xen_pfn_t scratch_gpfn;
> > @@ -380,7 +380,7 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t 
> > domid,
> >          return -1;
> >      }
> >  
> > -    rc = xc_dom_gnttab_seed(xch, domid,
> > +    rc = compat_gnttab_seed(xch, domid,
> >                              console_gpfn, xenstore_gpfn,
> >                              console_domid, xenstore_domid);
> >      if (rc != 0)
> > @@ -405,18 +405,78 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t 
> > domid,
> >      return 0;
> >  }
> >  
> > -int xc_dom_gnttab_init(struct xc_dom_image *dom)
> > +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> > +                       bool is_hvm, xen_pfn_t console_gmfn,
> > +                       xen_pfn_t xenstore_gmfn, domid_t console_domid,
> > +                       domid_t xenstore_domid)
> >  {
> > -    if ( xc_dom_translated(dom) ) {
> > -        return xc_dom_gnttab_hvm_seed(dom->xch, dom->guest_domid,
> > -                                      dom->console_pfn, dom->xenstore_pfn,
> > -                                      dom->console_domid, 
> > dom->xenstore_domid);
> > -    } else {
> > -        return xc_dom_gnttab_seed(dom->xch, dom->guest_domid,
> > -                                  xc_dom_p2m(dom, dom->console_pfn),
> > -                                  xc_dom_p2m(dom, dom->xenstore_pfn),
> > -                                  dom->console_domid, dom->xenstore_domid);
> > +    xenforeignmemory_handle* fmem = xch->fmem;
> > +    xenforeignmemory_resource_handle *fres;
> > +    void *addr = NULL;
> > +    grant_entry_v1_t *gnttab;
> > +
> > +    fres = xenforeignmemory_map_resource(fmem, guest_domid,
> > +                                         XENMEM_resource_grant_table,
> > +                                         0, 0, 1,
> > +                                         &addr, PROT_READ | PROT_WRITE, 0);
> > +    if ( !fres )
> > +    {
> > +        if ( errno == EOPNOTSUPP )
> > +            return is_hvm ?
> > +                compat_gnttab_hvm_seed(xch, guest_domid,
> > +                                       console_gmfn, xenstore_gmfn,
> > +                                       console_domid, xenstore_domid) :
> > +                compat_gnttab_seed(xch, guest_domid,
> > +                                   console_gmfn, xenstore_gmfn,
> > +                                   console_domid, xenstore_domid);
> > +
> > +        xc_dom_panic(xch, XC_INTERNAL_ERROR,
> > +                     "%s: failed to acquire grant table "
> > +                     "[errno=%d]\n",
> > +                     __FUNCTION__, errno);
> > +        return -1;
> >      }
> > +
> > +    gnttab = addr;
> > +
> > +    if ( guest_domid != console_domid  && console_gmfn != -1)
> > +    {
> > +        xc_dom_printf(xch, "%s: setting console pfn=0x%"PRI_xen_pfn,
> > +                      __FUNCTION__, console_gmfn);
> > +
> > +        gnttab[GNTTAB_RESERVED_CONSOLE].flags = GTF_permit_access;
> > +        gnttab[GNTTAB_RESERVED_CONSOLE].domid = console_domid;
> > +        gnttab[GNTTAB_RESERVED_CONSOLE].frame = console_gmfn;
> > +    }
> > +
> > +    if ( guest_domid != xenstore_domid && xenstore_gmfn != -1)
> > +    {
> > +        xc_dom_printf(xch, "%s: setting xenstore pfn=0x%"PRI_xen_pfn,
> > +                      __FUNCTION__, xenstore_gmfn);
> > +
> > +        gnttab[GNTTAB_RESERVED_XENSTORE].flags = GTF_permit_access;
> > +        gnttab[GNTTAB_RESERVED_XENSTORE].domid = xenstore_domid;
> > +        gnttab[GNTTAB_RESERVED_XENSTORE].frame = xenstore_gmfn;
> > +    }
> > +
> > +    xenforeignmemory_unmap_resource(fmem, fres);
> > +
> > +    return 0;
> > +}
> > +
> > +int xc_dom_gnttab_init(struct xc_dom_image *dom)
> > +{
> > +    xc_interface *xch = dom->xch;
> > +    domid_t guest_domid = dom->guest_domid;
> > +    bool is_hvm = xc_dom_translated(dom);
> > +    xen_pfn_t console_gmfn = xc_dom_p2m(dom, dom->console_pfn);
> > +    xen_pfn_t xenstore_gmfn = xc_dom_p2m(dom, dom->xenstore_pfn);
> > +    domid_t console_domid = dom->console_domid;
> > +    domid_t xenstore_domid = dom->xenstore_domid;
> > +
> > +    return xc_dom_gnttab_seed(xch, guest_domid, is_hvm,
> > +                              console_gmfn, xenstore_gmfn,
> > +                              console_domid, xenstore_domid);
> >  }
> >  
> >  /*
> > diff --git a/tools/libxc/xc_sr_restore_x86_hvm.c 
> > b/tools/libxc/xc_sr_restore_x86_hvm.c
> > index 1dca85354a..a5c661da8f 100644
> > --- a/tools/libxc/xc_sr_restore_x86_hvm.c
> > +++ b/tools/libxc/xc_sr_restore_x86_hvm.c
> > @@ -207,11 +207,11 @@ static int x86_hvm_stream_complete(struct 
> > xc_sr_context *ctx)
> >          return rc;
> >      }
> >  
> > -    rc = xc_dom_gnttab_hvm_seed(xch, ctx->domid,
> > -                                ctx->restore.console_gfn,
> > -                                ctx->restore.xenstore_gfn,
> > -                                ctx->restore.console_domid,
> > -                                ctx->restore.xenstore_domid);
> > +    rc = xc_dom_gnttab_seed(xch, ctx->domid, true,
> > +                            ctx->restore.console_gfn,
> > +                            ctx->restore.xenstore_gfn,
> > +                            ctx->restore.console_domid,
> > +                            ctx->restore.xenstore_domid);
> >      if ( rc )
> >      {
> >          PERROR("Failed to seed grant table");
> > diff --git a/tools/libxc/xc_sr_restore_x86_pv.c 
> > b/tools/libxc/xc_sr_restore_x86_pv.c
> > index 50e25c162c..10635d436b 100644
> > --- a/tools/libxc/xc_sr_restore_x86_pv.c
> > +++ b/tools/libxc/xc_sr_restore_x86_pv.c
> > @@ -1104,7 +1104,7 @@ static int x86_pv_stream_complete(struct 
> > xc_sr_context *ctx)
> >      if ( rc )
> >          return rc;
> >  
> > -    rc = xc_dom_gnttab_seed(xch, ctx->domid,
> > +    rc = xc_dom_gnttab_seed(xch, ctx->domid, false,
> >                              ctx->restore.console_gfn,
> >                              ctx->restore.xenstore_gfn,
> >                              ctx->restore.console_domid,
> > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> > index f54fd49a73..0d3e462c12 100644
> > --- a/tools/libxl/libxl_dom.c
> > +++ b/tools/libxl/libxl_dom.c
> > @@ -851,7 +851,6 @@ static int hvm_build_set_params(xc_interface *handle, 
> > uint32_t domid,
> >      *store_mfn = str_mfn;
> >      *console_mfn = cons_mfn;
> >  
> > -    xc_dom_gnttab_hvm_seed(handle, domid, *console_mfn, *store_mfn, 
> > console_domid, store_domid);
> >      return 0;
> >  }
> >  
> > diff --git a/tools/python/xen/lowlevel/xc/xc.c 
> > b/tools/python/xen/lowlevel/xc/xc.c
> > index aa9f8e4d9e..583ab52a6f 100644
> > --- a/tools/python/xen/lowlevel/xc/xc.c
> > +++ b/tools/python/xen/lowlevel/xc/xc.c
> > @@ -800,9 +800,9 @@ static PyObject *pyxc_gnttab_hvm_seed(XcObject *self,
> >                                   &console_domid, &xenstore_domid) )
> >          return NULL;
> >  
> > -    if ( xc_dom_gnttab_hvm_seed(self->xc_handle, dom,
> > -                           console_gmfn, xenstore_gmfn,
> > -                           console_domid, xenstore_domid) != 0 )
> > +    if ( xc_dom_gnttab_seed(self->xc_handle, dom, true,
> > +                            console_gmfn, xenstore_gmfn,
> > +                            console_domid, xenstore_domid) != 0 )
> >          return pyxc_error_to_exception(self->xc_handle);
> >  
> >      return Py_None;
> > -- 
> > 2.11.0
> > 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Attachment: signature.asc
Description: PGP signature

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.