This series fixes the following issues with the TRBE and Self-Hosted trace for CoreSight.
The Self-hosted trace filter control registers are now save-restored across CPU PM event. And more importantly the Trace Filtering is now used to control per ETM session (rather than allowing the trace throughout the life time of the system). i.e, ETM configuration of the given run is used to enforce trace filtering (TRFCR) along with the Trace Exclusion controls in TRCVICTLR.
For the TRBE, we were using the TRUNCATED flag in the AUX buffer on every IRQ to indicate that we may have lost a few bytes of trace. But this causes the event to be disabled until the userspace re-enables it back, even when there is space left in the ring buffer. To make things worse, we were restarting the AUX handle, which would soon be disabled, potentially creating 0 sized records (without truncation), which the perf tool tends to ignore. This might cause the event to be disabled permanently. Also, sometimes we leave the buffer TRUNCATED, but delay the closing of the handle to event schedule out, which could cause significant black out in the trace capture. This was reported by Tamas Zsoldos.
This series removes the use of TRUNCATED flag for every IRQ. Instead, uses the COLLISION flag to indicate that there was a WRAP event in the trace collection. A patch to report the COLLISIONs has been queued for the perf tool. The TRUNCATED is only used if we really run out of space in the buffer. And also we make sure the "handle" is closed immediately on TRUNCATED case, which triggers the userspace to take action. The core perf layer has been hardened to handle this case where a "handle" is closed out.
Finally, we make sure that the CPU trace is prohibited, when the TRBE is left disabled. The ETE/ETM driver will program the Trace Filtering appropriately since we do this dynamically now with the first half of the series.
Applies on coresigh/next.
The tree is also available here:
https://gitlab.arm.com/linux-arm/linux-skp/-/tree/coresight/trbe-self-hosted...
Changes since v2 [1]: - Address comments from Anshuman - Perf tool patch queued, to report COLLISIONs in a session.
Changes since v1 [0]: - Moved TRFCR related accessors to a new header file - Following a discussion, dropped the TRUNCATED flag from the TRBE IRQ handler on WRAP. Instead mark COLLISION. - Added new patches to harden the ETM perf layer to handle an error in the sink driver. - Fix TRBE spurious IRQ handling - Cleanup TRBE driver to make the "TRUNCATE" cases managed at a central place.
[0] https://lkml.kernel.org/r/20210712113830.2803257-1-suzuki.poulose@arm.com [1] https://lkml.kernel.org/r/20210723124611.3828908-1-suzuki.poulose@arm.com
Suzuki K Poulose (10): coresight: etm4x: Save restore TRFCR_EL1 coresight: etm4x: Use Trace Filtering controls dynamically coresight: etm-pmu: Ensure the AUX handle is valid coresight: trbe: Ensure the format flag is set always coresight: trbe: Drop duplicate TRUNCATE flags coresight: trbe: Fix handling of spurious interrupts coresight: trbe: Do not truncate buffer on IRQ coresight: trbe: Unify the enabling sequence coresight: trbe: End the AUX handle on truncation coresight: trbe: Prohibit trace before disabling TRBE
.../hwtracing/coresight/coresight-etm-perf.c | 27 ++++- .../coresight/coresight-etm4x-core.c | 100 ++++++++++++---- drivers/hwtracing/coresight/coresight-etm4x.h | 9 +- .../coresight/coresight-self-hosted-trace.h | 34 ++++++ drivers/hwtracing/coresight/coresight-trbe.c | 111 ++++++++++-------- 5 files changed, 200 insertions(+), 81 deletions(-) create mode 100644 drivers/hwtracing/coresight/coresight-self-hosted-trace.h
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.
The TRFCR related helpers are in a new header file, as we need to use them for TRBE in the later patches.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Anshuman Khandual anshuman.khandual@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Reviewed-by: Anshuman Khandual anshuman.khandual@arm.com Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- .../coresight/coresight-etm4x-core.c | 43 +++++++++++++------ drivers/hwtracing/coresight/coresight-etm4x.h | 2 + .../coresight/coresight-self-hosted-trace.h | 25 +++++++++++ 3 files changed, 58 insertions(+), 12 deletions(-) create mode 100644 drivers/hwtracing/coresight/coresight-self-hosted-trace.h
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index e24252eaf8e4..537c0d7ee1ed 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -40,6 +40,7 @@ #include "coresight-etm4x.h" #include "coresight-etm-perf.h" #include "coresight-etm4x-cfg.h" +#include "coresight-self-hosted-trace.h" #include "coresight-syscfg.h"
static int boot_enable; @@ -1011,7 +1012,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) @@ -1554,7 +1555,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; @@ -1693,7 +1694,23 @@ 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; + + /* Save the TRFCR irrespective of whether the ETM is ON */ + 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; @@ -1789,6 +1806,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) { @@ -1800,23 +1825,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; diff --git a/drivers/hwtracing/coresight/coresight-self-hosted-trace.h b/drivers/hwtracing/coresight/coresight-self-hosted-trace.h new file mode 100644 index 000000000000..53b35a28075e --- /dev/null +++ b/drivers/hwtracing/coresight/coresight-self-hosted-trace.h @@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +/* + * Arm v8 Self-Hosted trace support. + * + * Copyright (C) 2021 ARM Ltd. + */ + +#ifndef __CORESIGHT_SELF_HOSTED_TRACE_H +#define __CORESIGHT_SELF_HOSTED_TRACE_H + +#include <asm/sysreg.h> + +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(); +} + +#endif /* __CORESIGHT_SELF_HOSTED_TRACE_H */
Good morning,
On Tue, Sep 14, 2021 at 11:26:32AM +0100, Suzuki K Poulose wrote:
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.
The TRFCR related helpers are in a new header file, as we need to use them for TRBE in the later patches.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Anshuman Khandual anshuman.khandual@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Reviewed-by: Anshuman Khandual anshuman.khandual@arm.com Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
.../coresight/coresight-etm4x-core.c | 43 +++++++++++++------ drivers/hwtracing/coresight/coresight-etm4x.h | 2 + .../coresight/coresight-self-hosted-trace.h | 25 +++++++++++ 3 files changed, 58 insertions(+), 12 deletions(-) create mode 100644 drivers/hwtracing/coresight/coresight-self-hosted-trace.h
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index e24252eaf8e4..537c0d7ee1ed 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -40,6 +40,7 @@ #include "coresight-etm4x.h" #include "coresight-etm-perf.h" #include "coresight-etm4x-cfg.h" +#include "coresight-self-hosted-trace.h" #include "coresight-syscfg.h" static int boot_enable; @@ -1011,7 +1012,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) @@ -1554,7 +1555,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; @@ -1693,7 +1694,23 @@ 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;
- /* Save the TRFCR irrespective of whether the ETM is ON */
- 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; @@ -1789,6 +1806,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) { @@ -1800,23 +1825,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;
if (drvdata->state_needs_restore)etm4_cpu_restore(drvdata);
etm4_cpu_restore(drvdata);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;
diff --git a/drivers/hwtracing/coresight/coresight-self-hosted-trace.h b/drivers/hwtracing/coresight/coresight-self-hosted-trace.h new file mode 100644 index 000000000000..53b35a28075e --- /dev/null +++ b/drivers/hwtracing/coresight/coresight-self-hosted-trace.h @@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: GPL-2.0-only */
Extra line
+/*
- Arm v8 Self-Hosted trace support.
- Copyright (C) 2021 ARM Ltd.
- */
+#ifndef __CORESIGHT_SELF_HOSTED_TRACE_H +#define __CORESIGHT_SELF_HOSTED_TRACE_H
+#include <asm/sysreg.h>
+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();
+}
+#endif /* __CORESIGHT_SELF_HOSTED_TRACE_H */
This came out weird.
I fixed the above and applied the patch.
Thanks, Mathieu
-- 2.24.1
The Trace Filtering support (FEAT_TRF) ensures that the ETM can be prohibited from generating any trace for a given EL. This is much stricter knob, than the TRCVICTLR exception level masks, which doesn't prevent the ETM from generating Context packets for an "excluded" EL. At the moment, we do a onetime enable trace at user and kernel and leave it untouched for the kernel life time. This implies that the ETM could potentially generate trace packets containing the kernel addresses, and thus leaking the kernel virtual address in the trace.
This patch makes the switch dynamic, by honoring the filters set by the user and enforcing them in the TRFCR controls. We also rename the cpu_enable_tracing() appropriately to cpu_detect_trace_filtering() and the drvdata member trfc => trfcr to indicate the "value" of the TRFCR_EL1.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Al Grant al.grant@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- .../coresight/coresight-etm4x-core.c | 63 ++++++++++++++----- drivers/hwtracing/coresight/coresight-etm4x.h | 7 ++- .../coresight/coresight-self-hosted-trace.h | 7 +++ 3 files changed, 59 insertions(+), 18 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 537c0d7ee1ed..d9a85ca25b60 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -239,6 +239,45 @@ struct etm4_enable_arg { int rc; };
+/* + * etm4x_prohibit_trace - Prohibit the CPU from tracing at all ELs. + * When the CPU supports FEAT_TRF, we could move the ETM to a trace + * prohibited state by filtering the Exception levels via TRFCR_EL1. + */ +static void etm4x_prohibit_trace(struct etmv4_drvdata *drvdata) +{ + /* If the CPU doesn't support FEAT_TRF, nothing to do */ + if (!drvdata->trfcr) + return; + cpu_prohibit_trace(); +} + +/* + * etm4x_allow_trace - Allow CPU tracing in the respective ELs, + * as configured by the drvdata->config.mode for the current + * session. Even though we have TRCVICTLR bits to filter the + * trace in the ELs, it doesn't prevent the ETM from generating + * a packet (e.g, TraceInfo) that might contain the addresses from + * the excluded levels. Thus we use the additional controls provided + * via the Trace Filtering controls (FEAT_TRF) to make sure no trace + * is generated for the excluded ELs. + */ +static void etm4x_allow_trace(struct etmv4_drvdata *drvdata) +{ + u64 trfcr = drvdata->trfcr; + + /* If the CPU doesn't support FEAT_TRF, nothing to do */ + if (!trfcr) + return; + + if (drvdata->config.mode & ETM_MODE_EXCL_KERN) + trfcr &= ~TRFCR_ELx_ExTRE; + if (drvdata->config.mode & ETM_MODE_EXCL_USER) + trfcr &= ~TRFCR_ELx_E0TRE; + + write_trfcr(trfcr); +} + #ifdef CONFIG_ETM4X_IMPDEF_FEATURE
#define HISI_HIP08_AMBA_ID 0x000b6d01 @@ -443,6 +482,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) if (etm4x_is_ete(drvdata)) etm4x_relaxed_write32(csa, TRCRSR_TA, TRCRSR);
+ etm4x_allow_trace(drvdata); /* Enable the trace unit */ etm4x_relaxed_write32(csa, 1, TRCPRGCTLR);
@@ -738,7 +778,6 @@ static int etm4_enable(struct coresight_device *csdev, static void etm4_disable_hw(void *info) { u32 control; - u64 trfcr; struct etmv4_drvdata *drvdata = info; struct etmv4_config *config = &drvdata->config; struct coresight_device *csdev = drvdata->csdev; @@ -765,12 +804,7 @@ static void etm4_disable_hw(void *info) * If the CPU supports v8.4 Trace filter Control, * set the ETM to trace prohibited region. */ - if (drvdata->trfc) { - trfcr = read_sysreg_s(SYS_TRFCR_EL1); - write_sysreg_s(trfcr & ~(TRFCR_ELx_ExTRE | TRFCR_ELx_E0TRE), - SYS_TRFCR_EL1); - isb(); - } + etm4x_prohibit_trace(drvdata); /* * Make sure everything completes before disabling, as recommended * by section 7.3.77 ("TRCVICTLR, ViewInst Main Control Register, @@ -786,9 +820,6 @@ static void etm4_disable_hw(void *info) if (coresight_timeout(csa, TRCSTATR, TRCSTATR_PMSTABLE_BIT, 1)) dev_err(etm_dev, "timeout while waiting for PM stable Trace Status\n"); - if (drvdata->trfc) - write_sysreg_s(trfcr, SYS_TRFCR_EL1); - /* read the status of the single shot comparators */ for (i = 0; i < drvdata->nr_ss_cmp; i++) { config->ss_status[i] = @@ -990,15 +1021,15 @@ static bool etm4_init_csdev_access(struct etmv4_drvdata *drvdata, return false; }
-static void cpu_enable_tracing(struct etmv4_drvdata *drvdata) +static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata) { u64 dfr0 = read_sysreg(id_aa64dfr0_el1); u64 trfcr;
+ drvdata->trfcr = 0; if (!cpuid_feature_extract_unsigned_field(dfr0, ID_AA64DFR0_TRACE_FILT_SHIFT)) return;
- drvdata->trfc = true; /* * If the CPU supports v8.4 SelfHosted Tracing, enable * tracing at the kernel EL and EL0, forcing to use the @@ -1012,7 +1043,7 @@ static void cpu_enable_tracing(struct etmv4_drvdata *drvdata) if (is_kernel_in_hyp_mode()) trfcr |= TRFCR_EL2_CX;
- write_trfcr(trfcr); + drvdata->trfcr = trfcr; }
static void etm4_init_arch_data(void *info) @@ -1203,7 +1234,7 @@ static void etm4_init_arch_data(void *info) /* NUMCNTR, bits[30:28] number of counters available for tracing */ drvdata->nr_cntr = BMVAL(etmidr5, 28, 30); etm4_cs_lock(drvdata, csa); - cpu_enable_tracing(drvdata); + cpu_detect_trace_filtering(drvdata); }
static inline u32 etm4_get_victlr_access_type(struct etmv4_config *config) @@ -1699,7 +1730,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) int ret = 0;
/* Save the TRFCR irrespective of whether the ETM is ON */ - if (drvdata->trfc) + if (drvdata->trfcr) drvdata->save_trfcr = read_trfcr(); /* * Save and restore the ETM Trace registers only if @@ -1808,7 +1839,7 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
static void etm4_cpu_restore(struct etmv4_drvdata *drvdata) { - if (drvdata->trfc) + if (drvdata->trfcr) write_trfcr(drvdata->save_trfcr); if (drvdata->state_needs_restore) __etm4_cpu_restore(drvdata); diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 82cba16b73a6..3c4d69b096ca 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -919,7 +919,10 @@ struct etmv4_save_state { * @nooverflow: Indicate if overflow prevention is supported. * @atbtrig: If the implementation can support ATB triggers * @lpoverride: If the implementation can support low-power state over. - * @trfc: If the implementation supports Arm v8.4 trace filter controls. + * @trfcr: If the CPU supports FEAT_TRF, value of the TRFCR_ELx that + * allows tracing at all ELs. We don't want to compute this + * at runtime, due to the additional setting of TRFCR_CX when + * in EL2. Otherwise, 0. * @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 @@ -972,7 +975,7 @@ struct etmv4_drvdata { bool nooverflow; bool atbtrig; bool lpoverride; - bool trfc; + u64 trfcr; struct etmv4_config config; u64 save_trfcr; struct etmv4_save_state *save_state; diff --git a/drivers/hwtracing/coresight/coresight-self-hosted-trace.h b/drivers/hwtracing/coresight/coresight-self-hosted-trace.h index 53b35a28075e..586d26e0cba3 100644 --- a/drivers/hwtracing/coresight/coresight-self-hosted-trace.h +++ b/drivers/hwtracing/coresight/coresight-self-hosted-trace.h @@ -22,4 +22,11 @@ static inline void write_trfcr(u64 val) isb(); }
+static inline void cpu_prohibit_trace(void) +{ + u64 trfcr = read_trfcr(); + + /* Prohibit tracing at EL0 & the kernel EL */ + write_trfcr(trfcr & ~(TRFCR_ELx_ExTRE | TRFCR_ELx_E0TRE)); +} #endif /* __CORESIGHT_SELF_HOSTED_TRACE_H */
The ETM perf infrastructure closes out a handle during event_stop or on an error in starting the event. In either case, it is possible for a "sink" to update/close the handle, under certain circumstances. (e.g no space in ring buffer.). So, ensure that we handle this gracefully in the PMU driver by verifying the handle is still valid.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Anshuman Khandual anshuman.khandual@arm.com Cc: Leo Yan leo.yan@linaro.org Reviewed-by: Anshuman Khandual anshuman.khandual@arm.com Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- .../hwtracing/coresight/coresight-etm-perf.c | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 8ebd728d3a80..9153c8f30568 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -481,8 +481,15 @@ static void etm_event_start(struct perf_event *event, int flags) 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); + /* + * Check if the handle is still associated with the event, + * to handle cases where if the sink failed to start the + * trace and TRUNCATED the handle already. + */ + if (READ_ONCE(handle->event)) { + perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); + perf_aux_output_end(handle, 0); + } fail: event->hw.state = PERF_HES_STOPPED; goto out; @@ -550,7 +557,21 @@ static void etm_event_stop(struct perf_event *event, int mode)
size = sink_ops(sink)->update_buffer(sink, handle, event_data->snk_config); - perf_aux_output_end(handle, size); + /* + * Make sure the handle is still valid as the + * sink could have closed it from an IRQ. + * The sink driver must handle the race with + * update_buffer() and IRQ. Thus either we + * should get a valid handle and valid size + * (which may be 0). + * + * But we should never get a non-zero size with + * an invalid handle. + */ + if (READ_ONCE(handle->event)) + perf_aux_output_end(handle, size); + else + WARN_ON(size); }
/* Disabling the path make its elements available to other sessions */
When the TRBE is stopped on truncating an event, we may not set the FORMAT flag, even though the size of the record is 0. Let us be consistent and not confuse the user.
To ensure that the format flag is always set on all the records generated by TRBE, set the flag when we have a new handle. Rather than deferring to the "end" operation, which makes it clear. So, we can do this from
- arm_trbe_enable() -> When a new handle is provided by the CoreSight PMU, triggered via etm_event_start() - trbe_handle_overflow() -> When we begin a new handle after closing the previous on overflow.
Cc: Anshuman Khandual anshuman.khandual@arm.com Cc: Leo Yan leo.yan@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- Changes since v2: - Move the format flag setting to the beginning of a session than the ending (which can happen via various different paths) --- drivers/hwtracing/coresight/coresight-trbe.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 176868496879..81bf183a73a1 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -554,8 +554,6 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev, if (cpudata->mode != CS_MODE_PERF) return 0;
- perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW); - /* * We are about to disable the TRBE. And this could in turn * fill up the buffer triggering, an IRQ. This could be consumed @@ -648,6 +646,7 @@ static int arm_trbe_enable(struct coresight_device *csdev, u32 mode, void *data) if (mode != CS_MODE_PERF) return -EINVAL;
+ perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW); *this_cpu_ptr(drvdata->handle) = handle; cpudata->buf = buf; cpudata->mode = mode; @@ -710,8 +709,7 @@ 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. */ - perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW | - PERF_AUX_FLAG_TRUNCATED); + perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); perf_aux_output_end(handle, size); event_data = perf_aux_output_begin(handle, event); if (!event_data) { @@ -725,6 +723,7 @@ static void trbe_handle_overflow(struct perf_output_handle *handle) *this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL; return; } + perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW); 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) {
On Tue, Sep 14, 2021 at 11:26:35AM +0100, Suzuki K Poulose wrote:
When the TRBE is stopped on truncating an event, we may not set the FORMAT flag, even though the size of the record is 0. Let us be consistent and not confuse the user.
To ensure that the format flag is always set on all the records generated by TRBE, set the flag when we have a new handle. Rather than deferring to the "end" operation, which makes it clear. So, we can do this from
- arm_trbe_enable() -> When a new handle is provided by the CoreSight PMU, triggered via etm_event_start()
- trbe_handle_overflow() -> When we begin a new handle after closing the previous on overflow.
Cc: Anshuman Khandual anshuman.khandual@arm.com Cc: Leo Yan leo.yan@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
Changes since v2:
- Move the format flag setting to the beginning of a session than the ending (which can happen via various different paths)
drivers/hwtracing/coresight/coresight-trbe.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
I have applied patches 1 to 4.
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 176868496879..81bf183a73a1 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -554,8 +554,6 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev, if (cpudata->mode != CS_MODE_PERF) return 0;
- perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW);
- /*
- We are about to disable the TRBE. And this could in turn
- fill up the buffer triggering, an IRQ. This could be consumed
@@ -648,6 +646,7 @@ static int arm_trbe_enable(struct coresight_device *csdev, u32 mode, void *data) if (mode != CS_MODE_PERF) return -EINVAL;
- perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW); *this_cpu_ptr(drvdata->handle) = handle; cpudata->buf = buf; cpudata->mode = mode;
@@ -710,8 +709,7 @@ 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. */
- perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW |
PERF_AUX_FLAG_TRUNCATED);
- perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); perf_aux_output_end(handle, size); event_data = perf_aux_output_begin(handle, event); if (!event_data) {
@@ -725,6 +723,7 @@ static void trbe_handle_overflow(struct perf_output_handle *handle) *this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL; return; }
- perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW); 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) {
-- 2.24.1
We mark the buffer as TRUNCATED when there is no space left in the buffer. But we do it at different points. __trbe_normal_offset() and also, at all the callers of the above function via compute_trbe_buffer_limit(), when the limit == base (i.e offset = 0 as returned by the __trbe_normal_offset()).
So, given that the callers already mark the buffer as TRUNCATED drop the caller inside the __trbe_normal_offset().
This is in preparation to moving the handling of TRUNCATED into a central place.
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 --- Change since v2: - Added the flag for trbe_handle_spurious() - Anshuman --- drivers/hwtracing/coresight/coresight-trbe.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 81bf183a73a1..5297b11f26b7 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -252,13 +252,9 @@ static unsigned long __trbe_normal_offset(struct perf_output_handle *handle) * trbe_base trbe_base + nr_pages * * Perf aux buffer does not have any space for the driver to write into. - * Just communicate trace truncation event to the user space by marking - * it with PERF_AUX_FLAG_TRUNCATED. */ - if (!handle->size) { - perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); + if (!handle->size) return 0; - }
/* Compute the tail and wakeup indices now that we've aligned head */ tail = PERF_IDX2OFF(handle->head + handle->size, buf); @@ -360,7 +356,6 @@ static unsigned long __trbe_normal_offset(struct perf_output_handle *handle) return limit;
trbe_pad_buf(handle, handle->size); - perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); return 0; }
@@ -688,6 +683,7 @@ static void trbe_handle_spurious(struct perf_output_handle *handle) buf->trbe_write = buf->trbe_base + PERF_IDX2OFF(handle->head, buf); if (buf->trbe_limit == buf->trbe_base) { trbe_drain_and_disable_local(); + perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); return; } trbe_enable_hw(buf);
On a spurious IRQ, right now we disable the TRBE and then re-enable it back, resetting the "buffer" pointers(i.e BASE, LIMIT and more importantly WRITE) to the original pointers from the AUX handle. This implies that we overwrite any trace that was written so far, (by overwriting TRBPTR) while we should have ignored the IRQ.
This patch cleans the behavior, by only stopping the TRBE if the IRQ was indeed raised, as we can read the TRBSR without stopping the TRBE (Only writes to the TRBSR requires the TRBE disabled). And also, on detecting a spurious IRQ after examining the TRBSR, we simply re-enable the TRBE without touching the other parameters.
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 | 30 ++++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 5297b11f26b7..de99dd0aecd3 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -677,16 +677,16 @@ static int arm_trbe_disable(struct coresight_device *csdev)
static void trbe_handle_spurious(struct perf_output_handle *handle) { - struct trbe_buf *buf = etm_perf_sink_config(handle); + u64 limitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
- 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_drain_and_disable_local(); - perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); - return; - } - trbe_enable_hw(buf); + /* + * If the IRQ was spurious, simply re-enable the TRBE + * back without modifying the buffer parameters to + * retain the trace collected so far. + */ + limitr |= TRBLIMITR_ENABLE; + write_sysreg_s(limitr, SYS_TRBLIMITR_EL1); + isb(); }
static void trbe_handle_overflow(struct perf_output_handle *handle) @@ -759,12 +759,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) enum trbe_fault_action act; u64 status;
- /* - * Ensure the trace is visible to the CPUs and - * any external aborts have been resolved. - */ - trbe_drain_and_disable_local(); - + /* Reads to TRBSR_EL1 is fine when TRBE is active */ status = read_sysreg_s(SYS_TRBSR_EL1); /* * If the pending IRQ was handled by update_buffer callback @@ -773,6 +768,11 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) if (!is_trbe_irq(status)) return IRQ_NONE;
+ /* + * Ensure the trace is visible to the CPUs and + * any external aborts have been resolved. + */ + trbe_drain_and_disable_local(); clr_trbe_irq(); isb();
On Tue, Sep 14, 2021 at 11:26:37AM +0100, Suzuki K Poulose wrote:
On a spurious IRQ, right now we disable the TRBE and then re-enable it back, resetting the "buffer" pointers(i.e BASE, LIMIT and more importantly WRITE) to the original pointers from the AUX handle. This implies that we overwrite any trace that was written so far, (by overwriting TRBPTR) while we should have ignored the IRQ.
This patch cleans the behavior, by only stopping the TRBE if the IRQ was indeed raised, as we can read the TRBSR without stopping the TRBE (Only writes to the TRBSR requires the TRBE disabled). And also, on detecting a spurious IRQ after examining the TRBSR, we simply re-enable the TRBE without touching the other parameters.
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 | 30 ++++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 5297b11f26b7..de99dd0aecd3 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -677,16 +677,16 @@ static int arm_trbe_disable(struct coresight_device *csdev) static void trbe_handle_spurious(struct perf_output_handle *handle) {
- struct trbe_buf *buf = etm_perf_sink_config(handle);
- u64 limitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
- 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_drain_and_disable_local();perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);return;- }
- trbe_enable_hw(buf);
- /*
* If the IRQ was spurious, simply re-enable the TRBE* back without modifying the buffer parameters to* retain the trace collected so far.*/- limitr |= TRBLIMITR_ENABLE;
- write_sysreg_s(limitr, SYS_TRBLIMITR_EL1);
- isb();
I understand (and agree with) this part of the patch...
} static void trbe_handle_overflow(struct perf_output_handle *handle) @@ -759,12 +759,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) enum trbe_fault_action act; u64 status;
- /*
* Ensure the trace is visible to the CPUs and* any external aborts have been resolved.*/- trbe_drain_and_disable_local();
- /* Reads to TRBSR_EL1 is fine when TRBE is active */ status = read_sysreg_s(SYS_TRBSR_EL1); /*
- If the pending IRQ was handled by update_buffer callback
@@ -773,6 +768,11 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) if (!is_trbe_irq(status)) return IRQ_NONE;
- /*
* Ensure the trace is visible to the CPUs and* any external aborts have been resolved.*/- trbe_drain_and_disable_local();
But not this part... I can see why you'd want to move this after the check for is_trbe_irq(), but not how it relates to spurious interrupts. To me it seems like it is addressing another issue. If those code snippets are related then a good dose of comments is missing.
Thanks, Mathieu
clr_trbe_irq(); isb(); -- 2.24.1
On 21/09/2021 18:24, Mathieu Poirier wrote:
On Tue, Sep 14, 2021 at 11:26:37AM +0100, Suzuki K Poulose wrote:
On a spurious IRQ, right now we disable the TRBE and then re-enable it back, resetting the "buffer" pointers(i.e BASE, LIMIT and more importantly WRITE) to the original pointers from the AUX handle. This implies that we overwrite any trace that was written so far, (by overwriting TRBPTR) while we should have ignored the IRQ.
This patch cleans the behavior, by only stopping the TRBE if the IRQ was indeed raised, as we can read the TRBSR without stopping the TRBE (Only writes to the TRBSR requires the TRBE disabled). And also, on detecting a spurious IRQ after examining the TRBSR, we simply re-enable the TRBE without touching the other parameters.
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 | 30 ++++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 5297b11f26b7..de99dd0aecd3 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -677,16 +677,16 @@ static int arm_trbe_disable(struct coresight_device *csdev) static void trbe_handle_spurious(struct perf_output_handle *handle) {
- struct trbe_buf *buf = etm_perf_sink_config(handle);
- u64 limitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
- 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_drain_and_disable_local();perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);return;- }
- trbe_enable_hw(buf);
- /*
* If the IRQ was spurious, simply re-enable the TRBE* back without modifying the buffer parameters to* retain the trace collected so far.*/- limitr |= TRBLIMITR_ENABLE;
- write_sysreg_s(limitr, SYS_TRBLIMITR_EL1);
- isb();
I understand (and agree with) this part of the patch...
} static void trbe_handle_overflow(struct perf_output_handle *handle) @@ -759,12 +759,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) enum trbe_fault_action act; u64 status;
- /*
* Ensure the trace is visible to the CPUs and* any external aborts have been resolved.*/- trbe_drain_and_disable_local();
- /* Reads to TRBSR_EL1 is fine when TRBE is active */ status = read_sysreg_s(SYS_TRBSR_EL1); /*
- If the pending IRQ was handled by update_buffer callback
[0] See below
@@ -773,6 +768,11 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) if (!is_trbe_irq(status)) return IRQ_NONE;
- /*
* Ensure the trace is visible to the CPUs and* any external aborts have been resolved.*/- trbe_drain_and_disable_local();
But not this part... I can see why you'd want to move this after the check for is_trbe_irq(), but not how it relates to spurious interrupts. To me it seems like it is addressing another issue. If those code snippets are related then a good dose of comments is missing.
This step is to make sure that we stop the TRBE only when there was really something to process. (i.e, TRBSR indicates an IRQ was raised). Also, there is a comment [0] above, for handling a case where the TRBE event was consumed by the "update_buffer()" due to a race with IRQ handler. Thus we stop the TRBE only when we need to analyse the cause and take an action. I agree there is a bit of disconnect.
I can think of the following options:
- Split the patch to 2. with 1. Don't stop the trbe if there is no IRQ (the bit explained above) 2. Don't reset the TRBE ptrs on spurious IRQ
OR
- Add the above comment to the section.
The commit description has a hint, "This patch cleans the behavior, by only stopping the TRBE if the IRQ was indeed raised", but I agree that the code could be documented too.
Let me know what you think.
Thanks for the review.
Suzuki
On Tue, Sep 21, 2021 at 10:29:20PM +0100, Suzuki K Poulose wrote:
On 21/09/2021 18:24, Mathieu Poirier wrote:
On Tue, Sep 14, 2021 at 11:26:37AM +0100, Suzuki K Poulose wrote:
On a spurious IRQ, right now we disable the TRBE and then re-enable it back, resetting the "buffer" pointers(i.e BASE, LIMIT and more importantly WRITE) to the original pointers from the AUX handle. This implies that we overwrite any trace that was written so far, (by overwriting TRBPTR) while we should have ignored the IRQ.
This patch cleans the behavior, by only stopping the TRBE if the IRQ was indeed raised, as we can read the TRBSR without stopping the TRBE (Only writes to the TRBSR requires the TRBE disabled). And also, on detecting a spurious IRQ after examining the TRBSR, we simply re-enable the TRBE without touching the other parameters.
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 | 30 ++++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 5297b11f26b7..de99dd0aecd3 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -677,16 +677,16 @@ static int arm_trbe_disable(struct coresight_device *csdev) static void trbe_handle_spurious(struct perf_output_handle *handle) {
- struct trbe_buf *buf = etm_perf_sink_config(handle);
- u64 limitr = read_sysreg_s(SYS_TRBLIMITR_EL1);
- 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_drain_and_disable_local();perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);return;- }
- trbe_enable_hw(buf);
- /*
* If the IRQ was spurious, simply re-enable the TRBE* back without modifying the buffer parameters to* retain the trace collected so far.*/- limitr |= TRBLIMITR_ENABLE;
- write_sysreg_s(limitr, SYS_TRBLIMITR_EL1);
- isb();
I understand (and agree with) this part of the patch...
} static void trbe_handle_overflow(struct perf_output_handle *handle) @@ -759,12 +759,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) enum trbe_fault_action act; u64 status;
- /*
* Ensure the trace is visible to the CPUs and* any external aborts have been resolved.*/- trbe_drain_and_disable_local();
- /* Reads to TRBSR_EL1 is fine when TRBE is active */ status = read_sysreg_s(SYS_TRBSR_EL1); /*
- If the pending IRQ was handled by update_buffer callback
[0] See below
@@ -773,6 +768,11 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) if (!is_trbe_irq(status)) return IRQ_NONE;
- /*
* Ensure the trace is visible to the CPUs and* any external aborts have been resolved.*/- trbe_drain_and_disable_local();
But not this part... I can see why you'd want to move this after the check for is_trbe_irq(), but not how it relates to spurious interrupts. To me it seems like it is addressing another issue. If those code snippets are related then a good dose of comments is missing.
This step is to make sure that we stop the TRBE only when there was really something to process. (i.e, TRBSR indicates an IRQ was raised). Also, there is a comment [0] above, for handling a case where the TRBE event was consumed by the "update_buffer()" due to a race with IRQ handler. Thus we stop the TRBE only when we need to analyse the cause and take an action. I agree there is a bit of disconnect.
I can think of the following options:
- Split the patch to 2. with
- Don't stop the trbe if there is no IRQ (the bit explained above)
- Don't reset the TRBE ptrs on spurious IRQ
Please do two patches.
Other than this patch I commented on 07 and picked up 08. Patches 09 and 10 won't apply if 06 and 07 aren't present so please address comments for 06 and 07 and resend all 4 patches (06, 07, 09, 10).
Thanks, Mathieu
OR
- Add the above comment to the section.
The commit description has a hint, "This patch cleans the behavior, by only stopping the TRBE if the IRQ was indeed raised", but I agree that the code could be documented too.
Let me know what you think.
Thanks for the review.
Suzuki
The TRBE driver marks the AUX buffer as TRUNCATED when we get an IRQ on FILL event. This has rather unwanted side-effect of the event being disabled when there may be more space in the ring buffer.
So, instead of TRUNCATE we need a different flag to indicate that the trace may have lost a few bytes (i.e from the point of generating the FILL event until the IRQ is consumed). Anyways, the userspace must use the size from RECORD_AUX headers to restrict the "trace" decoding.
Using PARTIAL flag causes the perf tool to generate the following warning:
Warning: AUX data had gaps in it XX times out of YY!
Are you running a KVM guest in the background?
which is pointlessly scary for a user. The other remaining options are : - COLLISION - Use by SPE to indicate samples collided - Add a new flag - Specifically for CoreSight, doesn't sound so good, if we can re-use something.
Given that we don't already use the "COLLISION" flag, the above behavior can be notified using this flag for CoreSight.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: James Clark james.clark@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Anshuman Khandual anshuman.khandual@arm.com Cc: Leo Yan leo.yan@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- Changes since v2: - The perf tool patch for reporting collisions is queued. --- drivers/hwtracing/coresight/coresight-trbe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index de99dd0aecd3..a1a15fa6c4ae 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -612,7 +612,7 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev, * for correct size. Also, mark the buffer truncated. */ write = get_trbe_limit_pointer(); - perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); + perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION); }
offset = write - base; @@ -705,7 +705,7 @@ 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. */ - perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); + perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION); perf_aux_output_end(handle, size); event_data = perf_aux_output_begin(handle, event); if (!event_data) {
On Tue, Sep 14, 2021 at 11:26:38AM +0100, Suzuki K Poulose wrote:
The TRBE driver marks the AUX buffer as TRUNCATED when we get an IRQ on FILL event. This has rather unwanted side-effect of the event being disabled when there may be more space in the ring buffer.
So, instead of TRUNCATE we need a different flag to indicate that the trace may have lost a few bytes (i.e from the point of generating the FILL event until the IRQ is consumed). Anyways, the userspace must use the size from RECORD_AUX headers to restrict the "trace" decoding.
Using PARTIAL flag causes the perf tool to generate the following warning:
Warning: AUX data had gaps in it XX times out of YY!
Are you running a KVM guest in the background?
which is pointlessly scary for a user. The other remaining options are :
- COLLISION - Use by SPE to indicate samples collided
- Add a new flag - Specifically for CoreSight, doesn't sound so good, if we can re-use something.
Given that we don't already use the "COLLISION" flag, the above behavior can be notified using this flag for CoreSight.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: James Clark james.clark@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Anshuman Khandual anshuman.khandual@arm.com Cc: Leo Yan leo.yan@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
Changes since v2:
- The perf tool patch for reporting collisions is queued.
drivers/hwtracing/coresight/coresight-trbe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index de99dd0aecd3..a1a15fa6c4ae 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -612,7 +612,7 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev, * for correct size. Also, mark the buffer truncated. */ write = get_trbe_limit_pointer();
perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
The rational in the changelog should be added here as a comment.
} offset = write - base; @@ -705,7 +705,7 @@ 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. */
- perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
- perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
Same.
perf_aux_output_end(handle, size); event_data = perf_aux_output_begin(handle, event); if (!event_data) { -- 2.24.1
On Tue, Sep 21, 2021 at 11:41:10AM -0600, Mathieu Poirier wrote:
On Tue, Sep 14, 2021 at 11:26:38AM +0100, Suzuki K Poulose wrote:
The TRBE driver marks the AUX buffer as TRUNCATED when we get an IRQ on FILL event. This has rather unwanted side-effect of the event being disabled when there may be more space in the ring buffer.
So, instead of TRUNCATE we need a different flag to indicate that the trace may have lost a few bytes (i.e from the point of generating the FILL event until the IRQ is consumed). Anyways, the userspace must use the size from RECORD_AUX headers to restrict the "trace" decoding.
Using PARTIAL flag causes the perf tool to generate the following warning:
Warning: AUX data had gaps in it XX times out of YY!
Are you running a KVM guest in the background?
which is pointlessly scary for a user. The other remaining options are :
- COLLISION - Use by SPE to indicate samples collided
- Add a new flag - Specifically for CoreSight, doesn't sound so good, if we can re-use something.
Given that we don't already use the "COLLISION" flag, the above behavior can be notified using this flag for CoreSight.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: James Clark james.clark@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Anshuman Khandual anshuman.khandual@arm.com Cc: Leo Yan leo.yan@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
Changes since v2:
- The perf tool patch for reporting collisions is queued.
drivers/hwtracing/coresight/coresight-trbe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index de99dd0aecd3..a1a15fa6c4ae 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -612,7 +612,7 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev, * for correct size. Also, mark the buffer truncated. */ write = get_trbe_limit_pointer();
perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);The rational in the changelog should be added here as a comment.
} offset = write - base; @@ -705,7 +705,7 @@ 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. */
- perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED);
- perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
Same.
... and I'm out of time for today. I will continue tomorrow.
Thanks, Mathieu
perf_aux_output_end(handle, size); event_data = perf_aux_output_begin(handle, event); if (!event_data) { -- 2.24.1
Unify the sequence of enabling the TRBE. We do this from event_start and also from the TRBE IRQ handler. Lets move this to a common helper. The only minor functional change is returning an error when we fail to enable the TRBE. This should be handled already.
Since we now have unique entry point to trying to enable TRBE, move the format flag setting to the central place.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Reviewed-by: Anshuman Khandual anshuman.khandual@arm.com Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- Changes since v2: - Removed redundant handle book keeping (Anshuman) - Moved the format flag setting to the helper --- drivers/hwtracing/coresight/coresight-trbe.c | 37 ++++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index a1a15fa6c4ae..25c16d0f9e49 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -629,6 +629,21 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev, return size; }
+static int __arm_trbe_enable(struct trbe_buf *buf, + struct perf_output_handle *handle) +{ + perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW); + 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 -ENOSPC; + } + *this_cpu_ptr(buf->cpudata->drvdata->handle) = handle; + trbe_enable_hw(buf); + return 0; +} + static int arm_trbe_enable(struct coresight_device *csdev, u32 mode, void *data) { struct trbe_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); @@ -641,19 +656,11 @@ static int arm_trbe_enable(struct coresight_device *csdev, u32 mode, void *data) if (mode != CS_MODE_PERF) return -EINVAL;
- perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW); - *this_cpu_ptr(drvdata->handle) = handle; cpudata->buf = buf; cpudata->mode = mode; buf->cpudata = cpudata; - 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 0; - } - trbe_enable_hw(buf); - return 0; + + return __arm_trbe_enable(buf, handle); }
static int arm_trbe_disable(struct coresight_device *csdev) @@ -719,15 +726,7 @@ static void trbe_handle_overflow(struct perf_output_handle *handle) *this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL; return; } - perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW); - 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); + __arm_trbe_enable(buf, handle); }
static bool is_perf_trbe(struct perf_output_handle *handle)
On Tue, Sep 14, 2021 at 11:26:39AM +0100, Suzuki K Poulose wrote:
Unify the sequence of enabling the TRBE. We do this from event_start and also from the TRBE IRQ handler. Lets move this to a common helper. The only minor functional change is returning an error when we fail to enable the TRBE. This should be handled already.
Since we now have unique entry point to trying to enable TRBE, move the format flag setting to the central place.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Reviewed-by: Anshuman Khandual anshuman.khandual@arm.com Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
Changes since v2:
- Removed redundant handle book keeping (Anshuman)
- Moved the format flag setting to the helper
drivers/hwtracing/coresight/coresight-trbe.c | 37 ++++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-)
Applied.
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index a1a15fa6c4ae..25c16d0f9e49 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -629,6 +629,21 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev, return size; } +static int __arm_trbe_enable(struct trbe_buf *buf,
struct perf_output_handle *handle)+{
- perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW);
- 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 -ENOSPC;- }
- *this_cpu_ptr(buf->cpudata->drvdata->handle) = handle;
- trbe_enable_hw(buf);
- return 0;
+}
static int arm_trbe_enable(struct coresight_device *csdev, u32 mode, void *data) { struct trbe_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); @@ -641,19 +656,11 @@ static int arm_trbe_enable(struct coresight_device *csdev, u32 mode, void *data) if (mode != CS_MODE_PERF) return -EINVAL;
- perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW);
- *this_cpu_ptr(drvdata->handle) = handle; cpudata->buf = buf; cpudata->mode = mode; buf->cpudata = cpudata;
- 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 0;- }
- trbe_enable_hw(buf);
- return 0;
- return __arm_trbe_enable(buf, handle);
} static int arm_trbe_disable(struct coresight_device *csdev) @@ -719,15 +726,7 @@ static void trbe_handle_overflow(struct perf_output_handle *handle) *this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL; return; }
- perf_aux_output_flag(handle, PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW);
- 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);
- __arm_trbe_enable(buf, handle);
} static bool is_perf_trbe(struct perf_output_handle *handle) -- 2.24.1
When we detect that there isn't enough space left to start a meaningful session, we disable the TRBE, marking the buffer as TRUNCATED. But we delay the notification to the perf layer by perf_aux_output_end() until the event is scheduled out, triggered from the kernel perf layer. This will cause significant black outs in the trace. Now that the CoreSight PMU layer can handle a closed "AUX" handle properly, we can close the handle as soon as we detect the case, allowing the userspace to collect and re-enable the event.
Also, while in the IRQ handler, move the irq_work_run() after we have updated the handle, to make sure the "TRUNCATED" flag causes the event to be disabled as soon as possible.
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 Reviewed-by: Anshuman Khandual anshuman.khandual@arm.com Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 26 ++++++++++++-------- 1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 25c16d0f9e49..d05cb5ffc128 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -133,6 +133,7 @@ static void trbe_stop_and_truncate_event(struct perf_output_handle *handle) */ trbe_drain_and_disable_local(); perf_aux_output_flag(handle, PERF_AUX_FLAG_TRUNCATED); + perf_aux_output_end(handle, 0); *this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL; }
@@ -696,7 +697,7 @@ static void trbe_handle_spurious(struct perf_output_handle *handle) isb(); }
-static void trbe_handle_overflow(struct perf_output_handle *handle) +static int trbe_handle_overflow(struct perf_output_handle *handle) { struct perf_event *event = handle->event; struct trbe_buf *buf = etm_perf_sink_config(handle); @@ -724,9 +725,10 @@ static void trbe_handle_overflow(struct perf_output_handle *handle) */ trbe_drain_and_disable_local(); *this_cpu_ptr(buf->cpudata->drvdata->handle) = NULL; - return; + return -EINVAL; } - __arm_trbe_enable(buf, handle); + + return __arm_trbe_enable(buf, handle); }
static bool is_perf_trbe(struct perf_output_handle *handle) @@ -757,6 +759,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) struct perf_output_handle *handle = *handle_ptr; enum trbe_fault_action act; u64 status; + bool truncated = false;
/* Reads to TRBSR_EL1 is fine when TRBE is active */ status = read_sysreg_s(SYS_TRBSR_EL1); @@ -781,24 +784,27 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) if (!is_perf_trbe(handle)) return IRQ_NONE;
- /* - * Ensure perf callbacks have completed, which may disable - * the trace buffer in response to a TRUNCATION flag. - */ - irq_work_run(); - act = trbe_get_fault_act(status); switch (act) { case TRBE_FAULT_ACT_WRAP: - trbe_handle_overflow(handle); + truncated = !!trbe_handle_overflow(handle); break; case TRBE_FAULT_ACT_SPURIOUS: trbe_handle_spurious(handle); break; case TRBE_FAULT_ACT_FATAL: trbe_stop_and_truncate_event(handle); + truncated = true; break; } + + /* + * If the buffer was truncated, ensure perf callbacks + * have completed, which will disable the event. + */ + if (truncated) + irq_work_run(); + return IRQ_HANDLED; }
When the TRBE generates an IRQ, we stop the TRBE, collect the trace and then reprogram the TRBE with the updated buffer pointers, whenever possible. We might also leave the TRBE disabled, if there is not enough space left in the buffer. However, we do not touch the ETE at all during all of this. This means 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 (even if it is for a short duration).
So, we move the CPU into trace prohibited state always before disabling the TRBE, upon entering the IRQ handler. The state is restored if the TRBE is enabled back. Otherwise the trace remains prohibited.
Since, the ETM/ETE driver now controls the TRFCR_EL1 per session, the tracing can be restored/enabled back when the event is rescheduled in.
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 --- .../hwtracing/coresight/coresight-self-hosted-trace.h | 4 +++- drivers/hwtracing/coresight/coresight-trbe.c | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-self-hosted-trace.h b/drivers/hwtracing/coresight/coresight-self-hosted-trace.h index 586d26e0cba3..34372482a3cd 100644 --- a/drivers/hwtracing/coresight/coresight-self-hosted-trace.h +++ b/drivers/hwtracing/coresight/coresight-self-hosted-trace.h @@ -22,11 +22,13 @@ static inline void write_trfcr(u64 val) isb(); }
-static inline void cpu_prohibit_trace(void) +static inline u64 cpu_prohibit_trace(void) { u64 trfcr = read_trfcr();
/* Prohibit tracing at EL0 & the kernel EL */ write_trfcr(trfcr & ~(TRFCR_ELx_ExTRE | TRFCR_ELx_E0TRE)); + /* Return the original value of the TRFCR */ + return trfcr; } #endif /* __CORESIGHT_SELF_HOSTED_TRACE_H */ diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index d05cb5ffc128..d4c57aed05e5 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -16,6 +16,7 @@ #define pr_fmt(fmt) DRVNAME ": " fmt
#include <asm/barrier.h> +#include "coresight-self-hosted-trace.h" #include "coresight-trbe.h"
#define PERF_IDX2OFF(idx, buf) ((idx) % ((buf)->nr_pages << PAGE_SHIFT)) @@ -760,6 +761,7 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) enum trbe_fault_action act; u64 status; bool truncated = false; + u64 trfcr;
/* Reads to TRBSR_EL1 is fine when TRBE is active */ status = read_sysreg_s(SYS_TRBSR_EL1); @@ -770,6 +772,8 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) if (!is_trbe_irq(status)) return IRQ_NONE;
+ /* Prohibit the CPU from tracing before we disable the TRBE */ + trfcr = cpu_prohibit_trace(); /* * Ensure the trace is visible to the CPUs and * any external aborts have been resolved. @@ -801,9 +805,14 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) /* * If the buffer was truncated, ensure perf callbacks * have completed, which will disable the event. + * + * Otherwise, restore the trace filter controls to + * allow the tracing. */ if (truncated) irq_work_run(); + else + write_trfcr(trfcr);
return IRQ_HANDLED; }
-
Mathieu Poirier -
Suzuki K Poulose