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

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

On 17-06-29 12:00:12, Jan Beulich wrote:
> >>> Yi Sun <yi.y.sun@linux.intel.com> 06/14/17 3:25 AM >>>
> > +struct cos_write_info
> > +{
> > +    unsigned int cos;
> > +    struct feat_node *feature;
> > +    uint32_t *val;
> 
> const?
> 
The member of feature, 'cos_reg_val', will be written in 'do_write_psr_msrs'.
So, I cannot use const here.

> >  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

Very sorry for this obvious error!

> 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.
> 
Will implement 'skip-prior-features' function to do this. Thanks!

> Jan

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