[PATCH 2/3] perf: RISC-V: allow to use the highest available counter

[Atish Patra]

On Thu, Jun 23, 2022 at 4:27 AM Sergey Matyukevich <geomatsi at gmail.com> wrote:
>
> From: Sergey Matyukevich <sergey.matyukevich at syntacore.com>
>
> Both OpenSBI and Linux explicitly assume that there is no hardware counter
> with index 1: hardware uses that bit for TM control. So OpenSBI filters
> out that index in sanity checks. However its range sanity checks do not
> treat that index in a special way. As a result, OpenSBI does not allow
> to use the firmware counter with the highest index. Linux perf RISC-V
> SBI driver is adapted for the current OpenSBI behavior: it excludes the
> highest valid index from the counter mask passed to OpenSBI.
>
> This patch fixes ranges to re-enable the highest available counter.
>
> Accompanying OpenSBI fix to accept full mask:
> - https://github.com/riscv-software-src/opensbi/pull/260
>
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich at syntacore.com>
> ---
>  drivers/perf/riscv_pmu_sbi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index 3e0ea564b9b8..294d4bded59e 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -265,7 +265,7 @@ static int pmu_sbi_ctr_get_idx(struct perf_event *event)
>         struct sbiret ret;
>         int idx;
>         uint64_t cbase = 0;
> -       uint64_t cmask = GENMASK_ULL(rvpmu->num_counters - 1, 0);
> +       uint64_t cmask = GENMASK_ULL(rvpmu->num_counters, 0);
>         unsigned long cflags = 0;
>
>         if (event->attr.exclude_kernel)
> @@ -283,7 +283,7 @@ static int pmu_sbi_ctr_get_idx(struct perf_event *event)
>         }
>
>         idx = ret.value;
> -       if (idx >= rvpmu->num_counters || !pmu_ctr_list[idx].value)
> +       if (idx > rvpmu->num_counters || !pmu_ctr_list[idx].value)
>                 return -ENOENT;
>
>         /* Additional sanity check for the counter id */
> @@ -482,7 +482,7 @@ static inline void pmu_sbi_stop_all(struct riscv_pmu *pmu)
>          * which may include counters that are not enabled yet.
>          */
>         sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP,
> -                 0, GENMASK_ULL(pmu->num_counters - 1, 0), 0, 0, 0, 0);
> +                 0, GENMASK_ULL(pmu->num_counters, 0), 0, 0, 0, 0);
>  }
>
>  static inline void pmu_sbi_stop_hw_ctrs(struct riscv_pmu *pmu)
> --
> 2.36.1
>

This won't be required to support maximum firmware counter index. With
the fix, I proposed in your first patch in kernel
and OpenSBI, it should be doable. Both kernel & OpenSBI should use
zero indexed counter schemes to avoid confusion.


--
Regards,
Atish