Re: [PATCH V8 08/10] powerpc, perf: Change POWER8 PMU configuration to work with SW filters

[Daniel Axtens <dja@axtens.net>]

[-- Attachment #1: Type: text/plain, Size: 3597 bytes --]

On Mon, 2015-06-08 at 17:08 +0530, Anshuman Khandual wrote:
> The kernel now supports SW based branch filters for book3s systems with
> some specific requirements while dealing with HW supported branch filters
> in order to achieve overall OR semantics prevailing in perf branch stack
> sampling framework. This patch adapts the BHRB branch filter configuration
> to meet those protocols. POWER8 PMU can only handle one HW based branch
> filter request at any point of time. For all other combinations PMU will
> pass it on to the SW.
> 
> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> ---
>  arch/powerpc/perf/power8-pmu.c | 51 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 44 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
> index 5e17cb5..8fccf6c 100644
> --- a/arch/powerpc/perf/power8-pmu.c
> +++ b/arch/powerpc/perf/power8-pmu.c
> @@ -656,6 +656,16 @@ static int power8_generic_events[] = {
>  

This is, I think, the third time you've modified this function in this
patch series. I appreciate the fact that you're trying to keep logical
changes separate, but it seems to me like this change might be able to
be combined with patch 4, and given a single commit message that clearly
explains the complete scope of the changes.
>  static u64 power8_bhrb_filter_map(u64 branch_sample_type, u64 *bhrb_filter)
>  {
> +	u64 x, pmu_bhrb_filter;
> +
> +	pmu_bhrb_filter = 0;
> +	*bhrb_filter = 0;
> +
> +	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY) {
> +		*bhrb_filter = PERF_SAMPLE_BRANCH_ANY;
> +		return pmu_bhrb_filter;
> +	}
> +
>  	/* BHRB and regular PMU events share the same privilege state
>  	 * filter configuration. BHRB is always recorded along with a
>  	 * regular PMU event. As the privilege state filter is handled
> @@ -666,15 +676,42 @@ static u64 power8_bhrb_filter_map(u64 branch_sample_type, u64 *bhrb_filter)
>  	/* Ignore user, kernel, hv bits */
>  	branch_sample_type &= ~PERF_SAMPLE_BRANCH_PLM_ALL;
>  
> -	/* No branch filter requested */
> -	if (branch_sample_type == PERF_SAMPLE_BRANCH_ANY)
> -		return 0;
> +	/*
> +	 * POWER8 does not support ORing of PMU HW branch filters. Hence
> +	 * if multiple branch filters are requested which may include filters
> +	 * supported in PMU, still go ahead and clear the PMU based HW branch
> +	 * filter component as in this case all the filters will be processed
> +	 * in SW.
> +	 */
>  
> -	if (branch_sample_type == PERF_SAMPLE_BRANCH_ANY_CALL)
> -		return POWER8_MMCRA_IFM1;
> +	for_each_branch_sample_type(x) {
> +		/* Ignore privilege branch filters */
> +		if ((x == PERF_SAMPLE_BRANCH_USER)
> +			|| (x == PERF_SAMPLE_BRANCH_KERNEL)
> +				|| (x == PERF_SAMPLE_BRANCH_HV))
> +			continue;
> +
> +		if (!(branch_sample_type & x))
> +			continue;
> +
> +		/* Supported individual PMU branch filters */
> +		if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_CALL) {
> +			branch_sample_type &= ~PERF_SAMPLE_BRANCH_ANY_CALL;
> +			if (branch_sample_type) {
> +				/* Multiple filters will be processed in SW */
> +				pmu_bhrb_filter = 0;
> +				*bhrb_filter = 0;
> +				return pmu_bhrb_filter;
> +			} else {
> +				/* Individual filter will be processed in HW */
> +				pmu_bhrb_filter |= POWER8_MMCRA_IFM1;
> +				*bhrb_filter    |= PERF_SAMPLE_BRANCH_ANY_CALL;
> +				return pmu_bhrb_filter;
> +			}
> +		}
> +	}
>  
> -	/* Every thing else is unsupported */
> -	return -1;
> +	return pmu_bhrb_filter;
>  }

Regards,
Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 860 bytes --]