[PATCH 0/4] enable Memory Bandwidth Monitoring (MBM) for VMs

[PATCH 0/4] enable Memory Bandwidth Monitoring (MBM) for VMs

[Chao Peng]

Intel Memory Bandwidth Monitoring(MBM) is a new hardware feature
which builds on the CMT infrastructure to allow monitoring of system
memory bandwidth. Event codes are provided to monitor both "total"
and "local" bandwidth, meaning bandwidth over QPI and other external
links can be monitored.

For XEN, MBM is used to monitor memory bandwidth for VMs. Due to its
dependency on CMT, the software also makes use of most of CMT codes.
Actually, besides introducing two additional events and some cpuid
feature bits, there are no extra changes compared to cache occupancy
monitoring in CMT. Due to this, CMT should be enabled first to use
this feature.

For interface changes, the patch serial only introduces a new command
"XEN_SYSCTL_PSR_CMT_get_l3_event_mask" which exposes MBM feature
capability to user space and introduces two additional options for
"xl psr-cmt-show":
total_mem_bandwidth:     Show total memory bandwidth
local_mem_bandwidth:     Show local memory bandwidth

The usage flow keeps the same with CMT.

Chao Peng (4):
  x86: expose CMT L3 event mask to user space
  tools: libxc: add routine to get CMT L3 event mask
  tools: libxl: code preparation for MBM
  tools: add total/local memory bandwith monitoring

 docs/man/xl.pod.1             |    2 +
 tools/libxc/include/xenctrl.h |    3 ++
 tools/libxc/xc_psr.c          |   40 +++++++++++++++++
 tools/libxl/libxl.h           |    4 ++
 tools/libxl/libxl_psr.c       |  100 ++++++++++++++++++++++++++++++++++-------
 tools/libxl/libxl_types.idl   |    2 +
 tools/libxl/xl_cmdimpl.c      |   63 +++++++++++++++++---------
 tools/libxl/xl_cmdtable.c     |    4 +-
 xen/arch/x86/sysctl.c         |    5 +++
 xen/include/public/sysctl.h   |    1 +
 10 files changed, 186 insertions(+), 38 deletions(-)

-- 
1.7.9.5

[PATCH 1/4] x86: expose CMT L3 event mask to user space

[Chao Peng]

L3 event mask indicates the event types supported in host, including
cache occupancy event as well as local/total memory bandwidth events
for Memory Bandwidth Monitoring(MBM). Expose it so all these events
can be monitored in user space.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
 xen/arch/x86/sysctl.c       |    5 +++++
 xen/include/public/sysctl.h |    1 +
 2 files changed, 6 insertions(+)

diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 57ad992..0f08038 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -157,6 +157,11 @@ long arch_do_sysctl(
             sysctl->u.psr_cmt_op.u.data = (ret ? 0 : info.size);
             break;
         }
+        case XEN_SYSCTL_PSR_CMT_get_l3_event_mask:
+        {
+            sysctl->u.psr_cmt_op.u.data = psr_cmt->l3.features;
+            break;
+        }
         default:
             sysctl->u.psr_cmt_op.u.data = 0;
             ret = -ENOSYS;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index b3713b3..8552dc6 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -641,6 +641,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_coverage_op_t);
 /* The L3 cache size is returned in KB unit */
 #define XEN_SYSCTL_PSR_CMT_get_l3_cache_size         2
 #define XEN_SYSCTL_PSR_CMT_enabled                   3
+#define XEN_SYSCTL_PSR_CMT_get_l3_event_mask         4
 struct xen_sysctl_psr_cmt_op {
     uint32_t cmd;       /* IN: XEN_SYSCTL_PSR_CMT_* */
     uint32_t flags;     /* padding variable, may be extended for future use */
-- 
1.7.9.5

[PATCH 2/4] tools: libxc: add routine to get CMT L3 event mask

[Chao Peng]

This is the xc side wrapper for XEN_SYSCTL_PSR_CMT_get_l3_event_mask
of XEN_SYSCTL_psr_cmt_op. Additional check for event id against value
got from this routine is also added.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
 tools/libxc/include/xenctrl.h |    1 +
 tools/libxc/xc_psr.c          |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 0ad8b8d..96b357c 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2697,6 +2697,7 @@ int xc_psr_cmt_get_domain_rmid(xc_interface *xch, uint32_t domid,
 int xc_psr_cmt_get_total_rmid(xc_interface *xch, uint32_t *total_rmid);
 int xc_psr_cmt_get_l3_upscaling_factor(xc_interface *xch,
     uint32_t *upscaling_factor);
+int xc_psr_cmt_get_l3_event_mask(xc_interface *xch, uint32_t *event_mask);
 int xc_psr_cmt_get_l3_cache_size(xc_interface *xch, uint32_t cpu,
     uint32_t *l3_cache_size);
 int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid,
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index 872e6dc..94c8698 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -112,6 +112,30 @@ int xc_psr_cmt_get_l3_upscaling_factor(xc_interface *xch,
     return rc;
 }
 
+int xc_psr_cmt_get_l3_event_mask(xc_interface *xch, uint32_t *event_mask)
+{
+    static int val = 0;
+    int rc;
+    DECLARE_SYSCTL;
+
+    if ( val )
+    {
+        *event_mask = val;
+        return 0;
+    }
+
+    sysctl.cmd = XEN_SYSCTL_psr_cmt_op;
+    sysctl.u.psr_cmt_op.cmd =
+        XEN_SYSCTL_PSR_CMT_get_l3_event_mask;
+    sysctl.u.psr_cmt_op.flags = 0;
+
+    rc = xc_sysctl(xch, &sysctl);
+    if ( !rc )
+        val = *event_mask = sysctl.u.psr_cmt_op.u.data;
+
+    return rc;
+}
+
 int xc_psr_cmt_get_l3_cache_size(xc_interface *xch, uint32_t cpu,
                                       uint32_t *l3_cache_size)
 {
@@ -144,6 +168,7 @@ int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid,
     xc_resource_op_t op;
     xc_resource_entry_t entries[2];
     uint32_t evtid;
+    uint32_t event_mask;
     int rc;
 
     switch ( type )
@@ -155,6 +180,13 @@ int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid,
         return -1;
     }
 
+    rc = xc_psr_cmt_get_l3_event_mask(xch, &event_mask);
+    if ( rc < 0 )
+        return rc;
+
+    if ( !(event_mask & (1 << (evtid - 1))) )
+        return -1;
+
     entries[0].u.cmd = XEN_RESOURCE_OP_MSR_WRITE;
     entries[0].idx = MSR_IA32_CMT_EVTSEL;
     entries[0].val = (uint64_t)rmid << 32 | evtid;
-- 
1.7.9.5

[PATCH 3/4] tools: libxl: code preparation for MBM

[Chao Peng]

Make some internal routines common so that total/local memory bandwidth
monitoring in the next patch can make use of them.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
 tools/libxl/libxl_psr.c  |   42 ++++++++++++++++++++++----------------
 tools/libxl/xl_cmdimpl.c |   51 ++++++++++++++++++++++++++++------------------
 2 files changed, 56 insertions(+), 37 deletions(-)

diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 0437465..21ad819 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -160,40 +160,48 @@ out:
     return rc;
 }
 
-int libxl_psr_cmt_get_cache_occupancy(libxl_ctx *ctx, uint32_t domid,
-    uint32_t socketid, uint32_t *l3_cache_occupancy)
+static int libxl__psr_cmt_get_l3_monitoring_data(libxl__gc *gc, uint32_t domid,
+    xc_psr_cmt_type type, uint32_t socketid, uint64_t *data)
 {
-    GC_INIT(ctx);
 
     unsigned int rmid;
-    uint32_t upscaling_factor;
-    uint64_t monitor_data;
     int cpu, rc;
-    xc_psr_cmt_type type;
 
-    rc = xc_psr_cmt_get_domain_rmid(ctx->xch, domid, &rmid);
+    rc = xc_psr_cmt_get_domain_rmid(CTX->xch, domid, &rmid);
     if (rc < 0 || rmid == 0) {
         LOGE(ERROR, "fail to get the domain rmid, "
             "or domain is not attached with platform QoS monitoring service");
-        rc = ERROR_FAIL;
-        goto out;
+        return ERROR_FAIL;
     }
 
     cpu = libxl__pick_socket_cpu(gc, socketid);
     if (cpu < 0) {
         LOGE(ERROR, "failed to get socket cpu");
-        rc = ERROR_FAIL;
-        goto out;
+        return ERROR_FAIL;
     }
 
-    type = XC_PSR_CMT_L3_OCCUPANCY;
-    rc = xc_psr_cmt_get_data(ctx->xch, rmid, cpu, type, &monitor_data);
+    rc = xc_psr_cmt_get_data(CTX->xch, rmid, cpu, type, data);
     if (rc < 0) {
         LOGE(ERROR, "failed to get monitoring data");
-        rc = ERROR_FAIL;
-        goto out;
+        return ERROR_FAIL;
     }
 
+    return rc;
+}
+
+int libxl_psr_cmt_get_cache_occupancy(libxl_ctx *ctx, uint32_t domid,
+    uint32_t socketid, uint32_t *l3_cache_occupancy)
+{
+    GC_INIT(ctx);
+    uint64_t data;
+    uint32_t upscaling_factor;
+    int rc;
+
+    rc= libxl__psr_cmt_get_l3_monitoring_data(gc, domid,
+                    XC_PSR_CMT_L3_OCCUPANCY, socketid, &data);
+    if (rc < 0)
+            goto out;
+
     rc = xc_psr_cmt_get_l3_upscaling_factor(ctx->xch, &upscaling_factor);
     if (rc < 0) {
         LOGE(ERROR, "failed to get L3 upscaling factor");
@@ -201,8 +209,8 @@ int libxl_psr_cmt_get_cache_occupancy(libxl_ctx *ctx, uint32_t domid,
         goto out;
     }
 
-    *l3_cache_occupancy = upscaling_factor * monitor_data / 1024;
-    rc = 0;
+    *l3_cache_occupancy = upscaling_factor * data / 1024;
+
 out:
     GC_FREE;
     return rc;
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 3737c7e..f4534ec 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7845,12 +7845,13 @@ out:
 }
 
 #ifdef LIBXL_HAVE_PSR_CMT
-static void psr_cmt_print_domain_cache_occupancy(libxl_dominfo *dominfo,
+static void psr_cmt_print_domain_l3_info(libxl_dominfo *dominfo,
+                                                    libxl_psr_cmt_type type,
                                                     uint32_t nr_sockets)
 {
     char *domain_name;
     uint32_t socketid;
-    uint32_t l3_cache_occupancy;
+    uint32_t data;
 
     if (!libxl_psr_cmt_domain_attached(ctx, dominfo->domid))
         return;
@@ -7860,15 +7861,21 @@ static void psr_cmt_print_domain_cache_occupancy(libxl_dominfo *dominfo,
     free(domain_name);
 
     for (socketid = 0; socketid < nr_sockets; socketid++) {
-        if ( !libxl_psr_cmt_get_cache_occupancy(ctx, dominfo->domid,
-                 socketid, &l3_cache_occupancy) )
-            printf("%13u KB", l3_cache_occupancy);
+        switch (type) {
+        case LIBXL_PSR_CMT_TYPE_CACHE_OCCUPANCY:
+            if ( !libxl_psr_cmt_get_cache_occupancy(ctx, dominfo->domid,
+                 socketid, &data) )
+                printf("%13u KB", data);
+            break;
+        default:
+            return;
+        }
     }
 
     printf("\n");
 }
 
-static int psr_cmt_show_cache_occupancy(uint32_t domid)
+static int psr_cmt_show_l3_info(libxl_psr_cmt_type type, uint32_t domid)
 {
     uint32_t i, socketid, nr_sockets, total_rmid;
     uint32_t l3_cache_size;
@@ -7904,18 +7911,22 @@ static int psr_cmt_show_cache_occupancy(uint32_t domid)
         printf("%14s %d", "Socket", socketid);
     printf("\n");
 
-    /* Total L3 cache size */
-    printf("%-46s", "Total L3 Cache Size");
-    for (socketid = 0; socketid < nr_sockets; socketid++) {
-        rc = libxl_psr_cmt_get_l3_cache_size(ctx, socketid, &l3_cache_size);
-        if (rc < 0) {
-            fprintf(stderr, "Failed to get system l3 cache size for socket:%d\n",
-                            socketid);
-            return -1;
-        }
-        printf("%13u KB", l3_cache_size);
+    if ( type == LIBXL_PSR_CMT_TYPE_CACHE_OCCUPANCY ) {
+            /* Total L3 cache size */
+            printf("%-46s", "Total L3 Cache Size");
+            for (socketid = 0; socketid < nr_sockets; socketid++) {
+                rc = libxl_psr_cmt_get_l3_cache_size(ctx, socketid,
+                                &l3_cache_size);
+                if (rc < 0) {
+                    fprintf(stderr,
+                        "Failed to get system l3 cache size for socket:%d\n",
+                         socketid);
+                    return -1;
+                }
+                printf("%13u KB", l3_cache_size);
+            }
+            printf("\n");
     }
-    printf("\n");
 
     /* Each domain */
     if (domid != INVALID_DOMID) {
@@ -7924,7 +7935,7 @@ static int psr_cmt_show_cache_occupancy(uint32_t domid)
             fprintf(stderr, "Failed to get domain info for %d\n", domid);
             return -1;
         }
-        psr_cmt_print_domain_cache_occupancy(&dominfo, nr_sockets);
+        psr_cmt_print_domain_l3_info(&dominfo, type, nr_sockets);
     }
     else
     {
@@ -7934,7 +7945,7 @@ static int psr_cmt_show_cache_occupancy(uint32_t domid)
             return -1;
         }
         for (i = 0; i < nr_domains; i++)
-            psr_cmt_print_domain_cache_occupancy(list + i, nr_sockets);
+            psr_cmt_print_domain_l3_info(list + i, type, nr_sockets);
         libxl_dominfo_list_free(list, nr_domains);
     }
     return 0;
@@ -7993,7 +8004,7 @@ int main_psr_cmt_show(int argc, char **argv)
 
     switch (type) {
     case LIBXL_PSR_CMT_TYPE_CACHE_OCCUPANCY:
-        ret = psr_cmt_show_cache_occupancy(domid);
+        ret = psr_cmt_show_l3_info(type, domid);
         break;
     default:
         help("psr-cmt-show");
-- 
1.7.9.5

[PATCH 4/4] tools: add total/local memory bandwith monitoring

[Chao Peng]

Add Memory Bandwidth Monitoring(MBM) for VMs. Two types of monitoring
are supported: total and local memory bandwidth monitoring. To use it,
CMT should be enabled in hypervisor.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
 docs/man/xl.pod.1             |    2 ++
 tools/libxc/include/xenctrl.h |    2 ++
 tools/libxc/xc_psr.c          |    8 ++++++
 tools/libxl/libxl.h           |    4 +++
 tools/libxl/libxl_psr.c       |   58 +++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_types.idl   |    2 ++
 tools/libxl/xl_cmdimpl.c      |   12 +++++++++
 tools/libxl/xl_cmdtable.c     |    4 ++-
 8 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 6b89ba8..28d5006 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1476,6 +1476,8 @@ detach: Detach the platform shared resource monitoring service from a domain.
 Show monitoring data for a certain domain or all domains. Current supported
 monitor types are:
  - "cache-occupancy": showing the L3 cache occupancy.
+ - "total-mem-bandwidth": showing the total memory bandwidth.
+ - "local-mem-bandwidth": showing the local memory bandwidth.
 
 =back
 
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 96b357c..2b07e4e 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2688,6 +2688,8 @@ int xc_resource_op(xc_interface *xch, uint32_t nr_ops, xc_resource_op_t *ops);
 #if defined(__i386__) || defined(__x86_64__)
 enum xc_psr_cmt_type {
     XC_PSR_CMT_L3_OCCUPANCY,
+    XC_PSR_CMT_TOTAL_MEM_BANDWIDTH,
+    XC_PSR_CMT_LOCAL_MEM_BANDWIDTH,
 };
 typedef enum xc_psr_cmt_type xc_psr_cmt_type;
 int xc_psr_cmt_attach(xc_interface *xch, uint32_t domid);
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index 94c8698..e286184 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -23,6 +23,8 @@
 #define IA32_CMT_CTR_ERROR_MASK         (0x3ull << 62)
 
 #define EVTID_L3_OCCUPANCY             0x1
+#define EVTID_TOTAL_MEM_BANDWIDTH      0x2
+#define EVTID_LOCAL_MEM_BANDWIDTH      0x3
 
 int xc_psr_cmt_attach(xc_interface *xch, uint32_t domid)
 {
@@ -176,6 +178,12 @@ int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid,
     case XC_PSR_CMT_L3_OCCUPANCY:
         evtid = EVTID_L3_OCCUPANCY;
         break;
+    case XC_PSR_CMT_TOTAL_MEM_BANDWIDTH:
+        evtid = EVTID_TOTAL_MEM_BANDWIDTH;
+        break;
+    case XC_PSR_CMT_LOCAL_MEM_BANDWIDTH:
+        evtid = EVTID_LOCAL_MEM_BANDWIDTH;
+        break;
     default:
         return -1;
     }
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 0a123f1..bc6cc92 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1458,6 +1458,10 @@ int libxl_psr_cmt_get_l3_cache_size(libxl_ctx *ctx, uint32_t socketid,
     uint32_t *l3_cache_size);
 int libxl_psr_cmt_get_cache_occupancy(libxl_ctx *ctx, uint32_t domid,
     uint32_t socketid, uint32_t *l3_cache_occupancy);
+int libxl_psr_cmt_get_total_mem_bandwidth(libxl_ctx *ctx, uint32_t domid,
+    uint32_t socketid, uint32_t *bandwidth);
+int libxl_psr_cmt_get_local_mem_bandwidth(libxl_ctx *ctx, uint32_t domid,
+    uint32_t socketid, uint32_t *bandwidth);
 #endif
 
 /* misc */
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 21ad819..0f5e38f 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -216,6 +216,64 @@ out:
     return rc;
 }
 
+static int libxl__psr_cmt_get_mem_bandwidth(libxl__gc *gc, uint32_t domid,
+    xc_psr_cmt_type type, uint32_t socketid, uint32_t *bandwidth)
+{
+    uint64_t sample1, sample2;
+    uint32_t upscaling_factor;
+    int rc;
+
+    rc = libxl__psr_cmt_get_l3_monitoring_data(gc, domid,
+                    type, socketid, &sample1);
+    if (rc < 0)
+        return ERROR_FAIL;
+
+    usleep(10000);
+
+    rc = libxl__psr_cmt_get_l3_monitoring_data(gc, domid,
+                    type, socketid, &sample2);
+    if (rc < 0)
+       return ERROR_FAIL;
+
+    if (sample2 < sample1) {
+         LOGE(ERROR, "event counter overflowed between two samplings");
+         return ERROR_FAIL;
+    }
+
+    rc = xc_psr_cmt_get_l3_upscaling_factor(CTX->xch, &upscaling_factor);
+    if (rc < 0) {
+        LOGE(ERROR, "failed to get L3 upscaling factor");
+        return ERROR_FAIL;
+    }
+
+    *bandwidth = (sample2 - sample1) * 100 *  upscaling_factor / 1024;
+    return rc;
+}
+
+int libxl_psr_cmt_get_total_mem_bandwidth(libxl_ctx *ctx, uint32_t domid,
+    uint32_t socketid, uint32_t *bandwidth)
+{
+    GC_INIT(ctx);
+    int rc;
+
+    rc = libxl__psr_cmt_get_mem_bandwidth(gc, domid,
+                    XC_PSR_CMT_TOTAL_MEM_BANDWIDTH, socketid, bandwidth);
+    GC_FREE;
+    return rc;
+}
+
+int libxl_psr_cmt_get_local_mem_bandwidth(libxl_ctx *ctx, uint32_t domid,
+    uint32_t socketid, uint32_t *bandwidth)
+{
+    GC_INIT(ctx);
+    int rc;
+
+    rc = libxl__psr_cmt_get_mem_bandwidth(gc, domid,
+                    XC_PSR_CMT_LOCAL_MEM_BANDWIDTH, socketid, bandwidth);
+    GC_FREE;
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index f7fc695..8029a39 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -693,4 +693,6 @@ libxl_event = Struct("event",[
 
 libxl_psr_cmt_type = Enumeration("psr_cmt_type", [
     (1, "CACHE_OCCUPANCY"),
+    (2, "TOTAL_MEM_BANDWIDTH"),
+    (3, "LOCAL_MEM_BANDWIDTH"),
     ])
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index f4534ec..e0435dd 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7867,6 +7867,16 @@ static void psr_cmt_print_domain_l3_info(libxl_dominfo *dominfo,
                  socketid, &data) )
                 printf("%13u KB", data);
             break;
+        case LIBXL_PSR_CMT_TYPE_TOTAL_MEM_BANDWIDTH:
+            if ( !libxl_psr_cmt_get_total_mem_bandwidth(ctx, dominfo->domid,
+                 socketid, &data) )
+                printf("%11u KB/s", data);
+            break;
+        case LIBXL_PSR_CMT_TYPE_LOCAL_MEM_BANDWIDTH:
+            if ( !libxl_psr_cmt_get_local_mem_bandwidth(ctx, dominfo->domid,
+                 socketid, &data) )
+                printf("%11u KB/s", data);
+            break;
         default:
             return;
         }
@@ -8004,6 +8014,8 @@ int main_psr_cmt_show(int argc, char **argv)
 
     switch (type) {
     case LIBXL_PSR_CMT_TYPE_CACHE_OCCUPANCY:
+    case LIBXL_PSR_CMT_TYPE_TOTAL_MEM_BANDWIDTH:
+    case LIBXL_PSR_CMT_TYPE_LOCAL_MEM_BANDWIDTH:
         ret = psr_cmt_show_l3_info(type, domid);
         break;
     default:
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 4b30d3d..2d8f272 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -538,7 +538,9 @@ struct cmd_spec cmd_table[] = {
       "Show Cache Monitoring Technology information",
       "<PSR-CMT-Type> <Domain>",
       "Available monitor types:\n"
-      "\"cache_occupancy\":         Show L3 cache occupancy\n",
+      "\"cache_occupancy\":         Show L3 cache occupancy\n"
+      "\"total_mem_bandwidth\":     Show total memory bandwidth\n"
+      "\"local_mem_bandwidth\":     Show local memory bandwidth\n",
     },
 #endif
 };
-- 
1.7.9.5

Re: [PATCH 2/4] tools: libxc: add routine to get CMT L3 event mask

[Andrew Cooper]

On 23/12/2014 08:54, Chao Peng wrote:
> This is the xc side wrapper for XEN_SYSCTL_PSR_CMT_get_l3_event_mask
> of XEN_SYSCTL_psr_cmt_op. Additional check for event id against value
> got from this routine is also added.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> ---
>   tools/libxc/include/xenctrl.h |    1 +
>   tools/libxc/xc_psr.c          |   32 ++++++++++++++++++++++++++++++++
>   2 files changed, 33 insertions(+)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 0ad8b8d..96b357c 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2697,6 +2697,7 @@ int xc_psr_cmt_get_domain_rmid(xc_interface *xch, uint32_t domid,
>   int xc_psr_cmt_get_total_rmid(xc_interface *xch, uint32_t *total_rmid);
>   int xc_psr_cmt_get_l3_upscaling_factor(xc_interface *xch,
>       uint32_t *upscaling_factor);
> +int xc_psr_cmt_get_l3_event_mask(xc_interface *xch, uint32_t *event_mask);
>   int xc_psr_cmt_get_l3_cache_size(xc_interface *xch, uint32_t cpu,
>       uint32_t *l3_cache_size);
>   int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid,
> diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
> index 872e6dc..94c8698 100644
> --- a/tools/libxc/xc_psr.c
> +++ b/tools/libxc/xc_psr.c
> @@ -112,6 +112,30 @@ int xc_psr_cmt_get_l3_upscaling_factor(xc_interface *xch,
>       return rc;
>   }
>   
> +int xc_psr_cmt_get_l3_event_mask(xc_interface *xch, uint32_t *event_mask)
> +{
> +    static int val = 0;
> +    int rc;
> +    DECLARE_SYSCTL;
> +
> +    if ( val )
> +    {
> +        *event_mask = val;
> +        return 0;
> +    }
> +
> +    sysctl.cmd = XEN_SYSCTL_psr_cmt_op;
> +    sysctl.u.psr_cmt_op.cmd =
> +        XEN_SYSCTL_PSR_CMT_get_l3_event_mask;
> +    sysctl.u.psr_cmt_op.flags = 0;
> +
> +    rc = xc_sysctl(xch, &sysctl);
> +    if ( !rc )
> +        val = *event_mask = sysctl.u.psr_cmt_op.u.data;
> +
> +    return rc;
> +}
> +
>   int xc_psr_cmt_get_l3_cache_size(xc_interface *xch, uint32_t cpu,
>                                         uint32_t *l3_cache_size)
>   {
> @@ -144,6 +168,7 @@ int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid,
>       xc_resource_op_t op;
>       xc_resource_entry_t entries[2];
>       uint32_t evtid;
> +    uint32_t event_mask;
>       int rc;
>   
>       switch ( type )
> @@ -155,6 +180,13 @@ int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid,
>           return -1;
>       }
>   
> +    rc = xc_psr_cmt_get_l3_event_mask(xch, &event_mask);
> +    if ( rc < 0 )
> +        return rc;
> +
> +    if ( !(event_mask & (1 << (evtid - 1))) )
> +        return -1;
> +

This adds an extra hypercall on a common path to return a constant. As 
libxc is mostly a set of basic hypercall wrappers, I don't it should be 
validating its parameters like this.

The caller of xc_psr_cmt_get_data() must be aware of all the details, 
and whether certain events are supported or not.  I woud simply let the 
xc_resourse_op() fail (faulting on the wrmsr) if the caller passes a bad 
event id.

~Andrew

>       entries[0].u.cmd = XEN_RESOURCE_OP_MSR_WRITE;
>       entries[0].idx = MSR_IA32_CMT_EVTSEL;
>       entries[0].val = (uint64_t)rmid << 32 | evtid;

Re: [PATCH 0/4] enable Memory Bandwidth Monitoring (MBM) for VMs

[Andrew Cooper]

On 23/12/2014 08:54, Chao Peng wrote:
> Intel Memory Bandwidth Monitoring(MBM) is a new hardware feature
> which builds on the CMT infrastructure to allow monitoring of system
> memory bandwidth. Event codes are provided to monitor both "total"
> and "local" bandwidth, meaning bandwidth over QPI and other external
> links can be monitored.
>
> For XEN, MBM is used to monitor memory bandwidth for VMs. Due to its
> dependency on CMT, the software also makes use of most of CMT codes.
> Actually, besides introducing two additional events and some cpuid
> feature bits, there are no extra changes compared to cache occupancy
> monitoring in CMT. Due to this, CMT should be enabled first to use
> this feature.
>
> For interface changes, the patch serial only introduces a new command
> "XEN_SYSCTL_PSR_CMT_get_l3_event_mask" which exposes MBM feature
> capability to user space and introduces two additional options for
> "xl psr-cmt-show":
> total_mem_bandwidth:     Show total memory bandwidth
> local_mem_bandwidth:     Show local memory bandwidth
>
> The usage flow keeps the same with CMT.
>
> Chao Peng (4):
>    x86: expose CMT L3 event mask to user space
>    tools: libxc: add routine to get CMT L3 event mask
>    tools: libxl: code preparation for MBM
>    tools: add total/local memory bandwith monitoring

Please can you add a note about MBM in the command line documentation, 
beside the CMT information.

~Andrew

Re: [PATCH 1/4] x86: expose CMT L3 event mask to user space

[Andrew Cooper]

On 23/12/2014 08:54, Chao Peng wrote:
> L3 event mask indicates the event types supported in host, including
> cache occupancy event as well as local/total memory bandwidth events
> for Memory Bandwidth Monitoring(MBM). Expose it so all these events
> can be monitored in user space.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
>   xen/arch/x86/sysctl.c       |    5 +++++
>   xen/include/public/sysctl.h |    1 +
>   2 files changed, 6 insertions(+)
>
> diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> index 57ad992..0f08038 100644
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -157,6 +157,11 @@ long arch_do_sysctl(
>               sysctl->u.psr_cmt_op.u.data = (ret ? 0 : info.size);
>               break;
>           }
> +        case XEN_SYSCTL_PSR_CMT_get_l3_event_mask:
> +        {
> +            sysctl->u.psr_cmt_op.u.data = psr_cmt->l3.features;
> +            break;
> +        }
>           default:
>               sysctl->u.psr_cmt_op.u.data = 0;
>               ret = -ENOSYS;
> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index b3713b3..8552dc6 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -641,6 +641,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_coverage_op_t);
>   /* The L3 cache size is returned in KB unit */
>   #define XEN_SYSCTL_PSR_CMT_get_l3_cache_size         2
>   #define XEN_SYSCTL_PSR_CMT_enabled                   3
> +#define XEN_SYSCTL_PSR_CMT_get_l3_event_mask         4
>   struct xen_sysctl_psr_cmt_op {
>       uint32_t cmd;       /* IN: XEN_SYSCTL_PSR_CMT_* */
>       uint32_t flags;     /* padding variable, may be extended for future use */

Re: [PATCH 2/4] tools: libxc: add routine to get CMT L3 event mask

[Chao Peng]

On Tue, Dec 23, 2014 at 03:46:41PM +0000, Andrew Cooper wrote:
> 
> On 23/12/2014 08:54, Chao Peng wrote:
> >This is the xc side wrapper for XEN_SYSCTL_PSR_CMT_get_l3_event_mask
> >of XEN_SYSCTL_psr_cmt_op. Additional check for event id against value
> >got from this routine is also added.
> >
> >Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> >---
> >  tools/libxc/include/xenctrl.h |    1 +
> >  tools/libxc/xc_psr.c          |   32 ++++++++++++++++++++++++++++++++
> >  2 files changed, 33 insertions(+)
> >
> >diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> >index 0ad8b8d..96b357c 100644
> >--- a/tools/libxc/include/xenctrl.h
> >+++ b/tools/libxc/include/xenctrl.h
> >@@ -2697,6 +2697,7 @@ int xc_psr_cmt_get_domain_rmid(xc_interface *xch, uint32_t domid,
> >  int xc_psr_cmt_get_total_rmid(xc_interface *xch, uint32_t *total_rmid);
> >  int xc_psr_cmt_get_l3_upscaling_factor(xc_interface *xch,
> >      uint32_t *upscaling_factor);
> >+int xc_psr_cmt_get_l3_event_mask(xc_interface *xch, uint32_t *event_mask);
> >  int xc_psr_cmt_get_l3_cache_size(xc_interface *xch, uint32_t cpu,
> >      uint32_t *l3_cache_size);
> >  int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid,
> >diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
> >index 872e6dc..94c8698 100644
> >--- a/tools/libxc/xc_psr.c
> >+++ b/tools/libxc/xc_psr.c
> >@@ -112,6 +112,30 @@ int xc_psr_cmt_get_l3_upscaling_factor(xc_interface *xch,
> >      return rc;
> >  }
> >+int xc_psr_cmt_get_l3_event_mask(xc_interface *xch, uint32_t *event_mask)
> >+{
> >+    static int val = 0;
> >+    int rc;
> >+    DECLARE_SYSCTL;
> >+
> >+    if ( val )
> >+    {
> >+        *event_mask = val;
> >+        return 0;
> >+    }
> >+
> >+    sysctl.cmd = XEN_SYSCTL_psr_cmt_op;
> >+    sysctl.u.psr_cmt_op.cmd =
> >+        XEN_SYSCTL_PSR_CMT_get_l3_event_mask;
> >+    sysctl.u.psr_cmt_op.flags = 0;
> >+
> >+    rc = xc_sysctl(xch, &sysctl);
> >+    if ( !rc )
> >+        val = *event_mask = sysctl.u.psr_cmt_op.u.data;
> >+
> >+    return rc;
> >+}
> >+
> >  int xc_psr_cmt_get_l3_cache_size(xc_interface *xch, uint32_t cpu,
> >                                        uint32_t *l3_cache_size)
> >  {
> >@@ -144,6 +168,7 @@ int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid,
> >      xc_resource_op_t op;
> >      xc_resource_entry_t entries[2];
> >      uint32_t evtid;
> >+    uint32_t event_mask;
> >      int rc;
> >      switch ( type )
> >@@ -155,6 +180,13 @@ int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid,
> >          return -1;
> >      }
> >+    rc = xc_psr_cmt_get_l3_event_mask(xch, &event_mask);
> >+    if ( rc < 0 )
> >+        return rc;
> >+
> >+    if ( !(event_mask & (1 << (evtid - 1))) )
> >+        return -1;
> >+
> 
> This adds an extra hypercall on a common path to return a constant. As libxc
> is mostly a set of basic hypercall wrappers, I don't it should be validating
> its parameters like this.
> 
> The caller of xc_psr_cmt_get_data() must be aware of all the details, and
> whether certain events are supported or not.  I woud simply let the
> xc_resourse_op() fail (faulting on the wrmsr) if the caller passes a bad
> event id.
> 
Sounds reasonable. I'd move the validation to the caller, that's, the xl side.
Thanks Andrew.
> 
> >      entries[0].u.cmd = XEN_RESOURCE_OP_MSR_WRITE;
> >      entries[0].idx = MSR_IA32_CMT_EVTSEL;
> >      entries[0].val = (uint64_t)rmid << 32 | evtid;
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 0/4] enable Memory Bandwidth Monitoring (MBM) for VMs

[Chao Peng]

On Tue, Dec 23, 2014 at 03:47:35PM +0000, Andrew Cooper wrote:
> On 23/12/2014 08:54, Chao Peng wrote:
> >Intel Memory Bandwidth Monitoring(MBM) is a new hardware feature
> >which builds on the CMT infrastructure to allow monitoring of system
> >memory bandwidth. Event codes are provided to monitor both "total"
> >and "local" bandwidth, meaning bandwidth over QPI and other external
> >links can be monitored.
> >
> >For XEN, MBM is used to monitor memory bandwidth for VMs. Due to its
> >dependency on CMT, the software also makes use of most of CMT codes.
> >Actually, besides introducing two additional events and some cpuid
> >feature bits, there are no extra changes compared to cache occupancy
> >monitoring in CMT. Due to this, CMT should be enabled first to use
> >this feature.
> >
> >For interface changes, the patch serial only introduces a new command
> >"XEN_SYSCTL_PSR_CMT_get_l3_event_mask" which exposes MBM feature
> >capability to user space and introduces two additional options for
> >"xl psr-cmt-show":
> >total_mem_bandwidth:     Show total memory bandwidth
> >local_mem_bandwidth:     Show local memory bandwidth
> >
> >The usage flow keeps the same with CMT.
> >
> >Chao Peng (4):
> >   x86: expose CMT L3 event mask to user space
> >   tools: libxc: add routine to get CMT L3 event mask
> >   tools: libxl: code preparation for MBM
> >   tools: add total/local memory bandwith monitoring
> 
> Please can you add a note about MBM in the command line documentation,
> beside the CMT information.
Sure.
Chao
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 3/4] tools: libxl: code preparation for MBM

[Wei Liu]

On Tue, Dec 23, 2014 at 04:54:38PM +0800, Chao Peng wrote:
[...]
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 3737c7e..f4534ec 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -7845,12 +7845,13 @@ out:
>  }
>  
>  #ifdef LIBXL_HAVE_PSR_CMT
> -static void psr_cmt_print_domain_cache_occupancy(libxl_dominfo *dominfo,
> +static void psr_cmt_print_domain_l3_info(libxl_dominfo *dominfo,
> +                                                    libxl_psr_cmt_type type,
>                                                      uint32_t nr_sockets)

Indentation.

>  {
>      char *domain_name;
>      uint32_t socketid;
> -    uint32_t l3_cache_occupancy;
> +    uint32_t data;
>  
>      if (!libxl_psr_cmt_domain_attached(ctx, dominfo->domid))
>          return;
> @@ -7860,15 +7861,21 @@ static void psr_cmt_print_domain_cache_occupancy(libxl_dominfo *dominfo,
>      free(domain_name);
>  
>      for (socketid = 0; socketid < nr_sockets; socketid++) {
> -        if ( !libxl_psr_cmt_get_cache_occupancy(ctx, dominfo->domid,
> -                 socketid, &l3_cache_occupancy) )
> -            printf("%13u KB", l3_cache_occupancy);
> +        switch (type) {
> +        case LIBXL_PSR_CMT_TYPE_CACHE_OCCUPANCY:
> +            if ( !libxl_psr_cmt_get_cache_occupancy(ctx, dominfo->domid,
> +                 socketid, &data) )
> +                printf("%13u KB", data);
> +            break;
> +        default:
> +            return;
> +        }
>      }
>  
>      printf("\n");
>  }
>  
> -static int psr_cmt_show_cache_occupancy(uint32_t domid)
> +static int psr_cmt_show_l3_info(libxl_psr_cmt_type type, uint32_t domid)
>  {
>      uint32_t i, socketid, nr_sockets, total_rmid;
>      uint32_t l3_cache_size;
> @@ -7904,18 +7911,22 @@ static int psr_cmt_show_cache_occupancy(uint32_t domid)
>          printf("%14s %d", "Socket", socketid);
>      printf("\n");
>  
> -    /* Total L3 cache size */
> -    printf("%-46s", "Total L3 Cache Size");
> -    for (socketid = 0; socketid < nr_sockets; socketid++) {
> -        rc = libxl_psr_cmt_get_l3_cache_size(ctx, socketid, &l3_cache_size);
> -        if (rc < 0) {
> -            fprintf(stderr, "Failed to get system l3 cache size for socket:%d\n",
> -                            socketid);
> -            return -1;
> -        }
> -        printf("%13u KB", l3_cache_size);
> +    if ( type == LIBXL_PSR_CMT_TYPE_CACHE_OCCUPANCY ) {

Coding style, no space after "(" and before ")".

I missed this issue when I reviewed your previous patches.  You can fix
this style problem here while you're at it.

Wei.

Re: [PATCH 4/4] tools: add total/local memory bandwith monitoring

[Wei Liu]

On Tue, Dec 23, 2014 at 04:54:39PM +0800, Chao Peng wrote:
[...]
> +static int libxl__psr_cmt_get_mem_bandwidth(libxl__gc *gc, uint32_t domid,
> +    xc_psr_cmt_type type, uint32_t socketid, uint32_t *bandwidth)
> +{
> +    uint64_t sample1, sample2;
> +    uint32_t upscaling_factor;
> +    int rc;
> +
> +    rc = libxl__psr_cmt_get_l3_monitoring_data(gc, domid,
> +                    type, socketid, &sample1);
> +    if (rc < 0)
> +        return ERROR_FAIL;
> +
> +    usleep(10000);
> +
> +    rc = libxl__psr_cmt_get_l3_monitoring_data(gc, domid,
> +                    type, socketid, &sample2);
> +    if (rc < 0)
> +       return ERROR_FAIL;
> +
> +    if (sample2 < sample1) {
> +         LOGE(ERROR, "event counter overflowed between two samplings");
> +         return ERROR_FAIL;
> +    }
> +

What's the likelihood of counter overflows? Can we handle this more
gracefully? Say, retry (with maximum retry cap) when counter overflows?

> +    rc = xc_psr_cmt_get_l3_upscaling_factor(CTX->xch, &upscaling_factor);
> +    if (rc < 0) {
> +        LOGE(ERROR, "failed to get L3 upscaling factor");
> +        return ERROR_FAIL;
> +    }
> +
> +    *bandwidth = (sample2 - sample1) * 100 *  upscaling_factor / 1024;
> +    return rc;
> +}
> +
> +int libxl_psr_cmt_get_total_mem_bandwidth(libxl_ctx *ctx, uint32_t domid,
> +    uint32_t socketid, uint32_t *bandwidth)
> +{
> +    GC_INIT(ctx);
> +    int rc;
> +
> +    rc = libxl__psr_cmt_get_mem_bandwidth(gc, domid,
> +                    XC_PSR_CMT_TOTAL_MEM_BANDWIDTH, socketid, bandwidth);
> +    GC_FREE;
> +    return rc;
> +}
> +
> +int libxl_psr_cmt_get_local_mem_bandwidth(libxl_ctx *ctx, uint32_t domid,
> +    uint32_t socketid, uint32_t *bandwidth)
> +{
> +    GC_INIT(ctx);
> +    int rc;
> +
> +    rc = libxl__psr_cmt_get_mem_bandwidth(gc, domid,
> +                    XC_PSR_CMT_LOCAL_MEM_BANDWIDTH, socketid, bandwidth);
> +    GC_FREE;
> +    return rc;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index f7fc695..8029a39 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -693,4 +693,6 @@ libxl_event = Struct("event",[
>  
>  libxl_psr_cmt_type = Enumeration("psr_cmt_type", [
>      (1, "CACHE_OCCUPANCY"),
> +    (2, "TOTAL_MEM_BANDWIDTH"),
> +    (3, "LOCAL_MEM_BANDWIDTH"),
>      ])
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index f4534ec..e0435dd 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -7867,6 +7867,16 @@ static void psr_cmt_print_domain_l3_info(libxl_dominfo *dominfo,
>                   socketid, &data) )
>                  printf("%13u KB", data);
>              break;
> +        case LIBXL_PSR_CMT_TYPE_TOTAL_MEM_BANDWIDTH:
> +            if ( !libxl_psr_cmt_get_total_mem_bandwidth(ctx, dominfo->domid,

Coding style.

> +                 socketid, &data) )
> +                printf("%11u KB/s", data);
> +            break;
> +        case LIBXL_PSR_CMT_TYPE_LOCAL_MEM_BANDWIDTH:
> +            if ( !libxl_psr_cmt_get_local_mem_bandwidth(ctx, dominfo->domid,

Ditto.

Wei.

Re: [PATCH 3/4] tools: libxl: code preparation for MBM

[Chao Peng]

On Mon, Jan 05, 2015 at 12:25:05PM +0000, Wei Liu wrote:
> On Tue, Dec 23, 2014 at 04:54:38PM +0800, Chao Peng wrote:
> [...]
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index 3737c7e..f4534ec 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -7845,12 +7845,13 @@ out:
> >  }
> >  
> >  #ifdef LIBXL_HAVE_PSR_CMT
> > -static void psr_cmt_print_domain_cache_occupancy(libxl_dominfo *dominfo,
> > +static void psr_cmt_print_domain_l3_info(libxl_dominfo *dominfo,
> > +                                                    libxl_psr_cmt_type type,
> >                                                      uint32_t nr_sockets)
> 
> Indentation.
> 
> >  {
> >      char *domain_name;
> >      uint32_t socketid;
> > -    uint32_t l3_cache_occupancy;
> > +    uint32_t data;
> >  
> >      if (!libxl_psr_cmt_domain_attached(ctx, dominfo->domid))
> >          return;
> > @@ -7860,15 +7861,21 @@ static void psr_cmt_print_domain_cache_occupancy(libxl_dominfo *dominfo,
> >      free(domain_name);
> >  
> >      for (socketid = 0; socketid < nr_sockets; socketid++) {
> > -        if ( !libxl_psr_cmt_get_cache_occupancy(ctx, dominfo->domid,
> > -                 socketid, &l3_cache_occupancy) )
> > -            printf("%13u KB", l3_cache_occupancy);
> > +        switch (type) {
> > +        case LIBXL_PSR_CMT_TYPE_CACHE_OCCUPANCY:
> > +            if ( !libxl_psr_cmt_get_cache_occupancy(ctx, dominfo->domid,
> > +                 socketid, &data) )
> > +                printf("%13u KB", data);
> > +            break;
> > +        default:
> > +            return;
> > +        }
> >      }
> >  
> >      printf("\n");
> >  }
> >  
> > -static int psr_cmt_show_cache_occupancy(uint32_t domid)
> > +static int psr_cmt_show_l3_info(libxl_psr_cmt_type type, uint32_t domid)
> >  {
> >      uint32_t i, socketid, nr_sockets, total_rmid;
> >      uint32_t l3_cache_size;
> > @@ -7904,18 +7911,22 @@ static int psr_cmt_show_cache_occupancy(uint32_t domid)
> >          printf("%14s %d", "Socket", socketid);
> >      printf("\n");
> >  
> > -    /* Total L3 cache size */
> > -    printf("%-46s", "Total L3 Cache Size");
> > -    for (socketid = 0; socketid < nr_sockets; socketid++) {
> > -        rc = libxl_psr_cmt_get_l3_cache_size(ctx, socketid, &l3_cache_size);
> > -        if (rc < 0) {
> > -            fprintf(stderr, "Failed to get system l3 cache size for socket:%d\n",
> > -                            socketid);
> > -            return -1;
> > -        }
> > -        printf("%13u KB", l3_cache_size);
> > +    if ( type == LIBXL_PSR_CMT_TYPE_CACHE_OCCUPANCY ) {
> 
> Coding style, no space after "(" and before ")".
> 
> I missed this issue when I reviewed your previous patches.  You can fix
> this style problem here while you're at it.
Sure, I will fix them all.
Thanks.

Chao

Re: [PATCH 3/4] tools: libxl: code preparation for MBM

[Wei Liu]

On Tue, Jan 06, 2015 at 05:46:12PM +0800, Chao Peng wrote:
[...]
> > Coding style, no space after "(" and before ")".
> > 
> > I missed this issue when I reviewed your previous patches.  You can fix
> > this style problem here while you're at it.
> Sure, I will fix them all.

If you plan to fix them all and there are many instances, please use
preparatory patch -- don't mix them with functional changes.

Wei.

> Thanks.
> 
> Chao

Re: [PATCH 4/4] tools: add total/local memory bandwith monitoring

[Chao Peng]

On Mon, Jan 05, 2015 at 12:39:42PM +0000, Wei Liu wrote:
> On Tue, Dec 23, 2014 at 04:54:39PM +0800, Chao Peng wrote:
> [...]
> > +static int libxl__psr_cmt_get_mem_bandwidth(libxl__gc *gc, uint32_t domid,
> > +    xc_psr_cmt_type type, uint32_t socketid, uint32_t *bandwidth)
> > +{
> > +    uint64_t sample1, sample2;
> > +    uint32_t upscaling_factor;
> > +    int rc;
> > +
> > +    rc = libxl__psr_cmt_get_l3_monitoring_data(gc, domid,
> > +                    type, socketid, &sample1);
> > +    if (rc < 0)
> > +        return ERROR_FAIL;
> > +
> > +    usleep(10000);
> > +
> > +    rc = libxl__psr_cmt_get_l3_monitoring_data(gc, domid,
> > +                    type, socketid, &sample2);
> > +    if (rc < 0)
> > +       return ERROR_FAIL;
> > +
> > +    if (sample2 < sample1) {
> > +         LOGE(ERROR, "event counter overflowed between two samplings");
> > +         return ERROR_FAIL;
> > +    }
> > +
> 
> What's the likelihood of counter overflows? Can we handle this more
> gracefully? Say, retry (with maximum retry cap) when counter overflows?
The likelihood is very small here. Hardware guarantees the counter will
not overflow in one second even under maximum platform bandwidth conditions.
And we only sleep 0.01 second here. 

I'd like to adopt your suggestion to retry another time once that happens.
But only one retry and it should correct the overflow.

Thanks,
Chao
> 
> > +    rc = xc_psr_cmt_get_l3_upscaling_factor(CTX->xch, &upscaling_factor);
> > +    if (rc < 0) {
> > +        LOGE(ERROR, "failed to get L3 upscaling factor");
> > +        return ERROR_FAIL;
> > +    }
> > +
> > +    *bandwidth = (sample2 - sample1) * 100 *  upscaling_factor / 1024;
> > +    return rc;
> > +}
> > +
> > +int libxl_psr_cmt_get_total_mem_bandwidth(libxl_ctx *ctx, uint32_t domid,
> > +    uint32_t socketid, uint32_t *bandwidth)
> > +{
> > +    GC_INIT(ctx);
> > +    int rc;
> > +
> > +    rc = libxl__psr_cmt_get_mem_bandwidth(gc, domid,
> > +                    XC_PSR_CMT_TOTAL_MEM_BANDWIDTH, socketid, bandwidth);
> > +    GC_FREE;
> > +    return rc;
> > +}
> > +
> > +int libxl_psr_cmt_get_local_mem_bandwidth(libxl_ctx *ctx, uint32_t domid,
> > +    uint32_t socketid, uint32_t *bandwidth)
> > +{
> > +    GC_INIT(ctx);
> > +    int rc;
> > +
> > +    rc = libxl__psr_cmt_get_mem_bandwidth(gc, domid,
> > +                    XC_PSR_CMT_LOCAL_MEM_BANDWIDTH, socketid, bandwidth);
> > +    GC_FREE;
> > +    return rc;
> > +}
> > +
> >  /*
> >   * Local variables:
> >   * mode: C
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index f7fc695..8029a39 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -693,4 +693,6 @@ libxl_event = Struct("event",[
> >  
> >  libxl_psr_cmt_type = Enumeration("psr_cmt_type", [
> >      (1, "CACHE_OCCUPANCY"),
> > +    (2, "TOTAL_MEM_BANDWIDTH"),
> > +    (3, "LOCAL_MEM_BANDWIDTH"),
> >      ])
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index f4534ec..e0435dd 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -7867,6 +7867,16 @@ static void psr_cmt_print_domain_l3_info(libxl_dominfo *dominfo,
> >                   socketid, &data) )
> >                  printf("%13u KB", data);
> >              break;
> > +        case LIBXL_PSR_CMT_TYPE_TOTAL_MEM_BANDWIDTH:
> > +            if ( !libxl_psr_cmt_get_total_mem_bandwidth(ctx, dominfo->domid,
> 
> Coding style.
> 
> > +                 socketid, &data) )
> > +                printf("%11u KB/s", data);
> > +            break;
> > +        case LIBXL_PSR_CMT_TYPE_LOCAL_MEM_BANDWIDTH:
> > +            if ( !libxl_psr_cmt_get_local_mem_bandwidth(ctx, dominfo->domid,
> 
> Ditto.
> 
> Wei.

Re: [PATCH 3/4] tools: libxl: code preparation for MBM

[Chao Peng]

On Tue, Jan 06, 2015 at 09:51:37AM +0000, Wei Liu wrote:
> On Tue, Jan 06, 2015 at 05:46:12PM +0800, Chao Peng wrote:
> [...]
> > > Coding style, no space after "(" and before ")".
> > > 
> > > I missed this issue when I reviewed your previous patches.  You can fix
> > > this style problem here while you're at it.
> > Sure, I will fix them all.
> 
> If you plan to fix them all and there are many instances, please use
> preparatory patch -- don't mix them with functional changes.

I noticed this. So I will certainly follow your suggestion :)
Chao

Re: [PATCH 4/4] tools: add total/local memory bandwith monitoring

[Andrew Cooper]

On 06/01/15 10:09, Chao Peng wrote:
> On Mon, Jan 05, 2015 at 12:39:42PM +0000, Wei Liu wrote:
>> On Tue, Dec 23, 2014 at 04:54:39PM +0800, Chao Peng wrote:
>> [...]
>>> +static int libxl__psr_cmt_get_mem_bandwidth(libxl__gc *gc, uint32_t domid,
>>> +    xc_psr_cmt_type type, uint32_t socketid, uint32_t *bandwidth)
>>> +{
>>> +    uint64_t sample1, sample2;
>>> +    uint32_t upscaling_factor;
>>> +    int rc;
>>> +
>>> +    rc = libxl__psr_cmt_get_l3_monitoring_data(gc, domid,
>>> +                    type, socketid, &sample1);
>>> +    if (rc < 0)
>>> +        return ERROR_FAIL;
>>> +
>>> +    usleep(10000);
>>> +
>>> +    rc = libxl__psr_cmt_get_l3_monitoring_data(gc, domid,
>>> +                    type, socketid, &sample2);
>>> +    if (rc < 0)
>>> +       return ERROR_FAIL;
>>> +
>>> +    if (sample2 < sample1) {
>>> +         LOGE(ERROR, "event counter overflowed between two samplings");
>>> +         return ERROR_FAIL;
>>> +    }
>>> +
>> What's the likelihood of counter overflows? Can we handle this more
>> gracefully? Say, retry (with maximum retry cap) when counter overflows?
> The likelihood is very small here. Hardware guarantees the counter will
> not overflow in one second even under maximum platform bandwidth conditions.
> And we only sleep 0.01 second here. 
>
> I'd like to adopt your suggestion to retry another time once that happens.
> But only one retry and it should correct the overflow.
>
> Thanks,
> Chao

You have no possible way of guaranteeing that the actual elapsed time
between the two samples is less than 1 second.  On a very heavily loaded
system, even regular task scheduling could cause an actual elapsed time
of more than one second in that snippet of code.

~Andrew

Re: [PATCH 4/4] tools: add total/local memory bandwith monitoring

[Chao Peng]

On Tue, Jan 06, 2015 at 10:29:18AM +0000, Andrew Cooper wrote:
> On 06/01/15 10:09, Chao Peng wrote:
> > On Mon, Jan 05, 2015 at 12:39:42PM +0000, Wei Liu wrote:
> >> On Tue, Dec 23, 2014 at 04:54:39PM +0800, Chao Peng wrote:
> >> [...]
> >>> +static int libxl__psr_cmt_get_mem_bandwidth(libxl__gc *gc, uint32_t domid,
> >>> +    xc_psr_cmt_type type, uint32_t socketid, uint32_t *bandwidth)
> >>> +{
> >>> +    uint64_t sample1, sample2;
> >>> +    uint32_t upscaling_factor;
> >>> +    int rc;
> >>> +
> >>> +    rc = libxl__psr_cmt_get_l3_monitoring_data(gc, domid,
> >>> +                    type, socketid, &sample1);
> >>> +    if (rc < 0)
> >>> +        return ERROR_FAIL;
> >>> +
> >>> +    usleep(10000);
> >>> +
> >>> +    rc = libxl__psr_cmt_get_l3_monitoring_data(gc, domid,
> >>> +                    type, socketid, &sample2);
> >>> +    if (rc < 0)
> >>> +       return ERROR_FAIL;
> >>> +
> >>> +    if (sample2 < sample1) {
> >>> +         LOGE(ERROR, "event counter overflowed between two samplings");
> >>> +         return ERROR_FAIL;
> >>> +    }
> >>> +
> >> What's the likelihood of counter overflows? Can we handle this more
> >> gracefully? Say, retry (with maximum retry cap) when counter overflows?
> > The likelihood is very small here. Hardware guarantees the counter will
> > not overflow in one second even under maximum platform bandwidth conditions.
> > And we only sleep 0.01 second here. 
> >
> > I'd like to adopt your suggestion to retry another time once that happens.
> > But only one retry and it should correct the overflow.
> >
> > Thanks,
> > Chao
> 
> You have no possible way of guaranteeing that the actual elapsed time
> between the two samples is less than 1 second.  On a very heavily loaded
> system, even regular task scheduling could cause an actual elapsed time
> of more than one second in that snippet of code.
Yes, it's true. So the retry cap Wei suggested will be applied.
Thanks.
Chao
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 1/4] x86: expose CMT L3 event mask to user space

[Jan Beulich]

>>> On 23.12.14 at 09:54, <chao.p.peng@linux.intel.com> wrote:
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -157,6 +157,11 @@ long arch_do_sysctl(
>              sysctl->u.psr_cmt_op.u.data = (ret ? 0 : info.size);
>              break;
>          }
> +        case XEN_SYSCTL_PSR_CMT_get_l3_event_mask:
> +        {
> +            sysctl->u.psr_cmt_op.u.data = psr_cmt->l3.features;
> +            break;
> +        }

Stray figure braces. Other than that
Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH 4/4] tools: add total/local memory bandwith monitoring

[Chao Peng]

On Tue, Jan 06, 2015 at 10:29:18AM +0000, Andrew Cooper wrote:
> On 06/01/15 10:09, Chao Peng wrote:
> > On Mon, Jan 05, 2015 at 12:39:42PM +0000, Wei Liu wrote:
> >> On Tue, Dec 23, 2014 at 04:54:39PM +0800, Chao Peng wrote:
> >> [...]
> >>> +static int libxl__psr_cmt_get_mem_bandwidth(libxl__gc *gc, uint32_t domid,
> >>> +    xc_psr_cmt_type type, uint32_t socketid, uint32_t *bandwidth)
> >>> +{
> >>> +    uint64_t sample1, sample2;
> >>> +    uint32_t upscaling_factor;
> >>> +    int rc;
> >>> +
> >>> +    rc = libxl__psr_cmt_get_l3_monitoring_data(gc, domid,
> >>> +                    type, socketid, &sample1);
> >>> +    if (rc < 0)
> >>> +        return ERROR_FAIL;
> >>> +
> >>> +    usleep(10000);
> >>> +
> >>> +    rc = libxl__psr_cmt_get_l3_monitoring_data(gc, domid,
> >>> +                    type, socketid, &sample2);
> >>> +    if (rc < 0)
> >>> +       return ERROR_FAIL;
> >>> +
> >>> +    if (sample2 < sample1) {
> >>> +         LOGE(ERROR, "event counter overflowed between two samplings");
> >>> +         return ERROR_FAIL;
> >>> +    }
> >>> +
> >> What's the likelihood of counter overflows? Can we handle this more
> >> gracefully? Say, retry (with maximum retry cap) when counter overflows?
> > The likelihood is very small here. Hardware guarantees the counter will
> > not overflow in one second even under maximum platform bandwidth conditions.
> > And we only sleep 0.01 second here. 
> >
> > I'd like to adopt your suggestion to retry another time once that happens.
> > But only one retry and it should correct the overflow.
> >
> > Thanks,
> > Chao
> 
> You have no possible way of guaranteeing that the actual elapsed time
> between the two samples is less than 1 second.  On a very heavily loaded
> system, even regular task scheduling could cause an actual elapsed time
> of more than one second in that snippet of code.
> 
On further thought, this could not be right if implemented this only in
tool stack, due to the fact that the duration between two samples can’t
be guaranteed. Even got sample2 > sample1 here, the data may still wrong
as the hardware counter may overflowed more than one times during this
period.

What the hardware guaranteed here is that at most 1 overflow can happen
(which can be corrected by software) when the duration between two samples
is less than 1 second. So only the data that got from two samples which
duration is actually less than 1 second is valid.

The duration must be checked to use the data, this means something must
be done in hypervisor.

My initial solution is: Add a new hypercall to get both the counter
value and the timestamp at that moment(The two operations should be
atomic).
(Looks like not good to add this to existed resource_op hypercall)


Any suggestions?

Thanks,
Chao
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel