| Message ID | 1504522904-41135-1-git-send-email-yi.y.sun@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Sep 04, 2017 at 07:01:43PM +0800, Yi Sun wrote: > 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é <roger.pau@citrix.com> > Suggested-by: Wei Liu <wei.liu2@citrix.com> > Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> 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 > --- > - 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(-) > > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > index 229e289..6c087dd 100644 > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -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. > 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 > diff --git a/tools/xl/Makefile b/tools/xl/Makefile > index c868899..ff85d48 100644 > --- a/tools/xl/Makefile > +++ b/tools/xl/Makefile > @@ -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 I would rather add: XL_OBJS += $(XL_OBJS-y) Here rather than at the end of the first line. > > $(XL_OBJS): CFLAGS += $(CFLAGS_libxentoollog) > diff --git a/tools/xl/xl.h b/tools/xl/xl.h > index aa95b77..8d7b957 100644 > --- a/tools/xl/xl.h > +++ b/tools/xl/xl.h > @@ -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 Same here, it's going to fail to build at link time anyway AFAICT. Thanks, Roger.
On Mon, Sep 04, 2017 at 05:07:26PM +0100, Roger Pau Monné wrote: > On Mon, Sep 04, 2017 at 07:01:43PM +0800, Yi Sun wrote: > > 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é <roger.pau@citrix.com> > > Suggested-by: Wei Liu <wei.liu2@citrix.com> > > Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > > 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)
On Mon, Sep 04, 2017 at 05:07:26PM +0100, Roger Pau Monné wrote: > On Mon, Sep 04, 2017 at 07:01:43PM +0800, Yi Sun wrote: > > 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é <roger.pau@citrix.com> > > Suggested-by: Wei Liu <wei.liu2@citrix.com> > > Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > Thanks for your review. Both patches: Acked-by: Wei Liu <wei.liu2@citrix.com> > 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.
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 229e289..6c087dd 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -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 diff --git a/tools/xl/Makefile b/tools/xl/Makefile index c868899..ff85d48 100644 --- a/tools/xl/Makefile +++ b/tools/xl/Makefile @@ -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) diff --git a/tools/xl/xl.h b/tools/xl/xl.h index aa95b77..8d7b957 100644 --- a/tools/xl/xl.h +++ b/tools/xl/xl.h @@ -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 diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c index 2c71a9f..6baaed2 100644 --- a/tools/xl/xl_cmdtable.c +++ b/tools/xl/xl_cmdtable.c @@ -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, diff --git a/tools/xl/xl_psr.c b/tools/xl/xl_psr.c index 7309d4f..544f6f0 100644 --- a/tools/xl/xl_psr.c +++ b/tools/xl/xl_psr.c @@ -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
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é <roger.pau@citrix.com> Suggested-by: Wei Liu <wei.liu2@citrix.com> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> --- - 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(-)