[libvirt] [PATCH 0/5] Refresh QEMU caps when CPU microcode changes

Jiri Denemark (1): cpu_x86: Rename virCPUx86MapInitialize Paolo Bonzini (4): util: add virFileReadHeaderQuiet wrapper around virFileReadHeaderFD util: introduce virHostCPUGetMicrocodeVersion conf: include x86 microcode version in virsh capabilities qemu: capabilities: force update if the microcode version does not match src/conf/cpu_conf.c | 14 +++++++ src/conf/cpu_conf.h | 1 + src/cpu/cpu_x86.c | 17 +++++++-- src/libvirt_private.syms | 2 + src/qemu/qemu_capabilities.c | 40 +++++++++++++++++++- src/qemu/qemu_capabilities.h | 6 ++- src/qemu/qemu_capspriv.h | 5 +++ src/qemu/qemu_driver.c | 9 ++++- src/util/virfile.c | 19 ++++++++++ src/util/virfile.h | 2 + src/util/virhostcpu.c | 43 ++++++++++++++++++++++ src/util/virhostcpu.h | 2 + tests/qemucapabilitiesdata/caps_1.2.2.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.3.1.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.4.2.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 1 + .../caps_2.10.0-gicv2.aarch64.xml | 1 + .../caps_2.10.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + .../caps_2.6.0-gicv2.aarch64.xml | 1 + .../caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + tests/qemucapabilitiestest.c | 14 +++++-- tests/qemucapsprobe.c | 2 +- tests/testutilsqemu.c | 2 +- 40 files changed, 189 insertions(+), 14 deletions(-) -- 2.15.1

From: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 19 +++++++++++++++++++ src/util/virfile.h | 2 ++ 3 files changed, 22 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6662c8dac1..8d583e3e74 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1760,6 +1760,7 @@ virFileReadAll; virFileReadAllQuiet; virFileReadBufQuiet; virFileReadHeaderFD; +virFileReadHeaderQuiet; virFileReadLimFD; virFileReadLink; virFileReadValueBitmap; diff --git a/src/util/virfile.c b/src/util/virfile.c index 82cb36dbca..5e9bd2007a 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1367,6 +1367,25 @@ virFileReadHeaderFD(int fd, int maxlen, char **buf) } +int +virFileReadHeaderQuiet(const char *path, + int maxlen, + char **buf) +{ + int fd; + int len; + + fd = open(path, O_RDONLY); + if (fd < 0) + return -1; + + len = virFileReadHeaderFD(fd, maxlen, buf); + VIR_FORCE_CLOSE(fd); + + return len; +} + + /* A wrapper around saferead_lim that maps a failure due to exceeding the maximum size limitation to EOVERFLOW. */ int diff --git a/src/util/virfile.h b/src/util/virfile.h index b9b6b3c223..cd2a3867c2 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -131,6 +131,8 @@ int virFileDeleteTree(const char *dir); int virFileReadHeaderFD(int fd, int maxlen, char **buf) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(3); +int virFileReadHeaderQuiet(const char *path, int maxlen, char **buf) + ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); int virFileReadLimFD(int fd, int maxlen, char **buf) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(3); int virFileReadAll(const char *path, int maxlen, char **buf) -- 2.15.1

On Thu, Jan 04, 2018 at 15:58:08 +0100, Jiri Denemark wrote:
From: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 19 +++++++++++++++++++ src/util/virfile.h | 2 ++ 3 files changed, 22 insertions(+)
ACK

From: Paolo Bonzini <pbonzini@redhat.com> This new API reads host's CPU microcode version from /proc/cpuinfo. Unfortunately, there is no other way of reading microcode version which would be usable from both system and session daemon. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virhostcpu.c | 43 +++++++++++++++++++++++++++++++++++++++++++ src/util/virhostcpu.h | 2 ++ 3 files changed, 46 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8d583e3e74..a705fa846d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1869,6 +1869,7 @@ virHostCPUGetCount; virHostCPUGetInfo; virHostCPUGetKVMMaxVCPUs; virHostCPUGetMap; +virHostCPUGetMicrocodeVersion; virHostCPUGetOnline; virHostCPUGetOnlineBitmap; virHostCPUGetPresentBitmap; diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index c485a97211..713fdec553 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1206,3 +1206,46 @@ virHostCPUGetKVMMaxVCPUs(void) return -1; } #endif /* HAVE_LINUX_KVM_H */ + + +#ifdef __linux__ + +unsigned int +virHostCPUGetMicrocodeVersion(void) +{ + char *outbuf = NULL; + char *cur; + unsigned int version = 0; + + if (virFileReadHeaderQuiet(CPUINFO_PATH, 4096, &outbuf) < 0) { + char ebuf[1024]; + VIR_DEBUG("Failed to read microcode version from %s: %s", + CPUINFO_PATH, virStrerror(errno, ebuf, sizeof(ebuf))); + return 0; + } + + /* Account for format 'microcode : XXXX'*/ + if (!(cur = strstr(outbuf, "microcode")) || + !(cur = strchr(cur, ':'))) + goto cleanup; + cur++; + + /* Linux places the microcode revision in a 32-bit integer, so + * ui is fine for us too. */ + if (virStrToLong_ui(cur, &cur, 0, &version) < 0) + goto cleanup; + + cleanup: + VIR_FREE(outbuf); + return version; +} + +#else + +unsigned int +virHostCPUGetMicrocodeVersion(void) +{ + return 0; +} + +#endif diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h index 67033de842..f9f3359288 100644 --- a/src/util/virhostcpu.h +++ b/src/util/virhostcpu.h @@ -66,4 +66,6 @@ virBitmapPtr virHostCPUGetSiblingsList(unsigned int cpu); int virHostCPUGetOnline(unsigned int cpu, bool *online); +unsigned int virHostCPUGetMicrocodeVersion(void); + #endif /* __VIR_HOSTCPU_H__*/ -- 2.15.1

On Thu, Jan 04, 2018 at 15:58:09 +0100, Jiri Denemark wrote:
From: Paolo Bonzini <pbonzini@redhat.com>
This new API reads host's CPU microcode version from /proc/cpuinfo.
Unfortunately, there is no other way of reading microcode version which would be usable from both system and session daemon.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virhostcpu.c | 43 +++++++++++++++++++++++++++++++++++++++++++ src/util/virhostcpu.h | 2 ++ 3 files changed, 46 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8d583e3e74..a705fa846d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1869,6 +1869,7 @@ virHostCPUGetCount; virHostCPUGetInfo; virHostCPUGetKVMMaxVCPUs; virHostCPUGetMap; +virHostCPUGetMicrocodeVersion; virHostCPUGetOnline; virHostCPUGetOnlineBitmap; virHostCPUGetPresentBitmap; diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index c485a97211..713fdec553 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1206,3 +1206,46 @@ virHostCPUGetKVMMaxVCPUs(void) return -1; } #endif /* HAVE_LINUX_KVM_H */ + + +#ifdef __linux__ + +unsigned int +virHostCPUGetMicrocodeVersion(void) +{ + char *outbuf = NULL; + char *cur; + unsigned int version = 0; + + if (virFileReadHeaderQuiet(CPUINFO_PATH, 4096, &outbuf) < 0) {
In other places we read 1MiB of /proc/cpuinfo: src/util/virsysinfo.c:#define CPUINFO_FILE_LEN (1024*1024) /* 1MB limit for /proc/cpuinfo file */ Will this be enough?
+ char ebuf[1024]; + VIR_DEBUG("Failed to read microcode version from %s: %s", + CPUINFO_PATH, virStrerror(errno, ebuf, sizeof(ebuf)));
Formatting the error message into a debug message does not make much sense. I'd just log 'errno' in raw since this function does not really return any errors.
+ return 0; + } + + /* Account for format 'microcode : XXXX'*/ + if (!(cur = strstr(outbuf, "microcode")) || + !(cur = strchr(cur, ':'))) + goto cleanup; + cur++; + + /* Linux places the microcode revision in a 32-bit integer, so + * ui is fine for us too. */ + if (virStrToLong_ui(cur, &cur, 0, &version) < 0) + goto cleanup; + + cleanup: + VIR_FREE(outbuf); + return version; +} + +#else
#else /* __linux__ */
+ +unsigned int +virHostCPUGetMicrocodeVersion(void) +{ + return 0; +} + +#endif
#endif /* __linux__ */
diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h index 67033de842..f9f3359288 100644 --- a/src/util/virhostcpu.h +++ b/src/util/virhostcpu.h @@ -66,4 +66,6 @@ virBitmapPtr virHostCPUGetSiblingsList(unsigned int cpu);
int virHostCPUGetOnline(unsigned int cpu, bool *online);
+unsigned int virHostCPUGetMicrocodeVersion(void); + #endif /* __VIR_HOSTCPU_H__*/
ACK with pointless error formatting removed.
-- 2.15.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Jan 04, 2018 at 16:15:46 +0100, Peter Krempa wrote:
On Thu, Jan 04, 2018 at 15:58:09 +0100, Jiri Denemark wrote:
From: Paolo Bonzini <pbonzini@redhat.com>
This new API reads host's CPU microcode version from /proc/cpuinfo.
Unfortunately, there is no other way of reading microcode version which would be usable from both system and session daemon.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virhostcpu.c | 43 +++++++++++++++++++++++++++++++++++++++++++ src/util/virhostcpu.h | 2 ++ 3 files changed, 46 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8d583e3e74..a705fa846d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1869,6 +1869,7 @@ virHostCPUGetCount; virHostCPUGetInfo; virHostCPUGetKVMMaxVCPUs; virHostCPUGetMap; +virHostCPUGetMicrocodeVersion; virHostCPUGetOnline; virHostCPUGetOnlineBitmap; virHostCPUGetPresentBitmap; diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index c485a97211..713fdec553 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1206,3 +1206,46 @@ virHostCPUGetKVMMaxVCPUs(void) return -1; } #endif /* HAVE_LINUX_KVM_H */ + + +#ifdef __linux__ + +unsigned int +virHostCPUGetMicrocodeVersion(void) +{ + char *outbuf = NULL; + char *cur; + unsigned int version = 0; + + if (virFileReadHeaderQuiet(CPUINFO_PATH, 4096, &outbuf) < 0) {
In other places we read 1MiB of /proc/cpuinfo:
src/util/virsysinfo.c:#define CPUINFO_FILE_LEN (1024*1024) /* 1MB limit for /proc/cpuinfo file */
Will this be enough?
Should be (TM) :-) We're only interested in the first CPU and the microcode version is printed very early before the endlessly growing list of CPU features.
+ char ebuf[1024]; + VIR_DEBUG("Failed to read microcode version from %s: %s", + CPUINFO_PATH, virStrerror(errno, ebuf, sizeof(ebuf)));
Formatting the error message into a debug message does not make much sense. I'd just log 'errno' in raw since this function does not really return any errors.
...
ACK with pointless error formatting removed.
It's not pointless. It makes the log useful without having to look at /usr/include/asm-generic/errno-base.h. And it will only be done once in libvirtd lifetime so it's not really wasting CPU cycles or something. Jirka

On Thu, Jan 04, 2018 at 04:27:45PM +0100, Jiri Denemark wrote:
On Thu, Jan 04, 2018 at 16:15:46 +0100, Peter Krempa wrote:
On Thu, Jan 04, 2018 at 15:58:09 +0100, Jiri Denemark wrote:
+ char ebuf[1024]; + VIR_DEBUG("Failed to read microcode version from %s: %s", + CPUINFO_PATH, virStrerror(errno, ebuf, sizeof(ebuf)));
Formatting the error message into a debug message does not make much sense. I'd just log 'errno' in raw since this function does not really return any errors. ... ACK with pointless error formatting removed.
It's not pointless. It makes the log useful without having to look at /usr/include/asm-generic/errno-base.h. And it will only be done once in libvirtd lifetime so it's not really wasting CPU cycles or something.
ACK including the error formatting. I see the point in it, CPU cycles are cheaper than human cycles. Jan
Jirka
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Thursday, 4 January 2018 16:27:45 CET Jiri Denemark wrote:
It's not pointless. It makes the log useful without having to look at /usr/include/asm-generic/errno-base.h. And it will only be done once in libvirtd lifetime so it's not really wasting CPU cycles or something.
Theoretically you do not need to: if you install 'moreutils', you get the useful 'errno' utility to map from errno numbers to descriptions (even localized ones), and vice-versa. The only issue is that this works only for the same OS+architecture combination, since values of errno changes between OSes and architectures in the same OS. -- Pino Toscano

The function will be used to initialize internal data of the x86 CPU driver (including the CPU map). Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index f514fd2663..fddde64d8c 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -153,8 +153,8 @@ struct _virCPUx86Map { }; static virCPUx86MapPtr cpuMap; -int virCPUx86MapOnceInit(void); -VIR_ONCE_GLOBAL_INIT(virCPUx86Map); +int virCPUx86DriverOnceInit(void); +VIR_ONCE_GLOBAL_INIT(virCPUx86Driver); typedef enum { @@ -1404,7 +1404,7 @@ virCPUx86LoadMap(void) int -virCPUx86MapOnceInit(void) +virCPUx86DriverOnceInit(void) { if (!(cpuMap = virCPUx86LoadMap())) return -1; @@ -1416,7 +1416,7 @@ virCPUx86MapOnceInit(void) static virCPUx86MapPtr virCPUx86GetMap(void) { - if (virCPUx86MapInitialize() < 0) + if (virCPUx86DriverInitialize() < 0) return NULL; return cpuMap; -- 2.15.1

On Thu, Jan 04, 2018 at 15:58:10 +0100, Jiri Denemark wrote:
The function will be used to initialize internal data of the x86 CPU driver (including the CPU map).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
ACK

From: Paolo Bonzini <pbonzini@redhat.com> A microcode update can cause the CPUID bits to change; an example from the past was the update that disabled TSX on several Haswell and Broadwell machines. In order to track the x86 microcode version in the QEMU capabilities, we have to fetch it and store it in the host CPU. This also makes the version visible in "virsh capabilities", which is a nice side effect. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/cpu_conf.c | 14 ++++++++++++++ src/conf/cpu_conf.h | 1 + src/cpu/cpu_x86.c | 9 +++++++++ 3 files changed, 24 insertions(+) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 603cf0e471..43a3ab5dcd 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -130,6 +130,7 @@ virCPUDefCopyModelFilter(virCPUDefPtr dst, VIR_STRDUP(dst->vendor_id, src->vendor_id) < 0 || VIR_ALLOC_N(dst->features, src->nfeatures) < 0) return -1; + dst->microcodeVersion = src->microcodeVersion; dst->nfeatures_max = src->nfeatures; dst->nfeatures = 0; @@ -181,6 +182,7 @@ virCPUDefStealModel(virCPUDefPtr dst, VIR_STEAL_PTR(dst->model, src->model); VIR_STEAL_PTR(dst->features, src->features); + dst->microcodeVersion = src->microcodeVersion; dst->nfeatures_max = src->nfeatures_max; src->nfeatures_max = 0; dst->nfeatures = src->nfeatures; @@ -382,6 +384,14 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, goto cleanup; } VIR_FREE(arch); + + if (virXPathBoolean("boolean(./microcode[1]/@version)", ctxt) > 0 && + virXPathUInt("string(./microcode[1]/@version)", ctxt, + &def->microcodeVersion) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("invalid microcode version")); + goto cleanup; + } } if (!(def->model = virXPathString("string(./model[1])", ctxt)) && @@ -720,6 +730,10 @@ virCPUDefFormatBuf(virBufferPtr buf, if (formatModel && def->vendor) virBufferEscapeString(buf, "<vendor>%s</vendor>\n", def->vendor); + if (def->type == VIR_CPU_TYPE_HOST && def->microcodeVersion) + virBufferAsprintf(buf, "<microcode version='%u'/>\n", + def->microcodeVersion); + if (def->sockets && def->cores && def->threads) { virBufferAddLit(buf, "<topology"); virBufferAsprintf(buf, " sockets='%u'", def->sockets); diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index d1983f5d4f..9f2e7ee264 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -133,6 +133,7 @@ struct _virCPUDef { char *vendor_id; /* vendor id returned by CPUID in the guest */ int fallback; /* enum virCPUFallback */ char *vendor; + unsigned int microcodeVersion; unsigned int sockets; unsigned int cores; unsigned int threads; diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index fddde64d8c..26314a5b3a 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -33,6 +33,7 @@ #include "virbuffer.h" #include "virendian.h" #include "virstring.h" +#include "virhostcpu.h" #define VIR_FROM_THIS VIR_FROM_CPU @@ -153,6 +154,8 @@ struct _virCPUx86Map { }; static virCPUx86MapPtr cpuMap; +static unsigned int microcodeVersion; + int virCPUx86DriverOnceInit(void); VIR_ONCE_GLOBAL_INIT(virCPUx86Driver); @@ -1409,6 +1412,8 @@ virCPUx86DriverOnceInit(void) if (!(cpuMap = virCPUx86LoadMap())) return -1; + microcodeVersion = virHostCPUGetMicrocodeVersion(); + return 0; } @@ -2424,6 +2429,9 @@ virCPUx86GetHost(virCPUDefPtr cpu, virCPUDataPtr cpuData = NULL; int ret = -1; + if (virCPUx86DriverInitialize() < 0) + goto cleanup; + if (!(cpuData = virCPUDataNew(archs[0]))) goto cleanup; @@ -2432,6 +2440,7 @@ virCPUx86GetHost(virCPUDefPtr cpu, goto cleanup; ret = x86DecodeCPUData(cpu, cpuData, models); + cpu->microcodeVersion = microcodeVersion; cleanup: virCPUx86DataFree(cpuData); -- 2.15.1

On Thu, Jan 04, 2018 at 15:58:11 +0100, Jiri Denemark wrote:
From: Paolo Bonzini <pbonzini@redhat.com>
A microcode update can cause the CPUID bits to change; an example from the past was the update that disabled TSX on several Haswell and Broadwell machines.
In order to track the x86 microcode version in the QEMU capabilities, we have to fetch it and store it in the host CPU. This also makes the version visible in "virsh capabilities", which is a nice side effect.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/cpu_conf.c | 14 ++++++++++++++ src/conf/cpu_conf.h | 1 + src/cpu/cpu_x86.c | 9 +++++++++ 3 files changed, 24 insertions(+)
[...]
@@ -382,6 +384,14 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, goto cleanup; } VIR_FREE(arch); + + if (virXPathBoolean("boolean(./microcode[1]/@version)", ctxt) > 0 && + virXPathUInt("string(./microcode[1]/@version)", ctxt, + &def->microcodeVersion) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("invalid microcode version")); + goto cleanup; + } }
if (!(def->model = virXPathString("string(./model[1])", ctxt)) && @@ -720,6 +730,10 @@ virCPUDefFormatBuf(virBufferPtr buf, if (formatModel && def->vendor) virBufferEscapeString(buf, "<vendor>%s</vendor>\n", def->vendor);
+ if (def->type == VIR_CPU_TYPE_HOST && def->microcodeVersion)
Hmm, looks like you should add a comment to virHostCPUGetMicrocodeVersion stating that 0 is a special value.
+ virBufferAsprintf(buf, "<microcode version='%u'/>\n", + def->microcodeVersion); +
ACK

On Thu, Jan 04, 2018 at 16:25:49 +0100, Peter Krempa wrote:
On Thu, Jan 04, 2018 at 15:58:11 +0100, Jiri Denemark wrote:
From: Paolo Bonzini <pbonzini@redhat.com>
A microcode update can cause the CPUID bits to change; an example from the past was the update that disabled TSX on several Haswell and Broadwell machines.
In order to track the x86 microcode version in the QEMU capabilities, we have to fetch it and store it in the host CPU. This also makes the version visible in "virsh capabilities", which is a nice side effect.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/cpu_conf.c | 14 ++++++++++++++ src/conf/cpu_conf.h | 1 + src/cpu/cpu_x86.c | 9 +++++++++ 3 files changed, 24 insertions(+)
[...]
@@ -382,6 +384,14 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, goto cleanup; } VIR_FREE(arch); + + if (virXPathBoolean("boolean(./microcode[1]/@version)", ctxt) > 0 && + virXPathUInt("string(./microcode[1]/@version)", ctxt, + &def->microcodeVersion) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("invalid microcode version")); + goto cleanup; + } }
if (!(def->model = virXPathString("string(./model[1])", ctxt)) && @@ -720,6 +730,10 @@ virCPUDefFormatBuf(virBufferPtr buf, if (formatModel && def->vendor) virBufferEscapeString(buf, "<vendor>%s</vendor>\n", def->vendor);
+ if (def->type == VIR_CPU_TYPE_HOST && def->microcodeVersion)
Hmm, looks like you should add a comment to virHostCPUGetMicrocodeVersion stating that 0 is a special value.
Yeah, I added the comment there. Jirka

From: Paolo Bonzini <pbonzini@redhat.com> A microcode update can cause the CPUID bits to change; an example from the past was the update that disabled TSX on several Haswell and Broadwell machines. Therefore, place microcode version in the virQEMUCaps struct and XML, and rebuild the cache if the versions do not match. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 40 +++++++++++++++++++++- src/qemu/qemu_capabilities.h | 6 ++-- src/qemu/qemu_capspriv.h | 5 +++ src/qemu/qemu_driver.c | 9 ++++- tests/qemucapabilitiesdata/caps_1.2.2.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.3.1.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.4.2.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 1 + .../caps_2.10.0-gicv2.aarch64.xml | 1 + .../caps_2.10.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + .../caps_2.6.0-gicv2.aarch64.xml | 1 + .../caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + tests/qemucapabilitiestest.c | 14 +++++--- tests/qemucapsprobe.c | 2 +- tests/testutilsqemu.c | 2 +- 32 files changed, 93 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9673ef857b..abf02a9b17 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -507,6 +507,7 @@ struct _virQEMUCaps { unsigned int version; unsigned int kvmVersion; unsigned int libvirtVersion; + unsigned int microcodeVersion; char *package; virArch arch; @@ -2296,6 +2297,7 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) ret->version = qemuCaps->version; ret->kvmVersion = qemuCaps->kvmVersion; + ret->microcodeVersion = qemuCaps->microcodeVersion; if (VIR_STRDUP(ret->package, qemuCaps->package) < 0) goto error; @@ -3830,6 +3832,7 @@ struct _virQEMUCapsCachePriv { uid_t runUid; gid_t runGid; virArch hostArch; + unsigned int microcodeVersion; }; typedef struct _virQEMUCapsCachePriv virQEMUCapsCachePriv; typedef virQEMUCapsCachePriv *virQEMUCapsCachePrivPtr; @@ -3952,6 +3955,13 @@ virQEMUCapsLoadCache(virArch hostArch, goto cleanup; } + if (virXPathUInt("string(./microcodeVersion)", ctxt, + &qemuCaps->microcodeVersion) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing microcode version in QEMU capabilities cache")); + goto cleanup; + } + if (virXPathBoolean("boolean(./package)", ctxt) > 0) { qemuCaps->package = virXPathString("string(./package)", ctxt); if (!qemuCaps->package && @@ -4230,6 +4240,9 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps) virBufferAsprintf(&buf, "<kvmVersion>%d</kvmVersion>\n", qemuCaps->kvmVersion); + virBufferAsprintf(&buf, "<microcodeVersion>%u</microcodeVersion>\n", + qemuCaps->microcodeVersion); + if (qemuCaps->package) virBufferAsprintf(&buf, "<package>%s</package>\n", qemuCaps->package); @@ -4371,6 +4384,16 @@ virQEMUCapsIsValid(void *data, return false; } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) && + priv->microcodeVersion != qemuCaps->microcodeVersion) { + VIR_DEBUG("Outdated capabilities for '%s': microcode version changed " + "(%u vs %u)", + qemuCaps->binary, + priv->microcodeVersion, + qemuCaps->microcodeVersion); + return false; + } + return true; } @@ -5197,6 +5220,7 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch, const char *libDir, uid_t runUid, gid_t runGid, + unsigned int microcodeVersion, bool qmpOnly) { virQEMUCapsPtr qemuCaps; @@ -5253,6 +5277,9 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch, virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM); virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_QEMU); + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) + qemuCaps->microcodeVersion = microcodeVersion; + cleanup: VIR_FREE(qmperr); return qemuCaps; @@ -5274,6 +5301,7 @@ virQEMUCapsNewData(const char *binary, priv->libDir, priv->runUid, priv->runGid, + priv->microcodeVersion, false); } @@ -5356,7 +5384,8 @@ virFileCachePtr virQEMUCapsCacheNew(const char *libDir, const char *cacheDir, uid_t runUid, - gid_t runGid) + gid_t runGid, + unsigned int microcodeVersion) { char *capsCacheDir = NULL; virFileCachePtr cache = NULL; @@ -5379,6 +5408,7 @@ virQEMUCapsCacheNew(const char *libDir, priv->runUid = runUid; priv->runGid = runGid; + priv->microcodeVersion = microcodeVersion; cleanup: VIR_FREE(capsCacheDir); @@ -5856,3 +5886,11 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps, return -1; return 0; } + + +void +virQEMUCapsSetMicrocodeVersion(virQEMUCapsPtr qemuCaps, + unsigned int microcodeVersion) +{ + qemuCaps->microcodeVersion = microcodeVersion; +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index e73dbaa557..8bb0b6a232 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -520,8 +520,10 @@ void virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps, const char *machineType); virFileCachePtr virQEMUCapsCacheNew(const char *libDir, - const char *cacheDir, - uid_t uid, gid_t gid); + const char *cacheDir, + uid_t uid, + gid_t gid, + unsigned int microcodeVersion); virQEMUCapsPtr virQEMUCapsCacheLookup(virFileCachePtr cache, const char *binary); virQEMUCapsPtr virQEMUCapsCacheLookupCopy(virFileCachePtr cache, diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index 219daa3629..98d163b920 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -36,6 +36,7 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch, const char *libDir, uid_t runUid, gid_t runGid, + unsigned int microcodeVersion, bool qmpOnly); int virQEMUCapsLoadCache(virArch hostArch, @@ -102,4 +103,8 @@ int virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps, qemuMonitorPtr mon, bool tcg); + +void +virQEMUCapsSetMicrocodeVersion(virQEMUCapsPtr qemuCaps, + unsigned int microcodeVersion); #endif diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 97b194b057..1ebc1177f8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -633,6 +633,8 @@ qemuStateInitialize(bool privileged, char *hugepagePath = NULL; char *memoryBackingPath = NULL; size_t i; + virCPUDefPtr hostCPU = NULL; + unsigned int microcodeVersion = 0; if (VIR_ALLOC(qemu_driver) < 0) return -1; @@ -855,10 +857,15 @@ qemuStateInitialize(bool privileged, run_gid = cfg->group; } + if ((hostCPU = virCPUProbeHost(virArchFromHost()))) + microcodeVersion = hostCPU->microcodeVersion; + virCPUDefFree(hostCPU); + qemu_driver->qemuCapsCache = virQEMUCapsCacheNew(cfg->libDir, cfg->cacheDir, run_uid, - run_gid); + run_gid, + microcodeVersion); if (!qemu_driver->qemuCapsCache) goto error; diff --git a/tests/qemucapabilitiesdata/caps_1.2.2.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.2.2.x86_64.xml index d560811ab7..3001d487c6 100644 --- a/tests/qemucapabilitiesdata/caps_1.2.2.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.2.2.x86_64.xml @@ -112,6 +112,7 @@ <flag name='isa-serial'/> <version>1002002</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>26900</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='qemu64'/> diff --git a/tests/qemucapabilitiesdata/caps_1.3.1.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.3.1.x86_64.xml index 576475f7fa..283f30ef07 100644 --- a/tests/qemucapabilitiesdata/caps_1.3.1.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.3.1.x86_64.xml @@ -130,6 +130,7 @@ <flag name='isa-serial'/> <version>1003001</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>30198</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='qemu64'/> diff --git a/tests/qemucapabilitiesdata/caps_1.4.2.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.4.2.x86_64.xml index 0c271d3e41..200069ae86 100644 --- a/tests/qemucapabilitiesdata/caps_1.4.2.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.4.2.x86_64.xml @@ -131,6 +131,7 @@ <flag name='isa-serial'/> <version>1004002</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>30915</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml index 5c667975bf..e02c0961cd 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml @@ -143,6 +143,7 @@ <flag name='isa-serial'/> <version>1005003</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>47019</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml index 8ae07a91db..e3896685e9 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml @@ -148,6 +148,7 @@ <flag name='isa-serial'/> <version>1006000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>45248</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml index 34bd6be1cd..5b4d1ea661 100644 --- a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml @@ -150,6 +150,7 @@ <flag name='isa-serial'/> <version>1007000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>50692</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml index 0d7c144ff7..200e57adac 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml @@ -166,6 +166,7 @@ <flag name='isa-serial'/> <version>2001001</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>59488</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml index eeb1840d3f..51d19aacb1 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml @@ -187,6 +187,7 @@ <flag name='pl011'/> <version>2010000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>304138</microcodeVersion> <package> (v2.10.0)</package> <arch>aarch64</arch> <cpu type='kvm' name='pxa262'/> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml index 1e7b95ec02..b8309f35b3 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml @@ -187,6 +187,7 @@ <flag name='pl011'/> <version>2010000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>304138</microcodeVersion> <package> (v2.10.0)</package> <arch>aarch64</arch> <cpu type='kvm' name='pxa262'/> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml index ca55e11ebe..c866ce3ee8 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml @@ -185,6 +185,7 @@ <flag name='isa-serial'/> <version>2010000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>383421</microcodeVersion> <package> (v2.10.0)</package> <arch>ppc64</arch> <cpu type='kvm' name='default'/> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml index 64e70f54be..e9115d304e 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml @@ -147,6 +147,7 @@ <flag name='iscsi.password-secret'/> <version>2010000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>304153</microcodeVersion> <package></package> <arch>s390x</arch> <hostCPU type='kvm' model='z14-base' migratability='no'> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml index 8fea70a522..1687417081 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml @@ -230,6 +230,7 @@ <flag name='isa-serial'/> <version>2010000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>345185</microcodeVersion> <package> (v2.10.0)</package> <arch>x86_64</arch> <hostCPU type='kvm' model='base' migratability='yes'> diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml index 5007523c1f..9b315aecf4 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml @@ -191,6 +191,7 @@ <flag name='isa-serial'/> <version>2004000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>75653</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml index a9ad292d01..3096eadf72 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml @@ -197,6 +197,7 @@ <flag name='isa-serial'/> <version>2005000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>216775</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml index d3e2e18faa..4cdd894a97 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml @@ -176,6 +176,7 @@ <flag name='pl011'/> <version>2006000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>228838</microcodeVersion> <package></package> <arch>aarch64</arch> <cpu type='kvm' name='pxa262'/> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml index bc86d03537..5655af7d3d 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml @@ -176,6 +176,7 @@ <flag name='pl011'/> <version>2006000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>228838</microcodeVersion> <package></package> <arch>aarch64</arch> <cpu type='kvm' name='pxa262'/> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml index 27d99bd937..31701bb40b 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml @@ -171,6 +171,7 @@ <flag name='isa-serial'/> <version>2006000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>263602</microcodeVersion> <package></package> <arch>ppc64</arch> <cpu type='kvm' name='default'/> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml index 97621612ab..6ae19ffd36 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -207,6 +207,7 @@ <flag name='isa-serial'/> <version>2006000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>227579</microcodeVersion> <package></package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml index c2f310cd46..b6ec680d5c 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml @@ -138,6 +138,7 @@ <flag name='sclplmconsole'/> <version>2007000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>217559</microcodeVersion> <package></package> <arch>s390x</arch> <cpu type='kvm' name='host'/> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index e4ea9452c5..294ac126e5 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -211,6 +211,7 @@ <flag name='isa-serial'/> <version>2007000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>239276</microcodeVersion> <package> (v2.7.0)</package> <arch>x86_64</arch> <cpu type='kvm' name='Opteron_G5'/> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml index f6e024dc61..d788ad206e 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml @@ -140,6 +140,7 @@ <flag name='sclplmconsole'/> <version>2007093</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>242460</microcodeVersion> <package></package> <arch>s390x</arch> <hostCPU type='kvm' model='zEC12.2-base' migratability='no'> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml index c6d3e21d5c..156563d99a 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml @@ -213,6 +213,7 @@ <flag name='isa-serial'/> <version>2008000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>255931</microcodeVersion> <package> (v2.8.0)</package> <arch>x86_64</arch> <cpu type='kvm' name='host' usable='yes'/> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml index 96aa5d59fc..cca643a3a5 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml @@ -179,6 +179,7 @@ <flag name='isa-serial'/> <version>2009000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>347135</microcodeVersion> <package> (v2.9.0)</package> <arch>ppc64</arch> <cpu type='kvm' name='default'/> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml index baac0b7aeb..5d0f0aa6c6 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml @@ -143,6 +143,7 @@ <flag name='iscsi.password-secret'/> <version>2009000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>265878</microcodeVersion> <package></package> <arch>s390x</arch> <hostCPU type='kvm' model='z13.2-base' migratability='no'> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index 9f489129fb..907f543ee3 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -226,6 +226,7 @@ <flag name='isa-serial'/> <version>2009000</version> <kvmVersion>0</kvmVersion> + <microcodeVersion>321194</microcodeVersion> <package> (v2.9.0)</package> <arch>x86_64</arch> <hostCPU type='kvm' model='base' migratability='yes'> diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c index dde5f767b8..87807b4135 100644 --- a/tests/qemucapabilitiestest.c +++ b/tests/qemucapabilitiestest.c @@ -61,10 +61,16 @@ testQemuCaps(const void *opaque) qemuMonitorTestGetMonitor(mon)) < 0) goto cleanup; - if (virQEMUCapsGet(capsActual, QEMU_CAPS_KVM) && - virQEMUCapsInitQMPMonitorTCG(capsActual, - qemuMonitorTestGetMonitor(mon)) < 0) - goto cleanup; + if (virQEMUCapsGet(capsActual, QEMU_CAPS_KVM)) { + if (virQEMUCapsInitQMPMonitorTCG(capsActual, + qemuMonitorTestGetMonitor(mon)) < 0) + goto cleanup; + + /* Fill microcodeVersion with a "random" value which is the file + * length to provide a reproducible number for testing. + */ + virQEMUCapsSetMicrocodeVersion(capsActual, virFileLength(repliesFile, -1)); + } if (!(actual = virQEMUCapsFormatCache(capsActual))) goto cleanup; diff --git a/tests/qemucapsprobe.c b/tests/qemucapsprobe.c index 4b8d6229b4..a5f5a38b16 100644 --- a/tests/qemucapsprobe.c +++ b/tests/qemucapsprobe.c @@ -72,7 +72,7 @@ main(int argc, char **argv) return EXIT_FAILURE; if (!(caps = virQEMUCapsNewForBinaryInternal(VIR_ARCH_NONE, argv[1], "/tmp", - -1, -1, true))) + -1, -1, 0, true))) return EXIT_FAILURE; virObjectUnref(caps); diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 2c7124bf26..f8182033fc 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -603,7 +603,7 @@ int qemuTestDriverInit(virQEMUDriver *driver) /* Using /dev/null for libDir and cacheDir automatically produces errors * upon attempt to use any of them */ - driver->qemuCapsCache = virQEMUCapsCacheNew("/dev/null", "/dev/null", 0, 0); + driver->qemuCapsCache = virQEMUCapsCacheNew("/dev/null", "/dev/null", 0, 0, 0); if (!driver->qemuCapsCache) goto error; -- 2.15.1

On Thu, Jan 04, 2018 at 15:58:12 +0100, Jiri Denemark wrote:
From: Paolo Bonzini <pbonzini@redhat.com>
A microcode update can cause the CPUID bits to change; an example from the past was the update that disabled TSX on several Haswell and Broadwell machines.
Therefore, place microcode version in the virQEMUCaps struct and XML, and rebuild the cache if the versions do not match.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 40 +++++++++++++++++++++- src/qemu/qemu_capabilities.h | 6 ++-- src/qemu/qemu_capspriv.h | 5 +++ src/qemu/qemu_driver.c | 9 ++++- tests/qemucapabilitiesdata/caps_1.2.2.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.3.1.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.4.2.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 1 + .../caps_2.10.0-gicv2.aarch64.xml | 1 + .../caps_2.10.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + .../caps_2.6.0-gicv2.aarch64.xml | 1 + .../caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + tests/qemucapabilitiestest.c | 14 +++++--- tests/qemucapsprobe.c | 2 +- tests/testutilsqemu.c | 2 +- 32 files changed, 93 insertions(+), 10 deletions(-)
[...]
diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c index dde5f767b8..87807b4135 100644 --- a/tests/qemucapabilitiestest.c +++ b/tests/qemucapabilitiestest.c @@ -61,10 +61,16 @@ testQemuCaps(const void *opaque) qemuMonitorTestGetMonitor(mon)) < 0) goto cleanup;
- if (virQEMUCapsGet(capsActual, QEMU_CAPS_KVM) && - virQEMUCapsInitQMPMonitorTCG(capsActual, - qemuMonitorTestGetMonitor(mon)) < 0) - goto cleanup; + if (virQEMUCapsGet(capsActual, QEMU_CAPS_KVM)) { + if (virQEMUCapsInitQMPMonitorTCG(capsActual, + qemuMonitorTestGetMonitor(mon)) < 0) + goto cleanup; + + /* Fill microcodeVersion with a "random" value which is the file + * length to provide a reproducible number for testing. + */ + virQEMUCapsSetMicrocodeVersion(capsActual, virFileLength(repliesFile, -1)); + }
if (!(actual = virQEMUCapsFormatCache(capsActual))) goto cleanup;
Interresting approach. The only drawback will be that when updating capabilities this will generate 2 lines more of diff. ACK

No description so people might not have realized the implications of this patch series.... This patch series is a pre-requisite for the future patches that address the Spectre vulnerability. Those QEMU patches will introduce various new CPU models. When the Intel microcode update is installed, we need to be sure that libvirt refreshes its cache of QEMU capabilities and so detects the new CPU feature bits the microcode added and refreshes its understanding of QEMU CPU models accordingly. See this series for the new CPU models: https://www.redhat.com/archives/libvir-list/2018-January/msg00282.html On Thu, Jan 04, 2018 at 03:58:07PM +0100, Jiri Denemark wrote:
Jiri Denemark (1): cpu_x86: Rename virCPUx86MapInitialize
Paolo Bonzini (4): util: add virFileReadHeaderQuiet wrapper around virFileReadHeaderFD util: introduce virHostCPUGetMicrocodeVersion conf: include x86 microcode version in virsh capabilities qemu: capabilities: force update if the microcode version does not match
src/conf/cpu_conf.c | 14 +++++++ src/conf/cpu_conf.h | 1 + src/cpu/cpu_x86.c | 17 +++++++-- src/libvirt_private.syms | 2 + src/qemu/qemu_capabilities.c | 40 +++++++++++++++++++- src/qemu/qemu_capabilities.h | 6 ++- src/qemu/qemu_capspriv.h | 5 +++ src/qemu/qemu_driver.c | 9 ++++- src/util/virfile.c | 19 ++++++++++ src/util/virfile.h | 2 + src/util/virhostcpu.c | 43 ++++++++++++++++++++++ src/util/virhostcpu.h | 2 + tests/qemucapabilitiesdata/caps_1.2.2.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.3.1.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.4.2.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 1 + .../caps_2.10.0-gicv2.aarch64.xml | 1 + .../caps_2.10.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + .../caps_2.6.0-gicv2.aarch64.xml | 1 + .../caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + tests/qemucapabilitiestest.c | 14 +++++-- tests/qemucapsprobe.c | 2 +- tests/testutilsqemu.c | 2 +- 40 files changed, 189 insertions(+), 14 deletions(-)
-- 2.15.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (5)
-
Daniel P. Berrange
-
Jiri Denemark
-
Ján Tomko
-
Peter Krempa
-
Pino Toscano