Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220513015522.910856-1-atishp@rivosinc.com> <20220513015522.910856-4-atishp@rivosinc.com>
In-Reply-To: <20220513015522.910856-4-atishp@rivosinc.com>
From:   Anup Patel <anup@brainfault.org>
Date:   Tue, 24 May 2022 16:52:26 +0530
Message-ID: <CAAhSdy23vE8+HxU5Jxy2rBMjy3rBTrJt_4sriuROac_sEESSVw@mail.gmail.com>
Subject: Re: [PATCH v2 3/3] RISC-V: Fix SBI PMU calls for RV32
To:     Atish Patra <atishp@rivosinc.com>
Cc:     "linux-kernel@vger.kernel.org List" <linux-kernel@vger.kernel.org>,
        Atish Patra <atishp@atishpatra.org>,
        Jisheng Zhang <jszhang@kernel.org>,
        linux-riscv <linux-riscv@lists.infradead.org>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Rob Herring <robh+dt@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, May 13, 2022 at 7:25 AM Atish Patra <atishp@rivosinc.com> wrote:
>
> Some of the SBI PMU calls does not pass 64bit arguments
> correctly and not under RV32 compile time flags. Currently,
> this doesn't create any incorrect results as RV64 ignores
> any value in the additional register and qemu doesn't support
> raw events.
>
> Fix those SBI calls in order to set correct values for RV32.
>
> Fixes: e9991434596f ("RISC-V: Add perf platform driver based on SBI PMU extension")
>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>

Looks good to me.

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

Regards,
Anup

> ---
>  drivers/perf/riscv_pmu_sbi.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index 7ad92039a718..fab0dd497393 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -274,8 +274,13 @@ static int pmu_sbi_ctr_get_idx(struct perf_event *event)
>                 cflags |= SBI_PMU_CFG_FLAG_SET_UINH;
>
>         /* retrieve the available counter index */
> +#if defined(CONFIG_32BIT)
> +       ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_CFG_MATCH, cbase, cmask,
> +                       cflags, hwc->event_base, hwc->config, hwc->config >> 32);
> +#else
>         ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_CFG_MATCH, cbase, cmask,
>                         cflags, hwc->event_base, hwc->config, 0);
> +#endif
>         if (ret.error) {
>                 pr_debug("Not able to find a counter for event %lx config %llx\n",
>                         hwc->event_base, hwc->config);
> @@ -417,8 +422,13 @@ static void pmu_sbi_ctr_start(struct perf_event *event, u64 ival)
>         struct hw_perf_event *hwc = &event->hw;
>         unsigned long flag = SBI_PMU_START_FLAG_SET_INIT_VALUE;
>
> +#if defined(CONFIG_32BIT)
>         ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_START, hwc->idx,
>                         1, flag, ival, ival >> 32, 0);
> +#else
> +       ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_START, hwc->idx,
> +                       1, flag, ival, 0, 0);
> +#endif
>         if (ret.error && (ret.error != SBI_ERR_ALREADY_STARTED))
>                 pr_err("Starting counter idx %d failed with error %d\n",
>                         hwc->idx, sbi_err_map_linux_errno(ret.error));
> --
> 2.25.1
>