RE: [PATCH v5 08/18] xen/cpu: Expand core frequency calculation for AMD Family 1Ah CPUs

["Penny, Zheng" <penny.zheng@xxxxxxx>]

[Public]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: Thursday, June 12, 2025 9:08 PM
> To: Penny, Zheng <penny.zheng@xxxxxxx>
> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v5 08/18] xen/cpu: Expand core frequency calculation for
> AMD Family 1Ah CPUs
>
> On 27.05.2025 10:48, Penny Zheng wrote:
> > --- a/xen/arch/x86/cpu/amd.c
> > +++ b/xen/arch/x86/cpu/amd.c
> > @@ -583,12 +583,40 @@ static void amd_get_topology(struct cpuinfo_x86 *c)
> >                                                            :
> > c->cpu_core_id);  }
> >
> > +static unsigned int attr_const amd_parse_freq(unsigned int family,
> > +                                         unsigned int value)
> > +{
> > +   unsigned int freq = 0;
> > +
> > +   switch (family) {
> > +   case 0x10 ... 0x16:
> > +           freq = (((value & 0x3f) + 0x10) * 100) >> ((value >> 6) & 7);
> > +           break;
> > +
> > +   case 0x17 ... 0x19:
> > +           freq = ((value & 0xff) * 25 * 8) / ((value >> 8) & 0x3f);
> > +           break;
> > +
> > +   case 0x1A:
> > +           freq = (value & 0xfff) * 5;
> > +           break;
> > +
> > +   default:
> > +           printk(XENLOG_ERR
> > +                  "Unsupported cpu family 0x%x on cpufreq parsing",
> > +                  family);
>
> I think I requested before (elsewhere) to prefer %#x over 0x%x.
>
> However, why the log message? With ...
>
> > +           break;
> > +   }
> > +
> > +   return freq;
> > +}
> > +
> >  void amd_log_freq(const struct cpuinfo_x86 *c)  {
> >     unsigned int idx = 0, h;
> >     uint64_t hi, lo, val;
> >
> > -   if (c->x86 < 0x10 || c->x86 > 0x19 ||
> > +   if (c->x86 < 0x10 || c->x86 > 0x1A ||
>
> ... this condition, there simply could be ASSERT_UNREACHABLE() there? Happy
> to adjust while committing, so long as you agree. With the adjustment:
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>

Agreed, thx

> Jan