[PATCH] lib/sbi_pmu: Don't fallback to fixed counters when sscofpmf && !smcntrpmf

[Michael Ellerman <mpe@kernel.org>]

Currently when searching for a hardware counter for an event, if no
programmable counter is available, the code falls back to using a fixed
counter (mcycle/minstret) if one matches the event.

However the fallback is incorrect when sscofpmf is present but
smcntrpmf is not. That's because with sscofpmf, programmable counters
support mode filtering, but the fixed counters do not (without
smcntrpmf). Even if the caller didn't configure mode filtering, by
default programmable counters don't count M mode when sscofpmf is
present, whereas mcycle/minstret do.

Fix the logic to not fallback to a fixed counter if sscofpmf is present
but smcntrpmf is not.

Signed-off-by: Michael Ellerman <mpe@kernel.org>
---
Hi folks,

I noticed this while testing Linux perf with more events than
programmable counters.

Sometimes cycles gets counted using a mhpmcounter, and sometimes with
mcycle, and the cycle count differs between the two.

The count differs because mcycle doesn't support filtering when
smcntrpmf is not present.

It can be reproduced by running perf stat and specifying `cycles:u` N+1
times where N is the number of mhpmcounters, eg:

    $ perf stat -e cycles:u,cycles:u,... 
    ...
                392860      cycles:u
              61101120      cycles:u

The first cycles got an mhpmcounter and only counted userspace cycles.
The second used mcycle and counted everything.

cheers
---
 lib/sbi/sbi_pmu.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
index ae00ad5844b1fe92a8eebfb618902a5eef218a9e..8a9021e2e5bac2d71a0f07d3f81600ff370ee2fc 100644
--- a/lib/sbi/sbi_pmu.c
+++ b/lib/sbi/sbi_pmu.c
@@ -830,13 +830,20 @@ static int pmu_ctr_find_hw(struct sbi_pmu_hart_state *phs,
 
 	if (ctr_idx == SBI_ENOTSUPP) {
 		/**
-		 * We can't find any programmable counters for cycle/instret.
-		 * Return the fixed counter as they are mandatory anyways.
+		 * We can't find a programmable counter, see if we can use a
+		 * fixed counter instead if one was found for this event.
+		 *
+		 * If sscofpmf is present but smcntrpmf is not, we can't
+		 * fallback to a fixed counter, because the fixed counter
+		 * doesn't support filtering whereas a programmable counter
+		 * would.
 		 */
-		if (fixed_ctr >= 0)
-			return pmu_fixed_ctr_update_inhibit_bits(fixed_ctr, flags);
-		else
+		if (fixed_ctr < 0 ||
+		    ((sbi_hart_has_extension(scratch, SBI_HART_EXT_SSCOFPMF) &&
+		      !sbi_hart_has_extension(scratch, SBI_HART_EXT_SMCNTRPMF))))
 			return SBI_EFAIL;
+
+		return pmu_fixed_ctr_update_inhibit_bits(fixed_ctr, flags);
 	}
 	ret = pmu_update_hw_mhpmevent(temp, ctr_idx, flags, event_idx, data);
 

---
base-commit: 4813a2042096b7860655761aad973723293a552e
change-id: 20260324-mcycle-fix-3fc5682eee9c


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi