Re: [PATCH v8 08/30] powerpc: Use a function for getting the instruction op code

2020-05-17 Thread Jordan Niethe 
On Sat, May 16, 2020 at 9:08 PM Michael Ellerman  wrote:
>
> Jordan Niethe  writes:
> > mpe, as suggested by Christophe could you please add this.
>
> I did that and ...
>
> > diff --git a/arch/powerpc/include/asm/inst.h 
> > b/arch/powerpc/include/asm/inst.h
> > --- a/arch/powerpc/include/asm/inst.h
> > +++ b/arch/powerpc/include/asm/inst.h
> > @@ -2,6 +2,8 @@
> >  #ifndef _ASM_INST_H
> >  #define _ASM_INST_H
> >
> > +#include 
>
> .. this eventually breaks the build in some driver, because get_ra() is
> redefined.
>
> So I've backed out this change for now.
Thanks, that is fine with me.
>
> If we want to use the macros in disassemble.h we'll need to namespace
> them better, eg. make them ppc_get_ra() and so on.
>
> cheers
>
> >  /*
> >   * Instruction data type for POWER
> >   */
> > @@ -15,7 +17,7 @@ static inline u32 ppc_inst_val(u32 x)
> >
> >  static inline int ppc_inst_primary_opcode(u32 x)
> >  {
> > -return ppc_inst_val(x) >> 26;
> > +return get_op(ppc_inst_val(x));
> >  }
> >
> >  #endif /* _ASM_INST_H */
> > --
> > 2.17.1

Re: [PATCH v8 08/30] powerpc: Use a function for getting the instruction op code

2020-05-16 Thread Michael Ellerman 
Jordan Niethe  writes:
> mpe, as suggested by Christophe could you please add this.

I did that and ...

> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -2,6 +2,8 @@
>  #ifndef _ASM_INST_H
>  #define _ASM_INST_H
>
> +#include 

.. this eventually breaks the build in some driver, because get_ra() is
redefined.

So I've backed out this change for now.

If we want to use the macros in disassemble.h we'll need to namespace
them better, eg. make them ppc_get_ra() and so on.

cheers

>  /*
>   * Instruction data type for POWER
>   */
> @@ -15,7 +17,7 @@ static inline u32 ppc_inst_val(u32 x)
>
>  static inline int ppc_inst_primary_opcode(u32 x)
>  {
> -return ppc_inst_val(x) >> 26;
> +return get_op(ppc_inst_val(x));
>  }
>
>  #endif /* _ASM_INST_H */
> -- 
> 2.17.1

Re: [PATCH v8 08/30] powerpc: Use a function for getting the instruction op code

2020-05-15 Thread Jordan Niethe 
mpe, as suggested by Christophe could you please add this.
diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -2,6 +2,8 @@
 #ifndef _ASM_INST_H
 #define _ASM_INST_H

+#include 
+
 /*
  * Instruction data type for POWER
  */
@@ -15,7 +17,7 @@ static inline u32 ppc_inst_val(u32 x)

 static inline int ppc_inst_primary_opcode(u32 x)
 {
-return ppc_inst_val(x) >> 26;
+return get_op(ppc_inst_val(x));
 }

 #endif /* _ASM_INST_H */
-- 
2.17.1

[PATCH v8 08/30] powerpc: Use a function for getting the instruction op code

2020-05-05 Thread Jordan Niethe 
In preparation for using a data type for instructions that can not be
directly used with the '>>' operator use a function for getting the op
code of an instruction.

Reviewed-by: Alistair Popple 
Signed-off-by: Jordan Niethe 
---
v4: New to series
v6: - Rename ppc_inst_primary() to ppc_inst_primary_opcode()
- Use in vecemu.c, fault.c, sstep.c
- Move this patch after the ppc_inst_val() patch
---
 arch/powerpc/include/asm/inst.h  | 5 +
 arch/powerpc/kernel/align.c  | 2 +-
 arch/powerpc/kernel/vecemu.c | 3 ++-
 arch/powerpc/lib/code-patching.c | 4 ++--
 arch/powerpc/lib/sstep.c | 2 +-
 arch/powerpc/mm/fault.c  | 3 ++-
 6 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index 8a9e73bfbd27..442a95f20de7 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -13,4 +13,9 @@ static inline u32 ppc_inst_val(u32 x)
return x;
 }
 
+static inline int ppc_inst_primary_opcode(u32 x)
+{
+   return ppc_inst_val(x) >> 26;
+}
+
 #endif /* _ASM_INST_H */
diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index 44921001f84a..47dbba81a227 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -314,7 +314,7 @@ int fix_alignment(struct pt_regs *regs)
}
 
 #ifdef CONFIG_SPE
-   if ((ppc_inst_val(instr) >> 26) == 0x4) {
+   if (ppc_inst_primary_opcode(instr) == 0x4) {
int reg = (ppc_inst_val(instr) >> 21) & 0x1f;
PPC_WARN_ALIGNMENT(spe, regs);
return emulate_spe(regs, reg, instr);
diff --git a/arch/powerpc/kernel/vecemu.c b/arch/powerpc/kernel/vecemu.c
index 1f5e3b4c8ae4..a544590b90e5 100644
--- a/arch/powerpc/kernel/vecemu.c
+++ b/arch/powerpc/kernel/vecemu.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* Functions in vector.S */
 extern void vaddfp(vector128 *dst, vector128 *a, vector128 *b);
@@ -268,7 +269,7 @@ int emulate_altivec(struct pt_regs *regs)
return -EFAULT;
 
word = ppc_inst_val(instr);
-   if ((word >> 26) != 4)
+   if (ppc_inst_primary_opcode(instr) != 4)
return -EINVAL; /* not an altivec instruction */
vd = (word >> 21) & 0x1f;
va = (word >> 16) & 0x1f;
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index baa849b1a1f9..f5c6dcbac44b 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -231,7 +231,7 @@ bool is_offset_in_branch_range(long offset)
  */
 bool is_conditional_branch(unsigned int instr)
 {
-   unsigned int opcode = instr >> 26;
+   unsigned int opcode = ppc_inst_primary_opcode(instr);
 
if (opcode == 16)   /* bc, bca, bcl, bcla */
return true;
@@ -289,7 +289,7 @@ int create_cond_branch(unsigned int *instr, const unsigned 
int *addr,
 
 static unsigned int branch_opcode(unsigned int instr)
 {
-   return (instr >> 26) & 0x3F;
+   return ppc_inst_primary_opcode(instr) & 0x3F;
 }
 
 static int instr_is_branch_iform(unsigned int instr)
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 14c93ee4ffc8..7f7be154da7e 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1175,7 +1175,7 @@ int analyse_instr(struct instruction_op *op, const struct 
pt_regs *regs,
word = ppc_inst_val(instr);
op->type = COMPUTE;
 
-   opcode = instr >> 26;
+   opcode = ppc_inst_primary_opcode(instr);
switch (opcode) {
case 16:/* bc */
op->type = BRANCH;
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 9364921870df..0e7e145d5cad 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*
  * Check whether the instruction inst is a store using
@@ -52,7 +53,7 @@ static bool store_updates_sp(unsigned int inst)
if (((ppc_inst_val(inst) >> 16) & 0x1f) != 1)
return false;
/* check major opcode */
-   switch (inst >> 26) {
+   switch (ppc_inst_primary_opcode(inst)) {
case OP_STWU:
case OP_STBU:
case OP_STHU:
-- 
2.17.1