Re: [PATCH v2 04/11] xen/amd: export processor max frequency value

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.02.2025 07:14, Penny, Zheng wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi,
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: Tuesday, February 11, 2025 9:57 PM
>> To: Penny, Zheng <penny.zheng@xxxxxxx>
>> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; Andryuk, Jason
>> <Jason.Andryuk@xxxxxxx>; Andrew Cooper <andrew.cooper3@xxxxxxxxxx>;
>> Roger Pau Monné <roger.pau@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
>> Subject: Re: [PATCH v2 04/11] xen/amd: export processor max frequency value
>>
>> On 06.02.2025 09:32, Penny Zheng wrote:
>>> --- a/xen/arch/x86/cpu/amd.c
>>> +++ b/xen/arch/x86/cpu/amd.c
>>> @@ -56,6 +56,8 @@ bool __initdata amd_virt_spec_ctrl;
>>>
>>>  static bool __read_mostly fam17_c6_disabled;
>>>
>>> +DEFINE_PER_CPU_READ_MOSTLY(uint64_t, max_freq_mhz);
>>
>> Such an AMD-only variable would better have an amd_ prefix.
>>
>>> @@ -669,7 +671,12 @@ void amd_log_freq(const struct cpuinfo_x86 *c)
>>>             printk("CPU%u: %lu ... %lu MHz\n",
>>>                    smp_processor_id(), FREQ(lo), FREQ(hi));
>>>     else
>>> +   {
>>>             printk("CPU%u: %lu MHz\n", smp_processor_id(), FREQ(lo));
>>> +           return;
>>> +   }
>>> +
>>> +   per_cpu(max_freq_mhz, smp_processor_id()) = FREQ(hi);
>>
>> this_cpu() please, or latch the result of smp_processor_id() into a local 
>> variable
>> (there are further uses in the function which then would want replacing).
>>
>> The function has "log" in its name for a reason. Did you look at the 
>> conditional at its
>> very top? You won't get here for all CPUs. You won't get here at all for 
>> Fam1A
>> CPUs, as for them the logic will first need amending.
> 
> Sorry to overlook that
> Then I shall add a specific amd_export_cpufreq_mhz to cover all scenarios...
> For Fam1A, I could think of bringing back early DMI method right now...

How reliable is DMI data going to be? Not to speak of it being available
everwhere.

> May I ask what is the more addressed specific reason for not applying to 
> Fam1A?

I'm sorry, I may not understand the question. What I understand was already
addressed by me having said "for them the logic will first need amending".

Jan