[PATCH v3 0/5] VPHN parsing fixes

[PATCH v3 0/5] VPHN parsing fixes

[Greg Kurz]

Michael,

As suggested in...

https://lists.ozlabs.org/pipermail/linuxppc-dev/2015-February/124647.html

... I repost the whole series with an extra patch to implement selftests
for VPHN (this is the only change between v2 and v3).

Please review.

---

Greg Kurz (5):
      powerpc/vphn: clarify the H_HOME_NODE_ASSOCIATIVITY API
      powerpc/vphn: move endianness fixing to vphn_unpack_associativity()
      powerpc/vphn: move VPHN parsing logic to a separate file
      powerpc/vphn: parsing code rewrite
      selftests, powerpc: Add test for VPHN


 arch/powerpc/mm/Makefile                          |    1 
 arch/powerpc/mm/numa.c                            |   55 ---
 arch/powerpc/mm/vphn.c                            |   70 ++++
 arch/powerpc/mm/vphn.h                            |   16 +
 tools/testing/selftests/powerpc/Makefile          |    2 
 tools/testing/selftests/powerpc/utils.h           |    1 
 tools/testing/selftests/powerpc/vphn/.gitignore   |    1 
 tools/testing/selftests/powerpc/vphn/Makefile     |   15 +
 tools/testing/selftests/powerpc/vphn/parse-vphn.c |  403 +++++++++++++++++++++
 tools/testing/selftests/powerpc/vphn/vphn.c       |    1 
 tools/testing/selftests/powerpc/vphn/vphn.h       |    1 
 11 files changed, 513 insertions(+), 53 deletions(-)
 create mode 100644 arch/powerpc/mm/vphn.c
 create mode 100644 arch/powerpc/mm/vphn.h
 create mode 100644 tools/testing/selftests/powerpc/vphn/.gitignore
 create mode 100644 tools/testing/selftests/powerpc/vphn/Makefile
 create mode 100644 tools/testing/selftests/powerpc/vphn/parse-vphn.c
 create mode 120000 tools/testing/selftests/powerpc/vphn/vphn.c
 create mode 120000 tools/testing/selftests/powerpc/vphn/vphn.h

--
Greg

[PATCH v3 1/5] powerpc/vphn: clarify the H_HOME_NODE_ASSOCIATIVITY API

[Greg Kurz]

The number of values returned by the H_HOME_NODE_ASSOCIATIVITY h_call deserves
to be explicitly defined, for a better understanding of the code.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 arch/powerpc/mm/numa.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 0257a7d..7a5bc21 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1247,11 +1247,15 @@ static int update_cpu_associativity_changes_mask(void)
 	return cpumask_weight(changes);
 }
 
+/* The H_HOME_NODE_ASSOCIATIVITY h_call returns 6 64-bit registers.
+ */
+#define VPHN_REGISTER_COUNT 6
+
 /*
  * 6 64-bit registers unpacked into 12 32-bit associativity values. To form
  * the complete property we have to add the length in the first cell.
  */
-#define VPHN_ASSOC_BUFSIZE (6*sizeof(u64)/sizeof(u32) + 1)
+#define VPHN_ASSOC_BUFSIZE (VPHN_REGISTER_COUNT*sizeof(u64)/sizeof(u32) + 1)
 
 /*
  * Convert the associativity domain numbers returned from the hypervisor
@@ -1309,7 +1313,7 @@ static long hcall_vphn(unsigned long cpu, __be32 *associativity)
 	int i;
 
 	rc = plpar_hcall9(H_HOME_NODE_ASSOCIATIVITY, retbuf, flags, hwcpu);
-	for (i = 0; i < 6; i++)
+	for (i = 0; i < VPHN_REGISTER_COUNT; i++)
 		retbuf[i] = cpu_to_be64(retbuf[i]);
 	vphn_unpack_associativity(retbuf, associativity);
 

[PATCH v3 2/5] powerpc/vphn: move endianness fixing to vphn_unpack_associativity()

[Greg Kurz]

The first argument to vphn_unpack_associativity() is a const long *, but the
parsing code expects __be64 values actually. Let's move the endian fixing
down for consistency.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 arch/powerpc/mm/numa.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 7a5bc21..59196c5 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1263,13 +1263,18 @@ static int update_cpu_associativity_changes_mask(void)
  */
 static int vphn_unpack_associativity(const long *packed, __be32 *unpacked)
 {
+	__be64 be_packed[VPHN_REGISTER_COUNT];
 	int i, nr_assoc_doms = 0;
-	const __be16 *field = (const __be16 *) packed;
+	const __be16 *field = (const __be16 *) be_packed;
 
 #define VPHN_FIELD_UNUSED	(0xffff)
 #define VPHN_FIELD_MSB		(0x8000)
 #define VPHN_FIELD_MASK		(~VPHN_FIELD_MSB)
 
+	/* Let's recreate the original stream. */
+	for (i = 0; i < VPHN_REGISTER_COUNT; i++)
+		be_packed[i] = cpu_to_be64(packed[i]);
+
 	for (i = 1; i < VPHN_ASSOC_BUFSIZE; i++) {
 		if (be16_to_cpup(field) == VPHN_FIELD_UNUSED) {
 			/* All significant fields processed, and remaining
@@ -1310,11 +1315,8 @@ static long hcall_vphn(unsigned long cpu, __be32 *associativity)
 	long retbuf[PLPAR_HCALL9_BUFSIZE] = {0};
 	u64 flags = 1;
 	int hwcpu = get_hard_smp_processor_id(cpu);
-	int i;
 
 	rc = plpar_hcall9(H_HOME_NODE_ASSOCIATIVITY, retbuf, flags, hwcpu);
-	for (i = 0; i < VPHN_REGISTER_COUNT; i++)
-		retbuf[i] = cpu_to_be64(retbuf[i]);
 	vphn_unpack_associativity(retbuf, associativity);
 
 	return rc;

[PATCH v3 3/5] powerpc/vphn: move VPHN parsing logic to a separate file

[Greg Kurz]

The goal behind this patch is to be able to write userland tests for the
VPHN parsing code.

Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 arch/powerpc/mm/Makefile |    1 +
 arch/powerpc/mm/numa.c   |   61 ++--------------------------------------------
 arch/powerpc/mm/vphn.c   |   50 ++++++++++++++++++++++++++++++++++++++
 arch/powerpc/mm/vphn.h   |   16 ++++++++++++
 4 files changed, 70 insertions(+), 58 deletions(-)
 create mode 100644 arch/powerpc/mm/vphn.c
 create mode 100644 arch/powerpc/mm/vphn.h

diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
index 438dcd3..9c8770b 100644
--- a/arch/powerpc/mm/Makefile
+++ b/arch/powerpc/mm/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_40x)		+= 40x_mmu.o
 obj-$(CONFIG_44x)		+= 44x_mmu.o
 obj-$(CONFIG_PPC_FSL_BOOK3E)	+= fsl_booke_mmu.o
 obj-$(CONFIG_NEED_MULTIPLE_NODES) += numa.o
+obj-$(CONFIG_PPC_SPLPAR)	+= vphn.o
 obj-$(CONFIG_PPC_MM_SLICES)	+= slice.o
 obj-y				+= hugetlbpage.o
 ifeq ($(CONFIG_HUGETLB_PAGE),y)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 59196c5..c68471c 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1177,6 +1177,9 @@ u64 memory_hotplug_max(void)
 
 /* Virtual Processor Home Node (VPHN) support */
 #ifdef CONFIG_PPC_SPLPAR
+
+#include "vphn.h"
+
 struct topology_update_data {
 	struct topology_update_data *next;
 	unsigned int cpu;
@@ -1247,64 +1250,6 @@ static int update_cpu_associativity_changes_mask(void)
 	return cpumask_weight(changes);
 }
 
-/* The H_HOME_NODE_ASSOCIATIVITY h_call returns 6 64-bit registers.
- */
-#define VPHN_REGISTER_COUNT 6
-
-/*
- * 6 64-bit registers unpacked into 12 32-bit associativity values. To form
- * the complete property we have to add the length in the first cell.
- */
-#define VPHN_ASSOC_BUFSIZE (VPHN_REGISTER_COUNT*sizeof(u64)/sizeof(u32) + 1)
-
-/*
- * Convert the associativity domain numbers returned from the hypervisor
- * to the sequence they would appear in the ibm,associativity property.
- */
-static int vphn_unpack_associativity(const long *packed, __be32 *unpacked)
-{
-	__be64 be_packed[VPHN_REGISTER_COUNT];
-	int i, nr_assoc_doms = 0;
-	const __be16 *field = (const __be16 *) be_packed;
-
-#define VPHN_FIELD_UNUSED	(0xffff)
-#define VPHN_FIELD_MSB		(0x8000)
-#define VPHN_FIELD_MASK		(~VPHN_FIELD_MSB)
-
-	/* Let's recreate the original stream. */
-	for (i = 0; i < VPHN_REGISTER_COUNT; i++)
-		be_packed[i] = cpu_to_be64(packed[i]);
-
-	for (i = 1; i < VPHN_ASSOC_BUFSIZE; i++) {
-		if (be16_to_cpup(field) == VPHN_FIELD_UNUSED) {
-			/* All significant fields processed, and remaining
-			 * fields contain the reserved value of all 1's.
-			 * Just store them.
-			 */
-			unpacked[i] = *((__be32 *)field);
-			field += 2;
-		} else if (be16_to_cpup(field) & VPHN_FIELD_MSB) {
-			/* Data is in the lower 15 bits of this field */
-			unpacked[i] = cpu_to_be32(
-				be16_to_cpup(field) & VPHN_FIELD_MASK);
-			field++;
-			nr_assoc_doms++;
-		} else {
-			/* Data is in the lower 15 bits of this field
-			 * concatenated with the next 16 bit field
-			 */
-			unpacked[i] = *((__be32 *)field);
-			field += 2;
-			nr_assoc_doms++;
-		}
-	}
-
-	/* The first cell contains the length of the property */
-	unpacked[0] = cpu_to_be32(nr_assoc_doms);
-
-	return nr_assoc_doms;
-}
-
 /*
  * Retrieve the new associativity information for a virtual processor's
  * home node.
diff --git a/arch/powerpc/mm/vphn.c b/arch/powerpc/mm/vphn.c
new file mode 100644
index 0000000..c49ed51
--- /dev/null
+++ b/arch/powerpc/mm/vphn.c
@@ -0,0 +1,50 @@
+#include <asm/byteorder.h>
+#include "vphn.h"
+
+/*
+ * Convert the associativity domain numbers returned from the hypervisor
+ * to the sequence they would appear in the ibm,associativity property.
+ */
+int vphn_unpack_associativity(const long *packed, __be32 *unpacked)
+{
+	__be64 be_packed[VPHN_REGISTER_COUNT];
+	int i, nr_assoc_doms = 0;
+	const __be16 *field = (const __be16 *) be_packed;
+
+#define VPHN_FIELD_UNUSED	(0xffff)
+#define VPHN_FIELD_MSB		(0x8000)
+#define VPHN_FIELD_MASK		(~VPHN_FIELD_MSB)
+
+	/* Let's recreate the original stream. */
+	for (i = 0; i < VPHN_REGISTER_COUNT; i++)
+		be_packed[i] = cpu_to_be64(packed[i]);
+
+	for (i = 1; i < VPHN_ASSOC_BUFSIZE; i++) {
+		if (be16_to_cpup(field) == VPHN_FIELD_UNUSED) {
+			/* All significant fields processed, and remaining
+			 * fields contain the reserved value of all 1's.
+			 * Just store them.
+			 */
+			unpacked[i] = *((__be32 *)field);
+			field += 2;
+		} else if (be16_to_cpup(field) & VPHN_FIELD_MSB) {
+			/* Data is in the lower 15 bits of this field */
+			unpacked[i] = cpu_to_be32(
+				be16_to_cpup(field) & VPHN_FIELD_MASK);
+			field++;
+			nr_assoc_doms++;
+		} else {
+			/* Data is in the lower 15 bits of this field
+			 * concatenated with the next 16 bit field
+			 */
+			unpacked[i] = *((__be32 *)field);
+			field += 2;
+			nr_assoc_doms++;
+		}
+	}
+
+	/* The first cell contains the length of the property */
+	unpacked[0] = cpu_to_be32(nr_assoc_doms);
+
+	return nr_assoc_doms;
+}
diff --git a/arch/powerpc/mm/vphn.h b/arch/powerpc/mm/vphn.h
new file mode 100644
index 0000000..96af9a4
--- /dev/null
+++ b/arch/powerpc/mm/vphn.h
@@ -0,0 +1,16 @@
+#ifndef _ARCH_POWERPC_MM_VPHN_H_
+#define _ARCH_POWERPC_MM_VPHN_H_
+
+/* The H_HOME_NODE_ASSOCIATIVITY h_call returns 6 64-bit registers.
+ */
+#define VPHN_REGISTER_COUNT 6
+
+/*
+ * 6 64-bit registers unpacked into 12 32-bit associativity values. To form
+ * the complete property we have to add the length in the first cell.
+ */
+#define VPHN_ASSOC_BUFSIZE (VPHN_REGISTER_COUNT*sizeof(u64)/sizeof(u32) + 1)
+
+extern int vphn_unpack_associativity(const long *packed, __be32 *unpacked);
+
+#endif

[PATCH v3 4/5] powerpc/vphn: parsing code rewrite

[Greg Kurz]

The current VPHN parsing logic has some flaws that this patch aims to fix:

1) when the value 0xffff is read, the value 0xffffffff gets added to the
   the output list and its element count isn't incremented. This is wrong.
   According to PAPR+ the domain identifiers are packed into a sequence
   terminated by the "reserved value of all ones". This means that 0xffff
   is a stream terminator.

2) the combination of byteswaps and casts make the code hardly readable.
   Let's parse the stream one 16-bit field at a time instead.

3) it is assumed that the hypercall returns 12 32-bit values packed into
   6 64-bit registers. According to PAPR+, the domain identifiers may be
   streamed as 16-bit values. Let's increase the number of expected numbers
   to 24.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 arch/powerpc/mm/vphn.c |   54 +++++++++++++++++++++++++++++++++---------------
 arch/powerpc/mm/vphn.h |    6 +++--
 2 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/mm/vphn.c b/arch/powerpc/mm/vphn.c
index c49ed51..5f8ef50 100644
--- a/arch/powerpc/mm/vphn.c
+++ b/arch/powerpc/mm/vphn.c
@@ -2,44 +2,64 @@
 #include "vphn.h"
 
 /*
- * Convert the associativity domain numbers returned from the hypervisor
- * to the sequence they would appear in the ibm,associativity property.
+ * The associativity domain numbers are returned from the hypervisor as a
+ * stream of mixed 16-bit and 32-bit fields. The stream is terminated by the
+ * special value of "all ones" (aka. 0xffff) and its size may not exceed 48
+ * bytes.
+ *
+ *    --- 16-bit fields -->
+ *  _________________________
+ *  |  0  |  1  |  2  |  3  |   be_packed[0]
+ *  ------+-----+-----+------
+ *  _________________________
+ *  |  4  |  5  |  6  |  7  |   be_packed[1]
+ *  -------------------------
+ *            ...
+ *  _________________________
+ *  | 20  | 21  | 22  | 23  |   be_packed[5]
+ *  -------------------------
+ *
+ * Convert to the sequence they would appear in the ibm,associativity property.
  */
 int vphn_unpack_associativity(const long *packed, __be32 *unpacked)
 {
 	__be64 be_packed[VPHN_REGISTER_COUNT];
 	int i, nr_assoc_doms = 0;
 	const __be16 *field = (const __be16 *) be_packed;
+	u16 last = 0;
+	bool is_32bit = false;
 
 #define VPHN_FIELD_UNUSED	(0xffff)
 #define VPHN_FIELD_MSB		(0x8000)
 #define VPHN_FIELD_MASK		(~VPHN_FIELD_MSB)
 
-	/* Let's recreate the original stream. */
+	/* Let's fix the values returned by plpar_hcall9() */
 	for (i = 0; i < VPHN_REGISTER_COUNT; i++)
 		be_packed[i] = cpu_to_be64(packed[i]);
 
 	for (i = 1; i < VPHN_ASSOC_BUFSIZE; i++) {
-		if (be16_to_cpup(field) == VPHN_FIELD_UNUSED) {
-			/* All significant fields processed, and remaining
-			 * fields contain the reserved value of all 1's.
-			 * Just store them.
+		u16 new = be16_to_cpup(field++);
+
+		if (is_32bit) {
+			/* Let's concatenate the 16 bits of this field to the
+			 * 15 lower bits of the previous field
 			 */
-			unpacked[i] = *((__be32 *)field);
-			field += 2;
-		} else if (be16_to_cpup(field) & VPHN_FIELD_MSB) {
+			unpacked[++nr_assoc_doms] =
+				cpu_to_be32(last << 16 | new);
+			is_32bit = false;
+		} else if (new == VPHN_FIELD_UNUSED)
+			/* This is the list terminator */
+			break;
+		else if (new & VPHN_FIELD_MSB) {
 			/* Data is in the lower 15 bits of this field */
-			unpacked[i] = cpu_to_be32(
-				be16_to_cpup(field) & VPHN_FIELD_MASK);
-			field++;
-			nr_assoc_doms++;
+			unpacked[++nr_assoc_doms] =
+				cpu_to_be32(new & VPHN_FIELD_MASK);
 		} else {
 			/* Data is in the lower 15 bits of this field
 			 * concatenated with the next 16 bit field
 			 */
-			unpacked[i] = *((__be32 *)field);
-			field += 2;
-			nr_assoc_doms++;
+			last = new;
+			is_32bit = true;
 		}
 	}
 
diff --git a/arch/powerpc/mm/vphn.h b/arch/powerpc/mm/vphn.h
index 96af9a4..fe8b780 100644
--- a/arch/powerpc/mm/vphn.h
+++ b/arch/powerpc/mm/vphn.h
@@ -6,10 +6,10 @@
 #define VPHN_REGISTER_COUNT 6
 
 /*
- * 6 64-bit registers unpacked into 12 32-bit associativity values. To form
- * the complete property we have to add the length in the first cell.
+ * 6 64-bit registers unpacked into up to 24 be32 associativity values. To
+ * form the complete property we have to add the length in the first cell.
  */
-#define VPHN_ASSOC_BUFSIZE (VPHN_REGISTER_COUNT*sizeof(u64)/sizeof(u32) + 1)
+#define VPHN_ASSOC_BUFSIZE (VPHN_REGISTER_COUNT*sizeof(u64)/sizeof(u16) + 1)
 
 extern int vphn_unpack_associativity(const long *packed, __be32 *unpacked);
 

[PATCH v3 5/5] selftests, powerpc: Add test for VPHN

[Greg Kurz]

The goal is to verify vphn_unpack_associativity() parses VPHN numbers
correctly. We feed it with a variety of input values and compare with
expected results.

PAPR+ does not say much about VPHN parsing: I came up with a list of
tests that check many simple cases and some corner ones. I wouldn't
dare to say the list is exhaustive though.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 tools/testing/selftests/powerpc/Makefile          |    2 
 tools/testing/selftests/powerpc/utils.h           |    1 
 tools/testing/selftests/powerpc/vphn/.gitignore   |    1 
 tools/testing/selftests/powerpc/vphn/Makefile     |   15 +
 tools/testing/selftests/powerpc/vphn/parse-vphn.c |  403 +++++++++++++++++++++
 tools/testing/selftests/powerpc/vphn/vphn.c       |    1 
 tools/testing/selftests/powerpc/vphn/vphn.h       |    1 
 7 files changed, 423 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/powerpc/vphn/.gitignore
 create mode 100644 tools/testing/selftests/powerpc/vphn/Makefile
 create mode 100644 tools/testing/selftests/powerpc/vphn/parse-vphn.c
 create mode 120000 tools/testing/selftests/powerpc/vphn/vphn.c
 create mode 120000 tools/testing/selftests/powerpc/vphn/vphn.h

diff --git a/tools/testing/selftests/powerpc/Makefile b/tools/testing/selftests/powerpc/Makefile
index 1d5e7ad..476b8dd 100644
--- a/tools/testing/selftests/powerpc/Makefile
+++ b/tools/testing/selftests/powerpc/Makefile
@@ -13,7 +13,7 @@ CFLAGS := -Wall -O2 -flto -Wall -Werror -DGIT_VERSION='"$(GIT_VERSION)"' -I$(CUR
 
 export CC CFLAGS
 
-TARGETS = pmu copyloops mm tm primitives stringloops
+TARGETS = pmu copyloops mm tm primitives stringloops vphn
 
 endif
 
diff --git a/tools/testing/selftests/powerpc/utils.h b/tools/testing/selftests/powerpc/utils.h
index a93777a..2ec455e 100644
--- a/tools/testing/selftests/powerpc/utils.h
+++ b/tools/testing/selftests/powerpc/utils.h
@@ -15,6 +15,7 @@ typedef   signed long long s64;
 
 /* Just for familiarity */
 typedef uint32_t u32;
+typedef uint16_t u16;
 typedef uint8_t u8;
 
 
diff --git a/tools/testing/selftests/powerpc/vphn/.gitignore b/tools/testing/selftests/powerpc/vphn/.gitignore
new file mode 100644
index 0000000..dd3039a
--- /dev/null
+++ b/tools/testing/selftests/powerpc/vphn/.gitignore
@@ -0,0 +1 @@
+parse-vphn
diff --git a/tools/testing/selftests/powerpc/vphn/Makefile b/tools/testing/selftests/powerpc/vphn/Makefile
new file mode 100644
index 0000000..f1f7bd5
--- /dev/null
+++ b/tools/testing/selftests/powerpc/vphn/Makefile
@@ -0,0 +1,15 @@
+PROGS := parse-vphn
+
+all: $(PROGS)
+
+$(PROGS): ../harness.c
+
+run_tests: all
+	@-for PROG in $(PROGS); do \
+		./$$PROG; \
+	done;
+
+clean:
+	rm -f $(PROGS) *.o
+
+.PHONY: all run_tests clean
diff --git a/tools/testing/selftests/powerpc/vphn/parse-vphn.c b/tools/testing/selftests/powerpc/vphn/parse-vphn.c
new file mode 100644
index 0000000..49b6c43
--- /dev/null
+++ b/tools/testing/selftests/powerpc/vphn/parse-vphn.c
@@ -0,0 +1,403 @@
+#include <stdio.h>
+#include <byteswap.h>
+#include "utils.h"
+
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+#define cpu_to_be32(x)		bswap_32(x)
+#define be32_to_cpu(x)		bswap_32(x)
+#define be16_to_cpup(x)		bswap_16(*x)
+#define cpu_to_be64(x)		bswap_64(x)
+#else
+#define cpu_to_be32(x)		(x)
+#define be32_to_cpu(x)		(x)
+#define be16_to_cpup(x)		(*x)
+#define cpu_to_be64(x)		(x)
+#endif
+
+#include "vphn.c"
+
+static struct test {
+	char *descr;
+	long input[VPHN_REGISTER_COUNT];
+	u32 expected[VPHN_ASSOC_BUFSIZE];
+} all_tests[] = {
+	{
+		"vphn: no data",
+		{
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+		},
+		{
+			0x00000000
+		}
+	},
+	{
+		"vphn: 1 x 16-bit value",
+		{
+			0x8001ffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+		},
+		{
+			0x00000001,
+			0x00000001
+		}
+	},
+	{
+		"vphn: 2 x 16-bit values",
+		{
+			0x80018002ffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+		},
+		{
+			0x00000002,
+			0x00000001,
+			0x00000002
+		}
+	},
+	{
+		"vphn: 3 x 16-bit values",
+		{
+			0x800180028003ffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+		},
+		{
+			0x00000003,
+			0x00000001,
+			0x00000002,
+			0x00000003
+		}
+	},
+	{
+		"vphn: 4 x 16-bit values",
+		{
+			0x8001800280038004,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+		},
+		{
+			0x00000004,
+			0x00000001,
+			0x00000002,
+			0x00000003,
+			0x00000004
+		}
+	},
+	{
+		/* Parsing the next 16-bit value out of the next 64-bit input
+		 * value.
+		 */
+		"vphn: 5 x 16-bit values",
+		{
+			0x8001800280038004,
+			0x8005ffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+		},
+		{
+			0x00000005,
+			0x00000001,
+			0x00000002,
+			0x00000003,
+			0x00000004,
+			0x00000005
+		}
+	},
+	{
+		/* Parse at most 6 x 64-bit input values */
+		"vphn: 24 x 16-bit values",
+		{
+			0x8001800280038004,
+			0x8005800680078008,
+			0x8009800a800b800c,
+			0x800d800e800f8010,
+			0x8011801280138014,
+			0x8015801680178018
+		},
+		{
+			0x00000018,
+			0x00000001,
+			0x00000002,
+			0x00000003,
+			0x00000004,
+			0x00000005,
+			0x00000006,
+			0x00000007,
+			0x00000008,
+			0x00000009,
+			0x0000000a,
+			0x0000000b,
+			0x0000000c,
+			0x0000000d,
+			0x0000000e,
+			0x0000000f,
+			0x00000010,
+			0x00000011,
+			0x00000012,
+			0x00000013,
+			0x00000014,
+			0x00000015,
+			0x00000016,
+			0x00000017,
+			0x00000018
+		}
+	},
+	{
+		"vphn: 1 x 32-bit value",
+		{
+			0x00000001ffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff
+		},
+		{
+			0x00000001,
+			0x00000001
+		}
+	},
+	{
+		"vphn: 2 x 32-bit values",
+		{
+			0x0000000100000002,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff
+		},
+		{
+			0x00000002,
+			0x00000001,
+			0x00000002
+		}
+	},
+	{
+		/* Parsing the next 32-bit value out of the next 64-bit input
+		 * value.
+		 */
+		"vphn: 3 x 32-bit values",
+		{
+			0x0000000100000002,
+			0x00000003ffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff
+		},
+		{
+			0x00000003,
+			0x00000001,
+			0x00000002,
+			0x00000003
+		}
+	},
+	{
+		/* Parse at most 6 x 64-bit input values */
+		"vphn: 12 x 32-bit values",
+		{
+			0x0000000100000002,
+			0x0000000300000004,
+			0x0000000500000006,
+			0x0000000700000008,
+			0x000000090000000a,
+			0x0000000b0000000c
+		},
+		{
+			0x0000000c,
+			0x00000001,
+			0x00000002,
+			0x00000003,
+			0x00000004,
+			0x00000005,
+			0x00000006,
+			0x00000007,
+			0x00000008,
+			0x00000009,
+			0x0000000a,
+			0x0000000b,
+			0x0000000c
+		}
+	},
+	{
+		"vphn: 16-bit value followed by 32-bit value",
+		{
+			0x800100000002ffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff
+		},
+		{
+			0x00000002,
+			0x00000001,
+			0x00000002
+		}
+	},
+	{
+		"vphn: 32-bit value followed by 16-bit value",
+		{
+			0x000000018002ffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff
+		},
+		{
+			0x00000002,
+			0x00000001,
+			0x00000002
+		}
+	},
+	{
+		/* Parse a 32-bit value split accross two consecutives 64-bit
+		 * input values.
+		 */
+		"vphn: 16-bit value followed by 2 x 32-bit values",
+		{
+			0x8001000000020000,
+			0x0003ffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff
+		},
+		{
+			0x00000003,
+			0x00000001,
+			0x00000002,
+			0x00000003,
+			0x00000004,
+			0x00000005
+		}
+	},
+	{
+		/* The lower bits in 0x0001ffff don't get mixed up with the
+		 * 0xffff terminator.
+		 */
+		"vphn: 32-bit value has all ones in 16 lower bits",
+		{
+			0x0001ffff80028003,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff
+		},
+		{
+			0x00000003,
+			0x0001ffff,
+			0x00000002,
+			0x00000003
+		}
+	},
+	{
+		/* The following input doesn't follow the specification.
+		 */
+		"vphn: last 32-bit value is truncated",
+		{
+			0x0000000100000002,
+			0x0000000300000004,
+			0x0000000500000006,
+			0x0000000700000008,
+			0x000000090000000a,
+			0x0000000b800c2bad
+		},
+		{
+			0x0000000c,
+			0x00000001,
+			0x00000002,
+			0x00000003,
+			0x00000004,
+			0x00000005,
+			0x00000006,
+			0x00000007,
+			0x00000008,
+			0x00000009,
+			0x0000000a,
+			0x0000000b,
+			0x0000000c
+		}
+	},
+	{
+		"vphn: garbage after terminator",
+		{
+			0xffff2bad2bad2bad,
+			0x2bad2bad2bad2bad,
+			0x2bad2bad2bad2bad,
+			0x2bad2bad2bad2bad,
+			0x2bad2bad2bad2bad,
+			0x2bad2bad2bad2bad
+		},
+		{
+			0x00000000
+		}
+	},
+	{
+		NULL
+	}
+};
+
+static struct test *test;
+
+static int test_vphn(void)
+{
+	__be32 output[VPHN_ASSOC_BUFSIZE] = { 0 };
+	int i, len;
+
+	vphn_unpack_associativity(test->input, output);
+
+	len = be32_to_cpu(output[0]);
+	if (len != test->expected[0]) {
+		printf("expected %d elements, got %d\n", test->expected[0],
+		       len);
+		return 1;
+	}
+
+	for (i = 1; i < len; i++) {
+		u32 val = be32_to_cpu(output[i]);
+		if (val != test->expected[i]) {
+			printf("element #%d is 0x%x, should be 0x%x\n", i, val,
+			       test->expected[i]);
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+int main(int argc, char **argv)
+{
+	for (test = all_tests; test->descr; test++) {
+		int ret;
+
+		ret = test_harness(test_vphn, test->descr);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
diff --git a/tools/testing/selftests/powerpc/vphn/vphn.c b/tools/testing/selftests/powerpc/vphn/vphn.c
new file mode 120000
index 0000000..186b906
--- /dev/null
+++ b/tools/testing/selftests/powerpc/vphn/vphn.c
@@ -0,0 +1 @@
+../../../../../arch/powerpc/mm/vphn.c
\ No newline at end of file
diff --git a/tools/testing/selftests/powerpc/vphn/vphn.h b/tools/testing/selftests/powerpc/vphn/vphn.h
new file mode 120000
index 0000000..7131efe
--- /dev/null
+++ b/tools/testing/selftests/powerpc/vphn/vphn.h
@@ -0,0 +1 @@
+../../../../../arch/powerpc/mm/vphn.h
\ No newline at end of file

[PATCH v4 5/5] selftests/powerpc: Add test for VPHN

[Michael Ellerman]

From: Greg Kurz <gkurz@linux.vnet.ibm.com>

The goal is to verify vphn_unpack_associativity() parses VPHN numbers
correctly. We feed it with a variety of input values and compare with
expected results.

PAPR+ does not say much about VPHN parsing: I came up with a list of
tests that check many simple cases and some corner ones. I wouldn't
dare to say the list is exhaustive though.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
[mpe: Rework harness logic and rename to test-vphn]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---

I reworked this a little bit, let me know if it looks OK to you.

I renamed the test to test-vphn, and I changed the test loop to only call the
harness once, but report all the individual test results as well.


 tools/testing/selftests/powerpc/Makefile         |   2 +-
 tools/testing/selftests/powerpc/utils.h          |   1 +
 tools/testing/selftests/powerpc/vphn/.gitignore  |   1 +
 tools/testing/selftests/powerpc/vphn/Makefile    |  13 +
 tools/testing/selftests/powerpc/vphn/test-vphn.c | 410 +++++++++++++++++++++++
 tools/testing/selftests/powerpc/vphn/vphn.c      |   1 +
 tools/testing/selftests/powerpc/vphn/vphn.h      |   1 +
 7 files changed, 428 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/powerpc/vphn/.gitignore
 create mode 100644 tools/testing/selftests/powerpc/vphn/Makefile
 create mode 100644 tools/testing/selftests/powerpc/vphn/test-vphn.c
 create mode 120000 tools/testing/selftests/powerpc/vphn/vphn.c
 create mode 120000 tools/testing/selftests/powerpc/vphn/vphn.h

diff --git a/tools/testing/selftests/powerpc/Makefile b/tools/testing/selftests/powerpc/Makefile
index 1d5e7ad2c460..476b8dd9275f 100644
--- a/tools/testing/selftests/powerpc/Makefile
+++ b/tools/testing/selftests/powerpc/Makefile
@@ -13,7 +13,7 @@ CFLAGS := -Wall -O2 -flto -Wall -Werror -DGIT_VERSION='"$(GIT_VERSION)"' -I$(CUR
 
 export CC CFLAGS
 
-TARGETS = pmu copyloops mm tm primitives stringloops
+TARGETS = pmu copyloops mm tm primitives stringloops vphn
 
 endif
 
diff --git a/tools/testing/selftests/powerpc/utils.h b/tools/testing/selftests/powerpc/utils.h
index a93777ae0684..2ec455e37792 100644
--- a/tools/testing/selftests/powerpc/utils.h
+++ b/tools/testing/selftests/powerpc/utils.h
@@ -15,6 +15,7 @@ typedef   signed long long s64;
 
 /* Just for familiarity */
 typedef uint32_t u32;
+typedef uint16_t u16;
 typedef uint8_t u8;
 
 
diff --git a/tools/testing/selftests/powerpc/vphn/.gitignore b/tools/testing/selftests/powerpc/vphn/.gitignore
new file mode 100644
index 000000000000..dd3039a0f638
--- /dev/null
+++ b/tools/testing/selftests/powerpc/vphn/.gitignore
@@ -0,0 +1 @@
+parse-vphn
diff --git a/tools/testing/selftests/powerpc/vphn/Makefile b/tools/testing/selftests/powerpc/vphn/Makefile
new file mode 100644
index 000000000000..4bce3ca9d140
--- /dev/null
+++ b/tools/testing/selftests/powerpc/vphn/Makefile
@@ -0,0 +1,13 @@
+PROG := test-vphn
+
+all: $(PROG)
+
+$(PROG): ../harness.c
+
+run_tests: all
+	./$(PROG)
+
+clean:
+	rm -f $(PROG)
+
+.PHONY: all run_tests clean
diff --git a/tools/testing/selftests/powerpc/vphn/test-vphn.c b/tools/testing/selftests/powerpc/vphn/test-vphn.c
new file mode 100644
index 000000000000..5742f6876b25
--- /dev/null
+++ b/tools/testing/selftests/powerpc/vphn/test-vphn.c
@@ -0,0 +1,410 @@
+#include <stdio.h>
+#include <byteswap.h>
+#include "utils.h"
+#include "subunit.h"
+
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+#define cpu_to_be32(x)		bswap_32(x)
+#define be32_to_cpu(x)		bswap_32(x)
+#define be16_to_cpup(x)		bswap_16(*x)
+#define cpu_to_be64(x)		bswap_64(x)
+#else
+#define cpu_to_be32(x)		(x)
+#define be32_to_cpu(x)		(x)
+#define be16_to_cpup(x)		(*x)
+#define cpu_to_be64(x)		(x)
+#endif
+
+#include "vphn.c"
+
+static struct test {
+	char *descr;
+	long input[VPHN_REGISTER_COUNT];
+	u32 expected[VPHN_ASSOC_BUFSIZE];
+} all_tests[] = {
+	{
+		"vphn: no data",
+		{
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+		},
+		{
+			0x00000000
+		}
+	},
+	{
+		"vphn: 1 x 16-bit value",
+		{
+			0x8001ffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+		},
+		{
+			0x00000001,
+			0x00000001
+		}
+	},
+	{
+		"vphn: 2 x 16-bit values",
+		{
+			0x80018002ffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+		},
+		{
+			0x00000002,
+			0x00000001,
+			0x00000002
+		}
+	},
+	{
+		"vphn: 3 x 16-bit values",
+		{
+			0x800180028003ffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+		},
+		{
+			0x00000003,
+			0x00000001,
+			0x00000002,
+			0x00000003
+		}
+	},
+	{
+		"vphn: 4 x 16-bit values",
+		{
+			0x8001800280038004,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+		},
+		{
+			0x00000004,
+			0x00000001,
+			0x00000002,
+			0x00000003,
+			0x00000004
+		}
+	},
+	{
+		/* Parsing the next 16-bit value out of the next 64-bit input
+		 * value.
+		 */
+		"vphn: 5 x 16-bit values",
+		{
+			0x8001800280038004,
+			0x8005ffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+		},
+		{
+			0x00000005,
+			0x00000001,
+			0x00000002,
+			0x00000003,
+			0x00000004,
+			0x00000005
+		}
+	},
+	{
+		/* Parse at most 6 x 64-bit input values */
+		"vphn: 24 x 16-bit values",
+		{
+			0x8001800280038004,
+			0x8005800680078008,
+			0x8009800a800b800c,
+			0x800d800e800f8010,
+			0x8011801280138014,
+			0x8015801680178018
+		},
+		{
+			0x00000018,
+			0x00000001,
+			0x00000002,
+			0x00000003,
+			0x00000004,
+			0x00000005,
+			0x00000006,
+			0x00000007,
+			0x00000008,
+			0x00000009,
+			0x0000000a,
+			0x0000000b,
+			0x0000000c,
+			0x0000000d,
+			0x0000000e,
+			0x0000000f,
+			0x00000010,
+			0x00000011,
+			0x00000012,
+			0x00000013,
+			0x00000014,
+			0x00000015,
+			0x00000016,
+			0x00000017,
+			0x00000018
+		}
+	},
+	{
+		"vphn: 1 x 32-bit value",
+		{
+			0x00000001ffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff
+		},
+		{
+			0x00000001,
+			0x00000001
+		}
+	},
+	{
+		"vphn: 2 x 32-bit values",
+		{
+			0x0000000100000002,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff
+		},
+		{
+			0x00000002,
+			0x00000001,
+			0x00000002
+		}
+	},
+	{
+		/* Parsing the next 32-bit value out of the next 64-bit input
+		 * value.
+		 */
+		"vphn: 3 x 32-bit values",
+		{
+			0x0000000100000002,
+			0x00000003ffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff
+		},
+		{
+			0x00000003,
+			0x00000001,
+			0x00000002,
+			0x00000003
+		}
+	},
+	{
+		/* Parse at most 6 x 64-bit input values */
+		"vphn: 12 x 32-bit values",
+		{
+			0x0000000100000002,
+			0x0000000300000004,
+			0x0000000500000006,
+			0x0000000700000008,
+			0x000000090000000a,
+			0x0000000b0000000c
+		},
+		{
+			0x0000000c,
+			0x00000001,
+			0x00000002,
+			0x00000003,
+			0x00000004,
+			0x00000005,
+			0x00000006,
+			0x00000007,
+			0x00000008,
+			0x00000009,
+			0x0000000a,
+			0x0000000b,
+			0x0000000c
+		}
+	},
+	{
+		"vphn: 16-bit value followed by 32-bit value",
+		{
+			0x800100000002ffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff
+		},
+		{
+			0x00000002,
+			0x00000001,
+			0x00000002
+		}
+	},
+	{
+		"vphn: 32-bit value followed by 16-bit value",
+		{
+			0x000000018002ffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff
+		},
+		{
+			0x00000002,
+			0x00000001,
+			0x00000002
+		}
+	},
+	{
+		/* Parse a 32-bit value split accross two consecutives 64-bit
+		 * input values.
+		 */
+		"vphn: 16-bit value followed by 2 x 32-bit values",
+		{
+			0x8001000000020000,
+			0x0003ffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff
+		},
+		{
+			0x00000003,
+			0x00000001,
+			0x00000002,
+			0x00000003,
+			0x00000004,
+			0x00000005
+		}
+	},
+	{
+		/* The lower bits in 0x0001ffff don't get mixed up with the
+		 * 0xffff terminator.
+		 */
+		"vphn: 32-bit value has all ones in 16 lower bits",
+		{
+			0x0001ffff80028003,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff,
+			0xffffffffffffffff
+		},
+		{
+			0x00000003,
+			0x0001ffff,
+			0x00000002,
+			0x00000003
+		}
+	},
+	{
+		/* The following input doesn't follow the specification.
+		 */
+		"vphn: last 32-bit value is truncated",
+		{
+			0x0000000100000002,
+			0x0000000300000004,
+			0x0000000500000006,
+			0x0000000700000008,
+			0x000000090000000a,
+			0x0000000b800c2bad
+		},
+		{
+			0x0000000c,
+			0x00000001,
+			0x00000002,
+			0x00000003,
+			0x00000004,
+			0x00000005,
+			0x00000006,
+			0x00000007,
+			0x00000008,
+			0x00000009,
+			0x0000000a,
+			0x0000000b,
+			0x0000000c
+		}
+	},
+	{
+		"vphn: garbage after terminator",
+		{
+			0xffff2bad2bad2bad,
+			0x2bad2bad2bad2bad,
+			0x2bad2bad2bad2bad,
+			0x2bad2bad2bad2bad,
+			0x2bad2bad2bad2bad,
+			0x2bad2bad2bad2bad
+		},
+		{
+			0x00000000
+		}
+	},
+	{
+		NULL
+	}
+};
+
+static int test_one(struct test *test)
+{
+	__be32 output[VPHN_ASSOC_BUFSIZE] = { 0 };
+	int i, len;
+
+	vphn_unpack_associativity(test->input, output);
+
+	len = be32_to_cpu(output[0]);
+	if (len != test->expected[0]) {
+		printf("expected %d elements, got %d\n", test->expected[0],
+		       len);
+		return 1;
+	}
+
+	for (i = 1; i < len; i++) {
+		u32 val = be32_to_cpu(output[i]);
+		if (val != test->expected[i]) {
+			printf("element #%d is 0x%x, should be 0x%x\n", i, val,
+			       test->expected[i]);
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+static int test_vphn(void)
+{
+	static struct test *test;
+
+	for (test = all_tests; test->descr; test++) {
+		int ret;
+
+		ret = test_one(test);
+		test_finish(test->descr, ret);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+int main(int argc, char **argv)
+{
+	return test_harness(test_vphn, "test-vphn");
+}
diff --git a/tools/testing/selftests/powerpc/vphn/vphn.c b/tools/testing/selftests/powerpc/vphn/vphn.c
new file mode 120000
index 000000000000..186b906e66d5
--- /dev/null
+++ b/tools/testing/selftests/powerpc/vphn/vphn.c
@@ -0,0 +1 @@
+../../../../../arch/powerpc/mm/vphn.c
\ No newline at end of file
diff --git a/tools/testing/selftests/powerpc/vphn/vphn.h b/tools/testing/selftests/powerpc/vphn/vphn.h
new file mode 120000
index 000000000000..7131efe38c65
--- /dev/null
+++ b/tools/testing/selftests/powerpc/vphn/vphn.h
@@ -0,0 +1 @@
+../../../../../arch/powerpc/mm/vphn.h
\ No newline at end of file
-- 
2.1.0

Re: [PATCH v3 4/5] powerpc/vphn: parsing code rewrite

[Anshuman Khandual]

> diff --git a/arch/powerpc/mm/vphn.h b/arch/powerpc/mm/vphn.h
> index 96af9a4..fe8b780 100644
> --- a/arch/powerpc/mm/vphn.h
> +++ b/arch/powerpc/mm/vphn.h
> @@ -6,10 +6,10 @@
>  #define VPHN_REGISTER_COUNT 6
>  
>  /*
> - * 6 64-bit registers unpacked into 12 32-bit associativity values. To form
> - * the complete property we have to add the length in the first cell.
> + * 6 64-bit registers unpacked into up to 24 be32 associativity values. To

I guess its be16 instead of be32 here		^^^^

Re: [PATCH v3 4/5] powerpc/vphn: parsing code rewrite

[Greg Kurz]

On Tue, 17 Mar 2015 15:20:50 +0530
Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:

> > diff --git a/arch/powerpc/mm/vphn.h b/arch/powerpc/mm/vphn.h
> > index 96af9a4..fe8b780 100644
> > --- a/arch/powerpc/mm/vphn.h
> > +++ b/arch/powerpc/mm/vphn.h
> > @@ -6,10 +6,10 @@
> >  #define VPHN_REGISTER_COUNT 6
> >  
> >  /*
> > - * 6 64-bit registers unpacked into 12 32-bit associativity values. To form
> > - * the complete property we have to add the length in the first cell.
> > + * 6 64-bit registers unpacked into up to 24 be32 associativity values. To
> 
> I guess its be16 instead of be32 here		^^^^

The output array is supposed to mimic the ibm,associativity property in the
device tree. It is hence an array of be32 values, even if the hypervisor
passes them as be16 values.

Cheers.

--
Greg

Re: [PATCH v3 0/5] VPHN parsing fixes

[Anshuman Khandual]

On 02/23/2015 08:44 PM, Greg Kurz wrote:
> Michael,
> 
> As suggested in...
> 
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2015-February/124647.html
> 
> ... I repost the whole series with an extra patch to implement selftests
> for VPHN (this is the only change between v2 and v3).
> 
> Please review.

[Including the reworked test case from Michael]

Reviewed-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
Tested-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>

Re: [PATCH v4 5/5] selftests/powerpc: Add test for VPHN

[Greg Kurz]

On Tue, 17 Mar 2015 15:21:29 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:

> From: Greg Kurz <gkurz@linux.vnet.ibm.com>
> 
> The goal is to verify vphn_unpack_associativity() parses VPHN numbers
> correctly. We feed it with a variety of input values and compare with
> expected results.
> 
> PAPR+ does not say much about VPHN parsing: I came up with a list of
> tests that check many simple cases and some corner ones. I wouldn't
> dare to say the list is exhaustive though.
> 
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> [mpe: Rework harness logic and rename to test-vphn]
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> 
> I reworked this a little bit, let me know if it looks OK to you.
> 
> I renamed the test to test-vphn, and I changed the test loop to only call the
> harness once, but report all the individual test results as well.
> 

I guess it is safe to assume we don't have an infinite loop bug hidding in
each individual test case.

I am okay with the changes. I don't know if it is needed but here's my:

Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>


> 
>  tools/testing/selftests/powerpc/Makefile         |   2 +-
>  tools/testing/selftests/powerpc/utils.h          |   1 +
>  tools/testing/selftests/powerpc/vphn/.gitignore  |   1 +
>  tools/testing/selftests/powerpc/vphn/Makefile    |  13 +
>  tools/testing/selftests/powerpc/vphn/test-vphn.c | 410 +++++++++++++++++++++++
>  tools/testing/selftests/powerpc/vphn/vphn.c      |   1 +
>  tools/testing/selftests/powerpc/vphn/vphn.h      |   1 +
>  7 files changed, 428 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/powerpc/vphn/.gitignore
>  create mode 100644 tools/testing/selftests/powerpc/vphn/Makefile
>  create mode 100644 tools/testing/selftests/powerpc/vphn/test-vphn.c
>  create mode 120000 tools/testing/selftests/powerpc/vphn/vphn.c
>  create mode 120000 tools/testing/selftests/powerpc/vphn/vphn.h
> 
> diff --git a/tools/testing/selftests/powerpc/Makefile b/tools/testing/selftests/powerpc/Makefile
> index 1d5e7ad2c460..476b8dd9275f 100644
> --- a/tools/testing/selftests/powerpc/Makefile
> +++ b/tools/testing/selftests/powerpc/Makefile
> @@ -13,7 +13,7 @@ CFLAGS := -Wall -O2 -flto -Wall -Werror -DGIT_VERSION='"$(GIT_VERSION)"' -I$(CUR
> 
>  export CC CFLAGS
> 
> -TARGETS = pmu copyloops mm tm primitives stringloops
> +TARGETS = pmu copyloops mm tm primitives stringloops vphn
> 
>  endif
> 
> diff --git a/tools/testing/selftests/powerpc/utils.h b/tools/testing/selftests/powerpc/utils.h
> index a93777ae0684..2ec455e37792 100644
> --- a/tools/testing/selftests/powerpc/utils.h
> +++ b/tools/testing/selftests/powerpc/utils.h
> @@ -15,6 +15,7 @@ typedef   signed long long s64;
> 
>  /* Just for familiarity */
>  typedef uint32_t u32;
> +typedef uint16_t u16;
>  typedef uint8_t u8;
> 
> 
> diff --git a/tools/testing/selftests/powerpc/vphn/.gitignore b/tools/testing/selftests/powerpc/vphn/.gitignore
> new file mode 100644
> index 000000000000..dd3039a0f638
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/vphn/.gitignore
> @@ -0,0 +1 @@
> +parse-vphn
> diff --git a/tools/testing/selftests/powerpc/vphn/Makefile b/tools/testing/selftests/powerpc/vphn/Makefile
> new file mode 100644
> index 000000000000..4bce3ca9d140
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/vphn/Makefile
> @@ -0,0 +1,13 @@
> +PROG := test-vphn
> +
> +all: $(PROG)
> +
> +$(PROG): ../harness.c
> +
> +run_tests: all
> +	./$(PROG)
> +
> +clean:
> +	rm -f $(PROG)
> +
> +.PHONY: all run_tests clean
> diff --git a/tools/testing/selftests/powerpc/vphn/test-vphn.c b/tools/testing/selftests/powerpc/vphn/test-vphn.c
> new file mode 100644
> index 000000000000..5742f6876b25
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/vphn/test-vphn.c
> @@ -0,0 +1,410 @@
> +#include <stdio.h>
> +#include <byteswap.h>
> +#include "utils.h"
> +#include "subunit.h"
> +
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> +#define cpu_to_be32(x)		bswap_32(x)
> +#define be32_to_cpu(x)		bswap_32(x)
> +#define be16_to_cpup(x)		bswap_16(*x)
> +#define cpu_to_be64(x)		bswap_64(x)
> +#else
> +#define cpu_to_be32(x)		(x)
> +#define be32_to_cpu(x)		(x)
> +#define be16_to_cpup(x)		(*x)
> +#define cpu_to_be64(x)		(x)
> +#endif
> +
> +#include "vphn.c"
> +
> +static struct test {
> +	char *descr;
> +	long input[VPHN_REGISTER_COUNT];
> +	u32 expected[VPHN_ASSOC_BUFSIZE];
> +} all_tests[] = {
> +	{
> +		"vphn: no data",
> +		{
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +		},
> +		{
> +			0x00000000
> +		}
> +	},
> +	{
> +		"vphn: 1 x 16-bit value",
> +		{
> +			0x8001ffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +		},
> +		{
> +			0x00000001,
> +			0x00000001
> +		}
> +	},
> +	{
> +		"vphn: 2 x 16-bit values",
> +		{
> +			0x80018002ffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +		},
> +		{
> +			0x00000002,
> +			0x00000001,
> +			0x00000002
> +		}
> +	},
> +	{
> +		"vphn: 3 x 16-bit values",
> +		{
> +			0x800180028003ffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +		},
> +		{
> +			0x00000003,
> +			0x00000001,
> +			0x00000002,
> +			0x00000003
> +		}
> +	},
> +	{
> +		"vphn: 4 x 16-bit values",
> +		{
> +			0x8001800280038004,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +		},
> +		{
> +			0x00000004,
> +			0x00000001,
> +			0x00000002,
> +			0x00000003,
> +			0x00000004
> +		}
> +	},
> +	{
> +		/* Parsing the next 16-bit value out of the next 64-bit input
> +		 * value.
> +		 */
> +		"vphn: 5 x 16-bit values",
> +		{
> +			0x8001800280038004,
> +			0x8005ffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +		},
> +		{
> +			0x00000005,
> +			0x00000001,
> +			0x00000002,
> +			0x00000003,
> +			0x00000004,
> +			0x00000005
> +		}
> +	},
> +	{
> +		/* Parse at most 6 x 64-bit input values */
> +		"vphn: 24 x 16-bit values",
> +		{
> +			0x8001800280038004,
> +			0x8005800680078008,
> +			0x8009800a800b800c,
> +			0x800d800e800f8010,
> +			0x8011801280138014,
> +			0x8015801680178018
> +		},
> +		{
> +			0x00000018,
> +			0x00000001,
> +			0x00000002,
> +			0x00000003,
> +			0x00000004,
> +			0x00000005,
> +			0x00000006,
> +			0x00000007,
> +			0x00000008,
> +			0x00000009,
> +			0x0000000a,
> +			0x0000000b,
> +			0x0000000c,
> +			0x0000000d,
> +			0x0000000e,
> +			0x0000000f,
> +			0x00000010,
> +			0x00000011,
> +			0x00000012,
> +			0x00000013,
> +			0x00000014,
> +			0x00000015,
> +			0x00000016,
> +			0x00000017,
> +			0x00000018
> +		}
> +	},
> +	{
> +		"vphn: 1 x 32-bit value",
> +		{
> +			0x00000001ffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff
> +		},
> +		{
> +			0x00000001,
> +			0x00000001
> +		}
> +	},
> +	{
> +		"vphn: 2 x 32-bit values",
> +		{
> +			0x0000000100000002,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff
> +		},
> +		{
> +			0x00000002,
> +			0x00000001,
> +			0x00000002
> +		}
> +	},
> +	{
> +		/* Parsing the next 32-bit value out of the next 64-bit input
> +		 * value.
> +		 */
> +		"vphn: 3 x 32-bit values",
> +		{
> +			0x0000000100000002,
> +			0x00000003ffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff
> +		},
> +		{
> +			0x00000003,
> +			0x00000001,
> +			0x00000002,
> +			0x00000003
> +		}
> +	},
> +	{
> +		/* Parse at most 6 x 64-bit input values */
> +		"vphn: 12 x 32-bit values",
> +		{
> +			0x0000000100000002,
> +			0x0000000300000004,
> +			0x0000000500000006,
> +			0x0000000700000008,
> +			0x000000090000000a,
> +			0x0000000b0000000c
> +		},
> +		{
> +			0x0000000c,
> +			0x00000001,
> +			0x00000002,
> +			0x00000003,
> +			0x00000004,
> +			0x00000005,
> +			0x00000006,
> +			0x00000007,
> +			0x00000008,
> +			0x00000009,
> +			0x0000000a,
> +			0x0000000b,
> +			0x0000000c
> +		}
> +	},
> +	{
> +		"vphn: 16-bit value followed by 32-bit value",
> +		{
> +			0x800100000002ffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff
> +		},
> +		{
> +			0x00000002,
> +			0x00000001,
> +			0x00000002
> +		}
> +	},
> +	{
> +		"vphn: 32-bit value followed by 16-bit value",
> +		{
> +			0x000000018002ffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff
> +		},
> +		{
> +			0x00000002,
> +			0x00000001,
> +			0x00000002
> +		}
> +	},
> +	{
> +		/* Parse a 32-bit value split accross two consecutives 64-bit
> +		 * input values.
> +		 */
> +		"vphn: 16-bit value followed by 2 x 32-bit values",
> +		{
> +			0x8001000000020000,
> +			0x0003ffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff
> +		},
> +		{
> +			0x00000003,
> +			0x00000001,
> +			0x00000002,
> +			0x00000003,
> +			0x00000004,
> +			0x00000005
> +		}
> +	},
> +	{
> +		/* The lower bits in 0x0001ffff don't get mixed up with the
> +		 * 0xffff terminator.
> +		 */
> +		"vphn: 32-bit value has all ones in 16 lower bits",
> +		{
> +			0x0001ffff80028003,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff,
> +			0xffffffffffffffff
> +		},
> +		{
> +			0x00000003,
> +			0x0001ffff,
> +			0x00000002,
> +			0x00000003
> +		}
> +	},
> +	{
> +		/* The following input doesn't follow the specification.
> +		 */
> +		"vphn: last 32-bit value is truncated",
> +		{
> +			0x0000000100000002,
> +			0x0000000300000004,
> +			0x0000000500000006,
> +			0x0000000700000008,
> +			0x000000090000000a,
> +			0x0000000b800c2bad
> +		},
> +		{
> +			0x0000000c,
> +			0x00000001,
> +			0x00000002,
> +			0x00000003,
> +			0x00000004,
> +			0x00000005,
> +			0x00000006,
> +			0x00000007,
> +			0x00000008,
> +			0x00000009,
> +			0x0000000a,
> +			0x0000000b,
> +			0x0000000c
> +		}
> +	},
> +	{
> +		"vphn: garbage after terminator",
> +		{
> +			0xffff2bad2bad2bad,
> +			0x2bad2bad2bad2bad,
> +			0x2bad2bad2bad2bad,
> +			0x2bad2bad2bad2bad,
> +			0x2bad2bad2bad2bad,
> +			0x2bad2bad2bad2bad
> +		},
> +		{
> +			0x00000000
> +		}
> +	},
> +	{
> +		NULL
> +	}
> +};
> +
> +static int test_one(struct test *test)
> +{
> +	__be32 output[VPHN_ASSOC_BUFSIZE] = { 0 };
> +	int i, len;
> +
> +	vphn_unpack_associativity(test->input, output);
> +
> +	len = be32_to_cpu(output[0]);
> +	if (len != test->expected[0]) {
> +		printf("expected %d elements, got %d\n", test->expected[0],
> +		       len);
> +		return 1;
> +	}
> +
> +	for (i = 1; i < len; i++) {
> +		u32 val = be32_to_cpu(output[i]);
> +		if (val != test->expected[i]) {
> +			printf("element #%d is 0x%x, should be 0x%x\n", i, val,
> +			       test->expected[i]);
> +			return 1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int test_vphn(void)
> +{
> +	static struct test *test;
> +
> +	for (test = all_tests; test->descr; test++) {
> +		int ret;
> +
> +		ret = test_one(test);
> +		test_finish(test->descr, ret);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	return test_harness(test_vphn, "test-vphn");
> +}
> diff --git a/tools/testing/selftests/powerpc/vphn/vphn.c b/tools/testing/selftests/powerpc/vphn/vphn.c
> new file mode 120000
> index 000000000000..186b906e66d5
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/vphn/vphn.c
> @@ -0,0 +1 @@
> +../../../../../arch/powerpc/mm/vphn.c
> \ No newline at end of file
> diff --git a/tools/testing/selftests/powerpc/vphn/vphn.h b/tools/testing/selftests/powerpc/vphn/vphn.h
> new file mode 120000
> index 000000000000..7131efe38c65
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/vphn/vphn.h
> @@ -0,0 +1 @@
> +../../../../../arch/powerpc/mm/vphn.h
> \ No newline at end of file

Re: [PATCH v4 5/5] selftests/powerpc: Add test for VPHN

[Michael Ellerman]

On Tue, 2015-03-17 at 12:49 +0100, Greg Kurz wrote:
> On Tue, 17 Mar 2015 15:21:29 +1100
> Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
> > From: Greg Kurz <gkurz@linux.vnet.ibm.com>
> > 
> > The goal is to verify vphn_unpack_associativity() parses VPHN numbers
> > correctly. We feed it with a variety of input values and compare with
> > expected results.
> > 
> > PAPR+ does not say much about VPHN parsing: I came up with a list of
> > tests that check many simple cases and some corner ones. I wouldn't
> > dare to say the list is exhaustive though.
> > 
> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> > [mpe: Rework harness logic and rename to test-vphn]
> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> > ---
> > 
> > I reworked this a little bit, let me know if it looks OK to you.
> > 
> > I renamed the test to test-vphn, and I changed the test loop to only call the
> > harness once, but report all the individual test results as well.
> 
> I guess it is safe to assume we don't have an infinite loop bug hidding in
> each individual test case.

Well if we do the whole test will be killed by the harness, so we'll still
notice.

> I am okay with the changes. I don't know if it is needed but here's my:
> 
> Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
 
Thanks.

cheers