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

[Serhii]

Yes, the only essential change is shift.
However, I believe that handling the two paths separately is better
because (a) using different constants better highlights the semantic
difference of the values stored and (b) AMD currently reserves upper
bits, which could require handling in the future making separate paths
more appropriate.
What do you think, is it still better to merge back into a single path
with conditional shift for Intel?


On Mon, Feb 23, 2026 at 10:30 PM K Prateek Nayak <kprateek.nayak@xxxxxxx> wrote:
>
> On 2/24/2026 8:07 AM, Serhii Pievniev wrote:
> > @@ -9139,7 +9150,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));
>
> Since "MSR_IA32_UCODE_REV" and "MSR_AMD64_PATCH_LEVEL" are the same,
> all that essentially changes is the shift.
>
> Can't we just 0 the shift for "authentic_amd || hygon_genuine"?
>
> > +                     fprintf(outf, " microcode 0x%llx", ucode_patch);
> >               fputc('\n', outf);
> >
> >               fprintf(outf, "CPUID(0x80000000): max_extended_levels: 0x%x\n", max_extended_level);
>
> --
> Thanks and Regards,
> Prateek
>