[Qemu-devel] [PATCH 00/26] target/s390x: fix, improve and implement some more instructions

[Qemu-devel] [PATCH 00/26] target/s390x: fix, improve and implement some more instructions

[Aurelien Jarno]

This patchset tries to improve the s390x emulation by fixing and
improving some instructions. It implement some more instructions, from
the zArchitecture base or from the Extended-Translation Facility 2. The 
last patch updates the maximum TCG CPU to z800, as the ETF2 and Long-Displacement
Facility are now both fully implemented.

This patch series is based on the s390x unwind patches from Richard
Henderson. It trivally conflicts with the EXECUTE and cpu_models 
patches, but I got bored of rebasing my patches. Given the involved
patches, it's very likely that I will need to respin them, so I'll
fix the conflicts at that time, depending in which orders the other
series are going in.

Aurelien Jarno (26):
  target/s390x: remove dead code in translate.c
  target/s390x: make IPTE SMP aware
  target/s390x: implement TEST AND SET
  target/s390x: implement TEST ADDRESSING MODE
  target/s390x: implement PACK
  target/s390x: implement LOAD PAIR FROM QUADWORD
  target/s390x: implement STORE PAIR TO QUADWORD
  target/s390x: implement MOVE INVERSE
  target/s390x: implement MOVE NUMERICS
  target/s390x: implement MOVE WITH OFFSET
  target/s390x: implement MOVE ZONES
  target/s390x: improve 24-bit and 31-bit addresses read
  target/s390x: improve 24-bit and 31-bit addresses write
  target/s390x: improve 24-bit and 31-bit lengths read/write
  target/s390x: fix COMPARE LOGICAL LONG EXTENDED
  target/s390x: implement COMPARE LOGICAL LONG
  target/s390x: improve MOVE LONG and MOVE LONG EXTENDED
  target/s390x: implement COMPARE LOGICAL LONG UNICODE
  target/s390x: implement MOVE LONG UNICODE
  target/s390x: implement PACK ASCII
  target/s390x: implement PACK UNICODE
  target/s390x: implement UNPACK ASCII
  target/s390x: implement UNPACK UNICODE
  target/s390x: implement TEST DECIMAL
  target/s390x: implement TRANSLATE ONE/TWO TO ONE/TWO
  target/s390x: update maximum TCG model to z800

 target/s390x/cpu_models.c  |   4 +-
 target/s390x/helper.h      |  14 +
 target/s390x/insn-data.def |  51 ++++
 target/s390x/mem_helper.c  | 724 +++++++++++++++++++++++++++++++++++++--------
 target/s390x/misc_helper.c |   4 +-
 target/s390x/translate.c   | 223 +++++++++++++-
 6 files changed, 889 insertions(+), 131 deletions(-)

-- 
2.11.0

[Qemu-devel] [PATCH 01/26] target/s390x: remove dead code in translate.c

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/translate.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 4bd16d9f99..850df26ce1 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -5455,10 +5455,7 @@ void gen_intermediate_code(CPUS390XState *env, struct TranslationBlock *tb)
             gen_io_start();
         }
 
-        status = NO_EXIT;
-        if (status == NO_EXIT) {
-            status = translate_one(env, &dc);
-        }
+        status = translate_one(env, &dc);
 
         /* If we reach a page boundary, are single stepping,
            or exhaust instruction count, stop generation.  */
-- 
2.11.0

Re: [Qemu-devel] [PATCH 01/26] target/s390x: remove dead code in translate.c

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/translate.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)

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


r~

[Qemu-devel] [PATCH 02/26] target/s390x: make IPTE SMP aware

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/mem_helper.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 4b96c279e3..88e2a0f60d 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1109,8 +1109,6 @@ void HELPER(ipte)(CPUS390XState *env, uint64_t pte_addr, uint64_t vaddr)
     uint64_t page = vaddr & TARGET_PAGE_MASK;
     uint64_t pte = 0;
 
-    /* XXX broadcast to other CPUs */
-
     /* XXX Linux is nice enough to give us the exact pte address.
        According to spec we'd have to find it out ourselves */
     /* XXX Linux is fine with overwriting the pte, the spec requires
@@ -1119,13 +1117,13 @@ void HELPER(ipte)(CPUS390XState *env, uint64_t pte_addr, uint64_t vaddr)
 
     /* XXX we exploit the fact that Linux passes the exact virtual
        address here - it's not obliged to! */
-    tlb_flush_page(cs, page);
+    tlb_flush_page_all_cpus_synced(cs, page);
 
     /* XXX 31-bit hack */
     if (page & 0x80000000) {
-        tlb_flush_page(cs, page & ~0x80000000);
+        tlb_flush_page_all_cpus_synced(cs, page & ~0x80000000);
     } else {
-        tlb_flush_page(cs, page | 0x80000000);
+        tlb_flush_page_all_cpus_synced(cs, page | 0x80000000);
     }
 }
 
-- 
2.11.0

Re: [Qemu-devel] [PATCH 02/26] target/s390x: make IPTE SMP aware

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
>       /* XXX we exploit the fact that Linux passes the exact virtual
>          address here - it's not obliged to! */
> -    tlb_flush_page(cs, page);
> +    tlb_flush_page_all_cpus_synced(cs, page);
>   
>       /* XXX 31-bit hack */
>       if (page & 0x80000000) {
> -        tlb_flush_page(cs, page & ~0x80000000);
> +        tlb_flush_page_all_cpus_synced(cs, page & ~0x80000000);
>       } else {
> -        tlb_flush_page(cs, page | 0x80000000);
> +        tlb_flush_page_all_cpus_synced(cs, page | 0x80000000);
>       }

Ideally we would, at the same time, implement the local-pte facility, which 
examines a bit in the M4 field to *not* do this.  That said,

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


r~

[Qemu-devel] [PATCH 03/26] target/s390x: implement TEST AND SET

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/insn-data.def |  3 +++
 target/s390x/translate.c   | 10 ++++++++++
 2 files changed, 13 insertions(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index f818437069..0f70acea5c 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -810,6 +810,9 @@
 /* SUPERVISOR CALL */
     C(0x0a00, SVC,     I,     Z,   0, 0, 0, 0, svc, 0)
 
+/* TEST AND SET */
+    C(0x9300, TS,      S,     Z,   0, a2, 0, 0, ts, 0)
+
 /* TEST DATA CLASS */
     C(0xed10, TCEB,    RXE,   Z,   e1, a2, 0, 0, tceb, 0)
     C(0xed11, TCDB,    RXE,   Z,   f1_o, a2, 0, 0, tcdb, 0)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 850df26ce1..1e16f3d1f7 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4126,6 +4126,16 @@ static ExitStatus op_trt(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_ts(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 t1 = tcg_const_i32(0xff);
+    tcg_gen_atomic_fetch_or_i32(t1, o->in2, t1, get_mem_index(s), MO_UB);
+    tcg_gen_extract_i32(cc_op, t1, 7, 1);
+    tcg_temp_free_i32(t1);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_unpk(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
-- 
2.11.0

Re: [Qemu-devel] [PATCH 03/26] target/s390x: implement TEST AND SET

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> +    TCGv_i32 t1 = tcg_const_i32(0xff);
> +    tcg_gen_atomic_fetch_or_i32(t1, o->in2, t1, get_mem_index(s), MO_UB);

Better as tcg_gen_atomic_xchg_i32.


r~

[Qemu-devel] [PATCH 04/26] target/s390x: implement TEST ADDRESSING MODE

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/insn-data.def |  3 +++
 target/s390x/translate.c   | 11 +++++++++++
 2 files changed, 14 insertions(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 0f70acea5c..170b50ef2e 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -810,6 +810,9 @@
 /* SUPERVISOR CALL */
     C(0x0a00, SVC,     I,     Z,   0, 0, 0, 0, svc, 0)
 
+/* TEST ADDRESSING MODE */
+    C(0x010b, TAM,     E,     Z,   0, 0, 0, 0, tam, 0)
+
 /* TEST AND SET */
     C(0x9300, TS,      S,     Z,   0, a2, 0, 0, ts, 0)
 
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 1e16f3d1f7..d84138e38d 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4060,6 +4060,17 @@ static ExitStatus op_svc(DisasContext *s, DisasOps *o)
     return EXIT_NORETURN;
 }
 
+static ExitStatus op_tam(DisasContext *s, DisasOps *o)
+{
+    int cc = 0;
+
+    cc |= (s->tb->flags & FLAG_MASK_64) ? 2 : 0;
+    cc |= (s->tb->flags & FLAG_MASK_32) ? 1 : 0;
+    tcg_gen_movi_i32(cc_op, cc);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_tceb(DisasContext *s, DisasOps *o)
 {
     gen_helper_tceb(cc_op, cpu_env, o->in1, o->in2);
-- 
2.11.0

Re: [Qemu-devel] [PATCH 04/26] target/s390x: implement TEST ADDRESSING MODE

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> +    tcg_gen_movi_i32(cc_op, cc);
> +    set_cc_static(s);

gen_op_movi_cc


r~

[Qemu-devel] [PATCH 05/26] target/s390x: implement PACK

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  5 +++++
 target/s390x/mem_helper.c  | 37 +++++++++++++++++++++++++++++++++++++
 target/s390x/translate.c   |  8 ++++++++
 4 files changed, 51 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index cc451c70a6..fdd6e76410 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -75,6 +75,7 @@ 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_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(unpk, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(tr, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_4(tre, i64, env, i64, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 170b50ef2e..f92bfde4f8 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -639,6 +639,11 @@
     C(0x9600, OI,      SI,    Z,   m1_8u, i2_8u, new, m1_8, or, nz64)
     C(0xeb56, OIY,     SIY,   LD,  m1_8u, i2_8u, new, m1_8, or, nz64)
 
+/* PACK */
+    /* Really format SS_b, but we pack both lengths into one argument
+       for the helper call, so we might as well leave one 8-bit field.  */
+    C(0xf200, PACK,    SS_a,  Z,   la1, a2, 0, 0, pack, 0)
+
 /* PREFETCH */
     /* Implemented as nops of course.  */
     C(0xe336, PFD,     RXY_b, GIE, 0, 0, 0, 0, 0, 0)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 88e2a0f60d..f650b08779 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -676,6 +676,43 @@ uint64_t HELPER(cksm)(CPUS390XState *env, uint64_t r1,
     return len;
 }
 
+void HELPER(pack)(CPUS390XState *env, uint32_t len, uint64_t dest, uint64_t src)
+{
+    uintptr_t ra = GETPC();
+    int len_dest = len >> 4;
+    int len_src = len & 0xf;
+    uint8_t b;
+
+    dest += len_dest;
+    src += len_src;
+
+    /* last byte is special, it only flips the nibbles */
+    b = cpu_ldub_data_ra(env, src, ra);
+    cpu_stb_data_ra(env, dest, (b << 4) | (b >> 4), ra);
+    src--;
+    len_src--;
+
+    /* now pack every value */
+    while (len_dest >= 0) {
+        b = 0;
+
+        if (len_src > 0) {
+            b = cpu_ldub_data_ra(env, src, ra) & 0x0f;
+            src--;
+            len_src--;
+        }
+        if (len_src > 0) {
+            b |= cpu_ldub_data_ra(env, src, ra) << 4;
+            src--;
+            len_src--;
+        }
+
+        len_dest--;
+        dest--;
+        cpu_stb_data_ra(env, dest, b, ra);
+    }
+}
+
 void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
                   uint64_t src)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index d84138e38d..624e8667d7 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3136,6 +3136,14 @@ static ExitStatus op_ori(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_pack(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
+    gen_helper_pack(cpu_env, l, o->addr1, o->in2);
+    tcg_temp_free_i32(l);
+    return NO_EXIT;
+}
+
 static ExitStatus op_popcnt(DisasContext *s, DisasOps *o)
 {
     gen_helper_popcnt(o->out, o->in2);
-- 
2.11.0

Re: [Qemu-devel] [PATCH 05/26] target/s390x: implement PACK

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/helper.h      |  1 +
>   target/s390x/insn-data.def |  5 +++++
>   target/s390x/mem_helper.c  | 37 +++++++++++++++++++++++++++++++++++++
>   target/s390x/translate.c   |  8 ++++++++
>   4 files changed, 51 insertions(+)

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


r~

[Qemu-devel] [PATCH 06/26] target/s390x: implement LOAD PAIR FROM QUADWORD

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/insn-data.def |  2 ++
 target/s390x/translate.c   | 16 ++++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index f92bfde4f8..53c86d5832 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -507,6 +507,8 @@
 /* LOAD PAIR DISJOINT */
     D(0xc804, LPD,     SSF,   ILA, 0, 0, new_P, r3_P32, lpd, 0, MO_TEUL)
     D(0xc805, LPDG,    SSF,   ILA, 0, 0, new_P, r3_P64, lpd, 0, MO_TEQ)
+/* LOAD PAIR FROM QUADWORD */
+    C(0xe38f, LPQ,     RXY_a, Z,   0, a2, r1_P, 0, lpq, 0)
 /* LOAD POSITIVE */
     C(0x1000, LPR,     RR_a,  Z,   0, r2_32s, new, r1_32, abs, abs32)
     C(0xb900, LPGR,    RRE,   Z,   0, r2, r1, 0, abs, abs64)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 624e8667d7..09052972f1 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2827,6 +2827,22 @@ static ExitStatus op_lpd(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_lpq(DisasContext *s, DisasOps *o)
+{
+    /* In a parallel context, stop the world and single step.  */
+    if (parallel_cpus) {
+        potential_page_fault(s);
+        gen_exception(EXCP_ATOMIC);
+        return EXIT_NORETURN;
+    }
+
+    /* In a serial context, perform the two loads ... */
+    tcg_gen_qemu_ld64(o->out, o->in2, get_mem_index(s));
+    tcg_gen_addi_i64(o->in2, o->in2, 8);
+    tcg_gen_qemu_ld64(o->out2, o->in2, get_mem_index(s));
+    return NO_EXIT;
+}
+
 #ifndef CONFIG_USER_ONLY
 static ExitStatus op_lura(DisasContext *s, DisasOps *o)
 {
-- 
2.11.0

Re: [Qemu-devel] [PATCH 06/26] target/s390x: implement LOAD PAIR FROM QUADWORD

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> +static ExitStatus op_lpq(DisasContext *s, DisasOps *o)
> +{
> +    /* In a parallel context, stop the world and single step.  */
> +    if (parallel_cpus) {
> +        potential_page_fault(s);
> +        gen_exception(EXCP_ATOMIC);
> +        return EXIT_NORETURN;
> +    }
> +
> +    /* In a serial context, perform the two loads ... */
> +    tcg_gen_qemu_ld64(o->out, o->in2, get_mem_index(s));
> +    tcg_gen_addi_i64(o->in2, o->in2, 8);
> +    tcg_gen_qemu_ld64(o->out2, o->in2, get_mem_index(s));
> +    return NO_EXIT;
> +}

This can be implemented in a parallel context with helper_atomic_ldo_be_mmu, 
though that must be done with a helper function.


r~

[Qemu-devel] [PATCH 07/26] target/s390x: implement STORE PAIR TO QUADWORD

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/insn-data.def |  2 ++
 target/s390x/translate.c   | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 53c86d5832..751e3164dd 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -770,6 +770,8 @@
 /* STORE ACCESS MULTIPLE */
     C(0x9b00, STAM,    RS_a,  Z,   0, a2, 0, 0, stam, 0)
     C(0xeb9b, STAMY,   RSY_a, LD,  0, a2, 0, 0, stam, 0)
+/* STORE PAIR TO QUADWORD */
+    C(0xe38e, STPQ,    RXY_a, Z,   0, a2, 0, 0, stpq, 0)
 
 /* SUBTRACT */
     C(0x1b00, SR,      RR_a,  Z,   r1, r2, new, r1_32, sub, subs32)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 09052972f1..231b51c313 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4025,6 +4025,24 @@ static ExitStatus op_stmh(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_stpq(DisasContext *s, DisasOps *o)
+{
+    int r1 = get_field(s->fields, r1);
+
+    /* In a parallel context, stop the world and single step.  */
+    if (parallel_cpus) {
+        potential_page_fault(s);
+        gen_exception(EXCP_ATOMIC);
+        return EXIT_NORETURN;
+    }
+
+    /* In a serial context, perform the two stores ... */
+    tcg_gen_qemu_st64(regs[r1], o->in2, get_mem_index(s));
+    tcg_gen_addi_i64(o->in2, o->in2, 8);
+    tcg_gen_qemu_st64(regs[r1 + 1], o->in2, get_mem_index(s));
+    return NO_EXIT;
+}
+
 static ExitStatus op_srst(DisasContext *s, DisasOps *o)
 {
     gen_helper_srst(o->in1, cpu_env, regs[0], o->in1, o->in2);
-- 
2.11.0

Re: [Qemu-devel] [PATCH 07/26] target/s390x: implement STORE PAIR TO QUADWORD

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>   target/s390x/insn-data.def |  2 ++
>   target/s390x/translate.c   | 18 ++++++++++++++++++
>   2 files changed, 20 insertions(+)
> 
> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
> index 53c86d5832..751e3164dd 100644
> --- a/target/s390x/insn-data.def
> +++ b/target/s390x/insn-data.def
> @@ -770,6 +770,8 @@
>   /* STORE ACCESS MULTIPLE */
>       C(0x9b00, STAM,    RS_a,  Z,   0, a2, 0, 0, stam, 0)
>       C(0xeb9b, STAMY,   RSY_a, LD,  0, a2, 0, 0, stam, 0)
> +/* STORE PAIR TO QUADWORD */
> +    C(0xe38e, STPQ,    RXY_a, Z,   0, a2, 0, 0, stpq, 0)

Use prep_r1_P here, so that r1 gets validated as even....

> +    /* In a parallel context, stop the world and single step.  */
> +    if (parallel_cpus) {
> +        potential_page_fault(s);
> +        gen_exception(EXCP_ATOMIC);
> +        return EXIT_NORETURN;
> +    }
> +
> +    /* In a serial context, perform the two stores ... */
> +    tcg_gen_qemu_st64(regs[r1], o->in2, get_mem_index(s));
> +    tcg_gen_addi_i64(o->in2, o->in2, 8);
> +    tcg_gen_qemu_st64(regs[r1 + 1], o->in2, get_mem_index(s));

... then use o->out / o->out2 for the stores.

Similarly, helper_atomic_sto_be_mmu in a helper for the parallel context.


r~

[Qemu-devel] [PATCH 08/26] target/s390x: implement MOVE INVERSE

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/mem_helper.c  | 12 ++++++++++++
 target/s390x/translate.c   |  8 ++++++++
 4 files changed, 23 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index fdd6e76410..7d6f11abe8 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -3,6 +3,7 @@ DEF_HELPER_FLAGS_4(nc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(oc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(xc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(mvc, TCG_CALL_NO_WG, void, env, i32, i64, i64)
+DEF_HELPER_FLAGS_4(mvcin, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(clc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_3(mvcl, i32, env, i32, i32)
 DEF_HELPER_FLAGS_4(clm, TCG_CALL_NO_WG, i32, env, i32, i32, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 751e3164dd..52f943c71e 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -566,6 +566,8 @@
     C(0xe548, MVGHI,   SIL,   GIE, la1, i2, 0, m1_64, mov2, 0)
     C(0x9200, MVI,     SI,    Z,   la1, i2, 0, m1_8, mov2, 0)
     C(0xeb52, MVIY,    SIY,   LD,  la1, i2, 0, m1_8, mov2, 0)
+/* MOVE INVERSE */
+    C(0xe800, MVCIN,   SS_a,  Z,   la1, a2, 0, 0, mvcin, 0)
 /* MOVE LONG */
     C(0x0e00, MVCL,    RR_a,  Z,   0, 0, 0, 0, mvcl, 0)
 /* MOVE LONG EXTENDED */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index f650b08779..7faaf588a5 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -232,6 +232,18 @@ void HELPER(mvc)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
     do_helper_mvc(env, l, dest, src, GETPC());
 }
 
+/* move inverse  */
+void HELPER(mvcin)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
+{
+    uintptr_t ra = GETPC();
+    int i;
+
+    for (i = 0; i <= l; i++) {
+        uint8_t v = cpu_ldub_data_ra(env, src - i, ra);
+        cpu_stb_data_ra(env, dest + i, v, ra);
+    }
+}
+
 /* compare unsigned byte arrays */
 static uint32_t do_helper_clc(CPUS390XState *env, uint32_t l, uint64_t s1,
                               uint64_t s2, uintptr_t ra)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 231b51c313..b939c5a39f 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2925,6 +2925,14 @@ static ExitStatus op_mvc(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_mvcin(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
+    gen_helper_mvcin(cpu_env, l, o->addr1, o->in2);
+    tcg_temp_free_i32(l);
+    return NO_EXIT;
+}
+
 static ExitStatus op_mvcl(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
-- 
2.11.0

Re: [Qemu-devel] [PATCH 08/26] target/s390x: implement MOVE INVERSE

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/helper.h      |  1 +
>   target/s390x/insn-data.def |  2 ++
>   target/s390x/mem_helper.c  | 12 ++++++++++++
>   target/s390x/translate.c   |  8 ++++++++
>   4 files changed, 23 insertions(+)

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


r~

[Qemu-devel] [PATCH 09/26] target/s390x: implement MOVE NUMERICS

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/mem_helper.c  | 13 +++++++++++++
 target/s390x/translate.c   |  8 ++++++++
 4 files changed, 24 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 7d6f11abe8..0558d6dc21 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -13,6 +13,7 @@ DEF_HELPER_FLAGS_3(divs64, TCG_CALL_NO_WG, s64, env, s64, s64)
 DEF_HELPER_FLAGS_4(divu64, TCG_CALL_NO_WG, i64, env, i64, i64, i64)
 DEF_HELPER_4(srst, i64, env, i64, i64, i64)
 DEF_HELPER_4(clst, i64, env, i64, i64, i64)
+DEF_HELPER_FLAGS_4(mvn, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(mvpg, TCG_CALL_NO_WG, i32, env, i64, i64, i64)
 DEF_HELPER_4(mvst, i64, env, i64, i64, i64)
 DEF_HELPER_4(ex, void, env, i32, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 52f943c71e..518b87b160 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -572,6 +572,8 @@
     C(0x0e00, MVCL,    RR_a,  Z,   0, 0, 0, 0, mvcl, 0)
 /* MOVE LONG EXTENDED */
     C(0xa800, MVCLE,   RS_a,  Z,   0, a2, 0, 0, mvcle, 0)
+/* MOVE NUMERICS */
+    C(0xd100, MVN,     SS_a,  Z,   la1, a2, 0, 0, mvn, 0)
 /* MOVE PAGE */
     C(0xb254, MVPG,    RRE,   Z,   r1_o, r2_o, 0, 0, mvpg, 0)
 /* MOVE STRING */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 7faaf588a5..88be30555d 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -244,6 +244,19 @@ void HELPER(mvcin)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
     }
 }
 
+/* move numerics  */
+void HELPER(mvn)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
+{
+    uintptr_t ra = GETPC();
+    int i;
+
+    for (i = 0; i <= l; i++) {
+        uint8_t v = cpu_ldub_data_ra(env, dest + i, ra) & 0xf0;
+        v |= cpu_ldub_data_ra(env, src + i, ra) & 0x0f;
+        cpu_stb_data_ra(env, dest + i, v, ra);
+    }
+}
+
 /* compare unsigned byte arrays */
 static uint32_t do_helper_clc(CPUS390XState *env, uint32_t l, uint64_t s1,
                               uint64_t s2, uintptr_t ra)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index b939c5a39f..ec6b85c2d0 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2975,6 +2975,14 @@ static ExitStatus op_mvcs(DisasContext *s, DisasOps *o)
 }
 #endif
 
+static ExitStatus op_mvn(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
+    gen_helper_mvn(cpu_env, l, o->addr1, o->in2);
+    tcg_temp_free_i32(l);
+    return NO_EXIT;
+}
+
 static ExitStatus op_mvpg(DisasContext *s, DisasOps *o)
 {
     gen_helper_mvpg(cc_op, cpu_env, regs[0], o->in1, o->in2);
-- 
2.11.0

Re: [Qemu-devel] [PATCH 09/26] target/s390x: implement MOVE NUMERICS

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/helper.h      |  1 +
>   target/s390x/insn-data.def |  2 ++
>   target/s390x/mem_helper.c  | 13 +++++++++++++
>   target/s390x/translate.c   |  8 ++++++++
>   4 files changed, 24 insertions(+)

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

[Qemu-devel] [PATCH 10/26] target/s390x: implement MOVE WITH OFFSET

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  4 ++++
 target/s390x/mem_helper.c  | 31 +++++++++++++++++++++++++++++++
 target/s390x/translate.c   |  8 ++++++++
 4 files changed, 44 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 0558d6dc21..fe73d71b49 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -14,6 +14,7 @@ DEF_HELPER_FLAGS_4(divu64, TCG_CALL_NO_WG, i64, env, i64, i64, i64)
 DEF_HELPER_4(srst, i64, env, i64, i64, i64)
 DEF_HELPER_4(clst, i64, env, i64, i64, i64)
 DEF_HELPER_FLAGS_4(mvn, TCG_CALL_NO_WG, void, env, i32, i64, i64)
+DEF_HELPER_FLAGS_4(mvo, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(mvpg, TCG_CALL_NO_WG, i32, env, i64, i64, i64)
 DEF_HELPER_4(mvst, i64, env, i64, i64, i64)
 DEF_HELPER_4(ex, void, env, i32, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 518b87b160..be8b7af211 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -578,6 +578,10 @@
     C(0xb254, MVPG,    RRE,   Z,   r1_o, r2_o, 0, 0, mvpg, 0)
 /* MOVE STRING */
     C(0xb255, MVST,    RRE,   Z,   r1_o, r2_o, 0, 0, mvst, 0)
+/* MOVE WITH OFFSET */
+    /* Really format SS_b, but we pack both lengths into one argument
+       for the helper call, so we might as well leave one 8-bit field.  */
+    C(0xf100, MVO,     SS_a,  Z,   la1, a2, 0, 0, mvo, 0)
 
 /* MULTIPLY */
     C(0x1c00, MR,      RR_a,  Z,   r1p1_32s, r2_32s, new, r1_D32, mul, 0)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 88be30555d..4f523a8a8c 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -257,6 +257,37 @@ void HELPER(mvn)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
     }
 }
 
+/* move with offset  */
+void HELPER(mvo)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
+{
+    uintptr_t ra = GETPC();
+    int len_dest = l >> 4;
+    int len_src = l & 0xf;
+    uint8_t byte_dest, byte_src;
+    int i;
+
+    src += len_src;
+    dest += len_dest;
+
+    /* Handle rightmost byte */
+    byte_src = cpu_ldub_data_ra(env, src, ra);
+    byte_dest = cpu_ldub_data_ra(env, dest, ra);
+    byte_dest = (byte_dest & 0x0f) | (byte_src << 4);
+    cpu_stb_data_ra(env, dest, byte_dest, ra);
+
+    /* Process remaining bytes from right to left */
+    for (i = 1; i <= len_dest; i++) {
+        byte_dest = byte_src >> 4;
+        if (len_src - i >= 0) {
+            byte_src = cpu_ldub_data_ra(env, src - i, ra);
+        } else {
+            byte_src = 0;
+        }
+        byte_dest |= byte_src << 4;
+        cpu_stb_data_ra(env, dest - i, byte_dest, ra);
+    }
+}
+
 /* compare unsigned byte arrays */
 static uint32_t do_helper_clc(CPUS390XState *env, uint32_t l, uint64_t s1,
                               uint64_t s2, uintptr_t ra)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index ec6b85c2d0..6d2a1dfe1c 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2983,6 +2983,14 @@ static ExitStatus op_mvn(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_mvo(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
+    gen_helper_mvo(cpu_env, l, o->addr1, o->in2);
+    tcg_temp_free_i32(l);
+    return NO_EXIT;
+}
+
 static ExitStatus op_mvpg(DisasContext *s, DisasOps *o)
 {
     gen_helper_mvpg(cc_op, cpu_env, regs[0], o->in1, o->in2);
-- 
2.11.0

Re: [Qemu-devel] [PATCH 10/26] target/s390x: implement MOVE WITH OFFSET

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/helper.h      |  1 +
>   target/s390x/insn-data.def |  4 ++++
>   target/s390x/mem_helper.c  | 31 +++++++++++++++++++++++++++++++
>   target/s390x/translate.c   |  8 ++++++++
>   4 files changed, 44 insertions(+)

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


r~

[Qemu-devel] [PATCH 11/26] target/s390x: implement MOVE ZONES

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/mem_helper.c  | 13 +++++++++++++
 target/s390x/translate.c   |  8 ++++++++
 4 files changed, 24 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index fe73d71b49..e1ab8d5686 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -16,6 +16,7 @@ DEF_HELPER_4(clst, i64, env, i64, i64, i64)
 DEF_HELPER_FLAGS_4(mvn, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(mvo, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(mvpg, TCG_CALL_NO_WG, i32, env, i64, i64, i64)
+DEF_HELPER_FLAGS_4(mvz, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_4(mvst, i64, env, i64, i64, i64)
 DEF_HELPER_4(ex, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(stam, TCG_CALL_NO_WG, void, env, i32, i64, i32)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index be8b7af211..c69e3f4216 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -582,6 +582,8 @@
     /* Really format SS_b, but we pack both lengths into one argument
        for the helper call, so we might as well leave one 8-bit field.  */
     C(0xf100, MVO,     SS_a,  Z,   la1, a2, 0, 0, mvo, 0)
+/* MOVE ZONES */
+    C(0xd300, MVZ,     SS_a,  Z,   la1, a2, 0, 0, mvz, 0)
 
 /* MULTIPLY */
     C(0x1c00, MR,      RR_a,  Z,   r1p1_32s, r2_32s, new, r1_D32, mul, 0)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 4f523a8a8c..bc9fd50b24 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -288,6 +288,19 @@ void HELPER(mvo)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
     }
 }
 
+/* move zones  */
+void HELPER(mvz)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
+{
+    uintptr_t ra = GETPC();
+    int i;
+
+    for (i = 0; i <= l; i++) {
+        uint8_t b = cpu_ldub_data_ra(env, dest + i, ra) & 0x0f;
+        b |= cpu_ldub_data_ra(env, src + i, ra) & 0xf0;
+        cpu_stb_data_ra(env, dest + i, b, ra);
+    }
+}
+
 /* compare unsigned byte arrays */
 static uint32_t do_helper_clc(CPUS390XState *env, uint32_t l, uint64_t s1,
                               uint64_t s2, uintptr_t ra)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 6d2a1dfe1c..87408df07a 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3006,6 +3006,14 @@ static ExitStatus op_mvst(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_mvz(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
+    gen_helper_mvz(cpu_env, l, o->addr1, o->in2);
+    tcg_temp_free_i32(l);
+    return NO_EXIT;
+}
+
 static ExitStatus op_mul(DisasContext *s, DisasOps *o)
 {
     tcg_gen_mul_i64(o->out, o->in1, o->in2);
-- 
2.11.0

Re: [Qemu-devel] [PATCH 11/26] target/s390x: implement MOVE ZONES

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/helper.h      |  1 +
>   target/s390x/insn-data.def |  2 ++
>   target/s390x/mem_helper.c  | 13 +++++++++++++
>   target/s390x/translate.c   |  8 ++++++++
>   4 files changed, 24 insertions(+)

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


r~

[Qemu-devel] [PATCH 12/26] target/s390x: improve 24-bit and 31-bit addresses read

[Aurelien Jarno]

Improve fix_address to also handle the 24-bit mode. Rename fix_address
to wrap_address to better explain what is changed.

For the same reason, rename get_address into get_address_rel and
get_address_31fix into get_address_rel.

Finally replace many calls to get_address_rel with x2 = 0 and b2 = 0 by
call to wrap_address.

Note that get_address_relget_address_rel is only used in the EXECUTE
helper, so we can get rid of it as the same time as the helper.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/mem_helper.c | 90 +++++++++++++++++++++++++----------------------
 1 file changed, 47 insertions(+), 43 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index bc9fd50b24..2b0cde13b4 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -366,16 +366,22 @@ uint32_t HELPER(clm)(CPUS390XState *env, uint32_t r1, uint32_t mask,
     return cc;
 }
 
-static inline uint64_t fix_address(CPUS390XState *env, uint64_t a)
+static inline uint64_t wrap_address(CPUS390XState *env, uint64_t a)
 {
-    /* 31-Bit mode */
     if (!(env->psw.mask & PSW_MASK_64)) {
-        a &= 0x7fffffff;
+        if (!(env->psw.mask & PSW_MASK_32)) {
+            /* 24-Bit mode */
+            a &= 0x00ffffff;
+        } else {
+            /* 31-Bit mode */
+            a &= 0x7fffffff;
+        }
     }
     return a;
 }
 
-static inline uint64_t get_address(CPUS390XState *env, int x2, int b2, int d2)
+static inline uint64_t get_address_rel(CPUS390XState *env,
+                                       int x2, int b2, int d2)
 {
     uint64_t r = d2;
     if (x2) {
@@ -384,12 +390,12 @@ static inline uint64_t get_address(CPUS390XState *env, int x2, int b2, int d2)
     if (b2) {
         r += env->regs[b2];
     }
-    return fix_address(env, r);
+    return wrap_address(env, r);
 }
 
-static inline uint64_t get_address_31fix(CPUS390XState *env, int reg)
+static inline uint64_t get_address(CPUS390XState *env, int reg)
 {
-    return fix_address(env, env->regs[reg]);
+    return wrap_address(env, env->regs[reg]);
 }
 
 /* search string (c is byte to search, r2 is string, r1 end of string) */
@@ -400,8 +406,8 @@ uint64_t HELPER(srst)(CPUS390XState *env, uint64_t r0, uint64_t end,
     uint32_t len;
     uint8_t v, c = r0;
 
-    str = fix_address(env, str);
-    end = fix_address(env, end);
+    str = wrap_address(env, str);
+    end = wrap_address(env, end);
 
     /* Assume for now that R2 is unmodified.  */
     env->retxl = str;
@@ -435,8 +441,8 @@ uint64_t HELPER(clst)(CPUS390XState *env, uint64_t c, uint64_t s1, uint64_t s2)
     uint32_t len;
 
     c = c & 0xff;
-    s1 = fix_address(env, s1);
-    s2 = fix_address(env, s2);
+    s1 = wrap_address(env, s1);
+    s2 = wrap_address(env, s2);
 
     /* Lest we fail to service interrupts in a timely manner, limit the
        amount of work we're willing to do.  For now, let's cap at 8k.  */
@@ -482,8 +488,8 @@ uint64_t HELPER(mvst)(CPUS390XState *env, uint64_t c, uint64_t d, uint64_t s)
     uint32_t len;
 
     c = c & 0xff;
-    d = fix_address(env, d);
-    s = fix_address(env, s);
+    d = wrap_address(env, d);
+    s = wrap_address(env, s);
 
     /* Lest we fail to service interrupts in a timely manner, limit the
        amount of work we're willing to do.  For now, let's cap at 8k.  */
@@ -572,9 +578,9 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
 {
     uintptr_t ra = GETPC();
     uint64_t destlen = env->regs[r1 + 1] & 0xffffff;
-    uint64_t dest = get_address_31fix(env, r1);
+    uint64_t dest = get_address(env, r1);
     uint64_t srclen = env->regs[r2 + 1] & 0xffffff;
-    uint64_t src = get_address_31fix(env, r2);
+    uint64_t src = get_address(env, r2);
     uint8_t pad = env->regs[r2 + 1] >> 24;
     uint8_t v;
     uint32_t cc;
@@ -615,9 +621,9 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
 {
     uintptr_t ra = GETPC();
     uint64_t destlen = env->regs[r1 + 1];
-    uint64_t dest = env->regs[r1];
+    uint64_t dest = get_address(env, r1);
     uint64_t srclen = env->regs[r3 + 1];
-    uint64_t src = env->regs[r3];
+    uint64_t src = get_address(env, r3);
     uint8_t pad = a2 & 0xff;
     uint8_t v;
     uint32_t cc;
@@ -625,8 +631,6 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     if (!(env->psw.mask & PSW_MASK_64)) {
         destlen = (uint32_t)destlen;
         srclen = (uint32_t)srclen;
-        dest &= 0x7fffffff;
-        src &= 0x7fffffff;
     }
 
     if (destlen == srclen) {
@@ -666,9 +670,9 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
 {
     uintptr_t ra = GETPC();
     uint64_t destlen = env->regs[r1 + 1];
-    uint64_t dest = get_address_31fix(env, r1);
+    uint64_t dest = get_address(env, r1);
     uint64_t srclen = env->regs[r3 + 1];
-    uint64_t src = get_address_31fix(env, r3);
+    uint64_t src = get_address(env, r3);
     uint8_t pad = a2 & 0xff;
     uint32_t cc = 0;
 
@@ -1050,7 +1054,7 @@ uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t real_addr)
     uint64_t abs_addr;
     int i;
 
-    real_addr = fix_address(env, real_addr);
+    real_addr = wrap_address(env, real_addr);
     abs_addr = mmu_real2abs(env, real_addr) & TARGET_PAGE_MASK;
     if (!address_space_access_valid(&address_space_memory, abs_addr,
                                     TARGET_PAGE_SIZE, true)) {
@@ -1084,7 +1088,7 @@ uint64_t HELPER(iske)(CPUS390XState *env, uint64_t r2)
 {
     static S390SKeysState *ss;
     static S390SKeysClass *skeyclass;
-    uint64_t addr = get_address(env, 0, 0, r2);
+    uint64_t addr = wrap_address(env, r2);
     uint8_t key;
 
     if (addr > ram_size) {
@@ -1107,7 +1111,7 @@ void HELPER(sske)(CPUS390XState *env, uint64_t r1, uint64_t r2)
 {
     static S390SKeysState *ss;
     static S390SKeysClass *skeyclass;
-    uint64_t addr = get_address(env, 0, 0, r2);
+    uint64_t addr = wrap_address(env, r2);
     uint8_t key;
 
     if (addr > ram_size) {
@@ -1254,14 +1258,14 @@ uint64_t HELPER(lura)(CPUS390XState *env, uint64_t addr)
 {
     CPUState *cs = CPU(s390_env_get_cpu(env));
 
-    return (uint32_t)ldl_phys(cs->as, get_address(env, 0, 0, addr));
+    return (uint32_t)ldl_phys(cs->as, wrap_address(env, addr));
 }
 
 uint64_t HELPER(lurag)(CPUS390XState *env, uint64_t addr)
 {
     CPUState *cs = CPU(s390_env_get_cpu(env));
 
-    return ldq_phys(cs->as, get_address(env, 0, 0, addr));
+    return ldq_phys(cs->as, wrap_address(env, addr));
 }
 
 /* store using real address */
@@ -1269,7 +1273,7 @@ void HELPER(stura)(CPUS390XState *env, uint64_t addr, uint64_t v1)
 {
     CPUState *cs = CPU(s390_env_get_cpu(env));
 
-    stl_phys(cs->as, get_address(env, 0, 0, addr), (uint32_t)v1);
+    stl_phys(cs->as, wrap_address(env, addr), (uint32_t)v1);
 
     if ((env->psw.mask & PSW_MASK_PER) &&
         (env->cregs[9] & PER_CR9_EVENT_STORE) &&
@@ -1284,7 +1288,7 @@ void HELPER(sturg)(CPUS390XState *env, uint64_t addr, uint64_t v1)
 {
     CPUState *cs = CPU(s390_env_get_cpu(env));
 
-    stq_phys(cs->as, get_address(env, 0, 0, addr), v1);
+    stq_phys(cs->as, wrap_address(env, addr), v1);
 
     if ((env->psw.mask & PSW_MASK_PER) &&
         (env->cregs[9] & PER_CR9_EVENT_STORE) &&
@@ -1369,32 +1373,32 @@ void HELPER(ex)(CPUS390XState *env, uint32_t ilen, uint64_t r1, uint64_t addr)
         d2 = extract64(insn, 16, 12);
         switch (opc & 0xf) {
         case 0x2:
-            do_helper_mvc(env, l, get_address(env, 0, b1, d1),
-                          get_address(env, 0, b2, d2), 0);
+            do_helper_mvc(env, l, get_address_rel(env, 0, b1, d1),
+                          get_address_rel(env, 0, b2, d2), 0);
             return;
         case 0x4:
-            env->cc_op = do_helper_nc(env, l, get_address(env, 0, b1, d1),
-                                      get_address(env, 0, b2, d2), 0);
+            env->cc_op = do_helper_nc(env, l, get_address_rel(env, 0, b1, d1),
+                                      get_address_rel(env, 0, b2, d2), 0);
             return;
         case 0x5:
-            env->cc_op = do_helper_clc(env, l, get_address(env, 0, b1, d1),
-                                       get_address(env, 0, b2, d2), 0);
+            env->cc_op = do_helper_clc(env, l, get_address_rel(env, 0, b1, d1),
+                                       get_address_rel(env, 0, b2, d2), 0);
             return;
         case 0x6:
-            env->cc_op = do_helper_oc(env, l, get_address(env, 0, b1, d1),
-                                      get_address(env, 0, b2, d2), 0);
+            env->cc_op = do_helper_oc(env, l, get_address_rel(env, 0, b1, d1),
+                                      get_address_rel(env, 0, b2, d2), 0);
             return;
         case 0x7:
-            env->cc_op = do_helper_xc(env, l, get_address(env, 0, b1, d1),
-                                      get_address(env, 0, b2, d2), 0);
+            env->cc_op = do_helper_xc(env, l, get_address_rel(env, 0, b1, d1),
+                                      get_address_rel(env, 0, b2, d2), 0);
             return;
         case 0xc:
-            do_helper_tr(env, l, get_address(env, 0, b1, d1),
-                         get_address(env, 0, b2, d2), 0);
+            do_helper_tr(env, l, get_address_rel(env, 0, b1, d1),
+                         get_address_rel(env, 0, b2, d2), 0);
             return;
         case 0xd:
-            env->cc_op = do_helper_trt(env, l, get_address(env, 0, b1, d1),
-                                       get_address(env, 0, b2, d2), 0);
+            env->cc_op = do_helper_trt(env, l, get_address_rel(env, 0, b1, d1),
+                                       get_address_rel(env, 0, b2, d2), 0);
             return;
         }
     } else if (opc == 0x0a) {
@@ -1410,7 +1414,7 @@ void HELPER(ex)(CPUS390XState *env, uint32_t ilen, uint64_t r1, uint64_t addr)
         r3 = extract64(insn, 48, 4);
         b2 = extract64(insn, 44, 4);
         d2 = extract64(insn, 32, 12);
-        env->cc_op = helper_icm(env, r1, get_address(env, 0, b2, d2), r3);
+        env->cc_op = helper_icm(env, r1, get_address_rel(env, 0, b2, d2), r3);
         return;
     }
 
-- 
2.11.0

Re: [Qemu-devel] [PATCH 12/26] target/s390x: improve 24-bit and 31-bit addresses read

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> Improve fix_address to also handle the 24-bit mode. Rename fix_address
> to wrap_address to better explain what is changed.
> 
> For the same reason, rename get_address into get_address_rel and
> get_address_31fix into get_address_rel.
> 
> Finally replace many calls to get_address_rel with x2 = 0 and b2 = 0 by
> call to wrap_address.
> 
> Note that get_address_relget_address_rel is only used in the EXECUTE
> helper, so we can get rid of it as the same time as the helper.
> 
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/mem_helper.c | 90 +++++++++++++++++++++++++----------------------
>   1 file changed, 47 insertions(+), 43 deletions(-)

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


r~

[Qemu-devel] [PATCH 13/26] target/s390x: improve 24-bit and 31-bit addresses write

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/mem_helper.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 2b0cde13b4..bbb3eceb71 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -398,6 +398,29 @@ static inline uint64_t get_address(CPUS390XState *env, int reg)
     return wrap_address(env, env->regs[reg]);
 }
 
+static inline void set_address(CPUS390XState *env, int reg, uint64_t address)
+{
+    if (env->psw.mask & PSW_MASK_64) {
+        /* 64-Bit mode */
+        env->regs[reg] = address;
+    } else {
+        if (!(env->psw.mask & PSW_MASK_32)) {
+            /* 24-Bit mode. According to the PoO it is implementation
+            dependent if bits 32-39 remain unchanged or are set to
+            zeros.  Choose the former so that the function can also be
+            used for TRT.  */
+            env->regs[reg] = deposit64(env->regs[reg], 0, 24, address);
+        } else {
+            /* 31-Bit mode. According to the PoO it is implementation
+            dependent if bit 32 remains unchanged or is set to zero.
+            Choose the latter so that the function can also be used for
+            TRT.  */
+            address &= 0x7fffffff;
+            env->regs[reg] = deposit64(env->regs[reg], 0, 32, address);
+        }
+    }
+}
+
 /* search string (c is byte to search, r2 is string, r1 end of string) */
 uint64_t HELPER(srst)(CPUS390XState *env, uint64_t r0, uint64_t end,
                       uint64_t str)
@@ -609,8 +632,8 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
     env->regs[r1 + 1] = destlen;
     /* can't use srclen here, we trunc'ed it */
     env->regs[r2 + 1] -= src - env->regs[r2];
-    env->regs[r1] = dest;
-    env->regs[r2] = src;
+    set_address(env, r1, dest);
+    set_address(env, r2, src);
 
     return cc;
 }
@@ -658,8 +681,8 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     /* can't use srclen here, we trunc'ed it */
     /* FIXME: 31-bit mode! */
     env->regs[r3 + 1] -= src - env->regs[r3];
-    env->regs[r1] = dest;
-    env->regs[r3] = src;
+    set_address(env, r1, dest);
+    set_address(env, r3, src);
 
     return cc;
 }
@@ -696,8 +719,8 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     env->regs[r1 + 1] = destlen;
     /* can't use srclen here, we trunc'ed it */
     env->regs[r3 + 1] -= src - env->regs[r3];
-    env->regs[r1] = dest;
-    env->regs[r3] = src;
+    set_address(env, r1, dest);
+    set_address(env, r3, src);
 
     return cc;
 }
@@ -901,7 +924,7 @@ static uint32_t do_helper_trt(CPUS390XState *env, uint32_t len, uint64_t array,
         uint8_t sbyte = cpu_ldub_data_ra(env, trans + byte, ra);
 
         if (sbyte != 0) {
-            env->regs[1] = array + i;
+            set_address(env, 1, array + i);
             env->regs[2] = deposit64(env->regs[2], 0, 8, sbyte);
             return (i == len) ? 2 : 1;
         }
-- 
2.11.0

Re: [Qemu-devel] [PATCH 13/26] target/s390x: improve 24-bit and 31-bit addresses write

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/mem_helper.c | 37 ++++++++++++++++++++++++++++++-------
>   1 file changed, 30 insertions(+), 7 deletions(-)

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


r~

[Qemu-devel] [PATCH 14/26] target/s390x: improve 24-bit and 31-bit lengths read/write

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/mem_helper.c | 47 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 14 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index bbb3eceb71..1dc71fe5f0 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -421,6 +421,31 @@ static inline void set_address(CPUS390XState *env, int reg, uint64_t address)
     }
 }
 
+static inline uint64_t wrap_length(CPUS390XState *env, uint64_t length)
+{
+    if (!(env->psw.mask & PSW_MASK_64)) {
+        /* 24-Bit and 31-Bit mode */
+        length &= 0x7fffffff;
+    }
+    return length;
+}
+
+static inline uint64_t get_length(CPUS390XState *env, int reg)
+{
+    return wrap_length(env, env->regs[reg]);
+}
+
+static inline void set_length(CPUS390XState *env, int reg, uint64_t length)
+{
+    if (env->psw.mask & PSW_MASK_64) {
+        /* 64-Bit mode */
+        env->regs[reg] = length;
+    } else {
+        /* 24-Bit and 31-Bit mode */
+        env->regs[reg] = deposit64(env->regs[reg], 0, 32, length);
+    }
+}
+
 /* search string (c is byte to search, r2 is string, r1 end of string) */
 uint64_t HELPER(srst)(CPUS390XState *env, uint64_t r0, uint64_t end,
                       uint64_t str)
@@ -643,19 +668,14 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
                        uint32_t r3)
 {
     uintptr_t ra = GETPC();
-    uint64_t destlen = env->regs[r1 + 1];
+    uint64_t destlen = get_length(env, r1 + 1);
     uint64_t dest = get_address(env, r1);
-    uint64_t srclen = env->regs[r3 + 1];
+    uint64_t srclen = get_length(env, r3 + 1);
     uint64_t src = get_address(env, r3);
     uint8_t pad = a2 & 0xff;
     uint8_t v;
     uint32_t cc;
 
-    if (!(env->psw.mask & PSW_MASK_64)) {
-        destlen = (uint32_t)destlen;
-        srclen = (uint32_t)srclen;
-    }
-
     if (destlen == srclen) {
         cc = 0;
     } else if (destlen < srclen) {
@@ -677,10 +697,9 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
         cpu_stb_data_ra(env, dest, pad, ra);
     }
 
-    env->regs[r1 + 1] = destlen;
+    set_length(env, r1 + 1 , destlen);
     /* can't use srclen here, we trunc'ed it */
-    /* FIXME: 31-bit mode! */
-    env->regs[r3 + 1] -= src - env->regs[r3];
+    set_length(env, r3 + 1, env->regs[r3 + 1] - src - env->regs[r3]);
     set_address(env, r1, dest);
     set_address(env, r3, src);
 
@@ -692,9 +711,9 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
                        uint32_t r3)
 {
     uintptr_t ra = GETPC();
-    uint64_t destlen = env->regs[r1 + 1];
+    uint64_t destlen = get_length(env, r1 + 1);
     uint64_t dest = get_address(env, r1);
-    uint64_t srclen = env->regs[r3 + 1];
+    uint64_t srclen = get_length(env, r3 + 1);
     uint64_t src = get_address(env, r3);
     uint8_t pad = a2 & 0xff;
     uint32_t cc = 0;
@@ -716,9 +735,9 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
         }
     }
 
-    env->regs[r1 + 1] = destlen;
+    set_length(env, r1 + 1, destlen);
     /* can't use srclen here, we trunc'ed it */
-    env->regs[r3 + 1] -= src - env->regs[r3];
+    set_length(env, r3 + 1, env->regs[r3 + 1] - src - env->regs[r3]);
     set_address(env, r1, dest);
     set_address(env, r3, src);
 
-- 
2.11.0

Re: [Qemu-devel] [PATCH 14/26] target/s390x: improve 24-bit and 31-bit lengths read/write

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/mem_helper.c | 47 +++++++++++++++++++++++++++++++++--------------
>   1 file changed, 33 insertions(+), 14 deletions(-)

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


r~

[Qemu-devel] [PATCH 15/26] target/s390x: fix COMPARE LOGICAL LONG EXTENDED

[Aurelien Jarno]

There are multiple issues with the COMPARE LOGICAL LONG EXTENDED
instruction:
- The test between the two operands is inverted, leading to an inversion
  of the cc values 1 and 2.
- The address and length of an operand continue to be decreased after
  reaching the end of this operand. These values are then wrong write
  back to the registers.
- We should limit the amount of bytes to process, so that interrupts can
  be served correctly.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/mem_helper.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 1dc71fe5f0..bd3bce3623 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -716,28 +716,48 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     uint64_t srclen = get_length(env, r3 + 1);
     uint64_t src = get_address(env, r3);
     uint8_t pad = a2 & 0xff;
+    uint64_t len = MAX(srclen, destlen);
     uint32_t cc = 0;
 
     if (!(destlen || srclen)) {
         return cc;
     }
 
-    if (srclen > destlen) {
-        srclen = destlen;
+    /* Lest we fail to service interrupts in a timely manner, limit the
+       amount of work we're willing to do.  For now, let's cap at 8k.  */
+    if (len > 0x2000) {
+        len = 0x2000;
+        cc = 3;
     }
 
-    for (; destlen || srclen; src++, dest++, destlen--, srclen--) {
-        uint8_t v1 = srclen ? cpu_ldub_data_ra(env, src, ra) : pad;
-        uint8_t v2 = destlen ? cpu_ldub_data_ra(env, dest, ra) : pad;
+    for (; len; len--) {
+        uint8_t v1 = pad;
+        uint8_t v2 = pad;
+
+        if (srclen) {
+            v1 = cpu_ldub_data_ra(env, src, ra);
+        }
+        if (destlen) {
+            v2 = cpu_ldub_data_ra(env, dest, ra);
+        }
+
         if (v1 != v2) {
-            cc = (v1 < v2) ? 1 : 2;
+            cc = (v1 > v2) ? 1 : 2;
             break;
         }
+
+        if (srclen) {
+            src++;
+            srclen--;
+        }
+        if (destlen) {
+            dest++;
+            destlen--;
+        }
     }
 
     set_length(env, r1 + 1, destlen);
-    /* can't use srclen here, we trunc'ed it */
-    set_length(env, r3 + 1, env->regs[r3 + 1] - src - env->regs[r3]);
+    set_length(env, r3 + 1, srclen);
     set_address(env, r1, dest);
     set_address(env, r3, src);
 
-- 
2.11.0

Re: [Qemu-devel] [PATCH 15/26] target/s390x: fix COMPARE LOGICAL LONG EXTENDED

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> There are multiple issues with the COMPARE LOGICAL LONG EXTENDED
> instruction:
> - The test between the two operands is inverted, leading to an inversion
>    of the cc values 1 and 2.
> - The address and length of an operand continue to be decreased after
>    reaching the end of this operand. These values are then wrong write
>    back to the registers.
> - We should limit the amount of bytes to process, so that interrupts can
>    be served correctly.
> 
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>   target/s390x/mem_helper.c | 36 ++++++++++++++++++++++++++++--------
>   1 file changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 1dc71fe5f0..bd3bce3623 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -716,28 +716,48 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
>       uint64_t srclen = get_length(env, r3 + 1);
>       uint64_t src = get_address(env, r3);
>       uint8_t pad = a2 & 0xff;
> +    uint64_t len = MAX(srclen, destlen);
>       uint32_t cc = 0;
>   
>       if (!(destlen || srclen)) {

Might as well replace this with len == 0 now.

>           if (v1 != v2) {
> -            cc = (v1 < v2) ? 1 : 2;
> +            cc = (v1 > v2) ? 1 : 2;

This would have been less confusing, perhaps, if we'd not used "src" and "dest" 
-- and doubly so since they are two sources -- but rather "src1" and "src3", 
which corresponds better to the PoO text.

That said,

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


r~

[Qemu-devel] [PATCH 16/26] target/s390x: implement COMPARE LOGICAL LONG

[Aurelien Jarno]

As CLCL and CLCLE mostly differ by their operands, use a common do_clcl
helper. Another difference is that CLCL is not interruptible.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/mem_helper.c  | 86 ++++++++++++++++++++++++++++++++--------------
 target/s390x/translate.c   | 11 ++++++
 4 files changed, 75 insertions(+), 25 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index e1ab8d5686..32938d99de 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -6,6 +6,7 @@ DEF_HELPER_FLAGS_4(mvc, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(mvcin, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(clc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_3(mvcl, i32, env, i32, i32)
+DEF_HELPER_3(clcl, i32, env, i32, i32)
 DEF_HELPER_FLAGS_4(clm, TCG_CALL_NO_WG, i32, env, i32, i32, i64)
 DEF_HELPER_FLAGS_3(divs32, TCG_CALL_NO_WG, s64, env, s64, s64)
 DEF_HELPER_FLAGS_3(divu32, TCG_CALL_NO_WG, i64, env, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index c69e3f4216..e241c1e486 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -210,6 +210,8 @@
     C(0xc60e, CLGFRL,  RIL_b, GIE, r1_o, mri2_32u, 0, 0, 0, cmpu64)
     C(0xc607, CLHRL,   RIL_b, GIE, r1_o, mri2_16u, 0, 0, 0, cmpu32)
     C(0xc606, CLGHRL,  RIL_b, GIE, r1_o, mri2_16u, 0, 0, 0, cmpu64)
+/* COMPARE LOGICAL LONG */
+    C(0x0f00, CLCL,    RR_a,  Z,   0, 0, 0, 0, clcl, 0)
 /* COMPARE LOGICAL LONG EXTENDED */
     C(0xa900, CLCLE,   RS_a,  Z,   0, a2, 0, 0, clcle, 0)
 /* COMPARE LOGICAL CHARACTERS UNDER MASK */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index bd3bce3623..aa665fe0f7 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -706,27 +706,24 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     return cc;
 }
 
-/* compare logical long extended memcompare insn with padding */
-uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
-                       uint32_t r3)
-{
-    uintptr_t ra = GETPC();
-    uint64_t destlen = get_length(env, r1 + 1);
-    uint64_t dest = get_address(env, r1);
-    uint64_t srclen = get_length(env, r3 + 1);
-    uint64_t src = get_address(env, r3);
-    uint8_t pad = a2 & 0xff;
-    uint64_t len = MAX(srclen, destlen);
+/* compare logical long helper */
+static inline uint32_t do_clcl(CPUS390XState *env,
+                               uint64_t *dest, uint64_t *destlen,
+                               uint64_t *src, uint64_t *srclen,
+                               uint8_t pad, uint64_t limit,
+                               uintptr_t ra)
+{
+    uint64_t len = MAX(*srclen, *destlen);
     uint32_t cc = 0;
 
-    if (!(destlen || srclen)) {
+    if (!(*destlen || *srclen)) {
         return cc;
     }
 
     /* Lest we fail to service interrupts in a timely manner, limit the
-       amount of work we're willing to do.  For now, let's cap at 8k.  */
-    if (len > 0x2000) {
-        len = 0x2000;
+       amount of work we're willing to do.  */
+    if (len > limit) {
+        len = limit;
         cc = 3;
     }
 
@@ -734,11 +731,11 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
         uint8_t v1 = pad;
         uint8_t v2 = pad;
 
-        if (srclen) {
-            v1 = cpu_ldub_data_ra(env, src, ra);
+        if (*srclen) {
+            v1 = cpu_ldub_data_ra(env, *src, ra);
         }
-        if (destlen) {
-            v2 = cpu_ldub_data_ra(env, dest, ra);
+        if (*destlen) {
+            v2 = cpu_ldub_data_ra(env, *dest, ra);
         }
 
         if (v1 != v2) {
@@ -746,16 +743,55 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
             break;
         }
 
-        if (srclen) {
-            src++;
-            srclen--;
+        if (*srclen) {
+            *src += 1;
+            *srclen -= 1;
         }
-        if (destlen) {
-            dest++;
-            destlen--;
+        if (*destlen) {
+            *dest += 1;
+            *destlen -= 1;
         }
     }
 
+    return cc;
+}
+
+
+/* compare logical long */
+uint32_t HELPER(clcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
+{
+    uintptr_t ra = GETPC();
+    uint64_t destlen = extract64(env->regs[r1 + 1], 0, 24);
+    uint64_t dest = get_address(env, r1);
+    uint64_t srclen = extract64(env->regs[r2 + 1], 0, 24);
+    uint64_t src = get_address(env, r2);
+    uint8_t pad = env->regs[r2 + 1] >> 24;
+    uint32_t cc;
+
+    cc = do_clcl(env, &dest, &destlen, &src, &srclen, pad, -1, ra);
+
+    env->regs[r1 + 1] = deposit64(env->regs[r1 + 1], 0, 24, destlen);
+    env->regs[r2 + 1] = deposit64(env->regs[r2 + 1], 0, 24, srclen);
+    set_address(env, r1, dest);
+    set_address(env, r2, src);
+
+    return cc;
+}
+
+/* compare logical long extended memcompare insn with padding */
+uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
+                       uint32_t r3)
+{
+    uintptr_t ra = GETPC();
+    uint64_t destlen = get_length(env, r1 + 1);
+    uint64_t dest = get_address(env, r1);
+    uint64_t srclen = get_length(env, r3 + 1);
+    uint64_t src = get_address(env, r3);
+    uint8_t pad = a2;
+    uint32_t cc;
+
+    cc = do_clcl(env, &dest, &destlen, &src, &srclen, pad, 0x2000, ra);
+
     set_length(env, r1 + 1, destlen);
     set_length(env, r3 + 1, srclen);
     set_address(env, r1, dest);
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 87408df07a..6f8d75bc2e 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1911,6 +1911,17 @@ static ExitStatus op_clc(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_clcl(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
+    TCGv_i32 r2 = tcg_const_i32(get_field(s->fields, r2));
+    gen_helper_clcl(cc_op, cpu_env, r1, r2);
+    tcg_temp_free_i32(r1);
+    tcg_temp_free_i32(r2);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_clcle(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
-- 
2.11.0

Re: [Qemu-devel] [PATCH 16/26] target/s390x: implement COMPARE LOGICAL LONG

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> -        if (srclen) {
> -            v1 = cpu_ldub_data_ra(env, src, ra);
> +        if (*srclen) {
> +            v1 = cpu_ldub_data_ra(env, *src, ra);
>           }
> -        if (destlen) {
> -            v2 = cpu_ldub_data_ra(env, dest, ra);
> +        if (*destlen) {
> +            v2 = cpu_ldub_data_ra(env, *dest, ra);
>           }
>   
>           if (v1 != v2) {
> @@ -746,16 +743,55 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
>               break;
>           }
>   
> -        if (srclen) {
> -            src++;
> -            srclen--;
> +        if (*srclen) {
> +            *src += 1;
> +            *srclen -= 1;
>           }
> -        if (destlen) {
> -            dest++;
> -            destlen--;
> +        if (*destlen) {
> +            *dest += 1;
> +            *destlen -= 1;
>           }
>       }

If you don't access these as pointers in the inner loop like this, the compiler 
will give you better code without needing to force the function to be inlined.

Naming convention comment still applies.

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


r~

Re: [Qemu-devel] [PATCH 16/26] target/s390x: implement COMPARE LOGICAL LONG

[Aurelien Jarno]

On 2017-05-26 08:32, Richard Henderson wrote:
> On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> > -        if (srclen) {
> > -            v1 = cpu_ldub_data_ra(env, src, ra);
> > +        if (*srclen) {
> > +            v1 = cpu_ldub_data_ra(env, *src, ra);
> >           }
> > -        if (destlen) {
> > -            v2 = cpu_ldub_data_ra(env, dest, ra);
> > +        if (*destlen) {
> > +            v2 = cpu_ldub_data_ra(env, *dest, ra);
> >           }
> >           if (v1 != v2) {
> > @@ -746,16 +743,55 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
> >               break;
> >           }
> > -        if (srclen) {
> > -            src++;
> > -            srclen--;
> > +        if (*srclen) {
> > +            *src += 1;
> > +            *srclen -= 1;
> >           }
> > -        if (destlen) {
> > -            dest++;
> > -            destlen--;
> > +        if (*destlen) {
> > +            *dest += 1;
> > +            *destlen -= 1;
> >           }
> >       }
> 
> If you don't access these as pointers in the inner loop like this, the
> compiler will give you better code without needing to force the function to
> be inlined.

I agree that it would allow to drop the inline from the pointers point of
view. That said we still want to the compiler to optimize out the call to
cpu_ldusize_data_ra, check_alignment (both introduced in the CLCLU
patch), or length limit.

My goal of using a common code for clcl/clcle/clclu was mostly to unify
code to having different bugs depending on the function. That could also
have been done using the preprocessor.

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

Re: [Qemu-devel] [PATCH 16/26] target/s390x: implement COMPARE LOGICAL LONG

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> +static ExitStatus op_clcl(DisasContext *s, DisasOps *o)
> +{
> +    TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
> +    TCGv_i32 r2 = tcg_const_i32(get_field(s->fields, r2));

Need to force r1 and r2 even.  I guess we didn't do that for clcle either...


r~

[Qemu-devel] [PATCH 17/26] target/s390x: improve MOVE LONG and MOVE LONG EXTENDED

[Aurelien Jarno]

As MVCL and MVCLE only differ by their operands, use a common
do_mvcl helper. Optimize it calling fast_memmove and fast_memset.
Correctly write back addresses.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/mem_helper.c | 90 +++++++++++++++++++++--------------------------
 1 file changed, 40 insertions(+), 50 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index aa665fe0f7..6add413531 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -621,49 +621,60 @@ void HELPER(stam)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
     }
 }
 
-/* move long */
-uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
+/* move long helper */
+static inline uint32_t do_mvcl(CPUS390XState *env,
+                               uint64_t *dest, uint64_t *destlen,
+                               uint64_t *src, uint64_t *srclen,
+                               uint8_t pad, uintptr_t ra)
 {
-    uintptr_t ra = GETPC();
-    uint64_t destlen = env->regs[r1 + 1] & 0xffffff;
-    uint64_t dest = get_address(env, r1);
-    uint64_t srclen = env->regs[r2 + 1] & 0xffffff;
-    uint64_t src = get_address(env, r2);
-    uint8_t pad = env->regs[r2 + 1] >> 24;
-    uint8_t v;
+    uint64_t len = MIN(*srclen, *destlen);
     uint32_t cc;
 
-    if (destlen == srclen) {
+    if (*destlen == *srclen) {
         cc = 0;
-    } else if (destlen < srclen) {
+    } else if (*destlen < *srclen) {
         cc = 1;
     } else {
         cc = 2;
     }
 
-    if (srclen > destlen) {
-        srclen = destlen;
-    }
+    /* Copy the src array */
+    fast_memmove(env, *dest, *src, len, ra);
+    *src += len;
+    *srclen -= len;
+    *dest += len;
+    *destlen -= len;
 
-    for (; destlen && srclen; src++, dest++, destlen--, srclen--) {
-        v = cpu_ldub_data_ra(env, src, ra);
-        cpu_stb_data_ra(env, dest, v, ra);
-    }
+    /* Pad the remaining area */
+    fast_memset(env, *dest, pad, *destlen, ra);
+    *dest += *destlen;
+    *destlen = 0;
 
-    for (; destlen; dest++, destlen--) {
-        cpu_stb_data_ra(env, dest, pad, ra);
-    }
+    return cc;
+}
 
-    env->regs[r1 + 1] = destlen;
-    /* can't use srclen here, we trunc'ed it */
-    env->regs[r2 + 1] -= src - env->regs[r2];
+/* move long */
+uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
+{
+    uintptr_t ra = GETPC();
+    uint64_t destlen = env->regs[r1 + 1] & 0xffffff;
+    uint64_t dest = get_address(env, r1);
+    uint64_t srclen = env->regs[r2 + 1] & 0xffffff;
+    uint64_t src = get_address(env, r2);
+    uint8_t pad = env->regs[r2 + 1] >> 24;
+    uint32_t cc;
+
+    cc = do_mvcl(env, &dest, &destlen, &src, &srclen, pad, ra);
+
+    env->regs[r1 + 1] = deposit64(env->regs[r1 + 1], 0, 24, destlen);
+    env->regs[r2 + 1] = deposit64(env->regs[r2 + 1], 0, 24, srclen);
     set_address(env, r1, dest);
     set_address(env, r2, src);
 
     return cc;
 }
 
-/* move long extended another memcopy insn with more bells and whistles */
+/* move long extended */
 uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
                        uint32_t r3)
 {
@@ -672,34 +683,13 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     uint64_t dest = get_address(env, r1);
     uint64_t srclen = get_length(env, r3 + 1);
     uint64_t src = get_address(env, r3);
-    uint8_t pad = a2 & 0xff;
-    uint8_t v;
+    uint8_t pad = a2;
     uint32_t cc;
 
-    if (destlen == srclen) {
-        cc = 0;
-    } else if (destlen < srclen) {
-        cc = 1;
-    } else {
-        cc = 2;
-    }
-
-    if (srclen > destlen) {
-        srclen = destlen;
-    }
-
-    for (; destlen && srclen; src++, dest++, destlen--, srclen--) {
-        v = cpu_ldub_data_ra(env, src, ra);
-        cpu_stb_data_ra(env, dest, v, ra);
-    }
-
-    for (; destlen; dest++, destlen--) {
-        cpu_stb_data_ra(env, dest, pad, ra);
-    }
+    cc = do_mvcl(env, &dest, &destlen, &src, &srclen, pad, ra);
 
-    set_length(env, r1 + 1 , destlen);
-    /* can't use srclen here, we trunc'ed it */
-    set_length(env, r3 + 1, env->regs[r3 + 1] - src - env->regs[r3]);
+    set_length(env, r1 + 1, destlen);
+    set_length(env, r3 + 1, srclen);
     set_address(env, r1, dest);
     set_address(env, r3, src);
 
-- 
2.11.0

Re: [Qemu-devel] [PATCH 17/26] target/s390x: improve MOVE LONG and MOVE LONG EXTENDED

[Richard Henderson]

On 05/25/2017 02:04 PM, Aurelien Jarno wrote:
> As MVCL and MVCLE only differ by their operands, use a common
> do_mvcl helper. Optimize it calling fast_memmove and fast_memset.
> Correctly write back addresses.
> 
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/mem_helper.c | 90 +++++++++++++++++++++--------------------------
>   1 file changed, 40 insertions(+), 50 deletions(-)

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


r~

[Qemu-devel] [PATCH 18/26] target/s390x: implement COMPARE LOGICAL LONG UNICODE

[Aurelien Jarno]

For that we need to make program_interrupt available to qemu-user.
Fortunately there is almost nothing to change as both kvm_enabled and
CONFIG_KVM evaluate to false in that case.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/mem_helper.c  | 76 ++++++++++++++++++++++++++++++++++++++--------
 target/s390x/misc_helper.c |  4 +--
 target/s390x/translate.c   | 12 ++++++++
 5 files changed, 80 insertions(+), 15 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 32938d99de..1b82d1a368 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -24,6 +24,7 @@ DEF_HELPER_FLAGS_4(stam, TCG_CALL_NO_WG, void, env, i32, i64, i32)
 DEF_HELPER_FLAGS_4(lam, TCG_CALL_NO_WG, void, env, i32, i64, i32)
 DEF_HELPER_4(mvcle, i32, env, i32, i64, i32)
 DEF_HELPER_4(clcle, i32, env, i32, i64, i32)
+DEF_HELPER_4(clclu, i32, env, i32, i64, i32)
 DEF_HELPER_3(cegb, i64, env, s64, i32)
 DEF_HELPER_3(cdgb, i64, env, s64, i32)
 DEF_HELPER_3(cxgb, i64, env, s64, i32)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index e241c1e486..f9e69f9b09 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -214,6 +214,8 @@
     C(0x0f00, CLCL,    RR_a,  Z,   0, 0, 0, 0, clcl, 0)
 /* COMPARE LOGICAL LONG EXTENDED */
     C(0xa900, CLCLE,   RS_a,  Z,   0, a2, 0, 0, clcle, 0)
+/* COMPARE LOGICAL LONG UNICODE */
+    C(0xeb8f, CLCLU,   RSY_a, E2,  0, a2, 0, 0, clclu, 0)
 /* COMPARE LOGICAL CHARACTERS UNDER MASK */
     C(0xbd00, CLM,     RS_b,  Z,   r1_o, a2, 0, 0, clm, 0)
     C(0xeb21, CLMY,    RSY_b, LD,  r1_o, a2, 0, 0, clm, 0)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 6add413531..7ce7daaf11 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -67,6 +67,32 @@ static inline uint32_t adj_len_to_page(uint32_t len, uint64_t addr)
     return len;
 }
 
+/* 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)
+{
+    if (v % wordsize) {
+        CPUState *cs = CPU(s390_env_get_cpu(env));
+        cpu_restore_state(cs, ra);
+        program_interrupt(env, PGM_SPECIFICATION, 6);
+    }
+}
+
+/* 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)
+{
+    switch (wordsize) {
+    case 1:
+        return cpu_ldub_data_ra(env, addr, ra);
+    case 2:
+        return cpu_lduw_data_ra(env, addr, ra);
+    default:
+        abort();
+    }
+}
+
 static void fast_memset(CPUS390XState *env, uint64_t dest, uint8_t byte,
                         uint32_t l, uintptr_t ra)
 {
@@ -700,12 +726,14 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
 static inline uint32_t do_clcl(CPUS390XState *env,
                                uint64_t *dest, uint64_t *destlen,
                                uint64_t *src, uint64_t *srclen,
-                               uint8_t pad, uint64_t limit,
-                               uintptr_t ra)
+                               uint16_t pad, uint64_t limit,
+                               int wordsize, uintptr_t ra)
 {
     uint64_t len = MAX(*srclen, *destlen);
     uint32_t cc = 0;
 
+    check_alignment(env, *srclen | *destlen, wordsize, ra);
+
     if (!(*destlen || *srclen)) {
         return cc;
     }
@@ -717,15 +745,15 @@ static inline uint32_t do_clcl(CPUS390XState *env,
         cc = 3;
     }
 
-    for (; len; len--) {
-        uint8_t v1 = pad;
-        uint8_t v2 = pad;
+    for (; len; len -= wordsize) {
+        uint16_t v1 = pad;
+        uint16_t v2 = pad;
 
         if (*srclen) {
-            v1 = cpu_ldub_data_ra(env, *src, ra);
+            v1 = cpu_ldusize_data_ra(env, *src, wordsize, ra);
         }
         if (*destlen) {
-            v2 = cpu_ldub_data_ra(env, *dest, ra);
+            v2 = cpu_ldusize_data_ra(env, *dest, wordsize, ra);
         }
 
         if (v1 != v2) {
@@ -734,12 +762,12 @@ static inline uint32_t do_clcl(CPUS390XState *env,
         }
 
         if (*srclen) {
-            *src += 1;
-            *srclen -= 1;
+            *src += wordsize;
+            *srclen -= wordsize;
         }
         if (*destlen) {
-            *dest += 1;
-            *destlen -= 1;
+            *dest += wordsize;
+            *destlen -= wordsize;
         }
     }
 
@@ -758,7 +786,7 @@ uint32_t HELPER(clcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
     uint8_t pad = env->regs[r2 + 1] >> 24;
     uint32_t cc;
 
-    cc = do_clcl(env, &dest, &destlen, &src, &srclen, pad, -1, ra);
+    cc = do_clcl(env, &dest, &destlen, &src, &srclen, pad, -1, 1, ra);
 
     env->regs[r1 + 1] = deposit64(env->regs[r1 + 1], 0, 24, destlen);
     env->regs[r2 + 1] = deposit64(env->regs[r2 + 1], 0, 24, srclen);
@@ -780,7 +808,29 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     uint8_t pad = a2;
     uint32_t cc;
 
-    cc = do_clcl(env, &dest, &destlen, &src, &srclen, pad, 0x2000, ra);
+    cc = do_clcl(env, &dest, &destlen, &src, &srclen, pad, 0x2000, 1, ra);
+
+    set_length(env, r1 + 1, destlen);
+    set_length(env, r3 + 1, srclen);
+    set_address(env, r1, dest);
+    set_address(env, r3, src);
+
+    return cc;
+}
+
+/* compare logical long unicode memcompare insn with padding */
+uint32_t HELPER(clclu)(CPUS390XState *env, uint32_t r1, uint64_t a2,
+                       uint32_t r3)
+{
+    uintptr_t ra = GETPC();
+    uint64_t destlen = get_length(env, r1 + 1);
+    uint64_t dest = get_address(env, r1);
+    uint64_t srclen = get_length(env, r3 + 1);
+    uint64_t src = get_address(env, r3);
+    uint16_t pad = a2;
+    uint32_t cc = 0;
+
+    cc = do_clcl(env, &dest, &destlen, &src, &srclen, pad, 0x1000, 2, ra);
 
     set_length(env, r1 + 1, destlen);
     set_length(env, r3 + 1, srclen);
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 23ec52cf35..f083c8d3cf 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -80,8 +80,6 @@ void HELPER(exception)(CPUS390XState *env, uint32_t excp)
     cpu_loop_exit(cs);
 }
 
-#ifndef CONFIG_USER_ONLY
-
 void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
@@ -108,6 +106,8 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
     }
 }
 
+#ifndef CONFIG_USER_ONLY
+
 /* SCLP service call */
 uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 6f8d75bc2e..2ea96c0a8f 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1196,6 +1196,7 @@ typedef enum DisasFacility {
     FAC_ILA,                /* interlocked access facility 1 */
     FAC_LPP,                /* load-program-parameter */
     FAC_DAT_ENH,            /* DAT-enhancement */
+    FAC_E2,                 /* extended-translation facility 2 */
 } DisasFacility;
 
 struct DisasInsn {
@@ -1933,6 +1934,17 @@ static ExitStatus op_clcle(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_clclu(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
+    TCGv_i32 r3 = tcg_const_i32(get_field(s->fields, r3));
+    gen_helper_clclu(cc_op, cpu_env, r1, o->in2, r3);
+    tcg_temp_free_i32(r1);
+    tcg_temp_free_i32(r3);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_clm(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 m3 = tcg_const_i32(get_field(s->fields, m3));
-- 
2.11.0

Re: [Qemu-devel] [PATCH 18/26] target/s390x: implement COMPARE LOGICAL LONG UNICODE

[Richard Henderson]

On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
> For that we need to make program_interrupt available to qemu-user.
> Fortunately there is almost nothing to change as both kvm_enabled and
> CONFIG_KVM evaluate to false in that case.
> 
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/helper.h      |  1 +
>   target/s390x/insn-data.def |  2 ++
>   target/s390x/mem_helper.c  | 76 ++++++++++++++++++++++++++++++++++++++--------
>   target/s390x/misc_helper.c |  4 +--
>   target/s390x/translate.c   | 12 ++++++++
>   5 files changed, 80 insertions(+), 15 deletions(-)

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


r~

[Qemu-devel] [PATCH 19/26] target/s390x: implement MOVE LONG UNICODE

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/mem_helper.c  | 47 ++++++++++++++++++++++++++++++++++++++++------
 target/s390x/translate.c   | 11 +++++++++++
 4 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 1b82d1a368..40b7544c4d 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -23,6 +23,7 @@ DEF_HELPER_4(ex, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(stam, TCG_CALL_NO_WG, void, env, i32, i64, i32)
 DEF_HELPER_FLAGS_4(lam, TCG_CALL_NO_WG, void, env, i32, i64, i32)
 DEF_HELPER_4(mvcle, i32, env, i32, i64, i32)
+DEF_HELPER_4(mvclu, i32, env, i32, i64, i32)
 DEF_HELPER_4(clcle, i32, env, i32, i64, i32)
 DEF_HELPER_4(clclu, i32, env, i32, i64, i32)
 DEF_HELPER_3(cegb, i64, env, s64, i32)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index f9e69f9b09..ceaa245530 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -576,6 +576,8 @@
     C(0x0e00, MVCL,    RR_a,  Z,   0, 0, 0, 0, mvcl, 0)
 /* MOVE LONG EXTENDED */
     C(0xa800, MVCLE,   RS_a,  Z,   0, a2, 0, 0, mvcle, 0)
+/* MOVE LONG UNICODE */
+    C(0xeb8e, MVCLU,   RSY_a, E2,  0, a2, 0, 0, mvclu, 0)
 /* MOVE NUMERICS */
     C(0xd100, MVN,     SS_a,  Z,   la1, a2, 0, 0, mvn, 0)
 /* MOVE PAGE */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 7ce7daaf11..134dde03c2 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -651,7 +651,7 @@ void HELPER(stam)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
 static inline uint32_t do_mvcl(CPUS390XState *env,
                                uint64_t *dest, uint64_t *destlen,
                                uint64_t *src, uint64_t *srclen,
-                               uint8_t pad, uintptr_t ra)
+                               uint16_t pad, int wordsize, uintptr_t ra)
 {
     uint64_t len = MIN(*srclen, *destlen);
     uint32_t cc;
@@ -672,9 +672,22 @@ static inline uint32_t do_mvcl(CPUS390XState *env,
     *destlen -= len;
 
     /* Pad the remaining area */
-    fast_memset(env, *dest, pad, *destlen, ra);
-    *dest += *destlen;
-    *destlen = 0;
+    if (wordsize == 1) {
+        fast_memset(env, *dest, pad, *destlen, ra);
+        *dest += *destlen;
+        *destlen = 0;
+    } else {
+        /* If remaining length is odd, pad with odd byte first.  */
+        if (*destlen & 1) {
+            cpu_stb_data_ra(env, *dest, pad & 0xff, ra);
+	    *dest += 1;
+	    *destlen -= 1;
+        }
+        /* The remaining length is even, pad using words.  */
+        for (; *destlen; *dest += 2, *destlen -= 2) {
+            cpu_stw_data_ra(env, *dest, pad, ra);
+        }
+    }
 
     return cc;
 }
@@ -690,7 +703,7 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
     uint8_t pad = env->regs[r2 + 1] >> 24;
     uint32_t cc;
 
-    cc = do_mvcl(env, &dest, &destlen, &src, &srclen, pad, ra);
+    cc = do_mvcl(env, &dest, &destlen, &src, &srclen, pad, 1, ra);
 
     env->regs[r1 + 1] = deposit64(env->regs[r1 + 1], 0, 24, destlen);
     env->regs[r2 + 1] = deposit64(env->regs[r2 + 1], 0, 24, srclen);
@@ -712,7 +725,29 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     uint8_t pad = a2;
     uint32_t cc;
 
-    cc = do_mvcl(env, &dest, &destlen, &src, &srclen, pad, ra);
+    cc = do_mvcl(env, &dest, &destlen, &src, &srclen, pad, 1, ra);
+
+    set_length(env, r1 + 1, destlen);
+    set_length(env, r3 + 1, srclen);
+    set_address(env, r1, dest);
+    set_address(env, r3, src);
+
+    return cc;
+}
+
+/* move long unicode */
+uint32_t HELPER(mvclu)(CPUS390XState *env, uint32_t r1, uint64_t a2,
+                       uint32_t r3)
+{
+    uintptr_t ra = GETPC();
+    uint64_t destlen = get_length(env, r1 + 1);
+    uint64_t dest = get_address(env, r1);
+    uint64_t srclen = get_length(env, r3 + 1);
+    uint64_t src = get_address(env, r3);
+    uint16_t pad = a2;
+    uint32_t cc;
+
+    cc = do_mvcl(env, &dest, &destlen, &src, &srclen, pad, 2, ra);
 
     set_length(env, r1 + 1, destlen);
     set_length(env, r3 + 1, srclen);
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 2ea96c0a8f..4ecda97997 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2978,6 +2978,17 @@ static ExitStatus op_mvcle(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_mvclu(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
+    TCGv_i32 r3 = tcg_const_i32(get_field(s->fields, r3));
+    gen_helper_mvclu(cc_op, cpu_env, r1, o->in2, r3);
+    tcg_temp_free_i32(r1);
+    tcg_temp_free_i32(r3);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 #ifndef CONFIG_USER_ONLY
 static ExitStatus op_mvcp(DisasContext *s, DisasOps *o)
 {
-- 
2.11.0

Re: [Qemu-devel] [PATCH 19/26] target/s390x: implement MOVE LONG UNICODE

[Richard Henderson]

On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
> +            cpu_stb_data_ra(env, *dest, pad & 0xff, ra);
> +	    *dest += 1;
> +	    *destlen -= 1;

Tabs or alignment?

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


r~

[Qemu-devel] [PATCH 20/26] target/s390x: implement PACK ASCII

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/mem_helper.c  | 35 +++++++++++++++++++++++++++++++++++
 target/s390x/translate.c   | 16 ++++++++++++++++
 4 files changed, 54 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 40b7544c4d..253e1e679c 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -83,6 +83,7 @@ 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_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)
 DEF_HELPER_FLAGS_4(unpk, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(tr, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_4(tre, i64, env, i64, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index ceaa245530..b6d2215d85 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -661,6 +661,8 @@
     /* Really format SS_b, but we pack both lengths into one argument
        for the helper call, so we might as well leave one 8-bit field.  */
     C(0xf200, PACK,    SS_a,  Z,   la1, a2, 0, 0, pack, 0)
+/* PACK ASCII */
+    C(0xe900, PKA,     SS_f,  E2,  la1, a2, 0, 0, pka, 0)
 
 /* PREFETCH */
     /* Implemented as nops of course.  */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 134dde03c2..6d64497edc 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -959,6 +959,41 @@ void HELPER(pack)(CPUS390XState *env, uint32_t len, uint64_t dest, uint64_t src)
     }
 }
 
+void HELPER(pka)(CPUS390XState *env, uint64_t dest, uint64_t src,
+                 uint32_t srclen)
+{
+    uintptr_t ra = GETPC();
+    int i;
+    /* The destination operand is always 16 bytes long.  */
+    const int destlen = 16;
+
+    /* The operands are processed from right to left.  */
+    src += srclen - 1;
+    dest += destlen - 1;
+
+    for (i = 0; i < destlen; i++) {
+        uint8_t b = 0;
+
+        /* Start with a positive sign */
+        if (i == 0) {
+            b = 0xc;
+        } else if (srclen > 1) {
+            b = cpu_ldub_data_ra(env, src, ra) & 0x0f;
+            src--;
+            srclen--;
+        }
+
+        if (srclen > 1) {
+            b |= cpu_ldub_data_ra(env, src, ra) << 4;
+            src--;
+            srclen--;
+        }
+
+        cpu_stb_data_ra(env, dest, b, ra);
+        dest--;
+    }
+}
+
 void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
                   uint64_t src)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 4ecda97997..c74a80bf60 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3226,6 +3226,22 @@ static ExitStatus op_pack(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_pka(DisasContext *s, DisasOps *o)
+{
+    int l2 = get_field(s->fields, l2) + 1;
+    TCGv_i32 l;
+
+    /* The length must not exceed 32 bytes.  */
+    if (l2 > 32) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return EXIT_NORETURN;
+    }
+    l = tcg_const_i32(l2);
+    gen_helper_pka(cpu_env, o->addr1, o->in2, l);
+    tcg_temp_free_i32(l);
+    return NO_EXIT;
+}
+
 static ExitStatus op_popcnt(DisasContext *s, DisasOps *o)
 {
     gen_helper_popcnt(o->out, o->in2);
-- 
2.11.0

Re: [Qemu-devel] [PATCH 20/26] target/s390x: implement PACK ASCII

[Richard Henderson]

On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/helper.h      |  1 +
>   target/s390x/insn-data.def |  2 ++
>   target/s390x/mem_helper.c  | 35 +++++++++++++++++++++++++++++++++++
>   target/s390x/translate.c   | 16 ++++++++++++++++
>   4 files changed, 54 insertions(+)

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


r~

[Qemu-devel] [PATCH 21/26] target/s390x: implement PACK UNICODE

[Aurelien Jarno]

Use a common helper with PACK ASCII as the differences are limited to
the stride of the source operand.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/mem_helper.c  | 30 +++++++++++++++++++++---------
 target/s390x/translate.c   | 16 ++++++++++++++++
 4 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 253e1e679c..1011df122d 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -84,6 +84,7 @@ DEF_HELPER_FLAGS_3(sqxb, TCG_CALL_NO_WG, i64, env, i64, i64)
 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)
+DEF_HELPER_FLAGS_4(pku, TCG_CALL_NO_WG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_4(unpk, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(tr, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_4(tre, i64, env, i64, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index b6d2215d85..3335270182 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -663,6 +663,8 @@
     C(0xf200, PACK,    SS_a,  Z,   la1, a2, 0, 0, pack, 0)
 /* PACK ASCII */
     C(0xe900, PKA,     SS_f,  E2,  la1, a2, 0, 0, pka, 0)
+/* PACK UNICODE */
+    C(0xe100, PKU,     SS_f,  E2,  la1, a2, 0, 0, pku, 0)
 
 /* PREFETCH */
     /* Implemented as nops of course.  */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 6d64497edc..ab34a309ce 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -959,10 +959,9 @@ void HELPER(pack)(CPUS390XState *env, uint32_t len, uint64_t dest, uint64_t src)
     }
 }
 
-void HELPER(pka)(CPUS390XState *env, uint64_t dest, uint64_t src,
-                 uint32_t srclen)
+static inline void do_pkau(CPUS390XState *env, uint64_t dest, uint64_t src,
+                           uint32_t srclen, int ssize, uintptr_t ra)
 {
-    uintptr_t ra = GETPC();
     int i;
     /* The destination operand is always 16 bytes long.  */
     const int destlen = 16;
@@ -977,16 +976,16 @@ void HELPER(pka)(CPUS390XState *env, uint64_t dest, uint64_t src,
         /* Start with a positive sign */
         if (i == 0) {
             b = 0xc;
-        } else if (srclen > 1) {
+        } else if (srclen > ssize) {
             b = cpu_ldub_data_ra(env, src, ra) & 0x0f;
-            src--;
-            srclen--;
+            src -= ssize;
+            srclen -= ssize;
         }
 
-        if (srclen > 1) {
+        if (srclen > ssize) {
             b |= cpu_ldub_data_ra(env, src, ra) << 4;
-            src--;
-            srclen--;
+            src -= ssize;
+            srclen -= ssize;
         }
 
         cpu_stb_data_ra(env, dest, b, ra);
@@ -994,6 +993,19 @@ void HELPER(pka)(CPUS390XState *env, uint64_t dest, uint64_t src,
     }
 }
 
+
+void HELPER(pka)(CPUS390XState *env, uint64_t dest, uint64_t src,
+                 uint32_t srclen)
+{
+    do_pkau(env, dest, src, srclen, 1, GETPC());
+}
+
+void HELPER(pku)(CPUS390XState *env, uint64_t dest, uint64_t src,
+                 uint32_t srclen)
+{
+    do_pkau(env, dest, src, srclen, 2, GETPC());
+}
+
 void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
                   uint64_t src)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index c74a80bf60..a2a27c86ea 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3242,6 +3242,22 @@ static ExitStatus op_pka(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_pku(DisasContext *s, DisasOps *o)
+{
+    int l2 = get_field(s->fields, l2) + 1;
+    TCGv_i32 l;
+
+    /* The length must be even and should not exceed 64 bytes.  */
+    if ((l2 & 1) || (l2 > 64)) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return EXIT_NORETURN;
+    }
+    l = tcg_const_i32(l2);
+    gen_helper_pku(cpu_env, o->addr1, o->in2, l);
+    tcg_temp_free_i32(l);
+    return NO_EXIT;
+}
+
 static ExitStatus op_popcnt(DisasContext *s, DisasOps *o)
 {
     gen_helper_popcnt(o->out, o->in2);
-- 
2.11.0

Re: [Qemu-devel] [PATCH 21/26] target/s390x: implement PACK UNICODE

[Richard Henderson]

On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
> +        } else if (srclen > ssize) {
>               b = cpu_ldub_data_ra(env, src, ra) & 0x0f;
> -            src--;
> -            srclen--;
> +            src -= ssize;
> +            srclen -= ssize;

Surely we need to use lduw in order to correctly read the big-endian 16-bit 
value, or bias src by +1 to read the low byte of same.


r~

Re: [Qemu-devel] [PATCH 21/26] target/s390x: implement PACK UNICODE

[Aurelien Jarno]

On 2017-05-26 09:35, Richard Henderson wrote:
> On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
> > +        } else if (srclen > ssize) {
> >               b = cpu_ldub_data_ra(env, src, ra) & 0x0f;
> > -            src--;
> > -            srclen--;
> > +            src -= ssize;
> > +            srclen -= ssize;
> 
> Surely we need to use lduw in order to correctly read the big-endian 16-bit
> value, or bias src by +1 to read the low byte of same.

The operands are read from right to left, therefore src is adjusted to
point to the last byte before the for loop:

+    /* The operands are processed from right to left.  */
+    src += srclen - 1;
+    dest += destlen - 1;

Given s390 is big-endian, it means src initially always point to the
low byte independently to the size of the value.

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

Re: [Qemu-devel] [PATCH 21/26] target/s390x: implement PACK UNICODE

[Richard Henderson]

On 05/29/2017 04:22 AM, Aurelien Jarno wrote:
> On 2017-05-26 09:35, Richard Henderson wrote:
>> On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
>>> +        } else if (srclen > ssize) {
>>>                b = cpu_ldub_data_ra(env, src, ra) & 0x0f;
>>> -            src--;
>>> -            srclen--;
>>> +            src -= ssize;
>>> +            srclen -= ssize;
>>
>> Surely we need to use lduw in order to correctly read the big-endian 16-bit
>> value, or bias src by +1 to read the low byte of same.
> 
> The operands are read from right to left, therefore src is adjusted to
> point to the last byte before the for loop:
> 
> +    /* The operands are processed from right to left.  */
> +    src += srclen - 1;
> +    dest += destlen - 1;
> 
> Given s390 is big-endian, it means src initially always point to the
> low byte independently to the size of the value.
> 

Quite right.  I didn't consider the processing order.


r~

[Qemu-devel] [PATCH 22/26] target/s390x: implement UNPACK ASCII

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/mem_helper.c  | 51 ++++++++++++++++++++++++++++++++++++++++++++++
 target/s390x/translate.c   | 17 ++++++++++++++++
 4 files changed, 71 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 1011df122d..ffc0588e83 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -86,6 +86,7 @@ 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)
 DEF_HELPER_FLAGS_4(pku, TCG_CALL_NO_WG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_4(unpk, TCG_CALL_NO_WG, void, env, i32, i64, i64)
+DEF_HELPER_FLAGS_4(unpka, TCG_CALL_NO_WG, i32, env, i64, i32, i64)
 DEF_HELPER_FLAGS_4(tr, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_4(tre, i64, env, i64, i64, i64)
 DEF_HELPER_4(trt, i32, env, i32, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 3335270182..b0c0ea6687 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -869,6 +869,8 @@
     /* Really format SS_b, but we pack both lengths into one argument
        for the helper call, so we might as well leave one 8-bit field.  */
     C(0xf300, UNPK,    SS_a,  Z,   la1, a2, 0, 0, unpk, 0)
+/* UNPACK ASCII */
+    C(0xea00, UNPKA,   SS_a,  E2,  la1, a2, 0, 0, unpka, 0)
 
 #ifndef CONFIG_USER_ONLY
 /* COMPARE AND SWAP AND PURGE */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index ab34a309ce..aaabc63000 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1053,6 +1053,57 @@ void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
     }
 }
 
+uint32_t HELPER(unpka)(CPUS390XState *env, uint64_t dest, uint32_t destlen,
+                       uint64_t src)
+{
+    uintptr_t ra = GETPC();
+    int i;
+    uint32_t cc;
+    uint8_t b;
+    /* The source operand is always 16 bytes long.  */
+    const int srclen = 16;
+
+    /* The operands are processed from right to left.  */
+    src += srclen - 1;
+    dest += destlen - 1;
+
+    /* Check for the sign.  */
+    b = cpu_ldub_data_ra(env, src, ra);
+    src--;
+    switch (b & 0xf) {
+    case 0xa:
+    case 0xc:
+    case 0xe ... 0xf:
+        cc = 0;  /* plus */
+        break;
+    case 0xb:
+    case 0xd:
+        cc = 1;  /* minus */
+        break;
+    default:
+    case 0x0 ... 0x9:
+        cc = 3;  /* invalid */
+        break;
+    }
+
+    /* Now pad every nibble with 0x30, advancing one nibble at a time. */
+    for (i = 0; i < destlen; i++) {
+        if (i == 31) {
+            /* If length is 32 bytes, the leftmost byte is 0. */
+            b = 0;
+        } else if (i % 2) {
+            b = cpu_ldub_data_ra(env, src, ra);
+            src--;
+        } else {
+            b >>= 4;
+        }
+        cpu_stb_data_ra(env, dest, 0x30 + (b & 0xf), ra);
+        dest--;
+    }
+
+    return cc;
+}
+
 static void do_helper_tr(CPUS390XState *env, uint32_t len, uint64_t array,
                          uint64_t trans, uintptr_t ra)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index a2a27c86ea..590ff96301 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4295,6 +4295,23 @@ static ExitStatus op_unpk(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_unpka(DisasContext *s, DisasOps *o)
+{
+    int l1 = get_field(s->fields, l1) + 1;
+    TCGv_i32 l;
+
+    /* The length must not exceed 32 bytes.  */
+    if (l1 > 32) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return EXIT_NORETURN;
+    }
+    l = tcg_const_i32(l1);
+    gen_helper_unpka(cc_op, cpu_env, o->addr1, l, o->in2);
+    tcg_temp_free_i32(l);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_xc(DisasContext *s, DisasOps *o)
 {
     int d1 = get_field(s->fields, d1);
-- 
2.11.0

Re: [Qemu-devel] [PATCH 22/26] target/s390x: implement UNPACK ASCII

[Richard Henderson]

On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/helper.h      |  1 +
>   target/s390x/insn-data.def |  2 ++
>   target/s390x/mem_helper.c  | 51 ++++++++++++++++++++++++++++++++++++++++++++++
>   target/s390x/translate.c   | 17 ++++++++++++++++
>   4 files changed, 71 insertions(+)

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


r~

[Qemu-devel] [PATCH 23/26] target/s390x: implement UNPACK UNICODE

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/mem_helper.c  | 50 ++++++++++++++++++++++++++++++++++++----------
 target/s390x/translate.c   | 18 +++++++++++++++++
 4 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index ffc0588e83..8d94018a3f 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -87,6 +87,7 @@ DEF_HELPER_FLAGS_4(pka, TCG_CALL_NO_WG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_4(pku, TCG_CALL_NO_WG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_4(unpk, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(unpka, TCG_CALL_NO_WG, i32, env, i64, i32, i64)
+DEF_HELPER_FLAGS_4(unpku, TCG_CALL_NO_WG, i32, env, i64, i32, i64)
 DEF_HELPER_FLAGS_4(tr, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_4(tre, i64, env, i64, i64, i64)
 DEF_HELPER_4(trt, i32, env, i32, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index b0c0ea6687..56928c24a5 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -871,6 +871,8 @@
     C(0xf300, UNPK,    SS_a,  Z,   la1, a2, 0, 0, unpk, 0)
 /* UNPACK ASCII */
     C(0xea00, UNPKA,   SS_a,  E2,  la1, a2, 0, 0, unpka, 0)
+/* UNPACK UNICODE */
+    C(0xe200, UNPKU,   SS_a,  E2,  la1, a2, 0, 0, unpku, 0)
 
 #ifndef CONFIG_USER_ONLY
 /* COMPARE AND SWAP AND PURGE */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index aaabc63000..3366c2e799 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -93,6 +93,23 @@ static inline uint64_t cpu_ldusize_data_ra(CPUS390XState *env, uint64_t addr,
     }
 }
 
+/* Store a to memory according to its size.  */
+static inline void cpu_stsize_data_ra(CPUS390XState *env, uint64_t addr,
+                                      uint64_t value, int wordsize,
+                                      uintptr_t ra)
+{
+    switch (wordsize) {
+    case 1:
+        cpu_stb_data_ra(env, addr, value, ra);
+        break;
+    case 2:
+        cpu_stw_data_ra(env, addr, value, ra);
+        break;
+    default:
+        abort();
+    }
+}
+
 static void fast_memset(CPUS390XState *env, uint64_t dest, uint8_t byte,
                         uint32_t l, uintptr_t ra)
 {
@@ -1053,10 +1070,10 @@ void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
     }
 }
 
-uint32_t HELPER(unpka)(CPUS390XState *env, uint64_t dest, uint32_t destlen,
-                       uint64_t src)
+static inline uint32_t do_unpkau(CPUS390XState *env, uint64_t dest,
+                                 uint32_t destlen, int dsize, uint64_t src,
+                                 uintptr_t ra)
 {
-    uintptr_t ra = GETPC();
     int i;
     uint32_t cc;
     uint8_t b;
@@ -1065,7 +1082,7 @@ uint32_t HELPER(unpka)(CPUS390XState *env, uint64_t dest, uint32_t destlen,
 
     /* The operands are processed from right to left.  */
     src += srclen - 1;
-    dest += destlen - 1;
+    dest += destlen - dsize;
 
     /* Check for the sign.  */
     b = cpu_ldub_data_ra(env, src, ra);
@@ -1087,23 +1104,36 @@ uint32_t HELPER(unpka)(CPUS390XState *env, uint64_t dest, uint32_t destlen,
     }
 
     /* Now pad every nibble with 0x30, advancing one nibble at a time. */
-    for (i = 0; i < destlen; i++) {
-        if (i == 31) {
-            /* If length is 32 bytes, the leftmost byte is 0. */
+    for (i = 0; i < destlen; i += dsize) {
+        if (i == (31 * dsize)) {
+            /* If length is 32/64 bytes, the leftmost byte is 0. */
             b = 0;
-        } else if (i % 2) {
+        } else if (i % (2 * dsize)) {
             b = cpu_ldub_data_ra(env, src, ra);
             src--;
         } else {
             b >>= 4;
         }
-        cpu_stb_data_ra(env, dest, 0x30 + (b & 0xf), ra);
-        dest--;
+        cpu_stsize_data_ra(env, dest, 0x30 + (b & 0xf), dsize, ra);
+        dest -= dsize;
     }
 
     return cc;
 }
 
+
+uint32_t HELPER(unpka)(CPUS390XState *env, uint64_t dest, uint32_t destlen,
+                       uint64_t src)
+{
+    return do_unpkau(env, dest, destlen, 1, src, GETPC());
+}
+
+uint32_t HELPER(unpku)(CPUS390XState *env, uint64_t dest, uint32_t destlen,
+                       uint64_t src)
+{
+    return do_unpkau(env, dest, destlen, 2, src, GETPC());
+}
+
 static void do_helper_tr(CPUS390XState *env, uint32_t len, uint64_t array,
                          uint64_t trans, uintptr_t ra)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 590ff96301..d9a6a92984 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4312,6 +4312,24 @@ static ExitStatus op_unpka(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_unpku(DisasContext *s, DisasOps *o)
+{
+    int l1 = get_field(s->fields, l1) + 1;
+    TCGv_i32 l;
+
+    /* The length must be even and should not exceed 64 bytes.  */
+    if ((l1 & 1) || (l1 > 64)) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return EXIT_NORETURN;
+    }
+    l = tcg_const_i32(l1);
+    gen_helper_unpku(cc_op, cpu_env, o->addr1, l, o->in2);
+    tcg_temp_free_i32(l);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
+
 static ExitStatus op_xc(DisasContext *s, DisasOps *o)
 {
     int d1 = get_field(s->fields, d1);
-- 
2.11.0

Re: [Qemu-devel] [PATCH 23/26] target/s390x: implement UNPACK UNICODE

[Richard Henderson]

On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/helper.h      |  1 +
>   target/s390x/insn-data.def |  2 ++
>   target/s390x/mem_helper.c  | 50 ++++++++++++++++++++++++++++++++++++----------
>   target/s390x/translate.c   | 18 +++++++++++++++++
>   4 files changed, 61 insertions(+), 10 deletions(-)

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


r~

[Qemu-devel] [PATCH 24/26] target/s390x: implement TEST DECIMAL

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  3 +++
 target/s390x/mem_helper.c  | 23 +++++++++++++++++++++++
 target/s390x/translate.c   |  9 +++++++++
 4 files changed, 36 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 8d94018a3f..7bfb0223ce 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -88,6 +88,7 @@ DEF_HELPER_FLAGS_4(pku, TCG_CALL_NO_WG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_4(unpk, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(unpka, TCG_CALL_NO_WG, i32, env, i64, i32, i64)
 DEF_HELPER_FLAGS_4(unpku, TCG_CALL_NO_WG, i32, env, i64, i32, i64)
+DEF_HELPER_FLAGS_3(tp, TCG_CALL_NO_WG, i32, env, i64, i32)
 DEF_HELPER_FLAGS_4(tr, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_4(tre, i64, env, i64, i64, i64)
 DEF_HELPER_4(trt, i32, env, i32, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 56928c24a5..c0fe04e45c 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -850,6 +850,9 @@
     C(0xed11, TCDB,    RXE,   Z,   f1_o, a2, 0, 0, tcdb, 0)
     C(0xed12, TCXB,    RXE,   Z,   x1_o, a2, 0, 0, tcxb, 0)
 
+/* TEST DECIMAL */
+    C(0xebc0, TP,      RSL,   E2,  la1, 0, 0, 0, tp, 0)
+
 /* TEST UNDER MASK */
     C(0x9100, TM,      SI,    Z,   m1_8u, i2_8u, 0, 0, 0, tm32)
     C(0xeb51, TMY,     SIY,   LD,  m1_8u, i2_8u, 0, 0, 0, tm32)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 3366c2e799..b6e083c7e1 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1134,6 +1134,29 @@ uint32_t HELPER(unpku)(CPUS390XState *env, uint64_t dest, uint32_t destlen,
     return do_unpkau(env, dest, destlen, 2, src, GETPC());
 }
 
+uint32_t HELPER(tp)(CPUS390XState *env, uint64_t dest, uint32_t destlen)
+{
+    uintptr_t ra = GETPC();
+    uint32_t cc = 0;
+    int i;
+
+    for (i = 0; i < destlen; i++) {
+        uint8_t b = cpu_ldub_data_ra(env, dest + i, ra);
+        /* digit */
+        cc |= (b & 0xf0) > 0x90 ? 2 : 0;
+
+        if (i == (destlen - 1)) {
+            /* sign */
+            cc |= (b & 0xf) < 0xa ? 1 : 0;
+        } else {
+            /* digit */
+            cc |= (b & 0xf) > 0x9 ? 2 : 0;
+        }
+    }
+
+    return cc;
+}
+
 static void do_helper_tr(CPUS390XState *env, uint32_t len, uint64_t array,
                          uint64_t trans, uintptr_t ra)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index d9a6a92984..b51342583e 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4251,6 +4251,15 @@ static ExitStatus op_tprot(DisasContext *s, DisasOps *o)
 
 #endif
 
+static ExitStatus op_tp(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 l1 = tcg_const_i32(get_field(s->fields, l1) + 1);
+    gen_helper_tp(cc_op, cpu_env, o->addr1, l1);
+    tcg_temp_free_i32(l1);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_tr(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
-- 
2.11.0

Re: [Qemu-devel] [PATCH 24/26] target/s390x: implement TEST DECIMAL

[Richard Henderson]

On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/helper.h      |  1 +
>   target/s390x/insn-data.def |  3 +++
>   target/s390x/mem_helper.c  | 23 +++++++++++++++++++++++
>   target/s390x/translate.c   |  9 +++++++++
>   4 files changed, 36 insertions(+)

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


r~

[Qemu-devel] [PATCH 25/26] target/s390x: implement TRANSLATE ONE/TWO TO ONE/TWO

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  9 +++++++++
 target/s390x/mem_helper.c  | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 target/s390x/translate.c   | 13 +++++++++++++
 4 files changed, 69 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 7bfb0223ce..e469011196 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -92,6 +92,7 @@ DEF_HELPER_FLAGS_3(tp, TCG_CALL_NO_WG, i32, env, i64, i32)
 DEF_HELPER_FLAGS_4(tr, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_4(tre, i64, env, i64, i64, i64)
 DEF_HELPER_4(trt, i32, env, i32, i64, i64)
+DEF_HELPER_4(trXX, i32, env, i32, i32, i32)
 DEF_HELPER_4(cksm, i64, env, i64, i64, i64)
 DEF_HELPER_FLAGS_5(calc_cc, TCG_CALL_NO_RWG_SE, i32, env, i32, i64, i64, i64)
 DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index c0fe04e45c..68364dfec3 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -868,6 +868,15 @@
 /* TRANSLATE EXTENDED */
     C(0xb2a5, TRE,     RRE,   Z,   0, r2, r1_P, 0, tre, 0)
 
+/* TRANSLATE ONE TO ONE */
+    C(0xb993, TROO,    RRF_c, E2,  0, 0, 0, 0, trXX, 0)
+/* TRANSLATE ONE TO TWO */
+    C(0xb992, TROT,    RRF_c, E2,  0, 0, 0, 0, trXX, 0)
+/* TRANSLATE TWO TO ONE */
+    C(0xb991, TRTO,    RRF_c, E2,  0, 0, 0, 0, trXX, 0)
+/* TRANSLATE TWO TO TWO */
+    C(0xb990, TRTT,    RRF_c, E2,  0, 0, 0, 0, trXX, 0)
+
 /* UNPACK */
     /* Really format SS_b, but we pack both lengths into one argument
        for the helper call, so we might as well leave one 8-bit field.  */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index b6e083c7e1..47e772bb12 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1240,6 +1240,52 @@ uint32_t HELPER(trt)(CPUS390XState *env, uint32_t len, uint64_t array,
     return do_helper_trt(env, len, array, trans, GETPC());
 }
 
+/* Translate one/two to one/two */
+uint32_t HELPER(trXX)(CPUS390XState *env, uint32_t r1, uint32_t r2,
+                      uint32_t sizes)
+{
+    uintptr_t ra = GETPC();
+    int dsize = (sizes & 1) ? 1 : 2;
+    int ssize = (sizes & 2) ? 1 : 2;
+    uint16_t tst = env->regs[0] & ((1 << (8 * dsize)) - 1);
+    uint64_t tbl = get_address(env, 1) & ~7;
+    uint64_t dst = get_address(env, r1);
+    uint64_t len = get_length(env, r1 + 1);
+    uint64_t src = get_address(env, r2);
+    uint32_t cc = 3;
+    int i;
+
+    check_alignment(env, len, ssize, ra);
+
+    /* Lest we fail to service interrupts in a timely manner, */
+    /* limit the amount of work we're willing to do.   */
+    for (i = 0; i < 0x2000; i++) {
+        uint16_t sval = cpu_ldusize_data_ra(env, src, ssize, ra);
+        uint64_t tble = tbl + (sval * dsize);
+        uint16_t dval = cpu_ldusize_data_ra(env, tble, dsize, ra);
+        if (dval == tst) {
+            cc = 1;
+            break;
+        }
+        cpu_stsize_data_ra(env, dst, dval, dsize, ra);
+
+        len -= ssize;
+        src += ssize;
+        dst += dsize;
+
+        if (len == 0) {
+            cc = 0;
+            break;
+        }
+    }
+
+    set_address(env, r1, dst);
+    set_length(env, r1 + 1, len);
+    set_address(env, r2, src);
+
+    return cc;
+}
+
 void HELPER(cdsg)(CPUS390XState *env, uint64_t addr,
                   uint32_t r1, uint32_t r3)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index b51342583e..38c4bf1812 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4286,6 +4286,19 @@ static ExitStatus op_trt(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_trXX(DisasContext *s, DisasOps *o)
+{
+    TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
+    TCGv_i32 r2 = tcg_const_i32(get_field(s->fields, r2));
+    TCGv_i32 sizes = tcg_const_i32(s->insn->opc & 3);
+    gen_helper_trXX(cc_op, cpu_env, r1, r2, sizes);
+    tcg_temp_free_i32(r1);
+    tcg_temp_free_i32(r2);
+    tcg_temp_free_i32(sizes);
+    set_cc_static(s);
+    return NO_EXIT;
+}
+
 static ExitStatus op_ts(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 t1 = tcg_const_i32(0xff);
-- 
2.11.0

Re: [Qemu-devel] [PATCH 25/26] target/s390x: implement TRANSLATE ONE/TWO TO ONE/TWO

[Richard Henderson]

On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
> +uint32_t HELPER(trXX)(CPUS390XState *env, uint32_t r1, uint32_t r2,
> +                      uint32_t sizes)
> +{
> +    uintptr_t ra = GETPC();
> +    int dsize = (sizes & 1) ? 1 : 2;
> +    int ssize = (sizes & 2) ? 1 : 2;
> +    uint16_t tst = env->regs[0] & ((1 << (8 * dsize)) - 1);

I think you should pass in tst as an argument.  That way you can pass in an 
out-of-band value when we implement ETF2 and test field M3 bit 3.


r~

Re: [Qemu-devel] [PATCH 25/26] target/s390x: implement TRANSLATE ONE/TWO TO ONE/TWO

[Aurelien Jarno]

On 2017-05-26 10:10, Richard Henderson wrote:
> On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
> > +uint32_t HELPER(trXX)(CPUS390XState *env, uint32_t r1, uint32_t r2,
> > +                      uint32_t sizes)
> > +{
> > +    uintptr_t ra = GETPC();
> > +    int dsize = (sizes & 1) ? 1 : 2;
> > +    int ssize = (sizes & 2) ? 1 : 2;
> > +    uint16_t tst = env->regs[0] & ((1 << (8 * dsize)) - 1);
> 
> I think you should pass in tst as an argument.  That way you can pass in an
> out-of-band value when we implement ETF2 and test field M3 bit 3.

I don't mind passing r0 as an argument. That said if we want to pass tst
or bundle the M3 field, it means we need to use TCG instructions to do
so. I am not sure it brings a lot compare to doing so in the helper
side.

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

Re: [Qemu-devel] [PATCH 25/26] target/s390x: implement TRANSLATE ONE/TWO TO ONE/TWO

[Richard Henderson]

On 05/29/2017 04:17 AM, Aurelien Jarno wrote:
> On 2017-05-26 10:10, Richard Henderson wrote:
>> On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
>>> +uint32_t HELPER(trXX)(CPUS390XState *env, uint32_t r1, uint32_t r2,
>>> +                      uint32_t sizes)
>>> +{
>>> +    uintptr_t ra = GETPC();
>>> +    int dsize = (sizes & 1) ? 1 : 2;
>>> +    int ssize = (sizes & 2) ? 1 : 2;
>>> +    uint16_t tst = env->regs[0] & ((1 << (8 * dsize)) - 1);
>>
>> I think you should pass in tst as an argument.  That way you can pass in an
>> out-of-band value when we implement ETF2 and test field M3 bit 3.
> 
> I don't mind passing r0 as an argument. That said if we want to pass tst
> or bundle the M3 field, it means we need to use TCG instructions to do
> so. I am not sure it brings a lot compare to doing so in the helper
> side.

Not at all -- the M3 bit test would be a translation-time check.


r~

Re: [Qemu-devel] [PATCH 25/26] target/s390x: implement TRANSLATE ONE/TWO TO ONE/TWO

[Aurelien Jarno]

On 2017-05-30 09:45, Richard Henderson wrote:
> On 05/29/2017 04:17 AM, Aurelien Jarno wrote:
> > On 2017-05-26 10:10, Richard Henderson wrote:
> > > On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
> > > > +uint32_t HELPER(trXX)(CPUS390XState *env, uint32_t r1, uint32_t r2,
> > > > +                      uint32_t sizes)
> > > > +{
> > > > +    uintptr_t ra = GETPC();
> > > > +    int dsize = (sizes & 1) ? 1 : 2;
> > > > +    int ssize = (sizes & 2) ? 1 : 2;
> > > > +    uint16_t tst = env->regs[0] & ((1 << (8 * dsize)) - 1);
> > > 
> > > I think you should pass in tst as an argument.  That way you can pass in an
> > > out-of-band value when we implement ETF2 and test field M3 bit 3.
> > 
> > I don't mind passing r0 as an argument. That said if we want to pass tst
> > or bundle the M3 field, it means we need to use TCG instructions to do
> > so. I am not sure it brings a lot compare to doing so in the helper
> > side.
> 
> Not at all -- the M3 bit test would be a translation-time check.

I still don't really see the point. On the TCG side it means we need
something like that:

    if (m3 & 1) {
       tcg_gen_movi_tl(r0, -1);
    } else if (dsize == 1) {
       tcg_gen_ext8u(r0, regs[0]);
    } else if (dsize == 2) 
       tcg_gen_ext16u(r0, regs[0]);
    }

On the helper side we then need to check if the value passed equals -1
or not to get m3 instead of directly accessing the value in register 0.
If we want to bundle m3 with something else, it might be better to bundle
it with the "sizes" argument.

If what worries you is the cast of tst to uint8_t / uint16_t as function
of dsize in the helper, we can rename trXX into an inline function
do_trXX and use one helper per instruction instead of a common helper
for the 3 instructions.

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

Re: [Qemu-devel] [PATCH 25/26] target/s390x: implement TRANSLATE ONE/TWO TO ONE/TWO

[Richard Henderson]

On 05/30/2017 12:25 PM, Aurelien Jarno wrote:
> On 2017-05-30 09:45, Richard Henderson wrote:
>> On 05/29/2017 04:17 AM, Aurelien Jarno wrote:
>>> On 2017-05-26 10:10, Richard Henderson wrote:
>>>> On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
>>>>> +uint32_t HELPER(trXX)(CPUS390XState *env, uint32_t r1, uint32_t r2,
>>>>> +                      uint32_t sizes)
>>>>> +{
>>>>> +    uintptr_t ra = GETPC();
>>>>> +    int dsize = (sizes & 1) ? 1 : 2;
>>>>> +    int ssize = (sizes & 2) ? 1 : 2;
>>>>> +    uint16_t tst = env->regs[0] & ((1 << (8 * dsize)) - 1);
>>>>
>>>> I think you should pass in tst as an argument.  That way you can pass in an
>>>> out-of-band value when we implement ETF2 and test field M3 bit 3.
>>>
>>> I don't mind passing r0 as an argument. That said if we want to pass tst
>>> or bundle the M3 field, it means we need to use TCG instructions to do
>>> so. I am not sure it brings a lot compare to doing so in the helper
>>> side.
>>
>> Not at all -- the M3 bit test would be a translation-time check.
> 
> I still don't really see the point. On the TCG side it means we need
> something like that:
> 
>      if (m3 & 1) {
>         tcg_gen_movi_tl(r0, -1);
>      } else if (dsize == 1) {
>         tcg_gen_ext8u(r0, regs[0]);
>      } else if (dsize == 2)
>         tcg_gen_ext16u(r0, regs[0]);
>      }

Yes, exactly.

> On the helper side we then need to check if the value passed equals -1
> or not to get m3 instead of directly accessing the value in register 0.

How's that?  Why would you need to check for -1 at all?  The existing helper 
test works fine, with -1 not matching any value loaded.


r~

Re: [Qemu-devel] [PATCH 25/26] target/s390x: implement TRANSLATE ONE/TWO TO ONE/TWO

[Aurelien Jarno]

On 2017-05-30 12:42, Richard Henderson wrote:
> On 05/30/2017 12:25 PM, Aurelien Jarno wrote:
> > On 2017-05-30 09:45, Richard Henderson wrote:
> > > On 05/29/2017 04:17 AM, Aurelien Jarno wrote:
> > > > On 2017-05-26 10:10, Richard Henderson wrote:
> > > > > On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
> > > > > > +uint32_t HELPER(trXX)(CPUS390XState *env, uint32_t r1, uint32_t r2,
> > > > > > +                      uint32_t sizes)
> > > > > > +{
> > > > > > +    uintptr_t ra = GETPC();
> > > > > > +    int dsize = (sizes & 1) ? 1 : 2;
> > > > > > +    int ssize = (sizes & 2) ? 1 : 2;
> > > > > > +    uint16_t tst = env->regs[0] & ((1 << (8 * dsize)) - 1);
> > > > > 
> > > > > I think you should pass in tst as an argument.  That way you can pass in an
> > > > > out-of-band value when we implement ETF2 and test field M3 bit 3.
> > > > 
> > > > I don't mind passing r0 as an argument. That said if we want to pass tst
> > > > or bundle the M3 field, it means we need to use TCG instructions to do
> > > > so. I am not sure it brings a lot compare to doing so in the helper
> > > > side.
> > > 
> > > Not at all -- the M3 bit test would be a translation-time check.
> > 
> > I still don't really see the point. On the TCG side it means we need
> > something like that:
> > 
> >      if (m3 & 1) {
> >         tcg_gen_movi_tl(r0, -1);
> >      } else if (dsize == 1) {
> >         tcg_gen_ext8u(r0, regs[0]);
> >      } else if (dsize == 2)
> >         tcg_gen_ext16u(r0, regs[0]);
> >      }
> 
> Yes, exactly.
> 
> > On the helper side we then need to check if the value passed equals -1
> > or not to get m3 instead of directly accessing the value in register 0.
> 
> How's that?  Why would you need to check for -1 at all?  The existing helper
> test works fine, with -1 not matching any value loaded.

Ok, I get your point now, i'll implement that in the next version. 

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

[Qemu-devel] [PATCH 26/26] target/s390x: update maximum TCG model to z800

[Aurelien Jarno]

Now that the extended-translation facility 2 has been fully implemented,
it's possible to emulated a most a z800 CPU with TCG.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/cpu_models.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 8d27363b07..9c4227d2dd 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -670,8 +670,8 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
     if (kvm_enabled()) {
         kvm_s390_get_host_cpu_model(&max_model, errp);
     } else {
-        /* TCG emulates a z900 */
-        max_model.def = &s390_cpu_defs[0];
+        /* TCG emulates at most a z800 */
+        max_model.def = &s390_cpu_defs[3];
         bitmap_copy(max_model.features, max_model.def->default_feat,
                     S390_FEAT_MAX);
     }
-- 
2.11.0

Re: [Qemu-devel] [PATCH 26/26] target/s390x: update maximum TCG model to z800

[Thomas Huth]

On 25.05.2017 23:05, Aurelien Jarno wrote:
> Now that the extended-translation facility 2 has been fully implemented,
> it's possible to emulated a most a z800 CPU with TCG.
> 
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target/s390x/cpu_models.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 8d27363b07..9c4227d2dd 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -670,8 +670,8 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>      if (kvm_enabled()) {
>          kvm_s390_get_host_cpu_model(&max_model, errp);
>      } else {
> -        /* TCG emulates a z900 */
> -        max_model.def = &s390_cpu_defs[0];
> +        /* TCG emulates at most a z800 */
> +        max_model.def = &s390_cpu_defs[3];

These hard-coded offsets into s390_cpu_defs are kind of ugly ...
Maybe you could use

    max_model.def = s390_find_cpu_def(0x2066, 7, 3, NULL);

instead?

 Thomas

[Qemu-devel] [PATCH 27/26] target/s390x: fix adj_len_to_page

[Aurelien Jarno]

adj_len_to_page doesn't return the correct result when the address
is already page aligned and the length is bigger than a page. Fix that.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/mem_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

The patch 17 (improve MOVE LONG and MOVE LONG EXTENDED) triggered a bug
in adj_len_to_page. This is the fix, I just didn't want to respin the
series just for that.

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 6add413531..7841808fa2 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -61,7 +61,7 @@ static inline uint32_t adj_len_to_page(uint32_t len, uint64_t addr)
 {
 #ifndef CONFIG_USER_ONLY
     if ((addr & ~TARGET_PAGE_MASK) + len - 1 >= TARGET_PAGE_SIZE) {
-        return -addr & ~TARGET_PAGE_MASK;
+        return (~addr & ~TARGET_PAGE_MASK) + 1;
     }
 #endif
     return len;
-- 
2.11.0

Re: [Qemu-devel] [PATCH 27/26] target/s390x: fix adj_len_to_page

[Richard Henderson]

On 05/25/2017 05:32 PM, Aurelien Jarno wrote:
> adj_len_to_page doesn't return the correct result when the address
> is already page aligned and the length is bigger than a page. Fix that.
> 
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/s390x/mem_helper.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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


r~