Re: [PATCH v20 11/11] perf: arm_pmuv3: Add support for the Branch Record Buffer Extension (BRBE)

[Leo Yan <leo.yan@arm.com>]

On Tue, Feb 25, 2025 at 12:01:52PM +0000, Mark Rutland wrote:

[...]

> > > Critically, the brbe_enable() function merges the filters of all
> > > *active* events which have been installed into hardware. It does not
> > > track all events which can be rotated, and the resulting filter is not
> > > the same -- it can change as a result of rotation.
> > 
> > In a perf session has multiple events, and events have different branch
> > filters, seems to me, a simple way is to return error for this case.
> 
> FWIW, I'd generally prefer to do that since it avoids a number of
> horrible edge-cases and gets rid of the need to do SW filtering, which
> falls somewhere between "tricky" and "not entirely possible". However,
> that's not what LBR and others do, which is why we went with filter
> merging.
> 
> If folk on the tools side are happy with the kernel rejecting
> conflicting events, then I'd be more than happy to do that. What I don't
> want is that we start off with that approach and people immediately
> start to complain that the BRBE driver rejects events that the LBR
> driver accepts.
> 
> See the last time this came up.

Thanks for the shared links.  Based on the info, let's say we can have two
cases:

  Case 1: set different branch filters in a single perf session:

    perf record -e armv8_pmuv3_0/r03,branch_type=any_call/u \
                -e armv8_pmuv3_0/r04,branch_type=any_ret/k ...

  Case 2: set different branch filters in multiple perf sessions:

    perf record -e armv8_pmuv3_0/r03,branch_type=any_call/u ...

    perf record -e armv8_pmuv3_0/r04,branch_type=any_ret/k ...

In my previous reply, I was suggesting that we should reject the case 1.
IMO, it is not quite useful to configure different filters for events in
the same session, especially if this leads complexity in the driver due
to the hardware limitation.

For case 2, when create a new session, if the perf tool can read out the
current branch filter setting (e.g. via sysfs node) and give suggestion
what branch filter is compabile with existed sessions, seems to me, this
is a feasible solution.  My understanding this is a rare case, and a
clear guidance for users would be sufficient if this happens.  (Maybe
we can give recommendation for how to use BRBE in the perf doc).

To be clear, an important factor is the trace modes with modifier 'u'
(user) and 'k' (kernel) should be supported for different events and for
different sessions.  In a mixed cases (some events are userspace only
and some are kernel only), the BRBE driver needs to filter out branch
records for specific mode when taking a sample.

> > If we can unify branch filter within a perf session, would this be
> > much easier for handling?
> 
> Do you mean if the perf tool ensured that all events in a given session
> had the same filter? From the kernel's PoV there's no such thing as a
> "perf session", and I'm not sure whether you're suggesting doing that in
> userspace or withing the kernel.

My understanding is this would be not difficult to do such kind checking
in the tool.  E.g., the perf tool can iterate every event and check the
branch filter and detect incompabile issue.

> Doing that in the perf tool would certianly make a stronger argument for
> the kernel taking the "reject conflicting branch filters" option.
> 
> Doing that within the kernel isn't really possible.

As said above, if the BRBE driver can provide a knob in sysfs to indicate
what is the current branch filter in the existed sessions, this would be
helpful for the tool to do the checking and remind users.

I haven't done any experiments for this. If you think this is the way
to move forward, I might do a prototype and get back to you to ensure we
don't run into any unexpected issues.

[...]

To make the discussion easier, I would like reply separately regarding
the branch record save and restore issue.

Thanks,
Leo