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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Bypassing the fallback safety check for 64-bit user-access events when pmccntr is partitioned causes counter corruption.
--

commit 1df2bf6962076bf4d1bac00a5756584e40f646ca
Author: Colton Lewis <coltonlewis@google.com>

perf: arm_pmuv3: Check cntr_mask before using pmccntr

This commit adds a check for cntr_mask before using pmccntr to ensure it is
available. This is intended to handle partitioned PMUs where pmccntr might
be used by the guest and absent from the mask.

> 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 ba absent from cntr_mask.

This isn't a bug, but there's a typo (ba) in the commit message.

> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> index 1cceb1f614515..7ff3139dda893 100644
> --- a/drivers/perf/arm_pmuv3.c
> +++ b/drivers/perf/arm_pmuv3.c
> @@ -993,6 +993,9 @@ static bool armv8pmu_can_use_pmccntr(struct pmu_hw_events *cpuc,
>  	if (evtype != ARMV8_PMUV3_PERFCTR_CPU_CYCLES)
>  		return false;
>  
> +	if (!test_bit(ARMV8_PMU_CYCLE_IDX, cpu_pmu->cntr_mask))
> +		return false;
> +

Does moving this check into armv8pmu_can_use_pmccntr() introduce a regression
that bypasses the 64-bit user-access fallback safety check in
armv8pmu_get_event_idx()?

If pmccntr is reserved for the guest and removed from cntr_mask,
armv8pmu_can_use_pmccntr() will return false. Would this skip the -EAGAIN
safeguard for 64-bit user-access cycle events?

drivers/perf/arm_pmuv3.c:armv8pmu_get_event_idx() {
    ...
	if (armv8pmu_can_use_pmccntr(cpuc, event)) {
		if (!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) &&
			   !armv8pmu_has_long_event(cpu_pmu))
				return -EAGAIN;
	}
    ...
	if (armv8pmu_event_is_chained(event))
		return	armv8pmu_get_chain_idx(cpuc, cpu_pmu);
	else
		return armv8pmu_get_single_idx(cpuc, cpu_pmu);
}

If armv8pmu_can_use_pmccntr() returns false, the logic will fall through to
armv8pmu_get_single_idx() and allocate a 32-bit general-purpose counter for
the 64-bit user-access event.

Because the event maintains the ARMPMU_EVT_64BIT flag, will
armpmu_event_update() incorrectly calculate deltas using a 64-bit mask? If
the 32-bit hardware counter overflows, could this regression lead to massive
erroneous jumps in the counter?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260504211813.1804997-1-coltonlewis@google.com?part=5