[PATCH 1/5] powerpc/lib/sstep: Add cmpb instruction emulation

This patch adds emulation of the cmpb instruction, enabling xmon to
emulate this instruction.

Signed-off-by: Matt Brown <redacted>
---
 arch/powerpc/lib/sstep.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 33117f8..f3e9ba8 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c

@@ -596,6 +596,23 @@ static nokprobe_inline void do_cmp_unsigned(struct pt_regs *regs, unsigned long
 	regs->ccr = (regs->ccr & ~(0xf << shift)) | (crval << shift);
 }
 
+static nokprobe_inline void do_cmpb(struct pt_regs *regs, unsigned long v1,
+				unsigned long v2, int rd)
+{
+	unsigned long out_val, mask;
+	int i;
+
+	out_val = 0;
+	for (i = 0; i < 8; i++) {
+		mask = 0xff << (i * 8);
+
+		if ((v1 & mask) == (v2 & mask))
+			out_val |= mask;
+	}
+
+	regs->gpr[rd] = out_val;
+}
+
 static nokprobe_inline int trap_compare(long v1, long v2)
 {
 	int ret = 0;

@@ -1049,6 +1066,13 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
 			do_cmp_unsigned(regs, val, val2, rd >> 2);
 			goto instr_done;
 
+		case 19173952: /* cmpb */
+			val = regs->gpr[rd];
+			val2 = regs->gpr[rb];
+
+			do_cmpb(regs, val, val2, ra);
+			goto instr_done;
+
 /*
  * Arithmetic instructions
  */

-- 
2.9.3

This adds emulations for the popcntb, popcntw, and popcntd instructions.

Signed-off-by: Matt Brown <redacted>
---
 arch/powerpc/lib/sstep.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index f3e9ba8..cf69987 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c

@@ -613,6 +613,30 @@ static nokprobe_inline void do_cmpb(struct pt_regs *regs, unsigned long v1,
 	regs->gpr[rd] = out_val;
 }
 
+/*
+ * The size parameter is used to ajust the equivalent popcnt instruction.
+ * popcntb = 8, popcntw = 32, popcntd = 64
+ */
+static nokprobe_inline void do_popcnt(struct pt_regs *regs, unsigned long v1,
+				int size, int ra)
+{
+	unsigned int out_val, mask, n;
+	int i, j;
+
+	out_val = 0;
+
+	for (i = 0; i < (64 / size); i++) {
+		n = 0;
+		for (j = 0; j < size; j++) {
+			mask = 1 << (j + (i * size));
+			if (v1 & mask)
+				n++;
+		}
+		out_val |= n << (i * size);
+	}
+	regs->gpr[ra] = out_val;
+}
+
 static nokprobe_inline int trap_compare(long v1, long v2)
 {
 	int ret = 0;

@@ -1234,6 +1258,21 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
 			regs->gpr[ra] = (signed int) regs->gpr[rd];
 			goto logical_done;
 #endif
+		case 299528:	/* popcntb */
+			val = regs->gpr[rd];
+			do_popcnt(regs, val, 8, ra);
+			goto logical_done;
+
+		case 17076744:	/* popcntw */
+			val = regs->gpr[rd];
+			do_popcnt(regs, val, 32, ra);
+			goto logical_done;
+#ifdef __powerpc64__
+		case 19173896:	/* popcntd */
+			val = regs->gpr[rd];
+			do_popcnt(regs, val, 64, ra);
+			goto logical_done;
+#endif
 
 /*
  * Shift instructions

-- 
2.9.3

This adds emulation for the bpermd instruction.

Signed-off-by: Matt Brown <redacted>
---
 arch/powerpc/lib/sstep.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index cf69987..603654d 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c

@@ -637,6 +637,21 @@ static nokprobe_inline void do_popcnt(struct pt_regs *regs, unsigned long v1,
 	regs->gpr[ra] = out_val;
 }
 
+static nokprobe_inline void do_bpermd(struct pt_regs *regs, unsigned long v1,
+				unsigned long v2, int ra)
+{
+	unsigned int idx, i;
+	unsigned char perm;
+
+	perm = 0x0;
+	for (i = 0; i < 8; i++) {
+		idx = (v1 >> (i * 8)) & 0xff;
+		if (idx < 64)
+			perm |= (v2 & (1 << idx)) >> (idx - i);
+	}
+	regs->gpr[ra] = 0 | perm;
+}
+
 static nokprobe_inline int trap_compare(long v1, long v2)
 {
 	int ret = 0;

@@ -1274,6 +1289,14 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
 			goto logical_done;
 #endif
 
+#ifdef __powerpc64__
+		case 2396736:	/* bpermd */
+			val = regs->gpr[rd];
+			val2 = regs->gpr[rb];
+			do_bpermd(regs, val, val2, ra);
+			goto logical_done;
+#endif
+
 /*
  * Shift instructions
  */

-- 
2.9.3

This add emulation for the prtyw and prtyd instructions.

Signed-off-by: Matt Brown <redacted>
---
 arch/powerpc/lib/sstep.c | 58 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 52 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 603654d..3228783 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c

@@ -652,6 +652,42 @@ static nokprobe_inline void do_bpermd(struct pt_regs *regs, unsigned long v1,
 	regs->gpr[ra] = 0 | perm;
 }
 
+static nokprobe_inline void do_prtyw(struct pt_regs *regs, unsigned long v,
+				int ra)
+{
+	unsigned long low, high, out;
+	unsigned int i;
+
+	high = 0;
+	low = 0;
+	out = 0;
+
+	for (i = 0; i < 8; i++) {
+		if (v & (1 << (i * 8)))
+			(i < 4) ? (low++) : (high++);
+	}
+
+	if (low % 2)
+		out |= low;
+	if (high % 2)
+		out |= (high << 32);
+
+	regs->gpr[ra] = out;
+}
+
+static nokprobe_inline void do_prtyd(struct pt_regs *regs, unsigned long v,
+				int ra)
+{
+	unsigned int count, i;
+
+	count = 0;
+	for (i = 0; i < 8; i++) {
+		if (v & (1 << (i * 8)))
+			count++;
+	}
+	regs->gpr[ra] = count % 2;
+}
+
 static nokprobe_inline int trap_compare(long v1, long v2)
 {
 	int ret = 0;

@@ -1278,16 +1314,15 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
 			do_popcnt(regs, val, 8, ra);
 			goto logical_done;
 
-		case 17076744:	/* popcntw */
+		case 2101768:	/* prtyw */
 			val = regs->gpr[rd];
-			do_popcnt(regs, val, 32, ra);
+			do_prtyw(regs, val, ra);
 			goto logical_done;
-#ifdef __powerpc64__
-		case 19173896:	/* popcntd */
+
+		case 2134536:	/* prtyd */
 			val = regs->gpr[rd];
-			do_popcnt(regs, val, 64, ra);
+			do_prtyd(regs, val, ra);
 			goto logical_done;
-#endif
 
 #ifdef __powerpc64__
 		case 2396736:	/* bpermd */

@@ -1297,6 +1332,17 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
 			goto logical_done;
 #endif
 
+		case 17076744:	/* popcntw */
+			val = regs->gpr[rd];
+			do_popcnt(regs, val, 32, ra);
+			goto logical_done;
+#ifdef __powerpc64__
+		case 19173896:	/* popcntd */
+			val = regs->gpr[rd];
+			do_popcnt(regs, val, 64, ra);
+			goto logical_done;
+#endif
+
 /*
  * Shift instructions
  */

-- 
2.9.3

This add emulation for the isel instruction.

Signed-off-by: Matt Brown <redacted>
---
 arch/powerpc/lib/sstep.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 3228783..bb0e301 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c

@@ -1297,6 +1297,16 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
 			regs->gpr[ra] = ~(regs->gpr[rd] & regs->gpr[rb]);
 			goto logical_done;
 
+		case 585:	/* isel */
+			mb = (instr >> 6) & 0x1f; /* bc */
+			val = (regs->ccr >> (mb + 32)) & 1;
+
+			if (val)
+				regs->gpr[rd] = regs->gpr[ra];
+			else
+				regs->gpr[rd] = regs->gpr[rb];
+			goto logical_done;
+
 		case 922:	/* extsh */
 			regs->gpr[ra] = (signed short) regs->gpr[rd];
 			goto logical_done;

-- 
2.9.3

On 13/07/17 13:25, Matt Brown wrote:

This looks wrong and should never trigger, given that the switch 
statement is comparing against ((instr >> 1) & 0x3ff).

How did you get this value?

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

On Thu, Jul 13, 2017 at 01:51:30PM +1000, Andrew Donnellan wrote:

On 13/07/17 13:25, Matt Brown wrote:

quoted

@@ -1049,6 +1066,13 @@ int analyse_instr(struct instruction_op *op, struct 

pt_regs *regs,
			do_cmp_unsigned(regs, val, val2, rd >> 2);
			goto instr_done;

+		case 19173952: /* cmpb */

This looks wrong and should never trigger, given that the switch 
statement is comparing against ((instr >> 1) & 0x3ff).

How did you get this value?

The correct number is 508, and 19173952 = 37744*508.  How to get
37744 is a mystery though :-)


Segher

On Thu, Jul 13, 2017 at 01:25:44PM +1000, Matt Brown wrote:

+static nokprobe_inline void do_cmpb(struct pt_regs *regs, unsigned long v1,
+				unsigned long v2, int rd)
+{
+	unsigned long out_val, mask;
+	int i;
+
+	out_val = 0;
+	for (i = 0; i < 8; i++) {
+		mask = 0xff << (i * 8);

0xffUL ?

+
+		if ((v1 & mask) == (v2 & mask))
+			out_val |= mask;
+	}
+
+	regs->gpr[rd] = out_val;
+}

Segher

On Thu, Jul 13, 2017 at 01:43:53AM -0500, Segher Boessenkool wrote:

On Thu, Jul 13, 2017 at 01:51:30PM +1000, Andrew Donnellan wrote:

quoted

On 13/07/17 13:25, Matt Brown wrote:

quoted

@@ -1049,6 +1066,13 @@ int analyse_instr(struct instruction_op *op, struct 

pt_regs *regs,
			do_cmp_unsigned(regs, val, val2, rd >> 2);
			goto instr_done;

+		case 19173952: /* cmpb */

This looks wrong and should never trigger, given that the switch 
statement is comparing against ((instr >> 1) & 0x3ff).

How did you get this value?

The correct number is 508, and 19173952 = 37744*508.  How to get
37744 is a mystery though :-)

Ah.  Take the binary representation of 508, and interpret that as if
it were octal :-)  Those pesky meddling leading zeroes!


Segher

On Thu, Jul 13, 2017 at 01:25:46PM +1000, Matt Brown wrote:

Huh? What's the point of doing an or with 0?

The compiler will eliminate it, but it just confuses the reader.

	Gabriel

On Thu, Jul 13, 2017 at 01:25:46PM +1000, Matt Brown wrote:

+static nokprobe_inline void do_bpermd(struct pt_regs *regs, unsigned long v1,
+				unsigned long v2, int ra)
+{
+	unsigned int idx, i;
+	unsigned char perm;
+
+	perm = 0x0;
+	for (i = 0; i < 8; i++) {
+		idx = (v1 >> (i * 8)) & 0xff;
+		if (idx < 64)
+			perm |= (v2 & (1 << idx)) >> (idx - i);

That doesn't work I think, the bit numbers ("idx") are big-endian?

+	}
+	regs->gpr[ra] = 0 | perm;

And that is just silly :-)

+}

Segher

On Thu, Jul 13, 2017 at 01:25:47PM +1000, Matt Brown wrote:

+static nokprobe_inline void do_prtyw(struct pt_regs *regs, unsigned long v,
+				int ra)
+{
+	unsigned long low, high, out;
+	unsigned int i;
+
+	high = 0;
+	low = 0;
+	out = 0;
+
+	for (i = 0; i < 8; i++) {
+		if (v & (1 << (i * 8)))

1UL

+			(i < 4) ? (low++) : (high++);
+	}
+
+	if (low % 2)
+		out |= low;
+	if (high % 2)
+		out |= (high << 32);

Only the low bit of each word of the output can be set.  Something
like

  out = ((high & 1) << 32) | (low & 1);


Segher

On Thu, Jul 13, 2017 at 01:25:48PM +1000, Matt Brown wrote:

+		case 585:	/* isel */

The secondary opcode for isel is only 5 bits, not 10 like most other
insns have.

+			mb = (instr >> 6) & 0x1f; /* bc */
+			val = (regs->ccr >> (mb + 32)) & 1;

regs->ccr >> (31 - mb)  ?

+
+			if (val)
+				regs->gpr[rd] = regs->gpr[ra];

You need to treat ra=0 separately (as 0, not reg 0).

+			else
+				regs->gpr[rd] = regs->gpr[rb];
+			goto logical_done;

Segher

On Thu, Jul 13, 2017 at 5:37 PM, Segher Boessenkool
[off-list ref] wrote:

On Thu, Jul 13, 2017 at 01:25:47PM +1000, Matt Brown wrote:

quoted

+static nokprobe_inline void do_prtyw(struct pt_regs *regs, unsigned long v,
+                             int ra)
+{
+     unsigned long low, high, out;
+     unsigned int i;
+
+     high = 0;
+     low = 0;
+     out = 0;
+
+     for (i = 0; i < 8; i++) {
+             if (v & (1 << (i * 8)))

1UL

quoted

+                     (i < 4) ? (low++) : (high++);
+     }
+
+     if (low % 2)
+             out |= low;
+     if (high % 2)
+             out |= (high << 32);

Only the low bit of each word of the output can be set.  Something
like

  out = ((high & 1) << 32) | (low & 1);

Ah, I wasn't aware. That way is much more concise too :)

Thanks,
Matt

Segher

On Thu, Jul 13, 2017 at 5:47 PM, Segher Boessenkool
[off-list ref] wrote:

On Thu, Jul 13, 2017 at 01:25:48PM +1000, Matt Brown wrote:

quoted

+             case 585:       /* isel */

The secondary opcode for isel is only 5 bits, not 10 like most other
insns have.

Yet another conversion mistake, I'll get there one day!

quoted

+                     mb = (instr >> 6) & 0x1f; /* bc */
+                     val = (regs->ccr >> (mb + 32)) & 1;

regs->ccr >> (31 - mb)  ?

quoted

+
+                     if (val)
+                             regs->gpr[rd] = regs->gpr[ra];

You need to treat ra=0 separately (as 0, not reg 0).

Ah I missed that. The wording in the ISA doesn't make that completely obvious.

Thanks,
Matt

quoted

+                     else
+                             regs->gpr[rd] = regs->gpr[rb];
+                     goto logical_done;

Segher

On Thu, Jul 13, 2017 at 5:28 PM, Segher Boessenkool
[off-list ref] wrote:

On Thu, Jul 13, 2017 at 01:25:46PM +1000, Matt Brown wrote:

quoted

+static nokprobe_inline void do_bpermd(struct pt_regs *regs, unsigned long v1,
+                             unsigned long v2, int ra)
+{
+     unsigned int idx, i;
+     unsigned char perm;
+
+     perm = 0x0;
+     for (i = 0; i < 8; i++) {
+             idx = (v1 >> (i * 8)) & 0xff;
+             if (idx < 64)
+                     perm |= (v2 & (1 << idx)) >> (idx - i);

That doesn't work I think, the bit numbers ("idx") are big-endian?

Why would it be big-endian? Wouldn't it be in the same endian form as the arch?

quoted

+     }
+     regs->gpr[ra] = 0 | perm;

And that is just silly :-)

Yep that is silly.

Thanks,
Matt

quoted

+}

Segher

On Fri, Jul 14, 2017 at 02:19:34PM +1000, Matt Brown wrote:

quoted

quoted

+static nokprobe_inline void do_bpermd(struct pt_regs *regs, unsigned long v1,
+                             unsigned long v2, int ra)
+{
+     unsigned int idx, i;
+     unsigned char perm;
+
+     perm = 0x0;
+     for (i = 0; i < 8; i++) {
+             idx = (v1 >> (i * 8)) & 0xff;
+             if (idx < 64)
+                     perm |= (v2 & (1 << idx)) >> (idx - i);

That doesn't work I think, the bit numbers ("idx") are big-endian?

Why would it be big-endian? Wouldn't it be in the same endian form as the arch?

Because that is what the ISA says.  Bit ordering is always BE.  If any
instruction behaves differently in LE mode that is explicitly described.

Please somehow test that the emulation works correctly, and describe
how you tested it, to give people the warm fuzzies.


Segher

Hi Matt,

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.12 next-20170714]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matt-Brown/powerpc-lib-sstep-Add-cmpb-instruction-emulation/20170714-020837
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-chrp32_defconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   arch/powerpc/lib/sstep.c: In function 'do_prtyw':

quoted

arch/powerpc/lib/sstep.c:673:16: error: left shift count >= width of type [-Werror=shift-count-overflow]

      out |= (high << 32);
                   ^~
   cc1: all warnings being treated as errors

vim +673 arch/powerpc/lib/sstep.c

   654	
   655	static nokprobe_inline void do_prtyw(struct pt_regs *regs, unsigned long v,
   656					int ra)
   657	{
   658		unsigned long low, high, out;
   659		unsigned int i;
   660	
   661		high = 0;
   662		low = 0;
   663		out = 0;
   664	
   665		for (i = 0; i < 8; i++) {
   666			if (v & (1 << (i * 8)))
   667				(i < 4) ? (low++) : (high++);
   668		}
   669	
   670		if (low % 2)
   671			out |= low;
   672		if (high % 2)
 > 673			out |= (high << 32);
   674	
   675		regs->gpr[ra] = out;
   676	}
   677	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation