Re: [PATCH 3/3] perf/amd/ibs: Simplify event state handling with IBS_CAPS_DIS

[Jim Mattson]

On Wed, Oct 14, 2026 at 11:05 PM Ravi Bangoria <ravi.bangoria@xxxxxxx> wrote:
>
> The IBS_{FETCH|OP}_CTL2[Dis] bit allows IBS sampling to be stopped without
> a RMW of IBS_{FETCH,OP}_CTL. Systems providing this bit therefore do not
> require the IBS_STARTED and IBS_STOPPED states used to coordinate control
> MSR updates with late NMIs.

CTL2[Dis] doesn't make late NMIs go away. Hence, this patch needs a
50us poll. With the two-line fix in my original patch, the existing
scheme handles late NMIs indefinitely: IBS_STOPPED remains set until a
late NMI consumes it, even if the event has been restarted in the
meantime. This patch trades that for a 50us window, after which a late
NMI is unknown (which *is* fatal with unknown_nmi_panic). Moreover,
since we still have to handle hardware without IBS_CAPS_DIS, we now
have two state machines instead of one.

> Add a separate stop path for these systems. Preserve Val while disabling
> the event so that a delayed NMI can be identified and consumed, and wait
> briefly for such an NMI before restarting the event.
>
> Leave the existing handling unchanged on systems without IBS_CAPS_DIS.
>
> Reported-by: Jim Mattson <jmattson@xxxxxxxxxx>
> Closes: https://lore.kernel.org/r/20261007234815.4026028-1-jmattson@xxxxxxxxxx
> Fixes: efa5700ec0da ("perf/amd/ibs: Support IBS_{FETCH|OP}_CTL2[Dis] to eliminate RMW race")
> Signed-off-by: Ravi Bangoria <ravi.bangoria@xxxxxxx>
> ---
>  arch/x86/events/amd/ibs.c | 145 ++++++++++++++++++++++++++++++++------
>  1 file changed, 122 insertions(+), 23 deletions(-)

 [ ... ]

> +/* ->stop() for platforms with additional CTL2[Dis] bit. */
> +static void perf_ibs_stop_dis(struct perf_event *event, int flags)
> +{
> +       struct hw_perf_event *hwc = &event->hw;
> +       struct perf_ibs *perf_ibs = container_of(event->pmu, struct perf_ibs, pmu);
> +       struct cpu_perf_ibs *pcpu = this_cpu_ptr(perf_ibs->pcpu);
> +       u64 config;
> +       u64 ctl2;
> +
> +       WARN_ON_ONCE(test_bit(IBS_STOPPING, pcpu->state));
> +
> +       set_bit(IBS_STOPPING, pcpu->state);

This function needs to be reentrant. If an IBS event is a sibling in a
group led by a core PMU event, and the leader is throttled,
perf_event_throttle_group() will stop the IBS sibling from the core
PMU's NMI handler:

    amd_pmu_v2_handle_irq() ->
      perf_event_overflow() ->
        __perf_event_overflow() ->
          __perf_event_account_interrupt() ->
            perf_event_throttle_group() ->
              perf_event_throttle() ->
                perf_ibs_stop() ->
                  perf_ibs_stop_dis()

[ .. ]

> +       if (in_nmi())
> +               wrmsrq(hwc->config_base, config);

As we saw above, in_nmi() doesn't mean that we're in
perf_ibs_handle_irq() for this sample. If IBS Val=1 and the IBS NMI is
still pending, this clears Val. When the IBS NMI arrives,
perf_ibs_handle_irq() finds CTL2[Dis]=1 and Val=0, and therefore
returns 0, leading to an unknown NMI.