|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 REPOST 05/12] tools/libxenctrl: use new xenforeignmemory API to seed grant table
> -----Original Message-----
> From: Roger Pau Monne
> Sent: 24 August 2017 17:03
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Wei Liu <wei.liu2@xxxxxxxxxx>; Ian
> Jackson <Ian.Jackson@xxxxxxxxxx>
> Subject: Re: [Xen-devel] [PATCH v2 REPOST 05/12] tools/libxenctrl: use new
> xenforeignmemory API to seed grant table
>
> On Tue, Aug 22, 2017 at 03:50:59PM +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.
> >
> > 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>
> > Acked-by: Marek Marczykowski-Górecki
> <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> > ---
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > ---
> > 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);
>
> Could be written as:
>
> return (is_hvm ? compat_gnttab_hvm_seed : compat_gnttab_seed)
> (xch, guest_domid, console_gmfn, xenstore_gmfn, console_domid,
> xenstore_domid);
Is that preferable?
>
> > +
> > + 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)
> ^ extra space.
> > + {
> > + 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;
> > + }
>
> The code above is already present in xc_dom_gnttab_seed (now renamed
> to compat_gnttab_seed, isn't there anyway that you could re-use it?
Hmm. Maybe. I'll see if I can re-work it to remove the duplication.
Thanks,
Paul
>
> Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |