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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.06.17 at 09:59, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> On 17-05-31 03:44:31, Jan Beulich wrote:
>> >>> On 03.05.17 at 10:44, <yi.y.sun@xxxxxxxxxxxxxxx> 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).

Jan


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