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 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?

> +                       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
>