Re: [Xen-devel] [PATCH v4 12/24] x86: refactor psr: set value: implement write msr flow.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.12.16 at 05:07, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -186,6 +186,9 @@ struct feat_ops {
>      unsigned int (*exceeds_cos_max)(const uint64_t val[],
>                                      const struct feat_node *feat,
>                                      unsigned int cos);
> +    /* write_msr is used to write out feature MSR register. */
> +    int (*write_msr)(unsigned int cos, const uint64_t val[],
> +                     struct feat_node *feat);

Looks like this function again returns number-of-values, yet this time
without a comment saying so. While you don't need to replicate
that description multiple time, please at least has a brief reference.
That said, with the type checks moved out I think this return value
model won't be needed anymore - the caller, having checked the
type, could then simply call the get-num-val (or however it was
named) hook to know how many array entries to skip.

> @@ -889,9 +909,67 @@ static int alloc_new_cos(const struct psr_socket_info 
> *info,
>      return -ENOENT;
>  }
>  
> +static unsigned int get_socket_cpu(unsigned int socket)
> +{
> +    if ( likely(socket < nr_sockets) )
> +        return cpumask_any(socket_cpumask[socket]);
> +
> +    return nr_cpu_ids;
> +}
> +
> +struct cos_write_info
> +{
> +    unsigned int cos;
> +    struct list_head *feat_list;
> +    const uint64_t *val;
> +};
> +
> +static void do_write_psr_msr(void *data)
> +{
> +    struct cos_write_info *info = (struct cos_write_info *)data;
> +    unsigned int cos           = info->cos;
> +    struct list_head *feat_list= info->feat_list;
> +    const uint64_t *val        = info->val;
> +    struct feat_node *feat_tmp;
> +    int ret;
> +
> +    if ( !feat_list )
> +        return;
> +
> +    /* We need set all features values into MSRs. */
> +    list_for_each_entry(feat_tmp, feat_list, list)
> +    {
> +        ret = feat_tmp->ops.write_msr(cos, val, feat_tmp);
> +        if ( ret <= 0)

Missing blank.

> +            return;
> +
> +        val += ret;
> +    }
> +}
> +
>  static int write_psr_msr(unsigned int socket, unsigned int cos,
>                           const uint64_t *val)
>  {
> +    struct psr_socket_info *info = get_socket_info(socket);
> +
> +    struct cos_write_info data =

No blank lines between declarations please (unless there are
extraordinarily many).

Jan


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