Re: [PATCH 3/3] x86/Intel: also display CPU freq for family 0xf

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Feb 10, 2022 at 03:56:48PM +0100, Jan Beulich wrote:
> Actually we can do better than simply bailing for there not being any
> PLATFORM_INFO MSR on these. The "max" part of the information is
> available in another MSR, alongside the scaling factor (which is
> encoded in similar ways to Core/Core2, and hence the decoding table can
> be shared).
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

> ---
> The inner switch() is left indented one level too much (and with an
> extra pair of braces) to limit the diff. I'd prefer to make a follow-up
> patch reducing the indentation, unless I'm told to do so right here.

I'm fine with a followup patch.

> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -412,9 +412,9 @@ static int num_cpu_cores(struct cpuinfo_
>  
>  static void intel_log_freq(const struct cpuinfo_x86 *c)
>  {
> -    unsigned int eax, ebx, ecx, edx;
> +    unsigned int eax, ebx, ecx, edx, factor;
>      uint64_t msrval;
> -    uint8_t max_ratio;
> +    uint8_t max_ratio, min_ratio;
>  
>      if ( c->cpuid_level >= 0x15 )
>      {
> @@ -455,21 +455,22 @@ static void intel_log_freq(const struct
>          }
>      }
>  
> -    if ( c->x86 == 0xf || rdmsr_safe(MSR_INTEL_PLATFORM_INFO, msrval) )
> -        return;
> -    max_ratio = msrval >> 8;
> -
> -    if ( max_ratio )
> +    switch ( c->x86 )
>      {
> -        unsigned int factor = 10000;
> -        uint8_t min_ratio = msrval >> 40;
> +        static const unsigned short core_factors[] =

This no longer applies to Core models only, so I wouldn't be opposed
to renaming to scaling_factors or similar.

Thanks, Roger.