Hi Mike,
On Tue, Apr 01, 2025 at 01:50:52PM +0100, Mike Leach wrote:
[...]
> > 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?
The first call has a flow below, using flag 0 but not PERF_EF_RESUME.
etm_event_add()
`> etm_event_start(event, 0);
Note: for the first call, the tracer should be disabled if
'event->hw.aux_paused' is 1. This is ensured by patch 03.
Thanks,
Leo
> 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
