[PATCH v8 0/2] Add ThunderX2 SoC Performance Monitoring Unit driver

[PATCH v8 0/2] Add ThunderX2 SoC Performance Monitoring Unit driver

[Kulkarni, Ganapatrao]

This patchset adds PMU driver for Cavium's ThunderX2 SoC UNCORE devices.
The SoC has PMU support in L3 cache controller (L3C) and in the
DDR4 Memory Controller (DMC).

v8:
	Updated with Comments [4]

[4] https://lkml.org/lkml/2018/10/25/215

v7:
	Incorporated review comments [3].
	Modified driver as loadable module.
	Updated Documentation with Event description.
	Removed per-channel(no SMC calls) sampling implementation(
	Since DMC and L3C channels are interleave, we have decided to
	sample channel zero and prorate it to account for a Device).

[3] https://patchwork.kernel.org/patch/10479203/

v6:
	Rebased to 4.18-rc1
	Updated with comments from John Garry[3]

[3] https://lkml.org/lkml/2018/5/17/408

v5:
	Incorporated review comments from Mark Rutland[2]
v4:
	Incorporated review comments from Mark Rutland[1]

[1] https://www.spinics.net/lists/arm-kernel/msg588563.html
[2] https://lkml.org/lkml/2018/4/26/376

v3:
	Fixed warning reported by kbuild robot

v2:
	Rebased to 4.12-rc1
	Removed Arch VULCAN dependency.
	Update SMC call parameters as per latest firmware.

v1:
	Initial patch


Ganapatrao Kulkarni (2):
  perf, uncore: Adding documentation for ThunderX2 pmu uncore driver
  ThunderX2, perf : Add Cavium ThunderX2 SoC UNCORE PMU driver

 Documentation/perf/thunderx2-pmu.txt | 106 ++++
 drivers/perf/Kconfig                 |   9 +
 drivers/perf/Makefile                |   1 +
 drivers/perf/thunderx2_pmu.c         | 869 +++++++++++++++++++++++++++
 include/linux/cpuhotplug.h           |   1 +
 5 files changed, 986 insertions(+)
 create mode 100644 Documentation/perf/thunderx2-pmu.txt
 create mode 100644 drivers/perf/thunderx2_pmu.c

-- 
2.18.0

[PATCH v8 1/2] perf, uncore: Adding documentation for ThunderX2 pmu uncore driver

[Kulkarni, Ganapatrao]

The SoC has PMU support in its L3 cache controller (L3C) and in the
DDR4 Memory Controller (DMC).

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
---
 Documentation/perf/thunderx2-pmu.txt | 106 +++++++++++++++++++++++++++
 1 file changed, 106 insertions(+)
 create mode 100644 Documentation/perf/thunderx2-pmu.txt

diff --git a/Documentation/perf/thunderx2-pmu.txt b/Documentation/perf/thunderx2-pmu.txt
new file mode 100644
index 000000000000..9f5dd7459e68
--- /dev/null
+++ b/Documentation/perf/thunderx2-pmu.txt
@@ -0,0 +1,106 @@
+
+Cavium ThunderX2 SoC Performance Monitoring Unit (PMU UNCORE)
+==========================================================================
+
+ThunderX2 SoC PMU consists of independent system wide per Socket PMUs such
+as Level 3 Cache(L3C) and DDR4 Memory Controller(DMC).
+
+DMC has 8 interleave channels and L3C has 16 interleave tiles. Events are
+sampled for default channel(i.e channel 0) and prorated to total number of
+channels/tiles.
+
+DMC and L3C, Each PMU supports up to 4 counters. Counters are independently
+programmable and can be started and stopped individually. Each counter can
+be set to sample specific perf events. Counters are 32 bit and do not support
+overflow interrupt; they are sampled at every 2 seconds.
+
+PMU UNCORE (perf) driver:
+
+The thunderx2-pmu driver registers several perf PMUs for DMC and L3C devices.
+Each of the PMUs provides description of its available events
+and configuration options in sysfs.
+	see /sys/devices/uncore_<l3c_S/dmc_S/>
+
+S is socket id.
+Each PMU can be used to sample up to 4 events simultaneously.
+
+The "format" directory describes format of the config (event ID).
+The "events" directory provides configuration templates for all
+supported event types that can be used with perf tool.
+
+For example, "uncore_dmc_0/cnt_cycles/" is an
+equivalent of "uncore_dmc_0/config=0x1/".
+
+Each perf driver also provides a "cpumask" sysfs attribute, which contains a
+single CPU ID of the processor which is likely to be used to handle all the
+PMU events. It will be the first online CPU from the NUMA node of the PMU device.
+
+Example for perf tool use:
+
+perf stat -a -e uncore_dmc_0/cnt_cycles/ sleep 1
+
+perf stat -a -e \
+uncore_dmc_0/cnt_cycles/,\
+uncore_dmc_0/data_transfers/,\
+uncore_dmc_0/read_txns/,\
+uncore_dmc_0/write_txns/ sleep 1
+
+perf stat -a -e \
+uncore_l3c_0/read_request/,\
+uncore_l3c_0/read_hit/,\
+uncore_l3c_0/inv_request/,\
+uncore_l3c_0/inv_hit/ sleep 1
+
+The driver does not support sampling, therefore "perf record" will
+not work. Per-task (without "-a") perf sessions are not supported.
+
+L3C events:
+============
+
+read_request:
+	Number of Read requests received by the L3 Cache.
+	This include Read as well as Read Exclusives.
+
+read_hit:
+	Number of Read requests received by the L3 cache that were hit
+	in the L3 (Data provided form the L3)
+
+writeback_request:
+	Number of Write Backs received by the L3 Cache. These are basically
+	the L2 Evicts and writes from the PCIe Write Cache.
+
+inv_nwrite_request:
+	This is the Number of Invalidate and Write received by the L3 Cache.
+	Also Writes from IO that did not go through the PCIe Write Cache.
+
+inv_nwrite_hit
+	This is the Number of Invalidate and Write received by the L3 Cache
+	That were a hit in the L3 Cache.
+
+inv_request:
+	Number of Invalidate request received by the L3 Cache.
+
+inv_hit:
+	Number of Invalidate request received by the L3 Cache that were a
+	hit in L3.
+
+evict_request:
+	Number of Evicts that the L3 generated.
+
+NOTE:
+1. Granularity of all these events counter value is cache line length(64 Bytes).
+2. L3C cache Hit Ratio = (read_hit + inv_nwrite_hit + inv_hit) / (read_request + inv_nwrite_request + inv_request)
+
+DMC events:
+============
+cnt_cycles:
+	Count cycles (Clocks at the DMC clock rate)
+
+write_txns:
+	Number of 64 Bytes write transactions received by the DMC(s)
+
+read_txns:
+	Number of 64 Bytes Read transactions received by the DMC(s)
+
+data_transfers:
+	Number of 64 Bytes data transferred to or from DRAM.
-- 
2.18.0

[PATCH v8 2/2] ThunderX2, perf : Add Cavium ThunderX2 SoC UNCORE PMU driver

[Kulkarni, Ganapatrao]

This patch adds a perf driver for the PMU UNCORE devices DDR4 Memory
Controller(DMC) and Level 3 Cache(L3C). Each PMU supports up to 4
counters. All counters lack overflow interrupt and are
sampled periodically.

Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
---
 drivers/perf/Kconfig         |   9 +
 drivers/perf/Makefile        |   1 +
 drivers/perf/thunderx2_pmu.c | 869 +++++++++++++++++++++++++++++++++++
 include/linux/cpuhotplug.h   |   1 +
 4 files changed, 880 insertions(+)
 create mode 100644 drivers/perf/thunderx2_pmu.c

diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index 08ebaf7cca8b..af9bc178495d 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig
@@ -87,6 +87,15 @@ config QCOM_L3_PMU
 	   Adds the L3 cache PMU into the perf events subsystem for
 	   monitoring L3 cache events.
 
+config THUNDERX2_PMU
+	tristate "Cavium ThunderX2 SoC PMU UNCORE"
+	depends on ARCH_THUNDER2 && ARM64 && ACPI && NUMA
+	default m
+	help
+	   Provides support for ThunderX2 UNCORE events.
+	   The SoC has PMU support in its L3 cache controller (L3C) and
+	   in the DDR4 Memory Controller (DMC).
+
 config XGENE_PMU
         depends on ARCH_XGENE
         bool "APM X-Gene SoC PMU"
diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
index b3902bd37d53..909f27fd9db3 100644
--- a/drivers/perf/Makefile
+++ b/drivers/perf/Makefile
@@ -7,5 +7,6 @@ obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
 obj-$(CONFIG_HISI_PMU) += hisilicon/
 obj-$(CONFIG_QCOM_L2_PMU)	+= qcom_l2_pmu.o
 obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
+obj-$(CONFIG_THUNDERX2_PMU) += thunderx2_pmu.o
 obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
 obj-$(CONFIG_ARM_SPE_PMU) += arm_spe_pmu.o
diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
new file mode 100644
index 000000000000..e6509ba868ab
--- /dev/null
+++ b/drivers/perf/thunderx2_pmu.c
@@ -0,0 +1,869 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CAVIUM THUNDERX2 SoC PMU UNCORE
+ * Copyright (C) 2018 Cavium Inc.
+ * Author: Ganapatrao Kulkarni <gkulkarni@cavium.com>
+ */
+
+#include <linux/acpi.h>
+#include <linux/cpuhotplug.h>
+#include <linux/perf_event.h>
+#include <linux/platform_device.h>
+
+/* Each ThunderX2(TX2) Socket has a L3C and DMC UNCORE PMU device.
+ * Each UNCORE PMU device consists of 4 independent programmable counters.
+ * Counters are 32 bit and do not support overflow interrupt,
+ * they need to be sampled before overflow(i.e, at every 2 seconds).
+ */
+
+#define TX2_PMU_MAX_COUNTERS		4
+#define TX2_PMU_DMC_CHANNELS		8
+#define TX2_PMU_L3_TILES		16
+
+#define TX2_PMU_HRTIMER_INTERVAL	(2 * NSEC_PER_SEC)
+#define GET_EVENTID(ev)			((ev->hw.config) & 0x1ff)
+#define GET_COUNTERID(ev)		((ev->hw.idx) & 0x3)
+ /* 1 byte per counter(4 counters).
+  * Event id is encoded in bits [5:1] of a byte,
+  */
+#define DMC_EVENT_CFG(idx, val)		((val) << (((idx) * 8) + 1))
+
+#define L3C_COUNTER_CTL			0xA8
+#define L3C_COUNTER_DATA		0xAC
+#define DMC_COUNTER_CTL			0x234
+#define DMC_COUNTER_DATA		0x240
+
+/* L3C event IDs */
+#define L3_EVENT_READ_REQ		0xD
+#define L3_EVENT_WRITEBACK_REQ		0xE
+#define L3_EVENT_INV_N_WRITE_REQ	0xF
+#define L3_EVENT_INV_REQ		0x10
+#define L3_EVENT_EVICT_REQ		0x13
+#define L3_EVENT_INV_N_WRITE_HIT	0x14
+#define L3_EVENT_INV_HIT		0x15
+#define L3_EVENT_READ_HIT		0x17
+#define L3_EVENT_MAX			0x18
+
+/* DMC event IDs */
+#define DMC_EVENT_COUNT_CYCLES		0x1
+#define DMC_EVENT_WRITE_TXNS		0xB
+#define DMC_EVENT_DATA_TRANSFERS	0xD
+#define DMC_EVENT_READ_TXNS		0xF
+#define DMC_EVENT_MAX			0x10
+
+enum tx2_uncore_type {
+	PMU_TYPE_L3C,
+	PMU_TYPE_DMC,
+	PMU_TYPE_INVALID,
+};
+
+/*
+ * pmu on each socket has 2 uncore devices(dmc and l3c),
+ * each device has 4 counters.
+ */
+struct tx2_uncore_pmu {
+	struct hlist_node hpnode;
+	struct list_head  entry;
+	struct pmu pmu;
+	char *name;
+	int node;
+	int cpu;
+	u32    max_counters;
+	u32    prorate_factor;
+	u32    max_events;
+	u64 hrtimer_interval;
+	void __iomem *base;
+	DECLARE_BITMAP(active_counters, TX2_PMU_MAX_COUNTERS);
+	struct perf_event *events[TX2_PMU_MAX_COUNTERS];
+	struct device *dev;
+	struct hrtimer hrtimer;
+	const struct attribute_group **attr_groups;
+	enum tx2_uncore_type type;
+	void	(*init_cntr_base)(struct perf_event *event,
+			struct tx2_uncore_pmu *tx2_pmu);
+	void	(*stop_event)(struct perf_event *event);
+	void	(*start_event)(struct perf_event *event, int flags);
+};
+
+static LIST_HEAD(tx2_pmus);
+
+static inline struct tx2_uncore_pmu *pmu_to_tx2_pmu(struct pmu *pmu)
+{
+	return container_of(pmu, struct tx2_uncore_pmu, pmu);
+}
+
+/*
+ * sysfs format attributes
+ */
+static ssize_t tx2_pmu_format_show(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct dev_ext_attribute *eattr;
+
+	eattr = container_of(attr, struct dev_ext_attribute, attr);
+	return sprintf(buf, "%s\n", (char *) eattr->var);
+}
+
+#define FORMAT_ATTR(_name, _config) \
+	(&((struct dev_ext_attribute[]) { \
+	   { \
+	   .attr = __ATTR(_name, 0444, tx2_pmu_format_show, NULL), \
+	   .var = (void *) _config, \
+	   } \
+	})[0].attr.attr)
+
+static struct attribute *l3c_pmu_format_attrs[] = {
+	FORMAT_ATTR(event,	"config:0-4"),
+	NULL,
+};
+
+static struct attribute *dmc_pmu_format_attrs[] = {
+	FORMAT_ATTR(event,	"config:0-4"),
+	NULL,
+};
+
+static const struct attribute_group l3c_pmu_format_attr_group = {
+	.name = "format",
+	.attrs = l3c_pmu_format_attrs,
+};
+
+static const struct attribute_group dmc_pmu_format_attr_group = {
+	.name = "format",
+	.attrs = dmc_pmu_format_attrs,
+};
+
+/*
+ * sysfs event attributes
+ */
+static ssize_t tx2_pmu_event_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct dev_ext_attribute *eattr;
+
+	eattr = container_of(attr, struct dev_ext_attribute, attr);
+	return sprintf(buf, "config=0x%lx\n", (unsigned long) eattr->var);
+}
+
+#define EVENT_ATTR(_name, _config) \
+	(&((struct dev_ext_attribute[]) { \
+	   { \
+	   .attr = __ATTR(_name, 0444, tx2_pmu_event_show, NULL), \
+	   .var = (void *) _config, \
+	   } \
+	 })[0].attr.attr)
+
+static struct attribute *l3c_pmu_events_attrs[] = {
+	EVENT_ATTR(read_request,		L3_EVENT_READ_REQ),
+	EVENT_ATTR(writeback_request,		L3_EVENT_WRITEBACK_REQ),
+	EVENT_ATTR(inv_nwrite_request,		L3_EVENT_INV_N_WRITE_REQ),
+	EVENT_ATTR(inv_request,			L3_EVENT_INV_REQ),
+	EVENT_ATTR(evict_request,		L3_EVENT_EVICT_REQ),
+	EVENT_ATTR(inv_nwrite_hit,		L3_EVENT_INV_N_WRITE_HIT),
+	EVENT_ATTR(inv_hit,			L3_EVENT_INV_HIT),
+	EVENT_ATTR(read_hit,			L3_EVENT_READ_HIT),
+	NULL,
+};
+
+static struct attribute *dmc_pmu_events_attrs[] = {
+	EVENT_ATTR(cnt_cycles,			DMC_EVENT_COUNT_CYCLES),
+	EVENT_ATTR(write_txns,			DMC_EVENT_WRITE_TXNS),
+	EVENT_ATTR(data_transfers,		DMC_EVENT_DATA_TRANSFERS),
+	EVENT_ATTR(read_txns,			DMC_EVENT_READ_TXNS),
+	NULL,
+};
+
+static const struct attribute_group l3c_pmu_events_attr_group = {
+	.name = "events",
+	.attrs = l3c_pmu_events_attrs,
+};
+
+static const struct attribute_group dmc_pmu_events_attr_group = {
+	.name = "events",
+	.attrs = dmc_pmu_events_attrs,
+};
+
+/*
+ * sysfs cpumask attributes
+ */
+static ssize_t cpumask_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct tx2_uncore_pmu *tx2_pmu;
+
+	tx2_pmu = pmu_to_tx2_pmu(dev_get_drvdata(dev));
+	return cpumap_print_to_pagebuf(true, buf, cpumask_of(tx2_pmu->cpu));
+}
+static DEVICE_ATTR_RO(cpumask);
+
+static struct attribute *tx2_pmu_cpumask_attrs[] = {
+	&dev_attr_cpumask.attr,
+	NULL,
+};
+
+static const struct attribute_group pmu_cpumask_attr_group = {
+	.attrs = tx2_pmu_cpumask_attrs,
+};
+
+/*
+ * Per PMU device attribute groups
+ */
+static const struct attribute_group *l3c_pmu_attr_groups[] = {
+	&l3c_pmu_format_attr_group,
+	&pmu_cpumask_attr_group,
+	&l3c_pmu_events_attr_group,
+	NULL
+};
+
+static const struct attribute_group *dmc_pmu_attr_groups[] = {
+	&dmc_pmu_format_attr_group,
+	&pmu_cpumask_attr_group,
+	&dmc_pmu_events_attr_group,
+	NULL
+};
+
+static inline u32 reg_readl(unsigned long addr)
+{
+	return readl((void __iomem *)addr);
+}
+
+static inline void reg_writel(u32 val, unsigned long addr)
+{
+	writel(val, (void __iomem *)addr);
+}
+
+static int alloc_counter(struct tx2_uncore_pmu *tx2_pmu)
+{
+	int counter;
+
+	counter = find_first_zero_bit(tx2_pmu->active_counters,
+				tx2_pmu->max_counters);
+	if (counter == tx2_pmu->max_counters)
+		return -ENOSPC;
+
+	set_bit(counter, tx2_pmu->active_counters);
+	return counter;
+}
+
+static inline void free_counter(struct tx2_uncore_pmu *tx2_pmu, int counter)
+{
+	clear_bit(counter, tx2_pmu->active_counters);
+}
+
+static void init_cntr_base_l3c(struct perf_event *event,
+		struct tx2_uncore_pmu *tx2_pmu)
+{
+	struct hw_perf_event *hwc = &event->hw;
+
+	/* counter ctrl/data reg offset at 8 */
+	hwc->config_base = (unsigned long)tx2_pmu->base
+		+ L3C_COUNTER_CTL + (8 * GET_COUNTERID(event));
+	hwc->event_base =  (unsigned long)tx2_pmu->base
+		+ L3C_COUNTER_DATA + (8 * GET_COUNTERID(event));
+}
+
+static void init_cntr_base_dmc(struct perf_event *event,
+		struct tx2_uncore_pmu *tx2_pmu)
+{
+	struct hw_perf_event *hwc = &event->hw;
+
+	hwc->config_base = (unsigned long)tx2_pmu->base
+		+ DMC_COUNTER_CTL;
+	/* counter data reg offset at 0xc */
+	hwc->event_base = (unsigned long)tx2_pmu->base
+		+ DMC_COUNTER_DATA + (0xc * GET_COUNTERID(event));
+}
+
+static void uncore_start_event_l3c(struct perf_event *event, int flags)
+{
+	u32 val;
+	struct hw_perf_event *hwc = &event->hw;
+
+	/* event id encoded in bits [07:03] */
+	val = GET_EVENTID(event) << 3;
+	reg_writel(val, hwc->config_base);
+	local64_set(&hwc->prev_count, 0);
+	reg_writel(0, hwc->event_base);
+}
+
+static inline void uncore_stop_event_l3c(struct perf_event *event)
+{
+	reg_writel(0, event->hw.config_base);
+}
+
+static void uncore_start_event_dmc(struct perf_event *event, int flags)
+{
+	u32 val;
+	struct hw_perf_event *hwc = &event->hw;
+	int idx = GET_COUNTERID(event);
+	int event_id = GET_EVENTID(event);
+
+	/* enable and start counters.
+	 * 8 bits for each counter, bits[05:01] of a counter to set event type.
+	 */
+	val = reg_readl(hwc->config_base);
+	val &= ~DMC_EVENT_CFG(idx, 0x1f);
+	val |= DMC_EVENT_CFG(idx, event_id);
+	reg_writel(val, hwc->config_base);
+	local64_set(&hwc->prev_count, 0);
+	reg_writel(0, hwc->event_base);
+}
+
+static void uncore_stop_event_dmc(struct perf_event *event)
+{
+	u32 val;
+	struct hw_perf_event *hwc = &event->hw;
+	int idx = GET_COUNTERID(event);
+
+	/* clear event type(bits[05:01]) to stop counter */
+	val = reg_readl(hwc->config_base);
+	val &= ~DMC_EVENT_CFG(idx, 0x1f);
+	reg_writel(val, hwc->config_base);
+}
+
+static void tx2_uncore_event_update(struct perf_event *event)
+{
+	s64 prev, delta, new = 0;
+	struct hw_perf_event *hwc = &event->hw;
+	struct tx2_uncore_pmu *tx2_pmu;
+	enum tx2_uncore_type type;
+	u32    prorate_factor;
+
+	tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+	type = tx2_pmu->type;
+	prorate_factor = tx2_pmu->prorate_factor;
+
+	new = reg_readl(hwc->event_base);
+	prev = local64_xchg(&hwc->prev_count, new);
+
+	/* handles rollover of 32 bit counter */
+	delta = (u32)(((1UL << 32) - prev) + new);
+
+	/* DMC event data_transfers granularity is 16 Bytes, convert it to 64 */
+	if (type == PMU_TYPE_DMC &&
+			GET_EVENTID(event) == DMC_EVENT_DATA_TRANSFERS)
+		delta = delta/4;
+
+	/* L3C and DMC has 16 and 8 interleave channels respectively.
+	 * The sampled value is for channel 0 and multiplied with
+	 * prorate_factor to get the count for a device.
+	 */
+	local64_add(delta * prorate_factor, &event->count);
+}
+
+enum tx2_uncore_type get_tx2_pmu_type(struct acpi_device *adev)
+{
+	int i = 0;
+	struct acpi_tx2_pmu_device {
+		__u8 id[ACPI_ID_LEN];
+		enum tx2_uncore_type type;
+	} devices[] = {
+		{"CAV901D", PMU_TYPE_L3C},
+		{"CAV901F", PMU_TYPE_DMC},
+		{"", PMU_TYPE_INVALID}
+	};
+
+	while (devices[i].type != PMU_TYPE_INVALID) {
+		if (!strcmp(acpi_device_hid(adev), devices[i].id))
+			break;
+		i++;
+	}
+
+	return devices[i].type;
+}
+
+static bool tx2_uncore_validate_event(struct pmu *pmu,
+				  struct perf_event *event, int *counters)
+{
+	if (is_software_event(event))
+		return true;
+	/* Reject groups spanning multiple HW PMUs. */
+	if (event->pmu != pmu)
+		return false;
+
+	*counters = *counters + 1;
+		return true;
+}
+
+/*
+ * Make sure the group of events can be scheduled at once
+ * on the PMU.
+ */
+static bool tx2_uncore_validate_event_group(struct perf_event *event)
+{
+	struct perf_event *sibling, *leader = event->group_leader;
+	int counters = 0;
+
+	if (event->group_leader == event)
+		return true;
+
+	if (!tx2_uncore_validate_event(event->pmu, leader, &counters))
+		return false;
+
+	for_each_sibling_event(sibling, leader) {
+		if (!tx2_uncore_validate_event(event->pmu, sibling, &counters))
+			return false;
+	}
+
+	if (!tx2_uncore_validate_event(event->pmu, event, &counters))
+		return false;
+
+	/*
+	 * If the group requires more counters than the HW has,
+	 * it cannot ever be scheduled.
+	 */
+	return counters <= TX2_PMU_MAX_COUNTERS;
+}
+
+
+static int tx2_uncore_event_init(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct tx2_uncore_pmu *tx2_pmu;
+
+	/* Test the event attr type check for PMU enumeration */
+	if (event->attr.type != event->pmu->type)
+		return -ENOENT;
+
+	/*
+	 * SOC PMU counters are shared across all cores.
+	 * Therefore, it does not support per-process mode.
+	 * Also, it does not support event sampling mode.
+	 */
+	if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
+		return -EINVAL;
+
+	/* We have no filtering of any kind */
+	if (event->attr.exclude_user	||
+	    event->attr.exclude_kernel	||
+	    event->attr.exclude_hv	||
+	    event->attr.exclude_idle	||
+	    event->attr.exclude_host	||
+	    event->attr.exclude_guest)
+		return -EINVAL;
+
+	if (event->cpu < 0)
+		return -EINVAL;
+
+	tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+	if (tx2_pmu->cpu >= nr_cpu_ids)
+		return -EINVAL;
+	event->cpu = tx2_pmu->cpu;
+
+	if (event->attr.config >= tx2_pmu->max_events)
+		return -EINVAL;
+
+	/* store event id */
+	hwc->config = event->attr.config;
+
+	/* Validate the group */
+	if (!tx2_uncore_validate_event_group(event))
+		return -EINVAL;
+
+	return 0;
+}
+
+static void tx2_uncore_event_start(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct tx2_uncore_pmu *tx2_pmu;
+
+	hwc->state = 0;
+	tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+
+	tx2_pmu->start_event(event, flags);
+	perf_event_update_userpage(event);
+
+	/* Start timer for first event */
+	if (bitmap_weight(tx2_pmu->active_counters,
+				tx2_pmu->max_counters) == 1) {
+		hrtimer_start(&tx2_pmu->hrtimer,
+			ns_to_ktime(tx2_pmu->hrtimer_interval),
+			HRTIMER_MODE_REL_PINNED);
+	}
+}
+
+static void tx2_uncore_event_stop(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct tx2_uncore_pmu *tx2_pmu;
+
+	if (hwc->state & PERF_HES_UPTODATE)
+		return;
+
+	tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+	tx2_pmu->stop_event(event);
+	WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
+	hwc->state |= PERF_HES_STOPPED;
+	if (flags & PERF_EF_UPDATE) {
+		tx2_uncore_event_update(event);
+		hwc->state |= PERF_HES_UPTODATE;
+	}
+}
+
+static int tx2_uncore_event_add(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct tx2_uncore_pmu *tx2_pmu;
+
+	tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+
+	/* Allocate a free counter */
+	hwc->idx  = alloc_counter(tx2_pmu);
+	if (hwc->idx < 0)
+		return -EAGAIN;
+
+	tx2_pmu->events[hwc->idx] = event;
+	/* set counter control and data registers base address */
+	tx2_pmu->init_cntr_base(event, tx2_pmu);
+
+	hwc->state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
+	if (flags & PERF_EF_START)
+		tx2_uncore_event_start(event, flags);
+
+	return 0;
+}
+
+static void tx2_uncore_event_del(struct perf_event *event, int flags)
+{
+	struct tx2_uncore_pmu *tx2_pmu = pmu_to_tx2_pmu(event->pmu);
+	struct hw_perf_event *hwc = &event->hw;
+
+	tx2_uncore_event_stop(event, PERF_EF_UPDATE);
+
+	/* clear the assigned counter */
+	free_counter(tx2_pmu, GET_COUNTERID(event));
+
+	perf_event_update_userpage(event);
+	tx2_pmu->events[hwc->idx] = NULL;
+	hwc->idx = -1;
+}
+
+static void tx2_uncore_event_read(struct perf_event *event)
+{
+	tx2_uncore_event_update(event);
+}
+
+static enum hrtimer_restart tx2_hrtimer_callback(struct hrtimer *timer)
+{
+	struct tx2_uncore_pmu *tx2_pmu;
+	int max_counters, idx;
+
+	tx2_pmu = container_of(timer, struct tx2_uncore_pmu, hrtimer);
+	max_counters = tx2_pmu->max_counters;
+
+	if (bitmap_empty(tx2_pmu->active_counters, max_counters))
+		return HRTIMER_NORESTART;
+
+	for_each_set_bit(idx, tx2_pmu->active_counters, max_counters) {
+		struct perf_event *event = tx2_pmu->events[idx];
+
+		tx2_uncore_event_update(event);
+	}
+	hrtimer_forward_now(timer, ns_to_ktime(tx2_pmu->hrtimer_interval));
+	return HRTIMER_RESTART;
+}
+
+static int tx2_uncore_pmu_register(
+		struct tx2_uncore_pmu *tx2_pmu)
+{
+	struct device *dev = tx2_pmu->dev;
+	char *name = tx2_pmu->name;
+
+	/* Perf event registration */
+	tx2_pmu->pmu = (struct pmu) {
+		.module         = THIS_MODULE,
+		.attr_groups	= tx2_pmu->attr_groups,
+		.task_ctx_nr	= perf_invalid_context,
+		.event_init	= tx2_uncore_event_init,
+		.add		= tx2_uncore_event_add,
+		.del		= tx2_uncore_event_del,
+		.start		= tx2_uncore_event_start,
+		.stop		= tx2_uncore_event_stop,
+		.read		= tx2_uncore_event_read,
+	};
+
+	tx2_pmu->pmu.name = devm_kasprintf(dev, GFP_KERNEL,
+			"%s", name);
+
+	return perf_pmu_register(&tx2_pmu->pmu, tx2_pmu->pmu.name, -1);
+}
+
+static int tx2_uncore_pmu_add_dev(struct tx2_uncore_pmu *tx2_pmu)
+{
+	int ret, cpu;
+
+	cpu = cpumask_any_and(cpumask_of_node(tx2_pmu->node),
+			cpu_online_mask);
+
+	tx2_pmu->cpu = cpu;
+	hrtimer_init(&tx2_pmu->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	tx2_pmu->hrtimer.function = tx2_hrtimer_callback;
+
+	ret = tx2_uncore_pmu_register(tx2_pmu);
+	if (ret) {
+		dev_err(tx2_pmu->dev, "%s PMU: Failed to init driver\n",
+				tx2_pmu->name);
+		return -ENODEV;
+	}
+
+	/* register hotplug callback for the pmu */
+	ret = cpuhp_state_add_instance(
+			CPUHP_AP_PERF_ARM_TX2_UNCORE_ONLINE,
+			&tx2_pmu->hpnode);
+	if (ret) {
+		dev_err(tx2_pmu->dev, "Error %d registering hotplug", ret);
+		return ret;
+	}
+
+	/* Add to list */
+	list_add(&tx2_pmu->entry, &tx2_pmus);
+
+	dev_dbg(tx2_pmu->dev, "%s PMU UNCORE registered\n",
+			tx2_pmu->pmu.name);
+	return ret;
+}
+
+static struct tx2_uncore_pmu *tx2_uncore_pmu_init_dev(struct device *dev,
+		acpi_handle handle, struct acpi_device *adev, u32 type)
+{
+	struct tx2_uncore_pmu *tx2_pmu;
+	void __iomem *base;
+	struct resource res;
+	struct resource_entry *rentry;
+	struct list_head list;
+	int ret;
+
+	INIT_LIST_HEAD(&list);
+	ret = acpi_dev_get_resources(adev, &list, NULL, NULL);
+	if (ret <= 0) {
+		dev_err(dev, "failed to parse _CRS method, error %d\n", ret);
+		return NULL;
+	}
+
+	list_for_each_entry(rentry, &list, node) {
+		if (resource_type(rentry->res) == IORESOURCE_MEM) {
+			res = *rentry->res;
+			break;
+		}
+	}
+
+	if (!rentry->res)
+		return NULL;
+
+	acpi_dev_free_resource_list(&list);
+	base = devm_ioremap_resource(dev, &res);
+	if (IS_ERR(base)) {
+		dev_err(dev, "PMU type %d: Fail to map resource\n", type);
+		return NULL;
+	}
+
+	tx2_pmu = devm_kzalloc(dev, sizeof(*tx2_pmu), GFP_KERNEL);
+	if (!tx2_pmu)
+		return NULL;
+
+	tx2_pmu->dev = dev;
+	tx2_pmu->type = type;
+	tx2_pmu->base = base;
+	tx2_pmu->node = dev_to_node(dev);
+	INIT_LIST_HEAD(&tx2_pmu->entry);
+
+	switch (tx2_pmu->type) {
+	case PMU_TYPE_L3C:
+		tx2_pmu->max_counters = TX2_PMU_MAX_COUNTERS;
+		tx2_pmu->prorate_factor = TX2_PMU_L3_TILES;
+		tx2_pmu->max_events = L3_EVENT_MAX;
+		tx2_pmu->hrtimer_interval = TX2_PMU_HRTIMER_INTERVAL;
+		tx2_pmu->attr_groups = l3c_pmu_attr_groups;
+		tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL,
+				"uncore_l3c_%d", tx2_pmu->node);
+		tx2_pmu->init_cntr_base = init_cntr_base_l3c;
+		tx2_pmu->start_event = uncore_start_event_l3c;
+		tx2_pmu->stop_event = uncore_stop_event_l3c;
+		break;
+	case PMU_TYPE_DMC:
+		tx2_pmu->max_counters = TX2_PMU_MAX_COUNTERS;
+		tx2_pmu->prorate_factor = TX2_PMU_DMC_CHANNELS;
+		tx2_pmu->max_events = DMC_EVENT_MAX;
+		tx2_pmu->hrtimer_interval = TX2_PMU_HRTIMER_INTERVAL;
+		tx2_pmu->attr_groups = dmc_pmu_attr_groups;
+		tx2_pmu->name = devm_kasprintf(dev, GFP_KERNEL,
+				"uncore_dmc_%d", tx2_pmu->node);
+		tx2_pmu->init_cntr_base = init_cntr_base_dmc;
+		tx2_pmu->start_event = uncore_start_event_dmc;
+		tx2_pmu->stop_event = uncore_stop_event_dmc;
+		break;
+	case PMU_TYPE_INVALID:
+		devm_kfree(dev, tx2_pmu);
+		return NULL;
+	}
+
+	return tx2_pmu;
+}
+
+static acpi_status tx2_uncore_pmu_add(acpi_handle handle, u32 level,
+				    void *data, void **return_value)
+{
+	struct tx2_uncore_pmu *tx2_pmu;
+	struct acpi_device *adev;
+	enum tx2_uncore_type type;
+
+	if (acpi_bus_get_device(handle, &adev))
+		return AE_OK;
+	if (acpi_bus_get_status(adev) || !adev->status.present)
+		return AE_OK;
+
+	type = get_tx2_pmu_type(adev);
+	if (type == PMU_TYPE_INVALID)
+		return AE_OK;
+
+	tx2_pmu = tx2_uncore_pmu_init_dev((struct device *)data,
+			handle, adev, type);
+
+	if (!tx2_pmu)
+		return AE_ERROR;
+
+	if (tx2_uncore_pmu_add_dev(tx2_pmu)) {
+		/* Can't add the PMU device, abort */
+		return AE_ERROR;
+	}
+	return AE_OK;
+}
+
+static int tx2_uncore_pmu_online_cpu(unsigned int cpu,
+		struct hlist_node *hpnode)
+{
+	struct tx2_uncore_pmu *tx2_pmu;
+
+	tx2_pmu = hlist_entry_safe(hpnode,
+			struct tx2_uncore_pmu, hpnode);
+
+	/* Pick this CPU, If there is no CPU/PMU association and both are
+	 * from same node.
+	 */
+	if ((tx2_pmu->cpu >= nr_cpu_ids) &&
+		(tx2_pmu->node == cpu_to_node(cpu)))
+		tx2_pmu->cpu = cpu;
+
+	return 0;
+}
+
+static int tx2_uncore_pmu_offline_cpu(unsigned int cpu,
+		struct hlist_node *hpnode)
+{
+	int new_cpu;
+	struct tx2_uncore_pmu *tx2_pmu;
+	struct cpumask cpu_online_mask_temp;
+
+	tx2_pmu = hlist_entry_safe(hpnode,
+			struct tx2_uncore_pmu, hpnode);
+
+	if (cpu != tx2_pmu->cpu)
+		return 0;
+
+	hrtimer_cancel(&tx2_pmu->hrtimer);
+	cpumask_copy(&cpu_online_mask_temp, cpu_online_mask);
+	cpumask_clear_cpu(cpu, &cpu_online_mask_temp);
+	new_cpu = cpumask_any_and(
+			cpumask_of_node(tx2_pmu->node),
+			&cpu_online_mask_temp);
+
+	tx2_pmu->cpu = new_cpu;
+	if (new_cpu >= nr_cpu_ids)
+		return 0;
+	perf_pmu_migrate_context(&tx2_pmu->pmu, cpu, new_cpu);
+
+	return 0;
+}
+
+static const struct acpi_device_id tx2_uncore_acpi_match[] = {
+	{"CAV901C", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, tx2_uncore_acpi_match);
+
+static int tx2_uncore_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	acpi_handle handle;
+	acpi_status status;
+
+	set_dev_node(dev, acpi_get_node(ACPI_HANDLE(dev)));
+
+	if (!has_acpi_companion(dev))
+		return -ENODEV;
+
+	handle = ACPI_HANDLE(dev);
+	if (!handle)
+		return -EINVAL;
+
+	/* Walk through the tree for all PMU UNCORE devices */
+	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
+				     tx2_uncore_pmu_add,
+				     NULL, dev, NULL);
+	if (ACPI_FAILURE(status)) {
+		dev_err(dev, "failed to probe PMU devices\n");
+		return_ACPI_STATUS(status);
+	}
+
+	dev_info(dev, "node%d: pmu uncore registered\n", dev_to_node(dev));
+	return 0;
+}
+
+static int tx2_uncore_remove(struct platform_device *pdev)
+{
+	struct tx2_uncore_pmu *tx2_pmu, *temp;
+	struct device *dev = &pdev->dev;
+
+	if (!list_empty(&tx2_pmus)) {
+		list_for_each_entry_safe(tx2_pmu, temp, &tx2_pmus, entry) {
+			if (tx2_pmu->node == dev_to_node(dev)) {
+				cpuhp_state_remove_instance_nocalls(
+					CPUHP_AP_PERF_ARM_TX2_UNCORE_ONLINE,
+					&tx2_pmu->hpnode);
+				perf_pmu_unregister(&tx2_pmu->pmu);
+				list_del(&tx2_pmu->entry);
+			}
+		}
+	}
+	return 0;
+}
+
+static struct platform_driver tx2_uncore_driver = {
+	.driver = {
+		.name		= "tx2-uncore-pmu",
+		.acpi_match_table = ACPI_PTR(tx2_uncore_acpi_match),
+	},
+	.probe = tx2_uncore_probe,
+	.remove = tx2_uncore_remove,
+};
+
+static int __init tx2_uncore_driver_init(void)
+{
+	int ret;
+
+	ret = cpuhp_setup_state_multi(CPUHP_AP_PERF_ARM_TX2_UNCORE_ONLINE,
+				      "perf/tx2/uncore:online",
+				      tx2_uncore_pmu_online_cpu,
+				      tx2_uncore_pmu_offline_cpu);
+	if (ret) {
+		pr_err("TX2 PMU: setup hotplug failed(%d)\n", ret);
+		return ret;
+	}
+	ret = platform_driver_register(&tx2_uncore_driver);
+	if (ret)
+		cpuhp_remove_multi_state(CPUHP_AP_PERF_ARM_TX2_UNCORE_ONLINE);
+
+	return ret;
+}
+module_init(tx2_uncore_driver_init);
+
+static void __exit tx2_uncore_driver_exit(void)
+{
+	platform_driver_unregister(&tx2_uncore_driver);
+	cpuhp_remove_multi_state(CPUHP_AP_PERF_ARM_TX2_UNCORE_ONLINE);
+}
+module_exit(tx2_uncore_driver_exit);
+
+MODULE_DESCRIPTION("ThunderX2 UNCORE PMU driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Ganapatrao Kulkarni <gkulkarni@cavium.com>");
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index e0cd2baa8380..d50b711614f2 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -164,6 +164,7 @@ enum cpuhp_state {
 	CPUHP_AP_PERF_ARM_L2X0_ONLINE,
 	CPUHP_AP_PERF_ARM_QCOM_L2_ONLINE,
 	CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,
+	CPUHP_AP_PERF_ARM_TX2_UNCORE_ONLINE,
 	CPUHP_AP_PERF_POWERPC_NEST_IMC_ONLINE,
 	CPUHP_AP_PERF_POWERPC_CORE_IMC_ONLINE,
 	CPUHP_AP_PERF_POWERPC_THREAD_IMC_ONLINE,
-- 
2.18.0

Re: [PATCH v8 1/2] perf, uncore: Adding documentation for ThunderX2 pmu uncore driver

[Will Deacon]

On Thu, Nov 22, 2018 at 03:04:31AM +0000, Kulkarni, Ganapatrao wrote:
> The SoC has PMU support in its L3 cache controller (L3C) and in the
> DDR4 Memory Controller (DMC).
> 
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
>  Documentation/perf/thunderx2-pmu.txt | 106 +++++++++++++++++++++++++++
>  1 file changed, 106 insertions(+)
>  create mode 100644 Documentation/perf/thunderx2-pmu.txt

Thanks for writing the documentation, although I think it needs a bit of
help before we can merge it.

> diff --git a/Documentation/perf/thunderx2-pmu.txt b/Documentation/perf/thunderx2-pmu.txt
> new file mode 100644
> index 000000000000..9f5dd7459e68
> --- /dev/null
> +++ b/Documentation/perf/thunderx2-pmu.txt
> @@ -0,0 +1,106 @@
> +
> +Cavium ThunderX2 SoC Performance Monitoring Unit (PMU UNCORE)
> +==========================================================================
> +
> +ThunderX2 SoC PMU consists of independent system wide per Socket PMUs such
> +as Level 3 Cache(L3C) and DDR4 Memory Controller(DMC).

Please add some punctuation here.

> +
> +DMC has 8 interleave channels and L3C has 16 interleave tiles. Events are

*The* DMC and *the* L3C


> +sampled for default channel(i.e channel 0) and prorated to total number of

I'm not sure I understand this; are you saying it's not possible to sample
channels other than channel 0?

> +channels/tiles.
> +
> +DMC and L3C, Each PMU supports up to 4 counters. Counters are independently

The start of this sentence makes no sense and you've got a capital "Each".

> +programmable and can be started and stopped individually. Each counter can
> +be set to sample specific perf events. Counters are 32 bit and do not support
> +overflow interrupt; they are sampled at every 2 seconds.

I think this is unfortunate wording, because actually you don't support what
perf calls "sampling" at all.

> +
> +PMU UNCORE (perf) driver:
> +
> +The thunderx2-pmu driver registers several perf PMUs for DMC and L3C devices.

I think the driver name uses an underscore instead of a hyphen.

> +Each of the PMUs provides description of its available events
> +and configuration options in sysfs.
> +	see /sys/devices/uncore_<l3c_S/dmc_S/>
> +
> +S is socket id.

*the* socket id

> +Each PMU can be used to sample up to 4 events simultaneously.
> +
> +The "format" directory describes format of the config (event ID).
> +The "events" directory provides configuration templates for all
> +supported event types that can be used with perf tool.

You can drop this bit, since it's not specific to your PMU and is actually
describing the perf ABI via sysfs. If we want to document that someplace, it
should be in a separate file.

> +
> +For example, "uncore_dmc_0/cnt_cycles/" is an
> +equivalent of "uncore_dmc_0/config=0x1/".

Why is this helpful?

> +
> +Each perf driver also provides a "cpumask" sysfs attribute, which contains a
> +single CPU ID of the processor which is likely to be used to handle all the
> +PMU events. It will be the first online CPU from the NUMA node of the PMU device.

Again, I don't think this really belongs in here.

> +
> +Example for perf tool use:
> +
> +perf stat -a -e uncore_dmc_0/cnt_cycles/ sleep 1
> +
> +perf stat -a -e \
> +uncore_dmc_0/cnt_cycles/,\
> +uncore_dmc_0/data_transfers/,\
> +uncore_dmc_0/read_txns/,\
> +uncore_dmc_0/write_txns/ sleep 1
> +
> +perf stat -a -e \
> +uncore_l3c_0/read_request/,\
> +uncore_l3c_0/read_hit/,\
> +uncore_l3c_0/inv_request/,\
> +uncore_l3c_0/inv_hit/ sleep 1
> +
> +The driver does not support sampling, therefore "perf record" will
> +not work. Per-task (without "-a") perf sessions are not supported.

What do you mean by "not supported"? If I invoke perf as:

# ./perf stat -e uncore_dmc_0/cnt_cycles/ -- ls

then I get results back.

> +
> +L3C events:
> +============
> +
> +read_request:
> +	Number of Read requests received by the L3 Cache.
> +	This include Read as well as Read Exclusives.
> +
> +read_hit:
> +	Number of Read requests received by the L3 cache that were hit
> +	in the L3 (Data provided form the L3)
> +
> +writeback_request:
> +	Number of Write Backs received by the L3 Cache. These are basically
> +	the L2 Evicts and writes from the PCIe Write Cache.
> +
> +inv_nwrite_request:
> +	This is the Number of Invalidate and Write received by the L3 Cache.
> +	Also Writes from IO that did not go through the PCIe Write Cache.
> +
> +inv_nwrite_hit
> +	This is the Number of Invalidate and Write received by the L3 Cache
> +	That were a hit in the L3 Cache.
> +
> +inv_request:
> +	Number of Invalidate request received by the L3 Cache.
> +
> +inv_hit:
> +	Number of Invalidate request received by the L3 Cache that were a
> +	hit in L3.
> +
> +evict_request:
> +	Number of Evicts that the L3 generated.

Wouldn't this be better off in the perf tools sources, as part of the JSON
events file for your PMU?

> +
> +NOTE:
> +1. Granularity of all these events counter value is cache line length(64 Bytes).
> +2. L3C cache Hit Ratio = (read_hit + inv_nwrite_hit + inv_hit) / (read_request + inv_nwrite_request + inv_request)
> +
> +DMC events:
> +============
> +cnt_cycles:
> +	Count cycles (Clocks at the DMC clock rate)
> +
> +write_txns:
> +	Number of 64 Bytes write transactions received by the DMC(s)
> +
> +read_txns:
> +	Number of 64 Bytes Read transactions received by the DMC(s)
> +
> +data_transfers:
> +	Number of 64 Bytes data transferred to or from DRAM.

Same here.

Will

Re: [PATCH v8 2/2] ThunderX2, perf : Add Cavium ThunderX2 SoC UNCORE PMU driver

[Will Deacon]

On Thu, Nov 22, 2018 at 03:04:35AM +0000, Kulkarni, Ganapatrao wrote:
> This patch adds a perf driver for the PMU UNCORE devices DDR4 Memory
> Controller(DMC) and Level 3 Cache(L3C). Each PMU supports up to 4
> counters. All counters lack overflow interrupt and are
> sampled periodically.
> 
> Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> ---
>  drivers/perf/Kconfig         |   9 +
>  drivers/perf/Makefile        |   1 +
>  drivers/perf/thunderx2_pmu.c | 869 +++++++++++++++++++++++++++++++++++
>  include/linux/cpuhotplug.h   |   1 +
>  4 files changed, 880 insertions(+)
>  create mode 100644 drivers/perf/thunderx2_pmu.c
> 
> diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
> index 08ebaf7cca8b..af9bc178495d 100644
> --- a/drivers/perf/Kconfig
> +++ b/drivers/perf/Kconfig
> @@ -87,6 +87,15 @@ config QCOM_L3_PMU
>  	   Adds the L3 cache PMU into the perf events subsystem for
>  	   monitoring L3 cache events.
>  
> +config THUNDERX2_PMU
> +	tristate "Cavium ThunderX2 SoC PMU UNCORE"
> +	depends on ARCH_THUNDER2 && ARM64 && ACPI && NUMA
> +	default m
> +	help
> +	   Provides support for ThunderX2 UNCORE events.
> +	   The SoC has PMU support in its L3 cache controller (L3C) and
> +	   in the DDR4 Memory Controller (DMC).
> +
>  config XGENE_PMU
>          depends on ARCH_XGENE
>          bool "APM X-Gene SoC PMU"
> diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
> index b3902bd37d53..909f27fd9db3 100644
> --- a/drivers/perf/Makefile
> +++ b/drivers/perf/Makefile
> @@ -7,5 +7,6 @@ obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
>  obj-$(CONFIG_HISI_PMU) += hisilicon/
>  obj-$(CONFIG_QCOM_L2_PMU)	+= qcom_l2_pmu.o
>  obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
> +obj-$(CONFIG_THUNDERX2_PMU) += thunderx2_pmu.o
>  obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
>  obj-$(CONFIG_ARM_SPE_PMU) += arm_spe_pmu.o
> diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
> new file mode 100644
> index 000000000000..e6509ba868ab
> --- /dev/null
> +++ b/drivers/perf/thunderx2_pmu.c
> @@ -0,0 +1,869 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * CAVIUM THUNDERX2 SoC PMU UNCORE
> + * Copyright (C) 2018 Cavium Inc.
> + * Author: Ganapatrao Kulkarni <gkulkarni@cavium.com>
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/cpuhotplug.h>
> +#include <linux/perf_event.h>
> +#include <linux/platform_device.h>
> +
> +/* Each ThunderX2(TX2) Socket has a L3C and DMC UNCORE PMU device.
> + * Each UNCORE PMU device consists of 4 independent programmable counters.
> + * Counters are 32 bit and do not support overflow interrupt,
> + * they need to be sampled before overflow(i.e, at every 2 seconds).
> + */
> +
> +#define TX2_PMU_MAX_COUNTERS		4
> +#define TX2_PMU_DMC_CHANNELS		8
> +#define TX2_PMU_L3_TILES		16
> +
> +#define TX2_PMU_HRTIMER_INTERVAL	(2 * NSEC_PER_SEC)
> +#define GET_EVENTID(ev)			((ev->hw.config) & 0x1ff)

I think this should be 0x1f.

> +#define GET_COUNTERID(ev)		((ev->hw.idx) & 0x3)
> + /* 1 byte per counter(4 counters).
> +  * Event id is encoded in bits [5:1] of a byte,
> +  */
> +#define DMC_EVENT_CFG(idx, val)		((val) << (((idx) * 8) + 1))
> +
> +#define L3C_COUNTER_CTL			0xA8
> +#define L3C_COUNTER_DATA		0xAC
> +#define DMC_COUNTER_CTL			0x234
> +#define DMC_COUNTER_DATA		0x240
> +
> +/* L3C event IDs */
> +#define L3_EVENT_READ_REQ		0xD
> +#define L3_EVENT_WRITEBACK_REQ		0xE
> +#define L3_EVENT_INV_N_WRITE_REQ	0xF
> +#define L3_EVENT_INV_REQ		0x10
> +#define L3_EVENT_EVICT_REQ		0x13
> +#define L3_EVENT_INV_N_WRITE_HIT	0x14
> +#define L3_EVENT_INV_HIT		0x15
> +#define L3_EVENT_READ_HIT		0x17
> +#define L3_EVENT_MAX			0x18
> +
> +/* DMC event IDs */
> +#define DMC_EVENT_COUNT_CYCLES		0x1
> +#define DMC_EVENT_WRITE_TXNS		0xB
> +#define DMC_EVENT_DATA_TRANSFERS	0xD
> +#define DMC_EVENT_READ_TXNS		0xF
> +#define DMC_EVENT_MAX			0x10
> +
> +enum tx2_uncore_type {
> +	PMU_TYPE_L3C,
> +	PMU_TYPE_DMC,
> +	PMU_TYPE_INVALID,
> +};
> +
> +/*
> + * pmu on each socket has 2 uncore devices(dmc and l3c),
> + * each device has 4 counters.
> + */
> +struct tx2_uncore_pmu {
> +	struct hlist_node hpnode;
> +	struct list_head  entry;
> +	struct pmu pmu;
> +	char *name;
> +	int node;
> +	int cpu;
> +	u32    max_counters;
> +	u32    prorate_factor;
> +	u32    max_events;
> +	u64 hrtimer_interval;
> +	void __iomem *base;
> +	DECLARE_BITMAP(active_counters, TX2_PMU_MAX_COUNTERS);
> +	struct perf_event *events[TX2_PMU_MAX_COUNTERS];
> +	struct device *dev;
> +	struct hrtimer hrtimer;
> +	const struct attribute_group **attr_groups;
> +	enum tx2_uncore_type type;
> +	void	(*init_cntr_base)(struct perf_event *event,
> +			struct tx2_uncore_pmu *tx2_pmu);
> +	void	(*stop_event)(struct perf_event *event);
> +	void	(*start_event)(struct perf_event *event, int flags);
> +};
> +
> +static LIST_HEAD(tx2_pmus);
> +
> +static inline struct tx2_uncore_pmu *pmu_to_tx2_pmu(struct pmu *pmu)
> +{
> +	return container_of(pmu, struct tx2_uncore_pmu, pmu);
> +}
> +
> +/*
> + * sysfs format attributes
> + */
> +static ssize_t tx2_pmu_format_show(struct device *dev,
> +				     struct device_attribute *attr, char *buf)
> +{
> +	struct dev_ext_attribute *eattr;
> +
> +	eattr = container_of(attr, struct dev_ext_attribute, attr);
> +	return sprintf(buf, "%s\n", (char *) eattr->var);
> +}
> +
> +#define FORMAT_ATTR(_name, _config) \
> +	(&((struct dev_ext_attribute[]) { \
> +	   { \
> +	   .attr = __ATTR(_name, 0444, tx2_pmu_format_show, NULL), \
> +	   .var = (void *) _config, \
> +	   } \
> +	})[0].attr.attr)
> +
> +static struct attribute *l3c_pmu_format_attrs[] = {
> +	FORMAT_ATTR(event,	"config:0-4"),
> +	NULL,
> +};
> +
> +static struct attribute *dmc_pmu_format_attrs[] = {
> +	FORMAT_ATTR(event,	"config:0-4"),
> +	NULL,
> +};

We have PMU_FORMAT_ATTR, PMU_EVENT_ATTR etc in the core code to help here.
Please try to use them.

> +static const struct attribute_group l3c_pmu_format_attr_group = {
> +	.name = "format",
> +	.attrs = l3c_pmu_format_attrs,
> +};
> +
> +static const struct attribute_group dmc_pmu_format_attr_group = {
> +	.name = "format",
> +	.attrs = dmc_pmu_format_attrs,
> +};
> +
> +/*
> + * sysfs event attributes
> + */
> +static ssize_t tx2_pmu_event_show(struct device *dev,
> +				    struct device_attribute *attr, char *buf)
> +{
> +	struct dev_ext_attribute *eattr;
> +
> +	eattr = container_of(attr, struct dev_ext_attribute, attr);
> +	return sprintf(buf, "config=0x%lx\n", (unsigned long) eattr->var);
> +}

Shouldn't this be "event=" instead of "config="?

Will

Re: [PATCH v8 1/2] perf, uncore: Adding documentation for ThunderX2 pmu uncore driver

[Ganapatrao Kulkarni]

Hi Will,

On Mon, Dec 3, 2018 at 5:39 PM Will Deacon <will.deacon@arm.com> wrote:
>
> On Thu, Nov 22, 2018 at 03:04:31AM +0000, Kulkarni, Ganapatrao wrote:
> > The SoC has PMU support in its L3 cache controller (L3C) and in the
> > DDR4 Memory Controller (DMC).
> >
> > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> > ---
> >  Documentation/perf/thunderx2-pmu.txt | 106 +++++++++++++++++++++++++++
> >  1 file changed, 106 insertions(+)
> >  create mode 100644 Documentation/perf/thunderx2-pmu.txt
>
> Thanks for writing the documentation, although I think it needs a bit of
> help before we can merge it.

sure will send next version ASAP.
>
> > diff --git a/Documentation/perf/thunderx2-pmu.txt b/Documentation/perf/thunderx2-pmu.txt
> > new file mode 100644
> > index 000000000000..9f5dd7459e68
> > --- /dev/null
> > +++ b/Documentation/perf/thunderx2-pmu.txt
> > @@ -0,0 +1,106 @@
> > +
> > +Cavium ThunderX2 SoC Performance Monitoring Unit (PMU UNCORE)
> > +==========================================================================
> > +
> > +ThunderX2 SoC PMU consists of independent system wide per Socket PMUs such
> > +as Level 3 Cache(L3C) and DDR4 Memory Controller(DMC).
>
> Please add some punctuation here.

Thanks will do.
>
> > +
> > +DMC has 8 interleave channels and L3C has 16 interleave tiles. Events are
>
> *The* DMC and *the* L3C

ok
>
>
> > +sampled for default channel(i.e channel 0) and prorated to total number of
>
> I'm not sure I understand this; are you saying it's not possible to sample
> channels other than channel 0?

yes, sampling channel zero, since channels are interleave, multiplying
by number of channels will give fair data.
Removed per channel sample since it was involved SMC calls.

>
> > +channels/tiles.
> > +
> > +DMC and L3C, Each PMU supports up to 4 counters. Counters are independently
>
> The start of this sentence makes no sense and you've got a capital "Each".
>
> > +programmable and can be started and stopped individually. Each counter can
> > +be set to sample specific perf events. Counters are 32 bit and do not support
> > +overflow interrupt; they are sampled at every 2 seconds.
>
> I think this is unfortunate wording, because actually you don't support what
> perf calls "sampling" at all.

ok, let me rephrase it.
>
> > +
> > +PMU UNCORE (perf) driver:
> > +
> > +The thunderx2-pmu driver registers several perf PMUs for DMC and L3C devices.
>
> I think the driver name uses an underscore instead of a hyphen.

thanks.
>
> > +Each of the PMUs provides description of its available events
> > +and configuration options in sysfs.
> > +     see /sys/devices/uncore_<l3c_S/dmc_S/>
> > +
> > +S is socket id.
>
> *the* socket id

ok
>
> > +Each PMU can be used to sample up to 4 events simultaneously.
> > +
> > +The "format" directory describes format of the config (event ID).
> > +The "events" directory provides configuration templates for all
> > +supported event types that can be used with perf tool.
>
> You can drop this bit, since it's not specific to your PMU and is actually
> describing the perf ABI via sysfs. If we want to document that someplace, it
> should be in a separate file.

ok , let me drop sysfs part.
>
> > +
> > +For example, "uncore_dmc_0/cnt_cycles/" is an
> > +equivalent of "uncore_dmc_0/config=0x1/".
>
> Why is this helpful?

ok let me drop second line.
>
> > +
> > +Each perf driver also provides a "cpumask" sysfs attribute, which contains a
> > +single CPU ID of the processor which is likely to be used to handle all the
> > +PMU events. It will be the first online CPU from the NUMA node of the PMU device.
>
> Again, I don't think this really belongs in here.

ok.
>
> > +
> > +Example for perf tool use:
> > +
> > +perf stat -a -e uncore_dmc_0/cnt_cycles/ sleep 1
> > +
> > +perf stat -a -e \
> > +uncore_dmc_0/cnt_cycles/,\
> > +uncore_dmc_0/data_transfers/,\
> > +uncore_dmc_0/read_txns/,\
> > +uncore_dmc_0/write_txns/ sleep 1
> > +
> > +perf stat -a -e \
> > +uncore_l3c_0/read_request/,\
> > +uncore_l3c_0/read_hit/,\
> > +uncore_l3c_0/inv_request/,\
> > +uncore_l3c_0/inv_hit/ sleep 1
> > +
> > +The driver does not support sampling, therefore "perf record" will
> > +not work. Per-task (without "-a") perf sessions are not supported.
>
> What do you mean by "not supported"? If I invoke perf as:

I mean --per-core option, needs rephrasing.

>
> # ./perf stat -e uncore_dmc_0/cnt_cycles/ -- ls
>
> then I get results back.
>
> > +
> > +L3C events:
> > +============
> > +
> > +read_request:
> > +     Number of Read requests received by the L3 Cache.
> > +     This include Read as well as Read Exclusives.
> > +
> > +read_hit:
> > +     Number of Read requests received by the L3 cache that were hit
> > +     in the L3 (Data provided form the L3)
> > +
> > +writeback_request:
> > +     Number of Write Backs received by the L3 Cache. These are basically
> > +     the L2 Evicts and writes from the PCIe Write Cache.
> > +
> > +inv_nwrite_request:
> > +     This is the Number of Invalidate and Write received by the L3 Cache.
> > +     Also Writes from IO that did not go through the PCIe Write Cache.
> > +
> > +inv_nwrite_hit
> > +     This is the Number of Invalidate and Write received by the L3 Cache
> > +     That were a hit in the L3 Cache.
> > +
> > +inv_request:
> > +     Number of Invalidate request received by the L3 Cache.
> > +
> > +inv_hit:
> > +     Number of Invalidate request received by the L3 Cache that were a
> > +     hit in L3.
> > +
> > +evict_request:
> > +     Number of Evicts that the L3 generated.
>
> Wouldn't this be better off in the perf tools sources, as part of the JSON
> events file for your PMU?

That could be another effort to move all arm64 vendors uncore events
to JSON framework.
>
> > +
> > +NOTE:
> > +1. Granularity of all these events counter value is cache line length(64 Bytes).
> > +2. L3C cache Hit Ratio = (read_hit + inv_nwrite_hit + inv_hit) / (read_request + inv_nwrite_request + inv_request)
> > +
> > +DMC events:
> > +============
> > +cnt_cycles:
> > +     Count cycles (Clocks at the DMC clock rate)
> > +
> > +write_txns:
> > +     Number of 64 Bytes write transactions received by the DMC(s)
> > +
> > +read_txns:
> > +     Number of 64 Bytes Read transactions received by the DMC(s)
> > +
> > +data_transfers:
> > +     Number of 64 Bytes data transferred to or from DRAM.
>
> Same here.
>
> Will

thanks
Ganapat

Re: [PATCH v8 2/2] ThunderX2, perf : Add Cavium ThunderX2 SoC UNCORE PMU driver

[Ganapatrao Kulkarni]

On Mon, Dec 3, 2018 at 5:41 PM Will Deacon <will.deacon@arm.com> wrote:
>
> On Thu, Nov 22, 2018 at 03:04:35AM +0000, Kulkarni, Ganapatrao wrote:
> > This patch adds a perf driver for the PMU UNCORE devices DDR4 Memory
> > Controller(DMC) and Level 3 Cache(L3C). Each PMU supports up to 4
> > counters. All counters lack overflow interrupt and are
> > sampled periodically.
> >
> > Signed-off-by: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
> > ---
> >  drivers/perf/Kconfig         |   9 +
> >  drivers/perf/Makefile        |   1 +
> >  drivers/perf/thunderx2_pmu.c | 869 +++++++++++++++++++++++++++++++++++
> >  include/linux/cpuhotplug.h   |   1 +
> >  4 files changed, 880 insertions(+)
> >  create mode 100644 drivers/perf/thunderx2_pmu.c
> >
> > diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
> > index 08ebaf7cca8b..af9bc178495d 100644
> > --- a/drivers/perf/Kconfig
> > +++ b/drivers/perf/Kconfig
> > @@ -87,6 +87,15 @@ config QCOM_L3_PMU
> >          Adds the L3 cache PMU into the perf events subsystem for
> >          monitoring L3 cache events.
> >
> > +config THUNDERX2_PMU
> > +     tristate "Cavium ThunderX2 SoC PMU UNCORE"
> > +     depends on ARCH_THUNDER2 && ARM64 && ACPI && NUMA
> > +     default m
> > +     help
> > +        Provides support for ThunderX2 UNCORE events.
> > +        The SoC has PMU support in its L3 cache controller (L3C) and
> > +        in the DDR4 Memory Controller (DMC).
> > +
> >  config XGENE_PMU
> >          depends on ARCH_XGENE
> >          bool "APM X-Gene SoC PMU"
> > diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
> > index b3902bd37d53..909f27fd9db3 100644
> > --- a/drivers/perf/Makefile
> > +++ b/drivers/perf/Makefile
> > @@ -7,5 +7,6 @@ obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
> >  obj-$(CONFIG_HISI_PMU) += hisilicon/
> >  obj-$(CONFIG_QCOM_L2_PMU)    += qcom_l2_pmu.o
> >  obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
> > +obj-$(CONFIG_THUNDERX2_PMU) += thunderx2_pmu.o
> >  obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
> >  obj-$(CONFIG_ARM_SPE_PMU) += arm_spe_pmu.o
> > diff --git a/drivers/perf/thunderx2_pmu.c b/drivers/perf/thunderx2_pmu.c
> > new file mode 100644
> > index 000000000000..e6509ba868ab
> > --- /dev/null
> > +++ b/drivers/perf/thunderx2_pmu.c
> > @@ -0,0 +1,869 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * CAVIUM THUNDERX2 SoC PMU UNCORE
> > + * Copyright (C) 2018 Cavium Inc.
> > + * Author: Ganapatrao Kulkarni <gkulkarni@cavium.com>
> > + */
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/cpuhotplug.h>
> > +#include <linux/perf_event.h>
> > +#include <linux/platform_device.h>
> > +
> > +/* Each ThunderX2(TX2) Socket has a L3C and DMC UNCORE PMU device.
> > + * Each UNCORE PMU device consists of 4 independent programmable counters.
> > + * Counters are 32 bit and do not support overflow interrupt,
> > + * they need to be sampled before overflow(i.e, at every 2 seconds).
> > + */
> > +
> > +#define TX2_PMU_MAX_COUNTERS         4
> > +#define TX2_PMU_DMC_CHANNELS         8
> > +#define TX2_PMU_L3_TILES             16
> > +
> > +#define TX2_PMU_HRTIMER_INTERVAL     (2 * NSEC_PER_SEC)
> > +#define GET_EVENTID(ev)                      ((ev->hw.config) & 0x1ff)
>
> I think this should be 0x1f.

yes it should be, i will update it.
>
> > +#define GET_COUNTERID(ev)            ((ev->hw.idx) & 0x3)
> > + /* 1 byte per counter(4 counters).
> > +  * Event id is encoded in bits [5:1] of a byte,
> > +  */
> > +#define DMC_EVENT_CFG(idx, val)              ((val) << (((idx) * 8) + 1))
> > +
> > +#define L3C_COUNTER_CTL                      0xA8
> > +#define L3C_COUNTER_DATA             0xAC
> > +#define DMC_COUNTER_CTL                      0x234
> > +#define DMC_COUNTER_DATA             0x240
> > +
> > +/* L3C event IDs */
> > +#define L3_EVENT_READ_REQ            0xD
> > +#define L3_EVENT_WRITEBACK_REQ               0xE
> > +#define L3_EVENT_INV_N_WRITE_REQ     0xF
> > +#define L3_EVENT_INV_REQ             0x10
> > +#define L3_EVENT_EVICT_REQ           0x13
> > +#define L3_EVENT_INV_N_WRITE_HIT     0x14
> > +#define L3_EVENT_INV_HIT             0x15
> > +#define L3_EVENT_READ_HIT            0x17
> > +#define L3_EVENT_MAX                 0x18
> > +
> > +/* DMC event IDs */
> > +#define DMC_EVENT_COUNT_CYCLES               0x1
> > +#define DMC_EVENT_WRITE_TXNS         0xB
> > +#define DMC_EVENT_DATA_TRANSFERS     0xD
> > +#define DMC_EVENT_READ_TXNS          0xF
> > +#define DMC_EVENT_MAX                        0x10
> > +
> > +enum tx2_uncore_type {
> > +     PMU_TYPE_L3C,
> > +     PMU_TYPE_DMC,
> > +     PMU_TYPE_INVALID,
> > +};
> > +
> > +/*
> > + * pmu on each socket has 2 uncore devices(dmc and l3c),
> > + * each device has 4 counters.
> > + */
> > +struct tx2_uncore_pmu {
> > +     struct hlist_node hpnode;
> > +     struct list_head  entry;
> > +     struct pmu pmu;
> > +     char *name;
> > +     int node;
> > +     int cpu;
> > +     u32    max_counters;
> > +     u32    prorate_factor;
> > +     u32    max_events;
> > +     u64 hrtimer_interval;
> > +     void __iomem *base;
> > +     DECLARE_BITMAP(active_counters, TX2_PMU_MAX_COUNTERS);
> > +     struct perf_event *events[TX2_PMU_MAX_COUNTERS];
> > +     struct device *dev;
> > +     struct hrtimer hrtimer;
> > +     const struct attribute_group **attr_groups;
> > +     enum tx2_uncore_type type;
> > +     void    (*init_cntr_base)(struct perf_event *event,
> > +                     struct tx2_uncore_pmu *tx2_pmu);
> > +     void    (*stop_event)(struct perf_event *event);
> > +     void    (*start_event)(struct perf_event *event, int flags);
> > +};
> > +
> > +static LIST_HEAD(tx2_pmus);
> > +
> > +static inline struct tx2_uncore_pmu *pmu_to_tx2_pmu(struct pmu *pmu)
> > +{
> > +     return container_of(pmu, struct tx2_uncore_pmu, pmu);
> > +}
> > +
> > +/*
> > + * sysfs format attributes
> > + */
> > +static ssize_t tx2_pmu_format_show(struct device *dev,
> > +                                  struct device_attribute *attr, char *buf)
> > +{
> > +     struct dev_ext_attribute *eattr;
> > +
> > +     eattr = container_of(attr, struct dev_ext_attribute, attr);
> > +     return sprintf(buf, "%s\n", (char *) eattr->var);
> > +}
> > +
> > +#define FORMAT_ATTR(_name, _config) \
> > +     (&((struct dev_ext_attribute[]) { \
> > +        { \
> > +        .attr = __ATTR(_name, 0444, tx2_pmu_format_show, NULL), \
> > +        .var = (void *) _config, \
> > +        } \
> > +     })[0].attr.attr)
> > +
> > +static struct attribute *l3c_pmu_format_attrs[] = {
> > +     FORMAT_ATTR(event,      "config:0-4"),
> > +     NULL,
> > +};
> > +
> > +static struct attribute *dmc_pmu_format_attrs[] = {
> > +     FORMAT_ATTR(event,      "config:0-4"),
> > +     NULL,
> > +};
>
> We have PMU_FORMAT_ATTR, PMU_EVENT_ATTR etc in the core code to help here.
> Please try to use them.

ok, i will try to use it.
>
> > +static const struct attribute_group l3c_pmu_format_attr_group = {
> > +     .name = "format",
> > +     .attrs = l3c_pmu_format_attrs,
> > +};
> > +
> > +static const struct attribute_group dmc_pmu_format_attr_group = {
> > +     .name = "format",
> > +     .attrs = dmc_pmu_format_attrs,
> > +};
> > +
> > +/*
> > + * sysfs event attributes
> > + */
> > +static ssize_t tx2_pmu_event_show(struct device *dev,
> > +                                 struct device_attribute *attr, char *buf)
> > +{
> > +     struct dev_ext_attribute *eattr;
> > +
> > +     eattr = container_of(attr, struct dev_ext_attribute, attr);
> > +     return sprintf(buf, "config=0x%lx\n", (unsigned long) eattr->var);
> > +}
>
> Shouldn't this be "event=" instead of "config="?

yep, thanks
>
> Will

Thanks,
Ganapat