[PATCH v2 2/2] powerpc/perf: Fix unit_sel/cache_sel checks

[Michael Ellerman]

Madhavan Srinivasan <maddy at linux.vnet.ibm.com> writes:

> diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
> index 2efee3f196f5..9fd2e5c7a063 100644
> --- a/arch/powerpc/perf/isa207-common.c
> +++ b/arch/powerpc/perf/isa207-common.c
> @@ -285,10 +293,10 @@ int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp)
>  		 * have a cache selector of zero. The bank selector (bit 3) is
>  		 * irrelevant, as long as the rest of the value is 0.
>  		 */
> -		if (cache & 0x7)
> +		if (!cpu_has_feature(CPU_FTR_ARCH_300) && (cache & 0x7))
>  			return -1;
>  
> -	} else if (event & EVENT_IS_L1) {
> +	} else if (cpu_has_feature(CPU_FTR_ARCH_300) || (event & EVENT_IS_L1)) {
>  		mask  |= CNST_L1_QUAL_MASK;
>  		value |= CNST_L1_QUAL_VAL(cache);

I don't understand this. You're saying all non L2/L3 events are L1
events on Power9. That seems wrong.

>  	}
> @@ -391,11 +399,14 @@ int isa207_compute_mmcr(u64 event[], int n_ev,
>  		/* In continuous sampling mode, update SDAR on TLB miss */
>  		mmcra_sdar_mode(event[i], &mmcra);
>  
> -		if (event[i] & EVENT_IS_L1) {
> -			cache = event[i] >> EVENT_CACHE_SEL_SHIFT;
> -			mmcr1 |= (cache & 1) << MMCR1_IC_QUAL_SHIFT;
> -			cache >>= 1;
> -			mmcr1 |= (cache & 1) << MMCR1_DC_QUAL_SHIFT;
> +		if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> +			cache = dc_ic_rld_quad_l1_sel(event[i]);
> +			mmcr1 |= (cache) << MMCR1_DC_IC_QUAL_SHIFT;
> +		} else {
> +			if (event[i] & EVENT_IS_L1) {
> +				cache = dc_ic_rld_quad_l1_sel(event[i]);
> +				mmcr1 |= (cache) << MMCR1_DC_IC_QUAL_SHIFT;
> +			}
>  		}

I also don't understand this. Both legs of the if have the exact same
code, so at the very least it's factored badly.

But why are we doing it for all events on Power9?


cheers