[Qemu-devel] [PATCH v4 0/3] Some improvements in z/Arch instructions support

[Qemu-devel] [PATCH v4 0/3] Some improvements in z/Arch instructions support

[Pavel Zbitskiy]

Changes since v3:
* Patches 1-6 were accepted, thanks!
* Use in1_la2 in op_cvb().
* Free a temp in op_cvb().
* Make ra const in helper_cvb().
* Use wrap_address() in helper_cvb().
* Add a minor formatting patch.
* Add LPSW(E) alignment check.

Pavel Zbitskiy (3):
  target/s390x: use regular spaces in translate.c
  target/s390x: exception on non-aligned LPSW(E)
  target/s390x: implement CVB, CVBY and CVBG

 target/s390x/helper.h           |  2 ++
 target/s390x/insn-data.def      |  4 +++
 target/s390x/int_helper.c       | 52 +++++++++++++++++++++++++++++++++
 target/s390x/mem_helper.c       | 19 +++++++-----
 target/s390x/translate.c        | 23 +++++++++++++--
 tests/tcg/s390x/Makefile.target |  1 +
 tests/tcg/s390x/cvb.c           | 18 ++++++++++++
 7 files changed, 110 insertions(+), 9 deletions(-)
 create mode 100644 tests/tcg/s390x/cvb.c

-- 
2.18.0

[Qemu-devel] [PATCH v4 1/3] target/s390x: use regular spaces in translate.c

[Pavel Zbitskiy]

In a few places translate.c contains non-breaking spaces (0xc2 0xa0)
instead of regular ones (0x20):

  7c 7c c2 a0 63 63
  7c 7c 20    63 63
  |  |        c  c

This confuses some text editors.

Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
---
 target/s390x/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 40e12ca2c4..7363aabf3a 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -842,7 +842,7 @@ static void disas_jcc(DisasContext *s, DisasCompare *c, uint32_t mask)
             cond = TCG_COND_NE;
             c->u.s32.b = tcg_const_i32(1);
             break;
-        case 0x8 | 0x2: /* cc == 0 || cc == 2 => (cc & 1) == 0 */
+        case 0x8 | 0x2: /* cc == 0 || cc == 2 => (cc & 1) == 0 */
             cond = TCG_COND_EQ;
             c->g1 = false;
             c->u.s32.a = tcg_temp_new_i32();
@@ -861,7 +861,7 @@ static void disas_jcc(DisasContext *s, DisasCompare *c, uint32_t mask)
             cond = TCG_COND_NE;
             c->u.s32.b = tcg_const_i32(0);
             break;
-        case 0x4 | 0x1: /* cc == 1 || cc == 3 => (cc & 1) != 0 */
+        case 0x4 | 0x1: /* cc == 1 || cc == 3 => (cc & 1) != 0 */
             cond = TCG_COND_NE;
             c->g1 = false;
             c->u.s32.a = tcg_temp_new_i32();
-- 
2.18.0

[Qemu-devel] [PATCH v4 2/3] target/s390x: exception on non-aligned LPSW(E)

[Pavel Zbitskiy]

Both LPSW and LPSWE should raise a specification exception when their
operand is not doubleword aligned.

This could've been done without a helper, but this would introduce a
new basic block, which would require making o->in2 local. This
could've also been done in load_psw helper, but this is too late -
specification exception should be recognized before memory accesses
take place.

Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
---
 target/s390x/helper.h     |  1 +
 target/s390x/mem_helper.c | 19 ++++++++++++-------
 target/s390x/translate.c  |  8 ++++++++
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 97c60ca7bc..b0df3267e5 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -120,6 +120,7 @@ DEF_HELPER_4(cu41, i32, env, i32, i32, i32)
 DEF_HELPER_4(cu42, i32, env, i32, i32, i32)
 DEF_HELPER_5(msa, i32, env, i32, i32, i32, i32)
 DEF_HELPER_FLAGS_1(stpt, TCG_CALL_NO_RWG, i64, env)
+DEF_HELPER_FLAGS_3(check_alignment, TCG_CALL_NO_RWG, void, env, i64, i32)
 
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_3(servc, i32, env, i64, i64)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index bacae4f503..75ca1997ad 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -81,13 +81,18 @@ static inline uint32_t adj_len_to_page(uint32_t len, uint64_t addr)
 /* Trigger a SPECIFICATION exception if an address or a length is not
    naturally aligned.  */
 static inline void check_alignment(CPUS390XState *env, uint64_t v,
-                                   int wordsize, uintptr_t ra)
+                                   uint32_t wordsize, int ilen, uintptr_t ra)
 {
     if (v % wordsize) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 6, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ilen, ra);
     }
 }
 
+void HELPER(check_alignment)(CPUS390XState *env, uint64_t v, uint32_t wordsize)
+{
+    check_alignment(env, v, wordsize, ILEN_AUTO, GETPC());
+}
+
 /* Load a value from memory according to its size.  */
 static inline uint64_t cpu_ldusize_data_ra(CPUS390XState *env, uint64_t addr,
                                            int wordsize, uintptr_t ra)
@@ -847,7 +852,7 @@ static inline uint32_t do_clcl(CPUS390XState *env,
     uint64_t len = MAX(*src1len, *src3len);
     uint32_t cc = 0;
 
-    check_alignment(env, *src1len | *src3len, wordsize, ra);
+    check_alignment(env, *src1len | *src3len, wordsize, 6, ra);
 
     if (!len) {
         return cc;
@@ -1348,7 +1353,7 @@ uint32_t HELPER(trXX)(CPUS390XState *env, uint32_t r1, uint32_t r2,
         tbl &= -8;
     }
 
-    check_alignment(env, len, ssize, ra);
+    check_alignment(env, len, ssize, 6, ra);
 
     /* Lest we fail to service interrupts in a timely manner, */
     /* limit the amount of work we're willing to do.   */
@@ -1400,7 +1405,7 @@ static void do_cdsg(CPUS390XState *env, uint64_t addr,
     } else {
         uint64_t oldh, oldl;
 
-        check_alignment(env, addr, 16, ra);
+        check_alignment(env, addr, 16, 6, ra);
 
         oldh = cpu_ldq_data_ra(env, addr + 0, ra);
         oldl = cpu_ldq_data_ra(env, addr + 8, ra);
@@ -2116,7 +2121,7 @@ static uint64_t do_lpq(CPUS390XState *env, uint64_t addr, bool parallel)
         lo = int128_getlo(v);
 #endif
     } else {
-        check_alignment(env, addr, 16, ra);
+        check_alignment(env, addr, 16, 6, ra);
 
         hi = cpu_ldq_data_ra(env, addr + 0, ra);
         lo = cpu_ldq_data_ra(env, addr + 8, ra);
@@ -2153,7 +2158,7 @@ static void do_stpq(CPUS390XState *env, uint64_t addr,
         helper_atomic_sto_be_mmu(env, addr, v, oi, ra);
 #endif
     } else {
-        check_alignment(env, addr, 16, ra);
+        check_alignment(env, addr, 16, 6, ra);
 
         cpu_stq_data_ra(env, addr + 0, high, ra);
         cpu_stq_data_ra(env, addr + 8, low, ra);
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 7363aabf3a..4161bd0b1f 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2828,9 +2828,13 @@ static DisasJumpType op_lpp(DisasContext *s, DisasOps *o)
 
 static DisasJumpType op_lpsw(DisasContext *s, DisasOps *o)
 {
+    TCGv_i32 t0;
     TCGv_i64 t1, t2;
 
     check_privileged(s);
+    t0 = tcg_const_i32(8);
+    gen_helper_check_alignment(cpu_env, o->in2, t0);
+    tcg_temp_free_i32(t0);
     per_breaking_event(s);
 
     t1 = tcg_temp_new_i64();
@@ -2848,9 +2852,13 @@ static DisasJumpType op_lpsw(DisasContext *s, DisasOps *o)
 
 static DisasJumpType op_lpswe(DisasContext *s, DisasOps *o)
 {
+    TCGv_i32 t0;
     TCGv_i64 t1, t2;
 
     check_privileged(s);
+    t0 = tcg_const_i32(8);
+    gen_helper_check_alignment(cpu_env, o->in2, t0);
+    tcg_temp_free_i32(t0);
     per_breaking_event(s);
 
     t1 = tcg_temp_new_i64();
-- 
2.18.0

[Qemu-devel] [PATCH v4 3/3] target/s390x: implement CVB, CVBY and CVBG

[Pavel Zbitskiy]

Convert to Binary - counterparts of the already implemented Convert
to Decimal (CVD*) instructions.
Example from the Principles of Operation: 25594C becomes 63FA.

Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
---
 target/s390x/helper.h           |  1 +
 target/s390x/insn-data.def      |  4 +++
 target/s390x/int_helper.c       | 52 +++++++++++++++++++++++++++++++++
 target/s390x/translate.c        | 11 +++++++
 tests/tcg/s390x/Makefile.target |  1 +
 tests/tcg/s390x/cvb.c           | 18 ++++++++++++
 6 files changed, 87 insertions(+)
 create mode 100644 tests/tcg/s390x/cvb.c

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index b0df3267e5..2a9e9240df 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -88,6 +88,7 @@ DEF_HELPER_FLAGS_4(tcxb, TCG_CALL_NO_RWG_SE, i32, env, i64, i64, i64)
 DEF_HELPER_FLAGS_2(sqeb, TCG_CALL_NO_WG, i64, env, i64)
 DEF_HELPER_FLAGS_2(sqdb, TCG_CALL_NO_WG, i64, env, i64)
 DEF_HELPER_FLAGS_3(sqxb, TCG_CALL_NO_WG, i64, env, i64, i64)
+DEF_HELPER_FLAGS_3(cvb, TCG_CALL_NO_WG, i64, env, i64, i32)
 DEF_HELPER_FLAGS_1(cvd, TCG_CALL_NO_RWG_SE, i64, s32)
 DEF_HELPER_FLAGS_4(pack, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(pka, TCG_CALL_NO_WG, void, env, i64, i64, i32)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 9c7b434fca..0911180ca6 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -284,6 +284,10 @@
     D(0xec73, CLFIT,   RIE_a, GIE, r1_32u, i2_32u, 0, 0, ct, 0, 1)
     D(0xec71, CLGIT,   RIE_a, GIE, r1_o, i2_32u, 0, 0, ct, 0, 1)
 
+/* CONVERT TO BINARY */
+    C(0x4f00, CVB,     RX_a,  Z,   la2, 0, new, r1_32, cvb, 0)
+    C(0xe306, CVBY,    RXY_a, LD,  la2, 0, new, r1_32, cvb, 0)
+    C(0xe30e, CVBG,    RXY_a, Z,   la2, 0, r1, 0, cvb, 0)
 /* CONVERT TO DECIMAL */
     C(0x4e00, CVD,     RX_a,  Z,   r1_o, a2, 0, 0, cvd, 0)
     C(0xe326, CVDY,    RXY_a, LD,  r1_o, a2, 0, 0, cvd, 0)
diff --git a/target/s390x/int_helper.c b/target/s390x/int_helper.c
index abf77a94e6..3b12c11cee 100644
--- a/target/s390x/int_helper.c
+++ b/target/s390x/int_helper.c
@@ -24,6 +24,7 @@
 #include "exec/exec-all.h"
 #include "qemu/host-utils.h"
 #include "exec/helper-proto.h"
+#include "exec/cpu_ldst.h"
 
 /* #define DEBUG_HELPER */
 #ifdef DEBUG_HELPER
@@ -118,6 +119,57 @@ uint64_t HELPER(divu64)(CPUS390XState *env, uint64_t ah, uint64_t al,
     return ret;
 }
 
+static void general_operand_exception(CPUS390XState *env, uintptr_t ra)
+{
+#ifndef CONFIG_USER_ONLY
+    LowCore *lowcore;
+
+    lowcore = cpu_map_lowcore(env);
+    lowcore->data_exc_code = 0;
+    cpu_unmap_lowcore(lowcore);
+#endif
+    s390_program_interrupt(env, PGM_DATA, ILEN_AUTO, ra);
+}
+
+uint64_t HELPER(cvb)(CPUS390XState *env, uint64_t src, uint32_t n)
+{
+    int i, j;
+    uint64_t tmpsrc;
+    const uintptr_t ra = GETPC();
+    int64_t dec, sign = 0, digit, val = 0, pow10 = 0;
+
+    for (i = 0; i < n; i++) {
+        tmpsrc = wrap_address(env, src + (n - i - 1) * 8);
+        dec = cpu_ldq_data_ra(env, tmpsrc, ra);
+        for (j = 0; j < 16; j++, dec >>= 4) {
+            if (i == 0 && j == 0) {
+                sign = dec & 0xf;
+                if (sign < 0xa) {
+                    general_operand_exception(env, ra);
+                }
+                continue;
+            }
+            digit = dec & 0xf;
+            if (digit > 0x9) {
+                general_operand_exception(env, ra);
+            }
+            if (i == 0 && j == 1) {
+                if (sign == 0xb || sign == 0xd) {
+                    val = -digit;
+                    pow10 = -10;
+                } else {
+                    val = digit;
+                    pow10 = 10;
+                }
+            } else {
+                val += digit * pow10;
+                pow10 *= 10;
+            }
+        }
+    }
+    return val;
+}
+
 uint64_t HELPER(cvd)(int32_t reg)
 {
     /* positive 0 */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 4161bd0b1f..ff36dbe362 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2114,6 +2114,17 @@ static DisasJumpType op_csp(DisasContext *s, DisasOps *o)
 }
 #endif
 
+static DisasJumpType op_cvb(DisasContext *s, DisasOps *o)
+{
+    bool g = (s->fields->op == 0xE3) && (s->fields->op2 == 0x0E);
+    int32_t n = g ? 2 /* CVBG */ : 1 /* CVB, CVBY */;
+    TCGv_i32 tmp = tcg_const_i32(n);
+
+    gen_helper_cvb(o->out, cpu_env, o->addr1, tmp);
+    tcg_temp_free_i32(tmp);
+    return DISAS_NEXT;
+}
+
 static DisasJumpType op_cvd(DisasContext *s, DisasOps *o)
 {
     TCGv_i64 t1 = tcg_temp_new_i64();
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 151dc075aa..990dfb26ff 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -6,3 +6,4 @@ TESTS+=ipm
 TESTS+=exrl-trt
 TESTS+=exrl-trtr
 TESTS+=pack
+TESTS+=cvb
diff --git a/tests/tcg/s390x/cvb.c b/tests/tcg/s390x/cvb.c
new file mode 100644
index 0000000000..3a72e132aa
--- /dev/null
+++ b/tests/tcg/s390x/cvb.c
@@ -0,0 +1,18 @@
+#include <stdint.h>
+#include <unistd.h>
+
+int main(void)
+{
+    uint64_t data = 0x000000000025594cull;
+    uint64_t result = 0;
+
+    asm volatile(
+        "    cvb %[result],%[data]\n"
+        : [result] "+r" (result)
+        : [data] "m" (data));
+    if (result != 0x63fa) {
+        write(1, "bad result\n", 11);
+        return 1;
+    }
+    return 0;
+}
-- 
2.18.0

Re: [Qemu-devel] [PATCH v4 2/3] target/s390x: exception on non-aligned LPSW(E)

[David Hildenbrand]

On 22.08.2018 16:40, Pavel Zbitskiy wrote:
> Both LPSW and LPSWE should raise a specification exception when their
> operand is not doubleword aligned.
> 
> This could've been done without a helper, but this would introduce a
> new basic block, which would require making o->in2 local. This
> could've also been done in load_psw helper, but this is too late -
> specification exception should be recognized before memory accesses
> take place.
> 

In general, we can use MO_ALIGN to detect unaligned access (see e.g. op_sck)

tcg_gen_qemu_ld_i64(o->out, a1, get_mem_index(s), mop | MO_ALIGN);

If I remember correctly, the check should be performed before actually
loading.

Can you use that instead?


> Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
> ---
>  target/s390x/helper.h     |  1 +
>  target/s390x/mem_helper.c | 19 ++++++++++++-------
>  target/s390x/translate.c  |  8 ++++++++
>  3 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index 97c60ca7bc..b0df3267e5 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -120,6 +120,7 @@ DEF_HELPER_4(cu41, i32, env, i32, i32, i32)
>  DEF_HELPER_4(cu42, i32, env, i32, i32, i32)
>  DEF_HELPER_5(msa, i32, env, i32, i32, i32, i32)
>  DEF_HELPER_FLAGS_1(stpt, TCG_CALL_NO_RWG, i64, env)
> +DEF_HELPER_FLAGS_3(check_alignment, TCG_CALL_NO_RWG, void, env, i64, i32)
>  
>  #ifndef CONFIG_USER_ONLY
>  DEF_HELPER_3(servc, i32, env, i64, i64)
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index bacae4f503..75ca1997ad 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -81,13 +81,18 @@ static inline uint32_t adj_len_to_page(uint32_t len, uint64_t addr)
>  /* Trigger a SPECIFICATION exception if an address or a length is not
>     naturally aligned.  */
>  static inline void check_alignment(CPUS390XState *env, uint64_t v,
> -                                   int wordsize, uintptr_t ra)
> +                                   uint32_t wordsize, int ilen, uintptr_t ra)
>  {
>      if (v % wordsize) {
> -        s390_program_interrupt(env, PGM_SPECIFICATION, 6, ra);
> +        s390_program_interrupt(env, PGM_SPECIFICATION, ilen, ra);
>      }
>  }
>  
> +void HELPER(check_alignment)(CPUS390XState *env, uint64_t v, uint32_t wordsize)
> +{
> +    check_alignment(env, v, wordsize, ILEN_AUTO, GETPC());
> +}
> +
>  /* Load a value from memory according to its size.  */
>  static inline uint64_t cpu_ldusize_data_ra(CPUS390XState *env, uint64_t addr,
>                                             int wordsize, uintptr_t ra)
> @@ -847,7 +852,7 @@ static inline uint32_t do_clcl(CPUS390XState *env,
>      uint64_t len = MAX(*src1len, *src3len);
>      uint32_t cc = 0;
>  
> -    check_alignment(env, *src1len | *src3len, wordsize, ra);
> +    check_alignment(env, *src1len | *src3len, wordsize, 6, ra);
>  
>      if (!len) {
>          return cc;
> @@ -1348,7 +1353,7 @@ uint32_t HELPER(trXX)(CPUS390XState *env, uint32_t r1, uint32_t r2,
>          tbl &= -8;
>      }
>  
> -    check_alignment(env, len, ssize, ra);
> +    check_alignment(env, len, ssize, 6, ra);
>  
>      /* Lest we fail to service interrupts in a timely manner, */
>      /* limit the amount of work we're willing to do.   */
> @@ -1400,7 +1405,7 @@ static void do_cdsg(CPUS390XState *env, uint64_t addr,
>      } else {
>          uint64_t oldh, oldl;
>  
> -        check_alignment(env, addr, 16, ra);
> +        check_alignment(env, addr, 16, 6, ra);
>  
>          oldh = cpu_ldq_data_ra(env, addr + 0, ra);
>          oldl = cpu_ldq_data_ra(env, addr + 8, ra);
> @@ -2116,7 +2121,7 @@ static uint64_t do_lpq(CPUS390XState *env, uint64_t addr, bool parallel)
>          lo = int128_getlo(v);
>  #endif
>      } else {
> -        check_alignment(env, addr, 16, ra);
> +        check_alignment(env, addr, 16, 6, ra);
>  
>          hi = cpu_ldq_data_ra(env, addr + 0, ra);
>          lo = cpu_ldq_data_ra(env, addr + 8, ra);
> @@ -2153,7 +2158,7 @@ static void do_stpq(CPUS390XState *env, uint64_t addr,
>          helper_atomic_sto_be_mmu(env, addr, v, oi, ra);
>  #endif
>      } else {
> -        check_alignment(env, addr, 16, ra);
> +        check_alignment(env, addr, 16, 6, ra);
>  
>          cpu_stq_data_ra(env, addr + 0, high, ra);
>          cpu_stq_data_ra(env, addr + 8, low, ra);
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 7363aabf3a..4161bd0b1f 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -2828,9 +2828,13 @@ static DisasJumpType op_lpp(DisasContext *s, DisasOps *o)
>  
>  static DisasJumpType op_lpsw(DisasContext *s, DisasOps *o)
>  {
> +    TCGv_i32 t0;
>      TCGv_i64 t1, t2;
>  
>      check_privileged(s);
> +    t0 = tcg_const_i32(8);
> +    gen_helper_check_alignment(cpu_env, o->in2, t0);
> +    tcg_temp_free_i32(t0);
>      per_breaking_event(s);
>  
>      t1 = tcg_temp_new_i64();
> @@ -2848,9 +2852,13 @@ static DisasJumpType op_lpsw(DisasContext *s, DisasOps *o)
>  
>  static DisasJumpType op_lpswe(DisasContext *s, DisasOps *o)
>  {
> +    TCGv_i32 t0;
>      TCGv_i64 t1, t2;
>  
>      check_privileged(s);
> +    t0 = tcg_const_i32(8);
> +    gen_helper_check_alignment(cpu_env, o->in2, t0);
> +    tcg_temp_free_i32(t0);
>      per_breaking_event(s);
>  
>      t1 = tcg_temp_new_i64();
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v4 0/3] Some improvements in z/Arch instructions support

[David Hildenbrand]

On 22.08.2018 16:40, Pavel Zbitskiy wrote:
> Changes since v3:
> * Patches 1-6 were accepted, thanks!
> * Use in1_la2 in op_cvb().
> * Free a temp in op_cvb().
> * Make ra const in helper_cvb().
> * Use wrap_address() in helper_cvb().
> * Add a minor formatting patch.
> * Add LPSW(E) alignment check.
> 

Keep the patches coming, happy to see some activity :)

> Pavel Zbitskiy (3):
>   target/s390x: use regular spaces in translate.c
>   target/s390x: exception on non-aligned LPSW(E)
>   target/s390x: implement CVB, CVBY and CVBG
> 
>  target/s390x/helper.h           |  2 ++
>  target/s390x/insn-data.def      |  4 +++
>  target/s390x/int_helper.c       | 52 +++++++++++++++++++++++++++++++++
>  target/s390x/mem_helper.c       | 19 +++++++-----
>  target/s390x/translate.c        | 23 +++++++++++++--
>  tests/tcg/s390x/Makefile.target |  1 +
>  tests/tcg/s390x/cvb.c           | 18 ++++++++++++
>  7 files changed, 110 insertions(+), 9 deletions(-)
>  create mode 100644 tests/tcg/s390x/cvb.c
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v4 1/3] target/s390x: use regular spaces in translate.c

[David Hildenbrand]

On 22.08.2018 16:40, Pavel Zbitskiy wrote:
> In a few places translate.c contains non-breaking spaces (0xc2 0xa0)
> instead of regular ones (0x20):
> 
>   7c 7c c2 a0 63 63
>   7c 7c 20    63 63
>   |  |        c  c
> 
> This confuses some text editors.
> 
> Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
> ---
>  target/s390x/translate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 40e12ca2c4..7363aabf3a 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -842,7 +842,7 @@ static void disas_jcc(DisasContext *s, DisasCompare *c, uint32_t mask)
>              cond = TCG_COND_NE;
>              c->u.s32.b = tcg_const_i32(1);
>              break;
> -        case 0x8 | 0x2: /* cc == 0 || cc == 2 => (cc & 1) == 0 */
> +        case 0x8 | 0x2: /* cc == 0 || cc == 2 => (cc & 1) == 0 */
>              cond = TCG_COND_EQ;
>              c->g1 = false;
>              c->u.s32.a = tcg_temp_new_i32();
> @@ -861,7 +861,7 @@ static void disas_jcc(DisasContext *s, DisasCompare *c, uint32_t mask)
>              cond = TCG_COND_NE;
>              c->u.s32.b = tcg_const_i32(0);
>              break;
> -        case 0x4 | 0x1: /* cc == 1 || cc == 3 => (cc & 1) != 0 */
> +        case 0x4 | 0x1: /* cc == 1 || cc == 3 => (cc & 1) != 0 */
>              cond = TCG_COND_NE;
>              c->g1 = false;
>              c->u.s32.a = tcg_temp_new_i32();
> 

(can't spot a difference in my mail editor ;) )

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v4 2/3] target/s390x: exception on non-aligned LPSW(E)

[Richard Henderson]

On 08/22/2018 07:40 AM, Pavel Zbitskiy wrote:
> @@ -2828,9 +2828,13 @@ static DisasJumpType op_lpp(DisasContext *s, DisasOps *o)
>  
>  static DisasJumpType op_lpsw(DisasContext *s, DisasOps *o)
>  {
> +    TCGv_i32 t0;
>      TCGv_i64 t1, t2;
>  
>      check_privileged(s);
> +    t0 = tcg_const_i32(8);
> +    gen_helper_check_alignment(cpu_env, o->in2, t0);
> +    tcg_temp_free_i32(t0);
>      per_breaking_event(s);

This can be done without an external call.

-    tcg_gen_qemu_ld32u(t1, o->in2, get_mem_index(s));
+    tcg_gen_qemu_ld_i64(t1, o->in2, get_mem_index(s),
+                        MO_TEUL | MO_ALIGN_8);

(Which, for annoying reasons, will only have effect in
system mode, but that's true of many other alignment
checks as well.)


r~

Re: [Qemu-devel] [qemu-s390x] [PATCH v4 1/3] target/s390x: use regular spaces in translate.c

[Thomas Huth]

On 2018-08-22 16:40, Pavel Zbitskiy wrote:
> In a few places translate.c contains non-breaking spaces (0xc2 0xa0)
> instead of regular ones (0x20):
> 
>   7c 7c c2 a0 63 63
>   7c 7c 20    63 63
>   |  |        c  c
> 
> This confuses some text editors.

Without your patch:

$ file target/s390x/translate.c
target/s390x/translate.c: C source, UTF-8 Unicode text

With your patch:

$ file target/s390x/translate.c
target/s390x/translate.c: C source, ASCII text

Looks like you've got all spots :-)

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

Re: [Qemu-devel] [PATCH v4 1/3] target/s390x: use regular spaces in translate.c

[Cornelia Huck]

On Wed, 22 Aug 2018 10:40:37 -0400
Pavel Zbitskiy <pavel.zbitskiy@gmail.com> wrote:

> In a few places translate.c contains non-breaking spaces (0xc2 0xa0)
> instead of regular ones (0x20):
> 
>   7c 7c c2 a0 63 63
>   7c 7c 20    63 63
>   |  |        c  c
> 
> This confuses some text editors.

Fun :)

> 
> Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
> ---
>  target/s390x/translate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Thanks, applied.