Re: [PATCH v4 10/24] x86: refactor psr: set value: implement cos finding flow.

[Yi Sun <yi.y.sun@linux.intel.com>]

On 17-01-10 07:53:07, Jan Beulich wrote:
> >>> On 14.12.16 at 05:07, <yi.y.sun@linux.intel.com> wrote:
> > Continue with previous patch, we can try to find if there is a
> 
> Please take into consideration that a series may be applied in small
> steps. References such as "previous patch" are thus possibly
> meaningless. Please instead refer to the patch by title. Also I think
> you mean "continue from ...".
> 
Thank you!

> > @@ -666,7 +724,58 @@ static int find_cos(const uint64_t *val, uint32_t array_len,
> >                      enum cbm_type type,
> >                      const struct psr_socket_info *info)
> >  {
> > -    return 0;
> > +    unsigned int cos;
> > +    const unsigned int *ref = info->cos_ref;
> > +    const struct feat_node *feat_tmp;
> > +    const uint64_t *val_tmp = val;
> > +    int ret;
> > +    bool found = false;
> > +    unsigned int cos_max = 0;
> > +
> > +    /* cos_max is the one of the feature which is being set. */
> > +    list_for_each_entry(feat_tmp, &info->feat_list, list)
> > +    {
> > +        cos_max = feat_tmp->ops.get_cos_max_from_type(feat_tmp, type);
> > +        if ( cos_max > 0 )
> > +            break;
> > +    }
> > +
> > +    for ( cos = 0; cos <= cos_max; cos++ )
> > +    {
> > +        if ( cos && !ref[cos] )
> > +            continue;
> > +
> > +        /* Not found, need find again from beginning. */
> > +        val_tmp = val;
> > +        list_for_each_entry(feat_tmp, &info->feat_list, list)
> > +        {
> > +            /*
> > +             * Compare value according to feature list order.
> > +             * We must follow this order because value array is assembled
> > +             * as this order in get_old_set_new().
> > +             */
> > +            ret = feat_tmp->ops.compare_val(val_tmp, feat_tmp, cos, &found);
> > +            if ( ret < 0 )
> > +                return ret;
> > +
> > +            /* If fail to match, go to next cos to compare. */
> > +            if ( !found )
> > +                break;
> > +
> > +            val_tmp += ret;
> > +            if ( val_tmp - val > array_len )
> > +                return -EINVAL;
> > +        }
> > +
> > +        /*
> > +         * With this cos id, every entry of value array can match. This cos
> > +         * is what we find.
> > +         */
> 
> "can match" seems rather misleading to me. I think you mean
> something like "For this COS ID all entries in the values array did
> match. Use it."
> 
Yes, sorry for the wording.

> Other than that various of the comments given for earlier patches
> apply here, in particular the fact that the type matching should
> move out of the hook functions.
> 
Will try this. Thanks!

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

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