From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 06/18] lib{xc, xl}: Seed grant tables with xenstore and console grants Date: Wed, 18 Jan 2012 11:05:13 +0000 Message-ID: <1326884714.14689.191.camel@zakaz.uk.xensource.com> References: <1326302490-19428-1-git-send-email-dgdegra@tycho.nsa.gov> <1326411330-7915-1-git-send-email-dgdegra@tycho.nsa.gov> <1326411330-7915-7-git-send-email-dgdegra@tycho.nsa.gov> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1326411330-7915-7-git-send-email-dgdegra@tycho.nsa.gov> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Daniel De Graaf Cc: Alex Zeffertt , "xen-devel@lists.xensource.com" , Diego Ongaro List-Id: xen-devel@lists.xenproject.org On Thu, 2012-01-12 at 23:35 +0000, Daniel De Graaf wrote: > @@ -275,6 +276,169 @@ int xc_dom_boot_image(struct xc_dom_image *dom) > return rc; > } > > +static unsigned long xc_dom_gnttab_setup(xc_interface *xch, uint32_t domid) > +{ > + DECLARE_HYPERCALL; > + gnttab_setup_table_t setup_table; > + DECLARE_HYPERCALL_BUFFER(unsigned long, gmfnp); > + int rc; > + unsigned long gmfn; There's a bunch of weird alignment issues in this patch. Presumably some hard tabs have snuck in. > + > + gmfnp = xc_hypercall_buffer_alloc(xch, gmfnp, sizeof(*gmfnp)); > + if (gmfnp == NULL) > + return -1; Alignment. > + > + setup_table.dom = domid; > + setup_table.nr_frames = 1; > + set_xen_guest_handle(setup_table.frame_list, gmfnp); > + setup_table.status = 0; > + > + hypercall.op = __HYPERVISOR_grant_table_op; > + hypercall.arg[0] = GNTTABOP_setup_table; > + hypercall.arg[1] = (unsigned long) &setup_table; > + hypercall.arg[2] = 1; > + > + rc = do_xen_hypercall(xch, &hypercall); > + gmfn = *gmfnp; Alignment. I'm going to stop pointing these out, I guess grep will find them... [...] > +int xc_dom_gnttab_init(struct xc_dom_image *dom) > +{ > + unsigned long console_gmfn; > + unsigned long xenstore_gmfn; > + int autotranslated; > + > + autotranslated = xc_dom_feature_translated(dom); > + console_gmfn = autotranslated ? > + dom->console_pfn : xc_dom_p2m_host(dom, dom->console_pfn); > + xenstore_gmfn = autotranslated ? > + dom->xenstore_pfn : xc_dom_p2m_host(dom, dom->xenstore_pfn); > + > + return xc_dom_gnttab_seed(dom->xch, dom->guest_domid, > + console_gmfn, xenstore_gmfn, > + dom->console_domid, dom->xenstore_domid); So on build we have can do the p2m lookup and hence use the PV version of gnttab_seed, whereas on restore we don't have a p2m and hence can't? The internals of xc_domain_restore do have the p2m though, so in principal we could avoid the need for the hvm version and the reintroduction of the remove_from_physmap by making xc_domain_restore output the necessary mfns? I'm not sure it is worth it though. > diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c > index 3fda6f8..8bee684 100644 > --- a/tools/libxc/xc_domain_restore.c > +++ b/tools/libxc/xc_domain_restore.c > @@ -1259,7 +1259,8 @@ static int apply_batch(xc_interface *xch, uint32_t dom, struct restore_ctx *ctx, > > int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, > unsigned int store_evtchn, unsigned long *store_mfn, > - unsigned int console_evtchn, unsigned long *console_mfn, > + uint32_t store_domid, unsigned int console_evtchn, > + unsigned long *console_mfn, uint32_t console_domid, > unsigned int hvm, unsigned int pae, int superpages, > int no_incr_generationid, > unsigned long *vm_generationid_addr) > @@ -2018,6 +2019,14 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, > memcpy(ctx->live_p2m, ctx->p2m, dinfo->p2m_size * sizeof(xen_pfn_t)); > munmap(ctx->live_p2m, P2M_FL_ENTRIES * PAGE_SIZE); > > + rc = xc_dom_gnttab_seed(xch, dom, *console_mfn, *store_mfn, > + console_domid, store_domid); > + if (rc != 0) > + { > + ERROR("error seeding grant table"); > + goto out; > + } > + > DPRINTF("Domain ready to be built.\n"); > rc = 0; > goto out; > @@ -2076,6 +2085,14 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, > goto out; > } > > + rc = xc_dom_gnttab_hvm_seed(xch, dom, *console_mfn, *store_mfn, > + console_domid, store_domid); Oh, we don't even need to return them from xc_domain_restore since we could just translate from gfn to mfn right here (since we have a p2m in hand) and call xc_dom_gnttab_seed. Or am I missing something? > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index 5e39595..04db22f 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c [...] > @@ -305,6 +312,8 @@ static int hvm_build_set_params(xc_interface *handle, uint32_t domid, > xc_set_hvm_param(handle, domid, HVM_PARAM_NESTEDHVM, info->u.hvm.nested_hvm); > xc_set_hvm_param(handle, domid, HVM_PARAM_STORE_EVTCHN, store_evtchn); > xc_set_hvm_param(handle, domid, HVM_PARAM_CONSOLE_EVTCHN, console_evtchn); > + > + xc_dom_gnttab_hvm_seed(handle, domid, *console_mfn, *store_mfn, console_domid, store_domid); Didn't this already happen in xc_linux_build_internal which was called via xc_linux_build_mem? [...] Ian.