Re: [Xen-devel] [PATCH v2 11/15] tools: implement the new xl get hw info interface

[Yi Sun]

On 17-08-31 09:43:51, Roger Pau Monn� wrote:
> On Thu, Aug 31, 2017 at 01:57:09PM +0800, Yi Sun wrote:
> > On 17-08-30 10:23:18, Roger Pau Monn� wrote:
> > > On Thu, Aug 24, 2017 at 09:14:45AM +0800, Yi Sun wrote:
> > > > +static int psr_mba_hwinfo(void)
> > > > +{
> > > > +int rc;
> > > > +unsigned int i, nr;
> > > > +libxl_psr_hw_info *info;
> > > > +
> > > > +rc = libxl_psr_get_hw_info(ctx, , ,
> > > > +   LIBXL_PSR_FEAT_TYPE_MBA, 0);
> > > > +if (rc)
> > > > +return rc;
> > > > +
> > > > +printf("Memory Bandwidth Allocation (MBA):\n");
> > > > +
> > > > +for (i = 0; i < nr; i++) {
> > > > +printf("%-16s: %u\n", "Socket ID", info[i].id);
> > > > +printf("%-16s: %s\n", "Linear Mode",
> > > > +   info[i].u.mba.linear ? "Enabled" : "Disabled");
> > > > +printf("%-16s: %u\n", "Maximum COS", info[i].u.mba.cos_max);
> > > > +printf("%-16s: %u\n", "Maximum Throttling Value",
> > > > +   info[i].u.mba.thrtl_max);
> > > > +printf("%-16s: %u\n", "Default Throttling Value", 0);
> > > 
> > > If you really want to left-justify, shouldn't you choose a value that
> > > aligns everything nicely (strlen("Default Throttling Valu") is
> > > already greater than 16).
> > > 
> > Sorry for missing this.
> > 
> > > In fact you can do the alignment manually in the format string, and
> > > avoid passing the name as the first parameter.
> > > 
> > DYM a sentence like below?
> >   printf("%-*s: %u\n", 23, "Default Throttling Value", 0);
> 
> I was thinking more like:
> 
> printf(" Maximum COS: %u\n", ...
> printf("Maximum Throttling Value: %u\n", ...
> printf("Default Throttling Value: %u\n", ...
> 
> Or however you wish to align them. Although I don't have a strong
> opinion, as long as it's easy to read.
> 
Got it. To keep the format be same as CAT, I would like to keep left alignment:

  printf("Maximum COS : %u\n", ...
  printf("Maximum Throttling Value: %u\n", ...
  printf("Default Throttling Value: %u\n", ...

> Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 11/15] tools: implement the new xl get hw info interface

[Roger Pau Monn�]

On Thu, Aug 31, 2017 at 01:57:09PM +0800, Yi Sun wrote:
> On 17-08-30 10:23:18, Roger Pau Monn� wrote:
> > On Thu, Aug 24, 2017 at 09:14:45AM +0800, Yi Sun wrote:
> > > +static int psr_mba_hwinfo(void)
> > > +{
> > > +int rc;
> > > +unsigned int i, nr;
> > > +libxl_psr_hw_info *info;
> > > +
> > > +rc = libxl_psr_get_hw_info(ctx, , ,
> > > +   LIBXL_PSR_FEAT_TYPE_MBA, 0);
> > > +if (rc)
> > > +return rc;
> > > +
> > > +printf("Memory Bandwidth Allocation (MBA):\n");
> > > +
> > > +for (i = 0; i < nr; i++) {
> > > +printf("%-16s: %u\n", "Socket ID", info[i].id);
> > > +printf("%-16s: %s\n", "Linear Mode",
> > > +   info[i].u.mba.linear ? "Enabled" : "Disabled");
> > > +printf("%-16s: %u\n", "Maximum COS", info[i].u.mba.cos_max);
> > > +printf("%-16s: %u\n", "Maximum Throttling Value",
> > > +   info[i].u.mba.thrtl_max);
> > > +printf("%-16s: %u\n", "Default Throttling Value", 0);
> > 
> > If you really want to left-justify, shouldn't you choose a value that
> > aligns everything nicely (strlen("Default Throttling Valu") is
> > already greater than 16).
> > 
> Sorry for missing this.
> 
> > In fact you can do the alignment manually in the format string, and
> > avoid passing the name as the first parameter.
> > 
> DYM a sentence like below?
>   printf("%-*s: %u\n", 23, "Default Throttling Value", 0);

I was thinking more like:

printf(" Maximum COS: %u\n", ...
printf("Maximum Throttling Value: %u\n", ...
printf("Default Throttling Value: %u\n", ...

Or however you wish to align them. Although I don't have a strong
opinion, as long as it's easy to read.

Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 11/15] tools: implement the new xl get hw info interface

[Yi Sun]

On 17-08-30 10:23:18, Roger Pau Monn� wrote:
> On Thu, Aug 24, 2017 at 09:14:45AM +0800, Yi Sun wrote:
> > +static int psr_mba_hwinfo(void)
> > +{
> > +int rc;
> > +unsigned int i, nr;
> > +libxl_psr_hw_info *info;
> > +
> > +rc = libxl_psr_get_hw_info(ctx, , ,
> > +   LIBXL_PSR_FEAT_TYPE_MBA, 0);
> > +if (rc)
> > +return rc;
> > +
> > +printf("Memory Bandwidth Allocation (MBA):\n");
> > +
> > +for (i = 0; i < nr; i++) {
> > +printf("%-16s: %u\n", "Socket ID", info[i].id);
> > +printf("%-16s: %s\n", "Linear Mode",
> > +   info[i].u.mba.linear ? "Enabled" : "Disabled");
> > +printf("%-16s: %u\n", "Maximum COS", info[i].u.mba.cos_max);
> > +printf("%-16s: %u\n", "Maximum Throttling Value",
> > +   info[i].u.mba.thrtl_max);
> > +printf("%-16s: %u\n", "Default Throttling Value", 0);
> 
> If you really want to left-justify, shouldn't you choose a value that
> aligns everything nicely (strlen("Default Throttling Valu") is
> already greater than 16).
> 
Sorry for missing this.

> In fact you can do the alignment manually in the format string, and
> avoid passing the name as the first parameter.
> 
DYM a sentence like below?
  printf("%-*s: %u\n", 23, "Default Throttling Value", 0);

> > +}
> > +
> > +libxl_psr_hw_info_list_free(info, nr);
> > +return rc;
> > +}
> > +
> >  int main_psr_cat_cbm_set(int argc, char **argv)
> >  {
> >  uint32_t domid;
> > @@ -597,20 +624,24 @@ int main_psr_cat_show(int argc, char **argv)
> >  int main_psr_hwinfo(int argc, char **argv)
> >  {
> >  int opt, ret = 0;
> > -bool all = true, cmt = false, cat = false;
> > +bool all = true, cmt = false, cat = false, mba = false;
> >  static struct option opts[] = {
> 
> const?
> 
Ok, thanks!

> Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 11/15] tools: implement the new xl get hw info interface

[Roger Pau Monné]

On Thu, Aug 24, 2017 at 09:14:45AM +0800, Yi Sun wrote:
> This patch implements a new xl get HW info interface. A new argument
> is added for psr-hwinfo command to get and show MBA HW info.
> 
> Signed-off-by: Yi Sun 
> ---
> v2:
> - split out this patch from a big patch in v1.
>   (suggested by Wei Liu)
> - change 'MBA_INFO' to 'MBA'. Also, change 'mba_info' to 'mba'.
>   (suggested by Chao Peng)
> ---
>  tools/xl/xl_cmdtable.c |  1 +
>  tools/xl/xl_psr.c  | 39 +--
>  2 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
> index 2c71a9f..5ac8a7e 100644
> --- a/tools/xl/xl_cmdtable.c
> +++ b/tools/xl/xl_cmdtable.c
> @@ -524,6 +524,7 @@ struct cmd_spec cmd_table[] = {
>"[options]",
>"-m, --cmt   Show Cache Monitoring Technology (CMT) hardware 
> info\n"
>"-a, --cat   Show Cache Allocation Technology (CAT) hardware 
> info\n"
> +  "-b, --mba   Show Memory Bandwidth Allocation (MBA) hardware 
> info\n"
>  },
>  { "psr-cmt-attach",
>_psr_cmt_attach, 0, 1,
> diff --git a/tools/xl/xl_psr.c b/tools/xl/xl_psr.c
> index 7309d4f..ea37967 100644
> --- a/tools/xl/xl_psr.c
> +++ b/tools/xl/xl_psr.c
> @@ -479,6 +479,33 @@ static int psr_l2_cat_hwinfo(void)
>  return rc;
>  }
>  
> +static int psr_mba_hwinfo(void)
> +{
> +int rc;
> +unsigned int i, nr;
> +libxl_psr_hw_info *info;
> +
> +rc = libxl_psr_get_hw_info(ctx, , ,
> +   LIBXL_PSR_FEAT_TYPE_MBA, 0);
> +if (rc)
> +return rc;
> +
> +printf("Memory Bandwidth Allocation (MBA):\n");
> +
> +for (i = 0; i < nr; i++) {
> +printf("%-16s: %u\n", "Socket ID", info[i].id);
> +printf("%-16s: %s\n", "Linear Mode",
> +   info[i].u.mba.linear ? "Enabled" : "Disabled");
> +printf("%-16s: %u\n", "Maximum COS", info[i].u.mba.cos_max);
> +printf("%-16s: %u\n", "Maximum Throttling Value",
> +   info[i].u.mba.thrtl_max);
> +printf("%-16s: %u\n", "Default Throttling Value", 0);

If you really want to left-justify, shouldn't you choose a value that
aligns everything nicely (strlen("Default Throttling Valu") is
already greater than 16).

In fact you can do the alignment manually in the format string, and
avoid passing the name as the first parameter.

> +}
> +
> +libxl_psr_hw_info_list_free(info, nr);
> +return rc;
> +}
> +
>  int main_psr_cat_cbm_set(int argc, char **argv)
>  {
>  uint32_t domid;
> @@ -597,20 +624,24 @@ int main_psr_cat_show(int argc, char **argv)
>  int main_psr_hwinfo(int argc, char **argv)
>  {
>  int opt, ret = 0;
> -bool all = true, cmt = false, cat = false;
> +bool all = true, cmt = false, cat = false, mba = false;
>  static struct option opts[] = {

const?

Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 11/15] tools: implement the new xl get hw info interface

[Yi Sun]

This patch implements a new xl get HW info interface. A new argument
is added for psr-hwinfo command to get and show MBA HW info.

Signed-off-by: Yi Sun 
---
v2:
- split out this patch from a big patch in v1.
  (suggested by Wei Liu)
- change 'MBA_INFO' to 'MBA'. Also, change 'mba_info' to 'mba'.
  (suggested by Chao Peng)
---
 tools/xl/xl_cmdtable.c |  1 +
 tools/xl/xl_psr.c  | 39 +--
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 2c71a9f..5ac8a7e 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -524,6 +524,7 @@ struct cmd_spec cmd_table[] = {
   "[options]",
   "-m, --cmt   Show Cache Monitoring Technology (CMT) hardware info\n"
   "-a, --cat   Show Cache Allocation Technology (CAT) hardware info\n"
+  "-b, --mba   Show Memory Bandwidth Allocation (MBA) hardware info\n"
 },
 { "psr-cmt-attach",
   _psr_cmt_attach, 0, 1,
diff --git a/tools/xl/xl_psr.c b/tools/xl/xl_psr.c
index 7309d4f..ea37967 100644
--- a/tools/xl/xl_psr.c
+++ b/tools/xl/xl_psr.c
@@ -479,6 +479,33 @@ static int psr_l2_cat_hwinfo(void)
 return rc;
 }
 
+static int psr_mba_hwinfo(void)
+{
+int rc;
+unsigned int i, nr;
+libxl_psr_hw_info *info;
+
+rc = libxl_psr_get_hw_info(ctx, , ,
+   LIBXL_PSR_FEAT_TYPE_MBA, 0);
+if (rc)
+return rc;
+
+printf("Memory Bandwidth Allocation (MBA):\n");
+
+for (i = 0; i < nr; i++) {
+printf("%-16s: %u\n", "Socket ID", info[i].id);
+printf("%-16s: %s\n", "Linear Mode",
+   info[i].u.mba.linear ? "Enabled" : "Disabled");
+printf("%-16s: %u\n", "Maximum COS", info[i].u.mba.cos_max);
+printf("%-16s: %u\n", "Maximum Throttling Value",
+   info[i].u.mba.thrtl_max);
+printf("%-16s: %u\n", "Default Throttling Value", 0);
+}
+
+libxl_psr_hw_info_list_free(info, nr);
+return rc;
+}
+
 int main_psr_cat_cbm_set(int argc, char **argv)
 {
 uint32_t domid;
@@ -597,20 +624,24 @@ int main_psr_cat_show(int argc, char **argv)
 int main_psr_hwinfo(int argc, char **argv)
 {
 int opt, ret = 0;
-bool all = true, cmt = false, cat = false;
+bool all = true, cmt = false, cat = false, mba = false;
 static struct option opts[] = {
 {"cmt", 0, 0, 'm'},
 {"cat", 0, 0, 'a'},
+{"mba", 0, 0, 'b'},
 COMMON_LONG_OPTS
 };
 
-SWITCH_FOREACH_OPT(opt, "ma", opts, "psr-hwinfo", 0) {
+SWITCH_FOREACH_OPT(opt, "mab", opts, "psr-hwinfo", 0) {
 case 'm':
 all = false; cmt = true;
 break;
 case 'a':
 all = false; cat = true;
 break;
+case 'b':
+all = false; mba = true;
+break;
 }
 
 if (!ret && (all || cmt))
@@ -623,6 +654,10 @@ int main_psr_hwinfo(int argc, char **argv)
 if (all || cat)
 ret = psr_l2_cat_hwinfo();
 
+/* MBA is independent of CMT and CAT */
+if (all || mba)
+ret = psr_mba_hwinfo();
+
 return ret;
 }
 
-- 
1.9.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel