Hi Robin,
On Thu, 1 Jun 2023, Robin Murphy wrote:
On 2023-06-01 04:01, Ilkka Koskinen wrote:
Some platforms may use e.g. different filtering mechanism and, thus,
may need different way to validate the events.
Signed-off-by: Ilkka Koskinen <ilkka@xxxxxxxxxxxxxxxxxxxxxx>
---
drivers/perf/arm_cspmu/arm_cspmu.c | 4 ++++
drivers/perf/arm_cspmu/arm_cspmu.h | 2 ++
2 files changed, 6 insertions(+)
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c
b/drivers/perf/arm_cspmu/arm_cspmu.c
index b4c4ef81c719..a26f484e06b1 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -593,6 +593,10 @@ static int arm_cspmu_get_event_idx(struct
arm_cspmu_hw_events *hw_events,
if (idx >= cspmu->num_logical_ctrs)
return -EAGAIN;
+ if (cspmu->impl.ops.validate_event &&
+ !cspmu->impl.ops.validate_event(cspmu, event))
+ return -EAGAIN;
Seems like this should be -EINVAL, or maybe the callback should return int so
it can make its own distinction (yes, I know the outer logic doesn't actually
propagate it, but there's no reason that couldn't improve at some point as
well).
Makes sense to me.
Another thought is that once we get into imp-def conditions for whether an
event is valid in itself, we presumably also need to consider imp-def
conditions for whether a given pair of events are compatible to be grouped?
That's a good point. I'll take a look at it.
Cheers, Ilkka
Thanks,
Robin.
+
set_bit(idx, hw_events->used_ctrs);
return idx;
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h
b/drivers/perf/arm_cspmu/arm_cspmu.h
index 4a29b921f7e8..0e5c316c96f9 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.h
+++ b/drivers/perf/arm_cspmu/arm_cspmu.h
@@ -106,6 +106,8 @@ struct arm_cspmu_impl_ops {
void (*set_ev_filter)(struct arm_cspmu *cspmu,
struct hw_perf_event *hwc,
u32 filter);
+ /* Implementation specific event validation */
+ bool (*validate_event)(struct arm_cspmu *cspmu, struct perf_event
*new);
/* Hide/show unsupported events */
umode_t (*event_attr_is_visible)(struct kobject *kobj,
struct attribute *attr, int unused);