Re: [PATCH 1/2] perf: arm_pmuv3: Factor out PMCCNTR_EL0 use conditions

[Mark Rutland <mark.rutland@arm.com>]

On Tue, Aug 12, 2025 at 04:08:29PM +0800, Yicong Yang wrote:
> From: Yicong Yang <yangyicong@hisilicon.com>
> 
> PMCCNTR_EL0 is preferred for counting CPU_CYCLES under certain
> conditions. Factor out the condition check to a separate function
> for further extension. Add documents for better understanding.
> No functional changes intended.
> 
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>

FWIW, splitting this oudt looks fine to me (with one nit below), so:

Acked-by: Mark Rutland <mark.rutland@arm.com>

> ---
>  drivers/perf/arm_pmuv3.c | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> index f6d7bab5d555..95c899d07df5 100644
> --- a/drivers/perf/arm_pmuv3.c
> +++ b/drivers/perf/arm_pmuv3.c
> @@ -978,6 +978,33 @@ static int armv8pmu_get_chain_idx(struct pmu_hw_events *cpuc,
>  	return -EAGAIN;
>  }
>  
> +static bool armv8pmu_can_use_pmccntr(struct pmu_hw_events *cpuc,
> +				     struct perf_event *event)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	unsigned long evtype = hwc->config_base & ARMV8_PMU_EVTYPE_EVENT;
> +
> +	/* PMCCNTR_EL0 can only be used for CPU_CYCLES event */
> +	if (evtype != ARMV8_PMUV3_PERFCTR_CPU_CYCLES)
> +		return false;

Nit: I don't think this comment is useful, and it could be deleted.

Mark.

> +
> +	/*
> +	 * A CPU_CYCLES event with threshold counting cannot use PMCCNTR_EL0
> +	 * since it lacks threshold support.
> +	 */
> +	if (armv8pmu_event_get_threshold(&event->attr))
> +		return false;
> +
> +	/*
> +	 * PMCCNTR_EL0 is not affected by BRBE controls like BRBCR_ELx.FZP.
> +	 * So don't use it for branch events.
> +	 */
> +	if (has_branch_stack(event))
> +		return false;
> +
> +	return true;
> +}
> +
>  static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc,
>  				  struct perf_event *event)
>  {
> @@ -986,8 +1013,7 @@ static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc,
>  	unsigned long evtype = hwc->config_base & ARMV8_PMU_EVTYPE_EVENT;
>  
>  	/* Always prefer to place a cycle counter into the cycle counter. */
> -	if ((evtype == ARMV8_PMUV3_PERFCTR_CPU_CYCLES) &&
> -	    !armv8pmu_event_get_threshold(&event->attr) && !has_branch_stack(event)) {
> +	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) &&
> -- 
> 2.24.0
>