Skip to contentSkip to conversation

CCN fixes for 4.8

PATCH New Pawel Moll 2 participants 9 messages Aug 12, 2016 – Sep 2, 2016

lore
Share & tools
lore

Pawel Moll

author
#0 lore raw

cover CCN fixes for 4.8

Greetings, Arnd, Olof, Kevin,

This is a small set of various CCN driver fixes I accumulated over the last few weeks.

I'll send a pull request later, probably next week, that will also include Mark's patches he sent out yesterday.

Pawel Moll (5):
bus: arm-ccn: Fix PMU handling of MN
bus: arm-ccn: Do not attempt to configure XPs for cycle counter
bus: arm-ccn: Fix XP watchpoint settings bitmask
bus: arm-ccn: Correct required arguments for XP PMU events
bus: arm-ccn: Add missing event attribute exclusions for host/guest

 drivers/bus/arm-ccn.c | 42 +++++++++++++++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 9 deletions(-)
Signature · 1 line
2.7.4
author
#1 lore raw

PATCH 1/5 bus: arm-ccn: Fix PMU handling of MN

The "Miscellaneous Node" fell through cracks of node initialisation, as its ID is shared with HN-I.

This patch treats MN as a special case (which it is), adding separate validation check for it and pre-defining the node ID in relevant events descriptions. That way one can simply run:

	# perf stat -a -e ccn/mn_ecbarrier/ <workload>

Additionally, direction in the MN pseudo-events XP watchpoint definitions is corrected to be "TX" (1) as they are defined from the crosspoint point of view (thus barriers are transmitted from XP to MN).

Cc stable at vger.kernel.org # 3.17+
Signed-off-by Pawel Moll <[email protected]>
---
 drivers/bus/arm-ccn.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)
drivers/bus/arm-ccn.c +16 -3
index 97a9185..a11b9bb 100644
@@ -187,6 +187,7 @@ struct arm_ccn {
 	struct arm_ccn_component *xp;
 
 	struct arm_ccn_dt dt;
+	int mn_id;
 };
 
 static DEFINE_MUTEX(arm_ccn_mutex);

@@ -328,6 +329,7 @@ struct arm_ccn_pmu_event {
 static ssize_t arm_ccn_pmu_event_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
+	struct arm_ccn *ccn = pmu_to_arm_ccn(dev_get_drvdata(dev));
 	struct arm_ccn_pmu_event *event = container_of(attr,
 			struct arm_ccn_pmu_event, attr);
 	ssize_t res;

@@ -354,6 +356,9 @@ static ssize_t arm_ccn_pmu_event_show(struct device *dev,
 			res += snprintf(buf + res, PAGE_SIZE - res,
 					",cmp_l=?,cmp_h=?,mask=?");
 		break;
+	case CCN_TYPE_MN:
+		res += snprintf(buf + res, PAGE_SIZE - res, ",node=%d", ccn->mn_id);
+		break;
 	default:
 		res += snprintf(buf + res, PAGE_SIZE - res, ",node=?");
 		break;

@@ -383,9 +388,9 @@ static umode_t arm_ccn_pmu_events_is_visible(struct kobject *kobj,
 }
 
 static struct arm_ccn_pmu_event arm_ccn_pmu_events[] = {
-	CCN_EVENT_MN(eobarrier, "dir=0,vc=0,cmp_h=0x1c00", CCN_IDX_MASK_OPCODE),
-	CCN_EVENT_MN(ecbarrier, "dir=0,vc=0,cmp_h=0x1e00", CCN_IDX_MASK_OPCODE),
-	CCN_EVENT_MN(dvmop, "dir=0,vc=0,cmp_h=0x2800", CCN_IDX_MASK_OPCODE),
+	CCN_EVENT_MN(eobarrier, "dir=1,vc=0,cmp_h=0x1c00", CCN_IDX_MASK_OPCODE),
+	CCN_EVENT_MN(ecbarrier, "dir=1,vc=0,cmp_h=0x1e00", CCN_IDX_MASK_OPCODE),
+	CCN_EVENT_MN(dvmop, "dir=1,vc=0,cmp_h=0x2800", CCN_IDX_MASK_OPCODE),
 	CCN_EVENT_HNI(txdatflits, "dir=1,vc=3", CCN_IDX_MASK_ANY),
 	CCN_EVENT_HNI(rxdatflits, "dir=0,vc=3", CCN_IDX_MASK_ANY),
 	CCN_EVENT_HNI(txreqflits, "dir=1,vc=0", CCN_IDX_MASK_ANY),

@@ -759,6 +764,12 @@ static int arm_ccn_pmu_event_init(struct perf_event *event)
 
 	/* Validate node/xp vs topology */
 	switch (type) {
+	case CCN_TYPE_MN:
+		if (node_xp != ccn->mn_id) {
+			dev_warn(ccn->dev, "Invalid MN ID %d!\n", node_xp);
+			return -EINVAL;
+		}
+		break;
 	case CCN_TYPE_XP:
 		if (node_xp >= ccn->num_xps) {
 			dev_warn(ccn->dev, "Invalid XP ID %d!\n", node_xp);

@@ -1361,6 +1372,8 @@ static int arm_ccn_init_nodes(struct arm_ccn *ccn, int region,
 
 	switch (type) {
 	case CCN_TYPE_MN:
+		ccn->mn_id = id;
+		return 0;
 	case CCN_TYPE_DT:
 		return 0;
 	case CCN_TYPE_XP:
Signature · 1 line
2.7.4
author
#2 lore raw

PATCH 2/5 bus: arm-ccn: Do not attempt to configure XPs for cycle counter

In reply to Pawel Moll

Fuzzing the CCN perf driver revealed a small but definitely dangerous mistake in the event setup code. When a cycle counter is requested, the driver should not reconfigure the events bus at all, otherwise it will corrupt (in most but the simplest cases) its configuration and may end up accessing XP array out of its bounds.

Reported-by Mark Rutland <[email protected]>
Reviewed-by Mark Rutland <[email protected]>
Tested-by Mark Rutland <[email protected]>
Cc stable at vger.kernel.org # 3.17+
Signed-off-by Pawel Moll <[email protected]>
---
 drivers/bus/arm-ccn.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/bus/arm-ccn.c +4 -0
index a11b9bb..9bbb0ab 100644
@@ -897,6 +897,10 @@ static void arm_ccn_pmu_xp_dt_config(struct perf_event *event, int enable)
 	struct arm_ccn_component *xp;
 	u32 val, dt_cfg;
 
+	/* Nothing to do for cycle counter */
+	if (hw->idx == CCN_IDX_PMU_CYCLE_COUNTER)
+		return;
+
 	if (CCN_CONFIG_TYPE(event->attr.config) == CCN_TYPE_XP)
 		xp = &ccn->xp[CCN_CONFIG_XP(event->attr.config)];
 	else
Signature · 1 line
2.7.4
author
#3 lore raw

PATCH 3/5 bus: arm-ccn: Fix XP watchpoint settings bitmask

In reply to Pawel Moll

The code setting XP watchpoint comparator and mask registers should, in order to be fully compliant with specification, zero one or more most significant bits of each field. In both L cases it means zeroing bit 63. The bitmask doing this was wrong, though, zeroing bit 60 instead. Fortunately, due to a lucky coincidence, this turned out to be fairly innocent with the existing hardware.

Fixed now.

Cc stable at vger.kernel.org # 3.17+
Signed-off-by Pawel Moll <[email protected]>
---
 drivers/bus/arm-ccn.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/bus/arm-ccn.c +2 -2
index 9bbb0ab..647a27b 100644
@@ -1003,7 +1003,7 @@ static void arm_ccn_pmu_xp_watchpoint_config(struct perf_event *event)
 
 	/* Comparison values */
 	writel(cmp_l & 0xffffffff, source->base + CCN_XP_DT_CMP_VAL_L(wp));
-	writel((cmp_l >> 32) & 0xefffffff,
+	writel((cmp_l >> 32) & 0x7fffffff,
 			source->base + CCN_XP_DT_CMP_VAL_L(wp) + 4);
 	writel(cmp_h & 0xffffffff, source->base + CCN_XP_DT_CMP_VAL_H(wp));
 	writel((cmp_h >> 32) & 0x0fffffff,

@@ -1011,7 +1011,7 @@ static void arm_ccn_pmu_xp_watchpoint_config(struct perf_event *event)
 
 	/* Mask */
 	writel(mask_l & 0xffffffff, source->base + CCN_XP_DT_CMP_MASK_L(wp));
-	writel((mask_l >> 32) & 0xefffffff,
+	writel((mask_l >> 32) & 0x7fffffff,
 			source->base + CCN_XP_DT_CMP_MASK_L(wp) + 4);
 	writel(mask_h & 0xffffffff, source->base + CCN_XP_DT_CMP_MASK_H(wp));
 	writel((mask_h >> 32) & 0x0fffffff,
Signature · 1 line
2.7.4
author
#4 lore raw

PATCH 4/5 bus: arm-ccn: Correct required arguments for XP PMU events

In reply to Pawel Moll

XP can provide events from two sources: watchpoints, observing traffic on device ports and PMU looking at internal buses.

Unfortunately the sysfs definition of the PMU events was requiring port number (instead of bus number) and direction (the buses are unidirectional), as these fields were shared with the watchpoint event.

Although it does not introduce a major problem (port can be used as bus alias and direction is simply ignored for XP PMU events), it's better to fix it now, before external tools start depending on this behaviour.

Signed-off-by Pawel Moll <[email protected]>
---
 drivers/bus/arm-ccn.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
drivers/bus/arm-ccn.c +9 -3
index 647a27b..e7c2321 100644
@@ -213,6 +213,7 @@ static int arm_ccn_node_to_xp_port(int node)
 #define CCN_CONFIG_TYPE(_config)	(((_config) >> 8) & 0xff)
 #define CCN_CONFIG_EVENT(_config)	(((_config) >> 16) & 0xff)
 #define CCN_CONFIG_PORT(_config)	(((_config) >> 24) & 0x3)
+#define CCN_CONFIG_BUS(_config)		(((_config) >> 24) & 0x3)
 #define CCN_CONFIG_VC(_config)		(((_config) >> 26) & 0x7)
 #define CCN_CONFIG_DIR(_config)		(((_config) >> 29) & 0x1)
 #define CCN_CONFIG_MASK(_config)	(((_config) >> 30) & 0xf)

@@ -242,6 +243,7 @@ static CCN_FORMAT_ATTR(xp, "config:0-7");
 static CCN_FORMAT_ATTR(type, "config:8-15");
 static CCN_FORMAT_ATTR(event, "config:16-23");
 static CCN_FORMAT_ATTR(port, "config:24-25");
+static CCN_FORMAT_ATTR(bus, "config:24-25");
 static CCN_FORMAT_ATTR(vc, "config:26-28");
 static CCN_FORMAT_ATTR(dir, "config:29-29");
 static CCN_FORMAT_ATTR(mask, "config:30-33");

@@ -351,10 +353,14 @@ static ssize_t arm_ccn_pmu_event_show(struct device *dev,
 		break;
 	case CCN_TYPE_XP:
 		res += snprintf(buf + res, PAGE_SIZE - res,
-				",xp=?,port=?,vc=?,dir=?");
+				",xp=?,vc=?");
 		if (event->event == CCN_EVENT_WATCHPOINT)
 			res += snprintf(buf + res, PAGE_SIZE - res,
-					",cmp_l=?,cmp_h=?,mask=?");
+					",port=?,dir=?,cmp_l=?,cmp_h=?,mask=?");
+		else
+			res += snprintf(buf + res, PAGE_SIZE - res,
+					",bus=?");
+
 		break;
 	case CCN_TYPE_MN:
 		res += snprintf(buf + res, PAGE_SIZE - res, ",node=%d", ccn->mn_id);

@@ -1029,7 +1035,7 @@ static void arm_ccn_pmu_xp_event_config(struct perf_event *event)
 	hw->event_base = CCN_XP_DT_CONFIG__DT_CFG__XP_PMU_EVENT(hw->config_base);
 
 	id = (CCN_CONFIG_VC(event->attr.config) << 4) |
-			(CCN_CONFIG_PORT(event->attr.config) << 3) |
+			(CCN_CONFIG_BUS(event->attr.config) << 3) |
 			(CCN_CONFIG_EVENT(event->attr.config) << 0);
 
 	val = readl(source->base + CCN_XP_PMU_EVENT_SEL);
Signature · 1 line
2.7.4
author
#5 lore raw

PATCH 5/5 bus: arm-ccn: Add missing event attribute exclusions for host/guest

In reply to Pawel Moll

CCN PMUs have no knowledge into VM-related origins of the memory traffic, therefore can't handle requests for host-only or guest-only events. Add appropriate exclusions (they should have been there from the beginning).

Signed-off-by Pawel Moll <[email protected]>
---
 drivers/bus/arm-ccn.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/bus/arm-ccn.c +2 -1
index e7c2321..f2e099b 100644
@@ -744,7 +744,8 @@ static int arm_ccn_pmu_event_init(struct perf_event *event)
 
 	if (has_branch_stack(event) || event->attr.exclude_user ||
 			event->attr.exclude_kernel || event->attr.exclude_hv ||
-			event->attr.exclude_idle) {
+			event->attr.exclude_idle || event->attr.exclude_host ||
+			event->attr.exclude_guest) {
 		dev_warn(ccn->dev, "Can't exclude execution levels!\n");
 		return -EOPNOTSUPP;
 	}
Signature · 1 line
2.7.4
#6 lore raw

cover CCN fixes for 4.8

In reply to Pawel Moll
Quoted 8 lines · CEST Pawel Moll

Greetings, Arnd, Olof, Kevin,

This is a small set of various CCN driver fixes I accumulated over the last few weeks.

I'll send a pull request later, probably next week, that will also include Mark's patches he sent out yesterday.

This seems to have gotten lost, I picked it up into the fixes branch now.

Sorry for the delay.

Arnd

author
#7 lore raw

cover CCN fixes for 4.8

Quoted 8 lines · Arnd Bergmann
Quoted 7 lines · CEST Pawel Moll

Greetings, Arnd, Olof, Kevin,

This is a small set of various CCN driver fixes I accumulated over the
last few weeks.

Quoted 7 lines
Quoted 3 lines

I'll send a pull request later, probably next week, that will also include Mark's patches he sent out yesterday.

This seems to have gotten lost, I picked it up into the fixes branch now.

Sorry for the delay.

Cool, no problem - thanks!. Just want to make sure you pulled the latest version, as described in the v2 request:

http://www.spinics.net/lists/arm-kernel/msg526595.html

Cheers!

Pawe?

#8 lore raw

cover CCN fixes for 4.8

Quoted 15 lines · CEST Pawel Moll
Quoted 14 lines · Arnd Bergmann
Quoted 10 lines · CEST Pawel Moll

Greetings, Arnd, Olof, Kevin,

This is a small set of various CCN driver fixes I accumulated over the
last few weeks.

I'll send a pull request later, probably next week, that will also include Mark's patches he sent out yesterday.

This seems to have gotten lost, I picked it up into the fixes branch now.

Quoted 7 lines
Quoted 1 line

Sorry for the delay.

Cool, no problem - thanks!. Just want to make sure you pulled the latest version, as described in the v2 request:

http://www.spinics.net/lists/arm-kernel/msg526595.html

I took the wrong one and applied the patches from the original series manually.

I fixed it up now, thanks a lot for the heads-up!

Arnd