[PATCH 1/2] powerpc/perf: Fix the threshold compare group constraint for power10

[PATCH 1/2] powerpc/perf: Fix the threshold compare group constraint for power10

[Kajol Jain]

Thresh compare bits for a event is used to program thresh compare
field in Monitor Mode Control Register A (MMCRA: 8-18 bits for power10).
When scheduling events as a group, all events in that group should
match value in threshold bits. Otherwise event open for the sibling
events should fail. But in the current code, incase thresh compare bits are
not valid, we are not failing in group_constraint function which can result
in invalid group schduling.

Fix the issue by returning -1 incase event is threshold and threshold
compare value is not valid in group_constraint function.

Patch also fixes the p10_thresh_cmp_val function to return -1,
incase threshold bits are not valid and changes corresponding check in
is_thresh_cmp_valid function to return false only when the thresh_cmp
value is less then 0.

Thresh control bits in the event code is used to program thresh_ctl
field in Monitor Mode Control Register A (MMCRA: 48-55). In below example,
the scheduling of group events PM_MRK_INST_CMPL (3534401e0) and
PM_THRESH_MET (34340101ec) is expected to fail as both event
request different thresh control bits.

Result before the patch changes:

[command]# perf stat -e "{r35340401e0,r34340101ec}" sleep 1

 Performance counter stats for 'sleep 1':

             8,482      r35340401e0
                 0      r34340101ec

       1.001474838 seconds time elapsed

       0.001145000 seconds user
       0.000000000 seconds sys

Result after the patch changes:

[command]# perf stat -e "{r35340401e0,r34340101ec}" sleep 1

 Performance counter stats for 'sleep 1':

     <not counted>      r35340401e0
   <not supported>      r34340101ec

       1.001499607 seconds time elapsed

       0.000204000 seconds user
       0.000760000 seconds sys

Fixes: 82d2c16b350f7 ("powerpc/perf: Adds support for programming
of Thresholding in P10")
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
 arch/powerpc/perf/isa207-common.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
index a74d382ecbb7..013b06af6fe6 100644
--- a/arch/powerpc/perf/isa207-common.c
+++ b/arch/powerpc/perf/isa207-common.c
@@ -108,7 +108,7 @@ static void mmcra_sdar_mode(u64 event, unsigned long *mmcra)
 		*mmcra |= MMCRA_SDAR_MODE_TLB;
 }
 
-static u64 p10_thresh_cmp_val(u64 value)
+static int p10_thresh_cmp_val(u64 value)
 {
 	int exp = 0;
 	u64 result = value;
@@ -139,7 +139,7 @@ static u64 p10_thresh_cmp_val(u64 value)
 		 * exponent is also zero.
 		 */
 		if (!(value & 0xC0) && exp)
-			result = 0;
+			result = -1;
 		else
 			result = (exp << 8) | value;
 	}
@@ -187,7 +187,7 @@ static bool is_thresh_cmp_valid(u64 event)
 	unsigned int cmp, exp;
 
 	if (cpu_has_feature(CPU_FTR_ARCH_31))
-		return p10_thresh_cmp_val(event) != 0;
+		return p10_thresh_cmp_val(event) >= 0;
 
 	/*
 	 * Check the mantissa upper two bits are not zero, unless the
@@ -502,7 +502,8 @@ int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp,
 			value |= CNST_THRESH_CTL_SEL_VAL(event >> EVENT_THRESH_SHIFT);
 			mask  |= p10_CNST_THRESH_CMP_MASK;
 			value |= p10_CNST_THRESH_CMP_VAL(p10_thresh_cmp_val(event_config1));
-		}
+		} else if (event_is_threshold(event))
+			return -1;
 	} else if (cpu_has_feature(CPU_FTR_ARCH_300))  {
 		if (event_is_threshold(event) && is_thresh_cmp_valid(event)) {
 			mask  |= CNST_THRESH_MASK;
-- 
2.31.1

[PATCH 2/2] powerpc/perf: Fix the threshold compare group constraint for power9

[Kajol Jain]

Thresh compare bits for a event is used to program thresh compare
field in Monitor Mode Control Register A (MMCRA: 9-18 bits for power9).
When scheduling events as a group, all events in that group should
match value in threshold bits (like thresh compare, thresh control,
thresh select). Otherwise event open for the sibling events should fail.
But in the current code, incase thresh compare bits are not valid,
we are not failing in group_constraint function which can result
in invalid group schduling.

Fix the issue by returning -1 incase event is threshold and threshold
compare value is not valid.

Thresh control bits in the event code is used to program thresh_ctl
field in Monitor Mode Control Register A (MMCRA: 48-55). In below example,
the scheduling of group events PM_MRK_INST_CMPL (873534401e0) and
PM_THRESH_MET (8734340101ec) is expected to fail as both event
request different thresh control bits and invalid thresh compare value.

Result before the patch changes:

[command]# perf stat -e "{r8735340401e0,r8734340101ec}" sleep 1

 Performance counter stats for 'sleep 1':

            11,048      r8735340401e0
             1,967      r8734340101ec

       1.001354036 seconds time elapsed

       0.001421000 seconds user
       0.000000000 seconds sys

Result after the patch changes:

[command]# perf stat -e "{r8735340401e0,r8734340101ec}" sleep 1
Error:
The sys_perf_event_open() syscall returned with 22 (Invalid argument)
for event (r8735340401e0).
/bin/dmesg | grep -i perf may provide additional information.

Fixes: 78a16d9fc1206 ("powerpc/perf: Avoid FAB_*_MATCH checks for power9")
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
 arch/powerpc/perf/isa207-common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
index 013b06af6fe6..bb5d64862bc9 100644
--- a/arch/powerpc/perf/isa207-common.c
+++ b/arch/powerpc/perf/isa207-common.c
@@ -508,7 +508,8 @@ int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp,
 		if (event_is_threshold(event) && is_thresh_cmp_valid(event)) {
 			mask  |= CNST_THRESH_MASK;
 			value |= CNST_THRESH_VAL(event >> EVENT_THRESH_SHIFT);
-		}
+		} else if (event_is_threshold(event))
+			return -1;
 	} else {
 		/*
 		 * Special case for PM_MRK_FAB_RSP_MATCH and PM_MRK_FAB_RSP_MATCH_CYC,
-- 
2.31.1

Re: [PATCH 1/2] powerpc/perf: Fix the threshold compare group constraint for power10

[Athira Rajeev]

> On 06-May-2022, at 11:40 AM, Kajol Jain <kjain@linux.ibm.com> wrote:
> 
> Thresh compare bits for a event is used to program thresh compare
> field in Monitor Mode Control Register A (MMCRA: 8-18 bits for power10).
> When scheduling events as a group, all events in that group should
> match value in threshold bits. Otherwise event open for the sibling
> events should fail. But in the current code, incase thresh compare bits are
> not valid, we are not failing in group_constraint function which can result
> in invalid group schduling.
> 
> Fix the issue by returning -1 incase event is threshold and threshold
> compare value is not valid in group_constraint function.
> 
> Patch also fixes the p10_thresh_cmp_val function to return -1,
> incase threshold bits are not valid and changes corresponding check in
> is_thresh_cmp_valid function to return false only when the thresh_cmp
> value is less then 0.
> 
> Thresh control bits in the event code is used to program thresh_ctl
> field in Monitor Mode Control Register A (MMCRA: 48-55). In below example,
> the scheduling of group events PM_MRK_INST_CMPL (3534401e0) and
> PM_THRESH_MET (34340101ec) is expected to fail as both event
> request different thresh control bits.
> 
> Result before the patch changes:
> 
> [command]# perf stat -e "{r35340401e0,r34340101ec}" sleep 1
> 
> Performance counter stats for 'sleep 1':
> 
>             8,482      r35340401e0
>                 0      r34340101ec
> 
>       1.001474838 seconds time elapsed
> 
>       0.001145000 seconds user
>       0.000000000 seconds sys
> 
> Result after the patch changes:
> 
> [command]# perf stat -e "{r35340401e0,r34340101ec}" sleep 1
> 
> Performance counter stats for 'sleep 1':
> 
>     <not counted>      r35340401e0
>   <not supported>      r34340101ec
> 
>       1.001499607 seconds time elapsed
> 
>       0.000204000 seconds user
>       0.000760000 seconds sys
> 
> Fixes: 82d2c16b350f7 ("powerpc/perf: Adds support for programming
> of Thresholding in P10")
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>

Reviewed-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>

Thanks
Athira
> ---
> arch/powerpc/perf/isa207-common.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
> index a74d382ecbb7..013b06af6fe6 100644
> --- a/arch/powerpc/perf/isa207-common.c
> +++ b/arch/powerpc/perf/isa207-common.c
> @@ -108,7 +108,7 @@ static void mmcra_sdar_mode(u64 event, unsigned long *mmcra)
> 		*mmcra |= MMCRA_SDAR_MODE_TLB;
> }
> 
> -static u64 p10_thresh_cmp_val(u64 value)
> +static int p10_thresh_cmp_val(u64 value)
> {
> 	int exp = 0;
> 	u64 result = value;
> @@ -139,7 +139,7 @@ static u64 p10_thresh_cmp_val(u64 value)
> 		 * exponent is also zero.
> 		 */
> 		if (!(value & 0xC0) && exp)
> -			result = 0;
> +			result = -1;
> 		else
> 			result = (exp << 8) | value;
> 	}
> @@ -187,7 +187,7 @@ static bool is_thresh_cmp_valid(u64 event)
> 	unsigned int cmp, exp;
> 
> 	if (cpu_has_feature(CPU_FTR_ARCH_31))
> -		return p10_thresh_cmp_val(event) != 0;
> +		return p10_thresh_cmp_val(event) >= 0;
> 
> 	/*
> 	 * Check the mantissa upper two bits are not zero, unless the
> @@ -502,7 +502,8 @@ int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp,
> 			value |= CNST_THRESH_CTL_SEL_VAL(event >> EVENT_THRESH_SHIFT);
> 			mask  |= p10_CNST_THRESH_CMP_MASK;
> 			value |= p10_CNST_THRESH_CMP_VAL(p10_thresh_cmp_val(event_config1));
> -		}
> +		} else if (event_is_threshold(event))
> +			return -1;
> 	} else if (cpu_has_feature(CPU_FTR_ARCH_300))  {
> 		if (event_is_threshold(event) && is_thresh_cmp_valid(event)) {
> 			mask  |= CNST_THRESH_MASK;
> -- 
> 2.31.1
> 

Re: [PATCH 2/2] powerpc/perf: Fix the threshold compare group constraint for power9

[Athira Rajeev]

> On 06-May-2022, at 11:40 AM, Kajol Jain <kjain@linux.ibm.com> wrote:
> 
> Thresh compare bits for a event is used to program thresh compare
> field in Monitor Mode Control Register A (MMCRA: 9-18 bits for power9).
> When scheduling events as a group, all events in that group should
> match value in threshold bits (like thresh compare, thresh control,
> thresh select). Otherwise event open for the sibling events should fail.
> But in the current code, incase thresh compare bits are not valid,
> we are not failing in group_constraint function which can result
> in invalid group schduling.
> 
> Fix the issue by returning -1 incase event is threshold and threshold
> compare value is not valid.
> 
> Thresh control bits in the event code is used to program thresh_ctl
> field in Monitor Mode Control Register A (MMCRA: 48-55). In below example,
> the scheduling of group events PM_MRK_INST_CMPL (873534401e0) and
> PM_THRESH_MET (8734340101ec) is expected to fail as both event
> request different thresh control bits and invalid thresh compare value.
> 
> Result before the patch changes:
> 
> [command]# perf stat -e "{r8735340401e0,r8734340101ec}" sleep 1
> 
> Performance counter stats for 'sleep 1':
> 
>            11,048      r8735340401e0
>             1,967      r8734340101ec
> 
>       1.001354036 seconds time elapsed
> 
>       0.001421000 seconds user
>       0.000000000 seconds sys
> 
> Result after the patch changes:
> 
> [command]# perf stat -e "{r8735340401e0,r8734340101ec}" sleep 1
> Error:
> The sys_perf_event_open() syscall returned with 22 (Invalid argument)
> for event (r8735340401e0).
> /bin/dmesg | grep -i perf may provide additional information.
> 
> Fixes: 78a16d9fc1206 ("powerpc/perf: Avoid FAB_*_MATCH checks for power9")
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>

Reviewed-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>

Thanks
Athira
> ---
> arch/powerpc/perf/isa207-common.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
> index 013b06af6fe6..bb5d64862bc9 100644
> --- a/arch/powerpc/perf/isa207-common.c
> +++ b/arch/powerpc/perf/isa207-common.c
> @@ -508,7 +508,8 @@ int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp,
> 		if (event_is_threshold(event) && is_thresh_cmp_valid(event)) {
> 			mask  |= CNST_THRESH_MASK;
> 			value |= CNST_THRESH_VAL(event >> EVENT_THRESH_SHIFT);
> -		}
> +		} else if (event_is_threshold(event))
> +			return -1;
> 	} else {
> 		/*
> 		 * Special case for PM_MRK_FAB_RSP_MATCH and PM_MRK_FAB_RSP_MATCH_CYC,
> -- 
> 2.31.1
> 

Re: [PATCH 1/2] powerpc/perf: Fix the threshold compare group constraint for power10

[Michael Ellerman]

On Fri, 6 May 2022 11:40:14 +0530, Kajol Jain wrote:
> Thresh compare bits for a event is used to program thresh compare
> field in Monitor Mode Control Register A (MMCRA: 8-18 bits for power10).
> When scheduling events as a group, all events in that group should
> match value in threshold bits. Otherwise event open for the sibling
> events should fail. But in the current code, incase thresh compare bits are
> not valid, we are not failing in group_constraint function which can result
> in invalid group schduling.
> 
> [...]

Applied to powerpc/next.

[1/2] powerpc/perf: Fix the threshold compare group constraint for power10
      https://git.kernel.org/powerpc/c/505d31650ba96d6032313480fdb566d289a4698c
[2/2] powerpc/perf: Fix the threshold compare group constraint for power9
      https://git.kernel.org/powerpc/c/ab0cc6bbf0c812731c703ec757fcc3fc3a457a34

cheers