3 p.m.
When migrating a domain with invtsc CPU feature enabled, the TSC
frequency of the destination host must match the frequency used when the
domain was started on the source host or the destination host has to
support TSC scaling.
If the frequencies do not match and the destination host does not
support TSC scaling, QEMU will fail to set the right TSC frequency when
starting vCPUs on the destination and thus migration will fail. However,
this is quite late since both host might have spent significant time
transferring memory and perhaps even storage data.
By adding the check to libvirt we can let migration fail before any data
starts to be sent over. If for some reason libvirt is unable to detect
the host's TSC frequency or scaling support, we'll just let QEMU try and
the migration will either succeed or fail later.
Luckily, we mandate TSC frequency to be explicitly set in the domain XML
to even allow migration of domains with invtsc. We can just check
whether the requested frequency is compatible with the current host
before starting QEMU.
And to let libvirt client decide whether it should even start the
migration to a specific host, this series adds host's TSC frequency and
scaling support to the host CPU capabilities XML.
https://bugzilla.redhat.com/show_bug.cgi?id=1641702
Jiri Denemark (5):
util: Add virHostCPUGetTscInfo
conf: Report TSC frequency in host CPU capabilities
cpu_x86: Fix placement of *CheckFeature functions
cpu_x86: Probe TSC frequency and scaling support
qemu: Check TSC frequency before starting QEMU
src/conf/cpu_conf.c | 48 ++++++++++++++++++++++++
src/conf/cpu_conf.h | 2 +
src/cpu/cpu_x86.c | 81 ++++++++++++++++++++++-------------------
src/qemu/qemu_process.c | 53 +++++++++++++++++++++++++++
src/util/virhostcpu.c | 71 ++++++++++++++++++++++++++++++++++++
src/util/virhostcpu.h | 11 ++++++
6 files changed, 229 insertions(+), 37 deletions(-)
--
2.21.0
3 p.m.
New subject: [libvirt] [PATCH 1/5] util: Add virHostCPUGetTscInfo
On a KVM x86_64 host which supports invariant TSC this function can be
used to detect the TSC frequency and the availability of TSC scaling.
The magic MSR numbers required to check if VMX scaling is supported on
the host are documented in Volume 3 of the Intel® 64 and IA-32
Architectures Software Developer’s Manual.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/util/virhostcpu.c | 71 +++++++++++++++++++++++++++++++++++++++++++
src/util/virhostcpu.h | 11 +++++++
2 files changed, 82 insertions(+)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index f4a62c74fa..8c00804b0e 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -1324,6 +1324,69 @@ virHostCPUGetMSR(unsigned long index,
return virHostCPUGetMSRFromKVM(index, msr);
}
+
+# define VMX_PROCBASED_CTLS2_MSR 0x48b
+# define VMX_USE_TSC_SCALING (1 << 25)
+
+/*
+ * This function should only be called when the host CPU supports invariant TSC
+ * (invtsc CPUID feature).
+ *
+ * Returns pointer to the TSC info structure on success,
+ * NULL when TSC cannot be probed otherwise.
+ */
+virHostCPUTscInfoPtr
+virHostCPUGetTscInfo(void)
+{
+ virHostCPUTscInfoPtr info;
+ VIR_AUTOCLOSE kvmFd = -1;
+ VIR_AUTOCLOSE vmFd = -1;
+ VIR_AUTOCLOSE vcpuFd = -1;
+ uint64_t msr = 0;
+ int rc;
+
+ if ((kvmFd = open(KVM_DEVICE, O_RDONLY)) < 0) {
+ virReportSystemError(errno, _("Unable to open %s"), KVM_DEVICE);
+ return NULL;
+ }
+
+ if ((vmFd = ioctl(kvmFd, KVM_CREATE_VM, 0)) < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to create KVM VM for TSC probing"));
+ return NULL;
+ }
+
+ if ((vcpuFd = ioctl(vmFd, KVM_CREATE_VCPU, 0)) < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to create KVM vCPU for TSC probing"));
+ return NULL;
+ }
+
+ if ((rc = ioctl(vcpuFd, KVM_GET_TSC_KHZ)) < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to probe TSC frequency"));
+ return NULL;
+ }
+
+ if (VIR_ALLOC(info) < 0)
+ return NULL;
+
+ info->frequency = rc * 1000ULL;
+
+ if (virHostCPUGetMSR(VMX_PROCBASED_CTLS2_MSR, &msr) == 0) {
+ /* High 32 bits of the MSR value indicate whether specific control
+ * can be set to 1. */
+ msr >>= 32;
+
+ info->scaling = virTristateBoolFromBool(!!(msr & VMX_USE_TSC_SCALING));
+ }
+
+ VIR_DEBUG("Detected TSC frequency %llu Hz, scaling %s",
+ info->frequency, virTristateBoolTypeToString(info->scaling));
+
+ return info;
+}
+
#else
int
@@ -1335,6 +1398,14 @@ virHostCPUGetMSR(unsigned long index ATTRIBUTE_UNUSED,
return -1;
}
+virHostCPUTscInfoPtr
+virHostCPUGetTscInfo(void)
+{
+ virReportSystemError(ENOSYS, "%s",
+ _("Probing TSC is not supported on this platform"));
+ return NULL;
+}
+
#endif /* HAVE_LINUX_KVM_H && defined(KVM_GET_MSRS) && \
(defined(__i386__) || defined(__x86_64__)) && \
(defined(__linux__) || defined(__FreeBSD__)) */
diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h
index 0d20dbef61..b822bc11a8 100644
--- a/src/util/virhostcpu.h
+++ b/src/util/virhostcpu.h
@@ -25,6 +25,15 @@
# include "internal.h"
# include "virarch.h"
# include "virbitmap.h"
+# include "virenum.h"
+
+
+typedef struct _virHostCPUTscInfo virHostCPUTscInfo;
+typedef virHostCPUTscInfo *virHostCPUTscInfoPtr;
+struct _virHostCPUTscInfo {
+ unsigned long long frequency;
+ virTristateBool scaling;
+};
int virHostCPUGetStats(int cpuNum,
@@ -69,4 +78,6 @@ unsigned int virHostCPUGetMicrocodeVersion(void);
int virHostCPUGetMSR(unsigned long index,
uint64_t *msr);
+virHostCPUTscInfoPtr virHostCPUGetTscInfo(void);
+
#endif /* LIBVIRT_VIRHOSTCPU_H */
--
2.21.0
3 p.m.
New subject: [libvirt] [PATCH 2/5] conf: Report TSC frequency in host CPU capabilities
This patch adds a new
<counter name='tsc' frequency='N' scaling='on|off'/>
element into the host CPU capabilities XML.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/conf/cpu_conf.c | 48 +++++++++++++++++++++++++++++++++++++++++++++
src/conf/cpu_conf.h | 2 ++
2 files changed, 50 insertions(+)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index bd2beab33e..dc46e7f57a 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -112,6 +112,7 @@ virCPUDefFree(virCPUDefPtr def)
virCPUDefFreeModel(def);
VIR_FREE(def->cache);
+ VIR_FREE(def->tsc);
VIR_FREE(def);
}
@@ -233,6 +234,13 @@ virCPUDefCopyWithoutModel(const virCPUDef *cpu)
*copy->cache = *cpu->cache;
}
+ if (cpu->tsc) {
+ if (VIR_ALLOC(copy->tsc) < 0)
+ goto error;
+
+ *copy->tsc = *cpu->tsc;
+ }
+
return copy;
error:
@@ -286,6 +294,8 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
char *cpuMode;
char *fallback = NULL;
char *vendor_id = NULL;
+ char *tscScaling = NULL;
+ virHostCPUTscInfoPtr tsc = NULL;
int ret = -1;
*cpu = NULL;
@@ -402,6 +412,32 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
_("invalid microcode version"));
goto cleanup;
}
+
+ if (virXPathBoolean("boolean(./counter[@name='tsc'])", ctxt)
> 0) {
+ if (VIR_ALLOC(tsc) < 0)
+ goto cleanup;
+
+ if (virXPathULongLong("./counter[@name='tsc']/@frequency",
ctxt,
+ &tsc->frequency) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Invalid TSC frequency"));
+ goto cleanup;
+ }
+
+ tscScaling =
virXPathString("string(./counter[@name='tsc']/@scaling)",
+ ctxt);
+ if (tscScaling) {
+ int scaling = virTristateBoolTypeFromString(tscScaling);
+ if (scaling < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Invalid TSC scaling attribute"));
+ goto cleanup;
+ }
+ tsc->scaling = scaling;
+ }
+
+ VIR_STEAL_PTR(def->tsc, tsc);
+ }
}
if (!(def->model = virXPathString("string(./model[1])", ctxt))
&&
@@ -587,6 +623,8 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
VIR_FREE(fallback);
VIR_FREE(vendor_id);
VIR_FREE(nodes);
+ VIR_FREE(tscScaling);
+ VIR_FREE(tsc);
virCPUDefFree(def);
return ret;
}
@@ -744,6 +782,16 @@ virCPUDefFormatBuf(virBufferPtr buf,
virBufferAsprintf(buf, "<microcode version='%u'/>\n",
def->microcodeVersion);
+ if (def->type == VIR_CPU_TYPE_HOST && def->tsc) {
+ virBufferAddLit(buf, "<counter name='tsc'");
+ virBufferAsprintf(buf, " frequency='%llu'",
def->tsc->frequency);
+ if (def->tsc->scaling) {
+ virBufferAsprintf(buf, " scaling='%s'",
+ virTristateBoolTypeToString(def->tsc->scaling));
+ }
+ virBufferAddLit(buf, "/>\n");
+ }
+
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 c98db65693..51bf744fab 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -28,6 +28,7 @@
# include "virarch.h"
# include "numa_conf.h"
# include "virenum.h"
+# include "virhostcpu.h"
# define VIR_CPU_VENDOR_ID_LENGTH 12
@@ -139,6 +140,7 @@ struct _virCPUDef {
size_t nfeatures_max;
virCPUFeatureDefPtr features;
virCPUCacheDefPtr cache;
+ virHostCPUTscInfoPtr tsc;
};
--
2.21.0
3 p.m.
New subject: [libvirt] [PATCH 3/5] cpu_x86: Fix placement of *CheckFeature functions
Commit 0a97486e09 moved them outside #ifdef, but after virCPUx86GetHost,
which will start calling them in the following patch.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu/cpu_x86.c | 72 +++++++++++++++++++++++------------------------
1 file changed, 35 insertions(+), 37 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index f05bfa24e0..cb03123787 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2378,6 +2378,41 @@ x86Encode(virArch arch,
}
+static int
+virCPUx86CheckFeature(const virCPUDef *cpu,
+ const char *name)
+{
+ int ret = -1;
+ virCPUx86MapPtr map;
+ virCPUx86ModelPtr model = NULL;
+
+ if (!(map = virCPUx86GetMap()))
+ return -1;
+
+ if (!(model = x86ModelFromCPU(cpu, map, -1)))
+ goto cleanup;
+
+ ret = x86FeatureInData(name, &model->data, map);
+
+ cleanup:
+ x86ModelFree(model);
+ return ret;
+}
+
+
+static int
+virCPUx86DataCheckFeature(const virCPUData *data,
+ const char *name)
+{
+ virCPUx86MapPtr map;
+
+ if (!(map = virCPUx86GetMap()))
+ return -1;
+
+ return x86FeatureInData(name, &data->data.x86, map);
+}
+
+
#if defined(__i386__) || defined(__x86_64__)
static inline void
cpuidCall(virCPUx86CPUID *cpuid)
@@ -2707,8 +2742,6 @@ cpuidSet(uint32_t base, virCPUDataPtr data)
}
-
-
static int
virCPUx86GetHost(virCPUDefPtr cpu,
virDomainCapsCPUModelsPtr models)
@@ -2736,41 +2769,6 @@ virCPUx86GetHost(virCPUDefPtr cpu,
#endif
-static int
-virCPUx86CheckFeature(const virCPUDef *cpu,
- const char *name)
-{
- int ret = -1;
- virCPUx86MapPtr map;
- virCPUx86ModelPtr model = NULL;
-
- if (!(map = virCPUx86GetMap()))
- return -1;
-
- if (!(model = x86ModelFromCPU(cpu, map, -1)))
- goto cleanup;
-
- ret = x86FeatureInData(name, &model->data, map);
-
- cleanup:
- x86ModelFree(model);
- return ret;
-}
-
-
-static int
-virCPUx86DataCheckFeature(const virCPUData *data,
- const char *name)
-{
- virCPUx86MapPtr map;
-
- if (!(map = virCPUx86GetMap()))
- return -1;
-
- return x86FeatureInData(name, &data->data.x86, map);
-}
-
-
static virCPUDefPtr
virCPUx86Baseline(virCPUDefPtr *cpus,
unsigned int ncpus,
--
2.21.0
3 p.m.
New subject: [libvirt] [PATCH 4/5] cpu_x86: Probe TSC frequency and scaling support
When the host CPU supports invariant TSC the host CPU definition created
by virCPUx86GetHost will contain (unless probing fails for some reason)
addition TSC related data.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu/cpu_x86.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index cb03123787..689b6cdaf5 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2762,6 +2762,15 @@ virCPUx86GetHost(virCPUDefPtr cpu,
ret = x86DecodeCPUData(cpu, cpuData, models);
cpu->microcodeVersion = virHostCPUGetMicrocodeVersion();
+ /* Probing for TSC frequency makes sense only if the CPU supports
+ * invariant TSC (Linux calls this constant_tsc in /proc/cpuinfo). */
+ if (virCPUx86DataCheckFeature(cpuData, "invtsc") == 1) {
+ VIR_DEBUG("Checking invariant TSC frequency");
+ cpu->tsc = virHostCPUGetTscInfo();
+ } else {
+ VIR_DEBUG("Host CPU does not support invariant TSC");
+ }
+
cleanup:
virCPUx86DataFree(cpuData);
return ret;
--
2.21.0
10:24 a.m.
New subject: [libvirt] [PATCH 4/5] cpu_x86: Probe TSC frequency and scaling support
On Fri, May 31, 2019 at 02:00:10PM +0200, Jiri Denemark wrote:
When the host CPU supports invariant TSC the host CPU definition
created
by virCPUx86GetHost will contain (unless probing fails for some reason)
addition TSC related data.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu/cpu_x86.c | 9 +++++++++
1 file changed, 9 insertions(+)
This breaks the tests for me:
1) QEMU ARGV-2-XML machine-core-on ... Got unexpected
warning from qemuParseCommandLineString:
2019-06-03 07:19:24.832+0000: 15141: info : libvirt version: 5.4.0
2019-06-03 07:19:24.832+0000: 15141: info : hostname: icr
2019-06-03 07:19:24.832+0000: 15141: error : virHostCPUGetTscInfo:1349 : Unable to open
/dev/kvm: Permission denied
FAILED
Jano
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index cb03123787..689b6cdaf5 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2762,6 +2762,15 @@ virCPUx86GetHost(virCPUDefPtr cpu,
ret = x86DecodeCPUData(cpu, cpuData, models);
cpu->microcodeVersion = virHostCPUGetMicrocodeVersion();
+ /* Probing for TSC frequency makes sense only if the CPU supports
+ * invariant TSC (Linux calls this constant_tsc in /proc/cpuinfo). */
+ if (virCPUx86DataCheckFeature(cpuData, "invtsc") == 1) {
+ VIR_DEBUG("Checking invariant TSC frequency");
+ cpu->tsc = virHostCPUGetTscInfo();
+ } else {
+ VIR_DEBUG("Host CPU does not support invariant TSC");
+ }
+
cleanup:
virCPUx86DataFree(cpuData);
return ret;
--
2.21.0
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
2:43 p.m.
New subject: [libvirt] [PATCH 4/5] cpu_x86: Probe TSC frequency and scaling support
On Mon, Jun 03, 2019 at 09:24:14 +0200, Ján Tomko wrote:
On Fri, May 31, 2019 at 02:00:10PM +0200, Jiri Denemark wrote:
>When the host CPU supports invariant TSC the host CPU definition created
>by virCPUx86GetHost will contain (unless probing fails for some reason)
>addition TSC related data.
>
>Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
>---
> src/cpu/cpu_x86.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
This breaks the tests for me:
1) QEMU ARGV-2-XML machine-core-on ... Got unexpected
warning from qemuParseCommandLineString:
2019-06-03 07:19:24.832+0000: 15141: info : libvirt version: 5.4.0
2019-06-03 07:19:24.832+0000: 15141: info : hostname: icr
2019-06-03 07:19:24.832+0000: 15141: error : virHostCPUGetTscInfo:1349 : Unable to open
/dev/kvm: Permission denied
FAILED
Interesting. This shows an existing issue in qemuargv2xmltest which
would probe the host CPU directly. I'll fix this in v2.
Jirka
3 p.m.
New subject: [libvirt] [PATCH 5/5] qemu: Check TSC frequency before starting QEMU
When migrating a domain with invtsc CPU feature enabled, the TSC
frequency of the destination host must match the frequency used when the
domain was started on the source host or the destination host has to
support TSC scaling.
If the frequencies do not match and the destination host does not
support TSC scaling, QEMU will fail to set the right TSC frequency when
starting vCPUs on the destination and thus migration will fail. However,
this is quite late since both host might have spent significant time
transferring memory and perhaps even storage data.
By adding the check to libvirt we can let migration fail before any data
starts to be sent over. If for some reason libvirt is unable to detect
the host's TSC frequency or scaling support, we'll just let QEMU try and
the migration will either succeed or fail later.
Luckily, we mandate TSC frequency to be explicitly set in the domain XML
to even allow migration of domains with invtsc. We can just check
whether the requested frequency is compatible with the current host
before starting QEMU.
https://bugzilla.redhat.com/show_bug.cgi?id=1641702
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_process.c | 53 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index fa5909e9be..481daa937e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5282,6 +5282,56 @@ qemuProcessStartValidateDisks(virDomainObjPtr vm,
}
+static int
+qemuProcessStartValidateTSC(virDomainObjPtr vm,
+ virCapsPtr caps)
+{
+ size_t i;
+ unsigned long long freq = 0;
+ virHostCPUTscInfoPtr tsc;
+
+ for (i = 0; i < vm->def->clock.ntimers; i++) {
+ virDomainTimerDefPtr timer = vm->def->clock.timers[i];
+
+ if (timer->name == VIR_DOMAIN_TIMER_NAME_TSC &&
+ timer->frequency > 0) {
+ freq = timer->frequency;
+ break;
+ }
+ }
+
+ if (freq == 0)
+ return 0;
+
+ VIR_DEBUG("Requested TSC frequency %llu Hz", freq);
+
+ if (!caps->host.cpu || !caps->host.cpu->tsc) {
+ VIR_DEBUG("Host TSC frequency could not be probed");
+ return 0;
+ }
+
+ tsc = caps->host.cpu->tsc;
+ VIR_DEBUG("Host TSC frequency %llu Hz, scaling %s",
+ tsc->frequency, virTristateBoolTypeToString(tsc->scaling));
+
+ if (freq == tsc->frequency || tsc->scaling == VIR_TRISTATE_BOOL_YES)
+ return 0;
+
+ if (tsc->scaling == VIR_TRISTATE_BOOL_ABSENT) {
+ VIR_DEBUG("TSC frequencies do not match and scaling support is "
+ "unknown, QEMU will try and possibly fail later");
+ return 0;
+ }
+
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Requested TSC frequency %llu Hz does not match "
+ "host (%llu Hz) and TSC scaling is not supported "
+ "by the host CPU"),
+ freq, tsc->frequency);
+ return -1;
+}
+
+
/**
* qemuProcessStartValidate:
* @vm: domain object
@@ -5346,6 +5396,9 @@ qemuProcessStartValidate(virQEMUDriverPtr driver,
if (qemuProcessStartValidateDisks(vm, qemuCaps) < 0)
return -1;
+ if (qemuProcessStartValidateTSC(vm, caps) < 0)
+ return -1;
+
VIR_DEBUG("Checking for any possible (non-fatal) issues");
qemuProcessStartWarnShmem(vm);
--
2.21.0
2146
days inactive
2149
days old
7 comments
2 participants
participants (2)
-
Jiri Denemark -
Ján Tomko