LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 07/10] selftests/powerpc: Dump MMCR2 as part of the EBB HW state
From: Michael Ellerman @ 2014-07-23  7:31 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: sam.bobroff
In-Reply-To: <1406100700-9702-1-git-send-email-mpe@ellerman.id.au>

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 tools/testing/selftests/powerpc/pmu/ebb/ebb.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/powerpc/pmu/ebb/ebb.c b/tools/testing/selftests/powerpc/pmu/ebb/ebb.c
index b7ee607c0fca..d7a72ce696b5 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/ebb.c
+++ b/tools/testing/selftests/powerpc/pmu/ebb/ebb.c
@@ -224,6 +224,7 @@ void dump_ebb_hw_state(void)
 
 	printf("HW state:\n"		\
 	       "MMCR0 0x%016x %s\n"	\
+	       "MMCR2 0x%016lx\n"	\
 	       "EBBHR 0x%016lx\n"	\
 	       "BESCR 0x%016llx %s\n"	\
 	       "PMC1  0x%016lx\n"	\
@@ -233,10 +234,11 @@ void dump_ebb_hw_state(void)
 	       "PMC5  0x%016lx\n"	\
 	       "PMC6  0x%016lx\n"	\
 	       "SIAR  0x%016lx\n",
-	       mmcr0, decode_mmcr0(mmcr0), mfspr(SPRN_EBBHR), bescr,
-	       decode_bescr(bescr), mfspr(SPRN_PMC1), mfspr(SPRN_PMC2),
-	       mfspr(SPRN_PMC3), mfspr(SPRN_PMC4), mfspr(SPRN_PMC5),
-	       mfspr(SPRN_PMC6), mfspr(SPRN_SIAR));
+	       mmcr0, decode_mmcr0(mmcr0), mfspr(SPRN_MMCR2),
+	       mfspr(SPRN_EBBHR), bescr, decode_bescr(bescr),
+	       mfspr(SPRN_PMC1), mfspr(SPRN_PMC2), mfspr(SPRN_PMC3),
+	       mfspr(SPRN_PMC4), mfspr(SPRN_PMC5), mfspr(SPRN_PMC6),
+	       mfspr(SPRN_SIAR));
 }
 
 void dump_ebb_state(void)
-- 
1.9.1

^ permalink raw reply related

* [PATCH 08/10] selftests/powerpc: Add cycles test with MMCR2 handling
From: Michael Ellerman @ 2014-07-23  7:31 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: sam.bobroff
In-Reply-To: <1406100700-9702-1-git-send-email-mpe@ellerman.id.au>

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 tools/testing/selftests/powerpc/pmu/ebb/Makefile   |  3 +-
 .../powerpc/pmu/ebb/cycles_with_mmcr2_test.c       | 91 ++++++++++++++++++++++
 2 files changed, 93 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/powerpc/pmu/ebb/cycles_with_mmcr2_test.c

diff --git a/tools/testing/selftests/powerpc/pmu/ebb/Makefile b/tools/testing/selftests/powerpc/pmu/ebb/Makefile
index 251447eafe49..3dc4332698cb 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/Makefile
+++ b/tools/testing/selftests/powerpc/pmu/ebb/Makefile
@@ -13,7 +13,8 @@ PROGS := reg_access_test event_attributes_test cycles_test	\
 	 close_clears_pmcc_test instruction_count_test		\
 	 fork_cleanup_test ebb_on_child_test			\
 	 ebb_on_willing_child_test back_to_back_ebbs_test	\
-	 lost_exception_test no_handler_test
+	 lost_exception_test no_handler_test			\
+	 cycles_with_mmcr2_test
 
 all: $(PROGS)
 
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_mmcr2_test.c b/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_mmcr2_test.c
new file mode 100644
index 000000000000..d43029b0800c
--- /dev/null
+++ b/tools/testing/selftests/powerpc/pmu/ebb/cycles_with_mmcr2_test.c
@@ -0,0 +1,91 @@
+/*
+ * Copyright 2014, Michael Ellerman, IBM Corp.
+ * Licensed under GPLv2.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+
+#include "ebb.h"
+
+
+/*
+ * Test of counting cycles while manipulating the user accessible bits in MMCR2.
+ */
+
+/* We use two values because the first freezes PMC1 and so we would get no EBBs */
+#define MMCR2_EXPECTED_1 0x4020100804020000UL /* (FC1P|FC2P|FC3P|FC4P|FC5P|FC6P) */
+#define MMCR2_EXPECTED_2 0x0020100804020000UL /* (     FC2P|FC3P|FC4P|FC5P|FC6P) */
+
+
+int cycles_with_mmcr2(void)
+{
+	struct event event;
+	uint64_t val, expected[2], actual;
+	int i;
+	bool bad_mmcr2;
+
+	event_init_named(&event, 0x1001e, "cycles");
+	event_leader_ebb_init(&event);
+
+	event.attr.exclude_kernel = 1;
+	event.attr.exclude_hv = 1;
+	event.attr.exclude_idle = 1;
+
+	FAIL_IF(event_open(&event));
+
+	ebb_enable_pmc_counting(1);
+	setup_ebb_handler(standard_ebb_callee);
+	ebb_global_enable();
+
+	FAIL_IF(ebb_event_enable(&event));
+
+	mtspr(SPRN_PMC1, pmc_sample_period(sample_period));
+
+	/* XXX Set of MMCR2 must be after enable */
+	expected[0] = MMCR2_EXPECTED_1;
+	expected[1] = MMCR2_EXPECTED_2;
+	i = 0;
+	bad_mmcr2 = false;
+
+	/* Make sure we loop until we take at least one EBB */
+	while ((ebb_state.stats.ebb_count < 20 && !bad_mmcr2) ||
+		ebb_state.stats.ebb_count < 1)
+	{
+		mtspr(SPRN_MMCR2, expected[i % 2]);
+
+		FAIL_IF(core_busy_loop());
+
+		val = mfspr(SPRN_MMCR2);
+		if (val != expected[i % 2]) {
+			bad_mmcr2 = true;
+			actual = val;
+		}
+
+		i++;
+	}
+
+	ebb_global_disable();
+	ebb_freeze_pmcs();
+
+	count_pmc(1, sample_period);
+
+	dump_ebb_state();
+
+	event_close(&event);
+
+	FAIL_IF(ebb_state.stats.ebb_count == 0);
+
+	if (bad_mmcr2)
+		printf("Bad MMCR2 value seen is 0x%lx\n", actual);
+
+	FAIL_IF(bad_mmcr2);
+
+	return 0;
+}
+
+int main(void)
+{
+	return test_harness(cycles_with_mmcr2, "cycles_with_mmcr2");
+}
-- 
1.9.1

^ permalink raw reply related

* [PATCH 09/10] selftests/powerpc: Add a routine for retrieving an AUXV entry
From: Michael Ellerman @ 2014-07-23  7:31 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: sam.bobroff
In-Reply-To: <1406100700-9702-1-git-send-email-mpe@ellerman.id.au>

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 tools/testing/selftests/powerpc/pmu/lib.c | 48 +++++++++++++++++++++++++++++++
 tools/testing/selftests/powerpc/pmu/lib.h |  1 +
 2 files changed, 49 insertions(+)

diff --git a/tools/testing/selftests/powerpc/pmu/lib.c b/tools/testing/selftests/powerpc/pmu/lib.c
index 11e26e8cf33a..9768dea37bf3 100644
--- a/tools/testing/selftests/powerpc/pmu/lib.c
+++ b/tools/testing/selftests/powerpc/pmu/lib.c
@@ -5,10 +5,15 @@
 
 #define _GNU_SOURCE	/* For CPU_ZERO etc. */
 
+#include <elf.h>
 #include <errno.h>
+#include <fcntl.h>
+#include <link.h>
 #include <sched.h>
 #include <setjmp.h>
 #include <stdlib.h>
+#include <sys/stat.h>
+#include <sys/types.h>
 #include <sys/wait.h>
 
 #include "utils.h"
@@ -250,3 +255,46 @@ out_close:
 out:
 	return rc;
 }
+
+static char auxv[4096];
+
+void *get_auxv_entry(int type)
+{
+	ElfW(auxv_t) *p;
+	void *result;
+	ssize_t num;
+	int fd;
+
+	fd = open("/proc/self/auxv", O_RDONLY);
+	if (fd == -1) {
+		perror("open");
+		return NULL;
+	}
+
+	result = NULL;
+
+	num = read(fd, auxv, sizeof(auxv));
+	if (num < 0) {
+		perror("read");
+		goto out;
+	}
+
+	if (num > sizeof(auxv)) {
+		printf("Overflowed auxv buffer\n");
+		goto out;
+	}
+
+	p = (ElfW(auxv_t) *)auxv;
+
+	while (p->a_type != AT_NULL) {
+		if (p->a_type == type) {
+			result = (void *)p->a_un.a_val;
+			break;
+		}
+
+		p++;
+	}
+out:
+	close(fd);
+	return result;
+}
diff --git a/tools/testing/selftests/powerpc/pmu/lib.h b/tools/testing/selftests/powerpc/pmu/lib.h
index ca5d72ae3be6..0f0339c8a6f6 100644
--- a/tools/testing/selftests/powerpc/pmu/lib.h
+++ b/tools/testing/selftests/powerpc/pmu/lib.h
@@ -29,6 +29,7 @@ extern int notify_parent(union pipe write_pipe);
 extern int notify_parent_of_error(union pipe write_pipe);
 extern pid_t eat_cpu(int (test_function)(void));
 extern bool require_paranoia_below(int level);
+extern void *get_auxv_entry(int type);
 
 struct addr_range {
 	uint64_t first, last;
-- 
1.9.1

^ permalink raw reply related

* [PATCH 10/10] selftests/powerpc: Add test of per-event excludes
From: Michael Ellerman @ 2014-07-23  7:31 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: sam.bobroff
In-Reply-To: <1406100700-9702-1-git-send-email-mpe@ellerman.id.au>

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 tools/testing/selftests/powerpc/pmu/Makefile       |   2 +-
 .../selftests/powerpc/pmu/per_event_excludes.c     | 114 +++++++++++++++++++++
 2 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/powerpc/pmu/per_event_excludes.c

diff --git a/tools/testing/selftests/powerpc/pmu/Makefile b/tools/testing/selftests/powerpc/pmu/Makefile
index 8595cfd7e41b..c9f4263906a5 100644
--- a/tools/testing/selftests/powerpc/pmu/Makefile
+++ b/tools/testing/selftests/powerpc/pmu/Makefile
@@ -1,7 +1,7 @@
 noarg:
 	$(MAKE) -C ../
 
-PROGS := count_instructions l3_bank_test
+PROGS := count_instructions l3_bank_test per_event_excludes
 EXTRA_SOURCES := ../harness.c event.c lib.c
 
 SUB_TARGETS = ebb
diff --git a/tools/testing/selftests/powerpc/pmu/per_event_excludes.c b/tools/testing/selftests/powerpc/pmu/per_event_excludes.c
new file mode 100644
index 000000000000..fddbbc9cae2f
--- /dev/null
+++ b/tools/testing/selftests/powerpc/pmu/per_event_excludes.c
@@ -0,0 +1,114 @@
+/*
+ * Copyright 2014, Michael Ellerman, IBM Corp.
+ * Licensed under GPLv2.
+ */
+
+#define _GNU_SOURCE
+
+#include <elf.h>
+#include <limits.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <string.h>
+#include <sys/prctl.h>
+
+#include "event.h"
+#include "lib.h"
+#include "utils.h"
+
+/*
+ * Test that per-event excludes work.
+ */
+
+static int per_event_excludes(void)
+{
+	struct event *e, events[4];
+	char *platform;
+	int i;
+
+	platform = (char *)get_auxv_entry(AT_BASE_PLATFORM);
+	FAIL_IF(!platform);
+	SKIP_IF(strcmp(platform, "power8") != 0);
+
+	/*
+	 * We need to create the events disabled, otherwise the running/enabled
+	 * counts don't match up.
+	 */
+	e = &events[0];
+	event_init_opts(e, PERF_COUNT_HW_INSTRUCTIONS,
+			PERF_TYPE_HARDWARE, "instructions");
+	e->attr.disabled = 1;
+
+	e = &events[1];
+	event_init_opts(e, PERF_COUNT_HW_INSTRUCTIONS,
+			PERF_TYPE_HARDWARE, "instructions(k)");
+	e->attr.disabled = 1;
+	e->attr.exclude_user = 1;
+	e->attr.exclude_hv = 1;
+
+	e = &events[2];
+	event_init_opts(e, PERF_COUNT_HW_INSTRUCTIONS,
+			PERF_TYPE_HARDWARE, "instructions(h)");
+	e->attr.disabled = 1;
+	e->attr.exclude_user = 1;
+	e->attr.exclude_kernel = 1;
+
+	e = &events[3];
+	event_init_opts(e, PERF_COUNT_HW_INSTRUCTIONS,
+			PERF_TYPE_HARDWARE, "instructions(u)");
+	e->attr.disabled = 1;
+	e->attr.exclude_hv = 1;
+	e->attr.exclude_kernel = 1;
+
+	FAIL_IF(event_open(&events[0]));
+
+	/*
+	 * The open here will fail if we don't have per event exclude support,
+	 * because the second event has an incompatible set of exclude settings
+	 * and we're asking for the events to be in a group.
+	 */
+	for (i = 1; i < 4; i++)
+		FAIL_IF(event_open_with_group(&events[i], events[0].fd));
+
+	/*
+	 * Even though the above will fail without per-event excludes we keep
+	 * testing in order to be thorough.
+	 */
+	prctl(PR_TASK_PERF_EVENTS_ENABLE);
+
+	/* Spin for a while */
+	for (i = 0; i < INT_MAX; i++)
+		asm volatile("" : : : "memory");
+
+	prctl(PR_TASK_PERF_EVENTS_DISABLE);
+
+	for (i = 0; i < 4; i++) {
+		FAIL_IF(event_read(&events[i]));
+		event_report(&events[i]);
+	}
+
+	/*
+	 * We should see that all events have enabled == running. That
+	 * shows that they were all on the PMU at once.
+	 */
+	for (i = 0; i < 4; i++)
+		FAIL_IF(events[i].result.running != events[i].result.enabled);
+
+	/*
+	 * We can also check that the result for instructions is >= all the
+	 * other counts. That's because it is counting all instructions while
+	 * the others are counting a subset.
+	 */
+	for (i = 1; i < 4; i++)
+		FAIL_IF(events[0].result.value < events[i].result.value);
+
+	for (i = 0; i < 4; i++)
+		event_close(&events[i]);
+
+	return 0;
+}
+
+int main(void)
+{
+	return test_harness(per_event_excludes, "per_event_excludes");
+}
-- 
1.9.1

^ permalink raw reply related

* RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
From: mihai.caraman @ 2014-07-23  8:24 UTC (permalink / raw)
  To: Alexander Graf
  Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org,
	kvm-ppc@vger.kernel.org
In-Reply-To: <53CED5D9.9010609@suse.de>

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBrdm0tcHBjLW93bmVyQHZnZXIu
a2VybmVsLm9yZyBbbWFpbHRvOmt2bS1wcGMtDQo+IG93bmVyQHZnZXIua2VybmVsLm9yZ10gT24g
QmVoYWxmIE9mIEFsZXhhbmRlciBHcmFmDQo+IFNlbnQ6IFdlZG5lc2RheSwgSnVseSAyMywgMjAx
NCAxMjoyMSBBTQ0KPiBUbzogQ2FyYW1hbiBNaWhhaSBDbGF1ZGl1LUIwMjAwOA0KPiBDYzoga3Zt
LXBwY0B2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnOw0KPiBr
dm1Admdlci5rZXJuZWwub3JnDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggdjUgNC81XSBLVk06IFBQ
QzogQWxvdyBrdm1wcGNfZ2V0X2xhc3RfaW5zdCgpIHRvIGZhaWwNCj4gDQo+IA0KPiBPbiAyMS4w
Ny4xNCAxMTo1OSwgbWloYWkuY2FyYW1hbkBmcmVlc2NhbGUuY29tIHdyb3RlOg0KPiA+PiAtLS0t
LU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+PiBGcm9tOiBMaW51eHBwYy1kZXYgW21haWx0bzps
aW51eHBwYy1kZXYtDQo+ID4+IGJvdW5jZXMrbWloYWkuY2FyYW1hbj1mcmVlc2NhbGUuY29tQGxp
c3RzLm96bGFicy5vcmddIE9uIEJlaGFsZiBPZg0KPiA+PiBtaWhhaS5jYXJhbWFuQGZyZWVzY2Fs
ZS5jb20NCj4gPj4gU2VudDogRnJpZGF5LCBKdWx5IDE4LCAyMDE0IDEyOjA2IFBNDQo+ID4+IFRv
OiBBbGV4YW5kZXIgR3JhZjsga3ZtLXBwY0B2Z2VyLmtlcm5lbC5vcmcNCj4gPj4gQ2M6IGxpbnV4
cHBjLWRldkBsaXN0cy5vemxhYnMub3JnOyBrdm1Admdlci5rZXJuZWwub3JnDQo+ID4+IFN1Ympl
Y3Q6IFJFOiBbUEFUQ0ggdjUgNC81XSBLVk06IFBQQzogQWxvdyBrdm1wcGNfZ2V0X2xhc3RfaW5z
dCgpIHRvDQo+IGZhaWwNCj4gPj4NCj4gPj4+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+
ID4+PiBGcm9tOiBBbGV4YW5kZXIgR3JhZiBbbWFpbHRvOmFncmFmQHN1c2UuZGVdDQo+ID4+PiBT
ZW50OiBUaHVyc2RheSwgSnVseSAxNywgMjAxNCA1OjIxIFBNDQo+ID4+PiBUbzogQ2FyYW1hbiBN
aWhhaSBDbGF1ZGl1LUIwMjAwODsga3ZtLXBwY0B2Z2VyLmtlcm5lbC5vcmcNCj4gPj4+IENjOiBr
dm1Admdlci5rZXJuZWwub3JnOyBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZw0KPiA+Pj4g
U3ViamVjdDogUmU6IFtQQVRDSCB2NSA0LzVdIEtWTTogUFBDOiBBbG93IGt2bXBwY19nZXRfbGFz
dF9pbnN0KCkgdG8NCj4gPj4gZmFpbA0KPiA+Pj4NCj4gPj4+IE9uIDE3LjA3LjE0IDEzOjIyLCBN
aWhhaSBDYXJhbWFuIHdyb3RlOg0KPiA+Pj4+IE9uIGJvb2szZSwgZ3Vlc3QgbGFzdCBpbnN0cnVj
dGlvbiBpcyByZWFkIG9uIHRoZSBleGl0IHBhdGggdXNpbmcNCj4gbG9hZA0KPiA+Pj4+IGV4dGVy
bmFsIHBpZCAobHdlcHgpIGRlZGljYXRlZCBpbnN0cnVjdGlvbi4gVGhpcyBsb2FkIG9wZXJhdGlv
biBtYXkNCj4gPj4+IGZhaWwNCj4gPj4+PiBkdWUgdG8gVExCIGV2aWN0aW9uIGFuZCBleGVjdXRl
LWJ1dC1ub3QtcmVhZCBlbnRyaWVzLg0KPiA+Pj4+DQo+ID4+Pj4gVGhpcyBwYXRjaCBsYXkgZG93
biB0aGUgcGF0aCBmb3IgYW4gYWx0ZXJuYXRpdmUgc29sdXRpb24gdG8gcmVhZCB0aGUNCj4gPj4+
IGd1ZXN0DQo+ID4+Pj4gbGFzdCBpbnN0cnVjdGlvbiwgYnkgYWxsb3dpbmcga3ZtcHBjX2dldF9s
YXRfaW5zdCgpIGZ1bmN0aW9uIHRvDQo+IGZhaWwuDQo+ID4+Pj4gQXJjaGl0ZWN0dXJlIHNwZWNp
ZmljIGltcGxtZW50YXRpb25zIG9mIGt2bXBwY19sb2FkX2xhc3RfaW5zdCgpIG1heQ0KPiA+Pj4g
cmVhZA0KPiA+Pj4+IGxhc3QgZ3Vlc3QgaW5zdHJ1Y3Rpb24gYW5kIGluc3RydWN0IHRoZSBlbXVs
YXRpb24gbGF5ZXIgdG8gcmUtDQo+IGV4ZWN1dGUNCj4gPj4+IHRoZQ0KPiA+Pj4+IGd1ZXN0IGlu
IGNhc2Ugb2YgZmFpbHVyZS4NCj4gPj4+Pg0KPiA+Pj4+IE1ha2Uga3ZtcHBjX2dldF9sYXN0X2lu
c3QoKSBkZWZpbml0aW9uIGNvbW1vbiBiZXR3ZWVuIGFyY2hpdGVjdHVyZXMuDQo+ID4+Pj4NCj4g
Pj4+PiBTaWduZWQtb2ZmLWJ5OiBNaWhhaSBDYXJhbWFuIDxtaWhhaS5jYXJhbWFuQGZyZWVzY2Fs
ZS5jb20+DQo+ID4+Pj4gLS0tDQo+ID4+IC4uLg0KPiA+Pg0KPiA+Pj4+IGRpZmYgLS1naXQgYS9h
cmNoL3Bvd2VycGMvaW5jbHVkZS9hc20va3ZtX3BwYy5oDQo+ID4+PiBiL2FyY2gvcG93ZXJwYy9p
bmNsdWRlL2FzbS9rdm1fcHBjLmgNCj4gPj4+PiBpbmRleCBlMmZkNWExLi43ZjljNjM0IDEwMDY0
NA0KPiA+Pj4+IC0tLSBhL2FyY2gvcG93ZXJwYy9pbmNsdWRlL2FzbS9rdm1fcHBjLmgNCj4gPj4+
PiArKysgYi9hcmNoL3Bvd2VycGMvaW5jbHVkZS9hc20va3ZtX3BwYy5oDQo+ID4+Pj4gQEAgLTQ3
LDYgKzQ3LDExIEBAIGVudW0gZW11bGF0aW9uX3Jlc3VsdCB7DQo+ID4+Pj4gICAgCUVNVUxBVEVf
RVhJVF9VU0VSLCAgICAvKiBlbXVsYXRpb24gcmVxdWlyZXMgZXhpdCB0byB1c2VyLQ0KPiA+PiBz
cGFjZSAqLw0KPiA+Pj4+ICAgIH07DQo+ID4+Pj4NCj4gPj4+PiArZW51bSBpbnN0cnVjdGlvbl90
eXBlIHsNCj4gPj4+PiArCUlOU1RfR0VORVJJQywNCj4gPj4+PiArCUlOU1RfU0MsCQkvKiBzeXN0
ZW0gY2FsbCAqLw0KPiA+Pj4+ICt9Ow0KPiA+Pj4+ICsNCj4gPj4+PiAgICBleHRlcm4gaW50IGt2
bXBwY192Y3B1X3J1bihzdHJ1Y3Qga3ZtX3J1biAqa3ZtX3J1biwgc3RydWN0DQo+IGt2bV92Y3B1
DQo+ID4+PiAqdmNwdSk7DQo+ID4+Pj4gICAgZXh0ZXJuIGludCBfX2t2bXBwY192Y3B1X3J1bihz
dHJ1Y3Qga3ZtX3J1biAqa3ZtX3J1biwgc3RydWN0DQo+ID4+IGt2bV92Y3B1DQo+ID4+PiAqdmNw
dSk7DQo+ID4+Pj4gICAgZXh0ZXJuIHZvaWQga3ZtcHBjX2hhbmRsZXJfaGlnaG1lbSh2b2lkKTsN
Cj4gPj4+PiBAQCAtNjIsNiArNjcsOSBAQCBleHRlcm4gaW50IGt2bXBwY19oYW5kbGVfc3RvcmUo
c3RydWN0IGt2bV9ydW4NCj4gKnJ1biwNCj4gPj4+IHN0cnVjdCBrdm1fdmNwdSAqdmNwdSwNCj4g
Pj4+PiAgICAJCQkgICAgICAgdTY0IHZhbCwgdW5zaWduZWQgaW50IGJ5dGVzLA0KPiA+Pj4+ICAg
IAkJCSAgICAgICBpbnQgaXNfZGVmYXVsdF9lbmRpYW4pOw0KPiA+Pj4+DQo+ID4+Pj4gK2V4dGVy
biBpbnQga3ZtcHBjX2xvYWRfbGFzdF9pbnN0KHN0cnVjdCBrdm1fdmNwdSAqdmNwdSwNCj4gPj4+
PiArCQkJCSBlbnVtIGluc3RydWN0aW9uX3R5cGUgdHlwZSwgdTMyICppbnN0KTsNCj4gPj4+PiAr
DQo+ID4+Pj4gICAgZXh0ZXJuIGludCBrdm1wcGNfZW11bGF0ZV9pbnN0cnVjdGlvbihzdHJ1Y3Qg
a3ZtX3J1biAqcnVuLA0KPiA+Pj4+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgc3RydWN0IGt2bV92Y3B1ICp2Y3B1KTsNCj4gPj4+PiAgICBleHRlcm4gaW50IGt2bXBw
Y19lbXVsYXRlX21taW8oc3RydWN0IGt2bV9ydW4gKnJ1biwgc3RydWN0DQo+IGt2bV92Y3B1DQo+
ID4+PiAqdmNwdSk7DQo+ID4+Pj4gQEAgLTIzNCw2ICsyNDIsMjMgQEAgc3RydWN0IGt2bXBwY19v
cHMgew0KPiA+Pj4+ICAgIGV4dGVybiBzdHJ1Y3Qga3ZtcHBjX29wcyAqa3ZtcHBjX2h2X29wczsN
Cj4gPj4+PiAgICBleHRlcm4gc3RydWN0IGt2bXBwY19vcHMgKmt2bXBwY19wcl9vcHM7DQo+ID4+
Pj4NCj4gPj4+PiArc3RhdGljIGlubGluZSBpbnQga3ZtcHBjX2dldF9sYXN0X2luc3Qoc3RydWN0
IGt2bV92Y3B1ICp2Y3B1LA0KPiA+Pj4+ICsJCQkJCWVudW0gaW5zdHJ1Y3Rpb25fdHlwZSB0eXBl
LCB1MzINCj4gKmluc3QpDQo+ID4+Pj4gK3sNCj4gPj4+PiArCWludCByZXQgPSBFTVVMQVRFX0RP
TkU7DQo+ID4+Pj4gKw0KPiA+Pj4+ICsJLyogTG9hZCB0aGUgaW5zdHJ1Y3Rpb24gbWFudWFsbHkg
aWYgaXQgZmFpbGVkIHRvIGRvIHNvIGluIHRoZQ0KPiA+Pj4+ICsJICogZXhpdCBwYXRoICovDQo+
ID4+Pj4gKwlpZiAodmNwdS0+YXJjaC5sYXN0X2luc3QgPT0gS1ZNX0lOU1RfRkVUQ0hfRkFJTEVE
KQ0KPiA+Pj4+ICsJCXJldCA9IGt2bXBwY19sb2FkX2xhc3RfaW5zdCh2Y3B1LCB0eXBlLCAmdmNw
dS0NCj4gPj4+PiBhcmNoLmxhc3RfaW5zdCk7DQo+ID4+Pj4gKw0KPiA+Pj4+ICsNCj4gPj4+PiAr
CSppbnN0ID0gKHJldCA9PSBFTVVMQVRFX0RPTkUgJiYga3ZtcHBjX25lZWRfYnl0ZXN3YXAodmNw
dSkpID8NCj4gPj4+PiArCQlzd2FiMzIodmNwdS0+YXJjaC5sYXN0X2luc3QpIDogdmNwdS0+YXJj
aC5sYXN0X2luc3Q7DQo+ID4+PiBUaGlzIG1ha2VzIGV2ZW4gbGVzcyBzZW5zZSB0aGFuIHRoZSBw
cmV2aW91cyB2ZXJzaW9uLiBFaXRoZXIgeW91DQo+IHRyZWF0DQo+ID4+PiBpbnN0IGFzICJkZWZp
bml0ZWx5IG92ZXJ3cml0dGVuIiBvciBhcyAicHJlc2VydmVzIHByZXZpb3VzIGRhdGEgb24NCj4g
Pj4+IGZhaWx1cmUiLg0KPiA+PiBCb3RoIHY0IGFuZCB2NSB2ZXJzaW9ucyB0cmVhdCBpbnN0IGFz
ICJkZWZpbml0ZWx5IG92ZXJ3cml0dGVuIi4NCj4gPj4NCj4gPj4+IFNvIGVpdGhlciB5b3UgdW5j
b25kaXRpb25hbGx5IHN3YXAgbGlrZSB5b3UgZGlkIGJlZm9yZQ0KPiA+PiBJZiB3ZSBtYWtlIGFi
c3RyYWN0aW9uIG9mIGl0cyBzeW1tZXRyeSwgS1ZNX0lOU1RfRkVUQ0hfRkFJTEVEIGlzDQo+IG9w
ZXJhdGVkDQo+ID4+IGluIGhvc3QgZW5kaWFubmVzcywgc28gaXQgZG9lc24ndCBuZWVkIGJ5dGUg
c3dhcC4NCj4gPj4NCj4gPj4gSSBhZ3JlZSB3aXRoIHlvdXIgcmVhc29uaW5nIGlmIGxhc3RfaW5z
dCBpcyBpbml0aWFsaXplZCBhbmQgY29tcGFyZWQNCj4gd2l0aA0KPiA+PiBkYXRhIGluIGd1ZXN0
IGVuZGlhbmVzcywgd2hpY2ggaXMgbm90IHRoZSBjYXNlIHlldCBmb3INCj4gPj4gS1ZNX0lOU1Rf
RkVUQ0hfRkFJTEVELg0KPiA+IEFsZXgsIGFyZSB5b3UgcmVseWluZyBvbiB0aGUgZmFjdCB0aGF0
IEtWTV9JTlNUX0ZFVENIX0ZBSUxFRCB2YWx1ZSBpcw0KPiBzeW1tZXRyaWNhbD8NCj4gPiBXaXRo
IGEgbm9uIHN5bW1ldHJpY2FsIHZhbHVlIGxpa2UgMHhERUFEQkVFRiwgYW5kIGNvbnNpZGVyaW5n
IGEgbGl0dGxlLQ0KPiBlbmRpYW4gZ3Vlc3QNCj4gPiBvbiBhIGJpZy1lbmRpYW4gaG9zdCwgd2Ug
bmVlZCB0byBmaXgga3ZtIGxvZ2ljIHRvIGluaXRpYWxpemUgYW5kDQo+IGNvbXBhcmUgbGFzdF9p
bnN0DQo+ID4gd2l0aCAweEVGQkVBRERFIHN3YXBlZCB2YWx1ZS4NCj4gPg0KPiA+IFlvdXIgc3Vn
Z2VzdGlvbiB0byB1bmNvbmRpdGlvbmFsbHkgc3dhcCBtYWtlcyBzZW5zZSBvbmx5IHdpdGggdGhl
IGFib3ZlDQo+IGZpeCwgb3RoZXJ3aXNlDQo+ID4gaW5zdCBtYXkgZW5kIHVwIHdpdGggMHhFRkJF
QURERSBzd2FwZWQgdmFsdWUgd2l0aCBpcyB3cm9uZy4NCj4gDQo+IE9ubHkgZm9yICppbnN0IHdo
aWNoIHdlIHdvdWxkIHRyZWF0IGFzICJ1bmRlZmluZWQiIGFmdGVyIHRoZSBmdW5jdGlvbg0KPiBy
ZXR1cm5lZCBFTVVMQVRFX0FHQUlOLiANCg0KUmlnaHQuIFdpdGggdGhpcyBkbyB5b3UgYWNrbm93
bGVkZ2UgdGhhdCB2NSAoZGVmaW5pdGVseSBvdmVyd3JpdHRlbiBhcHByb2FjaCkNCmlzIG9rPw0K
DQotTWlrZQ0K

^ permalink raw reply

* Re: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
From: Alexander Graf @ 2014-07-23  8:39 UTC (permalink / raw)
  To: mihai.caraman@freescale.com
  Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org,
	kvm-ppc@vger.kernel.org
In-Reply-To: <72a612351ec34db5a94cda4f4883e608@BY2PR03MB508.namprd03.prod.outlook.com>



Am 23.07.2014 um 10:24 schrieb "mihai.caraman@freescale.com" <mihai.caraman@=
freescale.com>:

>> -----Original Message-----
>> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
>> owner@vger.kernel.org] On Behalf Of Alexander Graf
>> Sent: Wednesday, July 23, 2014 12:21 AM
>> To: Caraman Mihai Claudiu-B02008
>> Cc: kvm-ppc@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
>> kvm@vger.kernel.org
>> Subject: Re: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail=

>>=20
>>=20
>> On 21.07.14 11:59, mihai.caraman@freescale.com wrote:
>>>> -----Original Message-----
>>>> From: Linuxppc-dev [mailto:linuxppc-dev-
>>>> bounces+mihai.caraman=3Dfreescale.com@lists.ozlabs.org] On Behalf Of
>>>> mihai.caraman@freescale.com
>>>> Sent: Friday, July 18, 2014 12:06 PM
>>>> To: Alexander Graf; kvm-ppc@vger.kernel.org
>>>> Cc: linuxppc-dev@lists.ozlabs.org; kvm@vger.kernel.org
>>>> Subject: RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to
>> fail
>>>>=20
>>>>> -----Original Message-----
>>>>> From: Alexander Graf [mailto:agraf@suse.de]
>>>>> Sent: Thursday, July 17, 2014 5:21 PM
>>>>> To: Caraman Mihai Claudiu-B02008; kvm-ppc@vger.kernel.org
>>>>> Cc: kvm@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
>>>>> Subject: Re: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to
>>>> fail
>>>>>=20
>>>>>> On 17.07.14 13:22, Mihai Caraman wrote:
>>>>>> On book3e, guest last instruction is read on the exit path using
>> load
>>>>>> external pid (lwepx) dedicated instruction. This load operation may
>>>>> fail
>>>>>> due to TLB eviction and execute-but-not-read entries.
>>>>>>=20
>>>>>> This patch lay down the path for an alternative solution to read the
>>>>> guest
>>>>>> last instruction, by allowing kvmppc_get_lat_inst() function to
>> fail.
>>>>>> Architecture specific implmentations of kvmppc_load_last_inst() may
>>>>> read
>>>>>> last guest instruction and instruct the emulation layer to re-
>> execute
>>>>> the
>>>>>> guest in case of failure.
>>>>>>=20
>>>>>> Make kvmppc_get_last_inst() definition common between architectures.
>>>>>>=20
>>>>>> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>>>>>> ---
>>>> ...
>>>>=20
>>>>>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h
>>>>> b/arch/powerpc/include/asm/kvm_ppc.h
>>>>>> index e2fd5a1..7f9c634 100644
>>>>>> --- a/arch/powerpc/include/asm/kvm_ppc.h
>>>>>> +++ b/arch/powerpc/include/asm/kvm_ppc.h
>>>>>> @@ -47,6 +47,11 @@ enum emulation_result {
>>>>>>       EMULATE_EXIT_USER,    /* emulation requires exit to user-
>>>> space */
>>>>>>   };
>>>>>>=20
>>>>>> +enum instruction_type {
>>>>>> +    INST_GENERIC,
>>>>>> +    INST_SC,        /* system call */
>>>>>> +};
>>>>>> +
>>>>>>   extern int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct
>> kvm_vcpu
>>>>> *vcpu);
>>>>>>   extern int __kvmppc_vcpu_run(struct kvm_run *kvm_run, struct
>>>> kvm_vcpu
>>>>> *vcpu);
>>>>>>   extern void kvmppc_handler_highmem(void);
>>>>>> @@ -62,6 +67,9 @@ extern int kvmppc_handle_store(struct kvm_run
>> *run,
>>>>> struct kvm_vcpu *vcpu,
>>>>>>                      u64 val, unsigned int bytes,
>>>>>>                      int is_default_endian);
>>>>>>=20
>>>>>> +extern int kvmppc_load_last_inst(struct kvm_vcpu *vcpu,
>>>>>> +                 enum instruction_type type, u32 *inst);
>>>>>> +
>>>>>>   extern int kvmppc_emulate_instruction(struct kvm_run *run,
>>>>>>                                         struct kvm_vcpu *vcpu);
>>>>>>   extern int kvmppc_emulate_mmio(struct kvm_run *run, struct
>> kvm_vcpu
>>>>> *vcpu);
>>>>>> @@ -234,6 +242,23 @@ struct kvmppc_ops {
>>>>>>   extern struct kvmppc_ops *kvmppc_hv_ops;
>>>>>>   extern struct kvmppc_ops *kvmppc_pr_ops;
>>>>>>=20
>>>>>> +static inline int kvmppc_get_last_inst(struct kvm_vcpu *vcpu,
>>>>>> +                    enum instruction_type type, u32
>> *inst)
>>>>>> +{
>>>>>> +    int ret =3D EMULATE_DONE;
>>>>>> +
>>>>>> +    /* Load the instruction manually if it failed to do so in the
>>>>>> +     * exit path */
>>>>>> +    if (vcpu->arch.last_inst =3D=3D KVM_INST_FETCH_FAILED)
>>>>>> +        ret =3D kvmppc_load_last_inst(vcpu, type, &vcpu-
>>>>>> arch.last_inst);
>>>>>> +
>>>>>> +
>>>>>> +    *inst =3D (ret =3D=3D EMULATE_DONE && kvmppc_need_byteswap(vcpu)=
) ?
>>>>>> +        swab32(vcpu->arch.last_inst) : vcpu->arch.last_inst;
>>>>> This makes even less sense than the previous version. Either you
>> treat
>>>>> inst as "definitely overwritten" or as "preserves previous data on
>>>>> failure".
>>>> Both v4 and v5 versions treat inst as "definitely overwritten".
>>>>=20
>>>>> So either you unconditionally swap like you did before
>>>> If we make abstraction of its symmetry, KVM_INST_FETCH_FAILED is
>> operated
>>>> in host endianness, so it doesn't need byte swap.
>>>>=20
>>>> I agree with your reasoning if last_inst is initialized and compared
>> with
>>>> data in guest endianess, which is not the case yet for
>>>> KVM_INST_FETCH_FAILED.
>>> Alex, are you relying on the fact that KVM_INST_FETCH_FAILED value is
>> symmetrical?
>>> With a non symmetrical value like 0xDEADBEEF, and considering a little-
>> endian guest
>>> on a big-endian host, we need to fix kvm logic to initialize and
>> compare last_inst
>>> with 0xEFBEADDE swaped value.
>>>=20
>>> Your suggestion to unconditionally swap makes sense only with the above
>> fix, otherwise
>>> inst may end up with 0xEFBEADDE swaped value with is wrong.
>>=20
>> Only for *inst which we would treat as "undefined" after the function
>> returned EMULATE_AGAIN.
>=20
> Right. With this do you acknowledge that v5 (definitely overwritten approa=
ch)
> is ok?

I think I'm starting to understand your logic of v5. You write fetch_failed i=
nto *inst unswapped if the fetch failed.

I think that's ok, but I definitely do not like the code flow - it's too har=
d to understand at a glimpse. Just rewrite it to swab at local variable leve=
l, preferably with if()s and comments what this is about and have a single u=
nconditional *inst =3D fetched_inst; at the end of the function.

Alex

^ permalink raw reply

* [PATCH 1/2] powerpc/powernv: Change BUG_ON to WARN_ON in elog code
From: Vasant Hegde @ 2014-07-23  9:22 UTC (permalink / raw)
  To: linuxppc-dev

We can continue to read the error log (up to MAX size) even if
we get the elog size more than MAX size. Hence change BUG_ON to
WARN_ON.

Also updated error message.

Reported-by: Gopesh Kumar Chaudhary <gopchaud@in.ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Acked-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/opal-elog.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal-elog.c b/arch/powerpc/platforms/powernv/opal-elog.c
index 10268c4..0ad533b 100644
--- a/arch/powerpc/platforms/powernv/opal-elog.c
+++ b/arch/powerpc/platforms/powernv/opal-elog.c
@@ -249,7 +249,7 @@ static void elog_work_fn(struct work_struct *work)
 
 	rc = opal_get_elog_size(&id, &size, &type);
 	if (rc != OPAL_SUCCESS) {
-		pr_err("ELOG: Opal log read failed\n");
+		pr_err("ELOG: OPAL log info read failed\n");
 		return;
 	}
 
@@ -257,7 +257,7 @@ static void elog_work_fn(struct work_struct *work)
 	log_id = be64_to_cpu(id);
 	elog_type = be64_to_cpu(type);
 
-	BUG_ON(elog_size > OPAL_MAX_ERRLOG_SIZE);
+	WARN_ON(elog_size > OPAL_MAX_ERRLOG_SIZE);
 
 	if (elog_size >= OPAL_MAX_ERRLOG_SIZE)
 		elog_size  =  OPAL_MAX_ERRLOG_SIZE;

^ permalink raw reply related

* [PATCH 2/2] powerpc/powernv: Improve error message
From: Vasant Hegde @ 2014-07-23  9:22 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20140723092236.4740.33763.stgit@hegdevasant.in.ibm.com>

Presently we only support initiating Service Processor dump from host.
Hence update sysfs message. Also update couple of other error/info
messages.

Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/opal-dump.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal-dump.c b/arch/powerpc/platforms/powernv/opal-dump.c
index 788a197..3e33e28 100644
--- a/arch/powerpc/platforms/powernv/opal-dump.c
+++ b/arch/powerpc/platforms/powernv/opal-dump.c
@@ -112,7 +112,7 @@ static ssize_t init_dump_show(struct dump_obj *dump_obj,
 			      struct dump_attribute *attr,
 			      char *buf)
 {
-	return sprintf(buf, "1 - initiate dump\n");
+	return sprintf(buf, "1 - initiate Service Processor(FSP) dump\n");
 }
 
 static int64_t dump_fips_init(uint8_t type)
@@ -121,7 +121,7 @@ static int64_t dump_fips_init(uint8_t type)
 
 	rc = opal_dump_init(type);
 	if (rc)
-		pr_warn("%s: Failed to initiate FipS dump (%d)\n",
+		pr_warn("%s: Failed to initiate FSP dump (%d)\n",
 			__func__, rc);
 	return rc;
 }
@@ -131,8 +131,12 @@ static ssize_t init_dump_store(struct dump_obj *dump_obj,
 			       const char *buf,
 			       size_t count)
 {
-	dump_fips_init(DUMP_TYPE_FSP);
-	pr_info("%s: Initiated FSP dump\n", __func__);
+	int rc;
+
+	rc = dump_fips_init(DUMP_TYPE_FSP);
+	if (rc == OPAL_SUCCESS)
+		pr_info("%s: Initiated FSP dump\n", __func__);
+
 	return count;
 }
 
@@ -297,7 +301,7 @@ static ssize_t dump_attr_read(struct file *filep, struct kobject *kobj,
 			 * and rely on userspace to ask us to try
 			 * again.
 			 */
-			pr_info("%s: Platform dump partially read.ID = 0x%x\n",
+			pr_info("%s: Platform dump partially read. ID = 0x%x\n",
 				__func__, dump->id);
 			return -EIO;
 		}

^ permalink raw reply related

* [PATCH 2/2] powerpc/powernv: Interface to add opal dump region
From: Vasant Hegde @ 2014-07-23  9:25 UTC (permalink / raw)
  To: linux-kernel; +Cc: linuxppc-dev
In-Reply-To: <20140723092506.5170.34633.stgit@hegdevasant.in.ibm.com>

PowerNV platform is capable of capturing host memory region when system
crashes (because of host/firmware). We have new OPAL API to register
memory region to be capture when system crashes.

This patch adds support for new API and also registers kernel log buffer.

Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/opal.h                   |   12 +++++
 arch/powerpc/platforms/powernv/Makefile           |    2 -
 arch/powerpc/platforms/powernv/opal-dump-region.c |   46 +++++++++++++++++++++
 arch/powerpc/platforms/powernv/opal-wrappers.S    |    1 
 arch/powerpc/platforms/powernv/opal.c             |    2 +
 5 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/platforms/powernv/opal-dump-region.c

diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 0da1dbd..6b9c01e 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -147,6 +147,7 @@ struct opal_sg_list {
 #define OPAL_SET_PARAM				90
 #define OPAL_DUMP_RESEND			91
 #define OPAL_DUMP_INFO2				94
+#define OPAL_REGISTER_DUMP_REGION		98
 
 #ifndef __ASSEMBLY__
 
@@ -860,6 +861,7 @@ int64_t opal_get_param(uint64_t token, uint32_t param_id, uint64_t buffer,
 int64_t opal_set_param(uint64_t token, uint32_t param_id, uint64_t buffer,
 		uint64_t length);
 int64_t opal_sensor_read(uint32_t sensor_hndl, int token, __be32 *sensor_data);
+int64_t opal_register_dump_region(uint32_t id, uint64_t start, uint64_t end);
 
 /* Internal functions */
 extern int early_init_dt_scan_opal(unsigned long node, const char *uname,
@@ -899,6 +901,7 @@ extern int opal_elog_init(void);
 extern void opal_platform_dump_init(void);
 extern void opal_sys_param_init(void);
 extern void opal_msglog_init(void);
+extern void opal_dump_region_init(void);
 
 extern int opal_machine_check(struct pt_regs *regs);
 extern bool opal_mce_check_early_recovery(struct pt_regs *regs);
@@ -912,6 +915,15 @@ struct opal_sg_list *opal_vmalloc_to_sg_list(void *vmalloc_addr,
 					     unsigned long vmalloc_size);
 void opal_free_sg_list(struct opal_sg_list *sg);
 
+/*
+ * Dump regions ids
+ *   0x01 - 0x7F : OPAL
+ *   0x80 - 0xFF : Kernel
+ */
+#define DUMP_REGION_HOST_START		0x80
+#define DUMP_REGION_LOG_BUF		0x80
+#define DUMP_REGION_HOST_END		0xFF
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __OPAL_H */
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index 4ad227d..e2819aa 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -1,7 +1,7 @@
 obj-y			+= setup.o opal-wrappers.o opal.o opal-async.o
 obj-y			+= opal-rtc.o opal-nvram.o opal-lpc.o opal-flash.o
 obj-y			+= rng.o opal-elog.o opal-dump.o opal-sysparam.o opal-sensor.o
-obj-y			+= opal-msglog.o
+obj-y			+= opal-msglog.o opal-dump-region.o
 
 obj-$(CONFIG_SMP)	+= smp.o subcore.o subcore-asm.o
 obj-$(CONFIG_PCI)	+= pci.o pci-p5ioc2.o pci-ioda.o
diff --git a/arch/powerpc/platforms/powernv/opal-dump-region.c b/arch/powerpc/platforms/powernv/opal-dump-region.c
new file mode 100644
index 0000000..2cce388
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-dump-region.c
@@ -0,0 +1,46 @@
+/*
+ * PowerNV OPAL dump memory region interface
+ *
+ * Copyright 2014 IBM Corp.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/pagemap.h>
+#include <linux/printk.h>
+#include <asm/opal.h>
+
+/*
+ * Pass kernel address range (start, size) that will be included in
+ * system dump (collected by service processor whenever system crashes).
+ *
+ * @id		: Memory region ID (see opal.h)
+ * @addr	: Start address
+ * @size	: size in bytes
+ */
+static int add_dump_region_entry(uint32_t id, void *addr, uint64_t size)
+{
+	__be32 be_id = cpu_to_be32(id);
+	__be64 be_addr = cpu_to_be64(__pa(addr));
+	__be64 be_size = cpu_to_be64(size);
+
+	return opal_register_dump_region(be_id, be_addr, be_size);
+}
+
+void __init opal_dump_region_init(void)
+{
+	void *addr;
+	uint64_t size;
+	int rc;
+
+	/* Register kernel log buffer */
+	addr = get_log_buf_addr();
+	size = get_log_buf_len();
+	rc = add_dump_region_entry(DUMP_REGION_LOG_BUF, addr, size);
+	if (rc)
+		pr_warn("DUMP: Failed to register kernel log buffer. "
+			"rc = %d\n", rc);
+}
diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
index 4abbff2..935c30a 100644
--- a/arch/powerpc/platforms/powernv/opal-wrappers.S
+++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
@@ -146,3 +146,4 @@ OPAL_CALL(opal_sync_host_reboot,		OPAL_SYNC_HOST_REBOOT);
 OPAL_CALL(opal_sensor_read,			OPAL_SENSOR_READ);
 OPAL_CALL(opal_get_param,			OPAL_GET_PARAM);
 OPAL_CALL(opal_set_param,			OPAL_SET_PARAM);
+OPAL_CALL(opal_register_dump_region,		OPAL_REGISTER_DUMP_REGION);
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 1999756..2834303 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -616,6 +616,8 @@ static int __init opal_init(void)
 	/* Create "opal" kobject under /sys/firmware */
 	rc = opal_sysfs_init();
 	if (rc == 0) {
+		/* Setup dump region interface */
+		opal_dump_region_init();
 		/* Setup error log interface */
 		rc = opal_elog_init();
 		/* Setup code update interface */

^ permalink raw reply related

* [PATCH 1/2] printk: Add function to return log buffer address and size
From: Vasant Hegde @ 2014-07-23  9:25 UTC (permalink / raw)
  To: linux-kernel; +Cc: linuxppc-dev

Platforms like IBM Power Systems supports service processor
assisted dump. It provides interface to add memory region to
be captured when system is crashed.

During initialization/running we can add kernel memory region
to be collected.

Presently we don't have a way to get the log buffer base address
and size. This patch adds support to return log buffer address
and size.


Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>

---
Next patch extends arch specific code to add log buffer to platform
dump.

 include/linux/printk.h |    3 +++
 kernel/printk/printk.c |   12 ++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 319ff7e..aae82c4 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -10,6 +10,9 @@
 extern const char linux_banner[];
 extern const char linux_proc_banner[];
 
+extern void *get_log_buf_addr(void);
+extern u32 get_log_buf_len(void);
+
 static inline int printk_get_level(const char *buffer)
 {
 	if (buffer[0] == KERN_SOH_ASCII && buffer[1]) {
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 13e839d..4049f7b 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -270,6 +270,18 @@ static char __log_buf[__LOG_BUF_LEN] __aligned(LOG_ALIGN);
 static char *log_buf = __log_buf;
 static u32 log_buf_len = __LOG_BUF_LEN;
 
+/* Return log buffer address */
+void *get_log_buf_addr(void)
+{
+	return log_buf;
+}
+
+/* Return log buffer size */
+u32 get_log_buf_len(void)
+{
+	return log_buf_len;
+}
+
 /* human readable text of the record */
 static char *log_text(const struct printk_log *msg)
 {

^ permalink raw reply related

* Re: [PATCH] Add support for hardware threads on e6500.
From: Tudor Laurentiu @ 2014-07-23  9:34 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20140718214022.GA18530@home.buserror.net>

On 07/19/2014 12:40 AM, Scott Wood wrote:
> From: Andy Fleming <afleming@freescale.com>
>
> The general idea is that each core will release all of its
> threads into the secondary thread startup code, which will
> eventually wait in the secondary core holding area, for the
> appropriate bit in the PACA to be set. The kick_cpu function
> pointer will set that bit in the PACA, and thus "release"
> the core/thread to boot. We also need to do a few things that
> U-Boot normally does for CPUs (like enable branch prediction).
>
> Signed-off-by: Andy Fleming <afleming@freescale.com>
> [scottwood@freescale.com: various changes, including only enabling
>   threads if Linux wants to kick them]
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
>   arch/powerpc/include/asm/cputable.h   |  2 +-
>   arch/powerpc/include/asm/ppc-opcode.h |  9 ++++++++
>   arch/powerpc/include/asm/reg_booke.h  | 27 ++++++++++++++++++++++
>   arch/powerpc/kernel/head_64.S         | 22 ++++++++++++++++++
>   arch/powerpc/kernel/prom.c            | 10 ++++-----
>   arch/powerpc/kernel/setup-common.c    |  6 +++--
>   arch/powerpc/kernel/setup_64.c        |  6 ++++-
>   arch/powerpc/platforms/85xx/smp.c     | 42 +++++++++++++++++++++++++++++++++++
>   8 files changed, 114 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
> index bc23477..e91dec8 100644
> --- a/arch/powerpc/include/asm/cputable.h
> +++ b/arch/powerpc/include/asm/cputable.h
> @@ -396,7 +396,7 @@ extern const char *powerpc_base_platform;
>   	    CPU_FTR_L2CSR | CPU_FTR_LWSYNC | CPU_FTR_NOEXECUTE | \
>   	    CPU_FTR_DBELL | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
>   	    CPU_FTR_DEBUG_LVL_EXC | CPU_FTR_EMB_HV | CPU_FTR_ALTIVEC_COMP | \
> -	    CPU_FTR_CELL_TB_BUG)
> +	    CPU_FTR_CELL_TB_BUG | CPU_FTR_SMT)
>   #define CPU_FTRS_GENERIC_32	(CPU_FTR_COMMON | CPU_FTR_NODSISRALIGN)
>
>   /* 64-bit CPUs */
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index 3132bb9..e316dad 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -150,8 +150,10 @@
>   #define PPC_INST_MCRXR_MASK		0xfc0007fe
>   #define PPC_INST_MFSPR_PVR		0x7c1f42a6
>   #define PPC_INST_MFSPR_PVR_MASK		0xfc1fffff
> +#define PPC_INST_MFTMR			0x7c0002dc
>   #define PPC_INST_MSGSND			0x7c00019c
>   #define PPC_INST_MSGSNDP		0x7c00011c
> +#define PPC_INST_MTTMR			0x7c0003dc
>   #define PPC_INST_NOP			0x60000000
>   #define PPC_INST_POPCNTB		0x7c0000f4
>   #define PPC_INST_POPCNTB_MASK		0xfc0007fe
> @@ -369,4 +371,11 @@
>   #define TABORT(r)		stringify_in_c(.long PPC_INST_TABORT \
>   					       | __PPC_RA(r))
>
> +/* book3e thread control instructions */
> +#define TMRN(x)			((((x) & 0x1f) << 16) | (((x) & 0x3e0) << 6))
> +#define MTTMR(tmr, r)		stringify_in_c(.long PPC_INST_MTTMR | \
> +					       TMRN(tmr) | ___PPC_RS(r))
> +#define MFTMR(tmr, r)		stringify_in_c(.long PPC_INST_MFTMR | \
> +					       TMRN(tmr) | ___PPC_RT(r))
> +
>   #endif /* _ASM_POWERPC_PPC_OPCODE_H */
> diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
> index 6148292..cc5f395 100644
> --- a/arch/powerpc/include/asm/reg_booke.h
> +++ b/arch/powerpc/include/asm/reg_booke.h
> @@ -15,6 +15,8 @@
>   #ifndef __ASM_POWERPC_REG_BOOKE_H__
>   #define __ASM_POWERPC_REG_BOOKE_H__
>
> +#include <asm/ppc-opcode.h>
> +
>   /* Machine State Register (MSR) Fields */
>   #define MSR_GS		(1<<28) /* Guest state */
>   #define MSR_UCLE	(1<<26)	/* User-mode cache lock enable */
> @@ -598,6 +600,13 @@
>   /* Bit definitions for L1CSR2. */
>   #define L1CSR2_DCWS	0x40000000	/* Data Cache write shadow */
>
> +/* Bit definitions for BUCSR. */
> +#define BUCSR_STAC_EN	0x01000000	/* Segment Target Address Cache */
> +#define BUCSR_LS_EN	0x00400000	/* Link Stack */
> +#define BUCSR_BBFI	0x00000200	/* Branch Buffer flash invalidate */
> +#define BUCSR_BPEN	0x00000001	/* Branch prediction enable */
> +#define BUCSR_INIT	(BUCSR_STAC_EN | BUCSR_LS_EN | BUCSR_BBFI | BUCSR_BPEN)
> +
>   /* Bit definitions for L2CSR0. */
>   #define L2CSR0_L2E	0x80000000	/* L2 Cache Enable */
>   #define L2CSR0_L2PE	0x40000000	/* L2 Cache Parity/ECC Enable */
> @@ -721,5 +730,23 @@
>   #define MMUBE1_VBE4		0x00000002
>   #define MMUBE1_VBE5		0x00000001
>
> +#define TMRN_IMSR0	0x120	/* Initial MSR Register 0 (e6500) */
> +#define TMRN_IMSR1	0x121	/* Initial MSR Register 1 (e6500) */
> +#define TMRN_INIA0	0x140	/* Next Instruction Address Register 0 */
> +#define TMRN_INIA1	0x141	/* Next Instruction Address Register 1 */
> +#define SPRN_TENSR	0x1b5	/* Thread Enable Status Register */
> +#define SPRN_TENS	0x1b6	/* Thread Enable Set Register */
> +#define SPRN_TENC	0x1b7	/* Thread Enable Clear Register */
> +
> +#define TEN_THREAD(x)	(1 << (x))
> +
> +#ifndef __ASSEMBLY__
> +#define mftmr(rn)	({unsigned long rval; \
> +			asm volatile(MFTMR(rn, %0) : "=r" (rval)); rval;})
> +#define mttmr(rn, v)	asm volatile(MTTMR(rn, %0) : \
> +				     : "r" ((unsigned long)(v)) \
> +				     : "memory")
> +#endif /* !__ASSEMBLY__ */
> +
>   #endif /* __ASM_POWERPC_REG_BOOKE_H__ */
>   #endif /* __KERNEL__ */
> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> index a95145d..36ff6f0 100644
> --- a/arch/powerpc/kernel/head_64.S
> +++ b/arch/powerpc/kernel/head_64.S
> @@ -180,6 +180,28 @@ exception_marker:
>   #include "exceptions-64s.S"
>   #endif
>
> +#ifdef CONFIG_PPC_BOOK3E
> +_GLOBAL(fsl_secondary_thread_init)
> +	/* Enable branch prediction */
> +	lis     r3,BUCSR_INIT@h
> +	ori     r3,r3,BUCSR_INIT@l
> +	mtspr   SPRN_BUCSR,r3
> +	isync
> +
> +	/*
> +	 * Fix PIR to match the linear numbering in the device tree.
> +	 *
> +	 * On e6500, the reset value of PIR uses the low three bits for
> +	 * the thread within a core, and the upper bits for the core
> +	 * number.  There are two threads per core, so shift everything
> +	 * but the low bit right by two bits so that the cpu numbering is
> +	 * continuous.
> +	 */
> +	mfspr	r3, SPRN_PIR
> +	rlwimi	r3, r3, 30, 2, 30
> +	mtspr	SPRN_PIR, r3
> +#endif
> +
>   _GLOBAL(generic_secondary_thread_init)
>   	mr	r24,r3
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 613a860..0448b1e 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -309,12 +309,10 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
>
>   	/* Get physical cpuid */
>   	intserv = of_get_flat_dt_prop(node, "ibm,ppc-interrupt-server#s", &len);
> -	if (intserv) {
> -		nthreads = len / sizeof(int);
> -	} else {
> -		intserv = of_get_flat_dt_prop(node, "reg", NULL);
> -		nthreads = 1;
> -	}
> +	if (!intserv)
> +		intserv = of_get_flat_dt_prop(node, "reg", &len);
> +
> +	nthreads = len / sizeof(int);
>
>   	/*
>   	 * Now see if any of these threads match our boot cpu.
> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> index e5b022c..1b0e260 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -456,18 +456,20 @@ void __init smp_setup_cpu_maps(void)
>   		intserv = of_get_property(dn, "ibm,ppc-interrupt-server#s",
>   				&len);
>   		if (intserv) {
> -			nthreads = len / sizeof(int);
>   			DBG("    ibm,ppc-interrupt-server#s -> %d threads\n",
>   			    nthreads);
>   		} else {
>   			DBG("    no ibm,ppc-interrupt-server#s -> 1 thread\n");
> -			intserv = of_get_property(dn, "reg", NULL);
> +			intserv = of_get_property(dn, "reg", &len);
>   			if (!intserv) {
>   				cpu_be = cpu_to_be32(cpu);
>   				intserv = &cpu_be;	/* assume logical == phys */
> +				len = 4;
>   			}
>   		}
>
> +		nthreads = len / sizeof(int);
> +
>   		for (j = 0; j < nthreads && cpu < nr_cpu_ids; j++) {
>   			bool avail;
>
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index ee082d7..6d06947 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -507,7 +507,11 @@ void __init setup_system(void)
>   	check_smt_enabled();
>   	setup_tlb_core_data();
>
> -#ifdef CONFIG_SMP
> +	/*
> +	 * Freescale Book3e parts spin in a loop provided by firmware,
> +	 * so smp_release_cpus() does nothing for them
> +	 */
> +#if defined(CONFIG_SMP) && !defined(CONFIG_PPC_FSL_BOOK3E)
>   	/* Release secondary cpus out of their spinloops at 0x60 now that
>   	 * we can map physical -> logical CPU ids
>   	 */
> diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
> index ba093f5..d7111d9 100644
> --- a/arch/powerpc/platforms/85xx/smp.c
> +++ b/arch/powerpc/platforms/85xx/smp.c
> @@ -28,6 +28,7 @@
>   #include <asm/dbell.h>
>   #include <asm/fsl_guts.h>
>   #include <asm/code-patching.h>
> +#include <asm/cputhreads.h>
>
>   #include <sysdev/fsl_soc.h>
>   #include <sysdev/mpic.h>
> @@ -168,6 +169,22 @@ static inline u32 read_spin_table_addr_l(void *spin_table)
>   	return in_be32(&((struct epapr_spin_table *)spin_table)->addr_l);
>   }
>
> +static void wake_hw_thread(void *info)
> +{
> +	void fsl_secondary_thread_init(void);
> +	unsigned long imsr1, inia1;
> +	int nr = *(const int *)info;
> +
> +	imsr1 = (u32)MSR_KERNEL;
> +	inia1 = *(unsigned long *)fsl_secondary_thread_init;
> +
> +	mttmr(TMRN_IMSR1, imsr1);
> +	mttmr(TMRN_INIA1, inia1);
> +	mtspr(SPRN_TENS, TEN_THREAD(1));
> +
> +	smp_generic_kick_cpu(nr);
> +}

This needs to be wrapped in an #ifdef CONFIG_PPC64 / #endif otherwise 
you get:

arch/powerpc/platforms/85xx/smp.c:172:13: error: 'wake_hw_thread' 
defined but not used

---
Best Regards, Laurentiu

^ permalink raw reply

* Re: [PATCH 1/2] ASoC: fsl_sai: Reset FIFOs after disabling TE/RE
From: Nicolin Chen @ 2014-07-23  9:52 UTC (permalink / raw)
  To: broonie
  Cc: alsa-devel, b42378, b02247, linux-kernel, timur, Li.Xiubo,
	linuxppc-dev
In-Reply-To: <20140718101810.GA15150@MrMyself>

Sir,

I found this two patches are merged into for-next branch, although I haven't
got the 'applied' email. 

Is that possible for you to drop this one? If not, I'll send another patch
to fix this.

Thank you,
Nicolin

On Fri, Jul 18, 2014 at 06:18:12PM +0800, Nicolin Chen wrote:
> Mark,
> 
> 	Please disregard this single patch. 
> 
> On Thu, Jul 17, 2014 at 09:21:37PM +0800, Nicolin Chen wrote:
> > SAI will not clear their FIFOs after disabling TE/RE. Therfore, the driver
> > should take care the task so as not to let useless data remain in the FIFO.
> > 
> > Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
> > ---
> >  sound/soc/fsl/fsl_sai.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > index c5a0e8a..b10dbd8 100644
> > --- a/sound/soc/fsl/fsl_sai.c
> > +++ b/sound/soc/fsl/fsl_sai.c
> > @@ -371,10 +371,13 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
> >  
> >  		/* Check if the opposite FRDE is also disabled */
> >  		if (!(tx ? rcsr & FSL_SAI_CSR_FRDE : tcsr & FSL_SAI_CSR_FRDE)) {
> > +			/* Disable both directions and reset their FIFOs */
> >  			regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
> > -					   FSL_SAI_CSR_TERE, 0);
> > +					   FSL_SAI_CSR_TERE | FSL_SAI_CSR_FR,
> > +					   FSL_SAI_CSR_FR);
> >  			regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
> > -					   FSL_SAI_CSR_TERE, 0);
> > +					   FSL_SAI_CSR_TERE | FSL_SAI_CSR_FR,
> > +					   FSL_SAI_CSR_FR);
> 
> 
> The FR should be set _after_ clear TERE, not at the same time because it
> still may have tiny possibility to remain data.
> 
> I'll send another version later for this patch.
> 
> The other patch for isr() should be still fine.
> 
> Thank you,
> Nicolin
> 
> >  		}
> >  		break;
> >  	default:
> > -- 
> > 1.8.4
> > 

^ permalink raw reply

* RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
From: mihai.caraman @ 2014-07-23 10:06 UTC (permalink / raw)
  To: Alexander Graf
  Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org,
	kvm-ppc@vger.kernel.org
In-Reply-To: <4803B12E-A3CF-43C4-AF98-2AD87F76D151@suse.de>

> > Right. With this do you acknowledge that v5 (definitely overwritten
> approach)
> > is ok?
>=20
> I think I'm starting to understand your logic of v5. You write
> fetch_failed into *inst unswapped if the fetch failed.

"v5
  - don't swap when load fails" :)

>=20
> I think that's ok, but I definitely do not like the code flow - it's too
> hard to understand at a glimpse. Just rewrite it to swab at local
> variable level, preferably with if()s and comments what this is about and
> have a single unconditional *inst =3D fetched_inst; at the end of the
> function.

I will incorporate these change requests into v6.

Thanks,
-Mike

^ permalink raw reply

* Re: [PATCH 1/2] ASoC: fsl_sai: Reset FIFOs after disabling TE/RE
From: Mark Brown @ 2014-07-23 10:07 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: alsa-devel, b42378, b02247, linux-kernel, timur, Li.Xiubo,
	linuxppc-dev
In-Reply-To: <20140723095229.GA25639@MrMyself>

[-- Attachment #1: Type: text/plain, Size: 355 bytes --]

On Wed, Jul 23, 2014 at 05:52:32PM +0800, Nicolin Chen wrote:

> I found this two patches are merged into for-next branch, although I haven't
> got the 'applied' email. 

> Is that possible for you to drop this one? If not, I'll send another patch
> to fix this.

Please send a patch, I'd already applied it by the time you asked me to
drop it.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] ASoC: fsl_sai: Reset FIFOs after disabling TE/RE
From: Nicolin Chen @ 2014-07-23  9:59 UTC (permalink / raw)
  To: Mark Brown
  Cc: alsa-devel, b42378, b02247, linux-kernel, timur, Li.Xiubo,
	linuxppc-dev
In-Reply-To: <20140723100746.GC17528@sirena.org.uk>

On Wed, Jul 23, 2014 at 11:07:46AM +0100, Mark Brown wrote:
> On Wed, Jul 23, 2014 at 05:52:32PM +0800, Nicolin Chen wrote:
> 
> > I found this two patches are merged into for-next branch, although I haven't
> > got the 'applied' email. 
> 
> > Is that possible for you to drop this one? If not, I'll send another patch
> > to fix this.
> 
> Please send a patch, I'd already applied it by the time you asked me to
> drop it.

Got it. Thank you.
Nicolin

^ permalink raw reply

* [PATCH 1/3] powerpc/perf: Clear all MMCR settings before calling compute_mmcr()
From: Michael Ellerman @ 2014-07-23 11:12 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras

Because we reuse cpuhw->mmcr on each call to compute_mmcr() there's a
risk that we could forget to set one of the values and use whatever
value was in there previously.

Currently all the implementations are careful to set all the values, but
it's safer to clear them all before we call compute_mmcr().

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/perf/core-book3s.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index fe52db2eea6a..f82c0973866f 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1219,8 +1219,10 @@ static void power_pmu_enable(struct pmu *pmu)
 	}
 
 	/*
-	 * Compute MMCR* values for the new set of events
+	 * Clear all MMCR settings and recompute them for the new set of events.
 	 */
+	memset(cpuhw->mmcr, 0, sizeof(cpuhw->mmcr));
+
 	if (ppmu->compute_mmcr(cpuhw->events, cpuhw->n_events, hwc_index,
 			       cpuhw->mmcr)) {
 		/* shouldn't ever get here */
-- 
1.9.1

^ permalink raw reply related

* [PATCH 2/3] powerpc/perf: Pass the struct perf_events down to compute_mmcr()
From: Michael Ellerman @ 2014-07-23 11:12 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras
In-Reply-To: <1406113958-28653-1-git-send-email-mpe@ellerman.id.au>

To support per-event exclude settings on Power8 we need access to the
struct perf_events in compute_mmcr().

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/perf_event_server.h | 3 ++-
 arch/powerpc/perf/core-book3s.c              | 2 +-
 arch/powerpc/perf/power4-pmu.c               | 2 +-
 arch/powerpc/perf/power5+-pmu.c              | 2 +-
 arch/powerpc/perf/power5-pmu.c               | 2 +-
 arch/powerpc/perf/power6-pmu.c               | 2 +-
 arch/powerpc/perf/power7-pmu.c               | 2 +-
 arch/powerpc/perf/power8-pmu.c               | 3 ++-
 arch/powerpc/perf/ppc970-pmu.c               | 2 +-
 9 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
index b3e936027b26..17b419949036 100644
--- a/arch/powerpc/include/asm/perf_event_server.h
+++ b/arch/powerpc/include/asm/perf_event_server.h
@@ -30,7 +30,8 @@ struct power_pmu {
 	unsigned long	add_fields;
 	unsigned long	test_adder;
 	int		(*compute_mmcr)(u64 events[], int n_ev,
-				unsigned int hwc[], unsigned long mmcr[]);
+				unsigned int hwc[], unsigned long mmcr[],
+				struct perf_event *pevents[]);
 	int		(*get_constraint)(u64 event_id, unsigned long *mskp,
 				unsigned long *valp);
 	int		(*get_alternatives)(u64 event_id, unsigned int flags,
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index f82c0973866f..01b30238d7d1 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1224,7 +1224,7 @@ static void power_pmu_enable(struct pmu *pmu)
 	memset(cpuhw->mmcr, 0, sizeof(cpuhw->mmcr));
 
 	if (ppmu->compute_mmcr(cpuhw->events, cpuhw->n_events, hwc_index,
-			       cpuhw->mmcr)) {
+			       cpuhw->mmcr, cpuhw->event)) {
 		/* shouldn't ever get here */
 		printk(KERN_ERR "oops compute_mmcr failed\n");
 		goto out;
diff --git a/arch/powerpc/perf/power4-pmu.c b/arch/powerpc/perf/power4-pmu.c
index 9103a1de864d..ce6072fa481b 100644
--- a/arch/powerpc/perf/power4-pmu.c
+++ b/arch/powerpc/perf/power4-pmu.c
@@ -356,7 +356,7 @@ static int p4_get_alternatives(u64 event, unsigned int flags, u64 alt[])
 }
 
 static int p4_compute_mmcr(u64 event[], int n_ev,
-			   unsigned int hwc[], unsigned long mmcr[])
+			   unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
 {
 	unsigned long mmcr0 = 0, mmcr1 = 0, mmcra = 0;
 	unsigned int pmc, unit, byte, psel, lower;
diff --git a/arch/powerpc/perf/power5+-pmu.c b/arch/powerpc/perf/power5+-pmu.c
index b03b6dc0172d..0526dac66007 100644
--- a/arch/powerpc/perf/power5+-pmu.c
+++ b/arch/powerpc/perf/power5+-pmu.c
@@ -452,7 +452,7 @@ static int power5p_marked_instr_event(u64 event)
 }
 
 static int power5p_compute_mmcr(u64 event[], int n_ev,
-				unsigned int hwc[], unsigned long mmcr[])
+				unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
 {
 	unsigned long mmcr1 = 0;
 	unsigned long mmcra = 0;
diff --git a/arch/powerpc/perf/power5-pmu.c b/arch/powerpc/perf/power5-pmu.c
index 1e8ce423c3af..4dc99f9f7962 100644
--- a/arch/powerpc/perf/power5-pmu.c
+++ b/arch/powerpc/perf/power5-pmu.c
@@ -383,7 +383,7 @@ static int power5_marked_instr_event(u64 event)
 }
 
 static int power5_compute_mmcr(u64 event[], int n_ev,
-			       unsigned int hwc[], unsigned long mmcr[])
+			       unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
 {
 	unsigned long mmcr1 = 0;
 	unsigned long mmcra = MMCRA_SDAR_DCACHE_MISS | MMCRA_SDAR_ERAT_MISS;
diff --git a/arch/powerpc/perf/power6-pmu.c b/arch/powerpc/perf/power6-pmu.c
index 31128e086fed..9c9d646b68a1 100644
--- a/arch/powerpc/perf/power6-pmu.c
+++ b/arch/powerpc/perf/power6-pmu.c
@@ -175,7 +175,7 @@ static int power6_marked_instr_event(u64 event)
  * Assign PMC numbers and compute MMCR1 value for a set of events
  */
 static int p6_compute_mmcr(u64 event[], int n_ev,
-			   unsigned int hwc[], unsigned long mmcr[])
+			   unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
 {
 	unsigned long mmcr1 = 0;
 	unsigned long mmcra = MMCRA_SDAR_DCACHE_MISS | MMCRA_SDAR_ERAT_MISS;
diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
index 56c67bca2f75..5b62f2389290 100644
--- a/arch/powerpc/perf/power7-pmu.c
+++ b/arch/powerpc/perf/power7-pmu.c
@@ -245,7 +245,7 @@ static int power7_marked_instr_event(u64 event)
 }
 
 static int power7_compute_mmcr(u64 event[], int n_ev,
-			       unsigned int hwc[], unsigned long mmcr[])
+			       unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
 {
 	unsigned long mmcr1 = 0;
 	unsigned long mmcra = MMCRA_SDAR_DCACHE_MISS | MMCRA_SDAR_ERAT_MISS;
diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index 639cd9156585..19bbddf495dd 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -393,7 +393,8 @@ static int power8_get_constraint(u64 event, unsigned long *maskp, unsigned long
 }
 
 static int power8_compute_mmcr(u64 event[], int n_ev,
-			       unsigned int hwc[], unsigned long mmcr[])
+			       unsigned int hwc[], unsigned long mmcr[],
+			       struct perf_event *pevents[])
 {
 	unsigned long mmcra, mmcr1, unit, combine, psel, cache, val;
 	unsigned int pmc, pmc_inuse;
diff --git a/arch/powerpc/perf/ppc970-pmu.c b/arch/powerpc/perf/ppc970-pmu.c
index 20139ceeacf6..8b6a8a36fa38 100644
--- a/arch/powerpc/perf/ppc970-pmu.c
+++ b/arch/powerpc/perf/ppc970-pmu.c
@@ -257,7 +257,7 @@ static int p970_get_alternatives(u64 event, unsigned int flags, u64 alt[])
 }
 
 static int p970_compute_mmcr(u64 event[], int n_ev,
-			     unsigned int hwc[], unsigned long mmcr[])
+			     unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
 {
 	unsigned long mmcr0 = 0, mmcr1 = 0, mmcra = 0;
 	unsigned int pmc, unit, byte, psel;
-- 
1.9.1

^ permalink raw reply related

* [PATCH 3/3] powerpc/perf: Add per-event excludes on Power8
From: Michael Ellerman @ 2014-07-23 11:12 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras
In-Reply-To: <1406113958-28653-1-git-send-email-mpe@ellerman.id.au>

Power8 has a new register (MMCR2), which contains individual freeze bits
for each counter. This is an improvement on previous chips as it means
we can have multiple events on the PMU at the same time with different
exclude_{user,kernel,hv} settings. Previously we had to ensure all
events on the PMU had the same exclude settings.

The core of the patch is fairly simple. We use the 207S feature flag to
indicate that the PMU backend supports per-event excludes, if it's set
we skip the generic logic that enforces the equality of excludes between
events. We also use that flag to skip setting the freeze bits in MMCR0,
the PMU backend is expected to have handled setting them in MMCR2.

The complication arises with EBB. The FCxP bits in MMCR2 are accessible
R/W to a task using EBB. Which means a task using EBB will be able to
see that we are using MMCR2 for freezing, whereas the old logic which
used MMCR0 is not user visible.

The task can not see or affect exclude_kernel & exclude_hv, so we only
need to consider exclude_user.

The table below summarises the behaviour both before and after this
commit is applied:

 exclude_user           true  false
 ------------------------------------
        | User visible |  N    N
 Before | Can freeze   |  Y    Y
        | Can unfreeze |  N    Y
 ------------------------------------
        | User visible |  Y    Y
  After | Can freeze   |  Y    Y
        | Can unfreeze |  Y/N  Y
 ------------------------------------

So firstly I assert that the simple visibility of the exclude_user
setting in MMCR2 is a non-issue. The event belongs to the task, and
was most likely created by the task. So the exclude_user setting is not
privileged information in any way.

Secondly, the behaviour in the exclude_user = false case is unchanged.
This is important as it is the case that is actually useful, ie. the
event is created with no exclude setting and the task uses MMCR2 to
implement exclusion manually.

For exclude_user = true there is no meaningful change to freezing the
event. Previously the task could use MMCR2 to freeze the event, though
it was already frozen with MMCR0. With the new code the task can use
MMCR2 to freeze the event, though it was already frozen with MMCR2.

The only real change is when exclude_user = true and the task tries to
use MMCR2 to unfreeze the event. Previously this had no effect, because
the event was already frozen in MMCR0. With the new code the task can
unfreeze the event in MMCR2, but at some indeterminate time in the
future the kernel will overwrite its setting and refreeze the event.

Therefore my final assertion is that any task using exclude_user = true
and also fiddling with MMCR2 was deeply confused before this change, and
remains so after it.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/perf/core-book3s.c | 67 +++++++++++++++++++++++++++--------------
 arch/powerpc/perf/power8-pmu.c  | 24 +++++++++++++--
 2 files changed, 67 insertions(+), 24 deletions(-)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 01b30238d7d1..b7cd00b0171e 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -36,7 +36,12 @@ struct cpu_hw_events {
 	struct perf_event *event[MAX_HWEVENTS];
 	u64 events[MAX_HWEVENTS];
 	unsigned int flags[MAX_HWEVENTS];
-	unsigned long mmcr[3];
+	/*
+	 * The order of the MMCR array is:
+	 *  - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2
+	 *  - 32-bit, MMCR0, MMCR1, MMCR2
+	 */
+	unsigned long mmcr[4];
 	struct perf_event *limited_counter[MAX_LIMITED_HWCOUNTERS];
 	u8  limited_hwidx[MAX_LIMITED_HWCOUNTERS];
 	u64 alternatives[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES];
@@ -112,9 +117,9 @@ static bool is_ebb_event(struct perf_event *event) { return false; }
 static int ebb_event_check(struct perf_event *event) { return 0; }
 static void ebb_event_add(struct perf_event *event) { }
 static void ebb_switch_out(unsigned long mmcr0) { }
-static unsigned long ebb_switch_in(bool ebb, unsigned long mmcr0)
+static unsigned long ebb_switch_in(bool ebb, struct cpu_hw_events *cpuhw)
 {
-	return mmcr0;
+	return cpuhw->mmcr[0];
 }
 
 static inline void power_pmu_bhrb_enable(struct perf_event *event) {}
@@ -542,8 +547,10 @@ static void ebb_switch_out(unsigned long mmcr0)
 	current->thread.mmcr2 = mfspr(SPRN_MMCR2) & MMCR2_USER_MASK;
 }
 
-static unsigned long ebb_switch_in(bool ebb, unsigned long mmcr0)
+static unsigned long ebb_switch_in(bool ebb, struct cpu_hw_events *cpuhw)
 {
+	unsigned long mmcr0 = cpuhw->mmcr[0];
+
 	if (!ebb)
 		goto out;
 
@@ -568,7 +575,15 @@ static unsigned long ebb_switch_in(bool ebb, unsigned long mmcr0)
 	mtspr(SPRN_SIAR, current->thread.siar);
 	mtspr(SPRN_SIER, current->thread.sier);
 	mtspr(SPRN_SDAR, current->thread.sdar);
-	mtspr(SPRN_MMCR2, current->thread.mmcr2);
+
+	/*
+	 * Merge the kernel & user values of MMCR2. The semantics we implement
+	 * are that the user MMCR2 can set bits, ie. cause counters to freeze,
+	 * but not clear bits. If a task wants to be able to clear bits, ie.
+	 * unfreeze counters, it should not set exclude_xxx in its events and
+	 * instead manage the MMCR2 entirely by itself.
+	 */
+	mtspr(SPRN_MMCR2, cpuhw->mmcr[3] | current->thread.mmcr2);
 out:
 	return mmcr0;
 }
@@ -915,6 +930,14 @@ static int check_excludes(struct perf_event **ctrs, unsigned int cflags[],
 	int i, n, first;
 	struct perf_event *event;
 
+	/*
+	 * If the PMU we're on supports per event exclude settings then we
+	 * don't need to do any of this logic. NB. This assumes no PMU has both
+	 * per event exclude and limited PMCs.
+	 */
+	if (ppmu->flags & PPMU_ARCH_207S)
+		return 0;
+
 	n = n_prev + n_new;
 	if (n <= 1)
 		return 0;
@@ -1230,19 +1253,20 @@ static void power_pmu_enable(struct pmu *pmu)
 		goto out;
 	}
 
-	/*
-	 * Add in MMCR0 freeze bits corresponding to the
-	 * attr.exclude_* bits for the first event.
-	 * We have already checked that all events have the
-	 * same values for these bits as the first event.
-	 */
-	event = cpuhw->event[0];
-	if (event->attr.exclude_user)
-		cpuhw->mmcr[0] |= MMCR0_FCP;
-	if (event->attr.exclude_kernel)
-		cpuhw->mmcr[0] |= freeze_events_kernel;
-	if (event->attr.exclude_hv)
-		cpuhw->mmcr[0] |= MMCR0_FCHV;
+	if (!(ppmu->flags & PPMU_ARCH_207S)) {
+		/*
+		 * Add in MMCR0 freeze bits corresponding to the attr.exclude_*
+		 * bits for the first event. We have already checked that all
+		 * events have the same value for these bits as the first event.
+		 */
+		event = cpuhw->event[0];
+		if (event->attr.exclude_user)
+			cpuhw->mmcr[0] |= MMCR0_FCP;
+		if (event->attr.exclude_kernel)
+			cpuhw->mmcr[0] |= freeze_events_kernel;
+		if (event->attr.exclude_hv)
+			cpuhw->mmcr[0] |= MMCR0_FCHV;
+	}
 
 	/*
 	 * Write the new configuration to MMCR* with the freeze
@@ -1254,6 +1278,8 @@ static void power_pmu_enable(struct pmu *pmu)
 	mtspr(SPRN_MMCR1, cpuhw->mmcr[1]);
 	mtspr(SPRN_MMCR0, (cpuhw->mmcr[0] & ~(MMCR0_PMC1CE | MMCR0_PMCjCE))
 				| MMCR0_FC);
+	if (ppmu->flags & PPMU_ARCH_207S)
+		mtspr(SPRN_MMCR2, cpuhw->mmcr[3]);
 
 	/*
 	 * Read off any pre-existing events that need to move
@@ -1309,10 +1335,7 @@ static void power_pmu_enable(struct pmu *pmu)
  out_enable:
 	pmao_restore_workaround(ebb);
 
-	if (ppmu->flags & PPMU_ARCH_207S)
-		mtspr(SPRN_MMCR2, 0);
-
-	mmcr0 = ebb_switch_in(ebb, cpuhw->mmcr[0]);
+	mmcr0 = ebb_switch_in(ebb, cpuhw);
 
 	mb();
 	if (cpuhw->bhrb_users)
diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index 19bbddf495dd..396351db601b 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/perf_event.h>
 #include <asm/firmware.h>
+#include <asm/cputable.h>
 
 
 /*
@@ -266,6 +267,11 @@
 #define MMCRA_SDAR_MODE_TLB		(1ull << 42)
 #define MMCRA_IFM_SHIFT			30
 
+/* Bits in MMCR2 for POWER8 */
+#define MMCR2_FCS(pmc)			(1ull << (63 - (((pmc) - 1) * 9)))
+#define MMCR2_FCP(pmc)			(1ull << (62 - (((pmc) - 1) * 9)))
+#define MMCR2_FCH(pmc)			(1ull << (57 - (((pmc) - 1) * 9)))
+
 
 static inline bool event_is_fab_match(u64 event)
 {
@@ -396,7 +402,7 @@ static int power8_compute_mmcr(u64 event[], int n_ev,
 			       unsigned int hwc[], unsigned long mmcr[],
 			       struct perf_event *pevents[])
 {
-	unsigned long mmcra, mmcr1, unit, combine, psel, cache, val;
+	unsigned long mmcra, mmcr1, mmcr2, unit, combine, psel, cache, val;
 	unsigned int pmc, pmc_inuse;
 	int i;
 
@@ -411,7 +417,7 @@ static int power8_compute_mmcr(u64 event[], int n_ev,
 
 	/* In continous sampling mode, update SDAR on TLB miss */
 	mmcra = MMCRA_SDAR_MODE_TLB;
-	mmcr1 = 0;
+	mmcr1 = mmcr2 = 0;
 
 	/* Second pass: assign PMCs, set all MMCR1 fields */
 	for (i = 0; i < n_ev; ++i) {
@@ -473,6 +479,19 @@ static int power8_compute_mmcr(u64 event[], int n_ev,
 			mmcra |= val << MMCRA_IFM_SHIFT;
 		}
 
+		if (pevents[i]->attr.exclude_user)
+			mmcr2 |= MMCR2_FCP(pmc);
+
+		if (pevents[i]->attr.exclude_hv)
+			mmcr2 |= MMCR2_FCH(pmc);
+
+		if (pevents[i]->attr.exclude_kernel) {
+			if (cpu_has_feature(CPU_FTR_HVMODE))
+				mmcr2 |= MMCR2_FCH(pmc);
+			else
+				mmcr2 |= MMCR2_FCS(pmc);
+		}
+
 		hwc[i] = pmc - 1;
 	}
 
@@ -492,6 +511,7 @@ static int power8_compute_mmcr(u64 event[], int n_ev,
 
 	mmcr[1] = mmcr1;
 	mmcr[2] = mmcra;
+	mmcr[3] = mmcr2;
 
 	return 0;
 }
-- 
1.9.1

^ permalink raw reply related

* [PATCH 0/3] ASoC: fsl_sai: Fix some issues in fsl_sai_trigger()
From: Nicolin Chen @ 2014-07-23 11:23 UTC (permalink / raw)
  To: broonie
  Cc: alsa-devel, b42378, b02247, linux-kernel, timur, Li.Xiubo,
	linuxppc-dev

The series of patches focus on issue fix inside fsl_sai_trigger().

Nicolin Chen (3):
  ASoC: fsl_sai: Reduce race condition during TE/RE enabling
  ASoC: fsl_sai: Don't reset FIFO until TE/RE bit is unset
  ASoC: fsl_sai: Improve enable flow in fsl_sai_trigger()

 sound/soc/fsl/fsl_sai.c | 40 +++++++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 17 deletions(-)

-- 
1.8.4

^ permalink raw reply

* [PATCH 1/3] ASoC: fsl_sai: Reduce race condition during TE/RE enabling
From: Nicolin Chen @ 2014-07-23 11:23 UTC (permalink / raw)
  To: broonie
  Cc: alsa-devel, b42378, b02247, linux-kernel, timur, Li.Xiubo,
	linuxppc-dev
In-Reply-To: <cover.1406113865.git.nicoleotsuka@gmail.com>

From: Nicolin Chen <Guangyu.Chen@freescale.com>

For trigger start, we don't need to check if it's the first time to
enable TE/RE or second time. It doesn't hurt to enable them any way,
which in the meantime can reduce race condition for TE/RE enabling.

For trigger stop, we will definitely clear FRDE of current direction.
Thus the driver only needs to read the opposite one's.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
 sound/soc/fsl/fsl_sai.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 1b6ee2c..a437899 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -327,7 +327,7 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 {
 	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
 	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
-	u32 tcsr, rcsr;
+	u32 xcsr;
 
 	/*
 	 * The transmitter bit clock and frame sync are to be
@@ -338,9 +338,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 	regmap_update_bits(sai->regmap, FSL_SAI_RCR2, FSL_SAI_CR2_SYNC,
 			   FSL_SAI_CR2_SYNC);
 
-	regmap_read(sai->regmap, FSL_SAI_TCSR, &tcsr);
-	regmap_read(sai->regmap, FSL_SAI_RCSR, &rcsr);
-
 	/*
 	 * It is recommended that the transmitter is the last enabled
 	 * and the first disabled.
@@ -349,12 +346,10 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr & FSL_SAI_CSR_FRDE)) {
-			regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
-					   FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
-			regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
-					   FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
-		}
+		regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
+				   FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
+		regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
+				   FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
 
 		regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
 				   FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS);
@@ -370,7 +365,8 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 				   FSL_SAI_CSR_xIE_MASK, 0);
 
 		/* Check if the opposite FRDE is also disabled */
-		if (!(tx ? rcsr & FSL_SAI_CSR_FRDE : tcsr & FSL_SAI_CSR_FRDE)) {
+		regmap_read(sai->regmap, FSL_SAI_xCSR(!tx), &xcsr);
+		if (!(xcsr & FSL_SAI_CSR_FRDE)) {
 			/* Disable both directions and reset their FIFOs */
 			regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
 					   FSL_SAI_CSR_TERE | FSL_SAI_CSR_FR,
-- 
1.8.4

^ permalink raw reply related

* [PATCH 2/3] ASoC: fsl_sai: Don't reset FIFO until TE/RE bit is unset
From: Nicolin Chen @ 2014-07-23 11:23 UTC (permalink / raw)
  To: broonie
  Cc: alsa-devel, b42378, b02247, linux-kernel, timur, Li.Xiubo,
	linuxppc-dev
In-Reply-To: <cover.1406113865.git.nicoleotsuka@gmail.com>

From: Nicolin Chen <Guangyu.Chen@freescale.com>

TE/RE bit of T/RCSR will remain set untill the current frame is physically
finished. The FIFO reset operation should wait this bit's totally cleared
rather than ignoring its status which might cause TE/RE disabling failed.

This patch adds delay and timeout to wait for its completion before FIFO
reset.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
 sound/soc/fsl/fsl_sai.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index a437899..a79a9b0 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -327,7 +327,7 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 {
 	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
 	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
-	u32 xcsr;
+	u32 xcsr, count = 100;
 
 	/*
 	 * The transmitter bit clock and frame sync are to be
@@ -369,11 +369,20 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 		if (!(xcsr & FSL_SAI_CSR_FRDE)) {
 			/* Disable both directions and reset their FIFOs */
 			regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
-					   FSL_SAI_CSR_TERE | FSL_SAI_CSR_FR,
-					   FSL_SAI_CSR_FR);
+					   FSL_SAI_CSR_TERE, 0);
 			regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
-					   FSL_SAI_CSR_TERE | FSL_SAI_CSR_FR,
-					   FSL_SAI_CSR_FR);
+					   FSL_SAI_CSR_TERE, 0);
+
+			/* TERE will remain set till the end of current frame */
+			do {
+				udelay(10);
+				regmap_read(sai->regmap, FSL_SAI_xCSR(tx), &xcsr);
+			} while (--count && xcsr & FSL_SAI_CSR_TERE);
+
+			regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
+					   FSL_SAI_CSR_FR, FSL_SAI_CSR_FR);
+			regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
+					   FSL_SAI_CSR_FR, FSL_SAI_CSR_FR);
 		}
 		break;
 	default:
-- 
1.8.4

^ permalink raw reply related

* [PATCH 3/3] ASoC: fsl_sai: Improve enable flow in fsl_sai_trigger()
From: Nicolin Chen @ 2014-07-23 11:23 UTC (permalink / raw)
  To: broonie
  Cc: alsa-devel, b42378, b02247, linux-kernel, timur, Li.Xiubo,
	linuxppc-dev
In-Reply-To: <cover.1406113865.git.nicoleotsuka@gmail.com>

From: Nicolin Chen <Guangyu.Chen@freescale.com>

The previous enable flow:
1, Enable TE&RE (SAI starts to consume tx FIFO and feed rx FIFO)
2, Mask IRQ of Tx/Rx to enable its interrupt.
3, Enable DMA request of Tx/Rx.

As this flow would enable DMA request later than TERE, the Tx FIFO
would be easily emptied into underrun while Rx FIFO would be easily
stuffed into overrun due to the delayed DMA transfering.

This issue happened merely occational before the patch 'ASoC: fsl_sai:
Reset FIFOs after disabling TE/RE' because there were useless data
remaining in the FIFO for the gap. However, it manifested after FIFO
reset's implemented.

After this patch, the new flow:
1, Enable DMA request of Tx/Rx.
2, Enable TE&RE (SAI starts to consume tx FIFO and feed rx FIFO)
3, Mask IRQ of Tx/Rx to enable its interrupt.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
 sound/soc/fsl/fsl_sai.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index a79a9b0..364410b 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -346,6 +346,9 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
+				   FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
+
 		regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
 				   FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
 		regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
@@ -353,8 +356,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 
 		regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
 				   FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS);
-		regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
-				   FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
-- 
1.8.4

^ permalink raw reply related

* Re: [PATCH 2/2] powerpc/powernv: Improve error message
From: Stewart Smith @ 2014-07-23 12:16 UTC (permalink / raw)
  To: Vasant Hegde, linuxppc-dev
In-Reply-To: <20140723092246.4740.47235.stgit@hegdevasant.in.ibm.com>

Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes:
> Presently we only support initiating Service Processor dump from host.
> Hence update sysfs message. Also update couple of other error/info
> messages.
>
> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>

Acked-by: Stewart Smith <stewart@linux.vnet.ibm.com>

^ permalink raw reply

* Re: [PATCH 2/2] powerpc/powernv: Interface to add opal dump region
From: Stewart Smith @ 2014-07-23 12:33 UTC (permalink / raw)
  To: Vasant Hegde, linux-kernel; +Cc: linuxppc-dev
In-Reply-To: <20140723092525.5170.25926.stgit@hegdevasant.in.ibm.com>

Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes:
> PowerNV platform is capable of capturing host memory region when system
> crashes (because of host/firmware). We have new OPAL API to register
> memory region to be capture when system crashes.
>
> This patch adds support for new API and also registers kernel log
> buffer.

I think we do need a way to un-register regions, and I think this may be
important around kexec - unless the log buffer is in exactly the same
place, there's a window where we could dump some random memory contents
as kernel log buffer, probably causing some poor support person to
scratch their head for a good long while.

> +void __init opal_dump_region_init(void)
> +{
> +	void *addr;
> +	uint64_t size;
> +	int rc;
> +
> +	/* Register kernel log buffer */
> +	addr = get_log_buf_addr();
> +	size = get_log_buf_len();
> +	rc = add_dump_region_entry(DUMP_REGION_LOG_BUF, addr, size);
> +	if (rc)
> +		pr_warn("DUMP: Failed to register kernel log buffer. "
> +			"rc = %d\n", rc);

This is what's going to be printed in kernel log when running on OPAL
firmware that doesn't support that call. Does this include the first GA
release on POWER8 systems? If so, we probably want a nicer log message
pointing to the fact that firmware is too old to support that function.

^ permalink raw reply

* Re: [PATCH 1/2] powerpc/powernv: Change BUG_ON to WARN_ON in elog code
From: Stewart Smith @ 2014-07-23 12:15 UTC (permalink / raw)
  To: Vasant Hegde, linuxppc-dev
In-Reply-To: <20140723092236.4740.33763.stgit@hegdevasant.in.ibm.com>

Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes:
> We can continue to read the error log (up to MAX size) even if
> we get the elog size more than MAX size. Hence change BUG_ON to
> WARN_ON.
>
> Also updated error message.
>
> Reported-by: Gopesh Kumar Chaudhary <gopchaud@in.ibm.com>
> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
> Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Acked-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>

So, it turns out the BUG_ON actually caught an FSP bug, at the very
least in their documentation.

The WARN_ON is probably the best thing we can do, along with the
OPAL/skiboot side fixes that I've mentioned separately.

Acked-by: Stewart Smith <stewart@linux.vnet.ibm.com>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox