diff mbox

[v1,11/13] tools: implemet new get value interface suitable for all psr allocation features

Message ID 1502264512-4648-12-git-send-email-yi.y.sun@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Yi Sun Aug. 9, 2017, 7:41 a.m. UTC
This patch implements a new get value interface in tools suitable for all psr
allocation features and the whole flow. It also enables MBA support in tools
to get MBA value.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
---
 tools/libxc/include/xenctrl.h |   7 +-
 tools/libxc/xc_psr.c          |   9 ++-
 tools/libxl/libxl_psr.c       |  51 +++++++-----
 tools/xl/xl.h                 |   1 +
 tools/xl/xl_cmdtable.c        |   5 ++
 tools/xl/xl_psr.c             | 179 +++++++++++++++++++++++++++++-------------
 6 files changed, 173 insertions(+), 79 deletions(-)

Comments

Wei Liu Aug. 15, 2017, 11:24 a.m. UTC | #1
On Wed, Aug 09, 2017 at 03:41:50PM +0800, Yi Sun wrote:
> This patch implements a new get value interface in tools suitable for all psr
> allocation features and the whole flow. It also enables MBA support in tools
> to get MBA value.
> 
> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>

Again, I would appreciate you write down what is done in this patch in
the commit message.

Luckily this patch doesn't look too complex so:

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

I would also like an ack or review from Chao on the xl bits, to confirm
he is okay with the command line interface.
Chao Peng Aug. 21, 2017, 10:14 a.m. UTC | #2
>  
> -static void libxl__psr_cat_log_err_msg(libxl__gc *gc, int err)
> +static void libxl__psr_alloc_log_err_msg(libxl__gc *gc,
> +                                         int err,
> +                                         libxl_psr_cbm_type type)
>  {
> +    /*
> +     * Index is 'libxl_psr_cbm_type' so we set two 'CDP' to
> correspond to
> +     * DATA and CODE.
> +     */

Or just use "CDP CODE" and "CDP DATA" respectively?

> +    const char * const feat_name[6] = {
> +        "UNKNOWN",
> +        "L3 CAT",
> +        "CDP",
> +        "CDP",
> +        "L2 CAT",
> +        "MBA",
> +    };
>      char *msg;
>
Yi Sun Aug. 22, 2017, 2:24 a.m. UTC | #3
On 17-08-21 18:14:49, Chao Peng wrote:
> 
> >  
> > -static void libxl__psr_cat_log_err_msg(libxl__gc *gc, int err)
> > +static void libxl__psr_alloc_log_err_msg(libxl__gc *gc,
> > +                                         int err,
> > +                                         libxl_psr_cbm_type type)
> >  {
> > +    /*
> > +     * Index is 'libxl_psr_cbm_type' so we set two 'CDP' to
> > correspond to
> > +     * DATA and CODE.
> > +     */
> 
> Or just use "CDP CODE" and "CDP DATA" respectively?
> 
They are just value type but not feature name. So, I duplicate 'CDP'
here.

> > +    const char * const feat_name[6] = {
> > +        "UNKNOWN",
> > +        "L3 CAT",
> > +        "CDP",
> > +        "CDP",
> > +        "L2 CAT",
> > +        "MBA",
> > +    };
> >      char *msg;
> >
Chao Peng Aug. 22, 2017, 6:44 a.m. UTC | #4
On Tue, 2017-08-22 at 10:24 +0800, Yi Sun wrote:
> On 17-08-21 18:14:49, Chao Peng wrote:
> > 
> > 
> > > 
> > >  
> > > -static void libxl__psr_cat_log_err_msg(libxl__gc *gc, int err)
> > > +static void libxl__psr_alloc_log_err_msg(libxl__gc *gc,
> > > +                                         int err,
> > > +                                         libxl_psr_cbm_type type)
> > >  {
> > > +    /*
> > > +     * Index is 'libxl_psr_cbm_type' so we set two 'CDP' to
> > > correspond to
> > > +     * DATA and CODE.
> > > +     */
> > 
> > Or just use "CDP CODE" and "CDP DATA" respectively?
> > 
> They are just value type but not feature name. So, I duplicate 'CDP'
> here.

I'm fine.

Chao
> 
> > 
> > > 
> > > +    const char * const feat_name[6] = {
> > > +        "UNKNOWN",
> > > +        "L3 CAT",
> > > +        "CDP",
> > > +        "CDP",
> > > +        "L2 CAT",
> > > +        "MBA",
> > > +    };
> > >      char *msg;
> > >
diff mbox

Patch

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index f1c629e..d7eabfb 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2455,6 +2455,7 @@  enum xc_psr_val_type {
     XC_PSR_CAT_L3_CBM_CODE = 2,
     XC_PSR_CAT_L3_CBM_DATA = 3,
     XC_PSR_CAT_L2_CBM      = 4,
+    XC_PSR_MBA_THRTL       = 5,
 };
 typedef enum xc_psr_val_type xc_psr_val_type;
 
@@ -2501,9 +2502,9 @@  int xc_psr_cmt_enabled(xc_interface *xch);
 int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
                                xc_psr_val_type type, uint32_t target,
                                uint64_t data);
-int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
-                               xc_psr_val_type type, uint32_t target,
-                               uint64_t *data);
+int xc_psr_get_domain_data(xc_interface *xch, uint32_t domid,
+                           xc_psr_val_type type, uint32_t target,
+                           uint64_t *data);
 int xc_psr_get_hw_info(xc_interface *xch, uint32_t socket,
                        xc_psr_feat_type type, xc_psr_hw_info *hw_info);
 
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index c69e943..de501b0 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -283,9 +283,9 @@  int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
     return do_domctl(xch, &domctl);
 }
 
-int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
-                               xc_psr_val_type type, uint32_t target,
-                               uint64_t *data)
+int xc_psr_get_domain_data(xc_interface *xch, uint32_t domid,
+                           xc_psr_val_type type, uint32_t target,
+                           uint64_t *data)
 {
     int rc;
     DECLARE_DOMCTL;
@@ -305,6 +305,9 @@  int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
     case XC_PSR_CAT_L2_CBM:
         cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L2_CBM;
         break;
+    case XC_PSR_MBA_THRTL:
+        cmd = XEN_DOMCTL_PSR_MBA_OP_GET_THRTL;
+        break;
     default:
         errno = EINVAL;
         return -1;
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 1d75f33..4b91e9b 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -71,16 +71,30 @@  static void libxl__psr_cmt_log_err_msg(libxl__gc *gc, int err)
     LOGE(ERROR, "%s", msg);
 }
 
-static void libxl__psr_cat_log_err_msg(libxl__gc *gc, int err)
+static void libxl__psr_alloc_log_err_msg(libxl__gc *gc,
+                                         int err,
+                                         libxl_psr_cbm_type type)
 {
+    /*
+     * Index is 'libxl_psr_cbm_type' so we set two 'CDP' to correspond to
+     * DATA and CODE.
+     */
+    const char * const feat_name[6] = {
+        "UNKNOWN",
+        "L3 CAT",
+        "CDP",
+        "CDP",
+        "L2 CAT",
+        "MBA",
+    };
     char *msg;
 
     switch (err) {
     case ENODEV:
-        msg = "CAT is not supported in this system";
+        msg = "is not supported in this system";
         break;
     case ENOENT:
-        msg = "CAT is not enabled on the socket";
+        msg = "is not enabled on the socket";
         break;
     case EOVERFLOW:
         msg = "no free COS available";
@@ -106,7 +120,7 @@  static void libxl__psr_cat_log_err_msg(libxl__gc *gc, int err)
         return;
     }
 
-    LOGE(ERROR, "%s", msg);
+    LOGE(ERROR, "%s: %s", feat_name[type], msg);
 }
 
 static int libxl__pick_socket_cpu(libxl__gc *gc, uint32_t socketid)
@@ -333,7 +347,7 @@  int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
         xc_type = libxl__psr_cbm_type_to_libxc_psr_val_type(type);
         if (xc_psr_cat_set_domain_data(ctx->xch, domid, xc_type,
                                        socketid, cbm)) {
-            libxl__psr_cat_log_err_msg(gc, errno);
+            libxl__psr_alloc_log_err_msg(gc, errno, type);
             rc = ERROR_FAIL;
         }
     }
@@ -347,18 +361,7 @@  int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
                           libxl_psr_cbm_type type, uint32_t target,
                           uint64_t *cbm_r)
 {
-    GC_INIT(ctx);
-    int rc = 0;
-    xc_psr_val_type xc_type = libxl__psr_cbm_type_to_libxc_psr_val_type(type);
-
-    if (xc_psr_cat_get_domain_data(ctx->xch, domid, xc_type,
-                                   target, cbm_r)) {
-        libxl__psr_cat_log_err_msg(gc, errno);
-        rc = ERROR_FAIL;
-    }
-
-    GC_FREE;
-    return rc;
+    return libxl_psr_get_val(ctx, domid, type, target, cbm_r);
 }
 
 static inline int libxl__psr_hw_info_to_libxl_psr_cat_info(
@@ -438,7 +441,19 @@  int libxl_psr_get_val(libxl_ctx *ctx, uint32_t domid,
                       libxl_psr_cbm_type type, unsigned int target,
                       uint64_t *val)
 {
-    return ERROR_FAIL;
+    GC_INIT(ctx);
+    int rc = 0;
+
+    xc_psr_val_type xc_type = libxl__psr_cbm_type_to_libxc_psr_val_type(type);
+
+    if (xc_psr_get_domain_data(ctx->xch, domid, xc_type,
+                               target, val)) {
+        libxl__psr_alloc_log_err_msg(gc, errno, type);
+        rc = ERROR_FAIL;
+    }
+
+    GC_FREE;
+    return rc;
 }
 
 static inline xc_psr_feat_type libxl__psr_feat_type_to_libxc_psr_feat_type(
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index aa95b77..59065c4 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -206,6 +206,7 @@  int main_psr_cmt_show(int argc, char **argv);
 #ifdef LIBXL_HAVE_PSR_CAT
 int main_psr_cat_cbm_set(int argc, char **argv);
 int main_psr_cat_show(int argc, char **argv);
+int main_psr_mba_show(int argc, char **argv);
 #endif
 int main_qemu_monitor_command(int argc, char **argv);
 
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 5ac8a7e..19e5004 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -562,6 +562,11 @@  struct cmd_spec cmd_table[] = {
       "[options] <Domain>",
       "-l <level>        Specify the cache level to process, otherwise L3 cache is processed\n"
     },
+    { "psr-mba-show",
+      &main_psr_mba_show, 0, 1,
+      "Show Memory Bandwidth Allocation information",
+      "<Domain>",
+    },
 
 #endif
     { "usbctrl-attach",
diff --git a/tools/xl/xl_psr.c b/tools/xl/xl_psr.c
index bf41762..d7edab2 100644
--- a/tools/xl/xl_psr.c
+++ b/tools/xl/xl_psr.c
@@ -330,19 +330,21 @@  out:
     return rc;
 }
 
-static void psr_cat_print_one_domain_cbm_type(uint32_t domid, uint32_t socketid,
-                                              libxl_psr_cbm_type type)
+static void psr_print_one_domain_val_type(uint32_t domid, uint32_t socketid,
+                                          libxl_psr_cbm_type type)
 {
-    uint64_t cbm;
+    uint64_t val;
 
-    if (!libxl_psr_cat_get_cbm(ctx, domid, type, socketid, &cbm))
-        printf("%#16"PRIx64, cbm);
+    if (!libxl_psr_get_val(ctx, domid, type, socketid, &val))
+        printf("%#16"PRIx64, val);
     else
         printf("%16s", "error");
 }
 
-static void psr_cat_print_one_domain_cbm(uint32_t domid, uint32_t socketid,
-                                         bool cdp_enabled, unsigned int lvl)
+static void psr_print_one_domain_val(uint32_t domid,
+                                     libxl_psr_hw_info *info,
+                                     libxl_psr_feat_type type,
+                                     unsigned int lvl)
 {
     char *domain_name;
 
@@ -350,106 +352,152 @@  static void psr_cat_print_one_domain_cbm(uint32_t domid, uint32_t socketid,
     printf("%5d%25s", domid, domain_name);
     free(domain_name);
 
-    switch (lvl) {
-    case 3:
-        if (!cdp_enabled) {
-            psr_cat_print_one_domain_cbm_type(domid, socketid,
+    switch (type) {
+    case LIBXL_PSR_FEAT_TYPE_CAT_INFO:
+        switch (lvl) {
+        case 3:
+            if (!info->u.cat_info.cdp_enabled) {
+                psr_print_one_domain_val_type(domid, info->id,
                                               LIBXL_PSR_CBM_TYPE_L3_CBM);
-        } else {
-            psr_cat_print_one_domain_cbm_type(domid, socketid,
+            } else {
+                psr_print_one_domain_val_type(domid, info->id,
                                               LIBXL_PSR_CBM_TYPE_L3_CBM_CODE);
-            psr_cat_print_one_domain_cbm_type(domid, socketid,
+                psr_print_one_domain_val_type(domid, info->id,
                                               LIBXL_PSR_CBM_TYPE_L3_CBM_DATA);
-        }
-        break;
-    case 2:
-        psr_cat_print_one_domain_cbm_type(domid, socketid,
+            }
+            break;
+        case 2:
+            psr_print_one_domain_val_type(domid, info->id,
                                           LIBXL_PSR_CBM_TYPE_L2_CBM);
+            break;
+        default:
+            printf("Input lvl %d is wrong!", lvl);
+        }
         break;
-    default:
-        printf("Input lvl %d is wrong!", lvl);
+
+    case LIBXL_PSR_FEAT_TYPE_MBA_INFO:
+        psr_print_one_domain_val_type(domid, info->id,
+                                      LIBXL_PSR_CBM_TYPE_MBA_THRTL);
         break;
     }
 
     printf("\n");
 }
 
-static int psr_cat_print_domain_cbm(uint32_t domid, uint32_t socketid,
-                                    bool cdp_enabled, unsigned int lvl)
+static int psr_print_domain_val(uint32_t domid,
+                                libxl_psr_hw_info *info,
+                                libxl_psr_feat_type type,
+                                unsigned int lvl)
 {
     int i, nr_domains;
     libxl_dominfo *list;
 
     if (domid != INVALID_DOMID) {
-        psr_cat_print_one_domain_cbm(domid, socketid, cdp_enabled, lvl);
+        psr_print_one_domain_val(domid, info, type, lvl);
         return 0;
     }
 
     if (!(list = libxl_list_domain(ctx, &nr_domains))) {
-        fprintf(stderr, "Failed to get domain list for cbm display\n");
-        return -1;
+        fprintf(stderr, "Failed to get domain list for value display\n");
+        return EXIT_FAILURE;
     }
 
     for (i = 0; i < nr_domains; i++)
-        psr_cat_print_one_domain_cbm(list[i].domid, socketid, cdp_enabled, lvl);
+        psr_print_one_domain_val(list[i].domid, info, type, lvl);
     libxl_dominfo_list_free(list, nr_domains);
 
     return 0;
 }
 
-static int psr_cat_print_socket(uint32_t domid, libxl_psr_cat_info *info,
-                                unsigned int lvl)
+static int psr_print_socket(uint32_t domid,
+                            libxl_psr_hw_info *info,
+                            libxl_psr_feat_type type,
+                            unsigned int lvl)
 {
-    int rc;
-    uint32_t l3_cache_size;
-
     printf("%-16s: %u\n", "Socket ID", info->id);
 
-    /* So far, CMT only supports L3 cache. */
-    if (lvl == 3) {
-        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",
-                    info->id);
-            return -1;
+    switch (type) {
+    case LIBXL_PSR_FEAT_TYPE_CAT_INFO:
+    {
+        int rc;
+        uint32_t l3_cache_size;
+
+        /* So far, CMT only supports L3 cache. */
+        if (lvl == 3) {
+            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",
+                        info->id);
+                return -1;
+            }
+            printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
         }
-        printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
+
+        printf("%-16s: %#llx\n", "Default CBM",
+               (1ull << info->u.cat_info.cbm_len) - 1);
+        if (info->u.cat_info.cdp_enabled)
+            printf("%5s%25s%16s%16s\n", "ID", "NAME", "CBM (code)", "CBM (data)");
+        else
+            printf("%5s%25s%16s\n", "ID", "NAME", "CBM");
+
+        break;
     }
 
-    printf("%-16s: %#llx\n", "Default CBM", (1ull << info->cbm_len) - 1);
-    if (info->cdp_enabled)
-        printf("%5s%25s%16s%16s\n", "ID", "NAME", "CBM (code)", "CBM (data)");
-    else
-        printf("%5s%25s%16s\n", "ID", "NAME", "CBM");
+    case LIBXL_PSR_FEAT_TYPE_MBA_INFO:
+        printf("%-16s: %u\n", "Default THRTL", 0);
+        printf("%5s%25s%16s\n", "ID", "NAME", "THRTL");
+        break;
 
-    return psr_cat_print_domain_cbm(domid, info->id, info->cdp_enabled, lvl);
+    default:
+        fprintf(stderr, "Input feature type %d is wrong\n", type);
+        return EXIT_FAILURE;
+    }
+
+    return psr_print_domain_val(domid, info, type, lvl);
 }
 
-static int psr_cat_show(uint32_t domid, unsigned int lvl)
+static int psr_val_show(uint32_t domid,
+                        libxl_psr_feat_type type,
+                        unsigned int lvl)
 {
-    int i, nr;
+    unsigned int i, nr;
     int rc;
-    libxl_psr_cat_info *info;
+    libxl_psr_hw_info *info;
 
-    if (lvl != 2 && lvl != 3) {
-        fprintf(stderr, "Input lvl %d is wrong\n", lvl);
+    switch (type) {
+    case LIBXL_PSR_FEAT_TYPE_CAT_INFO:
+        if (lvl != 2 && lvl != 3) {
+            fprintf(stderr, "Input lvl %d is wrong\n", lvl);
+            return EXIT_FAILURE;
+        }
+        break;
+
+    case LIBXL_PSR_FEAT_TYPE_MBA_INFO:
+        if (lvl) {
+            fprintf(stderr, "Input lvl %d is wrong\n", lvl);
+            return EXIT_FAILURE;
+        }
+        break;
+
+    default:
+        fprintf(stderr, "Input feature type %d is wrong\n", type);
         return EXIT_FAILURE;
     }
 
-    rc = libxl_psr_cat_get_info(ctx, &info, &nr, lvl);
+    rc = libxl_psr_get_hw_info(ctx, &info, &nr, type, lvl);
     if (rc) {
-        fprintf(stderr, "Failed to get %s cat info\n", (lvl == 3)?"L3":"L2");
+        fprintf(stderr, "Failed to get info\n");
         return rc;
     }
 
     for (i = 0; i < nr; i++) {
-        rc = psr_cat_print_socket(domid, info + i, lvl);
+        rc = psr_print_socket(domid, info + i, type, lvl);
         if (rc)
             goto out;
     }
 
 out:
-    libxl_psr_cat_info_list_free(info, nr);
+    libxl_psr_hw_info_list_free(info, nr);
     return rc;
 }
 
@@ -479,6 +527,27 @@  static int psr_l2_cat_hwinfo(void)
     return rc;
 }
 
+int main_psr_mba_show(int argc, char **argv)
+{
+    int opt;
+    uint32_t domid;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "psr-mba-show", 0) {
+        /* No options */
+    }
+
+    if (optind >= argc)
+        domid = INVALID_DOMID;
+    else if (optind == argc - 1)
+        domid = find_domain(argv[optind]);
+    else {
+        help("psr-mba-show");
+        return 2;
+    }
+
+    return psr_val_show(domid, LIBXL_PSR_FEAT_TYPE_MBA_INFO, 0);
+}
+
 static int psr_mba_hwinfo(void)
 {
     int rc;
@@ -618,7 +687,7 @@  int main_psr_cat_show(int argc, char **argv)
         return 2;
     }
 
-    return psr_cat_show(domid, lvl);
+    return psr_val_show(domid, LIBXL_PSR_FEAT_TYPE_CAT_INFO, lvl);
 }
 
 int main_psr_hwinfo(int argc, char **argv)