* [PATCH v2 0/3] xen-ucode: print information about currently loaded ucode
@ 2023-02-28 17:39 Sergey Dyasli
2023-02-28 17:39 ` [PATCH v2/RFC 1/3] xen/hypfs: add initial cpuinfo directory Sergey Dyasli
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Sergey Dyasli @ 2023-02-28 17:39 UTC (permalink / raw
To: xen-devel
Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu,
Anthony PERARD, Juergen Gross, George Dunlap, Julien Grall,
Stefano Stabellini, Sergey Dyasli
I've split the patch into 3 parts. And now I'm using xenhypfs instead of
introducing another platform op. That's my first attempt at xenhypfs and
the patch itself is of RFC quality. Open questions are where to put the
new code and if it's possible to come up with a better hypfs functions.
Sergey Dyasli (3):
xen/hypfs: add initial cpuinfo directory
tools/xenctrl: add xc_get_cpu_version()
tools/xen-ucode: print information about currently loaded ucode
tools/include/xenctrl.h | 1 +
tools/libs/ctrl/xc_misc.c | 20 ++++++++
tools/misc/Makefile | 2 +-
tools/misc/xen-ucode.c | 97 +++++++++++++++++++++++++++++++++++++++
xen/arch/x86/cpu/common.c | 58 +++++++++++++++++++++++
5 files changed, 177 insertions(+), 1 deletion(-)
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2/RFC 1/3] xen/hypfs: add initial cpuinfo directory
2023-02-28 17:39 [PATCH v2 0/3] xen-ucode: print information about currently loaded ucode Sergey Dyasli
@ 2023-02-28 17:39 ` Sergey Dyasli
2023-02-28 17:39 ` [PATCH v2 2/3] tools/xenctrl: add xc_get_cpu_version() Sergey Dyasli
2023-02-28 17:39 ` [PATCH v2 3/3] tools/xen-ucode: print information about currently loaded ucode Sergey Dyasli
2 siblings, 0 replies; 8+ messages in thread
From: Sergey Dyasli @ 2023-02-28 17:39 UTC (permalink / raw
To: xen-devel
Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu,
Anthony PERARD, Juergen Gross, George Dunlap, Julien Grall,
Stefano Stabellini, Sergey Dyasli
Currently it's impossible to get CPU's microcode revision after late
loading without looking into Xen logs which is not always convenient.
Leverage xenhypfs to expose struct cpu_signature in a new cpuinfo dir.
The tree structure is:
/
cpuinfo/
cpu-signature
microcode-revision
processor-flags
The most useful bit is cpu microcode revision which will get updated
after late ucode loading.
Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
xen/arch/x86/cpu/common.c | 58 +++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 5ad347534a..aa864fdbab 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -1005,3 +1005,61 @@ const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id table[])
}
return NULL;
}
+
+#ifdef CONFIG_HYPFS
+#include <xen/hypfs.h>
+#include <xen/guest_access.h>
+#include <asm/microcode.h>
+
+static unsigned int cpu_signature;
+static unsigned int processor_flags;
+static unsigned int ucode_revision;
+
+int cf_check hypfs_read_cpusig(
+ const struct hypfs_entry *entry, XEN_GUEST_HANDLE_PARAM(void) uaddr)
+{
+ const struct hypfs_entry_leaf *l;
+ unsigned int size = entry->funcs->getsize(entry);
+ const struct cpu_signature *sig = &per_cpu(cpu_sig,
+ cpumask_first(&cpu_online_map));
+
+ l = container_of(entry, const struct hypfs_entry_leaf, e);
+
+ cpu_signature = sig->sig;
+ processor_flags = sig->pf;
+ ucode_revision = sig->rev;
+
+ return copy_to_guest(uaddr, l->u.content, size) ? -EFAULT : 0;
+}
+
+const struct hypfs_funcs ucode_rev_funcs = {
+ .enter = hypfs_node_enter,
+ .exit = hypfs_node_exit,
+ .read = hypfs_read_cpusig,
+ .write = hypfs_write_deny,
+ .getsize = hypfs_getsize,
+ .findentry = hypfs_leaf_findentry,
+};
+
+static HYPFS_DIR_INIT(cpuinfo, "cpuinfo");
+static HYPFS_FIXEDSIZE_INIT(signature, XEN_HYPFS_TYPE_UINT, "cpu-signature",
+ cpu_signature, &ucode_rev_funcs, 0);
+static HYPFS_FIXEDSIZE_INIT(pf, XEN_HYPFS_TYPE_UINT, "processor-flags",
+ processor_flags, &ucode_rev_funcs, 0);
+static HYPFS_FIXEDSIZE_INIT(revision, XEN_HYPFS_TYPE_UINT, "microcode-revision",
+ ucode_revision, &ucode_rev_funcs, 0);
+
+static int __init cf_check cpuinfo_init(void)
+{
+ hypfs_add_dir(&hypfs_root, &cpuinfo, true);
+ hypfs_add_leaf(&cpuinfo, &signature, true);
+
+ if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
+ hypfs_add_leaf(&cpuinfo, &pf, true);
+
+ hypfs_add_leaf(&cpuinfo, &revision, true);
+
+ return 0;
+}
+__initcall(cpuinfo_init);
+#endif /* CONFIG_HYPFS */
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] tools/xenctrl: add xc_get_cpu_version()
2023-02-28 17:39 [PATCH v2 0/3] xen-ucode: print information about currently loaded ucode Sergey Dyasli
2023-02-28 17:39 ` [PATCH v2/RFC 1/3] xen/hypfs: add initial cpuinfo directory Sergey Dyasli
@ 2023-02-28 17:39 ` Sergey Dyasli
2023-02-28 17:39 ` [PATCH v2 3/3] tools/xen-ucode: print information about currently loaded ucode Sergey Dyasli
2 siblings, 0 replies; 8+ messages in thread
From: Sergey Dyasli @ 2023-02-28 17:39 UTC (permalink / raw
To: xen-devel
Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu,
Anthony PERARD, Juergen Gross, George Dunlap, Julien Grall,
Stefano Stabellini, Sergey Dyasli
As a wrapper for XENPF_get_cpu_version platform op.
Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
tools/include/xenctrl.h | 1 +
tools/libs/ctrl/xc_misc.c | 20 ++++++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 23037874d3..8aa747dc2e 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -1186,6 +1186,7 @@ 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_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_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 265f15ec2d..f2f6e4348e 100644
--- a/tools/libs/ctrl/xc_misc.c
+++ b/tools/libs/ctrl/xc_misc.c
@@ -226,6 +226,26 @@ int xc_microcode_update(xc_interface *xch, const void *buf, size_t len)
return ret;
}
+int xc_get_cpu_version(xc_interface *xch, struct xenpf_pcpu_version *cpu_ver)
+{
+ int ret;
+ DECLARE_PLATFORM_OP;
+
+ if ( !xch || !cpu_ver )
+ return -1;
+
+ platform_op.cmd = XENPF_get_cpu_version;
+ platform_op.u.pcpu_version.xen_cpuid = cpu_ver->xen_cpuid;
+
+ ret = do_platform_op(xch, &platform_op);
+ if ( ret != 0 )
+ return ret;
+
+ *cpu_ver = platform_op.u.pcpu_version;
+
+ return 0;
+}
+
int xc_cputopoinfo(xc_interface *xch, unsigned *max_cpus,
xc_cputopo_t *cputopo)
{
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] tools/xen-ucode: print information about currently loaded ucode
2023-02-28 17:39 [PATCH v2 0/3] xen-ucode: print information about currently loaded ucode Sergey Dyasli
2023-02-28 17:39 ` [PATCH v2/RFC 1/3] xen/hypfs: add initial cpuinfo directory Sergey Dyasli
2023-02-28 17:39 ` [PATCH v2 2/3] tools/xenctrl: add xc_get_cpu_version() Sergey Dyasli
@ 2023-02-28 17:39 ` Sergey Dyasli
2023-03-01 11:31 ` Jan Beulich
2 siblings, 1 reply; 8+ messages in thread
From: Sergey Dyasli @ 2023-02-28 17:39 UTC (permalink / raw
To: xen-devel
Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Wei Liu,
Anthony PERARD, Juergen Gross, George Dunlap, Julien Grall,
Stefano Stabellini, Sergey Dyasli
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 cpuinfo directory in
xenhypfs and from XENPF_get_cpu_version platform op.
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>
---
tools/misc/Makefile | 2 +-
tools/misc/xen-ucode.c | 97 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/tools/misc/Makefile b/tools/misc/Makefile
index 1c6e1d6a04..e345ac76db 100644
--- a/tools/misc/Makefile
+++ b/tools/misc/Makefile
@@ -136,6 +136,6 @@ xencov: xencov.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
xen-ucode: xen-ucode.o
- $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
+ $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(LDLIBS_libxenhypfs) $(APPEND_LDFLAGS)
-include $(DEPS_INCLUDE)
diff --git a/tools/misc/xen-ucode.c b/tools/misc/xen-ucode.c
index ad32face2b..7e657689f4 100644
--- a/tools/misc/xen-ucode.c
+++ b/tools/misc/xen-ucode.c
@@ -11,6 +11,96 @@
#include <sys/stat.h>
#include <fcntl.h>
#include <xenctrl.h>
+#include <xenhypfs.h>
+
+static const char intel_id[] = "GenuineIntel";
+static const char amd_id[] = "AuthenticAMD";
+
+static const char sig_path[] = "/cpuinfo/cpu-signature";
+static const char rev_path[] = "/cpuinfo/microcode-revision";
+static const char pf_path[] = "/cpuinfo/processor-flags";
+
+static int hypfs_read_uint(struct xenhypfs_handle *hdl, const char *path,
+ unsigned int *var)
+{
+ char *result;
+ result = xenhypfs_read(hdl, path);
+ if ( !result )
+ return -1;
+
+ errno = 0;
+ *var = strtol(result, NULL, 10);
+ if ( errno )
+ return -1;
+
+ return 0;
+}
+
+static void show_curr_cpu(FILE *f)
+{
+ int ret;
+ struct xenhypfs_handle *hdl;
+ xc_interface *xch;
+ struct xenpf_pcpu_version cpu_ver = {0};
+ bool intel = false, amd = false;
+ unsigned int cpu_signature, pf, ucode_revision;
+
+ hdl = xenhypfs_open(NULL, 0);
+ if ( !hdl )
+ return;
+
+ xch = xc_interface_open(0, 0, 0);
+ if ( xch == NULL )
+ return;
+
+ ret = xc_get_cpu_version(xch, &cpu_ver);
+ 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 ( hypfs_read_uint(hdl, sig_path, &cpu_signature) != 0 )
+ return;
+
+ if ( hypfs_read_uint(hdl, rev_path, &ucode_revision) != 0 )
+ return;
+
+ if ( intel && hypfs_read_uint(hdl, pf_path, &pf) != 0 )
+ return;
+
+ /*
+ * 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
+ */
+ if ( intel )
+ {
+ fprintf(f, "Current CPU signature is: %02x-%02x-%02x (raw %#x)\n",
+ cpu_ver.family, cpu_ver.model, cpu_ver.stepping,
+ cpu_signature);
+ }
+ else if ( amd )
+ {
+ fprintf(f, "Current CPU signature is: fam%xh (raw %#x)\n",
+ cpu_ver.family, cpu_signature);
+ }
+
+ if ( intel || amd )
+ fprintf(f, "Current CPU microcode revision is: %#x\n", ucode_revision);
+
+ if ( intel )
+ fprintf(f, "Current CPU processor flags are: %#x\n", pf);
+
+ xc_interface_close(xch);
+ xenhypfs_close(hdl);
+}
int main(int argc, char *argv[])
{
@@ -25,9 +115,16 @@ int main(int argc, char *argv[])
fprintf(stderr,
"xen-ucode: Xen microcode updating tool\n"
"Usage: %s <microcode blob>\n", argv[0]);
+ show_curr_cpu(stderr);
exit(2);
}
+ if ( !strcmp(argv[1], "show-cpu-info") )
+ {
+ show_curr_cpu(stdout);
+ return 0;
+ }
+
filename = argv[1];
fd = open(filename, O_RDONLY);
if ( fd < 0 )
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/3] tools/xen-ucode: print information about currently loaded ucode
2023-02-28 17:39 ` [PATCH v2 3/3] tools/xen-ucode: print information about currently loaded ucode Sergey Dyasli
@ 2023-03-01 11:31 ` Jan Beulich
2023-03-01 18:01 ` Sergey Dyasli
0 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2023-03-01 11:31 UTC (permalink / raw
To: Sergey Dyasli
Cc: Andrew Cooper, Roger Pau Monné, Wei Liu, Anthony PERARD,
Juergen Gross, George Dunlap, Julien Grall, Stefano Stabellini,
xen-devel
On 28.02.2023 18:39, 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 cpuinfo directory in
> xenhypfs and from XENPF_get_cpu_version platform op.
While I don't mind the use of the platform-op, I'm little puzzled by the
mix. If CPU information is to be exposed in hypfs, can't we expose there
everything that's needed here?
Then again, perhaps in a different context, Andrew pointed out that hypfs
is an optional component, so relying on its presence in the underlying
hypervisor will need weighing against the alternative of adding a new
platform-op for the ucode-related data (as you had it in v1). Since I'm
unaware of a request to switch, are there specific reasons you did?
> --- a/tools/misc/xen-ucode.c
> +++ b/tools/misc/xen-ucode.c
> @@ -11,6 +11,96 @@
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <xenctrl.h>
> +#include <xenhypfs.h>
> +
> +static const char intel_id[] = "GenuineIntel";
> +static const char amd_id[] = "AuthenticAMD";
> +
> +static const char sig_path[] = "/cpuinfo/cpu-signature";
> +static const char rev_path[] = "/cpuinfo/microcode-revision";
> +static const char pf_path[] = "/cpuinfo/processor-flags";
Together with the use below I conclude (without having looked at patch 1
yet) that you only expose perhaps the BSP's data, rather than such for
all CPUs. (And I was actually going to put up the question whether data
like the one presented here might not also be of interest for parked
CPUs.)
> +static int hypfs_read_uint(struct xenhypfs_handle *hdl, const char *path,
> + unsigned int *var)
> +{
> + char *result;
> + result = xenhypfs_read(hdl, path);
> + if ( !result )
> + return -1;
> +
> + errno = 0;
> + *var = strtol(result, NULL, 10);
Better use strtoul() (as you're after an unsigned quantity), and perhaps
also better to not assume 10 as the radix (neither signature nor ucode
revision are very useful to expose as decimal numbers in hypfs)? Plus
perhaps deal with the result not fitting in "unsigned int"?
> + if ( errno )
> + return -1;
> +
> + return 0;
> +}
> +
> +static void show_curr_cpu(FILE *f)
> +{
> + int ret;
> + struct xenhypfs_handle *hdl;
> + xc_interface *xch;
> + struct xenpf_pcpu_version cpu_ver = {0};
> + bool intel = false, amd = false;
> + unsigned int cpu_signature, pf, ucode_revision;
> +
> + hdl = xenhypfs_open(NULL, 0);
> + if ( !hdl )
> + return;
> +
> + xch = xc_interface_open(0, 0, 0);
> + if ( xch == NULL )
> + return;
> +
> + ret = xc_get_cpu_version(xch, &cpu_ver);
> + 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 ( hypfs_read_uint(hdl, sig_path, &cpu_signature) != 0 )
> + return;
> +
> + if ( hypfs_read_uint(hdl, rev_path, &ucode_revision) != 0 )
> + return;
You consume these two fields only when "intel || amd"; without having
looked at patch 1 yet I'd also assume the node might not be present for
other vendors, and hence no output would be produced at all then, due
to the failure here.
> + if ( intel && hypfs_read_uint(hdl, pf_path, &pf) != 0 )
Nit: Stray double blank before "&pf".
> + return;
> +
> + /*
> + * 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
> + */
> + if ( intel )
> + {
> + fprintf(f, "Current CPU signature is: %02x-%02x-%02x (raw %#x)\n",
> + cpu_ver.family, cpu_ver.model, cpu_ver.stepping,
> + cpu_signature);
> + }
> + else if ( amd )
> + {
> + fprintf(f, "Current CPU signature is: fam%xh (raw %#x)\n",
> + cpu_ver.family, cpu_signature);
> + }
else
fprintf("...", cpu_ver.vendor_id,
cpu_ver.family, cpu_ver.model, cpu_ver.stepping);
? Otherwise some kind of error message would imo be needed, such that the
tool won't exit successfully without providing any output at all. Recall
that we consider Hygon an alive target CPU, just with (at present) no
ucode support.
> + if ( intel || amd )
> + fprintf(f, "Current CPU microcode revision is: %#x\n", ucode_revision);
> +
> + if ( intel )
> + fprintf(f, "Current CPU processor flags are: %#x\n", pf);
> +
> + xc_interface_close(xch);
> + xenhypfs_close(hdl);
Maybe do these earlier, as soon as you're done with each of the two items?
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/3] tools/xen-ucode: print information about currently loaded ucode
2023-03-01 11:31 ` Jan Beulich
@ 2023-03-01 18:01 ` Sergey Dyasli
2023-03-02 9:31 ` Jan Beulich
2023-03-13 16:31 ` Jan Beulich
0 siblings, 2 replies; 8+ messages in thread
From: Sergey Dyasli @ 2023-03-01 18:01 UTC (permalink / raw
To: Jan Beulich
Cc: Sergey Dyasli, Andrew Cooper, Roger Pau Monné, Wei Liu,
Anthony PERARD, Juergen Gross, George Dunlap, Julien Grall,
Stefano Stabellini, xen-devel
On Wed, Mar 1, 2023 at 11:31 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 28.02.2023 18:39, 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 cpuinfo directory in
> > xenhypfs and from XENPF_get_cpu_version platform op.
>
> While I don't mind the use of the platform-op, I'm little puzzled by the
> mix. If CPU information is to be exposed in hypfs, can't we expose there
> everything that's needed here?
>
> Then again, perhaps in a different context, Andrew pointed out that hypfs
> is an optional component, so relying on its presence in the underlying
> hypervisor will need weighing against the alternative of adding a new
> platform-op for the ucode-related data (as you had it in v1). Since I'm
> unaware of a request to switch, are there specific reasons you did?
Ideal situation would be microcode information in Dom0's /proc/cpuinfo
updated after late load, since that file already has most of the
information about the cpu. And the closest thing to /proc is xenhypfs.
It allows the user to query information directly, e.g.
# xenhypfs cat /cpuinfo/microcode-revision
33554509
Which could be used manually or in scripts, instead of relying on
xen-ucode utility. Though printing the value in hex would be nicer.
That was my motivation to go hypfs route. In general it feels like cpu
information is a good fit for hypfs, but agreement on its format and
exposed values is needed.
I can always switch back to a platform op if that would be the preference.
> > --- a/tools/misc/xen-ucode.c
> > +++ b/tools/misc/xen-ucode.c
> > @@ -11,6 +11,96 @@
> > #include <sys/stat.h>
> > #include <fcntl.h>
> > #include <xenctrl.h>
> > +#include <xenhypfs.h>
> > +
> > +static const char intel_id[] = "GenuineIntel";
> > +static const char amd_id[] = "AuthenticAMD";
> > +
> > +static const char sig_path[] = "/cpuinfo/cpu-signature";
> > +static const char rev_path[] = "/cpuinfo/microcode-revision";
> > +static const char pf_path[] = "/cpuinfo/processor-flags";
>
> Together with the use below I conclude (without having looked at patch 1
> yet) that you only expose perhaps the BSP's data, rather than such for
> all CPUs. (And I was actually going to put up the question whether data
> like the one presented here might not also be of interest for parked
> CPUs.)
Yes, that comes from the BSP. Xen must make sure that all CPUs have
the same ucode revision for the system to work correctly.
Sergey
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/3] tools/xen-ucode: print information about currently loaded ucode
2023-03-01 18:01 ` Sergey Dyasli
@ 2023-03-02 9:31 ` Jan Beulich
2023-03-13 16:31 ` Jan Beulich
1 sibling, 0 replies; 8+ messages in thread
From: Jan Beulich @ 2023-03-02 9:31 UTC (permalink / raw
To: Sergey Dyasli
Cc: Sergey Dyasli, Andrew Cooper, Roger Pau Monné, Wei Liu,
Anthony PERARD, Juergen Gross, George Dunlap, Julien Grall,
Stefano Stabellini, xen-devel
On 01.03.2023 19:01, Sergey Dyasli wrote:
> On Wed, Mar 1, 2023 at 11:31 AM Jan Beulich <jbeulich@suse.com> wrote:
>> On 28.02.2023 18:39, 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 cpuinfo directory in
>>> xenhypfs and from XENPF_get_cpu_version platform op.
>>
>> While I don't mind the use of the platform-op, I'm little puzzled by the
>> mix. If CPU information is to be exposed in hypfs, can't we expose there
>> everything that's needed here?
>>
>> Then again, perhaps in a different context, Andrew pointed out that hypfs
>> is an optional component, so relying on its presence in the underlying
>> hypervisor will need weighing against the alternative of adding a new
>> platform-op for the ucode-related data (as you had it in v1). Since I'm
>> unaware of a request to switch, are there specific reasons you did?
>
> Ideal situation would be microcode information in Dom0's /proc/cpuinfo
> updated after late load, since that file already has most of the
> information about the cpu.
If that was to represent host-wide information, Dom0 would need to gain
a parallel mechanism (e.g. /proc/pcpuinfo) covering pCPU-s instead of
the vCPU-s it has got.
> And the closest thing to /proc is xenhypfs.
> It allows the user to query information directly, e.g.
>
> # xenhypfs cat /cpuinfo/microcode-revision
> 33554509
>
> Which could be used manually or in scripts, instead of relying on
> xen-ucode utility. Though printing the value in hex would be nicer.
> That was my motivation to go hypfs route. In general it feels like cpu
> information is a good fit for hypfs, but agreement on its format and
> exposed values is needed.
> I can always switch back to a platform op if that would be the preference.
I agree exposing a certain amount of per-CPU information in hypfs is
desirable. But whether that is to be the source for a tool like
xen-ucode is a separate question. If Andrew doesn't respond to this
aspect here, you may want to talk to him directly.
>>> --- a/tools/misc/xen-ucode.c
>>> +++ b/tools/misc/xen-ucode.c
>>> @@ -11,6 +11,96 @@
>>> #include <sys/stat.h>
>>> #include <fcntl.h>
>>> #include <xenctrl.h>
>>> +#include <xenhypfs.h>
>>> +
>>> +static const char intel_id[] = "GenuineIntel";
>>> +static const char amd_id[] = "AuthenticAMD";
>>> +
>>> +static const char sig_path[] = "/cpuinfo/cpu-signature";
>>> +static const char rev_path[] = "/cpuinfo/microcode-revision";
>>> +static const char pf_path[] = "/cpuinfo/processor-flags";
>>
>> Together with the use below I conclude (without having looked at patch 1
>> yet) that you only expose perhaps the BSP's data, rather than such for
>> all CPUs. (And I was actually going to put up the question whether data
>> like the one presented here might not also be of interest for parked
>> CPUs.)
>
> Yes, that comes from the BSP. Xen must make sure that all CPUs have
> the same ucode revision for the system to work correctly.
Yet Xen may not be in the position to do so, and representing the "may
not work correctly" case may be helpful in diagnosing problem reports.
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/3] tools/xen-ucode: print information about currently loaded ucode
2023-03-01 18:01 ` Sergey Dyasli
2023-03-02 9:31 ` Jan Beulich
@ 2023-03-13 16:31 ` Jan Beulich
1 sibling, 0 replies; 8+ messages in thread
From: Jan Beulich @ 2023-03-13 16:31 UTC (permalink / raw
To: Sergey Dyasli
Cc: Sergey Dyasli, Andrew Cooper, Roger Pau Monné, Wei Liu,
Anthony PERARD, Juergen Gross, George Dunlap, Julien Grall,
Stefano Stabellini, xen-devel
On 01.03.2023 19:01, Sergey Dyasli wrote:
> On Wed, Mar 1, 2023 at 11:31 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 28.02.2023 18:39, 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 cpuinfo directory in
>>> xenhypfs and from XENPF_get_cpu_version platform op.
>>
>> While I don't mind the use of the platform-op, I'm little puzzled by the
>> mix. If CPU information is to be exposed in hypfs, can't we expose there
>> everything that's needed here?
>>
>> Then again, perhaps in a different context, Andrew pointed out that hypfs
>> is an optional component, so relying on its presence in the underlying
>> hypervisor will need weighing against the alternative of adding a new
>> platform-op for the ucode-related data (as you had it in v1). Since I'm
>> unaware of a request to switch, are there specific reasons you did?
>
> Ideal situation would be microcode information in Dom0's /proc/cpuinfo
> updated after late load, since that file already has most of the
> information about the cpu. And the closest thing to /proc is xenhypfs.
> It allows the user to query information directly, e.g.
>
> # xenhypfs cat /cpuinfo/microcode-revision
> 33554509
>
> Which could be used manually or in scripts, instead of relying on
> xen-ucode utility. Though printing the value in hex would be nicer.
> That was my motivation to go hypfs route. In general it feels like cpu
> information is a good fit for hypfs, but agreement on its format and
> exposed values is needed.
> I can always switch back to a platform op if that would be the preference.
I've confirmed with Andrew that I was remembering correctly and he would
not want to see a dependency on hypfs in such a tool. Since it's optional
in the hypervisor, you'd need an alternative source of the same info
anyway, and hence you can as well go just that non-hypfs route.
FTAOD: This isn't to say that some CPU info wouldn't be useful to expose
in hypfs. But that's then an independent task.
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-03-13 16:31 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-28 17:39 [PATCH v2 0/3] xen-ucode: print information about currently loaded ucode Sergey Dyasli
2023-02-28 17:39 ` [PATCH v2/RFC 1/3] xen/hypfs: add initial cpuinfo directory Sergey Dyasli
2023-02-28 17:39 ` [PATCH v2 2/3] tools/xenctrl: add xc_get_cpu_version() Sergey Dyasli
2023-02-28 17:39 ` [PATCH v2 3/3] tools/xen-ucode: print information about currently loaded ucode Sergey Dyasli
2023-03-01 11:31 ` Jan Beulich
2023-03-01 18:01 ` Sergey Dyasli
2023-03-02 9:31 ` Jan Beulich
2023-03-13 16:31 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).