Re: [PATCH v3 3/3] tools/xen-ucode: print information about currently loaded ucode

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 21/03/2023 11:47 am, Sergey Dyasli wrote:
> Add an option to xen-ucode tool to print the currently loaded ucode
> version and also print it during usage info.  Print CPU signature and
> processor flags as well.  The raw data comes from XENPF_get_cpu_version
> and XENPF_get_ucode_version platform ops.
>
> Example output:
>     Intel:
>     Current CPU signature is: 06-55-04 (raw 0x50654)
>     Current CPU microcode revision is: 0x2006e05
>     Current CPU processor flags are: 0x1

It's platform flags, not processor flags.  (And sadly, doesn't actually
capture all the platform specific data these days either...)

>
>     AMD:
>     Current CPU signature is: fam19h (raw 0xa00f11)
>     Current CPU microcode revision is: 0xa0011a8

This is unnecessarily verbose, and can fit into a single line.

CPU signature {XX-YY-ZZ / FamXXh} (raw X) {pf Y} revision Z.

> Signed-off-by: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
> ---
>  tools/misc/xen-ucode.c | 66 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
>
> diff --git a/tools/misc/xen-ucode.c b/tools/misc/xen-ucode.c
> index ad32face2b..b9037ce6a1 100644
> --- a/tools/misc/xen-ucode.c
> +++ b/tools/misc/xen-ucode.c
> @@ -12,6 +12,65 @@
>  #include <fcntl.h>
>  #include <xenctrl.h>
>  
> +static const char intel_id[] = "GenuineIntel";
> +static const char   amd_id[] = "AuthenticAMD";
> +
> +static void show_curr_cpu(FILE *f)
> +{
> +    int ret;
> +    xc_interface *xch;
> +    struct xenpf_pcpu_version cpu_ver = { .xen_cpuid = 0 };
> +    struct xenpf_ucode_version ucode_ver = { .xen_cpuid = 0 };
> +    bool intel = false, amd = false;
> +
> +    xch = xc_interface_open(0, 0, 0);

NULL, NULL, 0

but xch wants to be a global and opened once, rather than in each piece
of sub-functionality.

> +    if ( xch == NULL )
> +        return;
> +
> +    ret = xc_get_cpu_version(xch, &cpu_ver);
> +    if ( ret )
> +        return;
> +
> +    ret = xc_get_ucode_version(xch, &ucode_ver);
> +    if ( ret )
> +        return;

All 3 of these want to complain, rather than exiting silently.

See test-tsx/test-resource as examples.  It's fine to use err(1,
"message") to terminate fairly cleanly, and it renders errno to the user
which is far more useful than printing nothing and exiting success.

> +
> +    if ( memcmp(cpu_ver.vendor_id, intel_id,
> +                sizeof(cpu_ver.vendor_id)) == 0 )
> +        intel = true;
> +    else if ( memcmp(cpu_ver.vendor_id, amd_id,
> +                     sizeof(cpu_ver.vendor_id)) == 0 )
> +        amd = true;

else some kind of error, again so we don't exit silently.

> +
> +    /*
> +     * Print signature in a form that allows to quickly identify which ucode
> +     * blob to load, e.g.:
> +     *
> +     *      Intel:   /lib/firmware/intel-ucode/06-55-04
> +     *      AMD:     /lib/firmware/amd-ucode/microcode_amd_fam19h.bin

I'm not sure if this is relevant any more, but for Fam < 0x15,
everything is combined in microcode_amd.bin

In some copious free time (but not this patch), it might be nice to
support `xen-ucode --auto` to try and pick the right firmware file out
of the filesystem.  One less thing for end users to get wrong.

> +     */
> +    if ( intel )
> +    {
> +        fprintf(f, "Current CPU signature is: %02x-%02x-%02x (raw %#x)\n",
> +                   cpu_ver.family, cpu_ver.model, cpu_ver.stepping,
> +                   ucode_ver.cpu_signature);
> +    }
> +    else if ( amd )
> +    {
> +        fprintf(f, "Current CPU signature is: fam%xh (raw %#x)\n",
> +                   cpu_ver.family, ucode_ver.cpu_signature);
> +    }
> +
> +    if ( intel || amd )
> +        fprintf(f, "Current CPU microcode revision is: %#x\n",
> +                   ucode_ver.ucode_revision);

For the two raw fields, and the revision field, it's important to use 0x08x.

They're all exactly 32bit quantities, and both vendors encode data in
the higher bits, so visually it's very useful to know if you're looking
at the top of penultimate nibble.

~Andrew