On 01/05/2026 9:24 am, Suzuki K Poulose wrote:
On 28/04/2026 13:18, James Clark wrote:
ETE registers are always system registers so it's highly unlikely there will be an implementation that preserves them on CPU power down. Also the ETE DT binding never documented "arm,coresight-loses-context-with-cpu" so nobody would have legitimately been able to use that binding to fix it.
Fix it by hard coding the setting for ETE and add a warning if the user tried to use the module parameter. Don't add a warning if loses-context-with-cpu is present in the DT as it's not a documented binding anyway. etm4_init_pm_save() needs to happen after drvdata is initialised so etm4x_is_ete() can be called.
This fixes the following error when using Coresight with ACPI on the FVP which supports CPU PM:
coresight ete0: External agent took claim tag WARNING: drivers/hwtracing/coresight/coresight-core.c:248 at coresight_disclaim_device_unlocked+0xe0/0xe8, CPU#0: perf/117
Fixes: 35e1c9163e02 ("coresight: ete: Add support for ETE tracing") Signed-off-by: James Clark james.clark@linaro.org
drivers/hwtracing/coresight/coresight-etm4x-core.c | 41 ++++++++++++ +++------- 1 file changed, 29 insertions(+), 12 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/ drivers/hwtracing/coresight/coresight-etm4x-core.c index d565a73f0042..a7fb680dd383 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -56,10 +56,11 @@ MODULE_PARM_DESC(boot_enable, "Enable tracing on boot"); #define PARAM_PM_SAVE_NEVER 1 /* never save any state */ #define PARAM_PM_SAVE_SELF_HOSTED 2 /* save self-hosted state only */ +/* Save option for ETM4. ETE ignores this option and always saves */ static int pm_save_enable = PARAM_PM_SAVE_FIRMWARE; module_param(pm_save_enable, int, 0444); MODULE_PARM_DESC(pm_save_enable, - "Save/restore state on power down: 1 = never, 2 = self-hosted"); + "Save/restore state on power down: 1 = never, 2 = self-hosted. ETM4 only."); static struct etmv4_drvdata *etmdrvdata[NR_CPUS]; static void etm4_set_default_config(struct etmv4_config *config); @@ -1365,6 +1366,30 @@ static void etm4_fixup_wrong_ccitmin(struct etmv4_drvdata *drvdata) } } +static int etm4_init_pm_save(struct device *dev, struct etmv4_drvdata *drvdata) +{ + if (etm4x_is_ete(drvdata)) { + /* + * Always do PM save for ETE. It always uses system registers + * which will be lost on CPU power down. + */ + pm_save_enable = PARAM_PM_SAVE_SELF_HOSTED;
Should we do this instead based on if the ETM/ETE is accessed via sys instructions ? That would cover all implementations?
Suzuki
I did discuss that with Leo but we thought it might be a riskier change as ETM is older and nobody has reported any issues, and there is already the DT option to fix it that way. Turning it on by default could cause some performance regressions. Although it's only if the ETM is in use, so the impact is minimal.
In reality it probably does make sense as it's highly unlikely that sysreg ETM wouldn't need saving, so it might make some platforms with misconfigured DTs more stable.
I can send another version if you think it makes sense, what do you think Leo?
+ } else if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE) { + 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; + }
+ return 0; +}
static void etm4_init_arch_data(void *info) { u32 etmidr0; @@ -2247,6 +2272,9 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) return -ENOMEM; etm4_set_default(&drvdata->config); + ret = etm4_init_pm_save(dev, drvdata); + if (ret) + return ret; pdata = coresight_get_platform_data(dev); if (IS_ERR(pdata)) @@ -2305,17 +2333,6 @@ static int etm4_probe(struct device *dev) if (ret) return ret; - if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE) - 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);