[PATCH 1/2] powerpc/perf: Factor out bhrb functions

[PATCH 1/2] powerpc/perf: Factor out bhrb functions

[Madhavan Srinivasan]

Factor out the bhrb functions to "isa207-common.c"
to share with power9. Only code movement and no logic change

Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
 arch/powerpc/perf/isa207-common.c | 41 ++++++++++++++++++++++++++++++++
 arch/powerpc/perf/isa207-common.h |  9 +++++++
 arch/powerpc/perf/power8-pmu.c    | 49 ++-------------------------------------
 arch/powerpc/perf/power9-pmu.c    |  4 ++--
 4 files changed, 54 insertions(+), 49 deletions(-)

diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
index 50e598cf644b..ee4e3e89c04c 100644
--- a/arch/powerpc/perf/isa207-common.c
+++ b/arch/powerpc/perf/isa207-common.c
@@ -338,3 +338,44 @@ void isa207_disable_pmc(unsigned int pmc, unsigned long mmcr[])
 	if (pmc <= 3)
 		mmcr[1] &= ~(0xffUL << MMCR1_PMCSEL_SHIFT(pmc + 1));
 }
+
+u64 isa207_bhrb_filter_map(u64 branch_sample_type)
+{
+	u64 pmu_bhrb_filter = 0;
+
+	/* 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
+	 * in the basic PMC configuration of the accompanying regular
+	 * PMU event, we ignore any separate BHRB specific request.
+	 */
+
+	/* No branch filter requested */
+	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY)
+		return pmu_bhrb_filter;
+
+	/* Invalid branch filter options - HW does not support */
+	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_RETURN)
+		return -1;
+
+	if (branch_sample_type & PERF_SAMPLE_BRANCH_IND_CALL)
+		return -1;
+
+	if (branch_sample_type & PERF_SAMPLE_BRANCH_CALL)
+		return -1;
+
+	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_CALL) {
+		pmu_bhrb_filter |= MMCRA_IFM1;
+		return pmu_bhrb_filter;
+	}
+
+	/* Every thing else is unsupported */
+	return -1;
+}
+
+void isa207_config_bhrb(u64 pmu_bhrb_filter)
+{
+	/* Enable BHRB filter in PMU */
+	mtspr(SPRN_MMCRA, (mfspr(SPRN_MMCRA) | pmu_bhrb_filter));
+}
+
diff --git a/arch/powerpc/perf/isa207-common.h b/arch/powerpc/perf/isa207-common.h
index 90495f1580c7..e5e4182731da 100644
--- a/arch/powerpc/perf/isa207-common.h
+++ b/arch/powerpc/perf/isa207-common.h
@@ -244,6 +244,12 @@
 #define MMCRA_SDAR_MODE_TLB		(1ull << MMCRA_SDAR_MODE_SHIFT)
 #define MMCRA_IFM_SHIFT			30
 
+/* MMCRA IFM bits */
+#define MMCRA_IFM1		0x0000000040000000UL
+#define MMCRA_IFM2		0x0000000080000000UL
+#define MMCRA_IFM3		0x00000000C0000000UL
+
+
 /* MMCR1 Threshold Compare bit constant for power9 */
 #define p9_MMCRA_THR_CMP_SHIFT	45
 
@@ -261,4 +267,7 @@ int isa207_compute_mmcr(u64 event[], int n_ev,
 				struct perf_event *pevents[]);
 void isa207_disable_pmc(unsigned int pmc, unsigned long mmcr[]);
 
+u64 isa207_bhrb_filter_map(u64 branch_sample_type);
+void isa207_config_bhrb(u64 pmu_bhrb_filter);
+
 #endif
diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index d07186382f3a..91120bec4173 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -25,11 +25,6 @@ enum {
 
 #undef EVENT
 
-/* MMCRA IFM bits - POWER8 */
-#define	POWER8_MMCRA_IFM1		0x0000000040000000UL
-#define	POWER8_MMCRA_IFM2		0x0000000080000000UL
-#define	POWER8_MMCRA_IFM3		0x00000000C0000000UL
-
 /* PowerISA v2.07 format attribute structure*/
 extern struct attribute_group isa207_pmu_format_group;
 
@@ -195,46 +190,6 @@ static int power8_generic_events[] = {
 	[PERF_COUNT_HW_CACHE_MISSES] =			PM_LD_MISS_L1,
 };
 
-static u64 power8_bhrb_filter_map(u64 branch_sample_type)
-{
-	u64 pmu_bhrb_filter = 0;
-
-	/* 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
-	 * in the basic PMC configuration of the accompanying regular
-	 * PMU event, we ignore any separate BHRB specific request.
-	 */
-
-	/* No branch filter requested */
-	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY)
-		return pmu_bhrb_filter;
-
-	/* Invalid branch filter options - HW does not support */
-	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_RETURN)
-		return -1;
-
-	if (branch_sample_type & PERF_SAMPLE_BRANCH_IND_CALL)
-		return -1;
-
-	if (branch_sample_type & PERF_SAMPLE_BRANCH_CALL)
-		return -1;
-
-	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_CALL) {
-		pmu_bhrb_filter |= POWER8_MMCRA_IFM1;
-		return pmu_bhrb_filter;
-	}
-
-	/* Every thing else is unsupported */
-	return -1;
-}
-
-static void power8_config_bhrb(u64 pmu_bhrb_filter)
-{
-	/* Enable BHRB filter in PMU */
-	mtspr(SPRN_MMCRA, (mfspr(SPRN_MMCRA) | pmu_bhrb_filter));
-}
-
 #define C(x)	PERF_COUNT_HW_CACHE_##x
 
 /*
@@ -352,8 +307,8 @@ static struct power_pmu power8_pmu = {
 	.add_fields		= ISA207_ADD_FIELDS,
 	.test_adder		= ISA207_TEST_ADDER,
 	.compute_mmcr		= isa207_compute_mmcr,
-	.config_bhrb		= power8_config_bhrb,
-	.bhrb_filter_map	= power8_bhrb_filter_map,
+	.config_bhrb		= isa207_config_bhrb,
+	.bhrb_filter_map	= isa207_bhrb_filter_map,
 	.get_constraint		= isa207_get_constraint,
 	.get_alternatives	= power8_get_alternatives,
 	.disable_pmc		= isa207_disable_pmc,
diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
index 346010e8d463..56ad09801fff 100644
--- a/arch/powerpc/perf/power9-pmu.c
+++ b/arch/powerpc/perf/power9-pmu.c
@@ -380,8 +380,8 @@ static struct power_pmu power9_isa207_pmu = {
 	.add_fields		= ISA207_ADD_FIELDS,
 	.test_adder		= ISA207_TEST_ADDER,
 	.compute_mmcr		= isa207_compute_mmcr,
-	.config_bhrb		= power9_config_bhrb,
-	.bhrb_filter_map	= power9_bhrb_filter_map,
+	.config_bhrb		= isa207_config_bhrb,
+	.bhrb_filter_map	= isa207_bhrb_filter_map,
 	.get_constraint		= isa207_get_constraint,
 	.disable_pmc		= isa207_disable_pmc,
 	.flags			= PPMU_HAS_SIER | PPMU_ARCH_207S,
-- 
2.7.4

[PATCH 2/2] powerpc/perf: support new BHRB filtering modes

[Madhavan Srinivasan]

Add support for new BHRB filtering modes in
power9. Patch updates the some of the branch types
based on "Programming Note" information from
PowerISA v3.0 "BHRB Filtering" section.

Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
 arch/powerpc/perf/power9-pmu.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
index 56ad09801fff..7165f18934df 100644
--- a/arch/powerpc/perf/power9-pmu.c
+++ b/arch/powerpc/perf/power9-pmu.c
@@ -102,6 +102,11 @@ enum {
 #define POWER9_MMCRA_IFM1		0x0000000040000000UL
 #define POWER9_MMCRA_IFM2		0x0000000080000000UL
 #define POWER9_MMCRA_IFM3		0x00000000C0000000UL
+#define POWER9_MMCRA_FJ			0x0000000100000000UL
+#define POWER9_MMCRA_FR			0x0000000200000000UL
+#define POWER9_MMCRA_FC			0x0000000400000000UL
+#define POWER9_MMCRA_FU			0x0000000800000000UL
+#define POWER9_MMCRA_FD			0x0000001000000000UL
 
 /* PowerISA v2.07 format attribute structure*/
 extern struct attribute_group isa207_pmu_format_group;
@@ -240,14 +245,30 @@ static u64 power9_bhrb_filter_map(u64 branch_sample_type)
 		return pmu_bhrb_filter;
 
 	/* Invalid branch filter options - HW does not support */
-	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_RETURN)
-		return -1;
+	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_RETURN) {
+		pmu_bhrb_filter = POWER9_MMCRA_FC | POWER9_MMCRA_FJ;
+		return pmu_bhrb_filter;
+	}
+
+	if (branch_sample_type & PERF_SAMPLE_BRANCH_IND_CALL) {
+		pmu_bhrb_filter = POWER9_MMCRA_FD | POWER9_MMCRA_FR | POWER9_MMCRA_FJ;
+		return pmu_bhrb_filter;
+	}
+
+	if (branch_sample_type & PERF_SAMPLE_BRANCH_COND) {
+		pmu_bhrb_filter = POWER9_MMCRA_FU;
+		return pmu_bhrb_filter;
+	}
 
-	if (branch_sample_type & PERF_SAMPLE_BRANCH_IND_CALL)
-		return -1;
+	if (branch_sample_type & PERF_SAMPLE_BRANCH_IND_JUMP) {
+		pmu_bhrb_filter = POWER9_MMCRA_FD | POWER9_MMCRA_FC | POWER9_MMCRA_FR;
+		return pmu_bhrb_filter;
+	}
 
-	if (branch_sample_type & PERF_SAMPLE_BRANCH_CALL)
-		return -1;
+	if (branch_sample_type & PERF_SAMPLE_BRANCH_CALL) {
+		pmu_bhrb_filter = POWER9_MMCRA_FC;
+		return pmu_bhrb_filter;
+	}
 
 	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_CALL) {
 		pmu_bhrb_filter |= POWER9_MMCRA_IFM1;
-- 
2.7.4

Re: [PATCH 1/2] powerpc/perf: Factor out bhrb functions

[Anshuman Khandual]

On 12/24/2016 11:32 AM, Madhavan Srinivasan wrote:
> Factor out the bhrb functions to "isa207-common.c"
> to share with power9. Only code movement and no logic change

POWER9 is ISA 3.0, so dont you think the common code should not be in
a file named with "ISA 2.07" unless its going to be the same for both
POWER8 and POWER9 which is not the case here.

> 
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
>  arch/powerpc/perf/isa207-common.c | 41 ++++++++++++++++++++++++++++++++
>  arch/powerpc/perf/isa207-common.h |  9 +++++++
>  arch/powerpc/perf/power8-pmu.c    | 49 ++-------------------------------------
>  arch/powerpc/perf/power9-pmu.c    |  4 ++--
>  4 files changed, 54 insertions(+), 49 deletions(-)
> 
> diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
> index 50e598cf644b..ee4e3e89c04c 100644
> --- a/arch/powerpc/perf/isa207-common.c
> +++ b/arch/powerpc/perf/isa207-common.c
> @@ -338,3 +338,44 @@ void isa207_disable_pmc(unsigned int pmc, unsigned long mmcr[])
>  	if (pmc <= 3)
>  		mmcr[1] &= ~(0xffUL << MMCR1_PMCSEL_SHIFT(pmc + 1));
>  }
> +
> +u64 isa207_bhrb_filter_map(u64 branch_sample_type)
> +{
> +	u64 pmu_bhrb_filter = 0;
> +
> +	/* 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
> +	 * in the basic PMC configuration of the accompanying regular
> +	 * PMU event, we ignore any separate BHRB specific request.
> +	 */
> +
> +	/* No branch filter requested */
> +	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY)
> +		return pmu_bhrb_filter;
> +
> +	/* Invalid branch filter options - HW does not support */
> +	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_RETURN)
> +		return -1;
> +
> +	if (branch_sample_type & PERF_SAMPLE_BRANCH_IND_CALL)
> +		return -1;
> +
> +	if (branch_sample_type & PERF_SAMPLE_BRANCH_CALL)
> +		return -1;
> +
> +	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_CALL) {
> +		pmu_bhrb_filter |= MMCRA_IFM1;
> +		return pmu_bhrb_filter;
> +	}
> +
> +	/* Every thing else is unsupported */
> +	return -1;
> +}
> +
> +void isa207_config_bhrb(u64 pmu_bhrb_filter)
> +{
> +	/* Enable BHRB filter in PMU */
> +	mtspr(SPRN_MMCRA, (mfspr(SPRN_MMCRA) | pmu_bhrb_filter));
> +}
> +
> diff --git a/arch/powerpc/perf/isa207-common.h b/arch/powerpc/perf/isa207-common.h
> index 90495f1580c7..e5e4182731da 100644
> --- a/arch/powerpc/perf/isa207-common.h
> +++ b/arch/powerpc/perf/isa207-common.h
> @@ -244,6 +244,12 @@
>  #define MMCRA_SDAR_MODE_TLB		(1ull << MMCRA_SDAR_MODE_SHIFT)
>  #define MMCRA_IFM_SHIFT			30
> 
> +/* MMCRA IFM bits */
> +#define MMCRA_IFM1		0x0000000040000000UL
> +#define MMCRA_IFM2		0x0000000080000000UL
> +#define MMCRA_IFM3		0x00000000C0000000UL
> +
> +
>  /* MMCR1 Threshold Compare bit constant for power9 */
>  #define p9_MMCRA_THR_CMP_SHIFT	45
> 
> @@ -261,4 +267,7 @@ int isa207_compute_mmcr(u64 event[], int n_ev,
>  				struct perf_event *pevents[]);
>  void isa207_disable_pmc(unsigned int pmc, unsigned long mmcr[]);
> 
> +u64 isa207_bhrb_filter_map(u64 branch_sample_type);
> +void isa207_config_bhrb(u64 pmu_bhrb_filter);
> +
>  #endif
> diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
> index d07186382f3a..91120bec4173 100644
> --- a/arch/powerpc/perf/power8-pmu.c
> +++ b/arch/powerpc/perf/power8-pmu.c
> @@ -25,11 +25,6 @@ enum {
> 
>  #undef EVENT
> 
> -/* MMCRA IFM bits - POWER8 */
> -#define	POWER8_MMCRA_IFM1		0x0000000040000000UL
> -#define	POWER8_MMCRA_IFM2		0x0000000080000000UL
> -#define	POWER8_MMCRA_IFM3		0x00000000C0000000UL
> -
>  /* PowerISA v2.07 format attribute structure*/
>  extern struct attribute_group isa207_pmu_format_group;
> 
> @@ -195,46 +190,6 @@ static int power8_generic_events[] = {
>  	[PERF_COUNT_HW_CACHE_MISSES] =			PM_LD_MISS_L1,
>  };
> 
> -static u64 power8_bhrb_filter_map(u64 branch_sample_type)
> -{
> -	u64 pmu_bhrb_filter = 0;
> -
> -	/* 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
> -	 * in the basic PMC configuration of the accompanying regular
> -	 * PMU event, we ignore any separate BHRB specific request.
> -	 */
> -
> -	/* No branch filter requested */
> -	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY)
> -		return pmu_bhrb_filter;
> -
> -	/* Invalid branch filter options - HW does not support */
> -	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_RETURN)
> -		return -1;
> -
> -	if (branch_sample_type & PERF_SAMPLE_BRANCH_IND_CALL)
> -		return -1;
> -
> -	if (branch_sample_type & PERF_SAMPLE_BRANCH_CALL)
> -		return -1;
> -
> -	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_CALL) {
> -		pmu_bhrb_filter |= POWER8_MMCRA_IFM1;
> -		return pmu_bhrb_filter;
> -	}
> -
> -	/* Every thing else is unsupported */
> -	return -1;
> -}
> -
> -static void power8_config_bhrb(u64 pmu_bhrb_filter)
> -{
> -	/* Enable BHRB filter in PMU */
> -	mtspr(SPRN_MMCRA, (mfspr(SPRN_MMCRA) | pmu_bhrb_filter));
> -}
> -
>  #define C(x)	PERF_COUNT_HW_CACHE_##x
> 
>  /*
> @@ -352,8 +307,8 @@ static struct power_pmu power8_pmu = {
>  	.add_fields		= ISA207_ADD_FIELDS,
>  	.test_adder		= ISA207_TEST_ADDER,
>  	.compute_mmcr		= isa207_compute_mmcr,
> -	.config_bhrb		= power8_config_bhrb,
> -	.bhrb_filter_map	= power8_bhrb_filter_map,
> +	.config_bhrb		= isa207_config_bhrb,
> +	.bhrb_filter_map	= isa207_bhrb_filter_map,

This is alright. But

>  	.get_constraint		= isa207_get_constraint,
>  	.get_alternatives	= power8_get_alternatives,
>  	.disable_pmc		= isa207_disable_pmc,
> diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
> index 346010e8d463..56ad09801fff 100644
> --- a/arch/powerpc/perf/power9-pmu.c
> +++ b/arch/powerpc/perf/power9-pmu.c
> @@ -380,8 +380,8 @@ static struct power_pmu power9_isa207_pmu = {
>  	.add_fields		= ISA207_ADD_FIELDS,
>  	.test_adder		= ISA207_TEST_ADDER,
>  	.compute_mmcr		= isa207_compute_mmcr,
> -	.config_bhrb		= power9_config_bhrb,
> -	.bhrb_filter_map	= power9_bhrb_filter_map,
> +	.config_bhrb		= isa207_config_bhrb,
> +	.bhrb_filter_map	= isa207_bhrb_filter_map,

this is not. We are going to change the BHRB filter map for POWER9 with
additional stuff and the common function here can not be used on both
the processors.

Re: [PATCH 2/2] powerpc/perf: support new BHRB filtering modes

[Anshuman Khandual]

On 12/24/2016 11:32 AM, Madhavan Srinivasan wrote:
> Add support for new BHRB filtering modes in
> power9. Patch updates the some of the branch types
> based on "Programming Note" information from
> PowerISA v3.0 "BHRB Filtering" section.

Please reformat, reword, align the commit paragraph and provide more details.

> 
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
>  arch/powerpc/perf/power9-pmu.c | 33 +++++++++++++++++++++++++++------
>  1 file changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
> index 56ad09801fff..7165f18934df 100644
> --- a/arch/powerpc/perf/power9-pmu.c
> +++ b/arch/powerpc/perf/power9-pmu.c
> @@ -102,6 +102,11 @@ enum {
>  #define POWER9_MMCRA_IFM1		0x0000000040000000UL
>  #define POWER9_MMCRA_IFM2		0x0000000080000000UL
>  #define POWER9_MMCRA_IFM3		0x00000000C0000000UL
> +#define POWER9_MMCRA_FJ			0x0000000100000000UL
> +#define POWER9_MMCRA_FR			0x0000000200000000UL
> +#define POWER9_MMCRA_FC			0x0000000400000000UL
> +#define POWER9_MMCRA_FU			0x0000000800000000UL
> +#define POWER9_MMCRA_FD			0x0000001000000000UL

This requires more explanation on what kind of filtering they do and how.
Also it will be a good idea to explain the existing IFM1, IFM2, IFM3
filters as well.

> 
>  /* PowerISA v2.07 format attribute structure*/
>  extern struct attribute_group isa207_pmu_format_group;
> @@ -240,14 +245,30 @@ static u64 power9_bhrb_filter_map(u64 branch_sample_type)
>  		return pmu_bhrb_filter;
> 
>  	/* Invalid branch filter options - HW does not support */
> -	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_RETURN)
> -		return -1;
> +	if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_RETURN) {
> +		pmu_bhrb_filter = POWER9_MMCRA_FC | POWER9_MMCRA_FJ;
> +		return pmu_bhrb_filter;
> +	}
> +
> +	if (branch_sample_type & PERF_SAMPLE_BRANCH_IND_CALL) {
> +		pmu_bhrb_filter = POWER9_MMCRA_FD | POWER9_MMCRA_FR | POWER9_MMCRA_FJ;
> +		return pmu_bhrb_filter;
> +	}
> +
> +	if (branch_sample_type & PERF_SAMPLE_BRANCH_COND) {
> +		pmu_bhrb_filter = POWER9_MMCRA_FU;
> +		return pmu_bhrb_filter;
> +	}
> 
> -	if (branch_sample_type & PERF_SAMPLE_BRANCH_IND_CALL)
> -		return -1;
> +	if (branch_sample_type & PERF_SAMPLE_BRANCH_IND_JUMP) {
> +		pmu_bhrb_filter = POWER9_MMCRA_FD | POWER9_MMCRA_FC | POWER9_MMCRA_FR;
> +		return pmu_bhrb_filter;
> +	}
> 
> -	if (branch_sample_type & PERF_SAMPLE_BRANCH_CALL)
> -		return -1;
> +	if (branch_sample_type & PERF_SAMPLE_BRANCH_CALL) {
> +		pmu_bhrb_filter = POWER9_MMCRA_FC;
> +		return pmu_bhrb_filter;
> +	}

May be we can define something like POWER9_BHRB_FILTER_BRANCH_IND_JUMP as
(POWER9_MMCRA_FD | POWER9_MMCRA_FC | POWER9_MMCRA_FR) for all the filters
and use it instead.