Re: [Xen-devel] [PATCH v5 08/28] libxc: introduce a xc_dom_arch for hvm-3.0-x86_32 guests

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Aug 21, 2015 at 06:53:21PM +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>
> Reviewed-by: Andrew Cooper <andrew.cooper3@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 v4:
>  - Replace a malloc+memset with a calloc.
>  - Remove a != NULL check.
>  - Add Andrew Cooper Reviewed-by.
> 
> Changes since v3:
>  - Make sure c/s b9dbe33 is not reverted on this patch.
>  - Set the initial BSP state using {get/set}hvmcontext.
[...]
> +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;

Please avoid mixing return and goto when handling error cases. Use goto
style is preferred.

> +    }
> +
> +    full_ctx = calloc(1, rc);
> +    if ( full_ctx == NULL )
> +    {
> +        xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
> +                     "%s: unable to allocate memory for HVM context (rc=%d)",
> +                     __func__, rc);
> +        return -ENOMEM;

Ditto.

And you can also remove "if ( full_ctx )" in out path.

With those fixed:

Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>

Wei.

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