Re: [Xen-devel] [PATCH v9 13/25] x86: refactor psr: L3 CAT: set value: implement write msr flow.

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

On 17-03-27 04:46:01, Jan Beulich wrote:
> >>> On 16.03.17 at 12:08, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> > @@ -421,6 +425,18 @@ static bool cat_fits_cos_max(const uint32_t val[],
> >  }
> >  
> >  /* L3 CAT ops */
> > +static void l3_cat_write_msr(unsigned int cos, uint32_t val,
> > +                             enum cbm_type type, struct feat_node *feat)
> 
> "type" is an unused parameter. Please remove it from the hook
> and this function.
> 
This is used by CDP callback function. We need it to decide whether CODE
or DATA is written.

[...]
> > +struct cos_write_info
> > +{
> > +    unsigned int cos;
> > +    struct feat_node *feature;
> > +    uint32_t val;
> > +    enum cbm_type type;
> 
> With the "type" parameter removed above, this looks to be an
> unused field then too. After the removal of it, please re-order
> fields to leave no holes.
> 
Per above comment, 'type' is needed.

> > +static void do_write_psr_msr(void *data)
> > +{
> > +    struct cos_write_info *info = (struct cos_write_info *)data;
> 
> Unnecessary cast again.
> 
Thanks!

> > +    unsigned int cos            = info->cos;
> > +    struct feat_node *feat      = info->feature;
> > +
> > +    if ( !feat )
> > +        return;
> 
> You shouldn't even call this function when !feat.
> 
Ok, may I move the check to the 'write_psr_msr'?

> > +    if ( cos > feat->ops.get_cos_max(feat) )
> > +        return;
> > +
> > +    feat->ops.write_msr(cos, info->val, info->type, feat);
> > +}
> > +
> >  static int write_psr_msr(unsigned int socket, unsigned int cos,
> >                           uint32_t val, enum cbm_type type,
> 
> "type", according to the above comments, should then go away
> here too as it seems.
> 
Please check above comments.

> Jan

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