* [PATCH v4 0/4] arm64 SMMUv3 PMU driver with IORT support
@ 2018-10-16 12:49 Shameer Kolothum
2018-10-16 12:49 ` [PATCH v4 1/4] acpi: arm64: add iort support for PMCG Shameer Kolothum
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Shameer Kolothum @ 2018-10-16 12:49 UTC (permalink / raw)
To: lorenzo.pieralisi, robin.murphy, jean-philippe.brucker
Cc: will.deacon, mark.rutland, guohanjun, john.garry, pabba, vkilari,
rruigrok, linux-acpi, linux-kernel, linux-arm-kernel, linuxarm,
neil.m.leeder
This adds a driver for the SMMUv3 PMU into the perf framework.
It includes an IORT update to support PM Counter Groups.
This is based on the initial work done by Neil Leeder[1]
SMMUv3 PMCG devices are named as smmuv3_pmcg_<phys_addr_page>
where <phys_addr_page> is the physical page address of the SMMU PMCG.
For example, the PMCG at 0xff88840000 is named smmuv3_pmcg_ff88840
Usage example:
For common arch supported events:
perf stat -e smmuv3_pmcg_ff88840/transaction,filter_enable=1,
filter_span=1,filter_stream_id=0x42/ -a netperf
For IMP DEF events:
perf stat -e smmuv3_pmcg_ff88840/event=id/ -a netperf
This is sanity tested on a HiSilicon platform that requires
a quirk to run it properly. As per HiSilicon erratum #162001800,
PMCG event counter registers (SMMU_PMCG_EVCNTRn) on HiSilicon Hip08
platforms are read only and this prevents the software from setting
the initial period on event start. Unfortunately we were a bit late
in the cycle to detect this issue and now require software workaround
for this. Patch #4 is added to this series to provide a workaround
for this issue.
Further testing on supported platforms are very much welcome.
v3 --> v4
-Addressed comments from Jean and Robin.
-Merged dma config callbacks as per Lorenzo's comments(patch #1).
-Added handling of Global(Counter0) filter settings mode(patch #2).
-Added patch #4 to address HiSilicon erratum #162001800
-
v2 --> v3
-Addressed comments from Robin.
-Removed iort helper function to retrieve the PMCG reference smmu.
-PMCG devices are now named using the base address
v1 --> v2
- Addressed comments from Robin.
- Added an helper to retrieve the associated smmu dev and named PMUs
to make the association visible to user.
- Added MSI support for overflow irq
[1]https://www.spinics.net/lists/arm-kernel/msg598591.html
Neil Leeder (2):
acpi: arm64: add iort support for PMCG
perf: add arm64 smmuv3 pmu driver
Shameer Kolothum (2):
perf/smmuv3: Add MSI irq support
perf/smmuv3_pmu: Enable HiSilicon Erratum 162001800 quirk
drivers/acpi/arm64/iort.c | 97 ++++-
drivers/perf/Kconfig | 9 +
drivers/perf/Makefile | 1 +
drivers/perf/arm_smmuv3_pmu.c | 959 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 1045 insertions(+), 21 deletions(-)
create mode 100644 drivers/perf/arm_smmuv3_pmu.c
--
2.7.4
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 1/4] acpi: arm64: add iort support for PMCG
2018-10-16 12:49 [PATCH v4 0/4] arm64 SMMUv3 PMU driver with IORT support Shameer Kolothum
@ 2018-10-16 12:49 ` Shameer Kolothum
2018-10-16 12:49 ` [PATCH v4 2/4] perf: add arm64 smmuv3 pmu driver Shameer Kolothum
` (2 subsequent siblings)
3 siblings, 0 replies; 17+ messages in thread
From: Shameer Kolothum @ 2018-10-16 12:49 UTC (permalink / raw)
To: lorenzo.pieralisi, robin.murphy, jean-philippe.brucker
Cc: will.deacon, mark.rutland, guohanjun, john.garry, pabba, vkilari,
rruigrok, linux-acpi, linux-kernel, linux-arm-kernel, linuxarm,
neil.m.leeder
From: Neil Leeder <nleeder@codeaurora.org>
Add support for the SMMU Performance Monitor Counter Group
information from ACPI. This is in preparation for its use
in the SMMUv3 PMU driver.
Signed-off-by: Neil Leeder <nleeder@codeaurora.org>
Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
drivers/acpi/arm64/iort.c | 97 +++++++++++++++++++++++++++++++++++++----------
1 file changed, 76 insertions(+), 21 deletions(-)
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 08f26db..c44d8f6 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -356,7 +356,8 @@ static struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT ||
node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX ||
- node->type == ACPI_IORT_NODE_SMMU_V3) {
+ node->type == ACPI_IORT_NODE_SMMU_V3 ||
+ node->type == ACPI_IORT_NODE_PMCG) {
*id_out = map->output_base;
return parent;
}
@@ -394,6 +395,8 @@ static int iort_get_id_mapping_index(struct acpi_iort_node *node)
}
return smmu->id_mapping_index;
+ case ACPI_IORT_NODE_PMCG:
+ return 0;
default:
return -EINVAL;
}
@@ -1216,14 +1219,23 @@ static void __init arm_smmu_v3_init_resources(struct resource *res,
}
}
-static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
+static void __init arm_smmu_v3_dma_configure(struct device *dev,
+ struct acpi_iort_node *node)
{
struct acpi_iort_smmu_v3 *smmu;
+ enum dev_dma_attr attr;
/* Retrieve SMMUv3 specific data */
smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
- return smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;
+ attr = (smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE) ?
+ DEV_DMA_COHERENT : DEV_DMA_NON_COHERENT;
+
+ /* We expect the dma masks to be equivalent for all SMMUv3 set-ups */
+ dev->dma_mask = &dev->coherent_dma_mask;
+
+ /* Configure DMA for the page table walker */
+ acpi_dma_configure(dev, attr);
}
#if defined(CONFIG_ACPI_NUMA)
@@ -1299,20 +1311,64 @@ static void __init arm_smmu_init_resources(struct resource *res,
}
}
-static bool __init arm_smmu_is_coherent(struct acpi_iort_node *node)
+static void __init arm_smmu_dma_configure(struct device *dev,
+ struct acpi_iort_node *node)
{
struct acpi_iort_smmu *smmu;
+ enum dev_dma_attr attr;
/* Retrieve SMMU specific data */
smmu = (struct acpi_iort_smmu *)node->node_data;
- return smmu->flags & ACPI_IORT_SMMU_COHERENT_WALK;
+ attr = (smmu->flags & ACPI_IORT_SMMU_COHERENT_WALK) ?
+ DEV_DMA_COHERENT : DEV_DMA_NON_COHERENT;
+
+ /* We expect the dma masks to be equivalent for SMMU set-ups */
+ dev->dma_mask = &dev->coherent_dma_mask;
+
+ /* Configure DMA for the page table walker */
+ acpi_dma_configure(dev, attr);
+}
+
+static int __init arm_smmu_v3_pmcg_count_resources(struct acpi_iort_node *node)
+{
+ struct acpi_iort_pmcg *pmcg;
+
+ /* Retrieve PMCG specific data */
+ pmcg = (struct acpi_iort_pmcg *)node->node_data;
+
+ /*
+ * There are always 2 memory resources.
+ * If the overflow_gsiv is present then add that for a total of 3.
+ */
+ return pmcg->overflow_gsiv ? 3 : 2;
+}
+
+static void __init arm_smmu_v3_pmcg_init_resources(struct resource *res,
+ struct acpi_iort_node *node)
+{
+ struct acpi_iort_pmcg *pmcg;
+
+ /* Retrieve PMCG specific data */
+ pmcg = (struct acpi_iort_pmcg *)node->node_data;
+
+ res[0].start = pmcg->page0_base_address;
+ res[0].end = pmcg->page0_base_address + SZ_4K - 1;
+ res[0].flags = IORESOURCE_MEM;
+ res[1].start = pmcg->page1_base_address;
+ res[1].end = pmcg->page1_base_address + SZ_4K - 1;
+ res[1].flags = IORESOURCE_MEM;
+
+ if (pmcg->overflow_gsiv)
+ acpi_iort_register_irq(pmcg->overflow_gsiv, "overflow",
+ ACPI_EDGE_SENSITIVE, &res[2]);
}
struct iort_dev_config {
const char *name;
int (*dev_init)(struct acpi_iort_node *node);
- bool (*dev_is_coherent)(struct acpi_iort_node *node);
+ void (*dev_dma_configure)(struct device *dev,
+ struct acpi_iort_node *node);
int (*dev_count_resources)(struct acpi_iort_node *node);
void (*dev_init_resources)(struct resource *res,
struct acpi_iort_node *node);
@@ -1322,7 +1378,7 @@ struct iort_dev_config {
static const struct iort_dev_config iort_arm_smmu_v3_cfg __initconst = {
.name = "arm-smmu-v3",
- .dev_is_coherent = arm_smmu_v3_is_coherent,
+ .dev_dma_configure = arm_smmu_v3_dma_configure,
.dev_count_resources = arm_smmu_v3_count_resources,
.dev_init_resources = arm_smmu_v3_init_resources,
.dev_set_proximity = arm_smmu_v3_set_proximity,
@@ -1330,19 +1386,28 @@ static const struct iort_dev_config iort_arm_smmu_v3_cfg __initconst = {
static const struct iort_dev_config iort_arm_smmu_cfg __initconst = {
.name = "arm-smmu",
- .dev_is_coherent = arm_smmu_is_coherent,
+ .dev_dma_configure = arm_smmu_dma_configure,
.dev_count_resources = arm_smmu_count_resources,
- .dev_init_resources = arm_smmu_init_resources
+ .dev_init_resources = arm_smmu_init_resources,
+};
+
+static const struct iort_dev_config iort_arm_smmu_v3_pmcg_cfg __initconst = {
+ .name = "arm-smmu-v3-pmu",
+ .dev_count_resources = arm_smmu_v3_pmcg_count_resources,
+ .dev_init_resources = arm_smmu_v3_pmcg_init_resources,
};
static __init const struct iort_dev_config *iort_get_dev_cfg(
struct acpi_iort_node *node)
{
+
switch (node->type) {
case ACPI_IORT_NODE_SMMU_V3:
return &iort_arm_smmu_v3_cfg;
case ACPI_IORT_NODE_SMMU:
return &iort_arm_smmu_cfg;
+ case ACPI_IORT_NODE_PMCG:
+ return &iort_arm_smmu_v3_pmcg_cfg;
default:
return NULL;
}
@@ -1360,7 +1425,6 @@ static int __init iort_add_platform_device(struct acpi_iort_node *node,
struct fwnode_handle *fwnode;
struct platform_device *pdev;
struct resource *r;
- enum dev_dma_attr attr;
int ret, count;
pdev = platform_device_alloc(ops->name, PLATFORM_DEVID_AUTO);
@@ -1398,12 +1462,6 @@ static int __init iort_add_platform_device(struct acpi_iort_node *node,
if (ret)
goto dev_put;
- /*
- * We expect the dma masks to be equivalent for
- * all SMMUs set-ups
- */
- pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
-
fwnode = iort_get_fwnode(node);
if (!fwnode) {
@@ -1413,11 +1471,8 @@ static int __init iort_add_platform_device(struct acpi_iort_node *node,
pdev->dev.fwnode = fwnode;
- attr = ops->dev_is_coherent && ops->dev_is_coherent(node) ?
- DEV_DMA_COHERENT : DEV_DMA_NON_COHERENT;
-
- /* Configure DMA for the page table walker */
- acpi_dma_configure(&pdev->dev, attr);
+ if (ops->dev_dma_configure)
+ ops->dev_dma_configure(&pdev->dev, node);
iort_set_device_domain(&pdev->dev, node);
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 2/4] perf: add arm64 smmuv3 pmu driver
2018-10-16 12:49 [PATCH v4 0/4] arm64 SMMUv3 PMU driver with IORT support Shameer Kolothum
2018-10-16 12:49 ` [PATCH v4 1/4] acpi: arm64: add iort support for PMCG Shameer Kolothum
@ 2018-10-16 12:49 ` Shameer Kolothum
2018-10-17 21:53 ` kbuild test robot
2018-10-20 4:50 ` kbuild test robot
2018-10-16 12:49 ` [PATCH v4 3/4] perf/smmuv3: Add MSI irq support Shameer Kolothum
2018-10-16 12:49 ` [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum 162001800 quirk Shameer Kolothum
3 siblings, 2 replies; 17+ messages in thread
From: Shameer Kolothum @ 2018-10-16 12:49 UTC (permalink / raw)
To: lorenzo.pieralisi, robin.murphy, jean-philippe.brucker
Cc: will.deacon, mark.rutland, guohanjun, john.garry, pabba, vkilari,
rruigrok, linux-acpi, linux-kernel, linux-arm-kernel, linuxarm,
neil.m.leeder
From: Neil Leeder <nleeder@codeaurora.org>
Adds a new driver to support the SMMUv3 PMU and add it into the
perf events framework.
Each SMMU node may have multiple PMUs associated with it, each of
which may support different events.
SMMUv3 PMCG devices are named as smmuv3_pmcg_<phys_addr_page> where
<phys_addr_page> is the physical page address of the SMMU PMCG
wrapped to 4K boundary. For example, the PMCG at 0xff88840000 is
named smmuv3_pmcg_ff88840
Filtering by stream id is done by specifying filtering parameters
with the event. options are:
filter_enable - 0 = no filtering, 1 = filtering enabled
filter_span - 0 = exact match, 1 = pattern match
filter_stream_id - pattern to filter against
Example: perf stat -e smmuv3_pmcg_ff88840/transaction,filter_enable=1,
filter_span=1,filter_stream_id=0x42/ -a netperf
Applies filter pattern 0x42 to transaction events, which means events
matching stream ids 0x42 & 0x43 are counted as only upper StreamID
bits are required to match the given filter. Further filtering
information is available in the SMMU documentation.
SMMU events are not attributable to a CPU, so task mode and sampling
are not supported.
Signed-off-by: Neil Leeder <nleeder@codeaurora.org>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
drivers/perf/Kconfig | 9 +
drivers/perf/Makefile | 1 +
drivers/perf/arm_smmuv3_pmu.c | 778 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 788 insertions(+)
create mode 100644 drivers/perf/arm_smmuv3_pmu.c
diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index 08ebaf7..c5deb8a 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig
@@ -52,6 +52,15 @@ config ARM_PMU_ACPI
depends on ARM_PMU && ACPI
def_bool y
+config ARM_SMMU_V3_PMU
+ bool "ARM SMMUv3 Performance Monitors Extension"
+ depends on (ARM64 && ACPI && ARM_SMMU_V3) || COMPILE_TEST
+ help
+ Provides support for the SMMU version 3 performance monitor unit (PMU)
+ on ARM-based systems.
+ Adds the SMMU PMU into the perf events subsystem for
+ monitoring SMMU performance events.
+
config ARM_DSU_PMU
tristate "ARM DynamIQ Shared Unit (DSU) PMU"
depends on ARM64
diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
index b3902bd..f10a932 100644
--- a/drivers/perf/Makefile
+++ b/drivers/perf/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_ARM_CCN) += arm-ccn.o
obj-$(CONFIG_ARM_DSU_PMU) += arm_dsu_pmu.o
obj-$(CONFIG_ARM_PMU) += arm_pmu.o arm_pmu_platform.o
obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
+obj-$(CONFIG_ARM_SMMU_V3_PMU) += arm_smmuv3_pmu.o
obj-$(CONFIG_HISI_PMU) += hisilicon/
obj-$(CONFIG_QCOM_L2_PMU) += qcom_l2_pmu.o
obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
new file mode 100644
index 0000000..e30b939
--- /dev/null
+++ b/drivers/perf/arm_smmuv3_pmu.c
@@ -0,0 +1,778 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * This driver adds support for perf events to use the Performance
+ * Monitor Counter Groups (PMCG) associated with an SMMUv3 node
+ * to monitor that node.
+ *
+ * SMMUv3 PMCG devices are named as smmuv3_pmcg_<phys_addr_page> where
+ * <phys_addr_page> is the physical page address of the SMMU PMCG wrapped
+ * to 4K boundary. For example, the PMCG at 0xff88840000 is named
+ * smmuv3_pmcg_ff88840
+ *
+ * Filtering by stream id is done by specifying filtering parameters
+ * with the event. options are:
+ * filter_enable - 0 = no filtering, 1 = filtering enabled
+ * filter_span - 0 = exact match, 1 = pattern match
+ * filter_stream_id - pattern to filter against
+ *
+ * To match a partial StreamID where the X most-significant bits must match
+ * but the Y least-significant bits might differ, STREAMID is programmed
+ * with a value that contains:
+ * STREAMID[Y - 1] == 0.
+ * STREAMID[Y - 2:0] == 1 (where Y > 1).
+ * The remainder of implemented bits of STREAMID (X bits, from bit Y upwards)
+ * contain a value to match from the corresponding bits of event StreamID.
+ *
+ * Example: perf stat -e smmuv3_pmcg_ff88840/transaction,filter_enable=1,
+ * filter_span=1,filter_stream_id=0x42/ -a netperf
+ * Applies filter pattern 0x42 to transaction events, which means events
+ * matching stream ids 0x42 and 0x43 are counted. Further filtering
+ * information is available in the SMMU documentation.
+ *
+ * SMMU events are not attributable to a CPU, so task mode and sampling
+ * are not supported.
+ */
+
+#include <linux/acpi.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/cpuhotplug.h>
+#include <linux/cpumask.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/msi.h>
+#include <linux/perf_event.h>
+#include <linux/platform_device.h>
+#include <linux/smp.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+
+#define SMMU_PMCG_EVCNTR0 0x0
+#define SMMU_PMCG_EVCNTR(n, stride) (SMMU_PMCG_EVCNTR0 + (n) * (stride))
+#define SMMU_PMCG_EVTYPER0 0x400
+#define SMMU_PMCG_EVTYPER(n) (SMMU_PMCG_EVTYPER0 + (n) * 4)
+#define SMMU_PMCG_SID_SPAN_SHIFT 29
+#define SMMU_PMCG_SID_SPAN_MASK GENMASK(29, 29)
+#define SMMU_PMCG_SMR0 0xA00
+#define SMMU_PMCG_SMR(n) (SMMU_PMCG_SMR0 + (n) * 4)
+#define SMMU_PMCG_CNTENSET0 0xC00
+#define SMMU_PMCG_CNTENCLR0 0xC20
+#define SMMU_PMCG_INTENSET0 0xC40
+#define SMMU_PMCG_INTENCLR0 0xC60
+#define SMMU_PMCG_OVSCLR0 0xC80
+#define SMMU_PMCG_OVSSET0 0xCC0
+#define SMMU_PMCG_CFGR 0xE00
+#define SMMU_PMCG_CFGR_RELOC_CTRS BIT(20)
+#define SMMU_PMCG_CFGR_SID_FILTER_TYPE BIT(23)
+#define SMMU_PMCG_CFGR_SIZE_MASK GENMASK(13, 8)
+#define SMMU_PMCG_CFGR_NCTR_MASK GENMASK(5, 0)
+#define SMMU_PMCG_CR 0xE04
+#define SMMU_PMCG_CR_ENABLE BIT(0)
+#define SMMU_PMCG_CEID0 0xE20
+#define SMMU_PMCG_CEID1 0xE28
+#define SMMU_PMCG_IRQ_CTRL 0xE50
+#define SMMU_PMCG_IRQ_CTRL_IRQEN BIT(0)
+#define SMMU_PMCG_IRQ_CFG0 0xE58
+
+#define SMMU_DEFAULT_FILTER_SPAN 1
+#define SMMU_DEFAULT_FILTER_STREAM_ID GENMASK(31, 0)
+
+#define SMMU_MAX_COUNTERS 64
+#define SMMU_ARCH_MAX_EVENT_ID 0x7F
+#define SMMU_IMPDEF_MAX_EVENT_ID 0xFFFF
+#define SMMU_ARCH_MAX_EVENTS (SMMU_ARCH_MAX_EVENT_ID + 1)
+
+#define SMMU_PA_SHIFT 12
+
+static int cpuhp_state_num;
+
+struct smmu_pmu {
+ bool sid_filter_global;
+ struct hlist_node node;
+ struct perf_event *events[SMMU_MAX_COUNTERS];
+ DECLARE_BITMAP(used_counters, SMMU_MAX_COUNTERS);
+ DECLARE_BITMAP(supported_events, SMMU_ARCH_MAX_EVENTS);
+ unsigned int irq;
+ unsigned int on_cpu;
+ struct pmu pmu;
+ unsigned int num_counters;
+ struct device *dev;
+ void __iomem *reg_base;
+ void __iomem *reloc_base;
+ u64 counter_present_mask;
+ u64 counter_mask;
+};
+
+#define to_smmu_pmu(p) (container_of(p, struct smmu_pmu, pmu))
+
+#define SMMU_PMU_EVENT_ATTR_EXTRACTOR(_name, _config, _start, _end) \
+ static inline u32 get_##_name(struct perf_event *event) \
+ { \
+ return FIELD_GET(GENMASK_ULL(_end, _start), \
+ event->attr._config); \
+ } \
+
+SMMU_PMU_EVENT_ATTR_EXTRACTOR(event, config, 0, 15);
+SMMU_PMU_EVENT_ATTR_EXTRACTOR(filter_stream_id, config1, 0, 31);
+SMMU_PMU_EVENT_ATTR_EXTRACTOR(filter_span, config1, 32, 32);
+SMMU_PMU_EVENT_ATTR_EXTRACTOR(filter_enable, config1, 33, 33);
+
+static inline void smmu_pmu_enable(struct pmu *pmu)
+{
+ struct smmu_pmu *smmu_pmu = to_smmu_pmu(pmu);
+
+ writel(SMMU_PMCG_CR_ENABLE, smmu_pmu->reg_base + SMMU_PMCG_CR);
+ writel(SMMU_PMCG_IRQ_CTRL_IRQEN,
+ smmu_pmu->reg_base + SMMU_PMCG_IRQ_CTRL);
+}
+
+static inline void smmu_pmu_disable(struct pmu *pmu)
+{
+ struct smmu_pmu *smmu_pmu = to_smmu_pmu(pmu);
+
+ writel(0, smmu_pmu->reg_base + SMMU_PMCG_CR);
+ writel(0, smmu_pmu->reg_base + SMMU_PMCG_IRQ_CTRL);
+}
+
+static inline void smmu_pmu_counter_set_value(struct smmu_pmu *smmu_pmu,
+ u32 idx, u64 value)
+{
+ if (smmu_pmu->counter_mask & BIT(32))
+ writeq(value, smmu_pmu->reloc_base + SMMU_PMCG_EVCNTR(idx, 8));
+ else
+ writel(value, smmu_pmu->reloc_base + SMMU_PMCG_EVCNTR(idx, 4));
+}
+
+static inline u64 smmu_pmu_counter_get_value(struct smmu_pmu *smmu_pmu, u32 idx)
+{
+ u64 value;
+
+ if (smmu_pmu->counter_mask & BIT(32))
+ value = readq(smmu_pmu->reloc_base + SMMU_PMCG_EVCNTR(idx, 8));
+ else
+ value = readl(smmu_pmu->reloc_base + SMMU_PMCG_EVCNTR(idx, 4));
+
+ return value;
+}
+
+static inline void smmu_pmu_counter_enable(struct smmu_pmu *smmu_pmu, u32 idx)
+{
+ writeq(BIT(idx), smmu_pmu->reg_base + SMMU_PMCG_CNTENSET0);
+}
+
+static inline void smmu_pmu_counter_disable(struct smmu_pmu *smmu_pmu, u32 idx)
+{
+ writeq(BIT(idx), smmu_pmu->reg_base + SMMU_PMCG_CNTENCLR0);
+}
+
+static inline void smmu_pmu_interrupt_enable(struct smmu_pmu *smmu_pmu, u32 idx)
+{
+ writeq(BIT(idx), smmu_pmu->reg_base + SMMU_PMCG_INTENSET0);
+}
+
+static inline void smmu_pmu_interrupt_disable(struct smmu_pmu *smmu_pmu,
+ u32 idx)
+{
+ writeq(BIT(idx), smmu_pmu->reg_base + SMMU_PMCG_INTENCLR0);
+}
+
+static inline void smmu_pmu_set_evtyper(struct smmu_pmu *smmu_pmu, u32 idx,
+ u32 val)
+{
+ writel(val, smmu_pmu->reg_base + SMMU_PMCG_EVTYPER(idx));
+}
+
+static inline void smmu_pmu_set_smr(struct smmu_pmu *smmu_pmu, u32 idx, u32 val)
+{
+ writel(val, smmu_pmu->reg_base + SMMU_PMCG_SMR(idx));
+}
+
+static void smmu_pmu_event_update(struct perf_event *event)
+{
+ struct hw_perf_event *hwc = &event->hw;
+ struct smmu_pmu *smmu_pmu = to_smmu_pmu(event->pmu);
+ u64 delta, prev, now;
+ u32 idx = hwc->idx;
+
+ do {
+ prev = local64_read(&hwc->prev_count);
+ now = smmu_pmu_counter_get_value(smmu_pmu, idx);
+ } while (local64_cmpxchg(&hwc->prev_count, prev, now) != prev);
+
+ /* handle overflow. */
+ delta = now - prev;
+ delta &= smmu_pmu->counter_mask;
+
+ local64_add(delta, &event->count);
+}
+
+static void smmu_pmu_set_period(struct smmu_pmu *smmu_pmu,
+ struct hw_perf_event *hwc)
+{
+ u32 idx = hwc->idx;
+ u64 new;
+
+ /*
+ * We limit the max period to half the max counter value of the counter
+ * size, so that even in the case of extreme interrupt latency the
+ * counter will (hopefully) not wrap past its initial value.
+ */
+ new = smmu_pmu->counter_mask >> 1;
+
+ local64_set(&hwc->prev_count, new);
+ smmu_pmu_counter_set_value(smmu_pmu, idx, new);
+}
+
+static void smmu_pmu_get_event_filter(struct perf_event *event, u32 *span,
+ u32 *stream_id)
+{
+ bool filter_en = !!get_filter_enable(event);
+
+ *span = filter_en ? get_filter_span(event) : SMMU_DEFAULT_FILTER_SPAN;
+ *stream_id = filter_en ? get_filter_stream_id(event) :
+ SMMU_DEFAULT_FILTER_STREAM_ID;
+}
+
+static bool smmu_pmu_event_filter_valid(struct smmu_pmu *smmu_pmu,
+ struct perf_event *event, int idx)
+{
+ u32 filter_span, filter_sid;
+ u32 curr_span, curr_sid;
+
+ if (!idx || !smmu_pmu->sid_filter_global)
+ return true;
+
+ smmu_pmu_get_event_filter(event, &filter_span, &filter_sid);
+
+ /* Read the current global filter setting */
+ curr_span = FIELD_GET(SMMU_PMCG_SID_SPAN_MASK,
+ readl(smmu_pmu->reg_base + SMMU_PMCG_EVTYPER0));
+ curr_sid = readl(smmu_pmu->reg_base + SMMU_PMCG_SMR0);
+
+ if (filter_span != curr_span || filter_sid != curr_sid)
+ return false;
+
+ return true;
+}
+
+static int smmu_pmu_get_event_idx(struct smmu_pmu *smmu_pmu,
+ struct perf_event *event)
+{
+ unsigned int idx;
+ unsigned int num_ctrs = smmu_pmu->num_counters;
+
+ idx = find_first_zero_bit(smmu_pmu->used_counters, num_ctrs);
+ if (idx == num_ctrs)
+ /* The counters are all in use. */
+ return -EAGAIN;
+
+ if (!smmu_pmu_event_filter_valid(smmu_pmu, event, idx))
+ return -EAGAIN;
+
+ set_bit(idx, smmu_pmu->used_counters);
+
+ return idx;
+}
+
+/*
+ * Implementation of abstract pmu functionality required by
+ * the core perf events code.
+ */
+
+static int smmu_pmu_event_init(struct perf_event *event)
+{
+ struct hw_perf_event *hwc = &event->hw;
+ struct smmu_pmu *smmu_pmu = to_smmu_pmu(event->pmu);
+ struct device *dev = smmu_pmu->dev;
+ struct perf_event *sibling;
+ u32 event_id;
+
+ if (event->attr.type != event->pmu->type)
+ return -ENOENT;
+
+ if (hwc->sample_period) {
+ dev_dbg(dev, "Sampling not supported\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (event->cpu < 0) {
+ dev_dbg(dev, "Per-task mode not supported\n");
+ return -EOPNOTSUPP;
+ }
+
+ /* We cannot filter accurately so we just don't allow it. */
+ if (event->attr.exclude_user || event->attr.exclude_kernel ||
+ event->attr.exclude_hv || event->attr.exclude_idle) {
+ dev_dbg(dev, "Can't exclude execution levels\n");
+ return -EOPNOTSUPP;
+ }
+
+ /* Verify specified event is supported on this PMU */
+ event_id = get_event(event);
+ if (((event_id <= SMMU_ARCH_MAX_EVENT_ID) &&
+ (!test_bit(event_id, smmu_pmu->supported_events))) ||
+ (event_id > SMMU_IMPDEF_MAX_EVENT_ID)) {
+ dev_dbg(dev, "Invalid event %d for this PMU\n", event_id);
+ return -EINVAL;
+ }
+
+ /* Don't allow groups with mixed PMUs, except for s/w events */
+ if (event->group_leader->pmu != event->pmu &&
+ !is_software_event(event->group_leader)) {
+ dev_dbg(dev, "Can't create mixed PMU group\n");
+ return -EINVAL;
+ }
+
+ list_for_each_entry(sibling, &event->group_leader->sibling_list,
+ sibling_list)
+ if (sibling->pmu != event->pmu &&
+ !is_software_event(sibling)) {
+ dev_dbg(dev, "Can't create mixed PMU group\n");
+ return -EINVAL;
+ }
+
+ /* Ensure all events in a group are on the same cpu */
+ if ((event->group_leader != event) &&
+ (event->cpu != event->group_leader->cpu)) {
+ dev_dbg(dev, "Can't create group on CPUs %d and %d\n",
+ event->cpu, event->group_leader->cpu);
+ return -EINVAL;
+ }
+
+ hwc->idx = -1;
+
+ /*
+ * Ensure all events are on the same cpu so all events are in the
+ * same cpu context, to avoid races on pmu_enable etc.
+ */
+ event->cpu = smmu_pmu->on_cpu;
+
+ return 0;
+}
+
+static void smmu_pmu_event_start(struct perf_event *event, int flags)
+{
+ struct smmu_pmu *smmu_pmu = to_smmu_pmu(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+ int idx = hwc->idx;
+ u32 filter_span, filter_sid;
+ u32 evtyper;
+
+ hwc->state = 0;
+
+ smmu_pmu_set_period(smmu_pmu, hwc);
+
+ smmu_pmu_get_event_filter(event, &filter_span, &filter_sid);
+
+ evtyper = get_event(event) |
+ filter_span << SMMU_PMCG_SID_SPAN_SHIFT;
+
+ smmu_pmu_set_evtyper(smmu_pmu, idx, evtyper);
+ smmu_pmu_set_smr(smmu_pmu, idx, filter_sid);
+ smmu_pmu_interrupt_enable(smmu_pmu, idx);
+ smmu_pmu_counter_enable(smmu_pmu, idx);
+}
+
+static void smmu_pmu_event_stop(struct perf_event *event, int flags)
+{
+ struct smmu_pmu *smmu_pmu = to_smmu_pmu(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+ int idx = hwc->idx;
+
+ if (hwc->state & PERF_HES_STOPPED)
+ return;
+
+ smmu_pmu_counter_disable(smmu_pmu, idx);
+
+ if (flags & PERF_EF_UPDATE)
+ smmu_pmu_event_update(event);
+ hwc->state |= PERF_HES_STOPPED | PERF_HES_UPTODATE;
+}
+
+static int smmu_pmu_event_add(struct perf_event *event, int flags)
+{
+ struct hw_perf_event *hwc = &event->hw;
+ int idx;
+ struct smmu_pmu *smmu_pmu = to_smmu_pmu(event->pmu);
+
+ idx = smmu_pmu_get_event_idx(smmu_pmu, event);
+ if (idx < 0)
+ return idx;
+
+ hwc->idx = idx;
+ hwc->state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
+ smmu_pmu->events[idx] = event;
+ local64_set(&hwc->prev_count, 0);
+
+ if (flags & PERF_EF_START)
+ smmu_pmu_event_start(event, flags);
+
+ /* Propagate changes to the userspace mapping. */
+ perf_event_update_userpage(event);
+
+ return 0;
+}
+
+static void smmu_pmu_event_del(struct perf_event *event, int flags)
+{
+ struct hw_perf_event *hwc = &event->hw;
+ struct smmu_pmu *smmu_pmu = to_smmu_pmu(event->pmu);
+ int idx = hwc->idx;
+
+ smmu_pmu_event_stop(event, flags | PERF_EF_UPDATE);
+ smmu_pmu->events[idx] = NULL;
+ clear_bit(idx, smmu_pmu->used_counters);
+
+ perf_event_update_userpage(event);
+}
+
+static void smmu_pmu_event_read(struct perf_event *event)
+{
+ smmu_pmu_event_update(event);
+}
+
+/* cpumask */
+
+static ssize_t smmu_pmu_cpumask_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct smmu_pmu *smmu_pmu = to_smmu_pmu(dev_get_drvdata(dev));
+
+ return cpumap_print_to_pagebuf(true, buf, cpumask_of(smmu_pmu->on_cpu));
+}
+
+static struct device_attribute smmu_pmu_cpumask_attr =
+ __ATTR(cpumask, 0444, smmu_pmu_cpumask_show, NULL);
+
+static struct attribute *smmu_pmu_cpumask_attrs[] = {
+ &smmu_pmu_cpumask_attr.attr,
+ NULL
+};
+
+static struct attribute_group smmu_pmu_cpumask_group = {
+ .attrs = smmu_pmu_cpumask_attrs,
+};
+
+/* Events */
+
+ssize_t smmu_pmu_event_show(struct device *dev,
+ struct device_attribute *attr, char *page)
+{
+ struct perf_pmu_events_attr *pmu_attr;
+
+ pmu_attr = container_of(attr, struct perf_pmu_events_attr, attr);
+
+ return sprintf(page, "event=0x%02llx\n", pmu_attr->id);
+}
+
+#define SMMU_EVENT_ATTR(_name, _id) \
+ (&((struct perf_pmu_events_attr[]) { \
+ { .attr = __ATTR(_name, 0444, smmu_pmu_event_show, NULL), \
+ .id = _id, } \
+ })[0].attr.attr)
+
+static struct attribute *smmu_pmu_events[] = {
+ SMMU_EVENT_ATTR(cycles, 0),
+ SMMU_EVENT_ATTR(transaction, 1),
+ SMMU_EVENT_ATTR(tlb_miss, 2),
+ SMMU_EVENT_ATTR(config_cache_miss, 3),
+ SMMU_EVENT_ATTR(trans_table_walk_access, 4),
+ SMMU_EVENT_ATTR(config_struct_access, 5),
+ SMMU_EVENT_ATTR(pcie_ats_trans_rq, 6),
+ SMMU_EVENT_ATTR(pcie_ats_trans_passed, 7),
+ NULL
+};
+
+static umode_t smmu_pmu_event_is_visible(struct kobject *kobj,
+ struct attribute *attr, int unused)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct smmu_pmu *smmu_pmu = to_smmu_pmu(dev_get_drvdata(dev));
+ struct perf_pmu_events_attr *pmu_attr;
+
+ pmu_attr = container_of(attr, struct perf_pmu_events_attr, attr.attr);
+
+ if (test_bit(pmu_attr->id, smmu_pmu->supported_events))
+ return attr->mode;
+
+ return 0;
+}
+static struct attribute_group smmu_pmu_events_group = {
+ .name = "events",
+ .attrs = smmu_pmu_events,
+ .is_visible = smmu_pmu_event_is_visible,
+};
+
+/* Formats */
+PMU_FORMAT_ATTR(event, "config:0-15");
+PMU_FORMAT_ATTR(filter_stream_id, "config1:0-31");
+PMU_FORMAT_ATTR(filter_span, "config1:32");
+PMU_FORMAT_ATTR(filter_enable, "config1:33");
+
+static struct attribute *smmu_pmu_formats[] = {
+ &format_attr_event.attr,
+ &format_attr_filter_stream_id.attr,
+ &format_attr_filter_span.attr,
+ &format_attr_filter_enable.attr,
+ NULL
+};
+
+static struct attribute_group smmu_pmu_format_group = {
+ .name = "format",
+ .attrs = smmu_pmu_formats,
+};
+
+static const struct attribute_group *smmu_pmu_attr_grps[] = {
+ &smmu_pmu_cpumask_group,
+ &smmu_pmu_events_group,
+ &smmu_pmu_format_group,
+ NULL
+};
+
+/*
+ * Generic device handlers
+ */
+
+static int smmu_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
+{
+ struct smmu_pmu *smmu_pmu;
+ unsigned int target;
+
+ smmu_pmu = hlist_entry_safe(node, struct smmu_pmu, node);
+ if (cpu != smmu_pmu->on_cpu)
+ return 0;
+
+ target = cpumask_any_but(cpu_online_mask, cpu);
+ if (target >= nr_cpu_ids)
+ return 0;
+
+ perf_pmu_migrate_context(&smmu_pmu->pmu, cpu, target);
+ smmu_pmu->on_cpu = target;
+ WARN_ON(irq_set_affinity(smmu_pmu->irq, cpumask_of(target)));
+
+ return 0;
+}
+
+static irqreturn_t smmu_pmu_handle_irq(int irq_num, void *data)
+{
+ struct smmu_pmu *smmu_pmu = data;
+ u64 ovsr;
+ unsigned int idx;
+
+ ovsr = readq(smmu_pmu->reloc_base + SMMU_PMCG_OVSSET0);
+ if (!ovsr)
+ return IRQ_NONE;
+
+ writeq(ovsr, smmu_pmu->reloc_base + SMMU_PMCG_OVSCLR0);
+
+ for_each_set_bit(idx, (unsigned long *)&ovsr, smmu_pmu->num_counters) {
+ struct perf_event *event = smmu_pmu->events[idx];
+ struct hw_perf_event *hwc;
+
+ if (WARN_ON_ONCE(!event))
+ continue;
+
+ smmu_pmu_event_update(event);
+ hwc = &event->hw;
+
+ smmu_pmu_set_period(smmu_pmu, hwc);
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int smmu_pmu_setup_irq(struct smmu_pmu *pmu)
+{
+ unsigned long flags = IRQF_NOBALANCING | IRQF_SHARED | IRQF_NO_THREAD;
+ int irq, ret = -ENXIO;
+
+ irq = pmu->irq;
+ if (irq)
+ ret = devm_request_irq(pmu->dev, irq, smmu_pmu_handle_irq,
+ flags, "smmuv3-pmu", pmu);
+ return ret;
+}
+
+static void smmu_pmu_reset(struct smmu_pmu *smmu_pmu)
+{
+ smmu_pmu_disable(&smmu_pmu->pmu);
+
+ /* Disable counter and interrupt */
+ writeq_relaxed(smmu_pmu->counter_present_mask,
+ smmu_pmu->reg_base + SMMU_PMCG_CNTENCLR0);
+ writeq_relaxed(smmu_pmu->counter_present_mask,
+ smmu_pmu->reg_base + SMMU_PMCG_INTENCLR0);
+ writeq_relaxed(smmu_pmu->counter_present_mask,
+ smmu_pmu->reloc_base + SMMU_PMCG_OVSCLR0);
+}
+
+static int smmu_pmu_probe(struct platform_device *pdev)
+{
+ struct smmu_pmu *smmu_pmu;
+ struct resource *res_0, *res_1;
+ u32 cfgr, reg_size;
+ u64 ceid_64[2];
+ int irq, err;
+ char *name;
+ struct device *dev = &pdev->dev;
+
+ smmu_pmu = devm_kzalloc(dev, sizeof(*smmu_pmu), GFP_KERNEL);
+ if (!smmu_pmu)
+ return -ENOMEM;
+
+ smmu_pmu->dev = dev;
+ platform_set_drvdata(pdev, smmu_pmu);
+
+ smmu_pmu->pmu = (struct pmu) {
+ .task_ctx_nr = perf_invalid_context,
+ .pmu_enable = smmu_pmu_enable,
+ .pmu_disable = smmu_pmu_disable,
+ .event_init = smmu_pmu_event_init,
+ .add = smmu_pmu_event_add,
+ .del = smmu_pmu_event_del,
+ .start = smmu_pmu_event_start,
+ .stop = smmu_pmu_event_stop,
+ .read = smmu_pmu_event_read,
+ .attr_groups = smmu_pmu_attr_grps,
+ };
+
+ res_0 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ smmu_pmu->reg_base = devm_ioremap_resource(dev, res_0);
+ if (IS_ERR(smmu_pmu->reg_base))
+ return PTR_ERR(smmu_pmu->reg_base);
+
+ cfgr = readl_relaxed(smmu_pmu->reg_base + SMMU_PMCG_CFGR);
+
+ /* Determine if page 1 is present */
+ if (cfgr & SMMU_PMCG_CFGR_RELOC_CTRS) {
+ res_1 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ smmu_pmu->reloc_base = devm_ioremap_resource(dev, res_1);
+ if (IS_ERR(smmu_pmu->reloc_base))
+ return PTR_ERR(smmu_pmu->reloc_base);
+ } else {
+ smmu_pmu->reloc_base = smmu_pmu->reg_base;
+ }
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq > 0)
+ smmu_pmu->irq = irq;
+
+ ceid_64[0] = readq_relaxed(smmu_pmu->reg_base + SMMU_PMCG_CEID0);
+ ceid_64[1] = readq_relaxed(smmu_pmu->reg_base + SMMU_PMCG_CEID1);
+ bitmap_from_arr32(smmu_pmu->supported_events, (u32 *)ceid_64,
+ SMMU_ARCH_MAX_EVENTS);
+
+ smmu_pmu->num_counters = FIELD_GET(SMMU_PMCG_CFGR_NCTR_MASK, cfgr) + 1;
+ smmu_pmu->counter_present_mask = GENMASK(smmu_pmu->num_counters - 1, 0);
+
+ smmu_pmu->sid_filter_global = !!(cfgr & SMMU_PMCG_CFGR_SID_FILTER_TYPE);
+
+ reg_size = FIELD_GET(SMMU_PMCG_CFGR_SIZE_MASK, cfgr);
+ smmu_pmu->counter_mask = GENMASK_ULL(reg_size, 0);
+
+ smmu_pmu_reset(smmu_pmu);
+
+ err = smmu_pmu_setup_irq(smmu_pmu);
+ if (err) {
+ dev_err(dev, "Setup irq failed, PMU @%pa\n", &res_0->start);
+ return err;
+ }
+
+ name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "smmuv3_pmcg_%llx",
+ (res_0->start) >> SMMU_PA_SHIFT);
+ if (!name) {
+ dev_err(dev, "Create name failed, PMU @%pa\n", &res_0->start);
+ return -EINVAL;
+ }
+
+ /* Pick one CPU to be the preferred one to use */
+ smmu_pmu->on_cpu = get_cpu();
+ WARN_ON(irq_set_affinity(smmu_pmu->irq, cpumask_of(smmu_pmu->on_cpu)));
+
+ err = cpuhp_state_add_instance_nocalls(cpuhp_state_num,
+ &smmu_pmu->node);
+ if (err) {
+ dev_err(dev, "Error %d registering hotplug, PMU @%pa\n",
+ err, &res_0->start);
+ goto out_cpuhp_err;
+ }
+
+ err = perf_pmu_register(&smmu_pmu->pmu, name, -1);
+ if (err) {
+ dev_err(dev, "Error %d registering PMU @%pa\n",
+ err, &res_0->start);
+ goto out_unregister;
+ }
+
+ put_cpu();
+ dev_info(dev, "Registered PMU @ %pa using %d counters with %s filter settings\n",
+ &res_0->start, smmu_pmu->num_counters,
+ smmu_pmu->sid_filter_global ? "Global(Counter0)" :
+ "Individual");
+
+ return 0;
+
+out_unregister:
+ cpuhp_state_remove_instance_nocalls(cpuhp_state_num, &smmu_pmu->node);
+out_cpuhp_err:
+ put_cpu();
+ return err;
+}
+
+static int smmu_pmu_remove(struct platform_device *pdev)
+{
+ struct smmu_pmu *smmu_pmu = platform_get_drvdata(pdev);
+
+ perf_pmu_unregister(&smmu_pmu->pmu);
+ cpuhp_state_remove_instance_nocalls(cpuhp_state_num, &smmu_pmu->node);
+
+ return 0;
+}
+
+static void smmu_pmu_shutdown(struct platform_device *pdev)
+{
+ struct smmu_pmu *smmu_pmu = platform_get_drvdata(pdev);
+
+ smmu_pmu_disable(&smmu_pmu->pmu);
+}
+
+static struct platform_driver smmu_pmu_driver = {
+ .driver = {
+ .name = "arm-smmu-v3-pmu",
+ },
+ .probe = smmu_pmu_probe,
+ .remove = smmu_pmu_remove,
+ .shutdown = smmu_pmu_shutdown,
+};
+
+static int __init arm_smmu_pmu_init(void)
+{
+ cpuhp_state_num = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN,
+ "perf/arm/pmcg:online",
+ NULL,
+ smmu_pmu_offline_cpu);
+ if (cpuhp_state_num < 0)
+ return cpuhp_state_num;
+
+ return platform_driver_register(&smmu_pmu_driver);
+}
+module_init(arm_smmu_pmu_init);
+
+static void __exit arm_smmu_pmu_exit(void)
+{
+ platform_driver_unregister(&smmu_pmu_driver);
+ cpuhp_remove_multi_state(cpuhp_state_num);
+}
+
+module_exit(arm_smmu_pmu_exit);
+
+MODULE_DESCRIPTION("PMU driver for ARM SMMUv3 Performance Monitors Extension");
+MODULE_AUTHOR("Neil Leeder <nleeder@codeaurora.org>");
+MODULE_AUTHOR("Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 3/4] perf/smmuv3: Add MSI irq support
2018-10-16 12:49 [PATCH v4 0/4] arm64 SMMUv3 PMU driver with IORT support Shameer Kolothum
2018-10-16 12:49 ` [PATCH v4 1/4] acpi: arm64: add iort support for PMCG Shameer Kolothum
2018-10-16 12:49 ` [PATCH v4 2/4] perf: add arm64 smmuv3 pmu driver Shameer Kolothum
@ 2018-10-16 12:49 ` Shameer Kolothum
2018-10-17 3:35 ` kbuild test robot
2018-10-17 15:42 ` kbuild test robot
2018-10-16 12:49 ` [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum 162001800 quirk Shameer Kolothum
3 siblings, 2 replies; 17+ messages in thread
From: Shameer Kolothum @ 2018-10-16 12:49 UTC (permalink / raw)
To: lorenzo.pieralisi, robin.murphy, jean-philippe.brucker
Cc: will.deacon, mark.rutland, guohanjun, john.garry, pabba, vkilari,
rruigrok, linux-acpi, linux-kernel, linux-arm-kernel, linuxarm,
neil.m.leeder
This adds support for MSI-based counter overflow interrupt.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
drivers/perf/arm_smmuv3_pmu.c | 58 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
index e30b939..d927ef8 100644
--- a/drivers/perf/arm_smmuv3_pmu.c
+++ b/drivers/perf/arm_smmuv3_pmu.c
@@ -68,6 +68,7 @@
#define SMMU_PMCG_OVSSET0 0xCC0
#define SMMU_PMCG_CFGR 0xE00
#define SMMU_PMCG_CFGR_RELOC_CTRS BIT(20)
+#define SMMU_PMCG_CFGR_MSI BIT(21)
#define SMMU_PMCG_CFGR_SID_FILTER_TYPE BIT(23)
#define SMMU_PMCG_CFGR_SIZE_MASK GENMASK(13, 8)
#define SMMU_PMCG_CFGR_NCTR_MASK GENMASK(5, 0)
@@ -78,6 +79,12 @@
#define SMMU_PMCG_IRQ_CTRL 0xE50
#define SMMU_PMCG_IRQ_CTRL_IRQEN BIT(0)
#define SMMU_PMCG_IRQ_CFG0 0xE58
+#define SMMU_PMCG_IRQ_CFG1 0xE60
+#define SMMU_PMCG_IRQ_CFG2 0xE64
+
+/* MSI config fields */
+#define MSI_CFG0_ADDR_MASK GENMASK_ULL(51, 2)
+#define MSI_CFG2_MEMATTR_DEVICE_nGnRE 0x1
#define SMMU_DEFAULT_FILTER_SPAN 1
#define SMMU_DEFAULT_FILTER_STREAM_ID GENMASK(31, 0)
@@ -587,11 +594,62 @@ static irqreturn_t smmu_pmu_handle_irq(int irq_num, void *data)
return IRQ_HANDLED;
}
+static void smmu_pmu_free_msis(void *data)
+{
+ struct device *dev = data;
+
+ platform_msi_domain_free_irqs(dev);
+}
+
+static void smmu_pmu_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
+{
+ phys_addr_t doorbell;
+ struct device *dev = msi_desc_to_dev(desc);
+ struct smmu_pmu *pmu = dev_get_drvdata(dev);
+
+ doorbell = (((u64)msg->address_hi) << 32) | msg->address_lo;
+ doorbell &= MSI_CFG0_ADDR_MASK;
+
+ writeq_relaxed(doorbell, pmu->reg_base + SMMU_PMCG_IRQ_CFG0);
+ writel_relaxed(msg->data, pmu->reg_base + SMMU_PMCG_IRQ_CFG1);
+ writel_relaxed(MSI_CFG2_MEMATTR_DEVICE_nGnRE,
+ pmu->reg_base + SMMU_PMCG_IRQ_CFG2);
+}
+
+static void smmu_pmu_setup_msi(struct smmu_pmu *pmu)
+{
+ struct msi_desc *desc;
+ struct device *dev = pmu->dev;
+ int ret;
+
+ /* Clear MSI address reg */
+ writeq_relaxed(0, pmu->reg_base + SMMU_PMCG_IRQ_CFG0);
+
+ /* MSI supported or not */
+ if (!(readl(pmu->reg_base + SMMU_PMCG_CFGR) & SMMU_PMCG_CFGR_MSI))
+ return;
+
+ ret = platform_msi_domain_alloc_irqs(dev, 1, smmu_pmu_write_msi_msg);
+ if (ret) {
+ dev_warn(dev, "failed to allocate MSIs\n");
+ return;
+ }
+
+ desc = first_msi_entry(dev);
+ if (desc)
+ pmu->irq = desc->irq;
+
+ /* Add callback to free MSIs on teardown */
+ devm_add_action(dev, smmu_pmu_free_msis, dev);
+}
+
static int smmu_pmu_setup_irq(struct smmu_pmu *pmu)
{
unsigned long flags = IRQF_NOBALANCING | IRQF_SHARED | IRQF_NO_THREAD;
int irq, ret = -ENXIO;
+ smmu_pmu_setup_msi(pmu);
+
irq = pmu->irq;
if (irq)
ret = devm_request_irq(pmu->dev, irq, smmu_pmu_handle_irq,
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum 162001800 quirk
2018-10-16 12:49 [PATCH v4 0/4] arm64 SMMUv3 PMU driver with IORT support Shameer Kolothum
` (2 preceding siblings ...)
2018-10-16 12:49 ` [PATCH v4 3/4] perf/smmuv3: Add MSI irq support Shameer Kolothum
@ 2018-10-16 12:49 ` Shameer Kolothum
2018-10-18 11:43 ` Robin Murphy
3 siblings, 1 reply; 17+ messages in thread
From: Shameer Kolothum @ 2018-10-16 12:49 UTC (permalink / raw)
To: lorenzo.pieralisi, robin.murphy, jean-philippe.brucker
Cc: will.deacon, mark.rutland, guohanjun, john.garry, pabba, vkilari,
rruigrok, linux-acpi, linux-kernel, linux-arm-kernel, linuxarm,
neil.m.leeder
HiSilicon erratum 162001800 describes the limitation of
SMMUv3 PMCG implementation on HiSilicon Hip08 platforms.
On these platforms, the PMCG event counter registers
(SMMU_PMCG_EVCNTRn) are read only and as a result it is
not possible to set the initial counter period value on
event monitor start.
To work around this, the current value of the counter is
read and is used for delta calculations. This increases
the possibility of reporting incorrect values if counter
overflow happens and counter passes the initial value.
OEM information from ACPI header is used to identify the
affected hardware platform.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
drivers/perf/arm_smmuv3_pmu.c | 137 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 130 insertions(+), 7 deletions(-)
diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
index d927ef8..519545e 100644
--- a/drivers/perf/arm_smmuv3_pmu.c
+++ b/drivers/perf/arm_smmuv3_pmu.c
@@ -96,6 +96,8 @@
#define SMMU_PA_SHIFT 12
+#define SMMU_PMU_OPT_EVCNTR_RDONLY (1 << 0)
+
static int cpuhp_state_num;
struct smmu_pmu {
@@ -111,10 +113,55 @@ struct smmu_pmu {
struct device *dev;
void __iomem *reg_base;
void __iomem *reloc_base;
+ u32 options;
u64 counter_present_mask;
u64 counter_mask;
};
+struct erratum_acpi_oem_info {
+ char oem_id[ACPI_OEM_ID_SIZE + 1];
+ char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
+ u32 oem_revision;
+};
+
+static struct erratum_acpi_oem_info hisi_162001800_oem_info[] = {
+ /*
+ * Note that trailing spaces are required to properly match
+ * the OEM table information.
+ */
+ {
+ .oem_id = "HISI ",
+ .oem_table_id = "HIP08 ",
+ .oem_revision = 0,
+ },
+ { /* Sentinel indicating the end of the OEM array */ },
+};
+
+enum smmu_pmu_erratum_match_type {
+ se_match_acpi_oem,
+};
+
+void hisi_erratum_evcntr_rdonly(struct smmu_pmu *smmu_pmu)
+{
+ smmu_pmu->options |= SMMU_PMU_OPT_EVCNTR_RDONLY;
+}
+
+struct smmu_pmu_erratum_wa {
+ enum smmu_pmu_erratum_match_type match_type;
+ const void *id; /* Indicate the Erratum ID */
+ const char *desc_str;
+ void (*enable)(struct smmu_pmu *smmu_pmu);
+};
+
+static const struct smmu_pmu_erratum_wa smmu_pmu_wa[] = {
+ {
+ .match_type = se_match_acpi_oem,
+ .id = hisi_162001800_oem_info,
+ .desc_str = "HiSilicon erratum 162001800",
+ .enable = hisi_erratum_evcntr_rdonly,
+ },
+};
+
#define to_smmu_pmu(p) (container_of(p, struct smmu_pmu, pmu))
#define SMMU_PMU_EVENT_ATTR_EXTRACTOR(_name, _config, _start, _end) \
@@ -224,15 +271,20 @@ static void smmu_pmu_set_period(struct smmu_pmu *smmu_pmu,
u32 idx = hwc->idx;
u64 new;
- /*
- * We limit the max period to half the max counter value of the counter
- * size, so that even in the case of extreme interrupt latency the
- * counter will (hopefully) not wrap past its initial value.
- */
- new = smmu_pmu->counter_mask >> 1;
+ if (smmu_pmu->options & SMMU_PMU_OPT_EVCNTR_RDONLY) {
+ new = smmu_pmu_counter_get_value(smmu_pmu, idx);
+ } else {
+ /*
+ * We limit the max period to half the max counter value
+ * of the counter size, so that even in the case of extreme
+ * interrupt latency the counter will (hopefully) not wrap
+ * past its initial value.
+ */
+ new = smmu_pmu->counter_mask >> 1;
+ smmu_pmu_counter_set_value(smmu_pmu, idx, new);
+ }
local64_set(&hwc->prev_count, new);
- smmu_pmu_counter_set_value(smmu_pmu, idx, new);
}
static void smmu_pmu_get_event_filter(struct perf_event *event, u32 *span,
@@ -670,6 +722,69 @@ static void smmu_pmu_reset(struct smmu_pmu *smmu_pmu)
smmu_pmu->reloc_base + SMMU_PMCG_OVSCLR0);
}
+typedef bool (*se_match_fn_t)(const struct smmu_pmu_erratum_wa *,
+ const void *);
+
+bool smmu_pmu_check_acpi_erratum(const struct smmu_pmu_erratum_wa *wa,
+ const void *arg)
+{
+ static const struct erratum_acpi_oem_info empty_oem_info = {};
+ const struct erratum_acpi_oem_info *info = wa->id;
+ const struct acpi_table_header *hdr = arg;
+
+ /* Iterate over the ACPI OEM info array, looking for a match */
+ while (memcmp(info, &empty_oem_info, sizeof(*info))) {
+ if (!memcmp(info->oem_id, hdr->oem_id, ACPI_OEM_ID_SIZE) &&
+ !memcmp(info->oem_table_id, hdr->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) &&
+ info->oem_revision == hdr->oem_revision)
+ return true;
+
+ info++;
+ }
+
+ return false;
+
+}
+
+static void smmu_pmu_enable_errata(struct smmu_pmu *smmu_pmu,
+ enum smmu_pmu_erratum_match_type type,
+ se_match_fn_t match_fn,
+ void *arg)
+{
+ const struct smmu_pmu_erratum_wa *wa = smmu_pmu_wa;
+
+ for (; wa->desc_str; wa++) {
+ if (wa->match_type != type)
+ continue;
+
+ if (match_fn(wa, arg)) {
+ if (wa->enable) {
+ wa->enable(smmu_pmu);
+ dev_info(smmu_pmu->dev,
+ "Enabling workaround for %s\n",
+ wa->desc_str);
+ }
+ }
+ }
+}
+
+static void smmu_pmu_check_workarounds(struct smmu_pmu *smmu_pmu,
+ enum smmu_pmu_erratum_match_type type,
+ void *arg)
+{
+ se_match_fn_t match_fn = NULL;
+
+ switch (type) {
+ case se_match_acpi_oem:
+ match_fn = smmu_pmu_check_acpi_erratum;
+ break;
+ default:
+ return;
+ }
+
+ smmu_pmu_enable_errata(smmu_pmu, type, match_fn, arg);
+}
+
static int smmu_pmu_probe(struct platform_device *pdev)
{
struct smmu_pmu *smmu_pmu;
@@ -678,6 +793,7 @@ static int smmu_pmu_probe(struct platform_device *pdev)
u64 ceid_64[2];
int irq, err;
char *name;
+ struct acpi_table_header *table;
struct device *dev = &pdev->dev;
smmu_pmu = devm_kzalloc(dev, sizeof(*smmu_pmu), GFP_KERNEL);
@@ -749,6 +865,13 @@ static int smmu_pmu_probe(struct platform_device *pdev)
return -EINVAL;
}
+ if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_IORT, 0, &table))) {
+ dev_err(dev, "IORT get failed, PMU @%pa\n", &res_0->start);
+ return -EINVAL;
+ }
+
+ smmu_pmu_check_workarounds(smmu_pmu, se_match_acpi_oem, table);
+
/* Pick one CPU to be the preferred one to use */
smmu_pmu->on_cpu = get_cpu();
WARN_ON(irq_set_affinity(smmu_pmu->irq, cpumask_of(smmu_pmu->on_cpu)));
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v4 3/4] perf/smmuv3: Add MSI irq support
2018-10-16 12:49 ` [PATCH v4 3/4] perf/smmuv3: Add MSI irq support Shameer Kolothum
@ 2018-10-17 3:35 ` kbuild test robot
2018-10-17 9:48 ` Shameerali Kolothum Thodi
2018-10-17 15:42 ` kbuild test robot
1 sibling, 1 reply; 17+ messages in thread
From: kbuild test robot @ 2018-10-17 3:35 UTC (permalink / raw)
To: Shameer Kolothum
Cc: kbuild-all, lorenzo.pieralisi, robin.murphy,
jean-philippe.brucker, will.deacon, mark.rutland, guohanjun,
john.garry, pabba, vkilari, rruigrok, linux-acpi, linux-kernel,
linux-arm-kernel, linuxarm, neil.m.leeder
[-- Attachment #1: Type: text/plain, Size: 10011 bytes --]
Hi Shameer,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linux-sof-driver/master]
[also build test ERROR on v4.19-rc8 next-20181016]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Shameer-Kolothum/arm64-SMMUv3-PMU-driver-with-IORT-support/20181017-063949
base: https://github.com/thesofproject/linux master
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=sh
All error/warnings (new ones prefixed by >>):
In file included from include/linux/kernel.h:11:0,
from include/linux/list.h:9,
from include/linux/resource_ext.h:17,
from include/linux/acpi.h:26,
from drivers//perf/arm_smmuv3_pmu.c:37:
drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_counter_set_value':
include/linux/bitops.h:7:24: warning: left shift count >= width of type [-Wshift-count-overflow]
#define BIT(nr) (1UL << (nr))
^
drivers//perf/arm_smmuv3_pmu.c:152:31: note: in expansion of macro 'BIT'
if (smmu_pmu->counter_mask & BIT(32))
^~~
drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_counter_get_value':
include/linux/bitops.h:7:24: warning: left shift count >= width of type [-Wshift-count-overflow]
#define BIT(nr) (1UL << (nr))
^
drivers//perf/arm_smmuv3_pmu.c:162:31: note: in expansion of macro 'BIT'
if (smmu_pmu->counter_mask & BIT(32))
^~~
drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_free_msis':
>> drivers//perf/arm_smmuv3_pmu.c:601:2: error: implicit declaration of function 'platform_msi_domain_free_irqs'; did you mean 'platform_get_device_id'? [-Werror=implicit-function-declaration]
platform_msi_domain_free_irqs(dev);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
platform_get_device_id
drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_setup_msi':
>> drivers//perf/arm_smmuv3_pmu.c:632:8: error: implicit declaration of function 'platform_msi_domain_alloc_irqs'; did you mean 'platform_device_alloc'? [-Werror=implicit-function-declaration]
ret = platform_msi_domain_alloc_irqs(dev, 1, smmu_pmu_write_msi_msg);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
platform_device_alloc
In file included from include/linux/list.h:9:0,
from include/linux/resource_ext.h:17,
from include/linux/acpi.h:26,
from drivers//perf/arm_smmuv3_pmu.c:37:
include/linux/msi.h:114:38: error: 'struct device' has no member named 'msi_list'
#define dev_to_msi_list(dev) (&(dev)->msi_list)
^
include/linux/kernel.h:961:26: note: in definition of macro 'container_of'
void *__mptr = (void *)(ptr); \
^~~
include/linux/list.h:377:2: note: in expansion of macro 'list_entry'
list_entry((ptr)->next, type, member)
^~~~~~~~~~
include/linux/msi.h:116:2: note: in expansion of macro 'list_first_entry'
list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
^~~~~~~~~~~~~~~~
include/linux/msi.h:116:19: note: in expansion of macro 'dev_to_msi_list'
list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
^~~~~~~~~~~~~~~
>> drivers//perf/arm_smmuv3_pmu.c:638:9: note: in expansion of macro 'first_msi_entry'
desc = first_msi_entry(dev);
^~~~~~~~~~~~~~~
In file included from include/linux/ioport.h:13:0,
from include/linux/acpi.h:25,
from drivers//perf/arm_smmuv3_pmu.c:37:
include/linux/msi.h:114:38: error: 'struct device' has no member named 'msi_list'
#define dev_to_msi_list(dev) (&(dev)->msi_list)
^
include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
bool __cond = !(condition); \
^~~~~~~~~
include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/kernel.h:962:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~~~~~~
include/linux/kernel.h:962:20: note: in expansion of macro '__same_type'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~
include/linux/list.h:366:2: note: in expansion of macro 'container_of'
container_of(ptr, type, member)
^~~~~~~~~~~~
include/linux/list.h:377:2: note: in expansion of macro 'list_entry'
list_entry((ptr)->next, type, member)
^~~~~~~~~~
include/linux/msi.h:116:2: note: in expansion of macro 'list_first_entry'
list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
^~~~~~~~~~~~~~~~
include/linux/msi.h:116:19: note: in expansion of macro 'dev_to_msi_list'
list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
^~~~~~~~~~~~~~~
>> drivers//perf/arm_smmuv3_pmu.c:638:9: note: in expansion of macro 'first_msi_entry'
desc = first_msi_entry(dev);
^~~~~~~~~~~~~~~
include/linux/msi.h:114:38: error: 'struct device' has no member named 'msi_list'
#define dev_to_msi_list(dev) (&(dev)->msi_list)
^
include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
bool __cond = !(condition); \
^~~~~~~~~
include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/kernel.h:962:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~~~~~~
include/linux/kernel.h:963:6: note: in expansion of macro '__same_type'
!__same_type(*(ptr), void), \
^~~~~~~~~~~
include/linux/list.h:366:2: note: in expansion of macro 'container_of'
container_of(ptr, type, member)
^~~~~~~~~~~~
include/linux/list.h:377:2: note: in expansion of macro 'list_entry'
list_entry((ptr)->next, type, member)
^~~~~~~~~~
include/linux/msi.h:116:2: note: in expansion of macro 'list_first_entry'
list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
^~~~~~~~~~~~~~~~
include/linux/msi.h:116:19: note: in expansion of macro 'dev_to_msi_list'
list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
^~~~~~~~~~~~~~~
>> drivers//perf/arm_smmuv3_pmu.c:638:9: note: in expansion of macro 'first_msi_entry'
desc = first_msi_entry(dev);
^~~~~~~~~~~~~~~
drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_probe':
drivers//perf/arm_smmuv3_pmu.c:745:64: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'resource_size_t {aka unsigned int}' [-Wformat=]
name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "smmuv3_pmcg_%llx",
~~~^
%x
cc1: some warnings being treated as errors
vim +601 drivers//perf/arm_smmuv3_pmu.c
596
597 static void smmu_pmu_free_msis(void *data)
598 {
599 struct device *dev = data;
600
> 601 platform_msi_domain_free_irqs(dev);
602 }
603
604 static void smmu_pmu_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
605 {
606 phys_addr_t doorbell;
607 struct device *dev = msi_desc_to_dev(desc);
608 struct smmu_pmu *pmu = dev_get_drvdata(dev);
609
610 doorbell = (((u64)msg->address_hi) << 32) | msg->address_lo;
611 doorbell &= MSI_CFG0_ADDR_MASK;
612
613 writeq_relaxed(doorbell, pmu->reg_base + SMMU_PMCG_IRQ_CFG0);
614 writel_relaxed(msg->data, pmu->reg_base + SMMU_PMCG_IRQ_CFG1);
615 writel_relaxed(MSI_CFG2_MEMATTR_DEVICE_nGnRE,
616 pmu->reg_base + SMMU_PMCG_IRQ_CFG2);
617 }
618
619 static void smmu_pmu_setup_msi(struct smmu_pmu *pmu)
620 {
621 struct msi_desc *desc;
622 struct device *dev = pmu->dev;
623 int ret;
624
625 /* Clear MSI address reg */
626 writeq_relaxed(0, pmu->reg_base + SMMU_PMCG_IRQ_CFG0);
627
628 /* MSI supported or not */
629 if (!(readl(pmu->reg_base + SMMU_PMCG_CFGR) & SMMU_PMCG_CFGR_MSI))
630 return;
631
> 632 ret = platform_msi_domain_alloc_irqs(dev, 1, smmu_pmu_write_msi_msg);
633 if (ret) {
634 dev_warn(dev, "failed to allocate MSIs\n");
635 return;
636 }
637
> 638 desc = first_msi_entry(dev);
639 if (desc)
640 pmu->irq = desc->irq;
641
642 /* Add callback to free MSIs on teardown */
643 devm_add_action(dev, smmu_pmu_free_msis, dev);
644 }
645
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 47770 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v4 3/4] perf/smmuv3: Add MSI irq support
2018-10-17 3:35 ` kbuild test robot
@ 2018-10-17 9:48 ` Shameerali Kolothum Thodi
0 siblings, 0 replies; 17+ messages in thread
From: Shameerali Kolothum Thodi @ 2018-10-17 9:48 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, lorenzo.pieralisi, robin.murphy,
jean-philippe.brucker, will.deacon, mark.rutland,
Guohanjun (Hanjun Guo),
John Garry, pabba, vkilari, rruigrok, linux-acpi, linux-kernel,
linux-arm-kernel, Linuxarm, neil.m.leeder
> -----Original Message-----
> From: kbuild test robot [mailto:lkp@intel.com]
> Sent: 17 October 2018 04:36
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: kbuild-all@01.org; lorenzo.pieralisi@arm.com; robin.murphy@arm.com;
> jean-philippe.brucker@arm.com; will.deacon@arm.com;
> mark.rutland@arm.com; Guohanjun (Hanjun Guo) <guohanjun@huawei.com>;
> John Garry <john.garry@huawei.com>; pabba@codeaurora.org;
> vkilari@codeaurora.org; rruigrok@codeaurora.org; linux-
> acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; Linuxarm <linuxarm@huawei.com>;
> neil.m.leeder@gmail.com
> Subject: Re: [PATCH v4 3/4] perf/smmuv3: Add MSI irq support
>
> Hi Shameer,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on linux-sof-driver/master]
> [also build test ERROR on v4.19-rc8 next-20181016]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Shameer-Kolothum/arm64-
> SMMUv3-PMU-driver-with-IORT-support/20181017-063949
> base: https://github.com/thesofproject/linux master
> config: sh-allmodconfig (attached as .config)
> compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-
> tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=7.2.0 make.cross ARCH=sh
>
> All error/warnings (new ones prefixed by >>):
>
> In file included from include/linux/kernel.h:11:0,
> from include/linux/list.h:9,
> from include/linux/resource_ext.h:17,
> from include/linux/acpi.h:26,
> from drivers//perf/arm_smmuv3_pmu.c:37:
> drivers//perf/arm_smmuv3_pmu.c: In function
> 'smmu_pmu_counter_set_value':
> include/linux/bitops.h:7:24: warning: left shift count >= width of type [-Wshift-
> count-overflow]
> #define BIT(nr) (1UL << (nr))
> ^
> drivers//perf/arm_smmuv3_pmu.c:152:31: note: in expansion of macro 'BIT'
> if (smmu_pmu->counter_mask & BIT(32))
> ^~~
> drivers//perf/arm_smmuv3_pmu.c: In function
> 'smmu_pmu_counter_get_value':
> include/linux/bitops.h:7:24: warning: left shift count >= width of type [-Wshift-
> count-overflow]
> #define BIT(nr) (1UL << (nr))
> ^
> drivers//perf/arm_smmuv3_pmu.c:162:31: note: in expansion of macro 'BIT'
> if (smmu_pmu->counter_mask & BIT(32))
> ^~~
> drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_free_msis':
> >> drivers//perf/arm_smmuv3_pmu.c:601:2: error: implicit declaration of
> function 'platform_msi_domain_free_irqs'; did you mean
> 'platform_get_device_id'? [-Werror=implicit-function-declaration]
> platform_msi_domain_free_irqs(dev);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Ok. This is probably because of the COMPILE_TEST added to patch #2 and
this one will have dependency on PCI/PCI_MSI. I will remove that in next
revision.
Thanks,
Shameer
> platform_get_device_id
> drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_setup_msi':
> >> drivers//perf/arm_smmuv3_pmu.c:632:8: error: implicit declaration of
> function 'platform_msi_domain_alloc_irqs'; did you mean
> 'platform_device_alloc'? [-Werror=implicit-function-declaration]
> ret = platform_msi_domain_alloc_irqs(dev, 1, smmu_pmu_write_msi_msg);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> platform_device_alloc
> In file included from include/linux/list.h:9:0,
> from include/linux/resource_ext.h:17,
> from include/linux/acpi.h:26,
> from drivers//perf/arm_smmuv3_pmu.c:37:
> include/linux/msi.h:114:38: error: 'struct device' has no member named
> 'msi_list'
> #define dev_to_msi_list(dev) (&(dev)->msi_list)
> ^
> include/linux/kernel.h:961:26: note: in definition of macro 'container_of'
> void *__mptr = (void *)(ptr); \
> ^~~
> include/linux/list.h:377:2: note: in expansion of macro 'list_entry'
> list_entry((ptr)->next, type, member)
> ^~~~~~~~~~
> include/linux/msi.h:116:2: note: in expansion of macro 'list_first_entry'
> list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
> ^~~~~~~~~~~~~~~~
> include/linux/msi.h:116:19: note: in expansion of macro 'dev_to_msi_list'
> list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
> ^~~~~~~~~~~~~~~
> >> drivers//perf/arm_smmuv3_pmu.c:638:9: note: in expansion of macro
> 'first_msi_entry'
> desc = first_msi_entry(dev);
> ^~~~~~~~~~~~~~~
> In file included from include/linux/ioport.h:13:0,
> from include/linux/acpi.h:25,
> from drivers//perf/arm_smmuv3_pmu.c:37:
> include/linux/msi.h:114:38: error: 'struct device' has no member named
> 'msi_list'
> #define dev_to_msi_list(dev) (&(dev)->msi_list)
> ^
> include/linux/compiler.h:316:19: note: in definition of macro
> '__compiletime_assert'
> bool __cond = !(condition); \
> ^~~~~~~~~
> include/linux/compiler.h:339:2: note: in expansion of macro
> '_compiletime_assert'
> _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
> ^~~~~~~~~~~~~~~~~~~
> include/linux/build_bug.h:45:37: note: in expansion of macro
> 'compiletime_assert'
> #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> ^~~~~~~~~~~~~~~~~~
> include/linux/kernel.h:962:2: note: in expansion of macro
> 'BUILD_BUG_ON_MSG'
> BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
> ^~~~~~~~~~~~~~~~
> include/linux/kernel.h:962:20: note: in expansion of macro '__same_type'
> BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
> ^~~~~~~~~~~
> include/linux/list.h:366:2: note: in expansion of macro 'container_of'
> container_of(ptr, type, member)
> ^~~~~~~~~~~~
> include/linux/list.h:377:2: note: in expansion of macro 'list_entry'
> list_entry((ptr)->next, type, member)
> ^~~~~~~~~~
> include/linux/msi.h:116:2: note: in expansion of macro 'list_first_entry'
> list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
> ^~~~~~~~~~~~~~~~
> include/linux/msi.h:116:19: note: in expansion of macro 'dev_to_msi_list'
> list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
> ^~~~~~~~~~~~~~~
> >> drivers//perf/arm_smmuv3_pmu.c:638:9: note: in expansion of macro
> 'first_msi_entry'
> desc = first_msi_entry(dev);
> ^~~~~~~~~~~~~~~
> include/linux/msi.h:114:38: error: 'struct device' has no member named
> 'msi_list'
> #define dev_to_msi_list(dev) (&(dev)->msi_list)
> ^
> include/linux/compiler.h:316:19: note: in definition of macro
> '__compiletime_assert'
> bool __cond = !(condition); \
> ^~~~~~~~~
> include/linux/compiler.h:339:2: note: in expansion of macro
> '_compiletime_assert'
> _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
> ^~~~~~~~~~~~~~~~~~~
> include/linux/build_bug.h:45:37: note: in expansion of macro
> 'compiletime_assert'
> #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> ^~~~~~~~~~~~~~~~~~
> include/linux/kernel.h:962:2: note: in expansion of macro
> 'BUILD_BUG_ON_MSG'
> BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
> ^~~~~~~~~~~~~~~~
> include/linux/kernel.h:963:6: note: in expansion of macro '__same_type'
> !__same_type(*(ptr), void), \
> ^~~~~~~~~~~
> include/linux/list.h:366:2: note: in expansion of macro 'container_of'
> container_of(ptr, type, member)
> ^~~~~~~~~~~~
> include/linux/list.h:377:2: note: in expansion of macro 'list_entry'
> list_entry((ptr)->next, type, member)
> ^~~~~~~~~~
> include/linux/msi.h:116:2: note: in expansion of macro 'list_first_entry'
> list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
> ^~~~~~~~~~~~~~~~
> include/linux/msi.h:116:19: note: in expansion of macro 'dev_to_msi_list'
> list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
> ^~~~~~~~~~~~~~~
> >> drivers//perf/arm_smmuv3_pmu.c:638:9: note: in expansion of macro
> 'first_msi_entry'
> desc = first_msi_entry(dev);
> ^~~~~~~~~~~~~~~
> drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_probe':
> drivers//perf/arm_smmuv3_pmu.c:745:64: warning: format '%llx' expects
> argument of type 'long long unsigned int', but argument 4 has type
> 'resource_size_t {aka unsigned int}' [-Wformat=]
> name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "smmuv3_pmcg_%llx",
> ~~~^
> %x
> cc1: some warnings being treated as errors
>
> vim +601 drivers//perf/arm_smmuv3_pmu.c
>
> 596
> 597 static void smmu_pmu_free_msis(void *data)
> 598 {
> 599 struct device *dev = data;
> 600
> > 601 platform_msi_domain_free_irqs(dev);
> 602 }
> 603
> 604 static void smmu_pmu_write_msi_msg(struct msi_desc *desc, struct
> msi_msg *msg)
> 605 {
> 606 phys_addr_t doorbell;
> 607 struct device *dev = msi_desc_to_dev(desc);
> 608 struct smmu_pmu *pmu = dev_get_drvdata(dev);
> 609
> 610 doorbell = (((u64)msg->address_hi) << 32) | msg->address_lo;
> 611 doorbell &= MSI_CFG0_ADDR_MASK;
> 612
> 613 writeq_relaxed(doorbell, pmu->reg_base +
> SMMU_PMCG_IRQ_CFG0);
> 614 writel_relaxed(msg->data, pmu->reg_base +
> SMMU_PMCG_IRQ_CFG1);
> 615 writel_relaxed(MSI_CFG2_MEMATTR_DEVICE_nGnRE,
> 616 pmu->reg_base + SMMU_PMCG_IRQ_CFG2);
> 617 }
> 618
> 619 static void smmu_pmu_setup_msi(struct smmu_pmu *pmu)
> 620 {
> 621 struct msi_desc *desc;
> 622 struct device *dev = pmu->dev;
> 623 int ret;
> 624
> 625 /* Clear MSI address reg */
> 626 writeq_relaxed(0, pmu->reg_base +
> SMMU_PMCG_IRQ_CFG0);
> 627
> 628 /* MSI supported or not */
> 629 if (!(readl(pmu->reg_base + SMMU_PMCG_CFGR) &
> SMMU_PMCG_CFGR_MSI))
> 630 return;
> 631
> > 632 ret = platform_msi_domain_alloc_irqs(dev, 1,
> smmu_pmu_write_msi_msg);
> 633 if (ret) {
> 634 dev_warn(dev, "failed to allocate MSIs\n");
> 635 return;
> 636 }
> 637
> > 638 desc = first_msi_entry(dev);
> 639 if (desc)
> 640 pmu->irq = desc->irq;
> 641
> 642 /* Add callback to free MSIs on teardown */
> 643 devm_add_action(dev, smmu_pmu_free_msis, dev);
> 644 }
> 645
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 3/4] perf/smmuv3: Add MSI irq support
2018-10-16 12:49 ` [PATCH v4 3/4] perf/smmuv3: Add MSI irq support Shameer Kolothum
2018-10-17 3:35 ` kbuild test robot
@ 2018-10-17 15:42 ` kbuild test robot
1 sibling, 0 replies; 17+ messages in thread
From: kbuild test robot @ 2018-10-17 15:42 UTC (permalink / raw)
To: Shameer Kolothum
Cc: kbuild-all, lorenzo.pieralisi, robin.murphy,
jean-philippe.brucker, will.deacon, mark.rutland, guohanjun,
john.garry, pabba, vkilari, rruigrok, linux-acpi, linux-kernel,
linux-arm-kernel, linuxarm, neil.m.leeder
[-- Attachment #1: Type: text/plain, Size: 8108 bytes --]
Hi Shameer,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linux-sof-driver/master]
[also build test ERROR on v4.19-rc8 next-20181017]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Shameer-Kolothum/arm64-SMMUv3-PMU-driver-with-IORT-support/20181017-063949
base: https://github.com/thesofproject/linux master
config: sh-allyesconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=sh
All errors (new ones prefixed by >>):
In file included from include/linux/kernel.h:11:0,
from include/linux/list.h:9,
from include/linux/resource_ext.h:17,
from include/linux/acpi.h:26,
from drivers/perf/arm_smmuv3_pmu.c:37:
drivers/perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_counter_set_value':
include/linux/bitops.h:7:24: warning: left shift count >= width of type [-Wshift-count-overflow]
#define BIT(nr) (1UL << (nr))
^
drivers/perf/arm_smmuv3_pmu.c:152:31: note: in expansion of macro 'BIT'
if (smmu_pmu->counter_mask & BIT(32))
^~~
drivers/perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_counter_get_value':
include/linux/bitops.h:7:24: warning: left shift count >= width of type [-Wshift-count-overflow]
#define BIT(nr) (1UL << (nr))
^
drivers/perf/arm_smmuv3_pmu.c:162:31: note: in expansion of macro 'BIT'
if (smmu_pmu->counter_mask & BIT(32))
^~~
drivers/perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_free_msis':
>> drivers/perf/arm_smmuv3_pmu.c:601:2: error: implicit declaration of function 'platform_msi_domain_free_irqs'; did you mean 'platform_pm_freeze'? [-Werror=implicit-function-declaration]
platform_msi_domain_free_irqs(dev);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
platform_pm_freeze
drivers/perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_setup_msi':
drivers/perf/arm_smmuv3_pmu.c:632:8: error: implicit declaration of function 'platform_msi_domain_alloc_irqs'; did you mean 'platform_device_alloc'? [-Werror=implicit-function-declaration]
ret = platform_msi_domain_alloc_irqs(dev, 1, smmu_pmu_write_msi_msg);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
platform_device_alloc
In file included from include/linux/list.h:9:0,
from include/linux/resource_ext.h:17,
from include/linux/acpi.h:26,
from drivers/perf/arm_smmuv3_pmu.c:37:
include/linux/msi.h:114:38: error: 'struct device' has no member named 'msi_list'
#define dev_to_msi_list(dev) (&(dev)->msi_list)
^
include/linux/kernel.h:961:26: note: in definition of macro 'container_of'
void *__mptr = (void *)(ptr); \
^~~
include/linux/list.h:377:2: note: in expansion of macro 'list_entry'
list_entry((ptr)->next, type, member)
^~~~~~~~~~
include/linux/msi.h:116:2: note: in expansion of macro 'list_first_entry'
list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
^~~~~~~~~~~~~~~~
include/linux/msi.h:116:19: note: in expansion of macro 'dev_to_msi_list'
list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
^~~~~~~~~~~~~~~
drivers/perf/arm_smmuv3_pmu.c:638:9: note: in expansion of macro 'first_msi_entry'
desc = first_msi_entry(dev);
^~~~~~~~~~~~~~~
In file included from include/linux/ioport.h:13:0,
from include/linux/acpi.h:25,
from drivers/perf/arm_smmuv3_pmu.c:37:
include/linux/msi.h:114:38: error: 'struct device' has no member named 'msi_list'
#define dev_to_msi_list(dev) (&(dev)->msi_list)
^
include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
bool __cond = !(condition); \
^~~~~~~~~
include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/kernel.h:962:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~~~~~~
include/linux/kernel.h:962:20: note: in expansion of macro '__same_type'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~
include/linux/list.h:366:2: note: in expansion of macro 'container_of'
container_of(ptr, type, member)
^~~~~~~~~~~~
include/linux/list.h:377:2: note: in expansion of macro 'list_entry'
list_entry((ptr)->next, type, member)
^~~~~~~~~~
include/linux/msi.h:116:2: note: in expansion of macro 'list_first_entry'
list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
^~~~~~~~~~~~~~~~
include/linux/msi.h:116:19: note: in expansion of macro 'dev_to_msi_list'
list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
^~~~~~~~~~~~~~~
drivers/perf/arm_smmuv3_pmu.c:638:9: note: in expansion of macro 'first_msi_entry'
desc = first_msi_entry(dev);
^~~~~~~~~~~~~~~
include/linux/msi.h:114:38: error: 'struct device' has no member named 'msi_list'
#define dev_to_msi_list(dev) (&(dev)->msi_list)
^
include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
bool __cond = !(condition); \
^~~~~~~~~
include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/kernel.h:962:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~~~~~~
include/linux/kernel.h:963:6: note: in expansion of macro '__same_type'
!__same_type(*(ptr), void), \
^~~~~~~~~~~
include/linux/list.h:366:2: note: in expansion of macro 'container_of'
container_of(ptr, type, member)
^~~~~~~~~~~~
include/linux/list.h:377:2: note: in expansion of macro 'list_entry'
list_entry((ptr)->next, type, member)
^~~~~~~~~~
include/linux/msi.h:116:2: note: in expansion of macro 'list_first_entry'
list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
^~~~~~~~~~~~~~~~
include/linux/msi.h:116:19: note: in expansion of macro 'dev_to_msi_list'
list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list)
^~~~~~~~~~~~~~~
drivers/perf/arm_smmuv3_pmu.c:638:9: note: in expansion of macro 'first_msi_entry'
desc = first_msi_entry(dev);
^~~~~~~~~~~~~~~
drivers/perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_probe':
vim +601 drivers/perf/arm_smmuv3_pmu.c
596
597 static void smmu_pmu_free_msis(void *data)
598 {
599 struct device *dev = data;
600
> 601 platform_msi_domain_free_irqs(dev);
602 }
603
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 47775 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/4] perf: add arm64 smmuv3 pmu driver
2018-10-16 12:49 ` [PATCH v4 2/4] perf: add arm64 smmuv3 pmu driver Shameer Kolothum
@ 2018-10-17 21:53 ` kbuild test robot
2018-10-18 9:26 ` Shameerali Kolothum Thodi
2018-10-20 4:50 ` kbuild test robot
1 sibling, 1 reply; 17+ messages in thread
From: kbuild test robot @ 2018-10-17 21:53 UTC (permalink / raw)
To: Shameer Kolothum
Cc: kbuild-all, lorenzo.pieralisi, robin.murphy,
jean-philippe.brucker, will.deacon, mark.rutland, guohanjun,
john.garry, pabba, vkilari, rruigrok, linux-acpi, linux-kernel,
linux-arm-kernel, linuxarm, neil.m.leeder
[-- Attachment #1: Type: text/plain, Size: 8562 bytes --]
Hi Neil,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linux-sof-driver/master]
[also build test ERROR on v4.19-rc8 next-20181017]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Shameer-Kolothum/arm64-SMMUv3-PMU-driver-with-IORT-support/20181017-063949
base: https://github.com/thesofproject/linux master
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
In file included from include/linux/kernel.h:11,
from include/linux/list.h:9,
from include/linux/resource_ext.h:17,
from include/linux/acpi.h:26,
from drivers//perf/arm_smmuv3_pmu.c:37:
drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_counter_set_value':
include/linux/bitops.h:7:24: warning: left shift count >= width of type [-Wshift-count-overflow]
#define BIT(nr) (1UL << (nr))
^~
drivers//perf/arm_smmuv3_pmu.c:145:31: note: in expansion of macro 'BIT'
if (smmu_pmu->counter_mask & BIT(32))
^~~
drivers//perf/arm_smmuv3_pmu.c:146:3: error: implicit declaration of function 'writeq'; did you mean 'writel'? [-Werror=implicit-function-declaration]
writeq(value, smmu_pmu->reloc_base + SMMU_PMCG_EVCNTR(idx, 8));
^~~~~~
writel
In file included from include/linux/kernel.h:11,
from include/linux/list.h:9,
from include/linux/resource_ext.h:17,
from include/linux/acpi.h:26,
from drivers//perf/arm_smmuv3_pmu.c:37:
drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_counter_get_value':
include/linux/bitops.h:7:24: warning: left shift count >= width of type [-Wshift-count-overflow]
#define BIT(nr) (1UL << (nr))
^~
drivers//perf/arm_smmuv3_pmu.c:155:31: note: in expansion of macro 'BIT'
if (smmu_pmu->counter_mask & BIT(32))
^~~
drivers//perf/arm_smmuv3_pmu.c:156:11: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
value = readq(smmu_pmu->reloc_base + SMMU_PMCG_EVCNTR(idx, 8));
^~~~~
readl
drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_reset':
drivers//perf/arm_smmuv3_pmu.c:607:2: error: implicit declaration of function 'writeq_relaxed'; did you mean 'writel_relaxed'? [-Werror=implicit-function-declaration]
writeq_relaxed(smmu_pmu->counter_present_mask,
^~~~~~~~~~~~~~
writel_relaxed
drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_probe':
>> drivers//perf/arm_smmuv3_pmu.c:666:15: error: implicit declaration of function 'readq_relaxed'; did you mean 'readl_relaxed'? [-Werror=implicit-function-declaration]
ceid_64[0] = readq_relaxed(smmu_pmu->reg_base + SMMU_PMCG_CEID0);
^~~~~~~~~~~~~
readl_relaxed
drivers//perf/arm_smmuv3_pmu.c:687:64: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'resource_size_t' {aka 'unsigned int'} [-Wformat=]
name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "smmuv3_pmcg_%llx",
~~~^
%x
cc1: some warnings being treated as errors
vim +666 drivers//perf/arm_smmuv3_pmu.c
601
602 static void smmu_pmu_reset(struct smmu_pmu *smmu_pmu)
603 {
604 smmu_pmu_disable(&smmu_pmu->pmu);
605
606 /* Disable counter and interrupt */
> 607 writeq_relaxed(smmu_pmu->counter_present_mask,
608 smmu_pmu->reg_base + SMMU_PMCG_CNTENCLR0);
609 writeq_relaxed(smmu_pmu->counter_present_mask,
610 smmu_pmu->reg_base + SMMU_PMCG_INTENCLR0);
611 writeq_relaxed(smmu_pmu->counter_present_mask,
612 smmu_pmu->reloc_base + SMMU_PMCG_OVSCLR0);
613 }
614
615 static int smmu_pmu_probe(struct platform_device *pdev)
616 {
617 struct smmu_pmu *smmu_pmu;
618 struct resource *res_0, *res_1;
619 u32 cfgr, reg_size;
620 u64 ceid_64[2];
621 int irq, err;
622 char *name;
623 struct device *dev = &pdev->dev;
624
625 smmu_pmu = devm_kzalloc(dev, sizeof(*smmu_pmu), GFP_KERNEL);
626 if (!smmu_pmu)
627 return -ENOMEM;
628
629 smmu_pmu->dev = dev;
630 platform_set_drvdata(pdev, smmu_pmu);
631
632 smmu_pmu->pmu = (struct pmu) {
633 .task_ctx_nr = perf_invalid_context,
634 .pmu_enable = smmu_pmu_enable,
635 .pmu_disable = smmu_pmu_disable,
636 .event_init = smmu_pmu_event_init,
637 .add = smmu_pmu_event_add,
638 .del = smmu_pmu_event_del,
639 .start = smmu_pmu_event_start,
640 .stop = smmu_pmu_event_stop,
641 .read = smmu_pmu_event_read,
642 .attr_groups = smmu_pmu_attr_grps,
643 };
644
645 res_0 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
646 smmu_pmu->reg_base = devm_ioremap_resource(dev, res_0);
647 if (IS_ERR(smmu_pmu->reg_base))
648 return PTR_ERR(smmu_pmu->reg_base);
649
650 cfgr = readl_relaxed(smmu_pmu->reg_base + SMMU_PMCG_CFGR);
651
652 /* Determine if page 1 is present */
653 if (cfgr & SMMU_PMCG_CFGR_RELOC_CTRS) {
654 res_1 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
655 smmu_pmu->reloc_base = devm_ioremap_resource(dev, res_1);
656 if (IS_ERR(smmu_pmu->reloc_base))
657 return PTR_ERR(smmu_pmu->reloc_base);
658 } else {
659 smmu_pmu->reloc_base = smmu_pmu->reg_base;
660 }
661
662 irq = platform_get_irq(pdev, 0);
663 if (irq > 0)
664 smmu_pmu->irq = irq;
665
> 666 ceid_64[0] = readq_relaxed(smmu_pmu->reg_base + SMMU_PMCG_CEID0);
667 ceid_64[1] = readq_relaxed(smmu_pmu->reg_base + SMMU_PMCG_CEID1);
668 bitmap_from_arr32(smmu_pmu->supported_events, (u32 *)ceid_64,
669 SMMU_ARCH_MAX_EVENTS);
670
671 smmu_pmu->num_counters = FIELD_GET(SMMU_PMCG_CFGR_NCTR_MASK, cfgr) + 1;
672 smmu_pmu->counter_present_mask = GENMASK(smmu_pmu->num_counters - 1, 0);
673
674 smmu_pmu->sid_filter_global = !!(cfgr & SMMU_PMCG_CFGR_SID_FILTER_TYPE);
675
676 reg_size = FIELD_GET(SMMU_PMCG_CFGR_SIZE_MASK, cfgr);
677 smmu_pmu->counter_mask = GENMASK_ULL(reg_size, 0);
678
679 smmu_pmu_reset(smmu_pmu);
680
681 err = smmu_pmu_setup_irq(smmu_pmu);
682 if (err) {
683 dev_err(dev, "Setup irq failed, PMU @%pa\n", &res_0->start);
684 return err;
685 }
686
687 name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "smmuv3_pmcg_%llx",
688 (res_0->start) >> SMMU_PA_SHIFT);
689 if (!name) {
690 dev_err(dev, "Create name failed, PMU @%pa\n", &res_0->start);
691 return -EINVAL;
692 }
693
694 /* Pick one CPU to be the preferred one to use */
695 smmu_pmu->on_cpu = get_cpu();
696 WARN_ON(irq_set_affinity(smmu_pmu->irq, cpumask_of(smmu_pmu->on_cpu)));
697
698 err = cpuhp_state_add_instance_nocalls(cpuhp_state_num,
699 &smmu_pmu->node);
700 if (err) {
701 dev_err(dev, "Error %d registering hotplug, PMU @%pa\n",
702 err, &res_0->start);
703 goto out_cpuhp_err;
704 }
705
706 err = perf_pmu_register(&smmu_pmu->pmu, name, -1);
707 if (err) {
708 dev_err(dev, "Error %d registering PMU @%pa\n",
709 err, &res_0->start);
710 goto out_unregister;
711 }
712
713 put_cpu();
714 dev_info(dev, "Registered PMU @ %pa using %d counters with %s filter settings\n",
715 &res_0->start, smmu_pmu->num_counters,
716 smmu_pmu->sid_filter_global ? "Global(Counter0)" :
717 "Individual");
718
719 return 0;
720
721 out_unregister:
722 cpuhp_state_remove_instance_nocalls(cpuhp_state_num, &smmu_pmu->node);
723 out_cpuhp_err:
724 put_cpu();
725 return err;
726 }
727
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52821 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v4 2/4] perf: add arm64 smmuv3 pmu driver
2018-10-17 21:53 ` kbuild test robot
@ 2018-10-18 9:26 ` Shameerali Kolothum Thodi
0 siblings, 0 replies; 17+ messages in thread
From: Shameerali Kolothum Thodi @ 2018-10-18 9:26 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, lorenzo.pieralisi, robin.murphy,
jean-philippe.brucker, will.deacon, mark.rutland,
Guohanjun (Hanjun Guo),
John Garry, pabba, vkilari, rruigrok, linux-acpi, linux-kernel,
linux-arm-kernel, Linuxarm, neil.m.leeder
> -----Original Message-----
> From: kbuild test robot [mailto:lkp@intel.com]
> Sent: 17 October 2018 22:53
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: kbuild-all@01.org; lorenzo.pieralisi@arm.com; robin.murphy@arm.com;
> jean-philippe.brucker@arm.com; will.deacon@arm.com;
> mark.rutland@arm.com; Guohanjun (Hanjun Guo) <guohanjun@huawei.com>;
> John Garry <john.garry@huawei.com>; pabba@codeaurora.org;
> vkilari@codeaurora.org; rruigrok@codeaurora.org; linux-
> acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; Linuxarm <linuxarm@huawei.com>;
> neil.m.leeder@gmail.com
> Subject: Re: [PATCH v4 2/4] perf: add arm64 smmuv3 pmu driver
>
> Hi Neil,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on linux-sof-driver/master]
> [also build test ERROR on v4.19-rc8 next-20181017]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Shameer-Kolothum/arm64-
> SMMUv3-PMU-driver-with-IORT-support/20181017-063949
> base: https://github.com/thesofproject/linux master
> config: xtensa-allyesconfig (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 8.1.0
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-
> tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=8.1.0 make.cross ARCH=xtensa
>
> All errors (new ones prefixed by >>):
>
> In file included from include/linux/kernel.h:11,
> from include/linux/list.h:9,
> from include/linux/resource_ext.h:17,
> from include/linux/acpi.h:26,
> from drivers//perf/arm_smmuv3_pmu.c:37:
> drivers//perf/arm_smmuv3_pmu.c: In function
> 'smmu_pmu_counter_set_value':
> include/linux/bitops.h:7:24: warning: left shift count >= width of type [-Wshift-
> count-overflow]
> #define BIT(nr) (1UL << (nr))
> ^~
> drivers//perf/arm_smmuv3_pmu.c:145:31: note: in expansion of macro 'BIT'
> if (smmu_pmu->counter_mask & BIT(32))
> ^~~
> drivers//perf/arm_smmuv3_pmu.c:146:3: error: implicit declaration of
> function 'writeq'; did you mean 'writel'? [-Werror=implicit-function-
> declaration]
> writeq(value, smmu_pmu->reloc_base + SMMU_PMCG_EVCNTR(idx, 8));
> ^~~~~~
> writel
> In file included from include/linux/kernel.h:11,
> from include/linux/list.h:9,
> from include/linux/resource_ext.h:17,
> from include/linux/acpi.h:26,
> from drivers//perf/arm_smmuv3_pmu.c:37:
> drivers//perf/arm_smmuv3_pmu.c: In function
> 'smmu_pmu_counter_get_value':
> include/linux/bitops.h:7:24: warning: left shift count >= width of type [-Wshift-
> count-overflow]
> #define BIT(nr) (1UL << (nr))
> ^~
> drivers//perf/arm_smmuv3_pmu.c:155:31: note: in expansion of macro 'BIT'
> if (smmu_pmu->counter_mask & BIT(32))
> ^~~
> drivers//perf/arm_smmuv3_pmu.c:156:11: error: implicit declaration of
> function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
> value = readq(smmu_pmu->reloc_base + SMMU_PMCG_EVCNTR(idx, 8));
> ^~~~~
> readl
Right. This again is linked to the COMPILE_TEST added in this version of the series.
It looks like these functions has dependency on architecture (CONFIG_64BIT). I will
take care of this in next revision.
Thanks,
Shameer
> drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_reset':
> drivers//perf/arm_smmuv3_pmu.c:607:2: error: implicit declaration of
> function 'writeq_relaxed'; did you mean 'writel_relaxed'? [-Werror=implicit-
> function-declaration]
> writeq_relaxed(smmu_pmu->counter_present_mask,
> ^~~~~~~~~~~~~~
> writel_relaxed
> drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_probe':
> >> drivers//perf/arm_smmuv3_pmu.c:666:15: error: implicit declaration of
> function 'readq_relaxed'; did you mean 'readl_relaxed'? [-Werror=implicit-
> function-declaration]
> ceid_64[0] = readq_relaxed(smmu_pmu->reg_base +
> SMMU_PMCG_CEID0);
> ^~~~~~~~~~~~~
> readl_relaxed
> drivers//perf/arm_smmuv3_pmu.c:687:64: warning: format '%llx' expects
> argument of type 'long long unsigned int', but argument 4 has type
> 'resource_size_t' {aka 'unsigned int'} [-Wformat=]
> name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "smmuv3_pmcg_%llx",
> ~~~^
> %x
> cc1: some warnings being treated as errors
>
> vim +666 drivers//perf/arm_smmuv3_pmu.c
>
> 601
> 602 static void smmu_pmu_reset(struct smmu_pmu *smmu_pmu)
> 603 {
> 604 smmu_pmu_disable(&smmu_pmu->pmu);
> 605
> 606 /* Disable counter and interrupt */
> > 607 writeq_relaxed(smmu_pmu->counter_present_mask,
> 608 smmu_pmu->reg_base +
> SMMU_PMCG_CNTENCLR0);
> 609 writeq_relaxed(smmu_pmu->counter_present_mask,
> 610 smmu_pmu->reg_base +
> SMMU_PMCG_INTENCLR0);
> 611 writeq_relaxed(smmu_pmu->counter_present_mask,
> 612 smmu_pmu->reloc_base +
> SMMU_PMCG_OVSCLR0);
> 613 }
> 614
> 615 static int smmu_pmu_probe(struct platform_device *pdev)
> 616 {
> 617 struct smmu_pmu *smmu_pmu;
> 618 struct resource *res_0, *res_1;
> 619 u32 cfgr, reg_size;
> 620 u64 ceid_64[2];
> 621 int irq, err;
> 622 char *name;
> 623 struct device *dev = &pdev->dev;
> 624
> 625 smmu_pmu = devm_kzalloc(dev, sizeof(*smmu_pmu),
> GFP_KERNEL);
> 626 if (!smmu_pmu)
> 627 return -ENOMEM;
> 628
> 629 smmu_pmu->dev = dev;
> 630 platform_set_drvdata(pdev, smmu_pmu);
> 631
> 632 smmu_pmu->pmu = (struct pmu) {
> 633 .task_ctx_nr = perf_invalid_context,
> 634 .pmu_enable = smmu_pmu_enable,
> 635 .pmu_disable = smmu_pmu_disable,
> 636 .event_init = smmu_pmu_event_init,
> 637 .add = smmu_pmu_event_add,
> 638 .del = smmu_pmu_event_del,
> 639 .start = smmu_pmu_event_start,
> 640 .stop = smmu_pmu_event_stop,
> 641 .read = smmu_pmu_event_read,
> 642 .attr_groups = smmu_pmu_attr_grps,
> 643 };
> 644
> 645 res_0 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> 646 smmu_pmu->reg_base = devm_ioremap_resource(dev, res_0);
> 647 if (IS_ERR(smmu_pmu->reg_base))
> 648 return PTR_ERR(smmu_pmu->reg_base);
> 649
> 650 cfgr = readl_relaxed(smmu_pmu->reg_base +
> SMMU_PMCG_CFGR);
> 651
> 652 /* Determine if page 1 is present */
> 653 if (cfgr & SMMU_PMCG_CFGR_RELOC_CTRS) {
> 654 res_1 = platform_get_resource(pdev,
> IORESOURCE_MEM, 1);
> 655 smmu_pmu->reloc_base =
> devm_ioremap_resource(dev, res_1);
> 656 if (IS_ERR(smmu_pmu->reloc_base))
> 657 return PTR_ERR(smmu_pmu->reloc_base);
> 658 } else {
> 659 smmu_pmu->reloc_base = smmu_pmu->reg_base;
> 660 }
> 661
> 662 irq = platform_get_irq(pdev, 0);
> 663 if (irq > 0)
> 664 smmu_pmu->irq = irq;
> 665
> > 666 ceid_64[0] = readq_relaxed(smmu_pmu->reg_base +
> SMMU_PMCG_CEID0);
> 667 ceid_64[1] = readq_relaxed(smmu_pmu->reg_base +
> SMMU_PMCG_CEID1);
> 668 bitmap_from_arr32(smmu_pmu->supported_events, (u32
> *)ceid_64,
> 669 SMMU_ARCH_MAX_EVENTS);
> 670
> 671 smmu_pmu->num_counters =
> FIELD_GET(SMMU_PMCG_CFGR_NCTR_MASK, cfgr) + 1;
> 672 smmu_pmu->counter_present_mask = GENMASK(smmu_pmu-
> >num_counters - 1, 0);
> 673
> 674 smmu_pmu->sid_filter_global = !!(cfgr &
> SMMU_PMCG_CFGR_SID_FILTER_TYPE);
> 675
> 676 reg_size = FIELD_GET(SMMU_PMCG_CFGR_SIZE_MASK, cfgr);
> 677 smmu_pmu->counter_mask = GENMASK_ULL(reg_size, 0);
> 678
> 679 smmu_pmu_reset(smmu_pmu);
> 680
> 681 err = smmu_pmu_setup_irq(smmu_pmu);
> 682 if (err) {
> 683 dev_err(dev, "Setup irq failed, PMU @%pa\n", &res_0-
> >start);
> 684 return err;
> 685 }
> 686
> 687 name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> "smmuv3_pmcg_%llx",
> 688 (res_0->start) >> SMMU_PA_SHIFT);
> 689 if (!name) {
> 690 dev_err(dev, "Create name failed, PMU @%pa\n",
> &res_0->start);
> 691 return -EINVAL;
> 692 }
> 693
> 694 /* Pick one CPU to be the preferred one to use */
> 695 smmu_pmu->on_cpu = get_cpu();
> 696 WARN_ON(irq_set_affinity(smmu_pmu->irq,
> cpumask_of(smmu_pmu->on_cpu)));
> 697
> 698 err = cpuhp_state_add_instance_nocalls(cpuhp_state_num,
> 699 &smmu_pmu->node);
> 700 if (err) {
> 701 dev_err(dev, "Error %d registering hotplug, PMU
> @%pa\n",
> 702 err, &res_0->start);
> 703 goto out_cpuhp_err;
> 704 }
> 705
> 706 err = perf_pmu_register(&smmu_pmu->pmu, name, -1);
> 707 if (err) {
> 708 dev_err(dev, "Error %d registering PMU @%pa\n",
> 709 err, &res_0->start);
> 710 goto out_unregister;
> 711 }
> 712
> 713 put_cpu();
> 714 dev_info(dev, "Registered PMU @ %pa using %d counters with
> %s filter settings\n",
> 715 &res_0->start, smmu_pmu->num_counters,
> 716 smmu_pmu->sid_filter_global ? "Global(Counter0)" :
> 717 "Individual");
> 718
> 719 return 0;
> 720
> 721 out_unregister:
> 722 cpuhp_state_remove_instance_nocalls(cpuhp_state_num,
> &smmu_pmu->node);
> 723 out_cpuhp_err:
> 724 put_cpu();
> 725 return err;
> 726 }
> 727
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum 162001800 quirk
2018-10-16 12:49 ` [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum 162001800 quirk Shameer Kolothum
@ 2018-10-18 11:43 ` Robin Murphy
2018-10-18 13:34 ` Shameerali Kolothum Thodi
0 siblings, 1 reply; 17+ messages in thread
From: Robin Murphy @ 2018-10-18 11:43 UTC (permalink / raw)
To: Shameer Kolothum, lorenzo.pieralisi, jean-philippe.brucker
Cc: will.deacon, mark.rutland, guohanjun, john.garry, pabba, vkilari,
rruigrok, linux-acpi, linux-kernel, linux-arm-kernel, linuxarm,
neil.m.leeder
On 16/10/18 13:49, Shameer Kolothum wrote:
> HiSilicon erratum 162001800 describes the limitation of
> SMMUv3 PMCG implementation on HiSilicon Hip08 platforms.
>
> On these platforms, the PMCG event counter registers
> (SMMU_PMCG_EVCNTRn) are read only and as a result it is
> not possible to set the initial counter period value on
> event monitor start.
How the... oh well, never mind :(
> To work around this, the current value of the counter is
> read and is used for delta calculations. This increases
> the possibility of reporting incorrect values if counter
> overflow happens and counter passes the initial value.
>
> OEM information from ACPI header is used to identify the
> affected hardware platform.
I'm guessing they don't implement anything useful for SMMU_PMCG_ID_REGS?
(notwithstanding the known chicken-and-egg problem with how to interpret
those)
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
> drivers/perf/arm_smmuv3_pmu.c | 137 +++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 130 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
> index d927ef8..519545e 100644
> --- a/drivers/perf/arm_smmuv3_pmu.c
> +++ b/drivers/perf/arm_smmuv3_pmu.c
> @@ -96,6 +96,8 @@
>
> #define SMMU_PA_SHIFT 12
>
> +#define SMMU_PMU_OPT_EVCNTR_RDONLY (1 << 0)
> +
> static int cpuhp_state_num;
>
> struct smmu_pmu {
> @@ -111,10 +113,55 @@ struct smmu_pmu {
> struct device *dev;
> void __iomem *reg_base;
> void __iomem *reloc_base;
> + u32 options;
> u64 counter_present_mask;
> u64 counter_mask;
> };
>
> +struct erratum_acpi_oem_info {
> + char oem_id[ACPI_OEM_ID_SIZE + 1];
> + char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> + u32 oem_revision;
> +};
> +
> +static struct erratum_acpi_oem_info hisi_162001800_oem_info[] = {
> + /*
> + * Note that trailing spaces are required to properly match
> + * the OEM table information.
> + */
> + {
> + .oem_id = "HISI ",
> + .oem_table_id = "HIP08 ",
> + .oem_revision = 0,
> + },
> + { /* Sentinel indicating the end of the OEM array */ },
> +};
> +
> +enum smmu_pmu_erratum_match_type {
> + se_match_acpi_oem,
> +};
> +
> +void hisi_erratum_evcntr_rdonly(struct smmu_pmu *smmu_pmu)
> +{
> + smmu_pmu->options |= SMMU_PMU_OPT_EVCNTR_RDONLY;
> +}
> +
> +struct smmu_pmu_erratum_wa {
> + enum smmu_pmu_erratum_match_type match_type;
> + const void *id; /* Indicate the Erratum ID */
> + const char *desc_str;
> + void (*enable)(struct smmu_pmu *smmu_pmu);
> +};
> +
> +static const struct smmu_pmu_erratum_wa smmu_pmu_wa[] = {
> + {
> + .match_type = se_match_acpi_oem,
> + .id = hisi_162001800_oem_info,
> + .desc_str = "HiSilicon erratum 162001800",
> + .enable = hisi_erratum_evcntr_rdonly,
> + },
> +};
> +
There's an awful lot of raw ACPI internals splashed about here -
couldn't at least some of it be abstracted behind the IORT code? In
fact, can't IORT just set all this stuff up in advance like it does for
SMMUs?
> #define to_smmu_pmu(p) (container_of(p, struct smmu_pmu, pmu))
>
> #define SMMU_PMU_EVENT_ATTR_EXTRACTOR(_name, _config, _start, _end) \
> @@ -224,15 +271,20 @@ static void smmu_pmu_set_period(struct smmu_pmu *smmu_pmu,
> u32 idx = hwc->idx;
> u64 new;
>
> - /*
> - * We limit the max period to half the max counter value of the counter
> - * size, so that even in the case of extreme interrupt latency the
> - * counter will (hopefully) not wrap past its initial value.
> - */
> - new = smmu_pmu->counter_mask >> 1;
> + if (smmu_pmu->options & SMMU_PMU_OPT_EVCNTR_RDONLY) {
> + new = smmu_pmu_counter_get_value(smmu_pmu, idx);
Something's clearly missing, because if this happens to start at 0, the
current overflow handling code cannot possibly give the correct count.
Much as I hate the reset-to-half-period idiom for being impossible to
make sense of, it does make various aspects appear a lot simpler than
they really are. Wait, maybe that's yet another reason to hate it...
> + } else {
> + /*
> + * We limit the max period to half the max counter value
> + * of the counter size, so that even in the case of extreme
> + * interrupt latency the counter will (hopefully) not wrap
> + * past its initial value.
> + */
> + new = smmu_pmu->counter_mask >> 1;
> + smmu_pmu_counter_set_value(smmu_pmu, idx, new);
> + }
>
> local64_set(&hwc->prev_count, new);
> - smmu_pmu_counter_set_value(smmu_pmu, idx, new);
> }
>
> static void smmu_pmu_get_event_filter(struct perf_event *event, u32 *span,
> @@ -670,6 +722,69 @@ static void smmu_pmu_reset(struct smmu_pmu *smmu_pmu)
> smmu_pmu->reloc_base + SMMU_PMCG_OVSCLR0);
> }
>
> +typedef bool (*se_match_fn_t)(const struct smmu_pmu_erratum_wa *,
> + const void *);
> +
> +bool smmu_pmu_check_acpi_erratum(const struct smmu_pmu_erratum_wa *wa,
> + const void *arg)
> +{
> + static const struct erratum_acpi_oem_info empty_oem_info = {};
> + const struct erratum_acpi_oem_info *info = wa->id;
> + const struct acpi_table_header *hdr = arg;
> +
> + /* Iterate over the ACPI OEM info array, looking for a match */
> + while (memcmp(info, &empty_oem_info, sizeof(*info))) {
> + if (!memcmp(info->oem_id, hdr->oem_id, ACPI_OEM_ID_SIZE) &&
> + !memcmp(info->oem_table_id, hdr->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) &&
> + info->oem_revision == hdr->oem_revision)
> + return true;
> +
> + info++;
> + }
> +
> + return false;
> +
> +}
> +
> +static void smmu_pmu_enable_errata(struct smmu_pmu *smmu_pmu,
> + enum smmu_pmu_erratum_match_type type,
> + se_match_fn_t match_fn,
> + void *arg)
> +{
> + const struct smmu_pmu_erratum_wa *wa = smmu_pmu_wa;
> +
> + for (; wa->desc_str; wa++) {
> + if (wa->match_type != type)
> + continue;
> +
> + if (match_fn(wa, arg)) {
> + if (wa->enable) {
> + wa->enable(smmu_pmu);
> + dev_info(smmu_pmu->dev,
> + "Enabling workaround for %s\n",
> + wa->desc_str);
> + }
Just how many kinds of broken are we expecting here? Is this lifted from
the arm64 cpufeature framework, because it seems like absolute overkill
for a simple PMU driver which in all reality is only ever going to
wiggle a few flags in some data structure.
Robin.
> + }
> + }
> +}
> +
> +static void smmu_pmu_check_workarounds(struct smmu_pmu *smmu_pmu,
> + enum smmu_pmu_erratum_match_type type,
> + void *arg)
> +{
> + se_match_fn_t match_fn = NULL;
> +
> + switch (type) {
> + case se_match_acpi_oem:
> + match_fn = smmu_pmu_check_acpi_erratum;
> + break;
> + default:
> + return;
> + }
> +
> + smmu_pmu_enable_errata(smmu_pmu, type, match_fn, arg);
> +}
> +
> static int smmu_pmu_probe(struct platform_device *pdev)
> {
> struct smmu_pmu *smmu_pmu;
> @@ -678,6 +793,7 @@ static int smmu_pmu_probe(struct platform_device *pdev)
> u64 ceid_64[2];
> int irq, err;
> char *name;
> + struct acpi_table_header *table;
> struct device *dev = &pdev->dev;
>
> smmu_pmu = devm_kzalloc(dev, sizeof(*smmu_pmu), GFP_KERNEL);
> @@ -749,6 +865,13 @@ static int smmu_pmu_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> + if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_IORT, 0, &table))) {
> + dev_err(dev, "IORT get failed, PMU @%pa\n", &res_0->start);
> + return -EINVAL;
> + }
> +
> + smmu_pmu_check_workarounds(smmu_pmu, se_match_acpi_oem, table);
> +
> /* Pick one CPU to be the preferred one to use */
> smmu_pmu->on_cpu = get_cpu();
> WARN_ON(irq_set_affinity(smmu_pmu->irq, cpumask_of(smmu_pmu->on_cpu)));
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum 162001800 quirk
2018-10-18 11:43 ` Robin Murphy
@ 2018-10-18 13:34 ` Shameerali Kolothum Thodi
2018-10-18 15:27 ` Shameerali Kolothum Thodi
0 siblings, 1 reply; 17+ messages in thread
From: Shameerali Kolothum Thodi @ 2018-10-18 13:34 UTC (permalink / raw)
To: Robin Murphy, lorenzo.pieralisi, jean-philippe.brucker
Cc: will.deacon, mark.rutland, Guohanjun (Hanjun Guo),
John Garry, pabba, vkilari, rruigrok, linux-acpi, linux-kernel,
linux-arm-kernel, Linuxarm, neil.m.leeder
Hi Robin,
> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@arm.com]
> Sent: 18 October 2018 12:44
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> lorenzo.pieralisi@arm.com; jean-philippe.brucker@arm.com
> Cc: will.deacon@arm.com; mark.rutland@arm.com; Guohanjun (Hanjun Guo)
> <guohanjun@huawei.com>; John Garry <john.garry@huawei.com>;
> pabba@codeaurora.org; vkilari@codeaurora.org; rruigrok@codeaurora.org;
> linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; Linuxarm <linuxarm@huawei.com>;
> neil.m.leeder@gmail.com
> Subject: Re: [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum
> 162001800 quirk
>
> On 16/10/18 13:49, Shameer Kolothum wrote:
> > HiSilicon erratum 162001800 describes the limitation of
> > SMMUv3 PMCG implementation on HiSilicon Hip08 platforms.
> >
> > On these platforms, the PMCG event counter registers
> > (SMMU_PMCG_EVCNTRn) are read only and as a result it is
> > not possible to set the initial counter period value on
> > event monitor start.
>
> How the... oh well, never mind :(
>
> > To work around this, the current value of the counter is
> > read and is used for delta calculations. This increases
> > the possibility of reporting incorrect values if counter
> > overflow happens and counter passes the initial value.
> >
> > OEM information from ACPI header is used to identify the
> > affected hardware platform.
>
> I'm guessing they don't implement anything useful for
> SMMU_PMCG_ID_REGS?
> (notwithstanding the known chicken-and-egg problem with how to interpret
> those)
Your guess is right :(
> > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> > ---
> > drivers/perf/arm_smmuv3_pmu.c | 137
> +++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 130 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/perf/arm_smmuv3_pmu.c
> b/drivers/perf/arm_smmuv3_pmu.c
> > index d927ef8..519545e 100644
> > --- a/drivers/perf/arm_smmuv3_pmu.c
> > +++ b/drivers/perf/arm_smmuv3_pmu.c
> > @@ -96,6 +96,8 @@
> >
> > #define SMMU_PA_SHIFT 12
> >
> > +#define SMMU_PMU_OPT_EVCNTR_RDONLY (1 << 0)
> > +
> > static int cpuhp_state_num;
> >
> > struct smmu_pmu {
> > @@ -111,10 +113,55 @@ struct smmu_pmu {
> > struct device *dev;
> > void __iomem *reg_base;
> > void __iomem *reloc_base;
> > + u32 options;
> > u64 counter_present_mask;
> > u64 counter_mask;
> > };
> >
> > +struct erratum_acpi_oem_info {
> > + char oem_id[ACPI_OEM_ID_SIZE + 1];
> > + char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> > + u32 oem_revision;
> > +};
> > +
> > +static struct erratum_acpi_oem_info hisi_162001800_oem_info[] = {
> > + /*
> > + * Note that trailing spaces are required to properly match
> > + * the OEM table information.
> > + */
> > + {
> > + .oem_id = "HISI ",
> > + .oem_table_id = "HIP08 ",
> > + .oem_revision = 0,
> > + },
> > + { /* Sentinel indicating the end of the OEM array */ },
> > +};
> > +
> > +enum smmu_pmu_erratum_match_type {
> > + se_match_acpi_oem,
> > +};
> > +
> > +void hisi_erratum_evcntr_rdonly(struct smmu_pmu *smmu_pmu)
> > +{
> > + smmu_pmu->options |= SMMU_PMU_OPT_EVCNTR_RDONLY;
> > +}
> > +
> > +struct smmu_pmu_erratum_wa {
> > + enum smmu_pmu_erratum_match_type match_type;
> > + const void *id; /* Indicate the Erratum ID */
> > + const char *desc_str;
> > + void (*enable)(struct smmu_pmu *smmu_pmu);
> > +};
> > +
> > +static const struct smmu_pmu_erratum_wa smmu_pmu_wa[] = {
> > + {
> > + .match_type = se_match_acpi_oem,
> > + .id = hisi_162001800_oem_info,
> > + .desc_str = "HiSilicon erratum 162001800",
> > + .enable = hisi_erratum_evcntr_rdonly,
> > + },
> > +};
> > +
>
> There's an awful lot of raw ACPI internals splashed about here -
> couldn't at least some of it be abstracted behind the IORT code? In
> fact, can't IORT just set all this stuff up in advance like it does for
> SMMUs?
Hmmm.. Sorry, not clear to me. You mean to say associate the IORT node
with platform device and retrieve it in driver just like smmu does for
"model" checks? Not sure that works here if that’s what the above meant.
> > #define to_smmu_pmu(p) (container_of(p, struct smmu_pmu, pmu))
> >
> > #define SMMU_PMU_EVENT_ATTR_EXTRACTOR(_name, _config, _start,
> _end) \
> > @@ -224,15 +271,20 @@ static void smmu_pmu_set_period(struct
> smmu_pmu *smmu_pmu,
> > u32 idx = hwc->idx;
> > u64 new;
> >
> > - /*
> > - * We limit the max period to half the max counter value of the
> counter
> > - * size, so that even in the case of extreme interrupt latency the
> > - * counter will (hopefully) not wrap past its initial value.
> > - */
> > - new = smmu_pmu->counter_mask >> 1;
> > + if (smmu_pmu->options & SMMU_PMU_OPT_EVCNTR_RDONLY) {
> > + new = smmu_pmu_counter_get_value(smmu_pmu, idx);
>
> Something's clearly missing, because if this happens to start at 0, the
> current overflow handling code cannot possibly give the correct count.
> Much as I hate the reset-to-half-period idiom for being impossible to
> make sense of, it does make various aspects appear a lot simpler than
> they really are. Wait, maybe that's yet another reason to hate it...
Yes, if the counter starts at 0 and overflow happens, it won't possibly give
the correct count compared to the reset-to-half-period logic. Since this is a
64 bit counter, just hope that, it won't necessarily happen that often.
> > + } else {
> > + /*
> > + * We limit the max period to half the max counter value
> > + * of the counter size, so that even in the case of extreme
> > + * interrupt latency the counter will (hopefully) not wrap
> > + * past its initial value.
> > + */
> > + new = smmu_pmu->counter_mask >> 1;
> > + smmu_pmu_counter_set_value(smmu_pmu, idx, new);
> > + }
> >
> > local64_set(&hwc->prev_count, new);
> > - smmu_pmu_counter_set_value(smmu_pmu, idx, new);
> > }
> >
> > static void smmu_pmu_get_event_filter(struct perf_event *event, u32
> *span,
> > @@ -670,6 +722,69 @@ static void smmu_pmu_reset(struct smmu_pmu
> *smmu_pmu)
> > smmu_pmu->reloc_base + SMMU_PMCG_OVSCLR0);
> > }
> >
> > +typedef bool (*se_match_fn_t)(const struct smmu_pmu_erratum_wa *,
> > + const void *);
> > +
> > +bool smmu_pmu_check_acpi_erratum(const struct
> smmu_pmu_erratum_wa *wa,
> > + const void *arg)
> > +{
> > + static const struct erratum_acpi_oem_info empty_oem_info = {};
> > + const struct erratum_acpi_oem_info *info = wa->id;
> > + const struct acpi_table_header *hdr = arg;
> > +
> > + /* Iterate over the ACPI OEM info array, looking for a match */
> > + while (memcmp(info, &empty_oem_info, sizeof(*info))) {
> > + if (!memcmp(info->oem_id, hdr->oem_id, ACPI_OEM_ID_SIZE)
> &&
> > + !memcmp(info->oem_table_id, hdr->oem_table_id,
> ACPI_OEM_TABLE_ID_SIZE) &&
> > + info->oem_revision == hdr->oem_revision)
> > + return true;
> > +
> > + info++;
> > + }
> > +
> > + return false;
> > +
> > +}
> > +
> > +static void smmu_pmu_enable_errata(struct smmu_pmu *smmu_pmu,
> > + enum smmu_pmu_erratum_match_type type,
> > + se_match_fn_t match_fn,
> > + void *arg)
> > +{
> > + const struct smmu_pmu_erratum_wa *wa = smmu_pmu_wa;
> > +
> > + for (; wa->desc_str; wa++) {
> > + if (wa->match_type != type)
> > + continue;
> > +
> > + if (match_fn(wa, arg)) {
> > + if (wa->enable) {
> > + wa->enable(smmu_pmu);
> > + dev_info(smmu_pmu->dev,
> > + "Enabling workaround for %s\n",
> > + wa->desc_str);
> > + }
>
> Just how many kinds of broken are we expecting here? Is this lifted from
> the arm64 cpufeature framework, because it seems like absolute overkill
> for a simple PMU driver which in all reality is only ever going to
> wiggle a few flags in some data structure.
Yes, this erratum framework is based on the arm_arch_timer code. Agree that
this is an overkill if it is just to support this hardware. I am not sure this can be
extended to add the IMPLEMENTATION DEFINED events in future(I haven't
looked into that now). If this is not that useful in the near future, I will remove the
framework part and use the OEM info directly to set the flag. Please let me know
your thoughts..
Thanks,
Shameer
> Robin.
>
> > + }
> > + }
> > +}
> > +
> > +static void smmu_pmu_check_workarounds(struct smmu_pmu
> *smmu_pmu,
> > + enum smmu_pmu_erratum_match_type
> type,
> > + void *arg)
> > +{
> > + se_match_fn_t match_fn = NULL;
> > +
> > + switch (type) {
> > + case se_match_acpi_oem:
> > + match_fn = smmu_pmu_check_acpi_erratum;
> > + break;
> > + default:
> > + return;
> > + }
> > +
> > + smmu_pmu_enable_errata(smmu_pmu, type, match_fn, arg);
> > +}
> > +
> > static int smmu_pmu_probe(struct platform_device *pdev)
> > {
> > struct smmu_pmu *smmu_pmu;
> > @@ -678,6 +793,7 @@ static int smmu_pmu_probe(struct platform_device
> *pdev)
> > u64 ceid_64[2];
> > int irq, err;
> > char *name;
> > + struct acpi_table_header *table;
> > struct device *dev = &pdev->dev;
> >
> > smmu_pmu = devm_kzalloc(dev, sizeof(*smmu_pmu), GFP_KERNEL);
> > @@ -749,6 +865,13 @@ static int smmu_pmu_probe(struct platform_device
> *pdev)
> > return -EINVAL;
> > }
> >
> > + if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_IORT, 0, &table))) {
> > + dev_err(dev, "IORT get failed, PMU @%pa\n", &res_0->start);
> > + return -EINVAL;
> > + }
> > +
> > + smmu_pmu_check_workarounds(smmu_pmu, se_match_acpi_oem,
> table);
> > +
> > /* Pick one CPU to be the preferred one to use */
> > smmu_pmu->on_cpu = get_cpu();
> > WARN_ON(irq_set_affinity(smmu_pmu->irq, cpumask_of(smmu_pmu-
> >on_cpu)));
> >
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum 162001800 quirk
2018-10-18 13:34 ` Shameerali Kolothum Thodi
@ 2018-10-18 15:27 ` Shameerali Kolothum Thodi
2018-11-09 16:50 ` Shameerali Kolothum Thodi
2018-11-26 18:45 ` Robin Murphy
0 siblings, 2 replies; 17+ messages in thread
From: Shameerali Kolothum Thodi @ 2018-10-18 15:27 UTC (permalink / raw)
To: Robin Murphy, lorenzo.pieralisi, jean-philippe.brucker
Cc: mark.rutland, vkilari, neil.m.leeder, pabba, will.deacon,
rruigrok, Linuxarm, linux-kernel, linux-acpi, linux-arm-kernel
> -----Original Message-----
> From: Linuxarm [mailto:linuxarm-bounces@huawei.com] On Behalf Of
> Shameerali Kolothum Thodi
> Sent: 18 October 2018 14:34
> To: Robin Murphy <robin.murphy@arm.com>; lorenzo.pieralisi@arm.com;
> jean-philippe.brucker@arm.com
> Cc: mark.rutland@arm.com; vkilari@codeaurora.org;
> neil.m.leeder@gmail.com; pabba@codeaurora.org; will.deacon@arm.com;
> rruigrok@codeaurora.org; Linuxarm <linuxarm@huawei.com>; linux-
> kernel@vger.kernel.org; linux-acpi@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Subject: RE: [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum
> 162001800 quirk
>
> Hi Robin,
>
> > -----Original Message-----
> > From: Robin Murphy [mailto:robin.murphy@arm.com]
> > Sent: 18 October 2018 12:44
> > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> > lorenzo.pieralisi@arm.com; jean-philippe.brucker@arm.com
> > Cc: will.deacon@arm.com; mark.rutland@arm.com; Guohanjun (Hanjun Guo)
> > <guohanjun@huawei.com>; John Garry <john.garry@huawei.com>;
> > pabba@codeaurora.org; vkilari@codeaurora.org; rruigrok@codeaurora.org;
> > linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; Linuxarm <linuxarm@huawei.com>;
> > neil.m.leeder@gmail.com
> > Subject: Re: [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum
> > 162001800 quirk
[...]
> > > +static const struct smmu_pmu_erratum_wa smmu_pmu_wa[] = {
> > > + {
> > > + .match_type = se_match_acpi_oem,
> > > + .id = hisi_162001800_oem_info,
> > > + .desc_str = "HiSilicon erratum 162001800",
> > > + .enable = hisi_erratum_evcntr_rdonly,
> > > + },
> > > +};
> > > +
> >
> > There's an awful lot of raw ACPI internals splashed about here -
> > couldn't at least some of it be abstracted behind the IORT code? In
> > fact, can't IORT just set all this stuff up in advance like it does for
> > SMMUs?
>
> Hmmm.. Sorry, not clear to me. You mean to say associate the IORT node
> with platform device and retrieve it in driver just like smmu does for
> "model" checks? Not sure that works here if that’s what the above meant.
>
> > > #define to_smmu_pmu(p) (container_of(p, struct smmu_pmu, pmu))
> > >
> > > #define SMMU_PMU_EVENT_ATTR_EXTRACTOR(_name, _config, _start,
> > _end) \
> > > @@ -224,15 +271,20 @@ static void smmu_pmu_set_period(struct
> > smmu_pmu *smmu_pmu,
> > > u32 idx = hwc->idx;
> > > u64 new;
> > >
> > > - /*
> > > - * We limit the max period to half the max counter value of the
> > counter
> > > - * size, so that even in the case of extreme interrupt latency the
> > > - * counter will (hopefully) not wrap past its initial value.
> > > - */
> > > - new = smmu_pmu->counter_mask >> 1;
> > > + if (smmu_pmu->options & SMMU_PMU_OPT_EVCNTR_RDONLY) {
> > > + new = smmu_pmu_counter_get_value(smmu_pmu, idx);
> >
> > Something's clearly missing, because if this happens to start at 0, the
> > current overflow handling code cannot possibly give the correct count.
> > Much as I hate the reset-to-half-period idiom for being impossible to
> > make sense of, it does make various aspects appear a lot simpler than
> > they really are. Wait, maybe that's yet another reason to hate it...
>
> Yes, if the counter starts at 0 and overflow happens, it won't possibly give
> the correct count compared to the reset-to-half-period logic. Since this is a
> 64 bit counter, just hope that, it won't necessarily happen that often.
[...]
> > > +static void smmu_pmu_enable_errata(struct smmu_pmu *smmu_pmu,
> > > + enum smmu_pmu_erratum_match_type type,
> > > + se_match_fn_t match_fn,
> > > + void *arg)
> > > +{
> > > + const struct smmu_pmu_erratum_wa *wa = smmu_pmu_wa;
> > > +
> > > + for (; wa->desc_str; wa++) {
> > > + if (wa->match_type != type)
> > > + continue;
> > > +
> > > + if (match_fn(wa, arg)) {
> > > + if (wa->enable) {
> > > + wa->enable(smmu_pmu);
> > > + dev_info(smmu_pmu->dev,
> > > + "Enabling workaround for %s\n",
> > > + wa->desc_str);
> > > + }
> >
> > Just how many kinds of broken are we expecting here? Is this lifted from
> > the arm64 cpufeature framework, because it seems like absolute overkill
> > for a simple PMU driver which in all reality is only ever going to
> > wiggle a few flags in some data structure.
>
> Yes, this erratum framework is based on the arm_arch_timer code. Agree that
> this is an overkill if it is just to support this hardware. I am not sure this can be
> extended to add the IMPLEMENTATION DEFINED events in future(I haven't
> looked into that now). If this is not that useful in the near future, I will remove
> the
> framework part and use the OEM info directly to set the flag. Please let me
> know
> your thoughts..
Below is another take on this patch. Please let me know if this makes any sense..
Thanks,
Shameer
----8----
diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
index ef94b90..6f81b94 100644
--- a/drivers/perf/arm_smmuv3_pmu.c
+++ b/drivers/perf/arm_smmuv3_pmu.c
@@ -96,6 +96,8 @@
#define SMMU_PA_SHIFT 12
+#define SMMU_PMU_OPT_EVCNTR_RDONLY (1 << 0)
+
static int cpuhp_state_num;
struct smmu_pmu {
@@ -111,10 +113,38 @@ struct smmu_pmu {
struct device *dev;
void __iomem *reg_base;
void __iomem *reloc_base;
+ u32 options;
u64 counter_present_mask;
u64 counter_mask;
};
+struct erratum_acpi_oem_info {
+ char oem_id[ACPI_OEM_ID_SIZE + 1];
+ char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
+ u32 oem_revision;
+ void (*enable)(struct smmu_pmu *smmu_pmu);
+};
+
+void hisi_erratum_evcntr_rdonly(struct smmu_pmu *smmu_pmu)
+{
+ smmu_pmu->options |= SMMU_PMU_OPT_EVCNTR_RDONLY;
+ dev_info(smmu_pmu->dev, "Enabling HiSilicon erratum 162001800\n");
+}
+
+static struct erratum_acpi_oem_info acpi_oem_info[] = {
+ /*
+ * Note that trailing spaces are required to properly match
+ * the OEM table information.
+ */
+ {
+ .oem_id = "HISI ",
+ .oem_table_id = "HIP08 ",
+ .oem_revision = 0,
+ .enable = hisi_erratum_evcntr_rdonly,
+ },
+ { /* Sentinel indicating the end of the OEM array */ },
+};
+
#define to_smmu_pmu(p) (container_of(p, struct smmu_pmu, pmu))
#define SMMU_PMU_EVENT_ATTR_EXTRACTOR(_name, _config, _start, _end) \
@@ -224,15 +254,20 @@ static void smmu_pmu_set_period(struct smmu_pmu *smmu_pmu,
u32 idx = hwc->idx;
u64 new;
- /*
- * We limit the max period to half the max counter value of the counter
- * size, so that even in the case of extreme interrupt latency the
- * counter will (hopefully) not wrap past its initial value.
- */
- new = smmu_pmu->counter_mask >> 1;
+ if (smmu_pmu->options & SMMU_PMU_OPT_EVCNTR_RDONLY) {
+ new = smmu_pmu_counter_get_value(smmu_pmu, idx);
+ } else {
+ /*
+ * We limit the max period to half the max counter value
+ * of the counter size, so that even in the case of extreme
+ * interrupt latency the counter will (hopefully) not wrap
+ * past its initial value.
+ */
+ new = smmu_pmu->counter_mask >> 1;
+ smmu_pmu_counter_set_value(smmu_pmu, idx, new);
+ }
local64_set(&hwc->prev_count, new);
- smmu_pmu_counter_set_value(smmu_pmu, idx, new);
}
static void smmu_pmu_get_event_filter(struct perf_event *event, u32 *span,
@@ -670,6 +705,28 @@ static void smmu_pmu_reset(struct smmu_pmu *smmu_pmu)
smmu_pmu->reloc_base + SMMU_PMCG_OVSCLR0);
}
+static void smmu_pmu_check_acpi_workarounds(struct smmu_pmu *smmu_pmu)
+{
+ static const struct erratum_acpi_oem_info empty_oem_info = {};
+ const struct erratum_acpi_oem_info *info = acpi_oem_info;
+ struct acpi_table_header *hdr;
+
+ if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_IORT, 0, &hdr))) {
+ dev_err(smmu_pmu->dev, "failed to get IORT\n");
+ return;
+ }
+
+ /* Iterate over the ACPI OEM info array, looking for a match */
+ while (memcmp(info, &empty_oem_info, sizeof(*info))) {
+ if (!memcmp(info->oem_id, hdr->oem_id, ACPI_OEM_ID_SIZE) &&
+ !memcmp(info->oem_table_id, hdr->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) &&
+ info->oem_revision == hdr->oem_revision)
+ info->enable(smmu_pmu);
+
+ info++;
+ }
+}
+
static int smmu_pmu_probe(struct platform_device *pdev)
{
struct smmu_pmu *smmu_pmu;
@@ -749,6 +806,8 @@ static int smmu_pmu_probe(struct platform_device *pdev)
return -EINVAL;
}
+ smmu_pmu_check_acpi_workarounds(smmu_pmu);
+
/* Pick one CPU to be the preferred one to use */
smmu_pmu->on_cpu = get_cpu();
WARN_ON(irq_set_affinity(smmu_pmu->irq, cpumask_of(smmu_pmu->on_cpu)));
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/4] perf: add arm64 smmuv3 pmu driver
2018-10-16 12:49 ` [PATCH v4 2/4] perf: add arm64 smmuv3 pmu driver Shameer Kolothum
2018-10-17 21:53 ` kbuild test robot
@ 2018-10-20 4:50 ` kbuild test robot
1 sibling, 0 replies; 17+ messages in thread
From: kbuild test robot @ 2018-10-20 4:50 UTC (permalink / raw)
To: Shameer Kolothum
Cc: kbuild-all, lorenzo.pieralisi, robin.murphy,
jean-philippe.brucker, will.deacon, mark.rutland, guohanjun,
john.garry, pabba, vkilari, rruigrok, linux-acpi, linux-kernel,
linux-arm-kernel, linuxarm, neil.m.leeder
[-- Attachment #1: Type: text/plain, Size: 7751 bytes --]
Hi Neil,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linux-sof-driver/master]
[also build test ERROR on v4.19-rc8 next-20181019]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Shameer-Kolothum/arm64-SMMUv3-PMU-driver-with-IORT-support/20181017-063949
base: https://github.com/thesofproject/linux master
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_counter_set_value':
>> drivers//perf/arm_smmuv3_pmu.c:145:36: warning: left shift count >= width of type [-Wshift-count-overflow]
if (smmu_pmu->counter_mask & BIT(32))
^~
drivers//perf/arm_smmuv3_pmu.c:146:3: error: implicit declaration of function 'writeq'; did you mean 'writel'? [-Werror=implicit-function-declaration]
writeq(value, smmu_pmu->reloc_base + SMMU_PMCG_EVCNTR(idx, 8));
^~~~~~
writel
drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_counter_get_value':
drivers//perf/arm_smmuv3_pmu.c:155:36: warning: left shift count >= width of type [-Wshift-count-overflow]
if (smmu_pmu->counter_mask & BIT(32))
^~
drivers//perf/arm_smmuv3_pmu.c:156:11: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
value = readq(smmu_pmu->reloc_base + SMMU_PMCG_EVCNTR(idx, 8));
^~~~~
readl
drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_reset':
>> drivers//perf/arm_smmuv3_pmu.c:607:2: error: implicit declaration of function 'writeq_relaxed'; did you mean 'seq_release'? [-Werror=implicit-function-declaration]
writeq_relaxed(smmu_pmu->counter_present_mask,
^~~~~~~~~~~~~~
seq_release
drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_probe':
>> drivers//perf/arm_smmuv3_pmu.c:666:15: error: implicit declaration of function 'readq_relaxed'; did you mean 'seq_release'? [-Werror=implicit-function-declaration]
ceid_64[0] = readq_relaxed(smmu_pmu->reg_base + SMMU_PMCG_CEID0);
^~~~~~~~~~~~~
seq_release
drivers//perf/arm_smmuv3_pmu.c:687:108: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'resource_size_t {aka unsigned int}' [-Wformat=]
name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "smmuv3_pmcg_%llx",
^
(res_0->start) >> SMMU_PA_SHIFT);
~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +607 drivers//perf/arm_smmuv3_pmu.c
601
602 static void smmu_pmu_reset(struct smmu_pmu *smmu_pmu)
603 {
604 smmu_pmu_disable(&smmu_pmu->pmu);
605
606 /* Disable counter and interrupt */
> 607 writeq_relaxed(smmu_pmu->counter_present_mask,
608 smmu_pmu->reg_base + SMMU_PMCG_CNTENCLR0);
609 writeq_relaxed(smmu_pmu->counter_present_mask,
610 smmu_pmu->reg_base + SMMU_PMCG_INTENCLR0);
611 writeq_relaxed(smmu_pmu->counter_present_mask,
612 smmu_pmu->reloc_base + SMMU_PMCG_OVSCLR0);
613 }
614
615 static int smmu_pmu_probe(struct platform_device *pdev)
616 {
617 struct smmu_pmu *smmu_pmu;
618 struct resource *res_0, *res_1;
619 u32 cfgr, reg_size;
620 u64 ceid_64[2];
621 int irq, err;
622 char *name;
623 struct device *dev = &pdev->dev;
624
625 smmu_pmu = devm_kzalloc(dev, sizeof(*smmu_pmu), GFP_KERNEL);
626 if (!smmu_pmu)
627 return -ENOMEM;
628
629 smmu_pmu->dev = dev;
630 platform_set_drvdata(pdev, smmu_pmu);
631
632 smmu_pmu->pmu = (struct pmu) {
633 .task_ctx_nr = perf_invalid_context,
634 .pmu_enable = smmu_pmu_enable,
635 .pmu_disable = smmu_pmu_disable,
636 .event_init = smmu_pmu_event_init,
637 .add = smmu_pmu_event_add,
638 .del = smmu_pmu_event_del,
639 .start = smmu_pmu_event_start,
640 .stop = smmu_pmu_event_stop,
641 .read = smmu_pmu_event_read,
642 .attr_groups = smmu_pmu_attr_grps,
643 };
644
645 res_0 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
646 smmu_pmu->reg_base = devm_ioremap_resource(dev, res_0);
647 if (IS_ERR(smmu_pmu->reg_base))
648 return PTR_ERR(smmu_pmu->reg_base);
649
650 cfgr = readl_relaxed(smmu_pmu->reg_base + SMMU_PMCG_CFGR);
651
652 /* Determine if page 1 is present */
653 if (cfgr & SMMU_PMCG_CFGR_RELOC_CTRS) {
654 res_1 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
655 smmu_pmu->reloc_base = devm_ioremap_resource(dev, res_1);
656 if (IS_ERR(smmu_pmu->reloc_base))
657 return PTR_ERR(smmu_pmu->reloc_base);
658 } else {
659 smmu_pmu->reloc_base = smmu_pmu->reg_base;
660 }
661
662 irq = platform_get_irq(pdev, 0);
663 if (irq > 0)
664 smmu_pmu->irq = irq;
665
> 666 ceid_64[0] = readq_relaxed(smmu_pmu->reg_base + SMMU_PMCG_CEID0);
667 ceid_64[1] = readq_relaxed(smmu_pmu->reg_base + SMMU_PMCG_CEID1);
668 bitmap_from_arr32(smmu_pmu->supported_events, (u32 *)ceid_64,
669 SMMU_ARCH_MAX_EVENTS);
670
671 smmu_pmu->num_counters = FIELD_GET(SMMU_PMCG_CFGR_NCTR_MASK, cfgr) + 1;
672 smmu_pmu->counter_present_mask = GENMASK(smmu_pmu->num_counters - 1, 0);
673
674 smmu_pmu->sid_filter_global = !!(cfgr & SMMU_PMCG_CFGR_SID_FILTER_TYPE);
675
676 reg_size = FIELD_GET(SMMU_PMCG_CFGR_SIZE_MASK, cfgr);
677 smmu_pmu->counter_mask = GENMASK_ULL(reg_size, 0);
678
679 smmu_pmu_reset(smmu_pmu);
680
681 err = smmu_pmu_setup_irq(smmu_pmu);
682 if (err) {
683 dev_err(dev, "Setup irq failed, PMU @%pa\n", &res_0->start);
684 return err;
685 }
686
687 name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "smmuv3_pmcg_%llx",
688 (res_0->start) >> SMMU_PA_SHIFT);
689 if (!name) {
690 dev_err(dev, "Create name failed, PMU @%pa\n", &res_0->start);
691 return -EINVAL;
692 }
693
694 /* Pick one CPU to be the preferred one to use */
695 smmu_pmu->on_cpu = get_cpu();
696 WARN_ON(irq_set_affinity(smmu_pmu->irq, cpumask_of(smmu_pmu->on_cpu)));
697
698 err = cpuhp_state_add_instance_nocalls(cpuhp_state_num,
699 &smmu_pmu->node);
700 if (err) {
701 dev_err(dev, "Error %d registering hotplug, PMU @%pa\n",
702 err, &res_0->start);
703 goto out_cpuhp_err;
704 }
705
706 err = perf_pmu_register(&smmu_pmu->pmu, name, -1);
707 if (err) {
708 dev_err(dev, "Error %d registering PMU @%pa\n",
709 err, &res_0->start);
710 goto out_unregister;
711 }
712
713 put_cpu();
714 dev_info(dev, "Registered PMU @ %pa using %d counters with %s filter settings\n",
715 &res_0->start, smmu_pmu->num_counters,
716 smmu_pmu->sid_filter_global ? "Global(Counter0)" :
717 "Individual");
718
719 return 0;
720
721 out_unregister:
722 cpuhp_state_remove_instance_nocalls(cpuhp_state_num, &smmu_pmu->node);
723 out_cpuhp_err:
724 put_cpu();
725 return err;
726 }
727
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 62260 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum 162001800 quirk
2018-10-18 15:27 ` Shameerali Kolothum Thodi
@ 2018-11-09 16:50 ` Shameerali Kolothum Thodi
2018-11-26 18:45 ` Robin Murphy
1 sibling, 0 replies; 17+ messages in thread
From: Shameerali Kolothum Thodi @ 2018-11-09 16:50 UTC (permalink / raw)
To: Robin Murphy, lorenzo.pieralisi, jean-philippe.brucker
Cc: mark.rutland, vkilari, neil.m.leeder, pabba, will.deacon,
rruigrok, Linuxarm, linux-acpi, linux-arm-kernel, linux-kernel
Hi Robin,
> -----Original Message-----
> From: Linuxarm [mailto:linuxarm-bounces@huawei.com] On Behalf Of
> Shameerali Kolothum Thodi
> Sent: 18 October 2018 16:27
> To: Robin Murphy <robin.murphy@arm.com>; lorenzo.pieralisi@arm.com;
> jean-philippe.brucker@arm.com
> Cc: mark.rutland@arm.com; vkilari@codeaurora.org;
> neil.m.leeder@gmail.com; pabba@codeaurora.org; will.deacon@arm.com;
> rruigrok@codeaurora.org; Linuxarm <linuxarm@huawei.com>; linux-
> acpi@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: RE: [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum
> 162001800 quirk
[...]
>
> > > > +static const struct smmu_pmu_erratum_wa smmu_pmu_wa[] = {
> > > > + {
> > > > + .match_type = se_match_acpi_oem,
> > > > + .id = hisi_162001800_oem_info,
> > > > + .desc_str = "HiSilicon erratum 162001800",
> > > > + .enable = hisi_erratum_evcntr_rdonly,
> > > > + },
> > > > +};
> > > > +
> > >
> > > There's an awful lot of raw ACPI internals splashed about here -
> > > couldn't at least some of it be abstracted behind the IORT code? In
> > > fact, can't IORT just set all this stuff up in advance like it does for
> > > SMMUs?
> >
> > Hmmm.. Sorry, not clear to me. You mean to say associate the IORT node
> > with platform device and retrieve it in driver just like smmu does for
> > "model" checks? Not sure that works here if that’s what the above meant.
> >
> > > > #define to_smmu_pmu(p) (container_of(p, struct smmu_pmu, pmu))
> > > >
> > > > #define SMMU_PMU_EVENT_ATTR_EXTRACTOR(_name, _config, _start,
> > > _end) \
> > > > @@ -224,15 +271,20 @@ static void smmu_pmu_set_period(struct
> > > smmu_pmu *smmu_pmu,
> > > > u32 idx = hwc->idx;
> > > > u64 new;
> > > >
> > > > - /*
> > > > - * We limit the max period to half the max counter value of the
> > > counter
> > > > - * size, so that even in the case of extreme interrupt latency the
> > > > - * counter will (hopefully) not wrap past its initial value.
> > > > - */
> > > > - new = smmu_pmu->counter_mask >> 1;
> > > > + if (smmu_pmu->options & SMMU_PMU_OPT_EVCNTR_RDONLY) {
> > > > + new = smmu_pmu_counter_get_value(smmu_pmu, idx);
> > >
> > > Something's clearly missing, because if this happens to start at 0, the
> > > current overflow handling code cannot possibly give the correct count.
> > > Much as I hate the reset-to-half-period idiom for being impossible to
> > > make sense of, it does make various aspects appear a lot simpler than
> > > they really are. Wait, maybe that's yet another reason to hate it...
> >
> > Yes, if the counter starts at 0 and overflow happens, it won't possibly give
> > the correct count compared to the reset-to-half-period logic. Since this is a
> > 64 bit counter, just hope that, it won't necessarily happen that often.
>
> [...]
>
> > > > +static void smmu_pmu_enable_errata(struct smmu_pmu *smmu_pmu,
> > > > + enum smmu_pmu_erratum_match_type type,
> > > > + se_match_fn_t match_fn,
> > > > + void *arg)
> > > > +{
> > > > + const struct smmu_pmu_erratum_wa *wa = smmu_pmu_wa;
> > > > +
> > > > + for (; wa->desc_str; wa++) {
> > > > + if (wa->match_type != type)
> > > > + continue;
> > > > +
> > > > + if (match_fn(wa, arg)) {
> > > > + if (wa->enable) {
> > > > + wa->enable(smmu_pmu);
> > > > + dev_info(smmu_pmu->dev,
> > > > + "Enabling workaround for %s\n",
> > > > + wa->desc_str);
> > > > + }
> > >
> > > Just how many kinds of broken are we expecting here? Is this lifted from
> > > the arm64 cpufeature framework, because it seems like absolute overkill
> > > for a simple PMU driver which in all reality is only ever going to
> > > wiggle a few flags in some data structure.
> >
> > Yes, this erratum framework is based on the arm_arch_timer code. Agree
> that
> > this is an overkill if it is just to support this hardware. I am not sure this can
> be
> > extended to add the IMPLEMENTATION DEFINED events in future(I haven't
> > looked into that now). If this is not that useful in the near future, I will remove
> > the
> > framework part and use the OEM info directly to set the flag. Please let me
> > know
> > your thoughts..
>
> Below is another take on this patch. Please let me know if this makes any
> sense..
Could you please let me know whether the below "simplified" version of
erratum looks any better or not? And appreciate any other comments on
the rest of the series as well so that I can work on this and respin.
Thanks,
Shameer
> Thanks,
> Shameer
>
> ----8----
> diff --git a/drivers/perf/arm_smmuv3_pmu.c
> b/drivers/perf/arm_smmuv3_pmu.c
> index ef94b90..6f81b94 100644
> --- a/drivers/perf/arm_smmuv3_pmu.c
> +++ b/drivers/perf/arm_smmuv3_pmu.c
> @@ -96,6 +96,8 @@
>
> #define SMMU_PA_SHIFT 12
>
> +#define SMMU_PMU_OPT_EVCNTR_RDONLY (1 << 0)
> +
> static int cpuhp_state_num;
>
> struct smmu_pmu {
> @@ -111,10 +113,38 @@ struct smmu_pmu {
> struct device *dev;
> void __iomem *reg_base;
> void __iomem *reloc_base;
> + u32 options;
> u64 counter_present_mask;
> u64 counter_mask;
> };
>
> +struct erratum_acpi_oem_info {
> + char oem_id[ACPI_OEM_ID_SIZE + 1];
> + char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> + u32 oem_revision;
> + void (*enable)(struct smmu_pmu *smmu_pmu);
> +};
> +
> +void hisi_erratum_evcntr_rdonly(struct smmu_pmu *smmu_pmu)
> +{
> + smmu_pmu->options |= SMMU_PMU_OPT_EVCNTR_RDONLY;
> + dev_info(smmu_pmu->dev, "Enabling HiSilicon erratum
> 162001800\n");
> +}
> +
> +static struct erratum_acpi_oem_info acpi_oem_info[] = {
> + /*
> + * Note that trailing spaces are required to properly match
> + * the OEM table information.
> + */
> + {
> + .oem_id = "HISI ",
> + .oem_table_id = "HIP08 ",
> + .oem_revision = 0,
> + .enable = hisi_erratum_evcntr_rdonly,
> + },
> + { /* Sentinel indicating the end of the OEM array */ },
> +};
> +
> #define to_smmu_pmu(p) (container_of(p, struct smmu_pmu, pmu))
>
> #define SMMU_PMU_EVENT_ATTR_EXTRACTOR(_name, _config, _start, _end)
> \
> @@ -224,15 +254,20 @@ static void smmu_pmu_set_period(struct
> smmu_pmu *smmu_pmu,
> u32 idx = hwc->idx;
> u64 new;
>
> - /*
> - * We limit the max period to half the max counter value of the
> counter
> - * size, so that even in the case of extreme interrupt latency the
> - * counter will (hopefully) not wrap past its initial value.
> - */
> - new = smmu_pmu->counter_mask >> 1;
> + if (smmu_pmu->options & SMMU_PMU_OPT_EVCNTR_RDONLY) {
> + new = smmu_pmu_counter_get_value(smmu_pmu, idx);
> + } else {
> + /*
> + * We limit the max period to half the max counter value
> + * of the counter size, so that even in the case of extreme
> + * interrupt latency the counter will (hopefully) not wrap
> + * past its initial value.
> + */
> + new = smmu_pmu->counter_mask >> 1;
> + smmu_pmu_counter_set_value(smmu_pmu, idx, new);
> + }
>
> local64_set(&hwc->prev_count, new);
> - smmu_pmu_counter_set_value(smmu_pmu, idx, new);
> }
>
> static void smmu_pmu_get_event_filter(struct perf_event *event, u32 *span,
> @@ -670,6 +705,28 @@ static void smmu_pmu_reset(struct smmu_pmu
> *smmu_pmu)
> smmu_pmu->reloc_base + SMMU_PMCG_OVSCLR0);
> }
>
> +static void smmu_pmu_check_acpi_workarounds(struct smmu_pmu
> *smmu_pmu)
> +{
> + static const struct erratum_acpi_oem_info empty_oem_info = {};
> + const struct erratum_acpi_oem_info *info = acpi_oem_info;
> + struct acpi_table_header *hdr;
> +
> + if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_IORT, 0, &hdr))) {
> + dev_err(smmu_pmu->dev, "failed to get IORT\n");
> + return;
> + }
> +
> + /* Iterate over the ACPI OEM info array, looking for a match */
> + while (memcmp(info, &empty_oem_info, sizeof(*info))) {
> + if (!memcmp(info->oem_id, hdr->oem_id, ACPI_OEM_ID_SIZE)
> &&
> + !memcmp(info->oem_table_id, hdr->oem_table_id,
> ACPI_OEM_TABLE_ID_SIZE) &&
> + info->oem_revision == hdr->oem_revision)
> + info->enable(smmu_pmu);
> +
> + info++;
> + }
> +}
> +
> static int smmu_pmu_probe(struct platform_device *pdev)
> {
> struct smmu_pmu *smmu_pmu;
> @@ -749,6 +806,8 @@ static int smmu_pmu_probe(struct platform_device
> *pdev)
> return -EINVAL;
> }
>
> + smmu_pmu_check_acpi_workarounds(smmu_pmu);
> +
> /* Pick one CPU to be the preferred one to use */
> smmu_pmu->on_cpu = get_cpu();
> WARN_ON(irq_set_affinity(smmu_pmu->irq, cpumask_of(smmu_pmu-
> >on_cpu)));
>
> _______________________________________________
> Linuxarm mailing list
> Linuxarm@huawei.com
> http://hulk.huawei.com/mailman/listinfo/linuxarm
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum 162001800 quirk
2018-10-18 15:27 ` Shameerali Kolothum Thodi
2018-11-09 16:50 ` Shameerali Kolothum Thodi
@ 2018-11-26 18:45 ` Robin Murphy
2018-11-27 13:23 ` Shameerali Kolothum Thodi
1 sibling, 1 reply; 17+ messages in thread
From: Robin Murphy @ 2018-11-26 18:45 UTC (permalink / raw)
To: Shameerali Kolothum Thodi, lorenzo.pieralisi, jean-philippe.brucker
Cc: mark.rutland, vkilari, neil.m.leeder, pabba, will.deacon,
rruigrok, Linuxarm, linux-kernel, linux-acpi, linux-arm-kernel
Hi Shameer,
Sorry for the delay...
On 18/10/2018 16:27, Shameerali Kolothum Thodi wrote:
>
>
>> -----Original Message-----
>> From: Linuxarm [mailto:linuxarm-bounces@huawei.com] On Behalf Of
>> Shameerali Kolothum Thodi
>> Sent: 18 October 2018 14:34
>> To: Robin Murphy <robin.murphy@arm.com>; lorenzo.pieralisi@arm.com;
>> jean-philippe.brucker@arm.com
>> Cc: mark.rutland@arm.com; vkilari@codeaurora.org;
>> neil.m.leeder@gmail.com; pabba@codeaurora.org; will.deacon@arm.com;
>> rruigrok@codeaurora.org; Linuxarm <linuxarm@huawei.com>; linux-
>> kernel@vger.kernel.org; linux-acpi@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org
>> Subject: RE: [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum
>> 162001800 quirk
>>
>> Hi Robin,
>>
>>> -----Original Message-----
>>> From: Robin Murphy [mailto:robin.murphy@arm.com]
>>> Sent: 18 October 2018 12:44
>>> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
>>> lorenzo.pieralisi@arm.com; jean-philippe.brucker@arm.com
>>> Cc: will.deacon@arm.com; mark.rutland@arm.com; Guohanjun (Hanjun Guo)
>>> <guohanjun@huawei.com>; John Garry <john.garry@huawei.com>;
>>> pabba@codeaurora.org; vkilari@codeaurora.org; rruigrok@codeaurora.org;
>>> linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
>>> kernel@lists.infradead.org; Linuxarm <linuxarm@huawei.com>;
>>> neil.m.leeder@gmail.com
>>> Subject: Re: [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum
>>> 162001800 quirk
>
> [...]
>
>>>> +static const struct smmu_pmu_erratum_wa smmu_pmu_wa[] = {
>>>> + {
>>>> + .match_type = se_match_acpi_oem,
>>>> + .id = hisi_162001800_oem_info,
>>>> + .desc_str = "HiSilicon erratum 162001800",
>>>> + .enable = hisi_erratum_evcntr_rdonly,
>>>> + },
>>>> +};
>>>> +
>>>
>>> There's an awful lot of raw ACPI internals splashed about here -
>>> couldn't at least some of it be abstracted behind the IORT code? In
>>> fact, can't IORT just set all this stuff up in advance like it does for
>>> SMMUs?
>>
>> Hmmm.. Sorry, not clear to me. You mean to say associate the IORT node
>> with platform device and retrieve it in driver just like smmu does for
>> "model" checks? Not sure that works here if that’s what the above meant.
I don't think there's much of interest in the actual IORT node itself,
but I can't see that there would be any particular problem with passing
either some implementation identifier or just a ready-made set of quirk
flags to the PMCG driver via platdata.
>>>> #define to_smmu_pmu(p) (container_of(p, struct smmu_pmu, pmu))
>>>>
>>>> #define SMMU_PMU_EVENT_ATTR_EXTRACTOR(_name, _config, _start,
>>> _end) \
>>>> @@ -224,15 +271,20 @@ static void smmu_pmu_set_period(struct
>>> smmu_pmu *smmu_pmu,
>>>> u32 idx = hwc->idx;
>>>> u64 new;
>>>>
>>>> - /*
>>>> - * We limit the max period to half the max counter value of the
>>> counter
>>>> - * size, so that even in the case of extreme interrupt latency the
>>>> - * counter will (hopefully) not wrap past its initial value.
>>>> - */
>>>> - new = smmu_pmu->counter_mask >> 1;
>>>> + if (smmu_pmu->options & SMMU_PMU_OPT_EVCNTR_RDONLY) {
>>>> + new = smmu_pmu_counter_get_value(smmu_pmu, idx);
>>>
>>> Something's clearly missing, because if this happens to start at 0, the
>>> current overflow handling code cannot possibly give the correct count.
>>> Much as I hate the reset-to-half-period idiom for being impossible to
>>> make sense of, it does make various aspects appear a lot simpler than
>>> they really are. Wait, maybe that's yet another reason to hate it...
>>
>> Yes, if the counter starts at 0 and overflow happens, it won't possibly give
>> the correct count compared to the reset-to-half-period logic. Since this is a
>> 64 bit counter, just hope that, it won't necessarily happen that often.
OK, if the full 64 counter bits are implemented, then I suppose we're
probably OK to assume nobody's going to run a single profiling session
over 4+ years or so. It might be worth a comment just to remind
ourselves that we're (currently) relying on the counter size to mostly
mitigate overflow-related issues in this case.
>
> [...]
>
>>>> +static void smmu_pmu_enable_errata(struct smmu_pmu *smmu_pmu,
>>>> + enum smmu_pmu_erratum_match_type type,
>>>> + se_match_fn_t match_fn,
>>>> + void *arg)
>>>> +{
>>>> + const struct smmu_pmu_erratum_wa *wa = smmu_pmu_wa;
>>>> +
>>>> + for (; wa->desc_str; wa++) {
>>>> + if (wa->match_type != type)
>>>> + continue;
>>>> +
>>>> + if (match_fn(wa, arg)) {
>>>> + if (wa->enable) {
>>>> + wa->enable(smmu_pmu);
>>>> + dev_info(smmu_pmu->dev,
>>>> + "Enabling workaround for %s\n",
>>>> + wa->desc_str);
>>>> + }
>>>
>>> Just how many kinds of broken are we expecting here? Is this lifted from
>>> the arm64 cpufeature framework, because it seems like absolute overkill
>>> for a simple PMU driver which in all reality is only ever going to
>>> wiggle a few flags in some data structure.
>>
>> Yes, this erratum framework is based on the arm_arch_timer code. Agree that
>> this is an overkill if it is just to support this hardware. I am not sure this can be
>> extended to add the IMPLEMENTATION DEFINED events in future(I haven't
>> looked into that now). If this is not that useful in the near future, I will remove
>> the
>> framework part and use the OEM info directly to set the flag. Please let me
>> know
>> your thoughts..
>
> Below is another take on this patch. Please let me know if this makes any sense..
>
> Thanks,
> Shameer
>
> ----8----
> diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
> index ef94b90..6f81b94 100644
> --- a/drivers/perf/arm_smmuv3_pmu.c
> +++ b/drivers/perf/arm_smmuv3_pmu.c
> @@ -96,6 +96,8 @@
>
> #define SMMU_PA_SHIFT 12
>
> +#define SMMU_PMU_OPT_EVCNTR_RDONLY (1 << 0)
> +
> static int cpuhp_state_num;
>
> struct smmu_pmu {
> @@ -111,10 +113,38 @@ struct smmu_pmu {
> struct device *dev;
> void __iomem *reg_base;
> void __iomem *reloc_base;
> + u32 options;
> u64 counter_present_mask;
> u64 counter_mask;
> };
>
> +struct erratum_acpi_oem_info {
> + char oem_id[ACPI_OEM_ID_SIZE + 1];
> + char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> + u32 oem_revision;
> + void (*enable)(struct smmu_pmu *smmu_pmu);
> +};
> +
> +void hisi_erratum_evcntr_rdonly(struct smmu_pmu *smmu_pmu)
> +{
> + smmu_pmu->options |= SMMU_PMU_OPT_EVCNTR_RDONLY;
> + dev_info(smmu_pmu->dev, "Enabling HiSilicon erratum 162001800\n");
> +}
> +
> +static struct erratum_acpi_oem_info acpi_oem_info[] = {
> + /*
> + * Note that trailing spaces are required to properly match
> + * the OEM table information.
> + */
> + {
> + .oem_id = "HISI ",
> + .oem_table_id = "HIP08 ",
> + .oem_revision = 0,
> + .enable = hisi_erratum_evcntr_rdonly,
> + },
> + { /* Sentinel indicating the end of the OEM array */ },
> +};
> +
> #define to_smmu_pmu(p) (container_of(p, struct smmu_pmu, pmu))
>
> #define SMMU_PMU_EVENT_ATTR_EXTRACTOR(_name, _config, _start, _end) \
> @@ -224,15 +254,20 @@ static void smmu_pmu_set_period(struct smmu_pmu *smmu_pmu,
> u32 idx = hwc->idx;
> u64 new;
>
> - /*
> - * We limit the max period to half the max counter value of the counter
> - * size, so that even in the case of extreme interrupt latency the
> - * counter will (hopefully) not wrap past its initial value.
> - */
> - new = smmu_pmu->counter_mask >> 1;
> + if (smmu_pmu->options & SMMU_PMU_OPT_EVCNTR_RDONLY) {
> + new = smmu_pmu_counter_get_value(smmu_pmu, idx);
> + } else {
> + /*
> + * We limit the max period to half the max counter value
> + * of the counter size, so that even in the case of extreme
> + * interrupt latency the counter will (hopefully) not wrap
> + * past its initial value.
> + */
> + new = smmu_pmu->counter_mask >> 1;
> + smmu_pmu_counter_set_value(smmu_pmu, idx, new);
> + }
>
> local64_set(&hwc->prev_count, new);
> - smmu_pmu_counter_set_value(smmu_pmu, idx, new);
> }
>
> static void smmu_pmu_get_event_filter(struct perf_event *event, u32 *span,
> @@ -670,6 +705,28 @@ static void smmu_pmu_reset(struct smmu_pmu *smmu_pmu)
> smmu_pmu->reloc_base + SMMU_PMCG_OVSCLR0);
> }
>
> +static void smmu_pmu_check_acpi_workarounds(struct smmu_pmu *smmu_pmu)
> +{
> + static const struct erratum_acpi_oem_info empty_oem_info = {};
> + const struct erratum_acpi_oem_info *info = acpi_oem_info;
> + struct acpi_table_header *hdr;
> +
> + if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_IORT, 0, &hdr))) {
> + dev_err(smmu_pmu->dev, "failed to get IORT\n");
> + return;
> + }
> +
> + /* Iterate over the ACPI OEM info array, looking for a match */
> + while (memcmp(info, &empty_oem_info, sizeof(*info))) {
> + if (!memcmp(info->oem_id, hdr->oem_id, ACPI_OEM_ID_SIZE) &&
> + !memcmp(info->oem_table_id, hdr->oem_table_id, ACPI_OEM_TABLE_ID_SIZE) &&
> + info->oem_revision == hdr->oem_revision)
> + info->enable(smmu_pmu);
> +
> + info++;
> + }
> +}
FWIW, this looks awfully like acpi_match_platform_list()...
However, I still think that any parsing of IORT fields belongs in
iort.c, not in every driver which might ever need to detect a quirk. For
starters, that code has iort_table to hand, full knowledge of all the
other identifiable components, and a already contains a bunch of
system-specific quirk detection which could potentially be shared.
[ side note: do you know if 1620 still has the same ITS quirk as 161x,
or is it just the SMMU's MSI output that didn't get updated? ]
Robin.
> +
> static int smmu_pmu_probe(struct platform_device *pdev)
> {
> struct smmu_pmu *smmu_pmu;
> @@ -749,6 +806,8 @@ static int smmu_pmu_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> + smmu_pmu_check_acpi_workarounds(smmu_pmu);
> +
> /* Pick one CPU to be the preferred one to use */
> smmu_pmu->on_cpu = get_cpu();
> WARN_ON(irq_set_affinity(smmu_pmu->irq, cpumask_of(smmu_pmu->on_cpu)));
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum 162001800 quirk
2018-11-26 18:45 ` Robin Murphy
@ 2018-11-27 13:23 ` Shameerali Kolothum Thodi
0 siblings, 0 replies; 17+ messages in thread
From: Shameerali Kolothum Thodi @ 2018-11-27 13:23 UTC (permalink / raw)
To: Robin Murphy, lorenzo.pieralisi, jean-philippe.brucker
Cc: mark.rutland, vkilari, neil.m.leeder, pabba, will.deacon,
rruigrok, Linuxarm, linux-kernel, linux-acpi, linux-arm-kernel
> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@arm.com]
> Sent: 26 November 2018 18:45
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> lorenzo.pieralisi@arm.com; jean-philippe.brucker@arm.com
> Cc: mark.rutland@arm.com; vkilari@codeaurora.org;
> neil.m.leeder@gmail.com; pabba@codeaurora.org; will.deacon@arm.com;
> rruigrok@codeaurora.org; Linuxarm <linuxarm@huawei.com>; linux-
> kernel@vger.kernel.org; linux-acpi@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum
> 162001800 quirk
>
> Hi Shameer,
>
> Sorry for the delay...
>
> On 18/10/2018 16:27, Shameerali Kolothum Thodi wrote:
> >
> >
> >> -----Original Message-----
> >> From: Linuxarm [mailto:linuxarm-bounces@huawei.com] On Behalf Of
> >> Shameerali Kolothum Thodi
> >> Sent: 18 October 2018 14:34
> >> To: Robin Murphy <robin.murphy@arm.com>; lorenzo.pieralisi@arm.com;
> >> jean-philippe.brucker@arm.com
> >> Cc: mark.rutland@arm.com; vkilari@codeaurora.org;
> >> neil.m.leeder@gmail.com; pabba@codeaurora.org; will.deacon@arm.com;
> >> rruigrok@codeaurora.org; Linuxarm <linuxarm@huawei.com>; linux-
> >> kernel@vger.kernel.org; linux-acpi@vger.kernel.org; linux-arm-
> >> kernel@lists.infradead.org
> >> Subject: RE: [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum
> >> 162001800 quirk
> >>
> >> Hi Robin,
> >>
> >>> -----Original Message-----
> >>> From: Robin Murphy [mailto:robin.murphy@arm.com]
> >>> Sent: 18 October 2018 12:44
> >>> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>;
> >>> lorenzo.pieralisi@arm.com; jean-philippe.brucker@arm.com
> >>> Cc: will.deacon@arm.com; mark.rutland@arm.com; Guohanjun (Hanjun
> Guo)
> >>> <guohanjun@huawei.com>; John Garry <john.garry@huawei.com>;
> >>> pabba@codeaurora.org; vkilari@codeaurora.org;
> rruigrok@codeaurora.org;
> >>> linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> >>> kernel@lists.infradead.org; Linuxarm <linuxarm@huawei.com>;
> >>> neil.m.leeder@gmail.com
> >>> Subject: Re: [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum
> >>> 162001800 quirk
> >
> > [...]
> >
> >>>> +static const struct smmu_pmu_erratum_wa smmu_pmu_wa[] = {
> >>>> + {
> >>>> + .match_type = se_match_acpi_oem,
> >>>> + .id = hisi_162001800_oem_info,
> >>>> + .desc_str = "HiSilicon erratum 162001800",
> >>>> + .enable = hisi_erratum_evcntr_rdonly,
> >>>> + },
> >>>> +};
> >>>> +
> >>>
> >>> There's an awful lot of raw ACPI internals splashed about here -
> >>> couldn't at least some of it be abstracted behind the IORT code? In
> >>> fact, can't IORT just set all this stuff up in advance like it does for
> >>> SMMUs?
> >>
> >> Hmmm.. Sorry, not clear to me. You mean to say associate the IORT node
> >> with platform device and retrieve it in driver just like smmu does for
> >> "model" checks? Not sure that works here if that’s what the above meant.
>
> I don't think there's much of interest in the actual IORT node itself,
> but I can't see that there would be any particular problem with passing
> either some implementation identifier or just a ready-made set of quirk
> flags to the PMCG driver via platdata.
Ok.
> >>>> #define to_smmu_pmu(p) (container_of(p, struct smmu_pmu, pmu))
> >>>>
> >>>> #define SMMU_PMU_EVENT_ATTR_EXTRACTOR(_name, _config,
> _start,
> >>> _end) \
> >>>> @@ -224,15 +271,20 @@ static void smmu_pmu_set_period(struct
> >>> smmu_pmu *smmu_pmu,
> >>>> u32 idx = hwc->idx;
> >>>> u64 new;
> >>>>
> >>>> - /*
> >>>> - * We limit the max period to half the max counter value of the
> >>> counter
> >>>> - * size, so that even in the case of extreme interrupt latency the
> >>>> - * counter will (hopefully) not wrap past its initial value.
> >>>> - */
> >>>> - new = smmu_pmu->counter_mask >> 1;
> >>>> + if (smmu_pmu->options & SMMU_PMU_OPT_EVCNTR_RDONLY) {
> >>>> + new = smmu_pmu_counter_get_value(smmu_pmu, idx);
> >>>
> >>> Something's clearly missing, because if this happens to start at 0, the
> >>> current overflow handling code cannot possibly give the correct count.
> >>> Much as I hate the reset-to-half-period idiom for being impossible to
> >>> make sense of, it does make various aspects appear a lot simpler than
> >>> they really are. Wait, maybe that's yet another reason to hate it...
> >>
> >> Yes, if the counter starts at 0 and overflow happens, it won't possibly give
> >> the correct count compared to the reset-to-half-period logic. Since this is a
> >> 64 bit counter, just hope that, it won't necessarily happen that often.
>
> OK, if the full 64 counter bits are implemented, then I suppose we're
> probably OK to assume nobody's going to run a single profiling session
> over 4+ years or so. It might be worth a comment just to remind
> ourselves that we're (currently) relying on the counter size to mostly
> mitigate overflow-related issues in this case.
Sure, I will add a comment to make it clear.
> >
> > [...]
> >
> >>>> +static void smmu_pmu_enable_errata(struct smmu_pmu *smmu_pmu,
> >>>> + enum smmu_pmu_erratum_match_type type,
> >>>> + se_match_fn_t match_fn,
> >>>> + void *arg)
> >>>> +{
> >>>> + const struct smmu_pmu_erratum_wa *wa = smmu_pmu_wa;
> >>>> +
> >>>> + for (; wa->desc_str; wa++) {
> >>>> + if (wa->match_type != type)
> >>>> + continue;
> >>>> +
> >>>> + if (match_fn(wa, arg)) {
> >>>> + if (wa->enable) {
> >>>> + wa->enable(smmu_pmu);
> >>>> + dev_info(smmu_pmu->dev,
> >>>> + "Enabling workaround for %s\n",
> >>>> + wa->desc_str);
> >>>> + }
> >>>
> >>> Just how many kinds of broken are we expecting here? Is this lifted from
> >>> the arm64 cpufeature framework, because it seems like absolute overkill
> >>> for a simple PMU driver which in all reality is only ever going to
> >>> wiggle a few flags in some data structure.
> >>
> >> Yes, this erratum framework is based on the arm_arch_timer code. Agree
> that
> >> this is an overkill if it is just to support this hardware. I am not sure this can
> be
> >> extended to add the IMPLEMENTATION DEFINED events in future(I haven't
> >> looked into that now). If this is not that useful in the near future, I will
> remove
> >> the
> >> framework part and use the OEM info directly to set the flag. Please let me
> >> know
> >> your thoughts..
> >
> > Below is another take on this patch. Please let me know if this makes any
> sense..
> >
> > Thanks,
> > Shameer
> >
> > ----8----
> > diff --git a/drivers/perf/arm_smmuv3_pmu.c
> b/drivers/perf/arm_smmuv3_pmu.c
> > index ef94b90..6f81b94 100644
> > --- a/drivers/perf/arm_smmuv3_pmu.c
> > +++ b/drivers/perf/arm_smmuv3_pmu.c
> > @@ -96,6 +96,8 @@
> >
> > #define SMMU_PA_SHIFT 12
> >
> > +#define SMMU_PMU_OPT_EVCNTR_RDONLY (1 << 0)
> > +
> > static int cpuhp_state_num;
> >
> > struct smmu_pmu {
> > @@ -111,10 +113,38 @@ struct smmu_pmu {
> > struct device *dev;
> > void __iomem *reg_base;
> > void __iomem *reloc_base;
> > + u32 options;
> > u64 counter_present_mask;
> > u64 counter_mask;
> > };
> >
> > +struct erratum_acpi_oem_info {
> > + char oem_id[ACPI_OEM_ID_SIZE + 1];
> > + char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> > + u32 oem_revision;
> > + void (*enable)(struct smmu_pmu *smmu_pmu);
> > +};
> > +
> > +void hisi_erratum_evcntr_rdonly(struct smmu_pmu *smmu_pmu)
> > +{
> > + smmu_pmu->options |= SMMU_PMU_OPT_EVCNTR_RDONLY;
> > + dev_info(smmu_pmu->dev, "Enabling HiSilicon erratum
> 162001800\n");
> > +}
> > +
> > +static struct erratum_acpi_oem_info acpi_oem_info[] = {
> > + /*
> > + * Note that trailing spaces are required to properly match
> > + * the OEM table information.
> > + */
> > + {
> > + .oem_id = "HISI ",
> > + .oem_table_id = "HIP08 ",
> > + .oem_revision = 0,
> > + .enable = hisi_erratum_evcntr_rdonly,
> > + },
> > + { /* Sentinel indicating the end of the OEM array */ },
> > +};
> > +
> > #define to_smmu_pmu(p) (container_of(p, struct smmu_pmu, pmu))
> >
> > #define SMMU_PMU_EVENT_ATTR_EXTRACTOR(_name, _config, _start,
> _end) \
> > @@ -224,15 +254,20 @@ static void smmu_pmu_set_period(struct
> smmu_pmu *smmu_pmu,
> > u32 idx = hwc->idx;
> > u64 new;
> >
> > - /*
> > - * We limit the max period to half the max counter value of the
> counter
> > - * size, so that even in the case of extreme interrupt latency the
> > - * counter will (hopefully) not wrap past its initial value.
> > - */
> > - new = smmu_pmu->counter_mask >> 1;
> > + if (smmu_pmu->options & SMMU_PMU_OPT_EVCNTR_RDONLY) {
> > + new = smmu_pmu_counter_get_value(smmu_pmu, idx);
> > + } else {
> > + /*
> > + * We limit the max period to half the max counter value
> > + * of the counter size, so that even in the case of extreme
> > + * interrupt latency the counter will (hopefully) not wrap
> > + * past its initial value.
> > + */
> > + new = smmu_pmu->counter_mask >> 1;
> > + smmu_pmu_counter_set_value(smmu_pmu, idx, new);
> > + }
> >
> > local64_set(&hwc->prev_count, new);
> > - smmu_pmu_counter_set_value(smmu_pmu, idx, new);
> > }
> >
> > static void smmu_pmu_get_event_filter(struct perf_event *event, u32
> *span,
> > @@ -670,6 +705,28 @@ static void smmu_pmu_reset(struct smmu_pmu
> *smmu_pmu)
> > smmu_pmu->reloc_base + SMMU_PMCG_OVSCLR0);
> > }
> >
> > +static void smmu_pmu_check_acpi_workarounds(struct smmu_pmu
> *smmu_pmu)
> > +{
> > + static const struct erratum_acpi_oem_info empty_oem_info = {};
> > + const struct erratum_acpi_oem_info *info = acpi_oem_info;
> > + struct acpi_table_header *hdr;
> > +
> > + if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_IORT, 0, &hdr))) {
> > + dev_err(smmu_pmu->dev, "failed to get IORT\n");
> > + return;
> > + }
> > +
> > + /* Iterate over the ACPI OEM info array, looking for a match */
> > + while (memcmp(info, &empty_oem_info, sizeof(*info))) {
> > + if (!memcmp(info->oem_id, hdr->oem_id, ACPI_OEM_ID_SIZE)
> &&
> > + !memcmp(info->oem_table_id, hdr->oem_table_id,
> ACPI_OEM_TABLE_ID_SIZE) &&
> > + info->oem_revision == hdr->oem_revision)
> > + info->enable(smmu_pmu);
> > +
> > + info++;
> > + }
> > +}
>
> FWIW, this looks awfully like acpi_match_platform_list()...
>
> However, I still think that any parsing of IORT fields belongs in
> iort.c, not in every driver which might ever need to detect a quirk. For
> starters, that code has iort_table to hand, full knowledge of all the
> other identifiable components, and a already contains a bunch of
> system-specific quirk detection which could potentially be shared.
Ok. I will take a look into moving this into IORT code and sharing
through platform data.
> [ side note: do you know if 1620 still has the same ITS quirk as 161x,
> or is it just the SMMU's MSI output that didn't get updated? ]
We don’t have the MSI reserve region issue for 1620. But I think it still
uses the upper 4 bytes of GITS_TRANSLATER and requires the patch from
Lei Zhen that takes care of sync_count overwrite memory corruption issue.
Thanks,
Shameer
>
> > +
> > static int smmu_pmu_probe(struct platform_device *pdev)
> > {
> > struct smmu_pmu *smmu_pmu;
> > @@ -749,6 +806,8 @@ static int smmu_pmu_probe(struct platform_device
> *pdev)
> > return -EINVAL;
> > }
> >
> > + smmu_pmu_check_acpi_workarounds(smmu_pmu);
> > +
> > /* Pick one CPU to be the preferred one to use */
> > smmu_pmu->on_cpu = get_cpu();
> > WARN_ON(irq_set_affinity(smmu_pmu->irq, cpumask_of(smmu_pmu-
> >on_cpu)));
> >
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-11-27 13:23 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-16 12:49 [PATCH v4 0/4] arm64 SMMUv3 PMU driver with IORT support Shameer Kolothum
2018-10-16 12:49 ` [PATCH v4 1/4] acpi: arm64: add iort support for PMCG Shameer Kolothum
2018-10-16 12:49 ` [PATCH v4 2/4] perf: add arm64 smmuv3 pmu driver Shameer Kolothum
2018-10-17 21:53 ` kbuild test robot
2018-10-18 9:26 ` Shameerali Kolothum Thodi
2018-10-20 4:50 ` kbuild test robot
2018-10-16 12:49 ` [PATCH v4 3/4] perf/smmuv3: Add MSI irq support Shameer Kolothum
2018-10-17 3:35 ` kbuild test robot
2018-10-17 9:48 ` Shameerali Kolothum Thodi
2018-10-17 15:42 ` kbuild test robot
2018-10-16 12:49 ` [PATCH v4 4/4] perf/smmuv3_pmu: Enable HiSilicon Erratum 162001800 quirk Shameer Kolothum
2018-10-18 11:43 ` Robin Murphy
2018-10-18 13:34 ` Shameerali Kolothum Thodi
2018-10-18 15:27 ` Shameerali Kolothum Thodi
2018-11-09 16:50 ` Shameerali Kolothum Thodi
2018-11-26 18:45 ` Robin Murphy
2018-11-27 13:23 ` Shameerali Kolothum Thodi
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).