Re: [Xen-devel] [PATCH v4 09/24] x86: refactor psr: set value: assemble features value array.

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

On 17-01-11 06:57:30, Jan Beulich wrote:
> >>> On 11.01.17 at 07:07, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> > On 17-01-10 07:34:00, Jan Beulich wrote:
> >> >>> On 14.12.16 at 05:07, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> >> > +static int l3_cat_set_new_val(uint64_t val[],
> >> > +                              const struct feat_node *feat,
> >> > +                              unsigned int old_cos,
> >> > +                              enum cbm_type type,
> >> > +                              uint64_t m)
> >> > +{
> >> > +    if ( type != PSR_CBM_TYPE_L3 )
> >> > +        /* L3 CAT uses one COS. Skip it. */
> >> > +        return 1;
> >> 
> >> If this is the wrong type, how can you return 1 (or any positive
> >> value) here? This basically suggests that, as recommended for an
> >> earlier operation already, that you want the type check done in
> >> the caller. Or wait - isn't type not matching an error here, as you
> >> iterate the same list twice in the same order? Which then gets us
> >> back to me mentioning that the set-new-value should really be
> >> done in common code, not in the callbacks; it would really only be
> >> the check (psr_check_cbm() in the L3 case above) which would
> >> require a callback.
> >> 
> > Your understanding is right. The value array will be iterated twice. First,
> > assemble all features old value into it. Second, set the target value into
> > array according to the target feature's position in the array. Because
> > different features may have different behaviors, e.g. CDP uses two entries
> > of the array, I think it would be better to make 'set_new_val' be a callback
> > function.
> 
> Bot for common code to do that, all it takes is knowing the number
> of registers, which you already have a callback for.
> 
Ok, will consider it. Thanks!

> Jan

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