[PATCH] perf: riscv: fix PMU overflow counter restart state

Pengpeng Hou <[email protected]>
Newsgroups org.infradead.lists.linux-riscv,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-perf-users
Message-ID <[email protected]>
The PMU overflow handler keeps a 64-bit mask for all logical counters,
while SBI start calls consume an XLEN-sized mask relative to a counter
base.

On RV32, pmu_sbi_start_ovf_ctrs_sbi() applies the low overflow-mask word
to every used-counter word. Counters 32 through 63 can therefore be
classified using overflow bits 0 through 31.

The snapshot path and overflow handler also test and construct 64-bit
masks with XLEN-sized BIT(). In addition, snapshot restart uses the
completed inner-loop index instead of the outer word index to form
counter_idx_base. The latter selects an invalid counter base on both RV32
and RV64: for_each_set_bit() finishes with idx equal to BITS_PER_LONG, so
the current expression passes BITS_PER_LONG squared instead of the
expected i times BITS_PER_LONG.

Select the overflow word matching each SBI mask, use 64-bit bit operations
for global counter indices, and derive the snapshot start base from the
outer word index.

Fixes: b994cdfcdf7b ("drivers/perf: riscv: Fix counter mask iteration for RV32")
Fixes: a8625217a054 ("drivers/perf: riscv: Implement SBI PMU snapshot function")
Signed-off-by: Pengpeng Hou <[email protected]>
---
 drivers/perf/riscv_pmu_sbi.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
index 50220f7b46d9..6cfb1fc0ec6e 100644
--- a/drivers/perf/riscv_pmu_sbi.c
+++ b/drivers/perf/riscv_pmu_sbi.c
@@ -962,7 +962,9 @@ static inline void pmu_sbi_start_ovf_ctrs_sbi(struct cpu_hw_events *cpu_hw_evt,
 	u64 init_val = 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;
+		unsigned long word_ovf_mask =
+			ctr_ovf_mask >> (i * BITS_PER_LONG);
+		ctr_start_mask = cpu_hw_evt->used_hw_ctrs[i] & ~word_ovf_mask;
 		/* Start all the counters that did not overflow in a single shot */
 		if (ctr_start_mask) {
 			sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_START, i * BITS_PER_LONG,
@@ -1002,7 +1004,7 @@ static inline void pmu_sbi_start_ovf_ctrs_snapshot(struct cpu_hw_events *cpu_hw_
 	struct riscv_pmu_snapshot_data *sdata = cpu_hw_evt->snapshot_addr;
 
 	for_each_set_bit(idx, cpu_hw_evt->used_hw_ctrs, RISCV_MAX_COUNTERS) {
-		if (ctr_ovf_mask & BIT(idx)) {
+		if (ctr_ovf_mask & BIT_ULL(idx)) {
 			event = cpu_hw_evt->events[idx];
 			hwc = &event->hw;
 			max_period = riscv_pmu_ctr_get_width_mask(event);
@@ -1021,7 +1023,7 @@ static inline void pmu_sbi_start_ovf_ctrs_snapshot(struct cpu_hw_events *cpu_hw_
 			sdata->ctr_values[idx] =
 					cpu_hw_evt->snapshot_cval_shcopy[idx + i * BITS_PER_LONG];
 		/* Start all the counters in a single shot */
-		sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_START, idx * BITS_PER_LONG,
+		sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_START, i * BITS_PER_LONG,
 			  cpu_hw_evt->used_hw_ctrs[i], flag, 0, 0, 0);
 	}
 }
@@ -1109,14 +1111,14 @@ static irqreturn_t pmu_sbi_ovf_handler(int irq, void *dev)
 			hidx = info->csr - CSR_CYCLE;
 
 		/* check if the corresponding bit is set in scountovf or overflow mask in shmem */
-		if (!(overflow & BIT(hidx)))
+		if (!(overflow & BIT_ULL(hidx)))
 			continue;
 
 		/*
 		 * Keep a track of overflowed counters so that they can be started
 		 * with updated initial value.
 		 */
-		overflowed_ctrs |= BIT(lidx);
+		overflowed_ctrs |= BIT_ULL(lidx);
 		hw_evt = &event->hw;
 		/* Update the event states here so that we know the state while reading */
 		hw_evt->state |= PERF_HES_STOPPED;

base-commit: 1b78070aaef63512688aebfbc82365ef9d6660f1
-- 
2.43.0


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.