On 4/14/2026 12:15 PM, Jie Gan wrote:
>
> I dont think you have looped relevant maintainers and reviewers for the Coresight subsystem.
>
> Thanks,
> Jie
I will send it to all relevant maintainers and reviewers for the Coresight subsystem in next revision.
Thanks,
Zane
On Tue, Apr 14, 2026 at 11:30:39AM +0800, Jie Gan wrote:
[...]
> > > Tested-by: Jie Gan <jie.gan(a)oss.qualcomm.com>
> >
> > Just heads up: since Sashiko [1] pointed out a corner case where an SMP call
> > may fail when disabling the source device, the per-CPU path pointer
> > might not be cleared. If the ETMv4 device is then removed (e.g. if the
> > user unloads the ETMv4 module), CPU PM notifier might access the stale
> > path pointer. Though this is a rare case, we should handle it safely.
> > This is why the series was not picked for the v7.1 merge window.
> >
> > Thanks a lot for the testing, Jie! It's very helpful, and I will add
> > your test tags in the next spin.
> >
> > Anyway, please expect more iterations.
>
> Noted, will run the test cases on new iterations.
Thank you!
On Mon, Apr 13, 2026 at 03:19:54PM +0100, Yeoreum Yun wrote:
> Introduce struct etm4_caps to describe ETMv4 capabilities
> and move capabilities information into it.
>
> Signed-off-by: Yeoreum Yun <yeoreum.yun(a)arm.com>
LGTM:
Reviewed-by: Leo Yan <leo.yan(a)arm.com>
FWIW, two comments from Sashiko are valuable for me, please see below.
> ---
> .../coresight/coresight-etm4x-core.c | 234 +++++++++---------
> .../coresight/coresight-etm4x-sysfs.c | 190 ++++++++------
> drivers/hwtracing/coresight/coresight-etm4x.h | 176 ++++++-------
> 3 files changed, 328 insertions(+), 272 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index d565a73f0042..6443f3717b37 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -88,8 +88,9 @@ static int etm4_probe_cpu(unsigned int cpu);
> */
> static bool etm4x_sspcicrn_present(struct etmv4_drvdata *drvdata, int n)
> {
> - return (n < drvdata->nr_ss_cmp) &&
> - drvdata->nr_pe &&
> + const struct etmv4_caps *caps = &drvdata->caps;
> +
> + return (n < caps->nr_ss_cmp) && caps->nr_pe &&
> (drvdata->config.ss_status[n] & TRCSSCSRn_PC);
As Sashiko suggests:
"This isn't a regression introduced by this patch, but should this be
checking caps->nr_pe_cmp instead of caps->nr_pe?"
I confirmed the ETMv4 specification (ARM IHI0064H.b), the comment
above is valid as the we should check caps->nr_pe_cmp instead.
Could you first use a patch to fix the typo and then apply
capabilities afterwards? This is helpful for porting to stable
kernels.
[...]
> @@ -525,14 +530,14 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
> if (etm4x_wait_status(csa, TRCSTATR_IDLE_BIT, 1))
> dev_err(etm_dev,
> "timeout while waiting for Idle Trace Status\n");
> - if (drvdata->nr_pe)
> + if (caps->nr_pe)
> etm4x_relaxed_write32(csa, config->pe_sel, TRCPROCSELR);
> etm4x_relaxed_write32(csa, config->cfg, TRCCONFIGR);
> /* nothing specific implemented */
> etm4x_relaxed_write32(csa, 0x0, TRCAUXCTLR);
> etm4x_relaxed_write32(csa, config->eventctrl0, TRCEVENTCTL0R);
> etm4x_relaxed_write32(csa, config->eventctrl1, TRCEVENTCTL1R);
> - if (drvdata->stallctl)
> + if (caps->stallctl)
> etm4x_relaxed_write32(csa, config->stall_ctrl, TRCSTALLCTLR);
> etm4x_relaxed_write32(csa, config->ts_ctrl, TRCTSCTLR);
> etm4x_relaxed_write32(csa, config->syncfreq, TRCSYNCPR);
> @@ -542,17 +547,17 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
> etm4x_relaxed_write32(csa, config->vinst_ctrl, TRCVICTLR);
> etm4x_relaxed_write32(csa, config->viiectlr, TRCVIIECTLR);
> etm4x_relaxed_write32(csa, config->vissctlr, TRCVISSCTLR);
> - if (drvdata->nr_pe_cmp)
> + if (caps->nr_pe_cmp)
> etm4x_relaxed_write32(csa, config->vipcssctlr, TRCVIPCSSCTLR);
> - for (i = 0; i < drvdata->nrseqstate - 1; i++)
> + for (i = 0; i < caps->nrseqstate - 1; i++)
> etm4x_relaxed_write32(csa, config->seq_ctrl[i], TRCSEQEVRn(i));
Sashiko's comment:
"If the hardware does not implement a sequencer, caps->nrseqstate (a u8)
will be 0. Does 0 - 1 evaluate to -1 as an int, which then gets promoted
to ULONG_MAX against val (an unsigned long)?"
This is a good catch. The condition check should be:
for (i = 0; i < caps->nrseqstate; i++)
...;
The issue is irrelevant to your patch, but could you use a patch to fix
"nrseqstate - 1" first and then apply the cap refactoring on it? This
would be friendly for porting to stable kernel.
Thanks,
Leo
On Mon, Apr 13, 2026 at 06:30:18PM +0800, Jie Gan wrote:
[...]
> tested on QCOM sa8775-ride:
>
> === 1. Sysfs mode: basic enable/disable ===
> PASS: Sink tmc_etr0 enabled
> PASS: Source etm0 enabled
> PASS: Source etm0 disabled cleanly
> PASS: Sink tmc_etr0 disabled cleanly
>
> === 2. Sysfs mode: repeated enable/disable cycles (10x) ===
> PASS: 10 enable/disable cycles completed without error
>
> === 3. Sysfs mode: enable source with no active sink ===
> PASS: Enable without sink returned error (expected)
>
> === 4. Sysfs mode: enable/disable all per-CPU sources ===
> etm0 (cpu0): enabled OK
> etm1 (cpu1): enabled OK
> etm2 (cpu2): enabled OK
> etm3 (cpu3): enabled OK
> etm4 (cpu4): enabled OK
> etm5 (cpu5): enabled OK
> etm6 (cpu6): enabled OK
> etm7 (cpu7): enabled OK
> PASS: All online per-CPU sources enabled/disabled successfully
>
> === 5. CPU hotplug: offline CPU while sysfs tracing active ===
> Using source etm1 on cpu1
> Tracing active on cpu1, offlining CPU...
> [ 82.805359] psci: CPU1 killed (polled 0 ms)
> PASS: Source auto-disabled on CPU offline
> [ 83.346033] Detected PIPT I-cache on CPU1
> [ 83.346114] GICv3: CPU1: found redistributor 100 region
> 0:0x0000000017a80000
> [ 83.346283] CPU1: Booted secondary processor 0x0000000100 [0x410fd4b2]
> PASS: Source re-enabled after CPU re-online
>
> === 6. Sysfs: enable source on offline CPU (expect ENODEV) ===
> [ 84.013788] psci: CPU1 killed (polled 0 ms)
> PASS: Enable on offline cpu1 rejected (enable_source=0)
> [ 84.349558] Detected PIPT I-cache on CPU1
> [ 84.349640] GICv3: CPU1: found redistributor 100 region
> 0:0x0000000017a80000
> [ 84.349811] CPU1: Booted secondary processor 0x0000000100 [0x410fd4b2]
>
> === 7. CPU PM: trace survives CPU idle entry/exit ===
> Sleeping 3s to allow CPU idle entry...
> Idle entries on cpu0 during test: 35
> PASS: Source still enabled after idle (PM save/restore working)
>
> === 8. Perf mode: basic cs_etm recording ===
> SKIP: perf not found in PATH
>
> === 11. TRBE: check save/restore sysfs nodes (if present) ===
> SKIP: No TRBE devices found
>
> Tested-by: Jie Gan <jie.gan(a)oss.qualcomm.com>
Just heads up: since Sashiko [1] pointed out a corner case where an SMP call
may fail when disabling the source device, the per-CPU path pointer
might not be cleared. If the ETMv4 device is then removed (e.g. if the
user unloads the ETMv4 module), CPU PM notifier might access the stale
path pointer. Though this is a rare case, we should handle it safely.
This is why the series was not picked for the v7.1 merge window.
Thanks a lot for the testing, Jie! It's very helpful, and I will add
your test tags in the next spin.
Anyway, please expect more iterations.
Thanks,
Leo
[1] https://sashiko.dev/#/patchset/20260405-arm_coresight_path_power_management…
On Mon, Apr 13, 2026 at 01:45:50PM +0800, Jie Gan wrote:
[...]
> > @@ -1787,15 +1808,32 @@ static int coresight_pm_save(struct coresight_path *path)
> > to = list_prev_entry(coresight_path_last_node(path), link);
> > coresight_disable_path_from_to(path, from, to);
> > + ret = coresight_pm_device_save(coresight_get_sink(path));
> > + if (ret)
> > + goto sink_failed;
> > +
> > return 0;
> > +
> > +sink_failed:
> > + if (!coresight_enable_path_from_to(path, coresight_get_mode(source),
> > + from, to))
> > + coresight_pm_device_restore(source);
>
> I have go through the history messages. I have a question about this point
> here:
>
> how can we handle the scenario if coresight_enable_path_from_to failed? It
> means we are never calling coresight_pm_device_restore for the ETM and
> leaving the ETM with OS lock state until CPU reset?
From a design perspective, if any failure occurs in the idle flow, the
priority is to avoid further mess, especially partial enable/disable
sequences that could lead to lockups.
The case you mentioned is a typical risk - if a path after source to
sink fails to be enabled, it is unsafe to arbitrarily enable the source
further. We rely on the per-CPU flag "percpu_pm_failed" to disable idle
states, if ETE/TRBE fails to be disabled, if CPU is turned off, this
also might cause lockup.
> Consider we are calling etm4_disable_hw with OS lock:
> etm4_disable_hw -> etm4_disable_trace_unit -> etm4x_wait_status (may timeout
> here?)
This is expected. I don't want to introduce a _recovery_ mechanism for
CPU PM failures, which is complex and over-engineering. CPU PM notifier
is low level code, and in my experience, PM issues can be easily
observed once CPU idle is enabled and should be resolved during the
development phase.
In many cases PM issues are often not caused by CoreSight drivers but by
other modules (e.g., clock or regulator drivers). The log "Failed in
coresight PM save ..." reminds developers the bugs. As said,
percpu_pm_failed is used as a last resort to prevent the platform from
locking up if there is a PM bug.
Thanks,
Leo
This series focuses on CoreSight path power management. The changes can
be divided into four parts for review:
Patches 01 - 04: Refactor the CPU ID storing in csdev, later patches
consume csdev->cpu. Move CPU lock to sysfs layer so
it is safe for later changes.
Patches 05 - 09: Refactor the CPU idle flow with moving common code into
the CoreSight core layer.
Patches 10 - 15: Refactor path enabling and disabling with range, add
path control during CPU idle.
Patches 16 - 17: Support the sink (TRBE) control during CPU idle.
Patches 18 - 20: Move CPU hotplug into the core layer, and fix sysfs
mode for hotplug.
This series is rebased on the coresight-next branch and has been verified
on Juno-r2 (ETM + ETR) and FVP RevC (ETE + TRBE). Built successfully
for armv7 (ARCH=arm).
---
Changes in v10:
- Removed redundant checks in ETMv4 PM callbacks (sashiko).
- Added a new const structure etm4_cs_pm_ops (sashiko).
- Used fine-grained spinlock on sysfs_active_config (sashiko).
- Blocked notification after failures in save / restore to avoid lockups.
- Changed Change CPUHP_AP_ARM_CORESIGHT_STARTING to
CPUHP_AP_ARM_CORESIGHT_ONLINE so that the CPU hotplug callback runs in
the thread context (sashiko).
- Link to v9: https://lore.kernel.org/r/20260401-arm_coresight_path_power_management_impr…
Changes in v9:
- Changed to use per-CPU path pointer with lockless access.
- Removed the change for adding csdev->path, the related refactoring
will be sent separately.
- Re-orged patches to avoid intermediate breakage (sashiko).
- Link to v8: https://lore.kernel.org/r/20260325-arm_coresight_path_power_management_impr…
Changes in v8:
- Moved the "cpu" field in coresight_device for better pack with new
patch 01 (Suzuki).
- Added check if not set CPU for per_cpu_source/per_cpu_sink (Suzuki).
- Renamed spinlock name in syscfg (Suzuki).
- Refactored paired enable and disable path with new patches
10 and 12 (Suzuki).
- Link to v7: https://lore.kernel.org/r/20260320-arm_coresight_path_power_management_impr…
Changes in v7:
- Added a flag in coresight_desc to indicate CPU bound device (Suzuki).
- Used coresight_mutex to protect per-CPU source pointer (Suzuki).
- Added a spinlock for exclusive access per-CPU source pointer (Suzuki).
- Dropped .pm_is_needed() callback (Suzuki).
- Supported range in path enabling / disabling (Suzuki).
- Gathered test and review tags (Levi / James).
- Link to v6: https://lore.kernel.org/r/20260305-arm_coresight_path_power_management_impr…
Signed-off-by: Leo Yan <leo.yan(a)arm.com>
---
Leo Yan (19):
coresight: Extract device init into coresight_init_device()
coresight: Populate CPU ID into coresight_device
coresight: Remove .cpu_id() callback from source ops
coresight: Take hotplug lock in enable_source_store() for Sysfs mode
coresight: etm4x: Set per-CPU path on local CPU
coresight: etm3x: Set per-CPU path on local CPU
coresight: Register CPU PM notifier in core layer
coresight: etm4x: Hook CPU PM callbacks
coresight: etm4x: Remove redundant checks in PM save and restore
coresight: syscfg: Use IRQ-safe spinlock to protect active variables
coresight: Move source helper disabling to coresight_disable_path()
coresight: Control path with range
coresight: Use helpers to fetch first and last nodes
coresight: Introduce coresight_enable_source() helper
coresight: Control path during CPU idle
coresight: Add PM callbacks for sink device
coresight: sysfs: Increment refcount only for system tracers
coresight: Move CPU hotplug callbacks to core layer
coresight: sysfs: Validate CPU online status for per-CPU sources
Yabin Cui (1):
coresight: trbe: Save and restore state across CPU low power state
drivers/hwtracing/coresight/coresight-catu.c | 2 +-
drivers/hwtracing/coresight/coresight-core.c | 419 ++++++++++++++++++---
drivers/hwtracing/coresight/coresight-cti-core.c | 9 +-
drivers/hwtracing/coresight/coresight-etm-perf.c | 4 +-
drivers/hwtracing/coresight/coresight-etm3x-core.c | 73 +---
drivers/hwtracing/coresight/coresight-etm4x-core.c | 168 ++-------
drivers/hwtracing/coresight/coresight-priv.h | 4 +
drivers/hwtracing/coresight/coresight-syscfg.c | 35 +-
drivers/hwtracing/coresight/coresight-syscfg.h | 2 +
drivers/hwtracing/coresight/coresight-sysfs.c | 50 ++-
drivers/hwtracing/coresight/coresight-trbe.c | 61 ++-
include/linux/coresight.h | 13 +-
include/linux/cpuhotplug.h | 2 +-
13 files changed, 566 insertions(+), 276 deletions(-)
---
base-commit: ec687ba84000d7d50cf243558041f6729d1d8119
change-id: 20251104-arm_coresight_path_power_management_improvement-dab4966f8280
Best regards,
--
Leo Yan <leo.yan(a)arm.com>