Re: [PATCH v2] tools/turbostat: fix microcode patch level reading for AMD/Hygon

[Len Brown]

Applied, thanks!

ps. I added a Fixes tag...
Fixes: 3e4048466c39 ("tools/power turbostat: Add --no-msr option")

That is in Linux-6.9, where this cleanly applies.  Technically the logic
was already broken before that in an earlier 6.9 patch, and the original code
was broken years before that, but I expect going back 2-years is more
than sufficient.

On Wed, Feb 25, 2026 at 6:16 PM Serhii Pievniev <spevnev16@xxxxxxxxx> wrote:
>
> turbostat always used the same logic to read the microcode patch level,
> which is correct for Intel but not for AMD/Hygon.
> While Intel stores the patch level in the upper 32 bits of MSR, AMD
> stores it in the lower 32 bits, which causes turbostat to report the
> microcode version as 0x0 on AMD/Hygon.
>
> Fix by shifting right by 32 for non-AMD/Hygon, preserving the existing
> behavior for Intel and unknown vendors.
>
> Signed-off-by: Serhii Pievniev <spevnev16@xxxxxxxxx>
> ---
> v1 -> v2: Changed to single MSR path with conditional shift
>
> v1: https://lore.kernel.org/linux-pm/20260224023719.65165-1-spevnev16@xxxxxxxxx
> ---
>  tools/power/x86/turbostat/turbostat.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> index 1a2671c2820..7545142b3a6 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -9122,10 +9122,13 @@ void process_cpuid()
>         cpuid_has_hv = ecx_flags & (1 << 31);
>
>         if (!no_msr) {
> -               if (get_msr(sched_getcpu(), MSR_IA32_UCODE_REV, &ucode_patch))
> +               if (get_msr(sched_getcpu(), MSR_IA32_UCODE_REV, &ucode_patch)) {
>                         warnx("get_msr(UCODE)");
> -               else
> +               } else {
>                         ucode_patch_valid = true;
> +                       if (!authentic_amd && !hygon_genuine)
> +                               ucode_patch >>= 32;
> +               }
>         }
>
>         /*
> @@ -9139,7 +9142,7 @@ void process_cpuid()
>         if (!quiet) {
>                 fprintf(outf, "CPUID(1): family:model:stepping 0x%x:%x:%x (%d:%d:%d)", family, model, stepping, family, model, stepping);
>                 if (ucode_patch_valid)
> -                       fprintf(outf, " microcode 0x%x", (unsigned int)((ucode_patch >> 32) & 0xFFFFFFFF));
> +                       fprintf(outf, " microcode 0x%x", (unsigned int)ucode_patch);
>                 fputc('\n', outf);
>
>                 fprintf(outf, "CPUID(0x80000000): max_extended_levels: 0x%x\n", max_extended_level);
> --
> 2.53.0
>


-- 
Len Brown, Intel Open Source Technology Center