Re: [Xen-devel] [PATCH v4 08/31] libxc: rework BSP initialization

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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 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é <roger.pau@xxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  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 = ptr;
> +    vcpu_guest_context_any_t any_ctx;
> +    vcpu_guest_context_t *ctxt = &any_ctx.c;
>  

I think you still need to allocate hypercall safe buffer here, as well
as in other vcpu_* functions.

>      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 = xc_vcpu_setcontext(dom->xch, dom->guest_domid, 0, &any_ctx);
> +    if ( rc != 0 )
> +        xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
> +                     "%s: SETVCPUCONTEXT failed (rc=%d)", __func__, rc);
> +
> +    return rc;
>  }
>  
[...]
> -    ctxt = xc_hypercall_buffer_alloc(dom->xch, ctxt, sizeof(*ctxt));
> -    if ( ctxt == 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 = dom->arch_hooks->vcpu(dom, ctxt)) != 0 )
> +    if ( (rc = dom->arch_hooks->vcpu(dom)) != 0 )
>          return rc;
>      xc_dom_unmap_all(dom);

This is not your problem, but this xc_dom_unmap_all is really
suspicious.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel