On 14.04.2025 09:40, Penny Zheng wrote:
> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -57,7 +57,6 @@ bool __initdata amd_virt_spec_ctrl;
> static bool __read_mostly fam17_c6_disabled;
>
> static uint64_t attr_const amd_parse_freq(unsigned char c, uint64_t value);
> -#define INVAL_FREQ_MHZ ~(uint64_t)0
>
> static inline int rdmsr_amd_safe(unsigned int msr, unsigned int *lo,
> unsigned int *hi)
> @@ -596,14 +595,13 @@ static uint64_t amd_parse_freq(unsigned char c,
> uint64_t value)
> return freq;
> }
>
> -void amd_log_freq(const struct cpuinfo_x86 *c)
> +void amd_process_freq(const struct cpuinfo_x86 *c,
Misra demands that functions only used in a single CU be static.
> + uint64_t *low_mhz, uint64_t *nom_mhz, uint64_t *hi_mhz)
See comments on patch 07 as to types used here.
> @@ -684,20 +682,21 @@ void amd_log_freq(const struct cpuinfo_x86 *c)
>
> if (idx && idx < h &&
> !rdmsr_safe(0xC0010064 + idx, val) && (val >> 63) &&
> - !rdmsr_safe(0xC0010064, hi) && (hi >> 63))
> - printk("CPU%u: %lu (%lu ... %lu) MHz\n",
> - smp_processor_id(),
> - amd_parse_freq(c->x86, val),
> - amd_parse_freq(c->x86, lo),
> - amd_parse_freq(c->x86, hi));
> - else if (h && !rdmsr_safe(0xC0010064, hi) && (hi >> 63))
> - printk("CPU%u: %lu ... %lu MHz\n",
> - smp_processor_id(),
> - amd_parse_freq(c->x86, lo),
> - amd_parse_freq(c->x86, hi));
> - else
> - printk("CPU%u: %lu MHz\n", smp_processor_id(),
> - amd_parse_freq(c->x86, lo));
> + !rdmsr_safe(0xC0010064, hi) && (hi >> 63)) {
> + if (nom_mhz)
> + *nom_mhz = amd_parse_freq(c->x86, val);
> + if (low_mhz)
> + *low_mhz = amd_parse_freq(c->x86, lo);
> + if (hi_mhz)
> + *hi_mhz = amd_parse_freq(c->x86, hi);
> + } else if (h && !rdmsr_safe(0xC0010064, hi) && (hi >> 63)) {
> + if (low_mhz)
> + *low_mhz = amd_parse_freq(c->x86, lo);
> + if (hi_mhz)
> + *hi_mhz = amd_parse_freq(c->x86, hi);
> + } else
> + if (low_mhz)
Why does this "else if()" extend across two lines?
> @@ -708,6 +707,29 @@ void cf_check early_init_amd(struct cpuinfo_x86 *c)
> ctxt_switch_levelling(NULL);
> }
>
> +void amd_log_freq(const struct cpuinfo_x86 *c)
> +{
> + uint64_t low_mhz, nom_mhz, hi_mhz;
> +
> + if (c != &boot_cpu_data &&
> + (!opt_cpu_info || (c->apicid & (c->x86_num_siblings - 1))))
> + return;
> +
> + low_mhz = nom_mhz = hi_mhz = INVAL_FREQ_MHZ;
> + amd_process_freq(c, &low_mhz, &nom_mhz, &hi_mhz);
> +
> + if (low_mhz != INVAL_FREQ_MHZ && nom_mhz != INVAL_FREQ_MHZ &&
> + hi_mhz != INVAL_FREQ_MHZ)
> + printk("CPU%u: %lu (%lu ... %lu) MHz\n",
> + smp_processor_id(),
> + low_mhz, nom_mhz, hi_mhz);
This doesn't match the original order of frequencies logged.
Jan
