Re: [Xen-devel] [PATCH v4 04/15] x86: implement data structure and CPU init flow for MBA

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.09.17 at 10:38, <roger.pau@xxxxxxxxxx> wrote:
> On Sat, Sep 23, 2017 at 09:48:13AM +0000, Yi Sun wrote:
>> @@ -332,20 +355,58 @@ static int cat_init_feature(const struct cpuid_leaf 
>> *regs,
>>      }
>>  
>>      default:
>> -        return -ENOENT;
>> +        return false;
>>      }
>>  
>>      /* Add this feature into array. */
>>      info->features[type] = feat;
>>  
>>      if ( !opt_cpu_info )
>> -        return 0;
>> +        return true;
>>  
>>      printk(XENLOG_INFO "%s: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
>>             cat_feat_name[type], cpu_to_socket(smp_processor_id()),
>> -           feat->cos_max, feat->cbm_len);
>> +           feat->cos_max, feat->cat.cbm_len);
> 
> I would rather do:
> 
> if ( opt_cpu_info )
>     printk(...);
> 
> return true;
> 
> So that the function has a single return path for the success case.

But not in this patch, which only partly changes what's already
there. Yet fundamentally I agree.

>> -    return 0;
>> +    return true;
>> +}
>> +
>> +static bool mba_init_feature(const struct cpuid_leaf *regs,
>> +                            struct feat_node *feat,
>> +                            struct psr_socket_info *info,
>> +                            enum psr_feat_type type)
>> +{
>> +    /* No valid value so do not enable feature. */
>> +    if ( !regs->a || !regs->d || type != FEAT_TYPE_MBA )
>> +        return false;
>> +
>> +    feat->cos_max = min(opt_cos_max, regs->d & CAT_COS_MAX_MASK);
>> +    if ( feat->cos_max < 1 )
>> +        return false;
>> +
>> +    feat->mba.thrtl_max = (regs->a & MBA_THRTL_MAX_MASK) + 1;
>> +
>> +    if ( regs->c & MBA_LINEAR_MASK )
>> +    {
>> +        feat->mba.linear = true;
>> +
>> +        if ( feat->mba.thrtl_max >= 100 )
>> +            return false;
>> +    }
>> +
>> +    wrmsrl(MSR_IA32_PSR_MBA_MASK(0), 0);
>> +
>> +    /* Add this feature into array. */
>> +    info->features[type] = feat;
>> +
>> +    if ( !opt_cpu_info )
>> +        return true;
>> +
>> +    printk(XENLOG_INFO "MBA: enabled on socket %u, cos_max:%u, 
>> thrtl_max:%u, linear:%u.\n",
>                          ^ newline.

And no full stop please.

Jan


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