* [PATCH v3 0/4] perf: ampere: Add support for Ampere SoC PMUs
@ 2023-06-07 8:31 Ilkka Koskinen
2023-06-07 8:31 ` [PATCH v3 1/4] perf: arm_cspmu: Split 64-bit write to 32-bit writes Ilkka Koskinen
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Ilkka Koskinen @ 2023-06-07 8:31 UTC (permalink / raw)
To: Jonathan Corbet, Will Deacon, Mark Rutland, Besar Wicaksono,
Suzuki K Poulose, Robin Murphy
Cc: linux-doc, linux-kernel, linux-arm-kernel, Ilkka Koskinen
Changes since v2:
* Changed to use supports_64bits_atomics() and replaced the split writes
with lo_hi_writeq()
* Added implementation specific group validation to patch 3
* Dropped shared interrupt patch
* Removed unnecessary filter_enable parameter from ampere module
* Added group validation to ampere module
Changes since v1:
* Rather than creating a completely new driver, implemented as a submodule
of Arm CoreSight PMU driver
* Fixed shared filter handling
Ilkka Koskinen (4):
perf: arm_cspmu: Split 64-bit write to 32-bit writes
perf: arm_cspmu: Support implementation specific filters
perf: arm_cspmu: Support implementation specific validation
perf: arm_cspmu: ampere_cspmu: Add support for Ampere SoC PMU
.../admin-guide/perf/ampere_cspmu.rst | 29 +++
drivers/perf/arm_cspmu/Makefile | 2 +-
drivers/perf/arm_cspmu/ampere_cspmu.c | 243 ++++++++++++++++++
drivers/perf/arm_cspmu/ampere_cspmu.h | 17 ++
drivers/perf/arm_cspmu/arm_cspmu.c | 33 ++-
drivers/perf/arm_cspmu/arm_cspmu.h | 8 +
6 files changed, 327 insertions(+), 5 deletions(-)
create mode 100644 Documentation/admin-guide/perf/ampere_cspmu.rst
create mode 100644 drivers/perf/arm_cspmu/ampere_cspmu.c
create mode 100644 drivers/perf/arm_cspmu/ampere_cspmu.h
--
2.40.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/4] perf: arm_cspmu: Split 64-bit write to 32-bit writes
2023-06-07 8:31 [PATCH v3 0/4] perf: ampere: Add support for Ampere SoC PMUs Ilkka Koskinen
@ 2023-06-07 8:31 ` Ilkka Koskinen
2023-06-20 5:12 ` Besar Wicaksono
2023-06-07 8:31 ` [PATCH v3 2/4] perf: arm_cspmu: Support implementation specific filters Ilkka Koskinen
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Ilkka Koskinen @ 2023-06-07 8:31 UTC (permalink / raw)
To: Jonathan Corbet, Will Deacon, Mark Rutland, Besar Wicaksono,
Suzuki K Poulose, Robin Murphy
Cc: linux-doc, linux-kernel, linux-arm-kernel, Ilkka Koskinen
Split the 64-bit register accesses if 64-bit access is not supported
by the PMU.
Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
---
drivers/perf/arm_cspmu/arm_cspmu.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index a3f1c410b417..f8b4a149eb88 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -702,7 +702,10 @@ static void arm_cspmu_write_counter(struct perf_event *event, u64 val)
if (use_64b_counter_reg(cspmu)) {
offset = counter_offset(sizeof(u64), event->hw.idx);
- writeq(val, cspmu->base1 + offset);
+ if (supports_64bit_atomics(cspmu))
+ writeq(val, cspmu->base1 + offset);
+ else
+ lo_hi_writeq(val, cspmu->base1 + offset);
} else {
offset = counter_offset(sizeof(u32), event->hw.idx);
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/4] perf: arm_cspmu: Support implementation specific filters
2023-06-07 8:31 [PATCH v3 0/4] perf: ampere: Add support for Ampere SoC PMUs Ilkka Koskinen
2023-06-07 8:31 ` [PATCH v3 1/4] perf: arm_cspmu: Split 64-bit write to 32-bit writes Ilkka Koskinen
@ 2023-06-07 8:31 ` Ilkka Koskinen
2023-06-20 5:39 ` Besar Wicaksono
2023-06-07 8:31 ` [PATCH v3 3/4] perf: arm_cspmu: Support implementation specific validation Ilkka Koskinen
2023-06-07 8:31 ` [PATCH v3 4/4] perf: arm_cspmu: ampere_cspmu: Add support for Ampere SoC PMU Ilkka Koskinen
3 siblings, 1 reply; 10+ messages in thread
From: Ilkka Koskinen @ 2023-06-07 8:31 UTC (permalink / raw)
To: Jonathan Corbet, Will Deacon, Mark Rutland, Besar Wicaksono,
Suzuki K Poulose, Robin Murphy
Cc: linux-doc, linux-kernel, linux-arm-kernel, Ilkka Koskinen
Generic filters aren't used in all the platforms. Instead, the platforms
may use different means to filter events. Add support for implementation
specific filters.
Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
---
drivers/perf/arm_cspmu/arm_cspmu.c | 8 ++++++--
drivers/perf/arm_cspmu/arm_cspmu.h | 4 ++++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index f8b4a149eb88..72ca4f56347c 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -122,6 +122,9 @@
static unsigned long arm_cspmu_cpuhp_state;
+static void arm_cspmu_set_ev_filter(struct arm_cspmu *cspmu,
+ struct hw_perf_event *hwc, u32 filter);
+
/*
* In CoreSight PMU architecture, all of the MMIO registers are 32-bit except
* counter register. The counter register can be implemented as 32-bit or 64-bit
@@ -432,6 +435,7 @@ static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
CHECK_DEFAULT_IMPL_OPS(impl_ops, event_type);
CHECK_DEFAULT_IMPL_OPS(impl_ops, event_filter);
CHECK_DEFAULT_IMPL_OPS(impl_ops, event_attr_is_visible);
+ CHECK_DEFAULT_IMPL_OPS(impl_ops, set_ev_filter);
return 0;
}
@@ -798,7 +802,7 @@ static inline void arm_cspmu_set_event(struct arm_cspmu *cspmu,
writel(hwc->config, cspmu->base0 + offset);
}
-static inline void arm_cspmu_set_ev_filter(struct arm_cspmu *cspmu,
+static void arm_cspmu_set_ev_filter(struct arm_cspmu *cspmu,
struct hw_perf_event *hwc,
u32 filter)
{
@@ -832,7 +836,7 @@ static void arm_cspmu_start(struct perf_event *event, int pmu_flags)
arm_cspmu_set_cc_filter(cspmu, filter);
} else {
arm_cspmu_set_event(cspmu, hwc);
- arm_cspmu_set_ev_filter(cspmu, hwc, filter);
+ cspmu->impl.ops.set_ev_filter(cspmu, hwc, filter);
}
hwc->state = 0;
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
index 51323b175a4a..f89ae2077164 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.h
+++ b/drivers/perf/arm_cspmu/arm_cspmu.h
@@ -102,6 +102,10 @@ struct arm_cspmu_impl_ops {
u32 (*event_type)(const struct perf_event *event);
/* Decode filter value from configs */
u32 (*event_filter)(const struct perf_event *event);
+ /* Set event filter */
+ void (*set_ev_filter)(struct arm_cspmu *cspmu,
+ struct hw_perf_event *hwc,
+ u32 filter);
/* Hide/show unsupported events */
umode_t (*event_attr_is_visible)(struct kobject *kobj,
struct attribute *attr, int unused);
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 3/4] perf: arm_cspmu: Support implementation specific validation
2023-06-07 8:31 [PATCH v3 0/4] perf: ampere: Add support for Ampere SoC PMUs Ilkka Koskinen
2023-06-07 8:31 ` [PATCH v3 1/4] perf: arm_cspmu: Split 64-bit write to 32-bit writes Ilkka Koskinen
2023-06-07 8:31 ` [PATCH v3 2/4] perf: arm_cspmu: Support implementation specific filters Ilkka Koskinen
@ 2023-06-07 8:31 ` Ilkka Koskinen
2023-06-20 11:44 ` Robin Murphy
2023-06-07 8:31 ` [PATCH v3 4/4] perf: arm_cspmu: ampere_cspmu: Add support for Ampere SoC PMU Ilkka Koskinen
3 siblings, 1 reply; 10+ messages in thread
From: Ilkka Koskinen @ 2023-06-07 8:31 UTC (permalink / raw)
To: Jonathan Corbet, Will Deacon, Mark Rutland, Besar Wicaksono,
Suzuki K Poulose, Robin Murphy
Cc: linux-doc, linux-kernel, linux-arm-kernel, Ilkka Koskinen
Some platforms may use e.g. different filtering mechanism and, thus,
may need different way to validate the events and group.
Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
---
drivers/perf/arm_cspmu/arm_cspmu.c | 13 ++++++++++++-
drivers/perf/arm_cspmu/arm_cspmu.h | 4 ++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 72ca4f56347c..9021d1878250 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -559,7 +559,7 @@ static void arm_cspmu_disable(struct pmu *pmu)
static int arm_cspmu_get_event_idx(struct arm_cspmu_hw_events *hw_events,
struct perf_event *event)
{
- int idx;
+ int idx, ret;
struct arm_cspmu *cspmu = to_arm_cspmu(event->pmu);
if (supports_cycle_counter(cspmu)) {
@@ -593,6 +593,12 @@ 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) {
+ ret = cspmu->impl.ops.validate_event(cspmu, event);
+ if (ret)
+ return ret;
+ }
+
set_bit(idx, hw_events->used_ctrs);
return idx;
@@ -618,6 +624,7 @@ static bool arm_cspmu_validate_event(struct pmu *pmu,
*/
static bool arm_cspmu_validate_group(struct perf_event *event)
{
+ struct arm_cspmu *cspmu = to_arm_cspmu(event->pmu);
struct perf_event *sibling, *leader = event->group_leader;
struct arm_cspmu_hw_events fake_hw_events;
@@ -635,6 +642,10 @@ static bool arm_cspmu_validate_group(struct perf_event *event)
return false;
}
+ if (cspmu->impl.ops.validate_group &&
+ cspmu->impl.ops.validate_group(event))
+ return false;
+
return arm_cspmu_validate_event(event->pmu, &fake_hw_events, event);
}
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
index f89ae2077164..291cedb196ea 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.h
+++ b/drivers/perf/arm_cspmu/arm_cspmu.h
@@ -106,6 +106,10 @@ struct arm_cspmu_impl_ops {
void (*set_ev_filter)(struct arm_cspmu *cspmu,
struct hw_perf_event *hwc,
u32 filter);
+ /* Implementation specific group validation */
+ int (*validate_group)(struct perf_event *event);
+ /* Implementation specific event validation */
+ int (*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);
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 4/4] perf: arm_cspmu: ampere_cspmu: Add support for Ampere SoC PMU
2023-06-07 8:31 [PATCH v3 0/4] perf: ampere: Add support for Ampere SoC PMUs Ilkka Koskinen
` (2 preceding siblings ...)
2023-06-07 8:31 ` [PATCH v3 3/4] perf: arm_cspmu: Support implementation specific validation Ilkka Koskinen
@ 2023-06-07 8:31 ` Ilkka Koskinen
3 siblings, 0 replies; 10+ messages in thread
From: Ilkka Koskinen @ 2023-06-07 8:31 UTC (permalink / raw)
To: Jonathan Corbet, Will Deacon, Mark Rutland, Besar Wicaksono,
Suzuki K Poulose, Robin Murphy
Cc: linux-doc, linux-kernel, linux-arm-kernel, Ilkka Koskinen
Ampere SoC PMU follows CoreSight PMU architecture. It uses implementation
specific registers to filter events rather than PMEVFILTnR registers.
Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
---
.../admin-guide/perf/ampere_cspmu.rst | 29 +++
drivers/perf/arm_cspmu/Makefile | 2 +-
drivers/perf/arm_cspmu/ampere_cspmu.c | 243 ++++++++++++++++++
drivers/perf/arm_cspmu/ampere_cspmu.h | 17 ++
drivers/perf/arm_cspmu/arm_cspmu.c | 7 +
5 files changed, 297 insertions(+), 1 deletion(-)
create mode 100644 Documentation/admin-guide/perf/ampere_cspmu.rst
create mode 100644 drivers/perf/arm_cspmu/ampere_cspmu.c
create mode 100644 drivers/perf/arm_cspmu/ampere_cspmu.h
diff --git a/Documentation/admin-guide/perf/ampere_cspmu.rst b/Documentation/admin-guide/perf/ampere_cspmu.rst
new file mode 100644
index 000000000000..94f93f5aee6c
--- /dev/null
+++ b/Documentation/admin-guide/perf/ampere_cspmu.rst
@@ -0,0 +1,29 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============================================
+Ampere SoC Performance Monitoring Unit (PMU)
+============================================
+
+Ampere SoC PMU is a generic PMU IP that follows Arm CoreSight PMU architecture.
+Therefore, the driver is implemented as a submodule of arm_cspmu driver. At the
+first phase it's used for counting MCU events on AmpereOne.
+
+
+MCU PMU events
+--------------
+
+The PMU driver supports setting filters for "rank", "bank", and "threshold".
+Note, that the filters are per PMU instance rather than per event.
+
+
+Example for perf tool use::
+
+ / # perf list ampere
+
+ ampere_mcu_pmu_0/act_sent/ [Kernel PMU event]
+ <...>
+ ampere_mcu_pmu_1/rd_sent/ [Kernel PMU event]
+ <...>
+
+ / # perf stat -a -e ampere_mcu_pmu_0/act_sent,bank=5,rank=3,threshold=2/,ampere_mcu_pmu_1/rd_sent/ \
+ sleep 1
diff --git a/drivers/perf/arm_cspmu/Makefile b/drivers/perf/arm_cspmu/Makefile
index fedb17df982d..b80a8bd8da54 100644
--- a/drivers/perf/arm_cspmu/Makefile
+++ b/drivers/perf/arm_cspmu/Makefile
@@ -3,4 +3,4 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_ARM_CORESIGHT_PMU_ARCH_SYSTEM_PMU) += arm_cspmu_module.o
-arm_cspmu_module-y := arm_cspmu.o nvidia_cspmu.o
+arm_cspmu_module-y := arm_cspmu.o nvidia_cspmu.o ampere_cspmu.o
diff --git a/drivers/perf/arm_cspmu/ampere_cspmu.c b/drivers/perf/arm_cspmu/ampere_cspmu.c
new file mode 100644
index 000000000000..1c57feb4b6ce
--- /dev/null
+++ b/drivers/perf/arm_cspmu/ampere_cspmu.c
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Ampere SoC PMU (Performance Monitor Unit)
+ *
+ * Copyright (c) 2023, Ampere Computing LLC
+ */
+
+#include "ampere_cspmu.h"
+
+#define PMAUXR0 0xD80
+#define PMAUXR1 0xD84
+#define PMAUXR2 0xD88
+#define PMAUXR3 0xD8C
+
+#define to_ampere_cspmu_ctx(cspmu) ((struct ampere_cspmu_ctx *)(cspmu->impl.ctx))
+
+struct ampere_cspmu_ctx {
+ const char *name;
+ struct attribute **event_attr;
+ struct attribute **format_attr;
+};
+
+#define SOC_PMU_EVENT_ATTR_EXTRACTOR(_name, _config, _start, _end) \
+ static inline u32 get_##_name(const struct perf_event *event) \
+ { \
+ return FIELD_GET(GENMASK_ULL(_end, _start), \
+ event->attr._config); \
+ } \
+
+SOC_PMU_EVENT_ATTR_EXTRACTOR(event, config, 0, 8);
+SOC_PMU_EVENT_ATTR_EXTRACTOR(threshold, config1, 0, 7);
+SOC_PMU_EVENT_ATTR_EXTRACTOR(rank, config1, 8, 23);
+SOC_PMU_EVENT_ATTR_EXTRACTOR(bank, config2, 0, 31);
+
+static struct attribute *ampereone_mcu_pmu_event_attrs[] = {
+ ARM_CSPMU_EVENT_ATTR(cycle_count, 0x00),
+ ARM_CSPMU_EVENT_ATTR(act_sent, 0x01),
+ ARM_CSPMU_EVENT_ATTR(pre_sent, 0x02),
+ ARM_CSPMU_EVENT_ATTR(rd_sent, 0x03),
+ ARM_CSPMU_EVENT_ATTR(rda_sent, 0x04),
+ ARM_CSPMU_EVENT_ATTR(wr_sent, 0x05),
+ ARM_CSPMU_EVENT_ATTR(wra_sent, 0x06),
+ ARM_CSPMU_EVENT_ATTR(pd_entry_vld, 0x07),
+ ARM_CSPMU_EVENT_ATTR(sref_entry_vld, 0x08),
+ ARM_CSPMU_EVENT_ATTR(prea_sent, 0x09),
+ ARM_CSPMU_EVENT_ATTR(pre_sb_sent, 0x0a),
+ ARM_CSPMU_EVENT_ATTR(ref_sent, 0x0b),
+ ARM_CSPMU_EVENT_ATTR(rfm_sent, 0x0c),
+ ARM_CSPMU_EVENT_ATTR(ref_sb_sent, 0x0d),
+ ARM_CSPMU_EVENT_ATTR(rfm_sb_sent, 0x0e),
+ ARM_CSPMU_EVENT_ATTR(rd_rda_sent, 0x0f),
+ ARM_CSPMU_EVENT_ATTR(wr_wra_sent, 0x10),
+ ARM_CSPMU_EVENT_ATTR(raw_hazard, 0x11),
+ ARM_CSPMU_EVENT_ATTR(war_hazard, 0x12),
+ ARM_CSPMU_EVENT_ATTR(waw_hazard, 0x13),
+ ARM_CSPMU_EVENT_ATTR(rar_hazard, 0x14),
+ ARM_CSPMU_EVENT_ATTR(raw_war_waw_hazard, 0x15),
+ ARM_CSPMU_EVENT_ATTR(hprd_lprd_wr_req_vld, 0x16),
+ ARM_CSPMU_EVENT_ATTR(lprd_req_vld, 0x17),
+ ARM_CSPMU_EVENT_ATTR(hprd_req_vld, 0x18),
+ ARM_CSPMU_EVENT_ATTR(hprd_lprd_req_vld, 0x19),
+ ARM_CSPMU_EVENT_ATTR(prefetch_tgt, 0x1a),
+ ARM_CSPMU_EVENT_ATTR(wr_req_vld, 0x1b),
+ ARM_CSPMU_EVENT_ATTR(partial_wr_req_vld, 0x1c),
+ ARM_CSPMU_EVENT_ATTR(rd_retry, 0x1d),
+ ARM_CSPMU_EVENT_ATTR(wr_retry, 0x1e),
+ ARM_CSPMU_EVENT_ATTR(retry_gnt, 0x1f),
+ ARM_CSPMU_EVENT_ATTR(rank_change, 0x20),
+ ARM_CSPMU_EVENT_ATTR(dir_change, 0x21),
+ ARM_CSPMU_EVENT_ATTR(rank_dir_change, 0x22),
+ ARM_CSPMU_EVENT_ATTR(rank_active, 0x23),
+ ARM_CSPMU_EVENT_ATTR(rank_idle, 0x24),
+ ARM_CSPMU_EVENT_ATTR(rank_pd, 0x25),
+ ARM_CSPMU_EVENT_ATTR(rank_sref, 0x26),
+ ARM_CSPMU_EVENT_ATTR(queue_fill_gt_thresh, 0x27),
+ ARM_CSPMU_EVENT_ATTR(queue_rds_gt_thresh, 0x28),
+ ARM_CSPMU_EVENT_ATTR(queue_wrs_gt_thresh, 0x29),
+ ARM_CSPMU_EVENT_ATTR(phy_updt_complt, 0x2a),
+ ARM_CSPMU_EVENT_ATTR(tz_fail, 0x2b),
+ ARM_CSPMU_EVENT_ATTR(dram_errc, 0x2c),
+ ARM_CSPMU_EVENT_ATTR(dram_errd, 0x2d),
+ ARM_CSPMU_EVENT_ATTR(read_data_return, 0x32),
+ ARM_CSPMU_EVENT_ATTR(chi_wr_data_delta, 0x33),
+ ARM_CSPMU_EVENT_ATTR(zq_start, 0x34),
+ ARM_CSPMU_EVENT_ATTR(zq_latch, 0x35),
+ ARM_CSPMU_EVENT_ATTR(wr_fifo_full, 0x36),
+ ARM_CSPMU_EVENT_ATTR(info_fifo_full, 0x37),
+ ARM_CSPMU_EVENT_ATTR(cmd_fifo_full, 0x38),
+ ARM_CSPMU_EVENT_ATTR(dfi_nop, 0x39),
+ ARM_CSPMU_EVENT_ATTR(dfi_cmd, 0x3a),
+ ARM_CSPMU_EVENT_ATTR(rd_run_len, 0x3b),
+ ARM_CSPMU_EVENT_ATTR(wr_run_len, 0x3c),
+
+ ARM_CSPMU_EVENT_ATTR(cycles, ARM_CSPMU_EVT_CYCLES_DEFAULT),
+ NULL,
+};
+
+static struct attribute *ampereone_mcu_format_attrs[] = {
+ ARM_CSPMU_FORMAT_EVENT_ATTR,
+ ARM_CSPMU_FORMAT_ATTR(threshold, "config1:0-7"),
+ ARM_CSPMU_FORMAT_ATTR(rank, "config1:8-23"),
+ ARM_CSPMU_FORMAT_ATTR(bank, "config2:0-31"),
+ NULL,
+};
+
+static struct attribute **
+ampere_cspmu_get_event_attrs(const struct arm_cspmu *cspmu)
+{
+ const struct ampere_cspmu_ctx *ctx = to_ampere_cspmu_ctx(cspmu);
+
+ return ctx->event_attr;
+}
+
+static struct attribute **
+ampere_cspmu_get_format_attrs(const struct arm_cspmu *cspmu)
+{
+ const struct ampere_cspmu_ctx *ctx = to_ampere_cspmu_ctx(cspmu);
+
+ return ctx->format_attr;
+}
+
+static const char *
+ampere_cspmu_get_name(const struct arm_cspmu *cspmu)
+{
+ const struct ampere_cspmu_ctx *ctx = to_ampere_cspmu_ctx(cspmu);
+
+ return ctx->name;
+}
+
+static u32 ampere_cspmu_event_filter(const struct perf_event *event)
+{
+ return 0;
+}
+
+static void ampere_cspmu_set_ev_filter(struct arm_cspmu *cspmu,
+ struct hw_perf_event *hwc,
+ u32 filter)
+{
+ struct perf_event *event;
+ unsigned int idx;
+ u32 threshold, rank, bank;
+
+ /*
+ * At this point, all the events have the same filter settings.
+ * Therefore, take the first event and use its configuration.
+ */
+ idx = find_first_bit(cspmu->hw_events.used_ctrs,
+ cspmu->cycle_counter_logical_idx);
+
+ event = cspmu->hw_events.events[idx];
+
+ threshold = get_threshold(event);
+ rank = get_rank(event);
+ bank = get_bank(event);
+
+ writel(threshold, cspmu->base0 + PMAUXR0);
+ writel(rank, cspmu->base0 + PMAUXR1);
+ writel(bank, cspmu->base0 + PMAUXR2);
+}
+
+static int ampere_cspmu_validate_params(struct perf_event *event, struct perf_event *event2)
+{
+ if (get_threshold(event) != get_threshold(event2) ||
+ get_rank(event) != get_rank(event2) ||
+ get_bank(event) != get_bank(event2))
+ return -EINVAL;
+
+ return 0;
+}
+
+static int ampere_cspmu_validate_group(struct perf_event *event)
+{
+ struct perf_event *sibling, *leader = event->group_leader;
+ int ret;
+
+ ret = ampere_cspmu_validate_params(event, leader);
+ if (ret)
+ return ret;
+
+ for_each_sibling_event(sibling, leader) {
+ ret = ampere_cspmu_validate_params(sibling, leader);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ampere_cspmu_validate_event(struct arm_cspmu *cspmu,
+ struct perf_event *new)
+{
+ struct perf_event *curr;
+ unsigned int idx;
+
+ /* We compare the global filter settings to existing events */
+ idx = find_first_bit(cspmu->hw_events.used_ctrs,
+ cspmu->cycle_counter_logical_idx);
+
+ /* This is the first event */
+ if (idx == cspmu->cycle_counter_logical_idx)
+ return 0;
+
+ curr = cspmu->hw_events.events[idx];
+
+ return ampere_cspmu_validate_params(curr, new);
+}
+
+static char *ampere_cspmu_format_name(const struct arm_cspmu *cspmu,
+ const char *name_pattern)
+{
+ struct device *dev = cspmu->dev;
+ static atomic_t pmu_generic_idx = {0};
+
+ return devm_kasprintf(dev, GFP_KERNEL, name_pattern,
+ atomic_fetch_inc(&pmu_generic_idx));
+}
+
+int ampere_cspmu_init_ops(struct arm_cspmu *cspmu)
+{
+ struct device *dev = cspmu->dev;
+ struct ampere_cspmu_ctx *ctx;
+ struct arm_cspmu_impl_ops *impl_ops = &cspmu->impl.ops;
+
+ ctx = devm_kzalloc(dev, sizeof(struct ampere_cspmu_ctx), GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+
+ ctx->event_attr = ampereone_mcu_pmu_event_attrs;
+ ctx->format_attr = ampereone_mcu_format_attrs;
+ ctx->name = ampere_cspmu_format_name(cspmu,
+ "ampere_mcu_pmu_%u");
+ cspmu->impl.ctx = ctx;
+
+ impl_ops->event_filter = ampere_cspmu_event_filter;
+ impl_ops->set_ev_filter = ampere_cspmu_set_ev_filter;
+ impl_ops->validate_group = ampere_cspmu_validate_group;
+ impl_ops->validate_event = ampere_cspmu_validate_event;
+ impl_ops->get_name = ampere_cspmu_get_name;
+ impl_ops->get_event_attrs = ampere_cspmu_get_event_attrs;
+ impl_ops->get_format_attrs = ampere_cspmu_get_format_attrs;
+
+ return 0;
+}
diff --git a/drivers/perf/arm_cspmu/ampere_cspmu.h b/drivers/perf/arm_cspmu/ampere_cspmu.h
new file mode 100644
index 000000000000..9b3e1628d1d6
--- /dev/null
+++ b/drivers/perf/arm_cspmu/ampere_cspmu.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Ampere SoC PMU (Performance Monitor Unit)
+ *
+ * Copyright (c) 2023, Ampere Computing LLC
+ */
+
+#ifndef __AMPERE_CSPMU_H__
+#define __AMPERE_CSPMU_H__
+
+#include "arm_cspmu.h"
+
+/* Allocate AMPERE descriptor. */
+int ampere_cspmu_init_ops(struct arm_cspmu *cspmu);
+
+#endif /* __AMPERE_CSPMU_H__ */
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 9021d1878250..885393ed993e 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -30,6 +30,7 @@
#include <linux/platform_device.h>
#include <acpi/processor.h>
+#include "ampere_cspmu.h"
#include "arm_cspmu.h"
#include "nvidia_cspmu.h"
@@ -119,6 +120,7 @@
/* JEDEC-assigned JEP106 identification code */
#define ARM_CSPMU_IMPL_ID_NVIDIA 0x36B
+#define ARM_CSPMU_IMPL_ID_AMPERE 0xA16
static unsigned long arm_cspmu_cpuhp_state;
@@ -394,6 +396,11 @@ static const struct impl_match impl_match[] = {
.mask = ARM_CSPMU_PMIIDR_IMPLEMENTER,
.impl_init_ops = nv_cspmu_init_ops
},
+ {
+ .pmiidr = ARM_CSPMU_IMPL_ID_AMPERE,
+ .mask = ARM_CSPMU_PMIIDR_IMPLEMENTER,
+ .impl_init_ops = ampere_cspmu_init_ops
+ },
{}
};
--
2.40.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [PATCH v3 1/4] perf: arm_cspmu: Split 64-bit write to 32-bit writes
2023-06-07 8:31 ` [PATCH v3 1/4] perf: arm_cspmu: Split 64-bit write to 32-bit writes Ilkka Koskinen
@ 2023-06-20 5:12 ` Besar Wicaksono
2023-06-21 19:05 ` Ilkka Koskinen
0 siblings, 1 reply; 10+ messages in thread
From: Besar Wicaksono @ 2023-06-20 5:12 UTC (permalink / raw)
To: Ilkka Koskinen, Jonathan Corbet, Will Deacon, Mark Rutland,
Suzuki K Poulose, Robin Murphy
Cc: linux-doc, linux-kernel, linux-arm-kernel
> -----Original Message-----
> From: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> Sent: Wednesday, June 7, 2023 3:32 PM
> To: Jonathan Corbet <corbet@lwn.net>; Will Deacon <will@kernel.org>; Mark
> Rutland <mark.rutland@arm.com>; Besar Wicaksono
> <bwicaksono@nvidia.com>; Suzuki K Poulose <suzuki.poulose@arm.com>;
> Robin Murphy <robin.murphy@arm.com>
> Cc: linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; Ilkka Koskinen <ilkka@os.amperecomputing.com>
> Subject: [PATCH v3 1/4] perf: arm_cspmu: Split 64-bit write to 32-bit writes
>
> External email: Use caution opening links or attachments
>
>
> Split the 64-bit register accesses if 64-bit access is not supported
> by the PMU.
>
> Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> ---
> drivers/perf/arm_cspmu/arm_cspmu.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c
> b/drivers/perf/arm_cspmu/arm_cspmu.c
> index a3f1c410b417..f8b4a149eb88 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -702,7 +702,10 @@ static void arm_cspmu_write_counter(struct
> perf_event *event, u64 val)
> if (use_64b_counter_reg(cspmu)) {
> offset = counter_offset(sizeof(u64), event->hw.idx);
>
> - writeq(val, cspmu->base1 + offset);
> + if (supports_64bit_atomics(cspmu))
Looks good to me, but this function was recently replaced by
arm_cspmu::has_atomic_dword. Please rebase the patch.
Thanks,
Besar
> + writeq(val, cspmu->base1 + offset);
> + else
> + lo_hi_writeq(val, cspmu->base1 + offset);
> } else {
> offset = counter_offset(sizeof(u32), event->hw.idx);
>
> --
> 2.40.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v3 2/4] perf: arm_cspmu: Support implementation specific filters
2023-06-07 8:31 ` [PATCH v3 2/4] perf: arm_cspmu: Support implementation specific filters Ilkka Koskinen
@ 2023-06-20 5:39 ` Besar Wicaksono
0 siblings, 0 replies; 10+ messages in thread
From: Besar Wicaksono @ 2023-06-20 5:39 UTC (permalink / raw)
To: Ilkka Koskinen, Jonathan Corbet, Will Deacon, Mark Rutland,
Suzuki K Poulose, Robin Murphy
Cc: linux-doc, linux-kernel, linux-arm-kernel
> -----Original Message-----
> From: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> Sent: Wednesday, June 7, 2023 3:32 PM
> To: Jonathan Corbet <corbet@lwn.net>; Will Deacon <will@kernel.org>; Mark
> Rutland <mark.rutland@arm.com>; Besar Wicaksono
> <bwicaksono@nvidia.com>; Suzuki K Poulose <suzuki.poulose@arm.com>;
> Robin Murphy <robin.murphy@arm.com>
> Cc: linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; Ilkka Koskinen <ilkka@os.amperecomputing.com>
> Subject: [PATCH v3 2/4] perf: arm_cspmu: Support implementation specific
> filters
>
> External email: Use caution opening links or attachments
>
>
> Generic filters aren't used in all the platforms. Instead, the platforms
> may use different means to filter events. Add support for implementation
> specific filters.
>
> Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Reviewed-by: Besar Wicaksono <bwicaksono@nvidia.com>
> ---
> drivers/perf/arm_cspmu/arm_cspmu.c | 8 ++++++--
> drivers/perf/arm_cspmu/arm_cspmu.h | 4 ++++
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c
> b/drivers/perf/arm_cspmu/arm_cspmu.c
> index f8b4a149eb88..72ca4f56347c 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -122,6 +122,9 @@
>
> static unsigned long arm_cspmu_cpuhp_state;
>
> +static void arm_cspmu_set_ev_filter(struct arm_cspmu *cspmu,
> + struct hw_perf_event *hwc, u32 filter);
> +
> /*
> * In CoreSight PMU architecture, all of the MMIO registers are 32-bit except
> * counter register. The counter register can be implemented as 32-bit or 64-
> bit
> @@ -432,6 +435,7 @@ static int arm_cspmu_init_impl_ops(struct
> arm_cspmu *cspmu)
> CHECK_DEFAULT_IMPL_OPS(impl_ops, event_type);
> CHECK_DEFAULT_IMPL_OPS(impl_ops, event_filter);
> CHECK_DEFAULT_IMPL_OPS(impl_ops, event_attr_is_visible);
> + CHECK_DEFAULT_IMPL_OPS(impl_ops, set_ev_filter);
>
> return 0;
> }
> @@ -798,7 +802,7 @@ static inline void arm_cspmu_set_event(struct
> arm_cspmu *cspmu,
> writel(hwc->config, cspmu->base0 + offset);
> }
>
> -static inline void arm_cspmu_set_ev_filter(struct arm_cspmu *cspmu,
> +static void arm_cspmu_set_ev_filter(struct arm_cspmu *cspmu,
> struct hw_perf_event *hwc,
> u32 filter)
> {
> @@ -832,7 +836,7 @@ static void arm_cspmu_start(struct perf_event
> *event, int pmu_flags)
> arm_cspmu_set_cc_filter(cspmu, filter);
> } else {
> arm_cspmu_set_event(cspmu, hwc);
> - arm_cspmu_set_ev_filter(cspmu, hwc, filter);
> + cspmu->impl.ops.set_ev_filter(cspmu, hwc, filter);
> }
>
> hwc->state = 0;
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h
> b/drivers/perf/arm_cspmu/arm_cspmu.h
> index 51323b175a4a..f89ae2077164 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.h
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.h
> @@ -102,6 +102,10 @@ struct arm_cspmu_impl_ops {
> u32 (*event_type)(const struct perf_event *event);
> /* Decode filter value from configs */
> u32 (*event_filter)(const struct perf_event *event);
> + /* Set event filter */
> + void (*set_ev_filter)(struct arm_cspmu *cspmu,
> + struct hw_perf_event *hwc,
> + u32 filter);
> /* Hide/show unsupported events */
> umode_t (*event_attr_is_visible)(struct kobject *kobj,
> struct attribute *attr, int unused);
> --
> 2.40.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/4] perf: arm_cspmu: Support implementation specific validation
2023-06-07 8:31 ` [PATCH v3 3/4] perf: arm_cspmu: Support implementation specific validation Ilkka Koskinen
@ 2023-06-20 11:44 ` Robin Murphy
2023-06-21 22:09 ` Ilkka Koskinen
0 siblings, 1 reply; 10+ messages in thread
From: Robin Murphy @ 2023-06-20 11:44 UTC (permalink / raw)
To: Ilkka Koskinen, Jonathan Corbet, Will Deacon, Mark Rutland,
Besar Wicaksono, Suzuki K Poulose
Cc: linux-doc, linux-kernel, linux-arm-kernel
On 07/06/2023 9:31 am, Ilkka Koskinen wrote:
> Some platforms may use e.g. different filtering mechanism and, thus,
> may need different way to validate the events and group.
>
> Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> ---
> drivers/perf/arm_cspmu/arm_cspmu.c | 13 ++++++++++++-
> drivers/perf/arm_cspmu/arm_cspmu.h | 4 ++++
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index 72ca4f56347c..9021d1878250 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -559,7 +559,7 @@ static void arm_cspmu_disable(struct pmu *pmu)
> static int arm_cspmu_get_event_idx(struct arm_cspmu_hw_events *hw_events,
> struct perf_event *event)
> {
> - int idx;
> + int idx, ret;
> struct arm_cspmu *cspmu = to_arm_cspmu(event->pmu);
>
> if (supports_cycle_counter(cspmu)) {
> @@ -593,6 +593,12 @@ 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) {
> + ret = cspmu->impl.ops.validate_event(cspmu, event);
> + if (ret)
> + return ret;
> + }
> +
> set_bit(idx, hw_events->used_ctrs);
>
> return idx;
> @@ -618,6 +624,7 @@ static bool arm_cspmu_validate_event(struct pmu *pmu,
> */
> static bool arm_cspmu_validate_group(struct perf_event *event)
> {
> + struct arm_cspmu *cspmu = to_arm_cspmu(event->pmu);
> struct perf_event *sibling, *leader = event->group_leader;
> struct arm_cspmu_hw_events fake_hw_events;
>
> @@ -635,6 +642,10 @@ static bool arm_cspmu_validate_group(struct perf_event *event)
> return false;
> }
>
> + if (cspmu->impl.ops.validate_group &&
> + cspmu->impl.ops.validate_group(event))
> + return false;
Hmm, this means that any driver wanting to use it has to duplicate all
the group iteration logic, which isn't ideal. More than that, though,
the way you've implemented it in patch #4 I'm not sure even does
anything, since it only appears to be repeating the same checks that
already happen in this path:
arm_csmpu_validate_group()
arm_cspmu_validate_event()
arm_cspmu_get_event_idx()
ops.validate_event() -> ampere_cspmu_validate_params()
so there's no need for the ops.validate_group hook to just call
ampere_cspmu_validate_params() a second time when it's guaranteed to
succeed (because otherwise we'd have bailed out already).
I think what we want overall is an "is this event config valid at all"
hook from arm_cspmu_event_init() (which we don't really need to
implement yet unless you want to start sanity-checking your actual
rank/bank/threshold values), plus an "is this event schedulable in the
given PMU context" hook from arm_cspmu_get_event_idx(), which should
serve for both group validation via the fake context in event_init and
actual scheduling in the real context in add.
Thanks,
Robin.
> +
> return arm_cspmu_validate_event(event->pmu, &fake_hw_events, event);
> }
>
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
> index f89ae2077164..291cedb196ea 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.h
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.h
> @@ -106,6 +106,10 @@ struct arm_cspmu_impl_ops {
> void (*set_ev_filter)(struct arm_cspmu *cspmu,
> struct hw_perf_event *hwc,
> u32 filter);
> + /* Implementation specific group validation */
> + int (*validate_group)(struct perf_event *event);
> + /* Implementation specific event validation */
> + int (*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);
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v3 1/4] perf: arm_cspmu: Split 64-bit write to 32-bit writes
2023-06-20 5:12 ` Besar Wicaksono
@ 2023-06-21 19:05 ` Ilkka Koskinen
0 siblings, 0 replies; 10+ messages in thread
From: Ilkka Koskinen @ 2023-06-21 19:05 UTC (permalink / raw)
To: Besar Wicaksono
Cc: Ilkka Koskinen, Jonathan Corbet, Will Deacon, Mark Rutland,
Suzuki K Poulose, Robin Murphy, linux-doc, linux-kernel,
linux-arm-kernel
Hi Besar,
On Tue, 20 Jun 2023, Besar Wicaksono wrote:
>> -----Original Message-----
>> From: Ilkka Koskinen <ilkka@os.amperecomputing.com>
>> Sent: Wednesday, June 7, 2023 3:32 PM
>> To: Jonathan Corbet <corbet@lwn.net>; Will Deacon <will@kernel.org>; Mark
>> Rutland <mark.rutland@arm.com>; Besar Wicaksono
>> <bwicaksono@nvidia.com>; Suzuki K Poulose <suzuki.poulose@arm.com>;
>> Robin Murphy <robin.murphy@arm.com>
>> Cc: linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org; Ilkka Koskinen <ilkka@os.amperecomputing.com>
>> Subject: [PATCH v3 1/4] perf: arm_cspmu: Split 64-bit write to 32-bit writes
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Split the 64-bit register accesses if 64-bit access is not supported
>> by the PMU.
>>
>> Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
>> ---
>> drivers/perf/arm_cspmu/arm_cspmu.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c
>> b/drivers/perf/arm_cspmu/arm_cspmu.c
>> index a3f1c410b417..f8b4a149eb88 100644
>> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
>> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
>> @@ -702,7 +702,10 @@ static void arm_cspmu_write_counter(struct
>> perf_event *event, u64 val)
>> if (use_64b_counter_reg(cspmu)) {
>> offset = counter_offset(sizeof(u64), event->hw.idx);
>>
>> - writeq(val, cspmu->base1 + offset);
>> + if (supports_64bit_atomics(cspmu))
>
> Looks good to me, but this function was recently replaced by
> arm_cspmu::has_atomic_dword. Please rebase the patch.
Sure, I do that.
Cheers, Ilkka
>
> Thanks,
> Besar
>
>> + writeq(val, cspmu->base1 + offset);
>> + else
>> + lo_hi_writeq(val, cspmu->base1 + offset);
>> } else {
>> offset = counter_offset(sizeof(u32), event->hw.idx);
>>
>> --
>> 2.40.1
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/4] perf: arm_cspmu: Support implementation specific validation
2023-06-20 11:44 ` Robin Murphy
@ 2023-06-21 22:09 ` Ilkka Koskinen
0 siblings, 0 replies; 10+ messages in thread
From: Ilkka Koskinen @ 2023-06-21 22:09 UTC (permalink / raw)
To: Robin Murphy
Cc: Ilkka Koskinen, Jonathan Corbet, Will Deacon, Mark Rutland,
Besar Wicaksono, Suzuki K Poulose, linux-doc, linux-kernel,
linux-arm-kernel
Hi Robin,
On Tue, 20 Jun 2023, Robin Murphy wrote:
> On 07/06/2023 9:31 am, Ilkka Koskinen wrote:
>> Some platforms may use e.g. different filtering mechanism and, thus,
>> may need different way to validate the events and group.
>>
>> Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
>> ---
>> drivers/perf/arm_cspmu/arm_cspmu.c | 13 ++++++++++++-
>> drivers/perf/arm_cspmu/arm_cspmu.h | 4 ++++
>> 2 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c
>> b/drivers/perf/arm_cspmu/arm_cspmu.c
>> index 72ca4f56347c..9021d1878250 100644
>> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
>> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
>> @@ -559,7 +559,7 @@ static void arm_cspmu_disable(struct pmu *pmu)
>> static int arm_cspmu_get_event_idx(struct arm_cspmu_hw_events *hw_events,
>> struct perf_event *event)
>> {
>> - int idx;
>> + int idx, ret;
>> struct arm_cspmu *cspmu = to_arm_cspmu(event->pmu);
>> if (supports_cycle_counter(cspmu)) {
>> @@ -593,6 +593,12 @@ 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) {
>> + ret = cspmu->impl.ops.validate_event(cspmu, event);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> set_bit(idx, hw_events->used_ctrs);
>> return idx;
>> @@ -618,6 +624,7 @@ static bool arm_cspmu_validate_event(struct pmu *pmu,
>> */
>> static bool arm_cspmu_validate_group(struct perf_event *event)
>> {
>> + struct arm_cspmu *cspmu = to_arm_cspmu(event->pmu);
>> struct perf_event *sibling, *leader = event->group_leader;
>> struct arm_cspmu_hw_events fake_hw_events;
>> @@ -635,6 +642,10 @@ static bool arm_cspmu_validate_group(struct
>> perf_event *event)
>> return false;
>> }
>> + if (cspmu->impl.ops.validate_group &&
>> + cspmu->impl.ops.validate_group(event))
>> + return false;
>
> Hmm, this means that any driver wanting to use it has to duplicate all the
> group iteration logic, which isn't ideal. More than that, though, the way
> you've implemented it in patch #4 I'm not sure even does anything, since it
> only appears to be repeating the same checks that already happen in this
> path:
>
> arm_csmpu_validate_group()
> arm_cspmu_validate_event()
> arm_cspmu_get_event_idx()
> ops.validate_event() -> ampere_cspmu_validate_params()
>
> so there's no need for the ops.validate_group hook to just call
> ampere_cspmu_validate_params() a second time when it's guaranteed to succeed
> (because otherwise we'd have bailed out already).
Yeah, I took another look how the framework really does it and you're
absolutely correct, it's totally unnecessary.
>
> I think what we want overall is an "is this event config valid at all" hook
> from arm_cspmu_event_init() (which we don't really need to implement yet
> unless you want to start sanity-checking your actual rank/bank/threshold
> values), plus an "is this event schedulable in the given PMU context" hook
> from arm_cspmu_get_event_idx(), which should serve for both group validation
> via the fake context in event_init and actual scheduling in the real context
> in add.
Ah, that's true. I can already verify the group event has the same
rank/bank/threshold settings as the group leader in ops.validate_event().
Thus, one hook seems enough.
I fix and rebase the patchset.
Cheers, Ilkka
> Thanks,
> Robin.
>
>> +
>> return arm_cspmu_validate_event(event->pmu, &fake_hw_events, event);
>> }
>> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h
>> b/drivers/perf/arm_cspmu/arm_cspmu.h
>> index f89ae2077164..291cedb196ea 100644
>> --- a/drivers/perf/arm_cspmu/arm_cspmu.h
>> +++ b/drivers/perf/arm_cspmu/arm_cspmu.h
>> @@ -106,6 +106,10 @@ struct arm_cspmu_impl_ops {
>> void (*set_ev_filter)(struct arm_cspmu *cspmu,
>> struct hw_perf_event *hwc,
>> u32 filter);
>> + /* Implementation specific group validation */
>> + int (*validate_group)(struct perf_event *event);
>> + /* Implementation specific event validation */
>> + int (*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);
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-06-21 22:09 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-07 8:31 [PATCH v3 0/4] perf: ampere: Add support for Ampere SoC PMUs Ilkka Koskinen
2023-06-07 8:31 ` [PATCH v3 1/4] perf: arm_cspmu: Split 64-bit write to 32-bit writes Ilkka Koskinen
2023-06-20 5:12 ` Besar Wicaksono
2023-06-21 19:05 ` Ilkka Koskinen
2023-06-07 8:31 ` [PATCH v3 2/4] perf: arm_cspmu: Support implementation specific filters Ilkka Koskinen
2023-06-20 5:39 ` Besar Wicaksono
2023-06-07 8:31 ` [PATCH v3 3/4] perf: arm_cspmu: Support implementation specific validation Ilkka Koskinen
2023-06-20 11:44 ` Robin Murphy
2023-06-21 22:09 ` Ilkka Koskinen
2023-06-07 8:31 ` [PATCH v3 4/4] perf: arm_cspmu: ampere_cspmu: Add support for Ampere SoC PMU Ilkka Koskinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).