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

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: <20250224140317.GF8144@e132581.arm.com>
Date: Mon, 24 Feb 2025 14:03:17 +0000
From: Leo Yan <leo.yan@....com>
To: Rob Herring <robh@...nel.org>
Cc: Will Deacon <will@...nel.org>, Mark Rutland <mark.rutland@....com>,
	Catalin Marinas <catalin.marinas@....com>,
	Jonathan Corbet <corbet@....net>, Marc Zyngier <maz@...nel.org>,
	Oliver Upton <oliver.upton@...ux.dev>,
	Joey Gouly <joey.gouly@....com>,
	Suzuki K Poulose <suzuki.poulose@....com>,
	Zenghui Yu <yuzenghui@...wei.com>,
	James Clark <james.clark@...aro.org>,
	Anshuman Khandual <anshuman.khandual@....com>,
	linux-arm-kernel@...ts.infradead.org,
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-doc@...r.kernel.org, kvmarm@...ts.linux.dev
Subject: Re: [PATCH v20 11/11] perf: arm_pmuv3: Add support for the Branch
 Record Buffer Extension (BRBE)

On Mon, Feb 24, 2025 at 06:46:35AM -0600, Rob Herring wrote:
> On Mon, Feb 24, 2025 at 6:25 AM Leo Yan <leo.yan@....com> wrote:
> > On Tue, Feb 18, 2025 at 02:40:06PM -0600, Rob Herring (Arm) wrote:
> > >
> > > From: Anshuman Khandual <anshuman.khandual@....com>
> >
> > [...]
> >
> > > BRBE records are invalidated whenever events are reconfigured, a new
> > > task is scheduled in, or after recording is paused (and the records
> > > have been recorded for the event). The architecture allows branch
> > > records to be invalidated by the PE under implementation defined
> > > conditions. It is expected that these conditions are rare.
> >
> > [...]
> >
> > > +static void armv8pmu_sched_task(struct perf_event_pmu_context *pmu_ctx, bool sched_in)
> > > +{
> > > +       struct arm_pmu *armpmu = *this_cpu_ptr(&cpu_armpmu);
> > > +       struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
> > > +
> > > +       if (!hw_events->branch_users)
> > > +               return;
> > > +
> > > +       if (sched_in)
> > > +               brbe_invalidate();
> > > +}
> >
> > Just a minor concern.  I don't see any handling for task migration.
> > E.g., for a task is migrated from one CPU to another CPU, I expect we
> > need to save and restore branch records based on BRBE injection.  So
> > far, the driver simply invalidates all records.
> >
> > I think this topic is very likely discussed before.  If this is the
> > case, please ignore my comment.  Except this, the code looks good
> > to me.
> 
> Not really discussed on the list, but that was present in v18 (though
> not functional because .sched_task() hook wasn't actually enabled) and
> Mark removed it. His work is here[1].The only comment was:
> 
> Note: saving/restoring at context-switch doesn't interact well with
> event rotation (e.g. if filters change)

In the brbe_enable() function, it "Merge the permitted branch filters
of all events".  Based on current implementation, all events share the
same branch filter.

When event rotation happens, if without context switch, in theory we
should can directly use the branch record (no invalidation, no injection)
for all events.

For a context-switch case, we need to save and re-inject branch record.
BRBE record sticks to a process context, no matter what events have been
enabled.

Thanks,
Leo