Hi John,

在 2021/1/26 20:30, John Garry 写道:

On 31/12/2020 06:19, Shaokun Zhang wrote:

quoted

HiSilicon's Hip09 is comprised by multi-dies that can be connected by SLLC
module (Skyros Link Layer Controller), its has separate PMU registers which
the driver can program it freely and interrupt is supported to handle
counter overflow. Let's support its driver under the framework of HiSilicon
uncore PMU driver.

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: John Garry <redacted>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Co-developed-by: Qi Liu <redacted>
Signed-off-by: Qi Liu <redacted>
Signed-off-by: Shaokun Zhang <redacted>

layout looks consistent with other hisi uncore PMU drivers, but some small comments, below

Reviewed-by: John Garry <redacted>

quoted

---
  drivers/perf/hisilicon/Makefile               |   2 +-
  drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c | 530 ++++++++++++++++++++++++++
  include/linux/cpuhotplug.h                    |   1 +
  3 files changed, 532 insertions(+), 1 deletion(-)
  create mode 100644 drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c

diff --git a/drivers/perf/hisilicon/Makefile b/drivers/perf/hisilicon/Makefile
index e8377061845f..6600a9d45dd8 100644
--- a/drivers/perf/hisilicon/Makefile
+++ b/drivers/perf/hisilicon/Makefile

@@ -1,3 +1,3 @@

  # 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_hha_pmu.o hisi_uncore_ddrc_pmu.o hisi_uncore_sllc_pmu.o

diff --git a/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c

b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c
new file mode 100644
index 000000000000..6911c388a5ac

--- /dev/null
+++ b/drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c

@@ -0,0 +1,530 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * HiSilicon SLLC uncore Hardware event counters support
+ *
+ * Copyright (C) 2020 Hisilicon Limited
+ * Author: Shaokun Zhang <zhangshaokun@hisilicon.com>
+ *
+ * This code is based on the uncore PMUs like arm-cci and arm-ccn.
+ */
+#include <linux/acpi.h>
+#include <linux/cpuhotplug.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/list.h>
+#include <linux/smp.h>
+
+#include "hisi_uncore_pmu.h"
+
+/* SLLC register definition */
+#define SLLC_INT_MASK            0x0814
+#define SLLC_INT_STATUS            0x0818
+#define SLLC_INT_CLEAR            0x081C
+#define SLLC_PERF_CTRL            0x1C00
+#define SLLC_SRCID_CTRL            0x1C04
+#define SLLC_TGTID_CTRL            0x1C08
+#define SLLC_EVENT_CTRL            0x1C14
+#define SLLC_EVENT_TYPE0        0x1C18
+#define SLLC_VERSION            0x1CF0
+#define SLLC_EVENT_CNT0_L        0x1D00
+
+#define SLLC_EVTYPE_MASK        0xFF
+#define SLLC_PERF_CTRL_EN        BIT(0)
+#define SLLC_FILT_EN            BIT(1)
+#define SLLC_TRACETAG_EN        BIT(2)
+#define SLLC_SRCID_EN            BIT(4)
+#define SLLC_SRCID_NONE            0x0
+#define SLLC_TGTID_EN            BIT(5)
+#define SLLC_TGTID_NONE            0x0
+#define SLLC_TGTID_LO_SHIFT        1
+#define SLLC_TGTID_HI_SHIFT        12
+#define SLLC_SRCID_CMD_SHIFT        1
+#define SLLC_SRCID_MSK_SHIFT        12
+#define SLLC_NR_EVENTS            0x80
+
+HISI_PMU_EVENT_ATTR_EXTRACTOR(tgtid_lo, config1, 10, 0);
+HISI_PMU_EVENT_ATTR_EXTRACTOR(tgtid_hi, config1, 21, 11);
+HISI_PMU_EVENT_ATTR_EXTRACTOR(srcid_cmd, config1, 32, 22);
+HISI_PMU_EVENT_ATTR_EXTRACTOR(srcid_msk, config1, 43, 33);
+HISI_PMU_EVENT_ATTR_EXTRACTOR(tracetag_en, config1, 44, 44);
+
+static inline bool tgtid_is_valid(u32 hi, u32 lo)

nit: no need for inline

Ok,

quoted

+{
+    return lo > 0 && hi >= lo;
+}
+
+static inline void hisi_sllc_pmu_enable_tracetag(struct perf_event *event)
+{
+    struct hisi_pmu *sllc_pmu = to_hisi_pmu(event->pmu);
+    u32 tt_en = hisi_get_tracetag_en(event);
+
+    if (tt_en) {
+        u32 val;
+
+        val = readl(sllc_pmu->base + SLLC_PERF_CTRL);
+        val |= SLLC_TRACETAG_EN | SLLC_FILT_EN;
+        writel(val, sllc_pmu->base + SLLC_PERF_CTRL);
+    }
+}
+
+static inline void hisi_sllc_pmu_disable_tracetag(struct perf_event *event)
+{
+    struct hisi_pmu *sllc_pmu = to_hisi_pmu(event->pmu);
+    u32 tt_en = hisi_get_tracetag_en(event);
+
+    if (tt_en) {
+        u32 val;
+
+        val = readl(sllc_pmu->base + SLLC_PERF_CTRL);
+        val &= ~(SLLC_TRACETAG_EN | SLLC_FILT_EN);
+        writel(val, sllc_pmu->base + SLLC_PERF_CTRL);

note: i think that all these can be _relaxed variant, but that is for existing drivers as well, so
can be reviewed later

Ok,

quoted

+    }
+}
+
+static inline void hisi_sllc_pmu_config_tgtid(struct perf_event *event)
+{
+    struct hisi_pmu *sllc_pmu = to_hisi_pmu(event->pmu);
+    u32 lo = hisi_get_tgtid_lo(event);
+    u32 hi = hisi_get_tgtid_hi(event);
+
+    if (tgtid_is_valid(hi, lo)) {
+        u32 val = (hi << SLLC_TGTID_HI_SHIFT) | (lo << SLLC_TGTID_LO_SHIFT);
+
+        writel(val, sllc_pmu->base + SLLC_TGTID_CTRL);
+
+        /* Enable the tgtid */
+        val = readl(sllc_pmu->base + SLLC_PERF_CTRL);
+        val |= SLLC_TGTID_EN | SLLC_FILT_EN;
+        writel(val, sllc_pmu->base + SLLC_PERF_CTRL);
+    }
+}
+    name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hisi_sccl%u_sllc%u",
+                  sllc_pmu->sccl_id, sllc_pmu->index_id);
+    if (!name) {
+        dev_err(&pdev->dev, "failed to allocate name for PMU\n");

maybe this message is not so useful

Ok, if it is not useful, let's return -ENOMEM directly.

quoted

+        return -ENOMEM;
+    }
+
+    ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HISI_SLLC_ONLINE,
+                       &sllc_pmu->node);
+    if (ret) {
+        dev_err(&pdev->dev, "Error %d registering hotplug\n", ret);
+        return ret;
+    }
+
+    sllc_pmu->pmu = (struct pmu) {
+        .module        = THIS_MODULE,
+        .task_ctx_nr    = perf_invalid_context,
+        .event_init    = hisi_uncore_pmu_event_init,
+        .pmu_enable    = hisi_uncore_pmu_enable,
+        .pmu_disable    = hisi_uncore_pmu_disable,
+        .add        = hisi_uncore_pmu_add,
+        .del        = hisi_uncore_pmu_del,
+        .start        = hisi_uncore_pmu_start,
+        .stop        = hisi_uncore_pmu_stop,
+        .read        = hisi_uncore_pmu_read,
+        .attr_groups    = sllc_pmu->pmu_events.attr_groups,
+        .capabilities    = PERF_PMU_CAP_NO_EXCLUDE,
+    };
+
+    ret = perf_pmu_register(&sllc_pmu->pmu, name, -1);
+    if (ret) {
+        dev_err(sllc_pmu->dev, "PMU register failed, ret = %d\n", ret);
+        cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_HISI_SLLC_ONLINE,
+                        &sllc_pmu->node);
+        irq_set_affinity_hint(sllc_pmu->irq, NULL);
+    }
+
+    platform_set_drvdata(pdev, sllc_pmu);

strange that we still do this for an error

Oops, this shall be set when alloc sllc_pmu.

quoted

+
+    return ret;
+}
+
+static int hisi_sllc_pmu_remove(struct platform_device *pdev)
+{
+    struct hisi_pmu *sllc_pmu = platform_get_drvdata(pdev);
+
+    perf_pmu_unregister(&sllc_pmu->pmu);
+    cpuhp_state_remove_instance_nocalls(CPUHP_AP_PERF_ARM_HISI_SLLC_ONLINE,
+                        &sllc_pmu->node);
+    irq_set_affinity_hint(sllc_pmu->irq, NULL);
+
+    return 0;
+}
+
+static struct platform_driver hisi_sllc_pmu_driver = {
+    .driver = {
+        .name = "hisi_sllc_pmu",
+        .acpi_match_table = hisi_sllc_pmu_acpi_match,

please set unbind unsupported

Ok,

Thanks,
Shaokun

quoted

+    },
+    .probe = hisi_sllc_pmu_probe,
+    .remove = hisi_sllc_pmu_remove,
+};
+
+static int __init hisi_sllc_pmu_module_init(void)
+{
+    int ret;
+
+    ret = cpuhp_setup_state_multi(CPUHP_AP_PERF_ARM_HISI_SLLC_ONLINE,
+                      "AP_PERF_ARM_HISI_SLLC_ONLINE",
+                      hisi_uncore_pmu_online_cpu,
+                      hisi_uncore_pmu_offline_cpu);
+    if (ret) {
+        pr_err("SLLC PMU: cpuhp state setup failed, ret = %d\n", ret);
+        return ret;
+    }
+
+    ret = platform_driver_register(&hisi_sllc_pmu_driver);
+    if (ret)
+        cpuhp_remove_multi_state(CPUHP_AP_PERF_ARM_HISI_SLLC_ONLINE);
+
+    return ret;
+}
+module_init(hisi_sllc_pmu_module_init);
+
+static void __exit hisi_sllc_pmu_module_exit(void)
+{
+    platform_driver_unregister(&hisi_sllc_pmu_driver);
+    cpuhp_remove_multi_state(CPUHP_AP_PERF_ARM_HISI_SLLC_ONLINE);
+}
+module_exit(hisi_sllc_pmu_module_exit);
+
+MODULE_DESCRIPTION("HiSilicon SLLC uncore PMU driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Shaokun Zhang [off-list ref]");

diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 0042ef362511..b5e04f9b68f1 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h

@@ -174,6 +174,7 @@ enum cpuhp_state {

      CPUHP_AP_PERF_ARM_HISI_DDRC_ONLINE,
      CPUHP_AP_PERF_ARM_HISI_HHA_ONLINE,
      CPUHP_AP_PERF_ARM_HISI_L3_ONLINE,
+    CPUHP_AP_PERF_ARM_HISI_SLLC_ONLINE,
      CPUHP_AP_PERF_ARM_L2X0_ONLINE,
      CPUHP_AP_PERF_ARM_QCOM_L2_ONLINE,
      CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,

.

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