Patchwork [1/2] powerpc: Move opcode definitions from kvm/emulate.c to asm/ppc-opcode.h

login
register
mail settings
Submitter Hongtao Jia
Date April 23, 2013, 2:39 a.m.
Message ID <1366684776-10946-1-git-send-email-hongtao.jia@freescale.com>
Download mbox | patch
Permalink /patch/238747/
State Superseded
Headers show

Comments

Hongtao Jia - April 23, 2013, 2:39 a.m.
Opcode and xopcode are useful definitions not just for KVM. Move these
definitions to asm/ppc-opcode.h for public use.

Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
 arch/powerpc/include/asm/ppc-opcode.h | 45 +++++++++++++++++++++++++++++++++++
 arch/powerpc/kvm/emulate.c            | 44 +---------------------------------
 2 files changed, 46 insertions(+), 43 deletions(-)
Michael Ellerman - April 23, 2013, 5:29 a.m.
On Tue, Apr 23, 2013 at 10:39:35AM +0800, Jia Hongtao wrote:
> Opcode and xopcode are useful definitions not just for KVM. Move these
> definitions to asm/ppc-opcode.h for public use.

Agreed. Though nearly everything else in ppc-opcode.h uses PPC_INST_FOO,
or at least PPC_FOO, any reason not to update these to match?

cheers

> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index 8752bc8..18de83a 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -81,6 +81,51 @@
>  #define	__REGA0_R30	30
>  #define	__REGA0_R31	31
>  
> +/* opcode and xopcode for instructions */
> +#define OP_TRAP 3
> +#define OP_TRAP_64 2
> +
> +#define OP_31_XOP_TRAP      4
> +#define OP_31_XOP_LWZX      23
> +#define OP_31_XOP_LWZUX     55
> +#define OP_31_XOP_TRAP_64   68
> +#define OP_31_XOP_DCBF      86
> +#define OP_31_XOP_LBZX      87
> +#define OP_31_XOP_STWX      151
> +#define OP_31_XOP_STBX      215
> +#define OP_31_XOP_LBZUX     119
> +#define OP_31_XOP_STBUX     247
> +#define OP_31_XOP_LHZX      279
> +#define OP_31_XOP_LHZUX     311
> +#define OP_31_XOP_MFSPR     339
> +#define OP_31_XOP_LHAX      343
> +#define OP_31_XOP_STHX      407
> +#define OP_31_XOP_STHUX     439
> +#define OP_31_XOP_MTSPR     467
> +#define OP_31_XOP_DCBI      470
> +#define OP_31_XOP_LWBRX     534
> +#define OP_31_XOP_TLBSYNC   566
> +#define OP_31_XOP_STWBRX    662
> +#define OP_31_XOP_LHBRX     790
> +#define OP_31_XOP_STHBRX    918
> +
> +#define OP_LWZ  32
> +#define OP_LD   58
> +#define OP_LWZU 33
> +#define OP_LBZ  34
> +#define OP_LBZU 35
> +#define OP_STW  36
> +#define OP_STWU 37
> +#define OP_STD  62
> +#define OP_STB  38
> +#define OP_STBU 39
> +#define OP_LHZ  40
> +#define OP_LHZU 41
> +#define OP_LHA  42
> +#define OP_LHAU 43
> +#define OP_STH  44
> +#define OP_STHU 45
> +
>  /* sorted alphabetically */
>  #define PPC_INST_DCBA			0x7c0005ec
>  #define PPC_INST_DCBA_MASK		0xfc0007fe
> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
> index 7a73b6f..426d3f5 100644
> --- a/arch/powerpc/kvm/emulate.c
> +++ b/arch/powerpc/kvm/emulate.c
> @@ -30,52 +30,10 @@
>  #include <asm/byteorder.h>
>  #include <asm/kvm_ppc.h>
>  #include <asm/disassemble.h>
> +#include <asm/ppc-opcode.h>
>  #include "timing.h"
>  #include "trace.h"
>  
> -#define OP_TRAP 3
> -#define OP_TRAP_64 2
> -
> -#define OP_31_XOP_TRAP      4
> -#define OP_31_XOP_LWZX      23
> -#define OP_31_XOP_TRAP_64   68
> -#define OP_31_XOP_DCBF      86
> -#define OP_31_XOP_LBZX      87
> -#define OP_31_XOP_STWX      151
> -#define OP_31_XOP_STBX      215
> -#define OP_31_XOP_LBZUX     119
> -#define OP_31_XOP_STBUX     247
> -#define OP_31_XOP_LHZX      279
> -#define OP_31_XOP_LHZUX     311
> -#define OP_31_XOP_MFSPR     339
> -#define OP_31_XOP_LHAX      343
> -#define OP_31_XOP_STHX      407
> -#define OP_31_XOP_STHUX     439
> -#define OP_31_XOP_MTSPR     467
> -#define OP_31_XOP_DCBI      470
> -#define OP_31_XOP_LWBRX     534
> -#define OP_31_XOP_TLBSYNC   566
> -#define OP_31_XOP_STWBRX    662
> -#define OP_31_XOP_LHBRX     790
> -#define OP_31_XOP_STHBRX    918
> -
> -#define OP_LWZ  32
> -#define OP_LD   58
> -#define OP_LWZU 33
> -#define OP_LBZ  34
> -#define OP_LBZU 35
> -#define OP_STW  36
> -#define OP_STWU 37
> -#define OP_STD  62
> -#define OP_STB  38
> -#define OP_STBU 39
> -#define OP_LHZ  40
> -#define OP_LHZU 41
> -#define OP_LHA  42
> -#define OP_LHAU 43
> -#define OP_STH  44
> -#define OP_STHU 45
> -
>  void kvmppc_emulate_dec(struct kvm_vcpu *vcpu)
>  {
>  	unsigned long dec_nsec;
> -- 
> 1.8.0
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
Hongtao Jia - April 23, 2013, 6:36 a.m.
> -----Original Message-----
> From: Linuxppc-dev [mailto:linuxppc-dev-
> bounces+b38951=freescale.com@lists.ozlabs.org] On Behalf Of Michael
> Ellerman
> Sent: Tuesday, April 23, 2013 1:30 PM
> To: Jia Hongtao-B38951
> Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 1/2] powerpc: Move opcode definitions from
> kvm/emulate.c to asm/ppc-opcode.h
> 
> On Tue, Apr 23, 2013 at 10:39:35AM +0800, Jia Hongtao wrote:
> > Opcode and xopcode are useful definitions not just for KVM. Move these
> > definitions to asm/ppc-opcode.h for public use.
> 
> Agreed. Though nearly everything else in ppc-opcode.h uses PPC_INST_FOO,
> or at least PPC_FOO, any reason not to update these to match?
> 
> cheers

These definitions are firstly used by KVM defined like OP_31_XOP_TRAP.
Two ways to extract these definitions for public use:

1. Like this patch did. For keeping the KVM code that using these
   definitions unchanged we do not update them to match.

2. Move these definitions to another .h file like my last patch did:
   http://patchwork.ozlabs.org/patch/235646/
   You can see the comments there.

Thanks.
-Hongtao
Benjamin Herrenschmidt - April 24, 2013, 4:19 a.m.
On Tue, 2013-04-23 at 06:36 +0000, Jia Hongtao-B38951 wrote:
> These definitions are firstly used by KVM defined like OP_31_XOP_TRAP.
> Two ways to extract these definitions for public use:
> 
> 1. Like this patch did. For keeping the KVM code that using these
>    definitions unchanged we do not update them to match.
> 
> 2. Move these definitions to another .h file like my last patch did:
>    http://patchwork.ozlabs.org/patch/235646/
>    You can see the comments there.

There's a better way ... but it's more work.

All opcodes are based on a primary opcode and a potential secondary
opcode. You could/should rework ppc-opcodes.h to in fact define them
all that way as well, which would "reconcile" the KVM way and the
existing stuff.

Cheers,
Ben.

> Thanks.
> -Hongtao
> 
> 
> 
> 
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
Hongtao Jia - April 26, 2013, 2:40 a.m.
> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
> Sent: Wednesday, April 24, 2013 12:19 PM
> To: Jia Hongtao-B38951
> Cc: Michael Ellerman; Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 1/2] powerpc: Move opcode definitions from
> kvm/emulate.c to asm/ppc-opcode.h
> 
> On Tue, 2013-04-23 at 06:36 +0000, Jia Hongtao-B38951 wrote:
> > These definitions are firstly used by KVM defined like OP_31_XOP_TRAP.
> > Two ways to extract these definitions for public use:
> >
> > 1. Like this patch did. For keeping the KVM code that using these
> >    definitions unchanged we do not update them to match.
> >
> > 2. Move these definitions to another .h file like my last patch did:
> >    http://patchwork.ozlabs.org/patch/235646/
> >    You can see the comments there.
> 
> There's a better way ... but it's more work.
> 
> All opcodes are based on a primary opcode and a potential secondary
> opcode. You could/should rework ppc-opcodes.h to in fact define them
> all that way as well, which would "reconcile" the KVM way and the
> existing stuff.
> 
> Cheers,
> Ben.
> 

Agree. But I'm afraid to say that I'm too busy to do this "more work"
right now. Could we defer this work for some time?

Thanks.
- Hongtao

Patch

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 8752bc8..18de83a 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -81,6 +81,51 @@ 
 #define	__REGA0_R30	30
 #define	__REGA0_R31	31
 
+/* opcode and xopcode for instructions */
+#define OP_TRAP 3
+#define OP_TRAP_64 2
+
+#define OP_31_XOP_TRAP      4
+#define OP_31_XOP_LWZX      23
+#define OP_31_XOP_LWZUX     55
+#define OP_31_XOP_TRAP_64   68
+#define OP_31_XOP_DCBF      86
+#define OP_31_XOP_LBZX      87
+#define OP_31_XOP_STWX      151
+#define OP_31_XOP_STBX      215
+#define OP_31_XOP_LBZUX     119
+#define OP_31_XOP_STBUX     247
+#define OP_31_XOP_LHZX      279
+#define OP_31_XOP_LHZUX     311
+#define OP_31_XOP_MFSPR     339
+#define OP_31_XOP_LHAX      343
+#define OP_31_XOP_STHX      407
+#define OP_31_XOP_STHUX     439
+#define OP_31_XOP_MTSPR     467
+#define OP_31_XOP_DCBI      470
+#define OP_31_XOP_LWBRX     534
+#define OP_31_XOP_TLBSYNC   566
+#define OP_31_XOP_STWBRX    662
+#define OP_31_XOP_LHBRX     790
+#define OP_31_XOP_STHBRX    918
+
+#define OP_LWZ  32
+#define OP_LD   58
+#define OP_LWZU 33
+#define OP_LBZ  34
+#define OP_LBZU 35
+#define OP_STW  36
+#define OP_STWU 37
+#define OP_STD  62
+#define OP_STB  38
+#define OP_STBU 39
+#define OP_LHZ  40
+#define OP_LHZU 41
+#define OP_LHA  42
+#define OP_LHAU 43
+#define OP_STH  44
+#define OP_STHU 45
+
 /* sorted alphabetically */
 #define PPC_INST_DCBA			0x7c0005ec
 #define PPC_INST_DCBA_MASK		0xfc0007fe
diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
index 7a73b6f..426d3f5 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -30,52 +30,10 @@ 
 #include <asm/byteorder.h>
 #include <asm/kvm_ppc.h>
 #include <asm/disassemble.h>
+#include <asm/ppc-opcode.h>
 #include "timing.h"
 #include "trace.h"
 
-#define OP_TRAP 3
-#define OP_TRAP_64 2
-
-#define OP_31_XOP_TRAP      4
-#define OP_31_XOP_LWZX      23
-#define OP_31_XOP_TRAP_64   68
-#define OP_31_XOP_DCBF      86
-#define OP_31_XOP_LBZX      87
-#define OP_31_XOP_STWX      151
-#define OP_31_XOP_STBX      215
-#define OP_31_XOP_LBZUX     119
-#define OP_31_XOP_STBUX     247
-#define OP_31_XOP_LHZX      279
-#define OP_31_XOP_LHZUX     311
-#define OP_31_XOP_MFSPR     339
-#define OP_31_XOP_LHAX      343
-#define OP_31_XOP_STHX      407
-#define OP_31_XOP_STHUX     439
-#define OP_31_XOP_MTSPR     467
-#define OP_31_XOP_DCBI      470
-#define OP_31_XOP_LWBRX     534
-#define OP_31_XOP_TLBSYNC   566
-#define OP_31_XOP_STWBRX    662
-#define OP_31_XOP_LHBRX     790
-#define OP_31_XOP_STHBRX    918
-
-#define OP_LWZ  32
-#define OP_LD   58
-#define OP_LWZU 33
-#define OP_LBZ  34
-#define OP_LBZU 35
-#define OP_STW  36
-#define OP_STWU 37
-#define OP_STD  62
-#define OP_STB  38
-#define OP_STBU 39
-#define OP_LHZ  40
-#define OP_LHZU 41
-#define OP_LHA  42
-#define OP_LHAU 43
-#define OP_STH  44
-#define OP_STHU 45
-
 void kvmppc_emulate_dec(struct kvm_vcpu *vcpu)
 {
 	unsigned long dec_nsec;