Re: [Xen-devel] [PATCH v8 05/13] x86: expose CBM length and COS number information

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.05.15 at 10:41, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -216,6 +216,38 @@ void psr_ctxt_switch_to(struct domain *d)
>      }
>  }
>  
> +static int get_cat_socket_info(unsigned int socket,
> +                               struct psr_cat_socket_info **info)
> +{
> +    if ( !cat_socket_info )
> +        return -ENODEV;
> +
> +    if ( socket >= nr_sockets )
> +        return -EBADSLT;
> +
> +    if ( !test_bit(socket, cat_socket_enable) )
> +        return -ENOENT;
> +
> +    *info = cat_socket_info + socket;
> +
> +    return 0;
> +}
> +
> +int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
> +                        uint32_t *cos_max)
> +{
> +    struct psr_cat_socket_info *info;
> +    int ret = get_cat_socket_info(socket, &info);
> +
> +    if ( ret )
> +        return ret;
> +
> +    *cbm_len = info->cbm_len;
> +    *cos_max = info->cos_max;
> +
> +    return 0;
> +}

I doubt all supported compiler versions will be able to see that "info"
can't be used uninitialized here. Please make this explicit.

I also think this small a function shouldn't have two return points.

> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -694,6 +694,20 @@ struct xen_sysctl_pcitopoinfo {
>  typedef struct xen_sysctl_pcitopoinfo xen_sysctl_pcitopoinfo_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_pcitopoinfo_t);
>  
> +#define XEN_SYSCTL_PSR_CAT_get_l3_info               0
> +struct xen_sysctl_psr_cat_op {
> +    uint32_t cmd;       /* IN: XEN_SYSCTL_PSR_CAT_* */
> +    uint32_t target;    /* IN: socket to be operated on */

If this is always the socket number, why would the variable be
named anything other than "socket". If otoh subsequent patches
use it differently, I think the comment should be omitted now
rather than being dropped then (or it should be given its final
wording from the beginning).

Jan


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