[PATCH v2 for Xen 4.6 0/6] Several PSR fixes in libxl

[PATCH v2 for Xen 4.6 0/6] Several PSR fixes in libxl

[Chao Peng]

The patch series basically contain several PSR related fixes in libxl.
patch1-3: fix the socket display error in certain hotplug case.
patch4:   fix a minor range check.
patch5:   improve the PSR document.
patch6:   improve xl man page.

Detailed problem and fix please see commit message.

Change history
v2:
* Address comments from Wei/Dario.
* Add patch6.

Chao Peng (6):
  tools/libxl: introduce libxl_get_online_socketmap
  tools/libxl: fix socket display error for CMT
  tools/libxl: return socket id from libxl_psr_cat_get_l3_info
  tools/libxl: fix range check in main_psr_cat_cbm_set
  docs: make xl-psr.markdown more precise
  docs/man: resort sections

 docs/man/xl.pod.1           | 24 ++++++-------
 docs/misc/xl-psr.markdown   |  8 ++---
 tools/libxl/libxl.h         |  7 ++--
 tools/libxl/libxl_psr.c     | 23 +++++++++---
 tools/libxl/libxl_types.idl |  1 +
 tools/libxl/libxl_utils.c   | 22 ++++++++++++
 tools/libxl/libxl_utils.h   |  2 ++
 tools/libxl/xl_cmdimpl.c    | 86 +++++++++++++++++++++++----------------------
 8 files changed, 107 insertions(+), 66 deletions(-)

-- 
1.9.1

[PATCH v2 for Xen 4.6 1/6] tools/libxl: introduce libxl_get_online_socketmap

[Chao Peng]

It sets the bit on the given bitmap if the corresponding socket is
available and clears the bit when the corresponding socket is not
available.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
v2:
* rename libxl_socket_bitmap_fill => libxl_get_online_socketmap.
* fix blanklines.

NOTE:LIBXL_HAVE_SOCKET_BITMAP_ALLOC is changed as we are still in 4.6 release cycle.
---
 tools/libxl/libxl.h       |  7 ++++---
 tools/libxl/libxl_utils.c | 22 ++++++++++++++++++++++
 tools/libxl/libxl_utils.h |  2 ++
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 5f9047c..fa5aedd 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -807,11 +807,12 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
 #define LIBXL_HAVE_PCITOPOLOGY 1
 
 /*
- * LIBXL_HAVE_SOCKET_BITMAP_ALLOC
+ * LIBXL_HAVE_SOCKET_BITMAP
  *
- * If this is defined, then libxl_socket_bitmap_alloc exists.
+ * If this is defined, then libxl_socket_bitmap_alloc and
+ * libxl_get_online_socketmap exist.
  */
-#define LIBXL_HAVE_SOCKET_BITMAP_ALLOC 1
+#define LIBXL_HAVE_SOCKET_BITMAP 1
 
 /*
  * LIBXL_HAVE_SRM_V2
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index bfc9699..408ec85 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -886,6 +886,28 @@ int libxl_socket_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *socketmap,
 
 }
 
+int libxl_get_online_socketmap(libxl_ctx *ctx, libxl_bitmap *socketmap)
+{
+    libxl_cputopology *tinfo = NULL;
+    int nr_cpus = 0, i, rc = 0;
+
+    tinfo = libxl_get_cpu_topology(ctx, &nr_cpus);
+    if (tinfo == NULL) {
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    libxl_bitmap_set_none(socketmap);
+    for (i = 0; i < nr_cpus; i++)
+        if (tinfo[i].socket != XEN_INVALID_SOCKET_ID
+            && !libxl_bitmap_test(socketmap, tinfo[i].socket))
+            libxl_bitmap_set(socketmap, tinfo[i].socket);
+
+ out:
+    libxl_cputopology_list_free(tinfo, nr_cpus);
+    return rc;
+}
+
 int libxl_nodemap_to_cpumap(libxl_ctx *ctx,
                             const libxl_bitmap *nodemap,
                             libxl_bitmap *cpumap)
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 1e5ca8a..339ebdf 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -143,6 +143,8 @@ int libxl_node_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *nodemap,
                             int max_nodes);
 int libxl_socket_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *socketmap,
                               int max_sockets);
+/* Fill socketmap with the CPU topology information on the system. */
+int libxl_get_online_socketmap(libxl_ctx *ctx, libxl_bitmap *socketmap);
 
 /* Populate cpumap with the cpus spanned by the nodes in nodemap */
 int libxl_nodemap_to_cpumap(libxl_ctx *ctx,
-- 
1.9.1

Re: [PATCH v2 for Xen 4.6 1/6] tools/libxl: introduce libxl_get_online_socketmap

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 818 bytes --]

On Tue, 2015-09-29 at 15:49 +0800, Chao Peng wrote:
> It sets the bit on the given bitmap if the corresponding socket is
> available and clears the bit when the corresponding socket is not
> available.
> 
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> ---
> v2:
> * rename libxl_socket_bitmap_fill => libxl_get_online_socketmap.
> * fix blanklines.
> 
> NOTE:LIBXL_HAVE_SOCKET_BITMAP_ALLOC is changed as we are still in 4.6
> release cycle.
>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>

Thanks and Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH v2 for Xen 4.6 1/6] tools/libxl: introduce libxl_get_online_socketmap

[Wei Liu]

On Tue, Sep 29, 2015 at 03:49:50PM +0800, Chao Peng wrote:
> It sets the bit on the given bitmap if the corresponding socket is
> available and clears the bit when the corresponding socket is not
> available.
> 
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

[PATCH v2 for Xen 4.6 2/6] tools/libxl: fix socket display error for CMT

[Chao Peng]

When displaying the CMT information for all the sockets, we assume socket
number is continuous. This is not true in the hotplug case. For instance,
when the 3rd socket is plugged out on a 4-socket system, the available
sockets numbers are 1,2,4 but current we will display the CMT
information for socket 1,2,3.

The fix is getting the socket bitmap for all the sockets on the system
first and then displaying CMT information for_each_set_bit in that bitmap.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
v2:
* add libxl_bitmap_init().
---
 tools/libxl/xl_cmdimpl.c | 43 +++++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 2706759..f01d245 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -8192,7 +8192,7 @@ static int psr_cmt_get_mem_bandwidth(uint32_t domid,
 
 static void psr_cmt_print_domain_info(libxl_dominfo *dominfo,
                                       libxl_psr_cmt_type type,
-                                      uint32_t nr_sockets)
+                                      libxl_bitmap *socketmap)
 {
     char *domain_name;
     uint32_t socketid;
@@ -8205,7 +8205,7 @@ static void psr_cmt_print_domain_info(libxl_dominfo *dominfo,
     printf("%-40s %5d", domain_name, dominfo->domid);
     free(domain_name);
 
-    for (socketid = 0; socketid < nr_sockets; socketid++) {
+    libxl_for_each_set_bit(socketid, *socketmap) {
         switch (type) {
         case LIBXL_PSR_CMT_TYPE_CACHE_OCCUPANCY:
             if (!libxl_psr_cmt_get_sample(ctx, dominfo->domid, type, socketid,
@@ -8228,9 +8228,9 @@ static void psr_cmt_print_domain_info(libxl_dominfo *dominfo,
 
 static int psr_cmt_show(libxl_psr_cmt_type type, uint32_t domid)
 {
-    uint32_t i, socketid, nr_sockets, total_rmid;
+    uint32_t i, socketid, total_rmid;
     uint32_t l3_cache_size;
-    libxl_physinfo info;
+    libxl_bitmap socketmap;
     int rc, nr_domains;
 
     if (!libxl_psr_cmt_enabled(ctx)) {
@@ -8244,41 +8244,39 @@ static int psr_cmt_show(libxl_psr_cmt_type type, uint32_t domid)
         return -1;
     }
 
-    libxl_physinfo_init(&info);
-    rc = libxl_get_physinfo(ctx, &info);
+    libxl_bitmap_init(&socketmap);
+    libxl_socket_bitmap_alloc(ctx, &socketmap, 0);
+    rc = libxl_get_online_socketmap(ctx, &socketmap);
     if (rc < 0) {
-        fprintf(stderr, "Failed getting physinfo, rc: %d\n", rc);
-        libxl_physinfo_dispose(&info);
-        return -1;
+        fprintf(stderr, "Failed getting available sockets, rc: %d\n", rc);
+        goto out;
     }
-    nr_sockets = info.nr_cpus / info.threads_per_core / info.cores_per_socket;
-    libxl_physinfo_dispose(&info);
 
     rc = libxl_psr_cmt_get_total_rmid(ctx, &total_rmid);
     if (rc < 0) {
         fprintf(stderr, "Failed to get max RMID value\n");
-        return -1;
+        goto out;
     }
 
     printf("Total RMID: %d\n", total_rmid);
 
     /* Header */
     printf("%-40s %5s", "Name", "ID");
-    for (socketid = 0; socketid < nr_sockets; socketid++)
+    libxl_for_each_set_bit(socketid, socketmap)
         printf("%14s %d", "Socket", socketid);
     printf("\n");
 
     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++) {
+            libxl_for_each_set_bit(socketid, socketmap) {
                 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;
+                    goto out;
                 }
                 printf("%13u KB", l3_cache_size);
             }
@@ -8292,9 +8290,10 @@ static int psr_cmt_show(libxl_psr_cmt_type type, uint32_t domid)
         libxl_dominfo_init(&dominfo);
         if (libxl_domain_info(ctx, &dominfo, domid)) {
             fprintf(stderr, "Failed to get domain info for %d\n", domid);
-            return -1;
+            rc = -1;
+            goto out;
         }
-        psr_cmt_print_domain_info(&dominfo, type, nr_sockets);
+        psr_cmt_print_domain_info(&dominfo, type, &socketmap);
         libxl_dominfo_dispose(&dominfo);
     }
     else
@@ -8302,13 +8301,17 @@ static int psr_cmt_show(libxl_psr_cmt_type type, uint32_t domid)
         libxl_dominfo *list;
         if (!(list = libxl_list_domain(ctx, &nr_domains))) {
             fprintf(stderr, "Failed to get domain info for domain list.\n");
-            return -1;
+            rc = -1;
+            goto out;
         }
         for (i = 0; i < nr_domains; i++)
-            psr_cmt_print_domain_info(list + i, type, nr_sockets);
+            psr_cmt_print_domain_info(list + i, type, &socketmap);
         libxl_dominfo_list_free(list, nr_domains);
     }
-    return 0;
+
+out:
+    libxl_bitmap_dispose(&socketmap);
+    return rc;
 }
 
 int main_psr_cmt_attach(int argc, char **argv)
-- 
1.9.1

Re: [PATCH v2 for Xen 4.6 2/6] tools/libxl: fix socket display error for CMT

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1046 bytes --]

On Tue, 2015-09-29 at 15:49 +0800, Chao Peng wrote:
> When displaying the CMT information for all the sockets, we assume
> socket
> number is continuous. This is not true in the hotplug case. For
> instance,
> when the 3rd socket is plugged out on a 4-socket system, the
> available
> sockets numbers are 1,2,4 but current we will display the CMT
> information for socket 1,2,3.
> 
> The fix is getting the socket bitmap for all the sockets on the
> system
> first and then displaying CMT information for_each_set_bit in that
> bitmap.
> 
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v2:
> * add libxl_bitmap_init().
>
FWIW, Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

[PATCH v2 for Xen 4.6 3/6] tools/libxl: return socket id from libxl_psr_cat_get_l3_info

[Chao Peng]

The entries returned from libxl_psr_cat_get_l3_info are assumed
to be socket-continuous. But this is not true in the hotplug case.

This patch gets the socket bitmap for all the sockets on the system
first and stores the socket id in the structure libxl_psr_cat_info in
libxl_psr_cat_get_l3_info. The xl or similar consumers then can display
socket information correctly.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
v2:
* add libxl_bitmap_init();
* rename target_id to id.
* fix the iteration code in psr_cat_hwinfo().
---
 tools/libxl/libxl_psr.c     | 23 ++++++++++++++++++-----
 tools/libxl/libxl_types.idl |  1 +
 tools/libxl/xl_cmdimpl.c    | 41 ++++++++++++++++++++---------------------
 3 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 3378239..30740a1 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -339,30 +339,43 @@ int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
 {
     GC_INIT(ctx);
     int rc;
-    int i, nr_sockets;
+    int i = 0, socket, nr_sockets;
+    libxl_bitmap socketmap;
     libxl_psr_cat_info *ptr;
 
+    libxl_bitmap_init(&socketmap);
+
     rc = libxl__count_physical_sockets(gc, &nr_sockets);
     if (rc) {
         LOGE(ERROR, "failed to get system socket count");
         goto out;
     }
 
+    libxl_socket_bitmap_alloc(ctx, &socketmap, nr_sockets);
+    rc = libxl_get_online_socketmap(ctx, &socketmap);
+    if (rc < 0) {
+        LOGE(ERROR, "failed to get available sockets");
+        goto out;
+    }
+
     ptr = libxl__malloc(NOGC, nr_sockets * sizeof(libxl_psr_cat_info));
 
-    for (i = 0; i < nr_sockets; i++) {
-        if (xc_psr_cat_get_l3_info(ctx->xch, i, &ptr[i].cos_max,
-                                                &ptr[i].cbm_len)) {
+    libxl_for_each_set_bit(socket, socketmap) {
+        ptr[i].id = socket;
+        if (xc_psr_cat_get_l3_info(ctx->xch, socket, &ptr[i].cos_max,
+                                                     &ptr[i].cbm_len)) {
             libxl__psr_cat_log_err_msg(gc, errno);
             rc = ERROR_FAIL;
             free(ptr);
             goto out;
         }
+        i++;
     }
 
     *info = ptr;
-    *nr = nr_sockets;
+    *nr = i;
 out:
+    libxl_bitmap_dispose(&socketmap);
     GC_FREE;
     return rc;
 }
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 9f6ec00..dc84864 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -792,6 +792,7 @@ libxl_psr_cbm_type = Enumeration("psr_cbm_type", [
     ])
 
 libxl_psr_cat_info = Struct("psr_cat_info", [
+    ("id", uint32),
     ("cos_max", uint32),
     ("cbm_len", uint32),
     ])
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index f01d245..9947dba 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -8384,35 +8384,35 @@ int main_psr_cmt_show(int argc, char **argv)
 static int psr_cat_hwinfo(void)
 {
     int rc;
-    int socketid, nr_sockets;
+    int i, nr;
     uint32_t l3_cache_size;
     libxl_psr_cat_info *info;
 
     printf("Cache Allocation Technology (CAT):\n");
 
-    rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr_sockets);
+    rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr);
     if (rc) {
         fprintf(stderr, "Failed to get cat info\n");
         return rc;
     }
 
-    for (socketid = 0; socketid < nr_sockets; socketid++) {
-        rc = libxl_psr_cmt_get_l3_cache_size(ctx, socketid, &l3_cache_size);
+    for (i = 0; i < nr; i++) {
+        rc = libxl_psr_cmt_get_l3_cache_size(ctx, info[i].id, &l3_cache_size);
         if (rc) {
             fprintf(stderr, "Failed to get l3 cache size for socket:%d\n",
-                    socketid);
+                    info[i].id);
             goto out;
         }
-        printf("%-16s: %u\n", "Socket ID", socketid);
+        printf("%-16s: %u\n", "Socket ID", info[i].id);
         printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
-        printf("%-16s: %u\n", "Maximum COS", info->cos_max);
-        printf("%-16s: %u\n", "CBM length", info->cbm_len);
+        printf("%-16s: %u\n", "Maximum COS", info[i].cos_max);
+        printf("%-16s: %u\n", "CBM length", info[i].cbm_len);
         printf("%-16s: %#llx\n", "Default CBM",
-               (1ull << info->cbm_len) - 1);
+               (1ull << info[i].cbm_len) - 1);
     }
 
 out:
-    libxl_psr_cat_info_list_free(info, nr_sockets);
+    libxl_psr_cat_info_list_free(info, nr);
     return rc;
 }
 
@@ -8454,47 +8454,46 @@ static int psr_cat_print_domain_cbm(uint32_t domid, uint32_t socketid)
     return 0;
 }
 
-static int psr_cat_print_socket(uint32_t domid, uint32_t socketid,
-                                libxl_psr_cat_info *info)
+static int psr_cat_print_socket(uint32_t domid, libxl_psr_cat_info *info)
 {
     int rc;
     uint32_t l3_cache_size;
 
-    rc = libxl_psr_cmt_get_l3_cache_size(ctx, socketid, &l3_cache_size);
+    rc = libxl_psr_cmt_get_l3_cache_size(ctx, info->id, &l3_cache_size);
     if (rc) {
         fprintf(stderr, "Failed to get l3 cache size for socket:%d\n",
-                socketid);
+                info->id);
         return -1;
     }
 
-    printf("%-16s: %u\n", "Socket ID", socketid);
+    printf("%-16s: %u\n", "Socket ID", info->id);
     printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
     printf("%-16s: %#llx\n", "Default CBM", (1ull << info->cbm_len) - 1);
     printf("%5s%25s%16s\n", "ID", "NAME", "CBM");
 
-    return psr_cat_print_domain_cbm(domid, socketid);
+    return psr_cat_print_domain_cbm(domid, info->id);
 }
 
 static int psr_cat_show(uint32_t domid)
 {
-    int socketid, nr_sockets;
+    int i, nr;
     int rc;
     libxl_psr_cat_info *info;
 
-    rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr_sockets);
+    rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr);
     if (rc) {
         fprintf(stderr, "Failed to get cat info\n");
         return rc;
     }
 
-    for (socketid = 0; socketid < nr_sockets; socketid++) {
-        rc = psr_cat_print_socket(domid, socketid, info + socketid);
+    for (i = 0; i < nr; i++) {
+        rc = psr_cat_print_socket(domid, info + i);
         if (rc)
             goto out;
     }
 
 out:
-    libxl_psr_cat_info_list_free(info, nr_sockets);
+    libxl_psr_cat_info_list_free(info, nr);
     return rc;
 }
 
-- 
1.9.1

Re: [PATCH v2 for Xen 4.6 3/6] tools/libxl: return socket id from libxl_psr_cat_get_l3_info

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 863 bytes --]

On Tue, 2015-09-29 at 15:49 +0800, Chao Peng wrote:
> The entries returned from libxl_psr_cat_get_l3_info are assumed
> to be socket-continuous. But this is not true in the hotplug case.
> 
> This patch gets the socket bitmap for all the sockets on the system
> first and stores the socket id in the structure libxl_psr_cat_info in
> libxl_psr_cat_get_l3_info. The xl or similar consumers then can
> display
> socket information correctly.
> 
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>

Thanks and Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH v2 for Xen 4.6 3/6] tools/libxl: return socket id from libxl_psr_cat_get_l3_info

[Wei Liu]

On Tue, Sep 29, 2015 at 03:49:52PM +0800, Chao Peng wrote:
> The entries returned from libxl_psr_cat_get_l3_info are assumed
> to be socket-continuous. But this is not true in the hotplug case.
> 
> This patch gets the socket bitmap for all the sockets on the system
> first and stores the socket id in the structure libxl_psr_cat_info in
> libxl_psr_cat_get_l3_info. The xl or similar consumers then can display
> socket information correctly.
> 
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

[PATCH v2 for Xen 4.6 4/6] tools/libxl: fix range check in main_psr_cat_cbm_set

[Chao Peng]

The 'end' should be inclusive.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/xl_cmdimpl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 9947dba..639190e 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -8524,7 +8524,7 @@ int main_psr_cat_cbm_set(int argc, char **argv)
         len = libxl_string_list_length(&socket_list);
         for (i = 0; i < len; i++) {
             parse_range(socket_list[i], &start, &end);
-            for (j = start; j < end; j++)
+            for (j = start; j <= end; j++)
                 libxl_bitmap_set(&target_map, j);
         }
 
-- 
1.9.1

[PATCH v2 for Xen 4.6 5/6] docs: make xl-psr.markdown more precise

[Chao Peng]

Drop the chapter number as it can be confusing when it gets changed in
the referred document.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
v2:
* minor commit message adjustment.
---
 docs/misc/xl-psr.markdown | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown
index 3545912..737f0f7 100644
--- a/docs/misc/xl-psr.markdown
+++ b/docs/misc/xl-psr.markdown
@@ -14,7 +14,7 @@ tracks cache utilization of memory accesses according to the RMID and reports
 monitored data via a counter register.
 
 For more detailed information please refer to Intel SDM chapter
-"17.14 - Platform Shared Resource Monitoring: Cache Monitoring Technology".
+"Platform Shared Resource Monitoring: Cache Monitoring Technology".
 
 In Xen's implementation, each domain in the system can be assigned a RMID
 independently, while RMID=0 is reserved for monitoring domains that don't
@@ -52,7 +52,7 @@ event type to monitor system total/local memory bandwidth. The same RMID can
 be used to monitor both cache usage and memory bandwidth at the same time.
 
 For more detailed information please refer to Intel SDM chapter
-"17.14 - Platform Shared Resource Monitoring: Cache Monitoring Technology".
+"Overview of Cache Monitoring Technology and Memory Bandwidth Monitoring".
 
 In Xen's implementation, MBM shares the same set of underlying monitoring
 service with CMT and can be used to monitor memory bandwidth on a per domain
@@ -92,7 +92,7 @@ For example, assuming a system with 8 portions and 3 domains:
    access to one quarter each.
 
 For more detailed information please refer to Intel SDM chapter
-"17.15 - Platform Shared Resource Control: Cache Allocation Technology".
+"Platform Shared Resource Control: Cache Allocation Technology".
 
 In Xen's implementation, CBM can be configured with libxl/xl interfaces but
 COS is maintained in hypervisor only. The cache partition granularity is per
@@ -130,4 +130,4 @@ Per domain CBM settings can be shown by:
 ## Reference
 
 [1] Intel SDM
-(http://www.intel.com/content/www/us/en/processors/architectures-software-developer-manuals.html).
+(http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-system-programming-manual-325384.pdf).
-- 
1.9.1

Re: [PATCH v2 for Xen 4.6 5/6] docs: make xl-psr.markdown more precise

[Andrew Cooper]

On 29/09/15 08:49, Chao Peng wrote:
> Drop the chapter number as it can be confusing when it gets changed in
> the referred document.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v2:
> * minor commit message adjustment.
> ---
>  docs/misc/xl-psr.markdown | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown
> index 3545912..737f0f7 100644
> --- a/docs/misc/xl-psr.markdown
> +++ b/docs/misc/xl-psr.markdown
> @@ -14,7 +14,7 @@ tracks cache utilization of memory accesses according to the RMID and reports
>  monitored data via a counter register.
>  
>  For more detailed information please refer to Intel SDM chapter
> -"17.14 - Platform Shared Resource Monitoring: Cache Monitoring Technology".
> +"Platform Shared Resource Monitoring: Cache Monitoring Technology".
>  
>  In Xen's implementation, each domain in the system can be assigned a RMID
>  independently, while RMID=0 is reserved for monitoring domains that don't
> @@ -52,7 +52,7 @@ event type to monitor system total/local memory bandwidth. The same RMID can
>  be used to monitor both cache usage and memory bandwidth at the same time.
>  
>  For more detailed information please refer to Intel SDM chapter
> -"17.14 - Platform Shared Resource Monitoring: Cache Monitoring Technology".
> +"Overview of Cache Monitoring Technology and Memory Bandwidth Monitoring".
>  
>  In Xen's implementation, MBM shares the same set of underlying monitoring
>  service with CMT and can be used to monitor memory bandwidth on a per domain
> @@ -92,7 +92,7 @@ For example, assuming a system with 8 portions and 3 domains:
>     access to one quarter each.
>  
>  For more detailed information please refer to Intel SDM chapter
> -"17.15 - Platform Shared Resource Control: Cache Allocation Technology".
> +"Platform Shared Resource Control: Cache Allocation Technology".
>  
>  In Xen's implementation, CBM can be configured with libxl/xl interfaces but
>  COS is maintained in hypervisor only. The cache partition granularity is per
> @@ -130,4 +130,4 @@ Per domain CBM settings can be shown by:
>  ## Reference
>  
>  [1] Intel SDM
> -(http://www.intel.com/content/www/us/en/processors/architectures-software-developer-manuals.html).
> +(http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-system-programming-manual-325384.pdf).

The other two changes look fine, but this change now points to a
specific instance of the SDM, not the most recent version.  I would
leave the link as it previously was.

~Andrew

Re: [PATCH v2 for Xen 4.6 5/6] docs: make xl-psr.markdown more precise

[Ian Campbell]

On Tue, 2015-09-29 at 10:27 +0100, Andrew Cooper wrote:
> On 29/09/15 08:49, Chao Peng wrote:
> > Drop the chapter number as it can be confusing when it gets changed in
> > the referred document.
> > 
> > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> > Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > v2:
> > * minor commit message adjustment.
> > ---
> >  docs/misc/xl-psr.markdown | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown
> > index 3545912..737f0f7 100644
> > --- a/docs/misc/xl-psr.markdown
> > +++ b/docs/misc/xl-psr.markdown
> > @@ -14,7 +14,7 @@ tracks cache utilization of memory accesses according
> > to the RMID and reports
> >  monitored data via a counter register.
> >  
> >  For more detailed information please refer to Intel SDM chapter
> > -"17.14 - Platform Shared Resource Monitoring: Cache Monitoring
> > Technology".
> > +"Platform Shared Resource Monitoring: Cache Monitoring Technology".
> >  
> >  In Xen's implementation, each domain in the system can be assigned a
> > RMID
> >  independently, while RMID=0 is reserved for monitoring domains that
> > don't
> > @@ -52,7 +52,7 @@ event type to monitor system total/local memory
> > bandwidth. The same RMID can
> >  be used to monitor both cache usage and memory bandwidth at the same
> > time.
> >  
> >  For more detailed information please refer to Intel SDM chapter
> > -"17.14 - Platform Shared Resource Monitoring: Cache Monitoring
> > Technology".
> > +"Overview of Cache Monitoring Technology and Memory Bandwidth
> > Monitoring".
> >  
> >  In Xen's implementation, MBM shares the same set of underlying
> > monitoring
> >  service with CMT and can be used to monitor memory bandwidth on a per
> > domain
> > @@ -92,7 +92,7 @@ For example, assuming a system with 8 portions and 3
> > domains:
> >     access to one quarter each.
> >  
> >  For more detailed information please refer to Intel SDM chapter
> > -"17.15 - Platform Shared Resource Control: Cache Allocation
> > Technology".
> > +"Platform Shared Resource Control: Cache Allocation Technology".
> >  
> >  In Xen's implementation, CBM can be configured with libxl/xl
> > interfaces but
> >  COS is maintained in hypervisor only. The cache partition granularity
> > is per
> > @@ -130,4 +130,4 @@ Per domain CBM settings can be shown by:
> >  ## Reference
> >  
> >  [1] Intel SDM
> > -(http://www.intel.com/content/www/us/en/processors/architectures-softw
> > are-developer-manuals.html).
> > +(http://www.intel.com/content/dam/www/public/us/en/documents/manuals/6
> > 4-ia-32-architectures-software-developer-system-programming-manual
> > -325384.pdf).
> 
> The other two changes look fine, but this change now points to a
> specific instance of the SDM, not the most recent version.  I would
> leave the link as it previously was.

I agree, we either want a specific revision with specific chapter numbers
_or_ an up to date revision with just the titles and no specific chapter
numbers. This commit removes the chapter numbers but switches to a specific
revision, which is undesirable IMHO.

I therefore intend to drop this last hunk during commit.

Ian.

Re: [PATCH v2 for Xen 4.6 5/6] docs: make xl-psr.markdown more precise

[Chao Peng]

On Tue, Sep 29, 2015 at 10:55:40AM +0100, Ian Campbell wrote:
> On Tue, 2015-09-29 at 10:27 +0100, Andrew Cooper wrote:
> > On 29/09/15 08:49, Chao Peng wrote:
> > > Drop the chapter number as it can be confusing when it gets changed in
> > > the referred document.
> > > 
> > > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> > > Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
> > > Acked-by: Wei Liu <wei.liu2@citrix.com>
> > > ---
> > > v2:
> > > * minor commit message adjustment.
> > > ---
> > >  docs/misc/xl-psr.markdown | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown
> > > index 3545912..737f0f7 100644
> > > --- a/docs/misc/xl-psr.markdown
> > > +++ b/docs/misc/xl-psr.markdown
> > > @@ -14,7 +14,7 @@ tracks cache utilization of memory accesses according
> > > to the RMID and reports
> > >  monitored data via a counter register.
> > >  
> > >  For more detailed information please refer to Intel SDM chapter
> > > -"17.14 - Platform Shared Resource Monitoring: Cache Monitoring
> > > Technology".
> > > +"Platform Shared Resource Monitoring: Cache Monitoring Technology".
> > >  
> > >  In Xen's implementation, each domain in the system can be assigned a
> > > RMID
> > >  independently, while RMID=0 is reserved for monitoring domains that
> > > don't
> > > @@ -52,7 +52,7 @@ event type to monitor system total/local memory
> > > bandwidth. The same RMID can
> > >  be used to monitor both cache usage and memory bandwidth at the same
> > > time.
> > >  
> > >  For more detailed information please refer to Intel SDM chapter
> > > -"17.14 - Platform Shared Resource Monitoring: Cache Monitoring
> > > Technology".
> > > +"Overview of Cache Monitoring Technology and Memory Bandwidth
> > > Monitoring".
> > >  
> > >  In Xen's implementation, MBM shares the same set of underlying
> > > monitoring
> > >  service with CMT and can be used to monitor memory bandwidth on a per
> > > domain
> > > @@ -92,7 +92,7 @@ For example, assuming a system with 8 portions and 3
> > > domains:
> > >     access to one quarter each.
> > >  
> > >  For more detailed information please refer to Intel SDM chapter
> > > -"17.15 - Platform Shared Resource Control: Cache Allocation
> > > Technology".
> > > +"Platform Shared Resource Control: Cache Allocation Technology".
> > >  
> > >  In Xen's implementation, CBM can be configured with libxl/xl
> > > interfaces but
> > >  COS is maintained in hypervisor only. The cache partition granularity
> > > is per
> > > @@ -130,4 +130,4 @@ Per domain CBM settings can be shown by:
> > >  ## Reference
> > >  
> > >  [1] Intel SDM
> > > -(http://www.intel.com/content/www/us/en/processors/architectures-softw
> > > are-developer-manuals.html).
> > > +(http://www.intel.com/content/dam/www/public/us/en/documents/manuals/6
> > > 4-ia-32-architectures-software-developer-system-programming-manual
> > > -325384.pdf).
> > 
> > The other two changes look fine, but this change now points to a
> > specific instance of the SDM, not the most recent version.  I would
> > leave the link as it previously was.
> 
> I agree, we either want a specific revision with specific chapter numbers
> _or_ an up to date revision with just the titles and no specific chapter
> numbers. This commit removes the chapter numbers but switches to a specific
> revision, which is undesirable IMHO.
> 
> I therefore intend to drop this last hunk during commit.

I feel confortable, too. Actually this is a suggestion from Lars during
he reviewing the documents for the 4.6 release. Because when one opens
the generic page he/she will see several options (combined volume set,
three-volume set and seven-volume set), it may be not easy to find out
the related chapters and especially because we have chapter number which
is actually from the three-volume set, in such case it does look a bit
confusing.

As now we have removed the chapter number, then the confusion doesn't
exit because one can grab any volume set and will find the related
chapters. I think this addressed Lars' concern.

Chao

Re: [PATCH v2 for Xen 4.6 5/6] docs: make xl-psr.markdown more precise

[Ian Campbell]

On Wed, 2015-09-30 at 09:34 +0800, Chao Peng wrote:
> On Tue, Sep 29, 2015 at 10:55:40AM +0100, Ian Campbell wrote:
> > On Tue, 2015-09-29 at 10:27 +0100, Andrew Cooper wrote:
> > > On 29/09/15 08:49, Chao Peng wrote: 
> > > >  [1] Intel SDM
> > > > -(http://www.intel.com/content/www/us/en/processors/architectures-s
> > > > oftw
> > > > are-developer-manuals.html).
> > > > +(http://www.intel.com/content/dam/www/public/us/en/documents/manua
> > > > ls/6
> > > > 4-ia-32-architectures-software-developer-system-programming-manual
> > > > -325384.pdf).
> > > 
> > > The other two changes look fine, but this change now points to a
> > > specific instance of the SDM, not the most recent version.  I would
> > > leave the link as it previously was.
> > 
> > I agree, we either want a specific revision with specific chapter
> > numbers
> > _or_ an up to date revision with just the titles and no specific
> > chapter
> > numbers. This commit removes the chapter numbers but switches to a
> > specific
> > revision, which is undesirable IMHO.
> > 
> > I therefore intend to drop this last hunk during commit.
> 
> I feel confortable, too.

Thanks for confirming.

>  Actually this is a suggestion from Lars during
> he reviewing the documents for the 4.6 release. Because when one opens
> the generic page he/she will see several options (combined volume set,
> three-volume set and seven-volume set), it may be not easy to find out
> the related chapters and especially because we have chapter number which
> is actually from the three-volume set, in such case it does look a bit
> confusing.

Right. Would it be worth mentioning the specific volume which is of
interest as a guide?

> 
> As now we have removed the chapter number, then the confusion doesn't
> exit because one can grab any volume set and will find the related
> chapters. I think this addressed Lars' concern.
> 
> Chao
> 

Re: [PATCH v2 for Xen 4.6 5/6] docs: make xl-psr.markdown more precise

[Chao Peng]

> >  Actually this is a suggestion from Lars during
> > he reviewing the documents for the 4.6 release. Because when one opens
> > the generic page he/she will see several options (combined volume set,
> > three-volume set and seven-volume set), it may be not easy to find out
> > the related chapters and especially because we have chapter number which
> > is actually from the three-volume set, in such case it does look a bit
> > confusing.
> 
> Right. Would it be worth mentioning the specific volume which is of
> interest as a guide?

I think it would be good to have such info. While I also think is's not a
big deal, I feel okay without it.

> 
> > 
> > As now we have removed the chapter number, then the confusion doesn't
> > exit because one can grab any volume set and will find the related
> > chapters. I think this addressed Lars' concern.
> > 
> > Chao
> > 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

[PATCH v2 for Xen 4.6 6/6] docs/man: resort sections

[Chao Peng]

Section 'IGNORED FOR COMPATIBILITY WITH XM' separates 'CACHE MONITORING
TECHNOLOGY' and 'CACHE ALLOCATION TECHNOLOGY' but they really should be
put together.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
Current incorrect output can be seen at:
http://xenbits.xen.org/docs/unstable/man/xl.1.html 
---
 docs/man/xl.pod.1 | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index f22c3f3..d0cd612 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1509,18 +1509,6 @@ monitor types are:
 
 =back
 
-=head1 IGNORED FOR COMPATIBILITY WITH XM
-
-xl is mostly command-line compatible with the old xm utility used with
-the old Python xend.  For compatibility, the following options are
-ignored:
-
-=over 4
-
-=item B<xl migrate --live>
-
-=back
-
 =head2 CACHE ALLOCATION TECHNOLOGY
 
 Intel Broadwell and later server platforms offer capabilities to configure and
@@ -1553,6 +1541,18 @@ Show CAT settings for a certain domain or all domains.
 
 =back
 
+=head1 IGNORED FOR COMPATIBILITY WITH XM
+
+xl is mostly command-line compatible with the old xm utility used with
+the old Python xend.  For compatibility, the following options are
+ignored:
+
+=over 4
+
+=item B<xl migrate --live>
+
+=back
+
 =head1 TO BE DOCUMENTED
 
 We need better documentation for:
-- 
1.9.1

Re: [PATCH v2 for Xen 4.6 6/6] docs/man: resort sections

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 651 bytes --]

On Tue, 2015-09-29 at 15:49 +0800, Chao Peng wrote:
> Section 'IGNORED FOR COMPATIBILITY WITH XM' separates 'CACHE
> MONITORING
> TECHNOLOGY' and 'CACHE ALLOCATION TECHNOLOGY' but they really should
> be
> put together.
> 
Indeed.

> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>

Thanks and Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH v2 for Xen 4.6 6/6] docs/man: resort sections

[Wei Liu]

On Tue, Sep 29, 2015 at 03:49:55PM +0800, Chao Peng wrote:
> Section 'IGNORED FOR COMPATIBILITY WITH XM' separates 'CACHE MONITORING
> TECHNOLOGY' and 'CACHE ALLOCATION TECHNOLOGY' but they really should be
> put together.
> 
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Re: [PATCH v2 for Xen 4.6 0/6] Several PSR fixes in libxl

[Wei Liu]

Now the reasoning bits. Yes, I'm arguing with myself, :-)

We can of course fix it post-4.6, but the released APIs need to be
maintained forever (even if it is in fact broken). That would definitely
involve lots of compatibility cruft if we fix it post 4.6. 

This patch series is simple enough to reason about and has received
adequate review from expert in the field, so I have hight confidence in
it being correct.

I think the benefit of accepting it out-weights the downside.

Wei.

Re: [PATCH v2 for Xen 4.6 0/6] Several PSR fixes in libxl

[Ian Campbell]

On Tue, 2015-09-29 at 10:33 +0100, Wei Liu wrote:
> Now the reasoning bits. Yes, I'm arguing with myself, :-)
> 
> We can of course fix it post-4.6, but the released APIs need to be
> maintained forever (even if it is in fact broken). That would definitely
> involve lots of compatibility cruft if we fix it post 4.6. 
> 
> This patch series is simple enough to reason about and has received
> adequate review from expert in the field, so I have hight confidence in
> it being correct.
> 
> I think the benefit of accepting it out-weights the downside.

Applied all 6 to staging and staging-4.6, with the one tweak discussed in
reply to #5.

Re: [PATCH v2 for Xen 4.6 0/6] Several PSR fixes in libxl

[Chao Peng]

On Tue, Sep 29, 2015 at 11:30:53AM +0100, Ian Campbell wrote:
> On Tue, 2015-09-29 at 10:33 +0100, Wei Liu wrote:
> > Now the reasoning bits. Yes, I'm arguing with myself, :-)
> > 
> > We can of course fix it post-4.6, but the released APIs need to be
> > maintained forever (even if it is in fact broken). That would definitely
> > involve lots of compatibility cruft if we fix it post 4.6. 
> > 
> > This patch series is simple enough to reason about and has received
> > adequate review from expert in the field, so I have hight confidence in
> > it being correct.
> > 
> > I think the benefit of accepting it out-weights the downside.
> 
> Applied all 6 to staging and staging-4.6, with the one tweak discussed in
> reply to #5.

I see, thanks for the quick action and allow them to go into 4.6.

Chao