tools: use 'i386' and '__x86_64__' to replace PSR macros

PATCH In review Yi Sun 3 participants 5 messages Sep 4, 2017 – Sep 6, 2017

Share & tools

Yi Sun

author

2017-09-04 11:19 UTC #0 lore raw

PATCH v3 1/2 tools: use '__i386__' and '__x86_64__' to replace PSR macros

The libxl interfaces and related functions are not necessary to be included by 'LIBXL_HAVE_PSR_CMT' and 'LIBXL_HAVE_PSR_CAT'. So replace them to common x86 macros. Furthermore, only compile 'xl_psr.c' under x86.

Suggested-by Wei Liu <[email protected]>

Signed-off-by Yi Sun <[email protected]>

---
- This patch is not tested to be compiled under ARM.
v3:
- replace PSR macros to '__i386__' and '__x86_64__'. (suggested by Wei Liu)
- only compile 'xl_psr.c' under x86. (suggested by Roger Pau Monné)
v2:
- remove the macros in xl.h.
(suggested by Roger Pau Monné)

---
 tools/libxl/libxl.h    | 6 +-----
 tools/xl/Makefile      | 5 +++--
 tools/xl/xl.h          | 4 +---
 tools/xl/xl_cmdtable.c | 5 +----
 tools/xl/xl_psr.c      | 5 -----
 5 files changed, 6 insertions(+), 19 deletions(-)

tools/libxl/libxl.h +1 -5

index 229e289..6c087dd 100644
@@ -2166,7 +2166,7 @@ bool libxl_ms_vm_genid_is_zero(const libxl_ms_vm_genid *id);
 void libxl_ms_vm_genid_copy(libxl_ctx *ctx, libxl_ms_vm_genid *dst,
                             const libxl_ms_vm_genid *src);
 
-#ifdef LIBXL_HAVE_PSR_CMT
+#if defined(__i386__) || defined(__x86_64__)
 int libxl_psr_cmt_attach(libxl_ctx *ctx, uint32_t domid);
 int libxl_psr_cmt_detach(libxl_ctx *ctx, uint32_t domid);
 int libxl_psr_cmt_domain_attached(libxl_ctx *ctx, uint32_t domid);

@@ -2179,9 +2179,7 @@ int libxl_psr_cmt_get_cache_occupancy(libxl_ctx *ctx,
                                       uint32_t domid,
                                       uint32_t socketid,
                                       uint32_t *l3_cache_occupancy);
-#endif
 
-#ifdef LIBXL_HAVE_PSR_MBM
 int libxl_psr_cmt_type_supported(libxl_ctx *ctx, libxl_psr_cmt_type type);
 int libxl_psr_cmt_get_sample(libxl_ctx *ctx,
                              uint32_t domid,

@@ -2189,9 +2187,7 @@ int libxl_psr_cmt_get_sample(libxl_ctx *ctx,
                              uint64_t scope,
                              uint64_t *sample_r,
                              uint64_t *tsc_r);
-#endif
 
-#ifdef LIBXL_HAVE_PSR_CAT
 /*
  * Function to set a domain's cbm. It operates on a single or multiple
  * target(s) defined in 'target_map'. The definition of 'target_map' is

tools/xl/Makefile +3 -2

index c868899..ff85d48 100644
@@ -15,11 +15,12 @@ LDFLAGS += $(PTHREAD_LDFLAGS)
 CFLAGS_XL += $(CFLAGS_libxenlight)
 CFLAGS_XL += -Wshadow
 
-XL_OBJS = xl.o xl_cmdtable.o xl_sxp.o xl_utils.o
+XL_OBJS-$(CONFIG_X86) = xl_psr.o
+XL_OBJS = xl.o xl_cmdtable.o xl_sxp.o xl_utils.o $(XL_OBJS-y)
 XL_OBJS += xl_tmem.o xl_parse.o xl_cpupool.o xl_flask.o
 XL_OBJS += xl_vtpm.o xl_block.o xl_nic.o xl_usb.o
 XL_OBJS += xl_sched.o xl_pci.o xl_vcpu.o xl_cdrom.o xl_mem.o
-XL_OBJS += xl_psr.o xl_info.o xl_console.o xl_misc.o
+XL_OBJS += xl_info.o xl_console.o xl_misc.o
 XL_OBJS += xl_vmcontrol.o xl_saverestore.o xl_migrate.o
 
 $(XL_OBJS): CFLAGS += $(CFLAGS_libxentoollog)

tools/xl/xl.h +1 -3

index aa95b77..8d7b957 100644
@@ -197,13 +197,11 @@ int main_loadpolicy(int argc, char **argv);
 int main_remus(int argc, char **argv);
 #endif
 int main_devd(int argc, char **argv);
-#ifdef LIBXL_HAVE_PSR_CMT
+#if defined(__i386__) || defined(__x86_64__)
 int main_psr_hwinfo(int argc, char **argv);
 int main_psr_cmt_attach(int argc, char **argv);
 int main_psr_cmt_detach(int argc, char **argv);
 int main_psr_cmt_show(int argc, char **argv);
-#endif
-#ifdef LIBXL_HAVE_PSR_CAT
 int main_psr_cat_cbm_set(int argc, char **argv);
 int main_psr_cat_show(int argc, char **argv);
 #endif

tools/xl/xl_cmdtable.c +1 -4

index 2c71a9f..6baaed2 100644
@@ -517,7 +517,7 @@ struct cmd_spec cmd_table[] = {
       "-F                      Run in the foreground.\n"
       "-p, --pidfile [FILE]    Write PID to pidfile when daemonizing.",
     },
-#ifdef LIBXL_HAVE_PSR_CMT
+#if defined(__i386__) || defined(__x86_64__)
     { "psr-hwinfo",
       &main_psr_hwinfo, 0, 1,
       "Show hardware information for Platform Shared Resource",

@@ -544,8 +544,6 @@ struct cmd_spec cmd_table[] = {
       "\"total-mem-bandwidth\":     Show total memory bandwidth(KB/s)\n"
       "\"local-mem-bandwidth\":     Show local memory bandwidth(KB/s)\n",
     },
-#endif
-#ifdef LIBXL_HAVE_PSR_CAT
     { "psr-cat-set",
       &main_psr_cat_cbm_set, 0, 1,
       "Set cache capacity bitmasks(CBM) for a domain",

@@ -561,7 +559,6 @@ struct cmd_spec cmd_table[] = {
       "[options] <Domain>",
       "-l <level>        Specify the cache level to process, otherwise L3 cache is processed\n"
     },
-
 #endif
     { "usbctrl-attach",
       &main_usbctrl_attach, 0, 1,

tools/xl/xl_psr.c +0 -5

index 7309d4f..544f6f0 100644
@@ -25,7 +25,6 @@
 #include "xl_utils.h"
 #include "xl_parse.h"
 
-#ifdef LIBXL_HAVE_PSR_CMT
 static int psr_cmt_hwinfo(void)
 {
     int rc;

@@ -292,9 +291,7 @@ int main_psr_cmt_show(int argc, char **argv)
 
     return ret;
 }
-#endif
 
-#if defined(LIBXL_HAVE_PSR_CAT) || defined(LIBXL_HAVE_PSR_L2_CAT)
 static int psr_l3_cat_hwinfo(void)
 {
     int rc, nr;

@@ -626,8 +623,6 @@ int main_psr_hwinfo(int argc, char **argv)
     return ret;
 }
 
-#endif
-
 /*
  * Local variables:
  * mode: C

Signature · 7 lines

1.9.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Yi Sun

author

2017-09-04 11:20 UTC #1 lore raw

PATCH v3 2/2 tools: change the type of '*nr' in 'libxl_psr_cat_get_info'

Due to historical reason, type of parameter '*nr' in 'libxl_psr_cat_get_info' is 'int'. But this is not right. It should be 'unsigned int'. This patch fixes this and does related changes.

Signed-off-by Yi Sun <[email protected]>

---
- This patch depends on patch "tools: use '__i386__' and '__x86_64__' to replace PSR".
v2:
- assign 'num' to '*nr' if return value is success. (suggested by Roger Pau Monné)

---
 tools/libxl/libxl.h     | 2 +-
 tools/libxl/libxl_psr.c | 7 +++++--
 tools/xl/xl_psr.c       | 9 ++++-----
 3 files changed, 10 insertions(+), 8 deletions(-)

tools/libxl/libxl.h +1 -1

index 6c087dd..484b5b7 100644
@@ -2211,7 +2211,7 @@ int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
  * and the length in 'nr'.
  */
 int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
-                           int *nr, unsigned int lvl);
+                           unsigned int *nr, unsigned int lvl);
 int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
                               int *nr);
 void libxl_psr_cat_info_list_free(libxl_psr_cat_info *list, int nr);

tools/libxl/libxl_psr.c +5 -2

index f55ba1e..197505a 100644
@@ -362,7 +362,7 @@ int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
 }
 
 int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
-                           int *nr, unsigned int lvl)
+                           unsigned int *nr, unsigned int lvl)
 {
     GC_INIT(ctx);
     int rc;

@@ -410,8 +410,11 @@ int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
                               int *nr)
 {
     int rc;
+    unsigned int num;
 
-    rc = libxl_psr_cat_get_info(ctx, info, nr, 3);
+    rc = libxl_psr_cat_get_info(ctx, info, &num, 3);
+    if (!rc)
+        *nr = num;
 
     return rc;
 }

tools/xl/xl_psr.c +4 -5

index 544f6f0..ef00048 100644
@@ -294,8 +294,8 @@ int main_psr_cmt_show(int argc, char **argv)
 
 static int psr_l3_cat_hwinfo(void)
 {
-    int rc, nr;
-    unsigned int i;
+    int rc;
+    unsigned int i, nr;
     uint32_t l3_cache_size;
     libxl_psr_cat_info *info;

@@ -424,7 +424,7 @@ static int psr_cat_print_socket(uint32_t domid, libxl_psr_cat_info *info,
 
 static int psr_cat_show(uint32_t domid, unsigned int lvl)
 {
-    int i, nr;
+    unsigned int i, nr;
     int rc;
     libxl_psr_cat_info *info;

@@ -453,8 +453,7 @@ out:
 static int psr_l2_cat_hwinfo(void)
 {
     int rc;
-    unsigned int i;
-    int nr;
+    unsigned int i, nr;
     libxl_psr_cat_info *info;
 
     rc = libxl_psr_cat_get_info(ctx, &info, &nr, 2);

Signature · 7 lines

1.9.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Roger Pau Monné

Reviewed-by on v3 1/2

2017-09-04 16:07 UTC #2 lore raw

In reply to Yi Sun

Quoted 7 lines · Yi Sun

The libxl interfaces and related functions are not necessary to be included by 'LIBXL_HAVE_PSR_CMT' and 'LIBXL_HAVE_PSR_CAT'. So replace them to common x86 macros. Furthermore, only compile 'xl_psr.c' under x86.
Suggested-by Roger Pau Monné <[email protected]>
Suggested-by Wei Liu <[email protected]>
Signed-off-by Yi Sun <[email protected]>

Reviewed-by Roger Pau Monné <[email protected]>

With some comments, that I would let the maintainers decide what's best to do.

To make sure this doesn't break ARM I've pushed it to a git branch on my repo:

https://travis-ci.org/royger/xen/builds/271736602

Quoted 23 lines

---
- This patch is not tested to be compiled under ARM.
v3:
- replace PSR macros to '__i386__' and '__x86_64__'. (suggested by Wei Liu)
- only compile 'xl_psr.c' under x86. (suggested by Roger Pau Monné)
v2:
- remove the macros in xl.h.
(suggested by Roger Pau Monné)
---
 tools/libxl/libxl.h    | 6 +-----
 tools/xl/Makefile      | 5 +++--
 tools/xl/xl.h          | 4 +---
 tools/xl/xl_cmdtable.c | 5 +----
 tools/xl/xl_psr.c      | 5 -----
 5 files changed, 6 insertions(+), 19 deletions(-)
tools/libxl/libxl.h +0 -0
index 229e289..6c087dd 100644

tools/libxl/libxl.h bool libxl_ms_vm_genid_is_zero(const libxl_ms_vm_genid *id); · line 2169

Quoted 5 lines

tools/libxl/libxl.h +1 -1

@@ -2166,7 +2166,7 @@ bool libxl_ms_vm_genid_is_zero(const libxl_ms_vm_genid *id);
 void libxl_ms_vm_genid_copy(libxl_ctx *ctx, libxl_ms_vm_genid *dst,
                             const libxl_ms_vm_genid *src);
 
-#ifdef LIBXL_HAVE_PSR_CMT
+#if defined(__i386__) || defined(__x86_64__)

I not sure you actually need to guard the prototypes, the build is going to fail at link time anyway.

Quoted 30 lines

tools/libxl/libxl.h +0 -4

 int libxl_psr_cmt_attach(libxl_ctx *ctx, uint32_t domid);
 int libxl_psr_cmt_detach(libxl_ctx *ctx, uint32_t domid);
 int libxl_psr_cmt_domain_attached(libxl_ctx *ctx, uint32_t domid);

@@ -2179,9 +2179,7 @@ int libxl_psr_cmt_get_cache_occupancy(libxl_ctx *ctx,
                                       uint32_t domid,
                                       uint32_t socketid,
                                       uint32_t *l3_cache_occupancy);
-#endif
 
-#ifdef LIBXL_HAVE_PSR_MBM
 int libxl_psr_cmt_type_supported(libxl_ctx *ctx, libxl_psr_cmt_type type);
 int libxl_psr_cmt_get_sample(libxl_ctx *ctx,
                              uint32_t domid,

@@ -2189,9 +2187,7 @@ int libxl_psr_cmt_get_sample(libxl_ctx *ctx,
                              uint64_t scope,
                              uint64_t *sample_r,
                              uint64_t *tsc_r);
-#endif
 
-#ifdef LIBXL_HAVE_PSR_CAT
 /*
  * Function to set a domain's cbm. It operates on a single or multiple
  * target(s) defined in 'target_map'. The definition of 'target_map' is

tools/xl/Makefile +0 -0

index c868899..ff85d48 100644
@@ -15,11 +15,12 @@ LDFLAGS += $(PTHREAD_LDFLAGS)
 CFLAGS_XL += $(CFLAGS_libxenlight)
 CFLAGS_XL += -Wshadow

tools/xl/Makefile LDFLAGS += $(PTHREAD_LDFLAGS) · line 24

Quoted 10 lines

tools/xl/Makefile +3 -2

 
-XL_OBJS = xl.o xl_cmdtable.o xl_sxp.o xl_utils.o
+XL_OBJS-$(CONFIG_X86) = xl_psr.o
+XL_OBJS = xl.o xl_cmdtable.o xl_sxp.o xl_utils.o $(XL_OBJS-y)
 XL_OBJS += xl_tmem.o xl_parse.o xl_cpupool.o xl_flask.o
 XL_OBJS += xl_vtpm.o xl_block.o xl_nic.o xl_usb.o
 XL_OBJS += xl_sched.o xl_pci.o xl_vcpu.o xl_cdrom.o xl_mem.o
-XL_OBJS += xl_psr.o xl_info.o xl_console.o xl_misc.o
+XL_OBJS += xl_info.o xl_console.o xl_misc.o
 XL_OBJS += xl_vmcontrol.o xl_saverestore.o xl_migrate.o

I would rather add:

XL_OBJS += $(XL_OBJS-y)

Here rather than at the end of the first line.

Quoted 11 lines

tools/xl/Makefile +0 -0

 $(XL_OBJS): CFLAGS += $(CFLAGS_libxentoollog)

tools/xl/xl.h +0 -1

index aa95b77..8d7b957 100644
@@ -197,13 +197,11 @@ int main_loadpolicy(int argc, char **argv);
 int main_remus(int argc, char **argv);
 #endif
 int main_devd(int argc, char **argv);
-#ifdef LIBXL_HAVE_PSR_CMT

tools/xl/xl.h int main_loadpolicy(int argc, char **argv); · line 207

Quoted 10 lines

tools/xl/xl.h +1 -2

+#if defined(__i386__) || defined(__x86_64__)
 int main_psr_hwinfo(int argc, char **argv);
 int main_psr_cmt_attach(int argc, char **argv);
 int main_psr_cmt_detach(int argc, char **argv);
 int main_psr_cmt_show(int argc, char **argv);
-#endif
-#ifdef LIBXL_HAVE_PSR_CAT
 int main_psr_cat_cbm_set(int argc, char **argv);
 int main_psr_cat_show(int argc, char **argv);
 #endif

Same here, it's going to fail to build at link time anyway AFAICT.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Wei Liu

2017-09-04 16:13 UTC #3 lore raw

Quoted 11 lines · Roger Pau Monné

Quoted 7 lines · Yi Sun
The libxl interfaces and related functions are not necessary to be included by 'LIBXL_HAVE_PSR_CMT' and 'LIBXL_HAVE_PSR_CAT'. So replace them to common x86 macros. Furthermore, only compile 'xl_psr.c' under x86.
Suggested-by Roger Pau Monné <[email protected]>
Suggested-by Wei Liu <[email protected]>
Signed-off-by Yi Sun <[email protected]>
Reviewed-by Roger Pau Monné <[email protected]>

Quoted 7 lines

With some comments, that I would let the maintainers decide what's best to do.
To make sure this doesn't break ARM I've pushed it to a git branch on my repo:
https://travis-ci.org/royger/xen/builds/271736602

FWIW travis at the moment doesn't build tools on arm.

(will review the patch tomorrow)

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel

Wei Liu

Acked-by

2017-09-06 11:42 UTC #4 lore raw

In reply to Roger Pau Monné

Quoted 11 lines · Roger Pau Monné

Quoted 7 lines · Yi Sun
The libxl interfaces and related functions are not necessary to be included by 'LIBXL_HAVE_PSR_CMT' and 'LIBXL_HAVE_PSR_CAT'. So replace them to common x86 macros. Furthermore, only compile 'xl_psr.c' under x86.
Suggested-by Roger Pau Monné <[email protected]>
Suggested-by Wei Liu <[email protected]>
Signed-off-by Yi Sun <[email protected]>
Reviewed-by Roger Pau Monné <[email protected]>

Thanks for your review.

Both patches:

Acked-by Wei Liu <[email protected]>

Quoted 3 lines

With some comments, that I would let the maintainers decide what's best to do.

I think you comments make sense but probably not worthy of sending another round because of that. I believe Yi's time, and ours, is better spent working on things that are more important. :-)

I will commit this series shortly.

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel