All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm_pmu: Validate single/group leader events
@ 2022-04-08 20:33 ` Rob Herring
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2022-04-08 20:33 UTC (permalink / raw)
  To: Will Deacon, Mark Rutland; +Cc: Al Grant, linux-arm-kernel, linux-kernel

In the case where there is only a cycle counter available (i.e.
PMCR_EL0.N is 0) and an event other than CPU cycles is opened, the open
should fail as the event can never possibly be scheduled. However, the
event validation when an event is opened is skipped when the group
leader is opened. Fix this by always validating the group leader events.

Reported-by: Al Grant <al.grant@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/perf/arm_pmu.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 9694370651fa..59d3980b8ca2 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -400,6 +400,9 @@ validate_group(struct perf_event *event)
 	if (!validate_event(event->pmu, &fake_pmu, leader))
 		return -EINVAL;
 
+	if (event == leader)
+		return 0;
+
 	for_each_sibling_event(sibling, leader) {
 		if (!validate_event(event->pmu, &fake_pmu, sibling))
 			return -EINVAL;
@@ -489,12 +492,7 @@ __hw_perf_event_init(struct perf_event *event)
 		local64_set(&hwc->period_left, hwc->sample_period);
 	}
 
-	if (event->group_leader != event) {
-		if (validate_group(event) != 0)
-			return -EINVAL;
-	}
-
-	return 0;
+	return validate_group(event);
 }
 
 static int armpmu_event_init(struct perf_event *event)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH] arm_pmu: Validate single/group leader events
@ 2022-04-08 20:33 ` Rob Herring
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2022-04-08 20:33 UTC (permalink / raw)
  To: Will Deacon, Mark Rutland; +Cc: Al Grant, linux-arm-kernel, linux-kernel

In the case where there is only a cycle counter available (i.e.
PMCR_EL0.N is 0) and an event other than CPU cycles is opened, the open
should fail as the event can never possibly be scheduled. However, the
event validation when an event is opened is skipped when the group
leader is opened. Fix this by always validating the group leader events.

Reported-by: Al Grant <al.grant@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/perf/arm_pmu.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 9694370651fa..59d3980b8ca2 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -400,6 +400,9 @@ validate_group(struct perf_event *event)
 	if (!validate_event(event->pmu, &fake_pmu, leader))
 		return -EINVAL;
 
+	if (event == leader)
+		return 0;
+
 	for_each_sibling_event(sibling, leader) {
 		if (!validate_event(event->pmu, &fake_pmu, sibling))
 			return -EINVAL;
@@ -489,12 +492,7 @@ __hw_perf_event_init(struct perf_event *event)
 		local64_set(&hwc->period_left, hwc->sample_period);
 	}
 
-	if (event->group_leader != event) {
-		if (validate_group(event) != 0)
-			return -EINVAL;
-	}
-
-	return 0;
+	return validate_group(event);
 }
 
 static int armpmu_event_init(struct perf_event *event)
-- 
2.32.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm_pmu: Validate single/group leader events
  2022-04-08 20:33 ` Rob Herring
@ 2022-04-11 10:04   ` Mark Rutland
  -1 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2022-04-11 10:04 UTC (permalink / raw)
  To: Rob Herring; +Cc: Will Deacon, Al Grant, linux-arm-kernel, linux-kernel

Hi Rob,

On Fri, Apr 08, 2022 at 03:33:30PM -0500, Rob Herring wrote:
> In the case where there is only a cycle counter available (i.e.
> PMCR_EL0.N is 0) and an event other than CPU cycles is opened, the open
> should fail as the event can never possibly be scheduled. However, the
> event validation when an event is opened is skipped when the group
> leader is opened. Fix this by always validating the group leader events.
> 
> Reported-by: Al Grant <al.grant@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Rob Herring <robh@kernel.org>

This looks obviously correct to me, so FWIW:

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

Just to check, have you tested this (e.g. by running on a platform with
PMCR_EL0.N == 0, or hacking the PMU probing to report just the cycle counter)

Thanks,
Mark.

> ---
>  drivers/perf/arm_pmu.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index 9694370651fa..59d3980b8ca2 100644
> --- a/drivers/perf/arm_pmu.c
> +++ b/drivers/perf/arm_pmu.c
> @@ -400,6 +400,9 @@ validate_group(struct perf_event *event)
>  	if (!validate_event(event->pmu, &fake_pmu, leader))
>  		return -EINVAL;
>  
> +	if (event == leader)
> +		return 0;
> +
>  	for_each_sibling_event(sibling, leader) {
>  		if (!validate_event(event->pmu, &fake_pmu, sibling))
>  			return -EINVAL;
> @@ -489,12 +492,7 @@ __hw_perf_event_init(struct perf_event *event)
>  		local64_set(&hwc->period_left, hwc->sample_period);
>  	}
>  
> -	if (event->group_leader != event) {
> -		if (validate_group(event) != 0)
> -			return -EINVAL;
> -	}
> -
> -	return 0;
> +	return validate_group(event);
>  }
>  
>  static int armpmu_event_init(struct perf_event *event)
> -- 
> 2.32.0
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm_pmu: Validate single/group leader events
@ 2022-04-11 10:04   ` Mark Rutland
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2022-04-11 10:04 UTC (permalink / raw)
  To: Rob Herring; +Cc: Will Deacon, Al Grant, linux-arm-kernel, linux-kernel

Hi Rob,

On Fri, Apr 08, 2022 at 03:33:30PM -0500, Rob Herring wrote:
> In the case where there is only a cycle counter available (i.e.
> PMCR_EL0.N is 0) and an event other than CPU cycles is opened, the open
> should fail as the event can never possibly be scheduled. However, the
> event validation when an event is opened is skipped when the group
> leader is opened. Fix this by always validating the group leader events.
> 
> Reported-by: Al Grant <al.grant@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Rob Herring <robh@kernel.org>

This looks obviously correct to me, so FWIW:

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

Just to check, have you tested this (e.g. by running on a platform with
PMCR_EL0.N == 0, or hacking the PMU probing to report just the cycle counter)

Thanks,
Mark.

> ---
>  drivers/perf/arm_pmu.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index 9694370651fa..59d3980b8ca2 100644
> --- a/drivers/perf/arm_pmu.c
> +++ b/drivers/perf/arm_pmu.c
> @@ -400,6 +400,9 @@ validate_group(struct perf_event *event)
>  	if (!validate_event(event->pmu, &fake_pmu, leader))
>  		return -EINVAL;
>  
> +	if (event == leader)
> +		return 0;
> +
>  	for_each_sibling_event(sibling, leader) {
>  		if (!validate_event(event->pmu, &fake_pmu, sibling))
>  			return -EINVAL;
> @@ -489,12 +492,7 @@ __hw_perf_event_init(struct perf_event *event)
>  		local64_set(&hwc->period_left, hwc->sample_period);
>  	}
>  
> -	if (event->group_leader != event) {
> -		if (validate_group(event) != 0)
> -			return -EINVAL;
> -	}
> -
> -	return 0;
> +	return validate_group(event);
>  }
>  
>  static int armpmu_event_init(struct perf_event *event)
> -- 
> 2.32.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm_pmu: Validate single/group leader events
  2022-04-11 10:04   ` Mark Rutland
@ 2022-04-11 14:14     ` Rob Herring
  -1 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2022-04-11 14:14 UTC (permalink / raw)
  To: Mark Rutland; +Cc: Will Deacon, Al Grant, linux-arm-kernel, linux-kernel

On Mon, Apr 11, 2022 at 11:04:26AM +0100, Mark Rutland wrote:
> Hi Rob,
> 
> On Fri, Apr 08, 2022 at 03:33:30PM -0500, Rob Herring wrote:
> > In the case where there is only a cycle counter available (i.e.
> > PMCR_EL0.N is 0) and an event other than CPU cycles is opened, the open
> > should fail as the event can never possibly be scheduled. However, the
> > event validation when an event is opened is skipped when the group
> > leader is opened. Fix this by always validating the group leader events.
> > 
> > Reported-by: Al Grant <al.grant@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Signed-off-by: Rob Herring <robh@kernel.org>
> 
> This looks obviously correct to me, so FWIW:
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>

Thanks.

> Just to check, have you tested this (e.g. by running on a platform with
> PMCR_EL0.N == 0, or hacking the PMU probing to report just the cycle counter)

Yes, tested on FVP with 0 counters running the libperf evsel userspace 
access tests as that tries both the cycle counter and instruction 
counts.

Rob

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm_pmu: Validate single/group leader events
@ 2022-04-11 14:14     ` Rob Herring
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2022-04-11 14:14 UTC (permalink / raw)
  To: Mark Rutland; +Cc: Will Deacon, Al Grant, linux-arm-kernel, linux-kernel

On Mon, Apr 11, 2022 at 11:04:26AM +0100, Mark Rutland wrote:
> Hi Rob,
> 
> On Fri, Apr 08, 2022 at 03:33:30PM -0500, Rob Herring wrote:
> > In the case where there is only a cycle counter available (i.e.
> > PMCR_EL0.N is 0) and an event other than CPU cycles is opened, the open
> > should fail as the event can never possibly be scheduled. However, the
> > event validation when an event is opened is skipped when the group
> > leader is opened. Fix this by always validating the group leader events.
> > 
> > Reported-by: Al Grant <al.grant@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Signed-off-by: Rob Herring <robh@kernel.org>
> 
> This looks obviously correct to me, so FWIW:
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>

Thanks.

> Just to check, have you tested this (e.g. by running on a platform with
> PMCR_EL0.N == 0, or hacking the PMU probing to report just the cycle counter)

Yes, tested on FVP with 0 counters running the libperf evsel userspace 
access tests as that tries both the cycle counter and instruction 
counts.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm_pmu: Validate single/group leader events
  2022-04-11 14:14     ` Rob Herring
@ 2022-04-12 10:14       ` Mark Rutland
  -1 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2022-04-12 10:14 UTC (permalink / raw)
  To: Rob Herring, Will Deacon; +Cc: Al Grant, linux-arm-kernel, linux-kernel

On Mon, Apr 11, 2022 at 09:14:03AM -0500, Rob Herring wrote:
> On Mon, Apr 11, 2022 at 11:04:26AM +0100, Mark Rutland wrote:
> > Hi Rob,
> > 
> > On Fri, Apr 08, 2022 at 03:33:30PM -0500, Rob Herring wrote:
> > > In the case where there is only a cycle counter available (i.e.
> > > PMCR_EL0.N is 0) and an event other than CPU cycles is opened, the open
> > > should fail as the event can never possibly be scheduled. However, the
> > > event validation when an event is opened is skipped when the group
> > > leader is opened. Fix this by always validating the group leader events.
> > > 
> > > Reported-by: Al Grant <al.grant@arm.com>
> > > Cc: Will Deacon <will@kernel.org>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> > 
> > This looks obviously correct to me, so FWIW:
> > 
> > Acked-by: Mark Rutland <mark.rutland@arm.com>
> 
> Thanks.
> 
> > Just to check, have you tested this (e.g. by running on a platform with
> > PMCR_EL0.N == 0, or hacking the PMU probing to report just the cycle counter)
> 
> Yes, tested on FVP with 0 counters running the libperf evsel userspace 
> access tests as that tries both the cycle counter and instruction 
> counts.

Perfect, thanks!

Will, I assume that you'll pick this up.

Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm_pmu: Validate single/group leader events
@ 2022-04-12 10:14       ` Mark Rutland
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2022-04-12 10:14 UTC (permalink / raw)
  To: Rob Herring, Will Deacon; +Cc: Al Grant, linux-arm-kernel, linux-kernel

On Mon, Apr 11, 2022 at 09:14:03AM -0500, Rob Herring wrote:
> On Mon, Apr 11, 2022 at 11:04:26AM +0100, Mark Rutland wrote:
> > Hi Rob,
> > 
> > On Fri, Apr 08, 2022 at 03:33:30PM -0500, Rob Herring wrote:
> > > In the case where there is only a cycle counter available (i.e.
> > > PMCR_EL0.N is 0) and an event other than CPU cycles is opened, the open
> > > should fail as the event can never possibly be scheduled. However, the
> > > event validation when an event is opened is skipped when the group
> > > leader is opened. Fix this by always validating the group leader events.
> > > 
> > > Reported-by: Al Grant <al.grant@arm.com>
> > > Cc: Will Deacon <will@kernel.org>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> > 
> > This looks obviously correct to me, so FWIW:
> > 
> > Acked-by: Mark Rutland <mark.rutland@arm.com>
> 
> Thanks.
> 
> > Just to check, have you tested this (e.g. by running on a platform with
> > PMCR_EL0.N == 0, or hacking the PMU probing to report just the cycle counter)
> 
> Yes, tested on FVP with 0 counters running the libperf evsel userspace 
> access tests as that tries both the cycle counter and instruction 
> counts.

Perfect, thanks!

Will, I assume that you'll pick this up.

Mark.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm_pmu: Validate single/group leader events
  2022-04-08 20:33 ` Rob Herring
@ 2022-04-13 11:46   ` Will Deacon
  -1 siblings, 0 replies; 10+ messages in thread
From: Will Deacon @ 2022-04-13 11:46 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland
  Cc: catalin.marinas, kernel-team, Will Deacon, linux-kernel,
	Al Grant, linux-arm-kernel

On Fri, 8 Apr 2022 15:33:30 -0500, Rob Herring wrote:
> In the case where there is only a cycle counter available (i.e.
> PMCR_EL0.N is 0) and an event other than CPU cycles is opened, the open
> should fail as the event can never possibly be scheduled. However, the
> event validation when an event is opened is skipped when the group
> leader is opened. Fix this by always validating the group leader events.
> 
> 
> [...]

Applied to arm64 (for-next/fixes), thanks!

[1/1] arm_pmu: Validate single/group leader events
      https://git.kernel.org/arm64/c/e5c23779f93d

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm_pmu: Validate single/group leader events
@ 2022-04-13 11:46   ` Will Deacon
  0 siblings, 0 replies; 10+ messages in thread
From: Will Deacon @ 2022-04-13 11:46 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland
  Cc: catalin.marinas, kernel-team, Will Deacon, linux-kernel,
	Al Grant, linux-arm-kernel

On Fri, 8 Apr 2022 15:33:30 -0500, Rob Herring wrote:
> In the case where there is only a cycle counter available (i.e.
> PMCR_EL0.N is 0) and an event other than CPU cycles is opened, the open
> should fail as the event can never possibly be scheduled. However, the
> event validation when an event is opened is skipped when the group
> leader is opened. Fix this by always validating the group leader events.
> 
> 
> [...]

Applied to arm64 (for-next/fixes), thanks!

[1/1] arm_pmu: Validate single/group leader events
      https://git.kernel.org/arm64/c/e5c23779f93d

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-04-13 11:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-08 20:33 [PATCH] arm_pmu: Validate single/group leader events Rob Herring
2022-04-08 20:33 ` Rob Herring
2022-04-11 10:04 ` Mark Rutland
2022-04-11 10:04   ` Mark Rutland
2022-04-11 14:14   ` Rob Herring
2022-04-11 14:14     ` Rob Herring
2022-04-12 10:14     ` Mark Rutland
2022-04-12 10:14       ` Mark Rutland
2022-04-13 11:46 ` Will Deacon
2022-04-13 11:46   ` Will Deacon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.