Re: [Xen-devel] [PATCH v10 10/25] x86: refactor psr: L3 CAT: set value: assemble features value array.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.04.17 at 07:55, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> On 17-04-11 09:11:20, Jan Beulich wrote:
>> >>> On 01.04.17 at 15:53, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
>> > @@ -611,7 +679,40 @@ static int insert_val_to_array(uint32_t val[],
>> >                                 enum cbm_type type,
>> >                                 uint32_t new_val)
>> >  {
>> > -    return -EINVAL;
>> > +    const struct feat_node *feat;
>> > +    unsigned int i;
>> > +
>> > +    ASSERT(feat_type < PSR_SOCKET_MAX_FEAT);
>> > +
>> > +    /* Insert new value into array according to feature's position in 
>> > array. */
>> > +    for ( i = 0; i < feat_type; i++ )
>> > +    {
>> > +        feat = info->features[i];
>> > +        if ( !feat )
>> > +            continue;
>> > +
>> > +        if ( array_len <= feat->props->cos_num )
>> > +            return -ENOSPC;
>> > +
>> > +        array_len -= feat->props->cos_num;
>> > +
>> > +        val += feat->props->cos_num;
>> > +    }
>> > +
>> > +    feat = info->features[feat_type];
>> > +    if ( !feat )
>> > +        return -ENOENT;
>> > +
>> > +    if ( array_len < feat->props->cos_num )
>> > +        return -ENOSPC;
>> > +
>> > +    if ( !psr_check_cbm(feat->props->cbm_len, new_val) )
>> > +        return -EINVAL;
>> > +
>> > +    /* Value setting position is same as feature array. */
>> > +    val[0] = new_val;
>> 
>> How come this is array index 0 unconditionally, when cos_num
>> may be greater than 1?
>> 
> This patch is to implement L3 CAT so I do not introduce 'type[]' yet. The
> mechanism will be introduced in CDP patch.

Which imo is wrong, as I've pointed out in a later patch. The
moment you have the "cos_num" field (instead of implying 1
everywhere) you ought to be dealing with the field having a
value other than 1. Otherwise, as that later patch shows, you'd
then re-write _part_ of the logic without in fact _newly_
introducing "cos_num" being a variable.

Jan


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