[AMD Official Use Only - AMD Internal Distribution Only]
Hi,
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: Tuesday, February 11, 2025 9:57 PM
> To: Penny, Zheng <penny.zh...@amd.com>
> Cc: Huang, Ray <ray.hu...@amd.com>; Andryuk, Jason
> <jason.andr...@amd.com>; Andrew Cooper <andrew.coop...@citrix.com>;
> Roger Pau Monné <roger....@citrix.com>; xen-devel@lists.xenproject.org
> 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...
May I ask what is the more addressed specific reason for not applying to Fam1A?
>
> Jan
Many thanks,
Penny
