Re: [PATCH v3 07/15] xen/cpufreq: fix core frequency calculation for AMD Family 1Ah CPUs

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

On 2025-03-24 16:47, Jan Beulich wrote:
> On 06.03.2025 09:39, Penny Zheng wrote:
>> This commit fixes core frequency calculation for AMD Family 1Ah CPUs, 
>> due to
>> a change in the PStateDef MSR layout in AMD Family 1Ah+.
>> In AMD Family 1Ah+, Core current operating frequency in MHz is 
>> calculated as
>> follows:
> 

[...]

> 
>> @@ -658,19 +670,20 @@ void amd_log_freq(const struct cpuinfo_x86 *c)
>>  	if (!(lo >> 63))
>>  		return;
>> 
>> -#define FREQ(v) (c->x86 < 0x17 ? ((((v) & 0x3f) + 0x10) * 100) >> 
>> (((v) >> 6) & 7) \
>> -		                     : (((v) & 0xff) * 25 * 8) / (((v) >> 8) & 
>> 0x3f))
>>  	if (idx && idx < h &&
>>  	    !rdmsr_safe(0xC0010064 + idx, val) && (val >> 63) &&
>>  	    !rdmsr_safe(0xC0010064, hi) && (hi >> 63))
>>  		printk("CPU%u: %lu (%lu ... %lu) MHz\n",
>> -		       smp_processor_id(), FREQ(val), FREQ(lo), FREQ(hi));
>> +		       smp_processor_id(),
>> +		       amd_parse_freq(c, val),
>> +		       amd_parse_freq(c, lo), amd_parse_freq(c, hi));
> 
> I fear Misra won't like multiple function calls to evaluate the 
> parameters
> to pass to another function. Iirc smp_process_id() has special 
> exception,
> so that's okay here. This may be possible to alleviate by marking the 
> new
> helper pure or even const (see gcc doc as to caveats with passing 
> pointers
> to const functions). Cc-ing Nicola for possible clarification or 
> correction.
> 
> Jan

Yes, it would help. Currently there is only a property for 
smp_processor_id(), though there has been some discussion in the past 
about adding a formal deviation. Not a big problem either way since 
currently the rule is non-blocking, but definitely an attribute would 
help any future work on making that clean.

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253