When _CPC table could not provide processor frequency range
values for Xen governor, we need to read processor max frequency
as anchor point.

For AMD processors, we export max frequency value from amd_log_freq()

Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
---
v1 -> v2:
- new commit
---
xen/amd: export processor max frequency value

When _CPC table could not provide processor frequency range
values for Xen governor, we need to read processor max frequency
as anchor point.

For AMD processors, we export max frequency value from amd_log_freq()

Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
---
v1 -> v2:
- new commit
---
v2 -> v3:
- Replace per_cpu with this_cpu
- Add amd_ prefix for AMD-only variable
---
 xen/arch/x86/cpu/amd.c         | 10 +++++++++-
 xen/arch/x86/include/asm/amd.h |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 7fb1d76798..6ab1cff3e5 100644
--- 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, amd_max_freq_mhz);
+
 static inline int rdmsr_amd_safe(unsigned int msr, unsigned int *lo,
 				 unsigned int *hi)
 {
@@ -681,9 +683,15 @@ void amd_log_freq(const struct cpuinfo_x86 *c)
 		printk("CPU%u: %lu ... %lu MHz\n",
 		       smp_processor_id(),
 		       amd_parse_freq(c, lo), amd_parse_freq(c, hi));
-	else
+	else {
 		printk("CPU%u: %lu MHz\n", smp_processor_id(),
 		       amd_parse_freq(c, lo));
+		return;
+	}
+
+	/* Store max frequency for amd-cppc cpufreq driver */
+	if (hi >> 63)
+		this_cpu(amd_max_freq_mhz) = amd_parse_freq(c, hi);
 }
 
 void cf_check early_init_amd(struct cpuinfo_x86 *c)
diff --git a/xen/arch/x86/include/asm/amd.h b/xen/arch/x86/include/asm/amd.h
index 9c9599a622..cf9177c00a 100644
--- a/xen/arch/x86/include/asm/amd.h
+++ b/xen/arch/x86/include/asm/amd.h
@@ -174,4 +174,5 @@ bool amd_setup_legacy_ssbd(void);
 void amd_set_legacy_ssbd(bool enable);
 void amd_set_cpuid_user_dis(bool enable);
 
+DECLARE_PER_CPU(uint64_t, amd_max_freq_mhz);
 #endif /* __AMD_H__ */
-- 
2.34.1

On 06.03.2025 09:39, 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, amd_max_freq_mhz);
> +
>  static inline int rdmsr_amd_safe(unsigned int msr, unsigned int *lo,
>  				 unsigned int *hi)
>  {
> @@ -681,9 +683,15 @@ void amd_log_freq(const struct cpuinfo_x86 *c)
>  		printk("CPU%u: %lu ... %lu MHz\n",
>  		       smp_processor_id(),
>  		       amd_parse_freq(c, lo), amd_parse_freq(c, hi));
> -	else
> +	else {
>  		printk("CPU%u: %lu MHz\n", smp_processor_id(),
>  		       amd_parse_freq(c, lo));
> +		return;
> +	}
> +
> +	/* Store max frequency for amd-cppc cpufreq driver */
> +	if (hi >> 63)
> +		this_cpu(amd_max_freq_mhz) = amd_parse_freq(c, hi);
>  }

As before - typically only the BSP will make it here, due to the conditional
at the top of the function. IOW you'll observe zeros in the per-CPU data for
all other CPUs.

> --- a/xen/arch/x86/include/asm/amd.h
> +++ b/xen/arch/x86/include/asm/amd.h
> @@ -174,4 +174,5 @@ bool amd_setup_legacy_ssbd(void);
>  void amd_set_legacy_ssbd(bool enable);
>  void amd_set_cpuid_user_dis(bool enable);
>  
> +DECLARE_PER_CPU(uint64_t, amd_max_freq_mhz);
>  #endif /* __AMD_H__ */

I'm also pretty sure that I did ask before to maintain a blank line ahead of
the #endif. Please may I ask that you thoroughly address earlier review
comments, before submitting a new version?

Jan

[Public]

Hi,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Monday, March 24, 2025 11:52 PM
> To: Penny, Zheng <penny.zheng@amd.com>
> Cc: Huang, Ray <Ray.Huang@amd.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; Roger Pau Monné <roger.pau@citrix.com>; xen-
> devel@lists.xenproject.org
> Subject: Re: [PATCH v3 08/15] xen/amd: export processor max frequency value
>
> On 06.03.2025 09:39, 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, amd_max_freq_mhz);
> > +
> >  static inline int rdmsr_amd_safe(unsigned int msr, unsigned int *lo,
> >                              unsigned int *hi)
> >  {
> > @@ -681,9 +683,15 @@ void amd_log_freq(const struct cpuinfo_x86 *c)
> >             printk("CPU%u: %lu ... %lu MHz\n",
> >                    smp_processor_id(),
> >                    amd_parse_freq(c, lo), amd_parse_freq(c, hi));
> > -   else
> > +   else {
> >             printk("CPU%u: %lu MHz\n", smp_processor_id(),
> >                    amd_parse_freq(c, lo));
> > +           return;
> > +   }
> > +
> > +   /* Store max frequency for amd-cppc cpufreq driver */
> > +   if (hi >> 63)
> > +           this_cpu(amd_max_freq_mhz) = amd_parse_freq(c, hi);
> >  }
>
> As before - typically only the BSP will make it here, due to the conditional at the top
> of the function. IOW you'll observe zeros in the per-CPU data for all other CPUs.
>

I'll extract the processing frequency logic into a new helper, maybe amd_process_freq()

> > --- a/xen/arch/x86/include/asm/amd.h
> > +++ b/xen/arch/x86/include/asm/amd.h
> > @@ -174,4 +174,5 @@ bool amd_setup_legacy_ssbd(void);  void
> > amd_set_legacy_ssbd(bool enable);  void amd_set_cpuid_user_dis(bool
> > enable);
> >
> > +DECLARE_PER_CPU(uint64_t, amd_max_freq_mhz);
> >  #endif /* __AMD_H__ */
>
> I'm also pretty sure that I did ask before to maintain a blank line ahead of the
> #endif. Please may I ask that you thoroughly address earlier review comments,
> before submitting a new version?
>

Sorry, I'll be more careful.

> Jan