Hi Leo,
On Tue, 11 Mar 2025 at 17:05, Leo Yan <leo.yan@xxxxxxx> wrote:
>
> This commit supports AUX trace pause and resume in a perf session for
> Arm CoreSight.
>
> First, we need to decide which flag can indicate the CoreSight PMU event
> has started. The 'event->hw.state' cannot be used for this purpose
> because its initial value and the value after hardware trace enabling
> are both 0.
>
> On the other hand, the context value 'ctxt->event_data' stores the ETM
> private info. This pointer is valid only when the PMU event has been
> enabled. It is safe to permit AUX trace pause and resume operations only
> when it is not a NULL pointer.
>
> To achieve fine-grained control of the pause and resume, only the tracer
> is disabled and enabled. This avoids the unnecessary complexity and
> latency caused by manipulating the entire link path.
>
> Signed-off-by: Leo Yan <leo.yan@xxxxxxx>
> ---
> .../hwtracing/coresight/coresight-etm-perf.c | 45 ++++++++++++++++++-
> 1 file changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index f4cccd68e625..2dcf1809cb7f 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -365,6 +365,18 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
> continue;
> }
>
> + /*
> + * If AUX pause feature is enabled but the ETM driver does not
> + * support the operations, clear this CPU from the mask and
> + * continue to next one.
> + */
> + if (event->attr.aux_start_paused &&
> + (!source_ops(csdev)->pause_perf || !source_ops(csdev)->resume_perf)) {
> + dev_err_once(&csdev->dev, "AUX pause is not supported.\n");
> + cpumask_clear_cpu(cpu, mask);
> + continue;
> + }
> +
> /*
> * No sink provided - look for a default sink for all the ETMs,
> * where this event can be scheduled.
> @@ -450,6 +462,15 @@ 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)
> +{
> + if (!ctxt->event_data)
> + return 0;
> +
> + return coresight_resume_source(csdev);
> +}
> +
> static void etm_event_start(struct perf_event *event, int flags)
> {
> int cpu = smp_processor_id();
> @@ -463,6 +484,14 @@ static void etm_event_start(struct perf_event *event, int flags)
> if (!csdev)
> goto fail;
>
Is it possible here that the first call to etm_event_start() also has
the PERF_EF_RESUME flag set?
If so it looks like we need to fall through and do a "normal" start to
get all the ctxt->event_data set up.
> + if (flags & PERF_EF_RESUME) {
> + if (etm_event_resume(csdev, ctxt) < 0) {
> + dev_err(&csdev->dev, "Failed to resume ETM event.\n");
> + goto fail;
> + }
> + return;
> + }
> +
> /* Have we messed up our tracking ? */
> if (WARN_ON(ctxt->event_data))
> goto fail;
> @@ -545,6 +574,16 @@ static void etm_event_start(struct perf_event *event, int flags)
> return;
> }
>
> +static void etm_event_pause(struct coresight_device *csdev,
> + struct etm_ctxt *ctxt)
> +{
> + if (!ctxt->event_data)
> + return;
> +
> + /* Stop tracer */
> + coresight_pause_source(csdev);
> +}
> +
> static void etm_event_stop(struct perf_event *event, int mode)
> {
> int cpu = smp_processor_id();
> @@ -555,6 +594,9 @@ static void etm_event_stop(struct perf_event *event, int mode)
> struct etm_event_data *event_data;
> struct coresight_path *path;
>
> + if (mode & PERF_EF_PAUSE)
> + return etm_event_pause(csdev, ctxt);
> +
> /*
> * If we still have access to the event_data via handle,
> * confirm that we haven't messed up the tracking.
> @@ -899,7 +941,8 @@ int __init etm_perf_init(void)
> int ret;
>
> etm_pmu.capabilities = (PERF_PMU_CAP_EXCLUSIVE |
> - PERF_PMU_CAP_ITRACE);
> + PERF_PMU_CAP_ITRACE |
> + PERF_PMU_CAP_AUX_PAUSE);
>
> etm_pmu.attr_groups = etm_pmu_attr_groups;
> etm_pmu.task_ctx_nr = perf_sw_context;
> --
> 2.34.1
>
If the possible issue above is prevented by perf internals
Reviewed-by: Mike Leach <mike.leach@xxxxxxxxxx>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
