All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs
@ 2015-01-21 11:19 Chao Peng
  2015-01-21 11:19 ` [PATCH v5 1/6] x86: allow IRQ to be disabled for resource access Chao Peng
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: Chao Peng @ 2015-01-21 11:19 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	will.auld, JBeulich, keir

Changes from v4:
* Make the counter read and timestamp read atomic by disable IRQ;
* Treat MSR_IA32_TSC as a special case and return NOW() for read path;
* Add MBM description in xl command line.

Changes from v3:
* Get timestamp information from host along with the monitoring counter;
  This is required for counter overlow detection.
* Address comments from Wei on the last patch.

Changes from v2:
* Remove the usage of "static" to cache data in xc;
  NOTE: Other places that already existed before are not touched due to
        the needs for API change. Will fix in separate patch if desirable.
* Coding style;

Changes from v1:
* Move event type check from xc to xl;
* Add retry capability for MBM sampling;
* Fix Coding style/docs;

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 introduces a new command
"XEN_SYSCTL_PSR_CMT_get_l3_event_mask" which exposes MBM feature
capability to user space and modified "resource_op" to support reading
host timestamp.
Tool stack side, two additional options introduced 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 (5):
  x86: allow reading MSR_IA32_TSC with XENPF_resource_op
  tools: add routine to get CMT L3 event mask
  tools: correct coding style for psr
  tools: code refactoring for MBM
  tools, docs: add total/local memory bandwith monitoring

 docs/man/xl.pod.1                   |   9 +++
 docs/misc/xen-command-line.markdown |   3 +
 tools/libxc/include/xenctrl.h       |  14 ++--
 tools/libxc/xc_msr_x86.h            |   1 +
 tools/libxc/xc_psr.c                |  66 +++++++++++++---
 tools/libxl/libxl.h                 |  20 ++++-
 tools/libxl/libxl_psr.c             | 153 +++++++++++++++++++++++++++++++++---
 tools/libxl/libxl_types.idl         |   2 +
 tools/libxl/xl_cmdimpl.c            |  72 ++++++++++++-----
 tools/libxl/xl_cmdtable.c           |   4 +-
 xen/arch/x86/platform_hypercall.c   |  37 ++++++++-
 xen/include/public/platform.h       |   3 +-
 12 files changed, 324 insertions(+), 60 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v5 1/6] x86: allow IRQ to be disabled for resource access
  2015-01-21 11:19 [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs Chao Peng
@ 2015-01-21 11:19 ` Chao Peng
  2015-01-22 11:18   ` Jan Beulich
  2015-01-21 11:19 ` [PATCH v5 2/6] x86: allow reading MSR_IA32_TSC with XENPF_resource_op Chao Peng
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Chao Peng @ 2015-01-21 11:19 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	will.auld, JBeulich, keir

Add the ability to disable IRQ when operating on certain continuous
resource entries. If one entry is marked as
XEN_RESOURCE_ENTRY_FLAGS_DISABLE_IRQ, then the resource operation on
both the entry and the following entry will be done with IRQ disabled.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
 tools/libxc/xc_psr.c              |  4 ++--
 xen/arch/x86/platform_hypercall.c | 22 +++++++++++++++++++++-
 xen/include/public/platform.h     |  3 ++-
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index 872e6dc..87db3a5 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -158,12 +158,12 @@ int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid,
     entries[0].u.cmd = XEN_RESOURCE_OP_MSR_WRITE;
     entries[0].idx = MSR_IA32_CMT_EVTSEL;
     entries[0].val = (uint64_t)rmid << 32 | evtid;
-    entries[0].rsvd = 0;
+    entries[0].flags = 0;
 
     entries[1].u.cmd = XEN_RESOURCE_OP_MSR_READ;
     entries[1].idx = MSR_IA32_CMT_CTR;
     entries[1].val = 0;
-    entries[1].rsvd = 0;
+    entries[1].flags = 0;
 
     op.cpu = cpu;
     op.nr_entries = 2;
diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
index 32f39b2..df99bad 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -90,7 +90,9 @@ static void check_resource_access(struct xen_resource_access *ra)
         int ret = 0;
         xenpf_resource_entry_t *entry = ra->entries + i;
 
-        if ( entry->rsvd )
+        /* DISABLE_IRQ flag should never be set for the last entry */
+        if ( entry->flags & XEN_RESOURCE_ENTRY_FLAGS_DISABLE_IRQ &&
+             i == ra->nr_entries - 1 )
         {
             entry->u.ret = -EINVAL;
             break;
@@ -124,11 +126,19 @@ static void resource_access(void *info)
 {
     struct xen_resource_access *ra = info;
     unsigned int i;
+    unsigned long irqflags = 0;
+    bool_t irq_disabled = 0;
 
     for ( i = 0; i < ra->nr_done; i++ )
     {
         int ret;
         xenpf_resource_entry_t *entry = ra->entries + i;
+        if ( !irq_disabled &&
+             entry->flags & XEN_RESOURCE_ENTRY_FLAGS_DISABLE_IRQ )
+        {
+            irq_disabled = 1;
+            local_irq_save(irqflags);
+        }
 
         switch ( entry->u.cmd )
         {
@@ -143,6 +153,13 @@ static void resource_access(void *info)
             break;
         }
 
+        if ( irq_disabled &&
+             !(entry->flags & XEN_RESOURCE_ENTRY_FLAGS_DISABLE_IRQ) )
+        {
+            local_irq_restore(irqflags);
+            irq_disabled = 0;
+        }
+
         if ( ret )
         {
             entry->u.ret = ret;
@@ -150,6 +167,9 @@ static void resource_access(void *info)
         }
     }
 
+    if ( irq_disabled )
+        local_irq_restore(irqflags);
+
     ra->nr_done = i;
 }
 
diff --git a/xen/include/public/platform.h b/xen/include/public/platform.h
index 5c57615..ef30ac2 100644
--- a/xen/include/public/platform.h
+++ b/xen/include/public/platform.h
@@ -545,7 +545,8 @@ struct xenpf_resource_entry {
         uint32_t cmd;   /* IN: XEN_RESOURCE_OP_* */
         int32_t  ret;   /* OUT: return value for failed entry */
     } u;
-    uint32_t rsvd;      /* IN: padding and must be zero */
+#define XEN_RESOURCE_ENTRY_FLAGS_DISABLE_IRQ  1
+    uint32_t flags;     /* IN: XEN_RESOURCE_ENTRY_FLAGS_* */
     uint64_t idx;       /* IN: resource address to access */
     uint64_t val;       /* IN/OUT: resource value to set/get */
 };
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v5 2/6] x86: allow reading MSR_IA32_TSC with XENPF_resource_op
  2015-01-21 11:19 [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs Chao Peng
  2015-01-21 11:19 ` [PATCH v5 1/6] x86: allow IRQ to be disabled for resource access Chao Peng
@ 2015-01-21 11:19 ` Chao Peng
  2015-01-22 11:20   ` Jan Beulich
  2015-01-21 11:19 ` [PATCH v5 3/6] tools: add routine to get CMT L3 event mask Chao Peng
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Chao Peng @ 2015-01-21 11:19 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	will.auld, JBeulich, keir

Memory bandwidth monitoring requires system time information returned
along with the monitoring counter to verify the correctness of the
counter value and to calculate the time elapsed between two samplings.
Add MSR_IA32_TSC to the read path and increase
RESOURCE_ACCESS_MAX_ENTRIES to 3. MSR_IA32_TSC access returns NOW()
instead of timestamp which can be used as the scaled time(ns) directly.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
 xen/arch/x86/platform_hypercall.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
index df99bad..6b23701 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -61,7 +61,7 @@ long cpu_down_helper(void *data);
 long core_parking_helper(void *data);
 uint32_t get_cur_idle_nums(void);
 
-#define RESOURCE_ACCESS_MAX_ENTRIES 2
+#define RESOURCE_ACCESS_MAX_ENTRIES 3
 struct xen_resource_access {
     unsigned int nr_done;
     unsigned int nr_entries;
@@ -75,6 +75,7 @@ static bool_t allow_access_msr(unsigned int msr)
     /* MSR for CMT, refer to chapter 17.14 of Intel SDM. */
     case MSR_IA32_CMT_EVTSEL:
     case MSR_IA32_CMT_CTR:
+    case MSR_IA32_TSC:
         return 1;
     }
 
@@ -143,10 +144,18 @@ static void resource_access(void *info)
         switch ( entry->u.cmd )
         {
         case XEN_RESOURCE_OP_MSR_READ:
-            ret = rdmsr_safe(entry->idx, entry->val);
+            if ( entry->idx == MSR_IA32_TSC ) {
+                entry->val = NOW();
+                ret = 0;
+            }
+            else
+                ret = rdmsr_safe(entry->idx, entry->val);
             break;
         case XEN_RESOURCE_OP_MSR_WRITE:
-            ret = wrmsr_safe(entry->idx, entry->val);
+            if ( entry->idx == MSR_IA32_TSC )
+                ret = -EPERM;
+            else
+                ret = wrmsr_safe(entry->idx, entry->val);
             break;
         default:
             BUG();
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v5 3/6] tools: add routine to get CMT L3 event mask
  2015-01-21 11:19 [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs Chao Peng
  2015-01-21 11:19 ` [PATCH v5 1/6] x86: allow IRQ to be disabled for resource access Chao Peng
  2015-01-21 11:19 ` [PATCH v5 2/6] x86: allow reading MSR_IA32_TSC with XENPF_resource_op Chao Peng
@ 2015-01-21 11:19 ` Chao Peng
  2015-01-21 11:19 ` [PATCH v5 4/6] tools: correct coding style for psr Chao Peng
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Chao Peng @ 2015-01-21 11:19 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	will.auld, JBeulich, keir

This is the tools side wrapper for XEN_SYSCTL_PSR_CMT_get_l3_event_mask
of XEN_SYSCTL_psr_cmt_op.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
 tools/libxc/include/xenctrl.h |  1 +
 tools/libxc/xc_psr.c          | 17 +++++++++++++++++
 tools/libxl/libxl.h           |  1 +
 tools/libxl/libxl_psr.c       | 15 +++++++++++++++
 4 files changed, 34 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 87db3a5..2e9cbc2 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -112,6 +112,23 @@ 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)
+{
+    int rc;
+    DECLARE_SYSCTL;
+
+    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 )
+        *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)
 {
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 0a123f1..c9a64f9 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1456,6 +1456,7 @@ int libxl_psr_cmt_enabled(libxl_ctx *ctx);
 int libxl_psr_cmt_get_total_rmid(libxl_ctx *ctx, uint32_t *total_rmid);
 int libxl_psr_cmt_get_l3_cache_size(libxl_ctx *ctx, uint32_t socketid,
     uint32_t *l3_cache_size);
+int libxl_psr_cmt_get_l3_event_mask(libxl_ctx *ctx, uint32_t *event_mask);
 int libxl_psr_cmt_get_cache_occupancy(libxl_ctx *ctx, uint32_t domid,
     uint32_t socketid, uint32_t *l3_cache_occupancy);
 #endif
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 0437465..07f2aee 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -160,6 +160,21 @@ out:
     return rc;
 }
 
+int libxl_psr_cmt_get_l3_event_mask(libxl_ctx *ctx, uint32_t *event_mask)
+{
+    GC_INIT(ctx);
+    int rc;
+
+    rc = xc_psr_cmt_get_l3_event_mask(ctx->xch, event_mask);
+    if (rc < 0) {
+        libxl__psr_cmt_log_err_msg(gc, errno);
+        rc = ERROR_FAIL;
+    }
+
+    GC_FREE;
+    return rc;
+}
+
 int libxl_psr_cmt_get_cache_occupancy(libxl_ctx *ctx, uint32_t domid,
     uint32_t socketid, uint32_t *l3_cache_occupancy)
 {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v5 4/6] tools: correct coding style for psr
  2015-01-21 11:19 [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs Chao Peng
                   ` (2 preceding siblings ...)
  2015-01-21 11:19 ` [PATCH v5 3/6] tools: add routine to get CMT L3 event mask Chao Peng
@ 2015-01-21 11:19 ` Chao Peng
  2015-01-21 11:19 ` [PATCH v5 5/6] tools: code refactoring for MBM Chao Peng
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Chao Peng @ 2015-01-21 11:19 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	will.auld, JBeulich, keir

- space: remove space after '(' or before ')' in 'if' condition;
- indention: align function definition/call arguments;

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxc/include/xenctrl.h | 10 +++++-----
 tools/libxc/xc_psr.c          | 10 +++++-----
 tools/libxl/libxl.h           | 11 +++++++----
 tools/libxl/libxl_psr.c       | 11 +++++++----
 tools/libxl/xl_cmdimpl.c      | 11 ++++++-----
 5 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 96b357c..8a0eab6 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2693,15 +2693,15 @@ typedef enum xc_psr_cmt_type xc_psr_cmt_type;
 int xc_psr_cmt_attach(xc_interface *xch, uint32_t domid);
 int xc_psr_cmt_detach(xc_interface *xch, uint32_t domid);
 int xc_psr_cmt_get_domain_rmid(xc_interface *xch, uint32_t domid,
-    uint32_t *rmid);
+                               uint32_t *rmid);
 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);
+                                       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,
-    uint32_t cpu, uint32_t psr_cmt_type, uint64_t *monitor_data);
+                                 uint32_t *l3_cache_size);
+int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid, uint32_t cpu,
+                        uint32_t psr_cmt_type, uint64_t *monitor_data);
 int xc_psr_cmt_enabled(xc_interface *xch);
 #endif
 
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index 2e9cbc2..368aeb0 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -47,7 +47,7 @@ int xc_psr_cmt_detach(xc_interface *xch, uint32_t domid)
 }
 
 int xc_psr_cmt_get_domain_rmid(xc_interface *xch, uint32_t domid,
-                                    uint32_t *rmid)
+                               uint32_t *rmid)
 {
     int rc;
     DECLARE_DOMCTL;
@@ -88,7 +88,7 @@ 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)
+                                       uint32_t *upscaling_factor)
 {
     static int val = 0;
     int rc;
@@ -130,7 +130,7 @@ 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)
+                                 uint32_t *l3_cache_size)
 {
     static int val = 0;
     int rc;
@@ -155,8 +155,8 @@ int xc_psr_cmt_get_l3_cache_size(xc_interface *xch, uint32_t cpu,
     return rc;
 }
 
-int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid,
-    uint32_t cpu, xc_psr_cmt_type type, uint64_t *monitor_data)
+int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid, uint32_t cpu,
+                        xc_psr_cmt_type type, uint64_t *monitor_data)
 {
     xc_resource_op_t op;
     xc_resource_entry_t entries[2];
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index c9a64f9..596d2a0 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1454,11 +1454,14 @@ int libxl_psr_cmt_detach(libxl_ctx *ctx, uint32_t domid);
 int libxl_psr_cmt_domain_attached(libxl_ctx *ctx, uint32_t domid);
 int libxl_psr_cmt_enabled(libxl_ctx *ctx);
 int libxl_psr_cmt_get_total_rmid(libxl_ctx *ctx, uint32_t *total_rmid);
-int libxl_psr_cmt_get_l3_cache_size(libxl_ctx *ctx, uint32_t socketid,
-    uint32_t *l3_cache_size);
+int libxl_psr_cmt_get_l3_cache_size(libxl_ctx *ctx,
+                                    uint32_t socketid,
+                                    uint32_t *l3_cache_size);
 int libxl_psr_cmt_get_l3_event_mask(libxl_ctx *ctx, uint32_t *event_mask);
-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_cache_occupancy(libxl_ctx *ctx,
+                                      uint32_t domid,
+                                      uint32_t socketid,
+                                      uint32_t *l3_cache_occupancy);
 #endif
 
 /* misc */
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 07f2aee..84819e6 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -135,8 +135,9 @@ int libxl_psr_cmt_get_total_rmid(libxl_ctx *ctx, uint32_t *total_rmid)
     return rc;
 }
 
-int libxl_psr_cmt_get_l3_cache_size(libxl_ctx *ctx, uint32_t socketid,
-                                         uint32_t *l3_cache_size)
+int libxl_psr_cmt_get_l3_cache_size(libxl_ctx *ctx,
+                                    uint32_t socketid,
+                                    uint32_t *l3_cache_size)
 {
     GC_INIT(ctx);
 
@@ -175,8 +176,10 @@ int libxl_psr_cmt_get_l3_event_mask(libxl_ctx *ctx, uint32_t *event_mask)
     return rc;
 }
 
-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_cache_occupancy(libxl_ctx *ctx,
+                                      uint32_t domid,
+                                      uint32_t socketid,
+                                      uint32_t *l3_cache_occupancy)
 {
     GC_INIT(ctx);
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 0b02a6c..8fce979 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7808,7 +7808,7 @@ out:
 
 #ifdef LIBXL_HAVE_PSR_CMT
 static void psr_cmt_print_domain_cache_occupancy(libxl_dominfo *dominfo,
-                                                    uint32_t nr_sockets)
+                                                 uint32_t nr_sockets)
 {
     char *domain_name;
     uint32_t socketid;
@@ -7822,8 +7822,8 @@ 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) )
+        if (!libxl_psr_cmt_get_cache_occupancy(ctx, dominfo->domid, socketid,
+                                               &l3_cache_occupancy))
             printf("%13u KB", l3_cache_occupancy);
     }
 
@@ -7871,8 +7871,9 @@ static int psr_cmt_show_cache_occupancy(uint32_t domid)
     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);
+            fprintf(stderr,
+                    "Failed to get system l3 cache size for socket:%d\n",
+                    socketid);
             return -1;
         }
         printf("%13u KB", l3_cache_size);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v5 5/6] tools: code refactoring for MBM
  2015-01-21 11:19 [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs Chao Peng
                   ` (3 preceding siblings ...)
  2015-01-21 11:19 ` [PATCH v5 4/6] tools: correct coding style for psr Chao Peng
@ 2015-01-21 11:19 ` Chao Peng
  2015-01-21 11:19 ` [PATCH v5 6/6] tools, docs: add total/local memory bandwith monitoring Chao Peng
  2015-01-21 11:28 ` [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs Jan Beulich
  6 siblings, 0 replies; 19+ messages in thread
From: Chao Peng @ 2015-01-21 11:19 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	will.auld, JBeulich, keir

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>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_psr.c  | 44 +++++++++++++++++++++++++--------------
 tools/libxl/xl_cmdimpl.c | 54 ++++++++++++++++++++++++++++--------------------
 2 files changed, 61 insertions(+), 37 deletions(-)

diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 84819e6..c88c421 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -176,20 +176,16 @@ int libxl_psr_cmt_get_l3_event_mask(libxl_ctx *ctx, uint32_t *event_mask)
     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");
@@ -204,14 +200,32 @@ int libxl_psr_cmt_get_cache_occupancy(libxl_ctx *ctx,
         goto out;
     }
 
-    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;
     }
 
+out:
+    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");
@@ -219,8 +233,8 @@ int libxl_psr_cmt_get_cache_occupancy(libxl_ctx *ctx,
         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 8fce979..1827c63 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7807,12 +7807,13 @@ out:
 }
 
 #ifdef LIBXL_HAVE_PSR_CMT
-static void psr_cmt_print_domain_cache_occupancy(libxl_dominfo *dominfo,
-                                                 uint32_t nr_sockets)
+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;
@@ -7822,15 +7823,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;
@@ -7866,19 +7873,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) {
@@ -7887,7 +7897,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
     {
@@ -7897,7 +7907,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;
@@ -7956,7 +7966,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.9.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v5 6/6] tools, docs: add total/local memory bandwith monitoring
  2015-01-21 11:19 [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs Chao Peng
                   ` (4 preceding siblings ...)
  2015-01-21 11:19 ` [PATCH v5 5/6] tools: code refactoring for MBM Chao Peng
@ 2015-01-21 11:19 ` Chao Peng
  2015-01-21 11:28 ` [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs Jan Beulich
  6 siblings, 0 replies; 19+ messages in thread
From: Chao Peng @ 2015-01-21 11:19 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	will.auld, JBeulich, keir

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>
---
Changes in v5:
1. Add MBM description in xen command line.
2. Use the tsc from hypervisor directly which is already ns.
3. Call resource_op with DISABLE_IRQ flag.
Changes in v4:
1. Get timestamp from hypervisor and use that for bandwidth calculation.
2. Minor document and coding style fix.
---
 docs/man/xl.pod.1                   |   9 ++++
 docs/misc/xen-command-line.markdown |   3 ++
 tools/libxc/include/xenctrl.h       |   5 +-
 tools/libxc/xc_msr_x86.h            |   1 +
 tools/libxc/xc_psr.c                |  37 +++++++++++--
 tools/libxl/libxl.h                 |   8 +++
 tools/libxl/libxl_psr.c             | 101 ++++++++++++++++++++++++++++++++++--
 tools/libxl/libxl_types.idl         |   2 +
 tools/libxl/xl_cmdimpl.c            |  21 +++++++-
 tools/libxl/xl_cmdtable.c           |   4 +-
 10 files changed, 180 insertions(+), 11 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 6b89ba8..50759c0 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1461,6 +1461,13 @@ is domain level. To monitor a specific domain, just attach the domain id with
 the monitoring service. When the domain doesn't need to be monitored any more,
 detach the domain id from the monitoring service.
 
+Intel Broadwell and later server platforms also offer total/local memory
+bandwidth monitoring. Xen supports per-domain monitoring for these two
+additional monitoring types. Both memory bandwidth monitoring and L3 cache
+occupancy monitoring share the same set of underlying monitoring service. Once
+a domain is attached to the monitoring service, monitoring data can be showed
+for any of these monitoring types.
+
 =over 4
 
 =item B<psr-cmt-attach> [I<domain-id>]
@@ -1476,6 +1483,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/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index a061aa4..0491dbb 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1097,6 +1097,9 @@ The following resources are available:
   L3 cache occupancy.
   * `cmt` instructs Xen to enable/disable Cache Monitoring Technology.
   * `rmid_max` indicates the max value for rmid.
+* Memory Bandwidth Monitoring (Broadwell and later). Information regarding the
+  total/local memory bandwidth. Follow the same options with Cache Monitoring
+  Technology.
 
 ### reboot
 > `= t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]`
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 8a0eab6..8f964ad 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);
@@ -2701,7 +2703,8 @@ 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, uint32_t cpu,
-                        uint32_t psr_cmt_type, uint64_t *monitor_data);
+                        uint32_t psr_cmt_type, uint64_t *monitor_data,
+                        uint64_t *tsc);
 int xc_psr_cmt_enabled(xc_interface *xch);
 #endif
 
diff --git a/tools/libxc/xc_msr_x86.h b/tools/libxc/xc_msr_x86.h
index 7c3e1a3..7f100e7 100644
--- a/tools/libxc/xc_msr_x86.h
+++ b/tools/libxc/xc_msr_x86.h
@@ -20,6 +20,7 @@
 #ifndef XC_MSR_X86_H
 #define XC_MSR_X86_H
 
+#define MSR_IA32_TSC            0x00000010
 #define MSR_IA32_CMT_EVTSEL     0x00000c8d
 #define MSR_IA32_CMT_CTR        0x00000c8e
 
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index 368aeb0..fda6128 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)
 {
@@ -156,11 +158,12 @@ int xc_psr_cmt_get_l3_cache_size(xc_interface *xch, uint32_t cpu,
 }
 
 int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid, uint32_t cpu,
-                        xc_psr_cmt_type type, uint64_t *monitor_data)
+                        xc_psr_cmt_type type, uint64_t *monitor_data,
+                        uint64_t *tsc)
 {
     xc_resource_op_t op;
-    xc_resource_entry_t entries[2];
-    uint32_t evtid;
+    xc_resource_entry_t entries[3];
+    uint32_t evtid, nr_entries;
     int rc;
 
     switch ( type )
@@ -168,6 +171,12 @@ int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid, uint32_t cpu,
     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;
     }
@@ -182,19 +191,37 @@ int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid, uint32_t cpu,
     entries[1].val = 0;
     entries[1].flags = 0;
 
+    if ( type == XC_PSR_CMT_L3_OCCUPANCY )
+        nr_entries = 2;
+    else
+    {
+        entries[1].flags = XEN_RESOURCE_ENTRY_FLAGS_DISABLE_IRQ;
+
+        entries[2].u.cmd = XEN_RESOURCE_OP_MSR_READ;
+        entries[2].idx = MSR_IA32_TSC;
+        entries[2].val = 0;
+        entries[2].flags = 0;
+
+        nr_entries = 3;
+    }
+
     op.cpu = cpu;
-    op.nr_entries = 2;
+    op.nr_entries = nr_entries;
     op.entries = entries;
 
     rc = xc_resource_op(xch, 1, &op);
     if ( rc < 0 )
         return rc;
 
-    if ( op.result !=2 || entries[1].val & IA32_CMT_CTR_ERROR_MASK )
+    if ( op.result != nr_entries || entries[1].val & IA32_CMT_CTR_ERROR_MASK )
         return -1;
 
     *monitor_data = entries[1].val;
 
+    if ( type == XC_PSR_CMT_TOTAL_MEM_BANDWIDTH ||
+         type == XC_PSR_CMT_LOCAL_MEM_BANDWIDTH )
+        *tsc = entries[2].val;
+
     return 0;
 }
 
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 596d2a0..347ef52 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1462,6 +1462,14 @@ 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 c88c421..4c8bcc8 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -18,6 +18,7 @@
 
 
 #define IA32_QM_CTR_ERROR_MASK         (0x3ul << 62)
+#define MBM_SAMPLE_RETRY_MAX 4
 
 static void libxl__psr_cmt_log_err_msg(libxl__gc *gc, int err)
 {
@@ -180,7 +181,8 @@ 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)
+                                                 uint64_t *data,
+                                                 uint64_t *tsc)
 {
     unsigned int rmid;
     int cpu, rc;
@@ -200,7 +202,7 @@ static int libxl__psr_cmt_get_l3_monitoring_data(libxl__gc *gc,
         goto out;
     }
 
-    rc = xc_psr_cmt_get_data(CTX->xch, rmid, cpu, type, data);
+    rc = xc_psr_cmt_get_data(CTX->xch, rmid, cpu, type, data, tsc);
     if (rc < 0) {
         LOGE(ERROR, "failed to get monitoring data");
         rc = ERROR_FAIL;
@@ -222,7 +224,7 @@ int libxl_psr_cmt_get_cache_occupancy(libxl_ctx *ctx,
 
     rc = libxl__psr_cmt_get_l3_monitoring_data(gc, domid,
                                                XC_PSR_CMT_L3_OCCUPANCY,
-                                               socketid, &data);
+                                               socketid, &data, NULL);
     if (rc < 0)
             goto out;
 
@@ -240,6 +242,99 @@ 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;
+    uint64_t tsc1, tsc2;
+    uint32_t upscaling_factor;
+    int retry_attempts = 0;
+    int rc;
+
+    while (1) {
+        rc = libxl__psr_cmt_get_l3_monitoring_data(gc, domid, type, socketid,
+                                                   &sample1, &tsc1);
+        if (rc < 0) {
+            rc = ERROR_FAIL;
+            goto out;
+        }
+
+        usleep(10000);
+
+        rc = libxl__psr_cmt_get_l3_monitoring_data(gc, domid, type, socketid,
+                                                   &sample2, &tsc2);
+        if (rc < 0) {
+           rc = ERROR_FAIL;
+           goto out;
+        }
+        if (tsc2 <= tsc1) {
+            rc = ERROR_FAIL;
+            goto out;
+        }
+        /*
+         * Hardware guarantees at most 1 overflow can happen if the duration
+         * between two samples is less than 1 second. Note that tsc returned
+         * from hypervisor is already-scaled time(ns).
+         */
+        if (tsc2 - tsc1 < 1000000000 && sample2 >= sample1)
+            break;
+
+        if (retry_attempts < MBM_SAMPLE_RETRY_MAX) {
+            retry_attempts++;
+        } else {
+            LOGE(ERROR, "event counter overflowed");
+            rc = ERROR_FAIL;
+            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");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    *bandwidth = (sample2 - sample1) * 1000000000 / (tsc2 - tsc1)
+                 *  upscaling_factor / 1024;
+out:
+    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 1214d2e..46d160e 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -694,4 +694,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 1827c63..6d5c7af 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7829,6 +7829,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;
         }
@@ -7840,7 +7850,7 @@ static void psr_cmt_print_domain_l3_info(libxl_dominfo *dominfo,
 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;
+    uint32_t l3_cache_size, l3_event_mask;
     libxl_physinfo info;
     int rc, nr_domains;
 
@@ -7849,6 +7859,13 @@ static int psr_cmt_show_l3_info(libxl_psr_cmt_type type, uint32_t domid)
         return -1;
     }
 
+    rc = libxl_psr_cmt_get_l3_event_mask(ctx, &l3_event_mask);
+    if (rc < 0 || !(l3_event_mask & (1 << (type - 1)))) {
+        fprintf(stderr, "Monitor type '%s' is not supported in the system\n",
+                libxl_psr_cmt_type_to_string(type));
+        return -1;
+    }
+
     libxl_physinfo_init(&info);
     rc = libxl_get_physinfo(ctx, &info);
     if (rc < 0) {
@@ -7966,6 +7983,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.9.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs
  2015-01-21 11:19 [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs Chao Peng
                   ` (5 preceding siblings ...)
  2015-01-21 11:19 ` [PATCH v5 6/6] tools, docs: add total/local memory bandwith monitoring Chao Peng
@ 2015-01-21 11:28 ` Jan Beulich
  2015-01-22  1:12   ` Chao Peng
  6 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2015-01-21 11:28 UTC (permalink / raw)
  To: Chao Peng
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	xen-devel, will.auld, keir

>>> On 21.01.15 at 12:19, <chao.p.peng@linux.intel.com> wrote:
> Changes from v4:
> * Make the counter read and timestamp read atomic by disable IRQ;
> * Treat MSR_IA32_TSC as a special case and return NOW() for read path;
> * Add MBM description in xl command line.

You should really have answered Andrew's question regarding the
use of NOW() vs RDTSC before posting this new series.

You should also see to correct the Cc list of your postings - neither
should all patches always have the same set of people Cc-ed, nor
should you be missing to Cc maintainers of the code you modify.

Jan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs
  2015-01-21 11:28 ` [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs Jan Beulich
@ 2015-01-22  1:12   ` Chao Peng
  2015-01-22  7:59     ` Jan Beulich
  0 siblings, 1 reply; 19+ messages in thread
From: Chao Peng @ 2015-01-22  1:12 UTC (permalink / raw)
  To: Jan Beulich
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, keir

On Wed, Jan 21, 2015 at 11:28:34AM +0000, Jan Beulich wrote:
> >>> On 21.01.15 at 12:19, <chao.p.peng@linux.intel.com> wrote:
> > Changes from v4:
> > * Make the counter read and timestamp read atomic by disable IRQ;
> > * Treat MSR_IA32_TSC as a special case and return NOW() for read path;
> > * Add MBM description in xl command line.
> 
> You should really have answered Andrew's question regarding the
> use of NOW() vs RDTSC before posting this new series.

As I have adopted it so I agree with Andrew's suggestion. NOW() is much
fullfill my requirement. But perhaps not so semantic consistent with
MSR_IA32_TSC?

> 
> You should also see to correct the Cc list of your postings - neither
> should all patches always have the same set of people Cc-ed, nor
> should you be missing to Cc maintainers of the code you modify.
> 
Yeah, seems Andrew is the one I missed. CCed now.

Chao

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs
  2015-01-22  1:12   ` Chao Peng
@ 2015-01-22  7:59     ` Jan Beulich
  2015-01-22 10:36       ` Chao Peng
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2015-01-22  7:59 UTC (permalink / raw)
  To: Chao Peng
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, keir

>>> On 22.01.15 at 02:12, <chao.p.peng@linux.intel.com> wrote:
> On Wed, Jan 21, 2015 at 11:28:34AM +0000, Jan Beulich wrote:
>> >>> On 21.01.15 at 12:19, <chao.p.peng@linux.intel.com> wrote:
>> > Changes from v4:
>> > * Make the counter read and timestamp read atomic by disable IRQ;
>> > * Treat MSR_IA32_TSC as a special case and return NOW() for read path;
>> > * Add MBM description in xl command line.
>> 
>> You should really have answered Andrew's question regarding the
>> use of NOW() vs RDTSC before posting this new series.
> 
> As I have adopted it so I agree with Andrew's suggestion. NOW() is much
> fullfill my requirement. But perhaps not so semantic consistent with
> MSR_IA32_TSC?

Exactly - I didn't look at the patch itself yet, but at the very least
this would need to be very clearly documented in the public header.

Jan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs
  2015-01-22  7:59     ` Jan Beulich
@ 2015-01-22 10:36       ` Chao Peng
  0 siblings, 0 replies; 19+ messages in thread
From: Chao Peng @ 2015-01-22 10:36 UTC (permalink / raw)
  To: Jan Beulich
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, keir

On Thu, Jan 22, 2015 at 07:59:39AM +0000, Jan Beulich wrote:
> >>> On 22.01.15 at 02:12, <chao.p.peng@linux.intel.com> wrote:
> > On Wed, Jan 21, 2015 at 11:28:34AM +0000, Jan Beulich wrote:
> >> >>> On 21.01.15 at 12:19, <chao.p.peng@linux.intel.com> wrote:
> >> > Changes from v4:
> >> > * Make the counter read and timestamp read atomic by disable IRQ;
> >> > * Treat MSR_IA32_TSC as a special case and return NOW() for read path;
> >> > * Add MBM description in xl command line.
> >> 
> >> You should really have answered Andrew's question regarding the
> >> use of NOW() vs RDTSC before posting this new series.
> > 
> > As I have adopted it so I agree with Andrew's suggestion. NOW() is much
> > fullfill my requirement. But perhaps not so semantic consistent with
> > MSR_IA32_TSC?
> 
> Exactly - I didn't look at the patch itself yet, but at the very least
> this would need to be very clearly documented in the public header.

No problem. Or it's our time to create "XEN_RESOURCE_OP_SYSTIME_READ"
sub-op to omit adding such specific comment in platform.h?

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 1/6] x86: allow IRQ to be disabled for resource access
  2015-01-21 11:19 ` [PATCH v5 1/6] x86: allow IRQ to be disabled for resource access Chao Peng
@ 2015-01-22 11:18   ` Jan Beulich
  0 siblings, 0 replies; 19+ messages in thread
From: Jan Beulich @ 2015-01-22 11:18 UTC (permalink / raw)
  To: Chao Peng
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	xen-devel, will.auld, keir

>>> On 21.01.15 at 12:19, <chao.p.peng@linux.intel.com> wrote:
> --- a/xen/arch/x86/platform_hypercall.c
> +++ b/xen/arch/x86/platform_hypercall.c
> @@ -90,7 +90,9 @@ static void check_resource_access(struct xen_resource_access *ra)
>          int ret = 0;
>          xenpf_resource_entry_t *entry = ra->entries + i;
>  
> -        if ( entry->rsvd )
> +        /* DISABLE_IRQ flag should never be set for the last entry */
> +        if ( entry->flags & XEN_RESOURCE_ENTRY_FLAGS_DISABLE_IRQ &&

This needs checking that all the other 31 bits are clear as well as
parenthesizing.

> @@ -124,11 +126,19 @@ static void resource_access(void *info)
>  {
>      struct xen_resource_access *ra = info;
>      unsigned int i;
> +    unsigned long irqflags = 0;
> +    bool_t irq_disabled = 0;

I don't think you need two variables for this - this is x86-only code
and EFLAGS bit 1 is always set.

Jan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 2/6] x86: allow reading MSR_IA32_TSC with XENPF_resource_op
  2015-01-21 11:19 ` [PATCH v5 2/6] x86: allow reading MSR_IA32_TSC with XENPF_resource_op Chao Peng
@ 2015-01-22 11:20   ` Jan Beulich
  2015-01-22 12:53     ` Chao Peng
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2015-01-22 11:20 UTC (permalink / raw)
  To: Chao Peng
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	xen-devel, will.auld, keir

>>> On 21.01.15 at 12:19, <chao.p.peng@linux.intel.com> wrote:
> --- a/xen/arch/x86/platform_hypercall.c
> +++ b/xen/arch/x86/platform_hypercall.c
> @@ -61,7 +61,7 @@ long cpu_down_helper(void *data);
>  long core_parking_helper(void *data);
>  uint32_t get_cur_idle_nums(void);
>  
> -#define RESOURCE_ACCESS_MAX_ENTRIES 2
> +#define RESOURCE_ACCESS_MAX_ENTRIES 3

See my comment on an earlier version.

> @@ -75,6 +75,7 @@ static bool_t allow_access_msr(unsigned int msr)
>      /* MSR for CMT, refer to chapter 17.14 of Intel SDM. */
>      case MSR_IA32_CMT_EVTSEL:
>      case MSR_IA32_CMT_CTR:
> +    case MSR_IA32_TSC:
>          return 1;
>      }
>  
> @@ -143,10 +144,18 @@ static void resource_access(void *info)
>          switch ( entry->u.cmd )
>          {
>          case XEN_RESOURCE_OP_MSR_READ:
> -            ret = rdmsr_safe(entry->idx, entry->val);
> +            if ( entry->idx == MSR_IA32_TSC ) {

Coding style. unlikely()?

> +                entry->val = NOW();
> +                ret = 0;
> +            }
> +            else
> +                ret = rdmsr_safe(entry->idx, entry->val);
>              break;
>          case XEN_RESOURCE_OP_MSR_WRITE:
> -            ret = wrmsr_safe(entry->idx, entry->val);
> +            if ( entry->idx == MSR_IA32_TSC )

unlikely()?

Jan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 2/6] x86: allow reading MSR_IA32_TSC with XENPF_resource_op
  2015-01-22 11:20   ` Jan Beulich
@ 2015-01-22 12:53     ` Chao Peng
  2015-01-22 13:36       ` Jan Beulich
  0 siblings, 1 reply; 19+ messages in thread
From: Chao Peng @ 2015-01-22 12:53 UTC (permalink / raw)
  To: Jan Beulich
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	xen-devel, will.auld, keir

On Thu, Jan 22, 2015 at 11:20:15AM +0000, Jan Beulich wrote:
> >>> On 21.01.15 at 12:19, <chao.p.peng@linux.intel.com> wrote:
> > --- a/xen/arch/x86/platform_hypercall.c
> > +++ b/xen/arch/x86/platform_hypercall.c
> > @@ -61,7 +61,7 @@ long cpu_down_helper(void *data);
> >  long core_parking_helper(void *data);
> >  uint32_t get_cur_idle_nums(void);
> >  
> > -#define RESOURCE_ACCESS_MAX_ENTRIES 2
> > +#define RESOURCE_ACCESS_MAX_ENTRIES 3
> 
> See my comment on an earlier version.
The new added MSR_IA32_TSC and existed MSR_IA32_CMT_CTR should be
read in an atomic unit. How to achieve this if not increase
MAX_ENTRIES?

Chao
> 
> > @@ -75,6 +75,7 @@ static bool_t allow_access_msr(unsigned int msr)
> >      /* MSR for CMT, refer to chapter 17.14 of Intel SDM. */
> >      case MSR_IA32_CMT_EVTSEL:
> >      case MSR_IA32_CMT_CTR:
> > +    case MSR_IA32_TSC:
> >          return 1;
> >      }
> >  

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 2/6] x86: allow reading MSR_IA32_TSC with XENPF_resource_op
  2015-01-22 12:53     ` Chao Peng
@ 2015-01-22 13:36       ` Jan Beulich
  2015-01-22 14:03         ` Andrew Cooper
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2015-01-22 13:36 UTC (permalink / raw)
  To: Chao Peng
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	xen-devel, will.auld, keir

>>> On 22.01.15 at 13:53, <chao.p.peng@linux.intel.com> wrote:
> On Thu, Jan 22, 2015 at 11:20:15AM +0000, Jan Beulich wrote:
>> >>> On 21.01.15 at 12:19, <chao.p.peng@linux.intel.com> wrote:
>> > --- a/xen/arch/x86/platform_hypercall.c
>> > +++ b/xen/arch/x86/platform_hypercall.c
>> > @@ -61,7 +61,7 @@ long cpu_down_helper(void *data);
>> >  long core_parking_helper(void *data);
>> >  uint32_t get_cur_idle_nums(void);
>> >  
>> > -#define RESOURCE_ACCESS_MAX_ENTRIES 2
>> > +#define RESOURCE_ACCESS_MAX_ENTRIES 3
>> 
>> See my comment on an earlier version.
> The new added MSR_IA32_TSC and existed MSR_IA32_CMT_CTR should be
> read in an atomic unit. How to achieve this if not increase
> MAX_ENTRIES?

These are just two entries nevertheless.

Jan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 2/6] x86: allow reading MSR_IA32_TSC with XENPF_resource_op
  2015-01-22 13:36       ` Jan Beulich
@ 2015-01-22 14:03         ` Andrew Cooper
  2015-01-22 14:09           ` Jan Beulich
  0 siblings, 1 reply; 19+ messages in thread
From: Andrew Cooper @ 2015-01-22 14:03 UTC (permalink / raw)
  To: Jan Beulich, Chao Peng
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	xen-devel, will.auld, keir

On 22/01/15 13:36, Jan Beulich wrote:
>>>> On 22.01.15 at 13:53, <chao.p.peng@linux.intel.com> wrote:
>> On Thu, Jan 22, 2015 at 11:20:15AM +0000, Jan Beulich wrote:
>>>>>> On 21.01.15 at 12:19, <chao.p.peng@linux.intel.com> wrote:
>>>> --- a/xen/arch/x86/platform_hypercall.c
>>>> +++ b/xen/arch/x86/platform_hypercall.c
>>>> @@ -61,7 +61,7 @@ long cpu_down_helper(void *data);
>>>>  long core_parking_helper(void *data);
>>>>  uint32_t get_cur_idle_nums(void);
>>>>  
>>>> -#define RESOURCE_ACCESS_MAX_ENTRIES 2
>>>> +#define RESOURCE_ACCESS_MAX_ENTRIES 3
>>> See my comment on an earlier version.
>> The new added MSR_IA32_TSC and existed MSR_IA32_CMT_CTR should be
>> read in an atomic unit. How to achieve this if not increase
>> MAX_ENTRIES?
> These are just two entries nevertheless.

The reasons for two in the first place is that it is an indirect MSR read.

Upping MAX_ENTRIES to 3 and allowing the operation to get a timestamp as
is the only way to synchronously perform the indirect register read and
timestamp.

Having said this, the only useful timestamp will be at the same point as
performing the MSR read.  Having a 3rd operation tacked on the end to
get a timestamp will be some arbitrary time later, especially if
interrupts are enabled.

~Andrew

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 2/6] x86: allow reading MSR_IA32_TSC with XENPF_resource_op
  2015-01-22 14:03         ` Andrew Cooper
@ 2015-01-22 14:09           ` Jan Beulich
  2015-01-23  8:42             ` Chao Peng
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Beulich @ 2015-01-22 14:09 UTC (permalink / raw)
  To: Andrew Cooper, Chao Peng
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	xen-devel, will.auld, keir

>>> On 22.01.15 at 15:03, <andrew.cooper3@citrix.com> wrote:
> On 22/01/15 13:36, Jan Beulich wrote:
>>>>> On 22.01.15 at 13:53, <chao.p.peng@linux.intel.com> wrote:
>>> On Thu, Jan 22, 2015 at 11:20:15AM +0000, Jan Beulich wrote:
>>>>>>> On 21.01.15 at 12:19, <chao.p.peng@linux.intel.com> wrote:
>>>>> --- a/xen/arch/x86/platform_hypercall.c
>>>>> +++ b/xen/arch/x86/platform_hypercall.c
>>>>> @@ -61,7 +61,7 @@ long cpu_down_helper(void *data);
>>>>>  long core_parking_helper(void *data);
>>>>>  uint32_t get_cur_idle_nums(void);
>>>>>  
>>>>> -#define RESOURCE_ACCESS_MAX_ENTRIES 2
>>>>> +#define RESOURCE_ACCESS_MAX_ENTRIES 3
>>>> See my comment on an earlier version.
>>> The new added MSR_IA32_TSC and existed MSR_IA32_CMT_CTR should be
>>> read in an atomic unit. How to achieve this if not increase
>>> MAX_ENTRIES?
>> These are just two entries nevertheless.
> 
> The reasons for two in the first place is that it is an indirect MSR read.

Oh, right - I somehow thought this was needed for a plain MSR
read only.

> Upping MAX_ENTRIES to 3 and allowing the operation to get a timestamp as
> is the only way to synchronously perform the indirect register read and
> timestamp.

I agree then.

> Having said this, the only useful timestamp will be at the same point as
> performing the MSR read.  Having a 3rd operation tacked on the end to
> get a timestamp will be some arbitrary time later, especially if
> interrupts are enabled.

Perhaps the latching of NOW() could happen with the MSR read, and
the latched value then be stored upon encountering the respective
slot? That would also allow further limiting the interrupts disabled
period.

Jan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 2/6] x86: allow reading MSR_IA32_TSC with XENPF_resource_op
  2015-01-22 14:09           ` Jan Beulich
@ 2015-01-23  8:42             ` Chao Peng
  2015-01-23  8:58               ` Jan Beulich
  0 siblings, 1 reply; 19+ messages in thread
From: Chao Peng @ 2015-01-23  8:42 UTC (permalink / raw)
  To: Jan Beulich
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Andrew Cooper,
	Ian.Jackson, xen-devel, will.auld, keir

On Thu, Jan 22, 2015 at 02:09:37PM +0000, Jan Beulich wrote:
> >>> On 22.01.15 at 15:03, <andrew.cooper3@citrix.com> wrote:
> > On 22/01/15 13:36, Jan Beulich wrote:
> >>>>> On 22.01.15 at 13:53, <chao.p.peng@linux.intel.com> wrote:
> >>> On Thu, Jan 22, 2015 at 11:20:15AM +0000, Jan Beulich wrote:
> >>>>>>> On 21.01.15 at 12:19, <chao.p.peng@linux.intel.com> wrote:
> >>>>> --- a/xen/arch/x86/platform_hypercall.c
> >>>>> +++ b/xen/arch/x86/platform_hypercall.c
> 
> > Having said this, the only useful timestamp will be at the same point as
> > performing the MSR read.  Having a 3rd operation tacked on the end to
> > get a timestamp will be some arbitrary time later, especially if
> > interrupts are enabled.
> 
> Perhaps the latching of NOW() could happen with the MSR read, and
> the latched value then be stored upon encountering the respective
> slot? That would also allow further limiting the interrupts disabled
> period.

Except that MSR_IA32_TSC will looks heterogeneous. But since we already
treat it as a special case, I have no problem here. And if we do so,
even the first patch to add irq_disable ability is not needed. Sounds to
me that the MSR_IA32_TSC read should always imply irq disabled. Codes
would be like this:

        case XEN_RESOURCE_OP_MSR_READ:
            if ( unlikely(entry->idx == MSR_IA32_TSC) )
            { 
                entry->val = get_s_time_fixed(tsc);
                ret = 0;
            }   
            else
            {
                bool_t  read_tsc = 0;
                if ( i < ra->nr_done - 1 ) 
                {   
                    xenpf_resource_entry_t *next = ra->entries + i + 1;
                    if ( unlikely(next->idx == MSR_IA32_TSC) )
                        read_tsc = 1;
                }

                if ( unlikely(read_tsc) )
                    local_irq_save(irqflags);

                ret = rdmsr_safe(entry->idx, entry->val);

                if ( unlikely(read_tsc) )
                {
                    rdtscll(tsc);
                    local_irq_restore(irqflags);
                }
            }
            break;
        case XEN_RESOURCE_OP_MSR_WRITE:

Chao

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 2/6] x86: allow reading MSR_IA32_TSC with XENPF_resource_op
  2015-01-23  8:42             ` Chao Peng
@ 2015-01-23  8:58               ` Jan Beulich
  0 siblings, 0 replies; 19+ messages in thread
From: Jan Beulich @ 2015-01-23  8:58 UTC (permalink / raw)
  To: Chao Peng
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Andrew Cooper,
	Ian.Jackson, xen-devel, will.auld, keir

>>> On 23.01.15 at 09:42, <chao.p.peng@linux.intel.com> wrote:
> Except that MSR_IA32_TSC will looks heterogeneous. But since we already
> treat it as a special case, I have no problem here. And if we do so,
> even the first patch to add irq_disable ability is not needed. Sounds to
> me that the MSR_IA32_TSC read should always imply irq disabled. Codes
> would be like this:

Looks reasonable, minus some formatting issues and ...

>         case XEN_RESOURCE_OP_MSR_READ:
>             if ( unlikely(entry->idx == MSR_IA32_TSC) )
>             { 
>                 entry->val = get_s_time_fixed(tsc);
>                 ret = 0;
>             }   
>             else
>             {
>                 bool_t  read_tsc = 0;
>                 if ( i < ra->nr_done - 1 ) 
>                 {   
>                     xenpf_resource_entry_t *next = ra->entries + i + 1;
>                     if ( unlikely(next->idx == MSR_IA32_TSC) )

... this simply being entry[1].idx.

Jan

>                         read_tsc = 1;
>                 }
> 
>                 if ( unlikely(read_tsc) )
>                     local_irq_save(irqflags);
> 
>                 ret = rdmsr_safe(entry->idx, entry->val);
> 
>                 if ( unlikely(read_tsc) )
>                 {
>                     rdtscll(tsc);
>                     local_irq_restore(irqflags);
>                 }
>             }
>             break;
>         case XEN_RESOURCE_OP_MSR_WRITE:
> 
> Chao

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2015-01-23  8:58 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-21 11:19 [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs Chao Peng
2015-01-21 11:19 ` [PATCH v5 1/6] x86: allow IRQ to be disabled for resource access Chao Peng
2015-01-22 11:18   ` Jan Beulich
2015-01-21 11:19 ` [PATCH v5 2/6] x86: allow reading MSR_IA32_TSC with XENPF_resource_op Chao Peng
2015-01-22 11:20   ` Jan Beulich
2015-01-22 12:53     ` Chao Peng
2015-01-22 13:36       ` Jan Beulich
2015-01-22 14:03         ` Andrew Cooper
2015-01-22 14:09           ` Jan Beulich
2015-01-23  8:42             ` Chao Peng
2015-01-23  8:58               ` Jan Beulich
2015-01-21 11:19 ` [PATCH v5 3/6] tools: add routine to get CMT L3 event mask Chao Peng
2015-01-21 11:19 ` [PATCH v5 4/6] tools: correct coding style for psr Chao Peng
2015-01-21 11:19 ` [PATCH v5 5/6] tools: code refactoring for MBM Chao Peng
2015-01-21 11:19 ` [PATCH v5 6/6] tools, docs: add total/local memory bandwith monitoring Chao Peng
2015-01-21 11:28 ` [PATCH v5 0/6] enable Memory Bandwidth Monitoring (MBM) for VMs Jan Beulich
2015-01-22  1:12   ` Chao Peng
2015-01-22  7:59     ` Jan Beulich
2015-01-22 10:36       ` Chao Peng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.