Re: [Xen-devel] [PATCH v12 12/23] x86: refactor psr: L3 CAT: set value: implement write msr flow.

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> 06/14/17 3:25 AM >>>
> +struct cos_write_info
> +{
> +    unsigned int cos;
> +    struct feat_node *feature;
> +    uint32_t *val;

const?

>  static int write_psr_msrs(unsigned int socket, unsigned int cos,
>                            uint32_t val[], unsigned int array_len,
>                            enum psr_feat_type feat_type)
>  {
> -    return -ENOENT;
> +    unsigned int i;
> +    struct psr_socket_info *info = get_socket_info(socket);
> +    struct cos_write_info data =
> +    {
> +        .cos = cos,
> +        .feature = info->features[feat_type],
> +        .props = feat_props[feat_type],
> +    };
> +
> +    if ( cos > info->features[feat_type]->cos_max )
> +        return -EINVAL;
> +
> +    /* Skip to the feature's value head. */
> +    for ( i = 0; i < feat_type; i++ )
> +    {
> +        if ( !info->features[i] )
> +            continue;
> +
> +        if ( !feat_props[i] )
> +        {
> +            ASSERT_UNREACHABLE();
> +            return -ENOENT;
> +        }
> +
> +        if ( array_len <= feat_props[feat_type]->cos_num )
> +            return -ENOSPC;
> +
> +        array_len -= feat_props[feat_type]->cos_num;
> +
> +        val += feat_props[feat_type]->cos_num;

Well, you guess it. But additionally - doesn't the array index in all three
cases above need to be i? If so, please also check other patches (including
earlier ones, where I then may have overlooked this). It is anyway worth to
consider making this skip-prior-features loop a helper function, as this isn't
the first time this occurs. Otoh this would involve quite a bit of passing
return values via pointers, so maybe that wouldn't be too efficient. And I
guess macroizing this may end up looking a little clumsy / convoluted.

Jan

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