Re: [PATCH RFC] perf/x86/amd/lbr: Don't freeze the LBR on PMI for user-only filters

[Andrii Nakryiko]

On Thu, Nov 5, 2026 at 5:07 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Thu, Nov 5, 2026 at 9:03 AM Puranjay Mohan <puranjay@xxxxxxxxxx> wrote:
> >
> > On CPUs with X86_FEATURE_AMD_LBR_PMC_FREEZE the LBR is always frozen on
> > PMI, so the branch stack ends before the IP and callchain, which are
> > taken when the NMI is delivered. IBS rejects branch-stack events, so
> > there is no precise alternative.
> >
> > The freeze only keeps the PMI handler's branches out of the stack, which
> > matters only if branches into the kernel are recorded. Skip it when
> > LBR_SELECT suppresses them, and add a freeze format bit (config2:0) to
> > request it anyway.
> >
> > Signed-off-by: Puranjay Mohan <puranjay@xxxxxxxxxx>
> > ---
> > Thanks both. A user-only default with an explicit request works for me,
> > with one exception I would like to keep: when branches into the kernel
> > are recorded the freeze has to stay. On a Zen 5 part with the freeze
> > cleared and a user+kernel filter, every entry in every sample was a
> > branch taken by amd_pmu_v2_handle_irq() or amd_pmu_lbr_read(), so users
> > recording kernel branches would regress with a "never freeze" default.
> >
> > This RFC does the following:
> >  - LBR_SELECT suppresses kernel branches: no freeze, unless the new
> >    AMD-only freeze format bit (config2:0, advertised through the format
> >    attributes) is set.
> >  - Otherwise: freeze as today.
> >
> > That needs no tool changes for the user-only case and gives an explicit
> > way to request the freeze. This version is only compile tested; I will
> > retest on a Zen 5 part if the approach is acceptable.
> >
> >  arch/x86/events/amd/core.c | 14 ++++++++++++++
> >  arch/x86/events/amd/lbr.c  | 21 ++++++++++++++++++---
> >  2 files changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> > index 49b6b8fce5664..d63649c756a58 100644
> > --- a/arch/x86/events/amd/core.c
> > +++ b/arch/x86/events/amd/core.c
> > @@ -1404,8 +1404,22 @@ static struct attribute_group group_events_amd_brs = {
> >
> >  #endif /* CONFIG_PERF_EVENTS_AMD_BRS */
> >
> > +PMU_FORMAT_ATTR(freeze, "config2:0");
> > +
> > +static struct attribute *amd_pmu_lbr_format_attrs[] = {
> > +       &format_attr_freeze.attr,
> > +       NULL,
> > +};
> > +
> > +static struct attribute_group group_format_amd_lbr = {
> > +       .name       = "format",
> > +       .attrs      = amd_pmu_lbr_format_attrs,
> > +       .is_visible = amd_branches_is_visible,
> > +};
> > +
> >  static const struct attribute_group *amd_attr_update[] = {
> >         &group_caps_amd_branches,
> > +       &group_format_amd_lbr,
> >  #ifdef CONFIG_PERF_EVENTS_AMD_BRS
> >         &group_events_amd_brs,
> >  #endif
> > diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
> > index a55646fcb8465..f45e2c53566af 100644
> > --- a/arch/x86/events/amd/lbr.c
> > +++ b/arch/x86/events/amd/lbr.c
> > @@ -10,6 +10,12 @@
> >  /* LBR Branch Select valid bits */
> >  #define LBR_SELECT_MASK                0x1ff
> >
> > +/* attr.config2: freeze the LBR on PMI */
> > +#define LBR_CFG2_FREEZE                BIT_ULL(0)
> > +
> > +/* Software only, masked out before writing LBR_SELECT */
> > +#define LBR_FREEZE             BIT_ULL(63)
> > +
> >  /*
> >   * LBR Branch Select filter bits which when set, ensures that the
> >   * corresponding type of branches are not recorded
> > @@ -307,6 +313,9 @@ static int amd_pmu_lbr_setup_filter(struct perf_event *event)
> >         /* Filter bits operate in suppress mode */
> >         reg->config = mask ^ LBR_SELECT_MASK;
> >
> > +       if (event->attr.config2 & LBR_CFG2_FREEZE)
> > +               reg->config |= LBR_FREEZE;
> > +
> >         return 0;
> >  }
> >
> > @@ -392,20 +401,26 @@ void amd_pmu_lbr_sched_task(struct perf_event_pmu_context *pmu_ctx,
> >  void amd_pmu_lbr_enable_all(void)
> >  {
> >         struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> > -       u64 lbr_select, dbg_ctl, dbg_extn_cfg;
> > +       u64 lbr_cfg = 0, lbr_select = 0, dbg_ctl, dbg_extn_cfg;
> >
> >         if (!cpuc->lbr_users || !x86_pmu.lbr_nr)
> >                 return;
> >
> >         /* Set hardware branch filter */
> >         if (cpuc->lbr_select) {
> > -               lbr_select = cpuc->lbr_sel->config & LBR_SELECT_MASK;
> > +               lbr_cfg = cpuc->lbr_sel->config;
> > +               lbr_select = lbr_cfg & LBR_SELECT_MASK;
> >                 wrmsrq(MSR_AMD64_LBR_SELECT, lbr_select);
> >         }
> >
> >         if (cpu_feature_enabled(X86_FEATURE_AMD_LBR_PMC_FREEZE)) {
> >                 rdmsrq(MSR_IA32_DEBUGCTLMSR, dbg_ctl);
> > -               wrmsrq(MSR_IA32_DEBUGCTLMSR, dbg_ctl | DEBUGCTLMSR_FREEZE_LBRS_ON_PMI);
> > +               /* Skip the freeze if kernel branches are suppressed, unless asked */
> > +               if ((lbr_select & LBR_KERNEL) && !(lbr_cfg & LBR_FREEZE))
>
> heh, so we basically either
>
> a) opt-in freeze with *new config*, but do it *only for non-kernel LBR*
>
> or
>
> b) opt-out freeze *only for non-kernel LBR* _unconditionally_, but
> without adding any new config
>
> In both cases we have a behavior that makes sense in practice, but a)
> adds some new config, while b) doesn't add anything. End result is the
> same, just more configuration and need to change user space code to
> opt-out the freeze.
>
> Is a) worth it, or we should stick to b) and keep it simple, Sandipan?

ping

>
> > +                       dbg_ctl &= ~DEBUGCTLMSR_FREEZE_LBRS_ON_PMI;
> > +               else
> > +                       dbg_ctl |= DEBUGCTLMSR_FREEZE_LBRS_ON_PMI;
> > +               wrmsrq(MSR_IA32_DEBUGCTLMSR, dbg_ctl);
> >         }
> >
> >         rdmsrq(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg);
> >
> > base-commit: 0f443787a7f6ae907975742680aac2600483eaf1
> > --
> > 2.53.0-Meta
> >