[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6fa06233-d572-48a7-a8ef-73a7c5879c06@sifive.com>
Date: Fri, 19 Apr 2024 19:37:03 -0500
From: Samuel Holland <samuel.holland@...ive.com>
To: Atish Patra <atishp@...osinc.com>, linux-kernel@...r.kernel.org
Cc: Andrew Jones <ajones@...tanamicro.com>,
Ajay Kaher <ajay.kaher@...adcom.com>, Albert Ou <aou@...s.berkeley.edu>,
Alexandre Ghiti <alexghiti@...osinc.com>, Anup Patel <anup@...infault.org>,
Conor Dooley <conor.dooley@...rochip.com>, Juergen Gross <jgross@...e.com>,
kvm-riscv@...ts.infradead.org, kvm@...r.kernel.org,
linux-kselftest@...r.kernel.org, linux-riscv@...ts.infradead.org,
Mark Rutland <mark.rutland@....com>, Palmer Dabbelt <palmer@...belt.com>,
Paolo Bonzini <pbonzini@...hat.com>, Paul Walmsley
<paul.walmsley@...ive.com>, Shuah Khan <shuah@...nel.org>,
virtualization@...ts.linux.dev, Will Deacon <will@...nel.org>, x86@...nel.org
Subject: Re: [PATCH v8 08/24] drivers/perf: riscv: Fix counter mask iteration
for RV32
Hi Atish,
On 2024-04-20 10:17 AM, Atish Patra wrote:
> For RV32, used_hw_ctrs can have more than 1 word if the firmware chooses
> to interleave firmware/hardware counters indicies. Even though it's a
> unlikely scenario, handle that case by iterating over all the words
> instead of just using the first word.
>
> Reviewed-by: Andrew Jones <ajones@...tanamicro.com>
> Signed-off-by: Atish Patra <atishp@...osinc.com>
> ---
> drivers/perf/riscv_pmu_sbi.c | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index f23501898657..4eacd89141a9 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -652,10 +652,12 @@ static inline void pmu_sbi_stop_all(struct riscv_pmu *pmu)
> static inline void pmu_sbi_stop_hw_ctrs(struct riscv_pmu *pmu)
> {
> struct cpu_hw_events *cpu_hw_evt = this_cpu_ptr(pmu->hw_events);
> + int i;
>
> - /* No need to check the error here as we can't do anything about the error */
> - sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP, 0,
> - cpu_hw_evt->used_hw_ctrs[0], 0, 0, 0, 0);
> + for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++)
> + /* No need to check the error here as we can't do anything about the error */
> + sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_STOP, i * BITS_PER_LONG,
> + cpu_hw_evt->used_hw_ctrs[i], 0, 0, 0, 0);
> }
>
> /*
> @@ -667,7 +669,7 @@ static inline void pmu_sbi_stop_hw_ctrs(struct riscv_pmu *pmu)
> static inline void pmu_sbi_start_overflow_mask(struct riscv_pmu *pmu,
> unsigned long ctr_ovf_mask)
> {
> - int idx = 0;
> + int idx = 0, i;
> struct cpu_hw_events *cpu_hw_evt = this_cpu_ptr(pmu->hw_events);
> struct perf_event *event;
> unsigned long flag = SBI_PMU_START_FLAG_SET_INIT_VALUE;
> @@ -676,11 +678,12 @@ static inline void pmu_sbi_start_overflow_mask(struct riscv_pmu *pmu,
> struct hw_perf_event *hwc;
> u64 init_val = 0;
>
> - ctr_start_mask = cpu_hw_evt->used_hw_ctrs[0] & ~ctr_ovf_mask;
> -
> - /* Start all the counters that did not overflow in a single shot */
> - sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_START, 0, ctr_start_mask,
> - 0, 0, 0, 0);
> + for (i = 0; i < BITS_TO_LONGS(RISCV_MAX_COUNTERS); i++) {
> + ctr_start_mask = cpu_hw_evt->used_hw_ctrs[i] & ~ctr_ovf_mask;
This is applying the mask for the first 32 logical counters to the both sets of
32 logical counters. ctr_ovf_mask needs to be 64 bits wide here, so each loop
iteration can apply the correct half of the mask.
Regards,
Samuel
> + /* Start all the counters that did not overflow in a single shot */
> + sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_START, i * BITS_PER_LONG, ctr_start_mask,
> + 0, 0, 0, 0);
> + }
>
> /* Reinitialize and start all the counter that overflowed */
> while (ctr_ovf_mask) {
Powered by blists - more mailing lists