This series adds the support for using the tmc-etr in perf mode for storing the trace to system RAM. The ETR uses a separate buffer (double buffering) for storing the trace. This is copied back to the ring buffer when the event is stopped. We try to match the ETR buffer to the larger of perf ring buffer or the size configured for the ETR via sysfs. This allows tuning the buffer size to prevent overflows and loosing trace data, as we don't have overflow interrupt support (yet).
Applies on coresight/next
Changes since v2: - Split hotplug lock cleanup from per-cpu path management - Add support for tracing on hotplugged CPUs (based on patch by Mathieu) - Fix handling of perf mode in etb10 driver - Fix CS_UNLOCK() typo in ETR perf driver - Rename call backs from tmc_etr_<op>_perf_buffer => tmc_<op>_etr_buffer to be consistent with ETF driver - Modify the commit description for removing set_buffer call back.
Changes since v1 : - Fix path for each CPU where the event might be recorded. - Handle errors in enabling source - Remove set_buffer callback to avoid complicating the error handling. - Fix a bug in handling sysfs mode specific buffers
Changes since [0] : - Drop buffer rotation logic for etr-buf - Do not use perf ring buffer. (Add support later) - Fix handling of sink, preventing mixed modes of operation.
[0] - TMC ETR perf support - http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/574875.html
Suzuki K Poulose (13): coresight: Fix handling of sinks coresight: etb10: Fix handling of perf mode coresight: perf: Fix per cpu path management coresight: perf: Avoid unncessary CPU hotplug read lock coresight: perf: Allow tracing on hotplugged CPUs coresight: perf: Disable trace path upon source error coresight: tmc-etr: Handle driver mode specific ETR buffers coresight: tmc-etr: Relax collection of trace from sysfs mode coresight: Convert driver messages to dev_dbg coresight: perf: Remove reset_buffer call back for sinks coresight: perf: Add helper to retrieve sink configuration coresight: perf: Remove set_buffer call back coresight: etm-perf: Add support for ETR backend
.../coresight/coresight-dynamic-replicator.c | 4 +- drivers/hwtracing/coresight/coresight-etb10.c | 100 +++---- drivers/hwtracing/coresight/coresight-etm-perf.c | 134 +++++---- drivers/hwtracing/coresight/coresight-etm-perf.h | 26 ++ drivers/hwtracing/coresight/coresight-etm3x.c | 4 +- drivers/hwtracing/coresight/coresight-etm4x.c | 4 +- drivers/hwtracing/coresight/coresight-funnel.c | 4 +- drivers/hwtracing/coresight/coresight-priv.h | 2 +- drivers/hwtracing/coresight/coresight-replicator.c | 4 +- drivers/hwtracing/coresight/coresight-stm.c | 4 +- drivers/hwtracing/coresight/coresight-tmc-etf.c | 94 +++--- drivers/hwtracing/coresight/coresight-tmc-etr.c | 327 ++++++++++++++++++--- drivers/hwtracing/coresight/coresight-tmc.c | 4 +- drivers/hwtracing/coresight/coresight-tmc.h | 4 + drivers/hwtracing/coresight/coresight-tpiu.c | 6 +- drivers/hwtracing/coresight/coresight.c | 31 +- include/linux/coresight.h | 12 +- 17 files changed, 517 insertions(+), 247 deletions(-)
The coresight components could be operated either in sysfs mode or in perf mode. For some of the components, the mode of operation doesn't matter as they simply relay the data to the next component in the trace path. But for sinks, they need to be able to provide the trace data back to the user. Thus we need to make sure that "mode" is handled appropriately. e.g, the sysfs mode could have multiple sources driving the trace data, while perf mode doesn't allow sharing the sink.
The coresight_enable_sink() however doesn't really allow this check to trigger as it skips the "enable_sink" callback if the component is already enabled, irrespective of the mode. This could cause mixing of data from different modes or even same mode (in perf), if the sources are different. Also, if we fail to enable the sink while enabling a path (where sink is the first component enabled), we could end up in disabling the components in the "entire" path which were not enabled in this trial, causing disruptions in the existing trace paths.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index 3e07fd3..c0dabbd 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -132,12 +132,14 @@ static int coresight_enable_sink(struct coresight_device *csdev, u32 mode) { int ret;
- if (!csdev->enable) { - if (sink_ops(csdev)->enable) { - ret = sink_ops(csdev)->enable(csdev, mode); - if (ret) - return ret; - } + /* + * We need to make sure the "new" session is compatible with the + * existing "mode" of operation. + */ + if (sink_ops(csdev)->enable) { + ret = sink_ops(csdev)->enable(csdev, mode); + if (ret) + return ret; csdev->enable = true; }
@@ -339,8 +341,14 @@ int coresight_enable_path(struct list_head *path, u32 mode) switch (type) { case CORESIGHT_DEV_TYPE_SINK: ret = coresight_enable_sink(csdev, mode); + /* + * Sink is the first component turned on. If we + * failed to enable the sink, there are no components + * that need disabling. Disabling the path here + * would mean we could disrupt an existing session. + */ if (ret) - goto err; + goto out; break; case CORESIGHT_DEV_TYPE_SOURCE: /* sources are enabled from either sysFS or Perf */
If the ETB is already enabled in sysfs mode, the ETB reports success even if a perf mode is requested. Fix this by checking the requested mode.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-etb10.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 306119e..e5808fd 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -148,8 +148,12 @@ static int etb_enable(struct coresight_device *csdev, u32 mode) return -EBUSY;
/* Nothing to do, the tracer is already enabled. */ - if (val == CS_MODE_SYSFS) - goto out; + if (val == CS_MODE_SYSFS) { + if (mode == CS_MODE_SYSFS) + goto out; + else + return -EBUSY; + }
spin_lock_irqsave(&drvdata->spinlock, flags); etb_enable_hw(drvdata);
Hello,
On Thu, Jul 26, 2018 at 01:54:40PM +0100, Suzuki K Poulose wrote:
If the ETB is already enabled in sysfs mode, the ETB reports success even if a perf mode is requested. Fix this by checking the requested mode.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
drivers/hwtracing/coresight/coresight-etb10.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 306119e..e5808fd 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -148,8 +148,12 @@ static int etb_enable(struct coresight_device *csdev, u32 mode) return -EBUSY; /* Nothing to do, the tracer is already enabled. */
- if (val == CS_MODE_SYSFS)
goto out;
- if (val == CS_MODE_SYSFS) {
if (mode == CS_MODE_SYSFS)goto out;elsereturn -EBUSY;- }
How about:
if (val == CS_MODE_PERF) return -EBUSY;
+ /* Don't let perf disturb sysFS sessions */ + if (val == CS_MODE_SYSFS && mode = CS_MODE_PERF) + return -EBUSY; + /* Nothing to do, the tracer is already enabled. */ if (val == CS_MODE_SYSFS) goto out;
I just find it more readable that way.
Thanks, Mathieu
spin_lock_irqsave(&drvdata->spinlock, flags); etb_enable_hw(drvdata); -- 2.7.4
On 07/31/2018 06:07 PM, Mathieu Poirier wrote:
Hello,
On Thu, Jul 26, 2018 at 01:54:40PM +0100, Suzuki K Poulose wrote:
If the ETB is already enabled in sysfs mode, the ETB reports success even if a perf mode is requested. Fix this by checking the requested mode.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
drivers/hwtracing/coresight/coresight-etb10.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 306119e..e5808fd 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -148,8 +148,12 @@ static int etb_enable(struct coresight_device *csdev, u32 mode) return -EBUSY; /* Nothing to do, the tracer is already enabled. */
- if (val == CS_MODE_SYSFS)
goto out;
- if (val == CS_MODE_SYSFS) {
if (mode == CS_MODE_SYSFS)goto out;elsereturn -EBUSY;- }
How about:
if (val == CS_MODE_PERF) return -EBUSY;
/* Don't let perf disturb sysFS sessions */if (val == CS_MODE_SYSFS && mode = CS_MODE_PERF)return -EBUSY;/* Nothing to do, the tracer is already enabled. */ if (val == CS_MODE_SYSFS) goto out;I just find it more readable that way.
Sure, thats fine by me.
Cheers Suzuki
On Tue, 31 Jul 2018 at 15:32, Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 07/31/2018 06:07 PM, Mathieu Poirier wrote:
Hello,
On Thu, Jul 26, 2018 at 01:54:40PM +0100, Suzuki K Poulose wrote:
If the ETB is already enabled in sysfs mode, the ETB reports success even if a perf mode is requested. Fix this by checking the requested mode.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
drivers/hwtracing/coresight/coresight-etb10.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 306119e..e5808fd 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -148,8 +148,12 @@ static int etb_enable(struct coresight_device *csdev, u32 mode) return -EBUSY;
/* Nothing to do, the tracer is already enabled. */
- if (val == CS_MODE_SYSFS)
goto out;
- if (val == CS_MODE_SYSFS) {
if (mode == CS_MODE_SYSFS)goto out;elsereturn -EBUSY;- }
How about:
if (val == CS_MODE_PERF) return -EBUSY;
/* Don't let perf disturb sysFS sessions */if (val == CS_MODE_SYSFS && mode = CS_MODE_PERF)return -EBUSY;/* Nothing to do, the tracer is already enabled. */ if (val == CS_MODE_SYSFS) goto out;I just find it more readable that way.
Sure, thats fine by me.
Ok good. Doing another round just to address this is cumbersome so I just fixed it in my tree. Also, I still think the WARN_ON() in patch 5/13 is superfluous but once again not important enough to make a big deal out of it. I have applied this set.
Thanks, Mathieu
Cheers Suzuki
We create a coresight trace path for each online CPU when we start the event. We rely on the number of online CPUs and then go on to allocate an array matching the "number of online CPUs" for holding the path and then uses normal CPU id as the index to the array. This is problematic as we could have some offline CPUs causing us to access beyond the actual array size (e.g, on a dual SMP system, if CPU0 is offline, CPU1 could be really accessing beyond the array). The solution is to switch to per-cpu array for holding the path.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-etm-perf.c | 55 +++++++++++++++++------- 1 file changed, 40 insertions(+), 15 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 6776956..6338dd1 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -12,6 +12,7 @@ #include <linux/mm.h> #include <linux/init.h> #include <linux/perf_event.h> +#include <linux/percpu-defs.h> #include <linux/slab.h> #include <linux/types.h> #include <linux/workqueue.h> @@ -33,7 +34,7 @@ struct etm_event_data { struct work_struct work; cpumask_t mask; void *snk_config; - struct list_head **path; + struct list_head * __percpu *path; };
static DEFINE_PER_CPU(struct perf_output_handle, ctx_handle); @@ -61,6 +62,18 @@ static const struct attribute_group *etm_pmu_attr_groups[] = { NULL, };
+static inline struct list_head ** +etm_event_cpu_path_ptr(struct etm_event_data *data, int cpu) +{ + return per_cpu_ptr(data->path, cpu); +} + +static inline struct list_head * +etm_event_cpu_path(struct etm_event_data *data, int cpu) +{ + return *etm_event_cpu_path_ptr(data, cpu); +} + static void etm_event_read(struct perf_event *event) {}
static int etm_addr_filters_alloc(struct perf_event *event) @@ -120,23 +133,26 @@ static void free_event_data(struct work_struct *work) */ if (event_data->snk_config) { cpu = cpumask_first(mask); - sink = coresight_get_sink(event_data->path[cpu]); + sink = coresight_get_sink(etm_event_cpu_path(event_data, cpu)); if (sink_ops(sink)->free_buffer) sink_ops(sink)->free_buffer(event_data->snk_config); }
for_each_cpu(cpu, mask) { - if (!(IS_ERR_OR_NULL(event_data->path[cpu]))) - coresight_release_path(event_data->path[cpu]); + struct list_head **ppath; + + ppath = etm_event_cpu_path_ptr(event_data, cpu); + if (!(IS_ERR_OR_NULL(*ppath))) + coresight_release_path(*ppath); + *ppath = NULL; }
- kfree(event_data->path); + free_percpu(event_data->path); kfree(event_data); }
static void *alloc_event_data(int cpu) { - int size; cpumask_t *mask; struct etm_event_data *event_data;
@@ -147,7 +163,6 @@ static void *alloc_event_data(int cpu)
/* Make sure nothing disappears under us */ get_online_cpus(); - size = num_online_cpus();
mask = &event_data->mask; if (cpu != -1) @@ -164,8 +179,8 @@ static void *alloc_event_data(int cpu) * unused memory when dealing with single CPU trace scenarios is small * compared to the cost of searching through an optimized array. */ - event_data->path = kcalloc(size, - sizeof(struct list_head *), GFP_KERNEL); + event_data->path = alloc_percpu(struct list_head *); + if (!event_data->path) { kfree(event_data); return NULL; @@ -213,6 +228,7 @@ static void *etm_setup_aux(int event_cpu, void **pages,
/* Setup the path for each CPU in a trace session */ for_each_cpu(cpu, mask) { + struct list_head *path; struct coresight_device *csdev;
csdev = per_cpu(csdev_src, cpu); @@ -224,9 +240,11 @@ static void *etm_setup_aux(int event_cpu, void **pages, * list of devices from source to sink that can be * referenced later when the path is actually needed. */ - event_data->path[cpu] = coresight_build_path(csdev, sink); - if (IS_ERR(event_data->path[cpu])) + path = coresight_build_path(csdev, sink); + if (IS_ERR(path)) goto err; + + *etm_event_cpu_path_ptr(event_data, cpu) = path; }
if (!sink_ops(sink)->alloc_buffer) @@ -255,6 +273,7 @@ static void etm_event_start(struct perf_event *event, int flags) struct etm_event_data *event_data; struct perf_output_handle *handle = this_cpu_ptr(&ctx_handle); struct coresight_device *sink, *csdev = per_cpu(csdev_src, cpu); + struct list_head *path;
if (!csdev) goto fail; @@ -267,8 +286,9 @@ static void etm_event_start(struct perf_event *event, int flags) if (!event_data) goto fail;
+ path = etm_event_cpu_path(event_data, cpu); /* We need a sink, no need to continue without one */ - sink = coresight_get_sink(event_data->path[cpu]); + sink = coresight_get_sink(path); if (WARN_ON_ONCE(!sink || !sink_ops(sink)->set_buffer)) goto fail_end_stop;
@@ -278,7 +298,7 @@ static void etm_event_start(struct perf_event *event, int flags) goto fail_end_stop;
/* Nothing will happen without a path */ - if (coresight_enable_path(event_data->path[cpu], CS_MODE_PERF)) + if (coresight_enable_path(path, CS_MODE_PERF)) goto fail_end_stop;
/* Tell the perf core the event is alive */ @@ -306,6 +326,7 @@ static void etm_event_stop(struct perf_event *event, int mode) struct coresight_device *sink, *csdev = per_cpu(csdev_src, cpu); struct perf_output_handle *handle = this_cpu_ptr(&ctx_handle); struct etm_event_data *event_data = perf_get_aux(handle); + struct list_head *path;
if (event->hw.state == PERF_HES_STOPPED) return; @@ -313,7 +334,11 @@ static void etm_event_stop(struct perf_event *event, int mode) if (!csdev) return;
- sink = coresight_get_sink(event_data->path[cpu]); + path = etm_event_cpu_path(event_data, cpu); + if (!path) + return; + + sink = coresight_get_sink(path); if (!sink) return;
@@ -344,7 +369,7 @@ static void etm_event_stop(struct perf_event *event, int mode) }
/* Disabling the path make its elements available to other sessions */ - coresight_disable_path(event_data->path[cpu]); + coresight_disable_path(path); }
static int etm_event_add(struct perf_event *event, int mode)
We hold the read lock on CPU hotplug to simply copy the online mask, which is not really needed. And this can cause a lockdep warning, like :
[ 54.632093] ====================================================== [ 54.638207] WARNING: possible circular locking dependency detected [ 54.644322] 4.18.0-rc3-00042-g2d39e6356bb7-dirty #309 Not tainted [ 54.650350] ------------------------------------------------------ [ 54.656464] perf/2862 is trying to acquire lock: [ 54.661031] 000000007e21d170 (&event->mmap_mutex){+.+.}, at: perf_event_set_output+0x98/0x138 [ 54.669486] [ 54.669486] but task is already holding lock: [ 54.675256] 000000001080eb1b (&cpuctx_mutex){+.+.}, at: perf_event_ctx_lock_nested+0xf8/0x1f0 [ 54.683704] [ 54.683704] which lock already depends on the new lock. [ 54.683704] [ 54.691797] [ 54.691797] the existing dependency chain (in reverse order) is: [ 54.699201] [ 54.699201] -> #3 (&cpuctx_mutex){+.+.}: [ 54.704556] __mutex_lock+0x70/0x808 [ 54.708608] mutex_lock_nested+0x1c/0x28 [ 54.713005] perf_event_init_cpu+0x8c/0xd8 [ 54.717574] perf_event_init+0x194/0x1d4 [ 54.721971] start_kernel+0x2b8/0x42c [ 54.726107] [ 54.726107] -> #2 (pmus_lock){+.+.}: [ 54.731114] __mutex_lock+0x70/0x808 [ 54.735165] mutex_lock_nested+0x1c/0x28 [ 54.739560] perf_event_init_cpu+0x30/0xd8 [ 54.744129] cpuhp_invoke_callback+0x84/0x248 [ 54.748954] _cpu_up+0xe8/0x1c8 [ 54.752576] do_cpu_up+0xa8/0xc8 [ 54.756283] cpu_up+0x10/0x18 [ 54.759731] smp_init+0xa0/0x114 [ 54.763438] kernel_init_freeable+0x120/0x288 [ 54.768264] kernel_init+0x10/0x108 [ 54.772230] ret_from_fork+0x10/0x18 [ 54.776279] [ 54.776279] -> #1 (cpu_hotplug_lock.rw_sem){++++}: [ 54.782492] cpus_read_lock+0x34/0xb0 [ 54.786631] etm_setup_aux+0x5c/0x308 [ 54.790769] rb_alloc_aux+0x1ec/0x300 [ 54.794906] perf_mmap+0x284/0x610 [ 54.798787] mmap_region+0x388/0x570 [ 54.802838] do_mmap+0x344/0x4f8 [ 54.806544] vm_mmap_pgoff+0xe4/0x110 [ 54.810682] ksys_mmap_pgoff+0xa8/0x240 [ 54.814992] sys_mmap+0x18/0x28 [ 54.818613] el0_svc_naked+0x30/0x34 [ 54.822661] [ 54.822661] -> #0 (&event->mmap_mutex){+.+.}: [ 54.828445] lock_acquire+0x48/0x68 [ 54.832409] __mutex_lock+0x70/0x808 [ 54.836459] mutex_lock_nested+0x1c/0x28 [ 54.840855] perf_event_set_output+0x98/0x138 [ 54.845680] _perf_ioctl+0x2a0/0x6a0 [ 54.849731] perf_ioctl+0x3c/0x68 [ 54.853526] do_vfs_ioctl+0xb8/0xa20 [ 54.857577] ksys_ioctl+0x80/0xb8 [ 54.861370] sys_ioctl+0xc/0x18 [ 54.864990] el0_svc_naked+0x30/0x34 [ 54.869039] [ 54.869039] other info that might help us debug this: [ 54.869039] [ 54.876960] Chain exists of: [ 54.876960] &event->mmap_mutex --> pmus_lock --> &cpuctx_mutex [ 54.876960] [ 54.887217] Possible unsafe locking scenario: [ 54.887217] [ 54.893073] CPU0 CPU1 [ 54.897552] ---- ---- [ 54.902030] lock(&cpuctx_mutex); [ 54.905396] lock(pmus_lock); [ 54.910911] lock(&cpuctx_mutex); [ 54.916770] lock(&event->mmap_mutex); [ 54.920566] [ 54.920566] *** DEADLOCK *** [ 54.920566] [ 54.926424] 1 lock held by perf/2862: [ 54.930042] #0: 000000001080eb1b (&cpuctx_mutex){+.+.}, at: perf_event_ctx_lock_nested+0xf8/0x1f0
Since we have per-cpu array for the paths, we simply don't care about the number of online CPUs. This patch gets rid of the {get/put}_online_cpus().
Reported-by: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- Changes since v2: - Splitted from the original patch (per-cpu path management) - Added splat reported by Mathieu in the commit description. --- drivers/hwtracing/coresight/coresight-etm-perf.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 6338dd1..6beb662 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -161,15 +161,12 @@ static void *alloc_event_data(int cpu) if (!event_data) return NULL;
- /* Make sure nothing disappears under us */ - get_online_cpus();
mask = &event_data->mask; if (cpu != -1) cpumask_set_cpu(cpu, mask); else cpumask_copy(mask, cpu_online_mask); - put_online_cpus();
/* * Each CPU has a single path between source and destination. As such
At the moment, if there is no CPU specified for a given event, we use cpu_online_mask and try to build path for each of the CPUs in the mask. This could prevent any CPU that is turned online later to be used for the tracing.
This patch changes to use the cpu_present_mask and tries to build path for as much CPUs as possible ignoring the failures in building path for some of the CPUs. If ever we try to trace on those CPUs, we fail the operation.
Based on a patch from Mathieu Poirier.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-etm-perf.c | 46 +++++++++++++++--------- 1 file changed, 30 insertions(+), 16 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 6beb662..afe7e7f 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -127,11 +127,9 @@ static void free_event_data(struct work_struct *work)
event_data = container_of(work, struct etm_event_data, work); mask = &event_data->mask; - /* - * First deal with the sink configuration. See comment in - * etm_setup_aux() about why we take the first available path. - */ - if (event_data->snk_config) { + + /* Free the sink buffers, if there are any */ + if (event_data->snk_config && !WARN_ON(cpumask_empty(mask))) { cpu = cpumask_first(mask); sink = coresight_get_sink(etm_event_cpu_path(event_data, cpu)); if (sink_ops(sink)->free_buffer) @@ -166,7 +164,7 @@ static void *alloc_event_data(int cpu) if (cpu != -1) cpumask_set_cpu(cpu, mask); else - cpumask_copy(mask, cpu_online_mask); + cpumask_copy(mask, cpu_present_mask);
/* * Each CPU has a single path between source and destination. As such @@ -218,19 +216,32 @@ static void *etm_setup_aux(int event_cpu, void **pages, * on the cmd line. As such the "enable_sink" flag in sysFS is reset. */ sink = coresight_get_enabled_sink(true); - if (!sink) + if (!sink || !sink_ops(sink)->alloc_buffer) goto err;
mask = &event_data->mask;
- /* Setup the path for each CPU in a trace session */ + /* + * Setup the path for each CPU in a trace session. We try to build + * trace path for each CPU in the mask. If we don't find an ETM + * for the CPU or fail to build a path, we clear the CPU from the + * mask and continue with the rest. If ever we try to trace on those + * CPUs, we can handle it and fail the session. + */ for_each_cpu(cpu, mask) { struct list_head *path; struct coresight_device *csdev;
csdev = per_cpu(csdev_src, cpu); - if (!csdev) - goto err; + /* + * If there is no ETM associated with this CPU clear it from + * the mask and continue with the rest. If ever we try to trace + * on this CPU, we handle it accordingly. + */ + if (!csdev) { + cpumask_clear_cpu(cpu, mask); + continue; + }
/* * Building a path doesn't enable it, it simply builds a @@ -238,17 +249,20 @@ static void *etm_setup_aux(int event_cpu, void **pages, * referenced later when the path is actually needed. */ path = coresight_build_path(csdev, sink); - if (IS_ERR(path)) - goto err; + if (IS_ERR(path)) { + cpumask_clear_cpu(cpu, mask); + continue; + }
*etm_event_cpu_path_ptr(event_data, cpu) = path; }
- if (!sink_ops(sink)->alloc_buffer) - goto err; - + /* If we don't have any CPUs ready for tracing, abort */ cpu = cpumask_first(mask); - /* Get the AUX specific data from the sink buffer */ + if (cpu >= nr_cpu_ids) + goto err; + + /* Allocate the sink buffer for this session */ event_data->snk_config = sink_ops(sink)->alloc_buffer(sink, cpu, pages, nr_pages, overwrite);
On Thu, Jul 26, 2018 at 01:54:43PM +0100, Suzuki K Poulose wrote:
At the moment, if there is no CPU specified for a given event, we use cpu_online_mask and try to build path for each of the CPUs in the mask. This could prevent any CPU that is turned online later to be used for the tracing.
This patch changes to use the cpu_present_mask and tries to build path for as much CPUs as possible ignoring the failures in building path for some of the CPUs. If ever we try to trace on those CPUs, we fail the operation.
Based on a patch from Mathieu Poirier.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
drivers/hwtracing/coresight/coresight-etm-perf.c | 46 +++++++++++++++--------- 1 file changed, 30 insertions(+), 16 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 6beb662..afe7e7f 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -127,11 +127,9 @@ static void free_event_data(struct work_struct *work) event_data = container_of(work, struct etm_event_data, work); mask = &event_data->mask;
- /*
* First deal with the sink configuration. See comment in* etm_setup_aux() about why we take the first available path.*/- if (event_data->snk_config) {
- /* Free the sink buffers, if there are any */
- if (event_data->snk_config && !WARN_ON(cpumask_empty(mask))) {
I'm not sure the WARN_ON() is required.
Here @mask can't be empty since you've added a check in setup_aux(). So if @mask does end up being empty something has trampled the memory and we have far bigger problems.
cpu = cpumask_first(mask); sink = coresight_get_sink(etm_event_cpu_path(event_data, cpu)); if (sink_ops(sink)->free_buffer)@@ -166,7 +164,7 @@ static void *alloc_event_data(int cpu) if (cpu != -1) cpumask_set_cpu(cpu, mask); else
cpumask_copy(mask, cpu_online_mask);
cpumask_copy(mask, cpu_present_mask);/* * Each CPU has a single path between source and destination. As such @@ -218,19 +216,32 @@ static void *etm_setup_aux(int event_cpu, void **pages, * on the cmd line. As such the "enable_sink" flag in sysFS is reset. */ sink = coresight_get_enabled_sink(true);
- if (!sink)
- if (!sink || !sink_ops(sink)->alloc_buffer) goto err;
mask = &event_data->mask;
- /* Setup the path for each CPU in a trace session */
- /*
* Setup the path for each CPU in a trace session. We try to build* trace path for each CPU in the mask. If we don't find an ETM* for the CPU or fail to build a path, we clear the CPU from the* mask and continue with the rest. If ever we try to trace on those* CPUs, we can handle it and fail the session.*/csdev = per_cpu(csdev_src, cpu);
if (!csdev)goto err;
/** If there is no ETM associated with this CPU clear it from* the mask and continue with the rest. If ever we try to trace* on this CPU, we handle it accordingly.*/if (!csdev) {cpumask_clear_cpu(cpu, mask);continue;}/* * Building a path doesn't enable it, it simply builds a @@ -238,17 +249,20 @@ static void *etm_setup_aux(int event_cpu, void **pages, * referenced later when the path is actually needed. */ path = coresight_build_path(csdev, sink);
if (IS_ERR(path))goto err;
if (IS_ERR(path)) {cpumask_clear_cpu(cpu, mask);continue;}*etm_event_cpu_path_ptr(event_data, cpu) = path; }
- if (!sink_ops(sink)->alloc_buffer)
goto err;
- /* If we don't have any CPUs ready for tracing, abort */ cpu = cpumask_first(mask);
- /* Get the AUX specific data from the sink buffer */
- if (cpu >= nr_cpu_ids)
goto err;- /* Allocate the sink buffer for this session */ event_data->snk_config = sink_ops(sink)->alloc_buffer(sink, cpu, pages, nr_pages, overwrite);
-- 2.7.4
On 07/31/2018 06:46 PM, Mathieu Poirier wrote:
On Thu, Jul 26, 2018 at 01:54:43PM +0100, Suzuki K Poulose wrote:
At the moment, if there is no CPU specified for a given event, we use cpu_online_mask and try to build path for each of the CPUs in the mask. This could prevent any CPU that is turned online later to be used for the tracing.
This patch changes to use the cpu_present_mask and tries to build path for as much CPUs as possible ignoring the failures in building path for some of the CPUs. If ever we try to trace on those CPUs, we fail the operation.
Based on a patch from Mathieu Poirier.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
drivers/hwtracing/coresight/coresight-etm-perf.c | 46 +++++++++++++++--------- 1 file changed, 30 insertions(+), 16 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 6beb662..afe7e7f 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -127,11 +127,9 @@ static void free_event_data(struct work_struct *work) event_data = container_of(work, struct etm_event_data, work); mask = &event_data->mask;
- /*
* First deal with the sink configuration. See comment in* etm_setup_aux() about why we take the first available path.*/- if (event_data->snk_config) {
- /* Free the sink buffers, if there are any */
- if (event_data->snk_config && !WARN_ON(cpumask_empty(mask))) {
I'm not sure the WARN_ON() is required.
Here @mask can't be empty since you've added a check in setup_aux(). So if @mask does end up being empty something has trampled the memory and we have far bigger problems.
And thats exactly why I have a WARN_ON(). The issue is, we can't free the sink_config unless we are able to find the sink device. So, if we ever end up with a sink_config with an empty mask, there is something seriously wrong and hence the WARN_ON().
cpu = cpumask_first(mask); sink = coresight_get_sink(etm_event_cpu_path(event_data, cpu)); if (sink_ops(sink)->free_buffer)@@ -166,7 +164,7 @@ static void *alloc_event_data(int cpu)
...
- if (!sink_ops(sink)->alloc_buffer)
goto err;
- /* If we don't have any CPUs ready for tracing, abort */ cpu = cpumask_first(mask);
- /* Get the AUX specific data from the sink buffer */
- if (cpu >= nr_cpu_ids)
goto err;- /* Allocate the sink buffer for this session */ event_data->snk_config = sink_ops(sink)->alloc_buffer(sink, cpu, pages, nr_pages, overwrite);
Suzuki
We enable the trace path, before activating the source. If we fail to enable the source, we must disable the path to make sure it is available for another session.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-etm-perf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index afe7e7f..6db76ce 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -317,11 +317,13 @@ static void etm_event_start(struct perf_event *event, int flags)
/* Finally enable the tracer */ if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF)) - goto fail_end_stop; + goto fail_disable_path;
out: return;
+fail_disable_path: + coresight_disable_path(path); fail_end_stop: perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); perf_aux_output_end(handle, 0);
Since the ETR could be driven either by SYSFS or by perf, it becomes complicated how we deal with the buffers used for each of these modes. The ETR driver cannot simply free the current attached buffer without knowing the provider (i.e, sysfs vs perf).
To solve this issue, we provide: 1) the driver-mode specific etr buffer to be retained in the drvdata 2) the etr_buf for a session should be passed on when enabling the hardware, which will be stored in drvdata->etr_buf. This will be replaced (not free'd) as soon as the hardware is disabled, after necessary sync operation.
The advantages of this are :
1) The common code path doesn't need to worry about how to dispose an existing buffer, if it is about to start a new session with a different buffer, possibly in a different mode. 2) The driver mode can control its buffers and can get access to the saved session even when the hardware is operating in a different mode. (e.g, we can still access a trace buffer from a sysfs mode even if the etr is now used in perf mode, without disrupting the current session.)
Towards this, we introduce a sysfs specific data which will hold the etr_buf used for sysfs mode of operation, controlled solely by the sysfs mode handling code.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-tmc-etr.c | 58 ++++++++++++++++--------- drivers/hwtracing/coresight/coresight-tmc.h | 2 + 2 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 2eda5de..58c7935 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -895,10 +895,15 @@ static void tmc_sync_etr_buf(struct tmc_drvdata *drvdata) tmc_etr_buf_insert_barrier_packet(etr_buf, etr_buf->offset); }
-static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata) +static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata, + struct etr_buf *etr_buf) { u32 axictl, sts; - struct etr_buf *etr_buf = drvdata->etr_buf; + + /* Callers should provide an appropriate buffer for use */ + if (WARN_ON(!etr_buf || drvdata->etr_buf)) + return; + drvdata->etr_buf = etr_buf;
/* * If this ETR is connected to a CATU, enable it before we turn @@ -960,13 +965,16 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata) * also updating the @bufpp on where to find it. Since the trace data * starts at anywhere in the buffer, depending on the RRP, we adjust the * @len returned to handle buffer wrapping around. + * + * We are protected here by drvdata->reading != 0, which ensures the + * sysfs_buf stays alive. */ ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, loff_t pos, size_t len, char **bufpp) { s64 offset; ssize_t actual = len; - struct etr_buf *etr_buf = drvdata->etr_buf; + struct etr_buf *etr_buf = drvdata->sysfs_buf;
if (pos + actual > etr_buf->len) actual = etr_buf->len - pos; @@ -996,7 +1004,14 @@ tmc_etr_free_sysfs_buf(struct etr_buf *buf)
static void tmc_etr_sync_sysfs_buf(struct tmc_drvdata *drvdata) { - tmc_sync_etr_buf(drvdata); + struct etr_buf *etr_buf = drvdata->etr_buf; + + if (WARN_ON(drvdata->sysfs_buf != etr_buf)) { + tmc_etr_free_sysfs_buf(drvdata->sysfs_buf); + drvdata->sysfs_buf = NULL; + } else { + tmc_sync_etr_buf(drvdata); + } }
static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata) @@ -1017,6 +1032,8 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
/* Disable CATU device if this ETR is connected to one */ tmc_etr_disable_catu(drvdata); + /* Reset the ETR buf used by hardware */ + drvdata->etr_buf = NULL; }
static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) @@ -1024,7 +1041,7 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) int ret = 0; unsigned long flags; struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); - struct etr_buf *new_buf = NULL, *free_buf = NULL; + struct etr_buf *sysfs_buf = NULL, *new_buf = NULL, *free_buf = NULL;
/* * If we are enabling the ETR from disabled state, we need to make @@ -1035,7 +1052,8 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) * with the lock released. */ spin_lock_irqsave(&drvdata->spinlock, flags); - if (!drvdata->etr_buf || (drvdata->etr_buf->size != drvdata->size)) { + sysfs_buf = READ_ONCE(drvdata->sysfs_buf); + if (!sysfs_buf || (sysfs_buf->size != drvdata->size)) { spin_unlock_irqrestore(&drvdata->spinlock, flags);
/* Allocate memory with the locks released */ @@ -1064,14 +1082,14 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) * If we don't have a buffer or it doesn't match the requested size, * use the buffer allocated above. Otherwise reuse the existing buffer. */ - if (!drvdata->etr_buf || - (new_buf && drvdata->etr_buf->size != new_buf->size)) { - free_buf = drvdata->etr_buf; - drvdata->etr_buf = new_buf; + sysfs_buf = READ_ONCE(drvdata->sysfs_buf); + if (!sysfs_buf || (new_buf && sysfs_buf->size != new_buf->size)) { + free_buf = sysfs_buf; + drvdata->sysfs_buf = new_buf; }
drvdata->mode = CS_MODE_SYSFS; - tmc_etr_enable_hw(drvdata); + tmc_etr_enable_hw(drvdata, drvdata->sysfs_buf); out: spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -1156,13 +1174,13 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata) goto out; }
- /* If drvdata::etr_buf is NULL the trace data has been read already */ - if (drvdata->etr_buf == NULL) { + /* If sysfs_buf is NULL the trace data has been read already */ + if (!drvdata->sysfs_buf) { ret = -EINVAL; goto out; }
- /* Disable the TMC if need be */ + /* Disable the TMC if we are trying to read from a running session */ if (drvdata->mode == CS_MODE_SYSFS) tmc_etr_disable_hw(drvdata);
@@ -1176,7 +1194,7 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata) int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata) { unsigned long flags; - struct etr_buf *etr_buf = NULL; + struct etr_buf *sysfs_buf = NULL;
/* config types are set a boot time and never change */ if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR)) @@ -1191,22 +1209,22 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata) * buffer. Since the tracer is still enabled drvdata::buf can't * be NULL. */ - tmc_etr_enable_hw(drvdata); + tmc_etr_enable_hw(drvdata, drvdata->sysfs_buf); } else { /* * The ETR is not tracing and the buffer was just read. * As such prepare to free the trace buffer. */ - etr_buf = drvdata->etr_buf; - drvdata->etr_buf = NULL; + sysfs_buf = drvdata->sysfs_buf; + drvdata->sysfs_buf = NULL; }
drvdata->reading = false; spin_unlock_irqrestore(&drvdata->spinlock, flags);
/* Free allocated memory out side of the spinlock */ - if (etr_buf) - tmc_free_etr_buf(etr_buf); + if (sysfs_buf) + tmc_etr_free_sysfs_buf(sysfs_buf);
return 0; } diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 7027bd6..872f63e 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -170,6 +170,7 @@ struct etr_buf { * @trigger_cntr: amount of words to store after a trigger. * @etr_caps: Bitmask of capabilities of the TMC ETR, inferred from the * device configuration register (DEVID) + * @sysfs_data: SYSFS buffer for ETR. */ struct tmc_drvdata { void __iomem *base; @@ -189,6 +190,7 @@ struct tmc_drvdata { enum tmc_mem_intf_width memwidth; u32 trigger_cntr; u32 etr_caps; + struct etr_buf *sysfs_buf; };
struct etr_buf_operations {
Since the ETR now uses mode specific buffers, we can reliably provide the trace data captured in sysfs mode, even when the ETR is operating in PERF mode.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-tmc-etr.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 58c7935..671a2ba 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -1168,19 +1168,17 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata) goto out; }
- /* Don't interfere if operated from Perf */ - if (drvdata->mode == CS_MODE_PERF) { - ret = -EINVAL; - goto out; - } - - /* If sysfs_buf is NULL the trace data has been read already */ + /* + * We can safely allow reads even if the ETR is operating in PERF mode, + * since the sysfs session is captured in mode specific data. + * If drvdata::sysfs_data is NULL the trace data has been read already. + */ if (!drvdata->sysfs_buf) { ret = -EINVAL; goto out; }
- /* Disable the TMC if we are trying to read from a running session */ + /* Disable the TMC if we are trying to read from a running session. */ if (drvdata->mode == CS_MODE_SYSFS) tmc_etr_disable_hw(drvdata);
Convert component enable/disable messages from dev_info to dev_dbg. When used with perf, the components in the paths are enabled/disabled during each schedule of the run, which can flood the dmesg with these messages. Moreover, they are only useful for debug purposes. So, convert such messages to dev_dbg() which can be turned on as needed.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-dynamic-replicator.c | 4 ++-- drivers/hwtracing/coresight/coresight-etb10.c | 6 +++--- drivers/hwtracing/coresight/coresight-etm3x.c | 4 ++-- drivers/hwtracing/coresight/coresight-etm4x.c | 4 ++-- drivers/hwtracing/coresight/coresight-funnel.c | 4 ++-- drivers/hwtracing/coresight/coresight-replicator.c | 4 ++-- drivers/hwtracing/coresight/coresight-stm.c | 4 ++-- drivers/hwtracing/coresight/coresight-tmc-etf.c | 8 ++++---- drivers/hwtracing/coresight/coresight-tmc-etr.c | 4 ++-- drivers/hwtracing/coresight/coresight-tmc.c | 4 ++-- drivers/hwtracing/coresight/coresight-tpiu.c | 4 ++-- 11 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c index f6d0571..ebb8043 100644 --- a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c +++ b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c @@ -56,7 +56,7 @@ static int replicator_enable(struct coresight_device *csdev, int inport,
CS_LOCK(drvdata->base);
- dev_info(drvdata->dev, "REPLICATOR enabled\n"); + dev_dbg(drvdata->dev, "REPLICATOR enabled\n"); return 0; }
@@ -75,7 +75,7 @@ static void replicator_disable(struct coresight_device *csdev, int inport,
CS_LOCK(drvdata->base);
- dev_info(drvdata->dev, "REPLICATOR disabled\n"); + dev_dbg(drvdata->dev, "REPLICATOR disabled\n"); }
static const struct coresight_ops_link replicator_link_ops = { diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index e5808fd..474c247 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -160,7 +160,7 @@ static int etb_enable(struct coresight_device *csdev, u32 mode) spin_unlock_irqrestore(&drvdata->spinlock, flags);
out: - dev_info(drvdata->dev, "ETB enabled\n"); + dev_dbg(drvdata->dev, "ETB enabled\n"); return 0; }
@@ -266,7 +266,7 @@ static void etb_disable(struct coresight_device *csdev)
local_set(&drvdata->mode, CS_MODE_DISABLED);
- dev_info(drvdata->dev, "ETB disabled\n"); + dev_dbg(drvdata->dev, "ETB disabled\n"); }
static void *etb_alloc_buffer(struct coresight_device *csdev, int cpu, @@ -509,7 +509,7 @@ static void etb_dump(struct etb_drvdata *drvdata) } spin_unlock_irqrestore(&drvdata->spinlock, flags);
- dev_info(drvdata->dev, "ETB dumped\n"); + dev_dbg(drvdata->dev, "ETB dumped\n"); }
static int etb_open(struct inode *inode, struct file *file) diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c index 7c74263..9ce8fba 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x.c +++ b/drivers/hwtracing/coresight/coresight-etm3x.c @@ -501,7 +501,7 @@ static int etm_enable_sysfs(struct coresight_device *csdev) drvdata->sticky_enable = true; spin_unlock(&drvdata->spinlock);
- dev_info(drvdata->dev, "ETM tracing enabled\n"); + dev_dbg(drvdata->dev, "ETM tracing enabled\n"); return 0;
err: @@ -604,7 +604,7 @@ static void etm_disable_sysfs(struct coresight_device *csdev) spin_unlock(&drvdata->spinlock); cpus_read_unlock();
- dev_info(drvdata->dev, "ETM tracing disabled\n"); + dev_dbg(drvdata->dev, "ETM tracing disabled\n"); }
static void etm_disable(struct coresight_device *csdev, diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c index 1d94ebe..c1dcc7c 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.c +++ b/drivers/hwtracing/coresight/coresight-etm4x.c @@ -267,7 +267,7 @@ static int etm4_enable_sysfs(struct coresight_device *csdev) drvdata->sticky_enable = true; spin_unlock(&drvdata->spinlock);
- dev_info(drvdata->dev, "ETM tracing enabled\n"); + dev_dbg(drvdata->dev, "ETM tracing enabled\n"); return 0;
err: @@ -380,7 +380,7 @@ static void etm4_disable_sysfs(struct coresight_device *csdev) spin_unlock(&drvdata->spinlock); cpus_read_unlock();
- dev_info(drvdata->dev, "ETM tracing disabled\n"); + dev_dbg(drvdata->dev, "ETM tracing disabled\n"); }
static void etm4_disable(struct coresight_device *csdev, diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c index 448145a..ee7a30b 100644 --- a/drivers/hwtracing/coresight/coresight-funnel.c +++ b/drivers/hwtracing/coresight/coresight-funnel.c @@ -65,7 +65,7 @@ static int funnel_enable(struct coresight_device *csdev, int inport,
funnel_enable_hw(drvdata, inport);
- dev_info(drvdata->dev, "FUNNEL inport %d enabled\n", inport); + dev_dbg(drvdata->dev, "FUNNEL inport %d enabled\n", inport); return 0; }
@@ -89,7 +89,7 @@ static void funnel_disable(struct coresight_device *csdev, int inport,
funnel_disable_hw(drvdata, inport);
- dev_info(drvdata->dev, "FUNNEL inport %d disabled\n", inport); + dev_dbg(drvdata->dev, "FUNNEL inport %d disabled\n", inport); }
static const struct coresight_ops_link funnel_link_ops = { diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c index 8d2eaaa..feac983 100644 --- a/drivers/hwtracing/coresight/coresight-replicator.c +++ b/drivers/hwtracing/coresight/coresight-replicator.c @@ -35,7 +35,7 @@ static int replicator_enable(struct coresight_device *csdev, int inport, { struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
- dev_info(drvdata->dev, "REPLICATOR enabled\n"); + dev_dbg(drvdata->dev, "REPLICATOR enabled\n"); return 0; }
@@ -44,7 +44,7 @@ static void replicator_disable(struct coresight_device *csdev, int inport, { struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
- dev_info(drvdata->dev, "REPLICATOR disabled\n"); + dev_dbg(drvdata->dev, "REPLICATOR disabled\n"); }
static const struct coresight_ops_link replicator_link_ops = { diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c index c46c70a..35d6f97 100644 --- a/drivers/hwtracing/coresight/coresight-stm.c +++ b/drivers/hwtracing/coresight/coresight-stm.c @@ -211,7 +211,7 @@ static int stm_enable(struct coresight_device *csdev, stm_enable_hw(drvdata); spin_unlock(&drvdata->spinlock);
- dev_info(drvdata->dev, "STM tracing enabled\n"); + dev_dbg(drvdata->dev, "STM tracing enabled\n"); return 0; }
@@ -274,7 +274,7 @@ static void stm_disable(struct coresight_device *csdev, pm_runtime_put(drvdata->dev);
local_set(&drvdata->mode, CS_MODE_DISABLED); - dev_info(drvdata->dev, "STM tracing disabled\n"); + dev_dbg(drvdata->dev, "STM tracing disabled\n"); } }
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index 0549249..434003a 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -233,7 +233,7 @@ static int tmc_enable_etf_sink(struct coresight_device *csdev, u32 mode) if (ret) return ret;
- dev_info(drvdata->dev, "TMC-ETB/ETF enabled\n"); + dev_dbg(drvdata->dev, "TMC-ETB/ETF enabled\n"); return 0; }
@@ -256,7 +256,7 @@ static void tmc_disable_etf_sink(struct coresight_device *csdev)
spin_unlock_irqrestore(&drvdata->spinlock, flags);
- dev_info(drvdata->dev, "TMC-ETB/ETF disabled\n"); + dev_dbg(drvdata->dev, "TMC-ETB/ETF disabled\n"); }
static int tmc_enable_etf_link(struct coresight_device *csdev, @@ -275,7 +275,7 @@ static int tmc_enable_etf_link(struct coresight_device *csdev, drvdata->mode = CS_MODE_SYSFS; spin_unlock_irqrestore(&drvdata->spinlock, flags);
- dev_info(drvdata->dev, "TMC-ETF enabled\n"); + dev_dbg(drvdata->dev, "TMC-ETF enabled\n"); return 0; }
@@ -295,7 +295,7 @@ static void tmc_disable_etf_link(struct coresight_device *csdev, drvdata->mode = CS_MODE_DISABLED; spin_unlock_irqrestore(&drvdata->spinlock, flags);
- dev_info(drvdata->dev, "TMC-ETF disabled\n"); + dev_dbg(drvdata->dev, "TMC-ETF disabled\n"); }
static void *tmc_alloc_etf_buffer(struct coresight_device *csdev, int cpu, diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 671a2ba..85a3f59 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -1098,7 +1098,7 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) tmc_etr_free_sysfs_buf(free_buf);
if (!ret) - dev_info(drvdata->dev, "TMC-ETR enabled\n"); + dev_dbg(drvdata->dev, "TMC-ETR enabled\n");
return ret; } @@ -1141,7 +1141,7 @@ static void tmc_disable_etr_sink(struct coresight_device *csdev)
spin_unlock_irqrestore(&drvdata->spinlock, flags);
- dev_info(drvdata->dev, "TMC-ETR disabled\n"); + dev_dbg(drvdata->dev, "TMC-ETR disabled\n"); }
static const struct coresight_ops_sink tmc_etr_sink_ops = { diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c index 1b817ec..ea249f0 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.c +++ b/drivers/hwtracing/coresight/coresight-tmc.c @@ -81,7 +81,7 @@ static int tmc_read_prepare(struct tmc_drvdata *drvdata) }
if (!ret) - dev_info(drvdata->dev, "TMC read start\n"); + dev_dbg(drvdata->dev, "TMC read start\n");
return ret; } @@ -103,7 +103,7 @@ static int tmc_read_unprepare(struct tmc_drvdata *drvdata) }
if (!ret) - dev_info(drvdata->dev, "TMC read end\n"); + dev_dbg(drvdata->dev, "TMC read end\n");
return ret; } diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c index 01b7457..7daeb084 100644 --- a/drivers/hwtracing/coresight/coresight-tpiu.c +++ b/drivers/hwtracing/coresight/coresight-tpiu.c @@ -73,7 +73,7 @@ static int tpiu_enable(struct coresight_device *csdev, u32 mode)
tpiu_enable_hw(drvdata);
- dev_info(drvdata->dev, "TPIU enabled\n"); + dev_dbg(drvdata->dev, "TPIU enabled\n"); return 0; }
@@ -99,7 +99,7 @@ static void tpiu_disable(struct coresight_device *csdev)
tpiu_disable_hw(drvdata);
- dev_info(drvdata->dev, "TPIU disabled\n"); + dev_dbg(drvdata->dev, "TPIU disabled\n"); }
static const struct coresight_ops_sink tpiu_sink_ops = {
Right now we issue an update_buffer() and reset_buffer() call backs in succession when we stop tracing an event. The update_buffer is supposed to check the status of the buffer and make sure the ring buffer is updated with the trace data. And we store information about the size of the data collected only to be consumed by the reset_buffer callback which always follows the update_buffer. This was originally designed for handling future IPs which could trigger a buffer overflow interrupt. This patch gets rid of the reset_buffer callback altogether and performs the actions in update_buffer, making it return the size collected. We can always add the support for handling the overflow interrupt case later.
This removes some not-so pretty hack (storing the new head in the size field for snapshot mode) and cleans it up a little bit.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-etb10.c | 56 +++++------------------ drivers/hwtracing/coresight/coresight-etm-perf.c | 9 +--- drivers/hwtracing/coresight/coresight-tmc-etf.c | 58 +++++------------------- include/linux/coresight.h | 6 +-- 4 files changed, 26 insertions(+), 103 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 474c247..60bfc7b 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -319,37 +319,7 @@ static int etb_set_buffer(struct coresight_device *csdev, return ret; }
-static unsigned long etb_reset_buffer(struct coresight_device *csdev, - struct perf_output_handle *handle, - void *sink_config) -{ - unsigned long size = 0; - struct cs_buffers *buf = sink_config; - - if (buf) { - /* - * In snapshot mode ->data_size holds the new address of the - * ring buffer's head. The size itself is the whole address - * range since we want the latest information. - */ - if (buf->snapshot) - handle->head = local_xchg(&buf->data_size, - buf->nr_pages << PAGE_SHIFT); - - /* - * Tell the tracer PMU how much we got in this run and if - * something went wrong along the way. Nobody else can use - * this cs_buffers instance until we are done. As such - * resetting parameters here and squaring off with the ring - * buffer API in the tracer PMU is fine. - */ - size = local_xchg(&buf->data_size, 0); - } - - return size; -} - -static void etb_update_buffer(struct coresight_device *csdev, +static unsigned long etb_update_buffer(struct coresight_device *csdev, struct perf_output_handle *handle, void *sink_config) { @@ -358,13 +328,13 @@ static void etb_update_buffer(struct coresight_device *csdev, u8 *buf_ptr; const u32 *barrier; u32 read_ptr, write_ptr, capacity; - u32 status, read_data, to_read; - unsigned long offset; + u32 status, read_data; + unsigned long offset, to_read; struct cs_buffers *buf = sink_config; struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
if (!buf) - return; + return 0;
capacity = drvdata->buffer_depth * ETB_FRAME_SIZE_WORDS;
@@ -469,18 +439,17 @@ static void etb_update_buffer(struct coresight_device *csdev, writel_relaxed(0x0, drvdata->base + ETB_RAM_WRITE_POINTER);
/* - * In snapshot mode all we have to do is communicate to - * perf_aux_output_end() the address of the current head. In full - * trace mode the same function expects a size to move rb->aux_head - * forward. + * In snapshot mode we have to update the handle->head to point + * to the new location. */ - if (buf->snapshot) - local_set(&buf->data_size, (cur * PAGE_SIZE) + offset); - else - local_add(to_read, &buf->data_size); - + if (buf->snapshot) { + handle->head = (cur * PAGE_SIZE) + offset; + to_read = buf->nr_pages << PAGE_SHIFT; + } etb_enable_hw(drvdata); CS_LOCK(drvdata->base); + + return to_read; }
static const struct coresight_ops_sink etb_sink_ops = { @@ -489,7 +458,6 @@ static const struct coresight_ops_sink etb_sink_ops = { .alloc_buffer = etb_alloc_buffer, .free_buffer = etb_free_buffer, .set_buffer = etb_set_buffer, - .reset_buffer = etb_reset_buffer, .update_buffer = etb_update_buffer, };
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 6db76ce..ad87441 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -369,15 +369,8 @@ static void etm_event_stop(struct perf_event *event, int mode) if (!sink_ops(sink)->update_buffer) return;
- sink_ops(sink)->update_buffer(sink, handle, + size = sink_ops(sink)->update_buffer(sink, handle, event_data->snk_config); - - if (!sink_ops(sink)->reset_buffer) - return; - - size = sink_ops(sink)->reset_buffer(sink, handle, - event_data->snk_config); - perf_aux_output_end(handle, size); }
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index 434003a..31a98f9 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -349,36 +349,7 @@ static int tmc_set_etf_buffer(struct coresight_device *csdev, return ret; }
-static unsigned long tmc_reset_etf_buffer(struct coresight_device *csdev, - struct perf_output_handle *handle, - void *sink_config) -{ - long size = 0; - struct cs_buffers *buf = sink_config; - - if (buf) { - /* - * In snapshot mode ->data_size holds the new address of the - * ring buffer's head. The size itself is the whole address - * range since we want the latest information. - */ - if (buf->snapshot) - handle->head = local_xchg(&buf->data_size, - buf->nr_pages << PAGE_SHIFT); - /* - * Tell the tracer PMU how much we got in this run and if - * something went wrong along the way. Nobody else can use - * this cs_buffers instance until we are done. As such - * resetting parameters here and squaring off with the ring - * buffer API in the tracer PMU is fine. - */ - size = local_xchg(&buf->data_size, 0); - } - - return size; -} - -static void tmc_update_etf_buffer(struct coresight_device *csdev, +static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev, struct perf_output_handle *handle, void *sink_config) { @@ -387,17 +358,17 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev, const u32 *barrier; u32 *buf_ptr; u64 read_ptr, write_ptr; - u32 status, to_read; - unsigned long offset; + u32 status; + unsigned long offset, to_read; struct cs_buffers *buf = sink_config; struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
if (!buf) - return; + return 0;
/* This shouldn't happen */ if (WARN_ON_ONCE(drvdata->mode != CS_MODE_PERF)) - return; + return 0;
CS_UNLOCK(drvdata->base);
@@ -486,18 +457,14 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev, } }
- /* - * In snapshot mode all we have to do is communicate to - * perf_aux_output_end() the address of the current head. In full - * trace mode the same function expects a size to move rb->aux_head - * forward. - */ - if (buf->snapshot) - local_set(&buf->data_size, (cur * PAGE_SIZE) + offset); - else - local_add(to_read, &buf->data_size); - + /* In snapshot mode we have to update the head */ + if (buf->snapshot) { + handle->head = (cur * PAGE_SIZE) + offset; + to_read = buf->nr_pages << PAGE_SHIFT; + } CS_LOCK(drvdata->base); + + return to_read; }
static const struct coresight_ops_sink tmc_etf_sink_ops = { @@ -506,7 +473,6 @@ static const struct coresight_ops_sink tmc_etf_sink_ops = { .alloc_buffer = tmc_alloc_etf_buffer, .free_buffer = tmc_free_etf_buffer, .set_buffer = tmc_set_etf_buffer, - .reset_buffer = tmc_reset_etf_buffer, .update_buffer = tmc_update_etf_buffer, };
diff --git a/include/linux/coresight.h b/include/linux/coresight.h index d828a6e..eafe4d1 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -191,7 +191,6 @@ struct coresight_device { * @alloc_buffer: initialises perf's ring buffer for trace collection. * @free_buffer: release memory allocated in @get_config. * @set_buffer: initialises buffer mechanic before a trace session. - * @reset_buffer: finalises buffer mechanic after a trace session. * @update_buffer: update buffer pointers after a trace session. */ struct coresight_ops_sink { @@ -203,10 +202,7 @@ struct coresight_ops_sink { int (*set_buffer)(struct coresight_device *csdev, struct perf_output_handle *handle, void *sink_config); - unsigned long (*reset_buffer)(struct coresight_device *csdev, - struct perf_output_handle *handle, - void *sink_config); - void (*update_buffer)(struct coresight_device *csdev, + unsigned long (*update_buffer)(struct coresight_device *csdev, struct perf_output_handle *handle, void *sink_config); };
We can always find the sink configuration for a given perf_output_handle. Add a helper to retrieve the sink configuration for a given perf_output_handle. This will be used to get rid of the set_buffer() call back.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-etm-perf.c | 14 ------------- drivers/hwtracing/coresight/coresight-etm-perf.h | 26 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index ad87441..16b83d8 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -23,20 +23,6 @@ static struct pmu etm_pmu; static bool etm_perf_up;
-/** - * struct etm_event_data - Coresight specifics associated to an event - * @work: Handle to free allocated memory outside IRQ context. - * @mask: Hold the CPU(s) this event was set for. - * @snk_config: The sink configuration. - * @path: An array of path, each slot for one CPU. - */ -struct etm_event_data { - struct work_struct work; - cpumask_t mask; - void *snk_config; - struct list_head * __percpu *path; -}; - static DEFINE_PER_CPU(struct perf_output_handle, ctx_handle); static DEFINE_PER_CPU(struct coresight_device *, csdev_src);
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h index 4197df4..da7d933 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.h +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h @@ -7,6 +7,7 @@ #ifndef _CORESIGHT_ETM_PERF_H #define _CORESIGHT_ETM_PERF_H
+#include <linux/percpu-defs.h> #include "coresight-priv.h"
struct coresight_device; @@ -42,14 +43,39 @@ struct etm_filters { bool ssstatus; };
+/** + * struct etm_event_data - Coresight specifics associated to an event + * @work: Handle to free allocated memory outside IRQ context. + * @mask: Hold the CPU(s) this event was set for. + * @snk_config: The sink configuration. + * @path: An array of path, each slot for one CPU. + */ +struct etm_event_data { + struct work_struct work; + cpumask_t mask; + void *snk_config; + struct list_head * __percpu *path; +};
#ifdef CONFIG_CORESIGHT int etm_perf_symlink(struct coresight_device *csdev, bool link); +static inline void *etm_perf_sink_config(struct perf_output_handle *handle) +{ + struct etm_event_data *data = perf_get_aux(handle);
+ if (data) + return data->snk_config; + return NULL; +} #else static inline int etm_perf_symlink(struct coresight_device *csdev, bool link) { return -EINVAL; }
+static inline void *etm_perf_sink_config(struct perf_output_handle *handle) +{ + return NULL; +} + #endif /* CONFIG_CORESIGHT */
#endif
In coresight perf mode, we need to prepare the sink before starting a session, which is done via set_buffer call back. We then proceed to enable the tracing. If we fail to start the session successfully, we leave the sink configuration unchanged. In order to make the operation atomic and to avoid yet another call back to clear the buffer, we get rid of the "set_buffer" call back and pass the buffer details via enable() call back to the sink.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-etb10.c | 32 ++++++++++++++++++------ drivers/hwtracing/coresight/coresight-etm-perf.c | 9 ++----- drivers/hwtracing/coresight/coresight-priv.h | 2 +- drivers/hwtracing/coresight/coresight-tmc-etf.c | 28 ++++++++++++++------- drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 +++--- drivers/hwtracing/coresight/coresight-tpiu.c | 2 +- drivers/hwtracing/coresight/coresight.c | 11 ++++---- include/linux/coresight.h | 6 +---- 8 files changed, 59 insertions(+), 38 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 60bfc7b..70d6f1e 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -28,6 +28,7 @@
#include "coresight-priv.h" +#include "coresight-etm-perf.h"
#define ETB_RAM_DEPTH_REG 0x004 #define ETB_STATUS_REG 0x00c @@ -90,6 +91,9 @@ struct etb_drvdata { u32 trigger_cntr; };
+static int etb_set_buffer(struct coresight_device *csdev, + struct perf_output_handle *handle); + static unsigned int etb_get_buffer_depth(struct etb_drvdata *drvdata) { u32 depth = 0; @@ -131,12 +135,24 @@ static void etb_enable_hw(struct etb_drvdata *drvdata) CS_LOCK(drvdata->base); }
-static int etb_enable(struct coresight_device *csdev, u32 mode) +static int etb_enable(struct coresight_device *csdev, u32 mode, void *data) { + int ret = 0; u32 val; unsigned long flags; struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+ /* + * We don't have an internal state to clean up if we fail to setup + * the perf buffer. So we can perform the step before we turn the + * ETB on and leave without cleaning up. + */ + if (mode == CS_MODE_PERF) { + ret = etb_set_buffer(csdev, (struct perf_output_handle *)data); + if (ret) + goto out; + } + val = local_cmpxchg(&drvdata->mode, CS_MODE_DISABLED, mode); /* @@ -160,8 +176,9 @@ static int etb_enable(struct coresight_device *csdev, u32 mode) spin_unlock_irqrestore(&drvdata->spinlock, flags);
out: - dev_dbg(drvdata->dev, "ETB enabled\n"); - return 0; + if (!ret) + dev_dbg(drvdata->dev, "ETB enabled\n"); + return ret; }
static void etb_disable_hw(struct etb_drvdata *drvdata) @@ -298,12 +315,14 @@ static void etb_free_buffer(void *config) }
static int etb_set_buffer(struct coresight_device *csdev, - struct perf_output_handle *handle, - void *sink_config) + struct perf_output_handle *handle) { int ret = 0; unsigned long head; - struct cs_buffers *buf = sink_config; + struct cs_buffers *buf = etm_perf_sink_config(handle); + + if (!buf) + return -EINVAL;
/* wrap head around to the amount of space we have */ head = handle->head & ((buf->nr_pages << PAGE_SHIFT) - 1); @@ -457,7 +476,6 @@ static const struct coresight_ops_sink etb_sink_ops = { .disable = etb_disable, .alloc_buffer = etb_alloc_buffer, .free_buffer = etb_free_buffer, - .set_buffer = etb_set_buffer, .update_buffer = etb_update_buffer, };
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 16b83d8..abe8249 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -286,16 +286,11 @@ static void etm_event_start(struct perf_event *event, int flags) path = etm_event_cpu_path(event_data, cpu); /* We need a sink, no need to continue without one */ sink = coresight_get_sink(path); - if (WARN_ON_ONCE(!sink || !sink_ops(sink)->set_buffer)) - goto fail_end_stop; - - /* Configure the sink */ - if (sink_ops(sink)->set_buffer(sink, handle, - event_data->snk_config)) + if (WARN_ON_ONCE(!sink)) goto fail_end_stop;
/* Nothing will happen without a path */ - if (coresight_enable_path(path, CS_MODE_PERF)) + if (coresight_enable_path(path, CS_MODE_PERF, handle)) goto fail_end_stop;
/* Tell the perf core the event is alive */ diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 1a6cf35..c11da55 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -137,7 +137,7 @@ static inline void coresight_write_reg_pair(void __iomem *addr, u64 val, }
void coresight_disable_path(struct list_head *path); -int coresight_enable_path(struct list_head *path, u32 mode); +int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data); struct coresight_device *coresight_get_sink(struct list_head *path); struct coresight_device *coresight_get_enabled_sink(bool reset); struct list_head *coresight_build_path(struct coresight_device *csdev, diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index 31a98f9..4156c95 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -10,6 +10,10 @@ #include <linux/slab.h> #include "coresight-priv.h" #include "coresight-tmc.h" +#include "coresight-etm-perf.h" + +static int tmc_set_etf_buffer(struct coresight_device *csdev, + struct perf_output_handle *handle);
static void tmc_etb_enable_hw(struct tmc_drvdata *drvdata) { @@ -182,11 +186,12 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev) return ret; }
-static int tmc_enable_etf_sink_perf(struct coresight_device *csdev) +static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, void *data) { int ret = 0; unsigned long flags; struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct perf_output_handle *handle = data;
spin_lock_irqsave(&drvdata->spinlock, flags); if (drvdata->reading) { @@ -204,15 +209,19 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev) goto out; }
- drvdata->mode = CS_MODE_PERF; - tmc_etb_enable_hw(drvdata); + ret = tmc_set_etf_buffer(csdev, handle); + if (!ret) { + drvdata->mode = CS_MODE_PERF; + tmc_etb_enable_hw(drvdata); + } out: spin_unlock_irqrestore(&drvdata->spinlock, flags);
return ret; }
-static int tmc_enable_etf_sink(struct coresight_device *csdev, u32 mode) +static int tmc_enable_etf_sink(struct coresight_device *csdev, + u32 mode, void *data) { int ret; struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); @@ -222,7 +231,7 @@ static int tmc_enable_etf_sink(struct coresight_device *csdev, u32 mode) ret = tmc_enable_etf_sink_sysfs(csdev); break; case CS_MODE_PERF: - ret = tmc_enable_etf_sink_perf(csdev); + ret = tmc_enable_etf_sink_perf(csdev, data); break; /* We shouldn't be here */ default: @@ -328,12 +337,14 @@ static void tmc_free_etf_buffer(void *config) }
static int tmc_set_etf_buffer(struct coresight_device *csdev, - struct perf_output_handle *handle, - void *sink_config) + struct perf_output_handle *handle) { int ret = 0; unsigned long head; - struct cs_buffers *buf = sink_config; + struct cs_buffers *buf = etm_perf_sink_config(handle); + + if (!buf) + return -EINVAL;
/* wrap head around to the amount of space we have */ head = handle->head & ((buf->nr_pages << PAGE_SHIFT) - 1); @@ -472,7 +483,6 @@ static const struct coresight_ops_sink tmc_etf_sink_ops = { .disable = tmc_disable_etf_sink, .alloc_buffer = tmc_alloc_etf_buffer, .free_buffer = tmc_free_etf_buffer, - .set_buffer = tmc_set_etf_buffer, .update_buffer = tmc_update_etf_buffer, };
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 85a3f59..bc0c932 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -1103,19 +1103,20 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) return ret; }
-static int tmc_enable_etr_sink_perf(struct coresight_device *csdev) +static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data) { /* We don't support perf mode yet ! */ return -EINVAL; }
-static int tmc_enable_etr_sink(struct coresight_device *csdev, u32 mode) +static int tmc_enable_etr_sink(struct coresight_device *csdev, + u32 mode, void *data) { switch (mode) { case CS_MODE_SYSFS: return tmc_enable_etr_sink_sysfs(csdev); case CS_MODE_PERF: - return tmc_enable_etr_sink_perf(csdev); + return tmc_enable_etr_sink_perf(csdev, data); }
/* We shouldn't be here */ diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c index 7daeb084..d7dd5e4 100644 --- a/drivers/hwtracing/coresight/coresight-tpiu.c +++ b/drivers/hwtracing/coresight/coresight-tpiu.c @@ -67,7 +67,7 @@ static void tpiu_enable_hw(struct tpiu_drvdata *drvdata) CS_LOCK(drvdata->base); }
-static int tpiu_enable(struct coresight_device *csdev, u32 mode) +static int tpiu_enable(struct coresight_device *csdev, u32 mode, void *__unused) { struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index c0dabbd..fab87c9 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -128,7 +128,8 @@ static int coresight_find_link_outport(struct coresight_device *csdev, return -ENODEV; }
-static int coresight_enable_sink(struct coresight_device *csdev, u32 mode) +static int coresight_enable_sink(struct coresight_device *csdev, + u32 mode, void *data) { int ret;
@@ -137,7 +138,7 @@ static int coresight_enable_sink(struct coresight_device *csdev, u32 mode) * existing "mode" of operation. */ if (sink_ops(csdev)->enable) { - ret = sink_ops(csdev)->enable(csdev, mode); + ret = sink_ops(csdev)->enable(csdev, mode, data); if (ret) return ret; csdev->enable = true; @@ -315,7 +316,7 @@ void coresight_disable_path(struct list_head *path) } }
-int coresight_enable_path(struct list_head *path, u32 mode) +int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data) {
int ret = 0; @@ -340,7 +341,7 @@ int coresight_enable_path(struct list_head *path, u32 mode)
switch (type) { case CORESIGHT_DEV_TYPE_SINK: - ret = coresight_enable_sink(csdev, mode); + ret = coresight_enable_sink(csdev, mode, sink_data); /* * Sink is the first component turned on. If we * failed to enable the sink, there are no components @@ -643,7 +644,7 @@ int coresight_enable(struct coresight_device *csdev) goto out; }
- ret = coresight_enable_path(path, CS_MODE_SYSFS); + ret = coresight_enable_path(path, CS_MODE_SYSFS, NULL); if (ret) goto err_path;
diff --git a/include/linux/coresight.h b/include/linux/coresight.h index eafe4d1..3434758 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -190,18 +190,14 @@ struct coresight_device { * @disable: disables the sink. * @alloc_buffer: initialises perf's ring buffer for trace collection. * @free_buffer: release memory allocated in @get_config. - * @set_buffer: initialises buffer mechanic before a trace session. * @update_buffer: update buffer pointers after a trace session. */ struct coresight_ops_sink { - int (*enable)(struct coresight_device *csdev, u32 mode); + int (*enable)(struct coresight_device *csdev, u32 mode, void *data); void (*disable)(struct coresight_device *csdev); void *(*alloc_buffer)(struct coresight_device *csdev, int cpu, void **pages, int nr_pages, bool overwrite); void (*free_buffer)(void *config); - int (*set_buffer)(struct coresight_device *csdev, - struct perf_output_handle *handle, - void *sink_config); unsigned long (*update_buffer)(struct coresight_device *csdev, struct perf_output_handle *handle, void *sink_config);
Add support for using TMC-ETR as backend for ETM perf tracing. We use software double buffering at the moment. i.e, the TMC-ETR uses a separate buffer than the perf ring buffer. The data is copied to the perf ring buffer once a session completes.
The TMC-ETR would try to match the larger of perf ring buffer or the ETR buffer size configured via sysfs, scaling down to a minimum limit of 1MB.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- Changes since v2: - Fix typo with duplicated CS_UNLOCK - Rename call backs from tmc_etr_<op>_perf_buffer => tmc_<op>_etr_buffer to be consistent with ETF driver --- drivers/hwtracing/coresight/coresight-tmc-etr.c | 248 +++++++++++++++++++++++- drivers/hwtracing/coresight/coresight-tmc.h | 2 + 2 files changed, 248 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index bc0c932..a4e6c48 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -10,6 +10,7 @@ #include <linux/slab.h> #include <linux/vmalloc.h> #include "coresight-catu.h" +#include "coresight-etm-perf.h" #include "coresight-priv.h" #include "coresight-tmc.h"
@@ -21,6 +22,28 @@ struct etr_flat_buf { };
/* + * etr_perf_buffer - Perf buffer used for ETR + * @etr_buf - Actual buffer used by the ETR + * @snaphost - Perf session mode + * @head - handle->head at the beginning of the session. + * @nr_pages - Number of pages in the ring buffer. + * @pages - Array of Pages in the ring buffer. + */ +struct etr_perf_buffer { + struct etr_buf *etr_buf; + bool snapshot; + unsigned long head; + int nr_pages; + void **pages; +}; + +/* Convert the perf index to an offset within the ETR buffer */ +#define PERF_IDX2OFF(idx, buf) ((idx) % ((buf)->nr_pages << PAGE_SHIFT)) + +/* Lower limit for ETR hardware buffer */ +#define TMC_ETR_PERF_MIN_BUF_SIZE SZ_1M + +/* * The TMC ETR SG has a page size of 4K. The SG table contains pointers * to 4KB buffers. However, the OS may use a PAGE_SIZE different from * 4K (i.e, 16KB or 64KB). This implies that a single OS page could @@ -1103,10 +1126,228 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) return ret; }
+/* + * tmc_etr_setup_perf_buf: Allocate ETR buffer for use by perf. + * The size of the hardware buffer is dependent on the size configured + * via sysfs and the perf ring buffer size. We prefer to allocate the + * largest possible size, scaling down the size by half until it + * reaches a minimum limit (1M), beyond which we give up. + */ +static struct etr_perf_buffer * +tmc_etr_setup_perf_buf(struct tmc_drvdata *drvdata, int node, int nr_pages, + void **pages, bool snapshot) +{ + struct etr_buf *etr_buf; + struct etr_perf_buffer *etr_perf; + unsigned long size; + + etr_perf = kzalloc_node(sizeof(*etr_perf), GFP_KERNEL, node); + if (!etr_perf) + return ERR_PTR(-ENOMEM); + + /* + * Try to match the perf ring buffer size if it is larger + * than the size requested via sysfs. + */ + if ((nr_pages << PAGE_SHIFT) > drvdata->size) { + etr_buf = tmc_alloc_etr_buf(drvdata, (nr_pages << PAGE_SHIFT), + 0, node, NULL); + if (!IS_ERR(etr_buf)) + goto done; + } + + /* + * Else switch to configured size for this ETR + * and scale down until we hit the minimum limit. + */ + size = drvdata->size; + do { + etr_buf = tmc_alloc_etr_buf(drvdata, size, 0, node, NULL); + if (!IS_ERR(etr_buf)) + goto done; + size /= 2; + } while (size >= TMC_ETR_PERF_MIN_BUF_SIZE); + + kfree(etr_perf); + return ERR_PTR(-ENOMEM); + +done: + etr_perf->etr_buf = etr_buf; + return etr_perf; +} + + +static void *tmc_alloc_etr_buffer(struct coresight_device *csdev, + int cpu, void **pages, int nr_pages, + bool snapshot) +{ + struct etr_perf_buffer *etr_perf; + struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + + if (cpu == -1) + cpu = smp_processor_id(); + + etr_perf = tmc_etr_setup_perf_buf(drvdata, cpu_to_node(cpu), + nr_pages, pages, snapshot); + if (IS_ERR(etr_perf)) { + dev_dbg(drvdata->dev, "Unable to allocate ETR buffer\n"); + return NULL; + } + + etr_perf->snapshot = snapshot; + etr_perf->nr_pages = nr_pages; + etr_perf->pages = pages; + + return etr_perf; +} + +static void tmc_free_etr_buffer(void *config) +{ + struct etr_perf_buffer *etr_perf = config; + + if (etr_perf->etr_buf) + tmc_free_etr_buf(etr_perf->etr_buf); + kfree(etr_perf); +} + +/* + * tmc_etr_sync_perf_buffer: Copy the actual trace data from the hardware + * buffer to the perf ring buffer. + */ +static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf) +{ + long bytes, to_copy; + long pg_idx, pg_offset, src_offset; + unsigned long head = etr_perf->head; + char **dst_pages, *src_buf; + struct etr_buf *etr_buf = etr_perf->etr_buf; + + head = etr_perf->head; + pg_idx = head >> PAGE_SHIFT; + pg_offset = head & (PAGE_SIZE - 1); + dst_pages = (char **)etr_perf->pages; + src_offset = etr_buf->offset; + to_copy = etr_buf->len; + + while (to_copy > 0) { + /* + * In one iteration, we can copy minimum of : + * 1) what is available in the source buffer, + * 2) what is available in the source buffer, before it + * wraps around. + * 3) what is available in the destination page. + * in one iteration. + */ + bytes = tmc_etr_buf_get_data(etr_buf, src_offset, to_copy, + &src_buf); + if (WARN_ON_ONCE(bytes <= 0)) + break; + bytes = min(bytes, (long)(PAGE_SIZE - pg_offset)); + + memcpy(dst_pages[pg_idx] + pg_offset, src_buf, bytes); + + to_copy -= bytes; + + /* Move destination pointers */ + pg_offset += bytes; + if (pg_offset == PAGE_SIZE) { + pg_offset = 0; + if (++pg_idx == etr_perf->nr_pages) + pg_idx = 0; + } + + /* Move source pointers */ + src_offset += bytes; + if (src_offset >= etr_buf->size) + src_offset -= etr_buf->size; + } +} + +/* + * tmc_update_etr_buffer : Update the perf ring buffer with the + * available trace data. We use software double buffering at the moment. + * + * TODO: Add support for reusing the perf ring buffer. + */ +static unsigned long +tmc_update_etr_buffer(struct coresight_device *csdev, + struct perf_output_handle *handle, + void *config) +{ + bool lost = false; + unsigned long flags, size = 0; + struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct etr_perf_buffer *etr_perf = config; + struct etr_buf *etr_buf = etr_perf->etr_buf; + + spin_lock_irqsave(&drvdata->spinlock, flags); + if (WARN_ON(drvdata->perf_data != etr_perf)) { + lost = true; + spin_unlock_irqrestore(&drvdata->spinlock, flags); + goto out; + } + + CS_UNLOCK(drvdata->base); + + tmc_flush_and_stop(drvdata); + tmc_sync_etr_buf(drvdata); + + CS_LOCK(drvdata->base); + /* Reset perf specific data */ + drvdata->perf_data = NULL; + spin_unlock_irqrestore(&drvdata->spinlock, flags); + + size = etr_buf->len; + tmc_etr_sync_perf_buffer(etr_perf); + + /* + * Update handle->head in snapshot mode. Also update the size to the + * hardware buffer size if there was an overflow. + */ + if (etr_perf->snapshot) { + handle->head += size; + if (etr_buf->full) + size = etr_buf->size; + } + + lost |= etr_buf->full; +out: + if (lost) + perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); + return size; +} + static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data) { - /* We don't support perf mode yet ! */ - return -EINVAL; + int rc = 0; + unsigned long flags; + struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct perf_output_handle *handle = data; + struct etr_perf_buffer *etr_perf = etm_perf_sink_config(handle); + + spin_lock_irqsave(&drvdata->spinlock, flags); + /* + * There can be only one writer per sink in perf mode. If the sink + * is already open in SYSFS mode, we can't use it. + */ + if (drvdata->mode != CS_MODE_DISABLED || WARN_ON(drvdata->perf_data)) { + rc = -EBUSY; + goto unlock_out; + } + + if (WARN_ON(!etr_perf || !etr_perf->etr_buf)) { + rc = -EINVAL; + goto unlock_out; + } + + etr_perf->head = PERF_IDX2OFF(handle->head, etr_perf); + drvdata->perf_data = etr_perf; + drvdata->mode = CS_MODE_PERF; + tmc_etr_enable_hw(drvdata, etr_perf->etr_buf); + +unlock_out: + spin_unlock_irqrestore(&drvdata->spinlock, flags); + return rc; }
static int tmc_enable_etr_sink(struct coresight_device *csdev, @@ -1148,6 +1389,9 @@ static void tmc_disable_etr_sink(struct coresight_device *csdev) static const struct coresight_ops_sink tmc_etr_sink_ops = { .enable = tmc_enable_etr_sink, .disable = tmc_disable_etr_sink, + .alloc_buffer = tmc_alloc_etr_buffer, + .update_buffer = tmc_update_etr_buffer, + .free_buffer = tmc_free_etr_buffer, };
const struct coresight_ops tmc_etr_cs_ops = { diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 872f63e..487c537 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -170,6 +170,7 @@ struct etr_buf { * @trigger_cntr: amount of words to store after a trigger. * @etr_caps: Bitmask of capabilities of the TMC ETR, inferred from the * device configuration register (DEVID) + * @perf_data: PERF buffer for ETR. * @sysfs_data: SYSFS buffer for ETR. */ struct tmc_drvdata { @@ -191,6 +192,7 @@ struct tmc_drvdata { u32 trigger_cntr; u32 etr_caps; struct etr_buf *sysfs_buf; + void *perf_data; };
struct etr_buf_operations {
-
Mathieu Poirier -
Suzuki K Poulose