Re: [PATCH v11 15/23] x86: refactor psr: CDP: implement set value callback function.

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

On 17-06-06 01:48:13, Jan Beulich wrote:
> >>> On 02.06.17 at 09:59, <yi.y.sun@linux.intel.com> wrote:
> > On 17-05-31 03:44:31, Jan Beulich wrote:
> >> >>> On 03.05.17 at 10:44, <yi.y.sun@linux.intel.com> wrote:
> >> > @@ -765,7 +777,8 @@ static int insert_val_into_array(uint32_t val[],
> >> >  
> >> >      /* Value setting position is same as feature array. */
> >> >      for ( i = 0; i < props->cos_num; i++ )
> >> > -        if ( type == props->type[i] )
> >> > +        if ( type == props->type[i] ||
> >> > +             (feat_type == PSR_SOCKET_L3_CDP && type == PSR_CBM_TYPE_L3) )
> >> 
> >> Didn't the earlier patch take care of doing this substitution? Non-
> >> feature-specific code clearly shouldn't have such special cases if
> >> at all avoidable.
> >> 
> > User can set both DATA and CODE to same value at same time with below 
> > command:
> > xl psr-cat-set dom_id 0x3ff
> > 
> > Because no '-c' or '-d' is input, the cbm type will be 'PSR_CBM_TYPE_L3'.
> > 
> > To handle this case, we have to add a special case here. If the cbm tyep is
> > 'PSR_CBM_TYPE_L3' and the feature type is CDP, we set both DATA and CODE. This
> > should be the simplest way to handle this case.
> 
> Simplest or not, it is not really appropriate to have such special cases
> here. Along the lines of the earlier abstractions I've recommended
> (and which, at least afaic, made the overall series quite a bit more
> comprehensible), please re-consider how this can be done without
> having special case logic here (I can't immediately suggest an option,
> I'm sorry).
> 
How about a callback function here to handle this insertion? For L3/L2 CAT,
use a function just to assign new_val to val[]. For CDP, in its callback
function, check 'type' to decide insert new_val to both DATA and CODE or just
one item according to type.

> Jan

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