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

2017-07-14 Thread Segher Boessenkool 
On Fri, Jul 14, 2017 at 02:19:34PM +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?
> 
> 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

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

2017-07-13 Thread Matt Brown 
On Thu, Jul 13, 2017 at 5:28 PM, Segher Boessenkool
 wrote:
> 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?

Why would it be big-endian? Wouldn't it be in the same endian form as the arch?
>
>> + }
>> + regs->gpr[ra] = 0 | perm;
>
> And that is just silly :-)
>

Yep that is silly.

Thanks,
Matt

>> +}
>
>
> Segher

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

2017-07-13 Thread Segher Boessenkool 
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

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

2017-07-13 Thread Gabriel Paubert 
On Thu, Jul 13, 2017 at 01:25:46PM +1000, Matt Brown wrote:
> This adds emulation for the bpermd instruction.
> 
> Signed-off-by: Matt Brown 
> ---
>  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;

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

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

Gabriel
> +}
> +
>  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

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

2017-07-12 Thread Matt Brown 
This adds emulation for the bpermd instruction.

Signed-off-by: Matt Brown 
---
 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