linux-kernel - Re: [PATCH v3 4/6] coresight: perf: Support AUX trace pause and resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJ9a7ViKeqNp_c0+w2mkdujLvzs0UA=Xbm5bG7K-kA86AjJ=eA@mail.gmail.com>
Date: Wed, 2 Apr 2025 09:45:20 +0100
From: Mike Leach <mike.leach@...aro.org>
To: Leo Yan <leo.yan@....com>
Cc: Suzuki K Poulose <suzuki.poulose@....com>, James Clark <james.clark@...aro.org>, 
	Jonathan Corbet <corbet@....net>, Alexander Shishkin <alexander.shishkin@...ux.intel.com>, 
	coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org, 
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 4/6] coresight: perf: Support AUX trace pause and resume

Hi Leo

On Tue, 1 Apr 2025 at 16:00, Leo Yan <leo.yan@....com> wrote:
>
> 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);
>

When I looked at the vague comments in the perf source - it seemed to
imply that ->start() calls could overlap - so the associated event
that resumes trace could occur at the same time as the initialising
start from paused for the trace operations.

If we are guaranteed this cannot happen then we are good to go!

> Note: for the first call, the tracer should be disabled if
> 'event->hw.aux_paused' is 1.  This is ensured by patch 03.
>

Yes - I saw that.

Mike

> 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@...aro.org>
> >
> >
> > --
> > Mike Leach
> > Principal Engineer, ARM Ltd.
> > Manchester Design Centre. UK



-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK