[libvirt] [PATCH 1/3] cpu_map.xml: add cmt feature to x86
Some Intel processor families (e.g. the Intel Xeon processor E5 v3 family) introduced CMT (Cache Monitoring Technology) to measure the usage of cache by applications running on the platform. This patch add it into x86 part of cpu_map.xml. Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- .gnulib | 2 +- src/cpu/cpu_map.xml | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.gnulib b/.gnulib index f39477d..106a386 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit f39477dba778e99392948dd3dd19ec0d46aee932 +Subproject commit 106a3866d01f9dd57ab4f10dbeb0d5a8db73a9f7 diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index b9e95cf..14ccbd8 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -317,6 +317,9 @@ <feature name='rtm'> <cpuid function='0x00000007' ebx='0x00000800'/> </feature> + <feature name='cmt'> + <cpuid function='0x00000007' ebx='0x00001000'/> + </feature> <feature name='rdseed'> <cpuid function='0x00000007' ebx='0x00040000'/> </feature> -- 1.9.1
One RFC in https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html CMT (Cache Monitoring Technology) can be used to measure the usage of cache by VM running on the host. This patch will extend the bulk stats API (virDomainListGetStats) to add this field. Applications based on libvirt can use this API to achieve cache usage of VM. Because CMT implementation in Linux kernel is based on perf mechanism, this patch will enable perf event for CMT when VM is created and disable it when VM is destroyed. Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_domain.h | 3 ++ src/qemu/qemu_driver.c | 48 ++++++++++++++++++++++ src/qemu/qemu_process.c | 86 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 138 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index e8202cf..fb5e1f4 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1764,6 +1764,7 @@ typedef enum { VIR_DOMAIN_STATS_VCPU = (1 << 3), /* return domain virtual CPU info */ VIR_DOMAIN_STATS_INTERFACE = (1 << 4), /* return domain interfaces info */ VIR_DOMAIN_STATS_BLOCK = (1 << 5), /* return domain block info */ + VIR_DOMAIN_STATS_CACHE = (1 << 6), /* return domain block info */ } virDomainStatsTypes; typedef enum { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 54e1e7b..31bce33 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -196,6 +196,9 @@ struct _qemuDomainObjPrivate { bool hookRun; /* true if there was a hook run over this domain */ + int cmt_fd; /* perf handler for CMT */ + + /* Bitmaps below hold data from the auto NUMA feature */ virBitmapPtr autoNodeset; virBitmapPtr autoCpuset; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4cfae03..8c678c9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19320,6 +19320,53 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, #undef QEMU_ADD_COUNT_PARAM +static int +qemuDomainGetStatsCache(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + qemuDomainObjPrivatePtr priv = dom->privateData; + FILE *fd; + unsigned long long cache = 0; + int scaling_factor = 0; + + if (priv->cmt_fd <= 0) + return -1; + + if (read(priv->cmt_fd, &cache, sizeof(uint64_t)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to read cache data")); + return -1; + } + + fd = fopen("/sys/devices/intel_cqm/events/llc_occupancy.scale", "r"); + if (!fd) { + virReportSystemError(errno, "%s", + _("Unable to open CMT scale file")); + return -1; + } + if (fscanf(fd, "%d", &scaling_factor) != 1) { + virReportSystemError(errno, "%s", + _("Unable to read CMT scale file")); + VIR_FORCE_FCLOSE(fd); + return -1; + } + VIR_FORCE_FCLOSE(fd); + + cache *= scaling_factor; + + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cache.current", + cache) < 0) + return -1; + + return 0; +} + typedef int (*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver, virDomainObjPtr dom, @@ -19340,6 +19387,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, false }, { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE, false }, { qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK, true }, + { qemuDomainGetStatsCache, VIR_DOMAIN_STATS_CACHE, false }, { NULL, 0, false } }; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ba84182..00b889d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -25,8 +25,11 @@ #include <unistd.h> #include <signal.h> #include <sys/stat.h> +#include <sys/ioctl.h> +#include <sys/syscall.h> #if defined(__linux__) # include <linux/capability.h> +# include <linux/perf_event.h> #elif defined(__FreeBSD__) # include <sys/param.h> # include <sys/cpuset.h> @@ -4274,6 +4277,75 @@ qemuLogOperation(virDomainObjPtr vm, goto cleanup; } +/* + * Enable CMT(Cache Monitoring Technology) to measure the usage of + * cache by VM running on the node. + * + * Because the hypervisor implement CMT support basedon perf mechanism, + * we should enable perf event for CMT. The function 'sys_erf_event_open' + * is perf syscall wrapper. + */ +#ifdef __linux__ +static long sys_perf_event_open(struct perf_event_attr *hw_event, + pid_t pid, int cpu, int group_fd, + unsigned long flags) +{ + return syscall(__NR_perf_event_open, hw_event, pid, cpu, + group_fd, flags); +} +static int qemuCmtEnable(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + struct perf_event_attr cmt_attr; + int event_type; + FILE *fp; + + fp = fopen("/sys/devices/intel_cqm/type", "r"); + if (!fp) { + virReportSystemError(errno, "%s", + _("CMT is not available on this host")); + return -1; + } + if (fscanf(fp, "%d", &event_type) != 1) { + virReportSystemError(errno, "%s", + _("Unable to read event type file.")); + VIR_FORCE_FCLOSE(fp); + return -1; + } + VIR_FORCE_FCLOSE(fp); + + memset(&cmt_attr, 0, sizeof(struct perf_event_attr)); + cmt_attr.size = sizeof(struct perf_event_attr); + cmt_attr.type = event_type; + cmt_attr.config = 1; + cmt_attr.inherit = 1; + cmt_attr.disabled = 1; + cmt_attr.enable_on_exec = 0; + + priv->cmt_fd = sys_perf_event_open(&cmt_attr, vm->pid, -1, -1, 0); + if (priv->cmt_fd < 0) { + virReportSystemError(errno, + _("Unable to open perf type=%d for pid=%d"), + event_type, vm->pid); + return -1; + } + + if (ioctl(priv->cmt_fd, PERF_EVENT_IOC_ENABLE) < 0) { + virReportSystemError(errno, "%s", + _("Unable to enable perf event for CMT")); + return -1; + } + + return 0; +} +#else +static int qemuCmtEnable(virDomainObjPtr vm) +{ + virReportUnsupportedError(); + return -1; +} +#endif + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4954,6 +5026,11 @@ int qemuProcessStart(virConnectPtr conn, if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto cleanup; + VIR_DEBUG("Setting CMT perf counter"); + if (qemuCmtEnable(vm) < 0) + virReportSystemError(errno, "%s", + _("CMT is not available on this host")); + /* finally we can call the 'started' hook script if any */ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { char *xml = qemuDomainDefFormatXML(driver, vm->def, 0); @@ -5122,6 +5199,15 @@ void qemuProcessStop(virQEMUDriverPtr driver, virPortAllocatorRelease(driver->migrationPorts, priv->nbdPort); priv->nbdPort = 0; + /* Disable CMT */ + if (priv->cmt_fd > 0) { + if (ioctl(priv->cmt_fd, PERF_EVENT_IOC_DISABLE) < 0) { + virReportSystemError(errno, "%s", + _("Unable to disable perf event for CMT")); + } + VIR_FORCE_CLOSE(priv->cmt_fd); + } + if (priv->agent) { qemuAgentClose(priv->agent); priv->agent = NULL; -- 1.9.1
On Thu, Oct 29, 2015 at 14:02:29 +0800, Qiaowei Ren wrote:
One RFC in https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html
CMT (Cache Monitoring Technology) can be used to measure the usage of cache by VM running on the host. This patch will extend the bulk stats API (virDomainListGetStats) to add this field. Applications based on libvirt can use this API to achieve cache usage of VM. Because CMT implementation in Linux kernel is based on perf mechanism, this patch will enable perf event for CMT when VM is created and disable it when VM is destroyed.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_domain.h | 3 ++ src/qemu/qemu_driver.c | 48 ++++++++++++++++++++++ src/qemu/qemu_process.c | 86 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 138 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index e8202cf..fb5e1f4 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1764,6 +1764,7 @@ typedef enum { VIR_DOMAIN_STATS_VCPU = (1 << 3), /* return domain virtual CPU info */ VIR_DOMAIN_STATS_INTERFACE = (1 << 4), /* return domain interfaces info */ VIR_DOMAIN_STATS_BLOCK = (1 << 5), /* return domain block info */ + VIR_DOMAIN_STATS_CACHE = (1 << 6), /* return domain block info */
This flag is not documented anywhere and the comment is completely wrong.
} virDomainStatsTypes;
typedef enum { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 54e1e7b..31bce33 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -196,6 +196,9 @@ struct _qemuDomainObjPrivate {
bool hookRun; /* true if there was a hook run over this domain */
+ int cmt_fd; /* perf handler for CMT */
So you declare cmt_fd as int...
+ +
Why two empty lines?
/* Bitmaps below hold data from the auto NUMA feature */ virBitmapPtr autoNodeset; virBitmapPtr autoCpuset; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4cfae03..8c678c9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19320,6 +19320,53 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
#undef QEMU_ADD_COUNT_PARAM
+static int +qemuDomainGetStatsCache(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + qemuDomainObjPrivatePtr priv = dom->privateData; + FILE *fd; + unsigned long long cache = 0; + int scaling_factor = 0; + + if (priv->cmt_fd <= 0)
Shouldn't this only check for cmt_fd < 0?
+ return -1; + + if (read(priv->cmt_fd, &cache, sizeof(uint64_t)) < 0) {
Shouldn't cache be declared as uint64_t then?
+ virReportSystemError(errno, "%s", + _("Unable to read cache data")); + return -1; + } + + fd = fopen("/sys/devices/intel_cqm/events/llc_occupancy.scale", "r"); + if (!fd) { + virReportSystemError(errno, "%s", + _("Unable to open CMT scale file")); + return -1; + } + if (fscanf(fd, "%d", &scaling_factor) != 1) { + virReportSystemError(errno, "%s", + _("Unable to read CMT scale file")); + VIR_FORCE_FCLOSE(fd); + return -1; + } + VIR_FORCE_FCLOSE(fd); + + cache *= scaling_factor; + + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cache.current", + cache) < 0)
Any documentation of this new statistics is missing. The commit message doesn't really help understanding it either.
+ return -1; + + return 0; +} + typedef int (*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver, virDomainObjPtr dom, @@ -19340,6 +19387,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, false }, { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE, false }, { qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK, true }, + { qemuDomainGetStatsCache, VIR_DOMAIN_STATS_CACHE, false }, { NULL, 0, false } };
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ba84182..00b889d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -25,8 +25,11 @@ #include <unistd.h> #include <signal.h> #include <sys/stat.h> +#include <sys/ioctl.h> +#include <sys/syscall.h> #if defined(__linux__) # include <linux/capability.h> +# include <linux/perf_event.h>
What if there's no perf_event.h header file?
#elif defined(__FreeBSD__) # include <sys/param.h> # include <sys/cpuset.h> @@ -4274,6 +4277,75 @@ qemuLogOperation(virDomainObjPtr vm, goto cleanup; }
+/* + * Enable CMT(Cache Monitoring Technology) to measure the usage of + * cache by VM running on the node. + * + * Because the hypervisor implement CMT support basedon perf mechanism, + * we should enable perf event for CMT. The function 'sys_erf_event_open' + * is perf syscall wrapper. + */ +#ifdef __linux__ +static long sys_perf_event_open(struct perf_event_attr *hw_event, + pid_t pid, int cpu, int group_fd,
One parameter per line is easier to read.
+ unsigned long flags) +{ + return syscall(__NR_perf_event_open, hw_event, pid, cpu, + group_fd, flags);
Wrong indentation.
+} +static int qemuCmtEnable(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + struct perf_event_attr cmt_attr; + int event_type; + FILE *fp; + + fp = fopen("/sys/devices/intel_cqm/type", "r"); + if (!fp) { + virReportSystemError(errno, "%s", + _("CMT is not available on this host")); + return -1; + } + if (fscanf(fp, "%d", &event_type) != 1) { + virReportSystemError(errno, "%s", + _("Unable to read event type file.")); + VIR_FORCE_FCLOSE(fp); + return -1; + } + VIR_FORCE_FCLOSE(fp); + + memset(&cmt_attr, 0, sizeof(struct perf_event_attr)); + cmt_attr.size = sizeof(struct perf_event_attr); + cmt_attr.type = event_type; + cmt_attr.config = 1; + cmt_attr.inherit = 1; + cmt_attr.disabled = 1; + cmt_attr.enable_on_exec = 0; + + priv->cmt_fd = sys_perf_event_open(&cmt_attr, vm->pid, -1, -1, 0);
...while here you assign long (from sys_perf_event_open) to cmt_fd.
+ if (priv->cmt_fd < 0) { + virReportSystemError(errno, + _("Unable to open perf type=%d for pid=%d"), + event_type, vm->pid); + return -1; + } + + if (ioctl(priv->cmt_fd, PERF_EVENT_IOC_ENABLE) < 0) { + virReportSystemError(errno, "%s", + _("Unable to enable perf event for CMT")); + return -1; + } + + return 0; +} +#else +static int qemuCmtEnable(virDomainObjPtr vm) +{ + virReportUnsupportedError(); + return -1; +} +#endif + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4954,6 +5026,11 @@ int qemuProcessStart(virConnectPtr conn, if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto cleanup;
+ VIR_DEBUG("Setting CMT perf counter"); + if (qemuCmtEnable(vm) < 0) + virReportSystemError(errno, "%s", + _("CMT is not available on this host")); +
By setting en error here, you overwrite any useful error set inside qemuCmtEnable. Anyway since you apparently want to make the error non fatal, you should not even set it. The messages should be logged as warnings or even lower. If there is no performance or other impact of having cmt enabled, we could enable it on hosts that support it, but we either need to log any errors with info level or first check whether the host supports cmt and only try to enable it if it does, in which case we could report the errors as warnings. However, if a negative impact of enabling cmt is possible, we need an explicit configuration option (turned off by default) that would ask libvirt to enable cmt. Any errors could be logged as warnings in this case. And what if libvirtd is restarted? Shouldn't we call sys_perf_event_open again?
/* finally we can call the 'started' hook script if any */ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { char *xml = qemuDomainDefFormatXML(driver, vm->def, 0); @@ -5122,6 +5199,15 @@ void qemuProcessStop(virQEMUDriverPtr driver, virPortAllocatorRelease(driver->migrationPorts, priv->nbdPort); priv->nbdPort = 0;
+ /* Disable CMT */ + if (priv->cmt_fd > 0) {
cmt_fd >= 0 ?
+ if (ioctl(priv->cmt_fd, PERF_EVENT_IOC_DISABLE) < 0) { + virReportSystemError(errno, "%s", + _("Unable to disable perf event for CMT"));
A warning should be good enough.
+ } + VIR_FORCE_CLOSE(priv->cmt_fd); + } +
This should go into a dedicated function.
if (priv->agent) { qemuAgentClose(priv->agent); priv->agent = NULL;
Jirka
-----Original Message----- From: Jiri Denemark [mailto:jdenemar@redhat.com] Sent: Thursday, October 29, 2015 5:52 PM To: Ren, Qiaowei Cc: libvir-list@redhat.com; Feng, Shaohe Subject: Re: [libvirt] [PATCH 2/3] Qemu: add CMT support
On Thu, Oct 29, 2015 at 14:02:29 +0800, Qiaowei Ren wrote:
One RFC in https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html
CMT (Cache Monitoring Technology) can be used to measure the usage of cache by VM running on the host. This patch will extend the bulk stats API (virDomainListGetStats) to add this field. Applications based on libvirt can use this API to achieve cache usage of VM. Because CMT implementation in Linux kernel is based on perf mechanism, this patch will enable perf event for CMT when VM is created and disable it when VM is destroyed.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_domain.h | 3 ++ src/qemu/qemu_driver.c | 48 ++++++++++++++++++++++ src/qemu/qemu_process.c | 86 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 138 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index e8202cf..fb5e1f4 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1764,6 +1764,7 @@ typedef enum { VIR_DOMAIN_STATS_VCPU = (1 << 3), /* return domain virtual CPU info */ VIR_DOMAIN_STATS_INTERFACE = (1 << 4), /* return domain interfaces info */ VIR_DOMAIN_STATS_BLOCK = (1 << 5), /* return domain block info */ + VIR_DOMAIN_STATS_CACHE = (1 << 6), /* return domain block info */
This flag is not documented anywhere and the comment is completely wrong.
} virDomainStatsTypes;
typedef enum { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 54e1e7b..31bce33 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -196,6 +196,9 @@ struct _qemuDomainObjPrivate {
bool hookRun; /* true if there was a hook run over this domain */
+ int cmt_fd; /* perf handler for CMT */
So you declare cmt_fd as int...
+ +
Why two empty lines?
/* Bitmaps below hold data from the auto NUMA feature */ virBitmapPtr autoNodeset; virBitmapPtr autoCpuset; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4cfae03..8c678c9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19320,6 +19320,53 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
#undef QEMU_ADD_COUNT_PARAM
+static int +qemuDomainGetStatsCache(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) { + qemuDomainObjPrivatePtr priv = dom->privateData; + FILE *fd; + unsigned long long cache = 0; + int scaling_factor = 0; + + if (priv->cmt_fd <= 0)
Shouldn't this only check for cmt_fd < 0?
+ return -1; + + if (read(priv->cmt_fd, &cache, sizeof(uint64_t)) < 0) {
Shouldn't cache be declared as uint64_t then?
+ virReportSystemError(errno, "%s", + _("Unable to read cache data")); + return -1; + } + + fd = fopen("/sys/devices/intel_cqm/events/llc_occupancy.scale", "r"); + if (!fd) { + virReportSystemError(errno, "%s", + _("Unable to open CMT scale file")); + return -1; + } + if (fscanf(fd, "%d", &scaling_factor) != 1) { + virReportSystemError(errno, "%s", + _("Unable to read CMT scale file")); + VIR_FORCE_FCLOSE(fd); + return -1; + } + VIR_FORCE_FCLOSE(fd); + + cache *= scaling_factor; + + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cache.current", + cache) < 0)
Any documentation of this new statistics is missing. The commit message doesn't really help understanding it either.
+ return -1; + + return 0; +} + typedef int (*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver, virDomainObjPtr dom, @@ -19340,6 +19387,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, false }, { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE, false }, { qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK, true }, + { qemuDomainGetStatsCache, VIR_DOMAIN_STATS_CACHE, false }, { NULL, 0, false } };
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ba84182..00b889d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -25,8 +25,11 @@ #include <unistd.h> #include <signal.h> #include <sys/stat.h> +#include <sys/ioctl.h> +#include <sys/syscall.h> #if defined(__linux__) # include <linux/capability.h> +# include <linux/perf_event.h>
What if there's no perf_event.h header file?
#elif defined(__FreeBSD__) # include <sys/param.h> # include <sys/cpuset.h> @@ -4274,6 +4277,75 @@ qemuLogOperation(virDomainObjPtr vm, goto cleanup; }
+/* + * Enable CMT(Cache Monitoring Technology) to measure the usage of + * cache by VM running on the node. + * + * Because the hypervisor implement CMT support basedon perf +mechanism, + * we should enable perf event for CMT. The function 'sys_erf_event_open' + * is perf syscall wrapper. + */ +#ifdef __linux__ +static long sys_perf_event_open(struct perf_event_attr *hw_event, + pid_t pid, int cpu, int group_fd,
One parameter per line is easier to read.
+ unsigned long flags) { + return syscall(__NR_perf_event_open, hw_event, pid, cpu, + group_fd, flags);
Wrong indentation.
+} +static int qemuCmtEnable(virDomainObjPtr vm) { + qemuDomainObjPrivatePtr priv = vm->privateData; + struct perf_event_attr cmt_attr; + int event_type; + FILE *fp; + + fp = fopen("/sys/devices/intel_cqm/type", "r"); + if (!fp) { + virReportSystemError(errno, "%s", + _("CMT is not available on this host")); + return -1; + } + if (fscanf(fp, "%d", &event_type) != 1) { + virReportSystemError(errno, "%s", + _("Unable to read event type file.")); + VIR_FORCE_FCLOSE(fp); + return -1; + } + VIR_FORCE_FCLOSE(fp); + + memset(&cmt_attr, 0, sizeof(struct perf_event_attr)); + cmt_attr.size = sizeof(struct perf_event_attr); + cmt_attr.type = event_type; + cmt_attr.config = 1; + cmt_attr.inherit = 1; + cmt_attr.disabled = 1; + cmt_attr.enable_on_exec = 0; + + priv->cmt_fd = sys_perf_event_open(&cmt_attr, vm->pid, -1, -1, + 0);
...while here you assign long (from sys_perf_event_open) to cmt_fd.
+ if (priv->cmt_fd < 0) { + virReportSystemError(errno, + _("Unable to open perf type=%d for pid=%d"), + event_type, vm->pid); + return -1; + } + + if (ioctl(priv->cmt_fd, PERF_EVENT_IOC_ENABLE) < 0) { + virReportSystemError(errno, "%s", + _("Unable to enable perf event for CMT")); + return -1; + } + + return 0; +} +#else +static int qemuCmtEnable(virDomainObjPtr vm) { + virReportUnsupportedError(); + return -1; +} +#endif + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4954,6 +5026,11 @@ int qemuProcessStart(virConnectPtr conn, if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto cleanup;
+ VIR_DEBUG("Setting CMT perf counter"); + if (qemuCmtEnable(vm) < 0) + virReportSystemError(errno, "%s", + _("CMT is not available on this host")); +
By setting en error here, you overwrite any useful error set inside qemuCmtEnable. Anyway since you apparently want to make the error non fatal, you should not even set it. The messages should be logged as warnings or even lower.
If there is no performance or other impact of having cmt enabled, we could enable it on hosts that support it, but we either need to log any errors with info level or first check whether the host supports cmt and only try to enable it if it does, in which case we could report the errors as warnings.
However, if a negative impact of enabling cmt is possible, we need an explicit configuration option (turned off by default) that would ask libvirt to enable cmt. Any errors could be logged as warnings in this case.
And what if libvirtd is restarted? Shouldn't we call sys_perf_event_open again?
Jirka, thanks for your nice feedback, and I will fix these problems in next patch series. In addition, performance events won't slow down kernel and applications and so we could enable it on hosts that support it. Certainly this patch only check if kernel support cmt feature and does not check if host cpu support it through /proc/cpuinfo ... If libvirtd is restarted, perf handler need to be got again. I will think it over next version.
/* finally we can call the 'started' hook script if any */ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { char *xml = qemuDomainDefFormatXML(driver, vm->def, 0); @@ -5122,6 +5199,15 @@ void qemuProcessStop(virQEMUDriverPtr driver, virPortAllocatorRelease(driver->migrationPorts, priv->nbdPort); priv->nbdPort = 0;
+ /* Disable CMT */ + if (priv->cmt_fd > 0) {
cmt_fd >= 0 ?
+ if (ioctl(priv->cmt_fd, PERF_EVENT_IOC_DISABLE) < 0) { + virReportSystemError(errno, "%s", + _("Unable to disable perf event for + CMT"));
A warning should be good enough.
+ } + VIR_FORCE_CLOSE(priv->cmt_fd); + } +
This should go into a dedicated function.
if (priv->agent) { qemuAgentClose(priv->agent); priv->agent = NULL;
Jirka
On Thu, Oct 29, 2015 at 02:02:29PM +0800, Qiaowei Ren wrote:
One RFC in https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html
CMT (Cache Monitoring Technology) can be used to measure the usage of cache by VM running on the host. This patch will extend the bulk stats API (virDomainListGetStats) to add this field. Applications based on libvirt can use this API to achieve cache usage of VM. Because CMT implementation in Linux kernel is based on perf mechanism, this patch will enable perf event for CMT when VM is created and disable it when VM is destroyed.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
Thanks for re-sending this patchset, it has reminded me of the concerns / questions I had around this previously. Just ignoring the code for a minute, IIUC the design is - Open a file handle to the kernel perf system for each running VM - Associate that perf event file handle with the QEMU VM PID - Enable recording of the CMT perf event on that file handle - Report the CMT event values in the virDomainGetStats() API call when VIR_DOMAIN_STATS_CACHE is requested My two primary concerns are 1. Do we want to have a perf event FD open for every running VM all the time. 2. Is the virDomainGetStats() integration the right API approach For item 1, my concern is that the CMT event is only ever going to be consumed by OpenStack, and even then, only OpenStack installs which have the schedular plugin that cares about the CMT event data. It feels undesirable to have this perf system enabled for all libvirt VMs, when perhaps < 1 % of libvirt users actually want this data. It feels like we need some mechanism to decide when this event is enabled For item 2, my concern is first when virDomainGetStats is the right API. I think it probably *is* the right API, since I can't think of a better way. Should we however, be having a very special case VIR_DOMAIN_STATS_CACHE group, or should we have something more generic. For example, if I run 'perf event' I see List of pre-defined events (to be used in -e): branch-instructions OR branches [Hardware event] branch-misses [Hardware event] bus-cycles [Hardware event] cache-misses [Hardware event] cache-references [Hardware event] cpu-cycles OR cycles [Hardware event] instructions [Hardware event] ref-cycles [Hardware event] stalled-cycles-frontend OR idle-cycles-frontend [Hardware event] alignment-faults [Software event] context-switches OR cs [Software event] cpu-clock [Software event] cpu-migrations OR migrations [Software event] dummy [Software event] emulation-faults [Software event] major-faults [Software event] minor-faults [Software event] ...any many many more... Does it make sense to extend the virDomainStats API to *only* deal with reporting of 1 specific perf event that you care about right now. It feels like it might be better if we did something more general purpose. eg what if something wants to get 'major-faults' data in future ? So we add a VIR_DOMAIN_STATS_MAJOR_FAULT enum item, etc. Combining these two concerns, I think we might need 2 things - A new API to turn on/off collection of specific perf events This could be something like virDomainGetPerfEvents(virDOmainPtr dom, virTypedParameter params); This would fill virTypedParameters with one entry for each perf event, using the VIR_TYPED_PARAM_BOOLEAN type. A 'true' value would indicate that event is enabled for the VM. A corresponding virDomainSetPerfEvents(virDOmainPtr dom, virTypedParameter params); would enable you to toggle the flag, to enable/disable the particular list of perf events you care about. With that, we could have a 'VIR_DOMAIN_STATS_PERF_EVENT' enum item for virDomainStats which causes reporting of all previously enabled perf events This would avoid us needing to have the perf event enabled for all VMs all the time. Only applications using libvirt which actually need the data would turn it on. It would also be now scalable to all types of perf event, instead of just one specific event
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 54e1e7b..31bce33 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -196,6 +196,9 @@ struct _qemuDomainObjPrivate {
bool hookRun; /* true if there was a hook run over this domain */
+ int cmt_fd; /* perf handler for CMT */ + + /* Bitmaps below hold data from the auto NUMA feature */ virBitmapPtr autoNodeset; virBitmapPtr autoCpuset; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4cfae03..8c678c9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19320,6 +19320,53 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
#undef QEMU_ADD_COUNT_PARAM
+static int +qemuDomainGetStatsCache(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + qemuDomainObjPrivatePtr priv = dom->privateData; + FILE *fd; + unsigned long long cache = 0; + int scaling_factor = 0; + + if (priv->cmt_fd <= 0) + return -1; + + if (read(priv->cmt_fd, &cache, sizeof(uint64_t)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to read cache data")); + return -1; + } + + fd = fopen("/sys/devices/intel_cqm/events/llc_occupancy.scale", "r"); + if (!fd) { + virReportSystemError(errno, "%s", + _("Unable to open CMT scale file")); + return -1; + } + if (fscanf(fd, "%d", &scaling_factor) != 1) { + virReportSystemError(errno, "%s", + _("Unable to read CMT scale file")); + VIR_FORCE_FCLOSE(fd); + return -1; + } + VIR_FORCE_FCLOSE(fd); + + cache *= scaling_factor; + + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cache.current", + cache) < 0) + return -1; + + return 0; +} + typedef int (*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver, virDomainObjPtr dom, @@ -19340,6 +19387,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, false }, { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE, false }, { qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK, true }, + { qemuDomainGetStatsCache, VIR_DOMAIN_STATS_CACHE, false }, { NULL, 0, false } };
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ba84182..00b889d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -25,8 +25,11 @@ #include <unistd.h> #include <signal.h> #include <sys/stat.h> +#include <sys/ioctl.h> +#include <sys/syscall.h> #if defined(__linux__) # include <linux/capability.h> +# include <linux/perf_event.h> #elif defined(__FreeBSD__) # include <sys/param.h> # include <sys/cpuset.h> @@ -4274,6 +4277,75 @@ qemuLogOperation(virDomainObjPtr vm, goto cleanup; }
+/* + * Enable CMT(Cache Monitoring Technology) to measure the usage of + * cache by VM running on the node. + * + * Because the hypervisor implement CMT support basedon perf mechanism, + * we should enable perf event for CMT. The function 'sys_erf_event_open' + * is perf syscall wrapper. + */ +#ifdef __linux__ +static long sys_perf_event_open(struct perf_event_attr *hw_event, + pid_t pid, int cpu, int group_fd, + unsigned long flags) +{ + return syscall(__NR_perf_event_open, hw_event, pid, cpu, + group_fd, flags); +} +static int qemuCmtEnable(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + struct perf_event_attr cmt_attr; + int event_type; + FILE *fp; + + fp = fopen("/sys/devices/intel_cqm/type", "r"); + if (!fp) { + virReportSystemError(errno, "%s", + _("CMT is not available on this host")); + return -1; + } + if (fscanf(fp, "%d", &event_type) != 1) { + virReportSystemError(errno, "%s", + _("Unable to read event type file.")); + VIR_FORCE_FCLOSE(fp); + return -1; + } + VIR_FORCE_FCLOSE(fp); + + memset(&cmt_attr, 0, sizeof(struct perf_event_attr)); + cmt_attr.size = sizeof(struct perf_event_attr); + cmt_attr.type = event_type; + cmt_attr.config = 1; + cmt_attr.inherit = 1; + cmt_attr.disabled = 1; + cmt_attr.enable_on_exec = 0; + + priv->cmt_fd = sys_perf_event_open(&cmt_attr, vm->pid, -1, -1, 0); + if (priv->cmt_fd < 0) { + virReportSystemError(errno, + _("Unable to open perf type=%d for pid=%d"), + event_type, vm->pid); + return -1; + } + + if (ioctl(priv->cmt_fd, PERF_EVENT_IOC_ENABLE) < 0) { + virReportSystemError(errno, "%s", + _("Unable to enable perf event for CMT")); + return -1; + } + + return 0; +} +#else +static int qemuCmtEnable(virDomainObjPtr vm) +{ + virReportUnsupportedError(); + return -1; +} +#endif + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4954,6 +5026,11 @@ int qemuProcessStart(virConnectPtr conn, if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto cleanup;
+ VIR_DEBUG("Setting CMT perf counter"); + if (qemuCmtEnable(vm) < 0) + virReportSystemError(errno, "%s", + _("CMT is not available on this host")); + /* finally we can call the 'started' hook script if any */ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { char *xml = qemuDomainDefFormatXML(driver, vm->def, 0); @@ -5122,6 +5199,15 @@ void qemuProcessStop(virQEMUDriverPtr driver, virPortAllocatorRelease(driver->migrationPorts, priv->nbdPort); priv->nbdPort = 0;
+ /* Disable CMT */ + if (priv->cmt_fd > 0) { + if (ioctl(priv->cmt_fd, PERF_EVENT_IOC_DISABLE) < 0) { + virReportSystemError(errno, "%s", + _("Unable to disable perf event for CMT")); + } + VIR_FORCE_CLOSE(priv->cmt_fd); + } + if (priv->agent) { qemuAgentClose(priv->agent); priv->agent = NULL;
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
-----Original Message----- From: Daniel P. Berrange [mailto:berrange@redhat.com] Sent: Thursday, October 29, 2015 10:56 PM To: Ren, Qiaowei Cc: libvir-list@redhat.com; Feng, Shaohe Subject: Re: [PATCH 2/3] Qemu: add CMT support
On Thu, Oct 29, 2015 at 02:02:29PM +0800, Qiaowei Ren wrote:
One RFC in https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html
CMT (Cache Monitoring Technology) can be used to measure the usage of cache by VM running on the host. This patch will extend the bulk stats API (virDomainListGetStats) to add this field. Applications based on libvirt can use this API to achieve cache usage of VM. Because CMT implementation in Linux kernel is based on perf mechanism, this patch will enable perf event for CMT when VM is created and disable it when VM is destroyed.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
Thanks for re-sending this patchset, it has reminded me of the concerns / questions I had around this previously.
Just ignoring the code for a minute, IIUC the design is
- Open a file handle to the kernel perf system for each running VM - Associate that perf event file handle with the QEMU VM PID - Enable recording of the CMT perf event on that file handle - Report the CMT event values in the virDomainGetStats() API call when VIR_DOMAIN_STATS_CACHE is requested
My two primary concerns are
1. Do we want to have a perf event FD open for every running VM all the time. 2. Is the virDomainGetStats() integration the right API approach
For item 1, my concern is that the CMT event is only ever going to be consumed by OpenStack, and even then, only OpenStack installs which have the schedular plugin that cares about the CMT event data. It feels undesirable to have this perf system enabled for all libvirt VMs, when perhaps < 1 % of libvirt users actually want this data. It feels like we need some mechanism to decide when this event is enabled
For item 2, my concern is first when virDomainGetStats is the right API. I think it probably *is* the right API, since I can't think of a better way.
Should we however, be having a very special case VIR_DOMAIN_STATS_CACHE group, or should we have something more generic.
For example, if I run 'perf event' I see
List of pre-defined events (to be used in -e):
branch-instructions OR branches [Hardware event] branch-misses [Hardware event] bus-cycles [Hardware event] cache-misses [Hardware event] cache-references [Hardware event] cpu-cycles OR cycles [Hardware event] instructions [Hardware event] ref-cycles [Hardware event] stalled-cycles-frontend OR idle-cycles-frontend [Hardware event]
alignment-faults [Software event] context-switches OR cs [Software event] cpu-clock [Software event] cpu-migrations OR migrations [Software event] dummy [Software event] emulation-faults [Software event] major-faults [Software event] minor-faults [Software event] ...any many many more...
Does it make sense to extend the virDomainStats API to *only* deal with reporting of 1 specific perf event that you care about right now. It feels like it might be better if we did something more general purpose.
eg what if something wants to get 'major-faults' data in future ? So we add a VIR_DOMAIN_STATS_MAJOR_FAULT enum item, etc.
Combining these two concerns, I think we might need 2 things
- A new API to turn on/off collection of specific perf events
This could be something like
virDomainGetPerfEvents(virDOmainPtr dom, virTypedParameter params);
This would fill virTypedParameters with one entry for each perf event, using the VIR_TYPED_PARAM_BOOLEAN type. A 'true' value would indicate that event is enabled for the VM. A corresponding
virDomainSetPerfEvents(virDOmainPtr dom, virTypedParameter params);
would enable you to toggle the flag, to enable/disable the particular list of perf events you care about.
With that, we could have a 'VIR_DOMAIN_STATS_PERF_EVENT' enum item for virDomainStats which causes reporting of all previously enabled perf events
This would avoid us needing to have the perf event enabled for all VMs all the time. Only applications using libvirt which actually need the data would turn it on. It would also be now scalable to all types of perf event, instead of just one specific event
Daniel, thanks for your nice feedback. I will re-implement my patch according to your comments. Thanks, Qiaowei
This patch update domstats command to support CMT feature based on extended bulk stats API virDomainListGetStats. Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- tools/virsh-domain-monitor.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 1d4dc25..28f7bf8 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -2013,6 +2013,10 @@ static const vshCmdOptDef opts_domstats[] = { .type = VSH_OT_BOOL, .help = N_("report domain block device statistics"), }, + {.name = "cache", + .type = VSH_OT_BOOL, + .help = N_("report domain cache statistics"), + }, {.name = "list-active", .type = VSH_OT_BOOL, .help = N_("list only active domains"), @@ -2123,6 +2127,9 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "block")) stats |= VIR_DOMAIN_STATS_BLOCK; + if (vshCommandOptBool(cmd, "cache")) + stats |= VIR_DOMAIN_STATS_CACHE; + if (vshCommandOptBool(cmd, "list-active")) flags |= VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE; -- 1.9.1
On Thu, Oct 29, 2015 at 14:02:30 +0800, Qiaowei Ren wrote:
This patch update domstats command to support CMT feature based on extended bulk stats API virDomainListGetStats.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- tools/virsh-domain-monitor.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 1d4dc25..28f7bf8 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -2013,6 +2013,10 @@ static const vshCmdOptDef opts_domstats[] = { .type = VSH_OT_BOOL, .help = N_("report domain block device statistics"), }, + {.name = "cache", + .type = VSH_OT_BOOL, + .help = N_("report domain cache statistics"), + }, {.name = "list-active", .type = VSH_OT_BOOL, .help = N_("list only active domains"), @@ -2123,6 +2127,9 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "block")) stats |= VIR_DOMAIN_STATS_BLOCK;
+ if (vshCommandOptBool(cmd, "cache")) + stats |= VIR_DOMAIN_STATS_CACHE; + if (vshCommandOptBool(cmd, "list-active")) flags |= VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE;
You need to document the option in virsh manpage. Jirka
On Thu, Oct 29, 2015 at 14:02:28 +0800, Qiaowei Ren wrote:
Some Intel processor families (e.g. the Intel Xeon processor E5 v3 family) introduced CMT (Cache Monitoring Technology) to measure the usage of cache by applications running on the platform. This patch add it into x86 part of cpu_map.xml.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- .gnulib | 2 +- src/cpu/cpu_map.xml | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/.gnulib b/.gnulib index f39477d..106a386 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit f39477dba778e99392948dd3dd19ec0d46aee932 +Subproject commit 106a3866d01f9dd57ab4f10dbeb0d5a8db73a9f7
This hunk should not be here. Gnulib versions are changed separately. Also I doubt that it's necessary. Peter
-----Original Message----- From: Peter Krempa [mailto:pkrempa@redhat.com] Sent: Thursday, October 29, 2015 4:43 PM To: Ren, Qiaowei Cc: libvir-list@redhat.com; Feng, Shaohe Subject: Re: [libvirt] [PATCH 1/3] cpu_map.xml: add cmt feature to x86
On Thu, Oct 29, 2015 at 14:02:28 +0800, Qiaowei Ren wrote:
Some Intel processor families (e.g. the Intel Xeon processor E5 v3 family) introduced CMT (Cache Monitoring Technology) to measure the usage of cache by applications running on the platform. This patch add it into x86 part of cpu_map.xml.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- .gnulib | 2 +- src/cpu/cpu_map.xml | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/.gnulib b/.gnulib index f39477d..106a386 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit f39477dba778e99392948dd3dd19ec0d46aee932 +Subproject commit 106a3866d01f9dd57ab4f10dbeb0d5a8db73a9f7
This hunk should not be here. Gnulib versions are changed separately. Also I doubt that it's necessary.
Yes. This should be not necessary here. Thanks, Qiaowei
On Thu, Oct 29, 2015 at 14:02:28 +0800, Qiaowei Ren wrote:
Some Intel processor families (e.g. the Intel Xeon processor E5 v3 family) introduced CMT (Cache Monitoring Technology) to measure the usage of cache by applications running on the platform. This patch add it into x86 part of cpu_map.xml.
When sending a series of patches, please use --cover-letter to create a 0/n patch where you describe what the series is trying to achieve. As a nice side effect , individual patches will be sent as replies to the cover letter, which is much better than sending 2..n patches as replies to the first one.
Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com> --- .gnulib | 2 +- src/cpu/cpu_map.xml | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/.gnulib b/.gnulib index f39477d..106a386 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit f39477dba778e99392948dd3dd19ec0d46aee932 +Subproject commit 106a3866d01f9dd57ab4f10dbeb0d5a8db73a9f7
As Peter said, this hunk should be removed.
diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index b9e95cf..14ccbd8 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -317,6 +317,9 @@ <feature name='rtm'> <cpuid function='0x00000007' ebx='0x00000800'/> </feature> + <feature name='cmt'> + <cpuid function='0x00000007' ebx='0x00001000'/> + </feature> <feature name='rdseed'> <cpuid function='0x00000007' ebx='0x00040000'/> </feature>
This looks like it makes sense, but it really doesn't. This patches causes libvirt to report cmt feature on host CPUs which support it, but what's the point since we are apparently not interested in exposing the feature to guests? Not to mention that even if the host CPU supports cmt, the host kernel does not have to supported so advertising the CPU feature doesn't really say whether it's usable or not. Jirka
participants (5)
-
Daniel P. Berrange -
Jiri Denemark -
Peter Krempa -
Qiaowei Ren -
Ren, Qiaowei