From: Paul Durrant <Paul.Durrant@citrix.com>
To: "'Marek Marczykowski-Górecki'" <marmarek@invisiblethingslab.com>,
"Wei Liu" <wei.liu2@citrix.com>
Cc: "xen-devel@lists.xenproject.org" <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
Date: Mon, 7 Aug 2017 12:36:21 +0000 [thread overview]
Message-ID: <a4c288e68ee949ea8ff9c41afeb265d8@AMSPEX02CL01.citrite.net> (raw)
In-Reply-To: <20170804130203.GG8495@mail-itl>
> -----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?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2017-08-07 12:36 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-02 9:59 [PATCH 0/5] x86: guest resource mepping Paul Durrant
2017-08-02 9:59 ` [PATCH 1/5] [x86|arm]: remove code duplication Paul Durrant
2017-08-02 9:59 ` [PATCH 2/5] x86/mm: allow a privileged PV domain to map guest mfns Paul Durrant
2017-08-02 9:59 ` [PATCH 3/5] x86/mm: add HYPERVISOR_memory_op to acquire guest resources Paul Durrant
2017-08-02 9:59 ` [PATCH 4/5] tools/libxenforeignmemory: add support for resource mapping Paul Durrant
2017-08-04 12:15 ` Wei Liu
2017-08-07 12:41 ` Paul Durrant
2017-08-02 9:59 ` [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to seed grant table Paul Durrant
2017-08-04 12:26 ` Wei Liu
2017-08-04 13:02 ` Marek Marczykowski-Górecki
2017-08-04 13:21 ` Wei Liu
2017-08-04 13:39 ` Juergen Gross
2017-08-07 12:36 ` Paul Durrant [this message]
2017-08-07 12:35 ` Paul Durrant
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a4c288e68ee949ea8ff9c41afeb265d8@AMSPEX02CL01.citrite.net \
--to=paul.durrant@citrix.com \
--cc=Ian.Jackson@citrix.com \
--cc=marmarek@invisiblethingslab.com \
--cc=wei.liu2@citrix.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).