From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek =?utf-8?Q?Marczykowski-G=C3=B3recki?= Subject: Re: [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to seed grant table Date: Fri, 4 Aug 2017 15:02:03 +0200 Message-ID: <20170804130203.GG8495@mail-itl> References: <20170802095949.40677-1-paul.durrant@citrix.com> <20170802095949.40677-6-paul.durrant@citrix.com> <20170804122621.d5ptwybyk6avgypc@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3829834589460425733==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ddcF4-0002Jx-IF for xen-devel@lists.xenproject.org; Fri, 04 Aug 2017 13:02:14 +0000 In-Reply-To: <20170804122621.d5ptwybyk6avgypc@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Wei Liu Cc: xen-devel@lists.xenproject.org, Paul Durrant , Ian Jackson List-Id: xen-devel@lists.xenproject.org --===============3829834589460425733== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="AhhlLboLdkugWU4S" Content-Disposition: inline --AhhlLboLdkugWU4S Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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). > >=20 > > 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 whi= ch > > case the old scheme is used. > >=20 >=20 > The code mostly looks fine. >=20 > 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(= ). > >=20 > > Signed-off-by: Paul Durrant > > --- > > Cc: Ian Jackson > > Cc: Wei Liu >=20 > 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=C3=B3recki > > --- > > 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(-) > >=20 > > 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 *do= m, 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; > > } > > =20 > > -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) > > { > > =20 > > xen_pfn_t gnttab_gmfn; > > @@ -337,11 +337,11 @@ int xc_dom_gnttab_seed(xc_interface *xch, domid_t= domid, > > return 0; > > } > > =20 > > -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; > > } > > =20 > > - rc =3D xc_dom_gnttab_seed(xch, domid, > > + rc =3D compat_gnttab_seed(xch, domid, > > console_gpfn, xenstore_gpfn, > > console_domid, xenstore_domid); > > if (rc !=3D 0) > > @@ -405,18 +405,78 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, dom= id_t domid, > > return 0; > > } > > =20 > > -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->xenstor= e_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_do= mid); > > + xenforeignmemory_handle* fmem =3D xch->fmem; > > + xenforeignmemory_resource_handle *fres; > > + void *addr =3D NULL; > > + grant_entry_v1_t *gnttab; > > + > > + fres =3D xenforeignmemory_map_resource(fmem, guest_domid, > > + XENMEM_resource_grant_table, > > + 0, 0, 1, > > + &addr, PROT_READ | PROT_WRITE= , 0); > > + if ( !fres ) > > + { > > + if ( errno =3D=3D 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=3D%d]\n", > > + __FUNCTION__, errno); > > + return -1; > > } > > + > > + gnttab =3D addr; > > + > > + if ( guest_domid !=3D console_domid && console_gmfn !=3D -1) > > + { > > + xc_dom_printf(xch, "%s: setting console pfn=3D0x%"PRI_xen_pfn, > > + __FUNCTION__, console_gmfn); > > + > > + gnttab[GNTTAB_RESERVED_CONSOLE].flags =3D GTF_permit_access; > > + gnttab[GNTTAB_RESERVED_CONSOLE].domid =3D console_domid; > > + gnttab[GNTTAB_RESERVED_CONSOLE].frame =3D console_gmfn; > > + } > > + > > + if ( guest_domid !=3D xenstore_domid && xenstore_gmfn !=3D -1) > > + { > > + xc_dom_printf(xch, "%s: setting xenstore pfn=3D0x%"PRI_xen_pfn, > > + __FUNCTION__, xenstore_gmfn); > > + > > + gnttab[GNTTAB_RESERVED_XENSTORE].flags =3D GTF_permit_access; > > + gnttab[GNTTAB_RESERVED_XENSTORE].domid =3D xenstore_domid; > > + gnttab[GNTTAB_RESERVED_XENSTORE].frame =3D xenstore_gmfn; > > + } > > + > > + xenforeignmemory_unmap_resource(fmem, fres); > > + > > + return 0; > > +} > > + > > +int xc_dom_gnttab_init(struct xc_dom_image *dom) > > +{ > > + xc_interface *xch =3D dom->xch; > > + domid_t guest_domid =3D dom->guest_domid; > > + bool is_hvm =3D xc_dom_translated(dom); > > + xen_pfn_t console_gmfn =3D xc_dom_p2m(dom, dom->console_pfn); > > + xen_pfn_t xenstore_gmfn =3D xc_dom_p2m(dom, dom->xenstore_pfn); > > + domid_t console_domid =3D dom->console_domid; > > + domid_t xenstore_domid =3D dom->xenstore_domid; > > + > > + return xc_dom_gnttab_seed(xch, guest_domid, is_hvm, > > + console_gmfn, xenstore_gmfn, > > + console_domid, xenstore_domid); > > } > > =20 > > /* > > diff --git a/tools/libxc/xc_sr_restore_x86_hvm.c b/tools/libxc/xc_sr_re= store_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_c= ontext *ctx) > > return rc; > > } > > =20 > > - rc =3D 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 =3D 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_res= tore_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_co= ntext *ctx) > > if ( rc ) > > return rc; > > =20 > > - rc =3D xc_dom_gnttab_seed(xch, ctx->domid, > > + rc =3D 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 *handl= e, uint32_t domid, > > *store_mfn =3D str_mfn; > > *console_mfn =3D cons_mfn; > > =20 > > - xc_dom_gnttab_hvm_seed(handle, domid, *console_mfn, *store_mfn, co= nsole_domid, store_domid); > > return 0; > > } > > =20 > > diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowle= vel/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 *sel= f, > > &console_domid, &xenstore_domid) ) > > return NULL; > > =20 > > - if ( xc_dom_gnttab_hvm_seed(self->xc_handle, dom, > > - console_gmfn, xenstore_gmfn, > > - console_domid, xenstore_domid) !=3D 0 ) > > + if ( xc_dom_gnttab_seed(self->xc_handle, dom, true, > > + console_gmfn, xenstore_gmfn, > > + console_domid, xenstore_domid) !=3D 0 ) > > return pyxc_error_to_exception(self->xc_handle); > > =20 > > return Py_None; > > --=20 > > 2.11.0 > >=20 --=20 Best Regards, Marek Marczykowski-G=C3=B3recki 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? --AhhlLboLdkugWU4S Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJZhHBMAAoJENuP0xzK19csbCIH/A338YfPVd3+PBzX2OLEf0bl yWVZ9EEneIuTOXtrelY7FZuD3CpO4Hg8fbLgSde1t02pIIWKqScpZlwXgl69TZ2H 2obHARFG0fSPtw+OgH36hUisFNbhYnco979WkWRpY3/BhB/tRBhx+0zckJZBbqxm uC96fb8QrbdH9OcsNnYtnN9LmzpEd0KfqKCOPgMk7Z0DrX2HC+d+CwMLC5pSkeZI 9mndQBtvNVSCNoxUFVswl11f5kDoWRPvnAENlCbWVIog/8dMxkqa+0WXFYydpsN7 EZBD+pTiUlw7GS9GGgRboQuAs1F/1x6Av00GMjq2J+APexheZ5pOkZH0qcZeLS4= =NpMl -----END PGP SIGNATURE----- --AhhlLboLdkugWU4S-- --===============3829834589460425733== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============3829834589460425733==--