Re: [Xen-devel] [PATCH v4 09/31] libxc: introduce a xc_dom_arch for hvm-3.0-x86_32 guests

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Aug 07, 2015 at 12:17:46PM +0200, Roger Pau Monne wrote:
> This xc_dom_arch will be used in order to build HVM domains. The code is
> based on the existing xc_hvm_populate_memory and xc_hvm_populate_params
> functions.
> 
> 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>
> ---
> Changes since v3:
>  - Make sure c/s b9dbe33 is not reverted on this patch.
>  - Set the initial BSP state using {get/set}hvmcontext.
> ---
[...]
> +static void build_hvm_info(void *hvm_info_page, struct xc_dom_image *dom)
> +{
[...]
> +}
> +
> +static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
> +{
[...]
> +static int meminit_hvm(struct xc_dom_image *dom)
> +{

I didn't do a line by line comparison with original implementation for
every items in these functions, but if you forgot to move something here
I'm pretty sure we can easily spot that in the future. :-)

[...]
> +    xc_hvm_param_set(xch, domid, HVM_PARAM_IDENT_PT,
> +                     special_pfn(SPECIALPAGE_IDENT_PT) << PAGE_SHIFT);
> +
> +    dom->console_pfn = special_pfn(SPECIALPAGE_CONSOLE);
> +    dom->xenstore_pfn = special_pfn(SPECIALPAGE_XENSTORE);
> +    dom->parms.virt_hypercall = -1;
> +
> +    rc = 0;
> +    goto out;
> + error_out:
> +    rc = -1;
> + out:
> +
> +    return rc;

You can just write:

     rc = 0;
     return rc;
error_out:
     rc = -1
     return rc;

> +}
> +
>  /* ------------------------------------------------------------------------ 
> */
>  
>  static int start_info_x86_32(struct xc_dom_image *dom)
> @@ -682,6 +833,103 @@ static int vcpu_x86_64(struct xc_dom_image *dom)
>      return rc;
>  }
>  
> +static int vcpu_hvm(struct xc_dom_image *dom)
> +{
> +    struct {
> +        struct hvm_save_descriptor header_d;
> +        HVM_SAVE_TYPE(HEADER) header;
> +        struct hvm_save_descriptor cpu_d;
> +        HVM_SAVE_TYPE(CPU) cpu;
> +        struct hvm_save_descriptor end_d;
> +        HVM_SAVE_TYPE(END) end;
> +    } bsp_ctx;
> +    uint8_t *full_ctx = NULL;
> +    int rc;
> +
> +    DOMPRINTF_CALLED(dom->xch);
> +
> +    /*
> +     * Get the full HVM context in order to have the header, it is not
> +     * possible to get the header with getcontext_partial, and crafting one
> +     * from userspace is also not an option since cpuid is trapped and
> +     * modified by Xen.
> +     */
> +
> +    rc = xc_domain_hvm_getcontext(dom->xch, dom->guest_domid, NULL, 0);
> +    if ( rc <= 0 )
> +    {
> +        xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
> +                     "%s: unable to fetch HVM context size (rc=%d)",
> +                     __func__, rc);
> +        return rc;
> +    }
> +    full_ctx = malloc(rc);

xc_dom_malloc?

But you perhaps need hypercall safe buffer here.

The rest of this patch looks like purely mechanic changes to me so I
skip.

Wei.

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