Re: [Xen-devel] [PATCH v8 08/16] x86: implement set value flow for MBA

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.10.17 at 05:04, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> This patch implements set value flow for MBA including its callback
> function and domctl interface.
> 
> Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> v8:
>     - restore some unnecessary changes in 'cat_check_cbm'.
>       (suggested by Jan Beulich)

This reads as if you did exactly the opposite thing of what you
actually did.

> +static bool mba_sanitize_thrtl(const struct feat_node *feat, uint32_t *thrtl)
> +{
> +    if ( *thrtl > feat->mba.thrtl_max )
> +        return false;

Wouldn't it be better to do this check after ...

> +    /*
> +     * Per SDM (chapter "Memory Bandwidth Allocation Configuration"):
> +     * 1. Linear mode: In the linear mode the input precision is defined
> +     *    as 100-(MBA_MAX). For instance, if the MBA_MAX value is 90, the
> +     *    input precision is 10%. Values not an even multiple of the
> +     *    precision (e.g., 12%) will be rounded down (e.g., to 10% delay
> +     *    applied).
> +     * 2. Non-linear mode: Input delay values are powers-of-two from zero
> +     *    to the MBA_MAX value from CPUID. In this case any values not a
> +     *    power of two will be rounded down the next nearest power of two.
> +     */
> +    if ( feat->mba.linear )
> +        *thrtl -= *thrtl % (100 - feat->mba.thrtl_max);
> +    else
> +    {
> +        /* Not power of 2. */
> +        if ( *thrtl & (*thrtl - 1) )
> +            *thrtl = 1 << (fls(*thrtl) - 1);
> +    }

... these adjustments? That way someone specifying e.g. a linear
value of 95% would get 90% just like for 85% (s)he would get
80%.

> +    return true;

If so, the return statement could simply become

    return *thrtl <= feat->mba.thrtl_max;

My R-b also applies if you decide to make this change.

Jan


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