[Qemu-devel] [PATCH 0/2] target-ppc: vector instruction fixes

[Qemu-devel] [PATCH 0/2] target-ppc: vector instruction fixes

[Aurelien Jarno]

This patchset fixes some vector instructions which are incorrectly
decoded or implemented. The first patch is needed to run recent version
of openssl, as it enabled POWER8 instrutctions when it detects such a
CPU.

Aurelien Jarno (2):
  target-ppc: fix vcipher, vcipherlast, vncipherlast and vpermxor
  target-ppc: fix xscmpodp and xscmpudp decoding

 target-ppc/int_helper.c | 19 ++++++++++++++-----
 target-ppc/translate.c  | 11 +++++++++--
 2 files changed, 23 insertions(+), 7 deletions(-)

Cc: Tom Musta <tommusta@gmail.com>
Cc: Alexander Graf <agraf@suse.de>

-- 
2.1.4

[Qemu-devel] [PATCH 1/2] target-ppc: fix vcipher, vcipherlast, vncipherlast and vpermxor

[Aurelien Jarno]

For vector instructions, the helpers get pointers to the vector register
in arguments. Some operands might point to the same register, including
the operand holding the result.

When emulating instructions which access the vector elements in a
non-linear way, we need to store the result in an temporary variable.

This fixes openssl when emulating a POWER8 CPU.

Cc: Tom Musta <tommusta@gmail.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: qemu-stable@nongnu.org
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-ppc/int_helper.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 0a55d5e..b122868 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -2327,24 +2327,28 @@ void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
 
 void helper_vcipher(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
 {
+    ppc_avr_t result;
     int i;
 
     VECTOR_FOR_INORDER_I(i, u32) {
-        r->AVRW(i) = b->AVRW(i) ^
+        result.AVRW(i) = b->AVRW(i) ^
             (AES_Te0[a->AVRB(AES_shifts[4*i + 0])] ^
              AES_Te1[a->AVRB(AES_shifts[4*i + 1])] ^
              AES_Te2[a->AVRB(AES_shifts[4*i + 2])] ^
              AES_Te3[a->AVRB(AES_shifts[4*i + 3])]);
     }
+    *r = result;
 }
 
 void helper_vcipherlast(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
 {
+    ppc_avr_t result;
     int i;
 
     VECTOR_FOR_INORDER_I(i, u8) {
-        r->AVRB(i) = b->AVRB(i) ^ (AES_sbox[a->AVRB(AES_shifts[i])]);
+        result.AVRB(i) = b->AVRB(i) ^ (AES_sbox[a->AVRB(AES_shifts[i])]);
     }
+    *r = result;
 }
 
 void helper_vncipher(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
@@ -2369,11 +2373,13 @@ void helper_vncipher(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
 
 void helper_vncipherlast(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
 {
+    ppc_avr_t result;
     int i;
 
     VECTOR_FOR_INORDER_I(i, u8) {
-        r->AVRB(i) = b->AVRB(i) ^ (AES_isbox[a->AVRB(AES_ishifts[i])]);
+        result.AVRB(i) = b->AVRB(i) ^ (AES_isbox[a->AVRB(AES_ishifts[i])]);
     }
+    *r = result;
 }
 
 #define ROTRu32(v, n) (((v) >> (n)) | ((v) << (32-n)))
@@ -2460,16 +2466,19 @@ void helper_vshasigmad(ppc_avr_t *r,  ppc_avr_t *a, uint32_t st_six)
 
 void helper_vpermxor(ppc_avr_t *r,  ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t *c)
 {
+    ppc_avr_t result;
     int i;
+
     VECTOR_FOR_INORDER_I(i, u8) {
         int indexA = c->u8[i] >> 4;
         int indexB = c->u8[i] & 0xF;
 #if defined(HOST_WORDS_BIGENDIAN)
-        r->u8[i] = a->u8[indexA] ^ b->u8[indexB];
+        result.u8[i] = a->u8[indexA] ^ b->u8[indexB];
 #else
-        r->u8[i] = a->u8[15-indexA] ^ b->u8[15-indexB];
+        result.u8[i] = a->u8[15-indexA] ^ b->u8[15-indexB];
 #endif
     }
+    *r = result;
 }
 
 #undef VECTOR_FOR_INORDER_I
-- 
2.1.4

[Qemu-devel] [PATCH 2/2] target-ppc: fix xscmpodp and xscmpudp decoding

[Aurelien Jarno]

The xscmpodp and xscmpudp instructions only have the AX, BX bits in
there encoding, the lowest bit (usually TX) is marked as an invalid
bit. We therefore can't decode them with GEN_XX2FORM, which decodes
the two lowest bit.

Introduce a new form GEN_XX2FORM, which decodes AX and BX and mark
the lowest bit as invalid.

Cc: Tom Musta <tommusta@gmail.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: qemu-stable@nongnu.org
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-ppc/translate.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 84c5cea..c0eed13 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -10670,6 +10670,13 @@ GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 0, PPC_NONE, fl2), \
 GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 0, PPC_NONE, fl2), \
 GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 0, PPC_NONE, fl2)
 
+#undef GEN_XX2IFORM
+#define GEN_XX2IFORM(name, opc2, opc3, fl2)                           \
+GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0, opc3, 1, PPC_NONE, fl2), \
+GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 1, PPC_NONE, fl2), \
+GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 1, PPC_NONE, fl2), \
+GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 1, PPC_NONE, fl2)
+
 #undef GEN_XX3_RC_FORM
 #define GEN_XX3_RC_FORM(name, opc2, opc3, fl2)                          \
 GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x00, opc3 | 0x00, 0, PPC_NONE, fl2), \
@@ -10731,8 +10738,8 @@ GEN_XX3FORM(xsnmaddadp, 0x04, 0x14, PPC2_VSX),
 GEN_XX3FORM(xsnmaddmdp, 0x04, 0x15, PPC2_VSX),
 GEN_XX3FORM(xsnmsubadp, 0x04, 0x16, PPC2_VSX),
 GEN_XX3FORM(xsnmsubmdp, 0x04, 0x17, PPC2_VSX),
-GEN_XX2FORM(xscmpodp,  0x0C, 0x05, PPC2_VSX),
-GEN_XX2FORM(xscmpudp,  0x0C, 0x04, PPC2_VSX),
+GEN_XX2IFORM(xscmpodp,  0x0C, 0x05, PPC2_VSX),
+GEN_XX2IFORM(xscmpudp,  0x0C, 0x04, PPC2_VSX),
 GEN_XX3FORM(xsmaxdp, 0x00, 0x14, PPC2_VSX),
 GEN_XX3FORM(xsmindp, 0x00, 0x15, PPC2_VSX),
 GEN_XX2FORM(xscvdpsp, 0x12, 0x10, PPC2_VSX),
-- 
2.1.4

Re: [Qemu-devel] [PATCH 0/2] target-ppc: vector instruction fixes

[Richard Henderson]

On 09/13/2015 02:03 PM, Aurelien Jarno wrote:
> This patchset fixes some vector instructions which are incorrectly
> decoded or implemented. The first patch is needed to run recent version
> of openssl, as it enabled POWER8 instrutctions when it detects such a
> CPU.
>
> Aurelien Jarno (2):
>    target-ppc: fix vcipher, vcipherlast, vncipherlast and vpermxor
>    target-ppc: fix xscmpodp and xscmpudp decoding
>
>   target-ppc/int_helper.c | 19 ++++++++++++++-----
>   target-ppc/translate.c  | 11 +++++++++--
>   2 files changed, 23 insertions(+), 7 deletions(-)
>
> Cc: Tom Musta <tommusta@gmail.com>
> Cc: Alexander Graf <agraf@suse.de>
>

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH 2/2] target-ppc: fix xscmpodp and xscmpudp decoding

[Richard W.M. Jones]

On Sun, Sep 13, 2015 at 11:03:45PM +0200, Aurelien Jarno wrote:
> The xscmpodp and xscmpudp instructions only have the AX, BX bits in
> there encoding, the lowest bit (usually TX) is marked as an invalid
> bit. We therefore can't decode them with GEN_XX2FORM, which decodes
> the two lowest bit.
> 
> Introduce a new form GEN_XX2FORM, which decodes AX and BX and mark
> the lowest bit as invalid.
> 
> Cc: Tom Musta <tommusta@gmail.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target-ppc/translate.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 84c5cea..c0eed13 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -10670,6 +10670,13 @@ GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 0, PPC_NONE, fl2), \
>  GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 0, PPC_NONE, fl2), \
>  GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 0, PPC_NONE, fl2)
>  
> +#undef GEN_XX2IFORM
> +#define GEN_XX2IFORM(name, opc2, opc3, fl2)                           \
> +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0, opc3, 1, PPC_NONE, fl2), \
> +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 1, PPC_NONE, fl2), \
> +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 1, PPC_NONE, fl2), \
> +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 1, PPC_NONE, fl2)
> +
>  #undef GEN_XX3_RC_FORM
>  #define GEN_XX3_RC_FORM(name, opc2, opc3, fl2)                          \
>  GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x00, opc3 | 0x00, 0, PPC_NONE, fl2), \
> @@ -10731,8 +10738,8 @@ GEN_XX3FORM(xsnmaddadp, 0x04, 0x14, PPC2_VSX),
>  GEN_XX3FORM(xsnmaddmdp, 0x04, 0x15, PPC2_VSX),
>  GEN_XX3FORM(xsnmsubadp, 0x04, 0x16, PPC2_VSX),
>  GEN_XX3FORM(xsnmsubmdp, 0x04, 0x17, PPC2_VSX),
> -GEN_XX2FORM(xscmpodp,  0x0C, 0x05, PPC2_VSX),
> -GEN_XX2FORM(xscmpudp,  0x0C, 0x04, PPC2_VSX),
> +GEN_XX2IFORM(xscmpodp,  0x0C, 0x05, PPC2_VSX),
> +GEN_XX2IFORM(xscmpudp,  0x0C, 0x04, PPC2_VSX),
>  GEN_XX3FORM(xsmaxdp, 0x00, 0x14, PPC2_VSX),
>  GEN_XX3FORM(xsmindp, 0x00, 0x15, PPC2_VSX),
>  GEN_XX2FORM(xscvdpsp, 0x12, 0x10, PPC2_VSX),

This particular patch fixed a number of crashes I was experiencing in
libm in Fedora 22 (ppc64), which I traced to the xscmpudp instruction
causing SIGILL.

So:

  Tested-by: Richard W.M. Jones <rjones@redhat.com>

Cole: I think we should add this small patch series to Fedora, even
though it's not upstream yet, since it required for running F22/ppc64
guests reliably (on non-ppc hosts).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Re: [Qemu-devel] [PATCH 0/2] target-ppc: vector instruction fixes

[Alexander Graf]

On 17.09.15 06:58, Richard Henderson wrote:
> On 09/13/2015 02:03 PM, Aurelien Jarno wrote:
>> This patchset fixes some vector instructions which are incorrectly
>> decoded or implemented. The first patch is needed to run recent version
>> of openssl, as it enabled POWER8 instrutctions when it detects such a
>> CPU.
>>
>> Aurelien Jarno (2):
>>    target-ppc: fix vcipher, vcipherlast, vncipherlast and vpermxor
>>    target-ppc: fix xscmpodp and xscmpudp decoding
>>
>>   target-ppc/int_helper.c | 19 ++++++++++++++-----
>>   target-ppc/translate.c  | 11 +++++++++--
>>   2 files changed, 23 insertions(+), 7 deletions(-)
>>
>> Cc: Tom Musta <tommusta@gmail.com>
>> Cc: Alexander Graf <agraf@suse.de>
>>
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>

Thanks, applied both to ppc-next.

Richard and Aurelien, could you please try to get PPC and s390 TCG
patches into the tree without me for the next 2 months? I'm on parental
leave right now and will try my best to disappear from email for about 2
months starting tomorrow ;).

Also Aurelien, please make sure to CC qemu-ppc on patches to the ppc
code base. It makes it easier for people to keep track of your patches.


Thanks!

Alex

Re: [Qemu-devel] [PATCH 1/2] target-ppc: fix vcipher, vcipherlast, vncipherlast and vpermxor

[Thomas Huth]

On 13/09/15 23:03, Aurelien Jarno wrote:
> For vector instructions, the helpers get pointers to the vector register
> in arguments. Some operands might point to the same register, including
> the operand holding the result.
> 
> When emulating instructions which access the vector elements in a
> non-linear way, we need to store the result in an temporary variable.
> 
> This fixes openssl when emulating a POWER8 CPU.
> 
> Cc: Tom Musta <tommusta@gmail.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target-ppc/int_helper.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
> index 0a55d5e..b122868 100644
> --- a/target-ppc/int_helper.c
> +++ b/target-ppc/int_helper.c
> @@ -2327,24 +2327,28 @@ void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a)
>  
>  void helper_vcipher(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
>  {
> +    ppc_avr_t result;
>      int i;
>  
>      VECTOR_FOR_INORDER_I(i, u32) {
> -        r->AVRW(i) = b->AVRW(i) ^
> +        result.AVRW(i) = b->AVRW(i) ^
>              (AES_Te0[a->AVRB(AES_shifts[4*i + 0])] ^
>               AES_Te1[a->AVRB(AES_shifts[4*i + 1])] ^
>               AES_Te2[a->AVRB(AES_shifts[4*i + 2])] ^
>               AES_Te3[a->AVRB(AES_shifts[4*i + 3])]);
>      }
> +    *r = result;
>  }
>  
>  void helper_vcipherlast(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
>  {
> +    ppc_avr_t result;
>      int i;
>  
>      VECTOR_FOR_INORDER_I(i, u8) {
> -        r->AVRB(i) = b->AVRB(i) ^ (AES_sbox[a->AVRB(AES_shifts[i])]);
> +        result.AVRB(i) = b->AVRB(i) ^ (AES_sbox[a->AVRB(AES_shifts[i])]);
>      }
> +    *r = result;
>  }
>  
>  void helper_vncipher(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
> @@ -2369,11 +2373,13 @@ void helper_vncipher(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
>  
>  void helper_vncipherlast(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
>  {
> +    ppc_avr_t result;
>      int i;
>  
>      VECTOR_FOR_INORDER_I(i, u8) {
> -        r->AVRB(i) = b->AVRB(i) ^ (AES_isbox[a->AVRB(AES_ishifts[i])]);
> +        result.AVRB(i) = b->AVRB(i) ^ (AES_isbox[a->AVRB(AES_ishifts[i])]);
>      }
> +    *r = result;
>  }
>  
>  #define ROTRu32(v, n) (((v) >> (n)) | ((v) << (32-n)))
> @@ -2460,16 +2466,19 @@ void helper_vshasigmad(ppc_avr_t *r,  ppc_avr_t *a, uint32_t st_six)
>  
>  void helper_vpermxor(ppc_avr_t *r,  ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t *c)
>  {
> +    ppc_avr_t result;
>      int i;
> +
>      VECTOR_FOR_INORDER_I(i, u8) {
>          int indexA = c->u8[i] >> 4;
>          int indexB = c->u8[i] & 0xF;
>  #if defined(HOST_WORDS_BIGENDIAN)
> -        r->u8[i] = a->u8[indexA] ^ b->u8[indexB];
> +        result.u8[i] = a->u8[indexA] ^ b->u8[indexB];
>  #else
> -        r->u8[i] = a->u8[15-indexA] ^ b->u8[15-indexB];
> +        result.u8[i] = a->u8[15-indexA] ^ b->u8[15-indexB];
>  #endif
>      }
> +    *r = result;
>  }
>  
>  #undef VECTOR_FOR_INORDER_I
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH 2/2] target-ppc: fix xscmpodp and xscmpudp decoding

[Thomas Huth]

On 13/09/15 23:03, Aurelien Jarno wrote:
> The xscmpodp and xscmpudp instructions only have the AX, BX bits in
> there encoding, the lowest bit (usually TX) is marked as an invalid
> bit. We therefore can't decode them with GEN_XX2FORM, which decodes
> the two lowest bit.
> 
> Introduce a new form GEN_XX2FORM, which decodes AX and BX and mark
> the lowest bit as invalid.
> 
> Cc: Tom Musta <tommusta@gmail.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target-ppc/translate.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 84c5cea..c0eed13 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -10670,6 +10670,13 @@ GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 0, PPC_NONE, fl2), \
>  GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 0, PPC_NONE, fl2), \
>  GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 0, PPC_NONE, fl2)
>  
> +#undef GEN_XX2IFORM
> +#define GEN_XX2IFORM(name, opc2, opc3, fl2)                           \
> +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0, opc3, 1, PPC_NONE, fl2), \
> +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 1, PPC_NONE, fl2), \
> +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 1, PPC_NONE, fl2), \
> +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 1, PPC_NONE, fl2)
> +
>  #undef GEN_XX3_RC_FORM
>  #define GEN_XX3_RC_FORM(name, opc2, opc3, fl2)                          \
>  GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x00, opc3 | 0x00, 0, PPC_NONE, fl2), \
> @@ -10731,8 +10738,8 @@ GEN_XX3FORM(xsnmaddadp, 0x04, 0x14, PPC2_VSX),
>  GEN_XX3FORM(xsnmaddmdp, 0x04, 0x15, PPC2_VSX),
>  GEN_XX3FORM(xsnmsubadp, 0x04, 0x16, PPC2_VSX),
>  GEN_XX3FORM(xsnmsubmdp, 0x04, 0x17, PPC2_VSX),
> -GEN_XX2FORM(xscmpodp,  0x0C, 0x05, PPC2_VSX),
> -GEN_XX2FORM(xscmpudp,  0x0C, 0x04, PPC2_VSX),
> +GEN_XX2IFORM(xscmpodp,  0x0C, 0x05, PPC2_VSX),
> +GEN_XX2IFORM(xscmpudp,  0x0C, 0x04, PPC2_VSX),

According to PowerISA 2.07, xscmpodp and xscmpudp are of type XX3, not
of type XX2 ... so should this macro maybe rather be named XX3IFORM instead?

 Thomas

Re: [Qemu-devel] [PATCH 2/2] target-ppc: fix xscmpodp and xscmpudp decoding

[Aurelien Jarno]

On 2015-09-22 12:26, Thomas Huth wrote:
> On 13/09/15 23:03, Aurelien Jarno wrote:
> > The xscmpodp and xscmpudp instructions only have the AX, BX bits in
> > there encoding, the lowest bit (usually TX) is marked as an invalid
> > bit. We therefore can't decode them with GEN_XX2FORM, which decodes
> > the two lowest bit.
> > 
> > Introduce a new form GEN_XX2FORM, which decodes AX and BX and mark
> > the lowest bit as invalid.
> > 
> > Cc: Tom Musta <tommusta@gmail.com>
> > Cc: Alexander Graf <agraf@suse.de>
> > Cc: qemu-stable@nongnu.org
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> > ---
> >  target-ppc/translate.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> > index 84c5cea..c0eed13 100644
> > --- a/target-ppc/translate.c
> > +++ b/target-ppc/translate.c
> > @@ -10670,6 +10670,13 @@ GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 0, PPC_NONE, fl2), \
> >  GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 0, PPC_NONE, fl2), \
> >  GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 0, PPC_NONE, fl2)
> >  
> > +#undef GEN_XX2IFORM
> > +#define GEN_XX2IFORM(name, opc2, opc3, fl2)                           \
> > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0, opc3, 1, PPC_NONE, fl2), \
> > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 1, PPC_NONE, fl2), \
> > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 1, PPC_NONE, fl2), \
> > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 1, PPC_NONE, fl2)
> > +
> >  #undef GEN_XX3_RC_FORM
> >  #define GEN_XX3_RC_FORM(name, opc2, opc3, fl2)                          \
> >  GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x00, opc3 | 0x00, 0, PPC_NONE, fl2), \
> > @@ -10731,8 +10738,8 @@ GEN_XX3FORM(xsnmaddadp, 0x04, 0x14, PPC2_VSX),
> >  GEN_XX3FORM(xsnmaddmdp, 0x04, 0x15, PPC2_VSX),
> >  GEN_XX3FORM(xsnmsubadp, 0x04, 0x16, PPC2_VSX),
> >  GEN_XX3FORM(xsnmsubmdp, 0x04, 0x17, PPC2_VSX),
> > -GEN_XX2FORM(xscmpodp,  0x0C, 0x05, PPC2_VSX),
> > -GEN_XX2FORM(xscmpudp,  0x0C, 0x04, PPC2_VSX),
> > +GEN_XX2IFORM(xscmpodp,  0x0C, 0x05, PPC2_VSX),
> > +GEN_XX2IFORM(xscmpudp,  0x0C, 0x04, PPC2_VSX),
> 
> According to PowerISA 2.07, xscmpodp and xscmpudp are of type XX3, not
> of type XX2 ... so should this macro maybe rather be named XX3IFORM instead?

Indeed, I have chosen the name without actually realizing the manual
also give names. Then I do wonder if the lower bit is really decoded as
invalid, I wouldn't be surprised it is actually just ignored.

I'll try to do some tests on real hardware and come up with a fixup
patch.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 2/2] target-ppc: fix xscmpodp and xscmpudp decoding

[Tom Musta]

[-- Attachment #1: Type: text/plain, Size: 3545 bytes --]

The modern versions of the ISA require that reserved instruction bits be
ignored for server class processors (see Book I, section 1.3.3).  I believe
older versions of the ISA allowed for Illegal Instruction Interrupt or
"Boundedly Undefined", which is, of course, much less specific.  QEMU
supports implementations from both eras and, as a general rule, flags this
situation as an illegal instruction.

So I would expect that real hardware will ignore the bit.  You will still
be left with the choice of making this decoder consistent with the hardware
or consistent with the rest of QEMU :)   When I added support for VSX, the
intent was the latter.

On Tue, Sep 22, 2015 at 4:28 PM, Aurelien Jarno <aurelien@aurel32.net>
wrote:

> On 2015-09-22 12:26, Thomas Huth wrote:
> > On 13/09/15 23:03, Aurelien Jarno wrote:
> > > The xscmpodp and xscmpudp instructions only have the AX, BX bits in
> > > there encoding, the lowest bit (usually TX) is marked as an invalid
> > > bit. We therefore can't decode them with GEN_XX2FORM, which decodes
> > > the two lowest bit.
> > >
> > > Introduce a new form GEN_XX2FORM, which decodes AX and BX and mark
> > > the lowest bit as invalid.
> > >
> > > Cc: Tom Musta <tommusta@gmail.com>
> > > Cc: Alexander Graf <agraf@suse.de>
> > > Cc: qemu-stable@nongnu.org
> > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> > > ---
> > >  target-ppc/translate.c | 11 +++++++++--
> > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> > > index 84c5cea..c0eed13 100644
> > > --- a/target-ppc/translate.c
> > > +++ b/target-ppc/translate.c
> > > @@ -10670,6 +10670,13 @@ GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1,
> opc3, 0, PPC_NONE, fl2), \
> > >  GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 0, PPC_NONE, fl2), \
> > >  GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 0, PPC_NONE, fl2)
> > >
> > > +#undef GEN_XX2IFORM
> > > +#define GEN_XX2IFORM(name, opc2, opc3, fl2)
>  \
> > > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0, opc3, 1, PPC_NONE, fl2), \
> > > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 1, PPC_NONE, fl2), \
> > > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 1, PPC_NONE, fl2), \
> > > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 1, PPC_NONE, fl2)
> > > +
> > >  #undef GEN_XX3_RC_FORM
> > >  #define GEN_XX3_RC_FORM(name, opc2, opc3, fl2)
>   \
> > >  GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x00, opc3 | 0x00, 0,
> PPC_NONE, fl2), \
> > > @@ -10731,8 +10738,8 @@ GEN_XX3FORM(xsnmaddadp, 0x04, 0x14, PPC2_VSX),
> > >  GEN_XX3FORM(xsnmaddmdp, 0x04, 0x15, PPC2_VSX),
> > >  GEN_XX3FORM(xsnmsubadp, 0x04, 0x16, PPC2_VSX),
> > >  GEN_XX3FORM(xsnmsubmdp, 0x04, 0x17, PPC2_VSX),
> > > -GEN_XX2FORM(xscmpodp,  0x0C, 0x05, PPC2_VSX),
> > > -GEN_XX2FORM(xscmpudp,  0x0C, 0x04, PPC2_VSX),
> > > +GEN_XX2IFORM(xscmpodp,  0x0C, 0x05, PPC2_VSX),
> > > +GEN_XX2IFORM(xscmpudp,  0x0C, 0x04, PPC2_VSX),
> >
> > According to PowerISA 2.07, xscmpodp and xscmpudp are of type XX3, not
> > of type XX2 ... so should this macro maybe rather be named XX3IFORM
> instead?
>
> Indeed, I have chosen the name without actually realizing the manual
> also give names. Then I do wonder if the lower bit is really decoded as
> invalid, I wouldn't be surprised it is actually just ignored.
>
> I'll try to do some tests on real hardware and come up with a fixup
> patch.
>
> Aurelien
>
> --
> Aurelien Jarno                          GPG: 4096R/1DDD8C9B
> aurelien@aurel32.net                 http://www.aurel32.net
>

[-- Attachment #2: Type: text/html, Size: 4837 bytes --]