Received-SPF: pass (google.com: domain of linux-kernel+bounces-127286-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240331041830.2806741-1-andrii@kernel.org> <Zgp+lYLOZU3OkO7w@gmail.com>
In-Reply-To: <Zgp+lYLOZU3OkO7w@gmail.com>
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date: Mon, 1 Apr 2024 19:16:55 -0700
Message-ID: <CAEf4BzZEuDRt=dLN79uJ0EwQcxuxCp8LZx1ayNBsw82BytPTqg@mail.gmail.com>
Subject: Re: [PATCH v4 0/4] perf/x86/amd: add LBR capture support outside of
 hardware events
To: Ingo Molnar <mingo@kernel.org>
Cc: Andrii Nakryiko <andrii@kernel.org>, x86@kernel.org, peterz@infradead.org, 
	mingo@redhat.com, tglx@linutronix.de, bpf@vger.kernel.org, 
	linux-kernel@vger.kernel.org, jolsa@kernel.org, song@kernel.org, 
	kernel-team@meta.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Apr 1, 2024 at 2:30=E2=80=AFAM Ingo Molnar <mingo@kernel.org> wrote=
:
>
>
> * Andrii Nakryiko <andrii@kernel.org> wrote:
>
> > Add AMD-specific implementation of perf_snapshot_branch_stack static ca=
ll that
> > allows LBR capture from arbitrary points in the kernel. This is utilize=
d by
> > BPF programs. See patch #3 for all the details.
> >
> > Patches #1 and #2 are preparatory steps to ensure LBR freezing is compl=
etely
> > inlined and have no branches, to minimize LBR snapshot contamination.
> >
> > Patch #4 removes an artificial restriction on perf events with LBR enab=
led.
> >
> > Andrii Nakryiko (4):
> >   perf/x86/amd: ensure amd_pmu_core_disable_all() is always inlined
> >   perf/x86/amd: avoid taking branches before disabling LBR
> >   perf/x86/amd: support capturing LBR from software events
> >   perf/x86/amd: don't reject non-sampling events with configured LBR
> >
> >  arch/x86/events/amd/core.c   | 37 +++++++++++++++++++++++++++++++++++-
> >  arch/x86/events/amd/lbr.c    | 11 +----------
> >  arch/x86/events/perf_event.h | 11 +++++++++++
> >  3 files changed, 48 insertions(+), 11 deletions(-)
>
> So there's a new conflict with patch #2, probably due to interaction
> with this recent fix that is now upstream:
>
>    598c2fafc06f ("perf/x86/amd/lbr: Use freeze based on availability")
>
> I don't think it should change the logic of the snapshot feature
> materially, X86_FEATURE_AMD_LBR_PMC_FREEZE should be orthogonal to it,
> as the LBR snapshot isn't taken from a PMI.
>

Yep, seems like there was a parallel change to related code in
perf/urgent branch. And yes, you are right that it's orthogonal and
doesn't regress anything as far as branching and whatnot (just
retested everything on real hardware). So I've rebased my patches on
top of perf/urgent, will send v5 momentarily. Sorry for an extra round
on this.

> Thanks,
>
>         Ingo