Re: [PATCH 05/21] perf: arm_pmuv3: Check cntr_mask before using pmccntr

[Colton Lewis]

Hi Robin, thanks for the review

Robin Murphy <robin.mur...@arm.com> writes:

On 12/06/2026 8:28 pm, Colton Lewis wrote:

Check cntr_mask before using pmccntr to ensure it's available. With a
partitioned PMU, there may be instances where pmccntr is being used by
the guest and will be absent from cntr_mask.

Signed-off-by: Colton Lewis <coltonle...@google.com>
---
   drivers/perf/arm_pmuv3.c | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
index 1cceb1f614515..17bb1cfdc271c 100644
--- a/drivers/perf/arm_pmuv3.c
+++ b/drivers/perf/arm_pmuv3.c

@@ -1028,7 +1028,8 @@ static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc,

        /* Always prefer to place a cycle counter into the cycle counter. */
        if (armv8pmu_can_use_pmccntr(cpuc, event)) {

Something smells wrong here - we can use PMCCNTR but we can't?

-               if (!test_and_set_bit(ARMV8_PMU_CYCLE_IDX, cpuc->used_mask))
+               if (test_bit(ARMV8_PMU_CYCLE_IDX, cpu_pmu->cntr_mask) &&

...i.e. this test would seem to belong in the helper function.

+                   !test_and_set_bit(ARMV8_PMU_CYCLE_IDX, cpuc->used_mask))
                        return ARMV8_PMU_CYCLE_IDX;
                else if (armv8pmu_event_is_64bit(event) &&
                           armv8pmu_event_want_user_access(event) &&

Plus it also doesn't make much sense to fall through to the "we _could_
accommodate this event if only PMCCNTR was free" case here if we know
PMCCNTR can never be free because it's not even available.

I agree it would be a good idea to refactor these if statements.

Probably something like:

/* tests moved to `armv8pmu_can_use_pmccntr` */
if (armv8pmu_can_use_pmccntr()) {
        set_bit(ARMV8_PMU_CYCLE_IDX, cpu_pmu->used_mask);
        return ARMV8_PMU_CYCLE_IDX;
}

if (armv8pmu_event_is_64bit(event) &&
    armv8pmu_event_want_user_access(event) &&
    !armv8pmu_has_long_event(cpu_pmu))
        return -EAGAIN;