When the CPU enters a low power mode, the TRFCR_EL1 contents could be reset. Thus we need to save/restore the TRFCR_EL1 along with the ETM4x registers to allow the tracing.

Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- .../coresight/coresight-etm4x-core.c | 52 ++++++++++++++----- drivers/hwtracing/coresight/coresight-etm4x.h | 2 + 2 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index da27cd4a3c38..6bbe4da31d9b 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -236,6 +236,17 @@ struct etm4_enable_arg { int rc; };

+static inline u64 read_trfcr(void) +{ + return read_sysreg_s(SYS_TRFCR_EL1); +} + +static inline void write_trfcr(u64 val) +{ + write_sysreg_s(val, SYS_TRFCR_EL1); + isb(); +} + #ifdef CONFIG_ETM4X_IMPDEF_FEATURE

#define HISI_HIP08_AMBA_ID 0x000b6d01 @@ -985,7 +996,7 @@ static void cpu_enable_tracing(struct etmv4_drvdata *drvdata) if (is_kernel_in_hyp_mode()) trfcr |= TRFCR_EL2_CX;

- write_sysreg_s(trfcr, SYS_TRFCR_EL1); + write_trfcr(trfcr); }

static void etm4_init_arch_data(void *info) @@ -1528,7 +1539,7 @@ static void etm4_init_trace_id(struct etmv4_drvdata *drvdata) drvdata->trcid = coresight_get_trace_id(drvdata->cpu); }

-static int etm4_cpu_save(struct etmv4_drvdata *drvdata) +static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) { int i, ret = 0; struct etmv4_save_state *state; @@ -1667,7 +1678,22 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) return ret; }

-static void etm4_cpu_restore(struct etmv4_drvdata *drvdata) +static int etm4_cpu_save(struct etmv4_drvdata *drvdata) +{ + int ret = 0; + + if (drvdata->trfc) + drvdata->save_trfcr = read_trfcr(); + /* + * Save and restore the ETM Trace registers only if + * the ETM is active. + */ + if (local_read(&drvdata->mode) && drvdata->save_state) + ret = __etm4_cpu_save(drvdata); + return ret; +} + +static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata) { int i; struct etmv4_save_state *state = drvdata->save_state; @@ -1763,6 +1789,14 @@ static void etm4_cpu_restore(struct etmv4_drvdata *drvdata) etm4_cs_lock(drvdata, csa); }

+static void etm4_cpu_restore(struct etmv4_drvdata *drvdata) +{ + if (drvdata->trfc) + write_trfcr(drvdata->save_trfcr); + if (drvdata->state_needs_restore) + __etm4_cpu_restore(drvdata); +} + static int etm4_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, void *v) { @@ -1774,23 +1808,17 @@ static int etm4_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,

drvdata = etmdrvdata[cpu];

- if (!drvdata->save_state) - return NOTIFY_OK; - if (WARN_ON_ONCE(drvdata->cpu != cpu)) return NOTIFY_BAD;

switch (cmd) { case CPU_PM_ENTER: - /* save the state if self-hosted coresight is in use */ - if (local_read(&drvdata->mode)) - if (etm4_cpu_save(drvdata)) - return NOTIFY_BAD; + if (etm4_cpu_save(drvdata)) + return NOTIFY_BAD; break; case CPU_PM_EXIT: case CPU_PM_ENTER_FAILED: - if (drvdata->state_needs_restore) - etm4_cpu_restore(drvdata); + etm4_cpu_restore(drvdata); break; default: return NOTIFY_DONE; diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index e5b79bdb9851..82cba16b73a6 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -921,6 +921,7 @@ struct etmv4_save_state { * @lpoverride: If the implementation can support low-power state over. * @trfc: If the implementation supports Arm v8.4 trace filter controls. * @config: structure holding configuration parameters. + * @save_trfcr: Saved TRFCR_EL1 register during a CPU PM event. * @save_state: State to be preserved across power loss * @state_needs_restore: True when there is context to restore after PM exit * @skip_power_up: Indicates if an implementation can skip powering up @@ -973,6 +974,7 @@ struct etmv4_drvdata { bool lpoverride; bool trfc; struct etmv4_config config; + u64 save_trfcr; struct etmv4_save_state *save_state; bool state_needs_restore; bool skip_power_up;

When an AUX buffer is marked TRUNCATED, the kernel will disable the event (via irq_work) and can only be re-enabled by the userspace tool.

Also, since we *always* mark the buffer as TRUNCATED (which is needs to be reconsidered, see below), we need not re-enable the TRBE as the event is going to be now disabled. This follows the SPE driver behavior.

Without this change, we could leave the event disabled for ever under certain conditions. i.e, when we try to re-enable in the IRQ handler, if there is no space left in the buffer, we "TRUNCATE" the buffer and create a record of size 0. The perf tool skips such records and the event will remain disabled forever.

Regarding the use of TRUNCATED flag: With FILL mode, the TRBE stops collection when the buffer is full, raising the IRQ. Now, since the IRQ is asynchronous, we could loose some amount of trace, after the buffer was full until the IRQ is handled. Also the last packet may have been trimmed. The decoder can figure this out and cope with this. The side effect of this approach is:

We always disable the event when there is an IRQ, even when the other half of the ring buffer is free ! This is not ideal.

Now, we should switch to using PARTIAL to indicate that there was potentially some partial trace packets in the buffer and some data was lost. We should use TRUNCATED only when there is absolutely no space in the ring buffer. This change would also require some additional changes in the CoreSight PMU framework to allow, sinks to "close" the handle (rather than the PMU driver closing the handle upon event_stop). So, until that is sorted, let us keep the TRUNCATED flag and the rework can be addressed separately.

Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver") Reported-by: Tamas Zsoldos Tamas.Zsoldos@arm.com Cc: Anshuman Khandual anshuman.khandual@arm.com Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Peter Zijlstra (Intel) peterz@infradead.org Cc: Will Deacon will@kernel.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 34 +++++++------------- 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 176868496879..ec38cf17b693 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -696,10 +696,8 @@ static void trbe_handle_spurious(struct perf_output_handle *handle)

static void trbe_handle_overflow(struct perf_output_handle *handle) { - struct perf_event *event = handle->event; struct trbe_buf *buf = etm_perf_sink_config(handle); unsigned long offset, size; - struct etm_event_data *event_data;

offset = get_trbe_limit_pointer() - get_trbe_base_pointer(); size = offset - PERF_IDX2OFF(handle->head, buf); @@ -709,30 +707,22 @@ static void trbe_handle_overflow(struct perf_output_handle *handle) /* * Mark the buffer as truncated, as we have stopped the trace * collection upon the WRAP event, without stopping the source. + * + * We don't re-enable the TRBE here, as the event is + * bound to be disabled due to the TRUNCATED flag. + * This is not ideal, as we could use the available space in + * the ring buffer and continue the tracing. + * + * TODO: Revisit the use of TRUNCATED flag and may be instead use + * PARTIAL, to indicate trace may contain partial packets. + * And TRUNCATED can be used only if we do not have enough space + * in the buffer. This would need additional changes in + * etm_event_stop() to allow the sinks to leave a closed + * aux_handle. */ perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW | PERF_AUX_FLAG_TRUNCATED); perf_aux_output_end(handle, size); - event_data = perf_aux_output_begin(handle, event); - if (!event_data) { - /* - * We are unable to restart the trace collection, - * thus leave the TRBE disabled. The etm-perf driver - * is able to detect this with a disconnected handle - * (handle->event = NULL). - */ - trbe_drain_and_disable_local(); - *this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL; - return; - } - buf->trbe_limit = compute_trbe_buffer_limit(handle); - buf->trbe_write = buf->trbe_base + PERF_IDX2OFF(handle->head, buf); - if (buf->trbe_limit == buf->trbe_base) { - trbe_stop_and_truncate_event(handle); - return; - } - *this_cpu_ptr(buf->cpudata->drvdata->handle) = handle; - trbe_enable_hw(buf); }

static bool is_perf_trbe(struct perf_output_handle *handle)

When the TRBE generates an IRQ, we stop the TRBE, collect the trace and then reprogram the TRBE with the updated buffer pointers in case of a spurious IRQ. We might also leave the TRBE disabled, on an overflow interrupt, without touching the ETE. This means the the ETE is only disabled when the event is disabled later (via irq_work). This is incorrect, as the ETE trace is still ON without actually being captured and may be routed to the ATB.

So, we move the CPU into trace prohibited state (for all exception levels) upon entering the IRQ handler. The state is restored before enabling the TRBE back. Otherwise the trace remains prohibited. Since, the ETM/ETE driver controls the TRFCR_EL1 per session, (from commit "coresight: etm4x: Use Trace Filtering controls dynamically") the tracing can be restored/enabled back when the event is rescheduled in.

Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver") Cc: Anshuman Khandual anshuman.khandual@arm.com Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 43 ++++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index c0c264264427..e4d88e0de2a8 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -83,6 +83,31 @@ struct trbe_drvdata { struct platform_device *pdev; };

+static inline void write_trfcr(u64 val) +{ + write_sysreg_s(val, SYS_TRFCR_EL1); + isb(); +} + +/* + * Prohibit the CPU tracing at all ELs, in preparation to collect + * the trace buffer. + * + * Returns the original value of the trfcr for restoring later. + */ +static u64 cpu_prohibit_tracing(void) +{ + u64 trfcr = read_sysreg_s(SYS_TRFCR_EL1); + + write_trfcr(trfcr & ~(TRFCR_ELx_ExTRE | TRFCR_ELx_E0TRE)); + return trfcr; +} + +static void cpu_restore_tracing(u64 trfcr) +{ + write_trfcr(trfcr); +} + static int trbe_alloc_node(struct perf_event *event) { if (event->cpu == -1) @@ -681,7 +706,7 @@ static int arm_trbe_disable(struct coresight_device *csdev) return 0; }

-static void trbe_handle_spurious(struct perf_output_handle *handle) +static void trbe_handle_spurious(struct perf_output_handle *handle, u64 trfcr) { struct trbe_buf *buf = etm_perf_sink_config(handle);

@@ -691,6 +716,7 @@ static void trbe_handle_spurious(struct perf_output_handle *handle) trbe_drain_and_disable_local(); return; } + cpu_restore_tracing(trfcr); trbe_enable_hw(buf); }

@@ -760,7 +786,18 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) struct perf_output_handle **handle_ptr = dev; struct perf_output_handle *handle = *handle_ptr; enum trbe_fault_action act; - u64 status; + u64 status, trfcr; + + /* + * Prohibit the tracing, while we process this. We turn + * things back right, if we get to enabling the TRBE + * back again. Otherwise, the tracing still remains + * prohibited, until the perf event state changes + * or another event is scheduled. This ensures that + * the trace is not generated when it cannot be + * captured. + */ + trfcr = cpu_prohibit_tracing();

/* * Ensure the trace is visible to the CPUs and @@ -791,7 +828,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) trbe_handle_overflow(handle); break; case TRBE_FAULT_ACT_SPURIOUS: - trbe_handle_spurious(handle); + trbe_handle_spurious(handle, trfcr); break; case TRBE_FAULT_ACT_FATAL: trbe_stop_and_truncate_event(handle);

On 12/07/2021 12:38, Suzuki K Poulose wrote:

When an AUX buffer is marked TRUNCATED, the kernel will disable the event (via irq_work) and can only be re-enabled by the userspace tool.

Also, since we *always* mark the buffer as TRUNCATED (which is needs to be reconsidered, see below), we need not re-enable the TRBE as the event is going to be now disabled. This follows the SPE driver behavior.

Without this change, we could leave the event disabled for ever under certain conditions. i.e, when we try to re-enable in the IRQ handler, if there is no space left in the buffer, we "TRUNCATE" the buffer and create a record of size 0. The perf tool skips such records and the event will remain disabled forever.

Regarding the use of TRUNCATED flag: With FILL mode, the TRBE stops collection when the buffer is full, raising the IRQ. Now, since the IRQ is asynchronous, we could loose some amount of trace, after the buffer was full until the IRQ is handled. Also the last packet may have been trimmed. The decoder can figure this out and cope with this. The side effect of this approach is:

We always disable the event when there is an IRQ, even when the other half of the ring buffer is free ! This is not ideal.

Now, we should switch to using PARTIAL to indicate that there was potentially some partial trace packets in the buffer and some data was lost. We should use TRUNCATED only when there is absolutely no space in the ring buffer. This change would also require some additional changes in the CoreSight PMU framework to allow, sinks to "close" the handle (rather than the PMU driver closing the handle upon event_stop). So, until that is sorted, let us keep the TRUNCATED flag and the rework can be addressed separately.

Based on an offline discussion, we have decided to drop the TRUNCATED flag and use PARTIAL to indicate the IRQ fired and some trace packets may be trimmed. So, I will rework the fix to accommodate these changes and resend the series. Please ignore the patches 3-5 in the series.

The first two patches are still valid, but I will send them in as separate for ease of merging.

Kind regards Suzuki

Fixes: 3fbf7f011f24 ("coresight: sink: Add TRBE driver") Reported-by: Tamas Zsoldos Tamas.Zsoldos@arm.com Cc: Anshuman Khandual anshuman.khandual@arm.com Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Peter Zijlstra (Intel) peterz@infradead.org Cc: Will Deacon will@kernel.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

---

drivers/hwtracing/coresight/coresight-trbe.c | 34 +++++++------------- 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 176868496879..ec38cf17b693 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -696,10 +696,8 @@ static void trbe_handle_spurious(struct perf_output_handle *handle) static void trbe_handle_overflow(struct perf_output_handle *handle) {

• struct perf_event *event = handle->event; struct trbe_buf *buf = etm_perf_sink_config(handle); unsigned long offset, size;
• struct etm_event_data *event_data;

offset = get_trbe_limit_pointer() - get_trbe_base_pointer(); size = offset - PERF_IDX2OFF(handle->head, buf); @@ -709,30 +707,22 @@ static void trbe_handle_overflow(struct perf_output_handle *handle) /* * Mark the buffer as truncated, as we have stopped the trace * collection upon the WRAP event, without stopping the source.

• *
  

• * We don't re-enable the TRBE here, as the event is
  

• * bound to be disabled due to the TRUNCATED flag.
  

• * This is not ideal, as we could use the available space in
  

• * the ring buffer and continue the tracing.
  

• *
  

• * TODO: Revisit the use of TRUNCATED flag and may be instead use
  

• * PARTIAL, to indicate trace may contain partial packets.
  

• * And TRUNCATED can be used only if we do not have enough space
  

• * in the buffer. This would need additional changes in
  

• * etm_event_stop() to allow the sinks to leave a closed
  

• * aux_handle.
  

  */ perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW | PERF_AUX_FLAG_TRUNCATED); perf_aux_output_end(handle, size);

• event_data = perf_aux_output_begin(handle, event);
• if (!event_data) {

• /*
  

•  * We are unable to restart the trace collection,
  

•  * thus leave the TRBE disabled. The etm-perf driver
  

•  * is able to detect this with a disconnected handle
  

•  * (handle->event = NULL).
  

•  */
  

• trbe_drain_and_disable_local();
  

• *this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL;
  

• return;
  

• }
• buf->trbe_limit = compute_trbe_buffer_limit(handle);
• buf->trbe_write = buf->trbe_base + PERF_IDX2OFF(handle->head, buf);
• if (buf->trbe_limit == buf->trbe_base) {

• trbe_stop_and_truncate_event(handle);
  

• return;
  

• }
• *this_cpu_ptr(buf->cpudata->drvdata->handle) = handle;
• trbe_enable_hw(buf); }

static bool is_perf_trbe(struct perf_output_handle *handle)