This set adds the capability to communiate event specific configuration
to the PMU kernel driver using an ioctl(). The functionatlity is made
generic enough for anyone to use but is targeted at the identification
of CoreSight sinks when operating in CPU-wide trace scenarios.
Other than the changes highlighted below I have moved things around,
deciding to start with the kernel portion rather than user space -
hopefully this will be easier to review.
Applies cleanly on v4.18-rc3.
Thanks,
Mathieu
---
Changes for V2:
. Fixed s390 problem reported by buildbot.
. Removed uneeded check in perf_event_process_drv_config() (Jiri)
. Reordered data copy in perf_event_set_drv_config() (Jiri)
. Went from 2 to 1 step driver configuration process (Alex)
. Moved structure name "perf_drv_config" to "pmu_drv_config".
V1: https://lkml.org/lkml/2018/7/2/1008
Mathieu Poirier (7):
perf: Introduce ioctl to communicate driver configuration to kernel
perf/core: Use ioctl to communicate driver configuration to kernel
perf/aux: Make perf_event accessible to setup_aux()
coresight: Use PMU driver configuration for sink selection
perf tools: Use ioctl to communicate driver configuration to kernel
perf tools: Make perf_evsel accessible to PMU driver configuration
code
perf tools: Use ioctl function to send sink configuration to kernel
arch/s390/kernel/perf_cpum_sf.c | 6 +-
arch/x86/events/intel/bts.c | 4 +-
arch/x86/events/intel/pt.c | 5 +-
drivers/hwtracing/coresight/coresight-etm-perf.c | 136 ++++++++++++++++++++---
drivers/hwtracing/coresight/coresight-etm-perf.h | 4 +
drivers/perf/arm_spe_pmu.c | 6 +-
include/linux/perf_event.h | 47 +++++++-
include/uapi/linux/perf_event.h | 1 +
kernel/events/core.c | 77 +++++++++++++
kernel/events/ring_buffer.c | 2 +-
tools/include/uapi/linux/perf_event.h | 1 +
tools/perf/arch/arm/util/cs-etm.c | 60 +++-------
tools/perf/arch/arm/util/cs-etm.h | 3 +-
tools/perf/util/drv_configs.c | 30 ++---
tools/perf/util/evsel.c | 7 ++
tools/perf/util/evsel.h | 1 +
tools/perf/util/pmu.h | 3 +-
17 files changed, 300 insertions(+), 93 deletions(-)
--
2.7.4
Using sysFS to communicate sink information for a trace session doesn't
work when more than one CPU is involved in the scenario. As such
communicate the sink information to each event by using the SET_DRV_CONFIG
ioctl command.
Signed-off-by: Mathieu Poirier <[email protected]>
---
tools/perf/arch/arm/util/cs-etm.c | 54 ++++-----------------------------------
1 file changed, 5 insertions(+), 49 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index d8081c2e6d44..cb0978eb7181 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -599,55 +599,10 @@ struct auxtrace_record *cs_etm_record_init(int *err)
return NULL;
}
-static FILE *cs_device__open_file(const char *name)
+static int cs_etm_set_drv_config_term(struct perf_evsel *evsel,
+ const char *term)
{
- struct stat st;
- char path[PATH_MAX];
- const char *sysfs;
-
- sysfs = sysfs__mountpoint();
- if (!sysfs)
- return NULL;
-
- snprintf(path, PATH_MAX,
- "%s" CS_BUS_DEVICE_PATH "%s", sysfs, name);
-
- if (stat(path, &st) < 0)
- return NULL;
-
- return fopen(path, "w");
-
-}
-
-static int __printf(2, 3) cs_device__print_file(const char *name, const char *fmt, ...)
-{
- va_list args;
- FILE *file;
- int ret = -EINVAL;
-
- va_start(args, fmt);
- file = cs_device__open_file(name);
- if (file) {
- ret = vfprintf(file, fmt, args);
- fclose(file);
- }
- va_end(args);
- return ret;
-}
-
-static int cs_etm_set_drv_config_term(struct perf_evsel_config_term *term)
-{
- int ret;
- char enable_sink[ENABLE_SINK_MAX];
-
- snprintf(enable_sink, ENABLE_SINK_MAX, "%s/%s",
- term->val.drv_cfg, "enable_sink");
-
- ret = cs_device__print_file(enable_sink, "%d", 1);
- if (ret < 0)
- return ret;
-
- return 0;
+ return perf_evsel__apply_drv_config(evsel, term);
}
int cs_etm_set_drv_config(struct perf_evsel *evsel,
@@ -660,7 +615,8 @@ int cs_etm_set_drv_config(struct perf_evsel *evsel,
if (term->type != PERF_EVSEL__CONFIG_TERM_DRV_CFG)
continue;
- err = cs_etm_set_drv_config_term(term);
+ err = cs_etm_set_drv_config_term(evsel,
+ term->val.drv_cfg);
if (err) {
*err_term = term;
break;
--
2.7.4
Following in the footsteps of what was done for filters, adding the
necessary mechanic needed to push down driver specific configuration
to the kernel using an ioctl. By proceeding this way PMU specific
configuration such as CoreSight sink specification can be communicated
to each event.
Signed-off-by: Mathieu Poirier <[email protected]>
---
tools/perf/util/evsel.c | 7 +++++++
tools/perf/util/evsel.h | 1 +
2 files changed, 8 insertions(+)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 94fce4f537e9..534aca4c642c 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1162,6 +1162,13 @@ static int perf_evsel__append_filter(struct perf_evsel *evsel,
return -1;
}
+int perf_evsel__apply_drv_config(struct perf_evsel *evsel, const char *config)
+{
+ return perf_evsel__run_ioctl(evsel,
+ PERF_EVENT_IOC_SET_DRV_CONFIG,
+ (void *)config);
+}
+
int perf_evsel__append_tp_filter(struct perf_evsel *evsel, const char *filter)
{
return perf_evsel__append_filter(evsel, "(%s) && (%s)", filter);
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index d277930b19a1..0f671bd2a988 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -271,6 +271,7 @@ int perf_evsel__append_tp_filter(struct perf_evsel *evsel, const char *filter);
int perf_evsel__append_addr_filter(struct perf_evsel *evsel,
const char *filter);
int perf_evsel__apply_filter(struct perf_evsel *evsel, const char *filter);
+int perf_evsel__apply_drv_config(struct perf_evsel *evsel, const char *config);
int perf_evsel__enable(struct perf_evsel *evsel);
int perf_evsel__disable(struct perf_evsel *evsel);
--
2.7.4
Make structure perf_evsel available to the PMU driver configuration code.
That way function perf_evsel__apply_drv_config() can be used from within
that code and information pertaining to the 'perf_evsel_config_term' is
still available.
Signed-off-by: Mathieu Poirier <[email protected]>
---
tools/perf/arch/arm/util/cs-etm.c | 22 +++++++++++++++++++++-
tools/perf/arch/arm/util/cs-etm.h | 3 ++-
tools/perf/util/drv_configs.c | 30 +++++++-----------------------
tools/perf/util/pmu.h | 3 ++-
4 files changed, 32 insertions(+), 26 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index 2f595cd73da6..d8081c2e6d44 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -635,7 +635,7 @@ static int __printf(2, 3) cs_device__print_file(const char *name, const char *fm
return ret;
}
-int cs_etm_set_drv_config(struct perf_evsel_config_term *term)
+static int cs_etm_set_drv_config_term(struct perf_evsel_config_term *term)
{
int ret;
char enable_sink[ENABLE_SINK_MAX];
@@ -649,3 +649,23 @@ int cs_etm_set_drv_config(struct perf_evsel_config_term *term)
return 0;
}
+
+int cs_etm_set_drv_config(struct perf_evsel *evsel,
+ struct perf_evsel_config_term **err_term)
+{
+ int err = 0;
+ struct perf_evsel_config_term *term;
+
+ list_for_each_entry(term, &evsel->config_terms, list) {
+ if (term->type != PERF_EVSEL__CONFIG_TERM_DRV_CFG)
+ continue;
+
+ err = cs_etm_set_drv_config_term(term);
+ if (err) {
+ *err_term = term;
+ break;
+ }
+ }
+
+ return err;
+}
diff --git a/tools/perf/arch/arm/util/cs-etm.h b/tools/perf/arch/arm/util/cs-etm.h
index 1a12e64f5127..a3f8dde6ccef 100644
--- a/tools/perf/arch/arm/util/cs-etm.h
+++ b/tools/perf/arch/arm/util/cs-etm.h
@@ -10,6 +10,7 @@
#include "../../util/evsel.h"
struct auxtrace_record *cs_etm_record_init(int *err);
-int cs_etm_set_drv_config(struct perf_evsel_config_term *term);
+int cs_etm_set_drv_config(struct perf_evsel *evsel,
+ struct perf_evsel_config_term **err_term);
#endif
diff --git a/tools/perf/util/drv_configs.c b/tools/perf/util/drv_configs.c
index eec754243f4d..f7c1bcf08549 100644
--- a/tools/perf/util/drv_configs.c
+++ b/tools/perf/util/drv_configs.c
@@ -25,7 +25,6 @@ perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
{
bool found = false;
int err = 0;
- struct perf_evsel_config_term *term;
struct perf_pmu *pmu = NULL;
while ((pmu = perf_pmu__scan(pmu)) != NULL)
@@ -34,29 +33,14 @@ perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
break;
}
- list_for_each_entry(term, &evsel->config_terms, list) {
- if (term->type != PERF_EVSEL__CONFIG_TERM_DRV_CFG)
- continue;
+ /*
+ * No need to continue if we didn't get a match or if there is no
+ * driver configuration function for this PMU.
+ */
+ if (!found || !pmu->set_drv_config)
+ return err;
- /*
- * We have a configuration term, report an error if we
- * can't find the PMU or if the PMU driver doesn't support
- * cmd line driver configuration.
- */
- if (!found || !pmu->set_drv_config) {
- err = -EINVAL;
- *err_term = term;
- break;
- }
-
- err = pmu->set_drv_config(term);
- if (err) {
- *err_term = term;
- break;
- }
- }
-
- return err;
+ return pmu->set_drv_config(evsel, err_term);
}
int perf_evlist__apply_drv_configs(struct perf_evlist *evlist,
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 76fecec7b3f9..47f44394042b 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -29,7 +29,8 @@ struct perf_pmu {
struct list_head format; /* HEAD struct perf_pmu_format -> list */
struct list_head aliases; /* HEAD struct perf_pmu_alias -> list */
struct list_head list; /* ELEM */
- int (*set_drv_config) (struct perf_evsel_config_term *term);
+ int (*set_drv_config) (struct perf_evsel *evsel,
+ struct perf_evsel_config_term **err_term);
};
struct perf_pmu_info {
--
2.7.4
When pmu::setup_aux() is called the coresight PMU needs to know which
sink to use for the session by looking up the information in the
drv_config field of the hw_perf_event structure.
As such simply replace the cpu information by the complete perf_event
structure and change all affected customers.
Signed-off-by: Mathieu Poirier <[email protected]>
---
arch/s390/kernel/perf_cpum_sf.c | 6 +++---
arch/x86/events/intel/bts.c | 4 +++-
arch/x86/events/intel/pt.c | 5 +++--
drivers/hwtracing/coresight/coresight-etm-perf.c | 6 +++---
drivers/perf/arm_spe_pmu.c | 6 +++---
include/linux/perf_event.h | 2 +-
kernel/events/ring_buffer.c | 2 +-
7 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/arch/s390/kernel/perf_cpum_sf.c b/arch/s390/kernel/perf_cpum_sf.c
index 0292d68e7dde..2d55fc2bb185 100644
--- a/arch/s390/kernel/perf_cpum_sf.c
+++ b/arch/s390/kernel/perf_cpum_sf.c
@@ -1589,7 +1589,7 @@ static void aux_buffer_free(void *data)
/*
* aux_buffer_setup() - Setup AUX buffer for diagnostic mode sampling
- * @cpu: On which to allocate, -1 means current
+ * @event: Event the buffer is setup for, event->cpu == -1 means current
* @pages: Array of pointers to buffer pages passed from perf core
* @nr_pages: Total pages
* @snapshot: Flag for snapshot mode
@@ -1601,8 +1601,8 @@ static void aux_buffer_free(void *data)
*
* Return the private AUX buffer structure if success or NULL if fails.
*/
-static void *aux_buffer_setup(int cpu, void **pages, int nr_pages,
- bool snapshot)
+static void *aux_buffer_setup(struct perf_event *event, void **pages,
+ int nr_pages, bool snapshot)
{
struct sf_buffer *sfb;
struct aux_buffer *aux;
diff --git a/arch/x86/events/intel/bts.c b/arch/x86/events/intel/bts.c
index 24ffa1e88cf9..7139f6bf27ad 100644
--- a/arch/x86/events/intel/bts.c
+++ b/arch/x86/events/intel/bts.c
@@ -77,10 +77,12 @@ static size_t buf_size(struct page *page)
}
static void *
-bts_buffer_setup_aux(int cpu, void **pages, int nr_pages, bool overwrite)
+bts_buffer_setup_aux(struct perf_event *event, void **pages,
+ int nr_pages, bool overwrite)
{
struct bts_buffer *buf;
struct page *page;
+ int cpu = event->cpu;
int node = (cpu == -1) ? cpu : cpu_to_node(cpu);
unsigned long offset;
size_t size = nr_pages << PAGE_SHIFT;
diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index 8d016ce5b80d..8f4c98fdd03c 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -1104,10 +1104,11 @@ static int pt_buffer_init_topa(struct pt_buffer *buf, unsigned long nr_pages,
* Return: Our private PT buffer structure.
*/
static void *
-pt_buffer_setup_aux(int cpu, void **pages, int nr_pages, bool snapshot)
+pt_buffer_setup_aux(struct perf_event *event, void **pages,
+ int nr_pages, bool snapshot)
{
struct pt_buffer *buf;
- int node, ret;
+ int node, ret, cpu = event->cpu;
if (!nr_pages)
return NULL;
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 677695635211..0f5e03e4df22 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -181,15 +181,15 @@ static void etm_free_aux(void *data)
schedule_work(&event_data->work);
}
-static void *etm_setup_aux(int event_cpu, void **pages,
+static void *etm_setup_aux(struct perf_event *event, void **pages,
int nr_pages, bool overwrite)
{
- int cpu;
+ int cpu = event->cpu;
cpumask_t *mask;
struct coresight_device *sink;
struct etm_event_data *event_data = NULL;
- event_data = alloc_event_data(event_cpu);
+ event_data = alloc_event_data(cpu);
if (!event_data)
return NULL;
INIT_WORK(&event_data->work, free_event_data);
diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
index 54ec278d2fc4..4dcd7bf14dcc 100644
--- a/drivers/perf/arm_spe_pmu.c
+++ b/drivers/perf/arm_spe_pmu.c
@@ -824,10 +824,10 @@ static void arm_spe_pmu_read(struct perf_event *event)
{
}
-static void *arm_spe_pmu_setup_aux(int cpu, void **pages, int nr_pages,
- bool snapshot)
+static void *arm_spe_pmu_setup_aux(struct perf_event *event, void **pages,
+ int nr_pages, bool snapshot)
{
- int i;
+ int i, cpu = event->cpu;
struct page **pglist;
struct arm_spe_pmu_buf *buf;
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 7064b513ca2b..d67074f81c23 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -420,7 +420,7 @@ struct pmu {
/*
* Set up pmu-private data structures for an AUX area
*/
- void *(*setup_aux) (int cpu, void **pages,
+ void *(*setup_aux) (struct perf_event *event, void **pages,
int nr_pages, bool overwrite);
/* optional */
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 5d3cf407e374..4c96c7575224 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -648,7 +648,7 @@ int rb_alloc_aux(struct ring_buffer *rb, struct perf_event *event,
goto out;
}
- rb->aux_priv = event->pmu->setup_aux(event->cpu, rb->aux_pages, nr_pages,
+ rb->aux_priv = event->pmu->setup_aux(event, rb->aux_pages, nr_pages,
overwrite);
if (!rb->aux_priv)
goto out;
--
2.7.4
This patch uses the PMU driver configuration held in event::hw::drv_config
to select a sink for each event that is created (the old sysFS way of
working is kept around for backward compatibility).
By proceeding in this way a sink can be used by multiple sessions
without having to play games with entries in sysFS.
Signed-off-by: Mathieu Poirier <[email protected]>
---
drivers/hwtracing/coresight/coresight-etm-perf.c | 130 ++++++++++++++++++++---
drivers/hwtracing/coresight/coresight-etm-perf.h | 4 +
2 files changed, 122 insertions(+), 12 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 0f5e03e4df22..5c2cce60da5e 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -11,6 +11,7 @@
#include <linux/list.h>
#include <linux/mm.h>
#include <linux/init.h>
+#include <linux/parser.h>
#include <linux/perf_event.h>
#include <linux/slab.h>
#include <linux/types.h>
@@ -22,6 +23,8 @@
static struct pmu etm_pmu;
static bool etm_perf_up;
+#define CORESIGHT_DEVICE_MAX_NAME_LEN 256
+
/**
* struct etm_event_data - Coresight specifics associated to an event
* @work: Handle to free allocated memory outside IRQ context.
@@ -181,6 +184,74 @@ static void etm_free_aux(void *data)
schedule_work(&event_data->work);
}
+static char *etm_drv_config_sync(struct perf_event *event)
+{
+ char *sink;
+ int node = event->cpu == -1 ? -1 : cpu_to_node(event->cpu);
+ struct pmu_drv_config *drv_config = perf_event_get_drv_config(event);
+
+ sink = kmalloc_node(CORESIGHT_DEVICE_MAX_NAME_LEN, GFP_KERNEL, node);
+ if (!sink)
+ return NULL;
+
+ /*
+ * Make sure we don't race with perf_drv_config_replace() in
+ * kernel/events/core.c.
+ */
+ raw_spin_lock(&drv_config->lock);
+ memcpy(sink, drv_config->config, CORESIGHT_DEVICE_MAX_NAME_LEN);
+ raw_spin_unlock(&drv_config->lock);
+
+ return sink;
+}
+
+static struct coresight_device *etm_event_get_sink(struct perf_event *event)
+{
+ struct pmu_drv_config *drv_config = perf_event_get_drv_config(event);
+
+ /*
+ * Try the preferred method first, i.e getting the sink information
+ * using the ioctl() method.
+ */
+ if (drv_config->config) {
+ char *drv_config;
+ struct device *dev;
+ struct coresight_device *sink;
+
+ /*
+ * Get sink from event->hw.drv_config.config - see
+ * _perf_ioctl() _SET_DRV_CONFIG.
+ */
+ drv_config = etm_drv_config_sync(event);
+ if (!drv_config)
+ goto out;
+
+ /* Look for the device of that name on the CS bus. */
+ dev = bus_find_device_by_name(&coresight_bustype, NULL,
+ drv_config);
+ kfree(drv_config);
+
+ if (dev) {
+ sink = to_coresight_device(dev);
+ /* Put reference from 'bus_find_device()' */
+ put_device(dev);
+ return sink;
+ }
+ }
+
+ /*
+ * No luck with the above method, so we are working with an older user
+ * space. See if a sink has been set using sysFS. If this is the case
+ * CPU-wide session will only be able to use a single sink.
+ *
+ * When operated from sysFS users are responsible to enable the sink
+ * while from perf, the perf tools will do it based on the choice made
+ * on the cmd line. As such the "enable_sink" flag in sysFS is reset.
+ */
+out:
+ return coresight_get_enabled_sink(true);
+}
+
static void *etm_setup_aux(struct perf_event *event, void **pages,
int nr_pages, bool overwrite)
{
@@ -194,18 +265,8 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
return NULL;
INIT_WORK(&event_data->work, free_event_data);
- /*
- * In theory nothing prevent tracers in a trace session from being
- * associated with different sinks, nor having a sink per tracer. But
- * until we have HW with this kind of topology we need to assume tracers
- * in a trace session are using the same sink. Therefore go through
- * the coresight bus and pick the first enabled sink.
- *
- * When operated from sysFS users are responsible to enable the sink
- * while from perf, the perf tools will do it based on the choice made
- * on the cmd line. As such the "enable_sink" flag in sysFS is reset.
- */
- sink = coresight_get_enabled_sink(true);
+ /* First get the sink to use for this event */
+ sink = etm_event_get_sink(event);
if (!sink)
goto err;
@@ -442,6 +503,49 @@ static void etm_addr_filters_sync(struct perf_event *event)
filters->nr_filters = i;
}
+static const match_table_t config_tokens = {
+ { ETM_CFG_SINK, "%u.%s" },
+ { ETM_CFG_NONE, NULL },
+};
+
+static void *etm_drv_config_validate(struct perf_event *event, char *cstr)
+{
+ char *str, *to_parse, *sink = NULL;
+ int token, ret = -EINVAL;
+ substring_t args[MAX_OPT_ARGS];
+
+ to_parse = kstrdup(cstr, GFP_KERNEL);
+ if (!to_parse)
+ return ERR_PTR(-ENOMEM);
+
+ while ((str = strsep(&to_parse, " ,\n")) != NULL) {
+ if (!*str)
+ continue;
+
+ token = match_token(str, config_tokens, args);
+ switch (token) {
+ case ETM_CFG_SINK:
+ sink = kstrdup(str, GFP_KERNEL);
+ if (!sink) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ break;
+ default:
+ goto out;
+ }
+ }
+
+out:
+ kfree(to_parse);
+ return sink ? sink : ERR_PTR(ret);
+}
+
+static void etm_drv_config_free(void *drv_data)
+{
+ kfree(drv_data);
+}
+
int etm_perf_symlink(struct coresight_device *csdev, bool link)
{
char entry[sizeof("cpu9999999")];
@@ -486,6 +590,8 @@ static int __init etm_perf_init(void)
etm_pmu.addr_filters_sync = etm_addr_filters_sync;
etm_pmu.addr_filters_validate = etm_addr_filters_validate;
etm_pmu.nr_addr_filters = ETM_ADDR_CMP_MAX;
+ etm_pmu.drv_config_validate = etm_drv_config_validate;
+ etm_pmu.drv_config_free = etm_drv_config_free;
ret = perf_pmu_register(&etm_pmu, CORESIGHT_ETM_PMU_NAME, -1);
if (ret == 0)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h
index 4197df4faf5e..37a98230fc34 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.h
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.h
@@ -42,6 +42,10 @@ struct etm_filters {
bool ssstatus;
};
+enum etm_config_elem_type {
+ ETM_CFG_NONE = -1,
+ ETM_CFG_SINK,
+};
#ifdef CONFIG_CORESIGHT
int etm_perf_symlink(struct coresight_device *csdev, bool link);
--
2.7.4
This patch adds the mechanic needed for user space to send PMU specific
configuration to the kernel driver using an ioctl() command. That way
events can keep track of options that don't fit in the perf_event_attr
structure like the selection of a CoreSight sink to use for the session.
Signed-off-by: Mathieu Poirier <[email protected]>
---
include/linux/perf_event.h | 45 +++++++++++++++++++++++++++
kernel/events/core.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 122 insertions(+)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 1fa12887ec02..7064b513ca2b 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -114,6 +114,14 @@ struct hw_perf_event_extra {
int idx; /* index in shared_regs->regs[] */
};
+/*
+ * PMU driver configuration
+ */
+struct pmu_drv_config {
+ void *config;
+ raw_spinlock_t lock;
+};
+
/**
* struct hw_perf_event - performance event hardware details:
*/
@@ -178,6 +186,9 @@ struct hw_perf_event {
/* Last sync'ed generation of filters */
unsigned long addr_filters_gen;
+ /* PMU driver configuration */
+ struct pmu_drv_config drv_config;
+
/*
* hw_perf_event::state flags; used to track the PERF_EF_* state.
*/
@@ -447,6 +458,23 @@ struct pmu {
* Filter events for PMU-specific reasons.
*/
int (*filter_match) (struct perf_event *event); /* optional */
+
+ /*
+ * Valiate complex PMU configuration that don't fit in the
+ * perf_event_attr struct. Returns a PMU specific pointer or an error
+ * value < 0.
+ *
+ * As with addr_filters_validate(), runs in the context of the ioctl()
+ * process and is not serialized with the rest of the PMU callbacks.
+ */
+ void *(*drv_config_validate) (struct perf_event *event,
+ char *config_str);
+
+ /*
+ * Release PMU specific configuration acquired by
+ * drv_config_validate()
+ */
+ void (*drv_config_free) (void *drv_data);
};
enum perf_addr_filter_action_t {
@@ -1234,6 +1262,12 @@ static inline bool has_addr_filter(struct perf_event *event)
return event->pmu->nr_addr_filters;
}
+static inline bool has_drv_config(struct perf_event *event)
+{
+ return event->pmu->drv_config_validate &&
+ event->pmu->drv_config_free;
+}
+
/*
* An inherited event uses parent's filters
*/
@@ -1248,6 +1282,17 @@ perf_event_addr_filters(struct perf_event *event)
return ifh;
}
+static inline struct pmu_drv_config *
+perf_event_get_drv_config(struct perf_event *event)
+{
+ struct pmu_drv_config *cfg = &event->hw.drv_config;
+
+ if (event->parent)
+ cfg = &event->parent->hw.drv_config;
+
+ return cfg;
+}
+
extern void perf_event_addr_filters_sync(struct perf_event *event);
extern int perf_output_begin(struct perf_output_handle *handle,
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 8f0434a9951a..ccff64e9451e 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4410,6 +4410,7 @@ static bool exclusive_event_installable(struct perf_event *event,
static void perf_addr_filters_splice(struct perf_event *event,
struct list_head *head);
+static void perf_drv_config_replace(struct perf_event *event, void *drv_data);
static void _free_event(struct perf_event *event)
{
@@ -4440,6 +4441,7 @@ static void _free_event(struct perf_event *event)
perf_event_free_bpf_prog(event);
perf_addr_filters_splice(event, NULL);
kfree(event->addr_filters_offs);
+ perf_drv_config_replace(event, NULL);
if (event->destroy)
event->destroy(event);
@@ -5002,6 +5004,8 @@ static inline int perf_fget_light(int fd, struct fd *p)
static int perf_event_set_output(struct perf_event *event,
struct perf_event *output_event);
static int perf_event_set_filter(struct perf_event *event, void __user *arg);
+static int perf_event_set_drv_config(struct perf_event *event,
+ void __user *arg);
static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd);
static int perf_copy_attr(struct perf_event_attr __user *uattr,
struct perf_event_attr *attr);
@@ -5088,6 +5092,10 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon
return perf_event_modify_attr(event, &new_attr);
}
+
+ case PERF_EVENT_IOC_SET_DRV_CONFIG:
+ return perf_event_set_drv_config(event, (void __user *)arg);
+
default:
return -ENOTTY;
}
@@ -9086,6 +9094,75 @@ static int perf_event_set_filter(struct perf_event *event, void __user *arg)
return ret;
}
+static void perf_drv_config_replace(struct perf_event *event, void *drv_data)
+{
+ unsigned long flags;
+ void *old_drv_data;
+ struct pmu_drv_config *drv_config = &event->hw.drv_config;
+
+ if (!has_drv_config(event))
+ return;
+
+ /* Children take their configuration from their parent */
+ if (event->parent)
+ return;
+
+ /* Make sure the PMU doesn't get a handle on the data */
+ raw_spin_lock_irqsave(&drv_config->lock, flags);
+
+ old_drv_data = drv_config->config;
+ drv_config->config = drv_data;
+
+ raw_spin_unlock_irqrestore(&drv_config->lock, flags);
+
+ /* Free PMU private data allocated by pmu::drv_config_validate() */
+ event->pmu->drv_config_free(old_drv_data);
+}
+
+static int
+perf_event_process_drv_config(struct perf_event *event, char *config_str)
+{
+ int ret = -EINVAL;
+ void *drv_data;
+
+ /* Make sure ctx.mutex is held */
+ lockdep_assert_held(&event->ctx->mutex);
+
+ /* Children take their configuration from their parent */
+ if (WARN_ON_ONCE(event->parent))
+ goto out;
+
+ drv_data = event->pmu->drv_config_validate(event, config_str);
+ if (IS_ERR(drv_data)) {
+ ret = PTR_ERR(drv_data);
+ goto out;
+ }
+
+ perf_drv_config_replace(event, drv_data);
+
+ ret = 0;
+out:
+ return ret;
+}
+
+static int perf_event_set_drv_config(struct perf_event *event, void __user *arg)
+{
+ int ret = -EINVAL;
+ char *config_str;
+
+ if (!has_drv_config(event))
+ return ret;
+
+ config_str = strndup_user(arg, PAGE_SIZE);
+ if (IS_ERR(config_str))
+ return PTR_ERR(config_str);
+
+ ret = perf_event_process_drv_config(event, config_str);
+
+ kfree(config_str);
+ return ret;
+}
+
/*
* hrtimer based swevent callback
*/
--
2.7.4
Adding a new IOCTL command to communicate PMU specific configuration to
PMU kernel drivers. This can be anything a PMU might need for
configuration that doesn't fit in the perf_event_attr structure, such
as the CoreSight sink to use for a session.
Signed-off-by: Mathieu Poirier <[email protected]>
---
include/uapi/linux/perf_event.h | 1 +
tools/include/uapi/linux/perf_event.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index b8e288a1f740..b5b3241877df 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -460,6 +460,7 @@ struct perf_event_query_bpf {
#define PERF_EVENT_IOC_PAUSE_OUTPUT _IOW('$', 9, __u32)
#define PERF_EVENT_IOC_QUERY_BPF _IOWR('$', 10, struct perf_event_query_bpf *)
#define PERF_EVENT_IOC_MODIFY_ATTRIBUTES _IOW('$', 11, struct perf_event_attr *)
+#define PERF_EVENT_IOC_SET_DRV_CONFIG _IOW('$', 12, char *)
enum perf_event_ioc_flags {
PERF_IOC_FLAG_GROUP = 1U << 0,
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index b8e288a1f740..b5b3241877df 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -460,6 +460,7 @@ struct perf_event_query_bpf {
#define PERF_EVENT_IOC_PAUSE_OUTPUT _IOW('$', 9, __u32)
#define PERF_EVENT_IOC_QUERY_BPF _IOWR('$', 10, struct perf_event_query_bpf *)
#define PERF_EVENT_IOC_MODIFY_ATTRIBUTES _IOW('$', 11, struct perf_event_attr *)
+#define PERF_EVENT_IOC_SET_DRV_CONFIG _IOW('$', 12, char *)
enum perf_event_ioc_flags {
PERF_IOC_FLAG_GROUP = 1U << 0,
--
2.7.4
On Thu, 5 Jul 2018 16:13:42 -0600
Mathieu Poirier <[email protected]> wrote:
Hi Mathieu,
> This patch adds the mechanic needed for user space to send PMU specific
^^^^^^^^
I think you meant 'mechanism' here: mechanics fix cars :)
> +static void perf_drv_config_replace(struct perf_event *event, void *drv_data)
> +{
> + unsigned long flags;
> + void *old_drv_data;
> + struct pmu_drv_config *drv_config = &event->hw.drv_config;
> +
> + if (!has_drv_config(event))
> + return;
> +
> + /* Children take their configuration from their parent */
> + if (event->parent)
> + return;
> +
> + /* Make sure the PMU doesn't get a handle on the data */
> + raw_spin_lock_irqsave(&drv_config->lock, flags);
> +
> + old_drv_data = drv_config->config;
> + drv_config->config = drv_data;
> +
> + raw_spin_unlock_irqrestore(&drv_config->lock, flags);
> +
> + /* Free PMU private data allocated by pmu::drv_config_validate() */
> + event->pmu->drv_config_free(old_drv_data);
> +}
I got this stacktrace whilst testing a perf tool *without* this series
applied, running on a kernel *with* this series applied:
[ 132.942054] INFO: trying to register non-static key.
[ 132.946964] the code is fine but needs lockdep annotation.
[ 132.952389] turning off the locking correctness validator.
[ 132.957818] CPU: 2 PID: 2835 Comm: perf64-sans Not tainted 4.18.0-rc3-00196-g5b5d957532a8-dirty #146
[ 132.966856] Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno Development Platform, BIOS EDK II Jan 23 2017
[ 132.977527] Call trace:
[ 132.979947] dump_backtrace+0x0/0x1c0
[ 132.983567] show_stack+0x24/0x30
[ 132.986845] dump_stack+0x90/0xb4
[ 132.990122] register_lock_class+0x57c/0x580
[ 132.994343] __lock_acquire.isra.12+0x6c/0x980
[ 132.998736] lock_acquire+0x100/0x1e8
[ 133.002357] _raw_spin_lock_irqsave+0x58/0x78
[ 133.006667] perf_drv_config_replace+0x4c/0x80
[ 133.011061] _free_event+0xbc/0x460
[ 133.014507] put_event+0x2c/0x38
[ 133.017697] perf_event_release_kernel+0x1ac/0x300
[ 133.022434] perf_release+0x10/0x20
[ 133.025883] __fput+0xa8/0x1e0
[ 133.028901] ____fput+0x20/0x30
[ 133.032006] task_work_run+0xa0/0xd0
[ 133.035539] do_notify_resume+0x118/0x120
[ 133.039503] work_pending+0x8/0x10
Is a raw_spin_lock_init missing perhaps?
Thanks,
Kim
On Thu, 5 Jul 2018 16:13:40 -0600
Mathieu Poirier <[email protected]> wrote:
> This set adds the capability to communiate event specific configuration
> to the PMU kernel driver using an ioctl(). The functionatlity is made
> generic enough for anyone to use but is targeted at the identification
> of CoreSight sinks when operating in CPU-wide trace scenarios.
With this series, a --per-thread -less invocation looks like it
succeeds (instead of giving a "failed to mmap with 12 (Cannot allocate
memory)" error):
# perf record -e /cs_etm/@20010000.etf/ sleep 1
[ perf record: Woken up 3 times to write data ]
Warning:
AUX data lost 2 times out of 3!
[ perf record: Captured and wrote 0.182 MB perf.data ]
#
but now perf report - built with libopencsd - is unable to process the
perf.data file:
# perf report --stdio
0x3a0 [0x60]: failed to process type: 1
Error:
failed to process sample
# To display the perf.data header info, please use --header/--header-only options.
#
Also, a "record -a" invocation also acts like it's working, but Juno
has a hardware limitation where it can't record all cpus concurrently,
right? So, shouldn't record commands that exceed the h/w's
capabilities error out instead?
Thanks,
Kim
On Fri, 6 Jul 2018 at 17:22, Kim Phillips <[email protected]> wrote:
>
> On Thu, 5 Jul 2018 16:13:42 -0600
> Mathieu Poirier <[email protected]> wrote:
>
> Hi Mathieu,
>
> > This patch adds the mechanic needed for user space to send PMU specific
> ^^^^^^^^
> I think you meant 'mechanism' here: mechanics fix cars :)
I really meant "mechanic", as in "functional details or procedure" [1]
[1]. https://www.merriam-webster.com/dictionary/mechanics
>
> > +static void perf_drv_config_replace(struct perf_event *event, void *drv_data)
> > +{
> > + unsigned long flags;
> > + void *old_drv_data;
> > + struct pmu_drv_config *drv_config = &event->hw.drv_config;
> > +
> > + if (!has_drv_config(event))
> > + return;
> > +
> > + /* Children take their configuration from their parent */
> > + if (event->parent)
> > + return;
> > +
> > + /* Make sure the PMU doesn't get a handle on the data */
> > + raw_spin_lock_irqsave(&drv_config->lock, flags);
> > +
> > + old_drv_data = drv_config->config;
> > + drv_config->config = drv_data;
> > +
> > + raw_spin_unlock_irqrestore(&drv_config->lock, flags);
> > +
> > + /* Free PMU private data allocated by pmu::drv_config_validate() */
> > + event->pmu->drv_config_free(old_drv_data);
> > +}
>
> I got this stacktrace whilst testing a perf tool *without* this series
> applied, running on a kernel *with* this series applied:
That shouldn't matter as I kept the changes backward compatible
specifically to handle this situation.
>
> [ 132.942054] INFO: trying to register non-static key.
> [ 132.946964] the code is fine but needs lockdep annotation.
> [ 132.952389] turning off the locking correctness validator.
> [ 132.957818] CPU: 2 PID: 2835 Comm: perf64-sans Not tainted 4.18.0-rc3-00196-g5b5d957532a8-dirty #146
> [ 132.966856] Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno Development Platform, BIOS EDK II Jan 23 2017
> [ 132.977527] Call trace:
> [ 132.979947] dump_backtrace+0x0/0x1c0
> [ 132.983567] show_stack+0x24/0x30
> [ 132.986845] dump_stack+0x90/0xb4
> [ 132.990122] register_lock_class+0x57c/0x580
> [ 132.994343] __lock_acquire.isra.12+0x6c/0x980
> [ 132.998736] lock_acquire+0x100/0x1e8
> [ 133.002357] _raw_spin_lock_irqsave+0x58/0x78
> [ 133.006667] perf_drv_config_replace+0x4c/0x80
> [ 133.011061] _free_event+0xbc/0x460
> [ 133.014507] put_event+0x2c/0x38
> [ 133.017697] perf_event_release_kernel+0x1ac/0x300
> [ 133.022434] perf_release+0x10/0x20
> [ 133.025883] __fput+0xa8/0x1e0
> [ 133.028901] ____fput+0x20/0x30
> [ 133.032006] task_work_run+0xa0/0xd0
> [ 133.035539] do_notify_resume+0x118/0x120
> [ 133.039503] work_pending+0x8/0x10
>
> Is a raw_spin_lock_init missing perhaps?
Not that I can tell - I need to investigate.
>
> Thanks,
>
> Kim
On Fri, 6 Jul 2018 at 17:38, Kim Phillips <[email protected]> wrote:
>
> On Thu, 5 Jul 2018 16:13:40 -0600
> Mathieu Poirier <[email protected]> wrote:
>
> > This set adds the capability to communiate event specific configuration
> > to the PMU kernel driver using an ioctl(). The functionatlity is made
> > generic enough for anyone to use but is targeted at the identification
> > of CoreSight sinks when operating in CPU-wide trace scenarios.
>
> With this series, a --per-thread -less invocation looks like it
> succeeds (instead of giving a "failed to mmap with 12 (Cannot allocate
> memory)" error):
>
> # perf record -e /cs_etm/@20010000.etf/ sleep 1
> [ perf record: Woken up 3 times to write data ]
> Warning:
> AUX data lost 2 times out of 3!
>
> [ perf record: Captured and wrote 0.182 MB perf.data ]
> #
>
> but now perf report - built with libopencsd - is unable to process the
> perf.data file:
>
> # perf report --stdio
> 0x3a0 [0x60]: failed to process type: 1
> Error:
> failed to process sample
> # To display the perf.data header info, please use --header/--header-only options.
> #
Right, that's expected since the perf tools patches for CPU-wide trace
scenarios haven't been merged yet. The real problem is, as you
pointed above, that this patchset should still report an error as full
functionality isn't merged yet. I will fix that.
>
> Also, a "record -a" invocation also acts like it's working, but Juno
> has a hardware limitation where it can't record all cpus concurrently,
> right? So, shouldn't record commands that exceed the h/w's
> capabilities error out instead?
To be clear there is no hardware limitation preventing "record -a" to
work properly, just SW. This patchset is the first step in addressing
the situation.
Thanks,
Mathieu
>
> Thanks,
>
> Kim
On Mon, 9 Jul 2018 at 10:49, Mathieu Poirier <[email protected]> wrote:
>
> On Fri, 6 Jul 2018 at 17:22, Kim Phillips <[email protected]> wrote:
> >
> > On Thu, 5 Jul 2018 16:13:42 -0600
> > Mathieu Poirier <[email protected]> wrote:
> >
> > Hi Mathieu,
> >
> > > This patch adds the mechanic needed for user space to send PMU specific
> > ^^^^^^^^
> > I think you meant 'mechanism' here: mechanics fix cars :)
>
> I really meant "mechanic", as in "functional details or procedure" [1]
>
> [1]. https://www.merriam-webster.com/dictionary/mechanics
>
> >
> > > +static void perf_drv_config_replace(struct perf_event *event, void *drv_data)
> > > +{
> > > + unsigned long flags;
> > > + void *old_drv_data;
> > > + struct pmu_drv_config *drv_config = &event->hw.drv_config;
> > > +
> > > + if (!has_drv_config(event))
> > > + return;
> > > +
> > > + /* Children take their configuration from their parent */
> > > + if (event->parent)
> > > + return;
> > > +
> > > + /* Make sure the PMU doesn't get a handle on the data */
> > > + raw_spin_lock_irqsave(&drv_config->lock, flags);
> > > +
> > > + old_drv_data = drv_config->config;
> > > + drv_config->config = drv_data;
> > > +
> > > + raw_spin_unlock_irqrestore(&drv_config->lock, flags);
> > > +
> > > + /* Free PMU private data allocated by pmu::drv_config_validate() */
> > > + event->pmu->drv_config_free(old_drv_data);
> > > +}
> >
> > I got this stacktrace whilst testing a perf tool *without* this series
> > applied, running on a kernel *with* this series applied:
>
> That shouldn't matter as I kept the changes backward compatible
> specifically to handle this situation.
>
> >
> > [ 132.942054] INFO: trying to register non-static key.
> > [ 132.946964] the code is fine but needs lockdep annotation.
> > [ 132.952389] turning off the locking correctness validator.
> > [ 132.957818] CPU: 2 PID: 2835 Comm: perf64-sans Not tainted 4.18.0-rc3-00196-g5b5d957532a8-dirty #146
> > [ 132.966856] Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno Development Platform, BIOS EDK II Jan 23 2017
> > [ 132.977527] Call trace:
> > [ 132.979947] dump_backtrace+0x0/0x1c0
> > [ 132.983567] show_stack+0x24/0x30
> > [ 132.986845] dump_stack+0x90/0xb4
> > [ 132.990122] register_lock_class+0x57c/0x580
> > [ 132.994343] __lock_acquire.isra.12+0x6c/0x980
> > [ 132.998736] lock_acquire+0x100/0x1e8
> > [ 133.002357] _raw_spin_lock_irqsave+0x58/0x78
> > [ 133.006667] perf_drv_config_replace+0x4c/0x80
> > [ 133.011061] _free_event+0xbc/0x460
> > [ 133.014507] put_event+0x2c/0x38
> > [ 133.017697] perf_event_release_kernel+0x1ac/0x300
> > [ 133.022434] perf_release+0x10/0x20
> > [ 133.025883] __fput+0xa8/0x1e0
> > [ 133.028901] ____fput+0x20/0x30
> > [ 133.032006] task_work_run+0xa0/0xd0
> > [ 133.035539] do_notify_resume+0x118/0x120
> > [ 133.039503] work_pending+0x8/0x10
> >
> > Is a raw_spin_lock_init missing perhaps?
I read your email too fast... That's exactly what it was.
Thanks for giving this a spin,
Mathieu
>
> Not that I can tell - I need to investigate.
>
> >
> > Thanks,
> >
> > Kim