* [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support
@ 2023-03-30 16:45 Jonathan Cameron
2023-03-30 16:45 ` [PATCH v4 1/5] cxl: Add function to count regblocks of a given type Jonathan Cameron
` (5 more replies)
0 siblings, 6 replies; 21+ messages in thread
From: Jonathan Cameron @ 2023-03-30 16:45 UTC (permalink / raw)
To: Liang Kan, linux-cxl, peterz
Cc: mingo, acme, mark.rutland, will, dan.j.williams, linuxarm,
linux-perf-users, linux-kernel, Davidlohr Bueso, Dave Jiang
Changes since v3: Kan Liang gave feedback on v2 incorporated here.
- All updates in patch 4, see details there.
Updated cover letter.
The CXL rev 3.0 specification introduces a CXL Performance Monitoring
Unit definition. CXL components may have any number of these blocks. The
definition is highly flexible, but that does bring complexity in the
driver.
Notes.
1) The QEMU model against which this was developed needs tidying up.
Latest tree at https://gitlab.com/jic23/qemu branch cxl-2023-02-28
It's backed up behind other series that I plan to upstream first.
2) There are quite a lot of corner cases that will need working through
with variants of the model, or I'll have to design a pathological
set of CPMUs to hit all the corner cases in one go. So whilst I believe
the driver should be fine for what it supports we may find issues
as those corners of what is allowed are explored.
3) I'm not sure it makes sense to hang this of the cxl/pci driver but
couldn't really figure out where else in the current structure we could
make it fit cleanly.
4) Driver location. In past perf maintainers have requested perf drivers
for PCI etc be under drivers/perf. That would require moving some
CXL headers to be more generally visible, but is certainly possible
if there is agreement between CXL and perf maintainers on the correct
location.
5) Patch 1 led to a discussion of how to handle the self describing
and extensible nature of the CPMU counters. That is likely to involve
a description in the "caps" sysfs directory and perf tool code that is
aware of the different event combinations that make sense and can
establish which sets are available on a given device.
That is likely to take a while to converge on, so as the driver is useful
in the current state, I'm looking to upstream this first and deal with
the more complex handling later.
6) There is a lot of other functionality that can be added in future
include allowing for simpler hardware implementations that may not
support the minimum level of features currently required by the driver
(freeze, overflow interrupts etc).
CXL rev 3.0 specification available from https://www.computeexpresslink.org
Jonathan Cameron (5):
cxl: Add function to count regblocks of a given type
perf: Allow a PMU to have a parent
cxl/pci: Find and register CXL PMU devices
cxl: CXL Performance Monitoring Unit driver
docs: perf: Minimal introduction the the CXL PMU device and driver
Documentation/admin-guide/perf/cxl.rst | 65 ++
Documentation/admin-guide/perf/index.rst | 1 +
drivers/cxl/Kconfig | 13 +
drivers/cxl/Makefile | 1 +
drivers/cxl/core/Makefile | 1 +
drivers/cxl/core/core.h | 1 +
drivers/cxl/core/cpmu.c | 72 ++
drivers/cxl/core/pci.c | 2 +-
drivers/cxl/core/port.c | 4 +-
drivers/cxl/core/regs.c | 50 +-
drivers/cxl/cpmu.c | 940 +++++++++++++++++++++++
drivers/cxl/cpmu.h | 56 ++
drivers/cxl/cxl.h | 17 +-
drivers/cxl/cxlpci.h | 1 +
drivers/cxl/pci.c | 27 +-
include/linux/perf_event.h | 1 +
kernel/events/core.c | 1 +
17 files changed, 1245 insertions(+), 8 deletions(-)
create mode 100644 Documentation/admin-guide/perf/cxl.rst
create mode 100644 drivers/cxl/core/cpmu.c
create mode 100644 drivers/cxl/cpmu.c
create mode 100644 drivers/cxl/cpmu.h
--
2.37.2
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 1/5] cxl: Add function to count regblocks of a given type
2023-03-30 16:45 [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support Jonathan Cameron
@ 2023-03-30 16:45 ` Jonathan Cameron
2023-04-04 3:59 ` Dan Williams
2023-03-30 16:45 ` [PATCH v4 2/5] perf: Allow a PMU to have a parent Jonathan Cameron
` (4 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Jonathan Cameron @ 2023-03-30 16:45 UTC (permalink / raw)
To: Liang Kan, linux-cxl, peterz
Cc: mingo, acme, mark.rutland, will, dan.j.williams, linuxarm,
linux-perf-users, linux-kernel, Davidlohr Bueso, Dave Jiang
Until the recently release CXL 3.0 specification, there
was only ever one instance of any given register block pointed
to by the Register Block Locator DVSEC. Now, the specification allows
for multiple CXL PMU instances, each with their own register block.
To enable this add an index parameter to cxl_find_regblock()
and use that to implement cxl_count_regblock().
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v4: No change
---
drivers/cxl/core/pci.c | 2 +-
drivers/cxl/core/port.c | 2 +-
drivers/cxl/core/regs.c | 34 +++++++++++++++++++++++++++++++---
drivers/cxl/cxl.h | 3 ++-
drivers/cxl/pci.c | 2 +-
5 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 7328a2552411..c90251f60771 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -50,7 +50,7 @@ static int match_add_dports(struct pci_dev *pdev, void *data)
&lnkcap))
return 0;
- rc = cxl_find_regblock(pdev, CXL_REGLOC_RBI_COMPONENT, &map);
+ rc = cxl_find_regblock(pdev, CXL_REGLOC_RBI_COMPONENT, &map, 0);
if (rc)
dev_dbg(&port->dev, "failed to find component registers\n");
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 8ee6b6e2e2a4..97cc03dbceee 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1333,7 +1333,7 @@ static resource_size_t find_component_registers(struct device *dev)
pdev = to_pci_dev(dev);
- cxl_find_regblock(pdev, CXL_REGLOC_RBI_COMPONENT, &map);
+ cxl_find_regblock(pdev, CXL_REGLOC_RBI_COMPONENT, &map, 0);
return map.resource;
}
diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
index 1476a0299c9b..7389dd1af967 100644
--- a/drivers/cxl/core/regs.c
+++ b/drivers/cxl/core/regs.c
@@ -290,6 +290,7 @@ static bool cxl_decode_regblock(struct pci_dev *pdev, u32 reg_lo, u32 reg_hi,
* @pdev: The CXL PCI device to enumerate.
* @type: Register Block Indicator id
* @map: Enumeration output, clobbered on error
+ * @index: Index into which particular instance of a regblock we want.
*
* Return: 0 if register block enumerated, negative error code otherwise
*
@@ -297,9 +298,10 @@ static bool cxl_decode_regblock(struct pci_dev *pdev, u32 reg_lo, u32 reg_hi,
* by @type.
*/
int cxl_find_regblock(struct pci_dev *pdev, enum cxl_regloc_type type,
- struct cxl_register_map *map)
+ struct cxl_register_map *map, int index)
{
u32 regloc_size, regblocks;
+ int instance = 0;
int regloc, i;
map->resource = CXL_RESOURCE_NONE;
@@ -323,8 +325,11 @@ int cxl_find_regblock(struct pci_dev *pdev, enum cxl_regloc_type type,
if (!cxl_decode_regblock(pdev, reg_lo, reg_hi, map))
continue;
- if (map->reg_type == type)
- return 0;
+ if (map->reg_type == type) {
+ if (index == instance)
+ return 0;
+ instance++;
+ }
}
map->resource = CXL_RESOURCE_NONE;
@@ -332,6 +337,29 @@ int cxl_find_regblock(struct pci_dev *pdev, enum cxl_regloc_type type,
}
EXPORT_SYMBOL_NS_GPL(cxl_find_regblock, CXL);
+/**
+ * cxl_count_regblock() - Count instances of a given regblock type.
+ * @pdev: The CXL PCI device to enumerate.
+ * @type: Register Block Indicator id
+ *
+ * Some regblocks may be repeated. Count how many instances.
+ *
+ * Return: count of matching regblocks.
+ */
+int cxl_count_regblock(struct pci_dev *pdev, enum cxl_regloc_type type)
+{
+ struct cxl_register_map map;
+ int rc, count = 0;
+
+ while (1) {
+ rc = cxl_find_regblock(pdev, type, &map, count);
+ if (rc)
+ return count;
+ count++;
+ }
+}
+EXPORT_SYMBOL_NS_GPL(cxl_count_regblock, CXL);
+
resource_size_t cxl_rcrb_to_component(struct device *dev,
resource_size_t rcrb,
enum cxl_rcrb which)
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index aab87d74474d..0cc33e2a99d7 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -260,8 +260,9 @@ int cxl_map_device_regs(struct device *dev, struct cxl_device_regs *regs,
struct cxl_register_map *map);
enum cxl_regloc_type;
+int cxl_count_regblock(struct pci_dev *pdev, enum cxl_regloc_type type);
int cxl_find_regblock(struct pci_dev *pdev, enum cxl_regloc_type type,
- struct cxl_register_map *map);
+ struct cxl_register_map *map, int index);
enum cxl_rcrb {
CXL_RCRB_DOWNSTREAM,
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 60b23624d167..74443a5c3cc8 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -343,7 +343,7 @@ static int cxl_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
{
int rc;
- rc = cxl_find_regblock(pdev, type, map);
+ rc = cxl_find_regblock(pdev, type, map, 0);
if (rc)
return rc;
--
2.37.2
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v4 2/5] perf: Allow a PMU to have a parent
2023-03-30 16:45 [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support Jonathan Cameron
2023-03-30 16:45 ` [PATCH v4 1/5] cxl: Add function to count regblocks of a given type Jonathan Cameron
@ 2023-03-30 16:45 ` Jonathan Cameron
2023-04-04 4:03 ` Dan Williams
2023-03-30 16:45 ` [PATCH v4 3/5] cxl/pci: Find and register CXL PMU devices Jonathan Cameron
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Jonathan Cameron @ 2023-03-30 16:45 UTC (permalink / raw)
To: Liang Kan, linux-cxl, peterz
Cc: mingo, acme, mark.rutland, will, dan.j.williams, linuxarm,
linux-perf-users, linux-kernel, Davidlohr Bueso, Dave Jiang
Some PMUs have well defined parents such as PCI devices.
As the device_initialize() and device_add() are all within
pmu_dev_alloc() which is called from perf_pmu_register()
there is no opportunity to set the parent from within a driver.
Add a struct device *parent field to struct pmu and use that
to set the parent.
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v4: No change
Note that this may first merge as part of a larger series I
plan to post next week that adds parents for many of the of the
other struct pmu instances. If so please drop this patch whilst
applying.
---
include/linux/perf_event.h | 1 +
kernel/events/core.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index d5628a7b5eaa..b99db1eda72c 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -303,6 +303,7 @@ struct pmu {
struct module *module;
struct device *dev;
+ struct device *parent;
const struct attribute_group **attr_groups;
const struct attribute_group **attr_update;
const char *name;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index fb3e436bcd4a..a84c282221f2 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -11367,6 +11367,7 @@ static int pmu_dev_alloc(struct pmu *pmu)
dev_set_drvdata(pmu->dev, pmu);
pmu->dev->bus = &pmu_bus;
+ pmu->dev->parent = pmu->parent;
pmu->dev->release = pmu_dev_release;
ret = dev_set_name(pmu->dev, "%s", pmu->name);
--
2.37.2
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v4 3/5] cxl/pci: Find and register CXL PMU devices
2023-03-30 16:45 [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support Jonathan Cameron
2023-03-30 16:45 ` [PATCH v4 1/5] cxl: Add function to count regblocks of a given type Jonathan Cameron
2023-03-30 16:45 ` [PATCH v4 2/5] perf: Allow a PMU to have a parent Jonathan Cameron
@ 2023-03-30 16:45 ` Jonathan Cameron
2023-04-04 19:17 ` Dan Williams
2023-03-30 16:45 ` [PATCH v4 4/5] cxl: CXL Performance Monitoring Unit driver Jonathan Cameron
` (2 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Jonathan Cameron @ 2023-03-30 16:45 UTC (permalink / raw)
To: Liang Kan, linux-cxl, peterz
Cc: mingo, acme, mark.rutland, will, dan.j.williams, linuxarm,
linux-perf-users, linux-kernel, Davidlohr Bueso, Dave Jiang
CXL PMU devices can be found from entries in the Register
Locator DVSEC.
In order to register the minimum number of IRQ vectors necessary
to support all CPMUs found, separate the registration into two
steps. First find the devices, and query the IRQs used and then
register the devices. Between these two steps, request the
IRQ vectors necessary and enable bus master support.
Future IRQ users for CXL type 3 devices (e.g. DOEs) will need to
follow a similar pattern the number of vectors necessary is known
before any parts of the driver stack rely on their availability.
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v4:
- No change.
---
drivers/cxl/core/Makefile | 1 +
drivers/cxl/core/core.h | 1 +
drivers/cxl/core/cpmu.c | 72 +++++++++++++++++++++++++++++++++++++++
drivers/cxl/core/port.c | 2 ++
drivers/cxl/core/regs.c | 16 +++++++++
drivers/cxl/cpmu.h | 56 ++++++++++++++++++++++++++++++
drivers/cxl/cxl.h | 14 ++++++++
drivers/cxl/cxlpci.h | 1 +
drivers/cxl/pci.c | 25 +++++++++++++-
9 files changed, 187 insertions(+), 1 deletion(-)
diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
index ca4ae31d8f57..45e5543aff52 100644
--- a/drivers/cxl/core/Makefile
+++ b/drivers/cxl/core/Makefile
@@ -12,5 +12,6 @@ cxl_core-y += memdev.o
cxl_core-y += mbox.o
cxl_core-y += pci.o
cxl_core-y += hdm.o
+cxl_core-y += cpmu.o
cxl_core-$(CONFIG_TRACING) += trace.o
cxl_core-$(CONFIG_CXL_REGION) += region.o
diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index cde475e13216..63a5b709fe8d 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -6,6 +6,7 @@
extern const struct device_type cxl_nvdimm_bridge_type;
extern const struct device_type cxl_nvdimm_type;
+extern const struct device_type cxl_cpmu_type;
extern struct attribute_group cxl_base_attribute_group;
diff --git a/drivers/cxl/core/cpmu.c b/drivers/cxl/core/cpmu.c
new file mode 100644
index 000000000000..8aff35055c05
--- /dev/null
+++ b/drivers/cxl/core/cpmu.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2022 Huawei. All rights reserved. */
+
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/idr.h>
+#include <cxlmem.h>
+#include <cpmu.h>
+#include <cxl.h>
+#include "core.h"
+
+static DEFINE_IDA(cpmu_ida);
+
+static void cxl_cpmu_release(struct device *dev)
+{
+ struct cxl_cpmu *cpmu = container_of(dev, struct cxl_cpmu, dev);
+
+ ida_free(&cpmu_ida, cpmu->id);
+ kfree(cpmu);
+}
+
+const struct device_type cxl_cpmu_type = {
+ .name = "cxl_cpmu",
+ .release = cxl_cpmu_release,
+};
+
+static void remove_dev(void *dev)
+{
+ device_del(dev);
+}
+
+int devm_cxl_cpmu_add(struct device *parent, struct cxl_cpmu_regs *regs, int index)
+{
+ struct cxl_cpmu *cpmu;
+ struct device *dev;
+ int rc;
+
+ cpmu = kzalloc(sizeof(*cpmu), GFP_KERNEL);
+ if (!cpmu)
+ return -ENOMEM;
+
+ rc = ida_alloc(&cpmu_ida, GFP_KERNEL);
+ if (rc < 0) {
+ kfree(cpmu);
+ return rc;
+ }
+ cpmu->id = rc;
+
+ cpmu->base = regs->cpmu;
+ dev = &cpmu->dev;
+ device_initialize(dev);
+ device_set_pm_not_required(dev);
+ dev->parent = parent;
+ dev->bus = &cxl_bus_type;
+ dev->type = &cxl_cpmu_type;
+
+ rc = dev_set_name(dev, "cpmu%d", cpmu->id);
+ if (rc)
+ goto err;
+
+ rc = device_add(dev);
+ if (rc)
+ goto err;
+
+ return devm_add_action_or_reset(parent, remove_dev, dev);
+
+err:
+ put_device(&cpmu->dev);
+ return rc;
+}
+EXPORT_SYMBOL_NS_GPL(devm_cxl_cpmu_add, CXL);
+
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 97cc03dbceee..2154bf8d2aad 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -57,6 +57,8 @@ static int cxl_device_id(const struct device *dev)
return CXL_DEVICE_MEMORY_EXPANDER;
if (dev->type == CXL_REGION_TYPE())
return CXL_DEVICE_REGION;
+ if (dev->type == &cxl_cpmu_type)
+ return CXL_DEVICE_CPMU;
return 0;
}
diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
index 7389dd1af967..99d6ebe3aba9 100644
--- a/drivers/cxl/core/regs.c
+++ b/drivers/cxl/core/regs.c
@@ -6,6 +6,7 @@
#include <linux/pci.h>
#include <cxlmem.h>
#include <cxlpci.h>
+#include <cpmu.h>
#include "core.h"
@@ -360,6 +361,21 @@ int cxl_count_regblock(struct pci_dev *pdev, enum cxl_regloc_type type)
}
EXPORT_SYMBOL_NS_GPL(cxl_count_regblock, CXL);
+int cxl_map_cpmu_regs(struct pci_dev *pdev, struct cxl_cpmu_regs *regs,
+ struct cxl_register_map *map)
+{
+ struct device *dev = &pdev->dev;
+ resource_size_t phys_addr;
+
+ phys_addr = map->resource;
+ regs->cpmu = devm_cxl_iomap_block(dev, phys_addr, CPMU_REGMAP_SIZE);
+ if (!regs->cpmu)
+ return -ENOMEM;
+
+ return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_map_cpmu_regs, CXL);
+
resource_size_t cxl_rcrb_to_component(struct device *dev,
resource_size_t rcrb,
enum cxl_rcrb which)
diff --git a/drivers/cxl/cpmu.h b/drivers/cxl/cpmu.h
new file mode 100644
index 000000000000..2cf78a6b9b13
--- /dev/null
+++ b/drivers/cxl/cpmu.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright(c) 2022 Huawei
+ * CXL Specification rev 3.0 Setion 8.2.7 (CPMU Register Interface)
+ */
+#ifndef CXL_CPMU_H
+#define CXL_CPMU_H
+
+#define CPMU_CAP_REG 0x0
+#define CPMU_CAP_NUM_COUNTERS_MSK GENMASK_ULL(4, 0)
+#define CPMU_CAP_COUNTER_WIDTH_MSK GENMASK_ULL(15, 8)
+#define CPMU_CAP_NUM_EVN_CAP_REG_SUP_MSK GENMASK_ULL(24, 20)
+#define CPMU_CAP_FILTERS_SUP_MSK GENMASK_ULL(39, 32)
+#define CPMU_FILTER_HDM BIT(0)
+#define CPMU_FILTER_CHAN_RANK_BANK BIT(1)
+#define CPMU_CAP_MSI_N_MSK GENMASK_ULL(47, 44)
+#define CPMU_CAP_WRITEABLE_WHEN_FROZEN BIT_ULL(48)
+#define CPMU_CAP_FREEZE BIT_ULL(49)
+#define CPMU_CAP_INT BIT_ULL(50)
+#define CPMU_CAP_VERSION_MSK GENMASK_ULL(63, 60)
+
+#define CPMU_OVERFLOW_REG 0x10
+#define CPMU_FREEZE_REG 0x18
+#define CPMU_EVENT_CAP_REG(n) (0x100 + 8 * (n))
+#define CPMU_EVENT_CAP_SUPPORTED_EVENTS_MSK GENMASK_ULL(31, 0)
+#define CPMU_EVENT_CAP_GROUP_ID_MSK GENMASK_ULL(47, 32)
+#define CPMU_EVENT_CAP_VENDOR_ID_MSK GENMASK_ULL(63, 48)
+
+#define CPMU_COUNTER_CFG_REG(n) (0x200 + 8 * (n))
+#define CPMU_COUNTER_CFG_TYPE_MSK GENMASK_ULL(1, 0)
+#define CPMU_COUNTER_CFG_TYPE_FREE_RUN 0
+#define CPMU_COUNTER_CFG_TYPE_FIXED_FUN 1
+#define CPMU_COUNTER_CFG_TYPE_CONFIGURABLE 2
+#define CPMU_COUNTER_CFG_ENABLE BIT_ULL(8)
+#define CPMU_COUNTER_CFG_INT_ON_OVRFLW BIT_ULL(9)
+#define CPMU_COUNTER_CFG_FREEZE_ON_OVRFLW BIT_ULL(10)
+#define CPMU_COUNTER_CFG_EDGE BIT_ULL(11)
+#define CPMU_COUNTER_CFG_INVERT BIT_ULL(12)
+#define CPMU_COUNTER_CFG_THRESHOLD_MSK GENMASK_ULL(23, 16)
+#define CPMU_COUNTER_CFG_EVENTS_MSK GENMASK_ULL(55, 24)
+#define CPMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK GENMASK_ULL(63, 59)
+
+#define CPMU_FILTER_CFG_REG(n, f) (0x400 + 4 * ((f) + (n) * 8))
+#define CPMU_FILTER_CFG_VALUE_MSK GENMASK(15, 0)
+
+#define CPMU_COUNTER_REG(n) (0xc00 + 8 * (n))
+
+#define CPMU_REGMAP_SIZE 0xe00 /* Table 8-32 CXL 3.0 specification */
+struct cxl_cpmu {
+ struct device dev;
+ void __iomem *base;
+ int id;
+};
+
+#define to_cxl_cpmu(dev) container_of(dev, struct cxl_cpmu, dev)
+#endif
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 0cc33e2a99d7..a7b8a0c46715 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -209,6 +209,10 @@ struct cxl_regs {
struct_group_tagged(cxl_device_regs, device_regs,
void __iomem *status, *mbox, *memdev;
);
+
+ struct_group_tagged(cxl_cpmu_regs, cpmu_regs,
+ void __iomem *cpmu;
+ );
};
struct cxl_reg_map {
@@ -229,6 +233,10 @@ struct cxl_device_reg_map {
struct cxl_reg_map memdev;
};
+struct cxl_cpmu_reg_map {
+ struct cxl_reg_map cpmu;
+};
+
/**
* struct cxl_register_map - DVSEC harvested register block mapping parameters
* @base: virtual base of the register-block-BAR + @block_offset
@@ -237,6 +245,7 @@ struct cxl_device_reg_map {
* @reg_type: see enum cxl_regloc_type
* @component_map: cxl_reg_map for component registers
* @device_map: cxl_reg_maps for device registers
+ * @cpmu_map: cxl_reg_maps for CXL Performance Monitoring Units
*/
struct cxl_register_map {
void __iomem *base;
@@ -246,6 +255,7 @@ struct cxl_register_map {
union {
struct cxl_component_reg_map component_map;
struct cxl_device_reg_map device_map;
+ struct cxl_cpmu_reg_map cpmu_map;
};
};
@@ -258,6 +268,8 @@ int cxl_map_component_regs(struct device *dev, struct cxl_component_regs *regs,
unsigned long map_mask);
int cxl_map_device_regs(struct device *dev, struct cxl_device_regs *regs,
struct cxl_register_map *map);
+int cxl_map_cpmu_regs(struct pci_dev *pdev, struct cxl_cpmu_regs *regs,
+ struct cxl_register_map *map);
enum cxl_regloc_type;
int cxl_count_regblock(struct pci_dev *pdev, enum cxl_regloc_type type);
@@ -750,6 +762,7 @@ void cxl_driver_unregister(struct cxl_driver *cxl_drv);
#define CXL_DEVICE_REGION 6
#define CXL_DEVICE_PMEM_REGION 7
#define CXL_DEVICE_DAX_REGION 8
+#define CXL_DEVICE_CPMU 9
#define MODULE_ALIAS_CXL(type) MODULE_ALIAS("cxl:t" __stringify(type) "*")
#define CXL_MODALIAS_FMT "cxl:t%d"
@@ -789,6 +802,7 @@ static inline struct cxl_dax_region *to_cxl_dax_region(struct device *dev)
}
#endif
+int devm_cxl_cpmu_add(struct device *parent, struct cxl_cpmu_regs *regs, int idx);
/*
* Unit test builds overrides this to __weak, find the 'strong' version
* of these symbols in tools/testing/cxl/.
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index be6a2ef3cce3..2fd495ab3de1 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -65,6 +65,7 @@ enum cxl_regloc_type {
CXL_REGLOC_RBI_COMPONENT,
CXL_REGLOC_RBI_VIRT,
CXL_REGLOC_RBI_MEMDEV,
+ CXL_REGLOC_RBI_CPMU,
CXL_REGLOC_RBI_TYPES
};
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 74443a5c3cc8..e8bc36cc2724 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -704,7 +704,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
struct cxl_register_map map;
struct cxl_memdev *cxlmd;
struct cxl_dev_state *cxlds;
- int rc;
+ int i, rc, cpmu_count;
/*
* Double check the anonymous union trickery in struct cxl_regs
@@ -781,6 +781,29 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (rc)
return rc;
+ cpmu_count = cxl_count_regblock(pdev, CXL_REGLOC_RBI_CPMU);
+ for (i = 0; i < cpmu_count; i++) {
+ struct cxl_cpmu_regs cpmu_regs;
+
+ rc = cxl_find_regblock(pdev, CXL_REGLOC_RBI_CPMU, &map, i);
+ if (rc) {
+ dev_dbg(&pdev->dev, "Could not find CPMU regblock\n");
+ break;
+ }
+
+ rc = cxl_map_cpmu_regs(pdev, &cpmu_regs, &map);
+ if (rc) {
+ dev_dbg(&pdev->dev, "Could not map CPMU regs\n");
+ break;
+ }
+
+ rc = devm_cxl_cpmu_add(cxlds->dev, &cpmu_regs, i);
+ if (rc) {
+ dev_dbg(&pdev->dev, "Could not add CPMU instance\n");
+ break;
+ }
+ }
+
cxlmd = devm_cxl_add_memdev(cxlds);
if (IS_ERR(cxlmd))
return PTR_ERR(cxlmd);
--
2.37.2
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v4 4/5] cxl: CXL Performance Monitoring Unit driver
2023-03-30 16:45 [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support Jonathan Cameron
` (2 preceding siblings ...)
2023-03-30 16:45 ` [PATCH v4 3/5] cxl/pci: Find and register CXL PMU devices Jonathan Cameron
@ 2023-03-30 16:45 ` Jonathan Cameron
2023-04-03 17:32 ` Liang, Kan
2023-04-04 21:53 ` Dan Williams
2023-03-30 16:45 ` [PATCH v4 5/5] docs: perf: Minimal introduction the the CXL PMU device and driver Jonathan Cameron
2023-04-04 3:55 ` [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support Dan Williams
5 siblings, 2 replies; 21+ messages in thread
From: Jonathan Cameron @ 2023-03-30 16:45 UTC (permalink / raw)
To: Liang Kan, linux-cxl, peterz
Cc: mingo, acme, mark.rutland, will, dan.j.williams, linuxarm,
linux-perf-users, linux-kernel, Davidlohr Bueso, Dave Jiang
CXL rev 3.0 introduces a standard performance monitoring hardware
block to CXL. Instances are discovered using CXL Register Locator DVSEC
entries. Each CXL component may have multiple PMUs.
This initial driver supports on a subset of types of counter.
It support counters that are either fixed or configurable, but requires
that they support the ability to freeze and write value whilst frozen.
Development done with QEMU model which will be posted shortly.
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v4:
- Add defines for masks used for config, config1 etc and reorder
code so those can be next to the attrs. Note that one of the
bits was wrong showing the advantage of bring all this info
into one place.
- Use enabled counter bitmap rather than pointer array to make the
fixed counter handling look more like the variable counter handling.
- Drop PERF_EF_RELOAD handling that did nothing.
- Simplify code as nothing special to do if the counting is already
up to date (no harm in setting the already set flag).
---
drivers/cxl/Kconfig | 13 +
drivers/cxl/Makefile | 1 +
drivers/cxl/cpmu.c | 940 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 954 insertions(+)
diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
index ff4e78117b31..0be514b00b8f 100644
--- a/drivers/cxl/Kconfig
+++ b/drivers/cxl/Kconfig
@@ -139,4 +139,17 @@ config CXL_REGION_INVALIDATION_TEST
If unsure, or if this kernel is meant for production environments,
say N.
+config CXL_CPMU
+ tristate "CXL Performance Monitoring Unit"
+ default CXL_BUS
+ depends on PERF_EVENTS
+ help
+ Support performance monitoring as defined in CXL rev 3.0
+ section 13.2: Performance Monitoring. CXL components may have
+ one or more CXL Performance Monitoring Units (CPMUs).
+
+ Say 'y/m' to enable a driver that will attach to performance
+ monitoring units and provide standard perf based interfaces.
+
+ If unsure say 'm'.
endif
diff --git a/drivers/cxl/Makefile b/drivers/cxl/Makefile
index db321f48ba52..024bb739554b 100644
--- a/drivers/cxl/Makefile
+++ b/drivers/cxl/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_CXL_MEM) += cxl_mem.o
obj-$(CONFIG_CXL_ACPI) += cxl_acpi.o
obj-$(CONFIG_CXL_PMEM) += cxl_pmem.o
obj-$(CONFIG_CXL_PORT) += cxl_port.o
+obj-$(CONFIG_CXL_CPMU) += cpmu.o
cxl_mem-y := mem.o
cxl_pci-y := pci.o
diff --git a/drivers/cxl/cpmu.c b/drivers/cxl/cpmu.c
new file mode 100644
index 000000000000..e6821771a4f6
--- /dev/null
+++ b/drivers/cxl/cpmu.c
@@ -0,0 +1,940 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Copyright(c) 2023 Huawei
+ *
+ * The CXL 3.0 specification includes a standard Performance Monitoring Unit,
+ * called the CXL PMU, or CPMU. In order to allow a high degree of
+ * implementation flexibility the specification provides a wide range of
+ * options all of which are self describing.
+ *
+ * Details in CXL rev 3.0 section 8.2.7 CPMU Register Interface
+ *
+ * TODO
+ * o Discoverability of counters. Allow perftool to provide summed counters
+ * and vendor defined counters.
+ * o Support free running counters - copy the Intel uncore PMU handling for these.
+ * o CPMUs which do not support freeze.
+ * o Add filter validation in cpmu_event_init() so problems are detected earlier.
+ * o Reject configurations that the hardware is ignoring
+ * (e.g. invert when not invertible)
+ * o Support CPMUs with no interrupts using an HRTIMER.
+ */
+
+#include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/perf_event.h>
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/bits.h>
+#include <linux/list.h>
+#include <linux/bug.h>
+#include <linux/pci.h>
+
+#include "cpmu.h"
+#include "cxlpci.h"
+#include "cxl.h"
+
+/* CXL rev 3.0 Table 13-5 Events under CXL Vendor ID */
+#define CPMU_GID_CLOCK_TICKS 0x00
+#define CPMU_GID_D2H_REQ 0x0010
+#define CPMU_GID_D2H_RSP 0x0011
+#define CPMU_GID_H2D_REQ 0x0012
+#define CPMU_GID_H2D_RSP 0x0013
+#define CPMU_GID_CACHE_DATA 0x0014
+#define CPMU_GID_M2S_REQ 0x0020
+#define CPMU_GID_M2S_RWD 0x0021
+#define CPMU_GID_M2S_BIRSP 0x0022
+#define CPMU_GID_S2M_BISNP 0x0023
+#define CPMU_GID_S2M_NDR 0x0024
+#define CPMU_GID_S2M_DRS 0x0025
+#define CPMU_GID_DDR 0x8000
+
+static int cpmu_cpuhp_state_num;
+
+struct cpmu_event {
+ u16 vid;
+ u16 gid;
+ u32 msk;
+ union {
+ int counter_idx; /* fixed counters */
+ int event_idx; /* configurable counters */
+ };
+ struct list_head node;
+};
+
+#define CPMU_MAX_COUNTERS 64
+struct cpmu_info {
+ struct pmu pmu;
+ void __iomem *base;
+ struct perf_event **hw_events;
+ struct list_head events_configurable;
+ struct list_head events_fixed;
+ DECLARE_BITMAP(used_counter_bm, CPMU_MAX_COUNTERS);
+ DECLARE_BITMAP(conf_counter_bm, CPMU_MAX_COUNTERS);
+ u16 counter_width;
+ u8 num_counters;
+ u8 num_event_capabilities;
+ int on_cpu;
+ struct hlist_node node;
+ bool freeze_for_enable;
+ bool filter_hdm;
+ int irq;
+};
+
+#define pmu_to_cpmu_info(_pmu) container_of(_pmu, struct cpmu_info, pmu)
+
+/*
+ * All CPMU counters are discoverable via the Event Capabilities Registers.
+ * Each Event Capability register contains a a VID / GroupID.
+ * A counter may then count any combination (by summing) of events in
+ * that group which are in the Supported Events Bitmask.
+ * However, there are some complexities to the scheme.
+ * - Fixed function counters refer to an Event Capabilities register.
+ * That event capability register is not then used for Configurable
+ * counters.
+ */
+static int cpmu_parse_caps(struct device *dev, struct cpmu_info *info)
+{
+ DECLARE_BITMAP(fixed_counter_event_cap_bm, 32) = {0};
+ void __iomem *base = info->base;
+ u64 val, eval;
+ int i;
+
+ val = readq(base + CPMU_CAP_REG);
+ info->freeze_for_enable = FIELD_GET(CPMU_CAP_WRITEABLE_WHEN_FROZEN, val) &
+ FIELD_GET(CPMU_CAP_FREEZE, val);
+ if (!info->freeze_for_enable) {
+ dev_err(dev, "Driver does not support CPMUs that do not support freeze for enable\n");
+ return -ENODEV;
+ }
+
+ info->num_counters = FIELD_GET(CPMU_CAP_NUM_COUNTERS_MSK, val) + 1;
+ info->counter_width = FIELD_GET(CPMU_CAP_COUNTER_WIDTH_MSK, val);
+ info->num_event_capabilities = FIELD_GET(CPMU_CAP_NUM_EVN_CAP_REG_SUP_MSK, val) + 1;
+
+ info->filter_hdm = FIELD_GET(CPMU_CAP_FILTERS_SUP_MSK, val) & CPMU_FILTER_HDM;
+ if (FIELD_GET(CPMU_CAP_INT, val))
+ info->irq = FIELD_GET(CPMU_CAP_MSI_N_MSK, val);
+ else
+ info->irq = -1;
+
+ /* First handle fixed function counters; note if configurable counters found */
+ for (i = 0; i < info->num_counters; i++) {
+ struct cpmu_event *cpmu_ev;
+ u32 events_msk;
+ u8 group_idx;
+
+ val = readq(base + CPMU_COUNTER_CFG_REG(i));
+
+ if (FIELD_GET(CPMU_COUNTER_CFG_TYPE_MSK, val) ==
+ CPMU_COUNTER_CFG_TYPE_CONFIGURABLE) {
+ set_bit(i, info->conf_counter_bm);
+ }
+
+ if (FIELD_GET(CPMU_COUNTER_CFG_TYPE_MSK, val) !=
+ CPMU_COUNTER_CFG_TYPE_FIXED_FUN)
+ continue;
+
+ /* In this case we know which fields are const */
+ group_idx = FIELD_GET(CPMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK, val);
+ events_msk = FIELD_GET(CPMU_COUNTER_CFG_EVENTS_MSK, val);
+ eval = readq(base + CPMU_EVENT_CAP_REG(group_idx));
+ cpmu_ev = devm_kzalloc(dev, sizeof(*cpmu_ev), GFP_KERNEL);
+ if (!cpmu_ev)
+ return -ENOMEM;
+
+ cpmu_ev->vid = FIELD_GET(CPMU_EVENT_CAP_VENDOR_ID_MSK, eval);
+ cpmu_ev->gid = FIELD_GET(CPMU_EVENT_CAP_GROUP_ID_MSK, eval);
+ /* For a fixed purpose counter use the events mask from the counter CFG */
+ cpmu_ev->msk = events_msk;
+ cpmu_ev->counter_idx = i;
+ /* This list add is never unwound as all entries deleted on remove */
+ list_add(&cpmu_ev->node, &info->events_fixed);
+ /*
+ * Configurable counters must not use an Event Capability registers that
+ * is in use for a Fixed counter
+ */
+ set_bit(group_idx, fixed_counter_event_cap_bm);
+ }
+
+ if (!bitmap_empty(info->conf_counter_bm, CPMU_MAX_COUNTERS)) {
+ struct cpmu_event *cpmu_ev;
+ int j;
+ /* Walk event capabilities unused by fixed counters */
+ for_each_clear_bit(j, fixed_counter_event_cap_bm,
+ info->num_event_capabilities) {
+ cpmu_ev = devm_kzalloc(dev, sizeof(*cpmu_ev), GFP_KERNEL);
+ if (!cpmu_ev)
+ return -ENOMEM;
+
+ eval = readq(base + CPMU_EVENT_CAP_REG(j));
+ cpmu_ev->vid = FIELD_GET(CPMU_EVENT_CAP_VENDOR_ID_MSK, eval);
+ cpmu_ev->gid = FIELD_GET(CPMU_EVENT_CAP_GROUP_ID_MSK, eval);
+ cpmu_ev->msk = FIELD_GET(CPMU_EVENT_CAP_SUPPORTED_EVENTS_MSK, eval);
+ cpmu_ev->event_idx = j;
+ list_add(&cpmu_ev->node, &info->events_configurable);
+ }
+ }
+
+ return 0;
+}
+
+static ssize_t cpmu_format_sysfs_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 sysfs_emit(buf, "%s\n", (char *)eattr->var);
+}
+
+#define CPMU_FORMAT_ATTR(_name, _format)\
+ (&((struct dev_ext_attribute[]) { \
+ { \
+ .attr = __ATTR(_name, 0444, \
+ cpmu_format_sysfs_show, NULL), \
+ .var = (void *)_format \
+ } \
+ })[0].attr.attr)
+
+enum {
+ cpmu_mask_attr,
+ cpmu_gid_attr,
+ cpmu_vid_attr,
+ cpmu_threshold_attr,
+ cpmu_invert_attr,
+ cpmu_edge_attr,
+ cpmu_hdm_filter_en_attr,
+ cpmu_hdm_attr,
+};
+
+static struct attribute *cpmu_format_attr[] = {
+ [cpmu_mask_attr] = CPMU_FORMAT_ATTR(mask, "config:0-31"),
+ [cpmu_gid_attr] = CPMU_FORMAT_ATTR(gid, "config:32-47"),
+ [cpmu_vid_attr] = CPMU_FORMAT_ATTR(vid, "config:48-63"),
+ [cpmu_threshold_attr] = CPMU_FORMAT_ATTR(threshold, "config1:0-15"),
+ [cpmu_invert_attr] = CPMU_FORMAT_ATTR(invert, "config1:16"),
+ [cpmu_edge_attr] = CPMU_FORMAT_ATTR(edge, "config1:17"),
+ [cpmu_hdm_filter_en_attr] = CPMU_FORMAT_ATTR(hdm_filter_en, "config1:18"),
+ [cpmu_hdm_attr] = CPMU_FORMAT_ATTR(hdm, "config2:0-15"),
+ NULL
+};
+
+#define CPMU_ATTR_CONFIG_MASK_MSK GENMASK_ULL(63, 48)
+#define CPMU_ATTR_CONFIG_GID_MSK GENMASK_ULL(47, 32)
+#define CPMU_ATTR_CONFIG_VID_MSK GENMASK_ULL(63, 48)
+#define CPMU_ATTR_CONFIG1_THRESHOLD_MSK GENMASK_ULL(15, 0)
+#define CPMU_ATTR_CONFIG1_INVERT_MSK BIT(16)
+#define CPMU_ATTR_CONFIG1_EDGE_MSK BIT(17)
+#define CPMU_ATTR_CONFIG1_FILTER_EN_MSK BIT(18)
+#define CPMU_ATTR_CONFIG2_HDM_MSK GENMASK(15, 0)
+
+static umode_t cpmu_format_is_visible(struct kobject *kobj, struct attribute *attr, int a)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct cpmu_info *info = dev_get_drvdata(dev);
+
+ /*
+ * Filter capability at the CPMU level, so hide the attributes if the particular
+ * filter is not supported.
+ */
+ if (attr == cpmu_format_attr[cpmu_hdm_filter_en_attr] ||
+ attr == cpmu_format_attr[cpmu_hdm_attr]) {
+ if (info->filter_hdm)
+ return 0444;
+ else
+ return 0;
+ } else {
+ return 0444;
+ }
+}
+
+static const struct attribute_group cpmu_format_group = {
+ .name = "format",
+ .attrs = cpmu_format_attr,
+ .is_visible = cpmu_format_is_visible,
+};
+
+static u32 cpmu_config_get_mask(struct perf_event *event)
+{
+ return FIELD_GET(CPMU_ATTR_CONFIG_MASK_MSK, event->attr.config);
+}
+
+static u16 cpmu_config_get_gid(struct perf_event *event)
+{
+ return FIELD_GET(CPMU_ATTR_CONFIG_GID_MSK, event->attr.config);
+}
+
+static u16 cpmu_config_get_vid(struct perf_event *event)
+{
+ return FIELD_GET(CPMU_ATTR_CONFIG_VID_MSK, event->attr.config);
+}
+
+static u8 cpmu_config1_get_threshold(struct perf_event *event)
+{
+ return FIELD_GET(CPMU_ATTR_CONFIG1_THRESHOLD_MSK, event->attr.config1);
+}
+
+static bool cpmu_config1_get_invert(struct perf_event *event)
+{
+ return FIELD_GET(CPMU_ATTR_CONFIG1_INVERT_MSK, event->attr.config1);
+}
+
+static bool cpmu_config1_get_edge(struct perf_event *event)
+{
+ return FIELD_GET(CPMU_ATTR_CONFIG1_EDGE_MSK, event->attr.config1);
+}
+
+/*
+ * CPMU specification allows for 8 filters, each with a 16 bit value...
+ * So we need to find 8x16bits to store it in.
+ * As the value used for disable is 0xffff, a separate enable switch
+ * is needed.
+ */
+
+static bool cpmu_config1_hdm_filter_en(struct perf_event *event)
+{
+ return FIELD_GET(CPMU_ATTR_CONFIG1_FILTER_EN_MSK, event->attr.config1);
+}
+
+static u16 cpmu_config2_get_hdm_decoder(struct perf_event *event)
+{
+ return FIELD_GET(CPMU_ATTR_CONFIG2_HDM_MSK, event->attr.config2);
+}
+
+static ssize_t cpmu_event_sysfs_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct perf_pmu_events_attr *pmu_attr =
+ container_of(attr, struct perf_pmu_events_attr, attr);
+
+ return sysfs_emit(buf, "config=%#llx\n", pmu_attr->id);
+}
+
+#define CPMU_PMU_EVENT_ATTR(_name, _vid, _gid, _msk) \
+ PMU_EVENT_ATTR_ID(_name, cpmu_event_sysfs_show, \
+ ((u64)(_vid) << 48) | ((u64)(_gid) << 32) | (u64)(_msk))
+
+/* For CXL spec defined events */
+#define CPMU_PMU_EVENT_CXL_ATTR(_name, _gid, _msk) \
+ CPMU_PMU_EVENT_ATTR(_name, PCI_DVSEC_VENDOR_ID_CXL, _gid, _msk)
+
+static struct attribute *cpmu_event_attrs[] = {
+ CPMU_PMU_EVENT_CXL_ATTR(clock_ticks, CPMU_GID_CLOCK_TICKS, BIT(0)),
+ /* CXL rev 3.0 Table 3-17 - Device to Host Requests */
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_req_rdcurr, CPMU_GID_D2H_REQ, BIT(1)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_req_rdown, CPMU_GID_D2H_REQ, BIT(2)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_req_rdshared, CPMU_GID_D2H_REQ, BIT(3)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_req_rdany, CPMU_GID_D2H_REQ, BIT(4)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_req_rdownnodata, CPMU_GID_D2H_REQ, BIT(5)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_req_itomwr, CPMU_GID_D2H_REQ, BIT(6)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_req_wrcurr, CPMU_GID_D2H_REQ, BIT(7)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_req_clflush, CPMU_GID_D2H_REQ, BIT(8)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_req_cleanevict, CPMU_GID_D2H_REQ, BIT(9)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_req_dirtyevict, CPMU_GID_D2H_REQ, BIT(10)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_req_cleanevictnodata, CPMU_GID_D2H_REQ, BIT(11)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_req_wowrinv, CPMU_GID_D2H_REQ, BIT(12)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_req_wowrinvf, CPMU_GID_D2H_REQ, BIT(13)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_req_wrinv, CPMU_GID_D2H_REQ, BIT(14)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_req_cacheflushed, CPMU_GID_D2H_REQ, BIT(16)),
+ /* CXL rev 3.0 Table 3-20 - D2H Repsonse Encodings */
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspihiti, CPMU_GID_D2H_RSP, BIT(4)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspvhitv, CPMU_GID_D2H_RSP, BIT(6)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspihitse, CPMU_GID_D2H_RSP, BIT(5)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspshitse, CPMU_GID_D2H_RSP, BIT(1)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspsfwdm, CPMU_GID_D2H_RSP, BIT(7)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspifwdm, CPMU_GID_D2H_RSP, BIT(15)),
+ CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspvfwdv, CPMU_GID_D2H_RSP, BIT(22)),
+ /* CXL rev 3.0 Table 3-21 - CXL.cache - Mapping of H2D Requests to D2H Responses */
+ CPMU_PMU_EVENT_CXL_ATTR(h2d_req_snpdata, CPMU_GID_H2D_REQ, BIT(1)),
+ CPMU_PMU_EVENT_CXL_ATTR(h2d_req_snpinv, CPMU_GID_H2D_REQ, BIT(2)),
+ CPMU_PMU_EVENT_CXL_ATTR(h2d_req_snpcur, CPMU_GID_H2D_REQ, BIT(3)),
+ /* CXL rev 3.0 Table 3-22 - H2D Response Opcode Encodings */
+ CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_writepull, CPMU_GID_H2D_RSP, BIT(1)),
+ CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_go, CPMU_GID_H2D_RSP, BIT(4)),
+ CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_gowritepull, CPMU_GID_H2D_RSP, BIT(5)),
+ CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_extcmp, CPMU_GID_H2D_RSP, BIT(6)),
+ CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_gowritepulldrop, CPMU_GID_H2D_RSP, BIT(8)),
+ CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_fastgowritepull, CPMU_GID_H2D_RSP, BIT(13)),
+ CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_goerrwritepull, CPMU_GID_H2D_RSP, BIT(15)),
+ /* CXL rev 3.0 Table 13-5 directly lists these */
+ CPMU_PMU_EVENT_CXL_ATTR(cachedata_d2h_data, CPMU_GID_CACHE_DATA, BIT(0)),
+ CPMU_PMU_EVENT_CXL_ATTR(cachedata_h2d_data, CPMU_GID_CACHE_DATA, BIT(1)),
+ /* CXL rev 3.0 Table 3-29 M2S Req Memory Opcodes */
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_req_meminv, CPMU_GID_M2S_REQ, BIT(0)),
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memrd, CPMU_GID_M2S_REQ, BIT(1)),
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memrddata, CPMU_GID_M2S_REQ, BIT(2)),
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memrdfwd, CPMU_GID_M2S_REQ, BIT(3)),
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memwrfwd, CPMU_GID_M2S_REQ, BIT(4)),
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memspecrd, CPMU_GID_M2S_REQ, BIT(8)),
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_req_meminvnt, CPMU_GID_M2S_REQ, BIT(9)),
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memcleanevict, CPMU_GID_M2S_REQ, BIT(10)),
+ /* CXL rev 3.0 Table 3-35 M2S RwD Memory Opcodes */
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_rwd_memwr, CPMU_GID_M2S_RWD, BIT(1)),
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_rwd_memwrptl, CPMU_GID_M2S_RWD, BIT(2)),
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_rwd_biconflict, CPMU_GID_M2S_RWD, BIT(4)),
+ /* CXL rev 3.0 Table 3-38 M2S BIRsp Memory Opcodes */
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_i, CPMU_GID_M2S_BIRSP, BIT(0)),
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_s, CPMU_GID_M2S_BIRSP, BIT(1)),
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_e, CPMU_GID_M2S_BIRSP, BIT(2)),
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_iblk, CPMU_GID_M2S_BIRSP, BIT(4)),
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_sblk, CPMU_GID_M2S_BIRSP, BIT(5)),
+ CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_eblk, CPMU_GID_M2S_BIRSP, BIT(6)),
+ /* CXL rev 3.0 Table 3-40 S2M BISnp Opcodes */
+ CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_cur, CPMU_GID_S2M_BISNP, BIT(0)),
+ CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_data, CPMU_GID_S2M_BISNP, BIT(1)),
+ CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_inv, CPMU_GID_S2M_BISNP, BIT(2)),
+ CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_curblk, CPMU_GID_S2M_BISNP, BIT(4)),
+ CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_datblk, CPMU_GID_S2M_BISNP, BIT(5)),
+ CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_invblk, CPMU_GID_S2M_BISNP, BIT(6)),
+ /* CXL rev 3.0 Table 3-43 S2M NDR Opcopdes */
+ CPMU_PMU_EVENT_CXL_ATTR(s2m_ndr_cmp, CPMU_GID_S2M_NDR, BIT(0)),
+ CPMU_PMU_EVENT_CXL_ATTR(s2m_ndr_cmps, CPMU_GID_S2M_NDR, BIT(1)),
+ CPMU_PMU_EVENT_CXL_ATTR(s2m_ndr_cmpe, CPMU_GID_S2M_NDR, BIT(2)),
+ CPMU_PMU_EVENT_CXL_ATTR(s2m_ndr_biconflictack, CPMU_GID_S2M_NDR, BIT(3)),
+ /* CXL rev 3.0 Table 3-46 S2M DRS opcodes */
+ CPMU_PMU_EVENT_CXL_ATTR(s2m_drs_memdata, CPMU_GID_S2M_DRS, BIT(0)),
+ CPMU_PMU_EVENT_CXL_ATTR(s2m_drs_memdatanxm, CPMU_GID_S2M_DRS, BIT(1)),
+ /* CXL rev 3.0 Table 13-5 directly lists these */
+ CPMU_PMU_EVENT_CXL_ATTR(ddr_act, CPMU_GID_DDR, BIT(0)),
+ CPMU_PMU_EVENT_CXL_ATTR(ddr_pre, CPMU_GID_DDR, BIT(1)),
+ CPMU_PMU_EVENT_CXL_ATTR(ddr_casrd, CPMU_GID_DDR, BIT(2)),
+ CPMU_PMU_EVENT_CXL_ATTR(ddr_caswr, CPMU_GID_DDR, BIT(3)),
+ CPMU_PMU_EVENT_CXL_ATTR(ddr_refresh, CPMU_GID_DDR, BIT(4)),
+ CPMU_PMU_EVENT_CXL_ATTR(ddr_selfrefreshent, CPMU_GID_DDR, BIT(5)),
+ CPMU_PMU_EVENT_CXL_ATTR(ddr_rfm, CPMU_GID_DDR, BIT(6)),
+ NULL
+};
+
+static struct cpmu_event *cpmu_find_fixed_counter_event(struct cpmu_info *info,
+ int vid, int gid, int msk)
+{
+ struct cpmu_event *cpmu_ev;
+
+ list_for_each_entry(cpmu_ev, &info->events_fixed, node) {
+ if (vid != cpmu_ev->vid || gid != cpmu_ev->gid)
+ continue;
+
+ /* Precise match for fixed counter */
+ if (msk == cpmu_ev->msk)
+ return cpmu_ev;
+ }
+
+ return ERR_PTR(-EINVAL);
+}
+
+static struct cpmu_event *cpmu_find_config_counter_event(struct cpmu_info *info,
+ int vid, int gid, int msk)
+{
+ struct cpmu_event *cpmu_ev;
+
+ list_for_each_entry(cpmu_ev, &info->events_configurable, node) {
+ if (vid != cpmu_ev->vid || gid != cpmu_ev->gid)
+ continue;
+
+ /* Request mask must be subset of supported */
+ if (msk & ~cpmu_ev->msk)
+ continue;
+
+ return cpmu_ev;
+ }
+
+ return ERR_PTR(-EINVAL);
+}
+
+static umode_t cpmu_event_is_visible(struct kobject *kobj, struct attribute *attr, int a)
+{
+ struct device_attribute *dev_attr = container_of(attr, struct device_attribute, attr);
+ struct perf_pmu_events_attr *pmu_attr =
+ container_of(dev_attr, struct perf_pmu_events_attr, attr);
+ struct device *dev = kobj_to_dev(kobj);
+ struct cpmu_info *info = dev_get_drvdata(dev);
+ int vid = FIELD_GET(CPMU_ATTR_CONFIG_VID_MSK, pmu_attr->id);
+ int gid = FIELD_GET(CPMU_ATTR_CONFIG_GID_MSK, pmu_attr->id);
+ int msk = FIELD_GET(CPMU_ATTR_CONFIG_MASK_MSK, pmu_attr->id);
+
+ if (!IS_ERR(cpmu_find_fixed_counter_event(info, vid, gid, msk)))
+ return attr->mode;
+
+ if (!IS_ERR(cpmu_find_config_counter_event(info, vid, gid, msk)))
+ return attr->mode;
+
+ return 0;
+}
+
+static const struct attribute_group cpmu_events = {
+ .name = "events",
+ .attrs = cpmu_event_attrs,
+ .is_visible = cpmu_event_is_visible,
+};
+
+static ssize_t cpumask_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct cpmu_info *info = dev_get_drvdata(dev);
+
+ return cpumap_print_to_pagebuf(true, buf, cpumask_of(info->on_cpu));
+}
+static DEVICE_ATTR_RO(cpumask);
+
+static struct attribute *cpmu_cpumask_attrs[] = {
+ &dev_attr_cpumask.attr,
+ NULL
+};
+
+static const struct attribute_group cpmu_cpumask_group = {
+ .attrs = cpmu_cpumask_attrs,
+};
+
+static const struct attribute_group *cpmu_attr_groups[] = {
+ &cpmu_events,
+ &cpmu_format_group,
+ &cpmu_cpumask_group,
+ NULL
+};
+
+/* If counter_idx == NULL, don't try to allocate a counter. */
+static int cpmu_get_event_idx(struct perf_event *event, int *counter_idx, int *event_idx)
+{
+ struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
+ DECLARE_BITMAP(configurable_and_free, CPMU_MAX_COUNTERS);
+ struct cpmu_event *cpmu_ev;
+ u32 mask;
+ u16 gid, vid;
+ int i;
+
+ vid = cpmu_config_get_vid(event);
+ gid = cpmu_config_get_gid(event);
+ mask = cpmu_config_get_mask(event);
+
+ cpmu_ev = cpmu_find_fixed_counter_event(info, vid, gid, mask);
+ if (!IS_ERR(cpmu_ev)) {
+ if (!counter_idx)
+ return 0;
+ if (!test_bit(cpmu_ev->counter_idx, info->used_counter_bm)) {
+ *counter_idx = cpmu_ev->counter_idx;
+ return 0;
+ }
+ /* Fixed counter is in use, but maybe a configurable one? */
+ }
+
+ cpmu_ev = cpmu_find_config_counter_event(info, vid, gid, mask);
+ if (!IS_ERR(cpmu_ev)) {
+ if (!counter_idx)
+ return 0;
+
+ bitmap_andnot(configurable_and_free, info->conf_counter_bm,
+ info->used_counter_bm, CPMU_MAX_COUNTERS);
+
+ i = find_first_bit(configurable_and_free, CPMU_MAX_COUNTERS);
+ if (i == CPMU_MAX_COUNTERS)
+ return -EINVAL;
+
+ *counter_idx = i;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int cpmu_event_init(struct perf_event *event)
+{
+ struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
+
+ event->cpu = info->on_cpu;
+ /* Top level type sanity check - is this a Hardware Event being requested */
+ if (event->attr.type != event->pmu->type)
+ return -ENOENT;
+
+ if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
+ return -EOPNOTSUPP;
+ /* TODO: Validation of any filter */
+
+ /*
+ * Verify that it is possible to count what was requested. Either must
+ * be a fixed counter that is a precise match or a configurable counter
+ * where this is a subset.
+ */
+ return cpmu_get_event_idx(event, NULL, NULL);
+}
+
+static void cpmu_pmu_enable(struct pmu *pmu)
+{
+ struct cpmu_info *info = pmu_to_cpmu_info(pmu);
+ void __iomem *base = info->base;
+
+ /* We don't have a global enable, but we 'might' have a global freeze which we can use */
+ if (info->freeze_for_enable) {
+ /* Can assume frozen at this stage */
+ writeq(0, base + CPMU_FREEZE_REG);
+
+ return;
+ }
+}
+
+static void cpmu_pmu_disable(struct pmu *pmu)
+{
+ struct cpmu_info *info = pmu_to_cpmu_info(pmu);
+ void __iomem *base = info->base;
+
+ if (info->freeze_for_enable) {
+ /*
+ * Whilst bits above number of counters are RsvdZ
+ * they are unlikely to be repurposed given
+ * number of counters is allowed to be 64 leaving
+ * no reserved bits. Hence this is only slightly
+ * naughty.
+ */
+ writeq(GENMASK_ULL(63, 0), base + CPMU_FREEZE_REG);
+ return;
+ }
+}
+
+static void cpmu_event_start(struct perf_event *event, int flags)
+{
+ struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+ void __iomem *base = info->base;
+ u64 cfg;
+
+ if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
+ return;
+
+ WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
+ hwc->state = 0;
+
+ /*
+ * Currently only hdm filter control is implemnted, this code will
+ * want generalizing when more filters are added.
+ */
+ if (info->filter_hdm) {
+ if (cpmu_config1_hdm_filter_en(event))
+ cfg = cpmu_config2_get_hdm_decoder(event);
+ else
+ cfg = GENMASK(15, 0); /* No filtering if 0xFFFF_FFFF */
+ writeq(cfg, base + CPMU_FILTER_CFG_REG(hwc->idx, 0));
+ }
+
+ cfg = readq(base + CPMU_COUNTER_CFG_REG(hwc->idx));
+ cfg |= FIELD_PREP(CPMU_COUNTER_CFG_INT_ON_OVRFLW, 1);
+ cfg |= FIELD_PREP(CPMU_COUNTER_CFG_ENABLE, 1);
+ cfg |= FIELD_PREP(CPMU_COUNTER_CFG_EDGE, cpmu_config1_get_edge(event) ? 1 : 0);
+ cfg |= FIELD_PREP(CPMU_COUNTER_CFG_INVERT, cpmu_config1_get_invert(event) ? 1 : 0);
+
+ /* Fixed purpose counters have next two fields RO */
+ if (test_bit(hwc->idx, info->conf_counter_bm)) {
+ cfg |= FIELD_PREP(CPMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK, hwc->event_base);
+ cfg |= FIELD_PREP(CPMU_COUNTER_CFG_EVENTS_MSK, cpmu_config_get_mask(event));
+ }
+ cfg &= ~CPMU_COUNTER_CFG_THRESHOLD_MSK;
+ /*
+ * For events that generate only 1 count per clock the CXL 3.0 spec
+ * states the threshold shall be set to 1 but if set to 0 it will
+ * count the raw value anwyay?
+ * There is no definition of what events will count multiple per cycle
+ * and hence to which non 1 values of threshold can apply.
+ * (CXL 3.0 8.2.7.2.1 Counter Configuration - threshold field definition)
+ */
+ cfg |= FIELD_PREP(CPMU_COUNTER_CFG_THRESHOLD_MSK,
+ cpmu_config1_get_threshold(event));
+ writeq(cfg, base + CPMU_COUNTER_CFG_REG(hwc->idx));
+
+ local64_set(&hwc->prev_count, 0);
+ writeq(0, base + CPMU_COUNTER_REG(hwc->idx));
+
+ perf_event_update_userpage(event);
+}
+
+static u64 cpmu_read_counter(struct perf_event *event)
+{
+ struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
+ void __iomem *base = info->base;
+
+ return readq(base + CPMU_COUNTER_REG(event->hw.idx));
+}
+
+static void __cpmu_read(struct perf_event *event, bool overflow)
+{
+ struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+ u64 new_cnt, prev_cnt, delta;
+
+ do {
+ prev_cnt = local64_read(&hwc->prev_count);
+ new_cnt = cpmu_read_counter(event);
+ } while (local64_cmpxchg(&hwc->prev_count, prev_cnt, new_cnt) != prev_cnt);
+
+ /*
+ * If we know an overflow occur then take that into account.
+ * Note counter is not reset as that would lose events
+ */
+ delta = (new_cnt - prev_cnt) & GENMASK_ULL(info->counter_width - 1, 0);
+ if (overflow && delta < GENMASK_ULL(info->counter_width - 1, 0))
+ delta += (1UL << info->counter_width);
+
+ local64_add(delta, &event->count);
+}
+
+static void cpmu_read(struct perf_event *event)
+{
+ __cpmu_read(event, false);
+}
+
+static void cpmu_event_stop(struct perf_event *event, int flags)
+{
+ struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
+ void __iomem *base = info->base;
+ struct hw_perf_event *hwc = &event->hw;
+ u64 cfg;
+
+ cpmu_read(event);
+ WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
+ hwc->state |= PERF_HES_STOPPED;
+
+ cfg = readq(base + CPMU_COUNTER_CFG_REG(hwc->idx));
+ cfg &= ~(FIELD_PREP(CPMU_COUNTER_CFG_INT_ON_OVRFLW, 1) |
+ FIELD_PREP(CPMU_COUNTER_CFG_ENABLE, 1));
+ writeq(cfg, base + CPMU_COUNTER_CFG_REG(hwc->idx));
+
+ hwc->state |= PERF_HES_UPTODATE;
+}
+
+/*
+ * Reset ensures no possibility of any information leaking to wrong
+ * counter. Note that all fields written during start()
+ */
+static void cpmu_reset_counter(struct cpmu_info *info, int idx)
+{
+ void __iomem *base = info->base;
+
+ /* Much of this register is read only */
+ writeq(0, base + CPMU_EVENT_CAP_REG(idx));
+ /* Filters are not per counter, so no reset here */
+ writeq(0, base + CPMU_COUNTER_REG(idx));
+}
+
+static int cpmu_event_add(struct perf_event *event, int flags)
+{
+ struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+ int idx, rc;
+ int event_idx = 0;
+
+ hwc->state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
+
+ rc = cpmu_get_event_idx(event, &idx, &event_idx);
+ if (rc < 0)
+ return rc;
+
+ hwc->idx = idx;
+
+ /* Only set for configurable counters */
+ hwc->event_base = event_idx;
+ info->hw_events[idx] = event;
+ set_bit(idx, info->used_counter_bm);
+
+ cpmu_reset_counter(info, idx);
+
+ if (flags & PERF_EF_START)
+ cpmu_event_start(event, PERF_EF_RELOAD);
+
+ return 0;
+}
+
+static void cpmu_event_del(struct perf_event *event, int flags)
+{
+ struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+
+ cpmu_event_stop(event, PERF_EF_UPDATE);
+ clear_bit(hwc->idx, info->used_counter_bm);
+ info->hw_events[hwc->idx] = NULL;
+ perf_event_update_userpage(event);
+}
+
+static irqreturn_t cpmu_irq(int irq, void *data)
+{
+ struct cpmu_info *info = data;
+ void __iomem *base = info->base;
+ u64 overflowed;
+ DECLARE_BITMAP(overflowedbm, 64);
+ int i;
+
+ overflowed = readq(base + CPMU_OVERFLOW_REG);
+
+ /* Interrupt may be shared, so maybe it isn't ours */
+ if (!overflowed)
+ return IRQ_NONE;
+
+ bitmap_from_arr64(overflowedbm, &overflowed, 64);
+ for_each_set_bit(i, overflowedbm, info->num_counters) {
+ struct perf_event *event = info->hw_events[i];
+
+ if (!event) {
+ dev_dbg(info->pmu.dev,
+ "overflow but on non enabled counter %d\n", i);
+ continue;
+ }
+
+ __cpmu_read(event, true);
+ }
+
+ writeq(overflowed, base + CPMU_OVERFLOW_REG);
+
+ return IRQ_HANDLED;
+}
+
+static int cxl_cpmu_probe(struct device *dev)
+{
+ struct cxl_cpmu *cpmu = to_cxl_cpmu(dev);
+ struct pci_dev *pdev = to_pci_dev(dev->parent);
+ struct cpmu_info *info;
+ char *irq_name;
+ int rc, irq;
+
+ info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&info->events_fixed);
+ INIT_LIST_HEAD(&info->events_configurable);
+
+ info->base = cpmu->base;
+
+ info->on_cpu = -1;
+ rc = cpmu_parse_caps(dev, info);
+ if (rc)
+ return rc;
+
+ info->hw_events = devm_kcalloc(dev, sizeof(*info->hw_events),
+ info->num_counters, GFP_KERNEL);
+ if (!info->hw_events)
+ return -ENOMEM;
+
+ info->pmu = (struct pmu) {
+ .name = dev_name(dev),
+ .parent = dev,
+ .module = THIS_MODULE,
+ .event_init = cpmu_event_init,
+ .pmu_enable = cpmu_pmu_enable,
+ .pmu_disable = cpmu_pmu_disable,
+ .add = cpmu_event_add,
+ .del = cpmu_event_del,
+ .start = cpmu_event_start,
+ .stop = cpmu_event_stop,
+ .read = cpmu_read,
+ .task_ctx_nr = perf_invalid_context,
+ .attr_groups = cpmu_attr_groups,
+ .capabilities = PERF_PMU_CAP_NO_EXCLUDE,
+ };
+
+ if (info->irq <= 0)
+ return -EINVAL;
+
+ rc = pci_irq_vector(pdev, info->irq);
+ if (rc < 0)
+ return rc;
+ irq = rc;
+
+ irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s_overflow\n", dev_name(dev));
+ if (!irq_name)
+ return -ENOMEM;
+
+ rc = devm_request_irq(dev, irq, cpmu_irq, IRQF_SHARED, irq_name, info);
+ if (rc)
+ return rc;
+ info->irq = irq;
+
+ rc = cpuhp_state_add_instance(cpmu_cpuhp_state_num, &info->node);
+ if (rc)
+ return rc;
+
+ rc = perf_pmu_register(&info->pmu, info->pmu.name, -1);
+ if (rc)
+ return rc;
+
+ dev_set_drvdata(dev, info);
+
+ return 0;
+}
+
+static void cxl_cpmu_remove(struct device *dev)
+{
+ struct cpmu_info *info = dev_get_drvdata(dev);
+
+ perf_pmu_unregister(&info->pmu);
+ cpuhp_state_remove_instance_nocalls(cpmu_cpuhp_state_num, &info->node);
+}
+
+static struct cxl_driver cxl_cpmu_driver = {
+ .name = "cxl_cpmu",
+ .probe = cxl_cpmu_probe,
+ .remove = cxl_cpmu_remove,
+ .id = CXL_DEVICE_CPMU,
+};
+
+static int cpmu_online_cpu(unsigned int cpu, struct hlist_node *node)
+{
+ struct cpmu_info *info = hlist_entry_safe(node, struct cpmu_info, node);
+
+ if (info->on_cpu != -1)
+ return 0;
+
+ info->on_cpu = cpu;
+ WARN_ON(irq_set_affinity(info->irq, cpumask_of(cpu)));
+
+ return 0;
+}
+
+static int cpmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
+{
+ struct cpmu_info *info = hlist_entry_safe(node, struct cpmu_info, node);
+ unsigned int target;
+
+ if (info->on_cpu != cpu)
+ return 0;
+
+ info->on_cpu = -1;
+ target = cpumask_first(cpu_online_mask);
+ if (target >= nr_cpu_ids) {
+ dev_err(info->pmu.dev, "Unable to find a suitable CPU\n");
+ return 0;
+ }
+
+ perf_pmu_migrate_context(&info->pmu, cpu, target);
+ info->on_cpu = target;
+ WARN_ON(irq_set_affinity(info->irq, cpumask_of(target)));
+
+ return 0;
+}
+
+static __init int cxl_cpmu_init(void)
+{
+ int rc;
+
+ rc = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
+ "AP_PERF_CPMU_ONLINE",
+ cpmu_online_cpu, cpmu_offline_cpu);
+ if (rc < 0)
+ return rc;
+ cpmu_cpuhp_state_num = rc;
+
+ rc = cxl_driver_register(&cxl_cpmu_driver);
+ if (rc)
+ cpuhp_remove_multi_state(cpmu_cpuhp_state_num);
+
+ return rc;
+}
+
+static __exit void cxl_cpmu_exit(void)
+{
+ cxl_driver_unregister(&cxl_cpmu_driver);
+ cpuhp_remove_multi_state(cpmu_cpuhp_state_num);
+}
+
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(CXL);
+module_init(cxl_cpmu_init);
+module_exit(cxl_cpmu_exit);
+MODULE_ALIAS_CXL(CXL_DEVICE_CPMU);
--
2.37.2
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v4 5/5] docs: perf: Minimal introduction the the CXL PMU device and driver
2023-03-30 16:45 [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support Jonathan Cameron
` (3 preceding siblings ...)
2023-03-30 16:45 ` [PATCH v4 4/5] cxl: CXL Performance Monitoring Unit driver Jonathan Cameron
@ 2023-03-30 16:45 ` Jonathan Cameron
2023-04-03 17:45 ` Liang, Kan
2023-04-04 22:24 ` Dan Williams
2023-04-04 3:55 ` [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support Dan Williams
5 siblings, 2 replies; 21+ messages in thread
From: Jonathan Cameron @ 2023-03-30 16:45 UTC (permalink / raw)
To: Liang Kan, linux-cxl, peterz
Cc: mingo, acme, mark.rutland, will, dan.j.williams, linuxarm,
linux-perf-users, linux-kernel, Davidlohr Bueso, Dave Jiang
Very basic introduction to the device and the current driver support
provided. I expect to expand on this in future versions of this patch
set.
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
--
v4: No change
---
Documentation/admin-guide/perf/cxl.rst | 65 ++++++++++++++++++++++++
Documentation/admin-guide/perf/index.rst | 1 +
2 files changed, 66 insertions(+)
diff --git a/Documentation/admin-guide/perf/cxl.rst b/Documentation/admin-guide/perf/cxl.rst
new file mode 100644
index 000000000000..46235dff4b21
--- /dev/null
+++ b/Documentation/admin-guide/perf/cxl.rst
@@ -0,0 +1,65 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+======================================
+CXL Performance Monitoring Unit (CPMU)
+======================================
+
+The CXL rev 3.0 specification provides a definition of CXL Performance
+Monitoring Unit in section 13.2: Performance Monitoring.
+
+CXL components (e.g. Root Port, Switch Upstream Port, End Point) may have
+any number of CPMU instances. CPMU capabilities are fully discoverable from
+the devices. The specification provides event definitions for all CXL protocol
+message types and a set of additional events for things commonly counted on
+CXL devices (e.g. DRAM events).
+
+CPMU driver
+===========
+
+The CPMU driver register a perf PMU with the name cpmu<id> on the CXL bus.
+
+ /sys/bus/cxl/device/cpmu<id>
+
+The associated PMU is registered as
+
+ /sys/bus/event_sources/devices/cpmu<id>
+
+In common with other CXL bus devices, the id has no specific meaning and the
+relationship to specific CXL device should be established via the device parent
+of the device on the CXL bus.
+
+PMU driver provides description of available events and filter options in sysfs.
+
+The "format" directory describes all formats of the config (event vendor id,
+group id and mask) config1 (threshold, filter enables) and config2 (filter
+parameters) fields of the perf_event_attr structure. The "events" directory
+describes all documented events show in perf list.
+
+The events shown in perf list are the most fine grained events with a single
+bit of the event mask set. More general events may be enable by setting
+multiple mask bits in config. For example, all Device to Host Read Requests
+may be captured on a single counter by setting the bits for all of
+
+* d2h_req_rdcurr
+* d2h_req_rdown
+* d2h_req_rdshared
+* d2h_req_rdany
+* d2h_req_rdownnodata
+
+Example of usage::
+
+ $#perf list
+ cpmu0/clock_ticks/ [Kernel PMU event]
+ cpmu0/d2h_req_itomwr/ [Kernel PMU event]
+ cpmu0/d2h_req_rdany/ [Kernel PMU event]
+ cpmu0/d2h_req_rdcurr/ [Kernel PMU event]
+ -----------------------------------------------------------
+
+ $# perf stat -e cpmu0/clock_ticks/ -e cpmu0/d2h_req_itowrm/
+
+Vendor specific events may also be available and if so can be used via
+
+ $# perf stat -e cpmu0/vid=VID,gid=GID,mask=MASK/
+
+The driver does not support sampling. So "perf record" and attaching to
+a task are unsupported.
diff --git a/Documentation/admin-guide/perf/index.rst b/Documentation/admin-guide/perf/index.rst
index 9de64a40adab..f60be04e4e33 100644
--- a/Documentation/admin-guide/perf/index.rst
+++ b/Documentation/admin-guide/perf/index.rst
@@ -21,3 +21,4 @@ Performance monitor support
alibaba_pmu
nvidia-pmu
meson-ddr-pmu
+ cxl
--
2.37.2
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v4 4/5] cxl: CXL Performance Monitoring Unit driver
2023-03-30 16:45 ` [PATCH v4 4/5] cxl: CXL Performance Monitoring Unit driver Jonathan Cameron
@ 2023-04-03 17:32 ` Liang, Kan
2023-04-04 16:48 ` Jonathan Cameron
2023-04-04 21:53 ` Dan Williams
1 sibling, 1 reply; 21+ messages in thread
From: Liang, Kan @ 2023-04-03 17:32 UTC (permalink / raw)
To: Jonathan Cameron, linux-cxl, peterz
Cc: mingo, acme, mark.rutland, will, dan.j.williams, linuxarm,
linux-perf-users, linux-kernel, Davidlohr Bueso, Dave Jiang
On 2023-03-30 12:45 p.m., Jonathan Cameron wrote:
> CXL rev 3.0 introduces a standard performance monitoring hardware
> block to CXL. Instances are discovered using CXL Register Locator DVSEC
> entries. Each CXL component may have multiple PMUs.
>
> This initial driver supports on a subset of types of counter.
> It support counters that are either fixed or configurable, but requires
> that they support the ability to freeze and write value whilst frozen.
>
> Development done with QEMU model which will be posted shortly.
>
So the patch series is only tested with QEMU. Is there a real hardware
which supports the CXL PMON?
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> ---
> v4:
> - Add defines for masks used for config, config1 etc and reorder
> code so those can be next to the attrs. Note that one of the
> bits was wrong showing the advantage of bring all this info
> into one place.
> - Use enabled counter bitmap rather than pointer array to make the
> fixed counter handling look more like the variable counter handling.
> - Drop PERF_EF_RELOAD handling that did nothing.
> - Simplify code as nothing special to do if the counting is already
> up to date (no harm in setting the already set flag).
> ---
> drivers/cxl/Kconfig | 13 +
> drivers/cxl/Makefile | 1 +
> drivers/cxl/cpmu.c | 940 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 954 insertions(+)
>
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index ff4e78117b31..0be514b00b8f 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -139,4 +139,17 @@ config CXL_REGION_INVALIDATION_TEST
> If unsure, or if this kernel is meant for production environments,
> say N.
>
> +config CXL_CPMU
> + tristate "CXL Performance Monitoring Unit"
> + default CXL_BUS
> + depends on PERF_EVENTS
> + help
> + Support performance monitoring as defined in CXL rev 3.0
> + section 13.2: Performance Monitoring. CXL components may have
> + one or more CXL Performance Monitoring Units (CPMUs).
> +
> + Say 'y/m' to enable a driver that will attach to performance
> + monitoring units and provide standard perf based interfaces.
> +
> + If unsure say 'm'.
> endif
> diff --git a/drivers/cxl/Makefile b/drivers/cxl/Makefile
> index db321f48ba52..024bb739554b 100644
> --- a/drivers/cxl/Makefile
> +++ b/drivers/cxl/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_CXL_MEM) += cxl_mem.o
> obj-$(CONFIG_CXL_ACPI) += cxl_acpi.o
> obj-$(CONFIG_CXL_PMEM) += cxl_pmem.o
> obj-$(CONFIG_CXL_PORT) += cxl_port.o
> +obj-$(CONFIG_CXL_CPMU) += cpmu.o
>
> cxl_mem-y := mem.o
> cxl_pci-y := pci.o
> diff --git a/drivers/cxl/cpmu.c b/drivers/cxl/cpmu.c
> new file mode 100644
> index 000000000000..e6821771a4f6
> --- /dev/null
> +++ b/drivers/cxl/cpmu.c
> @@ -0,0 +1,940 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Copyright(c) 2023 Huawei
> + *
> + * The CXL 3.0 specification includes a standard Performance Monitoring Unit,
> + * called the CXL PMU, or CPMU. In order to allow a high degree of
> + * implementation flexibility the specification provides a wide range of
> + * options all of which are self describing.
> + *
> + * Details in CXL rev 3.0 section 8.2.7 CPMU Register Interface
> + *
> + * TODO
> + * o Discoverability of counters. Allow perftool to provide summed counters
> + * and vendor defined counters.
> + * o Support free running counters - copy the Intel uncore PMU handling for these.
> + * o CPMUs which do not support freeze.
> + * o Add filter validation in cpmu_event_init() so problems are detected earlier.
> + * o Reject configurations that the hardware is ignoring
> + * (e.g. invert when not invertible)
> + * o Support CPMUs with no interrupts using an HRTIMER.
> + */
> +
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/perf_event.h>
> +#include <linux/bitops.h>
> +#include <linux/device.h>
> +#include <linux/bits.h>
> +#include <linux/list.h>
> +#include <linux/bug.h>
> +#include <linux/pci.h>
> +
> +#include "cpmu.h"
> +#include "cxlpci.h"
> +#include "cxl.h"
> +
> +/* CXL rev 3.0 Table 13-5 Events under CXL Vendor ID */
> +#define CPMU_GID_CLOCK_TICKS 0x00
> +#define CPMU_GID_D2H_REQ 0x0010
> +#define CPMU_GID_D2H_RSP 0x0011
> +#define CPMU_GID_H2D_REQ 0x0012
> +#define CPMU_GID_H2D_RSP 0x0013
> +#define CPMU_GID_CACHE_DATA 0x0014
> +#define CPMU_GID_M2S_REQ 0x0020
> +#define CPMU_GID_M2S_RWD 0x0021
> +#define CPMU_GID_M2S_BIRSP 0x0022
> +#define CPMU_GID_S2M_BISNP 0x0023
> +#define CPMU_GID_S2M_NDR 0x0024
> +#define CPMU_GID_S2M_DRS 0x0025
> +#define CPMU_GID_DDR 0x8000
> +
> +static int cpmu_cpuhp_state_num;
> +
> +struct cpmu_event {
> + u16 vid;
> + u16 gid;
> + u32 msk;
> + union {
> + int counter_idx; /* fixed counters */
> + int event_idx; /* configurable counters */
> + };
> + struct list_head node;
> +};
> +
> +#define CPMU_MAX_COUNTERS 64
> +struct cpmu_info {
> + struct pmu pmu;
> + void __iomem *base;
> + struct perf_event **hw_events;
> + struct list_head events_configurable;
> + struct list_head events_fixed;
> + DECLARE_BITMAP(used_counter_bm, CPMU_MAX_COUNTERS);
> + DECLARE_BITMAP(conf_counter_bm, CPMU_MAX_COUNTERS);
> + u16 counter_width;
> + u8 num_counters;
> + u8 num_event_capabilities;
> + int on_cpu;
> + struct hlist_node node;
> + bool freeze_for_enable;
> + bool filter_hdm;
> + int irq;
> +};
> +
> +#define pmu_to_cpmu_info(_pmu) container_of(_pmu, struct cpmu_info, pmu)
> +
> +/*
> + * All CPMU counters are discoverable via the Event Capabilities Registers.
> + * Each Event Capability register contains a a VID / GroupID.
> + * A counter may then count any combination (by summing) of events in
> + * that group which are in the Supported Events Bitmask.
> + * However, there are some complexities to the scheme.
> + * - Fixed function counters refer to an Event Capabilities register.
> + * That event capability register is not then used for Configurable
> + * counters.
> + */
> +static int cpmu_parse_caps(struct device *dev, struct cpmu_info *info)
> +{
> + DECLARE_BITMAP(fixed_counter_event_cap_bm, 32) = {0};
> + void __iomem *base = info->base;
> + u64 val, eval;
> + int i;
> +
> + val = readq(base + CPMU_CAP_REG);
> + info->freeze_for_enable = FIELD_GET(CPMU_CAP_WRITEABLE_WHEN_FROZEN, val) &
> + FIELD_GET(CPMU_CAP_FREEZE, val);
> + if (!info->freeze_for_enable) {
> + dev_err(dev, "Driver does not support CPMUs that do not support freeze for enable\n");
> + return -ENODEV;
> + }
> +
> + info->num_counters = FIELD_GET(CPMU_CAP_NUM_COUNTERS_MSK, val) + 1;
> + info->counter_width = FIELD_GET(CPMU_CAP_COUNTER_WIDTH_MSK, val);
> + info->num_event_capabilities = FIELD_GET(CPMU_CAP_NUM_EVN_CAP_REG_SUP_MSK, val) + 1;
> +
> + info->filter_hdm = FIELD_GET(CPMU_CAP_FILTERS_SUP_MSK, val) & CPMU_FILTER_HDM;
> + if (FIELD_GET(CPMU_CAP_INT, val))
> + info->irq = FIELD_GET(CPMU_CAP_MSI_N_MSK, val);
> + else
> + info->irq = -1;
> +
> + /* First handle fixed function counters; note if configurable counters found */
> + for (i = 0; i < info->num_counters; i++) {
> + struct cpmu_event *cpmu_ev;
> + u32 events_msk;
> + u8 group_idx;
> +
> + val = readq(base + CPMU_COUNTER_CFG_REG(i));
> +
> + if (FIELD_GET(CPMU_COUNTER_CFG_TYPE_MSK, val) ==
> + CPMU_COUNTER_CFG_TYPE_CONFIGURABLE) {
> + set_bit(i, info->conf_counter_bm);
> + }
> +
> + if (FIELD_GET(CPMU_COUNTER_CFG_TYPE_MSK, val) !=
> + CPMU_COUNTER_CFG_TYPE_FIXED_FUN)
> + continue;
> +
> + /* In this case we know which fields are const */
> + group_idx = FIELD_GET(CPMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK, val);
> + events_msk = FIELD_GET(CPMU_COUNTER_CFG_EVENTS_MSK, val);
> + eval = readq(base + CPMU_EVENT_CAP_REG(group_idx));
> + cpmu_ev = devm_kzalloc(dev, sizeof(*cpmu_ev), GFP_KERNEL);
> + if (!cpmu_ev)
> + return -ENOMEM;
> +
> + cpmu_ev->vid = FIELD_GET(CPMU_EVENT_CAP_VENDOR_ID_MSK, eval);
> + cpmu_ev->gid = FIELD_GET(CPMU_EVENT_CAP_GROUP_ID_MSK, eval);
> + /* For a fixed purpose counter use the events mask from the counter CFG */
> + cpmu_ev->msk = events_msk;
> + cpmu_ev->counter_idx = i;
> + /* This list add is never unwound as all entries deleted on remove */
> + list_add(&cpmu_ev->node, &info->events_fixed);
> + /*
> + * Configurable counters must not use an Event Capability registers that
> + * is in use for a Fixed counter
> + */
> + set_bit(group_idx, fixed_counter_event_cap_bm);
> + }
> +
> + if (!bitmap_empty(info->conf_counter_bm, CPMU_MAX_COUNTERS)) {
> + struct cpmu_event *cpmu_ev;
> + int j;
> + /* Walk event capabilities unused by fixed counters */
> + for_each_clear_bit(j, fixed_counter_event_cap_bm,
> + info->num_event_capabilities) {
> + cpmu_ev = devm_kzalloc(dev, sizeof(*cpmu_ev), GFP_KERNEL);
> + if (!cpmu_ev)
> + return -ENOMEM;
> +
> + eval = readq(base + CPMU_EVENT_CAP_REG(j));
> + cpmu_ev->vid = FIELD_GET(CPMU_EVENT_CAP_VENDOR_ID_MSK, eval);
> + cpmu_ev->gid = FIELD_GET(CPMU_EVENT_CAP_GROUP_ID_MSK, eval);
> + cpmu_ev->msk = FIELD_GET(CPMU_EVENT_CAP_SUPPORTED_EVENTS_MSK, eval);
> + cpmu_ev->event_idx = j;
> + list_add(&cpmu_ev->node, &info->events_configurable);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static ssize_t cpmu_format_sysfs_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 sysfs_emit(buf, "%s\n", (char *)eattr->var);
> +}
> +
> +#define CPMU_FORMAT_ATTR(_name, _format)\
> + (&((struct dev_ext_attribute[]) { \
> + { \
> + .attr = __ATTR(_name, 0444, \
> + cpmu_format_sysfs_show, NULL), \
> + .var = (void *)_format \
> + } \
> + })[0].attr.attr)
> +
> +enum {
> + cpmu_mask_attr,
> + cpmu_gid_attr,
> + cpmu_vid_attr,
> + cpmu_threshold_attr,
> + cpmu_invert_attr,
> + cpmu_edge_attr,
> + cpmu_hdm_filter_en_attr,
> + cpmu_hdm_attr,
> +};
> +
> +static struct attribute *cpmu_format_attr[] = {
> + [cpmu_mask_attr] = CPMU_FORMAT_ATTR(mask, "config:0-31"),
> + [cpmu_gid_attr] = CPMU_FORMAT_ATTR(gid, "config:32-47"),
> + [cpmu_vid_attr] = CPMU_FORMAT_ATTR(vid, "config:48-63"),
> + [cpmu_threshold_attr] = CPMU_FORMAT_ATTR(threshold, "config1:0-15"),
> + [cpmu_invert_attr] = CPMU_FORMAT_ATTR(invert, "config1:16"),
> + [cpmu_edge_attr] = CPMU_FORMAT_ATTR(edge, "config1:17"),
> + [cpmu_hdm_filter_en_attr] = CPMU_FORMAT_ATTR(hdm_filter_en, "config1:18"),
> + [cpmu_hdm_attr] = CPMU_FORMAT_ATTR(hdm, "config2:0-15"),
> + NULL
> +};
> +
> +#define CPMU_ATTR_CONFIG_MASK_MSK GENMASK_ULL(63, 48)
(31, 0)?
> +#define CPMU_ATTR_CONFIG_GID_MSK GENMASK_ULL(47, 32)
> +#define CPMU_ATTR_CONFIG_VID_MSK GENMASK_ULL(63, 48)
> +#define CPMU_ATTR_CONFIG1_THRESHOLD_MSK GENMASK_ULL(15, 0)
> +#define CPMU_ATTR_CONFIG1_INVERT_MSK BIT(16)
> +#define CPMU_ATTR_CONFIG1_EDGE_MSK BIT(17)
> +#define CPMU_ATTR_CONFIG1_FILTER_EN_MSK BIT(18)
> +#define CPMU_ATTR_CONFIG2_HDM_MSK GENMASK(15, 0)
> +
> +static umode_t cpmu_format_is_visible(struct kobject *kobj, struct attribute *attr, int a)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct cpmu_info *info = dev_get_drvdata(dev);
> +
> + /*
> + * Filter capability at the CPMU level, so hide the attributes if the particular
> + * filter is not supported.
> + */
> + if (attr == cpmu_format_attr[cpmu_hdm_filter_en_attr] ||
> + attr == cpmu_format_attr[cpmu_hdm_attr]) {
> + if (info->filter_hdm)
> + return 0444;
> + else
> + return 0;
> + } else {
> + return 0444;
> + }
> +}
> +
> +static const struct attribute_group cpmu_format_group = {
> + .name = "format",
> + .attrs = cpmu_format_attr,
> + .is_visible = cpmu_format_is_visible,
> +};
> +
> +static u32 cpmu_config_get_mask(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG_MASK_MSK, event->attr.config);
> +}
> +
> +static u16 cpmu_config_get_gid(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG_GID_MSK, event->attr.config);
> +}
> +
> +static u16 cpmu_config_get_vid(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG_VID_MSK, event->attr.config);
> +}
> +
> +static u8 cpmu_config1_get_threshold(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG1_THRESHOLD_MSK, event->attr.config1);
> +}
> +
> +static bool cpmu_config1_get_invert(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG1_INVERT_MSK, event->attr.config1);
> +}
> +
> +static bool cpmu_config1_get_edge(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG1_EDGE_MSK, event->attr.config1);
> +}
> +
> +/*
> + * CPMU specification allows for 8 filters, each with a 16 bit value...
> + * So we need to find 8x16bits to store it in.
> + * As the value used for disable is 0xffff, a separate enable switch
> + * is needed.
> + */
> +
> +static bool cpmu_config1_hdm_filter_en(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG1_FILTER_EN_MSK, event->attr.config1);
> +}
> +
> +static u16 cpmu_config2_get_hdm_decoder(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG2_HDM_MSK, event->attr.config2);
> +}
> +
> +static ssize_t cpmu_event_sysfs_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct perf_pmu_events_attr *pmu_attr =
> + container_of(attr, struct perf_pmu_events_attr, attr);
> +
> + return sysfs_emit(buf, "config=%#llx\n", pmu_attr->id);
> +}
> +
> +#define CPMU_PMU_EVENT_ATTR(_name, _vid, _gid, _msk) \
> + PMU_EVENT_ATTR_ID(_name, cpmu_event_sysfs_show, \
> + ((u64)(_vid) << 48) | ((u64)(_gid) << 32) | (u64)(_msk))
> +
> +/* For CXL spec defined events */
> +#define CPMU_PMU_EVENT_CXL_ATTR(_name, _gid, _msk) \
> + CPMU_PMU_EVENT_ATTR(_name, PCI_DVSEC_VENDOR_ID_CXL, _gid, _msk)
> +
> +static struct attribute *cpmu_event_attrs[] = {
> + CPMU_PMU_EVENT_CXL_ATTR(clock_ticks, CPMU_GID_CLOCK_TICKS, BIT(0)),
> + /* CXL rev 3.0 Table 3-17 - Device to Host Requests */
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_rdcurr, CPMU_GID_D2H_REQ, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_rdown, CPMU_GID_D2H_REQ, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_rdshared, CPMU_GID_D2H_REQ, BIT(3)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_rdany, CPMU_GID_D2H_REQ, BIT(4)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_rdownnodata, CPMU_GID_D2H_REQ, BIT(5)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_itomwr, CPMU_GID_D2H_REQ, BIT(6)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_wrcurr, CPMU_GID_D2H_REQ, BIT(7)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_clflush, CPMU_GID_D2H_REQ, BIT(8)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_cleanevict, CPMU_GID_D2H_REQ, BIT(9)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_dirtyevict, CPMU_GID_D2H_REQ, BIT(10)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_cleanevictnodata, CPMU_GID_D2H_REQ, BIT(11)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_wowrinv, CPMU_GID_D2H_REQ, BIT(12)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_wowrinvf, CPMU_GID_D2H_REQ, BIT(13)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_wrinv, CPMU_GID_D2H_REQ, BIT(14)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_cacheflushed, CPMU_GID_D2H_REQ, BIT(16)),
> + /* CXL rev 3.0 Table 3-20 - D2H Repsonse Encodings */
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspihiti, CPMU_GID_D2H_RSP, BIT(4)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspvhitv, CPMU_GID_D2H_RSP, BIT(6)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspihitse, CPMU_GID_D2H_RSP, BIT(5)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspshitse, CPMU_GID_D2H_RSP, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspsfwdm, CPMU_GID_D2H_RSP, BIT(7)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspifwdm, CPMU_GID_D2H_RSP, BIT(15)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspvfwdv, CPMU_GID_D2H_RSP, BIT(22)),
> + /* CXL rev 3.0 Table 3-21 - CXL.cache - Mapping of H2D Requests to D2H Responses */
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_req_snpdata, CPMU_GID_H2D_REQ, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_req_snpinv, CPMU_GID_H2D_REQ, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_req_snpcur, CPMU_GID_H2D_REQ, BIT(3)),
> + /* CXL rev 3.0 Table 3-22 - H2D Response Opcode Encodings */
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_writepull, CPMU_GID_H2D_RSP, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_go, CPMU_GID_H2D_RSP, BIT(4)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_gowritepull, CPMU_GID_H2D_RSP, BIT(5)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_extcmp, CPMU_GID_H2D_RSP, BIT(6)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_gowritepulldrop, CPMU_GID_H2D_RSP, BIT(8)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_fastgowritepull, CPMU_GID_H2D_RSP, BIT(13)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_goerrwritepull, CPMU_GID_H2D_RSP, BIT(15)),
> + /* CXL rev 3.0 Table 13-5 directly lists these */
> + CPMU_PMU_EVENT_CXL_ATTR(cachedata_d2h_data, CPMU_GID_CACHE_DATA, BIT(0)),
> + CPMU_PMU_EVENT_CXL_ATTR(cachedata_h2d_data, CPMU_GID_CACHE_DATA, BIT(1)),
> + /* CXL rev 3.0 Table 3-29 M2S Req Memory Opcodes */
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_meminv, CPMU_GID_M2S_REQ, BIT(0)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memrd, CPMU_GID_M2S_REQ, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memrddata, CPMU_GID_M2S_REQ, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memrdfwd, CPMU_GID_M2S_REQ, BIT(3)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memwrfwd, CPMU_GID_M2S_REQ, BIT(4)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memspecrd, CPMU_GID_M2S_REQ, BIT(8)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_meminvnt, CPMU_GID_M2S_REQ, BIT(9)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memcleanevict, CPMU_GID_M2S_REQ, BIT(10)),
> + /* CXL rev 3.0 Table 3-35 M2S RwD Memory Opcodes */
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_rwd_memwr, CPMU_GID_M2S_RWD, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_rwd_memwrptl, CPMU_GID_M2S_RWD, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_rwd_biconflict, CPMU_GID_M2S_RWD, BIT(4)),
> + /* CXL rev 3.0 Table 3-38 M2S BIRsp Memory Opcodes */
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_i, CPMU_GID_M2S_BIRSP, BIT(0)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_s, CPMU_GID_M2S_BIRSP, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_e, CPMU_GID_M2S_BIRSP, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_iblk, CPMU_GID_M2S_BIRSP, BIT(4)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_sblk, CPMU_GID_M2S_BIRSP, BIT(5)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_eblk, CPMU_GID_M2S_BIRSP, BIT(6)),
> + /* CXL rev 3.0 Table 3-40 S2M BISnp Opcodes */
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_cur, CPMU_GID_S2M_BISNP, BIT(0)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_data, CPMU_GID_S2M_BISNP, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_inv, CPMU_GID_S2M_BISNP, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_curblk, CPMU_GID_S2M_BISNP, BIT(4)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_datblk, CPMU_GID_S2M_BISNP, BIT(5)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_invblk, CPMU_GID_S2M_BISNP, BIT(6)),
> + /* CXL rev 3.0 Table 3-43 S2M NDR Opcopdes */
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_ndr_cmp, CPMU_GID_S2M_NDR, BIT(0)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_ndr_cmps, CPMU_GID_S2M_NDR, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_ndr_cmpe, CPMU_GID_S2M_NDR, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_ndr_biconflictack, CPMU_GID_S2M_NDR, BIT(3)),
> + /* CXL rev 3.0 Table 3-46 S2M DRS opcodes */
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_drs_memdata, CPMU_GID_S2M_DRS, BIT(0)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_drs_memdatanxm, CPMU_GID_S2M_DRS, BIT(1)),
> + /* CXL rev 3.0 Table 13-5 directly lists these */
> + CPMU_PMU_EVENT_CXL_ATTR(ddr_act, CPMU_GID_DDR, BIT(0)),
> + CPMU_PMU_EVENT_CXL_ATTR(ddr_pre, CPMU_GID_DDR, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(ddr_casrd, CPMU_GID_DDR, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(ddr_caswr, CPMU_GID_DDR, BIT(3)),
> + CPMU_PMU_EVENT_CXL_ATTR(ddr_refresh, CPMU_GID_DDR, BIT(4)),
> + CPMU_PMU_EVENT_CXL_ATTR(ddr_selfrefreshent, CPMU_GID_DDR, BIT(5)),
> + CPMU_PMU_EVENT_CXL_ATTR(ddr_rfm, CPMU_GID_DDR, BIT(6)),
> + NULL
> +};
> +
> +static struct cpmu_event *cpmu_find_fixed_counter_event(struct cpmu_info *info,
> + int vid, int gid, int msk)
> +{
> + struct cpmu_event *cpmu_ev;
> +
> + list_for_each_entry(cpmu_ev, &info->events_fixed, node) {
> + if (vid != cpmu_ev->vid || gid != cpmu_ev->gid)
> + continue;
> +
> + /* Precise match for fixed counter */
> + if (msk == cpmu_ev->msk)
> + return cpmu_ev;
> + }
> +
> + return ERR_PTR(-EINVAL);
> +}
> +
> +static struct cpmu_event *cpmu_find_config_counter_event(struct cpmu_info *info,
> + int vid, int gid, int msk)
> +{
> + struct cpmu_event *cpmu_ev;
> +
> + list_for_each_entry(cpmu_ev, &info->events_configurable, node) {
> + if (vid != cpmu_ev->vid || gid != cpmu_ev->gid)
> + continue;
> +
> + /* Request mask must be subset of supported */
> + if (msk & ~cpmu_ev->msk)
> + continue;
> +
> + return cpmu_ev;
> + }
> +
> + return ERR_PTR(-EINVAL);
> +}
> +
> +static umode_t cpmu_event_is_visible(struct kobject *kobj, struct attribute *attr, int a)
> +{
> + struct device_attribute *dev_attr = container_of(attr, struct device_attribute, attr);
> + struct perf_pmu_events_attr *pmu_attr =
> + container_of(dev_attr, struct perf_pmu_events_attr, attr);
> + struct device *dev = kobj_to_dev(kobj);
> + struct cpmu_info *info = dev_get_drvdata(dev);
> + int vid = FIELD_GET(CPMU_ATTR_CONFIG_VID_MSK, pmu_attr->id);
> + int gid = FIELD_GET(CPMU_ATTR_CONFIG_GID_MSK, pmu_attr->id);
> + int msk = FIELD_GET(CPMU_ATTR_CONFIG_MASK_MSK, pmu_attr->id);
> +
> + if (!IS_ERR(cpmu_find_fixed_counter_event(info, vid, gid, msk)))
> + return attr->mode;
> +
> + if (!IS_ERR(cpmu_find_config_counter_event(info, vid, gid, msk)))
> + return attr->mode;
> +
> + return 0;
> +}
> +
> +static const struct attribute_group cpmu_events = {
> + .name = "events",
> + .attrs = cpmu_event_attrs,
> + .is_visible = cpmu_event_is_visible,
> +};
> +
> +static ssize_t cpumask_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct cpmu_info *info = dev_get_drvdata(dev);
> +
> + return cpumap_print_to_pagebuf(true, buf, cpumask_of(info->on_cpu));
> +}
> +static DEVICE_ATTR_RO(cpumask);
> +
> +static struct attribute *cpmu_cpumask_attrs[] = {
> + &dev_attr_cpumask.attr,
> + NULL
> +};
> +
> +static const struct attribute_group cpmu_cpumask_group = {
> + .attrs = cpmu_cpumask_attrs,
> +};
> +
> +static const struct attribute_group *cpmu_attr_groups[] = {
> + &cpmu_events,
> + &cpmu_format_group,
> + &cpmu_cpumask_group,
> + NULL
> +};
> +
> +/* If counter_idx == NULL, don't try to allocate a counter. */
> +static int cpmu_get_event_idx(struct perf_event *event, int *counter_idx, int *event_idx)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> + DECLARE_BITMAP(configurable_and_free, CPMU_MAX_COUNTERS);
> + struct cpmu_event *cpmu_ev;
> + u32 mask;
> + u16 gid, vid;
> + int i;
> +
> + vid = cpmu_config_get_vid(event);
> + gid = cpmu_config_get_gid(event);
> + mask = cpmu_config_get_mask(event);
> +
> + cpmu_ev = cpmu_find_fixed_counter_event(info, vid, gid, mask);
> + if (!IS_ERR(cpmu_ev)) {
> + if (!counter_idx)
> + return 0;
> + if (!test_bit(cpmu_ev->counter_idx, info->used_counter_bm)) {
> + *counter_idx = cpmu_ev->counter_idx;
> + return 0;
> + }
> + /* Fixed counter is in use, but maybe a configurable one? */
> + }
> +
> + cpmu_ev = cpmu_find_config_counter_event(info, vid, gid, mask);
> + if (!IS_ERR(cpmu_ev)) {
> + if (!counter_idx)
> + return 0;
> +
> + bitmap_andnot(configurable_and_free, info->conf_counter_bm,
> + info->used_counter_bm, CPMU_MAX_COUNTERS);
> +
> + i = find_first_bit(configurable_and_free, CPMU_MAX_COUNTERS);
> + if (i == CPMU_MAX_COUNTERS)
> + return -EINVAL;
> +
> + *counter_idx = i;
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int cpmu_event_init(struct perf_event *event)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> +
> + event->cpu = info->on_cpu;
> + /* Top level type sanity check - is this a Hardware Event being requested */
> + if (event->attr.type != event->pmu->type)
> + return -ENOENT;
> +
> + if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
> + return -EOPNOTSUPP;
> + /* TODO: Validation of any filter */
> +
> + /*
> + * Verify that it is possible to count what was requested. Either must
> + * be a fixed counter that is a precise match or a configurable counter
> + * where this is a subset.
> + */
> + return cpmu_get_event_idx(event, NULL, NULL);
> +}
> +
> +static void cpmu_pmu_enable(struct pmu *pmu)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(pmu);
> + void __iomem *base = info->base;
> +
> + /* We don't have a global enable, but we 'might' have a global freeze which we can use */
> + if (info->freeze_for_enable) {
> + /* Can assume frozen at this stage */
> + writeq(0, base + CPMU_FREEZE_REG);
> +
> + return;
> + }
> +}
> +
> +static void cpmu_pmu_disable(struct pmu *pmu)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(pmu);
> + void __iomem *base = info->base;
> +
> + if (info->freeze_for_enable) {
> + /*
> + * Whilst bits above number of counters are RsvdZ
> + * they are unlikely to be repurposed given
> + * number of counters is allowed to be 64 leaving
> + * no reserved bits. Hence this is only slightly
> + * naughty.
> + */
> + writeq(GENMASK_ULL(63, 0), base + CPMU_FREEZE_REG);
> + return;
> + }
> +}
> +
> +static void cpmu_event_start(struct perf_event *event, int flags)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> + void __iomem *base = info->base;
> + u64 cfg;
> +
> + if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
> + return;
> +
> + WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
> + hwc->state = 0;
> +
> + /*
> + * Currently only hdm filter control is implemnted, this code will
> + * want generalizing when more filters are added.
> + */
> + if (info->filter_hdm) {
> + if (cpmu_config1_hdm_filter_en(event))
> + cfg = cpmu_config2_get_hdm_decoder(event);
> + else
> + cfg = GENMASK(15, 0); /* No filtering if 0xFFFF_FFFF */
> + writeq(cfg, base + CPMU_FILTER_CFG_REG(hwc->idx, 0));
> + }
> +
> + cfg = readq(base + CPMU_COUNTER_CFG_REG(hwc->idx));
I don't think we need the previous value. Just overwrite it.
> + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_INT_ON_OVRFLW, 1);
> + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_ENABLE, 1);
> + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_EDGE, cpmu_config1_get_edge(event) ? 1 : 0);
> + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_INVERT, cpmu_config1_get_invert(event) ? 1 : 0);
> +
> + /* Fixed purpose counters have next two fields RO */
> + if (test_bit(hwc->idx, info->conf_counter_bm)) {
> + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK, hwc->event_base);
> + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_EVENTS_MSK, cpmu_config_get_mask(event));
> + }
> + cfg &= ~CPMU_COUNTER_CFG_THRESHOLD_MSK;
> + /*
> + * For events that generate only 1 count per clock the CXL 3.0 spec
> + * states the threshold shall be set to 1 but if set to 0 it will
> + * count the raw value anwyay?
> + * There is no definition of what events will count multiple per cycle
> + * and hence to which non 1 values of threshold can apply.
> + * (CXL 3.0 8.2.7.2.1 Counter Configuration - threshold field definition)
> + */
> + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_THRESHOLD_MSK,
> + cpmu_config1_get_threshold(event));
> + writeq(cfg, base + CPMU_COUNTER_CFG_REG(hwc->idx));
> +
> + local64_set(&hwc->prev_count, 0);
> + writeq(0, base + CPMU_COUNTER_REG(hwc->idx));
The counter is reset twice. The other is in add()->cpmu_reset_counter().
I think you can remove the one in add().
> +
> + perf_event_update_userpage(event);
> +}
> +
> +static u64 cpmu_read_counter(struct perf_event *event)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> + void __iomem *base = info->base;
> +
> + return readq(base + CPMU_COUNTER_REG(event->hw.idx));
> +}
> +
> +static void __cpmu_read(struct perf_event *event, bool overflow)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> + u64 new_cnt, prev_cnt, delta;
> +
> + do {
> + prev_cnt = local64_read(&hwc->prev_count);
> + new_cnt = cpmu_read_counter(event);
> + } while (local64_cmpxchg(&hwc->prev_count, prev_cnt, new_cnt) != prev_cnt);
> +
> + /*
> + * If we know an overflow occur then take that into account.
> + * Note counter is not reset as that would lose events
> + */
> + delta = (new_cnt - prev_cnt) & GENMASK_ULL(info->counter_width - 1, 0);
> + if (overflow && delta < GENMASK_ULL(info->counter_width - 1, 0))
> + delta += (1UL << info->counter_width);
> +
> + local64_add(delta, &event->count);
> +}
> +
> +static void cpmu_read(struct perf_event *event)
> +{
> + __cpmu_read(event, false);
> +}
> +
> +static void cpmu_event_stop(struct perf_event *event, int flags)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> + void __iomem *base = info->base;
> + struct hw_perf_event *hwc = &event->hw;
> + u64 cfg;
> +
> + cpmu_read(event);
> + WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
> + hwc->state |= PERF_HES_STOPPED;
> +
> + cfg = readq(base + CPMU_COUNTER_CFG_REG(hwc->idx));
> + cfg &= ~(FIELD_PREP(CPMU_COUNTER_CFG_INT_ON_OVRFLW, 1) |
> + FIELD_PREP(CPMU_COUNTER_CFG_ENABLE, 1));
> + writeq(cfg, base + CPMU_COUNTER_CFG_REG(hwc->idx));
> +
> + hwc->state |= PERF_HES_UPTODATE;
> +}
> +
> +/*
> + * Reset ensures no possibility of any information leaking to wrong
> + * counter. Note that all fields written during start()
> + */
> +static void cpmu_reset_counter(struct cpmu_info *info, int idx)
> +{
> + void __iomem *base = info->base;
> +
> + /* Much of this register is read only */
> + writeq(0, base + CPMU_EVENT_CAP_REG(idx));
CPMU_COUNTER_CFG_REG?
I don't think we need to reset the config here, since it will be rewrite
in cpmu_event_start().
> + /* Filters are not per counter, so no reset here */
> + writeq(0, base + CPMU_COUNTER_REG(idx));
> +}
> +
> +static int cpmu_event_add(struct perf_event *event, int flags)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> + int idx, rc;
> + int event_idx = 0;
> +
> + hwc->state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
> +
> + rc = cpmu_get_event_idx(event, &idx, &event_idx);
> + if (rc < 0)
> + return rc;
> +
> + hwc->idx = idx;
> +
> + /* Only set for configurable counters */
> + hwc->event_base = event_idx;
> + info->hw_events[idx] = event;
> + set_bit(idx, info->used_counter_bm);
> +
> + cpmu_reset_counter(info, idx);
> +
> + if (flags & PERF_EF_START)
> + cpmu_event_start(event, PERF_EF_RELOAD);
> +
> + return 0;
> +}
> +
> +static void cpmu_event_del(struct perf_event *event, int flags)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> +
> + cpmu_event_stop(event, PERF_EF_UPDATE);
> + clear_bit(hwc->idx, info->used_counter_bm);
> + info->hw_events[hwc->idx] = NULL;
> + perf_event_update_userpage(event);
> +}
> +
> +static irqreturn_t cpmu_irq(int irq, void *data)
> +{
> + struct cpmu_info *info = data;
> + void __iomem *base = info->base;
> + u64 overflowed;
> + DECLARE_BITMAP(overflowedbm, 64);
> + int i;
> +
> + overflowed = readq(base + CPMU_OVERFLOW_REG);
> +
> + /* Interrupt may be shared, so maybe it isn't ours */
> + if (!overflowed)
> + return IRQ_NONE;
> +
> + bitmap_from_arr64(overflowedbm, &overflowed, 64);
> + for_each_set_bit(i, overflowedbm, info->num_counters) {
> + struct perf_event *event = info->hw_events[i];
> +
> + if (!event) {
> + dev_dbg(info->pmu.dev,
> + "overflow but on non enabled counter %d\n", i);
> + continue;
> + }
> +
> + __cpmu_read(event, true);
> + }
> +
> + writeq(overflowed, base + CPMU_OVERFLOW_REG);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int cxl_cpmu_probe(struct device *dev)
> +{
> + struct cxl_cpmu *cpmu = to_cxl_cpmu(dev);
> + struct pci_dev *pdev = to_pci_dev(dev->parent);
> + struct cpmu_info *info;
> + char *irq_name;
> + int rc, irq;
> +
> + info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + INIT_LIST_HEAD(&info->events_fixed);
> + INIT_LIST_HEAD(&info->events_configurable);
> +
> + info->base = cpmu->base;
> +
> + info->on_cpu = -1;
> + rc = cpmu_parse_caps(dev, info);
> + if (rc)
> + return rc;
> +
> + info->hw_events = devm_kcalloc(dev, sizeof(*info->hw_events),
> + info->num_counters, GFP_KERNEL);
> + if (!info->hw_events)
> + return -ENOMEM;
> +
> + info->pmu = (struct pmu) {
> + .name = dev_name(dev),
> + .parent = dev,
I don't see the PMU parent is used anywhere.
Please remove this and drop the Patch 2 from this series.
If there will be other followup patches, please add the PMU parent
support there.
Thanks,
Kan
> + .module = THIS_MODULE,
> + .event_init = cpmu_event_init,
> + .pmu_enable = cpmu_pmu_enable,
> + .pmu_disable = cpmu_pmu_disable,
> + .add = cpmu_event_add,
> + .del = cpmu_event_del,
> + .start = cpmu_event_start,
> + .stop = cpmu_event_stop,
> + .read = cpmu_read,
> + .task_ctx_nr = perf_invalid_context,
> + .attr_groups = cpmu_attr_groups,
> + .capabilities = PERF_PMU_CAP_NO_EXCLUDE,
> + };
> +
> + if (info->irq <= 0)
> + return -EINVAL;
> +
> + rc = pci_irq_vector(pdev, info->irq);
> + if (rc < 0)
> + return rc;
> + irq = rc;
> +
> + irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s_overflow\n", dev_name(dev));
> + if (!irq_name)
> + return -ENOMEM;
> +
> + rc = devm_request_irq(dev, irq, cpmu_irq, IRQF_SHARED, irq_name, info);
> + if (rc)
> + return rc;
> + info->irq = irq;
> +
> + rc = cpuhp_state_add_instance(cpmu_cpuhp_state_num, &info->node);
> + if (rc)
> + return rc;
> +
> + rc = perf_pmu_register(&info->pmu, info->pmu.name, -1);
> + if (rc)
> + return rc;
> +
> + dev_set_drvdata(dev, info);
> +
> + return 0;
> +}
> +
> +static void cxl_cpmu_remove(struct device *dev)
> +{
> + struct cpmu_info *info = dev_get_drvdata(dev);
> +
> + perf_pmu_unregister(&info->pmu);
> + cpuhp_state_remove_instance_nocalls(cpmu_cpuhp_state_num, &info->node);
> +}
> +
> +static struct cxl_driver cxl_cpmu_driver = {
> + .name = "cxl_cpmu",
> + .probe = cxl_cpmu_probe,
> + .remove = cxl_cpmu_remove,
> + .id = CXL_DEVICE_CPMU,
> +};
> +
> +static int cpmu_online_cpu(unsigned int cpu, struct hlist_node *node)
> +{
> + struct cpmu_info *info = hlist_entry_safe(node, struct cpmu_info, node);
> +
> + if (info->on_cpu != -1)
> + return 0;
> +
> + info->on_cpu = cpu;
> + WARN_ON(irq_set_affinity(info->irq, cpumask_of(cpu)));
> +
> + return 0;
> +}
> +
> +static int cpmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> +{
> + struct cpmu_info *info = hlist_entry_safe(node, struct cpmu_info, node);
> + unsigned int target;
> +
> + if (info->on_cpu != cpu)
> + return 0;
> +
> + info->on_cpu = -1;
> + target = cpumask_first(cpu_online_mask);
> + if (target >= nr_cpu_ids) {
> + dev_err(info->pmu.dev, "Unable to find a suitable CPU\n");
> + return 0;
> + }
> +
> + perf_pmu_migrate_context(&info->pmu, cpu, target);
> + info->on_cpu = target;
> + WARN_ON(irq_set_affinity(info->irq, cpumask_of(target)));
> +
> + return 0;
> +}
> +
> +static __init int cxl_cpmu_init(void)
> +{
> + int rc;
> +
> + rc = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
> + "AP_PERF_CPMU_ONLINE",
> + cpmu_online_cpu, cpmu_offline_cpu);
> + if (rc < 0)
> + return rc;
> + cpmu_cpuhp_state_num = rc;
> +
> + rc = cxl_driver_register(&cxl_cpmu_driver);
> + if (rc)
> + cpuhp_remove_multi_state(cpmu_cpuhp_state_num);
> +
> + return rc;
> +}
> +
> +static __exit void cxl_cpmu_exit(void)
> +{
> + cxl_driver_unregister(&cxl_cpmu_driver);
> + cpuhp_remove_multi_state(cpmu_cpuhp_state_num);
> +}
> +
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(CXL);
> +module_init(cxl_cpmu_init);
> +module_exit(cxl_cpmu_exit);
> +MODULE_ALIAS_CXL(CXL_DEVICE_CPMU);
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 5/5] docs: perf: Minimal introduction the the CXL PMU device and driver
2023-03-30 16:45 ` [PATCH v4 5/5] docs: perf: Minimal introduction the the CXL PMU device and driver Jonathan Cameron
@ 2023-04-03 17:45 ` Liang, Kan
2023-04-04 16:55 ` Jonathan Cameron
2023-04-04 22:24 ` Dan Williams
1 sibling, 1 reply; 21+ messages in thread
From: Liang, Kan @ 2023-04-03 17:45 UTC (permalink / raw)
To: Jonathan Cameron, linux-cxl, peterz
Cc: mingo, acme, mark.rutland, will, dan.j.williams, linuxarm,
linux-perf-users, linux-kernel, Davidlohr Bueso, Dave Jiang
On 2023-03-30 12:45 p.m., Jonathan Cameron wrote:
> Very basic introduction to the device and the current driver support
> provided. I expect to expand on this in future versions of this patch
> set.
>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> --
> v4: No change
> ---
> Documentation/admin-guide/perf/cxl.rst | 65 ++++++++++++++++++++++++
> Documentation/admin-guide/perf/index.rst | 1 +
> 2 files changed, 66 insertions(+)
>
> diff --git a/Documentation/admin-guide/perf/cxl.rst b/Documentation/admin-guide/perf/cxl.rst
> new file mode 100644
> index 000000000000..46235dff4b21
> --- /dev/null
> +++ b/Documentation/admin-guide/perf/cxl.rst
> @@ -0,0 +1,65 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +======================================
> +CXL Performance Monitoring Unit (CPMU)
> +======================================
> +
> +The CXL rev 3.0 specification provides a definition of CXL Performance
> +Monitoring Unit in section 13.2: Performance Monitoring.
> +
> +CXL components (e.g. Root Port, Switch Upstream Port, End Point) may have
> +any number of CPMU instances. CPMU capabilities are fully discoverable from
> +the devices. The specification provides event definitions for all CXL protocol
> +message types and a set of additional events for things commonly counted on
> +CXL devices (e.g. DRAM events).
> +
> +CPMU driver
> +===========
> +
> +The CPMU driver register a perf PMU with the name cpmu<id> on the CXL bus.
> +
> + /sys/bus/cxl/device/cpmu<id>
> +
> +The associated PMU is registered as
> +
> + /sys/bus/event_sources/devices/cpmu<id>
> +
> +In common with other CXL bus devices, the id has no specific meaning and the
> +relationship to specific CXL device should be established via the device parent
> +of the device on the CXL bus.
> +
> +PMU driver provides description of available events and filter options in sysfs.
> +
> +The "format" directory describes all formats of the config (event vendor id,
> +group id and mask) config1 (threshold, filter enables) and config2 (filter
> +parameters) fields of the perf_event_attr structure. The "events" directory
> +describes all documented events show in perf list.
> +
> +The events shown in perf list are the most fine grained events with a single
> +bit of the event mask set. More general events may be enable by setting
> +multiple mask bits in config. For example, all Device to Host Read Requests
> +may be captured on a single counter by setting the bits for all of
> +
> +* d2h_req_rdcurr
> +* d2h_req_rdown
> +* d2h_req_rdshared
> +* d2h_req_rdany
> +* d2h_req_rdownnodata
> +
> +Example of usage::
> +
> + $#perf list
> + cpmu0/clock_ticks/ [Kernel PMU event]
> + cpmu0/d2h_req_itomwr/ [Kernel PMU event]
> + cpmu0/d2h_req_rdany/ [Kernel PMU event]
> + cpmu0/d2h_req_rdcurr/ [Kernel PMU event]
> + -----------------------------------------------------------
> +
> + $# perf stat -e cpmu0/clock_ticks/ -e cpmu0/d2h_req_itowrm/
> +
> +Vendor specific events may also be available and if so can be used via
> +
> + $# perf stat -e cpmu0/vid=VID,gid=GID,mask=MASK/
> +
> +The driver does not support sampling. So "perf record" and attaching to
> +a task are unsupported.
The PMU only supports system-wide counting. That's the reason it doesn't
support per-task profiling. Not because of missing sampling.
Thanks,
Kan
> diff --git a/Documentation/admin-guide/perf/index.rst b/Documentation/admin-guide/perf/index.rst
> index 9de64a40adab..f60be04e4e33 100644
> --- a/Documentation/admin-guide/perf/index.rst
> +++ b/Documentation/admin-guide/perf/index.rst
> @@ -21,3 +21,4 @@ Performance monitor support
> alibaba_pmu
> nvidia-pmu
> meson-ddr-pmu
> + cxl
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support
2023-03-30 16:45 [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support Jonathan Cameron
` (4 preceding siblings ...)
2023-03-30 16:45 ` [PATCH v4 5/5] docs: perf: Minimal introduction the the CXL PMU device and driver Jonathan Cameron
@ 2023-04-04 3:55 ` Dan Williams
2023-04-11 13:21 ` Jonathan Cameron
5 siblings, 1 reply; 21+ messages in thread
From: Dan Williams @ 2023-04-04 3:55 UTC (permalink / raw)
To: Jonathan Cameron, Liang Kan, linux-cxl, peterz
Cc: mingo, acme, mark.rutland, will, dan.j.williams, linuxarm,
linux-perf-users, linux-kernel, Davidlohr Bueso, Dave Jiang
Jonathan Cameron wrote:
> Changes since v3: Kan Liang gave feedback on v2 incorporated here.
> - All updates in patch 4, see details there.
>
> Updated cover letter.
>
> The CXL rev 3.0 specification introduces a CXL Performance Monitoring
> Unit definition. CXL components may have any number of these blocks. The
> definition is highly flexible, but that does bring complexity in the
> driver.
>
> Notes.
>
> 1) The QEMU model against which this was developed needs tidying up.
> Latest tree at https://gitlab.com/jic23/qemu branch cxl-2023-02-28
> It's backed up behind other series that I plan to upstream first.
> 2) There are quite a lot of corner cases that will need working through
> with variants of the model, or I'll have to design a pathological
> set of CPMUs to hit all the corner cases in one go. So whilst I believe
> the driver should be fine for what it supports we may find issues
> as those corners of what is allowed are explored.
> 3) I'm not sure it makes sense to hang this of the cxl/pci driver but
> couldn't really figure out where else in the current structure we could
> make it fit cleanly.
> 4) Driver location. In past perf maintainers have requested perf drivers
> for PCI etc be under drivers/perf. That would require moving some
> CXL headers to be more generally visible, but is certainly possible
> if there is agreement between CXL and perf maintainers on the correct
> location.
I am ok the bulk of the logic living under drivers/perf/
Are drivers/perf/ folks ok with a:
CFLAGS_cxl.o := -I$(srctree)/drivers/cxl/
...or similar in their Makefile, because I don't think this by itself is
a reason to push CXL headers to include/.
I say this without having looked at the code yet and whether this driver
will need new exports from the cxl/core etc.
> 5) Patch 1 led to a discussion of how to handle the self describing
> and extensible nature of the CPMU counters. That is likely to involve
> a description in the "caps" sysfs directory and perf tool code that is
> aware of the different event combinations that make sense and can
> establish which sets are available on a given device.
> That is likely to take a while to converge on, so as the driver is useful
> in the current state, I'm looking to upstream this first and deal with
> the more complex handling later.
What's "this" in this last sentence, a canned set of base counters?
> 6) There is a lot of other functionality that can be added in future
> include allowing for simpler hardware implementations that may not
> support the minimum level of features currently required by the driver
> (freeze, overflow interrupts etc).
Curious if the the minimal set determined by the specification, like the
minimal features a CXL Memory Expander device must implement, or by what
you decided was fit to be emulated in QEMU?
>
> CXL rev 3.0 specification available from https://www.computeexpresslink.org
>
>
> Jonathan Cameron (5):
> cxl: Add function to count regblocks of a given type
> perf: Allow a PMU to have a parent
> cxl/pci: Find and register CXL PMU devices
> cxl: CXL Performance Monitoring Unit driver
> docs: perf: Minimal introduction the the CXL PMU device and driver
>
> Documentation/admin-guide/perf/cxl.rst | 65 ++
> Documentation/admin-guide/perf/index.rst | 1 +
> drivers/cxl/Kconfig | 13 +
> drivers/cxl/Makefile | 1 +
> drivers/cxl/core/Makefile | 1 +
> drivers/cxl/core/core.h | 1 +
> drivers/cxl/core/cpmu.c | 72 ++
> drivers/cxl/core/pci.c | 2 +-
> drivers/cxl/core/port.c | 4 +-
> drivers/cxl/core/regs.c | 50 +-
> drivers/cxl/cpmu.c | 940 +++++++++++++++++++++++
> drivers/cxl/cpmu.h | 56 ++
> drivers/cxl/cxl.h | 17 +-
> drivers/cxl/cxlpci.h | 1 +
> drivers/cxl/pci.c | 27 +-
> include/linux/perf_event.h | 1 +
> kernel/events/core.c | 1 +
> 17 files changed, 1245 insertions(+), 8 deletions(-)
> create mode 100644 Documentation/admin-guide/perf/cxl.rst
> create mode 100644 drivers/cxl/core/cpmu.c
> create mode 100644 drivers/cxl/cpmu.c
> create mode 100644 drivers/cxl/cpmu.h
>
> --
> 2.37.2
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v4 1/5] cxl: Add function to count regblocks of a given type
2023-03-30 16:45 ` [PATCH v4 1/5] cxl: Add function to count regblocks of a given type Jonathan Cameron
@ 2023-04-04 3:59 ` Dan Williams
0 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2023-04-04 3:59 UTC (permalink / raw)
To: Jonathan Cameron, Liang Kan, linux-cxl, peterz
Cc: mingo, acme, mark.rutland, will, dan.j.williams, linuxarm,
linux-perf-users, linux-kernel, Davidlohr Bueso, Dave Jiang
Jonathan Cameron wrote:
> Until the recently release CXL 3.0 specification, there
> was only ever one instance of any given register block pointed
> to by the Register Block Locator DVSEC. Now, the specification allows
> for multiple CXL PMU instances, each with their own register block.
>
> To enable this add an index parameter to cxl_find_regblock()
> and use that to implement cxl_count_regblock().
>
> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
If PMUs are the only consumer of multi-instance register blocks and all
other callers remain oblivious perhaps leave cxl_find_regblock() alone
and have it call a new cxl_find_regblock_instance() internally?
Otherwise, looks good to me.
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v4 2/5] perf: Allow a PMU to have a parent
2023-03-30 16:45 ` [PATCH v4 2/5] perf: Allow a PMU to have a parent Jonathan Cameron
@ 2023-04-04 4:03 ` Dan Williams
0 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2023-04-04 4:03 UTC (permalink / raw)
To: Jonathan Cameron, Liang Kan, linux-cxl, peterz
Cc: mingo, acme, mark.rutland, will, dan.j.williams, linuxarm,
linux-perf-users, linux-kernel, Davidlohr Bueso, Dave Jiang
Jonathan Cameron wrote:
> Some PMUs have well defined parents such as PCI devices.
> As the device_initialize() and device_add() are all within
> pmu_dev_alloc() which is called from perf_pmu_register()
> there is no opportunity to set the parent from within a driver.
>
> Add a struct device *parent field to struct pmu and use that
> to set the parent.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v4: No change
> Note that this may first merge as part of a larger series I
> plan to post next week that adds parents for many of the of the
> other struct pmu instances. If so please drop this patch whilst
> applying.
Feel free to add my:
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
...whereever this gets applied, and yes it makes sense especially as
more device attached PMUs are showing up.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 4/5] cxl: CXL Performance Monitoring Unit driver
2023-04-03 17:32 ` Liang, Kan
@ 2023-04-04 16:48 ` Jonathan Cameron
0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2023-04-04 16:48 UTC (permalink / raw)
To: Liang, Kan
Cc: linux-cxl, peterz, mingo, acme, mark.rutland, will,
dan.j.williams, linuxarm, linux-perf-users, linux-kernel,
Davidlohr Bueso, Dave Jiang
On Mon, 3 Apr 2023 13:32:06 -0400
"Liang, Kan" <kan.liang@linux.intel.com> wrote:
> On 2023-03-30 12:45 p.m., Jonathan Cameron wrote:
> > CXL rev 3.0 introduces a standard performance monitoring hardware
> > block to CXL. Instances are discovered using CXL Register Locator DVSEC
> > entries. Each CXL component may have multiple PMUs.
> >
> > This initial driver supports on a subset of types of counter.
> > It support counters that are either fixed or configurable, but requires
> > that they support the ability to freeze and write value whilst frozen.
> >
> > Development done with QEMU model which will be posted shortly.
> >
>
> So the patch series is only tested with QEMU. Is there a real hardware
> which supports the CXL PMON?
In common with a lot of CXL stuff (and in general new standards based
architecture features), nothing that anyone is willing to talk about
except in general terms at this stage (this is a CXL 3.0 feature so
pretty recent). Much of the existing kernel CXL support has been written
against emulation of one type or another (and we've shaken out a lot of
bugs that aren't yet seen with real hardware. Switches are very
rare for example - I've not yet registered PMUs for those, but that's
on the todo list after we get this merged).
For this particular feature it's likely that initial devices will only
exercise some parts of the driver support. It's much easier to poke
the interesting corner cases with emulation given how extreme the
design space allowed by the CPMU specification is. To that end there
are a lot more features to add to this driver once we have a basic version
in place. One example is that we've had a request for fixed free running counter
support combined with vendor defined events as that's particularly useful
for debugging of new devices.
Note that there is still work to be done on the emulation to make it
easier to vary what is being emulated as right now I end up hacking it
to hit particular interesting cases and that's not going to be great
for CI testing this.
>
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > +
> > +#define CPMU_ATTR_CONFIG_MASK_MSK GENMASK_ULL(63, 48)
>
> (31, 0)?
Indeed. You'd have thought that having them right next
to the definition would have made that obvious when I messed
up the cut and paste. *sigh*
>
> > +#define CPMU_ATTR_CONFIG_GID_MSK GENMASK_ULL(47, 32)
> > +#define CPMU_ATTR_CONFIG_VID_MSK GENMASK_ULL(63, 48)
> > +#define CPMU_ATTR_CONFIG1_THRESHOLD_MSK GENMASK_ULL(15, 0)
> > +#define CPMU_ATTR_CONFIG1_INVERT_MSK BIT(16)
> > +#define CPMU_ATTR_CONFIG1_EDGE_MSK BIT(17)
> > +#define CPMU_ATTR_CONFIG1_FILTER_EN_MSK BIT(18)
> > +#define CPMU_ATTR_CONFIG2_HDM_MSK GENMASK(15, 0)
> > +static void cpmu_event_start(struct perf_event *event, int flags)
> > +{
> > + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> > + struct hw_perf_event *hwc = &event->hw;
> > + void __iomem *base = info->base;
> > + u64 cfg;
> > +
> > + if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
> > + return;
> > +
> > + WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
> > + hwc->state = 0;
> > +
> > + /*
> > + * Currently only hdm filter control is implemnted, this code will
> > + * want generalizing when more filters are added.
> > + */
> > + if (info->filter_hdm) {
> > + if (cpmu_config1_hdm_filter_en(event))
> > + cfg = cpmu_config2_get_hdm_decoder(event);
> > + else
> > + cfg = GENMASK(15, 0); /* No filtering if 0xFFFF_FFFF */
> > + writeq(cfg, base + CPMU_FILTER_CFG_REG(hwc->idx, 0));
> > + }
> > +
> > + cfg = readq(base + CPMU_COUNTER_CFG_REG(hwc->idx));
>
> I don't think we need the previous value. Just overwrite it.
Some bits are hwinit or RO. Whilst we can probably get away with just writing
random garbage (zeros) to those bits, but I'd prefer to write their
value back to them.
>
>
> > + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_INT_ON_OVRFLW, 1);
> > + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_ENABLE, 1);
> > + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_EDGE, cpmu_config1_get_edge(event) ? 1 : 0);
> > + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_INVERT, cpmu_config1_get_invert(event) ? 1 : 0);
> > +
> > + /* Fixed purpose counters have next two fields RO */
> > + if (test_bit(hwc->idx, info->conf_counter_bm)) {
> > + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK, hwc->event_base);
> > + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_EVENTS_MSK, cpmu_config_get_mask(event));
> > + }
> > + cfg &= ~CPMU_COUNTER_CFG_THRESHOLD_MSK;
> > + /*
> > + * For events that generate only 1 count per clock the CXL 3.0 spec
> > + * states the threshold shall be set to 1 but if set to 0 it will
> > + * count the raw value anwyay?
> > + * There is no definition of what events will count multiple per cycle
> > + * and hence to which non 1 values of threshold can apply.
> > + * (CXL 3.0 8.2.7.2.1 Counter Configuration - threshold field definition)
> > + */
> > + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_THRESHOLD_MSK,
> > + cpmu_config1_get_threshold(event));
> > + writeq(cfg, base + CPMU_COUNTER_CFG_REG(hwc->idx));
> > +
> > + local64_set(&hwc->prev_count, 0);
> > + writeq(0, base + CPMU_COUNTER_REG(hwc->idx));
>
> The counter is reset twice. The other is in add()->cpmu_reset_counter().
> I think you can remove the one in add().
After some tracing I think you are correct. This seems to be always called immediately
after add anyway so no point in doing it twice.
>
> > +
> > + perf_event_update_userpage(event);
> > +}
> > +
...
> > +
> > +/*
> > + * Reset ensures no possibility of any information leaking to wrong
> > + * counter. Note that all fields written during start()
> > + */
> > +static void cpmu_reset_counter(struct cpmu_info *info, int idx)
> > +{
> > + void __iomem *base = info->base;
> > +
> > + /* Much of this register is read only */
> > + writeq(0, base + CPMU_EVENT_CAP_REG(idx));
>
>
> CPMU_COUNTER_CFG_REG?
>
> I don't think we need to reset the config here, since it will be rewrite
> in cpmu_event_start().
You are correct. The above write is clearly a noop as that's a read only register
and as you have observed cpmu_event_start() writes almost all the bits that
are not HWInit. I'll make it write the remaining one as well.
As above this whole function can then go away.
>
>
> > + /* Filters are not per counter, so no reset here */
> > + writeq(0, base + CPMU_COUNTER_REG(idx));
> > +}
> > + info->pmu = (struct pmu) {
> > + .name = dev_name(dev),
> > + .parent = dev,
>
> I don't see the PMU parent is used anywhere.
> Please remove this and drop the Patch 2 from this series.
No. As per the discussion of v4, without this the driver is incorrectly
resulting in the device being listed in /sys/devices rather than
/sys/bus/devices/pcie0000:0c/0000:0c:00.0/0000:0d:00.0/cpmu0/cpmu0
I do not want to introduce more ABI that will then need fixing up later.
This issue is common with a lot of other drivers hence the series
https://lore.kernel.org/all/20230404134225.13408-1-Jonathan.Cameron@huawei.com/
to correct it for all drivers that should have a simple parent set.
>
> If there will be other followup patches, please add the PMU parent
> support there.
>
> Thanks,
> Kan
Thanks again for looking at this so quickly.
Jonathan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 5/5] docs: perf: Minimal introduction the the CXL PMU device and driver
2023-04-03 17:45 ` Liang, Kan
@ 2023-04-04 16:55 ` Jonathan Cameron
0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2023-04-04 16:55 UTC (permalink / raw)
To: Liang, Kan
Cc: linux-cxl, peterz, mingo, acme, mark.rutland, will,
dan.j.williams, linuxarm, linux-perf-users, linux-kernel,
Davidlohr Bueso, Dave Jiang
On Mon, 3 Apr 2023 13:45:52 -0400
"Liang, Kan" <kan.liang@linux.intel.com> wrote:
> On 2023-03-30 12:45 p.m., Jonathan Cameron wrote:
> > Very basic introduction to the device and the current driver support
> > provided. I expect to expand on this in future versions of this patch
> > set.
> >
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > --
> > v4: No change
> > ---
> > Documentation/admin-guide/perf/cxl.rst | 65 ++++++++++++++++++++++++
> > Documentation/admin-guide/perf/index.rst | 1 +
> > 2 files changed, 66 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/perf/cxl.rst b/Documentation/admin-guide/perf/cxl.rst
> > new file mode 100644
> > index 000000000000..46235dff4b21
> > --- /dev/null
> > +++ b/Documentation/admin-guide/perf/cxl.rst
> > @@ -0,0 +1,65 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +======================================
> > +CXL Performance Monitoring Unit (CPMU)
> > +======================================
> > +
> > +The CXL rev 3.0 specification provides a definition of CXL Performance
> > +Monitoring Unit in section 13.2: Performance Monitoring.
> > +
> > +CXL components (e.g. Root Port, Switch Upstream Port, End Point) may have
> > +any number of CPMU instances. CPMU capabilities are fully discoverable from
> > +the devices. The specification provides event definitions for all CXL protocol
> > +message types and a set of additional events for things commonly counted on
> > +CXL devices (e.g. DRAM events).
> > +
> > +CPMU driver
> > +===========
> > +
> > +The CPMU driver register a perf PMU with the name cpmu<id> on the CXL bus.
> > +
> > + /sys/bus/cxl/device/cpmu<id>
> > +
> > +The associated PMU is registered as
> > +
> > + /sys/bus/event_sources/devices/cpmu<id>
> > +
> > +In common with other CXL bus devices, the id has no specific meaning and the
> > +relationship to specific CXL device should be established via the device parent
> > +of the device on the CXL bus.
> > +
> > +PMU driver provides description of available events and filter options in sysfs.
> > +
> > +The "format" directory describes all formats of the config (event vendor id,
> > +group id and mask) config1 (threshold, filter enables) and config2 (filter
> > +parameters) fields of the perf_event_attr structure. The "events" directory
> > +describes all documented events show in perf list.
> > +
> > +The events shown in perf list are the most fine grained events with a single
> > +bit of the event mask set. More general events may be enable by setting
> > +multiple mask bits in config. For example, all Device to Host Read Requests
> > +may be captured on a single counter by setting the bits for all of
> > +
> > +* d2h_req_rdcurr
> > +* d2h_req_rdown
> > +* d2h_req_rdshared
> > +* d2h_req_rdany
> > +* d2h_req_rdownnodata
> > +
> > +Example of usage::
> > +
> > + $#perf list
> > + cpmu0/clock_ticks/ [Kernel PMU event]
> > + cpmu0/d2h_req_itomwr/ [Kernel PMU event]
> > + cpmu0/d2h_req_rdany/ [Kernel PMU event]
> > + cpmu0/d2h_req_rdcurr/ [Kernel PMU event]
> > + -----------------------------------------------------------
> > +
> > + $# perf stat -e cpmu0/clock_ticks/ -e cpmu0/d2h_req_itowrm/
> > +
> > +Vendor specific events may also be available and if so can be used via
> > +
> > + $# perf stat -e cpmu0/vid=VID,gid=GID,mask=MASK/
> > +
> > +The driver does not support sampling. So "perf record" and attaching to
> > +a task are unsupported.
>
> The PMU only supports system-wide counting. That's the reason it doesn't
> support per-task profiling. Not because of missing sampling.
Ah. I've managed to fuse two different conditions. I'll break them apart for
v5.
Thanks,
Jonathan
>
> Thanks,
> Kan
> > diff --git a/Documentation/admin-guide/perf/index.rst b/Documentation/admin-guide/perf/index.rst
> > index 9de64a40adab..f60be04e4e33 100644
> > --- a/Documentation/admin-guide/perf/index.rst
> > +++ b/Documentation/admin-guide/perf/index.rst
> > @@ -21,3 +21,4 @@ Performance monitor support
> > alibaba_pmu
> > nvidia-pmu
> > meson-ddr-pmu
> > + cxl
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v4 3/5] cxl/pci: Find and register CXL PMU devices
2023-03-30 16:45 ` [PATCH v4 3/5] cxl/pci: Find and register CXL PMU devices Jonathan Cameron
@ 2023-04-04 19:17 ` Dan Williams
2023-04-05 10:48 ` Jonathan Cameron
0 siblings, 1 reply; 21+ messages in thread
From: Dan Williams @ 2023-04-04 19:17 UTC (permalink / raw)
To: Jonathan Cameron, Liang Kan, linux-cxl, peterz
Cc: mingo, acme, mark.rutland, will, dan.j.williams, linuxarm,
linux-perf-users, linux-kernel, Davidlohr Bueso, Dave Jiang
Jonathan Cameron wrote:
> CXL PMU devices can be found from entries in the Register
> Locator DVSEC.
>
> In order to register the minimum number of IRQ vectors necessary
> to support all CPMUs found, separate the registration into two
> steps. First find the devices, and query the IRQs used and then
> register the devices. Between these two steps, request the
> IRQ vectors necessary and enable bus master support.
It's not clear why this patch is talking about irq vectors and bus
mastering when there is no irq query/setup logic in this patch?
> Future IRQ users for CXL type 3 devices (e.g. DOEs) will need to
> follow a similar pattern the number of vectors necessary is known
> before any parts of the driver stack rely on their availability.
With the new pci_msix_alloc_irq_at() it's not clear that this 2 step
approach is required, right?
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v4:
> - No change.
> ---
> drivers/cxl/core/Makefile | 1 +
> drivers/cxl/core/core.h | 1 +
> drivers/cxl/core/cpmu.c | 72 +++++++++++++++++++++++++++++++++++++++
A quibble with the naming, I prefer:
drivers/cxl/core/pmu.c
...since "cxl" is in the directory path. Also, usages of cpmu
already have a cxl in their symbol names, so just s/cpmu/pmu/ throught.
The usage of CPMU_ for register macros would seem be more clear, or at
least more consistent, as CXL_PMU_ like the other register offset
definitions in cxlpci.h.
> drivers/cxl/core/port.c | 2 ++
> drivers/cxl/core/regs.c | 16 +++++++++
> drivers/cxl/cpmu.h | 56 ++++++++++++++++++++++++++++++
drivers/cxl/pmu.h
> drivers/cxl/cxl.h | 14 ++++++++
> drivers/cxl/cxlpci.h | 1 +
> drivers/cxl/pci.c | 25 +++++++++++++-
> 9 files changed, 187 insertions(+), 1 deletion(-)
Other than those minor issues above, this looks good to me, with those
fixed up.
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v4 4/5] cxl: CXL Performance Monitoring Unit driver
2023-03-30 16:45 ` [PATCH v4 4/5] cxl: CXL Performance Monitoring Unit driver Jonathan Cameron
2023-04-03 17:32 ` Liang, Kan
@ 2023-04-04 21:53 ` Dan Williams
2023-04-05 16:08 ` Jonathan Cameron
1 sibling, 1 reply; 21+ messages in thread
From: Dan Williams @ 2023-04-04 21:53 UTC (permalink / raw)
To: Jonathan Cameron, Liang Kan, linux-cxl, peterz
Cc: mingo, acme, mark.rutland, will, dan.j.williams, linuxarm,
linux-perf-users, linux-kernel, Davidlohr Bueso, Dave Jiang
Jonathan Cameron wrote:
> CXL rev 3.0 introduces a standard performance monitoring hardware
> block to CXL. Instances are discovered using CXL Register Locator DVSEC
> entries. Each CXL component may have multiple PMUs.
>
> This initial driver supports on a subset of types of counter.
s/on//?
> It support counters that are either fixed or configurable, but requires
s/support/supports/
> that they support the ability to freeze and write value whilst frozen.
>
> Development done with QEMU model which will be posted shortly.
Per the comment on the cover letter is there a sense that set of
counters supported in QEMU and this patchset are representative of a
"common" set even if that is by convention and not specification? Just
trying to get a sense of how many real world CXL PMU instances this
might cover.
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
One thing I would have liked to see is a sample perf command line and
output.
One concern I have is how usable DPA based perf events is going to be
for end users. Is there a concept of a synthetic event that can turn
data gathered from all the devices in an interleave set into HPA
relative data? Or do you expect that is the responsibility of the perf
tool to grow that support in userspace?
>
> ---
> v4:
> - Add defines for masks used for config, config1 etc and reorder
> code so those can be next to the attrs. Note that one of the
> bits was wrong showing the advantage of bring all this info
> into one place.
> - Use enabled counter bitmap rather than pointer array to make the
> fixed counter handling look more like the variable counter handling.
> - Drop PERF_EF_RELOAD handling that did nothing.
> - Simplify code as nothing special to do if the counting is already
> up to date (no harm in setting the already set flag).
> ---
> drivers/cxl/Kconfig | 13 +
> drivers/cxl/Makefile | 1 +
> drivers/cxl/cpmu.c | 940 +++++++++++++++++++++++++++++++++++++++++++
Yeah, this is a bunch of code and ABI that is more relevant to drivers/perf/
than drivers/cxl/, so this wants to move to drivers/perf/cxl.c.
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index ff4e78117b31..0be514b00b8f 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -139,4 +139,17 @@ config CXL_REGION_INVALIDATION_TEST
> If unsure, or if this kernel is meant for production environments,
> say N.
>
> +config CXL_CPMU
> + tristate "CXL Performance Monitoring Unit"
> + default CXL_BUS
> + depends on PERF_EVENTS
> + help
> + Support performance monitoring as defined in CXL rev 3.0
> + section 13.2: Performance Monitoring. CXL components may have
> + one or more CXL Performance Monitoring Units (CPMUs).
> +
> + Say 'y/m' to enable a driver that will attach to performance
> + monitoring units and provide standard perf based interfaces.
> +
> + If unsure say 'm'.
> endif
...and this wants to move to drivers/perf/Kconfig as well.
It's similar to "CONFIG_DEV_DAX_CXL" as an external to drivers/cxl/
cxl-driver.
> diff --git a/drivers/cxl/Makefile b/drivers/cxl/Makefile
> index db321f48ba52..024bb739554b 100644
> --- a/drivers/cxl/Makefile
> +++ b/drivers/cxl/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_CXL_MEM) += cxl_mem.o
> obj-$(CONFIG_CXL_ACPI) += cxl_acpi.o
> obj-$(CONFIG_CXL_PMEM) += cxl_pmem.o
> obj-$(CONFIG_CXL_PORT) += cxl_port.o
> +obj-$(CONFIG_CXL_CPMU) += cpmu.o
>
> cxl_mem-y := mem.o
> cxl_pci-y := pci.o
> diff --git a/drivers/cxl/cpmu.c b/drivers/cxl/cpmu.c
> new file mode 100644
> index 000000000000..e6821771a4f6
> --- /dev/null
> +++ b/drivers/cxl/cpmu.c
> @@ -0,0 +1,940 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Copyright(c) 2023 Huawei
> + *
> + * The CXL 3.0 specification includes a standard Performance Monitoring Unit,
> + * called the CXL PMU, or CPMU. In order to allow a high degree of
> + * implementation flexibility the specification provides a wide range of
> + * options all of which are self describing.
> + *
> + * Details in CXL rev 3.0 section 8.2.7 CPMU Register Interface
> + *
> + * TODO
> + * o Discoverability of counters. Allow perftool to provide summed counters
> + * and vendor defined counters.
> + * o Support free running counters - copy the Intel uncore PMU handling for these.
> + * o CPMUs which do not support freeze.
Wouldn't this be driven by the arrival of hardware? I.e. is there
> + * o Add filter validation in cpmu_event_init() so problems are detected earlier.
> + * o Reject configurations that the hardware is ignoring
> + * (e.g. invert when not invertible)
> + * o Support CPMUs with no interrupts using an HRTIMER.
Is it worthwhile to maintain the TODO list in the code? I just fear this
getting stale over time.
> + */
> +
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/perf_event.h>
> +#include <linux/bitops.h>
> +#include <linux/device.h>
> +#include <linux/bits.h>
> +#include <linux/list.h>
> +#include <linux/bug.h>
> +#include <linux/pci.h>
> +
> +#include "cpmu.h"
> +#include "cxlpci.h"
> +#include "cxl.h"
> +
> +/* CXL rev 3.0 Table 13-5 Events under CXL Vendor ID */
> +#define CPMU_GID_CLOCK_TICKS 0x00
> +#define CPMU_GID_D2H_REQ 0x0010
> +#define CPMU_GID_D2H_RSP 0x0011
> +#define CPMU_GID_H2D_REQ 0x0012
> +#define CPMU_GID_H2D_RSP 0x0013
> +#define CPMU_GID_CACHE_DATA 0x0014
> +#define CPMU_GID_M2S_REQ 0x0020
> +#define CPMU_GID_M2S_RWD 0x0021
> +#define CPMU_GID_M2S_BIRSP 0x0022
> +#define CPMU_GID_S2M_BISNP 0x0023
> +#define CPMU_GID_S2M_NDR 0x0024
> +#define CPMU_GID_S2M_DRS 0x0025
> +#define CPMU_GID_DDR 0x8000
Last note about the naming quibble, I notice that CPMU exists in other
places in the kernel, but CXL_PMU does not, so there is a small
grep'ability win as well.
> +
> +static int cpmu_cpuhp_state_num;
> +
> +struct cpmu_event {
> + u16 vid;
> + u16 gid;
> + u32 msk;
> + union {
> + int counter_idx; /* fixed counters */
> + int event_idx; /* configurable counters */
> + };
> + struct list_head node;
> +};
> +
> +#define CPMU_MAX_COUNTERS 64
> +struct cpmu_info {
> + struct pmu pmu;
> + void __iomem *base;
> + struct perf_event **hw_events;
> + struct list_head events_configurable;
> + struct list_head events_fixed;
Linked lists? Didn't xarray kill the linked list?
> + DECLARE_BITMAP(used_counter_bm, CPMU_MAX_COUNTERS);
> + DECLARE_BITMAP(conf_counter_bm, CPMU_MAX_COUNTERS);
> + u16 counter_width;
> + u8 num_counters;
> + u8 num_event_capabilities;
> + int on_cpu;
> + struct hlist_node node;
> + bool freeze_for_enable;
> + bool filter_hdm;
> + int irq;
> +};
> +
> +#define pmu_to_cpmu_info(_pmu) container_of(_pmu, struct cpmu_info, pmu)
> +
> +/*
> + * All CPMU counters are discoverable via the Event Capabilities Registers.
> + * Each Event Capability register contains a a VID / GroupID.
> + * A counter may then count any combination (by summing) of events in
> + * that group which are in the Supported Events Bitmask.
> + * However, there are some complexities to the scheme.
> + * - Fixed function counters refer to an Event Capabilities register.
> + * That event capability register is not then used for Configurable
> + * counters.
> + */
> +static int cpmu_parse_caps(struct device *dev, struct cpmu_info *info)
> +{
> + DECLARE_BITMAP(fixed_counter_event_cap_bm, 32) = {0};
An 'unsigned long' is always at least 32-bits, and:
unsigned long fixed_counter_event_cap_bm = 0;
...would not have interrupted my reading of the code.
> + void __iomem *base = info->base;
> + u64 val, eval;
> + int i;
> +
> + val = readq(base + CPMU_CAP_REG);
> + info->freeze_for_enable = FIELD_GET(CPMU_CAP_WRITEABLE_WHEN_FROZEN, val) &
> + FIELD_GET(CPMU_CAP_FREEZE, val);
Why does 'struct cpmu_info' need to carry the freeze_for_enable flag if
its only used to gate loading the driver?
When / if support for "freeze_for_enable == false" arrives then I can
see 'struct cpmu_info' having a need to cache it.
> + if (!info->freeze_for_enable) {
> + dev_err(dev, "Driver does not support CPMUs that do not support freeze for enable\n");
Perhaps just:
"Counters not writable while frozen."
...will suffice, no need to mention what the driver supports, that's
implied.
> + return -ENODEV;
> + }
> +
> + info->num_counters = FIELD_GET(CPMU_CAP_NUM_COUNTERS_MSK, val) + 1;
> + info->counter_width = FIELD_GET(CPMU_CAP_COUNTER_WIDTH_MSK, val);
> + info->num_event_capabilities = FIELD_GET(CPMU_CAP_NUM_EVN_CAP_REG_SUP_MSK, val) + 1;
> +
> + info->filter_hdm = FIELD_GET(CPMU_CAP_FILTERS_SUP_MSK, val) & CPMU_FILTER_HDM;
> + if (FIELD_GET(CPMU_CAP_INT, val))
> + info->irq = FIELD_GET(CPMU_CAP_MSI_N_MSK, val);
> + else
> + info->irq = -1;
> +
> + /* First handle fixed function counters; note if configurable counters found */
> + for (i = 0; i < info->num_counters; i++) {
> + struct cpmu_event *cpmu_ev;
> + u32 events_msk;
> + u8 group_idx;
> +
> + val = readq(base + CPMU_COUNTER_CFG_REG(i));
> +
> + if (FIELD_GET(CPMU_COUNTER_CFG_TYPE_MSK, val) ==
> + CPMU_COUNTER_CFG_TYPE_CONFIGURABLE) {
> + set_bit(i, info->conf_counter_bm);
> + }
> +
> + if (FIELD_GET(CPMU_COUNTER_CFG_TYPE_MSK, val) !=
> + CPMU_COUNTER_CFG_TYPE_FIXED_FUN)
> + continue;
> +
> + /* In this case we know which fields are const */
> + group_idx = FIELD_GET(CPMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK, val);
> + events_msk = FIELD_GET(CPMU_COUNTER_CFG_EVENTS_MSK, val);
> + eval = readq(base + CPMU_EVENT_CAP_REG(group_idx));
> + cpmu_ev = devm_kzalloc(dev, sizeof(*cpmu_ev), GFP_KERNEL);
> + if (!cpmu_ev)
> + return -ENOMEM;
> +
> + cpmu_ev->vid = FIELD_GET(CPMU_EVENT_CAP_VENDOR_ID_MSK, eval);
> + cpmu_ev->gid = FIELD_GET(CPMU_EVENT_CAP_GROUP_ID_MSK, eval);
> + /* For a fixed purpose counter use the events mask from the counter CFG */
> + cpmu_ev->msk = events_msk;
> + cpmu_ev->counter_idx = i;
> + /* This list add is never unwound as all entries deleted on remove */
> + list_add(&cpmu_ev->node, &info->events_fixed);
> + /*
> + * Configurable counters must not use an Event Capability registers that
> + * is in use for a Fixed counter
> + */
> + set_bit(group_idx, fixed_counter_event_cap_bm);
> + }
> +
> + if (!bitmap_empty(info->conf_counter_bm, CPMU_MAX_COUNTERS)) {
> + struct cpmu_event *cpmu_ev;
> + int j;
> + /* Walk event capabilities unused by fixed counters */
> + for_each_clear_bit(j, fixed_counter_event_cap_bm,
> + info->num_event_capabilities) {
> + cpmu_ev = devm_kzalloc(dev, sizeof(*cpmu_ev), GFP_KERNEL);
> + if (!cpmu_ev)
> + return -ENOMEM;
> +
> + eval = readq(base + CPMU_EVENT_CAP_REG(j));
> + cpmu_ev->vid = FIELD_GET(CPMU_EVENT_CAP_VENDOR_ID_MSK, eval);
> + cpmu_ev->gid = FIELD_GET(CPMU_EVENT_CAP_GROUP_ID_MSK, eval);
> + cpmu_ev->msk = FIELD_GET(CPMU_EVENT_CAP_SUPPORTED_EVENTS_MSK, eval);
> + cpmu_ev->event_idx = j;
> + list_add(&cpmu_ev->node, &info->events_configurable);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static ssize_t cpmu_format_sysfs_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 sysfs_emit(buf, "%s\n", (char *)eattr->var);
> +}
> +
> +#define CPMU_FORMAT_ATTR(_name, _format)\
> + (&((struct dev_ext_attribute[]) { \
> + { \
> + .attr = __ATTR(_name, 0444, \
> + cpmu_format_sysfs_show, NULL), \
> + .var = (void *)_format \
> + } \
> + })[0].attr.attr)
> +
> +enum {
> + cpmu_mask_attr,
> + cpmu_gid_attr,
> + cpmu_vid_attr,
> + cpmu_threshold_attr,
> + cpmu_invert_attr,
> + cpmu_edge_attr,
> + cpmu_hdm_filter_en_attr,
> + cpmu_hdm_attr,
> +};
> +
> +static struct attribute *cpmu_format_attr[] = {
> + [cpmu_mask_attr] = CPMU_FORMAT_ATTR(mask, "config:0-31"),
> + [cpmu_gid_attr] = CPMU_FORMAT_ATTR(gid, "config:32-47"),
> + [cpmu_vid_attr] = CPMU_FORMAT_ATTR(vid, "config:48-63"),
> + [cpmu_threshold_attr] = CPMU_FORMAT_ATTR(threshold, "config1:0-15"),
> + [cpmu_invert_attr] = CPMU_FORMAT_ATTR(invert, "config1:16"),
> + [cpmu_edge_attr] = CPMU_FORMAT_ATTR(edge, "config1:17"),
> + [cpmu_hdm_filter_en_attr] = CPMU_FORMAT_ATTR(hdm_filter_en, "config1:18"),
> + [cpmu_hdm_attr] = CPMU_FORMAT_ATTR(hdm, "config2:0-15"),
> + NULL
> +};
Most of this is brand new ABI to me, so I leave it to perf folks to
check for correctness.
> +
> +#define CPMU_ATTR_CONFIG_MASK_MSK GENMASK_ULL(63, 48)
> +#define CPMU_ATTR_CONFIG_GID_MSK GENMASK_ULL(47, 32)
> +#define CPMU_ATTR_CONFIG_VID_MSK GENMASK_ULL(63, 48)
> +#define CPMU_ATTR_CONFIG1_THRESHOLD_MSK GENMASK_ULL(15, 0)
> +#define CPMU_ATTR_CONFIG1_INVERT_MSK BIT(16)
> +#define CPMU_ATTR_CONFIG1_EDGE_MSK BIT(17)
> +#define CPMU_ATTR_CONFIG1_FILTER_EN_MSK BIT(18)
> +#define CPMU_ATTR_CONFIG2_HDM_MSK GENMASK(15, 0)
> +
> +static umode_t cpmu_format_is_visible(struct kobject *kobj, struct attribute *attr, int a)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct cpmu_info *info = dev_get_drvdata(dev);
> +
> + /*
> + * Filter capability at the CPMU level, so hide the attributes if the particular
> + * filter is not supported.
> + */
> + if (attr == cpmu_format_attr[cpmu_hdm_filter_en_attr] ||
> + attr == cpmu_format_attr[cpmu_hdm_attr]) {
> + if (info->filter_hdm)
> + return 0444;
> + else
> + return 0;
> + } else {
> + return 0444;
> + }
> +}
Maybe simplify to:
if (!info->filter_hdm &&
(attr == cpmu_format_attr[cpmu_hdm_filter_en_attr] ||
attr == cpmu_format_attr[cpmu_hdm_attr]))
return 0;
return attr->mode;
> +
> +static const struct attribute_group cpmu_format_group = {
> + .name = "format",
> + .attrs = cpmu_format_attr,
> + .is_visible = cpmu_format_is_visible,
> +};
> +
> +static u32 cpmu_config_get_mask(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG_MASK_MSK, event->attr.config);
> +}
> +
> +static u16 cpmu_config_get_gid(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG_GID_MSK, event->attr.config);
> +}
> +
> +static u16 cpmu_config_get_vid(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG_VID_MSK, event->attr.config);
> +}
> +
> +static u8 cpmu_config1_get_threshold(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG1_THRESHOLD_MSK, event->attr.config1);
> +}
> +
> +static bool cpmu_config1_get_invert(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG1_INVERT_MSK, event->attr.config1);
> +}
> +
> +static bool cpmu_config1_get_edge(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG1_EDGE_MSK, event->attr.config1);
> +}
> +
> +/*
> + * CPMU specification allows for 8 filters, each with a 16 bit value...
> + * So we need to find 8x16bits to store it in.
> + * As the value used for disable is 0xffff, a separate enable switch
> + * is needed.
> + */
> +
> +static bool cpmu_config1_hdm_filter_en(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG1_FILTER_EN_MSK, event->attr.config1);
> +}
> +
> +static u16 cpmu_config2_get_hdm_decoder(struct perf_event *event)
> +{
> + return FIELD_GET(CPMU_ATTR_CONFIG2_HDM_MSK, event->attr.config2);
> +}
> +
> +static ssize_t cpmu_event_sysfs_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct perf_pmu_events_attr *pmu_attr =
> + container_of(attr, struct perf_pmu_events_attr, attr);
> +
> + return sysfs_emit(buf, "config=%#llx\n", pmu_attr->id);
> +}
> +
> +#define CPMU_PMU_EVENT_ATTR(_name, _vid, _gid, _msk) \
> + PMU_EVENT_ATTR_ID(_name, cpmu_event_sysfs_show, \
> + ((u64)(_vid) << 48) | ((u64)(_gid) << 32) | (u64)(_msk))
> +
> +/* For CXL spec defined events */
> +#define CPMU_PMU_EVENT_CXL_ATTR(_name, _gid, _msk) \
> + CPMU_PMU_EVENT_ATTR(_name, PCI_DVSEC_VENDOR_ID_CXL, _gid, _msk)
> +
> +static struct attribute *cpmu_event_attrs[] = {
> + CPMU_PMU_EVENT_CXL_ATTR(clock_ticks, CPMU_GID_CLOCK_TICKS, BIT(0)),
> + /* CXL rev 3.0 Table 3-17 - Device to Host Requests */
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_rdcurr, CPMU_GID_D2H_REQ, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_rdown, CPMU_GID_D2H_REQ, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_rdshared, CPMU_GID_D2H_REQ, BIT(3)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_rdany, CPMU_GID_D2H_REQ, BIT(4)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_rdownnodata, CPMU_GID_D2H_REQ, BIT(5)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_itomwr, CPMU_GID_D2H_REQ, BIT(6)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_wrcurr, CPMU_GID_D2H_REQ, BIT(7)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_clflush, CPMU_GID_D2H_REQ, BIT(8)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_cleanevict, CPMU_GID_D2H_REQ, BIT(9)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_dirtyevict, CPMU_GID_D2H_REQ, BIT(10)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_cleanevictnodata, CPMU_GID_D2H_REQ, BIT(11)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_wowrinv, CPMU_GID_D2H_REQ, BIT(12)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_wowrinvf, CPMU_GID_D2H_REQ, BIT(13)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_wrinv, CPMU_GID_D2H_REQ, BIT(14)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_req_cacheflushed, CPMU_GID_D2H_REQ, BIT(16)),
> + /* CXL rev 3.0 Table 3-20 - D2H Repsonse Encodings */
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspihiti, CPMU_GID_D2H_RSP, BIT(4)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspvhitv, CPMU_GID_D2H_RSP, BIT(6)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspihitse, CPMU_GID_D2H_RSP, BIT(5)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspshitse, CPMU_GID_D2H_RSP, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspsfwdm, CPMU_GID_D2H_RSP, BIT(7)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspifwdm, CPMU_GID_D2H_RSP, BIT(15)),
> + CPMU_PMU_EVENT_CXL_ATTR(d2h_rsp_rspvfwdv, CPMU_GID_D2H_RSP, BIT(22)),
> + /* CXL rev 3.0 Table 3-21 - CXL.cache - Mapping of H2D Requests to D2H Responses */
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_req_snpdata, CPMU_GID_H2D_REQ, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_req_snpinv, CPMU_GID_H2D_REQ, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_req_snpcur, CPMU_GID_H2D_REQ, BIT(3)),
> + /* CXL rev 3.0 Table 3-22 - H2D Response Opcode Encodings */
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_writepull, CPMU_GID_H2D_RSP, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_go, CPMU_GID_H2D_RSP, BIT(4)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_gowritepull, CPMU_GID_H2D_RSP, BIT(5)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_extcmp, CPMU_GID_H2D_RSP, BIT(6)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_gowritepulldrop, CPMU_GID_H2D_RSP, BIT(8)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_fastgowritepull, CPMU_GID_H2D_RSP, BIT(13)),
> + CPMU_PMU_EVENT_CXL_ATTR(h2d_rsp_goerrwritepull, CPMU_GID_H2D_RSP, BIT(15)),
> + /* CXL rev 3.0 Table 13-5 directly lists these */
> + CPMU_PMU_EVENT_CXL_ATTR(cachedata_d2h_data, CPMU_GID_CACHE_DATA, BIT(0)),
> + CPMU_PMU_EVENT_CXL_ATTR(cachedata_h2d_data, CPMU_GID_CACHE_DATA, BIT(1)),
> + /* CXL rev 3.0 Table 3-29 M2S Req Memory Opcodes */
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_meminv, CPMU_GID_M2S_REQ, BIT(0)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memrd, CPMU_GID_M2S_REQ, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memrddata, CPMU_GID_M2S_REQ, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memrdfwd, CPMU_GID_M2S_REQ, BIT(3)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memwrfwd, CPMU_GID_M2S_REQ, BIT(4)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memspecrd, CPMU_GID_M2S_REQ, BIT(8)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_meminvnt, CPMU_GID_M2S_REQ, BIT(9)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_req_memcleanevict, CPMU_GID_M2S_REQ, BIT(10)),
> + /* CXL rev 3.0 Table 3-35 M2S RwD Memory Opcodes */
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_rwd_memwr, CPMU_GID_M2S_RWD, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_rwd_memwrptl, CPMU_GID_M2S_RWD, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_rwd_biconflict, CPMU_GID_M2S_RWD, BIT(4)),
> + /* CXL rev 3.0 Table 3-38 M2S BIRsp Memory Opcodes */
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_i, CPMU_GID_M2S_BIRSP, BIT(0)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_s, CPMU_GID_M2S_BIRSP, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_e, CPMU_GID_M2S_BIRSP, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_iblk, CPMU_GID_M2S_BIRSP, BIT(4)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_sblk, CPMU_GID_M2S_BIRSP, BIT(5)),
> + CPMU_PMU_EVENT_CXL_ATTR(m2s_birsp_eblk, CPMU_GID_M2S_BIRSP, BIT(6)),
> + /* CXL rev 3.0 Table 3-40 S2M BISnp Opcodes */
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_cur, CPMU_GID_S2M_BISNP, BIT(0)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_data, CPMU_GID_S2M_BISNP, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_inv, CPMU_GID_S2M_BISNP, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_curblk, CPMU_GID_S2M_BISNP, BIT(4)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_datblk, CPMU_GID_S2M_BISNP, BIT(5)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_bisnp_invblk, CPMU_GID_S2M_BISNP, BIT(6)),
> + /* CXL rev 3.0 Table 3-43 S2M NDR Opcopdes */
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_ndr_cmp, CPMU_GID_S2M_NDR, BIT(0)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_ndr_cmps, CPMU_GID_S2M_NDR, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_ndr_cmpe, CPMU_GID_S2M_NDR, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_ndr_biconflictack, CPMU_GID_S2M_NDR, BIT(3)),
> + /* CXL rev 3.0 Table 3-46 S2M DRS opcodes */
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_drs_memdata, CPMU_GID_S2M_DRS, BIT(0)),
> + CPMU_PMU_EVENT_CXL_ATTR(s2m_drs_memdatanxm, CPMU_GID_S2M_DRS, BIT(1)),
> + /* CXL rev 3.0 Table 13-5 directly lists these */
> + CPMU_PMU_EVENT_CXL_ATTR(ddr_act, CPMU_GID_DDR, BIT(0)),
> + CPMU_PMU_EVENT_CXL_ATTR(ddr_pre, CPMU_GID_DDR, BIT(1)),
> + CPMU_PMU_EVENT_CXL_ATTR(ddr_casrd, CPMU_GID_DDR, BIT(2)),
> + CPMU_PMU_EVENT_CXL_ATTR(ddr_caswr, CPMU_GID_DDR, BIT(3)),
> + CPMU_PMU_EVENT_CXL_ATTR(ddr_refresh, CPMU_GID_DDR, BIT(4)),
> + CPMU_PMU_EVENT_CXL_ATTR(ddr_selfrefreshent, CPMU_GID_DDR, BIT(5)),
> + CPMU_PMU_EVENT_CXL_ATTR(ddr_rfm, CPMU_GID_DDR, BIT(6)),
> + NULL
> +};
> +
> +static struct cpmu_event *cpmu_find_fixed_counter_event(struct cpmu_info *info,
> + int vid, int gid, int msk)
> +{
> + struct cpmu_event *cpmu_ev;
> +
> + list_for_each_entry(cpmu_ev, &info->events_fixed, node) {
> + if (vid != cpmu_ev->vid || gid != cpmu_ev->gid)
> + continue;
> +
> + /* Precise match for fixed counter */
> + if (msk == cpmu_ev->msk)
> + return cpmu_ev;
It looks like the lookup parameters fit in 64-bits which if using an
xarray could do a direct lookup (assuming this driver 'depends on 64BIT'
which is not unreasonable).
> + }
> +
> + return ERR_PTR(-EINVAL);
> +}
> +
> +static struct cpmu_event *cpmu_find_config_counter_event(struct cpmu_info *info,
> + int vid, int gid, int msk)
> +{
> + struct cpmu_event *cpmu_ev;
> +
> + list_for_each_entry(cpmu_ev, &info->events_configurable, node) {
> + if (vid != cpmu_ev->vid || gid != cpmu_ev->gid)
> + continue;
> +
> + /* Request mask must be subset of supported */
> + if (msk & ~cpmu_ev->msk)
> + continue;
> +
> + return cpmu_ev;
> + }
> +
> + return ERR_PTR(-EINVAL);
> +}
> +
> +static umode_t cpmu_event_is_visible(struct kobject *kobj, struct attribute *attr, int a)
> +{
> + struct device_attribute *dev_attr = container_of(attr, struct device_attribute, attr);
> + struct perf_pmu_events_attr *pmu_attr =
> + container_of(dev_attr, struct perf_pmu_events_attr, attr);
> + struct device *dev = kobj_to_dev(kobj);
> + struct cpmu_info *info = dev_get_drvdata(dev);
> + int vid = FIELD_GET(CPMU_ATTR_CONFIG_VID_MSK, pmu_attr->id);
> + int gid = FIELD_GET(CPMU_ATTR_CONFIG_GID_MSK, pmu_attr->id);
> + int msk = FIELD_GET(CPMU_ATTR_CONFIG_MASK_MSK, pmu_attr->id);
> +
> + if (!IS_ERR(cpmu_find_fixed_counter_event(info, vid, gid, msk)))
> + return attr->mode;
> +
> + if (!IS_ERR(cpmu_find_config_counter_event(info, vid, gid, msk)))
> + return attr->mode;
> +
> + return 0;
> +}
> +
> +static const struct attribute_group cpmu_events = {
> + .name = "events",
> + .attrs = cpmu_event_attrs,
> + .is_visible = cpmu_event_is_visible,
> +};
> +
> +static ssize_t cpumask_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct cpmu_info *info = dev_get_drvdata(dev);
> +
> + return cpumap_print_to_pagebuf(true, buf, cpumask_of(info->on_cpu));
> +}
> +static DEVICE_ATTR_RO(cpumask);
> +
> +static struct attribute *cpmu_cpumask_attrs[] = {
> + &dev_attr_cpumask.attr,
> + NULL
> +};
> +
> +static const struct attribute_group cpmu_cpumask_group = {
> + .attrs = cpmu_cpumask_attrs,
> +};
> +
> +static const struct attribute_group *cpmu_attr_groups[] = {
> + &cpmu_events,
> + &cpmu_format_group,
> + &cpmu_cpumask_group,
> + NULL
> +};
> +
> +/* If counter_idx == NULL, don't try to allocate a counter. */
> +static int cpmu_get_event_idx(struct perf_event *event, int *counter_idx, int *event_idx)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> + DECLARE_BITMAP(configurable_and_free, CPMU_MAX_COUNTERS);
> + struct cpmu_event *cpmu_ev;
> + u32 mask;
> + u16 gid, vid;
> + int i;
> +
> + vid = cpmu_config_get_vid(event);
> + gid = cpmu_config_get_gid(event);
> + mask = cpmu_config_get_mask(event);
> +
> + cpmu_ev = cpmu_find_fixed_counter_event(info, vid, gid, mask);
> + if (!IS_ERR(cpmu_ev)) {
> + if (!counter_idx)
> + return 0;
> + if (!test_bit(cpmu_ev->counter_idx, info->used_counter_bm)) {
> + *counter_idx = cpmu_ev->counter_idx;
> + return 0;
> + }
> + /* Fixed counter is in use, but maybe a configurable one? */
> + }
> +
> + cpmu_ev = cpmu_find_config_counter_event(info, vid, gid, mask);
> + if (!IS_ERR(cpmu_ev)) {
> + if (!counter_idx)
> + return 0;
> +
> + bitmap_andnot(configurable_and_free, info->conf_counter_bm,
> + info->used_counter_bm, CPMU_MAX_COUNTERS);
> +
> + i = find_first_bit(configurable_and_free, CPMU_MAX_COUNTERS);
> + if (i == CPMU_MAX_COUNTERS)
> + return -EINVAL;
> +
> + *counter_idx = i;
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int cpmu_event_init(struct perf_event *event)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> +
> + event->cpu = info->on_cpu;
> + /* Top level type sanity check - is this a Hardware Event being requested */
> + if (event->attr.type != event->pmu->type)
> + return -ENOENT;
> +
> + if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
> + return -EOPNOTSUPP;
> + /* TODO: Validation of any filter */
> +
> + /*
> + * Verify that it is possible to count what was requested. Either must
> + * be a fixed counter that is a precise match or a configurable counter
> + * where this is a subset.
> + */
> + return cpmu_get_event_idx(event, NULL, NULL);
> +}
> +
> +static void cpmu_pmu_enable(struct pmu *pmu)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(pmu);
> + void __iomem *base = info->base;
> +
> + /* We don't have a global enable, but we 'might' have a global freeze which we can use */
> + if (info->freeze_for_enable) {
> + /* Can assume frozen at this stage */
> + writeq(0, base + CPMU_FREEZE_REG);
> +
> + return;
> + }
> +}
> +
> +static void cpmu_pmu_disable(struct pmu *pmu)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(pmu);
> + void __iomem *base = info->base;
> +
> + if (info->freeze_for_enable) {
> + /*
> + * Whilst bits above number of counters are RsvdZ
> + * they are unlikely to be repurposed given
> + * number of counters is allowed to be 64 leaving
> + * no reserved bits. Hence this is only slightly
> + * naughty.
> + */
> + writeq(GENMASK_ULL(63, 0), base + CPMU_FREEZE_REG);
> + return;
> + }
> +}
These can be swapped out for versions that do nothing in the
freeze_for_enable == false case when that support arrives, otherwise
these can be simplified because the check was resolved at init time.
> +
> +static void cpmu_event_start(struct perf_event *event, int flags)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> + void __iomem *base = info->base;
> + u64 cfg;
> +
> + if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
> + return;
> +
> + WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
Maybe a comment about why these warns will likely never fire in practice?
> + hwc->state = 0;
> +
> + /*
> + * Currently only hdm filter control is implemnted, this code will
> + * want generalizing when more filters are added.
> + */
> + if (info->filter_hdm) {
> + if (cpmu_config1_hdm_filter_en(event))
> + cfg = cpmu_config2_get_hdm_decoder(event);
> + else
> + cfg = GENMASK(15, 0); /* No filtering if 0xFFFF_FFFF */
> + writeq(cfg, base + CPMU_FILTER_CFG_REG(hwc->idx, 0));
> + }
> +
> + cfg = readq(base + CPMU_COUNTER_CFG_REG(hwc->idx));
> + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_INT_ON_OVRFLW, 1);
> + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_ENABLE, 1);
> + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_EDGE, cpmu_config1_get_edge(event) ? 1 : 0);
> + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_INVERT, cpmu_config1_get_invert(event) ? 1 : 0);
> +
> + /* Fixed purpose counters have next two fields RO */
> + if (test_bit(hwc->idx, info->conf_counter_bm)) {
> + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_EVENT_GRP_ID_IDX_MSK, hwc->event_base);
> + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_EVENTS_MSK, cpmu_config_get_mask(event));
> + }
> + cfg &= ~CPMU_COUNTER_CFG_THRESHOLD_MSK;
> + /*
> + * For events that generate only 1 count per clock the CXL 3.0 spec
> + * states the threshold shall be set to 1 but if set to 0 it will
> + * count the raw value anwyay?
> + * There is no definition of what events will count multiple per cycle
> + * and hence to which non 1 values of threshold can apply.
> + * (CXL 3.0 8.2.7.2.1 Counter Configuration - threshold field definition)
> + */
> + cfg |= FIELD_PREP(CPMU_COUNTER_CFG_THRESHOLD_MSK,
> + cpmu_config1_get_threshold(event));
> + writeq(cfg, base + CPMU_COUNTER_CFG_REG(hwc->idx));
> +
> + local64_set(&hwc->prev_count, 0);
> + writeq(0, base + CPMU_COUNTER_REG(hwc->idx));
> +
> + perf_event_update_userpage(event);
> +}
> +
> +static u64 cpmu_read_counter(struct perf_event *event)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> + void __iomem *base = info->base;
> +
> + return readq(base + CPMU_COUNTER_REG(event->hw.idx));
> +}
> +
> +static void __cpmu_read(struct perf_event *event, bool overflow)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> + u64 new_cnt, prev_cnt, delta;
> +
> + do {
> + prev_cnt = local64_read(&hwc->prev_count);
> + new_cnt = cpmu_read_counter(event);
> + } while (local64_cmpxchg(&hwc->prev_count, prev_cnt, new_cnt) != prev_cnt);
> +
> + /*
> + * If we know an overflow occur then take that into account.
> + * Note counter is not reset as that would lose events
> + */
> + delta = (new_cnt - prev_cnt) & GENMASK_ULL(info->counter_width - 1, 0);
> + if (overflow && delta < GENMASK_ULL(info->counter_width - 1, 0))
> + delta += (1UL << info->counter_width);
> +
> + local64_add(delta, &event->count);
> +}
> +
> +static void cpmu_read(struct perf_event *event)
> +{
> + __cpmu_read(event, false);
> +}
> +
> +static void cpmu_event_stop(struct perf_event *event, int flags)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> + void __iomem *base = info->base;
> + struct hw_perf_event *hwc = &event->hw;
> + u64 cfg;
> +
> + cpmu_read(event);
> + WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
> + hwc->state |= PERF_HES_STOPPED;
> +
> + cfg = readq(base + CPMU_COUNTER_CFG_REG(hwc->idx));
> + cfg &= ~(FIELD_PREP(CPMU_COUNTER_CFG_INT_ON_OVRFLW, 1) |
> + FIELD_PREP(CPMU_COUNTER_CFG_ENABLE, 1));
> + writeq(cfg, base + CPMU_COUNTER_CFG_REG(hwc->idx));
> +
> + hwc->state |= PERF_HES_UPTODATE;
> +}
> +
> +/*
> + * Reset ensures no possibility of any information leaking to wrong
> + * counter. Note that all fields written during start()
> + */
> +static void cpmu_reset_counter(struct cpmu_info *info, int idx)
> +{
> + void __iomem *base = info->base;
> +
> + /* Much of this register is read only */
> + writeq(0, base + CPMU_EVENT_CAP_REG(idx));
> + /* Filters are not per counter, so no reset here */
> + writeq(0, base + CPMU_COUNTER_REG(idx));
> +}
> +
> +static int cpmu_event_add(struct perf_event *event, int flags)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> + int idx, rc;
> + int event_idx = 0;
> +
> + hwc->state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
> +
> + rc = cpmu_get_event_idx(event, &idx, &event_idx);
> + if (rc < 0)
> + return rc;
> +
> + hwc->idx = idx;
> +
> + /* Only set for configurable counters */
> + hwc->event_base = event_idx;
> + info->hw_events[idx] = event;
> + set_bit(idx, info->used_counter_bm);
> +
> + cpmu_reset_counter(info, idx);
> +
> + if (flags & PERF_EF_START)
> + cpmu_event_start(event, PERF_EF_RELOAD);
> +
> + return 0;
> +}
> +
> +static void cpmu_event_del(struct perf_event *event, int flags)
> +{
> + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> +
> + cpmu_event_stop(event, PERF_EF_UPDATE);
> + clear_bit(hwc->idx, info->used_counter_bm);
> + info->hw_events[hwc->idx] = NULL;
> + perf_event_update_userpage(event);
> +}
> +
> +static irqreturn_t cpmu_irq(int irq, void *data)
> +{
> + struct cpmu_info *info = data;
> + void __iomem *base = info->base;
> + u64 overflowed;
> + DECLARE_BITMAP(overflowedbm, 64);
> + int i;
> +
> + overflowed = readq(base + CPMU_OVERFLOW_REG);
> +
> + /* Interrupt may be shared, so maybe it isn't ours */
> + if (!overflowed)
> + return IRQ_NONE;
> +
> + bitmap_from_arr64(overflowedbm, &overflowed, 64);
> + for_each_set_bit(i, overflowedbm, info->num_counters) {
> + struct perf_event *event = info->hw_events[i];
> +
> + if (!event) {
> + dev_dbg(info->pmu.dev,
> + "overflow but on non enabled counter %d\n", i);
> + continue;
> + }
> +
> + __cpmu_read(event, true);
> + }
> +
> + writeq(overflowed, base + CPMU_OVERFLOW_REG);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int cxl_cpmu_probe(struct device *dev)
> +{
> + struct cxl_cpmu *cpmu = to_cxl_cpmu(dev);
> + struct pci_dev *pdev = to_pci_dev(dev->parent);
> + struct cpmu_info *info;
> + char *irq_name;
> + int rc, irq;
> +
> + info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + INIT_LIST_HEAD(&info->events_fixed);
> + INIT_LIST_HEAD(&info->events_configurable);
> +
> + info->base = cpmu->base;
> +
> + info->on_cpu = -1;
> + rc = cpmu_parse_caps(dev, info);
> + if (rc)
> + return rc;
> +
> + info->hw_events = devm_kcalloc(dev, sizeof(*info->hw_events),
> + info->num_counters, GFP_KERNEL);
> + if (!info->hw_events)
> + return -ENOMEM;
> +
> + info->pmu = (struct pmu) {
> + .name = dev_name(dev),
> + .parent = dev,
> + .module = THIS_MODULE,
> + .event_init = cpmu_event_init,
> + .pmu_enable = cpmu_pmu_enable,
> + .pmu_disable = cpmu_pmu_disable,
> + .add = cpmu_event_add,
> + .del = cpmu_event_del,
> + .start = cpmu_event_start,
> + .stop = cpmu_event_stop,
> + .read = cpmu_read,
> + .task_ctx_nr = perf_invalid_context,
> + .attr_groups = cpmu_attr_groups,
> + .capabilities = PERF_PMU_CAP_NO_EXCLUDE,
> + };
> +
> + if (info->irq <= 0)
> + return -EINVAL;
> +
> + rc = pci_irq_vector(pdev, info->irq);
> + if (rc < 0)
> + return rc;
> + irq = rc;
> +
> + irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s_overflow\n", dev_name(dev));
> + if (!irq_name)
> + return -ENOMEM;
> +
> + rc = devm_request_irq(dev, irq, cpmu_irq, IRQF_SHARED, irq_name, info);
This also wants IRQF_ONESHOT, see:
5a84711fd734 cxl/pci: Fix irq oneshot expectations
> + if (rc)
> + return rc;
> + info->irq = irq;
> +
> + rc = cpuhp_state_add_instance(cpmu_cpuhp_state_num, &info->node);
> + if (rc)
> + return rc;
> +
> + rc = perf_pmu_register(&info->pmu, info->pmu.name, -1);
> + if (rc)
> + return rc;
> +
> + dev_set_drvdata(dev, info);
Is it really the case that drvdata can be set after the pmu is already
registered and live, or maybe just getting lucky with accessing the ABI
after probe returns?
> +
> + return 0;
> +}
> +
> +static void cxl_cpmu_remove(struct device *dev)
> +{
> + struct cpmu_info *info = dev_get_drvdata(dev);
> +
> + perf_pmu_unregister(&info->pmu);
> + cpuhp_state_remove_instance_nocalls(cpmu_cpuhp_state_num, &info->node);
How about add devm_add_action_or_reset() calls for these and delete
cxl_cpmu_remove()?
> +}
> +
> +static struct cxl_driver cxl_cpmu_driver = {
> + .name = "cxl_cpmu",
> + .probe = cxl_cpmu_probe,
> + .remove = cxl_cpmu_remove,
> + .id = CXL_DEVICE_CPMU,
> +};
> +
> +static int cpmu_online_cpu(unsigned int cpu, struct hlist_node *node)
> +{
> + struct cpmu_info *info = hlist_entry_safe(node, struct cpmu_info, node);
> +
> + if (info->on_cpu != -1)
> + return 0;
> +
> + info->on_cpu = cpu;
> + WARN_ON(irq_set_affinity(info->irq, cpumask_of(cpu)));
Is this really a kernel crashable scenario for folks that have
panic_on_warn() set? If it's a "should never" happen type situation then
maybe a comment, otherwise pr_warn().
> +
> + return 0;
> +}
> +
> +static int cpmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> +{
> + struct cpmu_info *info = hlist_entry_safe(node, struct cpmu_info, node);
> + unsigned int target;
> +
> + if (info->on_cpu != cpu)
> + return 0;
> +
> + info->on_cpu = -1;
> + target = cpumask_first(cpu_online_mask);
> + if (target >= nr_cpu_ids) {
> + dev_err(info->pmu.dev, "Unable to find a suitable CPU\n");
> + return 0;
> + }
> +
> + perf_pmu_migrate_context(&info->pmu, cpu, target);
> + info->on_cpu = target;
> + WARN_ON(irq_set_affinity(info->irq, cpumask_of(target)));
> +
> + return 0;
> +}
> +
> +static __init int cxl_cpmu_init(void)
> +{
> + int rc;
> +
> + rc = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
> + "AP_PERF_CPMU_ONLINE",
> + cpmu_online_cpu, cpmu_offline_cpu);
> + if (rc < 0)
> + return rc;
> + cpmu_cpuhp_state_num = rc;
> +
> + rc = cxl_driver_register(&cxl_cpmu_driver);
> + if (rc)
> + cpuhp_remove_multi_state(cpmu_cpuhp_state_num);
> +
> + return rc;
> +}
> +
> +static __exit void cxl_cpmu_exit(void)
> +{
> + cxl_driver_unregister(&cxl_cpmu_driver);
> + cpuhp_remove_multi_state(cpmu_cpuhp_state_num);
> +}
> +
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(CXL);
> +module_init(cxl_cpmu_init);
> +module_exit(cxl_cpmu_exit);
> +MODULE_ALIAS_CXL(CXL_DEVICE_CPMU);
> --
> 2.37.2
>
This looks good to me, I'd be happy to take it through the CXL tree when
it finalizes with perf folks acks, just let me know.
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v4 5/5] docs: perf: Minimal introduction the the CXL PMU device and driver
2023-03-30 16:45 ` [PATCH v4 5/5] docs: perf: Minimal introduction the the CXL PMU device and driver Jonathan Cameron
2023-04-03 17:45 ` Liang, Kan
@ 2023-04-04 22:24 ` Dan Williams
2023-04-06 16:33 ` Jonathan Cameron
1 sibling, 1 reply; 21+ messages in thread
From: Dan Williams @ 2023-04-04 22:24 UTC (permalink / raw)
To: Jonathan Cameron, Liang Kan, linux-cxl, peterz
Cc: mingo, acme, mark.rutland, will, dan.j.williams, linuxarm,
linux-perf-users, linux-kernel, Davidlohr Bueso, Dave Jiang
Jonathan Cameron wrote:
> Very basic introduction to the device and the current driver support
> provided. I expect to expand on this in future versions of this patch
> set.
>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> --
> v4: No change
> ---
> Documentation/admin-guide/perf/cxl.rst | 65 ++++++++++++++++++++++++
> Documentation/admin-guide/perf/index.rst | 1 +
> 2 files changed, 66 insertions(+)
>
> diff --git a/Documentation/admin-guide/perf/cxl.rst b/Documentation/admin-guide/perf/cxl.rst
> new file mode 100644
> index 000000000000..46235dff4b21
> --- /dev/null
> +++ b/Documentation/admin-guide/perf/cxl.rst
> @@ -0,0 +1,65 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +======================================
> +CXL Performance Monitoring Unit (CPMU)
> +======================================
> +
> +The CXL rev 3.0 specification provides a definition of CXL Performance
> +Monitoring Unit in section 13.2: Performance Monitoring.
> +
> +CXL components (e.g. Root Port, Switch Upstream Port, End Point) may have
> +any number of CPMU instances. CPMU capabilities are fully discoverable from
> +the devices. The specification provides event definitions for all CXL protocol
> +message types and a set of additional events for things commonly counted on
> +CXL devices (e.g. DRAM events).
> +
> +CPMU driver
> +===========
> +
> +The CPMU driver register a perf PMU with the name cpmu<id> on the CXL bus.
s/register/registers/
> +
> + /sys/bus/cxl/device/cpmu<id>
> +
> +The associated PMU is registered as
> +
> + /sys/bus/event_sources/devices/cpmu<id>
> +
> +In common with other CXL bus devices, the id has no specific meaning and the
> +relationship to specific CXL device should be established via the device parent
> +of the device on the CXL bus.
So I went to go add some text about how to identify PMUs in a persistent
manner from one boot to the next. For CXL memdevs this is done by the
'serial' attribute which is always stable regardless of the device init
order. That's harder to get to from the pmu device because it may be
associated with a device that does not have a memdev.
I think it's also going to be frustrating for userspace to see
randomized pmu ids across devices since that probing will happen in
parallel. So how about:
1/ Add serial as an attribute for each PMU to export
2/ Change the device name format to be "pmuX.Y" where X can just reuse
the memdev id for endpoints and be another value for switches, and Y is
guaranteed to be 0-based and in hardware discovery order.
...with that, someone can write a udev script that can persistently
identify PMU[Y] on device[serial] each boot.
That also cleans up a /sys/bus/cxl/devices listing to make it clear
which pmu instances belong together.
> +
> +PMU driver provides description of available events and filter options in sysfs.
> +
> +The "format" directory describes all formats of the config (event vendor id,
> +group id and mask) config1 (threshold, filter enables) and config2 (filter
> +parameters) fields of the perf_event_attr structure. The "events" directory
> +describes all documented events show in perf list.
> +
> +The events shown in perf list are the most fine grained events with a single
> +bit of the event mask set. More general events may be enable by setting
> +multiple mask bits in config. For example, all Device to Host Read Requests
> +may be captured on a single counter by setting the bits for all of
> +
> +* d2h_req_rdcurr
> +* d2h_req_rdown
> +* d2h_req_rdshared
> +* d2h_req_rdany
> +* d2h_req_rdownnodata
> +
> +Example of usage::
> +
> + $#perf list
> + cpmu0/clock_ticks/ [Kernel PMU event]
> + cpmu0/d2h_req_itomwr/ [Kernel PMU event]
> + cpmu0/d2h_req_rdany/ [Kernel PMU event]
> + cpmu0/d2h_req_rdcurr/ [Kernel PMU event]
> + -----------------------------------------------------------
> +
> + $# perf stat -e cpmu0/clock_ticks/ -e cpmu0/d2h_req_itowrm/
Ah here's the examples I was looking for in the last patch, nice.
> +
> +Vendor specific events may also be available and if so can be used via
> +
> + $# perf stat -e cpmu0/vid=VID,gid=GID,mask=MASK/
> +
> +The driver does not support sampling. So "perf record" and attaching to
> +a task are unsupported.
Is this a common restriction for CPU-external pmus, or do you see
sampling support required to get this upstream?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 3/5] cxl/pci: Find and register CXL PMU devices
2023-04-04 19:17 ` Dan Williams
@ 2023-04-05 10:48 ` Jonathan Cameron
0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2023-04-05 10:48 UTC (permalink / raw)
To: Dan Williams
Cc: Liang Kan, linux-cxl, peterz, mingo, acme, mark.rutland, will,
linuxarm, linux-perf-users, linux-kernel, Davidlohr Bueso,
Dave Jiang
On Tue, 4 Apr 2023 12:17:01 -0700
Dan Williams <dan.j.williams@intel.com> wrote:
> Jonathan Cameron wrote:
> > CXL PMU devices can be found from entries in the Register
> > Locator DVSEC.
> >
> > In order to register the minimum number of IRQ vectors necessary
> > to support all CPMUs found, separate the registration into two
> > steps. First find the devices, and query the IRQs used and then
> > register the devices. Between these two steps, request the
> > IRQ vectors necessary and enable bus master support.
>
> It's not clear why this patch is talking about irq vectors and bus
> mastering when there is no irq query/setup logic in this patch?
>
> > Future IRQ users for CXL type 3 devices (e.g. DOEs) will need to
> > follow a similar pattern the number of vectors necessary is known
> > before any parts of the driver stack rely on their availability.
>
> With the new pci_msix_alloc_irq_at() it's not clear that this 2 step
> approach is required, right?
Stale description. Will drop all that garbage.
>
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> > v4:
> > - No change.
> > ---
> > drivers/cxl/core/Makefile | 1 +
> > drivers/cxl/core/core.h | 1 +
> > drivers/cxl/core/cpmu.c | 72 +++++++++++++++++++++++++++++++++++++++
>
> A quibble with the naming, I prefer:
>
> drivers/cxl/core/pmu.c
>
> ...since "cxl" is in the directory path. Also, usages of cpmu
> already have a cxl in their symbol names, so just s/cpmu/pmu/ throught.
> The usage of CPMU_ for register macros would seem be more clear, or at
> least more consistent, as CXL_PMU_ like the other register offset
> definitions in cxlpci.h.
Makes sense. I'll leave the register defs as
CPMU_XXX to keep them compact but use the pmu naming for pretty much everything else.
>
> > drivers/cxl/core/port.c | 2 ++
> > drivers/cxl/core/regs.c | 16 +++++++++
> > drivers/cxl/cpmu.h | 56 ++++++++++++++++++++++++++++++
>
> drivers/cxl/pmu.h
>
> > drivers/cxl/cxl.h | 14 ++++++++
> > drivers/cxl/cxlpci.h | 1 +
> > drivers/cxl/pci.c | 25 +++++++++++++-
> > 9 files changed, 187 insertions(+), 1 deletion(-)
>
> Other than those minor issues above, this looks good to me, with those
> fixed up.
>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Thanks,
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 4/5] cxl: CXL Performance Monitoring Unit driver
2023-04-04 21:53 ` Dan Williams
@ 2023-04-05 16:08 ` Jonathan Cameron
2023-04-05 19:26 ` Dan Williams
0 siblings, 1 reply; 21+ messages in thread
From: Jonathan Cameron @ 2023-04-05 16:08 UTC (permalink / raw)
To: Dan Williams
Cc: Liang Kan, linux-cxl, peterz, mingo, acme, mark.rutland, will,
linuxarm, linux-perf-users, linux-kernel, Davidlohr Bueso,
Dave Jiang
Hi Dan,
Thanks for taking a look.
> > Development done with QEMU model which will be posted shortly.
>
> Per the comment on the cover letter is there a sense that set of
> counters supported in QEMU and this patchset are representative of a
> "common" set even if that is by convention and not specification?Just
> trying to get a sense of how many real world CXL PMU instances this
> might cover.
Good question. Events wise, the driver supports all the Spec defined events
(for the Vendor defined ones they "work" but are not described in this initial
version so you have to know the magic numbers). In discussion with Kan I think
we've established what discoverability will look like but it is a substantial
addition so I was planning that for a follow up series.
Features wise things I expect in hardware that don't 'yet' support include:
1) Implementations without freeze. They require more complex handling of
start / stop (and will never work as well as you can't start sets of
counters with one register right). Easy to see how to do this but
postponed for a follow up set.
2) Free running counters. Chances are we'll see these for vendor defined events
as they are a cheap way of getting support for stuff that is really debug related.
Again, well understood how to do this but adds more complexity to the driver
so postponed for future work.
So after this series merges, I'd expect to see 3 follow-ups to add each of the
above features, probably in the order:
1) Discoverability.
2) Free running counters
3) Implementations without freeze.
(might do last two in one series)
Note that I haven't yet added configurability via command line to the the
QEMU model. The hard coded emulation has a fairly random set of events
and counters meant to hit corner cases (overlapping sets, mixture of
configurable and not) rather than represent a typical real device.
Once that has an interface to specify the CPMU options it'll be easier
to have some 'normal' cases to test and some more insane ones to hit
the corners.
>
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> One thing I would have liked to see is a sample perf command line and
> output.
Sure. I'll add one here and consider improving the docs on this.
>
> One concern I have is how usable DPA based perf events is going to be
> for end users. Is there a concept of a synthetic event that can turn
> data gathered from all the devices in an interleave set into HPA
> relative data? Or do you expect that is the responsibility of the perf
> tool to grow that support in userspace?
Given we need to fuse counters from multiple devices up in userspace
I'd expect us to do cross device aggregation in perf tool. Will be very
system dependent on whether such aggregation makes sense (if you can't monitor
what you need in host or at the HB level.).
> > ---
> > drivers/cxl/Kconfig | 13 +
> > drivers/cxl/Makefile | 1 +
> > drivers/cxl/cpmu.c | 940 +++++++++++++++++++++++++++++++++++++++++++
>
> Yeah, this is a bunch of code and ABI that is more relevant to drivers/perf/
> than drivers/cxl/, so this wants to move to drivers/perf/cxl.c.
For consistency with other drivers in there it will be drivers/perf/cxl_pmu.c
with cxl_pmu.ko as module.
It's currently a little messy to get the includes from
../cxl/ but not bad enough that I think it's worth ripping those headers
apart to move some of the content to include/linux/cxl so I'll go with
#include "../cxl/cxlpci.h" (needed for the DVSEC ID).
#include "../cxl/pmu.h" (with stuff drivers/cxl doesn't need pushed into the cxl_pmu.c)
#include "../cxl/cxl.h" (For cxl_driver)
Similar to done in drivers/dax/cxl.c
>
> > diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> > index ff4e78117b31..0be514b00b8f 100644
> > --- a/drivers/cxl/Kconfig
> > +++ b/drivers/cxl/Kconfig
> > @@ -139,4 +139,17 @@ config CXL_REGION_INVALIDATION_TEST
> > If unsure, or if this kernel is meant for production environments,
> > say N.
> >
> > +config CXL_CPMU
> > + tristate "CXL Performance Monitoring Unit"
> > + default CXL_BUS
> > + depends on PERF_EVENTS
> > + help
> > + Support performance monitoring as defined in CXL rev 3.0
> > + section 13.2: Performance Monitoring. CXL components may have
> > + one or more CXL Performance Monitoring Units (CPMUs).
> > +
> > + Say 'y/m' to enable a driver that will attach to performance
> > + monitoring units and provide standard perf based interfaces.
> > +
> > + If unsure say 'm'.
> > endif
>
> ...and this wants to move to drivers/perf/Kconfig as well.
>
> It's similar to "CONFIG_DEV_DAX_CXL" as an external to drivers/cxl/
> cxl-driver.
Ah. I'd failed to notice that. Good example.
>
> > diff --git a/drivers/cxl/Makefile b/drivers/cxl/Makefile
> > index db321f48ba52..024bb739554b 100644
> > --- a/drivers/cxl/Makefile
> > +++ b/drivers/cxl/Makefile
> > @@ -5,6 +5,7 @@ obj-$(CONFIG_CXL_MEM) += cxl_mem.o
> > obj-$(CONFIG_CXL_ACPI) += cxl_acpi.o
> > obj-$(CONFIG_CXL_PMEM) += cxl_pmem.o
> > obj-$(CONFIG_CXL_PORT) += cxl_port.o
> > +obj-$(CONFIG_CXL_CPMU) += cpmu.o
> >
> > cxl_mem-y := mem.o
> > cxl_pci-y := pci.o
> > diff --git a/drivers/cxl/cpmu.c b/drivers/cxl/cpmu.c
> > new file mode 100644
> > index 000000000000..e6821771a4f6
> > --- /dev/null
> > +++ b/drivers/cxl/cpmu.c
> > @@ -0,0 +1,940 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +/*
> > + * Copyright(c) 2023 Huawei
> > + *
> > + * The CXL 3.0 specification includes a standard Performance Monitoring Unit,
> > + * called the CXL PMU, or CPMU. In order to allow a high degree of
> > + * implementation flexibility the specification provides a wide range of
> > + * options all of which are self describing.
> > + *
> > + * Details in CXL rev 3.0 section 8.2.7 CPMU Register Interface
> > + *
> > + * TODO
> > + * o Discoverability of counters. Allow perftool to provide summed counters
> > + * and vendor defined counters.
> > + * o Support free running counters - copy the Intel uncore PMU handling for these.
> > + * o CPMUs which do not support freeze.
>
> Wouldn't this be driven by the arrival of hardware? I.e. is there
It's a really ugly limitation on hardware that chooses not to implement freeze +
write when frozen as it will introduce a lot of skid across different counters.
Where I have influence I'll be pushing for no one to make that choice as it
reduces the usefulness of the PMU (how much depends on how fast / variable
the thing being counted is) There is an intermediate dance where you can only
write when disabled and it is more functional but the code more complex than
we have here.
Fairly easy to implement (I think we must need to walk all the individual enable
registers) but this initial submission is big enough already so I'm not
keen to add that until a follow up patch set.
I'll make it a bit more explicit in the cover letter than I expect this
to be only the initial submission and call out what I expect to be supported
via future patch sets.
> > + * o Add filter validation in cpmu_event_init() so problems are detected earlier.
> > + * o Reject configurations that the hardware is ignoring
> > + * (e.g. invert when not invertible)
> > + * o Support CPMUs with no interrupts using an HRTIMER.
>
> Is it worthwhile to maintain the TODO list in the code? I just fear this
> getting stale over time.
There is already some text covering the most important of these in the patch
description so I'll drop it from here. Patch descriptions shouldn't ever
be stale (says the person who just deleted the garbage that was stale in
previous patch... :(
> > +/* CXL rev 3.0 Table 13-5 Events under CXL Vendor ID */
> > +#define CPMU_GID_CLOCK_TICKS 0x00
> > +#define CPMU_GID_D2H_REQ 0x0010
> > +#define CPMU_GID_D2H_RSP 0x0011
> > +#define CPMU_GID_H2D_REQ 0x0012
> > +#define CPMU_GID_H2D_RSP 0x0013
> > +#define CPMU_GID_CACHE_DATA 0x0014
> > +#define CPMU_GID_M2S_REQ 0x0020
> > +#define CPMU_GID_M2S_RWD 0x0021
> > +#define CPMU_GID_M2S_BIRSP 0x0022
> > +#define CPMU_GID_S2M_BISNP 0x0023
> > +#define CPMU_GID_S2M_NDR 0x0024
> > +#define CPMU_GID_S2M_DRS 0x0025
> > +#define CPMU_GID_DDR 0x8000
>
> Last note about the naming quibble, I notice that CPMU exists in other
> places in the kernel, but CXL_PMU does not, so there is a small
> grep'ability win as well.
You mean unlike CXL in drivers/misc/cxl :)
I was going to leave these definitions alone, but fair enough I'll do these as well.
Anyone with a narrow terminal can blame you for the line wraps ;).
> > +
> > +#define CPMU_MAX_COUNTERS 64
> > +struct cpmu_info {
> > + struct pmu pmu;
> > + void __iomem *base;
> > + struct perf_event **hw_events;
> > + struct list_head events_configurable;
> > + struct list_head events_fixed;
>
> Linked lists? Didn't xarray kill the linked list?
No. Kan raised similar in an earlier review and I couldn't come
up with a way that worked (for the configurable ones anyway)
How would you index these given they are defined by a full 64
bits made up of VID, GID and MASK + we need to match in some
cases by exact mask (for events_fixed) and sometimes as a subset
(for events_configurable)? I see below you say we could make
this dependent on 64BIT to handle the fixed counters list but
that seems ugly to me.
There might be some magic data structure I'm unaware of we could
use but why bother? Expectation is these will have only a small
number of elements so a list is fine.
Looking at it again the naming is a bit misleading. These aren't
generally individual events but instead the "event capabilities".
Let me rename them to make that clear. event_caps_configurable,
even_caps_fixed + rename the struct cxl_pmu_event to cxl_pmu_ev_cap
As such the absolute maximum length of the sum of those two lists is
32. Most like they'll be a lot shorter than that.
>
> > + DECLARE_BITMAP(used_counter_bm, CPMU_MAX_COUNTERS);
> > + DECLARE_BITMAP(conf_counter_bm, CPMU_MAX_COUNTERS);
> > + u16 counter_width;
> > + u8 num_counters;
> > + u8 num_event_capabilities;
> > + int on_cpu;
> > + struct hlist_node node;
> > + bool freeze_for_enable;
> > + bool filter_hdm;
> > + int irq;
> > +};
> > +
> > +#define pmu_to_cpmu_info(_pmu) container_of(_pmu, struct cpmu_info, pmu)
> > +
> > +/*
> > + * All CPMU counters are discoverable via the Event Capabilities Registers.
> > + * Each Event Capability register contains a a VID / GroupID.
> > + * A counter may then count any combination (by summing) of events in
> > + * that group which are in the Supported Events Bitmask.
> > + * However, there are some complexities to the scheme.
> > + * - Fixed function counters refer to an Event Capabilities register.
> > + * That event capability register is not then used for Configurable
> > + * counters.
> > + */
> > +static int cpmu_parse_caps(struct device *dev, struct cpmu_info *info)
> > +{
> > + DECLARE_BITMAP(fixed_counter_event_cap_bm, 32) = {0};
>
> An 'unsigned long' is always at least 32-bits, and:
>
> unsigned long fixed_counter_event_cap_bm = 0;
>
> ...would not have interrupted my reading of the code.
Hmm. I'm in two minds about this. Given we are accessing it with
bitmap walking macros and hence this bitmap being replace by
an unsigned long (unlike the bigger ones) requires a bunch of
&fixed_counter_event_cap_bm I think it is messier to change.
Don't feel that strongly though so made the change.
>
> > + void __iomem *base = info->base;
> > + u64 val, eval;
> > + int i;
> > +
> > + val = readq(base + CPMU_CAP_REG);
> > + info->freeze_for_enable = FIELD_GET(CPMU_CAP_WRITEABLE_WHEN_FROZEN, val) &
> > + FIELD_GET(CPMU_CAP_FREEZE, val);
>
> Why does 'struct cpmu_info' need to carry the freeze_for_enable flag if
> its only used to gate loading the driver?
>
> When / if support for "freeze_for_enable == false" arrives then I can
> see 'struct cpmu_info' having a need to cache it.
Ok. I'll drop it for now.
>
> > + if (!info->freeze_for_enable) {
> > + dev_err(dev, "Driver does not support CPMUs that do not support freeze for enable\n");
>
> Perhaps just:
>
> "Counters not writable while frozen."
>
> ...will suffice, no need to mention what the driver supports, that's
> implied.
Ok. I think it looses some subtlety but I guess it's enough for anyone
who hasn't spent far too long discussing this flow on zoom calls.
...
> > +static struct cpmu_event *cpmu_find_fixed_counter_event(struct cpmu_info *info,
> > + int vid, int gid, int msk)
> > +{
> > + struct cpmu_event *cpmu_ev;
> > +
> > + list_for_each_entry(cpmu_ev, &info->events_fixed, node) {
> > + if (vid != cpmu_ev->vid || gid != cpmu_ev->gid)
> > + continue;
> > +
> > + /* Precise match for fixed counter */
> > + if (msk == cpmu_ev->msk)
> > + return cpmu_ev;
>
> It looks like the lookup parameters fit in 64-bits which if using an
> xarray could do a direct lookup (assuming this driver 'depends on 64BIT'
> which is not unreasonable).
We can't do that for the configurable counters because of the more complex
matching and I'd rather keep these looking similar.
Also as noted above these are short lists but the naming implied
they were a lot longer than in reality by suggesting they were lists
of individual events supported.
>
> > + }
> > +
> > + return ERR_PTR(-EINVAL);
> > +}
> > +
> > +
> > +static void cpmu_event_start(struct perf_event *event, int flags)
> > +{
> > + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> > + struct hw_perf_event *hwc = &event->hw;
> > + void __iomem *base = info->base;
> > + u64 cfg;
> > +
> > + if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
> > + return;
> > +
> > + WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
>
> Maybe a comment about why these warns will likely never fire in practice?
Hmm. Good question (I cut and paste without thinking about these :)
start is only either called:
a) from cxl_pmu_event_add() where these flags are set directly
b) from a few places in kernel/events/core.c
__perf_event_stop() (with restart set)
perf_adjust_period().
there pmu->stop() is called with PERF_EF_UPDATE and we set
the flags in our callback.
perf_adjust_freq_unthr_context()
One path calls pmu->stop() as above.
One path perf_pmu_disable() has just been called.
this is only path I can find that might leave these two flags not set
but I am fairly sure we can't trigger it because (in common with most
drivers in drivers/perf) we aren't counting interrupts as necessary to
trigger this path.
If anyone more familiar with perf can confirm / correct my logic
it would be much appreciated.
For now I'll just add a comment that we should only reach here via
the stop call or with the state explicitly set and hence these flags should
be set appropriately.
...
>
> > + if (rc)
> > + return rc;
> > + info->irq = irq;
> > +
> > + rc = cpuhp_state_add_instance(cpmu_cpuhp_state_num, &info->node);
> > + if (rc)
> > + return rc;
> > +
> > + rc = perf_pmu_register(&info->pmu, info->pmu.name, -1);
> > + if (rc)
> > + return rc;
> > +
> > + dev_set_drvdata(dev, info);
>
> Is it really the case that drvdata can be set after the pmu is already
> registered and live, or maybe just getting lucky with accessing the ABI
> after probe returns?
Lucky. Fixed.
>
> > +
> > + return 0;
> > +}
> > +
> > +static void cxl_cpmu_remove(struct device *dev)
> > +{
> > + struct cpmu_info *info = dev_get_drvdata(dev);
> > +
> > + perf_pmu_unregister(&info->pmu);
> > + cpuhp_state_remove_instance_nocalls(cpmu_cpuhp_state_num, &info->node);
>
> How about add devm_add_action_or_reset() calls for these and delete
> cxl_cpmu_remove()?
I agree. Though first time this gets done in a subsystem tends to be
'interesting'. Guess I'll give it a go.
>
> > +}
> > +
> > +static struct cxl_driver cxl_cpmu_driver = {
> > + .name = "cxl_cpmu",
> > + .probe = cxl_cpmu_probe,
> > + .remove = cxl_cpmu_remove,
> > + .id = CXL_DEVICE_CPMU,
> > +};
> > +
> > +static int cpmu_online_cpu(unsigned int cpu, struct hlist_node *node)
> > +{
> > + struct cpmu_info *info = hlist_entry_safe(node, struct cpmu_info, node);
> > +
> > + if (info->on_cpu != -1)
> > + return 0;
> > +
> > + info->on_cpu = cpu;
> > + WARN_ON(irq_set_affinity(info->irq, cpumask_of(cpu)));
>
> Is this really a kernel crashable scenario for folks that have
> panic_on_warn() set? If it's a "should never" happen type situation then
> maybe a comment, otherwise pr_warn().
Will see if I can come up with a comment beyond "shouldn't happen" as
if a hotplug notifier says it's online it better still be online within
that notifier callback.
I think the key here is these are called with the cpuhp lock held and as
such there is no race and we shoul always be able to set the irq affinity
appropriately.
>
> > +
> > + return 0;
> > +}
> > +
> > +static int cpmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> > +{
> > + struct cpmu_info *info = hlist_entry_safe(node, struct cpmu_info, node);
> > + unsigned int target;
> > +
> > + if (info->on_cpu != cpu)
> > + return 0;
> > +
> > + info->on_cpu = -1;
> > + target = cpumask_first(cpu_online_mask);
Bug noticed here whilst chasing down the comment for below. Should be cpumask_any_but()
as at time of calling the cpu that is going offline is still in the cpu_online_mask()
> > + if (target >= nr_cpu_ids) {
> > + dev_err(info->pmu.dev, "Unable to find a suitable CPU\n");
> > + return 0;
> > + }
> > +
> > + perf_pmu_migrate_context(&info->pmu, cpu, target);
> > + info->on_cpu = target;
> > + WARN_ON(irq_set_affinity(info->irq, cpumask_of(target)));
> > +
> > + return 0;
> > +}
...
> > 2.37.2
> >
>
> This looks good to me, I'd be happy to take it through the CXL tree when
> it finalizes with perf folks acks, just let me know.
>
Thanks
Jonathan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 4/5] cxl: CXL Performance Monitoring Unit driver
2023-04-05 16:08 ` Jonathan Cameron
@ 2023-04-05 19:26 ` Dan Williams
0 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2023-04-05 19:26 UTC (permalink / raw)
To: Jonathan Cameron, Dan Williams
Cc: Liang Kan, linux-cxl, peterz, mingo, acme, mark.rutland, will,
linuxarm, linux-perf-users, linux-kernel, Davidlohr Bueso,
Dave Jiang
Jonathan Cameron wrote:
> Hi Dan,
>
> Thanks for taking a look.
>
> > > Development done with QEMU model which will be posted shortly.
> >
> > Per the comment on the cover letter is there a sense that set of
> > counters supported in QEMU and this patchset are representative of a
> > "common" set even if that is by convention and not specification?Just
> > trying to get a sense of how many real world CXL PMU instances this
> > might cover.
>
> Good question. Events wise, the driver supports all the Spec defined events
> (for the Vendor defined ones they "work" but are not described in this initial
> version so you have to know the magic numbers). In discussion with Kan I think
> we've established what discoverability will look like but it is a substantial
> addition so I was planning that for a follow up series.
>
> Features wise things I expect in hardware that don't 'yet' support include:
> 1) Implementations without freeze. They require more complex handling of
> start / stop (and will never work as well as you can't start sets of
> counters with one register right). Easy to see how to do this but
> postponed for a follow up set.
> 2) Free running counters. Chances are we'll see these for vendor defined events
> as they are a cheap way of getting support for stuff that is really debug related.
> Again, well understood how to do this but adds more complexity to the driver
> so postponed for future work.
>
> So after this series merges, I'd expect to see 3 follow-ups to add each of the
> above features, probably in the order:
>
> 1) Discoverability.
> 2) Free running counters
> 3) Implementations without freeze.
I feel like this one might be an "only if a device shows up" situation,
but the others seems reasonable to plan in advance of practical hardware
examples. I think it's ok for Linux to be prickly towards hardware
implementations that make our lives harder.
> > One thing I would have liked to see is a sample perf command line and
> > output.
>
> Sure. I'll add one here and consider improving the docs on this.
I did eventually find the Documentation, but no harm putting an example
in here too...
> > One concern I have is how usable DPA based perf events is going to be
> > for end users. Is there a concept of a synthetic event that can turn
> > data gathered from all the devices in an interleave set into HPA
> > relative data? Or do you expect that is the responsibility of the perf
> > tool to grow that support in userspace?
>
> Given we need to fuse counters from multiple devices up in userspace
> I'd expect us to do cross device aggregation in perf tool. Will be very
> system dependent on whether such aggregation makes sense (if you can't monitor
> what you need in host or at the HB level.).
Ok, makes sense.
> > > drivers/cxl/Kconfig | 13 +
> > > drivers/cxl/Makefile | 1 +
> > > drivers/cxl/cpmu.c | 940 +++++++++++++++++++++++++++++++++++++++++++
> >
> > Yeah, this is a bunch of code and ABI that is more relevant to drivers/perf/
> > than drivers/cxl/, so this wants to move to drivers/perf/cxl.c.
> For consistency with other drivers in there it will be drivers/perf/cxl_pmu.c
> with cxl_pmu.ko as module.
Sure.
> It's currently a little messy to get the includes from
> ../cxl/ but not bad enough that I think it's worth ripping those headers
> apart to move some of the content to include/linux/cxl so I'll go with
> #include "../cxl/cxlpci.h" (needed for the DVSEC ID).
> #include "../cxl/pmu.h" (with stuff drivers/cxl doesn't need pushed into the cxl_pmu.c)
> #include "../cxl/cxl.h" (For cxl_driver)
>
> Similar to done in drivers/dax/cxl.c
Works for me or:
CFLAGS_cxl_pmu.o := -I$(srctree)/drivers/cxl/
...in drivers/perf/Makefile.
[..]
> > Is it worthwhile to maintain the TODO list in the code? I just fear this
> > getting stale over time.
>
> There is already some text covering the most important of these in the patch
> description so I'll drop it from here. Patch descriptions shouldn't ever
> be stale (says the person who just deleted the garbage that was stale in
> previous patch... :(
Hey, I resemble this remark, it happens.
>
> > > +/* CXL rev 3.0 Table 13-5 Events under CXL Vendor ID */
> > > +#define CPMU_GID_CLOCK_TICKS 0x00
> > > +#define CPMU_GID_D2H_REQ 0x0010
> > > +#define CPMU_GID_D2H_RSP 0x0011
> > > +#define CPMU_GID_H2D_REQ 0x0012
> > > +#define CPMU_GID_H2D_RSP 0x0013
> > > +#define CPMU_GID_CACHE_DATA 0x0014
> > > +#define CPMU_GID_M2S_REQ 0x0020
> > > +#define CPMU_GID_M2S_RWD 0x0021
> > > +#define CPMU_GID_M2S_BIRSP 0x0022
> > > +#define CPMU_GID_S2M_BISNP 0x0023
> > > +#define CPMU_GID_S2M_NDR 0x0024
> > > +#define CPMU_GID_S2M_DRS 0x0025
> > > +#define CPMU_GID_DDR 0x8000
> >
> > Last note about the naming quibble, I notice that CPMU exists in other
> > places in the kernel, but CXL_PMU does not, so there is a small
> > grep'ability win as well.
>
> You mean unlike CXL in drivers/misc/cxl :)
Touché.
> I was going to leave these definitions alone, but fair enough I'll do these as well.
> Anyone with a narrow terminal can blame you for the line wraps ;).
It was really the devm_cxl_add_cpmu() that got me into this naming
tangent, I admit it's a personal preference to remove redundancy when
pathnames or other prefixes cover.
> > > +#define CPMU_MAX_COUNTERS 64
> > > +struct cpmu_info {
> > > + struct pmu pmu;
> > > + void __iomem *base;
> > > + struct perf_event **hw_events;
> > > + struct list_head events_configurable;
> > > + struct list_head events_fixed;
> >
> > Linked lists? Didn't xarray kill the linked list?
>
> No. Kan raised similar in an earlier review and I couldn't come
> up with a way that worked (for the configurable ones anyway)
>
> How would you index these given they are defined by a full 64
> bits made up of VID, GID and MASK + we need to match in some
> cases by exact mask (for events_fixed) and sometimes as a subset
> (for events_configurable)? I see below you say we could make
> this dependent on 64BIT to handle the fixed counters list but
> that seems ugly to me.
>
> There might be some magic data structure I'm unaware of we could
> use but why bother? Expectation is these will have only a small
> number of elements so a list is fine.
>
> Looking at it again the naming is a bit misleading. These aren't
> generally individual events but instead the "event capabilities".
> Let me rename them to make that clear. event_caps_configurable,
> even_caps_fixed + rename the struct cxl_pmu_event to cxl_pmu_ev_cap
Ah, I indeed was thinking that this was lists of events, not
capabilities.
> As such the absolute maximum length of the sum of those two lists is
> 32. Most like they'll be a lot shorter than that.
Even without the (too?) fancy hack to try to make the (vid, did, mask)
tuple the xarray lookup key, you can still replace a doubly-linked list
with an xarray indexed by the pointer value:
xa_insert(&info->events_configurable, (unsigned long)ev, ev, GFP_KERNEL);
...and then walk it with xa_for_each(). Given the small list sizes the
benefit would be more for readability in the sense that I can look at
xa_for_each() and not worry about the locking context, but not with
list_for_each_entry(). I won't push hard on this since these lists are
small and not going to grow beyond 32.
> > > + DECLARE_BITMAP(used_counter_bm, CPMU_MAX_COUNTERS);
> > > + DECLARE_BITMAP(conf_counter_bm, CPMU_MAX_COUNTERS);
> > > + u16 counter_width;
> > > + u8 num_counters;
> > > + u8 num_event_capabilities;
> > > + int on_cpu;
> > > + struct hlist_node node;
> > > + bool freeze_for_enable;
> > > + bool filter_hdm;
> > > + int irq;
> > > +};
> > > +
> > > +#define pmu_to_cpmu_info(_pmu) container_of(_pmu, struct cpmu_info, pmu)
> > > +
> > > +/*
> > > + * All CPMU counters are discoverable via the Event Capabilities Registers.
> > > + * Each Event Capability register contains a a VID / GroupID.
> > > + * A counter may then count any combination (by summing) of events in
> > > + * that group which are in the Supported Events Bitmask.
> > > + * However, there are some complexities to the scheme.
> > > + * - Fixed function counters refer to an Event Capabilities register.
> > > + * That event capability register is not then used for Configurable
> > > + * counters.
> > > + */
> > > +static int cpmu_parse_caps(struct device *dev, struct cpmu_info *info)
> > > +{
> > > + DECLARE_BITMAP(fixed_counter_event_cap_bm, 32) = {0};
> >
> > An 'unsigned long' is always at least 32-bits, and:
> >
> > unsigned long fixed_counter_event_cap_bm = 0;
> >
> > ...would not have interrupted my reading of the code.
>
> Hmm. I'm in two minds about this. Given we are accessing it with
> bitmap walking macros and hence this bitmap being replace by
> an unsigned long (unlike the bigger ones) requires a bunch of
> &fixed_counter_event_cap_bm I think it is messier to change.
>
> Don't feel that strongly though so made the change.
If there were a BITMAP() macro that did the declaration and init,
similar to LIST_HEAD() then I think I would not have reacted. It's really
the "DECLARE_BITMAP() = { 0 }" that bothered me.
[..]
> > > +static void cpmu_event_start(struct perf_event *event, int flags)
> > > +{
> > > + struct cpmu_info *info = pmu_to_cpmu_info(event->pmu);
> > > + struct hw_perf_event *hwc = &event->hw;
> > > + void __iomem *base = info->base;
> > > + u64 cfg;
> > > +
> > > + if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
> > > + return;
> > > +
> > > + WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
> >
> > Maybe a comment about why these warns will likely never fire in practice?
>
> Hmm. Good question (I cut and paste without thinking about these :)
>
> start is only either called:
> a) from cxl_pmu_event_add() where these flags are set directly
> b) from a few places in kernel/events/core.c
> __perf_event_stop() (with restart set)
> perf_adjust_period().
> there pmu->stop() is called with PERF_EF_UPDATE and we set
> the flags in our callback.
>
> perf_adjust_freq_unthr_context()
> One path calls pmu->stop() as above.
> One path perf_pmu_disable() has just been called.
> this is only path I can find that might leave these two flags not set
> but I am fairly sure we can't trigger it because (in common with most
> drivers in drivers/perf) we aren't counting interrupts as necessary to
> trigger this path.
>
> If anyone more familiar with perf can confirm / correct my logic
> it would be much appreciated.
>
> For now I'll just add a comment that we should only reach here via
> the stop call or with the state explicitly set and hence these flags should
> be set appropriately.
That works for me, maybe a check_hw_perf_event_state() helper could
centralize these common assertions in the perf core?
[..]
> > > +static int cpmu_online_cpu(unsigned int cpu, struct hlist_node *node)
> > > +{
> > > + struct cpmu_info *info = hlist_entry_safe(node, struct cpmu_info, node);
> > > +
> > > + if (info->on_cpu != -1)
> > > + return 0;
> > > +
> > > + info->on_cpu = cpu;
> > > + WARN_ON(irq_set_affinity(info->irq, cpumask_of(cpu)));
> >
>
> > Is this really a kernel crashable scenario for folks that have
> > panic_on_warn() set? If it's a "should never" happen type situation then
> > maybe a comment, otherwise pr_warn().
>
> Will see if I can come up with a comment beyond "shouldn't happen" as
> if a hotplug notifier says it's online it better still be online within
> that notifier callback.
>
> I think the key here is these are called with the cpuhp lock held and as
> such there is no race and we shoul always be able to set the irq affinity
> appropriately.
I have started to make sure all my WARN_ONs are sane after Greg pointed
out that panic_on_warn() has users.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 5/5] docs: perf: Minimal introduction the the CXL PMU device and driver
2023-04-04 22:24 ` Dan Williams
@ 2023-04-06 16:33 ` Jonathan Cameron
0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2023-04-06 16:33 UTC (permalink / raw)
To: Dan Williams
Cc: Liang Kan, linux-cxl, peterz, mingo, acme, mark.rutland, will,
linuxarm, linux-perf-users, linux-kernel, Davidlohr Bueso,
Dave Jiang
>
> > +
> > + /sys/bus/cxl/device/cpmu<id>
> > +
> > +The associated PMU is registered as
> > +
> > + /sys/bus/event_sources/devices/cpmu<id>
> > +
> > +In common with other CXL bus devices, the id has no specific meaning and the
> > +relationship to specific CXL device should be established via the device parent
> > +of the device on the CXL bus.
>
> So I went to go add some text about how to identify PMUs in a persistent
> manner from one boot to the next. For CXL memdevs this is done by the
> 'serial' attribute which is always stable regardless of the device init
> order. That's harder to get to from the pmu device because it may be
> associated with a device that does not have a memdev.
>
> I think it's also going to be frustrating for userspace to see
> randomized pmu ids across devices since that probing will happen in
> parallel. So how about:
Solving this in general for perf PMU drivers was what the parent device thing
was about. There is an argument that enabling any other path to get to
this association is both unnecessary and just possibly unwise.
The nice advantage of just using an IDA and relying on parentage for the
association was that I could avoid naming questions for all the other
places these might turn in a CXL topology. The Lazy / efficient option ;)
You can now see exactly which PCI device a given instance is associated with.
Custom ABI is going to be harder for anyone to use than that.
I suppose we can potentially enable both paths - but it's not quite
as straight forwards as you suggest.
>
> 1/ Add serial as an attribute for each PMU to export
Where does it come from? We only have one source of serial number per device.
That's no where near enough to work out where a PMU is.
> 2/ Change the device name format to be "pmuX.Y" where X can just reuse
Could use something a little more detailed cxl bus, but the one registered and use
to address this in bus/event_sources needs to be cxl specific so a cxl_ prefix
is needed I think
Given we need to namespace what the ids refer to, I'm currently going with
pmu_memX.Y pmu_dspX.Y.Z pmu_uspX.Y
on the cxl bus and
cxl_pmu_memX.Y cxl_pmu_dspX.Y.Z cxl_pmu_uspX.Y on even sources bus.
(Z needed because dsp index from 0 for each usp)
We can figure out what to do about other CXL EPs later and for now at least
there is no way to hand a CPMU instance off a host bridge (nothing in CEDT
to tell you where to find it).
I've had a fun day hacking PMUs onto the other emulated CXL devices to test
this.
There is a can of worms I'll avoid for this series by just sticking to type3
device PMUs for now.
I have no idea yet how we handle the interrupts safely for ports as those
interrupts are in control the pcie port driver not the CXL dport one.
At somepoint I'll send out an RFC about that if no one gets to it before
me. For now I've hacked portdrv to always allocate max vectors and am ignoring the
lovely back traces due to thing getting torn down in the wrong order on shutdown.
For upstream ports I've hacked portdrv to pretend it knows there is something to handle.
As a starting point I think we'll need to teach portdrv enough about CXL
to be able to tell if it should provide interrupt services..
Hence I'll keep the code to register the other PMUs for a future patch set
and just make sure the code is structured to enable that in this series.
> the memdev id for endpoints and be another value for switches, and Y is
> guaranteed to be 0-based and in hardware discovery order.
Also need to change registration order as PMUs were registered before the
memdev, but that's easy enough to do.
>
> ...with that, someone can write a udev script that can persistently
> identify PMU[Y] on device[serial] each boot.
>
> That also cleans up a /sys/bus/cxl/devices listing to make it clear
> which pmu instances belong together.
>
> > +
> > +PMU driver provides description of available events and filter options in sysfs.
> > +
> > +The "format" directory describes all formats of the config (event vendor id,
> > +group id and mask) config1 (threshold, filter enables) and config2 (filter
> > +parameters) fields of the perf_event_attr structure. The "events" directory
> > +describes all documented events show in perf list.
> > +
> > +The events shown in perf list are the most fine grained events with a single
> > +bit of the event mask set. More general events may be enable by setting
> > +multiple mask bits in config. For example, all Device to Host Read Requests
> > +may be captured on a single counter by setting the bits for all of
> > +
> > +* d2h_req_rdcurr
> > +* d2h_req_rdown
> > +* d2h_req_rdshared
> > +* d2h_req_rdany
> > +* d2h_req_rdownnodata
> > +
> > +Example of usage::
> > +
> > + $#perf list
> > + cpmu0/clock_ticks/ [Kernel PMU event]
> > + cpmu0/d2h_req_itomwr/ [Kernel PMU event]
> > + cpmu0/d2h_req_rdany/ [Kernel PMU event]
> > + cpmu0/d2h_req_rdcurr/ [Kernel PMU event]
> > + -----------------------------------------------------------
> > +
> > + $# perf stat -e cpmu0/clock_ticks/ -e cpmu0/d2h_req_itowrm/
>
> Ah here's the examples I was looking for in the last patch, nice.
>
> > +
> > +Vendor specific events may also be available and if so can be used via
> > +
> > + $# perf stat -e cpmu0/vid=VID,gid=GID,mask=MASK/
> > +
> > +The driver does not support sampling. So "perf record" and attaching to
> > +a task are unsupported.
>
> Is this a common restriction for CPU-external pmus, or do you see
> sampling support required to get this upstream?
It's a common restriction. Whilst we could potentially implement sampling
based on the presence of a suitable clock_ticks event it don't see it
as a requirement initially.
Jonathan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support
2023-04-04 3:55 ` [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support Dan Williams
@ 2023-04-11 13:21 ` Jonathan Cameron
0 siblings, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2023-04-11 13:21 UTC (permalink / raw)
To: Dan Williams
Cc: Liang Kan, linux-cxl, peterz, mingo, acme, mark.rutland, will,
linuxarm, linux-perf-users, linux-kernel, Davidlohr Bueso,
Dave Jiang
On Mon, 3 Apr 2023 20:55:35 -0700
Dan Williams <dan.j.williams@intel.com> wrote:
> Jonathan Cameron wrote:
> > Changes since v3: Kan Liang gave feedback on v2 incorporated here.
> > - All updates in patch 4, see details there.
> >
> > Updated cover letter.
> >
> > The CXL rev 3.0 specification introduces a CXL Performance Monitoring
> > Unit definition. CXL components may have any number of these blocks. The
> > definition is highly flexible, but that does bring complexity in the
> > driver.
> >
> > Notes.
> >
> > 1) The QEMU model against which this was developed needs tidying up.
> > Latest tree at https://gitlab.com/jic23/qemu branch cxl-2023-02-28
> > It's backed up behind other series that I plan to upstream first.
> > 2) There are quite a lot of corner cases that will need working through
> > with variants of the model, or I'll have to design a pathological
> > set of CPMUs to hit all the corner cases in one go. So whilst I believe
> > the driver should be fine for what it supports we may find issues
> > as those corners of what is allowed are explored.
> > 3) I'm not sure it makes sense to hang this of the cxl/pci driver but
> > couldn't really figure out where else in the current structure we could
> > make it fit cleanly.
> > 4) Driver location. In past perf maintainers have requested perf drivers
> > for PCI etc be under drivers/perf. That would require moving some
> > CXL headers to be more generally visible, but is certainly possible
> > if there is agreement between CXL and perf maintainers on the correct
> > location.
>
> I am ok the bulk of the logic living under drivers/perf/
>
> Are drivers/perf/ folks ok with a:
>
> CFLAGS_cxl.o := -I$(srctree)/drivers/cxl/
>
> ...or similar in their Makefile, because I don't think this by itself is
> a reason to push CXL headers to include/.
For now I've gone with ../cxl/ which doesn't look too bad.
If cflags stuff is preferred can change that.
>
> I say this without having looked at the code yet and whether this driver
> will need new exports from the cxl/core etc.
>
> > 5) Patch 1 led to a discussion of how to handle the self describing
> > and extensible nature of the CPMU counters. That is likely to involve
> > a description in the "caps" sysfs directory and perf tool code that is
> > aware of the different event combinations that make sense and can
> > establish which sets are available on a given device.
> > That is likely to take a while to converge on, so as the driver is useful
> > in the current state, I'm looking to upstream this first and deal with
> > the more complex handling later.
>
> What's "this" in this last sentence, a canned set of base counters?
All the counters defined in the CXL spec itself without any summing of
counters (so you can't do all 'reads' for example without 'guessing' the
magic numbers).
>
> > 6) There is a lot of other functionality that can be added in future
> > include allowing for simpler hardware implementations that may not
> > support the minimum level of features currently required by the driver
> > (freeze, overflow interrupts etc).
>
> Curious if the the minimal set determined by the specification, like the
> minimal features a CXL Memory Expander device must implement, or by what
> you decided was fit to be emulated in QEMU?
There is no specification requirement to implement any particular subset
of counters or features for those counters. I think that's considered out
of scope for the CXL spec itself. Of course particularly customers or
industry bodies may define a minimal set of requirements. I don't think anyone
has done so yet.
Current set of counters is all the ones in the specification itself at their
finest granularity (summed cases for the configurable counters 'work' but
aren't advertised - working out sensible choices is a job for perftool +
an enhanced driver that advertises what combinations it can sum).
Counter features / types include the most generic, fully featured and flexible
versions. In many cases it's harder to support the less complex hardware
because a configuration dance can be needed.
I'd imagine the remaining 'likely' hardware support will be added over the
next kernel cycle or two.
Jonathan
>
> >
> > CXL rev 3.0 specification available from https://www.computeexpresslink.org
> >
> >
> > Jonathan Cameron (5):
> > cxl: Add function to count regblocks of a given type
> > perf: Allow a PMU to have a parent
> > cxl/pci: Find and register CXL PMU devices
> > cxl: CXL Performance Monitoring Unit driver
> > docs: perf: Minimal introduction the the CXL PMU device and driver
> >
> > Documentation/admin-guide/perf/cxl.rst | 65 ++
> > Documentation/admin-guide/perf/index.rst | 1 +
> > drivers/cxl/Kconfig | 13 +
> > drivers/cxl/Makefile | 1 +
> > drivers/cxl/core/Makefile | 1 +
> > drivers/cxl/core/core.h | 1 +
> > drivers/cxl/core/cpmu.c | 72 ++
> > drivers/cxl/core/pci.c | 2 +-
> > drivers/cxl/core/port.c | 4 +-
> > drivers/cxl/core/regs.c | 50 +-
> > drivers/cxl/cpmu.c | 940 +++++++++++++++++++++++
> > drivers/cxl/cpmu.h | 56 ++
> > drivers/cxl/cxl.h | 17 +-
> > drivers/cxl/cxlpci.h | 1 +
> > drivers/cxl/pci.c | 27 +-
> > include/linux/perf_event.h | 1 +
> > kernel/events/core.c | 1 +
> > 17 files changed, 1245 insertions(+), 8 deletions(-)
> > create mode 100644 Documentation/admin-guide/perf/cxl.rst
> > create mode 100644 drivers/cxl/core/cpmu.c
> > create mode 100644 drivers/cxl/cpmu.c
> > create mode 100644 drivers/cxl/cpmu.h
> >
> > --
> > 2.37.2
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-04-11 13:21 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-30 16:45 [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support Jonathan Cameron
2023-03-30 16:45 ` [PATCH v4 1/5] cxl: Add function to count regblocks of a given type Jonathan Cameron
2023-04-04 3:59 ` Dan Williams
2023-03-30 16:45 ` [PATCH v4 2/5] perf: Allow a PMU to have a parent Jonathan Cameron
2023-04-04 4:03 ` Dan Williams
2023-03-30 16:45 ` [PATCH v4 3/5] cxl/pci: Find and register CXL PMU devices Jonathan Cameron
2023-04-04 19:17 ` Dan Williams
2023-04-05 10:48 ` Jonathan Cameron
2023-03-30 16:45 ` [PATCH v4 4/5] cxl: CXL Performance Monitoring Unit driver Jonathan Cameron
2023-04-03 17:32 ` Liang, Kan
2023-04-04 16:48 ` Jonathan Cameron
2023-04-04 21:53 ` Dan Williams
2023-04-05 16:08 ` Jonathan Cameron
2023-04-05 19:26 ` Dan Williams
2023-03-30 16:45 ` [PATCH v4 5/5] docs: perf: Minimal introduction the the CXL PMU device and driver Jonathan Cameron
2023-04-03 17:45 ` Liang, Kan
2023-04-04 16:55 ` Jonathan Cameron
2023-04-04 22:24 ` Dan Williams
2023-04-06 16:33 ` Jonathan Cameron
2023-04-04 3:55 ` [PATCH v4 0/5] CXL 3.0 Performance Monitoring Unit support Dan Williams
2023-04-11 13:21 ` Jonathan Cameron
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).