LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64
@ 2015-03-10 15:18 Suzuki K. Poulose
  2015-03-10 15:18 ` [PATCH 1/5] arm-cci: Rearrange code for splitting PMU vs driver code Suzuki K. Poulose
                   ` (8 more replies)
  0 siblings, 9 replies; 24+ messages in thread
From: Suzuki K. Poulose @ 2015-03-10 15:18 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Nicolas Pitre, Bartlomiej Zolnierkiewicz, Kukjin Kim,
	Abhilash Kesavan, Arnd Bergmann, devicetree, linux-kernel,
	Liviu Dudau, Lorenzo Pieralisi, Olof Johansson, Pawel Moll,
	Punit Agrawal, Sudeep Holla, Will Deacon, Catalin Marinas,
	Suzuki K. Poulose

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

This series enables the PMU monitoring support for CCI400 on ARM64.
The existing CCI400 driver code is a mix of PMU driver and the MCPM
driver code. The MCPM driver is only used on ARM(32) and contains
arm32 assembly and hence can't be built on ARM64. This patch splits
the code to

 - ARM_CCI400_PORT_CTRL driver - depends on ARM && V7
 - ARM_CCI400_PMU driver

Accessing the Peripheral ID2 register(PID2) on CCI-400, to detect
the revision of the chipset, is a secure operation. Hence, it prevents
us from running this on non-secure platforms. The issue is overcome by
explicitly mentioning the revision number of the CCI PMU in the device tree
binding. The device-tree binding has been updated with the new bindings.

i.e,	arm-cci-400-pmu,r0 => revision 0
	arm-cci-400-pmu,r1 => revision 1
	arm-cci-400-pmu => (old) DEPRECATED

The old binding has been DEPRECATED and must be used only on ARM32
system with secure access. We don't have a reliable dynamic way to detect
if the system is running secure. This series tries to use the best safe
method by relying on the availability of MCPM(as it was prior to the series).
It is upto the MCPM platform driver to decide, if the system is secure before
it goes ahead and registers its drivers and pokes the CCI. This series doesn't
address/solve the problem of MCPM. I will be happy to use a better approach,
if there is any.

Tested on (non-secure)TC2 and A53x2.

Changes sinve V2

 - Include suggestions from Sudeep Holla.
 - Cleanup event validation checks.

Changes since V1 (Suggestions from Nicolas Pitre):

 - Split Patch 2 to separate the 'PMU' abstraction(now Patch 2/5)
   from the introduction of a new device-tree binding(now Patch 3/5)
 - Rename
	ARM_CCI400_MCPM => ARM_CCI400_PORT_CTRL
	CCI400_MCPM_PORTS_DATA => CCI400_PORTS_DATA
 - Select ARM_CCI400_COMMON for ARM_CCI400_PORT_CTRL
 - Better documentation in the git commit log about the ARM_CCI config.
 - Move the 'pr_info' to its apporpriate patch.


Suzuki K. Poulose (5):
  arm-cci: Rearrange code for splitting PMU vs driver code
  arm-cci: Abstract the CCI400 PMU speicific definitions
  arm-cci: Get rid of secure transactions for PMU driver
  arm-cci: Split the code for PMU vs driver support
  arm-cci: Fix CCI PMU event validation

 Documentation/devicetree/bindings/arm/cci.txt |    7 +-
 arch/arm/include/asm/arm-cci.h                |   42 +++
 arch/arm/mach-exynos/Kconfig                  |    2 +-
 arch/arm/mach-vexpress/Kconfig                |    4 +-
 arch/arm64/include/asm/arm-cci.h              |   27 ++
 drivers/bus/Kconfig                           |   28 +-
 drivers/bus/arm-cci.c                         |  496 ++++++++++++++-----------
 include/linux/arm-cci.h                       |    9 +-
 8 files changed, 384 insertions(+), 231 deletions(-)
 create mode 100644 arch/arm/include/asm/arm-cci.h
 create mode 100644 arch/arm64/include/asm/arm-cci.h

-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/5] arm-cci: Rearrange code for splitting PMU vs driver code
  2015-03-10 15:18 [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Suzuki K. Poulose
@ 2015-03-10 15:18 ` Suzuki K. Poulose
  2015-03-10 15:18 ` [PATCH 2/5] arm-cci: Abstract the CCI400 PMU speicific definitions Suzuki K. Poulose
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Suzuki K. Poulose @ 2015-03-10 15:18 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Nicolas Pitre, Bartlomiej Zolnierkiewicz, Kukjin Kim,
	Abhilash Kesavan, Arnd Bergmann, devicetree, linux-kernel,
	Liviu Dudau, Lorenzo Pieralisi, Olof Johansson, Pawel Moll,
	Punit Agrawal, Sudeep Holla, Will Deacon, Catalin Marinas,
	Suzuki K. Poulose

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

No functional changes, only code re-arrangements for easier split of the
PMU code vs low level driver code. Extracts the port handling code
to cci_probe_ports().

Change since V2:
 - Removed unnecessary goto. (Suggested-by: Sudeep Holla)

Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
---
 drivers/bus/arm-cci.c |  326 ++++++++++++++++++++++++-------------------------
 1 file changed, 163 insertions(+), 163 deletions(-)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index 84fd660..ea39fc2 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -29,42 +29,29 @@
 #include <asm/cacheflush.h>
 #include <asm/smp_plat.h>
 
-#define DRIVER_NAME		"CCI-400"
-#define DRIVER_NAME_PMU		DRIVER_NAME " PMU"
-
-#define CCI_PORT_CTRL		0x0
-#define CCI_CTRL_STATUS		0xc
-
-#define CCI_ENABLE_SNOOP_REQ	0x1
-#define CCI_ENABLE_DVM_REQ	0x2
-#define CCI_ENABLE_REQ		(CCI_ENABLE_SNOOP_REQ | CCI_ENABLE_DVM_REQ)
+static void __iomem *cci_ctrl_base;
+static unsigned long cci_ctrl_phys;
 
 struct cci_nb_ports {
 	unsigned int nb_ace;
 	unsigned int nb_ace_lite;
 };
 
-enum cci_ace_port_type {
-	ACE_INVALID_PORT = 0x0,
-	ACE_PORT,
-	ACE_LITE_PORT,
+static const struct cci_nb_ports cci400_ports = {
+	.nb_ace = 2,
+	.nb_ace_lite = 3
 };
 
-struct cci_ace_port {
-	void __iomem *base;
-	unsigned long phys;
-	enum cci_ace_port_type type;
-	struct device_node *dn;
+static const struct of_device_id arm_cci_matches[] = {
+	{.compatible = "arm,cci-400", .data = &cci400_ports },
+	{},
 };
 
-static struct cci_ace_port *ports;
-static unsigned int nb_cci_ports;
-
-static void __iomem *cci_ctrl_base;
-static unsigned long cci_ctrl_phys;
-
 #ifdef CONFIG_HW_PERF_EVENTS
 
+#define DRIVER_NAME		"CCI-400"
+#define DRIVER_NAME_PMU		DRIVER_NAME " PMU"
+
 #define CCI_PMCR		0x0100
 #define CCI_PID2		0x0fe8
 
@@ -75,6 +62,47 @@ static unsigned long cci_ctrl_phys;
 #define CCI_PID2_REV_MASK	0xf0
 #define CCI_PID2_REV_SHIFT	4
 
+#define CCI_PMU_EVT_SEL		0x000
+#define CCI_PMU_CNTR		0x004
+#define CCI_PMU_CNTR_CTRL	0x008
+#define CCI_PMU_OVRFLW		0x00c
+
+#define CCI_PMU_OVRFLW_FLAG	1
+
+#define CCI_PMU_CNTR_BASE(idx)	((idx) * SZ_4K)
+
+#define CCI_PMU_CNTR_MASK	((1ULL << 32) -1)
+
+#define CCI_PMU_EVENT_MASK		0xff
+#define CCI_PMU_EVENT_SOURCE(event)	((event >> 5) & 0x7)
+#define CCI_PMU_EVENT_CODE(event)	(event & 0x1f)
+
+#define CCI_PMU_MAX_HW_EVENTS 5   /* CCI PMU has 4 counters + 1 cycle counter */
+
+struct cci_pmu_hw_events {
+	struct perf_event *events[CCI_PMU_MAX_HW_EVENTS];
+	unsigned long used_mask[BITS_TO_LONGS(CCI_PMU_MAX_HW_EVENTS)];
+	raw_spinlock_t pmu_lock;
+};
+
+struct cci_pmu {
+	void __iomem *base;
+	struct pmu pmu;
+	int nr_irqs;
+	int irqs[CCI_PMU_MAX_HW_EVENTS];
+	unsigned long active_irqs;
+	struct pmu_port_event_ranges *port_ranges;
+	struct cci_pmu_hw_events hw_events;
+	struct platform_device *plat_device;
+	int num_events;
+	atomic_t active_events;
+	struct mutex reserve_mutex;
+	cpumask_t cpus;
+};
+static struct cci_pmu *pmu;
+
+#define to_cci_pmu(c)	(container_of(c, struct cci_pmu, pmu))
+
 /* Port ids */
 #define CCI_PORT_S0	0
 #define CCI_PORT_S1	1
@@ -89,17 +117,6 @@ static unsigned long cci_ctrl_phys;
 #define CCI_REV_R1		1
 #define CCI_REV_R1_PX		5
 
-#define CCI_PMU_EVT_SEL		0x000
-#define CCI_PMU_CNTR		0x004
-#define CCI_PMU_CNTR_CTRL	0x008
-#define CCI_PMU_OVRFLW		0x00c
-
-#define CCI_PMU_OVRFLW_FLAG	1
-
-#define CCI_PMU_CNTR_BASE(idx)	((idx) * SZ_4K)
-
-#define CCI_PMU_CNTR_MASK	((1ULL << 32) -1)
-
 /*
  * Instead of an event id to monitor CCI cycles, a dedicated counter is
  * provided. Use 0xff to represent CCI cycles and hope that no future revisions
@@ -109,12 +126,6 @@ enum cci400_perf_events {
 	CCI_PMU_CYCLES = 0xff
 };
 
-#define CCI_PMU_EVENT_MASK		0xff
-#define CCI_PMU_EVENT_SOURCE(event)	((event >> 5) & 0x7)
-#define CCI_PMU_EVENT_CODE(event)	(event & 0x1f)
-
-#define CCI_PMU_MAX_HW_EVENTS 5   /* CCI PMU has 4 counters + 1 cycle counter */
-
 #define CCI_PMU_CYCLE_CNTR_IDX		0
 #define CCI_PMU_CNTR0_IDX		1
 #define CCI_PMU_CNTR_LAST(cci_pmu)	(CCI_PMU_CYCLE_CNTR_IDX + cci_pmu->num_events - 1)
@@ -172,60 +183,6 @@ static char *const pmu_names[] = {
 	[CCI_REV_R1] = "CCI_400_r1",
 };
 
-struct cci_pmu_hw_events {
-	struct perf_event *events[CCI_PMU_MAX_HW_EVENTS];
-	unsigned long used_mask[BITS_TO_LONGS(CCI_PMU_MAX_HW_EVENTS)];
-	raw_spinlock_t pmu_lock;
-};
-
-struct cci_pmu {
-	void __iomem *base;
-	struct pmu pmu;
-	int nr_irqs;
-	int irqs[CCI_PMU_MAX_HW_EVENTS];
-	unsigned long active_irqs;
-	struct pmu_port_event_ranges *port_ranges;
-	struct cci_pmu_hw_events hw_events;
-	struct platform_device *plat_device;
-	int num_events;
-	atomic_t active_events;
-	struct mutex reserve_mutex;
-	cpumask_t cpus;
-};
-static struct cci_pmu *pmu;
-
-#define to_cci_pmu(c)	(container_of(c, struct cci_pmu, pmu))
-
-static bool is_duplicate_irq(int irq, int *irqs, int nr_irqs)
-{
-	int i;
-
-	for (i = 0; i < nr_irqs; i++)
-		if (irq == irqs[i])
-			return true;
-
-	return false;
-}
-
-static int probe_cci_revision(void)
-{
-	int rev;
-	rev = readl_relaxed(cci_ctrl_base + CCI_PID2) & CCI_PID2_REV_MASK;
-	rev >>= CCI_PID2_REV_SHIFT;
-
-	if (rev < CCI_REV_R1_PX)
-		return CCI_REV_R0;
-	else
-		return CCI_REV_R1;
-}
-
-static struct pmu_port_event_ranges *port_range_by_rev(void)
-{
-	int rev = probe_cci_revision();
-
-	return &port_event_range[rev];
-}
-
 static int pmu_is_valid_slave_event(u8 ev_code)
 {
 	return pmu->port_ranges->slave_min <= ev_code &&
@@ -265,6 +222,25 @@ static int pmu_validate_hw_event(u8 hw_event)
 	return -ENOENT;
 }
 
+static int probe_cci_revision(void)
+{
+	int rev;
+	rev = readl_relaxed(cci_ctrl_base + CCI_PID2) & CCI_PID2_REV_MASK;
+	rev >>= CCI_PID2_REV_SHIFT;
+
+	if (rev < CCI_REV_R1_PX)
+		return CCI_REV_R0;
+	else
+		return CCI_REV_R1;
+}
+
+static struct pmu_port_event_ranges *port_range_by_rev(void)
+{
+	int rev = probe_cci_revision();
+
+	return &port_event_range[rev];
+}
+
 static int pmu_is_valid_counter(struct cci_pmu *cci_pmu, int idx)
 {
 	return CCI_PMU_CYCLE_CNTR_IDX <= idx &&
@@ -893,6 +869,17 @@ static const struct of_device_id arm_cci_pmu_matches[] = {
 	{},
 };
 
+static bool is_duplicate_irq(int irq, int *irqs, int nr_irqs)
+{
+	int i;
+
+	for (i = 0; i < nr_irqs; i++)
+		if (irq == irqs[i])
+			return true;
+
+	return false;
+}
+
 static int cci_pmu_probe(struct platform_device *pdev)
 {
 	struct resource *res;
@@ -963,8 +950,65 @@ static int cci_platform_probe(struct platform_device *pdev)
 	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
 }
 
+static struct platform_driver cci_pmu_driver = {
+	.driver = {
+		   .name = DRIVER_NAME_PMU,
+		   .of_match_table = arm_cci_pmu_matches,
+		  },
+	.probe = cci_pmu_probe,
+};
+
+static struct platform_driver cci_platform_driver = {
+	.driver = {
+		   .name = DRIVER_NAME,
+		   .of_match_table = arm_cci_matches,
+		  },
+	.probe = cci_platform_probe,
+};
+
+static int __init cci_platform_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&cci_pmu_driver);
+	if (ret)
+		return ret;
+
+	return platform_driver_register(&cci_platform_driver);
+}
+
+#else /* !CONFIG_HW_PERF_EVENTS */
+
+static int __init cci_platform_init(void)
+{
+	return 0;
+}
+
 #endif /* CONFIG_HW_PERF_EVENTS */
 
+#define CCI_PORT_CTRL		0x0
+#define CCI_CTRL_STATUS		0xc
+
+#define CCI_ENABLE_SNOOP_REQ	0x1
+#define CCI_ENABLE_DVM_REQ	0x2
+#define CCI_ENABLE_REQ		(CCI_ENABLE_SNOOP_REQ | CCI_ENABLE_DVM_REQ)
+
+enum cci_ace_port_type {
+	ACE_INVALID_PORT = 0x0,
+	ACE_PORT,
+	ACE_LITE_PORT,
+};
+
+struct cci_ace_port {
+	void __iomem *base;
+	unsigned long phys;
+	enum cci_ace_port_type type;
+	struct device_node *dn;
+};
+
+static struct cci_ace_port *ports;
+static unsigned int nb_cci_ports;
+
 struct cpu_port {
 	u64 mpidr;
 	u32 port;
@@ -1284,36 +1328,20 @@ int notrace __cci_control_port_by_index(u32 port, bool enable)
 }
 EXPORT_SYMBOL_GPL(__cci_control_port_by_index);
 
-static const struct cci_nb_ports cci400_ports = {
-	.nb_ace = 2,
-	.nb_ace_lite = 3
-};
-
-static const struct of_device_id arm_cci_matches[] = {
-	{.compatible = "arm,cci-400", .data = &cci400_ports },
-	{},
-};
-
 static const struct of_device_id arm_cci_ctrl_if_matches[] = {
 	{.compatible = "arm,cci-400-ctrl-if", },
 	{},
 };
 
-static int cci_probe(void)
+static int cci_probe_ports(struct device_node *np)
 {
 	struct cci_nb_ports const *cci_config;
 	int ret, i, nb_ace = 0, nb_ace_lite = 0;
-	struct device_node *np, *cp;
+	struct device_node *cp;
 	struct resource res;
 	const char *match_str;
 	bool is_ace;
 
-	np = of_find_matching_node(NULL, arm_cci_matches);
-	if (!np)
-		return -ENODEV;
-
-	if (!of_device_is_available(np))
-		return -ENODEV;
 
 	cci_config = of_match_node(arm_cci_matches, np)->data;
 	if (!cci_config)
@@ -1325,17 +1353,6 @@ static int cci_probe(void)
 	if (!ports)
 		return -ENOMEM;
 
-	ret = of_address_to_resource(np, 0, &res);
-	if (!ret) {
-		cci_ctrl_base = ioremap(res.start, resource_size(&res));
-		cci_ctrl_phys =	res.start;
-	}
-	if (ret || !cci_ctrl_base) {
-		WARN(1, "unable to ioremap CCI ctrl\n");
-		ret = -ENXIO;
-		goto memalloc_err;
-	}
-
 	for_each_child_of_node(np, cp) {
 		if (!of_match_node(arm_cci_ctrl_if_matches, cp))
 			continue;
@@ -1395,12 +1412,31 @@ static int cci_probe(void)
 	sync_cache_w(&cpu_port);
 	__sync_cache_range_w(ports, sizeof(*ports) * nb_cci_ports);
 	pr_info("ARM CCI driver probed\n");
+
 	return 0;
+}
 
-memalloc_err:
+static int cci_probe(void)
+{
+	int ret;
+	struct device_node *np;
+	struct resource res;
 
-	kfree(ports);
-	return ret;
+	np = of_find_matching_node(NULL, arm_cci_matches);
+	if(!np || !of_device_is_available(np))
+		return -ENODEV;
+
+	ret = of_address_to_resource(np, 0, &res);
+	if (!ret) {
+		cci_ctrl_base = ioremap(res.start, resource_size(&res));
+		cci_ctrl_phys =	res.start;
+	}
+	if (ret || !cci_ctrl_base) {
+		WARN(1, "unable to ioremap CCI ctrl\n");
+		return -ENXIO;
+	}
+
+	return cci_probe_ports(np);
 }
 
 static int cci_init_status = -EAGAIN;
@@ -1418,42 +1454,6 @@ static int cci_init(void)
 	return cci_init_status;
 }
 
-#ifdef CONFIG_HW_PERF_EVENTS
-static struct platform_driver cci_pmu_driver = {
-	.driver = {
-		   .name = DRIVER_NAME_PMU,
-		   .of_match_table = arm_cci_pmu_matches,
-		  },
-	.probe = cci_pmu_probe,
-};
-
-static struct platform_driver cci_platform_driver = {
-	.driver = {
-		   .name = DRIVER_NAME,
-		   .of_match_table = arm_cci_matches,
-		  },
-	.probe = cci_platform_probe,
-};
-
-static int __init cci_platform_init(void)
-{
-	int ret;
-
-	ret = platform_driver_register(&cci_pmu_driver);
-	if (ret)
-		return ret;
-
-	return platform_driver_register(&cci_platform_driver);
-}
-
-#else
-
-static int __init cci_platform_init(void)
-{
-	return 0;
-}
-
-#endif
 /*
  * To sort out early init calls ordering a helper function is provided to
  * check if the CCI driver has beed initialized. Function check if the driver
-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/5] arm-cci: Abstract the CCI400 PMU speicific definitions
  2015-03-10 15:18 [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Suzuki K. Poulose
  2015-03-10 15:18 ` [PATCH 1/5] arm-cci: Rearrange code for splitting PMU vs driver code Suzuki K. Poulose
@ 2015-03-10 15:18 ` Suzuki K. Poulose
  2015-03-17 18:49   ` Will Deacon
  2015-03-10 15:18 ` [PATCH 3/5] arm-cci: Get rid of secure transactions for PMU driver Suzuki K. Poulose
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Suzuki K. Poulose @ 2015-03-10 15:18 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Nicolas Pitre, Bartlomiej Zolnierkiewicz, Kukjin Kim,
	Abhilash Kesavan, Arnd Bergmann, devicetree, linux-kernel,
	Liviu Dudau, Lorenzo Pieralisi, Olof Johansson, Pawel Moll,
	Punit Agrawal, Sudeep Holla, Will Deacon, Catalin Marinas,
	Suzuki K. Poulose

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

CCI400 has different event specifications for PMU, for revsion
0 and revision 1. As of now, we check the revision every single
time before using the parameters for the PMU. This patch abstracts
the details of the pmu models in a struct (cci_pmu_model) and
stores the information in cci_pmu at initialisation time, avoiding
multiple probe operations.

Changes since V2:
 - Cleanup event validation(pmu_validate_hw_event). Get rid of
   helper functions:
	pmu_is_valid_slave_event
	pmu_is_valid_master_event

Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
---
 drivers/bus/arm-cci.c |  141 ++++++++++++++++++++++++++++---------------------
 1 file changed, 81 insertions(+), 60 deletions(-)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index ea39fc2..f88383e 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -79,19 +79,38 @@ static const struct of_device_id arm_cci_matches[] = {
 
 #define CCI_PMU_MAX_HW_EVENTS 5   /* CCI PMU has 4 counters + 1 cycle counter */
 
+/* Types of interfaces that can generate events */
+enum {
+	CCI_IF_SLAVE,
+	CCI_IF_MASTER,
+	CCI_IF_MAX,
+};
+
+struct event_range {
+	u32 min;
+	u32 max;
+};
+
 struct cci_pmu_hw_events {
 	struct perf_event *events[CCI_PMU_MAX_HW_EVENTS];
 	unsigned long used_mask[BITS_TO_LONGS(CCI_PMU_MAX_HW_EVENTS)];
 	raw_spinlock_t pmu_lock;
 };
 
+struct cci_pmu_model {
+	char *name;
+	struct event_range event_ranges[CCI_IF_MAX];
+};
+
+static struct cci_pmu_model cci_pmu_models[];
+
 struct cci_pmu {
 	void __iomem *base;
 	struct pmu pmu;
 	int nr_irqs;
 	int irqs[CCI_PMU_MAX_HW_EVENTS];
 	unsigned long active_irqs;
-	struct pmu_port_event_ranges *port_ranges;
+	const struct cci_pmu_model *model;
 	struct cci_pmu_hw_events hw_events;
 	struct platform_device *plat_device;
 	int num_events;
@@ -152,53 +171,11 @@ enum cci400_perf_events {
 #define CCI_REV_R1_MASTER_PORT_MIN_EV	0x00
 #define CCI_REV_R1_MASTER_PORT_MAX_EV	0x11
 
-struct pmu_port_event_ranges {
-	u8 slave_min;
-	u8 slave_max;
-	u8 master_min;
-	u8 master_max;
-};
-
-static struct pmu_port_event_ranges port_event_range[] = {
-	[CCI_REV_R0] = {
-		.slave_min = CCI_REV_R0_SLAVE_PORT_MIN_EV,
-		.slave_max = CCI_REV_R0_SLAVE_PORT_MAX_EV,
-		.master_min = CCI_REV_R0_MASTER_PORT_MIN_EV,
-		.master_max = CCI_REV_R0_MASTER_PORT_MAX_EV,
-	},
-	[CCI_REV_R1] = {
-		.slave_min = CCI_REV_R1_SLAVE_PORT_MIN_EV,
-		.slave_max = CCI_REV_R1_SLAVE_PORT_MAX_EV,
-		.master_min = CCI_REV_R1_MASTER_PORT_MIN_EV,
-		.master_max = CCI_REV_R1_MASTER_PORT_MAX_EV,
-	},
-};
-
-/*
- * Export different PMU names for the different revisions so userspace knows
- * because the event ids are different
- */
-static char *const pmu_names[] = {
-	[CCI_REV_R0] = "CCI_400",
-	[CCI_REV_R1] = "CCI_400_r1",
-};
-
-static int pmu_is_valid_slave_event(u8 ev_code)
-{
-	return pmu->port_ranges->slave_min <= ev_code &&
-		ev_code <= pmu->port_ranges->slave_max;
-}
-
-static int pmu_is_valid_master_event(u8 ev_code)
-{
-	return pmu->port_ranges->master_min <= ev_code &&
-		ev_code <= pmu->port_ranges->master_max;
-}
-
 static int pmu_validate_hw_event(u8 hw_event)
 {
 	u8 ev_source = CCI_PMU_EVENT_SOURCE(hw_event);
 	u8 ev_code = CCI_PMU_EVENT_CODE(hw_event);
+	int if_type;
 
 	switch (ev_source) {
 	case CCI_PORT_S0:
@@ -207,18 +184,22 @@ static int pmu_validate_hw_event(u8 hw_event)
 	case CCI_PORT_S3:
 	case CCI_PORT_S4:
 		/* Slave Interface */
-		if (pmu_is_valid_slave_event(ev_code))
-			return hw_event;
+		if_type = CCI_IF_SLAVE;
 		break;
 	case CCI_PORT_M0:
 	case CCI_PORT_M1:
 	case CCI_PORT_M2:
 		/* Master Interface */
-		if (pmu_is_valid_master_event(ev_code))
-			return hw_event;
+		if_type = CCI_IF_MASTER;
 		break;
+	default:
+		return -ENOENT;
 	}
 
+	if (ev_code >= pmu->model->event_ranges[if_type].min &&
+		ev_code <= pmu->model->event_ranges[if_type].max)
+		return hw_event;
+
 	return -ENOENT;
 }
 
@@ -234,11 +215,9 @@ static int probe_cci_revision(void)
 		return CCI_REV_R1;
 }
 
-static struct pmu_port_event_ranges *port_range_by_rev(void)
+static const struct cci_pmu_model *probe_cci_model(struct platform_device *pdev)
 {
-	int rev = probe_cci_revision();
-
-	return &port_event_range[rev];
+	return &cci_pmu_models[probe_cci_revision()];
 }
 
 static int pmu_is_valid_counter(struct cci_pmu *cci_pmu, int idx)
@@ -807,9 +786,9 @@ static const struct attribute_group *pmu_attr_groups[] = {
 
 static int cci_pmu_init(struct cci_pmu *cci_pmu, struct platform_device *pdev)
 {
-	char *name = pmu_names[probe_cci_revision()];
+	char *name = cci_pmu->model->name;
 	cci_pmu->pmu = (struct pmu) {
-		.name		= pmu_names[probe_cci_revision()],
+		.name		= cci_pmu->model->name,
 		.task_ctx_nr	= perf_invalid_context,
 		.pmu_enable	= cci_pmu_enable,
 		.pmu_disable	= cci_pmu_disable,
@@ -862,6 +841,35 @@ static struct notifier_block cci_pmu_cpu_nb = {
 	.priority	= CPU_PRI_PERF + 1,
 };
 
+static struct cci_pmu_model cci_pmu_models[] = {
+	[CCI_REV_R0] = {
+		.name = "CCI_400",
+		.event_ranges = {
+			[CCI_IF_SLAVE] = {
+				CCI_REV_R0_SLAVE_PORT_MIN_EV,
+				CCI_REV_R0_SLAVE_PORT_MAX_EV,
+			},
+			[CCI_IF_MASTER] = {
+				CCI_REV_R0_MASTER_PORT_MIN_EV,
+				CCI_REV_R0_MASTER_PORT_MAX_EV,
+			},
+		},
+	},
+	[CCI_REV_R1] = {
+		.name = "CCI_400_r1",
+		.event_ranges = {
+			[CCI_IF_SLAVE] = {
+				CCI_REV_R1_SLAVE_PORT_MIN_EV,
+				CCI_REV_R1_SLAVE_PORT_MAX_EV,
+			},
+			[CCI_IF_MASTER] = {
+				CCI_REV_R1_MASTER_PORT_MIN_EV,
+				CCI_REV_R1_MASTER_PORT_MAX_EV,
+			},
+		},
+	},
+};
+
 static const struct of_device_id arm_cci_pmu_matches[] = {
 	{
 		.compatible = "arm,cci-400-pmu",
@@ -869,6 +877,16 @@ static const struct of_device_id arm_cci_pmu_matches[] = {
 	{},
 };
 
+static inline const struct cci_pmu_model *get_cci_model(struct platform_device *pdev)
+{
+	const struct of_device_id *match = of_match_node(arm_cci_pmu_matches,
+							pdev->dev.of_node);
+	if (!match)
+		return NULL;
+
+	return probe_cci_model(pdev);
+}
+
 static bool is_duplicate_irq(int irq, int *irqs, int nr_irqs)
 {
 	int i;
@@ -884,11 +902,19 @@ static int cci_pmu_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	int i, ret, irq;
+	const struct cci_pmu_model *model;
+
+	model = get_cci_model(pdev);
+	if (!model) {
+		dev_warn(&pdev->dev, "CCI PMU version not supported\n");
+		return -ENODEV;
+	}
 
 	pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
 	if (!pmu)
 		return -ENOMEM;
 
+	pmu->model = model;
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	pmu->base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(pmu->base))
@@ -920,12 +946,6 @@ static int cci_pmu_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	pmu->port_ranges = port_range_by_rev();
-	if (!pmu->port_ranges) {
-		dev_warn(&pdev->dev, "CCI PMU version not supported\n");
-		return -EINVAL;
-	}
-
 	raw_spin_lock_init(&pmu->hw_events.pmu_lock);
 	mutex_init(&pmu->reserve_mutex);
 	atomic_set(&pmu->active_events, 0);
@@ -939,6 +959,7 @@ static int cci_pmu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	pr_info("ARM %s PMU driver probed", pmu->model->name);
 	return 0;
 }
 
-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 3/5] arm-cci: Get rid of secure transactions for PMU driver
  2015-03-10 15:18 [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Suzuki K. Poulose
  2015-03-10 15:18 ` [PATCH 1/5] arm-cci: Rearrange code for splitting PMU vs driver code Suzuki K. Poulose
  2015-03-10 15:18 ` [PATCH 2/5] arm-cci: Abstract the CCI400 PMU speicific definitions Suzuki K. Poulose
@ 2015-03-10 15:18 ` Suzuki K. Poulose
  2015-03-17  9:51   ` [UPDATED] " Suzuki K. Poulose
  2015-03-10 15:18 ` [PATCH 4/5] arm-cci: Split the code for PMU vs driver support Suzuki K. Poulose
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Suzuki K. Poulose @ 2015-03-10 15:18 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Nicolas Pitre, Bartlomiej Zolnierkiewicz, Kukjin Kim,
	Abhilash Kesavan, Arnd Bergmann, devicetree, linux-kernel,
	Liviu Dudau, Lorenzo Pieralisi, Olof Johansson, Pawel Moll,
	Punit Agrawal, Sudeep Holla, Will Deacon, Catalin Marinas,
	Suzuki K. Poulose

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

Avoid secure transactions while probing the CCI PMU. The
existing code makes use of the Peripheral ID2 (PID2) register
to determine the revision of the CCI400, which requires a
secure transaction. This puts a limitation on the usage of the
driver on systems running non-secure Linux(e.g, ARM64).

Updated the device-tree binding for cci pmu node to add the explicit
revision number for the compatible field.

The supported strings are :
	arm,cci-400-pmu,r0
	arm,cci-400-pmu,r1
	arm,cci-400-pmu - DEPRECATED. See NOTE below

NOTE: If the revision is not mentioned, we need to probe the cci revision,
which could be fatal on a platform running non-secure. We need a reliable way
to know if we can poke the CCI registers at runtime on ARM32. We depend on
'mcpm_is_available()' when it is available. mcpm_is_available() returns true
only when there is a registered driver for mcpm. Otherwise, we assume that we
don't have secure access, and skips probing the revision number(ARM64 case).

The MCPM should figure out if it is safe to access the CCI. Unfortunately
there isn't a reliable way to indicate the same via dtb. This patch doesn't
address/change the current situation. It only deals with the CCI-PMU, leaving
the assumptions about the secure access as it has been, prior to this patch.

Changes since V2:
 - Use 'bool' instead of 'int' for platform_has_secure_cci_access().
   (Suggested-by: Sudeep Holla)

Cc: devicetree@vger.kernel.org
Cc: Punit Agrawal <punit.agrawal@arm.com>
Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
---
 Documentation/devicetree/bindings/arm/cci.txt |    7 +++--
 arch/arm/include/asm/arm-cci.h                |   42 +++++++++++++++++++++++++
 arch/arm64/include/asm/arm-cci.h              |   27 ++++++++++++++++
 drivers/bus/arm-cci.c                         |   17 +++++++++-
 include/linux/arm-cci.h                       |    2 ++
 5 files changed, 92 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/include/asm/arm-cci.h
 create mode 100644 arch/arm64/include/asm/arm-cci.h

diff --git a/Documentation/devicetree/bindings/arm/cci.txt b/Documentation/devicetree/bindings/arm/cci.txt
index f28d82b..0e4b6a7 100644
--- a/Documentation/devicetree/bindings/arm/cci.txt
+++ b/Documentation/devicetree/bindings/arm/cci.txt
@@ -94,8 +94,11 @@ specific to ARM.
 		- compatible
 			Usage: required
 			Value type: <string>
-			Definition: must be "arm,cci-400-pmu"
-
+			Definition: Supported strings are :
+				 "arm,cci-400-pmu,r0"
+				 "arm,cci-400-pmu,r1"
+				 "arm,cci-400-pmu"  - DEPRECATED, permitted only where OS has
+						      secure acces to CCI registers
 		- reg:
 			Usage: required
 			Value type: Integer cells. A register entry, expressed
diff --git a/arch/arm/include/asm/arm-cci.h b/arch/arm/include/asm/arm-cci.h
new file mode 100644
index 0000000..fe77f7a
--- /dev/null
+++ b/arch/arm/include/asm/arm-cci.h
@@ -0,0 +1,42 @@
+/*
+ * arch/arm/include/asm/arm-cci.h
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __ASM_ARM_CCI_H
+#define __ASM_ARM_CCI_H
+
+#ifdef CONFIG_MCPM
+#include <asm/mcpm.h>
+
+/*
+ * We don't have a reliable way of detecting whether,
+ * if we have access to secure-only registers, unless
+ * mcpm is registered.
+ */
+static inline bool platform_has_secure_cci_access(void)
+{
+	return mcpm_is_available();
+}
+
+#else
+static inline bool platform_has_secure_cci_access(void)
+{
+	return false;
+}
+#endif
+
+#endif
diff --git a/arch/arm64/include/asm/arm-cci.h b/arch/arm64/include/asm/arm-cci.h
new file mode 100644
index 0000000..f0b6371
--- /dev/null
+++ b/arch/arm64/include/asm/arm-cci.h
@@ -0,0 +1,27 @@
+/*
+ * arch/arm64/include/asm/arm-cci.h
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __ASM_ARM_CCI_H
+#define __ASM_ARM_CCI_H
+
+static inline bool platform_has_secure_cci_access(void)
+{
+	return false;
+}
+
+#endif
diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index f88383e..70dff09 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -217,7 +217,9 @@ static int probe_cci_revision(void)
 
 static const struct cci_pmu_model *probe_cci_model(struct platform_device *pdev)
 {
-	return &cci_pmu_models[probe_cci_revision()];
+	if (platform_has_secure_cci_access())
+		return &cci_pmu_models[probe_cci_revision()];
+	return NULL;
 }
 
 static int pmu_is_valid_counter(struct cci_pmu *cci_pmu, int idx)
@@ -873,6 +875,15 @@ static struct cci_pmu_model cci_pmu_models[] = {
 static const struct of_device_id arm_cci_pmu_matches[] = {
 	{
 		.compatible = "arm,cci-400-pmu",
+		.data	= NULL,
+	},
+	{
+		.compatible = "arm,cci-400-pmu,r0",
+		.data	= &cci_pmu_models[CCI_REV_R0],
+	},
+	{
+		.compatible = "arm,cci-400-pmu,r1",
+		.data	= &cci_pmu_models[CCI_REV_R1],
 	},
 	{},
 };
@@ -883,7 +894,11 @@ static inline const struct cci_pmu_model *get_cci_model(struct platform_device *
 							pdev->dev.of_node);
 	if (!match)
 		return NULL;
+	if (match->data)
+		return match->data;
 
+	dev_warn(&pdev->dev, "DEPERCATED compatible property,"
+			 "requires secure access to CCI registers");
 	return probe_cci_model(pdev);
 }
 
diff --git a/include/linux/arm-cci.h b/include/linux/arm-cci.h
index 79d6edf..aede5c7 100644
--- a/include/linux/arm-cci.h
+++ b/include/linux/arm-cci.h
@@ -24,6 +24,8 @@
 #include <linux/errno.h>
 #include <linux/types.h>
 
+#include <asm/arm-cci.h>
+
 struct device_node;
 
 #ifdef CONFIG_ARM_CCI
-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 4/5] arm-cci: Split the code for PMU vs driver support
  2015-03-10 15:18 [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Suzuki K. Poulose
                   ` (2 preceding siblings ...)
  2015-03-10 15:18 ` [PATCH 3/5] arm-cci: Get rid of secure transactions for PMU driver Suzuki K. Poulose
@ 2015-03-10 15:18 ` Suzuki K. Poulose
  2015-03-10 16:24   ` Sudeep Holla
  2015-03-10 15:18 ` [PATCH 5/5] arm-cci: Fix CCI PMU event validation Suzuki K. Poulose
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Suzuki K. Poulose @ 2015-03-10 15:18 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Nicolas Pitre, Bartlomiej Zolnierkiewicz, Kukjin Kim,
	Abhilash Kesavan, Arnd Bergmann, devicetree, linux-kernel,
	Liviu Dudau, Lorenzo Pieralisi, Olof Johansson, Pawel Moll,
	Punit Agrawal, Sudeep Holla, Will Deacon, Catalin Marinas,
	Suzuki K. Poulose, Nicolas Pitre

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

This patch separates the PMU driver code from the low level
CCI driver code and enables the PMU driver for ARM64.

Introduces config options for both.

 ARM_CCI400_PORT_CTRL	- controls the low level driver code for
			  CCI400 ports.
 ARM_CCI400_PMU 	- controls the PMU driver code
 ARM_CCI400_COMMON 	- Common defintions for CCI400

This patch also changes:
 ARM_CCI - common code for probing the CCI devices. This can be
   used for adding support for newer CCI versions(e.g, CCI-500).

Changes since V2:
  - Make ARM_CCI400_PMU default y (Suggested-by: Sudeep Holla)
Changes since V1 (Suggestions-by: Nicolas Pitre):
  - Renames
	CONFIG_ARM_CCI400_MCPM => CONFIG_ARM_CCI400_PORT_CTRL
	CCI400_MCPM_PORTS_DATA => CCI400_PORTS_DATA
  - Select ARM_CCI400_COMMON for ARM_CCI400_PORT_CTRL
  - Better documentation in the git commit log about the ARM_CCI config.

Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Kukjin Kim <kgene@kernel.org>
Cc: Abhilash Kesavan <a.kesavan@samsung.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
Cc: Punit Agrawal <punit.agrawal@arm.com>
Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
---
 arch/arm/mach-exynos/Kconfig   |    2 +-
 arch/arm/mach-vexpress/Kconfig |    4 ++--
 drivers/bus/Kconfig            |   28 ++++++++++++++++++++++++----
 drivers/bus/arm-cci.c          |   24 ++++++++++++++++++++----
 include/linux/arm-cci.h        |    7 ++++++-
 5 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index 603820e..81064cd 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -123,7 +123,7 @@ config SOC_EXYNOS5800
 config EXYNOS5420_MCPM
 	bool "Exynos5420 Multi-Cluster PM support"
 	depends on MCPM && SOC_EXYNOS5420
-	select ARM_CCI
+	select ARM_CCI400_PORT_CTRL
 	select ARM_CPU_SUSPEND
 	help
 	  This is needed to provide CPU and cluster power management
diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
index 3c2509b..daa7ab6 100644
--- a/arch/arm/mach-vexpress/Kconfig
+++ b/arch/arm/mach-vexpress/Kconfig
@@ -53,7 +53,7 @@ config ARCH_VEXPRESS_CORTEX_A5_A9_ERRATA
 config ARCH_VEXPRESS_DCSCB
 	bool "Dual Cluster System Control Block (DCSCB) support"
 	depends on MCPM
-	select ARM_CCI
+	select ARM_CCI400_PORT_CTRL
 	help
 	  Support for the Dual Cluster System Configuration Block (DCSCB).
 	  This is needed to provide CPU and cluster power management
@@ -71,7 +71,7 @@ config ARCH_VEXPRESS_SPC
 config ARCH_VEXPRESS_TC2_PM
 	bool "Versatile Express TC2 power management"
 	depends on MCPM
-	select ARM_CCI
+	select ARM_CCI400_PORT_CTRL
 	select ARCH_VEXPRESS_SPC
 	select ARM_CPU_SUSPEND
 	help
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index b99729e..79e297b 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -43,12 +43,32 @@ config OMAP_INTERCONNECT
 	help
 	  Driver to enable OMAP interconnect error handling driver.
 
-config ARM_CCI
-	bool "ARM CCI driver support"
+config ARM_CCI400_PORT_CTRL
+	bool
 	depends on ARM && OF && CPU_V7
+	select ARM_CCI400_COMMON
+	help
+	  Low level power management driver for CCI400 cache coherent
+	  interconnect for ARM platforms.
+
+config ARM_CCI400_PMU
+	bool "ARM CCI400 PMU support"
+	default y
+	depends on ARM || ARM64
+	depends on HW_PERF_EVENTS
+	select ARM_CCI400_COMMON
 	help
-	  Driver supporting the CCI cache coherent interconnect for ARM
-	  platforms.
+	  Support for PMU events monitoring on the ARM CCI cache coherent
+	  interconnect.
+
+	  If unsure, say Y
+
+config ARM_CCI400_COMMON
+	bool
+	select ARM_CCI
+
+config ARM_CCI
+	bool
 
 config ARM_CCN
 	bool "ARM CCN driver support"
diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index 70dff09..581190d 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -32,6 +32,7 @@
 static void __iomem *cci_ctrl_base;
 static unsigned long cci_ctrl_phys;
 
+#ifdef CONFIG_ARM_CCI400_PORT_CTRL
 struct cci_nb_ports {
 	unsigned int nb_ace;
 	unsigned int nb_ace_lite;
@@ -42,12 +43,19 @@ static const struct cci_nb_ports cci400_ports = {
 	.nb_ace_lite = 3
 };
 
+#define CCI400_PORTS_DATA	(&cci400_ports)
+#else
+#define CCI400_PORTS_DATA	(NULL)
+#endif
+
 static const struct of_device_id arm_cci_matches[] = {
-	{.compatible = "arm,cci-400", .data = &cci400_ports },
+#ifdef CONFIG_ARM_CCI400_COMMON
+	{.compatible = "arm,cci-400", .data = CCI400_PORTS_DATA },
+#endif
 	{},
 };
 
-#ifdef CONFIG_HW_PERF_EVENTS
+#ifdef CONFIG_ARM_CCI400_PMU
 
 #define DRIVER_NAME		"CCI-400"
 #define DRIVER_NAME_PMU		DRIVER_NAME " PMU"
@@ -1013,14 +1021,16 @@ static int __init cci_platform_init(void)
 	return platform_driver_register(&cci_platform_driver);
 }
 
-#else /* !CONFIG_HW_PERF_EVENTS */
+#else /* !CONFIG_ARM_CCI400_PMU */
 
 static int __init cci_platform_init(void)
 {
 	return 0;
 }
 
-#endif /* CONFIG_HW_PERF_EVENTS */
+#endif /* CONFIG_ARM_CCI400_PMU */
+
+#ifdef CONFIG_ARM_CCI400_PORT_CTRL
 
 #define CCI_PORT_CTRL		0x0
 #define CCI_CTRL_STATUS		0xc
@@ -1451,6 +1461,12 @@ static int cci_probe_ports(struct device_node *np)
 
 	return 0;
 }
+#else /* !CONFIG_ARM_CCI400_PORT_CTRL */
+static inline int cci_probe_ports(struct device_node *np)
+{
+	return 0;
+}
+#endif /* CONFIG_ARM_CCI400_PORT_CTRL */
 
 static int cci_probe(void)
 {
diff --git a/include/linux/arm-cci.h b/include/linux/arm-cci.h
index aede5c7..521ec1f 100644
--- a/include/linux/arm-cci.h
+++ b/include/linux/arm-cci.h
@@ -30,12 +30,16 @@ struct device_node;
 
 #ifdef CONFIG_ARM_CCI
 extern bool cci_probed(void);
+#else
+static inline bool cci_probed(void) { return false; }
+#endif
+
+#ifdef CONFIG_ARM_CCI400_PORT_CTRL
 extern int cci_ace_get_port(struct device_node *dn);
 extern int cci_disable_port_by_cpu(u64 mpidr);
 extern int __cci_control_port_by_device(struct device_node *dn, bool enable);
 extern int __cci_control_port_by_index(u32 port, bool enable);
 #else
-static inline bool cci_probed(void) { return false; }
 static inline int cci_ace_get_port(struct device_node *dn)
 {
 	return -ENODEV;
@@ -51,6 +55,7 @@ static inline int __cci_control_port_by_index(u32 port, bool enable)
 	return -ENODEV;
 }
 #endif
+
 #define cci_disable_port_by_device(dev) \
 	__cci_control_port_by_device(dev, false)
 #define cci_enable_port_by_device(dev) \
-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 5/5] arm-cci: Fix CCI PMU event validation
  2015-03-10 15:18 [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Suzuki K. Poulose
                   ` (3 preceding siblings ...)
  2015-03-10 15:18 ` [PATCH 4/5] arm-cci: Split the code for PMU vs driver support Suzuki K. Poulose
@ 2015-03-10 15:18 ` Suzuki K. Poulose
  2015-03-17 18:52   ` Will Deacon
  2015-03-10 16:09 ` [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Nicolas Pitre
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Suzuki K. Poulose @ 2015-03-10 15:18 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Nicolas Pitre, Bartlomiej Zolnierkiewicz, Kukjin Kim,
	Abhilash Kesavan, Arnd Bergmann, devicetree, linux-kernel,
	Liviu Dudau, Lorenzo Pieralisi, Olof Johansson, Pawel Moll,
	Punit Agrawal, Sudeep Holla, Will Deacon, Catalin Marinas,
	Suzuki K. Poulose

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

We mask the event with the CCI_PMU_EVENT_MASK, before passing
the config to pmu_validate_hw_event(), which causes extra bits
to be ignored and qualifies an invalid event code as valid.

e.g,
 $ perf stat -a -C 0 -e CCI_400/config=0x1ff,name=cycles/ sleep 1
   Performance counter stats for 'system wide':

         506951142      cycles

       1.013879626 seconds time elapsed

where, cycles has an event coding of 0xff. This patch also removes
the unnecessary 'event' mask in pmu_write_register, since the config_base
is set by the pmu code after the event is validated.

Changes since V2:
 - Switch to input unsigned long for pmu_validate_hw_event()

Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
---
 drivers/bus/arm-cci.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index 581190d..89c86e9 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -179,12 +179,15 @@ enum cci400_perf_events {
 #define CCI_REV_R1_MASTER_PORT_MIN_EV	0x00
 #define CCI_REV_R1_MASTER_PORT_MAX_EV	0x11
 
-static int pmu_validate_hw_event(u8 hw_event)
+static int pmu_validate_hw_event(unsigned long hw_event)
 {
 	u8 ev_source = CCI_PMU_EVENT_SOURCE(hw_event);
 	u8 ev_code = CCI_PMU_EVENT_CODE(hw_event);
 	int if_type;
 
+	if (hw_event & ~CCI_PMU_EVENT_MASK)
+		return -ENOENT;
+
 	switch (ev_source) {
 	case CCI_PORT_S0:
 	case CCI_PORT_S1:
@@ -258,7 +261,6 @@ static void pmu_enable_counter(int idx)
 
 static void pmu_set_event(int idx, unsigned long event)
 {
-	event &= CCI_PMU_EVENT_MASK;
 	pmu_write_register(event, idx, CCI_PMU_EVT_SEL);
 }
 
@@ -275,7 +277,7 @@ static int pmu_get_event_idx(struct cci_pmu_hw_events *hw, struct perf_event *ev
 {
 	struct cci_pmu *cci_pmu = to_cci_pmu(event->pmu);
 	struct hw_perf_event *hw_event = &event->hw;
-	unsigned long cci_event = hw_event->config_base & CCI_PMU_EVENT_MASK;
+	unsigned long cci_event = hw_event->config_base;
 	int idx;
 
 	if (cci_event == CCI_PMU_CYCLES) {
@@ -296,7 +298,7 @@ static int pmu_get_event_idx(struct cci_pmu_hw_events *hw, struct perf_event *ev
 static int pmu_map_event(struct perf_event *event)
 {
 	int mapping;
-	u8 config = event->attr.config & CCI_PMU_EVENT_MASK;
+	unsigned long config = event->attr.config;
 
 	if (event->attr.type < PERF_TYPE_MAX)
 		return -ENOENT;
-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64
  2015-03-10 15:18 [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Suzuki K. Poulose
                   ` (4 preceding siblings ...)
  2015-03-10 15:18 ` [PATCH 5/5] arm-cci: Fix CCI PMU event validation Suzuki K. Poulose
@ 2015-03-10 16:09 ` Nicolas Pitre
  2015-03-10 16:11   ` Suzuki K. Poulose
  2015-03-10 16:21 ` Sudeep Holla
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Nicolas Pitre @ 2015-03-10 16:09 UTC (permalink / raw)
  To: Suzuki K. Poulose
  Cc: linux-arm-kernel, Bartlomiej Zolnierkiewicz, Kukjin Kim,
	Abhilash Kesavan, Arnd Bergmann, devicetree, linux-kernel,
	Liviu Dudau, Lorenzo Pieralisi, Olof Johansson, Pawel Moll,
	Punit Agrawal, Sudeep Holla, Will Deacon, Catalin Marinas

On Tue, 10 Mar 2015, Suzuki K. Poulose wrote:

> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> 
> This series enables the PMU monitoring support for CCI400 on ARM64.
> The existing CCI400 driver code is a mix of PMU driver and the MCPM
> driver code. The MCPM driver is only used on ARM(32) and contains
> arm32 assembly and hence can't be built on ARM64. This patch splits
> the code to
> 
>  - ARM_CCI400_PORT_CTRL driver - depends on ARM && V7
>  - ARM_CCI400_PMU driver
> 
> Accessing the Peripheral ID2 register(PID2) on CCI-400, to detect
> the revision of the chipset, is a secure operation. Hence, it prevents
> us from running this on non-secure platforms. The issue is overcome by
> explicitly mentioning the revision number of the CCI PMU in the device tree
> binding. The device-tree binding has been updated with the new bindings.
> 
> i.e,	arm-cci-400-pmu,r0 => revision 0
> 	arm-cci-400-pmu,r1 => revision 1
> 	arm-cci-400-pmu => (old) DEPRECATED
> 
> The old binding has been DEPRECATED and must be used only on ARM32
> system with secure access. We don't have a reliable dynamic way to detect
> if the system is running secure. This series tries to use the best safe
> method by relying on the availability of MCPM(as it was prior to the series).
> It is upto the MCPM platform driver to decide, if the system is secure before
> it goes ahead and registers its drivers and pokes the CCI. This series doesn't
> address/solve the problem of MCPM. I will be happy to use a better approach,
> if there is any.
> 
> Tested on (non-secure)TC2 and A53x2.

Would be nice if you could also test it on secure TC2 making sure MCPM 
is still functional.

For patches 1, 3 and 4, you may add:

Acked-by: Nicolas Pitre <nico@linaro.org>

Patches 2 and 5 are purely PMU stuff and out of my area of expertise.


> Changes sinve V2
> 
>  - Include suggestions from Sudeep Holla.
>  - Cleanup event validation checks.
> 
> Changes since V1 (Suggestions from Nicolas Pitre):
> 
>  - Split Patch 2 to separate the 'PMU' abstraction(now Patch 2/5)
>    from the introduction of a new device-tree binding(now Patch 3/5)
>  - Rename
> 	ARM_CCI400_MCPM => ARM_CCI400_PORT_CTRL
> 	CCI400_MCPM_PORTS_DATA => CCI400_PORTS_DATA
>  - Select ARM_CCI400_COMMON for ARM_CCI400_PORT_CTRL
>  - Better documentation in the git commit log about the ARM_CCI config.
>  - Move the 'pr_info' to its apporpriate patch.
> 
> 
> Suzuki K. Poulose (5):
>   arm-cci: Rearrange code for splitting PMU vs driver code
>   arm-cci: Abstract the CCI400 PMU speicific definitions
>   arm-cci: Get rid of secure transactions for PMU driver
>   arm-cci: Split the code for PMU vs driver support
>   arm-cci: Fix CCI PMU event validation
> 
>  Documentation/devicetree/bindings/arm/cci.txt |    7 +-
>  arch/arm/include/asm/arm-cci.h                |   42 +++
>  arch/arm/mach-exynos/Kconfig                  |    2 +-
>  arch/arm/mach-vexpress/Kconfig                |    4 +-
>  arch/arm64/include/asm/arm-cci.h              |   27 ++
>  drivers/bus/Kconfig                           |   28 +-
>  drivers/bus/arm-cci.c                         |  496 ++++++++++++++-----------
>  include/linux/arm-cci.h                       |    9 +-
>  8 files changed, 384 insertions(+), 231 deletions(-)
>  create mode 100644 arch/arm/include/asm/arm-cci.h
>  create mode 100644 arch/arm64/include/asm/arm-cci.h
> 
> -- 
> 1.7.9.5
> 
> 
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64
  2015-03-10 16:09 ` [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Nicolas Pitre
@ 2015-03-10 16:11   ` Suzuki K. Poulose
  0 siblings, 0 replies; 24+ messages in thread
From: Suzuki K. Poulose @ 2015-03-10 16:11 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: linux-arm-kernel, Bartlomiej Zolnierkiewicz, Kukjin Kim,
	Abhilash Kesavan, Arnd Bergmann, devicetree, linux-kernel,
	Liviu Dudau, Lorenzo Pieralisi, Olof Johansson, Pawel Moll,
	Punit Agrawal, Sudeep Holla, Will Deacon, Catalin Marinas

On 10/03/15 16:09, Nicolas Pitre wrote:
> On Tue, 10 Mar 2015, Suzuki K. Poulose wrote:
>
>> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>>
>> This series enables the PMU monitoring support for CCI400 on ARM64.
>> The existing CCI400 driver code is a mix of PMU driver and the MCPM
>> driver code. The MCPM driver is only used on ARM(32) and contains
>> arm32 assembly and hence can't be built on ARM64. This patch splits
>> the code to
>>
>>   - ARM_CCI400_PORT_CTRL driver - depends on ARM && V7
>>   - ARM_CCI400_PMU driver
>>
>> Accessing the Peripheral ID2 register(PID2) on CCI-400, to detect
>> the revision of the chipset, is a secure operation. Hence, it prevents
>> us from running this on non-secure platforms. The issue is overcome by
>> explicitly mentioning the revision number of the CCI PMU in the device tree
>> binding. The device-tree binding has been updated with the new bindings.
>>
>> i.e,	arm-cci-400-pmu,r0 => revision 0
>> 	arm-cci-400-pmu,r1 => revision 1
>> 	arm-cci-400-pmu => (old) DEPRECATED
>>
>> The old binding has been DEPRECATED and must be used only on ARM32
>> system with secure access. We don't have a reliable dynamic way to detect
>> if the system is running secure. This series tries to use the best safe
>> method by relying on the availability of MCPM(as it was prior to the series).
>> It is upto the MCPM platform driver to decide, if the system is secure before
>> it goes ahead and registers its drivers and pokes the CCI. This series doesn't
>> address/solve the problem of MCPM. I will be happy to use a better approach,
>> if there is any.
>>
>> Tested on (non-secure)TC2 and A53x2.
>
> Would be nice if you could also test it on secure TC2 making sure MCPM
> is still functional.

Sudeep is testing those bits.
>
> For patches 1, 3 and 4, you may add:
>
> Acked-by: Nicolas Pitre <nico@linaro.org>
>
> Patches 2 and 5 are purely PMU stuff and out of my area of expertise.
>

Thanks
Suzuki



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64
  2015-03-10 15:18 [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Suzuki K. Poulose
                   ` (5 preceding siblings ...)
  2015-03-10 16:09 ` [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Nicolas Pitre
@ 2015-03-10 16:21 ` Sudeep Holla
  2015-03-10 16:24   ` Suzuki K. Poulose
  2015-03-11 11:40 ` Punit Agrawal
  2015-03-17 18:54 ` Will Deacon
  8 siblings, 1 reply; 24+ messages in thread
From: Sudeep Holla @ 2015-03-10 16:21 UTC (permalink / raw)
  To: Suzuki K. Poulose, linux-arm-kernel
  Cc: Sudeep Holla, Nicolas Pitre, Bartlomiej Zolnierkiewicz,
	Kukjin Kim, Abhilash Kesavan, Arnd Bergmann, devicetree,
	linux-kernel, Liviu Dudau, Lorenzo Pieralisi, Olof Johansson,
	Pawel Moll, Punit Agrawal, Will Deacon, Catalin Marinas



On 10/03/15 15:18, Suzuki K. Poulose wrote:
> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>
> This series enables the PMU monitoring support for CCI400 on ARM64.
> The existing CCI400 driver code is a mix of PMU driver and the MCPM
> driver code. The MCPM driver is only used on ARM(32) and contains
> arm32 assembly and hence can't be built on ARM64. This patch splits
> the code to
>
>   - ARM_CCI400_PORT_CTRL driver - depends on ARM && V7
>   - ARM_CCI400_PMU driver
>
> Accessing the Peripheral ID2 register(PID2) on CCI-400, to detect
> the revision of the chipset, is a secure operation. Hence, it prevents
> us from running this on non-secure platforms. The issue is overcome by
> explicitly mentioning the revision number of the CCI PMU in the device tree
> binding. The device-tree binding has been updated with the new bindings.
>
> i.e,	arm-cci-400-pmu,r0 => revision 0
> 	arm-cci-400-pmu,r1 => revision 1
> 	arm-cci-400-pmu => (old) DEPRECATED
>
> The old binding has been DEPRECATED and must be used only on ARM32
> system with secure access. We don't have a reliable dynamic way to detect
> if the system is running secure. This series tries to use the best safe
> method by relying on the availability of MCPM(as it was prior to the series).
> It is upto the MCPM platform driver to decide, if the system is secure before
> it goes ahead and registers its drivers and pokes the CCI. This series doesn't
> address/solve the problem of MCPM. I will be happy to use a better approach,
> if there is any.
>
> Tested on (non-secure)TC2 and A53x2.
>

For the series,
Tested-by: Sudeep Holla <sudeep.holla@arm.com>
(Tested on secure TC2 using MCPM)

Regards,
Sudeep


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64
  2015-03-10 16:21 ` Sudeep Holla
@ 2015-03-10 16:24   ` Suzuki K. Poulose
  0 siblings, 0 replies; 24+ messages in thread
From: Suzuki K. Poulose @ 2015-03-10 16:24 UTC (permalink / raw)
  To: Sudeep Holla, linux-arm-kernel
  Cc: Nicolas Pitre, Bartlomiej Zolnierkiewicz, Kukjin Kim,
	Abhilash Kesavan, Arnd Bergmann, devicetree, linux-kernel,
	Liviu Dudau, Lorenzo Pieralisi, Olof Johansson, Pawel Moll,
	Punit Agrawal, Will Deacon, Catalin Marinas

On 10/03/15 16:21, Sudeep Holla wrote:
>
>
> On 10/03/15 15:18, Suzuki K. Poulose wrote:
>> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>>
>> This series enables the PMU monitoring support for CCI400 on ARM64.
>> The existing CCI400 driver code is a mix of PMU driver and the MCPM
>> driver code. The MCPM driver is only used on ARM(32) and contains
>> arm32 assembly and hence can't be built on ARM64. This patch splits
>> the code to
>>
>>    - ARM_CCI400_PORT_CTRL driver - depends on ARM && V7
>>    - ARM_CCI400_PMU driver
>>
>> Accessing the Peripheral ID2 register(PID2) on CCI-400, to detect
>> the revision of the chipset, is a secure operation. Hence, it prevents
>> us from running this on non-secure platforms. The issue is overcome by
>> explicitly mentioning the revision number of the CCI PMU in the device tree
>> binding. The device-tree binding has been updated with the new bindings.
>>
>> i.e,	arm-cci-400-pmu,r0 => revision 0
>> 	arm-cci-400-pmu,r1 => revision 1
>> 	arm-cci-400-pmu => (old) DEPRECATED
>>
>> The old binding has been DEPRECATED and must be used only on ARM32
>> system with secure access. We don't have a reliable dynamic way to detect
>> if the system is running secure. This series tries to use the best safe
>> method by relying on the availability of MCPM(as it was prior to the series).
>> It is upto the MCPM platform driver to decide, if the system is secure before
>> it goes ahead and registers its drivers and pokes the CCI. This series doesn't
>> address/solve the problem of MCPM. I will be happy to use a better approach,
>> if there is any.
>>
>> Tested on (non-secure)TC2 and A53x2.
>>
>
> For the series,
> Tested-by: Sudeep Holla <sudeep.holla@arm.com>
> (Tested on secure TC2 using MCPM)
>
Thank you very much for the testing

Suzuki


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/5] arm-cci: Split the code for PMU vs driver support
  2015-03-10 15:18 ` [PATCH 4/5] arm-cci: Split the code for PMU vs driver support Suzuki K. Poulose
@ 2015-03-10 16:24   ` Sudeep Holla
  0 siblings, 0 replies; 24+ messages in thread
From: Sudeep Holla @ 2015-03-10 16:24 UTC (permalink / raw)
  To: Suzuki K. Poulose, linux-arm-kernel
  Cc: Sudeep Holla, Nicolas Pitre, Bartlomiej Zolnierkiewicz,
	Kukjin Kim, Abhilash Kesavan, Arnd Bergmann, devicetree,
	linux-kernel, Liviu Dudau, Lorenzo Pieralisi, Olof Johansson,
	Pawel Moll, Punit Agrawal, Will Deacon, Catalin Marinas,
	nicolas.pitre



On 10/03/15 15:18, Suzuki K. Poulose wrote:
> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>
> This patch separates the PMU driver code from the low level
> CCI driver code and enables the PMU driver for ARM64.
>
> Introduces config options for both.
>
>   ARM_CCI400_PORT_CTRL	- controls the low level driver code for
> 			  CCI400 ports.
>   ARM_CCI400_PMU 	- controls the PMU driver code
>   ARM_CCI400_COMMON 	- Common defintions for CCI400
>
> This patch also changes:
>   ARM_CCI - common code for probing the CCI devices. This can be
>     used for adding support for newer CCI versions(e.g, CCI-500).
>
> Changes since V2:
>    - Make ARM_CCI400_PMU default y (Suggested-by: Sudeep Holla)
> Changes since V1 (Suggestions-by: Nicolas Pitre):
>    - Renames
> 	CONFIG_ARM_CCI400_MCPM => CONFIG_ARM_CCI400_PORT_CTRL
> 	CCI400_MCPM_PORTS_DATA => CCI400_PORTS_DATA
>    - Select ARM_CCI400_COMMON for ARM_CCI400_PORT_CTRL
>    - Better documentation in the git commit log about the ARM_CCI config.
>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Cc: Kukjin Kim <kgene@kernel.org>
> Cc: Abhilash Kesavan <a.kesavan@samsung.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>

For vexpress changes,
Acked-by: Sudeep Holla <sudeep.holla@arm.com>

Regards,
Sudeep


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64
  2015-03-10 15:18 [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Suzuki K. Poulose
                   ` (6 preceding siblings ...)
  2015-03-10 16:21 ` Sudeep Holla
@ 2015-03-11 11:40 ` Punit Agrawal
  2015-03-17 18:54 ` Will Deacon
  8 siblings, 0 replies; 24+ messages in thread
From: Punit Agrawal @ 2015-03-11 11:40 UTC (permalink / raw)
  To: Suzuki K. Poulose
  Cc: linux-arm-kernel, Nicolas Pitre, Bartlomiej Zolnierkiewicz,
	Kukjin Kim, Abhilash Kesavan, Arnd Bergmann, devicetree,
	linux-kernel, Liviu Dudau, Lorenzo Pieralisi, Olof Johansson,
	Pawel Moll, Sudeep Holla, Will Deacon, Catalin Marinas

"Suzuki K. Poulose" <suzuki.poulose@arm.com> writes:

> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>
> This series enables the PMU monitoring support for CCI400 on ARM64.
> The existing CCI400 driver code is a mix of PMU driver and the MCPM
> driver code. The MCPM driver is only used on ARM(32) and contains
> arm32 assembly and hence can't be built on ARM64. This patch splits
> the code to
>
>  - ARM_CCI400_PORT_CTRL driver - depends on ARM && V7
>  - ARM_CCI400_PMU driver
>
> Accessing the Peripheral ID2 register(PID2) on CCI-400, to detect
> the revision of the chipset, is a secure operation. Hence, it prevents
> us from running this on non-secure platforms. The issue is overcome by
> explicitly mentioning the revision number of the CCI PMU in the device tree
> binding. The device-tree binding has been updated with the new bindings.
>
> i.e,	arm-cci-400-pmu,r0 => revision 0
> 	arm-cci-400-pmu,r1 => revision 1
> 	arm-cci-400-pmu => (old) DEPRECATED
>
> The old binding has been DEPRECATED and must be used only on ARM32
> system with secure access. We don't have a reliable dynamic way to detect
> if the system is running secure. This series tries to use the best safe
> method by relying on the availability of MCPM(as it was prior to the series).
> It is upto the MCPM platform driver to decide, if the system is secure before
> it goes ahead and registers its drivers and pokes the CCI. This series doesn't
> address/solve the problem of MCPM. I will be happy to use a better approach,
> if there is any.
>
> Tested on (non-secure)TC2 and A53x2.
>

For the series,

Acked-by: Punit Agrawal <punit.agrawal@arm.com>

Cheers,
Punit

[...]


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [UPDATED] [PATCH 3/5] arm-cci: Get rid of secure transactions for PMU driver
  2015-03-10 15:18 ` [PATCH 3/5] arm-cci: Get rid of secure transactions for PMU driver Suzuki K. Poulose
@ 2015-03-17  9:51   ` Suzuki K. Poulose
  2015-03-19 17:25     ` Mark Rutland
  2015-03-19 17:32     ` Mark Rutland
  0 siblings, 2 replies; 24+ messages in thread
From: Suzuki K. Poulose @ 2015-03-17  9:51 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Nicolas Pitre, Bartlomiej Zolnierkiewicz, Kukjin Kim,
	Abhilash Kesavan, Arnd Bergmann, devicetree, linux-kernel,
	Liviu Dudau, Lorenzo Pieralisi, Olof Johansson, Pawel Moll,
	Punit Agrawal, Sudeep Holla, Will Deacon, Catalin Marinas,
	Suzuki K. Poulose

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

A minor change, fixed missplled 'DEPRECATED' in the dev_warn().

Thanks
Suzuki

----8>----
Avoid secure transactions while probing the CCI PMU. The
existing code makes use of the Peripheral ID2 (PID2) register
to determine the revision of the CCI400, which requires a
secure transaction. This puts a limitation on the usage of the
driver on systems running non-secure Linux(e.g, ARM64).

Updated the device-tree binding for cci pmu node to add the explicit
revision number for the compatible field.

The supported strings are :
	arm,cci-400-pmu,r0
	arm,cci-400-pmu,r1
	arm,cci-400-pmu - DEPRECATED. See NOTE below

NOTE: If the revision is not mentioned, we need to probe the cci revision,
which could be fatal on a platform running non-secure. We need a reliable way
to know if we can poke the CCI registers at runtime on ARM32. We depend on
'mcpm_is_available()' when it is available. mcpm_is_available() returns true
only when there is a registered driver for mcpm. Otherwise, we assume that we
don't have secure access, and skips probing the revision number(ARM64 case).

The MCPM should figure out if it is safe to access the CCI. Unfortunately
there isn't a reliable way to indicate the same via dtb. This patch doesn't
address/change the current situation. It only deals with the CCI-PMU, leaving
the assumptions about the secure access as it has been, prior to this patch.

Changes since V2:
 - Use 'bool' instead of 'int' for platform_has_secure_cci_access().
   (Suggested-by: Sudeep Holla)

Cc: devicetree@vger.kernel.org
Cc: Punit Agrawal <punit.agrawal@arm.com>
Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
Tested-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
 Documentation/devicetree/bindings/arm/cci.txt |    7 +++--
 arch/arm/include/asm/arm-cci.h                |   42 +++++++++++++++++++++++++
 arch/arm64/include/asm/arm-cci.h              |   27 ++++++++++++++++
 drivers/bus/arm-cci.c                         |   17 +++++++++-
 include/linux/arm-cci.h                       |    2 ++
 5 files changed, 92 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/include/asm/arm-cci.h
 create mode 100644 arch/arm64/include/asm/arm-cci.h

diff --git a/Documentation/devicetree/bindings/arm/cci.txt b/Documentation/devicetree/bindings/arm/cci.txt
index f28d82b..0e4b6a7 100644
--- a/Documentation/devicetree/bindings/arm/cci.txt
+++ b/Documentation/devicetree/bindings/arm/cci.txt
@@ -94,8 +94,11 @@ specific to ARM.
 		- compatible
 			Usage: required
 			Value type: <string>
-			Definition: must be "arm,cci-400-pmu"
-
+			Definition: Supported strings are :
+				 "arm,cci-400-pmu,r0"
+				 "arm,cci-400-pmu,r1"
+				 "arm,cci-400-pmu"  - DEPRECATED, permitted only where OS has
+						      secure acces to CCI registers
 		- reg:
 			Usage: required
 			Value type: Integer cells. A register entry, expressed
diff --git a/arch/arm/include/asm/arm-cci.h b/arch/arm/include/asm/arm-cci.h
new file mode 100644
index 0000000..fe77f7a
--- /dev/null
+++ b/arch/arm/include/asm/arm-cci.h
@@ -0,0 +1,42 @@
+/*
+ * arch/arm/include/asm/arm-cci.h
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __ASM_ARM_CCI_H
+#define __ASM_ARM_CCI_H
+
+#ifdef CONFIG_MCPM
+#include <asm/mcpm.h>
+
+/*
+ * We don't have a reliable way of detecting whether,
+ * if we have access to secure-only registers, unless
+ * mcpm is registered.
+ */
+static inline bool platform_has_secure_cci_access(void)
+{
+	return mcpm_is_available();
+}
+
+#else
+static inline bool platform_has_secure_cci_access(void)
+{
+	return false;
+}
+#endif
+
+#endif
diff --git a/arch/arm64/include/asm/arm-cci.h b/arch/arm64/include/asm/arm-cci.h
new file mode 100644
index 0000000..f0b6371
--- /dev/null
+++ b/arch/arm64/include/asm/arm-cci.h
@@ -0,0 +1,27 @@
+/*
+ * arch/arm64/include/asm/arm-cci.h
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __ASM_ARM_CCI_H
+#define __ASM_ARM_CCI_H
+
+static inline bool platform_has_secure_cci_access(void)
+{
+	return false;
+}
+
+#endif
diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index f88383e..ecf25b3 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -217,7 +217,9 @@ static int probe_cci_revision(void)
 
 static const struct cci_pmu_model *probe_cci_model(struct platform_device *pdev)
 {
-	return &cci_pmu_models[probe_cci_revision()];
+	if (platform_has_secure_cci_access())
+		return &cci_pmu_models[probe_cci_revision()];
+	return NULL;
 }
 
 static int pmu_is_valid_counter(struct cci_pmu *cci_pmu, int idx)
@@ -873,6 +875,15 @@ static struct cci_pmu_model cci_pmu_models[] = {
 static const struct of_device_id arm_cci_pmu_matches[] = {
 	{
 		.compatible = "arm,cci-400-pmu",
+		.data	= NULL,
+	},
+	{
+		.compatible = "arm,cci-400-pmu,r0",
+		.data	= &cci_pmu_models[CCI_REV_R0],
+	},
+	{
+		.compatible = "arm,cci-400-pmu,r1",
+		.data	= &cci_pmu_models[CCI_REV_R1],
 	},
 	{},
 };
@@ -883,7 +894,11 @@ static inline const struct cci_pmu_model *get_cci_model(struct platform_device *
 							pdev->dev.of_node);
 	if (!match)
 		return NULL;
+	if (match->data)
+		return match->data;
 
+	dev_warn(&pdev->dev, "DEPRECATED compatible property,"
+			 "requires secure access to CCI registers");
 	return probe_cci_model(pdev);
 }
 
diff --git a/include/linux/arm-cci.h b/include/linux/arm-cci.h
index 79d6edf..aede5c7 100644
--- a/include/linux/arm-cci.h
+++ b/include/linux/arm-cci.h
@@ -24,6 +24,8 @@
 #include <linux/errno.h>
 #include <linux/types.h>
 
+#include <asm/arm-cci.h>
+
 struct device_node;
 
 #ifdef CONFIG_ARM_CCI
-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/5] arm-cci: Abstract the CCI400 PMU speicific definitions
  2015-03-10 15:18 ` [PATCH 2/5] arm-cci: Abstract the CCI400 PMU speicific definitions Suzuki K. Poulose
@ 2015-03-17 18:49   ` Will Deacon
  0 siblings, 0 replies; 24+ messages in thread
From: Will Deacon @ 2015-03-17 18:49 UTC (permalink / raw)
  To: Suzuki K. Poulose
  Cc: linux-arm-kernel, Nicolas Pitre, Bartlomiej Zolnierkiewicz,
	Kukjin Kim, Abhilash Kesavan, Arnd Bergmann, devicetree,
	linux-kernel, Liviu Dudau, Lorenzo Pieralisi, Olof Johansson,
	Pawel Moll, Punit Agrawal, Sudeep Holla, Catalin Marinas

On Tue, Mar 10, 2015 at 03:18:52PM +0000, Suzuki K. Poulose wrote:
> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> 
> CCI400 has different event specifications for PMU, for revsion
> 0 and revision 1. As of now, we check the revision every single
> time before using the parameters for the PMU. This patch abstracts
> the details of the pmu models in a struct (cci_pmu_model) and
> stores the information in cci_pmu at initialisation time, avoiding
> multiple probe operations.
> 
> Changes since V2:
>  - Cleanup event validation(pmu_validate_hw_event). Get rid of
>    helper functions:
> 	pmu_is_valid_slave_event
> 	pmu_is_valid_master_event
> 
> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
> ---

Looks good to me:

Reviewed-by: Will Deacon <will.deacon@arm.com>

Will

>  drivers/bus/arm-cci.c |  141 ++++++++++++++++++++++++++++---------------------
>  1 file changed, 81 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> index ea39fc2..f88383e 100644
> --- a/drivers/bus/arm-cci.c
> +++ b/drivers/bus/arm-cci.c
> @@ -79,19 +79,38 @@ static const struct of_device_id arm_cci_matches[] = {
>  
>  #define CCI_PMU_MAX_HW_EVENTS 5   /* CCI PMU has 4 counters + 1 cycle counter */
>  
> +/* Types of interfaces that can generate events */
> +enum {
> +	CCI_IF_SLAVE,
> +	CCI_IF_MASTER,
> +	CCI_IF_MAX,
> +};
> +
> +struct event_range {
> +	u32 min;
> +	u32 max;
> +};
> +
>  struct cci_pmu_hw_events {
>  	struct perf_event *events[CCI_PMU_MAX_HW_EVENTS];
>  	unsigned long used_mask[BITS_TO_LONGS(CCI_PMU_MAX_HW_EVENTS)];
>  	raw_spinlock_t pmu_lock;
>  };
>  
> +struct cci_pmu_model {
> +	char *name;
> +	struct event_range event_ranges[CCI_IF_MAX];
> +};
> +
> +static struct cci_pmu_model cci_pmu_models[];
> +
>  struct cci_pmu {
>  	void __iomem *base;
>  	struct pmu pmu;
>  	int nr_irqs;
>  	int irqs[CCI_PMU_MAX_HW_EVENTS];
>  	unsigned long active_irqs;
> -	struct pmu_port_event_ranges *port_ranges;
> +	const struct cci_pmu_model *model;
>  	struct cci_pmu_hw_events hw_events;
>  	struct platform_device *plat_device;
>  	int num_events;
> @@ -152,53 +171,11 @@ enum cci400_perf_events {
>  #define CCI_REV_R1_MASTER_PORT_MIN_EV	0x00
>  #define CCI_REV_R1_MASTER_PORT_MAX_EV	0x11
>  
> -struct pmu_port_event_ranges {
> -	u8 slave_min;
> -	u8 slave_max;
> -	u8 master_min;
> -	u8 master_max;
> -};
> -
> -static struct pmu_port_event_ranges port_event_range[] = {
> -	[CCI_REV_R0] = {
> -		.slave_min = CCI_REV_R0_SLAVE_PORT_MIN_EV,
> -		.slave_max = CCI_REV_R0_SLAVE_PORT_MAX_EV,
> -		.master_min = CCI_REV_R0_MASTER_PORT_MIN_EV,
> -		.master_max = CCI_REV_R0_MASTER_PORT_MAX_EV,
> -	},
> -	[CCI_REV_R1] = {
> -		.slave_min = CCI_REV_R1_SLAVE_PORT_MIN_EV,
> -		.slave_max = CCI_REV_R1_SLAVE_PORT_MAX_EV,
> -		.master_min = CCI_REV_R1_MASTER_PORT_MIN_EV,
> -		.master_max = CCI_REV_R1_MASTER_PORT_MAX_EV,
> -	},
> -};
> -
> -/*
> - * Export different PMU names for the different revisions so userspace knows
> - * because the event ids are different
> - */
> -static char *const pmu_names[] = {
> -	[CCI_REV_R0] = "CCI_400",
> -	[CCI_REV_R1] = "CCI_400_r1",
> -};
> -
> -static int pmu_is_valid_slave_event(u8 ev_code)
> -{
> -	return pmu->port_ranges->slave_min <= ev_code &&
> -		ev_code <= pmu->port_ranges->slave_max;
> -}
> -
> -static int pmu_is_valid_master_event(u8 ev_code)
> -{
> -	return pmu->port_ranges->master_min <= ev_code &&
> -		ev_code <= pmu->port_ranges->master_max;
> -}
> -
>  static int pmu_validate_hw_event(u8 hw_event)
>  {
>  	u8 ev_source = CCI_PMU_EVENT_SOURCE(hw_event);
>  	u8 ev_code = CCI_PMU_EVENT_CODE(hw_event);
> +	int if_type;
>  
>  	switch (ev_source) {
>  	case CCI_PORT_S0:
> @@ -207,18 +184,22 @@ static int pmu_validate_hw_event(u8 hw_event)
>  	case CCI_PORT_S3:
>  	case CCI_PORT_S4:
>  		/* Slave Interface */
> -		if (pmu_is_valid_slave_event(ev_code))
> -			return hw_event;
> +		if_type = CCI_IF_SLAVE;
>  		break;
>  	case CCI_PORT_M0:
>  	case CCI_PORT_M1:
>  	case CCI_PORT_M2:
>  		/* Master Interface */
> -		if (pmu_is_valid_master_event(ev_code))
> -			return hw_event;
> +		if_type = CCI_IF_MASTER;
>  		break;
> +	default:
> +		return -ENOENT;
>  	}
>  
> +	if (ev_code >= pmu->model->event_ranges[if_type].min &&
> +		ev_code <= pmu->model->event_ranges[if_type].max)
> +		return hw_event;
> +
>  	return -ENOENT;
>  }
>  
> @@ -234,11 +215,9 @@ static int probe_cci_revision(void)
>  		return CCI_REV_R1;
>  }
>  
> -static struct pmu_port_event_ranges *port_range_by_rev(void)
> +static const struct cci_pmu_model *probe_cci_model(struct platform_device *pdev)
>  {
> -	int rev = probe_cci_revision();
> -
> -	return &port_event_range[rev];
> +	return &cci_pmu_models[probe_cci_revision()];
>  }
>  
>  static int pmu_is_valid_counter(struct cci_pmu *cci_pmu, int idx)
> @@ -807,9 +786,9 @@ static const struct attribute_group *pmu_attr_groups[] = {
>  
>  static int cci_pmu_init(struct cci_pmu *cci_pmu, struct platform_device *pdev)
>  {
> -	char *name = pmu_names[probe_cci_revision()];
> +	char *name = cci_pmu->model->name;
>  	cci_pmu->pmu = (struct pmu) {
> -		.name		= pmu_names[probe_cci_revision()],
> +		.name		= cci_pmu->model->name,
>  		.task_ctx_nr	= perf_invalid_context,
>  		.pmu_enable	= cci_pmu_enable,
>  		.pmu_disable	= cci_pmu_disable,
> @@ -862,6 +841,35 @@ static struct notifier_block cci_pmu_cpu_nb = {
>  	.priority	= CPU_PRI_PERF + 1,
>  };
>  
> +static struct cci_pmu_model cci_pmu_models[] = {
> +	[CCI_REV_R0] = {
> +		.name = "CCI_400",
> +		.event_ranges = {
> +			[CCI_IF_SLAVE] = {
> +				CCI_REV_R0_SLAVE_PORT_MIN_EV,
> +				CCI_REV_R0_SLAVE_PORT_MAX_EV,
> +			},
> +			[CCI_IF_MASTER] = {
> +				CCI_REV_R0_MASTER_PORT_MIN_EV,
> +				CCI_REV_R0_MASTER_PORT_MAX_EV,
> +			},
> +		},
> +	},
> +	[CCI_REV_R1] = {
> +		.name = "CCI_400_r1",
> +		.event_ranges = {
> +			[CCI_IF_SLAVE] = {
> +				CCI_REV_R1_SLAVE_PORT_MIN_EV,
> +				CCI_REV_R1_SLAVE_PORT_MAX_EV,
> +			},
> +			[CCI_IF_MASTER] = {
> +				CCI_REV_R1_MASTER_PORT_MIN_EV,
> +				CCI_REV_R1_MASTER_PORT_MAX_EV,
> +			},
> +		},
> +	},
> +};
> +
>  static const struct of_device_id arm_cci_pmu_matches[] = {
>  	{
>  		.compatible = "arm,cci-400-pmu",
> @@ -869,6 +877,16 @@ static const struct of_device_id arm_cci_pmu_matches[] = {
>  	{},
>  };
>  
> +static inline const struct cci_pmu_model *get_cci_model(struct platform_device *pdev)
> +{
> +	const struct of_device_id *match = of_match_node(arm_cci_pmu_matches,
> +							pdev->dev.of_node);
> +	if (!match)
> +		return NULL;
> +
> +	return probe_cci_model(pdev);
> +}
> +
>  static bool is_duplicate_irq(int irq, int *irqs, int nr_irqs)
>  {
>  	int i;
> @@ -884,11 +902,19 @@ static int cci_pmu_probe(struct platform_device *pdev)
>  {
>  	struct resource *res;
>  	int i, ret, irq;
> +	const struct cci_pmu_model *model;
> +
> +	model = get_cci_model(pdev);
> +	if (!model) {
> +		dev_warn(&pdev->dev, "CCI PMU version not supported\n");
> +		return -ENODEV;
> +	}
>  
>  	pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
>  	if (!pmu)
>  		return -ENOMEM;
>  
> +	pmu->model = model;
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	pmu->base = devm_ioremap_resource(&pdev->dev, res);
>  	if (IS_ERR(pmu->base))
> @@ -920,12 +946,6 @@ static int cci_pmu_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	pmu->port_ranges = port_range_by_rev();
> -	if (!pmu->port_ranges) {
> -		dev_warn(&pdev->dev, "CCI PMU version not supported\n");
> -		return -EINVAL;
> -	}
> -
>  	raw_spin_lock_init(&pmu->hw_events.pmu_lock);
>  	mutex_init(&pmu->reserve_mutex);
>  	atomic_set(&pmu->active_events, 0);
> @@ -939,6 +959,7 @@ static int cci_pmu_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	pr_info("ARM %s PMU driver probed", pmu->model->name);
>  	return 0;
>  }
>  
> -- 
> 1.7.9.5
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 5/5] arm-cci: Fix CCI PMU event validation
  2015-03-10 15:18 ` [PATCH 5/5] arm-cci: Fix CCI PMU event validation Suzuki K. Poulose
@ 2015-03-17 18:52   ` Will Deacon
  0 siblings, 0 replies; 24+ messages in thread
From: Will Deacon @ 2015-03-17 18:52 UTC (permalink / raw)
  To: Suzuki K. Poulose
  Cc: linux-arm-kernel, Nicolas Pitre, Bartlomiej Zolnierkiewicz,
	Kukjin Kim, Abhilash Kesavan, Arnd Bergmann, devicetree,
	linux-kernel, Liviu Dudau, Lorenzo Pieralisi, Olof Johansson,
	Pawel Moll, Punit Agrawal, Sudeep Holla, Catalin Marinas

On Tue, Mar 10, 2015 at 03:18:55PM +0000, Suzuki K. Poulose wrote:
> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> 
> We mask the event with the CCI_PMU_EVENT_MASK, before passing
> the config to pmu_validate_hw_event(), which causes extra bits
> to be ignored and qualifies an invalid event code as valid.
> 
> e.g,
>  $ perf stat -a -C 0 -e CCI_400/config=0x1ff,name=cycles/ sleep 1
>    Performance counter stats for 'system wide':
> 
>          506951142      cycles
> 
>        1.013879626 seconds time elapsed
> 
> where, cycles has an event coding of 0xff. This patch also removes
> the unnecessary 'event' mask in pmu_write_register, since the config_base
> is set by the pmu code after the event is validated.
> 
> Changes since V2:
>  - Switch to input unsigned long for pmu_validate_hw_event()
> 
> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/bus/arm-cci.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> index 581190d..89c86e9 100644
> --- a/drivers/bus/arm-cci.c
> +++ b/drivers/bus/arm-cci.c
> @@ -179,12 +179,15 @@ enum cci400_perf_events {
>  #define CCI_REV_R1_MASTER_PORT_MIN_EV	0x00
>  #define CCI_REV_R1_MASTER_PORT_MAX_EV	0x11
>  
> -static int pmu_validate_hw_event(u8 hw_event)
> +static int pmu_validate_hw_event(unsigned long hw_event)
>  {
>  	u8 ev_source = CCI_PMU_EVENT_SOURCE(hw_event);
>  	u8 ev_code = CCI_PMU_EVENT_CODE(hw_event);
>  	int if_type;
>  
> +	if (hw_event & ~CCI_PMU_EVENT_MASK)
> +		return -ENOENT;

Given that you want to build this for arm64, shouldn't CCI_PMU_EVENT_MASK
be an unsigned long too (i.e. 0xffUL)? Otherwise you won't detect set bits
in the upper word here.

Will

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64
  2015-03-10 15:18 [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Suzuki K. Poulose
                   ` (7 preceding siblings ...)
  2015-03-11 11:40 ` Punit Agrawal
@ 2015-03-17 18:54 ` Will Deacon
  2015-03-18 10:09   ` Suzuki K. Poulose
  8 siblings, 1 reply; 24+ messages in thread
From: Will Deacon @ 2015-03-17 18:54 UTC (permalink / raw)
  To: Suzuki K. Poulose
  Cc: linux-arm-kernel, Nicolas Pitre, Bartlomiej Zolnierkiewicz,
	Kukjin Kim, Abhilash Kesavan, Arnd Bergmann, devicetree,
	linux-kernel, Liviu Dudau, Lorenzo Pieralisi, Olof Johansson,
	Pawel Moll, Punit Agrawal, Sudeep Holla, Catalin Marinas

On Tue, Mar 10, 2015 at 03:18:50PM +0000, Suzuki K. Poulose wrote:
> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> 
> This series enables the PMU monitoring support for CCI400 on ARM64.
> The existing CCI400 driver code is a mix of PMU driver and the MCPM
> driver code. The MCPM driver is only used on ARM(32) and contains
> arm32 assembly and hence can't be built on ARM64. This patch splits
> the code to
> 
>  - ARM_CCI400_PORT_CTRL driver - depends on ARM && V7
>  - ARM_CCI400_PMU driver

If you repost this with acks added and my feedback addressed, then I'm
happy to put together a branch for arm-soc along with your other CCI PMU
fix for event validation.

Will

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64
  2015-03-17 18:54 ` Will Deacon
@ 2015-03-18 10:09   ` Suzuki K. Poulose
  0 siblings, 0 replies; 24+ messages in thread
From: Suzuki K. Poulose @ 2015-03-18 10:09 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-arm-kernel, Nicolas Pitre, Bartlomiej Zolnierkiewicz,
	Kukjin Kim, Abhilash Kesavan, Arnd Bergmann, devicetree,
	linux-kernel, Liviu Dudau, Lorenzo Pieralisi, Olof Johansson,
	Pawel Moll, Punit Agrawal, Sudeep Holla, Catalin Marinas

On 17/03/15 18:54, Will Deacon wrote:
> On Tue, Mar 10, 2015 at 03:18:50PM +0000, Suzuki K. Poulose wrote:
>> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>>
>> This series enables the PMU monitoring support for CCI400 on ARM64.
>> The existing CCI400 driver code is a mix of PMU driver and the MCPM
>> driver code. The MCPM driver is only used on ARM(32) and contains
>> arm32 assembly and hence can't be built on ARM64. This patch splits
>> the code to
>>
>>   - ARM_CCI400_PORT_CTRL driver - depends on ARM && V7
>>   - ARM_CCI400_PMU driver
>
> If you repost this with acks added and my feedback addressed, then I'm
> happy to put together a branch for arm-soc along with your other CCI PMU
> fix for event validation.

Sure, I will do that.

Thanks
Suzuki

>
> Will
>



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [UPDATED] [PATCH 3/5] arm-cci: Get rid of secure transactions for PMU driver
  2015-03-17  9:51   ` [UPDATED] " Suzuki K. Poulose
@ 2015-03-19 17:25     ` Mark Rutland
  2015-03-19 17:32     ` Mark Rutland
  1 sibling, 0 replies; 24+ messages in thread
From: Mark Rutland @ 2015-03-19 17:25 UTC (permalink / raw)
  To: Suzuki K. Poulose
  Cc: linux-arm-kernel, Nicolas Pitre, Bartlomiej Zolnierkiewicz,
	Kukjin Kim, Abhilash Kesavan, Arnd Bergmann, devicetree,
	linux-kernel, Liviu Dudau, Lorenzo Pieralisi, Olof Johansson,
	Pawel Moll, Punit Agrawal, Sudeep Holla, Will Deacon,
	Catalin Marinas

On Tue, Mar 17, 2015 at 09:51:41AM +0000, Suzuki K. Poulose wrote:
> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> 
> A minor change, fixed missplled 'DEPRECATED' in the dev_warn().
> 
> Thanks
> Suzuki
> 
> ----8>----
> Avoid secure transactions while probing the CCI PMU. The
> existing code makes use of the Peripheral ID2 (PID2) register
> to determine the revision of the CCI400, which requires a
> secure transaction. This puts a limitation on the usage of the
> driver on systems running non-secure Linux(e.g, ARM64).
> 
> Updated the device-tree binding for cci pmu node to add the explicit
> revision number for the compatible field.
> 
> The supported strings are :
> 	arm,cci-400-pmu,r0
> 	arm,cci-400-pmu,r1
> 	arm,cci-400-pmu - DEPRECATED. See NOTE below
> 
> NOTE: If the revision is not mentioned, we need to probe the cci revision,
> which could be fatal on a platform running non-secure. We need a reliable way
> to know if we can poke the CCI registers at runtime on ARM32. We depend on
> 'mcpm_is_available()' when it is available. mcpm_is_available() returns true
> only when there is a registered driver for mcpm. Otherwise, we assume that we
> don't have secure access, and skips probing the revision number(ARM64 case).
> 
> The MCPM should figure out if it is safe to access the CCI. Unfortunately
> there isn't a reliable way to indicate the same via dtb. This patch doesn't
> address/change the current situation. It only deals with the CCI-PMU, leaving
> the assumptions about the secure access as it has been, prior to this patch.
> 
> Changes since V2:
>  - Use 'bool' instead of 'int' for platform_has_secure_cci_access().
>    (Suggested-by: Sudeep Holla)
> 
> Cc: devicetree@vger.kernel.org
> Cc: Punit Agrawal <punit.agrawal@arm.com>
> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
> Tested-by: Sudeep Holla <sudeep.holla@arm.com>
> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> ---
>  Documentation/devicetree/bindings/arm/cci.txt |    7 +++--
>  arch/arm/include/asm/arm-cci.h                |   42 +++++++++++++++++++++++++
>  arch/arm64/include/asm/arm-cci.h              |   27 ++++++++++++++++
>  drivers/bus/arm-cci.c                         |   17 +++++++++-
>  include/linux/arm-cci.h                       |    2 ++
>  5 files changed, 92 insertions(+), 3 deletions(-)
>  create mode 100644 arch/arm/include/asm/arm-cci.h
>  create mode 100644 arch/arm64/include/asm/arm-cci.h
> 
> diff --git a/Documentation/devicetree/bindings/arm/cci.txt b/Documentation/devicetree/bindings/arm/cci.txt
> index f28d82b..0e4b6a7 100644
> --- a/Documentation/devicetree/bindings/arm/cci.txt
> +++ b/Documentation/devicetree/bindings/arm/cci.txt
> @@ -94,8 +94,11 @@ specific to ARM.
>  		- compatible
>  			Usage: required
>  			Value type: <string>
> -			Definition: must be "arm,cci-400-pmu"
> -
> +			Definition: Supported strings are :

I'd prefer we said "Must contain one of:".

> +				 "arm,cci-400-pmu,r0"
> +				 "arm,cci-400-pmu,r1"
> +				 "arm,cci-400-pmu"  - DEPRECATED, permitted only where OS has
> +						      secure acces to CCI registers

Otherwise, the binding change looks fine to me, so:

Acked-by: Mark Rutland <mark.rutland@arm.com> [DT binding]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [UPDATED] [PATCH 3/5] arm-cci: Get rid of secure transactions for PMU driver
  2015-03-17  9:51   ` [UPDATED] " Suzuki K. Poulose
  2015-03-19 17:25     ` Mark Rutland
@ 2015-03-19 17:32     ` Mark Rutland
  2015-03-19 17:38       ` Sudeep Holla
  1 sibling, 1 reply; 24+ messages in thread
From: Mark Rutland @ 2015-03-19 17:32 UTC (permalink / raw)
  To: Suzuki K. Poulose
  Cc: linux-arm-kernel, Nicolas Pitre, Bartlomiej Zolnierkiewicz,
	Kukjin Kim, Abhilash Kesavan, Arnd Bergmann, devicetree,
	linux-kernel, Liviu Dudau, Lorenzo Pieralisi, Olof Johansson,
	Pawel Moll, Punit Agrawal, Sudeep Holla, Will Deacon,
	Catalin Marinas

One more thing:

> @@ -883,7 +894,11 @@ static inline const struct cci_pmu_model *get_cci_model(struct platform_device *
>  							pdev->dev.of_node);
>  	if (!match)
>  		return NULL;
> +	if (match->data)
> +		return match->data;
>  
> +	dev_warn(&pdev->dev, "DEPRECATED compatible property,"
> +			 "requires secure access to CCI registers");
>  	return probe_cci_model(pdev);
>  }

Before the probe, could we please have:

	if (!IS_ENABLED(CONFIG_ARM))
		return -EINVAL;

On arm64 we require a model-specific string, and we shouldn't go
touching secure-only registers.

Mark.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [UPDATED] [PATCH 3/5] arm-cci: Get rid of secure transactions for PMU driver
  2015-03-19 17:32     ` Mark Rutland
@ 2015-03-19 17:38       ` Sudeep Holla
  2015-03-19 17:52         ` Suzuki K. Poulose
  0 siblings, 1 reply; 24+ messages in thread
From: Sudeep Holla @ 2015-03-19 17:38 UTC (permalink / raw)
  To: Mark Rutland, Suzuki Poulose
  Cc: Sudeep Holla, linux-arm-kernel, Nicolas Pitre,
	Bartlomiej Zolnierkiewicz, Kukjin Kim, Abhilash Kesavan,
	Arnd Bergmann, devicetree, linux-kernel, Liviu Dudau,
	Lorenzo Pieralisi, Olof Johansson, Pawel Moll, Punit Agrawal,
	Will Deacon, Catalin Marinas



On 19/03/15 17:32, Mark Rutland wrote:
> One more thing:
>
>> @@ -883,7 +894,11 @@ static inline const struct cci_pmu_model *get_cci_model(struct platform_device *
>>   							pdev->dev.of_node);
>>   	if (!match)
>>   		return NULL;
>> +	if (match->data)
>> +		return match->data;
>>
>> +	dev_warn(&pdev->dev, "DEPRECATED compatible property,"
>> +			 "requires secure access to CCI registers");
>>   	return probe_cci_model(pdev);
>>   }
>
> Before the probe, could we please have:
>
> 	if (!IS_ENABLED(CONFIG_ARM))
> 		return -EINVAL;
>
> On arm64 we require a model-specific string, and we shouldn't go
> touching secure-only registers.
>

IIUC platform_has_secure_cci_access always return false for ARM64
preventing any secure access. No ?

Regards,
Sudeep


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [UPDATED] [PATCH 3/5] arm-cci: Get rid of secure transactions for PMU driver
  2015-03-19 17:38       ` Sudeep Holla
@ 2015-03-19 17:52         ` Suzuki K. Poulose
  2015-03-19 17:54           ` Mark Rutland
  0 siblings, 1 reply; 24+ messages in thread
From: Suzuki K. Poulose @ 2015-03-19 17:52 UTC (permalink / raw)
  To: Sudeep Holla, Mark Rutland
  Cc: linux-arm-kernel, Nicolas Pitre, Bartlomiej Zolnierkiewicz,
	Kukjin Kim, Abhilash Kesavan, Arnd Bergmann, devicetree,
	linux-kernel, Liviu Dudau, Lorenzo Pieralisi, Olof Johansson,
	Pawel Moll, Punit Agrawal, Will Deacon, Catalin Marinas

On 19/03/15 17:38, Sudeep Holla wrote:
>
>
> On 19/03/15 17:32, Mark Rutland wrote:
>> One more thing:
>>
>>> @@ -883,7 +894,11 @@ static inline const struct cci_pmu_model *get_cci_model(struct platform_device *
>>>    							pdev->dev.of_node);
>>>    	if (!match)
>>>    		return NULL;
>>> +	if (match->data)
>>> +		return match->data;
>>>
>>> +	dev_warn(&pdev->dev, "DEPRECATED compatible property,"
>>> +			 "requires secure access to CCI registers");
>>>    	return probe_cci_model(pdev);
>>>    }
>>
>> Before the probe, could we please have:
>>
>> 	if (!IS_ENABLED(CONFIG_ARM))
>> 		return -EINVAL;
>>
>> On arm64 we require a model-specific string, and we shouldn't go
>> touching secure-only registers.
>>
>
> IIUC platform_has_secure_cci_access always return false for ARM64
> preventing any secure access. No ?
>
Yes, you are right. The check has been abstracted away with the 
platform_has_secure_cci_access().

Cheers
Suzuki



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [UPDATED] [PATCH 3/5] arm-cci: Get rid of secure transactions for PMU driver
  2015-03-19 17:52         ` Suzuki K. Poulose
@ 2015-03-19 17:54           ` Mark Rutland
  0 siblings, 0 replies; 24+ messages in thread
From: Mark Rutland @ 2015-03-19 17:54 UTC (permalink / raw)
  To: Suzuki K. Poulose
  Cc: Sudeep Holla, linux-arm-kernel, Nicolas Pitre,
	Bartlomiej Zolnierkiewicz, Kukjin Kim, Abhilash Kesavan,
	Arnd Bergmann, devicetree, linux-kernel, Liviu Dudau,
	Lorenzo Pieralisi, Olof Johansson, Pawel Moll, Punit Agrawal,
	Will Deacon, Catalin Marinas

On Thu, Mar 19, 2015 at 05:52:54PM +0000, Suzuki K. Poulose wrote:
> On 19/03/15 17:38, Sudeep Holla wrote:
> >
> >
> > On 19/03/15 17:32, Mark Rutland wrote:
> >> One more thing:
> >>
> >>> @@ -883,7 +894,11 @@ static inline const struct cci_pmu_model *get_cci_model(struct platform_device *
> >>>    							pdev->dev.of_node);
> >>>    	if (!match)
> >>>    		return NULL;
> >>> +	if (match->data)
> >>> +		return match->data;
> >>>
> >>> +	dev_warn(&pdev->dev, "DEPRECATED compatible property,"
> >>> +			 "requires secure access to CCI registers");
> >>>    	return probe_cci_model(pdev);
> >>>    }
> >>
> >> Before the probe, could we please have:
> >>
> >> 	if (!IS_ENABLED(CONFIG_ARM))
> >> 		return -EINVAL;
> >>
> >> On arm64 we require a model-specific string, and we shouldn't go
> >> touching secure-only registers.
> >>
> >
> > IIUC platform_has_secure_cci_access always return false for ARM64
> > preventing any secure access. No ?
> >
> Yes, you are right. The check has been abstracted away with the 
> platform_has_secure_cci_access().

Ah, that's fine then.

Sorry for the noise!

Mark.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/5] arm-cci: Abstract the CCI400 PMU speicific definitions
  2015-03-18 12:24 [PATCHv4 " Suzuki K. Poulose
@ 2015-03-18 12:24 ` Suzuki K. Poulose
  0 siblings, 0 replies; 24+ messages in thread
From: Suzuki K. Poulose @ 2015-03-18 12:24 UTC (permalink / raw)
  To: will.deacon
  Cc: Nicolas Pitre, Bartlomiej Zolnierkiewicz, Kukjin Kim,
	Abhilash Kesavan, Arnd Bergmann, devicetree, linux-kernel,
	Liviu Dudau, Lorenzo Pieralisi, Olof Johansson, Pawel Moll,
	Punit Agrawal, Sudeep Holla, Catalin Marinas, Suzuki K. Poulose

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

CCI400 has different event specifications for PMU, for revsion
0 and revision 1. As of now, we check the revision every single
time before using the parameters for the PMU. This patch abstracts
the details of the pmu models in a struct (cci_pmu_model) and
stores the information in cci_pmu at initialisation time, avoiding
multiple probe operations.

Changes since V2:
 - Cleanup event validation(pmu_validate_hw_event). Get rid of
   helper functions:
	pmu_is_valid_slave_event
	pmu_is_valid_master_event

Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
Tested-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Punit Agrawal <punit.agrawal@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
---
 drivers/bus/arm-cci.c |  141 ++++++++++++++++++++++++++++---------------------
 1 file changed, 81 insertions(+), 60 deletions(-)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index 5d29ec3..ae3864d 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -79,19 +79,38 @@ static const struct of_device_id arm_cci_matches[] = {
 
 #define CCI_PMU_MAX_HW_EVENTS 5   /* CCI PMU has 4 counters + 1 cycle counter */
 
+/* Types of interfaces that can generate events */
+enum {
+	CCI_IF_SLAVE,
+	CCI_IF_MASTER,
+	CCI_IF_MAX,
+};
+
+struct event_range {
+	u32 min;
+	u32 max;
+};
+
 struct cci_pmu_hw_events {
 	struct perf_event *events[CCI_PMU_MAX_HW_EVENTS];
 	unsigned long used_mask[BITS_TO_LONGS(CCI_PMU_MAX_HW_EVENTS)];
 	raw_spinlock_t pmu_lock;
 };
 
+struct cci_pmu_model {
+	char *name;
+	struct event_range event_ranges[CCI_IF_MAX];
+};
+
+static struct cci_pmu_model cci_pmu_models[];
+
 struct cci_pmu {
 	void __iomem *base;
 	struct pmu pmu;
 	int nr_irqs;
 	int irqs[CCI_PMU_MAX_HW_EVENTS];
 	unsigned long active_irqs;
-	struct pmu_port_event_ranges *port_ranges;
+	const struct cci_pmu_model *model;
 	struct cci_pmu_hw_events hw_events;
 	struct platform_device *plat_device;
 	int num_events;
@@ -152,53 +171,11 @@ enum cci400_perf_events {
 #define CCI_REV_R1_MASTER_PORT_MIN_EV	0x00
 #define CCI_REV_R1_MASTER_PORT_MAX_EV	0x11
 
-struct pmu_port_event_ranges {
-	u8 slave_min;
-	u8 slave_max;
-	u8 master_min;
-	u8 master_max;
-};
-
-static struct pmu_port_event_ranges port_event_range[] = {
-	[CCI_REV_R0] = {
-		.slave_min = CCI_REV_R0_SLAVE_PORT_MIN_EV,
-		.slave_max = CCI_REV_R0_SLAVE_PORT_MAX_EV,
-		.master_min = CCI_REV_R0_MASTER_PORT_MIN_EV,
-		.master_max = CCI_REV_R0_MASTER_PORT_MAX_EV,
-	},
-	[CCI_REV_R1] = {
-		.slave_min = CCI_REV_R1_SLAVE_PORT_MIN_EV,
-		.slave_max = CCI_REV_R1_SLAVE_PORT_MAX_EV,
-		.master_min = CCI_REV_R1_MASTER_PORT_MIN_EV,
-		.master_max = CCI_REV_R1_MASTER_PORT_MAX_EV,
-	},
-};
-
-/*
- * Export different PMU names for the different revisions so userspace knows
- * because the event ids are different
- */
-static char *const pmu_names[] = {
-	[CCI_REV_R0] = "CCI_400",
-	[CCI_REV_R1] = "CCI_400_r1",
-};
-
-static int pmu_is_valid_slave_event(u8 ev_code)
-{
-	return pmu->port_ranges->slave_min <= ev_code &&
-		ev_code <= pmu->port_ranges->slave_max;
-}
-
-static int pmu_is_valid_master_event(u8 ev_code)
-{
-	return pmu->port_ranges->master_min <= ev_code &&
-		ev_code <= pmu->port_ranges->master_max;
-}
-
 static int pmu_validate_hw_event(u8 hw_event)
 {
 	u8 ev_source = CCI_PMU_EVENT_SOURCE(hw_event);
 	u8 ev_code = CCI_PMU_EVENT_CODE(hw_event);
+	int if_type;
 
 	switch (ev_source) {
 	case CCI_PORT_S0:
@@ -207,18 +184,22 @@ static int pmu_validate_hw_event(u8 hw_event)
 	case CCI_PORT_S3:
 	case CCI_PORT_S4:
 		/* Slave Interface */
-		if (pmu_is_valid_slave_event(ev_code))
-			return hw_event;
+		if_type = CCI_IF_SLAVE;
 		break;
 	case CCI_PORT_M0:
 	case CCI_PORT_M1:
 	case CCI_PORT_M2:
 		/* Master Interface */
-		if (pmu_is_valid_master_event(ev_code))
-			return hw_event;
+		if_type = CCI_IF_MASTER;
 		break;
+	default:
+		return -ENOENT;
 	}
 
+	if (ev_code >= pmu->model->event_ranges[if_type].min &&
+		ev_code <= pmu->model->event_ranges[if_type].max)
+		return hw_event;
+
 	return -ENOENT;
 }
 
@@ -234,11 +215,9 @@ static int probe_cci_revision(void)
 		return CCI_REV_R1;
 }
 
-static struct pmu_port_event_ranges *port_range_by_rev(void)
+static const struct cci_pmu_model *probe_cci_model(struct platform_device *pdev)
 {
-	int rev = probe_cci_revision();
-
-	return &port_event_range[rev];
+	return &cci_pmu_models[probe_cci_revision()];
 }
 
 static int pmu_is_valid_counter(struct cci_pmu *cci_pmu, int idx)
@@ -816,9 +795,9 @@ static const struct attribute_group *pmu_attr_groups[] = {
 
 static int cci_pmu_init(struct cci_pmu *cci_pmu, struct platform_device *pdev)
 {
-	char *name = pmu_names[probe_cci_revision()];
+	char *name = cci_pmu->model->name;
 	cci_pmu->pmu = (struct pmu) {
-		.name		= pmu_names[probe_cci_revision()],
+		.name		= cci_pmu->model->name,
 		.task_ctx_nr	= perf_invalid_context,
 		.pmu_enable	= cci_pmu_enable,
 		.pmu_disable	= cci_pmu_disable,
@@ -871,6 +850,35 @@ static struct notifier_block cci_pmu_cpu_nb = {
 	.priority	= CPU_PRI_PERF + 1,
 };
 
+static struct cci_pmu_model cci_pmu_models[] = {
+	[CCI_REV_R0] = {
+		.name = "CCI_400",
+		.event_ranges = {
+			[CCI_IF_SLAVE] = {
+				CCI_REV_R0_SLAVE_PORT_MIN_EV,
+				CCI_REV_R0_SLAVE_PORT_MAX_EV,
+			},
+			[CCI_IF_MASTER] = {
+				CCI_REV_R0_MASTER_PORT_MIN_EV,
+				CCI_REV_R0_MASTER_PORT_MAX_EV,
+			},
+		},
+	},
+	[CCI_REV_R1] = {
+		.name = "CCI_400_r1",
+		.event_ranges = {
+			[CCI_IF_SLAVE] = {
+				CCI_REV_R1_SLAVE_PORT_MIN_EV,
+				CCI_REV_R1_SLAVE_PORT_MAX_EV,
+			},
+			[CCI_IF_MASTER] = {
+				CCI_REV_R1_MASTER_PORT_MIN_EV,
+				CCI_REV_R1_MASTER_PORT_MAX_EV,
+			},
+		},
+	},
+};
+
 static const struct of_device_id arm_cci_pmu_matches[] = {
 	{
 		.compatible = "arm,cci-400-pmu",
@@ -878,6 +886,16 @@ static const struct of_device_id arm_cci_pmu_matches[] = {
 	{},
 };
 
+static inline const struct cci_pmu_model *get_cci_model(struct platform_device *pdev)
+{
+	const struct of_device_id *match = of_match_node(arm_cci_pmu_matches,
+							pdev->dev.of_node);
+	if (!match)
+		return NULL;
+
+	return probe_cci_model(pdev);
+}
+
 static bool is_duplicate_irq(int irq, int *irqs, int nr_irqs)
 {
 	int i;
@@ -893,11 +911,19 @@ static int cci_pmu_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	int i, ret, irq;
+	const struct cci_pmu_model *model;
+
+	model = get_cci_model(pdev);
+	if (!model) {
+		dev_warn(&pdev->dev, "CCI PMU version not supported\n");
+		return -ENODEV;
+	}
 
 	pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
 	if (!pmu)
 		return -ENOMEM;
 
+	pmu->model = model;
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	pmu->base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(pmu->base))
@@ -929,12 +955,6 @@ static int cci_pmu_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	pmu->port_ranges = port_range_by_rev();
-	if (!pmu->port_ranges) {
-		dev_warn(&pdev->dev, "CCI PMU version not supported\n");
-		return -EINVAL;
-	}
-
 	raw_spin_lock_init(&pmu->hw_events.pmu_lock);
 	mutex_init(&pmu->reserve_mutex);
 	atomic_set(&pmu->active_events, 0);
@@ -948,6 +968,7 @@ static int cci_pmu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	pr_info("ARM %s PMU driver probed", pmu->model->name);
 	return 0;
 }
 
-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/5] arm-cci: Abstract the CCI400 PMU speicific definitions
  2015-03-02 11:29 [PATCH v2 0/5] arm-cci400: PMU monitoring support on ARM64 Suzuki K. Poulose
@ 2015-03-02 11:29 ` Suzuki K. Poulose
  0 siblings, 0 replies; 24+ messages in thread
From: Suzuki K. Poulose @ 2015-03-02 11:29 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: nico, b.zolnierkie, kgene, a.kesavan, arnd, devicetree,
	linux-kernel, Liviu.Dudau, Lorenzo.Pieralisi, Pawel.Moll, olof,
	Sudeep.Holla, Punit.Agrawal, Will.Deacon, Catalin.Marinas,
	Suzuki K. Poulose

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

CCI400 has different event specifications for PMU, for revsion0 and
revision 1. As of now, we check the revision twice, for using the
parameters for the PMU. This patch abstracts the details of the pmu
models in a struct (cci_pmu_model) and stores the information in cci_pmu
at initialisation time, avoiding multiple probe operations.

This will also help us to add new PMU versions.

Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
---
 drivers/bus/arm-cci.c |  124 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 76 insertions(+), 48 deletions(-)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index f27cf56..bec014b 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -79,19 +79,38 @@ static const struct of_device_id arm_cci_matches[] = {
 
 #define CCI_PMU_MAX_HW_EVENTS 5   /* CCI PMU has 4 counters + 1 cycle counter */
 
+/* Types of interfaces that can generate events */
+enum {
+	CCI_IF_SLAVE,
+	CCI_IF_MASTER,
+	CCI_IF_MAX,
+};
+
+struct event_range {
+	u32 min;
+	u32 max;
+};
+
 struct cci_pmu_hw_events {
 	struct perf_event *events[CCI_PMU_MAX_HW_EVENTS];
 	unsigned long used_mask[BITS_TO_LONGS(CCI_PMU_MAX_HW_EVENTS)];
 	raw_spinlock_t pmu_lock;
 };
 
+struct cci_pmu_model {
+	char *name;
+	struct event_range event_ranges[CCI_IF_MAX];
+};
+
+static struct cci_pmu_model cci_pmu_models[];
+
 struct cci_pmu {
 	void __iomem *base;
 	struct pmu pmu;
 	int nr_irqs;
 	int irqs[CCI_PMU_MAX_HW_EVENTS];
 	unsigned long active_irqs;
-	struct pmu_port_event_ranges *port_ranges;
+	const struct cci_pmu_model *model;
 	struct cci_pmu_hw_events hw_events;
 	struct platform_device *plat_device;
 	int num_events;
@@ -152,47 +171,16 @@ enum cci400_perf_events {
 #define CCI_REV_R1_MASTER_PORT_MIN_EV	0x00
 #define CCI_REV_R1_MASTER_PORT_MAX_EV	0x11
 
-struct pmu_port_event_ranges {
-	u8 slave_min;
-	u8 slave_max;
-	u8 master_min;
-	u8 master_max;
-};
-
-static struct pmu_port_event_ranges port_event_range[] = {
-	[CCI_REV_R0] = {
-		.slave_min = CCI_REV_R0_SLAVE_PORT_MIN_EV,
-		.slave_max = CCI_REV_R0_SLAVE_PORT_MAX_EV,
-		.master_min = CCI_REV_R0_MASTER_PORT_MIN_EV,
-		.master_max = CCI_REV_R0_MASTER_PORT_MAX_EV,
-	},
-	[CCI_REV_R1] = {
-		.slave_min = CCI_REV_R1_SLAVE_PORT_MIN_EV,
-		.slave_max = CCI_REV_R1_SLAVE_PORT_MAX_EV,
-		.master_min = CCI_REV_R1_MASTER_PORT_MIN_EV,
-		.master_max = CCI_REV_R1_MASTER_PORT_MAX_EV,
-	},
-};
-
-/*
- * Export different PMU names for the different revisions so userspace knows
- * because the event ids are different
- */
-static char *const pmu_names[] = {
-	[CCI_REV_R0] = "CCI_400",
-	[CCI_REV_R1] = "CCI_400_r1",
-};
-
 static int pmu_is_valid_slave_event(u8 ev_code)
 {
-	return pmu->port_ranges->slave_min <= ev_code &&
-		ev_code <= pmu->port_ranges->slave_max;
+	return pmu->model->event_ranges[CCI_IF_SLAVE].min <= ev_code &&
+		ev_code <= pmu->model->event_ranges[CCI_IF_SLAVE].max;
 }
 
 static int pmu_is_valid_master_event(u8 ev_code)
 {
-	return pmu->port_ranges->master_min <= ev_code &&
-		ev_code <= pmu->port_ranges->master_max;
+	return pmu->model->event_ranges[CCI_IF_MASTER].min <= ev_code &&
+		ev_code <= pmu->model->event_ranges[CCI_IF_MASTER].max;
 }
 
 static int pmu_validate_hw_event(u8 hw_event)
@@ -234,11 +222,9 @@ static int probe_cci_revision(void)
 		return CCI_REV_R1;
 }
 
-static struct pmu_port_event_ranges *port_range_by_rev(void)
+static const struct cci_pmu_model *probe_cci_model(struct platform_device *pdev)
 {
-	int rev = probe_cci_revision();
-
-	return &port_event_range[rev];
+	return &cci_pmu_models[probe_cci_revision()];
 }
 
 static int pmu_is_valid_counter(struct cci_pmu *cci_pmu, int idx)
@@ -807,9 +793,9 @@ static const struct attribute_group *pmu_attr_groups[] = {
 
 static int cci_pmu_init(struct cci_pmu *cci_pmu, struct platform_device *pdev)
 {
-	char *name = pmu_names[probe_cci_revision()];
+	char *name = cci_pmu->model->name;
 	cci_pmu->pmu = (struct pmu) {
-		.name		= pmu_names[probe_cci_revision()],
+		.name		= cci_pmu->model->name,
 		.task_ctx_nr	= perf_invalid_context,
 		.pmu_enable	= cci_pmu_enable,
 		.pmu_disable	= cci_pmu_disable,
@@ -862,6 +848,35 @@ static struct notifier_block cci_pmu_cpu_nb = {
 	.priority	= CPU_PRI_PERF + 1,
 };
 
+static struct cci_pmu_model cci_pmu_models[] = {
+	[CCI_REV_R0] = {
+		.name = "CCI_400",
+		.event_ranges = {
+			[CCI_IF_SLAVE] = {
+				CCI_REV_R0_SLAVE_PORT_MIN_EV,
+				CCI_REV_R0_SLAVE_PORT_MAX_EV,
+			},
+			[CCI_IF_MASTER] = {
+				CCI_REV_R0_MASTER_PORT_MIN_EV,
+				CCI_REV_R0_MASTER_PORT_MAX_EV,
+			},
+		},
+	},
+	[CCI_REV_R1] = {
+		.name = "CCI_400_r1",
+		.event_ranges = {
+			[CCI_IF_SLAVE] = {
+				CCI_REV_R1_SLAVE_PORT_MIN_EV,
+				CCI_REV_R1_SLAVE_PORT_MAX_EV,
+			},
+			[CCI_IF_MASTER] = {
+				CCI_REV_R1_MASTER_PORT_MIN_EV,
+				CCI_REV_R1_MASTER_PORT_MAX_EV,
+			},
+		},
+	},
+};
+
 static const struct of_device_id arm_cci_pmu_matches[] = {
 	{
 		.compatible = "arm,cci-400-pmu",
@@ -869,6 +884,16 @@ static const struct of_device_id arm_cci_pmu_matches[] = {
 	{},
 };
 
+static inline const struct cci_pmu_model *get_cci_model(struct platform_device *pdev)
+{
+	const struct of_device_id *match = of_match_node(arm_cci_pmu_matches,
+							pdev->dev.of_node);
+	if (!match)
+		return NULL;
+
+	return probe_cci_model(pdev);
+}
+
 static bool is_duplicate_irq(int irq, int *irqs, int nr_irqs)
 {
 	int i;
@@ -884,11 +909,19 @@ static int cci_pmu_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	int i, ret, irq;
+	const struct cci_pmu_model *model;
+
+	model = get_cci_model(pdev);
+	if (!model) {
+		dev_warn(&pdev->dev, "CCI PMU version not supported\n");
+		return -EINVAL;
+	}
 
 	pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
 	if (!pmu)
 		return -ENOMEM;
 
+	pmu->model = model;
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	pmu->base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(pmu->base))
@@ -920,12 +953,6 @@ static int cci_pmu_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	pmu->port_ranges = port_range_by_rev();
-	if (!pmu->port_ranges) {
-		dev_warn(&pdev->dev, "CCI PMU version not supported\n");
-		return -EINVAL;
-	}
-
 	raw_spin_lock_init(&pmu->hw_events.pmu_lock);
 	mutex_init(&pmu->reserve_mutex);
 	atomic_set(&pmu->active_events, 0);
@@ -939,6 +966,7 @@ static int cci_pmu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	pr_info("ARM %s PMU driver probed", pmu->model->name);
 	return 0;
 }
 
-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2015-03-19 17:54 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-10 15:18 [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Suzuki K. Poulose
2015-03-10 15:18 ` [PATCH 1/5] arm-cci: Rearrange code for splitting PMU vs driver code Suzuki K. Poulose
2015-03-10 15:18 ` [PATCH 2/5] arm-cci: Abstract the CCI400 PMU speicific definitions Suzuki K. Poulose
2015-03-17 18:49   ` Will Deacon
2015-03-10 15:18 ` [PATCH 3/5] arm-cci: Get rid of secure transactions for PMU driver Suzuki K. Poulose
2015-03-17  9:51   ` [UPDATED] " Suzuki K. Poulose
2015-03-19 17:25     ` Mark Rutland
2015-03-19 17:32     ` Mark Rutland
2015-03-19 17:38       ` Sudeep Holla
2015-03-19 17:52         ` Suzuki K. Poulose
2015-03-19 17:54           ` Mark Rutland
2015-03-10 15:18 ` [PATCH 4/5] arm-cci: Split the code for PMU vs driver support Suzuki K. Poulose
2015-03-10 16:24   ` Sudeep Holla
2015-03-10 15:18 ` [PATCH 5/5] arm-cci: Fix CCI PMU event validation Suzuki K. Poulose
2015-03-17 18:52   ` Will Deacon
2015-03-10 16:09 ` [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Nicolas Pitre
2015-03-10 16:11   ` Suzuki K. Poulose
2015-03-10 16:21 ` Sudeep Holla
2015-03-10 16:24   ` Suzuki K. Poulose
2015-03-11 11:40 ` Punit Agrawal
2015-03-17 18:54 ` Will Deacon
2015-03-18 10:09   ` Suzuki K. Poulose
  -- strict thread matches above, loose matches on Subject: below --
2015-03-18 12:24 [PATCHv4 " Suzuki K. Poulose
2015-03-18 12:24 ` [PATCH 2/5] arm-cci: Abstract the CCI400 PMU speicific definitions Suzuki K. Poulose
2015-03-02 11:29 [PATCH v2 0/5] arm-cci400: PMU monitoring support on ARM64 Suzuki K. Poulose
2015-03-02 11:29 ` [PATCH 2/5] arm-cci: Abstract the CCI400 PMU speicific definitions Suzuki K. Poulose

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).