Re: [PATCH 2/3] drivers/perf: hisi: Add support for HiSilicon UC PMU driver
From: hejunhao
Date: Sat May 27 2023 - 05:37:19 EST
Hi Yicong,
On 2023/5/25 21:39, Yicong Yang wrote:
Hi Junhao,
On 2023/5/23 21:18, Junhao He wrote:
On HiSilicon Hip09 platform, there is a UC (unified cache) module
on each chip SCCL (Super CPU Cluster). UC is a cache that provides
coherence between NUMA and UMA domains. It is located between L2
and Memory System. While PA uncore PMU model is the same as other
Hip09 PMU modules and many PMU events are supported. Let's support
the PMU driver using the HiSilicon uncore PMU framework.
* rd_req_en : rd_req_en is the abbreviation of read request tracetag enable
and allows user to count only read operations.
details are listed in the hisi-pmu document.
* srcid_en & srcid: allows user to filter statistics that come from
specific CPU/ICL by configuration source ID.
* uring_channel: Allows users to filter statistical information based on
the specified tx request uring channel.
uring_channel only supported events: [0x47 ~ 0x59].
Just a quick glance and some minor comments below.
Thanks for your comments.
Signed-off-by: Junhao He <hejunhao3@xxxxxxxxxx>
---
drivers/perf/hisilicon/Makefile | 2 +-
drivers/perf/hisilicon/hisi_uncore_pmu.c | 5 +-
drivers/perf/hisilicon/hisi_uncore_pmu.h | 6 +
drivers/perf/hisilicon/hisi_uncore_uc_pmu.c | 581 ++++++++++++++++++++
4 files changed, 592 insertions(+), 2 deletions(-)
create mode 100644 drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
diff --git a/drivers/perf/hisilicon/Makefile b/drivers/perf/hisilicon/Makefile
index 4d2c9abe3372..48dcc8381ea7 100644
--- a/drivers/perf/hisilicon/Makefile
+++ b/drivers/perf/hisilicon/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_HISI_PMU) += hisi_uncore_pmu.o hisi_uncore_l3c_pmu.o \
hisi_uncore_hha_pmu.o hisi_uncore_ddrc_pmu.o hisi_uncore_sllc_pmu.o \
- hisi_uncore_pa_pmu.o hisi_uncore_cpa_pmu.o
+ hisi_uncore_pa_pmu.o hisi_uncore_cpa_pmu.o hisi_uncore_uc_pmu.o
obj-$(CONFIG_HISI_PCIE_PMU) += hisi_pcie_pmu.o
obj-$(CONFIG_HNS3_PMU) += hns3_pmu.o
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
index 2823f381930d..c5ac1c1a9fc1 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -20,7 +20,6 @@
#include "hisi_uncore_pmu.h"
-#define HISI_GET_EVENTID(ev) (ev->hw.config_base & 0xff)
#define HISI_MAX_PERIOD(nr) (GENMASK_ULL((nr) - 1, 0))
/*
@@ -226,6 +225,10 @@ int hisi_uncore_pmu_event_init(struct perf_event *event)
hwc->idx = -1;
hwc->config_base = event->attr.config;
+ if (hisi_pmu->ops->check_format)
+ if (hisi_pmu->ops->check_format(event))
It can be merged into one if.
Yes, I will do that.
+ return -EINVAL;
+
/* Enforce to use the same CPU for all events in this PMU */
event->cpu = hisi_pmu->on_cpu;
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h
index 73574cc0a243..af6529e99c13 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.h
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
@@ -49,9 +49,15 @@ enum hisi_pmu_version {
HISI_PMU_MAX
};
+#define HISI_GET_EVENTID(ev) (ev->hw.config_base & 0xff)
+
+#define HISI_PMU_EVTYPE_BITS 8
+#define HISI_PMU_EVTYPE_SHIFT(idx) ((idx) % 4 * HISI_PMU_EVTYPE_BITS)
+
struct hisi_pmu;
struct hisi_uncore_ops {
+ int (*check_format)(struct perf_event *event);
void (*write_evtype)(struct hisi_pmu *, int, u32);
int (*get_event_idx)(struct perf_event *);
u64 (*read_counter)(struct hisi_pmu *, struct hw_perf_event *);
diff --git a/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c b/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
new file mode 100644
index 000000000000..385966eab816
--- /dev/null
+++ b/drivers/perf/hisilicon/hisi_uncore_uc_pmu.c
@@ -0,0 +1,581 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * HiSilicon SoC UC (unified cache) uncore Hardware event counters support
+ *
+ * Copyright (C) 2023 HiSilicon Limited
+ *
+ * This code is based on the uncore PMUs like hisi_uncore_l3c_pmu.
+ */
+#include <linux/cpuhotplug.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/list.h>
+#include <linux/mod_devicetable.h>
+#include <linux/property.h>
+
+#include "hisi_uncore_pmu.h"
+
+/* Dynamic CPU hotplug state used by UC PMU */
+static enum cpuhp_state hisi_uc_pmu_online;
+
+/* UC register definition */
+#define HISI_UC_INT_MASK_REG 0x0800
+#define HISI_UC_INT_STS_REG 0x0808
+#define HISI_UC_INT_CLEAR_REG 0x080c
+#define HISI_UC_TRACETAG_CTRL_REG 0x1b2c
+#define HISI_UC_TRACETAG_REQ_MSK GENMASK(9, 7)
+#define HISI_UC_TRACETAG_MARK_EN BIT(0)
+#define HISI_UC_TRACETAG_REQ_EN (HISI_UC_TRACETAG_MARK_EN | BIT(2))
+#define HISI_UC_TRACETAG_SRCID_EN BIT(3)
+#define HISI_UC_SRCID_CTRL_REG 0x1b40
+#define HISI_UC_SRCID_MSK GENMASK(14, 1)
+#define HISI_UC_EVENT_CTRL_REG 0x1c00
+#define HISI_UC_EVENT_TRACETAG_EN BIT(29)
+#define HISI_UC_EVENT_URING_MSK GENMASK(28, 27)
+#define HISI_UC_EVENT_GLB_EN BIT(26)
+#define HISI_UC_VERSION_REG 0x1cf0
+#define HISI_UC_EVTYPE0_REG 0x1d00
+#define HISI_UC_EVTYPE_REG(n) (HISI_UC_EVTYPE0_REG + (n) * 4)
+#define HISI_UC_EVTYPE_MASK GENMASK(7, 0)
+#define HISI_UC_CNTR0_REG 0x1e00
+#define HISI_UC_CNTR_REG(n) (HISI_UC_CNTR0_REG + (n) * 8)
+
+#define HISI_UC_NR_COUNTERS 0x8
+#define HISI_UC_V2_NR_EVENTS 0xFF
+#define HISI_UC_CNTR_REG_BITS 64
+
+#define HISI_UC_RD_REQ_TRACETAG 0x4
+#define HISI_UC_URING_EVENT_MIN 0x47
+#define HISI_UC_URING_EVENT_MAX 0x59
+
+HISI_PMU_EVENT_ATTR_EXTRACTOR(rd_req_en, config1, 0, 0);
+HISI_PMU_EVENT_ATTR_EXTRACTOR(uring_channel, config1, 5, 4);
+HISI_PMU_EVENT_ATTR_EXTRACTOR(srcid, config1, 19, 6);
+HISI_PMU_EVENT_ATTR_EXTRACTOR(srcid_en, config1, 20, 20);
+
+static int hisi_uc_pmu_check_format(struct perf_event *event)
check_config or check_event will be clearer.
Sure, Will fix in next version.
+{
+ struct hisi_pmu *uc_pmu = to_hisi_pmu(event->pmu);
+
+ if (hisi_get_srcid_en(event) && !hisi_get_rd_req_en(event)) {
+ dev_err(uc_pmu->dev,
+ "Failed to set srcid: depending on read request enabled!\n");
+ return -EINVAL;
+ }
+
+ if (!hisi_get_uring_channel(event))
+ return 0;
+
+ if ((HISI_GET_EVENTID(event) < HISI_UC_URING_EVENT_MIN) ||
+ (HISI_GET_EVENTID(event) > HISI_UC_URING_EVENT_MAX))
+ dev_warn(uc_pmu->dev,
+ "Only events: [%#x ~ %#x] support channel filtering!",
+ HISI_UC_URING_EVENT_MIN, HISI_UC_URING_EVENT_MAX);
+
+ return 0;
+}
+
+static void hisi_uc_pmu_config_req_tracetag(struct perf_event *event)
+{
+ struct hisi_pmu *uc_pmu = to_hisi_pmu(event->pmu);
+ u32 val;
+
+ if (!hisi_get_rd_req_en(event))
+ return;
+
+ val = readl(uc_pmu->base + HISI_UC_TRACETAG_CTRL_REG);
+
+ /* The request-type has been configured */
+ if (FIELD_GET(HISI_UC_TRACETAG_REQ_MSK, val) == HISI_UC_RD_REQ_TRACETAG)
+ return;
+
+ /* Set request-type for tracetag, only read request is supported! */
+ val &= ~HISI_UC_TRACETAG_REQ_MSK;
+ val |= FIELD_PREP(HISI_UC_TRACETAG_REQ_MSK, HISI_UC_RD_REQ_TRACETAG);
+ val |= HISI_UC_TRACETAG_REQ_EN;
+ writel(val, uc_pmu->base + HISI_UC_TRACETAG_CTRL_REG);
+}
+
+static void hisi_uc_pmu_clear_req_tracetag(struct perf_event *event)
+{
+ struct hisi_pmu *uc_pmu = to_hisi_pmu(event->pmu);
+ u32 val;
+
+ if (!hisi_get_rd_req_en(event))
+ return;
+
+ val = readl(uc_pmu->base + HISI_UC_TRACETAG_CTRL_REG);
+
+ /* Do nothing, the request-type tracetag has been cleaned up */
+ if (FIELD_GET(HISI_UC_TRACETAG_REQ_MSK, val) == 0)
+ return;
+
+ /* Clear request-type */
+ val &= ~HISI_UC_TRACETAG_REQ_MSK;
+ val &= ~HISI_UC_TRACETAG_REQ_EN;
+ writel(val, uc_pmu->base + HISI_UC_TRACETAG_CTRL_REG);
+}
+
+static void hisi_uc_pmu_config_srcid_tracetag(struct perf_event *event)
+{
+ struct hisi_pmu *uc_pmu = to_hisi_pmu(event->pmu);
+ u32 val;
+
+ if (!hisi_get_srcid_en(event))
+ return;
+
+ val = readl(uc_pmu->base + HISI_UC_TRACETAG_CTRL_REG);
+
+ /* Do nothing, the source id has been configured */
+ if (FIELD_GET(HISI_UC_TRACETAG_SRCID_EN, val))
+ return;
+
+ /* Enable source id tracetag */
+ val |= HISI_UC_TRACETAG_SRCID_EN;
+ writel(val, uc_pmu->base + HISI_UC_TRACETAG_CTRL_REG);
+
+ val = readl(uc_pmu->base + HISI_UC_SRCID_CTRL_REG);
+ val &= ~HISI_UC_SRCID_MSK;
+ val |= FIELD_PREP(HISI_UC_SRCID_MSK, hisi_get_srcid(event));
+ writel(val, uc_pmu->base + HISI_UC_SRCID_CTRL_REG);
+
+ /* Depend on request-type tracetag enabled */
+ hisi_uc_pmu_config_req_tracetag(event);
+}
+
+static void hisi_uc_pmu_clear_srcid_tracetag(struct perf_event *event)
+{
+ struct hisi_pmu *uc_pmu = to_hisi_pmu(event->pmu);
+ u32 val;
+
+ if (!hisi_get_srcid_en(event))
+ return;
+
+ val = readl(uc_pmu->base + HISI_UC_TRACETAG_CTRL_REG);
+
+ /* Do nothing, the source id has been cleaned up */
+ if (FIELD_GET(HISI_UC_TRACETAG_SRCID_EN, val) == 0)
+ return;
+
+ hisi_uc_pmu_clear_req_tracetag(event);
+
+ /* Disable source id tracetag */
+ val &= ~HISI_UC_TRACETAG_SRCID_EN;
+ writel(val, uc_pmu->base + HISI_UC_TRACETAG_CTRL_REG);
+
+ val = readl(uc_pmu->base + HISI_UC_SRCID_CTRL_REG);
+ val &= ~HISI_UC_SRCID_MSK;
+ writel(val, uc_pmu->base + HISI_UC_SRCID_CTRL_REG);
+}
+
+static void hisi_uc_pmu_config_uring_channel(struct perf_event *event)
+{
+ struct hisi_pmu *uc_pmu = to_hisi_pmu(event->pmu);
+ u32 uring_channel = hisi_get_uring_channel(event);
+ u32 val;
+
+ /* Do nothing if not being set or is set explicitly to zero (default) */
+ if (uring_channel == 0)
+ return;
+
+ val = readl(uc_pmu->base + HISI_UC_EVENT_CTRL_REG);
+
+ /* Do nothing, the uring_channel has been configured */
+ if (uring_channel == FIELD_GET(HISI_UC_EVENT_URING_MSK, val))
+ return;
+
+ val &= ~HISI_UC_EVENT_URING_MSK;
+ val |= FIELD_PREP(HISI_UC_EVENT_URING_MSK, uring_channel);
+ writel(val, uc_pmu->base + HISI_UC_EVENT_CTRL_REG);
+}
+
+static void hisi_uc_pmu_clear_uring_channel(struct perf_event *event)
+{
+ struct hisi_pmu *uc_pmu = to_hisi_pmu(event->pmu);
+ u32 val;
+
+ /* Do nothing if not being set or is set explicitly to zero (default) */
+ if (hisi_get_uring_channel(event) == 0)
+ return;
+
+ val = readl(uc_pmu->base + HISI_UC_EVENT_CTRL_REG);
+
+ /* Do nothing, the uring_channel has been cleaned up */
+ if (FIELD_GET(HISI_UC_EVENT_URING_MSK, val) == 0)
+ return;
+
+ val &= ~HISI_UC_EVENT_URING_MSK;
+ writel(val, uc_pmu->base + HISI_UC_EVENT_CTRL_REG);
+}
+
+static void hisi_uc_pmu_enable_filter(struct perf_event *event)
+{
+ if (event->attr.config1 == 0)
+ return;
+
+ hisi_uc_pmu_config_uring_channel(event);
+ hisi_uc_pmu_config_req_tracetag(event);
+ hisi_uc_pmu_config_srcid_tracetag(event);
+}
+
+static void hisi_uc_pmu_disable_filter(struct perf_event *event)
+{
+ if (event->attr.config1 == 0)
+ return;
+
+ hisi_uc_pmu_clear_srcid_tracetag(event);
+ hisi_uc_pmu_clear_req_tracetag(event);
+ hisi_uc_pmu_clear_uring_channel(event);
+}
+
+static void hisi_uc_pmu_write_evtype(struct hisi_pmu *uc_pmu, int idx, u32 type)
+{
+ u32 val;
+
+ /*
+ * Select the appropriate event select register.
+ * There are 2 32-bit event select registers for the
+ * 8 hardware counters, each event code is 8-bit wide.
+ */
+ val = readl(uc_pmu->base + HISI_UC_EVTYPE_REG(idx / 4));
+ val &= ~(HISI_UC_EVTYPE_MASK << HISI_PMU_EVTYPE_SHIFT(idx));
+ val |= (type << HISI_PMU_EVTYPE_SHIFT(idx));
+ writel(val, uc_pmu->base + HISI_UC_EVTYPE_REG(idx / 4));
+}
+
+static void hisi_uc_pmu_start_counters(struct hisi_pmu *uc_pmu)
+{
+ u32 val;
+
+ val = readl(uc_pmu->base + HISI_UC_EVENT_CTRL_REG);
+ val |= HISI_UC_EVENT_GLB_EN;
+ writel(val, uc_pmu->base + HISI_UC_EVENT_CTRL_REG);
+}
+
+static void hisi_uc_pmu_stop_counters(struct hisi_pmu *uc_pmu)
+{
+ u32 val;
+
+ val = readl(uc_pmu->base + HISI_UC_EVENT_CTRL_REG);
+ val &= ~HISI_UC_EVENT_GLB_EN;
+ writel(val, uc_pmu->base + HISI_UC_EVENT_CTRL_REG);
+}
+
+static void hisi_uc_pmu_enable_counter(struct hisi_pmu *uc_pmu,
+ struct hw_perf_event *hwc)
+{
+ u32 val;
+
+ /* Enable counter index */
+ val = readl(uc_pmu->base + HISI_UC_EVENT_CTRL_REG);
+ val |= (1 << hwc->idx);
+ writel(val, uc_pmu->base + HISI_UC_EVENT_CTRL_REG);
+}
+
+static void hisi_uc_pmu_disable_counter(struct hisi_pmu *uc_pmu,
+ struct hw_perf_event *hwc)
+{
+ u32 val;
+
+ /* Clear counter index */
+ val = readl(uc_pmu->base + HISI_UC_EVENT_CTRL_REG);
+ val &= ~(1 << hwc->idx);
+ writel(val, uc_pmu->base + HISI_UC_EVENT_CTRL_REG);
+}
+
+static u64 hisi_uc_pmu_read_counter(struct hisi_pmu *uc_pmu,
+ struct hw_perf_event *hwc)
+{
+ return readq(uc_pmu->base + HISI_UC_CNTR_REG(hwc->idx));
+}
+
+static void hisi_uc_pmu_write_counter(struct hisi_pmu *uc_pmu,
+ struct hw_perf_event *hwc, u64 val)
+{
+ hisi_uc_pmu_start_counters(uc_pmu);
+ writeq(val, uc_pmu->base + HISI_UC_CNTR_REG(hwc->idx));
+}
+
+static void hisi_uc_pmu_enable_counter_int(struct hisi_pmu *uc_pmu,
+ struct hw_perf_event *hwc)
+{
+ u32 val;
+
+ val = readl(uc_pmu->base + HISI_UC_INT_MASK_REG);
+ /* Write 0 to enable interrupt */
The comment here and below maybe redundant. The code is self explanatory
Ok, i will drop this
+ val &= ~(1 << hwc->idx);
+ writel(val, uc_pmu->base + HISI_UC_INT_MASK_REG);
+}
+
+static void hisi_uc_pmu_disable_counter_int(struct hisi_pmu *uc_pmu,
+ struct hw_perf_event *hwc)
+{
+ u32 val;
+
+ val = readl(uc_pmu->base + HISI_UC_INT_MASK_REG);
+ /* Write 1 to mask interrupt */
+ val |= (1 << hwc->idx);
+ writel(val, uc_pmu->base + HISI_UC_INT_MASK_REG);
+}
+
+static u32 hisi_uc_pmu_get_int_status(struct hisi_pmu *uc_pmu)
+{
+ return readl(uc_pmu->base + HISI_UC_INT_STS_REG);
+}
+
+static void hisi_uc_pmu_clear_int_status(struct hisi_pmu *uc_pmu, int idx)
+{
+ writel(1 << idx, uc_pmu->base + HISI_UC_INT_CLEAR_REG);
+}
+
+static int hisi_uc_pmu_init_data(struct platform_device *pdev,
+ struct hisi_pmu *uc_pmu)
+{
+ /*
+ * Use the SCCL_ID and CCL_ID to identify the UC PMU, while
+ * SCCL_ID is in MPIDR[aff2] and CCL_ID is in MPIDR[aff1].
+ */
The comment doesn't match what you've done here, these information
is gotten from device properties rather than MPIDR.
Thanks.
Will update the comment
Thanks.
Best regards,
Junhao.