Re: [Xen-devel] [PATCH v2 2/4] x86: add domctl cmd to set/get CDP code/data CBM

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.09.15 at 09:24, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
> On Wed, Sep 09, 2015 at 01:16:46PM +0800, He Chen wrote:
>> -int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm)
>> +int psr_get_l3_cbm(struct domain *d, unsigned int socket,
>> +                   uint64_t *cbm, enum cbm_type type)
>>  {
>>      struct psr_cat_socket_info *info = get_cat_socket_info(socket);
>>  
>>      if ( IS_ERR(info) )
>>          return PTR_ERR(info);
>>  
>> -    *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
>> +    if ( type == PSR_CBM_TYPE_L3 )
>> +    {
>> +        if ( info->cdp_enabled )
>> +            return -EXDEV;
>> +        *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
>> +    }
>> +    else if ( type == PSR_CBM_TYPE_L3_CODE )
>> +    {
>> +        if ( !info->cdp_enabled )
>> +            return -EXDEV;
>> +        *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.code;
>> +    }
>> +    else
>> +    {
>> +        if ( !info->cdp_enabled )
>> +            return -EXDEV;
>> +        *cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.data;
>> +    }
> 
> Could I suggest to use switch? And the check can be done in advance.

Indeed. And please consider carefully what the default case ought
to be.

>> @@ -371,65 +389,136 @@ static int write_l3_cbm(unsigned int socket, unsigned 
>> int cos,
>>      return 0;
>>  }
>>  
>> -int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
>> +static int find_cos(struct psr_cat_cbm *map, int cos_max,
>> +                    uint64_t cbm_code, uint64_t cbm_data, bool_t 
>> cdp_enabled)
>>  {
>> -    unsigned int old_cos, cos;
>> -    struct psr_cat_cbm *map, *found = NULL;
>> +    unsigned int cos;
>> +
>> +    if ( !cdp_enabled )
>> +    {
>> +        for ( cos = 0; cos <= cos_max; cos++ )
>> +            if ( map[cos].ref && map[cos].u.cbm == cbm_code )
>> +                return cos;
>> +    }
>> +    else
>> +    {
>> +        for ( cos = 0; cos <= cos_max; cos++ )
>> +            if ( map[cos].ref && map[cos].u.cdp.code == cbm_code &&
>> +                 map[cos].u.cdp.data == cbm_data )
>> +                return cos;
>> +    }
> 
> I guess the '{' and '}' can be omitted if the body contains only one
> sentence.

Not really: They are required on the outer if(), and hence for
consistency they should be retained on the corresponding else.

Jan


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