From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?windows-1252?Q?Roger_Pau_Monn=E9?= Subject: Re: [PATCH v4 08/31] libxc: rework BSP initialization Date: Fri, 7 Aug 2015 14:04:21 +0200 Message-ID: <55C49EC5.6090609@citrix.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> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" 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 1ZNgO3-0002KV-Up for xen-devel@lists.xenproject.org; Fri, 07 Aug 2015 12:04:36 +0000 In-Reply-To: <20150807113143.GG6005@zion.uk.xensource.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: Wei Liu Cc: xen-devel@lists.xenproject.org, Ian Jackson , Ian Campbell , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org 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 hypercall >> buffer into the arch-specific vcpu hooks. This is needed for the next pa= tch, >> 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 *dom= , 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, vo= id *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__, r= c); >> + >> + 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. Roger.