arm_pmu: Validate single/group leader events

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 <[email protected]>

Cc Will Deacon <[email protected]>

Cc Mark Rutland <[email protected]>

Signed-off-by Rob Herring <[email protected]>

---
 drivers/perf/arm_pmu.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

drivers/perf/arm_pmu.c +4 -6

index 9694370651fa..59d3980b8ca2 100644
@@ -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)

Signature · 1 line

2.32.0

Hi Rob,

Quoted 6 lines · Rob Herring

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.

Quoted 4 lines

Reported-by Al Grant <[email protected]>

Cc Will Deacon <[email protected]>

Cc Mark Rutland <[email protected]>

Signed-off-by Rob Herring <[email protected]>

This looks obviously correct to me, so FWIW:

Acked-by Mark Rutland <[email protected]>

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)

Quoted 9 lines · Mark Rutland

Hi Rob,

Quoted 6 lines · Rob Herring

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.

Quoted 8 lines

Quoted 4 lines

Reported-by Al Grant <[email protected]>

Cc Will Deacon <[email protected]>

Cc Mark Rutland <[email protected]>

Signed-off-by Rob Herring <[email protected]>

This looks obviously correct to me, so FWIW:

Acked-by Mark Rutland <[email protected]>

Thanks.

Quoted 2 lines

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.

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.

[...]