From: Jie Gan jie.gan@oss.qualcomm.com

In coresight_enable_sysfs(), for non-CPU sources (SOFTWARE, TPDM, OTHERS), the source device is enabled via coresight_enable_source_sysfs() before idr_alloc_u32() maps the path. If idr_alloc_u32() fails, the original code jumped directly to err_source, which only calls coresight_disable_path() and coresight_release_path(). The source device was left enabled with an incremented refcnt but no path tracked for it, leaving the device in an inconsistent state.

Disable the source before jumping to err_source so the enable and path operations are fully unwound.

Fixes: 5c0016d7b343 ("coresight: core: Use IDR for non-cpu bound sources' paths.") Signed-off-by: Jie Gan jie.gan@oss.qualcomm.com --- drivers/hwtracing/coresight/coresight-sysfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c index b6a870399e83419dce0552099562fa3ae7b2bd69..da6f22b512c92ab0cff5bb239495a1ab6d2a0dbe 100644 --- a/drivers/hwtracing/coresight/coresight-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-sysfs.c @@ -244,8 +244,10 @@ int coresight_enable_sysfs(struct coresight_device *csdev) */ hash = hashlen_hash(hashlen_string(NULL, dev_name(&csdev->dev))); ret = idr_alloc_u32(&path_idr, path, &hash, hash, GFP_KERNEL); - if (ret) + if (ret) { + coresight_disable_source_sysfs(csdev, NULL); goto err_source; + } break; default: /* We can't be here */

This commit extracts the allocation and initialization of the coresight device structure into a separate function to make future extensions easier.

Tested-by: Jie Gan jie.gan@oss.qualcomm.com Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Reviewed-by: James Clark james.clark@linaro.org Tested-by: James Clark james.clark@linaro.org Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 256f6a32621b8e086d02427317e5d35d0f29a3c9..d5d18168b48125c91f556f3867f0719592f0ccc8 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1334,20 +1334,16 @@ void coresight_release_platform_data(struct device *dev, devm_kfree(dev, pdata); }

-struct coresight_device *coresight_register(struct coresight_desc *desc) +static struct coresight_device * +coresight_init_device(struct coresight_desc *desc) { - int ret; struct coresight_device *csdev; - bool registered = false;

csdev = kzalloc_obj(*csdev); - if (!csdev) { - ret = -ENOMEM; - goto err_out; - } + if (!csdev) + return ERR_PTR(-ENOMEM);

csdev->pdata = desc->pdata; - csdev->type = desc->type; csdev->subtype = desc->subtype; csdev->ops = desc->ops; @@ -1360,6 +1356,21 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) csdev->dev.release = coresight_device_release; csdev->dev.bus = &coresight_bustype;

+ return csdev; +} + +struct coresight_device *coresight_register(struct coresight_desc *desc) +{ + int ret; + struct coresight_device *csdev; + bool registered = false; + + csdev = coresight_init_device(desc); + if (IS_ERR(csdev)) { + ret = PTR_ERR(csdev); + goto err_out; + } + if (csdev->type == CORESIGHT_DEV_TYPE_SINK || csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) { raw_spin_lock_init(&csdev->perf_sink_id_map.lock);

The CPU ID can be fetched directly from the coresight_device structure, so the .cpu_id() callback is no longer needed.

Remove the .cpu_id() callback from source ops and update callers accordingly.

Tested-by: Jie Gan jie.gan@oss.qualcomm.com Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Reviewed-by: James Clark james.clark@linaro.org Tested-by: James Clark james.clark@linaro.org Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 8 ++++---- drivers/hwtracing/coresight/coresight-etm-perf.c | 2 +- drivers/hwtracing/coresight/coresight-etm3x-core.c | 8 -------- drivers/hwtracing/coresight/coresight-etm4x-core.c | 8 -------- drivers/hwtracing/coresight/coresight-sysfs.c | 4 ++-- include/linux/coresight.h | 3 --- 6 files changed, 7 insertions(+), 26 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index ffed314d1313518b1db1e702986edb0dfc6644d3..5c711e5175014d2a7dce1ec544cd6c89f60d3a7a 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -799,7 +799,7 @@ static int _coresight_build_path(struct coresight_device *csdev, goto out;

if (coresight_is_percpu_source(csdev) && coresight_is_percpu_sink(sink) && - sink == per_cpu(csdev_sink, source_ops(csdev)->cpu_id(csdev))) { + sink == per_cpu(csdev_sink, csdev->cpu)) { if (_coresight_build_path(sink, source, sink, path) == 0) { found = true; goto out; @@ -1026,7 +1026,7 @@ coresight_find_default_sink(struct coresight_device *csdev) /* look for a default sink if we have not found for this device */ if (!csdev->def_sink) { if (coresight_is_percpu_source(csdev)) - csdev->def_sink = per_cpu(csdev_sink, source_ops(csdev)->cpu_id(csdev)); + csdev->def_sink = per_cpu(csdev_sink, csdev->cpu); if (!csdev->def_sink) csdev->def_sink = coresight_find_sink(csdev, &depth); } @@ -1764,10 +1764,10 @@ int coresight_etm_get_trace_id(struct coresight_device *csdev, enum cs_mode mode { int cpu, trace_id;

- if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE || !source_ops(csdev)->cpu_id) + if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE) return -EINVAL;

- cpu = source_ops(csdev)->cpu_id(csdev); + cpu = csdev->cpu; switch (mode) { case CS_MODE_SYSFS: trace_id = coresight_trace_id_get_cpu_id(cpu); diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 89ba7c9a66137007a9f39efb3f04d55e8237eacb..7434f68d448253ed3de5acae45ded50e68ff2dcf 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -825,7 +825,7 @@ static void etm_addr_filters_sync(struct perf_event *event) int etm_perf_symlink(struct coresight_device *csdev, bool link) { char entry[sizeof("cpu9999999")]; - int ret = 0, cpu = source_ops(csdev)->cpu_id(csdev); + int ret = 0, cpu = csdev->cpu; struct device *pmu_dev = etm_pmu.dev; struct device *cs_dev = &csdev->dev;

diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c index eb665db1a37d9970f7f55395c0aa23b98a7f3118..ab47f69e923fb191b48b82367dce465c79b3a93d 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -466,13 +466,6 @@ static void etm_enable_sysfs_smp_call(void *info) coresight_set_mode(csdev, CS_MODE_DISABLED); }

-static int etm_cpu_id(struct coresight_device *csdev) -{ - struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); - - return drvdata->cpu; -} - void etm_release_trace_id(struct etm_drvdata *drvdata) { coresight_trace_id_put_cpu_id(drvdata->cpu); @@ -684,7 +677,6 @@ static void etm_disable(struct coresight_device *csdev, }

static const struct coresight_ops_source etm_source_ops = { - .cpu_id = etm_cpu_id, .enable = etm_enable, .disable = etm_disable, }; diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 8f270bfc74723f1fc1cead2da1725ab6b50f910e..b7312570a7ae1f0355c01b6f7a54ea2dd8891097 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -231,13 +231,6 @@ static void etm4_cs_unlock(struct etmv4_drvdata *drvdata, CS_UNLOCK(csa->base); }

-static int etm4_cpu_id(struct coresight_device *csdev) -{ - struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); - - return drvdata->cpu; -} - void etm4_release_trace_id(struct etmv4_drvdata *drvdata) { coresight_trace_id_put_cpu_id(drvdata->cpu); @@ -1205,7 +1198,6 @@ static void etm4_pause_perf(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, diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c index da6f22b512c92ab0cff5bb239495a1ab6d2a0dbe..905c973e99cea884e242eab460b31544c49378ad 100644 --- a/drivers/hwtracing/coresight/coresight-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-sysfs.c @@ -232,7 +232,7 @@ int coresight_enable_sysfs(struct coresight_device *csdev) * be a single session per tracer (when working from sysFS) * a per-cpu variable will do just fine. */ - cpu = source_ops(csdev)->cpu_id(csdev); + cpu = csdev->cpu; per_cpu(tracer_path, cpu) = path; break; case CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE: @@ -284,7 +284,7 @@ void coresight_disable_sysfs(struct coresight_device *csdev)

switch (csdev->subtype.source_subtype) { case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC: - cpu = source_ops(csdev)->cpu_id(csdev); + cpu = csdev->cpu; path = per_cpu(tracer_path, cpu); per_cpu(tracer_path, cpu) = NULL; break; diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 687190ca11ddeaa83193caa3903a480bac3060d1..e9c20ceb9016fa3db256b8c1147c1fd2027b7b0d 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -395,15 +395,12 @@ struct coresight_ops_link { /** * struct coresight_ops_source - basic operations for a source * Operations available for sources. - * @cpu_id: returns the value of the CPU number this component - * is associated to. * @enable: enables tracing for a source. * @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. */ struct coresight_ops_source { - int (*cpu_id)(struct coresight_device *csdev); int (*enable)(struct coresight_device *csdev, struct perf_event *event, enum cs_mode mode, struct coresight_path *path); void (*disable)(struct coresight_device *csdev,

ETM perf callbacks currently use the per-CPU csdev_src pointer, which can race with updates during device registration and unregistration.

The AUX setup already builds and stores the path in the event data. Use this path to retrieve the source instead of csdev_src to avoid the race.

Export coresight_get_source() and add etm_event_get_ctxt_path() to retrieve the context's path and its source with READ_ONCE() / WRITE_ONCE() accessors. Give the comments to explain why this approach is safe when pause or resume callbacks preempt the disable callback (e.g. via NMI).

Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Reviewed-by: James Clark james.clark@linaro.org Tested-by: James Clark james.clark@linaro.org Tested-by: Jie Gan jie.gan@oss.qualcomm.com Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 2 +- drivers/hwtracing/coresight/coresight-etm-perf.c | 114 ++++++++++++++--------- drivers/hwtracing/coresight/coresight-priv.h | 1 + 3 files changed, 74 insertions(+), 43 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 5c711e5175014d2a7dce1ec544cd6c89f60d3a7a..3fc6a5db778c89acad6ee05fe15ec0b8b8dab6bc 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -82,7 +82,7 @@ struct coresight_device *coresight_get_percpu_sink(int cpu) } EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);

-static struct coresight_device *coresight_get_source(struct coresight_path *path) +struct coresight_device *coresight_get_source(struct coresight_path *path) { struct coresight_device *csdev;

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 7434f68d448253ed3de5acae45ded50e68ff2dcf..bab85f7dac4757173912639ea016cdc32e5616fb 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -315,6 +315,35 @@ static bool sinks_compatible(struct coresight_device *a, (sink_ops(a) == sink_ops(b)); }

+/* + * This helper is used for fetching the path pointer via the ctxt. + * + * Perf event callbacks run on the same CPU in atomic context, but AUX pause + * and resume may run in NMI context and preempt other callbacks. Since the + * event stop callback clears ctxt->event_data before the data is released, + * AUX pause/resume will either observe a NULL pointer and stop fetching the + * path pointer, or safely access event_data and the path, as the data has + * not yet been freed. + */ +static struct coresight_path *etm_event_get_ctxt_path(struct etm_ctxt *ctxt) +{ + struct etm_event_data *event_data; + struct coresight_path *path; + + if (!ctxt) + return NULL; + + event_data = READ_ONCE(ctxt->event_data); + if (!event_data) + return NULL; + + path = etm_event_cpu_path(event_data, smp_processor_id()); + if (!path) + return NULL; + + return path; +} + static void *etm_setup_aux(struct perf_event *event, void **pages, int nr_pages, bool overwrite) { @@ -465,13 +494,23 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, goto out; }

-static int etm_event_resume(struct coresight_device *csdev, - struct etm_ctxt *ctxt) +static int etm_event_resume(struct coresight_path *path) { - if (!ctxt->event_data) + struct coresight_device *source; + int ret; + + if (!path) return 0;

- return coresight_resume_source(csdev); + source = coresight_get_source(path); + if (!source) + return 0; + + ret = coresight_resume_source(source); + if (ret < 0) + dev_err(&source->dev, "Failed to resume ETM event.\n"); + + return ret; }

static void etm_event_start(struct perf_event *event, int flags) @@ -480,23 +519,19 @@ static void etm_event_start(struct perf_event *event, int flags) struct etm_event_data *event_data; struct etm_ctxt *ctxt = this_cpu_ptr(&etm_ctxt); struct perf_output_handle *handle = &ctxt->handle; - struct coresight_device *sink, *csdev = per_cpu(csdev_src, cpu); + struct coresight_device *source, *sink; struct coresight_path *path; u64 hw_id;

- if (!csdev) - goto fail; - if (flags & PERF_EF_RESUME) { - if (etm_event_resume(csdev, ctxt) < 0) { - dev_err(&csdev->dev, "Failed to resume ETM event.\n"); + path = etm_event_get_ctxt_path(ctxt); + if (etm_event_resume(path) < 0) goto fail; - } return; }

/* Have we messed up our tracking ? */ - if (WARN_ON(ctxt->event_data)) + if (WARN_ON(READ_ONCE(ctxt->event_data))) goto fail;

/* @@ -524,9 +559,10 @@ static void etm_event_start(struct perf_event *event, int flags)

path = etm_event_cpu_path(event_data, cpu); path->handle = handle; - /* We need a sink, no need to continue without one */ + /* We need source and sink, no need to continue if any is not set */ + source = coresight_get_source(path); sink = coresight_get_sink(path); - if (WARN_ON_ONCE(!sink)) + if (WARN_ON_ONCE(!source || !sink)) goto fail_end_stop;

/* Nothing will happen without a path */ @@ -534,7 +570,7 @@ static void etm_event_start(struct perf_event *event, int flags) goto fail_end_stop;

/* Finally enable the tracer */ - if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF, path)) + if (source_ops(source)->enable(source, event, CS_MODE_PERF, path)) goto fail_disable_path;

/* @@ -558,7 +594,7 @@ static void etm_event_start(struct perf_event *event, int flags) /* Tell the perf core the event is alive */ event->hw.state = 0; /* Save the event_data for this ETM */ - ctxt->event_data = event_data; + WRITE_ONCE(ctxt->event_data, event_data); return;

fail_disable_path: @@ -578,27 +614,26 @@ static void etm_event_start(struct perf_event *event, int flags) return; }

-static void etm_event_pause(struct perf_event *event, - struct coresight_device *csdev, +static void etm_event_pause(struct coresight_path *path, + struct perf_event *event, struct etm_ctxt *ctxt) { - int cpu = smp_processor_id(); - struct coresight_device *sink; struct perf_output_handle *handle = &ctxt->handle; - struct coresight_path *path; + struct coresight_device *source, *sink; + struct etm_event_data *event_data; unsigned long size;

- if (!ctxt->event_data) + if (!path) return;

- /* Stop tracer */ - coresight_pause_source(csdev); - - path = etm_event_cpu_path(ctxt->event_data, cpu); + source = coresight_get_source(path); sink = coresight_get_sink(path); - if (WARN_ON_ONCE(!sink)) + if (WARN_ON_ONCE(!source || !sink)) return;

+ /* Stop tracer */ + coresight_pause_source(source); + /* * The per CPU sink has own interrupt handling, it might have * race condition with updating buffer on AUX trace pause if @@ -614,8 +649,9 @@ static void etm_event_pause(struct perf_event *event, if (!sink_ops(sink)->update_buffer) return;

+ event_data = READ_ONCE(ctxt->event_data); size = sink_ops(sink)->update_buffer(sink, handle, - ctxt->event_data->snk_config); + event_data->snk_config); if (READ_ONCE(handle->event)) { if (!size) return; @@ -631,14 +667,14 @@ static void etm_event_stop(struct perf_event *event, int mode) { int cpu = smp_processor_id(); unsigned long size; - struct coresight_device *sink, *csdev = per_cpu(csdev_src, cpu); + struct coresight_device *source, *sink; struct etm_ctxt *ctxt = this_cpu_ptr(&etm_ctxt); struct perf_output_handle *handle = &ctxt->handle; + struct coresight_path *path = etm_event_get_ctxt_path(ctxt); struct etm_event_data *event_data; - struct coresight_path *path;

if (mode & PERF_EF_PAUSE) - return etm_event_pause(event, csdev, ctxt); + return etm_event_pause(path, event, ctxt);

/* * If we still have access to the event_data via handle, @@ -648,9 +684,9 @@ static void etm_event_stop(struct perf_event *event, int mode) WARN_ON(perf_get_aux(handle) != ctxt->event_data)) return;

- event_data = ctxt->event_data; + event_data = READ_ONCE(ctxt->event_data); /* Clear the event_data as this ETM is stopping the trace. */ - ctxt->event_data = NULL; + WRITE_ONCE(ctxt->event_data, NULL);

if (event->hw.state == PERF_HES_STOPPED) return; @@ -672,19 +708,13 @@ static void etm_event_stop(struct perf_event *event, int mode) return; }

- if (!csdev) - return; - - path = etm_event_cpu_path(event_data, cpu); - if (!path) - return; - + source = coresight_get_source(path); sink = coresight_get_sink(path); - if (!sink) + if (!source || !sink) return;

/* stop tracer */ - coresight_disable_source(csdev, event); + coresight_disable_source(source, event);

/* tell the core */ event->hw.state = PERF_HES_STOPPED; diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 34c7e792adbd9933e48ab710b7b5894f22c72eb8..75029744561f7744225c7b866eee60e0f7cf9e10 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -248,6 +248,7 @@ void coresight_add_helper(struct coresight_device *csdev,

void coresight_set_percpu_sink(int cpu, struct coresight_device *csdev); struct coresight_device *coresight_get_percpu_sink(int cpu); +struct coresight_device *coresight_get_source(struct coresight_path *path); void coresight_disable_source(struct coresight_device *csdev, void *data); void coresight_pause_source(struct coresight_device *csdev); int coresight_resume_source(struct coresight_device *csdev);

Move the per-CPU source pointer from ETM perf to the core layer, as this will be used for not only perf session and also for CPU PM notifiers.

Provides coresight_{set|clear|get}_percpu_source() helpers to access the per-CPU source pointer. Add a raw spinlock to protect exclusive access.

Device registration and unregistration call the set and clear helpers for init and teardown the pointers.

Update callers to invoke coresight_get_percpu_source() for retrieving the pointer.

Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Reviewed-by: James Clark james.clark@linaro.org Tested-by: James Clark james.clark@linaro.org Tested-by: Jie Gan jie.gan@oss.qualcomm.com Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 38 ++++++++++++++++++++++++ drivers/hwtracing/coresight/coresight-etm-perf.c | 14 +++------ drivers/hwtracing/coresight/coresight-priv.h | 1 + 3 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 6b098c3cab0b8a710863bceb2c5f9584ad0bd8af..995a4a1bab3d983141237f453b6f630e73e69e5e 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -35,6 +35,9 @@ DEFINE_MUTEX(coresight_mutex); static DEFINE_PER_CPU(struct coresight_device *, csdev_sink);

+static DEFINE_RAW_SPINLOCK(coresight_dev_lock); +static DEFINE_PER_CPU(struct coresight_device *, csdev_source); + /** * struct coresight_node - elements of a path, from source to sink * @csdev: Address of an element. @@ -82,6 +85,39 @@ struct coresight_device *coresight_get_percpu_sink(int cpu) } EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);

+static void coresight_set_percpu_source(struct coresight_device *csdev) +{ + if (!csdev || !coresight_is_percpu_source(csdev)) + return; + + guard(raw_spinlock_irqsave)(&coresight_dev_lock); + + /* Expect no device to be set yet */ + WARN_ON(per_cpu(csdev_source, csdev->cpu)); + per_cpu(csdev_source, csdev->cpu) = csdev; +} + +static void coresight_clear_percpu_source(struct coresight_device *csdev) +{ + if (!csdev || !coresight_is_percpu_source(csdev)) + return; + + guard(raw_spinlock_irqsave)(&coresight_dev_lock); + + /* The per-CPU pointer should contain the same csdev */ + WARN_ON(per_cpu(csdev_source, csdev->cpu) != csdev); + per_cpu(csdev_source, csdev->cpu) = NULL; +} + +struct coresight_device *coresight_get_percpu_source(int cpu) +{ + if (WARN_ON(cpu < 0)) + return NULL; + + guard(raw_spinlock_irqsave)(&coresight_dev_lock); + return per_cpu(csdev_source, cpu); +} + struct coresight_device *coresight_get_source(struct coresight_path *path) { struct coresight_device *csdev; @@ -1452,6 +1488,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) if (ret) goto out_unlock;

+ coresight_set_percpu_source(csdev); mutex_unlock(&coresight_mutex);

if (cti_assoc_ops && cti_assoc_ops->add) @@ -1481,6 +1518,7 @@ void coresight_unregister(struct coresight_device *csdev) cti_assoc_ops->remove(csdev);

mutex_lock(&coresight_mutex); + coresight_clear_percpu_source(csdev); etm_perf_del_symlink_sink(csdev); coresight_remove_conns(csdev); coresight_clear_default_sink(csdev); diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index bab85f7dac4757173912639ea016cdc32e5616fb..b9e556818c3c6873ed53e5a1b8052443dd7740d0 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -49,7 +49,6 @@ struct etm_ctxt { };

static DEFINE_PER_CPU(struct etm_ctxt, etm_ctxt); -static DEFINE_PER_CPU(struct coresight_device *, csdev_src);

GEN_PMU_FORMAT_ATTR(cycacc); GEN_PMU_FORMAT_ATTR(timestamp); @@ -386,7 +385,7 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, struct coresight_path *path; struct coresight_device *csdev;

- csdev = per_cpu(csdev_src, cpu); + csdev = coresight_get_percpu_source(cpu); /* * If there is no ETM associated with this CPU clear it from * the mask and continue with the rest. If ever we try to trace @@ -864,17 +863,12 @@ int etm_perf_symlink(struct coresight_device *csdev, bool link) if (!etm_perf_up) return -EPROBE_DEFER;

- if (link) { + if (link) ret = sysfs_create_link(&pmu_dev->kobj, &cs_dev->kobj, entry); - if (ret) - return ret; - per_cpu(csdev_src, cpu) = csdev; - } else { + else sysfs_remove_link(&pmu_dev->kobj, entry); - per_cpu(csdev_src, cpu) = NULL; - }

- return 0; + return ret; } EXPORT_SYMBOL_GPL(etm_perf_symlink);

diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 75029744561f7744225c7b866eee60e0f7cf9e10..4d6f5bc1df9cc5dfd974a8b27820328e7916f169 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -249,6 +249,7 @@ void coresight_add_helper(struct coresight_device *csdev, void coresight_set_percpu_sink(int cpu, struct coresight_device *csdev); struct coresight_device *coresight_get_percpu_sink(int cpu); struct coresight_device *coresight_get_source(struct coresight_path *path); +struct coresight_device *coresight_get_percpu_source(int cpu); void coresight_disable_source(struct coresight_device *csdev, void *data); void coresight_pause_source(struct coresight_device *csdev); int coresight_resume_source(struct coresight_device *csdev);

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 device provides callbacks for saving and restoring context, these callbacks will be invoked in CPU suspend and resume.

Reviewed-by: James Clark james.clark@linaro.org Tested-by: James Clark james.clark@linaro.org Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Tested-by: Jie Gan jie.gan@oss.qualcomm.com Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 93 ++++++++++++++++++++++++++++ include/linux/coresight.h | 2 + 2 files changed, 95 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index fc4855c3dfaafe04b3bdc2dd607128353f38e984..309f3a844e8fc653771fb0bf19aeb43efc998dd7 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -6,6 +6,7 @@ #include <linux/acpi.h> #include <linux/bitfield.h> #include <linux/build_bug.h> +#include <linux/cpu_pm.h> #include <linux/kernel.h> #include <linux/init.h> #include <linux/types.h> @@ -1741,6 +1742,91 @@ static void coresight_release_device_list(void) } }

+static struct coresight_device *coresight_cpu_get_active_source(void) +{ + struct coresight_device *source; + bool is_active = false; + + source = coresight_get_percpu_source_ref(smp_processor_id()); + if (!source) + return NULL; + + if (coresight_get_mode(source) != CS_MODE_DISABLED) + is_active = true; + + coresight_put_percpu_source_ref(source); + + /* + * It is expected to run in atomic context, so it cannot be preempted + * to disable the source. Here returns the active source pointer + * without concern that its state may change. Since the build path has + * taken a reference on the component, the source can be safely used + * by the caller. + */ + return is_active ? source : NULL; +} + +static int coresight_pm_is_needed(struct coresight_device *csdev) +{ + if (!csdev) + return 0; + + /* pm_save_disable() and pm_restore_enable() must be paired */ + if (coresight_ops(csdev)->pm_save_disable && + coresight_ops(csdev)->pm_restore_enable) + return 1; + + return 0; +} + +static int coresight_pm_device_save(struct coresight_device *csdev) +{ + return coresight_ops(csdev)->pm_save_disable(csdev); +} + +static void coresight_pm_device_restore(struct coresight_device *csdev) +{ + coresight_ops(csdev)->pm_restore_enable(csdev); +} + +static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, + void *v) +{ + struct coresight_device *csdev = coresight_cpu_get_active_source(); + + if (!coresight_pm_is_needed(csdev)) + return NOTIFY_DONE; + + switch (cmd) { + case CPU_PM_ENTER: + if (coresight_pm_device_save(csdev)) + return NOTIFY_BAD; + break; + case CPU_PM_EXIT: + case CPU_PM_ENTER_FAILED: + coresight_pm_device_restore(csdev); + 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", }; @@ -1795,9 +1881,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: @@ -1809,6 +1901,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 e9c20ceb9016fa3db256b8c1147c1fd2027b7b0d..5f9d7ea9f5941ab01eb6a084ca558a9417c7727f 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -438,6 +438,8 @@ struct coresight_ops_panic { struct coresight_ops { int (*trace_id)(struct coresight_device *csdev, enum cs_mode mode, struct coresight_device *sink); + int (*pm_save_disable)(struct coresight_device *csdev); + void (*pm_restore_enable)(struct coresight_device *csdev); const struct coresight_ops_sink *sink_ops; const struct coresight_ops_link *link_ops; const struct coresight_ops_source *source_ops;

ETMv4 driver save/restore callbacks still re-check conditions that are already validated by the core layer before the callbacks are invoked.

Remove the duplicated checks and fold the two-level functions into direct callback implementations. The obsolete WARN_ON() checks are removed as well.

Reviewed-by: Mike Leach mike.leach@linaro.org Tested-by: James Clark james.clark@linaro.org Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Reviewed-by: James Clark james.clark@linaro.org Tested-by: Jie Gan jie.gan@oss.qualcomm.com Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-etm4x-core.c | 41 +++------------------- 1 file changed, 4 insertions(+), 37 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 5cd70f0f3710a5ec6406a89c034d30230f782b13..fef1270439e9d770fc0459e9ab8cab7cb40827ee 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1853,17 +1853,14 @@ static inline bool etm4_pm_save_needed(struct etmv4_drvdata *drvdata) return !!drvdata->save_state; }

-static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) +static int etm4_cpu_save(struct coresight_device *csdev) { int i, ret = 0; + struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); struct etmv4_save_state *state; - struct coresight_device *csdev = drvdata->csdev; struct csdev_access *csa; struct device *etm_dev;

- if (WARN_ON(!csdev)) - return -ENODEV; - etm_dev = &csdev->dev; csa = &csdev->access;

@@ -1995,32 +1992,13 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) return ret; }

-static int etm4_cpu_save(struct coresight_device *csdev) -{ - struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); - int ret = 0; - - if (!etm4_pm_save_needed(drvdata)) - return 0; - - /* - * Save and restore the ETM Trace registers only if - * the ETM is active. - */ - if (coresight_get_mode(drvdata->csdev)) - ret = __etm4_cpu_save(drvdata); - return ret; -} - -static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata) +static void etm4_cpu_restore(struct coresight_device *csdev) { int i; + struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); 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);

@@ -2113,17 +2091,6 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata) etm4_cs_lock(drvdata, csa); }

-static void etm4_cpu_restore(struct coresight_device *csdev) -{ - struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); - - if (!etm4_pm_save_needed(drvdata)) - return; - - if (coresight_get_mode(drvdata->csdev)) - __etm4_cpu_restore(drvdata); -} - static const struct coresight_ops etm4_cs_ops = { .trace_id = coresight_etm_get_trace_id, .source_ops = &etm4_source_ops,

coresight_enable_path() enables helpers attached to every device in the path, including those bound to the source. However, coresight_disable_path() skips the source node, so source helpers had to be disabled separately in coresight_disable_source().

Move source helper disabling into coresight_disable_path() instead. Make coresight_disable_path_from() start from the passed node nd, so it can also disable helpers on the source. Update the comments accordingly.

As coresight_disable_path_from() now changes its semantics from "start beyond nd" to "start from nd", update the failure handling in coresight_enable_path(). If enabling a node fails, iterate to the previous node (the last successfully enabled one) and pass it to coresight_disable_path_from() for rollback.

Tested-by: Jie Gan jie.gan@oss.qualcomm.com Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Reviewed-by: James Clark james.clark@linaro.org Tested-by: James Clark james.clark@linaro.org Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 309f3a844e8fc653771fb0bf19aeb43efc998dd7..d65cdce1dfef9bb1df40eb7df67274d2f6b30750 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -456,19 +456,12 @@ static void coresight_disable_helpers(struct coresight_device *csdev, }

/* - * Helper function to call source_ops(csdev)->disable and also disable the - * helpers. - * - * There is an imbalance between coresight_enable_path() and - * coresight_disable_path(). Enabling also enables the source's helpers as part - * of the path, but disabling always skips the first item in the path (which is - * the source), so sources and their helpers don't get disabled as part of that - * function and we need the extra step here. + * coresight_disable_source() only disables the source, but do nothing for + * the associated helpers, which are controlled as part of the path. */ void coresight_disable_source(struct coresight_device *csdev, void *data) { source_ops(csdev)->disable(csdev, data); - coresight_disable_helpers(csdev, NULL); } EXPORT_SYMBOL_GPL(coresight_disable_source);

@@ -495,9 +488,9 @@ int coresight_resume_source(struct coresight_device *csdev) EXPORT_SYMBOL_GPL(coresight_resume_source);

/* - * 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 - * disabled. + * coresight_disable_path_from : Disable components in the given path starting + * from @nd in the list. If @nd is NULL, all the components, except the SOURCE + * are disabled. */ static void coresight_disable_path_from(struct coresight_path *path, struct coresight_node *nd) @@ -508,7 +501,7 @@ static void coresight_disable_path_from(struct coresight_path *path, if (!nd) nd = list_first_entry(&path->path_list, struct coresight_node, link);

- list_for_each_entry_continue(nd, &path->path_list, link) { + list_for_each_entry_from(nd, &path->path_list, link) { csdev = nd->csdev; type = csdev->type;

@@ -528,12 +521,6 @@ static void coresight_disable_path_from(struct coresight_path *path, coresight_disable_sink(csdev); break; case CORESIGHT_DEV_TYPE_SOURCE: - /* - * We skip the first node in the path assuming that it - * is the source. So we don't expect a source device in - * the middle of a path. - */ - WARN_ON(1); break; case CORESIGHT_DEV_TYPE_LINK: parent = list_prev_entry(nd, link)->csdev; @@ -648,6 +635,8 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode) err_disable_helpers: coresight_disable_helpers(csdev, path); err_disable_path: + /* Fetch the previous node, the last successfully enabled one */ + nd = list_next_entry(nd, link); coresight_disable_path_from(path, nd); goto out; }

Replace open code with coresight_path_first_node() and coresight_path_last_node() for fetching the nodes.

Check that the node is not NULL before accessing csdev field.

Tested-by: Jie Gan jie.gan@oss.qualcomm.com Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Reviewed-by: James Clark james.clark@linaro.org Tested-by: James Clark james.clark@linaro.org Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 62a12a90988949ccc8c181c8edde7939e4f986ab..61ae7b6c7a836caafaf87787af91a1f5326c16ec 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -174,11 +174,16 @@ void coresight_put_percpu_source_ref(struct coresight_device *csdev) struct coresight_device *coresight_get_source(struct coresight_path *path) { struct coresight_device *csdev; + struct coresight_node *nd;

if (!path) return NULL;

- csdev = list_first_entry(&path->path_list, struct coresight_node, link)->csdev; + nd = coresight_path_first_node(path); + if (!nd) + return NULL; + + csdev = nd->csdev; if (!coresight_is_device_source(csdev)) return NULL;

@@ -711,11 +716,16 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode) struct coresight_device *coresight_get_sink(struct coresight_path *path) { struct coresight_device *csdev; + struct coresight_node *nd;

if (!path) return NULL;

- csdev = list_last_entry(&path->path_list, struct coresight_node, link)->csdev; + nd = coresight_path_last_node(path); + if (!nd) + return NULL; + + csdev = nd->csdev; if (csdev->type != CORESIGHT_DEV_TYPE_SINK && csdev->type != CORESIGHT_DEV_TYPE_LINKSINK) return NULL;

This commit only set the path pointer for system tracers (e.g. STM) in coresight_{enable|disable}_source().

Later changes will set the path pointer locally for per-CPU sources. This is because the mode and path pointer must be set together, so that they are observed atomically by the CPU PM notifier.

Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Reviewed-by: James Clark james.clark@linaro.org Tested-by: James Clark james.clark@linaro.org Tested-by: Jie Gan jie.gan@oss.qualcomm.com Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 23 ++++++++++++++++++++++- include/linux/coresight.h | 2 ++ 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index f833b73ebc168b2b039e5f0900594d0bde5ff916..7f6febd20faa64bc99336b368d0193221706fc2d 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -487,10 +487,28 @@ int coresight_enable_source(struct coresight_device *csdev, struct perf_event *event, enum cs_mode mode, struct coresight_path *path) { + int ret; + if (!coresight_is_device_source(csdev)) return -EINVAL;

- return source_ops(csdev)->enable(csdev, event, mode, path); + ret = source_ops(csdev)->enable(csdev, event, mode, path); + if (ret) + return ret; + + /* + * Update the path pointer until after the source is enabled to avoid + * races where multiple paths attempt to enable the same source. + * + * Do not set the path pointer here for per-CPU sources; set it locally + * on the CPU instead. Otherwise, there is a window where the path is + * enabled but the pointer is not yet set, causing CPU PM notifiers to + * miss PM operations due to reading a NULL pointer. + */ + if (!coresight_is_percpu_source(csdev)) + csdev->path = path; + + return 0; }

void coresight_disable_source(struct coresight_device *csdev, void *data) @@ -498,6 +516,9 @@ void coresight_disable_source(struct coresight_device *csdev, void *data) if (!coresight_is_device_source(csdev)) return;

+ if (!coresight_is_percpu_source(csdev)) + csdev->path = NULL; + source_ops(csdev)->disable(csdev, data); } EXPORT_SYMBOL_GPL(coresight_disable_source); diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 5f9d7ea9f5941ab01eb6a084ca558a9417c7727f..58d474b269806d32cad6ed87da96550b06f1f30f 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -257,6 +257,7 @@ struct coresight_trace_id_map { * by @coresight_ops. * @access: Device i/o access abstraction for this device. * @dev: The device entity associated to this component. + * @path: Activated path pointer (only used for source device). * @mode: The device mode, i.e sysFS, Perf or disabled. This is actually * an 'enum cs_mode' but stored in an atomic type. Access is always * through atomic APIs, ensuring SMP-safe synchronisation between @@ -291,6 +292,7 @@ struct coresight_device { const struct coresight_ops *ops; struct csdev_access access; struct device dev; + struct coresight_path *path; atomic_t mode; int refcnt; int cpu;

Set the path pointer on the target CPU during ETM enable and disable.

This ensures the device mode and path pointer are updated together and observed atomically by the CPU PM notifier.

Tested-by: James Clark james.clark@linaro.org Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Reviewed-by: James Clark james.clark@linaro.org Tested-by: Jie Gan jie.gan@oss.qualcomm.com Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-etm3x-core.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c index aeeb284abdbe4b6a0960da45baa1138e203f3e3c..c6fe8b25b855a4119110fee4162f55c0154c3d05 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -441,6 +441,7 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)

struct etm_enable_arg { struct etm_drvdata *drvdata; + struct coresight_path *path; int rc; };

@@ -462,8 +463,12 @@ static void etm_enable_sysfs_smp_call(void *info) arg->rc = etm_enable_hw(arg->drvdata);

/* The tracer didn't start */ - if (arg->rc) + if (arg->rc) { coresight_set_mode(csdev, CS_MODE_DISABLED); + return; + } + + csdev->path = arg->path; }

void etm_release_trace_id(struct etm_drvdata *drvdata) @@ -492,10 +497,13 @@ static int etm_enable_perf(struct coresight_device *csdev, ret = etm_enable_hw(drvdata);

/* Failed to start tracer; roll back to DISABLED mode */ - if (ret) + if (ret) { coresight_set_mode(csdev, CS_MODE_DISABLED); + return ret; + }

- return ret; + csdev->path = path; + return 0; }

static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_path *path) @@ -514,6 +522,7 @@ static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_pat */ if (cpu_online(drvdata->cpu)) { arg.drvdata = drvdata; + arg.path = path; ret = smp_call_function_single(drvdata->cpu, etm_enable_sysfs_smp_call, &arg, 1); if (!ret) @@ -583,6 +592,7 @@ static void etm_disable_sysfs_smp_call(void *info)

etm_disable_hw(drvdata);

+ drvdata->csdev->path = NULL; coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED); }

@@ -607,6 +617,7 @@ static void etm_disable_perf(struct coresight_device *csdev)

CS_LOCK(drvdata->csa.base);

+ drvdata->csdev->path = NULL; coresight_set_mode(drvdata->csdev, CS_MODE_DISABLED);

/*

Extend the CPU PM flow to control the path: disable from source up to the node before the sink, then re-enable the same range on restore. To avoid latency, control it up to the node before the sink.

Track per-CPU PM restore failures using percpu_pm_failed. Once a CPU hits a restore failure, set the percpu_pm_failed and return NOTIFY_BAD on subsequent notifications to avoid repeating half-completed transitions.

Setting percpu_pm_failed permanently blocks CPU PM on that CPU. Such failures are typically seen during development; disabling PM operations simplifies the implementation, and a warning highlights the issue. Clear this flag when the source device is unregistered.

Reviewed-by: James Clark james.clark@linaro.org Tested-by: Jie Gan jie.gan@oss.qualcomm.com Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Tested-by: James Clark james.clark@linaro.org Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 93 +++++++++++++++++++++++----- 1 file changed, 78 insertions(+), 15 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 7f6febd20faa64bc99336b368d0193221706fc2d..fc60b72a4abff9f217d48165bdb3f38a621906e7 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -38,6 +38,7 @@ static DEFINE_PER_CPU(struct coresight_device *, csdev_sink);

static DEFINE_RAW_SPINLOCK(coresight_dev_lock); static DEFINE_PER_CPU(struct coresight_device *, csdev_source); +static DEFINE_PER_CPU(bool, percpu_pm_failed);

/** * struct coresight_node - elements of a path, from source to sink @@ -121,6 +122,9 @@ static void coresight_clear_percpu_source(struct coresight_device *csdev) if (!csdev || !coresight_is_percpu_source(csdev)) return;

+ /* Clear percpu_pm_failed */ + per_cpu(percpu_pm_failed, csdev->cpu) = false; + guard(raw_spinlock_irqsave)(&coresight_dev_lock);

/* The per-CPU pointer should contain the same csdev */ @@ -1843,7 +1847,7 @@ static void coresight_release_device_list(void) } }

-static struct coresight_device *coresight_cpu_get_active_source(void) +static struct coresight_path *coresight_cpu_get_active_path(void) { struct coresight_device *source; bool is_active = false; @@ -1859,22 +1863,32 @@ static struct coresight_device *coresight_cpu_get_active_source(void)

/* * It is expected to run in atomic context, so it cannot be preempted - * to disable the source. Here returns the active source pointer - * without concern that its state may change. Since the build path has - * taken a reference on the component, the source can be safely used - * by the caller. + * to disable the path. Here returns the active path pointer without + * concern that its state may change. Since the build path has taken + * a reference on the component, the path can be safely used by the + * caller. */ - return is_active ? source : NULL; + return is_active ? source->path : NULL; }

-static int coresight_pm_is_needed(struct coresight_device *csdev) +/* Return: 1 if PM is required, 0 if skip, or a negative error */ +static int coresight_pm_is_needed(struct coresight_path *path) { - if (!csdev) + struct coresight_device *source; + + if (this_cpu_read(percpu_pm_failed)) + return -EIO; + + if (!path) + return 0; + + source = coresight_get_source(path); + if (!source) return 0;

/* pm_save_disable() and pm_restore_enable() must be paired */ - if (coresight_ops(csdev)->pm_save_disable && - coresight_ops(csdev)->pm_restore_enable) + if (coresight_ops(source)->pm_save_disable && + coresight_ops(source)->pm_restore_enable) return 1;

return 0; @@ -1890,22 +1904,71 @@ static void coresight_pm_device_restore(struct coresight_device *csdev) coresight_ops(csdev)->pm_restore_enable(csdev); }

+static int coresight_pm_save(struct coresight_path *path) +{ + struct coresight_device *source = coresight_get_source(path); + struct coresight_node *from, *to; + int ret; + + ret = coresight_pm_device_save(source); + if (ret) + return ret; + + from = coresight_path_first_node(path); + /* Disable up to the node before sink */ + to = list_prev_entry(coresight_path_last_node(path), link); + coresight_disable_path_from_to(path, from, to); + + return 0; +} + +static void coresight_pm_restore(struct coresight_path *path) +{ + struct coresight_device *source = coresight_get_source(path); + struct coresight_node *from, *to; + int ret; + + from = coresight_path_first_node(path); + /* Enable up to the node before sink */ + to = list_prev_entry(coresight_path_last_node(path), link); + ret = coresight_enable_path_from_to(path, coresight_get_mode(source), + from, to); + if (ret) + goto path_failed; + + coresight_pm_device_restore(source); + return; + +path_failed: + pr_err("Failed in coresight PM restore on CPU%d: %d\n", + smp_processor_id(), ret); + + /* + * Once PM fails on a CPU, set percpu_pm_failed and leave it set until + * reboot. This prevents repeated partial transitions during idle + * entry and exit. + */ + this_cpu_write(percpu_pm_failed, true); +} + static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, void *v) { - struct coresight_device *csdev = coresight_cpu_get_active_source(); + struct coresight_path *path = coresight_cpu_get_active_path(); + int ret;

- if (!coresight_pm_is_needed(csdev)) - return NOTIFY_DONE; + ret = coresight_pm_is_needed(path); + if (ret <= 0) + return ret ? NOTIFY_BAD : NOTIFY_DONE;

switch (cmd) { case CPU_PM_ENTER: - if (coresight_pm_device_save(csdev)) + if (coresight_pm_save(path)) return NOTIFY_BAD; break; case CPU_PM_EXIT: case CPU_PM_ENTER_FAILED: - coresight_pm_device_restore(csdev); + coresight_pm_restore(path); break; default: return NOTIFY_DONE;

From: Yabin Cui yabinc@google.com

TRBE context can be lost when a CPU enters low power states. If a trace source is restored while TRBE is not, tracing may run without an active sink, which can lead to hangs on some devices (e.g., Pixel 9).

The save and restore flows are described in the section K5.5 "Context switching" of Arm ARM (ARM DDI 0487 L.a). This commit adds save and restore callbacks with following the software usages defined in the architecture manual.

During the restore flow, since TRBLIMITR_EL1.E resets to 0 on a warm reset, the trace buffer unit is disabled when idle resume, it is safe to restore base/pointer/status registers first and program TRBLIMITR_EL1 last.

Signed-off-by: Yabin Cui yabinc@google.com Tested-by: James Clark james.clark@linaro.org Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Reviewed-by: James Clark james.clark@linaro.org Co-developed-by: Leo Yan leo.yan@arm.com Tested-by: Jie Gan jie.gan@oss.qualcomm.com Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 59 +++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 14e35b9660d76e47619cc6026b94929b3bb3e02b..c7cbca45f2debd4047b93283ea9fe5dd9e1f2ebf 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -116,6 +116,20 @@ static int trbe_errata_cpucaps[] = { */ #define TRBE_WORKAROUND_OVERWRITE_FILL_MODE_SKIP_BYTES 256

+/* + * struct trbe_save_state: Register values representing TRBE state + * @trblimitr - Trace Buffer Limit Address Register value + * @trbbaser - Trace Buffer Base Register value + * @trbptr - Trace Buffer Write Pointer Register value + * @trbsr - Trace Buffer Status Register value + */ +struct trbe_save_state { + u64 trblimitr; + u64 trbbaser; + u64 trbptr; + u64 trbsr; +}; + /* * struct trbe_cpudata: TRBE instance specific data * @trbe_flag - TRBE dirty/access flag support @@ -134,6 +148,7 @@ struct trbe_cpudata { enum cs_mode mode; struct trbe_buf *buf; struct trbe_drvdata *drvdata; + struct trbe_save_state save_state; DECLARE_BITMAP(errata, TRBE_ERRATA_MAX); };

@@ -1189,6 +1204,46 @@ static irqreturn_t arm_trbe_irq_handler(int irq, void *dev) return IRQ_HANDLED; }

+static int arm_trbe_save(struct coresight_device *csdev) +{ + struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev); + struct trbe_save_state *state = &cpudata->save_state; + + state->trblimitr = read_sysreg_s(SYS_TRBLIMITR_EL1); + + /* Disable the unit, ensure the writes to memory are complete */ + if (state->trblimitr & TRBLIMITR_EL1_E) + trbe_drain_and_disable_local(cpudata); + + state->trbbaser = read_sysreg_s(SYS_TRBBASER_EL1); + state->trbptr = read_sysreg_s(SYS_TRBPTR_EL1); + state->trbsr = read_sysreg_s(SYS_TRBSR_EL1); + return 0; +} + +static void arm_trbe_restore(struct coresight_device *csdev) +{ + struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev); + struct trbe_save_state *state = &cpudata->save_state; + + write_sysreg_s(state->trbbaser, SYS_TRBBASER_EL1); + write_sysreg_s(state->trbptr, SYS_TRBPTR_EL1); + write_sysreg_s(state->trbsr, SYS_TRBSR_EL1); + + if (!(state->trblimitr & TRBLIMITR_EL1_E)) { + write_sysreg_s(state->trblimitr, SYS_TRBLIMITR_EL1); + } else { + /* + * The section K5.5 Context switching, Arm ARM (ARM DDI 0487 + * L.a), S_PKLXF requires a Context synchronization event to + * guarantee the Trace Buffer Unit will observe the new values + * of the system registers. + */ + isb(); + set_trbe_enabled(cpudata, state->trblimitr); + } +} + static const struct coresight_ops_sink arm_trbe_sink_ops = { .enable = arm_trbe_enable, .disable = arm_trbe_disable, @@ -1198,7 +1253,9 @@ static const struct coresight_ops_sink arm_trbe_sink_ops = { };

static const struct coresight_ops arm_trbe_cs_ops = { - .sink_ops = &arm_trbe_sink_ops, + .pm_save_disable = arm_trbe_save, + .pm_restore_enable = arm_trbe_restore, + .sink_ops = &arm_trbe_sink_ops, };

static ssize_t align_show(struct device *dev, struct device_attribute *attr, char *buf)

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.

Add a 'mode' argument to coresight_pm_get_active_path() so it only returns active paths for the relevant mode. Define the enum with bit flags so it is safe for bitwise operations.

Change CPUHP_AP_ARM_CORESIGHT_STARTING to CPUHP_AP_ARM_CORESIGHT_ONLINE so that the CPU hotplug callback runs in the online state and thread context, allowing coresight_disable_sysfs() to be called directly to disable the path.

Tested-by: James Clark james.clark@linaro.org Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Reviewed-by: James Clark james.clark@linaro.org Tested-by: Jie Gan jie.gan@oss.qualcomm.com Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 48 ++++++++++++++++++---- drivers/hwtracing/coresight/coresight-etm3x-core.c | 40 ------------------ drivers/hwtracing/coresight/coresight-etm4x-core.c | 37 ----------------- include/linux/coresight.h | 6 +-- include/linux/cpuhotplug.h | 2 +- 5 files changed, 43 insertions(+), 90 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index e3de4b388372a4600cfa349452bffacfa0c755a7..f7b1308a759c3347c305e2618b4d4b471c496a76 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1847,7 +1847,7 @@ static void coresight_release_device_list(void) } }

-static struct coresight_path *coresight_cpu_get_active_path(void) +static struct coresight_path *coresight_cpu_get_active_path(enum cs_mode mode) { struct coresight_device *source; bool is_active = false; @@ -1856,17 +1856,17 @@ static struct coresight_path *coresight_cpu_get_active_path(void) if (!source) return NULL;

- if (coresight_get_mode(source) != CS_MODE_DISABLED) + if (coresight_get_mode(source) & mode) is_active = true;

coresight_put_percpu_source_ref(source);

/* - * It is expected to run in atomic context, so it cannot be preempted - * to disable the path. Here returns the active path pointer without - * concern that its state may change. Since the build path has taken - * a reference on the component, the path can be safely used by the - * caller. + * It is expected to run in atomic context or with the CPU lock held for + * sysfs mode, so it cannot be preempted to disable the path. Here + * returns the active path pointer without concern that its state may + * change. Since the build path has taken a reference on the component, + * the path can be safely used by the caller. */ return is_active ? source->path : NULL; } @@ -1985,7 +1985,8 @@ static void coresight_pm_restore(struct coresight_path *path) static int coresight_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, void *v) { - struct coresight_path *path = coresight_cpu_get_active_path(); + struct coresight_path *path = + coresight_cpu_get_active_path(CS_MODE_SYSFS | CS_MODE_PERF); int ret;

ret = coresight_pm_is_needed(path); @@ -2012,13 +2013,42 @@ static struct notifier_block coresight_cpu_pm_nb = { .notifier_call = coresight_cpu_pm_notify, };

+static int coresight_dying_cpu(unsigned int cpu) +{ + struct coresight_path *path; + + /* + * The perf event layer will disable PMU events in the CPU + * hotplug. Here only handles SYSFS case. + */ + path = coresight_cpu_get_active_path(CS_MODE_SYSFS); + if (!path) + return 0; + + coresight_disable_sysfs(coresight_get_source(path)); + return 0; +} + 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_ONLINE, + "arm/coresight-core:dying", + NULL, 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_ONLINE); cpu_pm_unregister_notifier(&coresight_cpu_pm_nb); }

diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c index c6fe8b25b855a4119110fee4162f55c0154c3d05..862ad0786699c41433eae8683406b3c1340a6cb6 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -699,35 +699,6 @@ static int etm_online_cpu(unsigned int cpu) return 0; }

-static int etm_starting_cpu(unsigned int cpu) -{ - if (!etmdrvdata[cpu]) - return 0; - - spin_lock(&etmdrvdata[cpu]->spinlock); - if (!etmdrvdata[cpu]->os_unlock) { - etm_os_unlock(etmdrvdata[cpu]); - etmdrvdata[cpu]->os_unlock = true; - } - - if (coresight_get_mode(etmdrvdata[cpu]->csdev)) - etm_enable_hw(etmdrvdata[cpu]); - spin_unlock(&etmdrvdata[cpu]->spinlock); - return 0; -} - -static int etm_dying_cpu(unsigned int cpu) -{ - if (!etmdrvdata[cpu]) - return 0; - - spin_lock(&etmdrvdata[cpu]->spinlock); - if (coresight_get_mode(etmdrvdata[cpu]->csdev)) - etm_disable_hw(etmdrvdata[cpu]); - spin_unlock(&etmdrvdata[cpu]->spinlock); - return 0; -} - static bool etm_arch_supported(u8 arch) { switch (arch) { @@ -795,13 +766,6 @@ static int __init etm_hp_setup(void) { int ret;

- ret = cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING, - "arm/coresight:starting", - etm_starting_cpu, etm_dying_cpu); - - if (ret) - return ret; - ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "arm/coresight:online", etm_online_cpu, NULL); @@ -812,15 +776,11 @@ static int __init etm_hp_setup(void) return 0; }

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

static void etm_hp_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/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 343ba9ce946a7ea3776c06d43364cdce823e2c80..14bb31bd6a0b979051dd17963218c00165a0ebb8 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1833,33 +1833,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]); - 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 inline bool etm4_pm_save_needed(struct etmv4_drvdata *drvdata) { return !!drvdata->save_state; @@ -2120,13 +2093,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); @@ -2137,14 +2103,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 76ef4c0965125cd11830df0151a6707d3e3b638d..add0579cad884c62b8c8e5ff82264966ff0613b7 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -344,9 +344,9 @@ struct coresight_path { };

enum cs_mode { - CS_MODE_DISABLED, - CS_MODE_SYSFS, - CS_MODE_PERF, + CS_MODE_DISABLED = 0, + CS_MODE_SYSFS = BIT(0), + CS_MODE_PERF = BIT(1), };

#define coresight_ops(csdev) csdev->ops diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h index 22ba327ec2278c132572950848ade2b814787eb5..0fb3a2a62eb001bcc813422eba2ce8fbf92c260a 100644 --- a/include/linux/cpuhotplug.h +++ b/include/linux/cpuhotplug.h @@ -180,7 +180,6 @@ enum cpuhp_state { CPUHP_AP_DUMMY_TIMER_STARTING, CPUHP_AP_ARM_XEN_STARTING, CPUHP_AP_ARM_XEN_RUNSTATE_STARTING, - CPUHP_AP_ARM_CORESIGHT_STARTING, CPUHP_AP_ARM_CORESIGHT_CTI_STARTING, CPUHP_AP_ARM64_ISNDEP_STARTING, CPUHP_AP_SMPCFD_DYING, @@ -200,6 +199,7 @@ enum cpuhp_state { CPUHP_AP_IRQ_AFFINITY_ONLINE, CPUHP_AP_BLK_MQ_ONLINE, CPUHP_AP_ARM_MVEBU_SYNC_CLOCKS, + CPUHP_AP_ARM_CORESIGHT_ONLINE, CPUHP_AP_X86_INTEL_EPB_ONLINE, CPUHP_AP_PERF_ONLINE, CPUHP_AP_PERF_X86_ONLINE,