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

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

[Matt Brown]

This patch adds emulation of the cmpb instruction, enabling xmon to
emulate this instruction.
Tested for correctness against the cmpb asm instruction on ppc64le.

Signed-off-by: Matt Brown <matthew.brown.dev@gmail.com>
---
v2: 
	- fixed opcode
	- fixed mask typecasting
---
 arch/powerpc/lib/sstep.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 33117f8..87d277f 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -596,6 +596,22 @@ 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 long out_val, mask;
+	int i;
+
+	out_val = 0;
+	for (i = 0; i < 8; i++) {
+		mask = 0xffUL << (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 +1065,10 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
 			do_cmp_unsigned(regs, val, val2, rd >> 2);
 			goto instr_done;
 
+		case 508: /* cmpb */
+			do_cmpb(regs, regs->gpr[rd], regs->gpr[rb], ra);
+			goto instr_done;
+
 /*
  * Arithmetic instructions
  */
-- 
2.9.3

[PATCH v3 2/5] powerpc/lib/sstep: Add popcnt instruction emulation

[Matt Brown]

This adds emulations for the popcntb, popcntw, and popcntd instructions.
Tested for correctness against the popcnt{b,w,d} instructions on ppc64le.

Signed-off-by: Matt Brown <matthew.brown.dev@gmail.com>
---
v3:
	- optimised using the Giles-Miller method of side-ways addition
v2:
	- fixed opcodes
	- fixed typecasting
	- fixed bitshifting error for both 32 and 64bit arch
---
 arch/powerpc/lib/sstep.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 87d277f..c1f9cdb 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -612,6 +612,32 @@ static nokprobe_inline void do_cmpb(struct pt_regs *regs, unsigned long v1,
 	regs->gpr[rd] = out_val;
 }
 
+/*
+ * The size parameter is used to adjust 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 long long out = v1;
+
+	out = (0x5555555555555555 & out) + (0x5555555555555555 & (out >> 1));
+	out = (0x3333333333333333 & out) + (0x3333333333333333 & (out >> 2));
+	out = (0x0f0f0f0f0f0f0f0f & out) + (0x0f0f0f0f0f0f0f0f & (out >> 4));
+	if (size == 8) {	/* popcntb */
+		regs->gpr[ra] = out;
+		return;
+	}
+	out = (0x001f001f001f001f & out) + (0x001f001f001f001f & (out >> 8));
+	out = (0x0000003f0000003f & out) + (0x0000003f0000003f & (out >> 16));
+	if (size == 32) {	/* popcntw */
+		regs->gpr[ra] = out;
+		return;
+	}
+	out = (0x000000000000007f & out) + (0x000000000000007f & (out >> 32));
+	regs->gpr[ra] = out;	/* popcntd */
+}
+
 static nokprobe_inline int trap_compare(long v1, long v2)
 {
 	int ret = 0;
@@ -1194,6 +1220,10 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
 			regs->gpr[ra] = regs->gpr[rd] & ~regs->gpr[rb];
 			goto logical_done;
 
+		case 122:	/* popcntb */
+			do_popcnt(regs, regs->gpr[rd], 8, ra);
+			goto logical_done;
+
 		case 124:	/* nor */
 			regs->gpr[ra] = ~(regs->gpr[rd] | regs->gpr[rb]);
 			goto logical_done;
@@ -1206,6 +1236,10 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
 			regs->gpr[ra] = regs->gpr[rd] ^ regs->gpr[rb];
 			goto logical_done;
 
+		case 378:	/* popcntw */
+			do_popcnt(regs, regs->gpr[rd], 32, ra);
+			goto logical_done;
+
 		case 412:	/* orc */
 			regs->gpr[ra] = regs->gpr[rd] | ~regs->gpr[rb];
 			goto logical_done;
@@ -1217,7 +1251,11 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
 		case 476:	/* nand */
 			regs->gpr[ra] = ~(regs->gpr[rd] & regs->gpr[rb]);
 			goto logical_done;
-
+#ifdef __powerpc64__
+		case 506:	/* popcntd */
+			do_popcnt(regs, regs->gpr[rd], 64, ra);
+			goto logical_done;
+#endif
 		case 922:	/* extsh */
 			regs->gpr[ra] = (signed short) regs->gpr[rd];
 			goto logical_done;
-- 
2.9.3

Re: [PATCH v3 2/5] powerpc/lib/sstep: Add popcnt instruction emulation

[Balbir Singh]

On Tue, 2017-07-25 at 13:33 +1000, Matt Brown wrote:
> This adds emulations for the popcntb, popcntw, and popcntd instructions.
> Tested for correctness against the popcnt{b,w,d} instructions on ppc64le.
> 
> Signed-off-by: Matt Brown <matthew.brown.dev@gmail.com>
> ---
> v3:
> 	- optimised using the Giles-Miller method of side-ways addition
> v2:
> 	- fixed opcodes
> 	- fixed typecasting
> 	- fixed bitshifting error for both 32 and 64bit arch
> ---
>  arch/powerpc/lib/sstep.c | 40 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index 87d277f..c1f9cdb 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -612,6 +612,32 @@ static nokprobe_inline void do_cmpb(struct pt_regs *regs, unsigned long v1,
>  	regs->gpr[rd] = out_val;
>  }
>  
> +/*
> + * The size parameter is used to adjust 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 long long out = v1;
> +
> +	out = (0x5555555555555555 & out) + (0x5555555555555555 & (out >> 1));
> +	out = (0x3333333333333333 & out) + (0x3333333333333333 & (out >> 2));
> +	out = (0x0f0f0f0f0f0f0f0f & out) + (0x0f0f0f0f0f0f0f0f & (out >> 4));
> +	if (size == 8) {	/* popcntb */
> +		regs->gpr[ra] = out;
> +		return;
> +	}
> +	out = (0x001f001f001f001f & out) + (0x001f001f001f001f & (out >> 8));

Why are we using 0x001f001f here? Now that we've got things in the
bytes with 0's prefixing, we can directly use

out = out + out >> 8

> +	out = (0x0000003f0000003f & out) + (0x0000003f0000003f & (out >> 16));

Same as above

> +	if (size == 32) {	/* popcntw */
> +		regs->gpr[ra] = out;
> +		return;
> +	}
> +	out = (0x000000000000007f & out) + (0x000000000000007f & (out >> 32));
> +	regs->gpr[ra] = out;	/* popcntd */

Ditto

Otherwise looks good!

Balbir Singh.

RE: [PATCH v3 2/5] powerpc/lib/sstep: Add popcnt instruction emulation

[David Laight]

From: Linuxppc-dev [mailto:linuxppc-dev-bounces+david.laight=3Daculab.com@l=
ists.ozlabs.org] On Behalf Of
> Matt Brown
> Sent: 25 July 2017 04:33
> To: linuxppc-dev@lists.ozlabs.org
> Subject: [PATCH v3 2/5] powerpc/lib/sstep: Add popcnt instruction emulati=
on
>=20
> This adds emulations for the popcntb, popcntw, and popcntd instructions.
> Tested for correctness against the popcnt{b,w,d} instructions on ppc64le.
>=20
> Signed-off-by: Matt Brown <matthew.brown.dev@gmail.com>
> ---
> v3:
> 	- optimised using the Giles-Miller method of side-ways addition
> v2:
> 	- fixed opcodes
> 	- fixed typecasting
> 	- fixed bitshifting error for both 32 and 64bit arch
> ---
>  arch/powerpc/lib/sstep.c | 40 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
>=20
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index 87d277f..c1f9cdb 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -612,6 +612,32 @@ static nokprobe_inline void do_cmpb(struct pt_regs *=
regs, unsigned long v1,
>  	regs->gpr[rd] =3D out_val;
>  }
>=20
> +/*
> + * The size parameter is used to adjust the equivalent popcnt instructio=
n.
> + * popcntb =3D 8, popcntw =3D 32, popcntd =3D 64
> + */
> +static nokprobe_inline void do_popcnt(struct pt_regs *regs, unsigned lon=
g v1,
> +				int size, int ra)
> +{
> +	unsigned long long out =3D v1;
> +
> +	out =3D (0x5555555555555555 & out) + (0x5555555555555555 & (out >> 1));
> +	out =3D (0x3333333333333333 & out) + (0x3333333333333333 & (out >> 2));
> +	out =3D (0x0f0f0f0f0f0f0f0f & out) + (0x0f0f0f0f0f0f0f0f & (out >> 4));
> +	if (size =3D=3D 8) {	/* popcntb */
> +		regs->gpr[ra] =3D out;

I'm pretty sure you need to mask the result with 7.

	David

Re: [PATCH v3 2/5] powerpc/lib/sstep: Add popcnt instruction emulation

[Balbir Singh]

On Tue, Jul 25, 2017 at 8:24 PM, David Laight <David.Laight@aculab.com> wrote:
> From: Linuxppc-dev [mailto:linuxppc-dev-bounces+david.laight=aculab.com@lists.ozlabs.org] On Behalf Of
>> Matt Brown
>> Sent: 25 July 2017 04:33
>> To: linuxppc-dev@lists.ozlabs.org
>> Subject: [PATCH v3 2/5] powerpc/lib/sstep: Add popcnt instruction emulation
>>
>> This adds emulations for the popcntb, popcntw, and popcntd instructions.
>> Tested for correctness against the popcnt{b,w,d} instructions on ppc64le.
>>
>> Signed-off-by: Matt Brown <matthew.brown.dev@gmail.com>
>> ---
>> v3:
>>       - optimised using the Giles-Miller method of side-ways addition
>> v2:
>>       - fixed opcodes
>>       - fixed typecasting
>>       - fixed bitshifting error for both 32 and 64bit arch
>> ---
>>  arch/powerpc/lib/sstep.c | 40 +++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
>> index 87d277f..c1f9cdb 100644
>> --- a/arch/powerpc/lib/sstep.c
>> +++ b/arch/powerpc/lib/sstep.c
>> @@ -612,6 +612,32 @@ static nokprobe_inline void do_cmpb(struct pt_regs *regs, unsigned long v1,
>>       regs->gpr[rd] = out_val;
>>  }
>>
>> +/*
>> + * The size parameter is used to adjust 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 long long out = v1;
>> +
>> +     out = (0x5555555555555555 & out) + (0x5555555555555555 & (out >> 1));
>> +     out = (0x3333333333333333 & out) + (0x3333333333333333 & (out >> 2));
>> +     out = (0x0f0f0f0f0f0f0f0f & out) + (0x0f0f0f0f0f0f0f0f & (out >> 4));
>> +     if (size == 8) {        /* popcntb */
>> +             regs->gpr[ra] = out;
>
> I'm pretty sure you need to mask the result with 7.
>
Absolutely! Good catch!

Balbir Singh.

Re: [PATCH v3 2/5] powerpc/lib/sstep: Add popcnt instruction emulation

[Gabriel Paubert]

On Tue, Jul 25, 2017 at 01:33:17PM +1000, Matt Brown wrote:
> This adds emulations for the popcntb, popcntw, and popcntd instructions.
> Tested for correctness against the popcnt{b,w,d} instructions on ppc64le.
> 
> Signed-off-by: Matt Brown <matthew.brown.dev@gmail.com>
> ---
> v3:
> 	- optimised using the Giles-Miller method of side-ways addition
> v2:
> 	- fixed opcodes
> 	- fixed typecasting
> 	- fixed bitshifting error for both 32 and 64bit arch
> ---
>  arch/powerpc/lib/sstep.c | 40 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index 87d277f..c1f9cdb 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -612,6 +612,32 @@ static nokprobe_inline void do_cmpb(struct pt_regs *regs, unsigned long v1,
>  	regs->gpr[rd] = out_val;
>  }
>  
> +/*
> + * The size parameter is used to adjust 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 long long out = v1;
> +
> +	out = (0x5555555555555555 & out) + (0x5555555555555555 & (out >> 1));

This can be simplified in a less obvious way as:
	out -= (out >> 1) & 0x5555555555555555;

which maps each pair of bits according to the following:
00 -> 00
01 -> 01
10 -> 01
11 -> 10

This should save one instruction.

> +	out = (0x3333333333333333 & out) + (0x3333333333333333 & (out >> 2));

Ok, but now each nibble is between 0 and 4, so the addition of two
nibbles can't overflow or generate carry into the higher one.

> +	out = (0x0f0f0f0f0f0f0f0f & out) + (0x0f0f0f0f0f0f0f0f & (out >> 4));

	out += out >> 4;
	out &= 0x0f0f0f0f0f0f0f0f;

which should also save one instruction

> +	if (size == 8) {	/* popcntb */
> +		regs->gpr[ra] = out;
> +		return;
> +	}

At this point each count occupies at least one byte and can no more
overflow, so masking is only needed before returning.

> +	out = (0x001f001f001f001f & out) + (0x001f001f001f001f & (out >> 8));
	out += out >> 8;

> +	out = (0x0000003f0000003f & out) + (0x0000003f0000003f & (out >> 16));

	out += out >> 16;

> +	if (size == 32) {	/* popcntw */
> +		regs->gpr[ra] = out;
		regs->gpr[ra] = out & 0x0000003f0000003f;

> +		return;
> +	}
> +	out = (0x000000000000007f & out) + (0x000000000000007f & (out >> 32));
	out = (out + (out >> 32)) & 0x7f;


	Gabriel

> +	regs->gpr[ra] = out;	/* popcntd */
> +}
> +
>  static nokprobe_inline int trap_compare(long v1, long v2)
>  {
>  	int ret = 0;
> @@ -1194,6 +1220,10 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
>  			regs->gpr[ra] = regs->gpr[rd] & ~regs->gpr[rb];
>  			goto logical_done;
>  
> +		case 122:	/* popcntb */
> +			do_popcnt(regs, regs->gpr[rd], 8, ra);
> +			goto logical_done;
> +
>  		case 124:	/* nor */
>  			regs->gpr[ra] = ~(regs->gpr[rd] | regs->gpr[rb]);
>  			goto logical_done;
> @@ -1206,6 +1236,10 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
>  			regs->gpr[ra] = regs->gpr[rd] ^ regs->gpr[rb];
>  			goto logical_done;
>  
> +		case 378:	/* popcntw */
> +			do_popcnt(regs, regs->gpr[rd], 32, ra);
> +			goto logical_done;
> +
>  		case 412:	/* orc */
>  			regs->gpr[ra] = regs->gpr[rd] | ~regs->gpr[rb];
>  			goto logical_done;
> @@ -1217,7 +1251,11 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
>  		case 476:	/* nand */
>  			regs->gpr[ra] = ~(regs->gpr[rd] & regs->gpr[rb]);
>  			goto logical_done;
> -
> +#ifdef __powerpc64__
> +		case 506:	/* popcntd */
> +			do_popcnt(regs, regs->gpr[rd], 64, ra);
> +			goto logical_done;
> +#endif
>  		case 922:	/* extsh */
>  			regs->gpr[ra] = (signed short) regs->gpr[rd];
>  			goto logical_done;
> -- 
> 2.9.3

[PATCH v3 3/5] powerpc/lib/sstep: Add bpermd instruction emulation

[Matt Brown]

This adds emulation for the bpermd instruction.
Tested for correctness against the bpermd instruction on ppc64le.

Signed-off-by: Matt Brown <matthew.brown.dev@gmail.com>
---
v2:
	- fixed opcode
	- added ifdef tags to do_bpermd func
	- fixed bitshifting errors
---
 arch/powerpc/lib/sstep.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index c1f9cdb..6a79618 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -638,6 +638,24 @@ static nokprobe_inline void do_popcnt(struct pt_regs *regs, unsigned long v1,
 	regs->gpr[ra] = out;	/* popcntd */
 }
 
+#ifdef __powerpc64__
+static nokprobe_inline void do_bpermd(struct pt_regs *regs, unsigned long v1,
+				unsigned long v2, int ra)
+{
+	unsigned char perm, idx;
+	unsigned int i;
+
+	perm = 0;
+	for (i = 0; i < 8; i++) {
+		idx = (v1 >> (i * 8)) & 0xff;
+		if (idx < 64)
+			if (v2 & PPC_BIT(idx))
+				perm |= 1 << i;
+	}
+	regs->gpr[ra] = perm;
+}
+#endif /* __powerpc64__ */
+
 static nokprobe_inline int trap_compare(long v1, long v2)
 {
 	int ret = 0;
@@ -1227,7 +1245,11 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
 		case 124:	/* nor */
 			regs->gpr[ra] = ~(regs->gpr[rd] | regs->gpr[rb]);
 			goto logical_done;
-
+#ifdef __powerpc64__
+		case 252:	/* bpermd */
+			do_bpermd(regs, regs->gpr[rd], regs->gpr[rb], ra);
+			goto logical_done;
+#endif
 		case 284:	/* xor */
 			regs->gpr[ra] = ~(regs->gpr[rd] ^ regs->gpr[rb]);
 			goto logical_done;
-- 
2.9.3

[PATCH v3 4/5] powerpc/lib/sstep: Add prty instruction emulation

[Matt Brown]

This adds emulation for the prtyw and prtyd instructions.
Tested for logical correctness against the prtyw and prtyd instructions
on ppc64le.

Signed-off-by: Matt Brown <matthew.brown.dev@gmail.com>
---
v3:
	- optimised using the Giles-Miller method of side-ways addition
v2:
	- fixed opcodes
	- fixed bitshifting and typecast errors
	- merged do_prtyw and do_prtyd into single function
---
 arch/powerpc/lib/sstep.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 6a79618..0bcf631 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -655,6 +655,25 @@ static nokprobe_inline void do_bpermd(struct pt_regs *regs, unsigned long v1,
 	regs->gpr[ra] = perm;
 }
 #endif /* __powerpc64__ */
+/*
+ * The size parameter adjusts the equivalent prty instruction.
+ * prtyw = 32, prtyd = 64
+ */
+static nokprobe_inline void do_prty(struct pt_regs *regs, unsigned long v,
+				int size, int ra)
+{
+	unsigned long long res = v;
+
+	res = (0x0001000100010001 & res) + (0x0001000100010001 & (res >> 8));
+	res = (0x0000000700000007 & res) + (0x0000000700000007 & (res >> 16));
+	if (size == 32) {		/* prtyw */
+		regs->gpr[ra] = (0x0000000100000001 & res);
+		return;
+	}
+
+	res = (0x000000000000000f & res) + (0x000000000000000f & (res >> 32));
+	regs->gpr[ra] = res & 1;	/*prtyd */
+}
 
 static nokprobe_inline int trap_compare(long v1, long v2)
 {
@@ -1245,6 +1264,14 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
 		case 124:	/* nor */
 			regs->gpr[ra] = ~(regs->gpr[rd] | regs->gpr[rb]);
 			goto logical_done;
+
+		case 154:	/* prtyw */
+			do_prty(regs, regs->gpr[rd], 32, ra);
+			goto logical_done;
+
+		case 186:	/* prtyd */
+			do_prty(regs, regs->gpr[rd], 64, ra);
+			goto logical_done;
 #ifdef __powerpc64__
 		case 252:	/* bpermd */
 			do_bpermd(regs, regs->gpr[rd], regs->gpr[rb], ra);
-- 
2.9.3

Re: [PATCH v3 4/5] powerpc/lib/sstep: Add prty instruction emulation

[Balbir Singh]

On Tue, 2017-07-25 at 13:33 +1000, Matt Brown wrote:
> This adds emulation for the prtyw and prtyd instructions.
> Tested for logical correctness against the prtyw and prtyd instructions
> on ppc64le.
> 
> Signed-off-by: Matt Brown <matthew.brown.dev@gmail.com>
> ---
> v3:
> 	- optimised using the Giles-Miller method of side-ways addition
> v2:
> 	- fixed opcodes
> 	- fixed bitshifting and typecast errors
> 	- merged do_prtyw and do_prtyd into single function
> ---
>  arch/powerpc/lib/sstep.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index 6a79618..0bcf631 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -655,6 +655,25 @@ static nokprobe_inline void do_bpermd(struct pt_regs *regs, unsigned long v1,
>  	regs->gpr[ra] = perm;
>  }
>  #endif /* __powerpc64__ */
> +/*
> + * The size parameter adjusts the equivalent prty instruction.
> + * prtyw = 32, prtyd = 64
> + */
> +static nokprobe_inline void do_prty(struct pt_regs *regs, unsigned long v,
> +				int size, int ra)
> +{
> +	unsigned long long res = v;
> +
> +	res = (0x0001000100010001 & res) + (0x0001000100010001 & (res >> 8));
> +	res = (0x0000000700000007 & res) + (0x0000000700000007 & (res >> 16));
> +	if (size == 32) {		/* prtyw */
> +		regs->gpr[ra] = (0x0000000100000001 & res);
> +		return;
> +	}
> +
> +	res = (0x000000000000000f & res) + (0x000000000000000f & (res >> 32));
> +	regs->gpr[ra] = res & 1;	/*prtyd */

Looks good, you can also xor instead of adding, but the masks would need
to change in that case.

Balbir Singh.

Re: [PATCH v3 4/5] powerpc/lib/sstep: Add prty instruction emulation

[Gabriel Paubert]

On Tue, Jul 25, 2017 at 06:08:05PM +1000, Balbir Singh wrote:
> On Tue, 2017-07-25 at 13:33 +1000, Matt Brown wrote:
> > This adds emulation for the prtyw and prtyd instructions.
> > Tested for logical correctness against the prtyw and prtyd instructions
> > on ppc64le.
> > 
> > Signed-off-by: Matt Brown <matthew.brown.dev@gmail.com>
> > ---
> > v3:
> > 	- optimised using the Giles-Miller method of side-ways addition
> > v2:
> > 	- fixed opcodes
> > 	- fixed bitshifting and typecast errors
> > 	- merged do_prtyw and do_prtyd into single function
> > ---
> >  arch/powerpc/lib/sstep.c | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> > index 6a79618..0bcf631 100644
> > --- a/arch/powerpc/lib/sstep.c
> > +++ b/arch/powerpc/lib/sstep.c
> > @@ -655,6 +655,25 @@ static nokprobe_inline void do_bpermd(struct pt_regs *regs, unsigned long v1,
> >  	regs->gpr[ra] = perm;
> >  }
> >  #endif /* __powerpc64__ */
> > +/*
> > + * The size parameter adjusts the equivalent prty instruction.
> > + * prtyw = 32, prtyd = 64
> > + */
> > +static nokprobe_inline void do_prty(struct pt_regs *regs, unsigned long v,
> > +				int size, int ra)
> > +{
> > +	unsigned long long res = v;
> > +
> > +	res = (0x0001000100010001 & res) + (0x0001000100010001 & (res >> 8));
> > +	res = (0x0000000700000007 & res) + (0x0000000700000007 & (res >> 16));
> > +	if (size == 32) {		/* prtyw */
> > +		regs->gpr[ra] = (0x0000000100000001 & res);
> > +		return;
> > +	}
> > +
> > +	res = (0x000000000000000f & res) + (0x000000000000000f & (res >> 32));
> > +	regs->gpr[ra] = res & 1;	/*prtyd */
> 
> Looks good, you can also xor instead of adding, but the masks would need
> to change in that case.

Actually, I'd prefer to use xor for this case, since you hardly ever
need any mask, except at the end. Most masks are only needed because of
carry propagation, which the xor completely avoid.

In other words:

	unsigned long long res = v ^ (v >> 8);
	res ^= res >> 16; 
	if (size == 32) {
		regs->gpr[ra] = res & 0x0000000100000001;
		return;
	}
	res ^= res >> 32;
	regs-> gpr[ra] = res & 1;

should work, but I've not even compiled it.

	Gabriel

Re: [PATCH v3 4/5] powerpc/lib/sstep: Add prty instruction emulation

[Segher Boessenkool]

On Tue, Jul 25, 2017 at 01:33:19PM +1000, Matt Brown wrote:
> +static nokprobe_inline void do_prty(struct pt_regs *regs, unsigned long v,
> +				int size, int ra)
> +{
> +	unsigned long long res = v;
> +
> +	res = (0x0001000100010001 & res) + (0x0001000100010001 & (res >> 8));
> +	res = (0x0000000700000007 & res) + (0x0000000700000007 & (res >> 16));
> +	if (size == 32) {		/* prtyw */
> +		regs->gpr[ra] = (0x0000000100000001 & res);
> +		return;
> +	}
> +
> +	res = (0x000000000000000f & res) + (0x000000000000000f & (res >> 32));
> +	regs->gpr[ra] = res & 1;	/*prtyd */
> +}

Does 7's and 0xf look strange (since the top bit in the values there
is always 0).  You always "and" the values with 1 later, you could do
that immediately (and change + to ^).

A general question about these patches: some things are inside #ifdef
__powerpc64__, some are not.  It seems it is the wrong macro, and it
should be used (or not used) consistently?


Segher

Re: [PATCH v3 4/5] powerpc/lib/sstep: Add prty instruction emulation

[Michael Ellerman]

Segher Boessenkool <segher@kernel.crashing.org> writes:

> On Tue, Jul 25, 2017 at 01:33:19PM +1000, Matt Brown wrote:
>> +static nokprobe_inline void do_prty(struct pt_regs *regs, unsigned long v,
>> +				int size, int ra)
>> +{
>> +	unsigned long long res = v;
>> +
>> +	res = (0x0001000100010001 & res) + (0x0001000100010001 & (res >> 8));
>> +	res = (0x0000000700000007 & res) + (0x0000000700000007 & (res >> 16));
>> +	if (size == 32) {		/* prtyw */
>> +		regs->gpr[ra] = (0x0000000100000001 & res);
>> +		return;
>> +	}
>> +
>> +	res = (0x000000000000000f & res) + (0x000000000000000f & (res >> 32));
>> +	regs->gpr[ra] = res & 1;	/*prtyd */
>> +}
>
> Does 7's and 0xf look strange (since the top bit in the values there
> is always 0).  You always "and" the values with 1 later, you could do
> that immediately (and change + to ^).
>
> A general question about these patches: some things are inside #ifdef
> __powerpc64__, some are not.  It seems it is the wrong macro, and it
> should be used (or not used) consistently?

Why is it the wrong macro? Because we tend to use CONFIG_PPC64 you mean?

I thought the reason some are #ifdef'ed is that some are 64-bit only.
ie. bpermd is 64-bit only ?

cheers

Re: [PATCH v3 4/5] powerpc/lib/sstep: Add prty instruction emulation

[Segher Boessenkool]

On Wed, Jul 26, 2017 at 08:03:30PM +1000, Michael Ellerman wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> > A general question about these patches: some things are inside #ifdef
> > __powerpc64__, some are not.  It seems it is the wrong macro, and it
> > should be used (or not used) consistently?
> 
> Why is it the wrong macro? Because we tend to use CONFIG_PPC64 you mean?

Yeah.  But I see sstep.c already mixes those two at will (or if there
is a distinction, I'm not seeing it :-) )

> I thought the reason some are #ifdef'ed is that some are 64-bit only.
> ie. bpermd is 64-bit only ?

64-bit only, in what way?  It's not clear what the rules are.

It's not instructions that can only run in 64-bit mode.
It's not instructions that only give a usable result with 64-bit regs
implemented.
It's not instructions only implemented on 64-bit CPUs.
It's not even "all instructions that would not give a correct result
in the low 32 bits of GPRs if the high 32 bits are not implemented".


Segher

Re: [PATCH v3 4/5] powerpc/lib/sstep: Add prty instruction emulation

[Michael Ellerman]

Segher Boessenkool <segher@kernel.crashing.org> writes:

> On Wed, Jul 26, 2017 at 08:03:30PM +1000, Michael Ellerman wrote:
>> Segher Boessenkool <segher@kernel.crashing.org> writes:
>> > A general question about these patches: some things are inside #ifdef
>> > __powerpc64__, some are not.  It seems it is the wrong macro, and it
>> > should be used (or not used) consistently?
>> 
>> Why is it the wrong macro? Because we tend to use CONFIG_PPC64 you mean?
>
> Yeah.  But I see sstep.c already mixes those two at will (or if there
> is a distinction, I'm not seeing it :-) )

Yeah OK. In practice they're equivalent, if CONFIG_PPC64=y then the
kernel is built 64-bit and therefore __powerpc64__ is defined.

But I agree it's a mess, we should use CONFIG_PPC64 exclusively unless
there's some reason not to (which I don't think there ever is).

>> I thought the reason some are #ifdef'ed is that some are 64-bit only.
>> ie. bpermd is 64-bit only ?
>
> 64-bit only, in what way?  It's not clear what the rules are.

Instructions that have "d" in the name? :P

> It's not instructions that can only run in 64-bit mode.
> It's not instructions that only give a usable result with 64-bit regs
> implemented.
> It's not instructions only implemented on 64-bit CPUs.

I think it's trying to be that ^

If you build a 32-bit kernel then instructions that are only defined on
64-bit CPUs should be treated as illegal, so the easiest way to achieve
that is to #ifdef off the code for those instructions.

cheers

Re: [PATCH v3 4/5] powerpc/lib/sstep: Add prty instruction emulation

[Matt Brown]

On Thu, Jul 27, 2017 at 11:26 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
>
>> On Wed, Jul 26, 2017 at 08:03:30PM +1000, Michael Ellerman wrote:
>>> Segher Boessenkool <segher@kernel.crashing.org> writes:
>>> > A general question about these patches: some things are inside #ifdef
>>> > __powerpc64__, some are not.  It seems it is the wrong macro, and it
>>> > should be used (or not used) consistently?
>>>
>>> Why is it the wrong macro? Because we tend to use CONFIG_PPC64 you mean?
>>
>> Yeah.  But I see sstep.c already mixes those two at will (or if there
>> is a distinction, I'm not seeing it :-) )
>
> Yeah OK. In practice they're equivalent, if CONFIG_PPC64=y then the
> kernel is built 64-bit and therefore __powerpc64__ is defined.
>
> But I agree it's a mess, we should use CONFIG_PPC64 exclusively unless
> there's some reason not to (which I don't think there ever is).
>
>>> I thought the reason some are #ifdef'ed is that some are 64-bit only.
>>> ie. bpermd is 64-bit only ?
>>
>> 64-bit only, in what way?  It's not clear what the rules are.
>
> Instructions that have "d" in the name? :P
>
>> It's not instructions that can only run in 64-bit mode.
>> It's not instructions that only give a usable result with 64-bit regs
>> implemented.
>> It's not instructions only implemented on 64-bit CPUs.
>
> I think it's trying to be that ^
>
> If you build a 32-bit kernel then instructions that are only defined on
> 64-bit CPUs should be treated as illegal, so the easiest way to achieve
> that is to #ifdef off the code for those instructions.
>

I'll fixup this up to use the xor implementation, and change the
series to use CONFIG_PPC64 for the ifdef.

Thanks,
Matt
> cheers

Re: [PATCH v3 4/5] powerpc/lib/sstep: Add prty instruction emulation

[Michael Ellerman]

Matt Brown <matthew.brown.dev@gmail.com> writes:

> On Thu, Jul 27, 2017 at 11:26 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Segher Boessenkool <segher@kernel.crashing.org> writes:
>>
>>> On Wed, Jul 26, 2017 at 08:03:30PM +1000, Michael Ellerman wrote:
>>>> Segher Boessenkool <segher@kernel.crashing.org> writes:
>>>> > A general question about these patches: some things are inside #ifdef
>>>> > __powerpc64__, some are not.  It seems it is the wrong macro, and it
>>>> > should be used (or not used) consistently?
>>>>
>>>> Why is it the wrong macro? Because we tend to use CONFIG_PPC64 you mean?
>>>
>>> Yeah.  But I see sstep.c already mixes those two at will (or if there
>>> is a distinction, I'm not seeing it :-) )
>>
>> Yeah OK. In practice they're equivalent, if CONFIG_PPC64=y then the
>> kernel is built 64-bit and therefore __powerpc64__ is defined.
>>
>> But I agree it's a mess, we should use CONFIG_PPC64 exclusively unless
>> there's some reason not to (which I don't think there ever is).
>>
>>>> I thought the reason some are #ifdef'ed is that some are 64-bit only.
>>>> ie. bpermd is 64-bit only ?
>>>
>>> 64-bit only, in what way?  It's not clear what the rules are.
>>
>> Instructions that have "d" in the name? :P
>>
>>> It's not instructions that can only run in 64-bit mode.
>>> It's not instructions that only give a usable result with 64-bit regs
>>> implemented.
>>> It's not instructions only implemented on 64-bit CPUs.
>>
>> I think it's trying to be that ^
>>
>> If you build a 32-bit kernel then instructions that are only defined on
>> 64-bit CPUs should be treated as illegal, so the easiest way to achieve
>> that is to #ifdef off the code for those instructions.
>
> I'll fixup this up to use the xor implementation, and change the
> series to use CONFIG_PPC64 for the ifdef.

Thanks.

In terms of the actual algorithms, we want to use something reasonably
efficient, but don't sweat too much on the performance. The overhead of
the emulation is in the exception we took to get there. So readable and
obviously correct source is the main goal.

cheers

[PATCH v3 5/5] powerpc/lib/sstep: Add isel instruction emulation

[Matt Brown]

This adds emulation for the isel instruction.
Tested for correctness against the isel instruction and its extended
mnemonics (lt, gt, eq) on ppc64le.

Signed-off-by: Matt Brown <matthew.brown.dev@gmail.com>
---
v2:
	- fixed opcode
	- fixed definition to include the 'if RA=0, a=0' clause
	- fixed ccr bitshifting error
---
 arch/powerpc/lib/sstep.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 0bcf631..de3d558 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1239,6 +1239,17 @@ int analyse_instr(struct instruction_op *op, struct pt_regs *regs,
 /*
  * Logical instructions
  */
+		case 15:	/* isel */
+			mb = (instr >> 6) & 0x1f; /* bc */
+			val = (regs->ccr >> (31 - mb)) & 1;
+			val2 = (ra) ? regs->gpr[ra] : 0;
+
+			if (val)
+				regs->gpr[rd] = val2;
+			else
+				regs->gpr[rd] = regs->gpr[rb];
+			goto logical_done;
+
 		case 26:	/* cntlzw */
 			asm("cntlzw %0,%1" : "=r" (regs->gpr[ra]) :
 			    "r" (regs->gpr[rd]));
-- 
2.9.3