When enabling a tracer via SysFS interface, the device mode may be set by any CPU - not necessarily the target CPU. This can lead to race condition in SMP, and may result in incorrect mode values being read.

Consider the following example, where CPU0 attempts to enable the tracer on CPU1 (the target CPU):

CPU0 CPU1 etm4_enable() ` coresight_take_mode(SYSFS) ` etm4_enable_sysfs() ` smp_call_function_single() ----> etm4_enable_hw_smp_call() / / CPU idle: / etm4_cpu_save() / ` coresight_get_mode() Failed to enable h/w / ^^^ ` coresight_set_mode(DISABLED) <-' Read the intermediate SYSFS mode

In this case, CPU0 initiates the operation by taking the SYSFS mode to avoid conflicts with the Perf mode. It then sends an IPI to CPU1 to configure the tracer registers. If any error occurs during this process, CPU0 rolls back by setting the mode to DISABLED.

However, if CPU1 enters an idle state during this time, it might read the intermediate SYSFS mode. As a result, the CPU PM flow could wrongly save and restore tracer context that is actually disabled.

To resolve the issue, this commit moves the device mode setting logic on the target CPU. This ensures that the device mode is only modified by the target CPU, eliminating race condition between mode writes and reads across CPUs.

An additional change introduces the etm4_disable_hw_smp_call() function for SMP calls, which disables the tracer and explicitly set the mode to DISABLED during SysFS operations.

The flow is updated with this change:

CPU0 CPU1 etm4_enable() ` etm4_enable_sysfs() ` smp_call_function_single() ----> etm4_enable_hw_smp_call() ` coresight_take_mode(SYSFS) Failed, set back to DISABLED ` coresight_set_mode(DISABLED)

CPU idle: etm4_cpu_save() ` coresight_get_mode() ^^^ Read out the DISABLED mode

Fixes: c38a9ec2b2c1 ("coresight: etm4x: moving etm_drvdata::enable to atomic field") Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-etm4x-core.c | 48 +++++++++++++++------- 1 file changed, 33 insertions(+), 15 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 42e5d37403addc6ec81f2e3184522d67d1677c04..ee405c88ea5faa130819f96b00b8307f8764d58a 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -590,10 +590,23 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) static void etm4_enable_hw_smp_call(void *info) { struct etm4_enable_arg *arg = info; + struct coresight_device *csdev;

if (WARN_ON(!arg)) return; + + csdev = arg->drvdata->csdev; + if (!coresight_take_mode(csdev, CS_MODE_SYSFS)) { + /* Someone is already using the tracer */ + arg->rc = -EBUSY; + return; + } + arg->rc = etm4_enable_hw(arg->drvdata); + + /* The tracer didn't start */ + if (arg->rc) + coresight_set_mode(csdev, CS_MODE_DISABLED); }

/* @@ -809,6 +822,9 @@ static int etm4_enable_perf(struct coresight_device *csdev, int ret = 0; struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);

+ if (!coresight_take_mode(csdev, CS_MODE_PERF)) + return -EBUSY; + if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) { ret = -EINVAL; goto out; @@ -828,6 +844,9 @@ static int etm4_enable_perf(struct coresight_device *csdev, ret = etm4_enable_hw(drvdata);

out: + /* The tracer didn't start */ + if (ret) + coresight_set_mode(csdev, CS_MODE_DISABLED); return ret; }

@@ -880,11 +899,6 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event, { int ret;

- if (!coresight_take_mode(csdev, mode)) { - /* Someone is already using the tracer */ - return -EBUSY; - } - switch (mode) { case CS_MODE_SYSFS: ret = etm4_enable_sysfs(csdev, path); @@ -896,10 +910,6 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event, ret = -EINVAL; }

- /* The tracer didn't start */ - if (ret) - coresight_set_mode(csdev, CS_MODE_DISABLED); - return ret; }

@@ -951,10 +961,9 @@ static void etm4_disable_trace_unit(struct etmv4_drvdata *drvdata) isb(); }

-static void etm4_disable_hw(void *info) +static void etm4_disable_hw(struct etmv4_drvdata *drvdata) { u32 control; - struct etmv4_drvdata *drvdata = info; struct etmv4_config *config = &drvdata->config; struct coresight_device *csdev = drvdata->csdev; struct csdev_access *csa = &csdev->access; @@ -991,6 +1000,15 @@ static void etm4_disable_hw(void *info) "cpu: %d disable smp call done\n", drvdata->cpu); }

+static void etm4_disable_hw_smp_call(void *info) +{ + struct etmv4_drvdata *drvdata = info; + + etm4_disable_hw(drvdata); + + coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED); +} + static int etm4_disable_perf(struct coresight_device *csdev, struct perf_event *event) { @@ -1020,6 +1038,8 @@ static int etm4_disable_perf(struct coresight_device *csdev, /* TRCVICTLR::SSSTATUS, bit[9] */ filters->ssstatus = (control & BIT(9));

+ coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED); + /* * perf will release trace ids when _free_aux() is * called at the end of the session. @@ -1045,7 +1065,8 @@ static void etm4_disable_sysfs(struct coresight_device *csdev) * Executing etm4_disable_hw on the cpu whose ETM is being disabled * ensures that register writes occur when cpu is powered. */ - smp_call_function_single(drvdata->cpu, etm4_disable_hw, drvdata, 1); + smp_call_function_single(drvdata->cpu, etm4_disable_hw_smp_call, + drvdata, 1);

raw_spin_unlock(&drvdata->spinlock);

@@ -1085,9 +1106,6 @@ static void etm4_disable(struct coresight_device *csdev, etm4_disable_perf(csdev, event); break; } - - if (mode) - coresight_set_mode(csdev, CS_MODE_DISABLED); }

static int etm4_resume_perf(struct coresight_device *csdev)

Since commit 4ff6039ffb79 ("coresight-etm4x: add isb() before reading the TRCSTATR"), the code has incorrectly been polling the PMSTABLE bit instead of the IDLE bit.

This commit corrects the typo.

Fixes: 4ff6039ffb79 ("coresight-etm4x: add isb() before reading the TRCSTATR") Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index ee405c88ea5faa130819f96b00b8307f8764d58a..0f2a8b8459c93ca29d270b6fa05928244e0761c5 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1921,7 +1921,7 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) state->trcpdcr = etm4x_read32(csa, TRCPDCR);

/* wait for TRCSTATR.IDLE to go up */ - if (etm4x_wait_status(csa, TRCSTATR_PMSTABLE_BIT, 1)) { + if (etm4x_wait_status(csa, TRCSTATR_IDLE_BIT, 1)) { dev_err(etm_dev, "timeout while waiting for Idle Trace Status\n"); etm4_os_unlock(drvdata);

According to the software usage PKLXF in Arm ARM (ARM DDI 0487 L.a), a Context synchronization event is required before enabling the trace unit. An ISB is added to meet this requirement.

Improved the barrier comments to provide more accurate information by specifying which section of the document the requirement comes from and clarifying its intended purpose.

Fixes: 1ab3bb9df5e3 ("coresight: etm4x: Add necessary synchronization for sysreg access") Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-etm4x-core.c | 37 +++++++++++++++++++--- 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index af9d3b2319c5f49ccd40dfa0ccf0f694ce9e2f4f..e3a73718d0c903ee9c72b97028b56565f5ee1053 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -445,13 +445,37 @@ static int etm4_enable_trace_unit(struct etmv4_drvdata *drvdata) etm4x_relaxed_write32(csa, TRCRSR_TA, TRCRSR);

etm4x_allow_trace(drvdata); + + /* + * According to software usage PKLXF in Arm ARM (ARM DDI 0487 L.a), + * execute a Context synchronization event to guarantee the trace unit + * will observe the new values of the System registers. + */ + if (!csa->io_mem) + isb(); + /* Enable the trace unit */ etm4x_relaxed_write32(csa, 1, TRCPRGCTLR);

- /* Synchronize the register updates for sysreg access */ + /* + * As recommended by section 4.3.7 ("Synchronization when using system + * instructions to progrom the trace unit") of ARM IHI 0064H.b, the + * self-hosted trace analyzer must perform a Context synchronization + * event between writing to the TRCPRGCTLR and reading the TRCSTATR. + */ if (!csa->io_mem) isb();

+ /* + * For the memory-mapped interface, the registers are mapped as Device + * type (Device-nGnRE). As described in section 4.3.7 ("Synchronization + * of register updates") of ARM IHI0064H.b, read back the value of any + * register in the trace unit to ensure writes have completed. + * + * Therefore, polling on TRCSTATR ensures that the writing TRCPRGCTLR + * is complete. + */ + /* wait for TRCSTATR.IDLE to go back down to '0' */ if (etm4x_wait_status(csa, TRCSTATR_IDLE_BIT, 0)) { dev_err(etm_dev, @@ -931,11 +955,16 @@ static void etm4_disable_trace_unit(struct etmv4_drvdata *drvdata) */ etm4x_prohibit_trace(drvdata); /* - * Make sure everything completes before disabling, as recommended - * by section 7.3.77 ("TRCVICTLR, ViewInst Main Control Register, - * SSTATUS") of ARM IHI 0064D + * Prevent being speculative at the point of disabling the trace unit, + * as recommended by section 7.3.77 ("TRCVICTLR, ViewInst Main Control + * Register, SSTATUS") of ARM IHI 0064D */ dsb(sy); + /* + * According to software usage VKHHY in Arm ARM (ARM DDI 0487 L.a), + * execute a Context synchronization event to guarantee no new + * program-flow trace is generated. + */ isb(); /* Trace synchronization barrier, is a nop if not supported */ tsb_csync();

When the restore flow is invoked, it means no error occurred during the save phase. Otherwise, if any errors happened while saving the context, the function would return an error and abort the suspend sequence.

Therefore, the state_needs_restore flag is unnecessary. The save and restore functions are changed to check two conditions:

1) The global flag pm_save_enable is SELF_HOSTED mode; 2) The device is in active mode (non DISABLED).

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-etm4x-core.c | 14 ++++++++------ drivers/hwtracing/coresight/coresight-etm4x.h | 2 -- 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 6cd76a2527cf11752963a7cb1b3b0e9a8be241f0..1faf8614366f38eea4909152558fe8e69730bde4 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1981,8 +1981,6 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) goto out; }

- drvdata->state_needs_restore = true; - /* * Power can be removed from the trace unit now. We do this to * potentially save power on systems that respect the TRCPDCR_PU @@ -2000,11 +1998,14 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) { int ret = 0;

+ if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED) + return 0; + /* * Save and restore the ETM Trace registers only if * the ETM is active. */ - if (coresight_get_mode(drvdata->csdev) && drvdata->save_state) + if (coresight_get_mode(drvdata->csdev)) ret = __etm4_cpu_save(drvdata); return ret; } @@ -2093,8 +2094,6 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata) if (!drvdata->skip_power_up) etm4x_relaxed_write32(csa, state->trcpdcr, TRCPDCR);

- drvdata->state_needs_restore = false; - /* * As recommended by section 4.3.7 ("Synchronization when using the * memory-mapped interface") of ARM IHI 0064D @@ -2120,7 +2119,10 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)

static void etm4_cpu_restore(struct etmv4_drvdata *drvdata) { - if (drvdata->state_needs_restore) + if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED) + return; + + if (coresight_get_mode(drvdata->csdev)) __etm4_cpu_restore(drvdata); }

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 4d32605a84ce91eae2101cd83cad5f5317e2638a..06438ed1071478cd5c1a143c6ef58a926d6992fb 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -978,7 +978,6 @@ struct etmv4_save_state { * in EL2. Otherwise, 0. * @config: structure holding configuration parameters. * @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 * the trace unit. * @paused: Indicates if the trace unit is paused. @@ -1032,7 +1031,6 @@ struct etmv4_drvdata { u64 trfcr; struct etmv4_config config; struct etmv4_save_state *save_state; - bool state_needs_restore; bool skip_power_up; bool paused; DECLARE_BITMAP(arch_features, ETM4_IMPDEF_FEATURE_MAX);

The etm4_enable_trace_unit() function is almost identical to the restore flow, with a few differences listed below:

1) TRCQCTLR register

The TRCQCTLR register is saved and restored during CPU idle, but it is never touched in the normal flow. Given the Q element is not enabled (TRCCONFIGR.QE bits), it is acceptable to omit saving and restoring this register during idle.

2) TRCSSCSRn.STATUS bit

The restore flow does not explicitly clear the TRCSSCSRn.STATUS bit but instead directly loads the saved value. In contrast, the normal flow clears this bit to 0 when re-enabling single-shot control.

Therefore, the restore function passes the restart_ss argument as false to etm4_enable_hw() to avoid re-enabling single-shot mode.

3) Claim Tag Handling

The claim tag is acquired and released in normal flow, on the other hand, the claim protocol is not applied in CPU idle flow - it simply restores the saved value.

The claim bits serve two purpose:

* Exclusive access between the kernel driver and an external debugger. During CPU idle, the kernel driver locks the OS Lock, ensuring that the external debugger cannot access the trace unit. Therefore, it is safe to release the claim tag during idle.

* Notification to supervisory software to save/restore context for external debuggers. The kernel driver does not touch the external debugger's claim bit (ETMCLAIM[0]).

Based on this, it is safe to acquire and release claim tag in the idle sequence.

4) OS Lock Behavior

The OS Lock should be locked during CPU idle states. This differs from the normal flow, which unlock it. This special handling remains in the CPU save path.

This commit reuses the normal enable and disable logic in the CPU idle path. The only addition is locking the OS Lock upon entering idle to ensure exclusive access.

The save context in the driver data is no longer needed and has been removed.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-etm4x-core.c | 212 +-------------------- drivers/hwtracing/coresight/coresight-etm4x.h | 57 ------ 2 files changed, 5 insertions(+), 264 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 7bbdee92859cd227fd4077df6ff25470b9d76950..6ef1f2ea1a805b07bcac70b6034bab85cec604be 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1851,8 +1851,7 @@ static int etm4_dying_cpu(unsigned int cpu)

static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) { - int i, ret = 0; - struct etmv4_save_state *state; + int ret = 0; struct coresight_device *csdev = drvdata->csdev; struct csdev_access *csa; struct device *etm_dev; @@ -1882,98 +1881,11 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) ret = -EBUSY; goto out; } + etm4_cs_lock(drvdata, csa);

- if (!drvdata->paused) - etm4_disable_trace_unit(drvdata); - - /* - * As recommended by section 4.3.7 (Synchronization of register updates) - * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an - * ISB instruction after programming the trace unit registers. - */ - isb(); - - state = drvdata->save_state; - - if (drvdata->nr_pe) - state->trcprocselr = etm4x_read32(csa, TRCPROCSELR); - state->trcconfigr = etm4x_read32(csa, TRCCONFIGR); - state->trcauxctlr = etm4x_read32(csa, TRCAUXCTLR); - state->trceventctl0r = etm4x_read32(csa, TRCEVENTCTL0R); - state->trceventctl1r = etm4x_read32(csa, TRCEVENTCTL1R); - if (drvdata->stallctl) - state->trcstallctlr = etm4x_read32(csa, TRCSTALLCTLR); - state->trctsctlr = etm4x_read32(csa, TRCTSCTLR); - state->trcsyncpr = etm4x_read32(csa, TRCSYNCPR); - state->trcccctlr = etm4x_read32(csa, TRCCCCTLR); - state->trcbbctlr = etm4x_read32(csa, TRCBBCTLR); - state->trctraceidr = etm4x_read32(csa, TRCTRACEIDR); - if (drvdata->q_filt) - state->trcqctlr = etm4x_read32(csa, TRCQCTLR); - - state->trcvictlr = etm4x_read32(csa, TRCVICTLR); - state->trcviiectlr = etm4x_read32(csa, TRCVIIECTLR); - state->trcvissctlr = etm4x_read32(csa, TRCVISSCTLR); - if (drvdata->nr_pe_cmp) - state->trcvipcssctlr = etm4x_read32(csa, TRCVIPCSSCTLR); - - for (i = 0; i < drvdata->nrseqstate - 1; i++) - state->trcseqevr[i] = etm4x_read32(csa, TRCSEQEVRn(i)); - - if (drvdata->nrseqstate) { - state->trcseqrstevr = etm4x_read32(csa, TRCSEQRSTEVR); - state->trcseqstr = etm4x_read32(csa, TRCSEQSTR); - } - state->trcextinselr = etm4x_read32(csa, TRCEXTINSELR); - - for (i = 0; i < drvdata->nr_cntr; i++) { - state->trccntrldvr[i] = etm4x_read32(csa, TRCCNTRLDVRn(i)); - state->trccntctlr[i] = etm4x_read32(csa, TRCCNTCTLRn(i)); - state->trccntvr[i] = etm4x_read32(csa, TRCCNTVRn(i)); - } - - /* Resource selector pair 0 is reserved */ - for (i = 2; i < drvdata->nr_resource * 2; i++) - state->trcrsctlr[i] = etm4x_read32(csa, TRCRSCTLRn(i)); - - for (i = 0; i < drvdata->nr_ss_cmp; i++) { - state->trcssccr[i] = etm4x_read32(csa, TRCSSCCRn(i)); - state->trcsscsr[i] = etm4x_read32(csa, TRCSSCSRn(i)); - if (etm4x_sspcicrn_present(drvdata, i)) - state->trcsspcicr[i] = etm4x_read32(csa, TRCSSPCICRn(i)); - } - - for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) { - state->trcacvr[i] = etm4x_read64(csa, TRCACVRn(i)); - state->trcacatr[i] = etm4x_read64(csa, TRCACATRn(i)); - } - - /* - * Data trace stream is architecturally prohibited for A profile cores - * so we don't save (or later restore) trcdvcvr and trcdvcmr - As per - * section 1.3.4 ("Possible functional configurations of an ETMv4 trace - * unit") of ARM IHI 0064D. - */ - - for (i = 0; i < drvdata->numcidc; i++) - state->trccidcvr[i] = etm4x_read64(csa, TRCCIDCVRn(i)); - - for (i = 0; i < drvdata->numvmidc; i++) - state->trcvmidcvr[i] = etm4x_read64(csa, TRCVMIDCVRn(i)); - - state->trccidcctlr0 = etm4x_read32(csa, TRCCIDCCTLR0); - if (drvdata->numcidc > 4) - state->trccidcctlr1 = etm4x_read32(csa, TRCCIDCCTLR1); - - state->trcvmidcctlr0 = etm4x_read32(csa, TRCVMIDCCTLR0); - if (drvdata->numvmidc > 4) - state->trcvmidcctlr0 = etm4x_read32(csa, TRCVMIDCCTLR1); - - state->trcclaimset = etm4x_read32(csa, TRCCLAIMCLR); - - if (!drvdata->skip_power_up) - state->trcpdcr = etm4x_read32(csa, TRCPDCR); + etm4_disable_hw(drvdata);

+ etm4_cs_unlock(drvdata, csa); /* wait for TRCSTATR.IDLE to go up */ if (etm4x_wait_status(csa, TRCSTATR_IDLE_BIT, 1)) { dev_err(etm_dev, @@ -1983,14 +1895,6 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) goto out; }

- /* - * Power can be removed from the trace unit now. We do this to - * potentially save power on systems that respect the TRCPDCR_PU - * despite requesting software to save/restore state. - */ - if (!drvdata->skip_power_up) - etm4x_relaxed_write32(csa, (state->trcpdcr & ~TRCPDCR_PU), - TRCPDCR); out: etm4_cs_lock(drvdata, csa); return ret; @@ -2014,109 +1918,10 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata)

static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata) { - int i; - struct etmv4_save_state *state = drvdata->save_state; - struct csdev_access *csa = &drvdata->csdev->access; - if (WARN_ON(!drvdata->csdev)) return;

- etm4_cs_unlock(drvdata, csa); - etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET); - - if (drvdata->nr_pe) - etm4x_relaxed_write32(csa, state->trcprocselr, TRCPROCSELR); - etm4x_relaxed_write32(csa, state->trcconfigr, TRCCONFIGR); - etm4x_relaxed_write32(csa, state->trcauxctlr, TRCAUXCTLR); - etm4x_relaxed_write32(csa, state->trceventctl0r, TRCEVENTCTL0R); - etm4x_relaxed_write32(csa, state->trceventctl1r, TRCEVENTCTL1R); - if (drvdata->stallctl) - etm4x_relaxed_write32(csa, state->trcstallctlr, TRCSTALLCTLR); - etm4x_relaxed_write32(csa, state->trctsctlr, TRCTSCTLR); - etm4x_relaxed_write32(csa, state->trcsyncpr, TRCSYNCPR); - etm4x_relaxed_write32(csa, state->trcccctlr, TRCCCCTLR); - etm4x_relaxed_write32(csa, state->trcbbctlr, TRCBBCTLR); - etm4x_relaxed_write32(csa, state->trctraceidr, TRCTRACEIDR); - if (drvdata->q_filt) - etm4x_relaxed_write32(csa, state->trcqctlr, TRCQCTLR); - - etm4x_relaxed_write32(csa, state->trcvictlr, TRCVICTLR); - etm4x_relaxed_write32(csa, state->trcviiectlr, TRCVIIECTLR); - etm4x_relaxed_write32(csa, state->trcvissctlr, TRCVISSCTLR); - if (drvdata->nr_pe_cmp) - etm4x_relaxed_write32(csa, state->trcvipcssctlr, TRCVIPCSSCTLR); - - for (i = 0; i < drvdata->nrseqstate - 1; i++) - etm4x_relaxed_write32(csa, state->trcseqevr[i], TRCSEQEVRn(i)); - - if (drvdata->nrseqstate) { - etm4x_relaxed_write32(csa, state->trcseqrstevr, TRCSEQRSTEVR); - etm4x_relaxed_write32(csa, state->trcseqstr, TRCSEQSTR); - } - etm4x_relaxed_write32(csa, state->trcextinselr, TRCEXTINSELR); - - for (i = 0; i < drvdata->nr_cntr; i++) { - etm4x_relaxed_write32(csa, state->trccntrldvr[i], TRCCNTRLDVRn(i)); - etm4x_relaxed_write32(csa, state->trccntctlr[i], TRCCNTCTLRn(i)); - etm4x_relaxed_write32(csa, state->trccntvr[i], TRCCNTVRn(i)); - } - - /* Resource selector pair 0 is reserved */ - for (i = 2; i < drvdata->nr_resource * 2; i++) - etm4x_relaxed_write32(csa, state->trcrsctlr[i], TRCRSCTLRn(i)); - - for (i = 0; i < drvdata->nr_ss_cmp; i++) { - etm4x_relaxed_write32(csa, state->trcssccr[i], TRCSSCCRn(i)); - etm4x_relaxed_write32(csa, state->trcsscsr[i], TRCSSCSRn(i)); - if (etm4x_sspcicrn_present(drvdata, i)) - etm4x_relaxed_write32(csa, state->trcsspcicr[i], TRCSSPCICRn(i)); - } - - for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) { - etm4x_relaxed_write64(csa, state->trcacvr[i], TRCACVRn(i)); - etm4x_relaxed_write64(csa, state->trcacatr[i], TRCACATRn(i)); - } - - for (i = 0; i < drvdata->numcidc; i++) - etm4x_relaxed_write64(csa, state->trccidcvr[i], TRCCIDCVRn(i)); - - for (i = 0; i < drvdata->numvmidc; i++) - etm4x_relaxed_write64(csa, state->trcvmidcvr[i], TRCVMIDCVRn(i)); - - etm4x_relaxed_write32(csa, state->trccidcctlr0, TRCCIDCCTLR0); - if (drvdata->numcidc > 4) - etm4x_relaxed_write32(csa, state->trccidcctlr1, TRCCIDCCTLR1); - - etm4x_relaxed_write32(csa, state->trcvmidcctlr0, TRCVMIDCCTLR0); - if (drvdata->numvmidc > 4) - etm4x_relaxed_write32(csa, state->trcvmidcctlr0, TRCVMIDCCTLR1); - - etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET); - - if (!drvdata->skip_power_up) - etm4x_relaxed_write32(csa, state->trcpdcr, TRCPDCR); - - /* - * As recommended by section 4.3.7 ("Synchronization when using the - * memory-mapped interface") of ARM IHI 0064D - */ - dsb(sy); - isb(); - - /* Unlock the OS lock to re-enable trace and external debug access */ - etm4_os_unlock(drvdata); - - if (!drvdata->paused) - etm4_enable_trace_unit(drvdata); - - /* - * As recommended by section 4.3.7 (Synchronization of register updates) - * of ARM IHI 0064H.b, the self-hosted trace analyzer always executes an - * ISB instruction after programming the trace unit registers. - */ - isb(); - - etm4_cs_lock(drvdata, csa); + etm4_enable_hw(drvdata, false); }

static void etm4_cpu_restore(struct etmv4_drvdata *drvdata) @@ -2298,13 +2103,6 @@ static int etm4_probe(struct device *dev) pm_save_enable = coresight_loses_context_with_cpu(dev) ? PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER;

- if (pm_save_enable != PARAM_PM_SAVE_NEVER) { - drvdata->save_state = devm_kmalloc(dev, - sizeof(struct etmv4_save_state), GFP_KERNEL); - if (!drvdata->save_state) - return -ENOMEM; - } - raw_spin_lock_init(&drvdata->spinlock);

drvdata->cpu = coresight_get_cpu(dev); diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 06438ed1071478cd5c1a143c6ef58a926d6992fb..f361ae62b122d84b5126fca6f8ba93d99dc2f992 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -861,61 +861,6 @@ struct etmv4_config { u8 s_ex_level; };

-/** - * struct etm4_save_state - state to be preserved when ETM is without power - */ -struct etmv4_save_state { - u32 trcprocselr; - u32 trcconfigr; - u32 trcauxctlr; - u32 trceventctl0r; - u32 trceventctl1r; - u32 trcstallctlr; - u32 trctsctlr; - u32 trcsyncpr; - u32 trcccctlr; - u32 trcbbctlr; - u32 trctraceidr; - u32 trcqctlr; - - u32 trcvictlr; - u32 trcviiectlr; - u32 trcvissctlr; - u32 trcvipcssctlr; - - u32 trcseqevr[ETM_MAX_SEQ_STATES]; - u32 trcseqrstevr; - u32 trcseqstr; - u32 trcextinselr; - u32 trccntrldvr[ETMv4_MAX_CNTR]; - u32 trccntctlr[ETMv4_MAX_CNTR]; - u32 trccntvr[ETMv4_MAX_CNTR]; - - u32 trcrsctlr[ETM_MAX_RES_SEL]; - - u32 trcssccr[ETM_MAX_SS_CMP]; - u32 trcsscsr[ETM_MAX_SS_CMP]; - u32 trcsspcicr[ETM_MAX_SS_CMP]; - - u64 trcacvr[ETM_MAX_SINGLE_ADDR_CMP]; - u64 trcacatr[ETM_MAX_SINGLE_ADDR_CMP]; - u64 trccidcvr[ETMv4_MAX_CTXID_CMP]; - u64 trcvmidcvr[ETM_MAX_VMID_CMP]; - u32 trccidcctlr0; - u32 trccidcctlr1; - u32 trcvmidcctlr0; - u32 trcvmidcctlr1; - - u32 trcclaimset; - - u32 cntr_val[ETMv4_MAX_CNTR]; - u32 seq_state; - u32 vinst_ctrl; - u32 ss_status[ETM_MAX_SS_CMP]; - - u32 trcpdcr; -}; - /** * struct etm4_drvdata - specifics associated to an ETM component * @pclk APB clock if present, otherwise NULL @@ -977,7 +922,6 @@ struct etmv4_save_state { * at runtime, due to the additional setting of TRFCR_CX when * in EL2. Otherwise, 0. * @config: structure holding configuration parameters. - * @save_state: State to be preserved across power loss * @skip_power_up: Indicates if an implementation can skip powering up * the trace unit. * @paused: Indicates if the trace unit is paused. @@ -1030,7 +974,6 @@ struct etmv4_drvdata { bool lpoverride; u64 trfcr; struct etmv4_config config; - struct etmv4_save_state *save_state; bool skip_power_up; bool paused; DECLARE_BITMAP(arch_features, ETM4_IMPDEF_FEATURE_MAX);

The current SysFS flow first enables the links and sink, then rolls back to disable them if the source fails to enable. This failure can occur if the associated CPU is offline, which causes the SMP call to fail.

Validate whether the associated CPU is online for a per-CPU tracer. If the CPU is offline, return -ENODEV and bail out.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-sysfs.c | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c index feadaf065b5318c8426aa53420b8e5f67a258683..14ee15297b98115122068cbe932f0b2ce004b77e 100644 --- a/drivers/hwtracing/coresight/coresight-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-sysfs.c @@ -161,6 +161,9 @@ static int coresight_validate_source_sysfs(struct coresight_device *csdev, return -EINVAL; }

+ if (coresight_is_percpu_source(csdev) && !cpu_online(csdev->cpu)) + return -ENODEV; + return 0; }

The current implementation only saves and restores the context for ETM sources while ignoring the context of links. However, if funnels or replicators on a linked path resides in a CPU or cluster power domain, the hardware context for the link will be lost after resuming from low power states.

To support context management for links during CPU low power modes, a better way is to implement CPU PM callbacks in the Arm CoreSight core layer. As the core layer has sufficient information for linked paths, from tracers to links, which can be used for power management.

As a first step, this patch registers CPU PM notifier in the core layer. If a source driver provides callbacks for saving and restoring context, these callbacks will be invoked in CPU suspend and resume.

Further changes will extend for controlling path.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 62 ++++++++++++++++++++++++++++ include/linux/coresight.h | 4 ++ 2 files changed, 66 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 042e8e646ff521a15dc18d7264faa427f1ac0dc7..233e128bc61c9e6ef69182e5387fe0dadf532324 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -4,6 +4,7 @@ */

#include <linux/build_bug.h> +#include <linux/cpu_pm.h> #include <linux/kernel.h> #include <linux/init.h> #include <linux/types.h> @@ -420,6 +421,21 @@ int coresight_resume_source(struct coresight_device *csdev) } EXPORT_SYMBOL_GPL(coresight_resume_source);

+static int coresight_save_source(struct coresight_device *csdev) +{ + if (csdev && source_ops(csdev)->save) + return source_ops(csdev)->save(csdev); + + /* Return success if callback is not supported */ + return 0; +} + +static void coresight_restore_source(struct coresight_device *csdev) +{ + if (csdev && source_ops(csdev)->restore) + source_ops(csdev)->restore(csdev); +} + /* * coresight_disable_path_from : Disable components in the given path beyond * @nd in the list. If @nd is NULL, all the components, except the SOURCE are @@ -1572,6 +1588,45 @@ char *coresight_alloc_device_name(struct coresight_dev_list *dict, } EXPORT_SYMBOL_GPL(coresight_alloc_device_name);

+static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, + void *v) +{ + unsigned int cpu = smp_processor_id(); + struct coresight_device *source = per_cpu(csdev_source, cpu); + + if (!source) + return NOTIFY_OK; + + switch (cmd) { + case CPU_PM_ENTER: + if (coresight_save_source(source)) + return NOTIFY_BAD; + break; + case CPU_PM_EXIT: + case CPU_PM_ENTER_FAILED: + coresight_restore_source(source); + break; + default: + return NOTIFY_DONE; + } + + return NOTIFY_OK; +} + +static struct notifier_block coresight_cpu_pm_nb = { + .notifier_call = coresight_cpu_pm_notify, +}; + +static int __init coresight_pm_setup(void) +{ + return cpu_pm_register_notifier(&coresight_cpu_pm_nb); +} + +static void coresight_pm_cleanup(void) +{ + cpu_pm_unregister_notifier(&coresight_cpu_pm_nb); +} + const struct bus_type coresight_bustype = { .name = "coresight", }; @@ -1626,9 +1681,15 @@ static int __init coresight_init(void)

/* initialise the coresight syscfg API */ ret = cscfg_init(); + if (ret) + goto exit_notifier; + + ret = coresight_pm_setup(); if (!ret) return 0;

+ cscfg_exit(); +exit_notifier: atomic_notifier_chain_unregister(&panic_notifier_list, &coresight_notifier); exit_perf: @@ -1640,6 +1701,7 @@ static int __init coresight_init(void)

static void __exit coresight_exit(void) { + coresight_pm_cleanup(); cscfg_exit(); atomic_notifier_chain_unregister(&panic_notifier_list, &coresight_notifier); diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 4b15c67c200624fd46a258628dbff401bd1396d7..8d6f7cb354d6e487c757d9fe86cda895ccb1a588 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -400,6 +400,8 @@ struct coresight_ops_link { * @disable: disables tracing for a source. * @resume_perf: resumes tracing for a source in perf session. * @pause_perf: pauses tracing for a source in perf session. + * @save: save context for a source. + * @restore: restore context for a source. */ struct coresight_ops_source { int (*cpu_id)(struct coresight_device *csdev); @@ -409,6 +411,8 @@ struct coresight_ops_source { struct perf_event *event); int (*resume_perf)(struct coresight_device *csdev); void (*pause_perf)(struct coresight_device *csdev); + int (*save)(struct coresight_device *csdev); + void (*restore)(struct coresight_device *csdev); };

/**

Add a callback in the source device that returns a boolean indicating whether context save and restore operations are required. The save and restore flow is skipped if the callback returns false.

The ETMv4 driver implements its own version's callback.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 16 +++++++++++- drivers/hwtracing/coresight/coresight-etm4x-core.c | 30 +++++++++++++++++----- include/linux/coresight.h | 2 ++ 3 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 233e128bc61c9e6ef69182e5387fe0dadf532324..45f15a1f51295b298073551cb328d20cc2c36cdb 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -421,6 +421,20 @@ int coresight_resume_source(struct coresight_device *csdev) } EXPORT_SYMBOL_GPL(coresight_resume_source);

+static bool coresight_need_save_restore_source(struct coresight_device *csdev) +{ + if (!csdev) + return false; + + if (source_ops(csdev)->need_context_save_restore) + return source_ops(csdev)->need_context_save_restore(csdev); + + if (coresight_get_mode(csdev)) + return true; + + return false; +} + static int coresight_save_source(struct coresight_device *csdev) { if (csdev && source_ops(csdev)->save) @@ -1594,7 +1608,7 @@ static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, unsigned int cpu = smp_processor_id(); struct coresight_device *source = per_cpu(csdev_source, cpu);

- if (!source) + if (!coresight_need_save_restore_source(source)) return NOTIFY_OK;

switch (cmd) { diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index b76107aef9d3e42214d4bc97598795ddd96ff2d8..92ded5c87614b2a74736005c4d127e266cb9b0e2 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1836,6 +1836,21 @@ static int etm4_dying_cpu(unsigned int cpu) return 0; }

+static bool etm4_need_context_save_restore(struct coresight_device *csdev) +{ + if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED) + return false; + + /* + * Save and restore the ETM Trace registers only if + * the ETM is active. + */ + if (coresight_get_mode(csdev)) + return true; + + return false; +} + static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) { int ret = 0; @@ -1924,13 +1939,14 @@ static void etm4_cpu_restore(struct coresight_device *csdev) }

static const struct coresight_ops_source etm4_source_ops = { - .cpu_id = etm4_cpu_id, - .enable = etm4_enable, - .disable = etm4_disable, - .resume_perf = etm4_resume_perf, - .pause_perf = etm4_pause_perf, - .save = etm4_cpu_save, - .restore = etm4_cpu_restore, + .cpu_id = etm4_cpu_id, + .enable = etm4_enable, + .disable = etm4_disable, + .resume_perf = etm4_resume_perf, + .pause_perf = etm4_pause_perf, + .save = etm4_cpu_save, + .restore = etm4_cpu_restore, + .need_context_save_restore = etm4_need_context_save_restore, };

static const struct coresight_ops etm4_cs_ops = { diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 8d6f7cb354d6e487c757d9fe86cda895ccb1a588..4d5c07b7ddc67991a3871851fe45463f92bd32c8 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -400,6 +400,7 @@ struct coresight_ops_link { * @disable: disables tracing for a source. * @resume_perf: resumes tracing for a source in perf session. * @pause_perf: pauses tracing for a source in perf session. + * @need_context_save_restore: if need to save and restore context. * @save: save context for a source. * @restore: restore context for a source. */ @@ -411,6 +412,7 @@ struct coresight_ops_source { struct perf_event *event); int (*resume_perf)(struct coresight_device *csdev); void (*pause_perf)(struct coresight_device *csdev); + bool (*need_context_save_restore)(struct coresight_device *csdev); int (*save)(struct coresight_device *csdev); void (*restore)(struct coresight_device *csdev); };

A CTI device can be enabled as a helper in a CoreSight path or directly via the SysFS knob. Currently, the driver lacks protection when multiple flows contend for access to the same CTI device.

Use the device mode to mark the CTI as in use, and reset it to the DISABLED state when the device is no longer needed. Any conflict will cause the attempt to acquire the mode to fail, ensuring the device can only be enabled exclusively.

Fixes: 1b5b1646e63d ("coresight: Fix CTI module refcount leak by making it a helper device") Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-cti-core.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c index 2c8bf5dbe8b8206c92ae5ea64a26c947ef5b9582..ba4635dfc2e30b4e9ae4972f91bdc6647975b719 100644 --- a/drivers/hwtracing/coresight/coresight-cti-core.c +++ b/drivers/hwtracing/coresight/coresight-cti-core.c @@ -87,20 +87,31 @@ void cti_write_all_hw_regs(struct cti_drvdata *drvdata) }

/* write regs to hardware and enable */ -static int cti_enable_hw(struct cti_drvdata *drvdata) +static int cti_enable_hw(struct cti_drvdata *drvdata, enum cs_mode mode) { struct cti_config *config = &drvdata->config; + struct coresight_device *csdev = drvdata->csdev; unsigned long flags; int rc = 0;

raw_spin_lock_irqsave(&drvdata->spinlock, flags);

+ if (!drvdata->config.enable_req_count) { + coresight_set_mode(csdev, mode); + } else { + /* The device has been configured with an incompatible mode */ + if (coresight_get_mode(csdev) != mode) { + rc = -EBUSY; + goto cti_err_not_enabled; + } + } + /* no need to do anything if enabled or unpowered*/ if (config->hw_enabled || !config->hw_powered) goto cti_state_unchanged;

/* claim the device */ - rc = coresight_claim_device(drvdata->csdev); + rc = coresight_claim_device(csdev); if (rc) goto cti_err_not_enabled;

@@ -116,6 +127,8 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)

/* cannot enable due to error */ cti_err_not_enabled: + if (!drvdata->config.enable_req_count) + coresight_set_mode(csdev, CS_MODE_DISABLED); raw_spin_unlock_irqrestore(&drvdata->spinlock, flags); return rc; } @@ -165,6 +178,8 @@ static int cti_disable_hw(struct cti_drvdata *drvdata) if (--drvdata->config.enable_req_count > 0) goto cti_not_disabled;

+ coresight_set_mode(csdev, CS_MODE_DISABLED); + /* no need to do anything if disabled or cpu unpowered */ if (!config->hw_enabled || !config->hw_powered) goto cti_not_disabled; @@ -803,7 +818,7 @@ int cti_enable(struct coresight_device *csdev, enum cs_mode mode, void *data) { struct cti_drvdata *drvdata = csdev_to_cti_drvdata(csdev);

- return cti_enable_hw(drvdata); + return cti_enable_hw(drvdata, mode); }

int cti_disable(struct coresight_device *csdev, void *data)

This commit distinguishes CPU PM flows based on the mode.

The CTI driver retains its existing behavior for the CS_MODE_DEBUG mode.

For modes other than DEBUG, a future change will be made to manage CTI devices by iterating through the CoreSight path in the core layer. To avoid conflicts, the CTI driver no longer controls CTI hardware in CPU PM notifiers for non DEBUG modes.

However, the CTI driver continues to update the hw_powered flag for all modes to reflect the device's power state.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-cti-core.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c index ba4635dfc2e30b4e9ae4972f91bdc6647975b719..c91d339d7ccbf71db71cb7156f9a6f2961ae2198 100644 --- a/drivers/hwtracing/coresight/coresight-cti-core.c +++ b/drivers/hwtracing/coresight/coresight-cti-core.c @@ -141,6 +141,9 @@ static void cti_cpuhp_enable_hw(struct cti_drvdata *drvdata) raw_spin_lock(&drvdata->spinlock); config->hw_powered = true;

+ if (coresight_get_mode(drvdata->csdev) != CS_MODE_DEBUG) + goto cti_hp_not_enabled; + /* no need to do anything if no enable request */ if (!drvdata->config.enable_req_count) goto cti_hp_not_enabled; @@ -697,21 +700,27 @@ static int cti_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, case CPU_PM_ENTER: /* CTI regs all static - we have a copy & nothing to save */ drvdata->config.hw_powered = false; - if (drvdata->config.hw_enabled) + if ((coresight_get_mode(drvdata->csdev) == CS_MODE_DEBUG) && + drvdata->config.hw_enabled) coresight_disclaim_device(csdev); break;

case CPU_PM_ENTER_FAILED: drvdata->config.hw_powered = true; - if (drvdata->config.hw_enabled) { + if ((coresight_get_mode(drvdata->csdev) == CS_MODE_DEBUG) && + drvdata->config.hw_enabled) { if (coresight_claim_device(csdev)) drvdata->config.hw_enabled = false; } break;

case CPU_PM_EXIT: - /* write hardware registers to re-enable. */ drvdata->config.hw_powered = true; + + if (coresight_get_mode(drvdata->csdev) != CS_MODE_DEBUG) + break; + + /* write hardware registers to re-enable. */ drvdata->config.hw_enabled = false;

/* check enable reference count to enable HW */ @@ -760,7 +769,8 @@ static int cti_dying_cpu(unsigned int cpu)

raw_spin_lock(&drvdata->spinlock); drvdata->config.hw_powered = false; - if (drvdata->config.hw_enabled) + if ((coresight_get_mode(drvdata->csdev) == CS_MODE_DEBUG) && + drvdata->config.hw_enabled) coresight_disclaim_device(drvdata->csdev); raw_spin_unlock(&drvdata->spinlock); return 0;

Introduce an in_idle argument to the path enable and disable functions. When set to true, it skips to touch the sink device. To avoid invoking sink related helpers, the condition check is moved before the enable helpers are called.

This is a preparation for managing the path during CPU idle.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 29 +++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index ef2a0de366d9b12b4f609027c8b67f690cf84558..b1c122d1c4164e3ca6f1aaad0bd24917032626be 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -477,7 +477,8 @@ static void coresight_restore_source(struct coresight_device *csdev) * disabled. */ static void coresight_disable_path_from(struct coresight_path *path, - struct coresight_node *nd) + struct coresight_node *nd, + bool in_idle) { u32 type; struct coresight_device *csdev, *parent, *child; @@ -500,6 +501,10 @@ static void coresight_disable_path_from(struct coresight_path *path, CORESIGHT_DEV_TYPE_SINK : CORESIGHT_DEV_TYPE_LINK;

+ /* To reduce latency, CPU idle does not touch the sink */ + if (in_idle && type == CORESIGHT_DEV_TYPE_SINK) + continue; + switch (type) { case CORESIGHT_DEV_TYPE_SINK: coresight_disable_sink(csdev); @@ -535,7 +540,7 @@ void coresight_disable_path(struct coresight_path *path) if (coresight_is_percpu_source(source)) per_cpu(csdev_cpu_path, source->cpu) = NULL;

- coresight_disable_path_from(path, NULL); + coresight_disable_path_from(path, NULL, false); } EXPORT_SYMBOL_GPL(coresight_disable_path);

@@ -559,7 +564,8 @@ static int coresight_enable_helpers(struct coresight_device *csdev, }

static int _coresight_enable_path(struct coresight_path *path, - enum cs_mode mode, void *sink_data) + enum cs_mode mode, void *sink_data, + bool in_idle) { int ret = 0; u32 type; @@ -572,10 +578,6 @@ static int _coresight_enable_path(struct coresight_path *path, csdev = nd->csdev; type = csdev->type;

- /* Enable all helpers adjacent to the path first */ - ret = coresight_enable_helpers(csdev, mode, path); - if (ret) - goto err_disable_path; /* * ETF devices are tricky... They can be a link or a sink, * depending on how they are configured. If an ETF has been @@ -587,6 +589,15 @@ static int _coresight_enable_path(struct coresight_path *path, CORESIGHT_DEV_TYPE_SINK : CORESIGHT_DEV_TYPE_LINK;

+ /* To reduce latency, CPU idle does not touch the sink */ + if (in_idle && type == CORESIGHT_DEV_TYPE_SINK) + continue; + + /* Enable all helpers adjacent to the path first */ + ret = coresight_enable_helpers(csdev, mode, path); + if (ret) + goto err_disable_path; + switch (type) { case CORESIGHT_DEV_TYPE_SINK: ret = coresight_enable_sink(csdev, mode, sink_data); @@ -622,7 +633,7 @@ static int _coresight_enable_path(struct coresight_path *path, err_disable_helpers: coresight_disable_helpers(csdev, path); err_disable_path: - coresight_disable_path_from(path, nd); + coresight_disable_path_from(path, nd, false); goto out; }

@@ -632,7 +643,7 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode, int ret; struct coresight_device *source;

- ret = _coresight_enable_path(path, mode, sink_data); + ret = _coresight_enable_path(path, mode, sink_data, false); if (ret) return ret;

Unlike a system level's sink, the per-CPU sink may lose power during CPU idle states. Currently, this refers specifically to TRBE as the sink. This commit registers save and restore callbacks for the per-CPU sink via the PM notifier.

There are no changes to the coresight_enable_helpers() function; the code movement is solely for compilation purposes.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 101 +++++++++++++++++++++------ include/linux/coresight.h | 4 ++ 2 files changed, 82 insertions(+), 23 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 7693a0eade1a8de6d0960d66f6de682b5d5aff17..9978737d21177ab7cfcd449cf67a0b0736fcca5a 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -391,6 +391,25 @@ static void coresight_disable_helper(struct coresight_device *csdev, void *data) helper_ops(csdev)->disable(csdev, data); }

+static int coresight_enable_helpers(struct coresight_device *csdev, + enum cs_mode mode, void *data) +{ + int i, ret = 0; + struct coresight_device *helper; + + for (i = 0; i < csdev->pdata->nr_outconns; ++i) { + helper = csdev->pdata->out_conns[i]->dest_dev; + if (!helper || !coresight_is_helper(helper)) + continue; + + ret = coresight_enable_helper(helper, mode, data); + if (ret) + return ret; + } + + return 0; +} + static void coresight_disable_helpers(struct coresight_device *csdev, void *data) { int i; @@ -478,6 +497,43 @@ static void coresight_restore_source(struct coresight_device *csdev) source_ops(csdev)->restore(csdev); }

+static int coresight_save_percpu_sink(struct coresight_device *csdev) +{ + int ret; + + if (csdev && sink_ops(csdev)->save) { + ret = sink_ops(csdev)->save(csdev); + if (ret) + return ret; + + coresight_disable_helpers(csdev, NULL); + } + + /* Return success if callback is not supported */ + return 0; +} + +static int coresight_restore_percpu_sink(struct coresight_device *csdev, + struct coresight_path *path, + enum cs_mode mode) +{ + int ret = 0; + + if (csdev && sink_ops(csdev)->restore) { + ret = coresight_enable_helpers(csdev, mode, path); + if (ret) + return ret; + + ret = sink_ops(csdev)->restore(csdev); + if (ret) { + coresight_disable_helpers(csdev, path); + return ret; + } + } + + return ret; +} + /* * coresight_disable_path_from : Disable components in the given path beyond * @nd in the list. If @nd is NULL, all the components, except the SOURCE are @@ -551,25 +607,6 @@ void coresight_disable_path(struct coresight_path *path) } EXPORT_SYMBOL_GPL(coresight_disable_path);

-static int coresight_enable_helpers(struct coresight_device *csdev, - enum cs_mode mode, void *data) -{ - int i, ret = 0; - struct coresight_device *helper; - - for (i = 0; i < csdev->pdata->nr_outconns; ++i) { - helper = csdev->pdata->out_conns[i]->dest_dev; - if (!helper || !coresight_is_helper(helper)) - continue; - - ret = coresight_enable_helper(helper, mode, data); - if (ret) - return ret; - } - - return 0; -} - static int _coresight_enable_path(struct coresight_path *path, enum cs_mode mode, void *sink_data, bool in_idle) @@ -1667,9 +1704,12 @@ EXPORT_SYMBOL_GPL(coresight_alloc_device_name); static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, void *v) { + int ret; unsigned int cpu = smp_processor_id(); struct coresight_device *source = per_cpu(csdev_source, cpu); struct coresight_path *path; + struct coresight_device *sink; + enum cs_mode mode;

if (!coresight_need_save_restore_source(source)) return NOTIFY_OK; @@ -1682,18 +1722,33 @@ static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, if (WARN_ON(!path)) return NOTIFY_BAD;

+ sink = coresight_get_sink(path); + mode = coresight_get_mode(source); + switch (cmd) { case CPU_PM_ENTER: if (coresight_save_source(source)) return NOTIFY_BAD;

- coresight_disable_path_from(path, NULL, true); + ret = 0; + if (coresight_is_percpu_sink(sink)) + ret = coresight_save_percpu_sink(sink); + else + coresight_disable_path_from(path, NULL, true); + + if (ret) { + coresight_restore_source(source); + return NOTIFY_BAD; + } break; case CPU_PM_EXIT: case CPU_PM_ENTER_FAILED: - if (_coresight_enable_path(path, - coresight_get_mode(source), - NULL, true)) + if (coresight_is_percpu_sink(sink)) + ret = coresight_restore_percpu_sink(sink, path, mode); + else + ret = _coresight_enable_path(path, mode, NULL, true); + + if (ret) return NOTIFY_BAD;

coresight_restore_source(source); diff --git a/include/linux/coresight.h b/include/linux/coresight.h index f52e834640b72534ea83ab223aae7544b195bbaa..e551a36c40cc2311cd72948b799db5425b93fe68 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -363,6 +363,8 @@ enum cs_mode { * @alloc_buffer: initialises perf's ring buffer for trace collection. * @free_buffer: release memory allocated in @get_config. * @update_buffer: update buffer pointers after a trace session. + * @save: save context for a sink. + * @restore: restore context for a sink. */ struct coresight_ops_sink { int (*enable)(struct coresight_device *csdev, enum cs_mode mode, @@ -375,6 +377,8 @@ struct coresight_ops_sink { unsigned long (*update_buffer)(struct coresight_device *csdev, struct perf_output_handle *handle, void *sink_config); + int (*save)(struct coresight_device *csdev); + int (*restore)(struct coresight_device *csdev); };

/**

This commit moves CPU hotplug callbacks from ETMv4 driver to core layer. The motivation is the core layer can control all components on an activated path rather but not only managing tracer in ETMv4 driver.

The perf event layer will disable CoreSight PMU event 'cs_etm' when hotplug off a CPU. That means a perf mode will be always converted to disabled mode in CPU hotplug. Arm CoreSight CPU hotplug callbacks only need to handle the Sysfs mode and ignore the perf mode.

The core layer invokes a high level API coresight_disable_source() to disable a source when hotplug-off a CPU. It disables a tracer and changes the tracer's mode to CS_MODE_DISABLED.

When hotplug-in a CPU, if a activated path is detected - when the activated path pointer is not NULL - in this case, the tracer will be re-enabled.

Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 61 +++++++++++++++++++++- drivers/hwtracing/coresight/coresight-etm4x-core.c | 37 ------------- include/linux/coresight.h | 17 +++--- 3 files changed, 70 insertions(+), 45 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 9978737d21177ab7cfcd449cf67a0b0736fcca5a..e1659b05738e73fcb6c66c74c8f1ce3a24ab2da4 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1701,6 +1701,52 @@ char *coresight_alloc_device_name(struct coresight_dev_list *dict, } EXPORT_SYMBOL_GPL(coresight_alloc_device_name);

+static int coresight_starting_cpu(unsigned int cpu) +{ + struct coresight_device *source = per_cpu(csdev_source, cpu); + struct coresight_path *path; + + if (!source) + return 0; + + /* Re-enable components on an activated path */ + path = per_cpu(csdev_cpu_path, cpu); + if (!path) + return 0; + + if (path->saved_mode != CS_MODE_SYSFS) + return 0; + + source_ops(source)->enable(source, NULL, path->saved_mode, path); + return 0; +} + +static int coresight_dying_cpu(unsigned int cpu) +{ + struct coresight_device *source = per_cpu(csdev_source, cpu); + struct coresight_path *path; + + if (!source) + return 0; + + /* Don't proceed if no path is activated */ + path = per_cpu(csdev_cpu_path, cpu); + if (!path) + return 0; + + path->saved_mode = coresight_get_mode(source); + + /* + * The perf event layer will disable PMU events in the CPU hotplug. + * CoreSight driver should never handle the CS_MODE_PERF case. + */ + if (WARN_ON(path->saved_mode != CS_MODE_SYSFS)) + return 0; + + source_ops(source)->disable(source, NULL); + return 0; +} + static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, void *v) { @@ -1766,11 +1812,24 @@ static struct notifier_block coresight_cpu_pm_nb = {

static int __init coresight_pm_setup(void) { - return cpu_pm_register_notifier(&coresight_cpu_pm_nb); + int ret; + + ret = cpu_pm_register_notifier(&coresight_cpu_pm_nb); + if (ret) + return ret; + + ret = cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING, + "arm/coresight-core:starting", + coresight_starting_cpu, coresight_dying_cpu); + if (ret) + cpu_pm_unregister_notifier(&coresight_cpu_pm_nb); + + return ret; }

static void coresight_pm_cleanup(void) { + cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING); cpu_pm_unregister_notifier(&coresight_cpu_pm_nb); }

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index f99a48511850cd6e9682533880b22a3b8fc43135..94872226dd63c5b9ed9ef95f17d717656c4e5589 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1802,33 +1802,6 @@ static int etm4_online_cpu(unsigned int cpu) return 0; }

-static int etm4_starting_cpu(unsigned int cpu) -{ - if (!etmdrvdata[cpu]) - return 0; - - raw_spin_lock(&etmdrvdata[cpu]->spinlock); - if (!etmdrvdata[cpu]->os_unlock) - etm4_os_unlock(etmdrvdata[cpu]); - - if (coresight_get_mode(etmdrvdata[cpu]->csdev)) - etm4_enable_hw(etmdrvdata[cpu], true); - raw_spin_unlock(&etmdrvdata[cpu]->spinlock); - return 0; -} - -static int etm4_dying_cpu(unsigned int cpu) -{ - if (!etmdrvdata[cpu]) - return 0; - - raw_spin_lock(&etmdrvdata[cpu]->spinlock); - if (coresight_get_mode(etmdrvdata[cpu]->csdev)) - etm4_disable_hw(etmdrvdata[cpu]); - raw_spin_unlock(&etmdrvdata[cpu]->spinlock); - return 0; -} - static bool etm4_need_context_save_restore(struct coresight_device *csdev) { if (pm_save_enable != PARAM_PM_SAVE_SELF_HOSTED) @@ -1923,13 +1896,6 @@ static int __init etm4_pm_setup(void) { int ret;

- ret = cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING, - "arm/coresight4:starting", - etm4_starting_cpu, etm4_dying_cpu); - - if (ret) - return ret; - ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "arm/coresight4:online", etm4_online_cpu, NULL); @@ -1940,14 +1906,11 @@ static int __init etm4_pm_setup(void) return 0; }

- /* failed dyn state - remove others */ - cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING); return ret; }

static void etm4_pm_clear(void) { - cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING); if (hp_online) { cpuhp_remove_state_nocalls(hp_online); hp_online = 0; diff --git a/include/linux/coresight.h b/include/linux/coresight.h index e551a36c40cc2311cd72948b799db5425b93fe68..6daed3a75190617464515c4e1e0628d2169b26e7 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -330,21 +330,24 @@ static struct coresight_dev_list (var) = { \

#define to_coresight_device(d) container_of(d, struct coresight_device, dev)

+enum cs_mode { + CS_MODE_DISABLED, + CS_MODE_SYSFS, + CS_MODE_PERF, + CS_MODE_DEBUG, +}; + /** * struct coresight_path - data needed by enable/disable path * @path_list: path from source to sink. * @trace_id: trace_id of the whole path. + * @saved_mode: The saved device mode. It stores the source device's mode + * to represent the path mode during CPU hotplug off. */ struct coresight_path { struct list_head path_list; u8 trace_id; -}; - -enum cs_mode { - CS_MODE_DISABLED, - CS_MODE_SYSFS, - CS_MODE_PERF, - CS_MODE_DEBUG, + enum cs_mode saved_mode; };

#define coresight_ops(csdev) csdev->ops