Re: [Xen-devel] [PATCH v8 07/24] x86: refactor psr: implement get value flow.

[Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>]

On 17-02-28 12:44:34, Roger Pau Monn� wrote:
> On Wed, Feb 15, 2017 at 04:49:22PM +0800, Yi Sun wrote:
> > +static bool l3_cat_get_val(const struct feat_node *feat, unsigned int cos,
> > +                           enum cbm_type type, uint64_t *val)
> > +{
> > +    if ( cos > feat->info.l3_cat_info.cos_max )
> > +        /* Use default value. */
> > +        cos = 0;
> 
> I don't know much, but shouldn't this return false instead of silently
> defaulting to 0? This doesn't seem to be what the caller expects.
> 
If cos exceeds the cos_max, we should return default value saved in
cos_reg_val[0]. Let me explain more, different features have different
cos_max, e.g. L3 CAT cos_max=16, L2 CAT cos_max=8, user can set L3 CAT
COS[9] for a domain. When COS ID 9 is set to ASSOC register, it works
for all features. HW automatically works as default value for L2 CAT
because the COS ID exceeds the max.

> > @@ -498,6 +516,15 @@ int psr_get_info(unsigned int socket, enum cbm_type 
> > type,
> >          if ( feat->feature != feat_type )
> >              continue;
> >  
> > +        if ( d )
> > +        {
> > +            cos = d->arch.psr_cos_ids[socket];
> > +            if ( feat->ops.get_val(feat, cos, type, val) )
> > +                return 0;
> > +            else
> 
> No need for the "else" branch here.
> 
Thanks!

> Roger.

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