Re: [PATCH] lib/sbi_pmu: Don't fallback to fixed counters when sscofpmf && !smcntrpmf

On Tue, Mar 24, 2026 at 5:59 PM Michael Ellerman <[email protected]> wrote:
>
> Currently when searching for a hardware counter for an event, if no
> programmable counter is available, the code falls back to using a fixed
> counter (mcycle/minstret) if one matches the event.
>
> However the fallback is incorrect when sscofpmf is present but
> smcntrpmf is not. That's because with sscofpmf, programmable counters
> support mode filtering, but the fixed counters do not (without
> smcntrpmf). Even if the caller didn't configure mode filtering, by
> default programmable counters don't count M mode when sscofpmf is
> present, whereas mcycle/minstret do.
>
> Fix the logic to not fallback to a fixed counter if sscofpmf is present
> but smcntrpmf is not.
>
> Signed-off-by: Michael Ellerman <[email protected]>

Fixes tag is missing but I will add this at the time of merging.

Reviewed-by: Anup Patel <[email protected]>

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup


> ---
> Hi folks,
>
> I noticed this while testing Linux perf with more events than
> programmable counters.
>
> Sometimes cycles gets counted using a mhpmcounter, and sometimes with
> mcycle, and the cycle count differs between the two.
>
> The count differs because mcycle doesn't support filtering when
> smcntrpmf is not present.
>
> It can be reproduced by running perf stat and specifying `cycles:u` N+1
> times where N is the number of mhpmcounters, eg:
>
>     $ perf stat -e cycles:u,cycles:u,...
>     ...
>                 392860      cycles:u
>               61101120      cycles:u
>
> The first cycles got an mhpmcounter and only counted userspace cycles.
> The second used mcycle and counted everything.
>
> cheers
> ---
>  lib/sbi/sbi_pmu.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index ae00ad5844b1fe92a8eebfb618902a5eef218a9e..8a9021e2e5bac2d71a0f07d3f81600ff370ee2fc 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -830,13 +830,20 @@ static int pmu_ctr_find_hw(struct sbi_pmu_hart_state *phs,
>
>         if (ctr_idx == SBI_ENOTSUPP) {
>                 /**
> -                * We can't find any programmable counters for cycle/instret.
> -                * Return the fixed counter as they are mandatory anyways.
> +                * We can't find a programmable counter, see if we can use a
> +                * fixed counter instead if one was found for this event.
> +                *
> +                * If sscofpmf is present but smcntrpmf is not, we can't
> +                * fallback to a fixed counter, because the fixed counter
> +                * doesn't support filtering whereas a programmable counter
> +                * would.
>                  */
> -               if (fixed_ctr >= 0)
> -                       return pmu_fixed_ctr_update_inhibit_bits(fixed_ctr, flags);
> -               else
> +               if (fixed_ctr < 0 ||
> +                   ((sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF) &&
> +                     !sbi_hart_has_extension(scratch, SBI_HART_EXT_SMCNTRPMF))))
>                         return SBI_EFAIL;
> +
> +               return pmu_fixed_ctr_update_inhibit_bits(fixed_ctr, flags);
>         }
>         ret = pmu_update_hw_mhpmevent(temp, ctr_idx, flags, event_idx, data);
>
>
> ---
> base-commit: 4813a2042096b7860655761aad973723293a552e
> change-id: 20260324-mcycle-fix-3fc5682eee9c
>
>
> --
> opensbi mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/opensbi

-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi