Re: [Xen-devel] [PATCH v5 01/28] HYPERCALL_version_op. New hypercall mirroring XENVER_ but sane.

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/03/16 20:00, Konrad Rzeszutek Wilk wrote:
> This hypercall mirrors the XENVER_ in that it has similar functionality.
> However it is designed differently:
>  - No compat layer. The data structures are the same size on 32
>    as on 64-bit.
>  - The hypercall accepts three arguments - the command, pointer to
>    an buffer, and the length of the buffer.

"a buffer"

> +/* Computed by kernel_cache_init. */
> +static xen_capabilities_info_t __read_mostly cached_cap;
> +static unsigned int __read_mostly cached_cap_len;
> +
> +/*
> + * Similar to HYPERVISOR_xen_version but with a sane interface
> + * (has a length, one can probe for the length) and with one less sub-ops:
> + * missing XENVER_compile_info.
> + */
> +DO(version_op)(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg,
> +               unsigned int len)
> +{
> +    union {
> +        xen_version_op_val_t val;
> +        xen_feature_info_t fi;
> +    } u = {};
> +    unsigned int sz = 0;
> +    const void *ptr = NULL;
> +    int rc = xsm_version_op(XSM_OTHER, cmd);
> +
> +    /* We can safely return -EPERM! */
> +    if ( rc )
> +        return rc;
> +
> +    /*
> +     * The HYPERVISOR_xen_version differs in that some return the value,
> +     * and some copy it on back on argument. We follow the same rule for all
> +     * sub-ops: return 0 on success, positive value of bytes returned, and

You can't return both 0 and a positive number for success.  I would
recommend "return the number of bytes written, or negative errno on
failure".

Other than these, LGTM.

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

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