From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: Re: [PATCH v4 08/31] libxc: rework BSP initialization Date: Fri, 7 Aug 2015 13:06:14 +0100 Message-ID: <20150807120614.GJ6005@zion.uk.xensource.com> References: <1438942688-7610-1-git-send-email-roger.pau@citrix.com> <1438942688-7610-9-git-send-email-roger.pau@citrix.com> <20150807113143.GG6005@zion.uk.xensource.com> <55C49EC5.6090609@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZNgPl-0002R3-Dr for xen-devel@lists.xenproject.org; Fri, 07 Aug 2015 12:06:21 +0000 Content-Disposition: inline In-Reply-To: <55C49EC5.6090609@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Roger Pau =?iso-8859-1?Q?Monn=E9?= Cc: xen-devel@lists.xenproject.org, Ian Campbell , Wei Liu , Ian Jackson , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On Fri, Aug 07, 2015 at 02:04:21PM +0200, Roger Pau Monn=E9 wrote: > El 07/08/15 a les 13.31, Wei Liu ha escrit: > > On Fri, Aug 07, 2015 at 12:17:45PM +0200, Roger Pau Monne wrote: > >> Place the calls to xc_vcpu_setcontext and the allocation of the hyperc= all > >> buffer into the arch-specific vcpu hooks. This is needed for the next = patch, > >> so x86 HVM guests can initialize the BSP using XEN_DOMCTL_sethvmcontext > >> instead of XEN_DOMCTL_setvcpucontext. > >> > >> This patch should not introduce any functional change. > >> > >> Signed-off-by: Roger Pau Monn=E9 > >> Cc: Ian Jackson > >> Cc: Stefano Stabellini > >> Cc: Ian Campbell > >> Cc: Wei Liu > >> --- > >> tools/libxc/include/xc_dom.h | 2 +- > >> tools/libxc/xc_dom_arm.c | 22 +++++++++++++++++----- > >> tools/libxc/xc_dom_boot.c | 23 +---------------------- > >> tools/libxc/xc_dom_x86.c | 26 ++++++++++++++++++++------ > >> 4 files changed, 39 insertions(+), 34 deletions(-) > >> > >> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom= .h > >> index 5c1bb0f..0245d24 100644 > >> --- a/tools/libxc/include/xc_dom.h > >> +++ b/tools/libxc/include/xc_dom.h > >> @@ -221,7 +221,7 @@ struct xc_dom_arch { > >> /* arch-specific data structs setup */ > >> int (*start_info) (struct xc_dom_image * dom); > >> int (*shared_info) (struct xc_dom_image * dom, void *shared_info); > >> - int (*vcpu) (struct xc_dom_image * dom, void *vcpu_ctxt); > >> + int (*vcpu) (struct xc_dom_image * dom); > >> int (*bootearly) (struct xc_dom_image * dom); > >> int (*bootlate) (struct xc_dom_image * dom); > >> = > >> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c > >> index 7548dae..8865097 100644 > >> --- a/tools/libxc/xc_dom_arm.c > >> +++ b/tools/libxc/xc_dom_arm.c > >> @@ -119,9 +119,10 @@ static int shared_info_arm(struct xc_dom_image *d= om, void *ptr) > >> = > >> /* ------------------------------------------------------------------= ------ */ > >> = > >> -static int vcpu_arm32(struct xc_dom_image *dom, void *ptr) > >> +static int vcpu_arm32(struct xc_dom_image *dom) > >> { > >> - vcpu_guest_context_t *ctxt =3D ptr; > >> + vcpu_guest_context_any_t any_ctx; > >> + vcpu_guest_context_t *ctxt =3D &any_ctx.c; > >> = > > = > > I think you still need to allocate hypercall safe buffer here, as well > > as in other vcpu_* functions. > = > xc_vcpu_setcontext already bounces the buffer before doing the > hypercall, so I think it's safe. > = > >> DOMPRINTF_CALLED(dom->xch); > >> = > >> @@ -154,12 +155,18 @@ static int vcpu_arm32(struct xc_dom_image *dom, = void *ptr) > >> DOMPRINTF("Initial state CPSR %#"PRIx32" PC %#"PRIx32, > >> ctxt->user_regs.cpsr, ctxt->user_regs.pc32); > >> = > >> - return 0; > >> + rc =3D xc_vcpu_setcontext(dom->xch, dom->guest_domid, 0, &any_ctx= ); > >> + if ( rc !=3D 0 ) > >> + xc_dom_panic(dom->xch, XC_INTERNAL_ERROR, > >> + "%s: SETVCPUCONTEXT failed (rc=3D%d)", __func__,= rc); > >> + > >> + return rc; > >> } > >> = > > [...] > >> - ctxt =3D xc_hypercall_buffer_alloc(dom->xch, ctxt, sizeof(*ctxt)); > >> - if ( ctxt =3D=3D NULL ) > >> - return -1; > >> - > >> DOMPRINTF_CALLED(dom->xch); > >> = > >> /* misc stuff*/ > >> @@ -259,13 +241,10 @@ int xc_dom_boot_image(struct xc_dom_image *dom) > >> return rc; > >> = > >> /* let the vm run */ > >> - memset(ctxt, 0, sizeof(*ctxt)); > >> - if ( (rc =3D dom->arch_hooks->vcpu(dom, ctxt)) !=3D 0 ) > >> + if ( (rc =3D dom->arch_hooks->vcpu(dom)) !=3D 0 ) > >> return rc; > >> xc_dom_unmap_all(dom); > > = > > This is not your problem, but this xc_dom_unmap_all is really > > suspicious. > = > I agree, xc_dom_release already calls xc_dom_unmap_all. I guess it's not > a big issue since xc_dom_boot_image is almost the last function called > when doing domain creation, but I'm also not that familiar with libxc in > order to figure out if there's a reason for calling xc_dom_unmap_all here. > = Yeah, let's worry about that later. Wei. > Roger.