| Message ID | 20230321114745.11044-3-sergey.dyasli@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen-ucode: print information about currently loaded ucode | expand |
On 21/03/2023 11:47 am, 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. It's not impossible (you can do `modprobe msr; rdmsr 0x8b`), but it is inconvenient for library code. > > Add a new platform op in order to get the required data from Xen and > provide a wrapper for libxenctrl. > > Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com> > --- > tools/include/xenctrl.h | 2 ++ > tools/libs/ctrl/xc_misc.c | 21 +++++++++++++++++ > xen/arch/x86/platform_hypercall.c | 30 ++++++++++++++++++++++++ > xen/arch/x86/x86_64/platform_hypercall.c | 4 ++++ > xen/include/public/platform.h | 12 ++++++++++ > xen/include/xlat.lst | 1 + > 6 files changed, 70 insertions(+) > > diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h > index 8aa747dc2e..d3ef7a48a5 100644 > --- a/tools/include/xenctrl.h > +++ b/tools/include/xenctrl.h > @@ -1187,6 +1187,8 @@ int xc_cputopoinfo(xc_interface *xch, unsigned *max_cpus, > xc_cputopo_t *cputopo); > int xc_microcode_update(xc_interface *xch, const void *buf, size_t len); > int xc_get_cpu_version(xc_interface *xch, struct xenpf_pcpu_version *cpu_ver); > +int xc_get_ucode_version(xc_interface *xch, > + struct xenpf_ucode_version *ucode_ver); Throughout, we should use "revision" rather than "version" as that is the terminology used by both Intel and AMD. > int xc_numainfo(xc_interface *xch, unsigned *max_nodes, > xc_meminfo_t *meminfo, uint32_t *distance); > int xc_pcitopoinfo(xc_interface *xch, unsigned num_devs, > diff --git a/tools/libs/ctrl/xc_misc.c b/tools/libs/ctrl/xc_misc.c > index f2f6e4348e..b93477d189 100644 > --- a/tools/libs/ctrl/xc_misc.c > +++ b/tools/libs/ctrl/xc_misc.c > @@ -246,6 +246,27 @@ int xc_get_cpu_version(xc_interface *xch, struct xenpf_pcpu_version *cpu_ver) > return 0; > } > > +int xc_get_ucode_version(xc_interface *xch, > + struct xenpf_ucode_version *ucode_ver) > +{ > + int ret; > + DECLARE_PLATFORM_OP; > + > + if ( !xch || !ucode_ver ) > + return -1; > + > + platform_op.cmd = XENPF_get_ucode_version; > + platform_op.u.ucode_version.xen_cpuid = ucode_ver->xen_cpuid; Same notes as per patch 1. > diff --git a/xen/include/public/platform.h b/xen/include/public/platform.h > index 60caa5ce7e..232df79d5f 100644 > --- a/xen/include/public/platform.h > +++ b/xen/include/public/platform.h > @@ -614,6 +614,17 @@ 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. */ We commonly call this just "cpu". As a platform op pertaining to microcode, it cannot plausibly be confused with vcpus. > + uint32_t cpu_signature; /* OUT: CPU signature (CPUID.1.EAX). */ The cpu_ prefix can be dropped, as can ... > + uint32_t pf; /* OUT: Processor Flags. */ > + /* Only applicable to Intel. */ > + uint32_t ucode_revision; /* OUT: Microcode Revision. */ the ucode_ prefix here. I'm tempted to say that I'm happy to fix all of this on commit as it's all mechanical changes. ~Andrew
On 21.03.2023 12:47, Sergey Dyasli wrote: > --- a/xen/arch/x86/platform_hypercall.c > +++ b/xen/arch/x86/platform_hypercall.c > @@ -640,6 +640,36 @@ 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) ) > + { > + ret = -ENOENT; > + } Nit: Unnecessary braces. > + 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; > + } While I don't mean to insist on making this work right in this patch, I'd like to re-raise my earlier comment regarding it also being of possible interest to know ucode revisions for parked CPUs (they are, after all, an active entity in the system). Parked CPUs have their per-CPU data retained, so getting at that data shouldn't be overly difficult (the main aspect being to tell parked from fully offline CPUs). Jan
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h index 8aa747dc2e..d3ef7a48a5 100644 --- a/tools/include/xenctrl.h +++ b/tools/include/xenctrl.h @@ -1187,6 +1187,8 @@ int xc_cputopoinfo(xc_interface *xch, unsigned *max_cpus, xc_cputopo_t *cputopo); int xc_microcode_update(xc_interface *xch, const void *buf, size_t len); int xc_get_cpu_version(xc_interface *xch, struct xenpf_pcpu_version *cpu_ver); +int xc_get_ucode_version(xc_interface *xch, + struct xenpf_ucode_version *ucode_ver); int xc_numainfo(xc_interface *xch, unsigned *max_nodes, xc_meminfo_t *meminfo, uint32_t *distance); int xc_pcitopoinfo(xc_interface *xch, unsigned num_devs, diff --git a/tools/libs/ctrl/xc_misc.c b/tools/libs/ctrl/xc_misc.c index f2f6e4348e..b93477d189 100644 --- a/tools/libs/ctrl/xc_misc.c +++ b/tools/libs/ctrl/xc_misc.c @@ -246,6 +246,27 @@ int xc_get_cpu_version(xc_interface *xch, struct xenpf_pcpu_version *cpu_ver) return 0; } +int xc_get_ucode_version(xc_interface *xch, + struct xenpf_ucode_version *ucode_ver) +{ + int ret; + DECLARE_PLATFORM_OP; + + if ( !xch || !ucode_ver ) + return -1; + + platform_op.cmd = XENPF_get_ucode_version; + platform_op.u.ucode_version.xen_cpuid = ucode_ver->xen_cpuid; + + ret = do_platform_op(xch, &platform_op); + if ( ret != 0 ) + return ret; + + *ucode_ver = platform_op.u.ucode_version; + + return 0; +} + int xc_cputopoinfo(xc_interface *xch, unsigned *max_cpus, xc_cputopo_t *cputopo) { diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c index a2d9526355..d0818fea47 100644 --- a/xen/arch/x86/platform_hypercall.c +++ b/xen/arch/x86/platform_hypercall.c @@ -640,6 +640,36 @@ 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) ) + { + ret = -ENOENT; + } + 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/arch/x86/x86_64/platform_hypercall.c b/xen/arch/x86/x86_64/platform_hypercall.c index 5bf6b958d2..b876fd0c4a 100644 --- a/xen/arch/x86/x86_64/platform_hypercall.c +++ b/xen/arch/x86/x86_64/platform_hypercall.c @@ -28,6 +28,10 @@ CHECK_pf_pcpuinfo; CHECK_pf_pcpu_version; #undef xen_pf_pcpu_version +#define xen_pf_ucode_version xenpf_ucode_version +CHECK_pf_ucode_version; +#undef xen_pf_pucode_version + #define xen_pf_enter_acpi_sleep xenpf_enter_acpi_sleep CHECK_pf_enter_acpi_sleep; #undef xen_pf_enter_acpi_sleep diff --git a/xen/include/public/platform.h b/xen/include/public/platform.h index 60caa5ce7e..232df79d5f 100644 --- a/xen/include/public/platform.h +++ b/xen/include/public/platform.h @@ -614,6 +614,17 @@ 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. */ + 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*); @@ -645,6 +656,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 a new platform op in order to get the required data from Xen and provide a wrapper for libxenctrl. Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com> --- tools/include/xenctrl.h | 2 ++ tools/libs/ctrl/xc_misc.c | 21 +++++++++++++++++ xen/arch/x86/platform_hypercall.c | 30 ++++++++++++++++++++++++ xen/arch/x86/x86_64/platform_hypercall.c | 4 ++++ xen/include/public/platform.h | 12 ++++++++++ xen/include/xlat.lst | 1 + 6 files changed, 70 insertions(+)