[PATCH] lib: sbi_pmu: Fix multiple FW counter start operations with custom PMU device

[Anup Patel]

On Fri, Jan 16, 2026 at 10:23 PM James Raphael Tiovalen
<jamestiotio at gmail.com> wrote:
>
> Currently, we immediately return the result of `fw_counter_start` if the
> event code is 0xFFFF. However, this skips setting the bit in the
> `fw_counters_started` bitmap even if the platform-specific call
> succeeds. Restore the original behavior of returning early only on an
> error so that we still set the bit in the bitmap. This prevents multiple
> starts of the same FW counter. This also aligns the expectations of
> `pmu_ctr_start_fw` with `pmu_ctr_stop_fw` since we cannot assume that
> the platform-specific functions to start and stop FW counters will
> modify the bitmap state.
>
> Fixes: 57d3aa3b0dbd ("lib: sbi_pmu: Introduce fw_counter_write_value API")
> Signed-off-by: James Raphael Tiovalen <jamestiotio at gmail.com>

LGTM.

Reviewed-by: Anup Patel <anup at brainfault.org>

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup

> ---
>  lib/sbi/sbi_pmu.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> index e084005d..75038f91 100644
> --- a/lib/sbi/sbi_pmu.c
> +++ b/lib/sbi/sbi_pmu.c
> @@ -448,6 +448,8 @@ static int pmu_ctr_start_fw(struct sbi_pmu_hart_state *phs,
>                             uint64_t event_data, uint64_t ival,
>                             bool ival_update)
>  {
> +       int ret;
> +
>         if ((event_code >= SBI_PMU_FW_MAX &&
>             event_code <= SBI_PMU_FW_RESERVED_MAX) ||
>             event_code > SBI_PMU_FW_PLATFORM)
> @@ -468,9 +470,11 @@ static int pmu_ctr_start_fw(struct sbi_pmu_hart_state *phs,
>                                                         cidx - num_hw_ctrs,
>                                                         ival);
>
> -               return pmu_dev->fw_counter_start(phs->hartid,
> +               ret = pmu_dev->fw_counter_start(phs->hartid,
>                                                  cidx - num_hw_ctrs,
>                                                  event_data);
> +               if (ret)
> +                       return ret;
>         } else {
>                 if (ival_update)
>                         phs->fw_counters_data[cidx - num_hw_ctrs] = ival;
> --
> 2.43.0
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi