Hi, Thanks for your patch. I have a few questions below.
On Wed, Jul 8, 2015 at 2:00 AM, Deepthi Dharwar <deep...@linux.vnet.ibm.com> wrote: > > Enable collection of Nest PMUs on POWERPC platform > > From: Deepthi Dharwar <deep...@linux.vnet.ibm.com> > > Currently, POWER8 platforms supports reading of Nest/off-core > PMUs like memory read/write counters. > > Required kernel framework to enable collection of these > nest PMUs is currently being discussed in the > community for mainline inclusion. > Link to the latest patch series: https://lkml.org/lkml/2015/6/25/170 > > Following patch enables collection of nest read and > write memory PMUs for any user using this library. > > Signed-off-by: Deepthi Dharwar <deep...@linux.vnet.ibm.com> > Signed-off-by: Hemant Kumar <hem...@linux.vnet.ibm.com> > --- > include/perfmon/pfmlib.h | 3 ++ > lib/Makefile | 5 ++- > lib/events/powerpc_nest_events.h | 62 > ++++++++++++++++++++++++++++++++++++++ > lib/pfmlib_common.c | 2 + > lib/pfmlib_power_priv.h | 1 + > lib/pfmlib_powerpc_nest.c | 60 +++++++++++++++++++++++++++++++++++++ > lib/pfmlib_powerpc_perf_event.c | 55 ++++++++++++++++++++++++++++++++++ > lib/pfmlib_priv.h | 2 + > 8 files changed, 188 insertions(+), 2 deletions(-) > create mode 100644 lib/events/powerpc_nest_events.h > create mode 100644 lib/pfmlib_powerpc_nest.c > > diff --git a/include/perfmon/pfmlib.h b/include/perfmon/pfmlib.h > index c5aef21..2fb4744 100644 > --- a/include/perfmon/pfmlib.h > +++ b/include/perfmon/pfmlib.h > @@ -289,6 +289,9 @@ typedef enum { > PFM_PMU_INTEL_HSWEP_UNC_SB2, /* Intel Haswell-EP S-Box 2 uncore */ > PFM_PMU_INTEL_HSWEP_UNC_SB3, /* Intel Haswell-EP S-Box 3 uncore */ > > + PFM_PMU_POWERPC_NEST_MCS_READ_BW, /* POWERPC Nest Memory Read > bandwidth */ > + PFM_PMU_POWERPC_NEST_MCS_WRITE_BW, /* POWERPC Nest Memory Write > bandwidth */ > + Why do you need to create two PMUs to support the read vs. write memory bandwidth? Are they implemented by two distinct PMUs on the HW? If not, then they should appear under the same PMU with different event names,. libpfm4 would take care of the encoding for perf_events. Or is this coming from the kernel interface for those events? > /* MUST ADD NEW PMU MODELS HERE */ > > PFM_PMU_MAX /* end marker */ > diff --git a/lib/Makefile b/lib/Makefile > index 162b10e..1cc8765 100644 > --- a/lib/Makefile > +++ b/lib/Makefile > @@ -117,7 +117,7 @@ SRCS += pfmlib_powerpc_perf_event.c > endif > > INCARCH = $(INC_POWERPC) > -SRCS += pfmlib_powerpc.c pfmlib_power4.c pfmlib_ppc970.c pfmlib_power5.c > pfmlib_power6.c pfmlib_power7.c pfmlib_torrent.c pfmlib_power8.c > +SRCS += pfmlib_powerpc.c pfmlib_power4.c pfmlib_ppc970.c pfmlib_power5.c > pfmlib_power6.c pfmlib_power7.c pfmlib_torrent.c pfmlib_power8.c > pfmlib_powerpc_nest.c > CFLAGS += -DCONFIG_PFMLIB_ARCH_POWERPC > endif > > @@ -282,7 +282,8 @@ INC_POWERPC=events/ppc970_events.h \ > events/power6_events.h \ > events/power7_events.h \ > events/power8_events.h \ > - events/torrent_events.h > + events/torrent_events.h \ > + events/powerpc_nest_events.h > > INC_S390X=pfmlib_s390x_priv.h \ > events/s390x_cpumf_events.h > diff --git a/lib/events/powerpc_nest_events.h > b/lib/events/powerpc_nest_events.h > new file mode 100644 > index 0000000..2c8f155 > --- /dev/null > +++ b/lib/events/powerpc_nest_events.h > @@ -0,0 +1,62 @@ > +#ifndef __POWERPC_NEST_EVENTS_H__ > +#define __POWERPC_NEST_EVENTS_H__ > + > +#define POWERPC_PME_NEST_MCS_00 0 > +#define POWERPC_PME_NEST_MCS_01 1 > +#define POWERPC_PME_NEST_MCS_02 2 > +#define POWERPC_PME_NEST_MCS_03 3 > + > +static const pme_power_entry_t powerpc_nest_read_pe[] = { > + [ POWERPC_PME_NEST_MCS_00 ] = { > + .pme_name = "MCS_00", > + .pme_code = 0x118, > + .pme_short_desc = "Total Read Bandwidth seen on both MCS of MC0", > + .pme_long_desc = "Total Read Bandwidth seen on both MCS of MC0", > + }, > + [ POWERPC_PME_NEST_MCS_01 ] = { > + .pme_name = "MCS_01", > + .pme_code = 0x120, > + .pme_short_desc = "Total Read Bandwidth seen on both MCS of MC1", > + .pme_long_desc = "Total Read Bandwidth seen on both MCS of MC1", > + }, > + [ POWERPC_PME_NEST_MCS_02 ] = { > + .pme_name = "MCS_02", > + .pme_code = 0x128, > + .pme_short_desc = "Total Read Bandwidth seen on both MCS of MC2", > + .pme_long_desc = "Total Read Bandwidth seen on both MCS of MC2", > + }, > + [ POWERPC_PME_NEST_MCS_03 ] = { > + .pme_name = "MCS_03", > + .pme_code = 0x130, > + .pme_short_desc = "Total Read Bandwidth seen on both MCS of MC3", > + .pme_long_desc = "Total Read Bandwidth seen on both MCS of MC3", > + }, > +}; > + > +static const pme_power_entry_t powerpc_nest_write_pe[] = { > + [ POWERPC_PME_NEST_MCS_00 ] = { > + .pme_name = "MCS_00", > + .pme_code = 0x198, > + .pme_short_desc = "Total Write Bandwidth seen on both MCS of MC0", > + .pme_long_desc = "Total Write Bandwidth seen on both MCS of MC0", > + }, > + [ POWERPC_PME_NEST_MCS_01 ] = { > + .pme_name = "MCS_01", > + .pme_code = 0x1a0, > + .pme_short_desc = "Total Write Bandwidth seen on both MCS of MC1", > + .pme_long_desc = "Total Write Bandwidth seen on both MCS of MC1", > + }, > + [ POWERPC_PME_NEST_MCS_02 ] = { > + .pme_name = "MCS_02", > + .pme_code = 0x1a8, > + .pme_short_desc = "Total Write Bandwidth seen on both MCS of MC2", > + .pme_long_desc = "Total Write Bandwidth seen on both MCS of MC2", > + }, > + [ POWERPC_PME_NEST_MCS_03 ] = { > + .pme_name = "MCS_03", > + .pme_code = 0x1b0, > + .pme_short_desc = "Total Write Bandwidth seen on both MCS of MC3", > + .pme_long_desc = "Total Write Bandwidth seen on both MCS of MC3", > + }, > +}; > +#endif > diff --git a/lib/pfmlib_common.c b/lib/pfmlib_common.c > index 1bd9097..a6a364e 100644 > --- a/lib/pfmlib_common.c > +++ b/lib/pfmlib_common.c > @@ -220,6 +220,8 @@ static pfmlib_pmu_t *pfmlib_pmus[]= > &power7_support, > &power8_support, > &torrent_support, > + &powerpc_nest_mcs_read_support, > + &powerpc_nest_mcs_write_support, > #endif > > #ifdef CONFIG_PFMLIB_ARCH_SPARC > diff --git a/lib/pfmlib_power_priv.h b/lib/pfmlib_power_priv.h > index ce44d52..0d8b473 100644 > --- a/lib/pfmlib_power_priv.h > +++ b/lib/pfmlib_power_priv.h > @@ -105,6 +105,7 @@ extern int pfm_gen_powerpc_validate_table(void *this, > FILE *fp); > extern void pfm_gen_powerpc_perf_validate_pattrs(void *this, > pfmlib_event_desc_t *e); > > extern int pfm_gen_powerpc_get_perf_encoding(void *this, pfmlib_event_desc_t > *e); > +extern int pfm_gen_powerpc_get_nest_perf_encoding(void *this, > pfmlib_event_desc_t *e); > #endif /* _POWER_REG_H */ > #endif > > diff --git a/lib/pfmlib_powerpc_nest.c b/lib/pfmlib_powerpc_nest.c > new file mode 100644 > index 0000000..8a8bf5b > --- /dev/null > +++ b/lib/pfmlib_powerpc_nest.c > @@ -0,0 +1,60 @@ > +/* > + * pfmlib_powerpc_nest.c > + */ > + > +#include "pfmlib_priv.h" > +#include "pfmlib_power_priv.h" > +#include "events/powerpc_nest_events.h" > + > +static int pfm_powerpc_nest_detect(void* this) > +{ > + if (__is_processor(PV_POWER8)) > + return PFM_SUCCESS; > + return PFM_ERR_NOTSUPP; > +} > + > +pfmlib_pmu_t powerpc_nest_mcs_read_support={ > + .desc = "POWERPC_NEST_MCS_RD_BW", > + .name = "powerpc_nest_mcs_read", > + .pmu = PFM_PMU_POWERPC_NEST_MCS_READ_BW, > + .perf_name = "Nest_MCS_Read_BW", > + .pme_count = LIBPFM_ARRAY_SIZE(powerpc_nest_read_pe), > + .type = PFM_PMU_TYPE_UNCORE, > + .num_cntrs = 4, > + .num_fixed_cntrs = 0, > + .max_encoding = 1, > + .pe = powerpc_nest_read_pe, > + .pmu_detect = pfm_powerpc_nest_detect, > + .get_event_encoding[PFM_OS_NONE] = pfm_gen_powerpc_get_encoding, > + PFMLIB_ENCODE_PERF(pfm_gen_powerpc_get_nest_perf_encoding), > + PFMLIB_VALID_PERF_PATTRS(pfm_gen_powerpc_perf_validate_pattrs), > + .get_event_first = pfm_gen_powerpc_get_event_first, > + .get_event_next = pfm_gen_powerpc_get_event_next, > + .event_is_valid = pfm_gen_powerpc_event_is_valid, > + .validate_table = pfm_gen_powerpc_validate_table, > + .get_event_info = pfm_gen_powerpc_get_event_info, > + .get_event_attr_info = pfm_gen_powerpc_get_event_attr_info, > +}; > + > +pfmlib_pmu_t powerpc_nest_mcs_write_support={ > + .desc = "POWERPC_NEST_MCS_WR_BW", > + .name = "powerpc_nest_mcs_write", > + .pmu = PFM_PMU_POWERPC_NEST_MCS_WRITE_BW, > + .perf_name = "Nest_MCS_Write_BW", > + .pme_count = LIBPFM_ARRAY_SIZE(powerpc_nest_write_pe), > + .type = PFM_PMU_TYPE_UNCORE, > + .num_cntrs = 4, > + .num_fixed_cntrs = 0, > + .max_encoding = 1, > + .pe = powerpc_nest_write_pe, > + .pmu_detect = pfm_powerpc_nest_detect, > + .get_event_encoding[PFM_OS_NONE] = pfm_gen_powerpc_get_encoding, > + PFMLIB_ENCODE_PERF(pfm_gen_powerpc_get_nest_perf_encoding), > + PFMLIB_VALID_PERF_PATTRS(pfm_gen_powerpc_perf_validate_pattrs), > + .get_event_first = pfm_gen_powerpc_get_event_first, > + .get_event_next = pfm_gen_powerpc_get_event_next, > + .event_is_valid = pfm_gen_powerpc_event_is_valid, > + .validate_table = pfm_gen_powerpc_validate_table, > + .get_event_info = pfm_gen_powerpc_get_event_info, > + .get_event_attr_info = pfm_gen_powerpc_get_event_attr_info, > +}; > diff --git a/lib/pfmlib_powerpc_perf_event.c b/lib/pfmlib_powerpc_perf_event.c > index ec371c9..642439b 100644 > --- a/lib/pfmlib_powerpc_perf_event.c > +++ b/lib/pfmlib_powerpc_perf_event.c > @@ -24,6 +24,7 @@ > #include <sys/types.h> > #include <string.h> > #include <stdlib.h> > +#include <limits.h> > > /* private headers */ > #include "pfmlib_priv.h" /* library private */ > @@ -53,6 +54,60 @@ pfm_gen_powerpc_get_perf_encoding(void *this, > pfmlib_event_desc_t *e) > return PFM_SUCCESS; > } > > +static int > +find_pmu_type_by_name(const char *name) > +{ > + char filename[PATH_MAX]; > + FILE *fp; > + int ret, type; > + > + if (!name) > + return PFM_ERR_NOTSUPP; > + > + sprintf(filename, "/sys/bus/event_source/devices/%s/type", name); > + > + fp = fopen(filename, "r"); > + if (!fp) > + return PFM_ERR_NOTSUPP; > + > + ret = fscanf(fp, "%d", &type); > + if (ret != 1) > + type = PFM_ERR_NOTSUPP; > + > + fclose(fp); > + > + return type; > +} > + > + > +int > +pfm_gen_powerpc_get_nest_perf_encoding(void *this, pfmlib_event_desc_t *e) > +{ > + pfmlib_pmu_t *pmu = this; > + struct perf_event_attr *attr = e->os_data; > + int ret; > + > + if (!pmu->get_event_encoding[PFM_OS_NONE]) > + return PFM_ERR_NOTSUPP; > + > + /* > + * encoding routine changes based on PMU model > + */ > + ret = pmu->get_event_encoding[PFM_OS_NONE](this, e); > + if (ret != PFM_SUCCESS) > + return ret; > + > + ret = find_pmu_type_by_name(pmu->perf_name); > + if (ret < 0) > + return ret; > + > + attr->type = ret; > + attr->config = e->codes[0]; > + > + return PFM_SUCCESS; > +} > + > + > void > pfm_gen_powerpc_perf_validate_pattrs(void *this, pfmlib_event_desc_t *e) > { > diff --git a/lib/pfmlib_priv.h b/lib/pfmlib_priv.h > index 41ccf7b..b5e2d0c 100644 > --- a/lib/pfmlib_priv.h > +++ b/lib/pfmlib_priv.h > @@ -359,6 +359,8 @@ extern pfmlib_pmu_t power6_support; > extern pfmlib_pmu_t power7_support; > extern pfmlib_pmu_t power8_support; > extern pfmlib_pmu_t torrent_support; > +extern pfmlib_pmu_t powerpc_nest_mcs_read_support; > +extern pfmlib_pmu_t powerpc_nest_mcs_write_support; > extern pfmlib_pmu_t sparc_support; > extern pfmlib_pmu_t sparc_ultra12_support; > extern pfmlib_pmu_t sparc_ultra3_support; > > Regards, > Deepthi > > > ------------------------------------------------------------------------------ > Don't Limit Your Business. Reach for the Cloud. > GigeNET's Cloud Solutions provide you with the tools and support that > you need to offload your IT needs and focus on growing your business. > Configured For All Businesses. Start Your Cloud Today. > https://www.gigenetcloud.com/ > _______________________________________________ > perfmon2-devel mailing list > perfmon2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/perfmon2-devel ------------------------------------------------------------------------------ Don't Limit Your Business. Reach for the Cloud. GigeNET's Cloud Solutions provide you with the tools and support that you need to offload your IT needs and focus on growing your business. Configured For All Businesses. Start Your Cloud Today. https://www.gigenetcloud.com/ _______________________________________________ perfmon2-devel mailing list perfmon2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
