| Message ID | 20170802095949.40677-6-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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? > 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@citrix.com> > --- > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> BTW Marek needs to be CC on changes to Python bindings. I've done that for you. > --- > 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 >
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@citrix.com> > > --- > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > > Cc: Wei Liu <wei.liu2@citrix.com> > > 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@invisiblethingslab.com> > > --- > > 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 > >
On Fri, Aug 04, 2017 at 03:02:03PM +0200, Marek Marczykowski-Górecki wrote: > 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? libxc isn't stable, so functions might get deleted / updated at any point. Most libxc functions are quite simple and are quite "close" to the hypervisor. The non-trivial functions I know of are VMI related ones but I think users in that area already have the right expectation. An important user of unstable libxc APIs is QEMU. It probes for specific functions in the configure script to handle compatibility issues.
On 04/08/17 15:21, Wei Liu wrote: > On Fri, Aug 04, 2017 at 03:02:03PM +0200, Marek Marczykowski-Górecki wrote: >> 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? > > libxc isn't stable, so functions might get deleted / updated at any > point. > > Most libxc functions are quite simple and are quite "close" to the > hypervisor. > > The non-trivial functions I know of are VMI related ones but I think > users in that area already have the right expectation. > > An important user of unstable libxc APIs is QEMU. It probes for specific > functions in the configure script to handle compatibility issues. Right now qemu can just use the version of libxc instead of doing it via tests. :-) Juergen
> -----Original Message----- > From: Wei Liu [mailto:wei.liu2@citrix.com] > Sent: 04 August 2017 13:26 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: xen-devel@lists.xenproject.org; Ian Jackson <Ian.Jackson@citrix.com>; > Wei Liu <wei.liu2@citrix.com>; Marek Marczykowski-Górecki > <marmarek@invisiblethingslab.com> > Subject: Re: [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to > seed grant table > > 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. > Ok, thanks :-) > What's the benefit of doing this? The main benefit is not having to insert the grant table into an HVM guest's P2M, which potentially shatters a super-page mapping. Paul > > > 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@citrix.com> > > --- > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > > Cc: Wei Liu <wei.liu2@citrix.com> > > BTW Marek needs to be CC on changes to Python bindings. I've done that > for you. > > > --- > > 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 > >
> -----Original Message----- > From: Marek Marczykowski-Górecki > [mailto:marmarek@invisiblethingslab.com] > Sent: 04 August 2017 14:02 > To: Wei Liu <wei.liu2@citrix.com> > Cc: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org; > Ian Jackson <Ian.Jackson@citrix.com> > Subject: Re: [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@citrix.com> > > > --- > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > > > Cc: Wei Liu <wei.liu2@citrix.com> > > > > 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@invisiblethingslab.com> Thanks :-) Paul > > > > --- > > > 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?
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;
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. 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@citrix.com> --- Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> --- 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(-)