* [PATCH] tools/xen-ucode: print information about currently loaded ucode
@ 2023-01-13 11:56 Sergey Dyasli
2023-01-16 15:21 ` Jan Beulich
0 siblings, 1 reply; 2+ messages in thread
From: Sergey Dyasli @ 2023-01-13 11:56 UTC (permalink / raw)
To: xen-devel
Cc: Sergey Dyasli, Wei Liu, Anthony PERARD, Juergen Gross,
Andrew Cooper, George Dunlap, Jan Beulich, Julien Grall,
Stefano Stabellini, Roger Pau Monné
Currently it's impossible to get CPU's microcode revision after late
loading without looking into Xen logs which is not always convenient.
Add an option to xen-ucode tool to print the currently loaded ucode
version and also print it during usage info.
Add a new platform op in order to get the required data from Xen.
Print CPU signature and processor flags as well.
Example output:
Intel:
Current CPU signature is: 06-55-04 (raw 0x50654)
Current CPU microcode revision is: 0x2006e05
Current CPU processor flags are: 0x1
AMD:
Current CPU signature is: fam19h (raw 0xa00f11)
Current CPU microcode revision is: 0xa0011a8
Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
CC: Juergen Gross <jgross@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: George Dunlap <george.dunlap@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Julien Grall <julien@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: "Roger Pau Monné" <roger.pau@citrix.com>
---
tools/include/xenctrl.h | 1 +
tools/libs/ctrl/xc_misc.c | 5 +++
tools/misc/xen-ucode.c | 68 +++++++++++++++++++++++++++++++
xen/arch/x86/platform_hypercall.c | 32 +++++++++++++++
xen/include/public/platform.h | 14 +++++++
xen/include/xlat.lst | 1 +
6 files changed, 121 insertions(+)
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 23037874d3..e9911da5ea 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -1185,6 +1185,7 @@ typedef uint32_t xc_node_to_node_dist_t;
int xc_physinfo(xc_interface *xch, xc_physinfo_t *info);
int xc_cputopoinfo(xc_interface *xch, unsigned *max_cpus,
xc_cputopo_t *cputopo);
+int xc_platform_op(xc_interface *xch, struct xen_platform_op *op);
int xc_microcode_update(xc_interface *xch, const void *buf, size_t len);
int xc_numainfo(xc_interface *xch, unsigned *max_nodes,
xc_meminfo_t *meminfo, uint32_t *distance);
diff --git a/tools/libs/ctrl/xc_misc.c b/tools/libs/ctrl/xc_misc.c
index 265f15ec2d..d03c240d14 100644
--- a/tools/libs/ctrl/xc_misc.c
+++ b/tools/libs/ctrl/xc_misc.c
@@ -226,6 +226,11 @@ int xc_microcode_update(xc_interface *xch, const void *buf, size_t len)
return ret;
}
+int xc_platform_op(xc_interface *xch, struct xen_platform_op *op)
+{
+ return do_platform_op(xch, op);
+}
+
int xc_cputopoinfo(xc_interface *xch, unsigned *max_cpus,
xc_cputopo_t *cputopo)
{
diff --git a/tools/misc/xen-ucode.c b/tools/misc/xen-ucode.c
index ad32face2b..c4cb4fbb50 100644
--- a/tools/misc/xen-ucode.c
+++ b/tools/misc/xen-ucode.c
@@ -12,6 +12,67 @@
#include <fcntl.h>
#include <xenctrl.h>
+static const char *intel_id = "GenuineIntel";
+static const char *amd_id = "AuthenticAMD";
+
+void show_curr_cpu(FILE *f)
+{
+ int ret;
+ xc_interface *xch;
+ struct xen_platform_op op_cpu = {0}, op_ucode = {0};
+ struct xenpf_pcpu_version *cpu_ver = &op_cpu.u.pcpu_version;
+ struct xenpf_ucode_version *ucode_ver = &op_ucode.u.ucode_version;
+ bool intel = false, amd = false;
+
+ xch = xc_interface_open(0, 0, 0);
+ if ( xch == NULL )
+ return;
+
+ op_cpu.cmd = XENPF_get_cpu_version;
+ op_cpu.interface_version = XENPF_INTERFACE_VERSION;
+ op_cpu.u.pcpu_version.xen_cpuid = 0;
+
+ ret = xc_platform_op(xch, &op_cpu);
+ if ( ret )
+ return;
+
+ op_ucode.cmd = XENPF_get_ucode_version;
+ op_ucode.interface_version = XENPF_INTERFACE_VERSION;
+ op_ucode.u.pcpu_version.xen_cpuid = 0;
+
+ ret = xc_platform_op(xch, &op_ucode);
+ if ( ret )
+ return;
+
+ 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;
+
+ 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);
+
+ if ( intel )
+ fprintf(f, "Current CPU processor flags are: %#x\n", ucode_ver->pf);
+
+ xc_interface_close(xch);
+}
+
int main(int argc, char *argv[])
{
int fd, ret;
@@ -20,11 +81,18 @@ int main(int argc, char *argv[])
struct stat st;
xc_interface *xch;
+ if ( argc >= 2 && !strcmp(argv[1], "show-cpu-info") )
+ {
+ show_curr_cpu(stdout);
+ return 0;
+ }
+
if ( argc < 2 )
{
fprintf(stderr,
"xen-ucode: Xen microcode updating tool\n"
"Usage: %s <microcode blob>\n", argv[0]);
+ show_curr_cpu(stderr);
exit(2);
}
diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
index 08ab2fea62..fcb7d81db1 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -640,6 +640,38 @@ ret_t do_platform_op(
}
break;
+ case XENPF_get_ucode_version:
+ {
+ struct xenpf_ucode_version *ver = &op->u.ucode_version;
+
+ if ( !get_cpu_maps() )
+ {
+ ret = -EBUSY;
+ break;
+ }
+
+ if ( (ver->xen_cpuid >= nr_cpu_ids) || !cpu_online(ver->xen_cpuid) )
+ {
+ ver->cpu_signature = 0;
+ ver->pf = 0;
+ ver->ucode_revision = 0;
+ }
+ else
+ {
+ const struct cpu_signature *sig = &per_cpu(cpu_sig, ver->xen_cpuid);
+
+ ver->cpu_signature = sig->sig;
+ ver->pf = sig->pf;
+ ver->ucode_revision = sig->rev;
+ }
+
+ put_cpu_maps();
+
+ if ( __copy_field_to_guest(u_xenpf_op, op, u.ucode_version) )
+ ret = -EFAULT;
+ }
+ break;
+
case XENPF_cpu_online:
{
int cpu = op->u.cpu_ol.cpuid;
diff --git a/xen/include/public/platform.h b/xen/include/public/platform.h
index 14784dfa77..1a24dc24c0 100644
--- a/xen/include/public/platform.h
+++ b/xen/include/public/platform.h
@@ -610,6 +610,19 @@ DEFINE_XEN_GUEST_HANDLE(xenpf_symdata_t);
typedef struct dom0_vga_console_info xenpf_dom0_console_t;
DEFINE_XEN_GUEST_HANDLE(xenpf_dom0_console_t);
+#define XENPF_get_ucode_version 65
+struct xenpf_ucode_version {
+ uint32_t xen_cpuid; /* IN: CPU number to get the revision from. */
+ /* Return data should be equal among all */
+ /* the CPUs. */
+ uint32_t cpu_signature; /* OUT: CPU signature (CPUID.1.EAX). */
+ uint32_t pf; /* OUT: Processor Flags. */
+ /* Only applicable to Intel. */
+ uint32_t ucode_revision; /* OUT: Microcode Revision. */
+};
+typedef struct xenpf_ucode_version xenpf_ucode_version_t;
+DEFINE_XEN_GUEST_HANDLE(xenpf_ucode_version_t);
+
/*
* ` enum neg_errnoval
* ` HYPERVISOR_platform_op(const struct xen_platform_op*);
@@ -641,6 +654,7 @@ struct xen_platform_op {
xenpf_resource_op_t resource_op;
xenpf_symdata_t symdata;
xenpf_dom0_console_t dom0_console;
+ xenpf_ucode_version_t ucode_version;
uint8_t pad[128];
} u;
};
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index d601a8a984..164f700eb6 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -157,6 +157,7 @@
? xenpf_pcpuinfo platform.h
? xenpf_pcpu_version platform.h
? xenpf_resource_entry platform.h
+? xenpf_ucode_version platform.h
? pmu_data pmu.h
? pmu_params pmu.h
! sched_poll sched.h
--
2.17.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] tools/xen-ucode: print information about currently loaded ucode
2023-01-13 11:56 [PATCH] tools/xen-ucode: print information about currently loaded ucode Sergey Dyasli
@ 2023-01-16 15:21 ` Jan Beulich
0 siblings, 0 replies; 2+ messages in thread
From: Jan Beulich @ 2023-01-16 15:21 UTC (permalink / raw)
To: Sergey Dyasli
Cc: Wei Liu, Anthony PERARD, Juergen Gross, Andrew Cooper,
George Dunlap, Julien Grall, Stefano Stabellini,
Roger Pau Monné,
xen-devel
On 13.01.2023 12:56, Sergey Dyasli wrote:
> Currently it's impossible to get CPU's microcode revision after late
> loading without looking into Xen logs which is not always convenient.
> Add an option to xen-ucode tool to print the currently loaded ucode
> version and also print it during usage info.
>
> Add a new platform op in order to get the required data from Xen.
> Print CPU signature and processor flags as well.
>
> Example output:
> Intel:
> Current CPU signature is: 06-55-04 (raw 0x50654)
> Current CPU microcode revision is: 0x2006e05
> Current CPU processor flags are: 0x1
>
> AMD:
> Current CPU signature is: fam19h (raw 0xa00f11)
So quite a bit less precise information than on Intel in the non-raw
part. Is there a reason for this?
> --- a/tools/libs/ctrl/xc_misc.c
> +++ b/tools/libs/ctrl/xc_misc.c
> @@ -226,6 +226,11 @@ int xc_microcode_update(xc_interface *xch, const void *buf, size_t len)
> return ret;
> }
>
> +int xc_platform_op(xc_interface *xch, struct xen_platform_op *op)
> +{
> + return do_platform_op(xch, op);
> +}
Wouldn't it make sense to simply rename do_platform_op()?
> --- a/tools/misc/xen-ucode.c
> +++ b/tools/misc/xen-ucode.c
> @@ -12,6 +12,67 @@
> #include <fcntl.h>
> #include <xenctrl.h>
>
> +static const char *intel_id = "GenuineIntel";
> +static const char *amd_id = "AuthenticAMD";
Do these need to be (non-const) pointers, rather than const char[]?
> +void show_curr_cpu(FILE *f)
> +{
> + int ret;
> + xc_interface *xch;
> + struct xen_platform_op op_cpu = {0}, op_ucode = {0};
Instead of the dummy initializers, can't you make ...
> + struct xenpf_pcpu_version *cpu_ver = &op_cpu.u.pcpu_version;
> + struct xenpf_ucode_version *ucode_ver = &op_ucode.u.ucode_version;
> + bool intel = false, amd = false;
> +
> + xch = xc_interface_open(0, 0, 0);
> + if ( xch == NULL )
> + return;
> +
> + op_cpu.cmd = XENPF_get_cpu_version;
> + op_cpu.interface_version = XENPF_INTERFACE_VERSION;
> + op_cpu.u.pcpu_version.xen_cpuid = 0;
... this and ...
> + ret = xc_platform_op(xch, &op_cpu);
> + if ( ret )
> + return;
> +
> + op_ucode.cmd = XENPF_get_ucode_version;
> + op_ucode.interface_version = XENPF_INTERFACE_VERSION;
> + op_ucode.u.pcpu_version.xen_cpuid = 0;
... this the initializers?
> @@ -20,11 +81,18 @@ int main(int argc, char *argv[])
> struct stat st;
> xc_interface *xch;
>
> + if ( argc >= 2 && !strcmp(argv[1], "show-cpu-info") )
> + {
> + show_curr_cpu(stdout);
> + return 0;
> + }
> +
> if ( argc < 2 )
> {
> fprintf(stderr,
> "xen-ucode: Xen microcode updating tool\n"
> "Usage: %s <microcode blob>\n", argv[0]);
> + show_curr_cpu(stderr);
> exit(2);
> }
Personally I'd find it mode logical if this remained first and you
inserted your new fragment right afterwards. That way you also don't
need to check argc twice.
> --- a/xen/arch/x86/platform_hypercall.c
> +++ b/xen/arch/x86/platform_hypercall.c
> @@ -640,6 +640,38 @@ ret_t do_platform_op(
> }
> break;
>
> + case XENPF_get_ucode_version:
> + {
> + struct xenpf_ucode_version *ver = &op->u.ucode_version;
> +
> + if ( !get_cpu_maps() )
> + {
> + ret = -EBUSY;
> + break;
> + }
> +
> + if ( (ver->xen_cpuid >= nr_cpu_ids) || !cpu_online(ver->xen_cpuid) )
> + {
> + ver->cpu_signature = 0;
> + ver->pf = 0;
> + ver->ucode_revision = 0;
Better return -ENOENT in this case?
> + }
> + else
> + {
> + const struct cpu_signature *sig = &per_cpu(cpu_sig, ver->xen_cpuid);
> +
> + ver->cpu_signature = sig->sig;
> + ver->pf = sig->pf;
> + ver->ucode_revision = sig->rev;
Here you read what is actually present, which ...
> --- a/xen/include/public/platform.h
> +++ b/xen/include/public/platform.h
> @@ -610,6 +610,19 @@ DEFINE_XEN_GUEST_HANDLE(xenpf_symdata_t);
> typedef struct dom0_vga_console_info xenpf_dom0_console_t;
> DEFINE_XEN_GUEST_HANDLE(xenpf_dom0_console_t);
>
> +#define XENPF_get_ucode_version 65
> +struct xenpf_ucode_version {
> + uint32_t xen_cpuid; /* IN: CPU number to get the revision from. */
> + /* Return data should be equal among all */
> + /* the CPUs. */
... doesn't necessarily match the promise here. Perhaps weaken the
"should", or clarify what the conditionsare for this to be the case?
Also your addition to xen-ucode builds on this, which can easily
end up misleading when it's not really the case.
> --- a/xen/include/xlat.lst
> +++ b/xen/include/xlat.lst
> @@ -157,6 +157,7 @@
> ? xenpf_pcpuinfo platform.h
> ? xenpf_pcpu_version platform.h
> ? xenpf_resource_entry platform.h
> +? xenpf_ucode_version platform.h
You also want to invoke the resulting macro, so that the intended checking
actually occurs.
Jan
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-01-16 15:21 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-13 11:56 [PATCH] tools/xen-ucode: print information about currently loaded ucode Sergey Dyasli
2023-01-16 15:21 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.