linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] coresight: implementing address filtering
@ 2016-07-18 19:51 Mathieu Poirier
  2016-07-18 19:51 ` [PATCH 01/10] coresight: etm-perf: pass struct perf_event to source::enable/disable() Mathieu Poirier
                   ` (9 more replies)
  0 siblings, 10 replies; 16+ messages in thread
From: Mathieu Poirier @ 2016-07-18 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

This patchset builds on the generic address filtering framework
in the perf core to implement range and start/stop filters for
ARM CoreSight.

Most of the code is moving things around in order to provide an
environment where filters can be acquired from perf and then configured
in the CoreSight drivers.

The set is based on 4.7-rc7 and depends on these patches [1].

[1]. https://lkml.org/lkml/2016/7/18/457

Mathieu Poirier (10):
  coresight: etm-perf: pass struct perf_event to
    source::enable/disable()
  coresight: remove duplicated enumeration
  coresight: etm-perf: configuring filters from perf core
  coresight: etm4x: split default and filter configuration
  coresight: etm4x: cleaning up default filter configuration
  coresight: etm4x: adding range filter configuration function
  coresight: etm4x: configuring include/exclude function
  coresight: etm4x: adding configurable address range filtering
  coresight: etm4x: adding configurable start/stop filtering
  coresight: documenting range and start/stop filtering

 Documentation/trace/coresight.txt                  |  48 +++
 drivers/hwtracing/coresight/coresight-etm-perf.c   | 150 +++++++--
 drivers/hwtracing/coresight/coresight-etm-perf.h   |  32 ++
 drivers/hwtracing/coresight/coresight-etm.h        |   8 -
 .../hwtracing/coresight/coresight-etm3x-sysfs.c    |   1 +
 drivers/hwtracing/coresight/coresight-etm3x.c      |  14 +-
 .../hwtracing/coresight/coresight-etm4x-sysfs.c    |   1 +
 drivers/hwtracing/coresight/coresight-etm4x.c      | 356 +++++++++++++++++----
 drivers/hwtracing/coresight/coresight-etm4x.h      |   8 -
 drivers/hwtracing/coresight/coresight-priv.h       |   8 +
 drivers/hwtracing/coresight/coresight-stm.c        |   7 +-
 drivers/hwtracing/coresight/coresight.c            |   2 +-
 include/linux/coresight.h                          |   5 +-
 13 files changed, 530 insertions(+), 110 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 01/10] coresight: etm-perf: pass struct perf_event to source::enable/disable()
  2016-07-18 19:51 [PATCH 00/10] coresight: implementing address filtering Mathieu Poirier
@ 2016-07-18 19:51 ` Mathieu Poirier
  2016-07-20 15:34   ` Suzuki K Poulose
  2016-07-18 19:51 ` [PATCH 02/10] coresight: remove duplicated enumeration Mathieu Poirier
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Mathieu Poirier @ 2016-07-18 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

With this commit [1] address range filter information is now found
in the struct hw_perf_event::addr_filters.  As such pass the event
itself to the coresight_source::enable/disable() functions so that
both event attribute and filter can be accessible for configuration.

[1] 'commit 375637bc5249 ("perf/core: Introduce address range filtering")'

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm-perf.c |  4 ++--
 drivers/hwtracing/coresight/coresight-etm3x.c    | 14 ++++++++------
 drivers/hwtracing/coresight/coresight-etm4x.c    | 19 +++++++++++--------
 drivers/hwtracing/coresight/coresight-stm.c      |  7 ++++---
 drivers/hwtracing/coresight/coresight.c          |  2 +-
 include/linux/coresight.h                        |  5 +++--
 6 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 8fbb1dd9e243..78a1bc0013a2 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -283,7 +283,7 @@ static void etm_event_start(struct perf_event *event, int flags)
 	event->hw.state = 0;
 
 	/* Finally enable the tracer */
-	if (source_ops(csdev)->enable(csdev, &event->attr, CS_MODE_PERF))
+	if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF))
 		goto fail_end_stop;
 
 out:
@@ -316,7 +316,7 @@ static void etm_event_stop(struct perf_event *event, int mode)
 		return;
 
 	/* stop tracer */
-	source_ops(csdev)->disable(csdev);
+	source_ops(csdev)->disable(csdev, event);
 
 	/* tell the core */
 	event->hw.state = PERF_HES_STOPPED;
diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
index d83ab82672e4..38828bd8d332 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x.c
@@ -309,9 +309,10 @@ void etm_config_trace_mode(struct etm_config *config)
 #define ETM3X_SUPPORTED_OPTIONS (ETMCR_CYC_ACC | ETMCR_TIMESTAMP_EN)
 
 static int etm_parse_event_config(struct etm_drvdata *drvdata,
-				  struct perf_event_attr *attr)
+				  struct perf_event *event)
 {
 	struct etm_config *config = &drvdata->config;
+	struct perf_event_attr *attr = &event->attr;
 
 	if (!attr)
 		return -EINVAL;
@@ -457,7 +458,7 @@ static int etm_trace_id(struct coresight_device *csdev)
 }
 
 static int etm_enable_perf(struct coresight_device *csdev,
-			   struct perf_event_attr *attr)
+			   struct perf_event *event)
 {
 	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
@@ -465,7 +466,7 @@ static int etm_enable_perf(struct coresight_device *csdev,
 		return -EINVAL;
 
 	/* Configure the tracer based on the session's specifics */
-	etm_parse_event_config(drvdata, attr);
+	etm_parse_event_config(drvdata, event);
 	/* And enable it */
 	etm_enable_hw(drvdata);
 
@@ -503,7 +504,7 @@ err:
 }
 
 static int etm_enable(struct coresight_device *csdev,
-		      struct perf_event_attr *attr, u32 mode)
+		      struct perf_event *event, u32 mode)
 {
 	int ret;
 	u32 val;
@@ -520,7 +521,7 @@ static int etm_enable(struct coresight_device *csdev,
 		ret = etm_enable_sysfs(csdev);
 		break;
 	case CS_MODE_PERF:
-		ret = etm_enable_perf(csdev, attr);
+		ret = etm_enable_perf(csdev, event);
 		break;
 	default:
 		ret = -EINVAL;
@@ -600,7 +601,8 @@ static void etm_disable_sysfs(struct coresight_device *csdev)
 	dev_info(drvdata->dev, "ETM tracing disabled\n");
 }
 
-static void etm_disable(struct coresight_device *csdev)
+static void etm_disable(struct coresight_device *csdev,
+			struct perf_event *event)
 {
 	u32 mode;
 	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 462f0dc15757..1fe1f0e86daf 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -193,9 +193,10 @@ static void etm4_enable_hw(void *info)
 }
 
 static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,
-				   struct perf_event_attr *attr)
+				   struct perf_event *event)
 {
 	struct etmv4_config *config = &drvdata->config;
+	struct perf_event_attr *attr = &event->attr;
 
 	if (!attr)
 		return -EINVAL;
@@ -229,7 +230,7 @@ static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,
 }
 
 static int etm4_enable_perf(struct coresight_device *csdev,
-			    struct perf_event_attr *attr)
+			    struct perf_event *event)
 {
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
@@ -237,7 +238,7 @@ static int etm4_enable_perf(struct coresight_device *csdev,
 		return -EINVAL;
 
 	/* Configure the tracer based on the session's specifics */
-	etm4_parse_event_config(drvdata, attr);
+	etm4_parse_event_config(drvdata, event);
 	/* And enable it */
 	etm4_enable_hw(drvdata);
 
@@ -272,7 +273,7 @@ err:
 }
 
 static int etm4_enable(struct coresight_device *csdev,
-		       struct perf_event_attr *attr, u32 mode)
+		       struct perf_event *event, u32 mode)
 {
 	int ret;
 	u32 val;
@@ -289,7 +290,7 @@ static int etm4_enable(struct coresight_device *csdev,
 		ret = etm4_enable_sysfs(csdev);
 		break;
 	case CS_MODE_PERF:
-		ret = etm4_enable_perf(csdev, attr);
+		ret = etm4_enable_perf(csdev, event);
 		break;
 	default:
 		ret = -EINVAL;
@@ -324,7 +325,8 @@ static void etm4_disable_hw(void *info)
 	dev_dbg(drvdata->dev, "cpu: %d disable smp call done\n", drvdata->cpu);
 }
 
-static int etm4_disable_perf(struct coresight_device *csdev)
+static int etm4_disable_perf(struct coresight_device *csdev,
+			     struct perf_event *event)
 {
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
@@ -360,7 +362,8 @@ static void etm4_disable_sysfs(struct coresight_device *csdev)
 	dev_info(drvdata->dev, "ETM tracing disabled\n");
 }
 
-static void etm4_disable(struct coresight_device *csdev)
+static void etm4_disable(struct coresight_device *csdev,
+			 struct perf_event *event)
 {
 	u32 mode;
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
@@ -379,7 +382,7 @@ static void etm4_disable(struct coresight_device *csdev)
 		etm4_disable_sysfs(csdev);
 		break;
 	case CS_MODE_PERF:
-		etm4_disable_perf(csdev);
+		etm4_disable_perf(csdev, event);
 		break;
 	}
 
diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
index 73be58a11e4f..d8fee8fff3da 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -196,7 +196,7 @@ static void stm_enable_hw(struct stm_drvdata *drvdata)
 }
 
 static int stm_enable(struct coresight_device *csdev,
-		      struct perf_event_attr *attr, u32 mode)
+		      struct perf_event *event, u32 mode)
 {
 	u32 val;
 	struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
@@ -258,7 +258,8 @@ static void stm_disable_hw(struct stm_drvdata *drvdata)
 		stm_hwevent_disable_hw(drvdata);
 }
 
-static void stm_disable(struct coresight_device *csdev)
+static void stm_disable(struct coresight_device *csdev,
+			struct perf_event *event)
 {
 	struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
@@ -353,7 +354,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
 	if (!drvdata || !drvdata->csdev)
 		return;
 
-	stm_disable(drvdata->csdev);
+	stm_disable(drvdata->csdev, NULL);
 }
 
 static long stm_generic_set_options(struct stm_data *stm_data,
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
index 508532b3fcac..60dff8915822 100644
--- a/drivers/hwtracing/coresight/coresight.c
+++ b/drivers/hwtracing/coresight/coresight.c
@@ -257,7 +257,7 @@ static void coresight_disable_source(struct coresight_device *csdev)
 {
 	if (atomic_dec_return(csdev->refcnt) == 0) {
 		if (source_ops(csdev)->disable) {
-			source_ops(csdev)->disable(csdev);
+			source_ops(csdev)->disable(csdev, NULL);
 			csdev->enable = false;
 		}
 	}
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 385d62e64abb..2a5982c37dfb 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -232,8 +232,9 @@ struct coresight_ops_source {
 	int (*cpu_id)(struct coresight_device *csdev);
 	int (*trace_id)(struct coresight_device *csdev);
 	int (*enable)(struct coresight_device *csdev,
-		      struct perf_event_attr *attr,  u32 mode);
-	void (*disable)(struct coresight_device *csdev);
+		      struct perf_event *event,  u32 mode);
+	void (*disable)(struct coresight_device *csdev,
+			struct perf_event *event);
 };
 
 struct coresight_ops {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 02/10] coresight: remove duplicated enumeration
  2016-07-18 19:51 [PATCH 00/10] coresight: implementing address filtering Mathieu Poirier
  2016-07-18 19:51 ` [PATCH 01/10] coresight: etm-perf: pass struct perf_event to source::enable/disable() Mathieu Poirier
@ 2016-07-18 19:51 ` Mathieu Poirier
  2016-07-18 19:51 ` [PATCH 03/10] coresight: etm-perf: configuring filters from perf core Mathieu Poirier
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2016-07-18 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

Both ETMv3 and ETMv4 drivers are declaring an 'enum etm_addr_type',
creating reduncancy.

This patch removes the enumeration from the driver files and adds
it to a common header.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm.h         | 8 --------
 drivers/hwtracing/coresight/coresight-etm3x-sysfs.c | 1 +
 drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 1 +
 drivers/hwtracing/coresight/coresight-etm4x.h       | 8 --------
 drivers/hwtracing/coresight/coresight-priv.h        | 8 ++++++++
 5 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h
index 51597cb2c08a..4a18ee499965 100644
--- a/drivers/hwtracing/coresight/coresight-etm.h
+++ b/drivers/hwtracing/coresight/coresight-etm.h
@@ -259,14 +259,6 @@ struct etm_drvdata {
 	struct etm_config		config;
 };
 
-enum etm_addr_type {
-	ETM_ADDR_TYPE_NONE,
-	ETM_ADDR_TYPE_SINGLE,
-	ETM_ADDR_TYPE_RANGE,
-	ETM_ADDR_TYPE_START,
-	ETM_ADDR_TYPE_STOP,
-};
-
 static inline void etm_writel(struct etm_drvdata *drvdata,
 			      u32 val, u32 off)
 {
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
index 02d4b629891f..3f73cee7a094 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
@@ -18,6 +18,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/sysfs.h>
 #include "coresight-etm.h"
+#include "coresight-priv.h"
 
 static ssize_t nr_addr_cmp_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index 7c84308c5564..6ae26ffeaefd 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -18,6 +18,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/sysfs.h>
 #include "coresight-etm4x.h"
+#include "coresight-priv.h"
 
 static int etm4_set_mode_exclude(struct etmv4_drvdata *drvdata, bool exclude)
 {
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 5359c5197c1d..111f4bb3a6b2 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -407,14 +407,6 @@ enum etm_addr_ctxtype {
 	ETM_CTX_CTXID_VMID,
 };
 
-enum etm_addr_type {
-	ETM_ADDR_TYPE_NONE,
-	ETM_ADDR_TYPE_SINGLE,
-	ETM_ADDR_TYPE_RANGE,
-	ETM_ADDR_TYPE_START,
-	ETM_ADDR_TYPE_STOP,
-};
-
 extern const struct attribute_group *coresight_etmv4_groups[];
 void etm4_config_trace_mode(struct etmv4_config *config);
 #endif
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 3cb574b3cdd9..1f6b580ef1c4 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -47,6 +47,14 @@ static ssize_t name##_show(struct device *_dev,				\
 }									\
 static DEVICE_ATTR_RO(name)
 
+enum etm_addr_type {
+	ETM_ADDR_TYPE_NONE,
+	ETM_ADDR_TYPE_SINGLE,
+	ETM_ADDR_TYPE_RANGE,
+	ETM_ADDR_TYPE_START,
+	ETM_ADDR_TYPE_STOP,
+};
+
 enum cs_mode {
 	CS_MODE_DISABLED,
 	CS_MODE_SYSFS,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 03/10] coresight: etm-perf: configuring filters from perf core
  2016-07-18 19:51 [PATCH 00/10] coresight: implementing address filtering Mathieu Poirier
  2016-07-18 19:51 ` [PATCH 01/10] coresight: etm-perf: pass struct perf_event to source::enable/disable() Mathieu Poirier
  2016-07-18 19:51 ` [PATCH 02/10] coresight: remove duplicated enumeration Mathieu Poirier
@ 2016-07-18 19:51 ` Mathieu Poirier
  2016-07-20 16:07   ` Suzuki K Poulose
  2016-07-18 19:51 ` [PATCH 04/10] coresight: etm4x: split default and filter configuration Mathieu Poirier
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Mathieu Poirier @ 2016-07-18 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

This patch implements the required API needed to access
and retrieve range and start/stop filters from the perf core.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm-perf.c | 146 ++++++++++++++++++++---
 drivers/hwtracing/coresight/coresight-etm-perf.h |  32 +++++
 2 files changed, 162 insertions(+), 16 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 78a1bc0013a2..fde7f42149c5 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -29,6 +29,7 @@
 #include <linux/workqueue.h>
 
 #include "coresight-priv.h"
+#include "coresight-etm-perf.h"
 
 static struct pmu etm_pmu;
 static bool etm_perf_up;
@@ -83,12 +84,44 @@ static const struct attribute_group *etm_pmu_attr_groups[] = {
 
 static void etm_event_read(struct perf_event *event) {}
 
+static int etm_addr_filters_alloc(struct perf_event *event)
+{
+	struct etm_filters *filters;
+	int node = event->cpu == -1 ? -1 : cpu_to_node(event->cpu);
+
+	filters = kzalloc_node(sizeof(struct etm_filters), GFP_KERNEL, node);
+	if (!filters)
+		return -ENOMEM;
+
+	if (event->parent)
+		memcpy(filters, event->parent->hw.addr_filters,
+		       sizeof(*filters));
+
+	event->hw.addr_filters = filters;
+
+	return 0;
+}
+
+static void etm_event_destroy(struct perf_event *event)
+{
+	kfree(event->hw.addr_filters);
+	event->hw.addr_filters = NULL;
+}
+
 static int etm_event_init(struct perf_event *event)
 {
+	int ret;
+
 	if (event->attr.type != etm_pmu.type)
 		return -ENOENT;
 
-	return 0;
+	ret = etm_addr_filters_alloc(event);
+	if (ret)
+		return ret;
+
+	event->destroy = etm_event_destroy;
+
+	return ret;
 }
 
 static void free_event_data(struct work_struct *work)
@@ -456,6 +489,85 @@ static void etm_free_drv_configs(struct perf_event *event)
 	}
 }
 
+static int etm_addr_filters_validate(struct list_head *filters)
+{
+	bool range = false, address = false;
+	int index = 0;
+	struct perf_addr_filter *filter;
+
+	list_for_each_entry(filter, filters, entry) {
+		/*
+		 * No need to go further if there's no more
+		 * room for filters.
+		 */
+		if (++index > ETM_ADDR_CMP_MAX)
+			return -EOPNOTSUPP;
+
+		/*
+		 * As taken from the struct perf_addr_filter documentation:
+		 *	@range:	1: range, 0: address
+		 *
+		 * At this time we don't allow range and start/stop filtering
+		 * to cohabitate, they have to be mutually exclusive.
+		 */
+		if ((filter->range == 1) && address)
+			return -EOPNOTSUPP;
+
+		if ((filter->range == 0) && range)
+			return -EOPNOTSUPP;
+
+		/*
+		 * For range filtering, the second address in the address
+		 * range comparator needs to be higher than the first.
+		 * Invalid otherwise.
+		 */
+		if (filter->range && filter->size == 0)
+			return -EINVAL;
+
+		/*
+		 * Everything checks out with this filter, record what we've
+		 * received before moving on to the next one.
+		 */
+		if (filter->range)
+			range = true;
+		else
+			address = true;
+	}
+
+	return 0;
+}
+
+static void etm_addr_filters_sync(struct perf_event *event)
+{
+	struct perf_addr_filters_head *head = perf_event_addr_filters(event);
+	unsigned long start, stop, *offs = event->addr_filters_offs;
+	struct etm_filters *filters = event->hw.addr_filters;
+	struct perf_addr_filter *filter;
+	int i = 0;
+
+	list_for_each_entry(filter, &head->list, entry) {
+		start = filter->offset + offs[i];
+		stop = start + filter->size;
+
+		if (filter->range == 1) {
+			filters->filter[i].start_addr = start;
+			filters->filter[i].stop_addr = stop;
+			filters->filter[i].type = ETM_ADDR_TYPE_RANGE;
+		} else {
+			if (filter->filter == 1) {
+				filters->filter[i].start_addr = start;
+				filters->filter[i].type = ETM_ADDR_TYPE_START;
+			} else {
+				filters->filter[i].stop_addr = stop;
+				filters->filter[i].type = ETM_ADDR_TYPE_STOP;
+			}
+		}
+		i++;
+	}
+
+	filters->nr_filters = i;
+}
+
 int etm_perf_symlink(struct coresight_device *csdev, bool link)
 {
 	char entry[sizeof("cpu9999999")];
@@ -485,21 +597,23 @@ static int __init etm_perf_init(void)
 {
 	int ret;
 
-	etm_pmu.capabilities	= PERF_PMU_CAP_EXCLUSIVE;
-
-	etm_pmu.attr_groups	= etm_pmu_attr_groups;
-	etm_pmu.task_ctx_nr	= perf_sw_context;
-	etm_pmu.read		= etm_event_read;
-	etm_pmu.event_init	= etm_event_init;
-	etm_pmu.setup_aux	= etm_setup_aux;
-	etm_pmu.free_aux	= etm_free_aux;
-	etm_pmu.start		= etm_event_start;
-	etm_pmu.stop		= etm_event_stop;
-	etm_pmu.add		= etm_event_add;
-	etm_pmu.del		= etm_event_del;
-	etm_pmu.get_drv_configs	= etm_get_drv_configs;
-	etm_pmu.free_drv_configs
-				= etm_free_drv_configs;
+	etm_pmu.capabilities		= PERF_PMU_CAP_EXCLUSIVE;
+
+	etm_pmu.attr_groups		= etm_pmu_attr_groups;
+	etm_pmu.task_ctx_nr		= perf_sw_context;
+	etm_pmu.read			= etm_event_read;
+	etm_pmu.event_init		= etm_event_init;
+	etm_pmu.setup_aux		= etm_setup_aux;
+	etm_pmu.free_aux		= etm_free_aux;
+	etm_pmu.start			= etm_event_start;
+	etm_pmu.stop			= etm_event_stop;
+	etm_pmu.add			= etm_event_add;
+	etm_pmu.del			= etm_event_del;
+	etm_pmu.get_drv_configs		= etm_get_drv_configs;
+	etm_pmu.free_drv_configs	= etm_free_drv_configs;
+	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;
 
 	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 87f5a134eb6f..28be38a9be60 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.h
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.h
@@ -20,6 +20,38 @@
 
 struct coresight_device;
 
+/*
+ * In both ETMv3 and v4 the maximum number of address comparator implentable
+ * is 8.  The actual number is implementation specific and will be checked
+ * when filters are applied.
+ */
+#define ETM_ADDR_CMP_MAX	8
+
+/**
+ * struct etm_filter - single instruction range or start/stop configuration.
+ * @start_addr:	The address to start tracing on.
+ * @stop_addr:	The address to stop tracing on.
+ * @type:	Is this a range or start/stop filter.
+ */
+struct etm_filter {
+	unsigned long start_addr;
+	unsigned long stop_addr;
+	enum etm_addr_type type;
+};
+
+/**
+ * struct etm_filters - set of filters for a session
+ * @etm_filter:	All the filters for this session.
+ * @nr_filters:	Number of filters
+ * @ssstatus:	Status of the start/stop logic.
+ */
+struct etm_filters {
+	struct etm_filter	filter[ETM_ADDR_CMP_MAX];
+	unsigned int		nr_filters;
+	bool			ssstatus;
+};
+
+
 #ifdef CONFIG_CORESIGHT
 int etm_perf_symlink(struct coresight_device *csdev, bool link);
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 04/10] coresight: etm4x: split default and filter configuration
  2016-07-18 19:51 [PATCH 00/10] coresight: implementing address filtering Mathieu Poirier
                   ` (2 preceding siblings ...)
  2016-07-18 19:51 ` [PATCH 03/10] coresight: etm-perf: configuring filters from perf core Mathieu Poirier
@ 2016-07-18 19:51 ` Mathieu Poirier
  2016-07-18 19:51 ` [PATCH 05/10] coresight: etm4x: cleaning up default " Mathieu Poirier
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2016-07-18 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

Splitting the steps involved in the configuration of a tracer.
The first part is generic and can be reused for both sysFS and
Perf methods.

The second part pertains to the configuration of filters
themselves where the source of the information used to
configure the filters will vary depending on the access
methods.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm4x.c | 36 ++++++++++++++++-----------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 1fe1f0e86daf..9978cf7a5600 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -565,21 +565,8 @@ static void etm4_init_arch_data(void *info)
 	CS_LOCK(drvdata->base);
 }
 
-static void etm4_set_default(struct etmv4_config *config)
+static void etm4_set_default_config(struct etmv4_config *config)
 {
-	if (WARN_ON_ONCE(!config))
-		return;
-
-	/*
-	 * Make default initialisation trace everything
-	 *
-	 * Select the "always true" resource selector on the
-	 * "Enablign Event" line and configure address range comparator
-	 * '0' to trace all the possible address range.  From there
-	 * configure the "include/exclude" engine to include address
-	 * range comparator '0'.
-	 */
-
 	/* disable all events tracing */
 	config->eventctrl0 = 0x0;
 	config->eventctrl1 = 0x0;
@@ -595,7 +582,10 @@ static void etm4_set_default(struct etmv4_config *config)
 
 	/* TRCVICTLR::EVENT = 0x01, select the always on logic */
 	config->vinst_ctrl |= BIT(0);
+}
 
+static void etm4_set_default_filter(struct etmv4_config *config)
+{
 	/*
 	 * TRCVICTLR::SSSTATUS == 1, the start-stop logic is
 	 * in the started state
@@ -641,6 +631,24 @@ static void etm4_set_default(struct etmv4_config *config)
 	config->vissctlr = 0x0;
 }
 
+static void etm4_set_default(struct etmv4_config *config)
+{
+	if (WARN_ON_ONCE(!config))
+		return;
+
+	/*
+	 * Make default initialisation trace everything
+	 *
+	 * Select the "always true" resource selector on the
+	 * "Enablign Event" line and configure address range comparator
+	 * '0' to trace all the possible address range.  From there
+	 * configure the "include/exclude" engine to include address
+	 * range comparator '0'.
+	 */
+	etm4_set_default_config(config);
+	etm4_set_default_filter(config);
+}
+
 void etm4_config_trace_mode(struct etmv4_config *config)
 {
 	u32 addr_acc, mode;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 05/10] coresight: etm4x: cleaning up default filter configuration
  2016-07-18 19:51 [PATCH 00/10] coresight: implementing address filtering Mathieu Poirier
                   ` (3 preceding siblings ...)
  2016-07-18 19:51 ` [PATCH 04/10] coresight: etm4x: split default and filter configuration Mathieu Poirier
@ 2016-07-18 19:51 ` Mathieu Poirier
  2016-07-18 19:51 ` [PATCH 06/10] coresight: etm4x: adding range filter configuration function Mathieu Poirier
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2016-07-18 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

The default filter configuration was hard to read and included
some redundancy.  This patch attempts to stream line configuration
and improve readability.

No change of functionality is included.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm4x.c | 53 ++++++++++++++-------------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 9978cf7a5600..e348c18086f5 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -586,39 +586,34 @@ static void etm4_set_default_config(struct etmv4_config *config)
 
 static void etm4_set_default_filter(struct etmv4_config *config)
 {
-	/*
-	 * TRCVICTLR::SSSTATUS == 1, the start-stop logic is
-	 * in the started state
-	 */
-	config->vinst_ctrl |= BIT(9);
+	u64 start, stop, access_type = 0;
 
 	/*
 	 * Configure address range comparator '0' to encompass all
 	 * possible addresses.
 	 */
+	start = 0x0;
+	stop = ~0x0;
 
-	/* First half of default address comparator: start at address 0 */
-	config->addr_val[ETM_DEFAULT_ADDR_COMP] = 0x0;
-	/* trace instruction addresses */
-	config->addr_acc[ETM_DEFAULT_ADDR_COMP] &= ~(BIT(0) | BIT(1));
-	/* EXLEVEL_NS, bits[12:15], only trace application and kernel space */
-	config->addr_acc[ETM_DEFAULT_ADDR_COMP] |= ETM_EXLEVEL_NS_HYP;
-	/* EXLEVEL_S, bits[11:8], don't trace anything in secure state */
-	config->addr_acc[ETM_DEFAULT_ADDR_COMP] |= (ETM_EXLEVEL_S_APP |
-						    ETM_EXLEVEL_S_OS |
-						    ETM_EXLEVEL_S_HYP);
-	config->addr_type[ETM_DEFAULT_ADDR_COMP] = ETM_ADDR_TYPE_RANGE;
+	/* EXLEVEL_NS, bits[12:15], always stay away from hypervisor mode. */
+	access_type = ETM_EXLEVEL_NS_HYP;
 
 	/*
-	 * Second half of default address comparator: go all
-	 * the way to the top.
-	*/
-	config->addr_val[ETM_DEFAULT_ADDR_COMP + 1] = ~0x0;
-	/* trace instruction addresses */
-	config->addr_acc[ETM_DEFAULT_ADDR_COMP + 1] &= ~(BIT(0) | BIT(1));
-	/* Address comparator type must be equal for both halves */
-	config->addr_acc[ETM_DEFAULT_ADDR_COMP + 1] =
-					config->addr_acc[ETM_DEFAULT_ADDR_COMP];
+	 * EXLEVEL_S, bits[11:8], don't trace anything happening
+	 * in secure state.
+	 */
+	access_type |= (ETM_EXLEVEL_S_APP	|
+			ETM_EXLEVEL_S_OS	|
+			ETM_EXLEVEL_S_HYP);
+
+	/* First half of default address comparator */
+	config->addr_val[ETM_DEFAULT_ADDR_COMP] = start;
+	config->addr_acc[ETM_DEFAULT_ADDR_COMP] = access_type;
+	config->addr_type[ETM_DEFAULT_ADDR_COMP] = ETM_ADDR_TYPE_RANGE;
+
+	/* Second half of default address comparator */
+	config->addr_val[ETM_DEFAULT_ADDR_COMP + 1] = stop;
+	config->addr_acc[ETM_DEFAULT_ADDR_COMP + 1] = access_type;
 	config->addr_type[ETM_DEFAULT_ADDR_COMP + 1] = ETM_ADDR_TYPE_RANGE;
 
 	/*
@@ -627,7 +622,13 @@ static void etm4_set_default_filter(struct etmv4_config *config)
 	 */
 	config->viiectlr = BIT(0);
 
-	/* no start-stop filtering for ViewInst */
+	/*
+	 * TRCVICTLR::SSSTATUS == 1, the start-stop logic is
+	 * in the started state
+	 */
+	config->vinst_ctrl |= BIT(9);
+
+	/* No start-stop filtering for ViewInst */
 	config->vissctlr = 0x0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 06/10] coresight: etm4x: adding range filter configuration function
  2016-07-18 19:51 [PATCH 00/10] coresight: implementing address filtering Mathieu Poirier
                   ` (4 preceding siblings ...)
  2016-07-18 19:51 ` [PATCH 05/10] coresight: etm4x: cleaning up default " Mathieu Poirier
@ 2016-07-18 19:51 ` Mathieu Poirier
  2016-07-18 19:51 ` [PATCH 07/10] coresight: etm4x: configuring include/exclude function Mathieu Poirier
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2016-07-18 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

Introducing a new function to do address range configuration
generic enough to work for any address range and any comparator.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm4x.c | 56 ++++++++++++++++++---------
 1 file changed, 38 insertions(+), 18 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index e348c18086f5..4a4d2ef35ad6 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -584,16 +584,10 @@ static void etm4_set_default_config(struct etmv4_config *config)
 	config->vinst_ctrl |= BIT(0);
 }
 
-static void etm4_set_default_filter(struct etmv4_config *config)
+static void etm4_set_comparator_filter(struct etmv4_config *config,
+				       u64 start, u64 stop, int comparator)
 {
-	u64 start, stop, access_type = 0;
-
-	/*
-	 * Configure address range comparator '0' to encompass all
-	 * possible addresses.
-	 */
-	start = 0x0;
-	stop = ~0x0;
+	u64 access_type = 0;
 
 	/* EXLEVEL_NS, bits[12:15], always stay away from hypervisor mode. */
 	access_type = ETM_EXLEVEL_NS_HYP;
@@ -607,20 +601,46 @@ static void etm4_set_default_filter(struct etmv4_config *config)
 			ETM_EXLEVEL_S_HYP);
 
 	/* First half of default address comparator */
-	config->addr_val[ETM_DEFAULT_ADDR_COMP] = start;
-	config->addr_acc[ETM_DEFAULT_ADDR_COMP] = access_type;
-	config->addr_type[ETM_DEFAULT_ADDR_COMP] = ETM_ADDR_TYPE_RANGE;
+	config->addr_val[comparator] = start;
+	config->addr_acc[comparator] = access_type;
+	config->addr_type[comparator] = ETM_ADDR_TYPE_RANGE;
 
 	/* Second half of default address comparator */
-	config->addr_val[ETM_DEFAULT_ADDR_COMP + 1] = stop;
-	config->addr_acc[ETM_DEFAULT_ADDR_COMP + 1] = access_type;
-	config->addr_type[ETM_DEFAULT_ADDR_COMP + 1] = ETM_ADDR_TYPE_RANGE;
+	config->addr_val[comparator + 1] = stop;
+	config->addr_acc[comparator + 1] = access_type;
+	config->addr_type[comparator + 1] = ETM_ADDR_TYPE_RANGE;
+
+	/*
+	 * Configure the ViewInst function to include this address range
+	 * comparator.
+	 *
+	 * @comparator is divided by two since it is the index in the
+	 * etmv4_config::addr_val array but register TRCVIIECTLR deals with
+	 * address range comparator _pairs_.
+	 *
+	 * Therefore:
+	 *	index 0 -> compatator pair 0
+	 *	index 2 -> comparator pair 1
+	 *	index 4 -> comparator pair 2
+	 *	...
+	 *	index 14 -> comparator pair 7
+	 */
+	config->viiectlr |= BIT(comparator / 2);
+}
+
+static void etm4_set_default_filter(struct etmv4_config *config)
+{
+	u64 start, stop;
 
 	/*
-	 * Configure the ViewInst function to filter on address range
-	 * comparator '0'.
+	 * Configure address range comparator '0' to encompass all
+	 * possible addresses.
 	 */
-	config->viiectlr = BIT(0);
+	start = 0x0;
+	stop = ~0x0;
+
+	etm4_set_comparator_filter(config, start, stop,
+				   ETM_DEFAULT_ADDR_COMP);
 
 	/*
 	 * TRCVICTLR::SSSTATUS == 1, the start-stop logic is
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 07/10] coresight: etm4x: configuring include/exclude function
  2016-07-18 19:51 [PATCH 00/10] coresight: implementing address filtering Mathieu Poirier
                   ` (5 preceding siblings ...)
  2016-07-18 19:51 ` [PATCH 06/10] coresight: etm4x: adding range filter configuration function Mathieu Poirier
@ 2016-07-18 19:51 ` Mathieu Poirier
  2016-07-18 19:51 ` [PATCH 08/10] coresight: etm4x: adding configurable address range filtering Mathieu Poirier
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2016-07-18 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

The include/exclude function of a tracer is applicable to address
range and start/stop filters.  To avoid duplication and reuse code
moving the include/exclude configuration to a function of its own.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm4x.c | 35 +++++++++++++++++++--------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 4a4d2ef35ad6..9eba6d8f4633 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -213,13 +213,6 @@ static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,
 	/* Always start from the default config */
 	etm4_set_default(config);
 
-	/*
-	 * By default the tracers are configured to trace the whole address
-	 * range.  Narrow the field only if requested by user space.
-	 */
-	if (config->mode)
-		etm4_config_trace_mode(config);
-
 	/* Go from generic option to ETMv4 specifics */
 	if (attr->config & BIT(ETM_OPT_CYCACC))
 		config->cfg |= ETMv4_MODE_CYCACC;
@@ -584,14 +577,28 @@ static void etm4_set_default_config(struct etmv4_config *config)
 	config->vinst_ctrl |= BIT(0);
 }
 
-static void etm4_set_comparator_filter(struct etmv4_config *config,
-				       u64 start, u64 stop, int comparator)
+static u64 etm4_get_access_type(struct etmv4_config *config)
 {
 	u64 access_type = 0;
 
-	/* EXLEVEL_NS, bits[12:15], always stay away from hypervisor mode. */
+	/*
+	 * EXLEVEL_NS, bits[15:12]
+	 * The Exception levels are:
+	 *   Bit[12] Exception level 0 - Application
+	 *   Bit[13] Exception level 1 - OS
+	 *   Bit[14] Exception level 2 - Hypervisor
+	 *   Bit[15] Never implemented
+	 *
+	 * Always stay away from hypervisor mode.
+	 */
 	access_type = ETM_EXLEVEL_NS_HYP;
 
+	if (config->mode & ETM_MODE_EXCL_KERN)
+		access_type |= ETM_EXLEVEL_NS_OS;
+
+	if (config->mode & ETM_MODE_EXCL_USER)
+		access_type |= ETM_EXLEVEL_NS_APP;
+
 	/*
 	 * EXLEVEL_S, bits[11:8], don't trace anything happening
 	 * in secure state.
@@ -600,6 +607,14 @@ static void etm4_set_comparator_filter(struct etmv4_config *config,
 			ETM_EXLEVEL_S_OS	|
 			ETM_EXLEVEL_S_HYP);
 
+	return access_type;
+}
+
+static void etm4_set_comparator_filter(struct etmv4_config *config,
+				       u64 start, u64 stop, int comparator)
+{
+	u64 access_type = etm4_get_access_type(config);
+
 	/* First half of default address comparator */
 	config->addr_val[comparator] = start;
 	config->addr_acc[comparator] = access_type;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 08/10] coresight: etm4x: adding configurable address range filtering
  2016-07-18 19:51 [PATCH 00/10] coresight: implementing address filtering Mathieu Poirier
                   ` (6 preceding siblings ...)
  2016-07-18 19:51 ` [PATCH 07/10] coresight: etm4x: configuring include/exclude function Mathieu Poirier
@ 2016-07-18 19:51 ` Mathieu Poirier
  2016-07-18 19:51 ` [PATCH 09/10] coresight: etm4x: adding configurable start/stop filtering Mathieu Poirier
  2016-07-18 19:51 ` [PATCH 10/10] coresight: documenting range and " Mathieu Poirier
  9 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2016-07-18 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds the capability to specify address ranges from
the perf cmd line using the --filter option.  If the IP
falls within the range(s) program flow traces are generated.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm4x.c | 128 ++++++++++++++++++++++++--
 1 file changed, 119 insertions(+), 9 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 9eba6d8f4633..b807333ce6a1 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -46,7 +46,9 @@ module_param_named(boot_enable, boot_enable, int, S_IRUGO);
 /* The number of ETMv4 currently registered */
 static int etm4_count;
 static struct etmv4_drvdata *etmdrvdata[NR_CPUS];
-static void etm4_set_default(struct etmv4_config *config);
+static void etm4_set_default_config(struct etmv4_config *config);
+static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
+				  struct perf_event *event);
 
 static void etm4_os_unlock(struct etmv4_drvdata *drvdata)
 {
@@ -195,11 +197,14 @@ static void etm4_enable_hw(void *info)
 static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,
 				   struct perf_event *event)
 {
+	int ret = 0;
 	struct etmv4_config *config = &drvdata->config;
 	struct perf_event_attr *attr = &event->attr;
 
-	if (!attr)
-		return -EINVAL;
+	if (!attr) {
+		ret = -EINVAL;
+		goto out;
+	}
 
 	/* Clear configuration from previous run */
 	memset(config, 0, sizeof(struct etmv4_config));
@@ -211,7 +216,12 @@ static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,
 		config->mode = ETM_MODE_EXCL_USER;
 
 	/* Always start from the default config */
-	etm4_set_default(config);
+	etm4_set_default_config(config);
+
+	/* Configure filters specified on the perf cmd line, if any. */
+	ret = etm4_set_event_filters(drvdata, event);
+	if (ret)
+		goto out;
 
 	/* Go from generic option to ETMv4 specifics */
 	if (attr->config & BIT(ETM_OPT_CYCACC))
@@ -219,23 +229,30 @@ static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,
 	if (attr->config & BIT(ETM_OPT_TS))
 		config->cfg |= ETMv4_MODE_TIMESTAMP;
 
-	return 0;
+out:
+	return ret;
 }
 
 static int etm4_enable_perf(struct coresight_device *csdev,
 			    struct perf_event *event)
 {
+	int ret = 0;
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
-	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id()))
-		return -EINVAL;
+	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) {
+		ret = -EINVAL;
+		goto out;
+	}
 
 	/* Configure the tracer based on the session's specifics */
-	etm4_parse_event_config(drvdata, event);
+	ret = etm4_parse_event_config(drvdata, event);
+	if (ret)
+		goto out;
 	/* And enable it */
 	etm4_enable_hw(drvdata);
 
-	return 0;
+out:
+	return ret;
 }
 
 static int etm4_enable_sysfs(struct coresight_device *csdev)
@@ -685,6 +702,99 @@ static void etm4_set_default(struct etmv4_config *config)
 	etm4_set_default_filter(config);
 }
 
+static int etm4_get_next_comparator(struct etmv4_drvdata *drvdata, u32 type)
+{
+	int nr_comparator, index = 0;
+	struct etmv4_config *config = &drvdata->config;
+
+	/*
+	 * nr_addr_cmp holds the number of comparator _pair_, so time 2
+	 * for the total number of comparators.
+	 */
+	nr_comparator = drvdata->nr_addr_cmp * 2;
+
+	/* Go through the tally of comparators looking for a free one. */
+	while (index < nr_comparator) {
+		switch (type) {
+		case ETM_ADDR_TYPE_RANGE:
+			if (config->addr_type[index] == ETM_ADDR_TYPE_NONE &&
+			    config->addr_type[index + 1] == ETM_ADDR_TYPE_NONE)
+				return index;
+
+			/* Address range comparators go in pairs */
+			index += 2;
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	/* If we are here all the comparators have been used. */
+	return -ENOSPC;
+}
+
+static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
+				  struct perf_event *event)
+{
+	int i, comparator, ret = 0;
+	struct etmv4_config *config = &drvdata->config;
+	struct etm_filters *filters = event->hw.addr_filters;
+
+	if (!filters)
+		goto default_filter;
+
+	/* Sync events with what Perf got */
+	perf_event_addr_filters_sync(event);
+
+	/*
+	 * If there are no filters to deal with simply go ahead with
+	 * the default filter, i.e the entire address range.
+	 */
+	if (!filters->nr_filters)
+		goto default_filter;
+
+	for (i = 0; i < filters->nr_filters; i++) {
+		struct etm_filter *filter = &filters->filter[i];
+		enum etm_addr_type type = filter->type;
+
+		/* See if a comparator is free. */
+		comparator = etm4_get_next_comparator(drvdata, type);
+		if (comparator < 0) {
+			ret = comparator;
+			goto out;
+		}
+
+		switch (type) {
+		case ETM_ADDR_TYPE_RANGE:
+			etm4_set_comparator_filter(config,
+						   filter->start_addr,
+						   filter->stop_addr,
+						   comparator);
+			/*
+			 * TRCVICTLR::SSSTATUS == 1, the start-stop logic is
+			 * in the started state
+			 */
+			config->vinst_ctrl |= BIT(9);
+
+			/* No start-stop filtering for ViewInst */
+			config->vissctlr = 0x0;
+			break;
+		default:
+			ret = -EINVAL;
+			goto out;
+		}
+	}
+
+	goto out;
+
+
+default_filter:
+	etm4_set_default_filter(config);
+
+out:
+	return ret;
+}
+
 void etm4_config_trace_mode(struct etmv4_config *config)
 {
 	u32 addr_acc, mode;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 09/10] coresight: etm4x: adding configurable start/stop filtering
  2016-07-18 19:51 [PATCH 00/10] coresight: implementing address filtering Mathieu Poirier
                   ` (7 preceding siblings ...)
  2016-07-18 19:51 ` [PATCH 08/10] coresight: etm4x: adding configurable address range filtering Mathieu Poirier
@ 2016-07-18 19:51 ` Mathieu Poirier
  2016-07-18 19:51 ` [PATCH 10/10] coresight: documenting range and " Mathieu Poirier
  9 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2016-07-18 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

With this patch we add start/stop filtering as specified on
the perf cmd line.  When the IP matches the start address
trace generation gets triggered.  The stop condition is
achieved when the IP matches the stop address.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm4x.c | 71 +++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index b807333ce6a1..58fd37021db4 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -338,12 +338,25 @@ static void etm4_disable_hw(void *info)
 static int etm4_disable_perf(struct coresight_device *csdev,
 			     struct perf_event *event)
 {
+	u32 control;
+	struct etm_filters *filters = event->hw.addr_filters;
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
 	if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id()))
 		return -EINVAL;
 
 	etm4_disable_hw(drvdata);
+
+	/*
+	 * Check if the start/stop logic was active when the unit was stopped.
+	 * That way we can re-enable the start/stop logic when the process is
+	 * scheduled again.  Configuration of the start/stop logic happens in
+	 * function etm4_set_event_filters().
+	 */
+	control = readl_relaxed(drvdata->base + TRCVICTLR);
+	/* TRCVICTLR::SSSTATUS, bit[9] */
+	filters->ssstatus = (control & BIT(9));
+
 	return 0;
 }
 
@@ -660,6 +673,27 @@ static void etm4_set_comparator_filter(struct etmv4_config *config,
 	config->viiectlr |= BIT(comparator / 2);
 }
 
+static void etm4_set_start_stop_filter(struct etmv4_config *config,
+				       u64 address, int comparator,
+				       enum etm_addr_type type)
+{
+	int shift;
+	u64 access_type = etm4_get_access_type(config);
+
+	/* Configure the comparator */
+	config->addr_val[comparator] = address;
+	config->addr_acc[comparator] = access_type;
+	config->addr_type[comparator] = type;
+
+	/*
+	 * Configure ViewInst Start-Stop control register.
+	 * Addresses configured to start tracing go from bit 0 to n-1,
+	 * while those configured to stop tracing from 16 to 16 + n-1.
+	 */
+	shift = (type == ETM_ADDR_TYPE_START ? 0 : 16);
+	config->vissctlr |= BIT(shift + comparator);
+}
+
 static void etm4_set_default_filter(struct etmv4_config *config)
 {
 	u64 start, stop;
@@ -724,6 +758,14 @@ static int etm4_get_next_comparator(struct etmv4_drvdata *drvdata, u32 type)
 			/* Address range comparators go in pairs */
 			index += 2;
 			break;
+		case ETM_ADDR_TYPE_START:
+		case ETM_ADDR_TYPE_STOP:
+			if (config->addr_type[index] == ETM_ADDR_TYPE_NONE)
+				return index;
+
+			/* Start/stop address can have odd indexes */
+			index += 1;
+			break;
 		default:
 			return -EINVAL;
 		}
@@ -737,6 +779,7 @@ static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
 				  struct perf_event *event)
 {
 	int i, comparator, ret = 0;
+	u64 address;
 	struct etmv4_config *config = &drvdata->config;
 	struct etm_filters *filters = event->hw.addr_filters;
 
@@ -779,6 +822,34 @@ static int etm4_set_event_filters(struct etmv4_drvdata *drvdata,
 			/* No start-stop filtering for ViewInst */
 			config->vissctlr = 0x0;
 			break;
+		case ETM_ADDR_TYPE_START:
+		case ETM_ADDR_TYPE_STOP:
+			/* Get the right start or stop address */
+			address = (type == ETM_ADDR_TYPE_START ?
+				   filter->start_addr :
+				   filter->stop_addr);
+
+			/* Configure comparator */
+			etm4_set_start_stop_filter(config, address,
+						   comparator, type);
+
+			/*
+			 * If filters::ssstatus == 1, trace acquisition was
+			 * started but the process was yanked away before the
+			 * the stop address was hit.  As such the start/stop
+			 * logic needs to be re-started so that tracing can
+			 * resume where it left.
+			 *
+			 * The start/stop logic status when a process is
+			 * scheduled out is checked in function
+			 * etm4_disable_perf().
+			 */
+			if (filters->ssstatus)
+				config->vinst_ctrl |= BIT(9);
+
+			/* No include/exclude filtering for ViewInst */
+			config->viiectlr = 0x0;
+			break;
 		default:
 			ret = -EINVAL;
 			goto out;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 10/10] coresight: documenting range and start/stop filtering
  2016-07-18 19:51 [PATCH 00/10] coresight: implementing address filtering Mathieu Poirier
                   ` (8 preceding siblings ...)
  2016-07-18 19:51 ` [PATCH 09/10] coresight: etm4x: adding configurable start/stop filtering Mathieu Poirier
@ 2016-07-18 19:51 ` Mathieu Poirier
  9 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2016-07-18 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

To reduce the amount of traces generated by tracers range
and start/stop address filtering has been implemented.  This
patch provides example on how to use the various options
currently supported.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: linux-doc at vger.kernel.org
---
 Documentation/trace/coresight.txt | 48 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/Documentation/trace/coresight.txt b/Documentation/trace/coresight.txt
index a2e7ccb62c2b..a19d7e60937a 100644
--- a/Documentation/trace/coresight.txt
+++ b/Documentation/trace/coresight.txt
@@ -408,6 +408,54 @@ Instruction     13570831        0x8026B584      E28DD00C        false   ADD
 Instruction     0       0x8026B588      E8BD8000        true    LDM      sp!,{pc}
 Timestamp                                       Timestamp: 17107041535
 
+Address filtering
+-----------------
+
+CoreSight tracers produce an astonishing amount of traces, more often than not
+much more than trace buffers can handle, and definilty far beyond the
+processing capabilities of humans.  To trim down the amount of trace generated
+address filters can be specified from the perf command line.
+
+Currently supported filters include range and start/stop address.  When a
+range is specified traces will be collected when the instruction pointer falls
+within the specified addresses.  For start and stop filters, traces collection
+start when the instuction pointer matches the start address, and stop when
+the stop address has been reached.
+
+Filters can be specified from the perf command line as follow:
+
+- To start trace collection when the IP equal a specific function in a binary
+  executable or library:
+
+	perf record -e cs_etm// --filter 'start 0x72c@/opt/lib/libcstest.so.1.0' --per-thread ./main
+
+- To stop trace collection when the IP equal a specific function in a binary
+  executable of library:
+
+	perf record -e cs_etm// --filter 'stop 0x790@/opt/lib/libcstest.so.1.0' --per-thread ./main
+
+- To start and stop tracing at various stages of kernel execution:
+
+	perf record -e cs_etm// --filter 'start 0xffffff800856bc50,stop 0xffffff800856bcb0,start 0xffffff8008562d0c,stop 0xffffff8008562d30' --per-thread uname
+
+The only requirement on the number of start/stop filters is that they don't go
+beyond the amount of comparator implemented on the system.  Range filters are
+specified using a slightly different syntax:
+
+- To collect traces in a specific range in a binary executable or a library:
+
+	perf record -e cs_etm// --filter 'filter 0x72c/0x40@/opt/lib/libcstest.so.1.0' --per-thread ./main
+
+- To collect traces in a specific range in kernel space:
+
+	perf record -e cs_etm// --filter 'filter 0xffffff8008562d0c/0x48' --per-thread  uname
+
+This time the number of comparator _pairs_ implemented in the trace engine is
+the only restriction on the amount of filters that can be specified.
+
+It is possible to mix user and kernel space filters, as long as they are the
+same type.  Mixing range and start/stop filters is currently not supported.
+
 How to use the STM module
 -------------------------
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 01/10] coresight: etm-perf: pass struct perf_event to source::enable/disable()
  2016-07-18 19:51 ` [PATCH 01/10] coresight: etm-perf: pass struct perf_event to source::enable/disable() Mathieu Poirier
@ 2016-07-20 15:34   ` Suzuki K Poulose
  2016-07-21 15:23     ` Mathieu Poirier
  0 siblings, 1 reply; 16+ messages in thread
From: Suzuki K Poulose @ 2016-07-20 15:34 UTC (permalink / raw)
  To: linux-arm-kernel

On 18/07/16 20:51, Mathieu Poirier wrote:
> With this commit [1] address range filter information is now found
> in the struct hw_perf_event::addr_filters.  As such pass the event
> itself to the coresight_source::enable/disable() functions so that
> both event attribute and filter can be accessible for configuration.
>
> [1] 'commit 375637bc5249 ("perf/core: Introduce address range filtering")'

> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 385d62e64abb..2a5982c37dfb 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -232,8 +232,9 @@ struct coresight_ops_source {
>  	int (*cpu_id)(struct coresight_device *csdev);
>  	int (*trace_id)(struct coresight_device *csdev);
>  	int (*enable)(struct coresight_device *csdev,
> -		      struct perf_event_attr *attr,  u32 mode);
> -	void (*disable)(struct coresight_device *csdev);
> +		      struct perf_event *event,  u32 mode);
> +	void (*disable)(struct coresight_device *csdev,
> +			struct perf_event *event);

nit:

Should we make this a a bit more generic API rather than hard coding
the perf stuff in there ? i.e,

how about :

int (*enable)(struct coresight_device *csdev, void *data, u32 mode)

void (*disable)(struct coresight_device *csdev, void *data, u32 mode)

where data is specific to the mode of operation. That way the API is
cleaner and each mode could pass their own data (even though sysfs
doesn't use any at the moment).

Suzuki

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 03/10] coresight: etm-perf: configuring filters from perf core
  2016-07-18 19:51 ` [PATCH 03/10] coresight: etm-perf: configuring filters from perf core Mathieu Poirier
@ 2016-07-20 16:07   ` Suzuki K Poulose
  2016-07-21 15:15     ` Mathieu Poirier
  0 siblings, 1 reply; 16+ messages in thread
From: Suzuki K Poulose @ 2016-07-20 16:07 UTC (permalink / raw)
  To: linux-arm-kernel

On 18/07/16 20:51, Mathieu Poirier wrote:
> This patch implements the required API needed to access
> and retrieve range and start/stop filters from the perf core.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  drivers/hwtracing/coresight/coresight-etm-perf.c | 146 ++++++++++++++++++++---
>  drivers/hwtracing/coresight/coresight-etm-perf.h |  32 +++++
>  2 files changed, 162 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index 78a1bc0013a2..fde7f42149c5 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -29,6 +29,7 @@
>  #include <linux/workqueue.h>
>
>  #include "coresight-priv.h"
> +#include "coresight-etm-perf.h"
>
>  static struct pmu etm_pmu;
>  static bool etm_perf_up;
> @@ -83,12 +84,44 @@ static const struct attribute_group *etm_pmu_attr_groups[] = {
>
>  static void etm_event_read(struct perf_event *event) {}
>
> +static int etm_addr_filters_alloc(struct perf_event *event)
> +{

...

> +	return 0;
> +}
> +


> +
>  static int etm_event_init(struct perf_event *event)
>  {
> +	int ret;
> +
>  	if (event->attr.type != etm_pmu.type)
>  		return -ENOENT;
>
> -	return 0;
> +	ret = etm_addr_filters_alloc(event);


>  }
>
>  static void free_event_data(struct work_struct *work)
> @@ -456,6 +489,85 @@ static void etm_free_drv_configs(struct perf_event *event)
>  	}
>  }
>
> +static int etm_addr_filters_validate(struct list_head *filters)
> +{

> +
> +	return 0;
> +}
> +
> +static void etm_addr_filters_sync(struct perf_event *event)
> +{
> +	struct perf_addr_filters_head *head = perf_event_addr_filters(event);
> +	unsigned long start, stop, *offs = event->addr_filters_offs;
> +	struct etm_filters *filters = event->hw.addr_filters;
> +	struct perf_addr_filter *filter;
> +	int i = 0;

Is it possible to delay the etm_addr_filters_alloc() until this point ?
I understand that this function cannot report back failures if we fail
to allocate memory. Or may be do a lazy allocation from addr_filters_validate(),
when we get the first filter added.

Of course this could be done as a follow up patch to improve things once
we get the initial framework in.



> +
> +	list_for_each_entry(filter, &head->list, entry) {
> +		start = filter->offset + offs[i];
> +		stop = start + filter->size;
> +
> +		if (filter->range == 1) {
> +			filters->filter[i].start_addr = start;
> +			filters->filter[i].stop_addr = stop;
> +			filters->filter[i].type = ETM_ADDR_TYPE_RANGE;
> +		} else {
> +			if (filter->filter == 1) {
> +				filters->filter[i].start_addr = start;
> +				filters->filter[i].type = ETM_ADDR_TYPE_START;
> +			} else {
> +				filters->filter[i].stop_addr = stop;
> +				filters->filter[i].type = ETM_ADDR_TYPE_STOP;
> +			}
> +		}
> +		i++;
> +	}
> +
> +	filters->nr_filters = i;
> +/**
> + * struct etm_filters - set of filters for a session
> + * @etm_filter:	All the filters for this session.
> + * @nr_filters:	Number of filters
> + * @ssstatus:	Status of the start/stop logic.
> + */
> +struct etm_filters {
> +	struct etm_filter	filter[ETM_ADDR_CMP_MAX];

nit: having the variable renamed to etm_filter will make the code a bit more readable
where we populate/validate the filters.

Otherwise looks good

Suzuki

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 03/10] coresight: etm-perf: configuring filters from perf core
  2016-07-20 16:07   ` Suzuki K Poulose
@ 2016-07-21 15:15     ` Mathieu Poirier
  2016-07-21 17:12       ` Mathieu Poirier
  0 siblings, 1 reply; 16+ messages in thread
From: Mathieu Poirier @ 2016-07-21 15:15 UTC (permalink / raw)
  To: linux-arm-kernel

On 20 July 2016 at 10:07, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 18/07/16 20:51, Mathieu Poirier wrote:
>>
>> This patch implements the required API needed to access
>> and retrieve range and start/stop filters from the perf core.
>>
>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> ---
>>  drivers/hwtracing/coresight/coresight-etm-perf.c | 146
>> ++++++++++++++++++++---
>>  drivers/hwtracing/coresight/coresight-etm-perf.h |  32 +++++
>>  2 files changed, 162 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c
>> b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> index 78a1bc0013a2..fde7f42149c5 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> @@ -29,6 +29,7 @@
>>  #include <linux/workqueue.h>
>>
>>  #include "coresight-priv.h"
>> +#include "coresight-etm-perf.h"
>>
>>  static struct pmu etm_pmu;
>>  static bool etm_perf_up;
>> @@ -83,12 +84,44 @@ static const struct attribute_group
>> *etm_pmu_attr_groups[] = {
>>
>>  static void etm_event_read(struct perf_event *event) {}
>>
>> +static int etm_addr_filters_alloc(struct perf_event *event)
>> +{
>
>
> ...
>
>> +       return 0;
>> +}
>> +
>
>
>
>> +
>>  static int etm_event_init(struct perf_event *event)
>>  {
>> +       int ret;
>> +
>>         if (event->attr.type != etm_pmu.type)
>>                 return -ENOENT;
>>
>> -       return 0;
>> +       ret = etm_addr_filters_alloc(event);
>
>
>
>>  }
>>
>>  static void free_event_data(struct work_struct *work)
>> @@ -456,6 +489,85 @@ static void etm_free_drv_configs(struct perf_event
>> *event)
>>         }
>>  }
>>
>> +static int etm_addr_filters_validate(struct list_head *filters)
>> +{
>
>
>> +
>> +       return 0;
>> +}
>> +
>> +static void etm_addr_filters_sync(struct perf_event *event)
>> +{
>> +       struct perf_addr_filters_head *head =
>> perf_event_addr_filters(event);
>> +       unsigned long start, stop, *offs = event->addr_filters_offs;
>> +       struct etm_filters *filters = event->hw.addr_filters;
>> +       struct perf_addr_filter *filter;
>> +       int i = 0;
>
>
> Is it possible to delay the etm_addr_filters_alloc() until this point ?
> I understand that this function cannot report back failures if we fail
> to allocate memory. Or may be do a lazy allocation from
> addr_filters_validate(),
> when we get the first filter added.

Humm... You want to avoid allocating memory that may never be used if
filters aren't specified.  Ok, let's do the allocation in
addr_filters_validate().

>
> Of course this could be done as a follow up patch to improve things once
> we get the initial framework in.
>
>
>
>> +
>> +       list_for_each_entry(filter, &head->list, entry) {
>> +               start = filter->offset + offs[i];
>> +               stop = start + filter->size;
>> +
>> +               if (filter->range == 1) {
>> +                       filters->filter[i].start_addr = start;
>> +                       filters->filter[i].stop_addr = stop;
>> +                       filters->filter[i].type = ETM_ADDR_TYPE_RANGE;
>> +               } else {
>> +                       if (filter->filter == 1) {
>> +                               filters->filter[i].start_addr = start;
>> +                               filters->filter[i].type =
>> ETM_ADDR_TYPE_START;
>> +                       } else {
>> +                               filters->filter[i].stop_addr = stop;
>> +                               filters->filter[i].type =
>> ETM_ADDR_TYPE_STOP;
>> +                       }
>> +               }
>> +               i++;
>> +       }
>> +
>> +       filters->nr_filters = i;
>> +/**
>> + * struct etm_filters - set of filters for a session
>> + * @etm_filter:        All the filters for this session.
>> + * @nr_filters:        Number of filters
>> + * @ssstatus:  Status of the start/stop logic.
>> + */
>> +struct etm_filters {
>> +       struct etm_filter       filter[ETM_ADDR_CMP_MAX];
>
>
> nit: having the variable renamed to etm_filter will make the code a bit more
> readable
> where we populate/validate the filters.

Very well.

Thanks,
Mathieu

>
> Otherwise looks good
>
> Suzuki

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 01/10] coresight: etm-perf: pass struct perf_event to source::enable/disable()
  2016-07-20 15:34   ` Suzuki K Poulose
@ 2016-07-21 15:23     ` Mathieu Poirier
  0 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2016-07-21 15:23 UTC (permalink / raw)
  To: linux-arm-kernel

On 20 July 2016 at 09:34, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 18/07/16 20:51, Mathieu Poirier wrote:
>>
>> With this commit [1] address range filter information is now found
>> in the struct hw_perf_event::addr_filters.  As such pass the event
>> itself to the coresight_source::enable/disable() functions so that
>> both event attribute and filter can be accessible for configuration.
>>
>> [1] 'commit 375637bc5249 ("perf/core: Introduce address range filtering")'
>
>
>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>> index 385d62e64abb..2a5982c37dfb 100644
>> --- a/include/linux/coresight.h
>> +++ b/include/linux/coresight.h
>> @@ -232,8 +232,9 @@ struct coresight_ops_source {
>>         int (*cpu_id)(struct coresight_device *csdev);
>>         int (*trace_id)(struct coresight_device *csdev);
>>         int (*enable)(struct coresight_device *csdev,
>> -                     struct perf_event_attr *attr,  u32 mode);
>> -       void (*disable)(struct coresight_device *csdev);
>> +                     struct perf_event *event,  u32 mode);
>> +       void (*disable)(struct coresight_device *csdev,
>> +                       struct perf_event *event);
>
>
> nit:
>
> Should we make this a a bit more generic API rather than hard coding
> the perf stuff in there ? i.e,
>
> how about :
>
> int (*enable)(struct coresight_device *csdev, void *data, u32 mode)
>
> void (*disable)(struct coresight_device *csdev, void *data, u32 mode)
>
> where data is specific to the mode of operation. That way the API is
> cleaner and each mode could pass their own data (even though sysfs
> doesn't use any at the moment).

We've been faced with the dilemma of writing code that may cater to
future extensions or stick with the right code for the current
situation a couple of times already.  Each time we've opted for the
latter, something I would be inclined to continue doing here.

If need be further decoupling of the perf and sysFS access methods
could be achieved by using specific enable/disable ops for each mode,
i.e enable/disable_perf() and enable/disable_sysfs() but we are not
there yet.

Thanks,
Mathieu


>
> Suzuki

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 03/10] coresight: etm-perf: configuring filters from perf core
  2016-07-21 15:15     ` Mathieu Poirier
@ 2016-07-21 17:12       ` Mathieu Poirier
  0 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2016-07-21 17:12 UTC (permalink / raw)
  To: linux-arm-kernel

On 21 July 2016 at 09:15, Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> On 20 July 2016 at 10:07, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
>> On 18/07/16 20:51, Mathieu Poirier wrote:
>>>
>>> This patch implements the required API needed to access
>>> and retrieve range and start/stop filters from the perf core.
>>>
>>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> ---
>>>  drivers/hwtracing/coresight/coresight-etm-perf.c | 146
>>> ++++++++++++++++++++---
>>>  drivers/hwtracing/coresight/coresight-etm-perf.h |  32 +++++
>>>  2 files changed, 162 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c
>>> b/drivers/hwtracing/coresight/coresight-etm-perf.c
>>> index 78a1bc0013a2..fde7f42149c5 100644
>>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>>> @@ -29,6 +29,7 @@
>>>  #include <linux/workqueue.h>
>>>
>>>  #include "coresight-priv.h"
>>> +#include "coresight-etm-perf.h"
>>>
>>>  static struct pmu etm_pmu;
>>>  static bool etm_perf_up;
>>> @@ -83,12 +84,44 @@ static const struct attribute_group
>>> *etm_pmu_attr_groups[] = {
>>>
>>>  static void etm_event_read(struct perf_event *event) {}
>>>
>>> +static int etm_addr_filters_alloc(struct perf_event *event)
>>> +{
>>
>>
>> ...
>>
>>> +       return 0;
>>> +}
>>> +
>>
>>
>>
>>> +
>>>  static int etm_event_init(struct perf_event *event)
>>>  {
>>> +       int ret;
>>> +
>>>         if (event->attr.type != etm_pmu.type)
>>>                 return -ENOENT;
>>>
>>> -       return 0;
>>> +       ret = etm_addr_filters_alloc(event);
>>
>>
>>
>>>  }
>>>
>>>  static void free_event_data(struct work_struct *work)
>>> @@ -456,6 +489,85 @@ static void etm_free_drv_configs(struct perf_event
>>> *event)
>>>         }
>>>  }
>>>
>>> +static int etm_addr_filters_validate(struct list_head *filters)
>>> +{
>>
>>
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static void etm_addr_filters_sync(struct perf_event *event)
>>> +{
>>> +       struct perf_addr_filters_head *head =
>>> perf_event_addr_filters(event);
>>> +       unsigned long start, stop, *offs = event->addr_filters_offs;
>>> +       struct etm_filters *filters = event->hw.addr_filters;
>>> +       struct perf_addr_filter *filter;
>>> +       int i = 0;
>>
>>
>> Is it possible to delay the etm_addr_filters_alloc() until this point ?
>> I understand that this function cannot report back failures if we fail
>> to allocate memory. Or may be do a lazy allocation from
>> addr_filters_validate(),
>> when we get the first filter added.
>
> Humm... You want to avoid allocating memory that may never be used if
> filters aren't specified.  Ok, let's do the allocation in
> addr_filters_validate().
>

On second thought we don't have access to the perf event in
addr_filters_validate() so the previous strategy won't work.  And as
you said etm_addr_filters_sync() doesn't return an error code so that
won't work either.  As such I will keep the current code as is.

Mathieu

>>
>> Of course this could be done as a follow up patch to improve things once
>> we get the initial framework in.
>>
>>
>>
>>> +
>>> +       list_for_each_entry(filter, &head->list, entry) {
>>> +               start = filter->offset + offs[i];
>>> +               stop = start + filter->size;
>>> +
>>> +               if (filter->range == 1) {
>>> +                       filters->filter[i].start_addr = start;
>>> +                       filters->filter[i].stop_addr = stop;
>>> +                       filters->filter[i].type = ETM_ADDR_TYPE_RANGE;
>>> +               } else {
>>> +                       if (filter->filter == 1) {
>>> +                               filters->filter[i].start_addr = start;
>>> +                               filters->filter[i].type =
>>> ETM_ADDR_TYPE_START;
>>> +                       } else {
>>> +                               filters->filter[i].stop_addr = stop;
>>> +                               filters->filter[i].type =
>>> ETM_ADDR_TYPE_STOP;
>>> +                       }
>>> +               }
>>> +               i++;
>>> +       }
>>> +
>>> +       filters->nr_filters = i;
>>> +/**
>>> + * struct etm_filters - set of filters for a session
>>> + * @etm_filter:        All the filters for this session.
>>> + * @nr_filters:        Number of filters
>>> + * @ssstatus:  Status of the start/stop logic.
>>> + */
>>> +struct etm_filters {
>>> +       struct etm_filter       filter[ETM_ADDR_CMP_MAX];
>>
>>
>> nit: having the variable renamed to etm_filter will make the code a bit more
>> readable
>> where we populate/validate the filters.
>
> Very well.
>
> Thanks,
> Mathieu
>
>>
>> Otherwise looks good
>>
>> Suzuki

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-07-21 17:12 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-18 19:51 [PATCH 00/10] coresight: implementing address filtering Mathieu Poirier
2016-07-18 19:51 ` [PATCH 01/10] coresight: etm-perf: pass struct perf_event to source::enable/disable() Mathieu Poirier
2016-07-20 15:34   ` Suzuki K Poulose
2016-07-21 15:23     ` Mathieu Poirier
2016-07-18 19:51 ` [PATCH 02/10] coresight: remove duplicated enumeration Mathieu Poirier
2016-07-18 19:51 ` [PATCH 03/10] coresight: etm-perf: configuring filters from perf core Mathieu Poirier
2016-07-20 16:07   ` Suzuki K Poulose
2016-07-21 15:15     ` Mathieu Poirier
2016-07-21 17:12       ` Mathieu Poirier
2016-07-18 19:51 ` [PATCH 04/10] coresight: etm4x: split default and filter configuration Mathieu Poirier
2016-07-18 19:51 ` [PATCH 05/10] coresight: etm4x: cleaning up default " Mathieu Poirier
2016-07-18 19:51 ` [PATCH 06/10] coresight: etm4x: adding range filter configuration function Mathieu Poirier
2016-07-18 19:51 ` [PATCH 07/10] coresight: etm4x: configuring include/exclude function Mathieu Poirier
2016-07-18 19:51 ` [PATCH 08/10] coresight: etm4x: adding configurable address range filtering Mathieu Poirier
2016-07-18 19:51 ` [PATCH 09/10] coresight: etm4x: adding configurable start/stop filtering Mathieu Poirier
2016-07-18 19:51 ` [PATCH 10/10] coresight: documenting range and " Mathieu Poirier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).