Re: [Xen-devel] [PATCH v9 12/25] x86: refactor psr: L3 CAT: set value: implement cos id picking flow.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.03.17 at 12:08, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> +static bool cat_fits_cos_max(const uint32_t val[],
> +                             const struct feat_node *feat,
> +                             unsigned int cos)
> +{
> +    if ( cos > feat->info.cat_info.cos_max &&
> +         val[0] != feat->cos_reg_val[0] )
> +            /*
> +             * Exceed cos_max and value to set is not default,
> +             * return error.
> +             */
> +            return false;
> +
> +    return true;
> +}

Same here - with cos_max moved out, the hook would seem to
become unnecessary.

>  static int pick_avail_cos(const struct psr_socket_info *info,
>                            const uint32_t val[], uint32_t array_len,
>                            unsigned int old_cos,
>                            enum psr_feat_type feat_type)
>  {
> +    unsigned int cos;
> +    unsigned int cos_max = 0;
> +    const struct feat_node *feat;
> +    const unsigned int *ref = info->cos_ref;
> +
>      ASSERT(spin_is_locked((spinlock_t *)(&info->ref_lock)));
> -    return -ENOENT;
> +
> +    /* cos_max is the one of the feature which is being set. */
> +    feat = info->features[feat_type];
> +    if ( !feat )
> +        return -ENOENT;
> +
> +    cos_max = feat->ops.get_cos_max(feat);
> +    if ( !cos_max )
> +        return -ENOENT;
> +
> +    /*
> +     * If old cos is referred only by the domain, then use it. And, we cannot

"the domain" here is lacking context - there's no domain involved
in the function - "... the domain currently using it, then re-use it"?

> +     * use id 0 because it stores the default values.
> +     */
> +    if ( old_cos && ref[old_cos] == 1 &&
> +         fits_cos_max(val, array_len, info, old_cos) )
> +            return old_cos;
> +
> +    /* Find an unused one other than cos0. */
> +    for ( cos = 1; cos <= cos_max; cos++ )
> +    {
> +        /*
> +         * ref is 0 means this COS is not used by other domain and
> +         * can be used for current setting.
> +         */
> +        if ( !ref[cos] )
> +        {
> +            if ( !fits_cos_max(val, array_len, info, cos) )
> +                return -EOVERFLOW;

Perhaps better use "break;" here.

> +            return cos;
> +        }
> +    }
> +
> +    return -EOVERFLOW;
>  }

Jan


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