| Message ID | 20230113115630.22264-1-sergey.dyasli@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tools/xen-ucode: print information about currently loaded ucode | expand |
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
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
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(+)