[Qemu-devel] [PATCH 00/31] target/s390x unwind patches

[Qemu-devel] [PATCH 00/31] target/s390x unwind patches

[Richard Henderson]

We can use cpu_restore_state (and via *_ra memory helpers) to avoid
storing cpu state in expectation of the unlikely case of page fault
or specification error.

There are more that can be fixed, but this is all of mem_helper.c,
and this patch set is large enough.


r~


Richard Henderson (31):
  target/s390: Use cpu_loop_exit_restore for tlb_fill
  target/s390x: Implement EXECUTE via new TranslationBlock
  target/s390x: Use unwind data for helper_nc
  target/s390x: Use unwind data for helper_xc
  target/s390x: Use unwind data for helper_oc
  target/s390x: Use unwind data for helper_mvc
  target/s390x: Use unwind data for helper_clc
  target/s390x: Use unwind data for helper_clm
  target/s390x: Use unwind data for helper_srst
  target/s390x: Use unwind data for helper_clst
  target/s390x: Use unwind data for helper_mvpg
  target/s390x: Use unwind data for helper_mvst
  target/s390x: Use unwind data for helper_lam
  target/s390x: Use unwind data for helper_stam
  target/s390x: Use unwind data for helper_mvcl
  target/s390x: Use unwind data for helper_mvcle
  target/s390x: Use unwind data for helper_clcle
  target/s390x: Use unwind data for helper_cksm
  target/s390x: Use unwind data for helper_unpk
  target/s390x: Use unwind data for helper_tr
  target/s390x: Use unwind data for helper_tre
  target/s390x: Use unwind data for helper_trt
  target/s390x: Use unwind data for helper_lctlg
  target/s390x: Use unwind data for helper_lctl
  target/s390x: Use unwind data for helper_stctl
  target/s390x: Use unwind data for helper_testblock
  target/s390x: Use unwind data for helper_tprot
  target/s390x: Use unwind data for helper_lra
  target/s390x: Use atomic operations for COMPARE SWAP PURGE
  target/s390x: Implement CSPG
  target/s390x: Use unwind data for helper_mvcs/mvcp

 target/s390x/cpu.h         |   4 +-
 target/s390x/helper.h      |   6 +-
 target/s390x/insn-data.def |   7 +-
 target/s390x/machine.c     |  19 ++
 target/s390x/mem_helper.c  | 428 +++++++++++++++++++--------------------------
 target/s390x/translate.c   | 197 ++++++++++++---------
 6 files changed, 321 insertions(+), 340 deletions(-)

-- 
2.9.4

[Qemu-devel] [PATCH 01/31] target/s390: Use cpu_loop_exit_restore for tlb_fill

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 0c6a0d9..e3325a4 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -41,15 +41,9 @@
 void tlb_fill(CPUState *cs, target_ulong addr, MMUAccessType access_type,
               int mmu_idx, uintptr_t retaddr)
 {
-    int ret;
-
-    ret = s390_cpu_handle_mmu_fault(cs, addr, access_type, mmu_idx);
+    int ret = s390_cpu_handle_mmu_fault(cs, addr, access_type, mmu_idx);
     if (unlikely(ret != 0)) {
-        if (likely(retaddr)) {
-            /* now we have a real cpu fault */
-            cpu_restore_state(cs, retaddr);
-        }
-        cpu_loop_exit(cs);
+        cpu_loop_exit_restore(cs, retaddr);
     }
 }
 
-- 
2.9.4

Re: [Qemu-devel] [PATCH 01/31] target/s390: Use cpu_loop_exit_restore for tlb_fill

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 0c6a0d9..e3325a4 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -41,15 +41,9 @@
>  void tlb_fill(CPUState *cs, target_ulong addr, MMUAccessType access_type,
>                int mmu_idx, uintptr_t retaddr)
>  {
> -    int ret;
> -
> -    ret = s390_cpu_handle_mmu_fault(cs, addr, access_type, mmu_idx);
> +    int ret = s390_cpu_handle_mmu_fault(cs, addr, access_type, mmu_idx);
>      if (unlikely(ret != 0)) {
> -        if (likely(retaddr)) {
> -            /* now we have a real cpu fault */
> -            cpu_restore_state(cs, retaddr);
> -        }
> -        cpu_loop_exit(cs);
> +        cpu_loop_exit_restore(cs, retaddr);
>      }
>  }

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

Re: [Qemu-devel] [PATCH 01/31] target/s390: Use cpu_loop_exit_restore for tlb_fill

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 02/31] target/s390x: Implement EXECUTE via new TranslationBlock

[Richard Henderson]

Previously, helper_ex would construct the insn and then implement
the insn via direct calls other helpers.  This was sufficient to
boot Linux but that is all.

It is easy enough to go the whole nine yards by stashing state for
EXECUTE within the cpu, and then relying on a new TB to be created
that properly and completely interprets the insn.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/cpu.h         |   4 +-
 target/s390x/helper.h      |   2 +-
 target/s390x/insn-data.def |   4 +-
 target/s390x/machine.c     |  19 +++++++
 target/s390x/mem_helper.c  | 136 +++++++++++----------------------------------
 target/s390x/translate.c   | 124 +++++++++++++++++++++++++----------------
 6 files changed, 133 insertions(+), 156 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 4f38ba0..79235cf 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -103,6 +103,8 @@ typedef struct CPUS390XState {
     uint64_t cc_dst;
     uint64_t cc_vr;
 
+    uint64_t ex_value;
+
     uint64_t __excp_addr;
     uint64_t psa;
 
@@ -391,7 +393,7 @@ static inline void cpu_get_tb_cpu_state(CPUS390XState* env, target_ulong *pc,
                                         target_ulong *cs_base, uint32_t *flags)
 {
     *pc = env->psw.addr;
-    *cs_base = 0;
+    *cs_base = env->ex_value;
     *flags = ((env->psw.mask >> 32) & ~FLAG_MASK_CC) |
              ((env->psw.mask & PSW_MASK_32) ? FLAG_MASK_32 : 0);
 }
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 1fae191..d6cc513 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -14,7 +14,7 @@ DEF_HELPER_4(srst, i64, env, i64, i64, i64)
 DEF_HELPER_4(clst, i64, env, i64, i64, i64)
 DEF_HELPER_4(mvpg, void, env, i64, i64, i64)
 DEF_HELPER_4(mvst, i64, env, i64, i64, i64)
-DEF_HELPER_5(ex, i32, env, i32, i64, i64, i64)
+DEF_HELPER_FLAGS_4(ex, TCG_CALL_NO_WG, 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)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index cac0f51..3c3541c 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -327,9 +327,9 @@
     C(0xeb57, XIY,     SIY,   LD,  m1_8u, i2_8u, new, m1_8, xor, nz64)
 
 /* EXECUTE */
-    C(0x4400, EX,      RX_a,  Z,   r1_o, a2, 0, 0, ex, 0)
+    C(0x4400, EX,      RX_a,  Z,   0, a2, 0, 0, ex, 0)
 /* EXECUTE RELATIVE LONG */
-    C(0xc600, EXRL,    RIL_b, EE,  r1_o, ri2, 0, 0, ex, 0)
+    C(0xc600, EXRL,    RIL_b, EE,  0, ri2, 0, 0, ex, 0)
 
 /* EXTRACT ACCESS */
     C(0xb24f, EAR,     RRE,   Z,   0, 0, new, r1_32, ear, 0)
diff --git a/target/s390x/machine.c b/target/s390x/machine.c
index 8503fa1..8f908bb 100644
--- a/target/s390x/machine.c
+++ b/target/s390x/machine.c
@@ -34,6 +34,7 @@ static int cpu_post_load(void *opaque, int version_id)
 
     return 0;
 }
+
 static void cpu_pre_save(void *opaque)
 {
     S390CPU *cpu = opaque;
@@ -156,6 +157,23 @@ const VMStateDescription vmstate_riccb = {
     }
 };
 
+static bool exval_needed(void *opaque)
+{
+    S390CPU *cpu = opaque;
+    return cpu->env.ex_value != 0;
+}
+
+const VMStateDescription vmstate_exval = {
+    .name = "cpu/exval",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = exval_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(env.ex_value, S390CPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 const VMStateDescription vmstate_s390_cpu = {
     .name = "cpu",
     .post_load = cpu_post_load,
@@ -188,6 +206,7 @@ const VMStateDescription vmstate_s390_cpu = {
         &vmstate_fpu,
         &vmstate_vregs,
         &vmstate_riccb,
+        &vmstate_exval,
         NULL
     },
 };
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index e3325a4..db80d53 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -405,115 +405,41 @@ uint64_t HELPER(mvst)(CPUS390XState *env, uint64_t c, uint64_t d, uint64_t s)
     return d + len;
 }
 
-static uint32_t helper_icm(CPUS390XState *env, uint32_t r1, uint64_t address,
-                           uint32_t mask)
-{
-    int pos = 24; /* top of the lower half of r1 */
-    uint64_t rmask = 0xff000000ULL;
-    uint8_t val = 0;
-    int ccd = 0;
-    uint32_t cc = 0;
+/* Execute instruction.  This instruction executes an insn modified with
+   the contents of r1.  It does not change the executed instruction in memory;
+   it does not change the program counter.
 
-    while (mask) {
-        if (mask & 8) {
-            env->regs[r1] &= ~rmask;
-            val = cpu_ldub_data(env, address);
-            if ((val & 0x80) && !ccd) {
-                cc = 1;
-            }
-            ccd = 1;
-            if (val && cc == 0) {
-                cc = 2;
-            }
-            env->regs[r1] |= (uint64_t)val << pos;
-            address++;
-        }
-        mask = (mask << 1) & 0xf;
-        pos -= 8;
-        rmask >>= 8;
-    }
-
-    return cc;
-}
-
-/* execute instruction
-   this instruction executes an insn modified with the contents of r1
-   it does not change the executed instruction in memory
-   it does not change the program counter
-   in other words: tricky...
-   currently implemented by interpreting the cases it is most commonly used in
+   Perform this by recording the modified instruction in env->ex_value.
+   This will be noticed by cpu_get_tb_cpu_state and thus tb translation.
 */
-uint32_t HELPER(ex)(CPUS390XState *env, uint32_t cc, uint64_t v1,
-                    uint64_t addr, uint64_t ret)
+void HELPER(ex)(CPUS390XState *env, uint32_t ilen, uint64_t v1, uint64_t addr)
 {
-    S390CPU *cpu = s390_env_get_cpu(env);
-    uint16_t insn = cpu_lduw_code(env, addr);
-
-    HELPER_LOG("%s: v1 0x%lx addr 0x%lx insn 0x%x\n", __func__, v1, addr,
-               insn);
-    if ((insn & 0xf0ff) == 0xd000) {
-        uint32_t l, insn2, b1, b2, d1, d2;
-
-        l = v1 & 0xff;
-        insn2 = cpu_ldl_code(env, addr + 2);
-        b1 = (insn2 >> 28) & 0xf;
-        b2 = (insn2 >> 12) & 0xf;
-        d1 = (insn2 >> 16) & 0xfff;
-        d2 = insn2 & 0xfff;
-        switch (insn & 0xf00) {
-        case 0x200:
-            helper_mvc(env, l, get_address(env, 0, b1, d1),
-                       get_address(env, 0, b2, d2));
-            break;
-        case 0x400:
-            cc = helper_nc(env, l, get_address(env, 0, b1, d1),
-                            get_address(env, 0, b2, d2));
-            break;
-        case 0x500:
-            cc = helper_clc(env, l, get_address(env, 0, b1, d1),
-                            get_address(env, 0, b2, d2));
-            break;
-        case 0x600:
-            cc = helper_oc(env, l, get_address(env, 0, b1, d1),
-                            get_address(env, 0, b2, d2));
-            break;
-        case 0x700:
-            cc = helper_xc(env, l, get_address(env, 0, b1, d1),
-                           get_address(env, 0, b2, d2));
-            break;
-        case 0xc00:
-            helper_tr(env, l, get_address(env, 0, b1, d1),
-                      get_address(env, 0, b2, d2));
-            break;
-        case 0xd00:
-            cc = helper_trt(env, l, get_address(env, 0, b1, d1),
-                            get_address(env, 0, b2, d2));
-            break;
-        default:
-            goto abort;
-        }
-    } else if ((insn & 0xff00) == 0x0a00) {
-        /* supervisor call */
-        HELPER_LOG("%s: svc %ld via execute\n", __func__, (insn | v1) & 0xff);
-        env->psw.addr = ret - 4;
-        env->int_svc_code = (insn | v1) & 0xff;
-        env->int_svc_ilen = 4;
-        helper_exception(env, EXCP_SVC);
-    } else if ((insn & 0xff00) == 0xbf00) {
-        uint32_t insn2, r1, r3, b2, d2;
-
-        insn2 = cpu_ldl_code(env, addr + 2);
-        r1 = (insn2 >> 20) & 0xf;
-        r3 = (insn2 >> 16) & 0xf;
-        b2 = (insn2 >> 12) & 0xf;
-        d2 = insn2 & 0xfff;
-        cc = helper_icm(env, r1, get_address(env, 0, b2, d2), r3);
-    } else {
-    abort:
-        cpu_abort(CPU(cpu), "EXECUTE on instruction prefix 0x%x not implemented\n",
-                  insn);
+    uintptr_t ra = GETPC();
+    uint64_t insn = cpu_lduw_code_ra(env, addr, ra);
+
+    /* Or in the contents of R1[56:63].  */
+    insn |= v1 & 0xff;
+
+    /* Load the rest of the instruction.  */
+    insn <<= 48;
+    switch (get_ilen(insn >> 56)) {
+    case 2:
+        break;
+    case 4:
+        insn |= (uint64_t)cpu_lduw_code_ra(env, addr + 2, ra) << 32;
+        break;
+    case 6:
+        insn |= (uint64_t)(uint32_t)cpu_ldl_code_ra(env, addr + 2, ra) << 16;
+        break;
+    default:
+        g_assert_not_reached();
     }
-    return cc;
+
+    /* Record the insn we want to execute as well as the ilen to use
+       during the execution of the target insn.  This will also ensure
+       that ex_value is non-zero, which flags that we are in a state
+       that requires such execution.  */
+    env->ex_value = insn | ilen;
 }
 
 /* load access registers r1 to r3 from memory at a2 */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index d6736e4..3a72c38 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -57,7 +57,9 @@ struct DisasContext {
     struct TranslationBlock *tb;
     const DisasInsn *insn;
     DisasFields *fields;
+    uint64_t ex_value;
     uint64_t pc, next_pc;
+    uint32_t ilen;
     enum cc_op cc_op;
     bool singlestep_enabled;
 };
@@ -349,7 +351,7 @@ static void gen_program_exception(DisasContext *s, int code)
     tcg_gen_st_i32(tmp, cpu_env, offsetof(CPUS390XState, int_pgm_code));
     tcg_temp_free_i32(tmp);
 
-    tmp = tcg_const_i32(s->next_pc - s->pc);
+    tmp = tcg_const_i32(s->ilen);
     tcg_gen_st_i32(tmp, cpu_env, offsetof(CPUS390XState, int_pgm_ilen));
     tcg_temp_free_i32(tmp);
 
@@ -2153,27 +2155,30 @@ static ExitStatus op_epsw(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_ex(DisasContext *s, DisasOps *o)
 {
-    /* ??? Perhaps a better way to implement EXECUTE is to set a bit in
-       tb->flags, (ab)use the tb->cs_base field as the address of
-       the template in memory, and grab 8 bits of tb->flags/cflags for
-       the contents of the register.  We would then recognize all this
-       in gen_intermediate_code_internal, generating code for exactly
-       one instruction.  This new TB then gets executed normally.
-
-       On the other hand, this seems to be mostly used for modifying
-       MVC inside of memcpy, which needs a helper call anyway.  So
-       perhaps this doesn't bear thinking about any further.  */
-
-    TCGv_i64 tmp;
+    int r1 = get_field(s->fields, r1);
+    TCGv_i32 ilen;
+    TCGv v1;
 
-    update_psw_addr(s);
-    gen_op_calc_cc(s);
+    /* Nested EXECUTE is not allowed.  */
+    if (unlikely(s->ex_value)) {
+        gen_program_exception(s, PGM_EXECUTE);
+        return EXIT_NORETURN;
+    }
 
-    tmp = tcg_const_i64(s->next_pc);
-    gen_helper_ex(cc_op, cpu_env, cc_op, o->in1, o->in2, tmp);
-    tcg_temp_free_i64(tmp);
+    if (r1 == 0) {
+        v1 = tcg_const_i64(0);
+    } else {
+        v1 = regs[r1];
+    }
+    ilen = tcg_const_i32(s->ilen);
+    gen_helper_ex(cpu_env, ilen, v1, o->in2);
+    tcg_temp_free_i32(ilen);
+    if (r1 == 0) {
+        tcg_temp_free_i64(v1);
+    }
 
-    return NO_EXIT;
+    /* End the TB; a new TB will be created for modified insn.  */
+    return EXIT_PC_STALE;
 }
 
 static ExitStatus op_fieb(DisasContext *s, DisasOps *o)
@@ -4027,7 +4032,7 @@ static ExitStatus op_svc(DisasContext *s, DisasOps *o)
     tcg_gen_st_i32(t, cpu_env, offsetof(CPUS390XState, int_svc_code));
     tcg_temp_free_i32(t);
 
-    t = tcg_const_i32(s->next_pc - s->pc);
+    t = tcg_const_i32(s->ilen);
     tcg_gen_st_i32(t, cpu_env, offsetof(CPUS390XState, int_svc_ilen));
     tcg_temp_free_i32(t);
 
@@ -5169,23 +5174,38 @@ static const DisasInsn *extract_insn(CPUS390XState *env, DisasContext *s,
     int op, op2, ilen;
     const DisasInsn *info;
 
-    insn = ld_code2(env, pc);
-    op = (insn >> 8) & 0xff;
-    ilen = get_ilen(op);
-    s->next_pc = s->pc + ilen;
-
-    switch (ilen) {
-    case 2:
-        insn = insn << 48;
-        break;
-    case 4:
-        insn = ld_code4(env, pc) << 32;
-        break;
-    case 6:
-        insn = (insn << 48) | (ld_code4(env, pc + 2) << 16);
-        break;
-    default:
-        abort();
+    if (unlikely(s->ex_value)) {
+        /* Drop the EX data now, so that it's clear on exception paths.  */
+        TCGv_i64 zero = tcg_const_i64(0);
+        tcg_gen_st_i64(zero, cpu_env, offsetof(CPUS390XState, ex_value));
+        tcg_temp_free_i64(zero);
+
+        /* Extract the values saved by EXECUTE.  */
+        insn = s->ex_value & 0xffffffffffff0000ull;
+        ilen = s->ex_value & 0xff;
+        op = insn >> 56;
+        s->ilen = ilen;
+        s->next_pc = s->pc;
+    } else {
+        insn = ld_code2(env, pc);
+        op = (insn >> 8) & 0xff;
+        ilen = get_ilen(op);
+        s->ilen = ilen;
+        s->next_pc = s->pc + ilen;
+
+        switch (ilen) {
+        case 2:
+            insn = insn << 48;
+            break;
+        case 4:
+            insn = ld_code4(env, pc) << 32;
+            break;
+        case 6:
+            insn = (insn << 48) | (ld_code4(env, pc + 2) << 16);
+            break;
+        default:
+            g_assert_not_reached();
+        }
     }
 
     /* We can't actually determine the insn format until we've looked up
@@ -5403,6 +5423,7 @@ void gen_intermediate_code(CPUS390XState *env, struct TranslationBlock *tb)
     dc.tb = tb;
     dc.pc = pc_start;
     dc.cc_op = CC_OP_DYNAMIC;
+    dc.ex_value = tb->cs_base;
     do_debug = dc.singlestep_enabled = cs->singlestep_enabled;
 
     next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
@@ -5444,13 +5465,17 @@ void gen_intermediate_code(CPUS390XState *env, struct TranslationBlock *tb)
 
         /* If we reach a page boundary, are single stepping,
            or exhaust instruction count, stop generation.  */
-        if (status == NO_EXIT
-            && (dc.pc >= next_page_start
-                || tcg_op_buf_full()
-                || num_insns >= max_insns
-                || singlestep
-                || cs->singlestep_enabled)) {
-            status = EXIT_PC_STALE;
+        if (status == NO_EXIT) {
+            if (unlikely(dc.ex_value)) {
+                /* The PC on entry is already advanced.  */
+                status = EXIT_PC_UPDATED;
+            } else if (dc.pc >= next_page_start
+                       || tcg_op_buf_full()
+                       || num_insns >= max_insns
+                       || singlestep
+                       || cs->singlestep_enabled) {
+                status = EXIT_PC_STALE;
+            }
         }
     } while (status == NO_EXIT);
 
@@ -5489,9 +5514,14 @@ void gen_intermediate_code(CPUS390XState *env, struct TranslationBlock *tb)
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
         && qemu_log_in_addr_range(pc_start)) {
         qemu_log_lock();
-        qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(cs, pc_start, dc.pc - pc_start, 1);
-        qemu_log("\n");
+        if (unlikely(dc.ex_value)) {
+            /* ??? Unfortunately log_target_disas can't use host memory.  */
+            qemu_log("IN: EXECUTE %016" PRIx64 "\n", dc.ex_value);
+        } else {
+            qemu_log("IN: %s\n", lookup_symbol(pc_start));
+            log_target_disas(cs, pc_start, dc.pc - pc_start, 1);
+            qemu_log("\n");
+        }
         qemu_log_unlock();
     }
 #endif
-- 
2.9.4

Re: [Qemu-devel] [PATCH 02/31] target/s390x: Implement EXECUTE via new TranslationBlock

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Previously, helper_ex would construct the insn and then implement
> the insn via direct calls other helpers.  This was sufficient to
> boot Linux but that is all.
> 
> It is easy enough to go the whole nine yards by stashing state for
> EXECUTE within the cpu, and then relying on a new TB to be created
> that properly and completely interprets the insn.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/cpu.h         |   4 +-
>  target/s390x/helper.h      |   2 +-
>  target/s390x/insn-data.def |   4 +-
>  target/s390x/machine.c     |  19 +++++++
>  target/s390x/mem_helper.c  | 136 +++++++++++----------------------------------
>  target/s390x/translate.c   | 124 +++++++++++++++++++++++++----------------
>  6 files changed, 133 insertions(+), 156 deletions(-)

This looks good on the principle, and finally removes a big hack. That
said it prevent my test system to boot. I haven't investigated why yet.

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

Re: [Qemu-devel] [PATCH 02/31] target/s390x: Implement EXECUTE via new TranslationBlock

[Richard Henderson]

On 05/23/2017 03:48 AM, Aurelien Jarno wrote:
> On 2017-05-22 20:02, Richard Henderson wrote:
>> Previously, helper_ex would construct the insn and then implement
>> the insn via direct calls other helpers.  This was sufficient to
>> boot Linux but that is all.
>>
>> It is easy enough to go the whole nine yards by stashing state for
>> EXECUTE within the cpu, and then relying on a new TB to be created
>> that properly and completely interprets the insn.
>>
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>> ---
>>   target/s390x/cpu.h         |   4 +-
>>   target/s390x/helper.h      |   2 +-
>>   target/s390x/insn-data.def |   4 +-
>>   target/s390x/machine.c     |  19 +++++++
>>   target/s390x/mem_helper.c  | 136 +++++++++++----------------------------------
>>   target/s390x/translate.c   | 124 +++++++++++++++++++++++++----------------
>>   6 files changed, 133 insertions(+), 156 deletions(-)
> 
> This looks good on the principle, and finally removes a big hack. That
> said it prevent my test system to boot. I haven't investigated why yet.

Hmm.  I've not got a complete environment -- merely booting a kernel up to the 
point it fails to find a rootfs.  Which did find several problems with my first 
attempts at this, but wouldn't have exercised paging.  I'll try again to get a 
full install working...

I wonder if I needed to adjust s390_cpu_handle_mmu_fault (and its myriad 
subroutines) to handle setting ILEN correctly.

There might be a simpler fix though.  Currently I advance the PC and remember 
the ilen of the EX(RL).  Maybe better to *not* advance the PC so as to have the 
original EX(RL) right there for ILEN_LATER and ILEN_LATER_INC to operate on.

Something like this, as a delta patch.


r~


diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 67c85f0..5773f92 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2206,8 +2206,10 @@ static ExitStatus op_ex(DisasContext *s, DisasOps *o)
          tcg_temp_free_i64(v1);
      }

-    /* End the TB; a new TB will be created for modified insn.  */
-    return EXIT_PC_STALE;
+    /* End the TB; a new TB will be created for modified insn.
+       Note that the modified insn runs with this same PC.  */
+    update_psw_addr(s);
+    return EXIT_PC_UPDATED;
  }

  static ExitStatus op_fieb(DisasContext *s, DisasOps *o)
@@ -5189,14 +5191,10 @@ static const DisasInsn *extract_insn
          insn = s->ex_value & 0xffffffffffff0000ull;
          ilen = s->ex_value & 0xff;
          op = insn >> 56;
-        s->ilen = ilen;
-        s->next_pc = s->pc;
      } else {
          insn = ld_code2(env, pc);
          op = (insn >> 8) & 0xff;
          ilen = get_ilen(op);
-        s->ilen = ilen;
-        s->next_pc = s->pc + ilen;

          switch (ilen) {
          case 2:
@@ -5212,6 +5210,8 @@ static const DisasInsn *extract_insn
              g_assert_not_reached();
          }
      }
+    s->next_pc = s->pc + ilen;
+    s->ilen = ilen;

      /* We can't actually determine the insn format until we've looked up
         the full insn opcode.  Which we can't do without locating the
@@ -5470,17 +5470,14 @@ void gen_intermediate_code

          /* If we reach a page boundary, are single stepping,
             or exhaust instruction count, stop generation.  */
-        if (status == NO_EXIT) {
-            if (unlikely(dc.ex_value)) {
-                /* The PC on entry is already advanced.  */
-                status = EXIT_PC_UPDATED;
-            } else if (dc.pc >= next_page_start
-                       || tcg_op_buf_full()
-                       || num_insns >= max_insns
-                       || singlestep
-                       || cs->singlestep_enabled) {
-                status = EXIT_PC_STALE;
-            }
+        if (status == NO_EXIT
+            && (dc.pc >= next_page_start
+                || tcg_op_buf_full()
+                || num_insns >= max_insns
+                || singlestep
+                || cs->singlestep_enabled
+                || dc.ex_value)) {
+            status = EXIT_PC_STALE;
          }
      } while (status == NO_EXIT);

Re: [Qemu-devel] [PATCH 02/31] target/s390x: Implement EXECUTE via new TranslationBlock

[Aurelien Jarno]

On 2017-05-23 08:54, Richard Henderson wrote:
> On 05/23/2017 03:48 AM, Aurelien Jarno wrote:
> > On 2017-05-22 20:02, Richard Henderson wrote:
> > > Previously, helper_ex would construct the insn and then implement
> > > the insn via direct calls other helpers.  This was sufficient to
> > > boot Linux but that is all.
> > > 
> > > It is easy enough to go the whole nine yards by stashing state for
> > > EXECUTE within the cpu, and then relying on a new TB to be created
> > > that properly and completely interprets the insn.
> > > 
> > > Signed-off-by: Richard Henderson <rth@twiddle.net>
> > > ---
> > >   target/s390x/cpu.h         |   4 +-
> > >   target/s390x/helper.h      |   2 +-
> > >   target/s390x/insn-data.def |   4 +-
> > >   target/s390x/machine.c     |  19 +++++++
> > >   target/s390x/mem_helper.c  | 136 +++++++++++----------------------------------
> > >   target/s390x/translate.c   | 124 +++++++++++++++++++++++++----------------
> > >   6 files changed, 133 insertions(+), 156 deletions(-)
> > 
> > This looks good on the principle, and finally removes a big hack. That
> > said it prevent my test system to boot. I haven't investigated why yet.
> 
> Hmm.  I've not got a complete environment -- merely booting a kernel up to
> the point it fails to find a rootfs.  Which did find several problems with
> my first attempts at this, but wouldn't have exercised paging.  I'll try
> again to get a full install working...
> 
> I wonder if I needed to adjust s390_cpu_handle_mmu_fault (and its myriad
> subroutines) to handle setting ILEN correctly.
> 
> There might be a simpler fix though.  Currently I advance the PC and
> remember the ilen of the EX(RL).  Maybe better to *not* advance the PC so as
> to have the original EX(RL) right there for ILEN_LATER and ILEN_LATER_INC to
> operate on.
> 
> Something like this, as a delta patch.

Unfortunately it doesn't work. So far I have no real idea what could be
the root cause of the issue. I have just determined that up to the crash,
only a very limited set of instructions are being executed. They are the
4 bytes long versions of MVC, CLC, XC, TR.

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

Re: [Qemu-devel] [PATCH 02/31] target/s390x: Implement EXECUTE via new TranslationBlock

[Richard Henderson]

On 05/23/2017 10:28 AM, Aurelien Jarno wrote:
>> Something like this, as a delta patch.
> 
> Unfortunately it doesn't work. So far I have no real idea what could be
> the root cause of the issue. I have just determined that up to the crash,
> only a very limited set of instructions are being executed. They are the
> 4 bytes long versions of MVC, CLC, XC, TR.

Yeah, it appears XC is the culprit, though I have not yet determined exactly 
what's going wrong.

That said, perhaps I'll delay this for later and just add some extra helpers 
for now.  It does seem slightly wasteful to create a TB for at least these 
common cases.


r~

Re: [Qemu-devel] [PATCH 02/31] target/s390x: Implement EXECUTE via new TranslationBlock

[Aurelien Jarno]

On 2017-05-23 16:21, Richard Henderson wrote:
> On 05/23/2017 10:28 AM, Aurelien Jarno wrote:
> > > Something like this, as a delta patch.

I confirm this patch is really needed, otherwise the executed
instruction seems to be executed at the next instruction.

> > Unfortunately it doesn't work. So far I have no real idea what could be
> > the root cause of the issue. I have just determined that up to the crash,
> > only a very limited set of instructions are being executed. They are the
> > 4 bytes long versions of MVC, CLC, XC, TR.
> 
> Yeah, it appears XC is the culprit, though I have not yet determined exactly
> what's going wrong.

It seems the problem arise if an interrupt happens when the TB
containing the EXECUTE instruction is being executed. In that case at
the end of the TB, the interruption code is translated with the ex_value
set, which means with the wrong PC, wrong permissions and wrong return
address.

This is the same kind of issue I identified on SH4 recently:
https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03880.html

The same king of solution also works, that is disabling the interrupts
when the ex_value is set:

diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 6f81b1a16c..a33abdef16 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -655,6 +657,10 @@ bool s390_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
         S390CPU *cpu = S390_CPU(cs);
         CPUS390XState *env = &cpu->env;
 
+        if (env->ex_value) {
+            return false;
+        }
+
         if (env->psw.mask & PSW_MASK_EXT) {
             s390_cpu_do_interrupt(cs);
             return true;

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

Re: [Qemu-devel] [PATCH 02/31] target/s390x: Implement EXECUTE via new TranslationBlock

[Richard Henderson]

On 05/24/2017 10:54 AM, Aurelien Jarno wrote:
> It seems the problem arise if an interrupt happens when the TB
> containing the EXECUTE instruction is being executed. In that case at
> the end of the TB, the interruption code is translated with the ex_value
> set, which means with the wrong PC, wrong permissions and wrong return
> address.
> 
> This is the same kind of issue I identified on SH4 recently:
> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03880.html
> 
> The same king of solution also works, that is disabling the interrupts
> when the ex_value is set:
> 
> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> index 6f81b1a16c..a33abdef16 100644
> --- a/target/s390x/helper.c
> +++ b/target/s390x/helper.c
> @@ -655,6 +657,10 @@ bool s390_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>           S390CPU *cpu = S390_CPU(cs);
>           CPUS390XState *env = &cpu->env;
>   
> +        if (env->ex_value) {
> +            return false;
> +        }
> +
>           if (env->psw.mask & PSW_MASK_EXT) {
>               s390_cpu_do_interrupt(cs);
>               return true;
> 

Thanks for the research.  I've incorporated this into my patch set.


r~

Re: [Qemu-devel] [PATCH 02/31] target/s390x: Implement EXECUTE via new TranslationBlock

[Thomas Huth]

On 23.05.2017 17:54, Richard Henderson wrote:
> On 05/23/2017 03:48 AM, Aurelien Jarno wrote:
>> On 2017-05-22 20:02, Richard Henderson wrote:
>>> Previously, helper_ex would construct the insn and then implement
>>> the insn via direct calls other helpers.  This was sufficient to
>>> boot Linux but that is all.
>>>
>>> It is easy enough to go the whole nine yards by stashing state for
>>> EXECUTE within the cpu, and then relying on a new TB to be created
>>> that properly and completely interprets the insn.
>>>
>>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>>> ---
>>>   target/s390x/cpu.h         |   4 +-
>>>   target/s390x/helper.h      |   2 +-
>>>   target/s390x/insn-data.def |   4 +-
>>>   target/s390x/machine.c     |  19 +++++++
>>>   target/s390x/mem_helper.c  | 136
>>> +++++++++++----------------------------------
>>>   target/s390x/translate.c   | 124
>>> +++++++++++++++++++++++++----------------
>>>   6 files changed, 133 insertions(+), 156 deletions(-)
>>
>> This looks good on the principle, and finally removes a big hack. That
>> said it prevent my test system to boot. I haven't investigated why yet.
> 
> Hmm.  I've not got a complete environment -- merely booting a kernel up
> to the point it fails to find a rootfs.  Which did find several problems
> with my first attempts at this, but wouldn't have exercised paging. 
> I'll try again to get a full install working...

Something nice for a quick test is also:

http://www.qemu-advent-calendar.org/2014/download/s390-moon-buggy.tar.xz

Not sure whether it will trigger your EXECUTE problem, though.

 Thomas

Re: [Qemu-devel] [PATCH 02/31] target/s390x: Implement EXECUTE via new TranslationBlock

[Aurelien Jarno]

On 2017-05-23 12:48, Aurelien Jarno wrote:
> On 2017-05-22 20:02, Richard Henderson wrote:
> > Previously, helper_ex would construct the insn and then implement
> > the insn via direct calls other helpers.  This was sufficient to
> > boot Linux but that is all.
> > 
> > It is easy enough to go the whole nine yards by stashing state for
> > EXECUTE within the cpu, and then relying on a new TB to be created
> > that properly and completely interprets the insn.
> > 
> > Signed-off-by: Richard Henderson <rth@twiddle.net>
> > ---
> >  target/s390x/cpu.h         |   4 +-
> >  target/s390x/helper.h      |   2 +-
> >  target/s390x/insn-data.def |   4 +-
> >  target/s390x/machine.c     |  19 +++++++
> >  target/s390x/mem_helper.c  | 136 +++++++++++----------------------------------
> >  target/s390x/translate.c   | 124 +++++++++++++++++++++++++----------------
> >  6 files changed, 133 insertions(+), 156 deletions(-)
> 
> This looks good on the principle, and finally removes a big hack. That
> said it prevent my test system to boot. I haven't investigated why yet.

This can aslo be reproduced using the kernel and initrd from the daily
Debian installer:

  https://d-i.debian.org/daily-images/s390x/daily/generic/

I am personally using the following command line:

  qemu-system-s390x -M s390-ccw-virtio -m 512 -nographic -kernel kernel.debian -initrd initrd.debian

Aurelien

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

[Qemu-devel] [PATCH 03/31] target/s390x: Use unwind data for helper_nc

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 18 +++++++++---------
 target/s390x/translate.c  |  1 -
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index db80d53..1018fe0 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -122,20 +122,20 @@ static void fast_memmove(CPUS390XState *env, uint64_t dest, uint64_t src,
 uint32_t HELPER(nc)(CPUS390XState *env, uint32_t l, uint64_t dest,
                     uint64_t src)
 {
-    int i;
-    unsigned char x;
-    uint32_t cc = 0;
+    uintptr_t ra = GETPC();
+    uint8_t cc = 0;
+    uint32_t i;
 
     HELPER_LOG("%s l %d dest %" PRIx64 " src %" PRIx64 "\n",
                __func__, l, dest, src);
+
     for (i = 0; i <= l; i++) {
-        x = cpu_ldub_data(env, dest + i) & cpu_ldub_data(env, src + i);
-        if (x) {
-            cc = 1;
-        }
-        cpu_stb_data(env, dest + i, x);
+        uint8_t x = cpu_ldub_data_ra(env, dest + i, ra);
+        x &= cpu_ldub_data_ra(env, src + i, ra);
+        cc |= x;
+        cpu_stb_data_ra(env, dest + i, x, ra);
     }
-    return cc;
+    return cc != 0;
 }
 
 /* xor on array */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 3a72c38..6aa9c90 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3048,7 +3048,6 @@ static ExitStatus op_nabsf128(DisasContext *s, DisasOps *o)
 static ExitStatus op_nc(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
-    potential_page_fault(s);
     gen_helper_nc(cc_op, cpu_env, l, o->addr1, o->in2);
     tcg_temp_free_i32(l);
     set_cc_static(s);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 03/31] target/s390x: Use unwind data for helper_nc

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 18 +++++++++---------
>  target/s390x/translate.c  |  1 -
>  2 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index db80d53..1018fe0 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -122,20 +122,20 @@ static void fast_memmove(CPUS390XState *env, uint64_t dest, uint64_t src,
>  uint32_t HELPER(nc)(CPUS390XState *env, uint32_t l, uint64_t dest,
>                      uint64_t src)
>  {
> -    int i;
> -    unsigned char x;
> -    uint32_t cc = 0;
> +    uintptr_t ra = GETPC();
> +    uint8_t cc = 0;
> +    uint32_t i;
>  
>      HELPER_LOG("%s l %d dest %" PRIx64 " src %" PRIx64 "\n",
>                 __func__, l, dest, src);
> +
>      for (i = 0; i <= l; i++) {
> -        x = cpu_ldub_data(env, dest + i) & cpu_ldub_data(env, src + i);
> -        if (x) {
> -            cc = 1;
> -        }
> -        cpu_stb_data(env, dest + i, x);
> +        uint8_t x = cpu_ldub_data_ra(env, dest + i, ra);
> +        x &= cpu_ldub_data_ra(env, src + i, ra);

I'd maybe do the access to src first, before  the access to dest ...
though I did not find anything in the PoP that really indicates that
access exceptions for src would have a higher priority here... so it
likely does not really matter...

> +        cc |= x;
> +        cpu_stb_data_ra(env, dest + i, x, ra);
>      }
> -    return cc;
> +    return cc != 0;
>  }
>  
>  /* xor on array */
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 3a72c38..6aa9c90 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -3048,7 +3048,6 @@ static ExitStatus op_nabsf128(DisasContext *s, DisasOps *o)
>  static ExitStatus op_nc(DisasContext *s, DisasOps *o)
>  {
>      TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
> -    potential_page_fault(s);
>      gen_helper_nc(cc_op, cpu_env, l, o->addr1, o->in2);
>      tcg_temp_free_i32(l);
>      set_cc_static(s);
> 

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

Re: [Qemu-devel] [PATCH 03/31] target/s390x: Use unwind data for helper_nc

[Aurelien Jarno]

On 2017-05-23 10:33, Thomas Huth wrote:
> On 23.05.2017 05:02, Richard Henderson wrote:
> > Signed-off-by: Richard Henderson <rth@twiddle.net>
> > ---
> >  target/s390x/mem_helper.c | 18 +++++++++---------
> >  target/s390x/translate.c  |  1 -
> >  2 files changed, 9 insertions(+), 10 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 04/31] target/s390x: Use unwind data for helper_xc

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 25 ++++++++++++-------------
 target/s390x/translate.c  |  1 -
 2 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 1018fe0..e0a6fad 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -68,7 +68,7 @@ static inline uint64_t adj_len_to_page(uint64_t len, uint64_t addr)
 }
 
 static void fast_memset(CPUS390XState *env, uint64_t dest, uint8_t byte,
-                        uint32_t l)
+                        uint32_t l, uintptr_t ra)
 {
     int mmu_idx = cpu_mmu_index(env, false);
 
@@ -83,7 +83,7 @@ static void fast_memset(CPUS390XState *env, uint64_t dest, uint8_t byte,
         } else {
             /* We failed to get access to the whole page. The next write
                access will likely fill the QEMU TLB for the next iteration.  */
-            cpu_stb_data(env, dest, byte);
+            cpu_stb_data_ra(env, dest, byte, ra);
             dest++;
             l--;
         }
@@ -142,27 +142,26 @@ uint32_t HELPER(nc)(CPUS390XState *env, uint32_t l, uint64_t dest,
 uint32_t HELPER(xc)(CPUS390XState *env, uint32_t l, uint64_t dest,
                     uint64_t src)
 {
-    int i;
-    unsigned char x;
-    uint32_t cc = 0;
+    uintptr_t ra = GETPC();
+    uint8_t cc = 0;
+    uint32_t i;
 
     HELPER_LOG("%s l %d dest %" PRIx64 " src %" PRIx64 "\n",
                __func__, l, dest, src);
 
     /* xor with itself is the same as memset(0) */
     if (src == dest) {
-        fast_memset(env, dest, 0, l + 1);
+        fast_memset(env, dest, 0, l + 1, ra);
         return 0;
     }
 
     for (i = 0; i <= l; i++) {
-        x = cpu_ldub_data(env, dest + i) ^ cpu_ldub_data(env, src + i);
-        if (x) {
-            cc = 1;
-        }
-        cpu_stb_data(env, dest + i, x);
+        uint8_t x = cpu_ldub_data_ra(env, dest + i, ra);
+        x ^= cpu_ldub_data_ra(env, src + i, ra);
+        cc |= x;
+        cpu_stb_data_ra(env, dest + i, x, ra);
     }
-    return cc;
+    return cc != 0;
 }
 
 /* or on array */
@@ -196,7 +195,7 @@ void HELPER(mvc)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
     /* mvc with source pointing to the byte after the destination is the
        same as memset with the first source byte */
     if (dest == (src + 1)) {
-        fast_memset(env, dest, cpu_ldub_data(env, src), l + 1);
+        fast_memset(env, dest, cpu_ldub_data(env, src), l + 1, 0);
         return;
     }
 
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 6aa9c90..a770407 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4166,7 +4166,6 @@ static ExitStatus op_xc(DisasContext *s, DisasOps *o)
     /* But in general we'll defer to a helper.  */
     o->in2 = get_address(s, 0, b2, d2);
     t32 = tcg_const_i32(l);
-    potential_page_fault(s);
     gen_helper_xc(cc_op, cpu_env, t32, o->addr1, o->in2);
     tcg_temp_free_i32(t32);
     set_cc_static(s);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 04/31] target/s390x: Use unwind data for helper_xc

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 25 ++++++++++++-------------
>  target/s390x/translate.c  |  1 -
>  2 files changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 1018fe0..e0a6fad 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -68,7 +68,7 @@ static inline uint64_t adj_len_to_page(uint64_t len, uint64_t addr)
>  }
>  
>  static void fast_memset(CPUS390XState *env, uint64_t dest, uint8_t byte,
> -                        uint32_t l)
> +                        uint32_t l, uintptr_t ra)
>  {
>      int mmu_idx = cpu_mmu_index(env, false);
>  
> @@ -83,7 +83,7 @@ static void fast_memset(CPUS390XState *env, uint64_t dest, uint8_t byte,
>          } else {
>              /* We failed to get access to the whole page. The next write
>                 access will likely fill the QEMU TLB for the next iteration.  */
> -            cpu_stb_data(env, dest, byte);
> +            cpu_stb_data_ra(env, dest, byte, ra);
>              dest++;
>              l--;
>          }
> @@ -142,27 +142,26 @@ uint32_t HELPER(nc)(CPUS390XState *env, uint32_t l, uint64_t dest,
>  uint32_t HELPER(xc)(CPUS390XState *env, uint32_t l, uint64_t dest,
>                      uint64_t src)
>  {
> -    int i;
> -    unsigned char x;
> -    uint32_t cc = 0;
> +    uintptr_t ra = GETPC();
> +    uint8_t cc = 0;
> +    uint32_t i;
>  
>      HELPER_LOG("%s l %d dest %" PRIx64 " src %" PRIx64 "\n",
>                 __func__, l, dest, src);
>  
>      /* xor with itself is the same as memset(0) */
>      if (src == dest) {
> -        fast_memset(env, dest, 0, l + 1);
> +        fast_memset(env, dest, 0, l + 1, ra);
>          return 0;
>      }
>  
>      for (i = 0; i <= l; i++) {
> -        x = cpu_ldub_data(env, dest + i) ^ cpu_ldub_data(env, src + i);
> -        if (x) {
> -            cc = 1;
> -        }
> -        cpu_stb_data(env, dest + i, x);
> +        uint8_t x = cpu_ldub_data_ra(env, dest + i, ra);
> +        x ^= cpu_ldub_data_ra(env, src + i, ra);

Maybe access here src first, too?

> +        cc |= x;
> +        cpu_stb_data_ra(env, dest + i, x, ra);
>      }
> -    return cc;
> +    return cc != 0;
>  }
>  
>  /* or on array */
> @@ -196,7 +195,7 @@ void HELPER(mvc)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
>      /* mvc with source pointing to the byte after the destination is the
>         same as memset with the first source byte */
>      if (dest == (src + 1)) {
> -        fast_memset(env, dest, cpu_ldub_data(env, src), l + 1);
> +        fast_memset(env, dest, cpu_ldub_data(env, src), l + 1, 0);
>          return;
>      }
>  
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 6aa9c90..a770407 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -4166,7 +4166,6 @@ static ExitStatus op_xc(DisasContext *s, DisasOps *o)
>      /* But in general we'll defer to a helper.  */
>      o->in2 = get_address(s, 0, b2, d2);
>      t32 = tcg_const_i32(l);
> -    potential_page_fault(s);
>      gen_helper_xc(cc_op, cpu_env, t32, o->addr1, o->in2);
>      tcg_temp_free_i32(t32);
>      set_cc_static(s);

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

Re: [Qemu-devel] [PATCH 04/31] target/s390x: Use unwind data for helper_xc

[Aurelien Jarno]

On 2017-05-23 10:56, Thomas Huth wrote:
> On 23.05.2017 05:02, Richard Henderson wrote:
> > Signed-off-by: Richard Henderson <rth@twiddle.net>
> > ---
> >  target/s390x/mem_helper.c | 25 ++++++++++++-------------
> >  target/s390x/translate.c  |  1 -
> >  2 files changed, 12 insertions(+), 14 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 05/31] target/s390x: Use unwind data for helper_oc

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 18 +++++++++---------
 target/s390x/translate.c  |  1 -
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index e0a6fad..e75c2de0 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -168,20 +168,20 @@ uint32_t HELPER(xc)(CPUS390XState *env, uint32_t l, uint64_t dest,
 uint32_t HELPER(oc)(CPUS390XState *env, uint32_t l, uint64_t dest,
                     uint64_t src)
 {
-    int i;
-    unsigned char x;
-    uint32_t cc = 0;
+    uintptr_t ra = GETPC();
+    uint8_t cc = 0;
+    uint32_t i;
 
     HELPER_LOG("%s l %d dest %" PRIx64 " src %" PRIx64 "\n",
                __func__, l, dest, src);
+
     for (i = 0; i <= l; i++) {
-        x = cpu_ldub_data(env, dest + i) | cpu_ldub_data(env, src + i);
-        if (x) {
-            cc = 1;
-        }
-        cpu_stb_data(env, dest + i, x);
+        uint8_t x = cpu_ldub_data_ra(env, dest + i, ra);
+        x |= cpu_ldub_data_ra(env, src + i, ra);
+        cc |= x;
+        cpu_stb_data_ra(env, dest + i, x, ra);
     }
-    return cc;
+    return cc != 0;
 }
 
 /* memmove */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index a770407..6a51d56 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3082,7 +3082,6 @@ static ExitStatus op_negf128(DisasContext *s, DisasOps *o)
 static ExitStatus op_oc(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
-    potential_page_fault(s);
     gen_helper_oc(cc_op, cpu_env, l, o->addr1, o->in2);
     tcg_temp_free_i32(l);
     set_cc_static(s);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 05/31] target/s390x: Use unwind data for helper_oc

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 18 +++++++++---------
>  target/s390x/translate.c  |  1 -
>  2 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index e0a6fad..e75c2de0 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -168,20 +168,20 @@ uint32_t HELPER(xc)(CPUS390XState *env, uint32_t l, uint64_t dest,
>  uint32_t HELPER(oc)(CPUS390XState *env, uint32_t l, uint64_t dest,
>                      uint64_t src)
>  {
> -    int i;
> -    unsigned char x;
> -    uint32_t cc = 0;
> +    uintptr_t ra = GETPC();
> +    uint8_t cc = 0;
> +    uint32_t i;
>  
>      HELPER_LOG("%s l %d dest %" PRIx64 " src %" PRIx64 "\n",
>                 __func__, l, dest, src);
> +
>      for (i = 0; i <= l; i++) {
> -        x = cpu_ldub_data(env, dest + i) | cpu_ldub_data(env, src + i);
> -        if (x) {
> -            cc = 1;
> -        }
> -        cpu_stb_data(env, dest + i, x);
> +        uint8_t x = cpu_ldub_data_ra(env, dest + i, ra);
> +        x |= cpu_ldub_data_ra(env, src + i, ra);

Maybe access to src first?

> +        cc |= x;
> +        cpu_stb_data_ra(env, dest + i, x, ra);
>      }
> -    return cc;
> +    return cc != 0;
>  }
>  
>  /* memmove */
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index a770407..6a51d56 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -3082,7 +3082,6 @@ static ExitStatus op_negf128(DisasContext *s, DisasOps *o)
>  static ExitStatus op_oc(DisasContext *s, DisasOps *o)
>  {
>      TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
> -    potential_page_fault(s);
>      gen_helper_oc(cc_op, cpu_env, l, o->addr1, o->in2);
>      tcg_temp_free_i32(l);
>      set_cc_static(s);
> 

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

Re: [Qemu-devel] [PATCH 05/31] target/s390x: Use unwind data for helper_oc

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 18 +++++++++---------
>  target/s390x/translate.c  |  1 -
>  2 files changed, 9 insertions(+), 10 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

Re: [Qemu-devel] [PATCH 05/31] target/s390x: Use unwind data for helper_oc

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 18 +++++++++---------
>  target/s390x/translate.c  |  1 -
>  2 files changed, 9 insertions(+), 10 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 06/31] target/s390x: Use unwind data for helper_mvc

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 19 ++++++++++---------
 target/s390x/translate.c  |  1 -
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index e75c2de0..0295485 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -91,7 +91,7 @@ static void fast_memset(CPUS390XState *env, uint64_t dest, uint8_t byte,
 }
 
 static void fast_memmove(CPUS390XState *env, uint64_t dest, uint64_t src,
-                         uint32_t l)
+                         uint32_t l, uintptr_t ra)
 {
     int mmu_idx = cpu_mmu_index(env, false);
 
@@ -110,7 +110,7 @@ static void fast_memmove(CPUS390XState *env, uint64_t dest, uint64_t src,
             /* We failed to get access to one or both whole pages. The next
                read or write access will likely fill the QEMU TLB for the
                next iteration.  */
-            cpu_stb_data(env, dest, cpu_ldub_data(env, src));
+            cpu_stb_data_ra(env, dest, cpu_ldub_data_ra(env, src, ra), ra);
             src++;
             dest++;
             l--;
@@ -187,27 +187,28 @@ uint32_t HELPER(oc)(CPUS390XState *env, uint32_t l, uint64_t dest,
 /* memmove */
 void HELPER(mvc)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
 {
-    int i = 0;
+    uintptr_t ra = GETPC();
+    uint32_t i;
 
     HELPER_LOG("%s l %d dest %" PRIx64 " src %" PRIx64 "\n",
                __func__, l, dest, src);
 
     /* mvc with source pointing to the byte after the destination is the
        same as memset with the first source byte */
-    if (dest == (src + 1)) {
-        fast_memset(env, dest, cpu_ldub_data(env, src), l + 1, 0);
+    if (dest == src + 1) {
+        fast_memset(env, dest, cpu_ldub_data_ra(env, src, ra), l + 1, ra);
         return;
     }
 
     /* mvc and memmove do not behave the same when areas overlap! */
-    if ((dest < src) || (src + l < dest)) {
-        fast_memmove(env, dest, src, l + 1);
+    if (dest < src || src + l < dest) {
+        fast_memmove(env, dest, src, l + 1, ra);
         return;
     }
 
     /* slow version with byte accesses which always work */
     for (i = 0; i <= l; i++) {
-        cpu_stb_data(env, dest + i, cpu_ldub_data(env, src + i));
+        cpu_stb_data_ra(env, dest + i, cpu_ldub_data_ra(env, src + i, ra), ra);
     }
 }
 
@@ -373,7 +374,7 @@ void HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t r2)
 {
     /* XXX missing r0 handling */
     env->cc_op = 0;
-    fast_memmove(env, r1, r2, TARGET_PAGE_SIZE);
+    fast_memmove(env, r1, r2, TARGET_PAGE_SIZE, 0);
 }
 
 /* string copy (c is string terminator) */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 6a51d56..66f3cd6 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2871,7 +2871,6 @@ static ExitStatus op_movx(DisasContext *s, DisasOps *o)
 static ExitStatus op_mvc(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
-    potential_page_fault(s);
     gen_helper_mvc(cpu_env, l, o->addr1, o->in2);
     tcg_temp_free_i32(l);
     return NO_EXIT;
-- 
2.9.4

Re: [Qemu-devel] [PATCH 06/31] target/s390x: Use unwind data for helper_mvc

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 19 ++++++++++---------
>  target/s390x/translate.c  |  1 -
>  2 files changed, 10 insertions(+), 10 deletions(-)

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

Re: [Qemu-devel] [PATCH 06/31] target/s390x: Use unwind data for helper_mvc

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 19 ++++++++++---------
>  target/s390x/translate.c  |  1 -
>  2 files changed, 10 insertions(+), 10 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 07/31] target/s390x: Use unwind data for helper_clc

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 18 +++++++++---------
 target/s390x/translate.c  |  1 -
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 0295485..d04850b 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -215,26 +215,26 @@ void HELPER(mvc)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
 /* compare unsigned byte arrays */
 uint32_t HELPER(clc)(CPUS390XState *env, uint32_t l, uint64_t s1, uint64_t s2)
 {
-    int i;
-    unsigned char x, y;
-    uint32_t cc;
+    uintptr_t ra = GETPC();
+    uint32_t cc = 0;
+    uint32_t i;
 
     HELPER_LOG("%s l %d s1 %" PRIx64 " s2 %" PRIx64 "\n",
                __func__, l, s1, s2);
+
     for (i = 0; i <= l; i++) {
-        x = cpu_ldub_data(env, s1 + i);
-        y = cpu_ldub_data(env, s2 + i);
+        uint8_t x = cpu_ldub_data_ra(env, s1 + i, ra);
+        uint8_t y = cpu_ldub_data_ra(env, s2 + i, ra);
         HELPER_LOG("%02x (%c)/%02x (%c) ", x, x, y, y);
         if (x < y) {
             cc = 1;
-            goto done;
+            break;
         } else if (x > y) {
             cc = 2;
-            goto done;
+            break;
         }
     }
-    cc = 0;
- done:
+
     HELPER_LOG("\n");
     return cc;
 }
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 66f3cd6..aefbc90 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1903,7 +1903,6 @@ static ExitStatus op_clc(DisasContext *s, DisasOps *o)
         tcg_gen_qemu_ld64(cc_dst, o->in2, get_mem_index(s));
         break;
     default:
-        potential_page_fault(s);
         vl = tcg_const_i32(l);
         gen_helper_clc(cc_op, cpu_env, vl, o->addr1, o->in2);
         tcg_temp_free_i32(vl);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 07/31] target/s390x: Use unwind data for helper_clc

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 18 +++++++++---------
>  target/s390x/translate.c  |  1 -
>  2 files changed, 9 insertions(+), 10 deletions(-)

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

Re: [Qemu-devel] [PATCH 07/31] target/s390x: Use unwind data for helper_clc

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 18 +++++++++---------
>  target/s390x/translate.c  |  1 -
>  2 files changed, 9 insertions(+), 10 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 08/31] target/s390x: Use unwind data for helper_clm

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 11 ++++++-----
 target/s390x/translate.c  |  1 -
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index d04850b..5f38ac3 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -243,16 +243,16 @@ uint32_t HELPER(clc)(CPUS390XState *env, uint32_t l, uint64_t s1, uint64_t s2)
 uint32_t HELPER(clm)(CPUS390XState *env, uint32_t r1, uint32_t mask,
                      uint64_t addr)
 {
-    uint8_t r, d;
-    uint32_t cc;
+    uintptr_t ra = GETPC();
+    uint32_t cc = 0;
 
     HELPER_LOG("%s: r1 0x%x mask 0x%x addr 0x%" PRIx64 "\n", __func__, r1,
                mask, addr);
-    cc = 0;
+
     while (mask) {
         if (mask & 8) {
-            d = cpu_ldub_data(env, addr);
-            r = (r1 & 0xff000000UL) >> 24;
+            uint8_t d = cpu_ldub_data_ra(env, addr, ra);
+            uint8_t r = extract32(r1, 24, 8);
             HELPER_LOG("mask 0x%x %02x/%02x (0x%" PRIx64 ") ", mask, r, d,
                        addr);
             if (r < d) {
@@ -267,6 +267,7 @@ uint32_t HELPER(clm)(CPUS390XState *env, uint32_t r1, uint32_t mask,
         mask = (mask << 1) & 0xf;
         r1 <<= 8;
     }
+
     HELPER_LOG("\n");
     return cc;
 }
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index aefbc90..84f09b1 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1930,7 +1930,6 @@ static ExitStatus op_clm(DisasContext *s, DisasOps *o)
     TCGv_i32 m3 = tcg_const_i32(get_field(s->fields, m3));
     TCGv_i32 t1 = tcg_temp_new_i32();
     tcg_gen_extrl_i64_i32(t1, o->in1);
-    potential_page_fault(s);
     gen_helper_clm(cc_op, cpu_env, t1, m3, o->in2);
     set_cc_static(s);
     tcg_temp_free_i32(t1);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 08/31] target/s390x: Use unwind data for helper_clm

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 11 ++++++-----
>  target/s390x/translate.c  |  1 -
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index d04850b..5f38ac3 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -243,16 +243,16 @@ uint32_t HELPER(clc)(CPUS390XState *env, uint32_t l, uint64_t s1, uint64_t s2)
>  uint32_t HELPER(clm)(CPUS390XState *env, uint32_t r1, uint32_t mask,
>                       uint64_t addr)
>  {
> -    uint8_t r, d;
> -    uint32_t cc;
> +    uintptr_t ra = GETPC();
> +    uint32_t cc = 0;
>  
>      HELPER_LOG("%s: r1 0x%x mask 0x%x addr 0x%" PRIx64 "\n", __func__, r1,
>                 mask, addr);
> -    cc = 0;
> +
>      while (mask) {
>          if (mask & 8) {
> -            d = cpu_ldub_data(env, addr);
> -            r = (r1 & 0xff000000UL) >> 24;
> +            uint8_t d = cpu_ldub_data_ra(env, addr, ra);
> +            uint8_t r = extract32(r1, 24, 8);
>              HELPER_LOG("mask 0x%x %02x/%02x (0x%" PRIx64 ") ", mask, r, d,
>                         addr);
>              if (r < d) {
> @@ -267,6 +267,7 @@ uint32_t HELPER(clm)(CPUS390XState *env, uint32_t r1, uint32_t mask,
>          mask = (mask << 1) & 0xf;
>          r1 <<= 8;
>      }
> +
>      HELPER_LOG("\n");
>      return cc;
>  }
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index aefbc90..84f09b1 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -1930,7 +1930,6 @@ static ExitStatus op_clm(DisasContext *s, DisasOps *o)
>      TCGv_i32 m3 = tcg_const_i32(get_field(s->fields, m3));
>      TCGv_i32 t1 = tcg_temp_new_i32();
>      tcg_gen_extrl_i64_i32(t1, o->in1);
> -    potential_page_fault(s);
>      gen_helper_clm(cc_op, cpu_env, t1, m3, o->in2);
>      set_cc_static(s);
>      tcg_temp_free_i32(t1);
> 

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

Re: [Qemu-devel] [PATCH 08/31] target/s390x: Use unwind data for helper_clm

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 11 ++++++-----
>  target/s390x/translate.c  |  1 -
>  2 files changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 09/31] target/s390x: Use unwind data for helper_srst

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 3 ++-
 target/s390x/translate.c  | 1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 5f38ac3..3c28f3a 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -302,6 +302,7 @@ static inline uint64_t get_address_31fix(CPUS390XState *env, int reg)
 uint64_t HELPER(srst)(CPUS390XState *env, uint64_t r0, uint64_t end,
                       uint64_t str)
 {
+    uintptr_t ra = GETPC();
     uint32_t len;
     uint8_t v, c = r0;
 
@@ -319,7 +320,7 @@ uint64_t HELPER(srst)(CPUS390XState *env, uint64_t r0, uint64_t end,
             env->cc_op = 2;
             return end;
         }
-        v = cpu_ldub_data(env, str + len);
+        v = cpu_ldub_data_ra(env, str + len, ra);
         if (v == c) {
             /* Character found.  Set R1 to the location; R2 is unmodified.  */
             env->cc_op = 1;
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 84f09b1..ba7d0f9 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3977,7 +3977,6 @@ static ExitStatus op_stmh(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_srst(DisasContext *s, DisasOps *o)
 {
-    potential_page_fault(s);
     gen_helper_srst(o->in1, cpu_env, regs[0], o->in1, o->in2);
     set_cc_static(s);
     return_low128(o->in2);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 09/31] target/s390x: Use unwind data for helper_srst

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 3 ++-
>  target/s390x/translate.c  | 1 -
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 5f38ac3..3c28f3a 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -302,6 +302,7 @@ static inline uint64_t get_address_31fix(CPUS390XState *env, int reg)
>  uint64_t HELPER(srst)(CPUS390XState *env, uint64_t r0, uint64_t end,
>                        uint64_t str)
>  {
> +    uintptr_t ra = GETPC();
>      uint32_t len;
>      uint8_t v, c = r0;
>  
> @@ -319,7 +320,7 @@ uint64_t HELPER(srst)(CPUS390XState *env, uint64_t r0, uint64_t end,
>              env->cc_op = 2;
>              return end;
>          }
> -        v = cpu_ldub_data(env, str + len);
> +        v = cpu_ldub_data_ra(env, str + len, ra);
>          if (v == c) {
>              /* Character found.  Set R1 to the location; R2 is unmodified.  */
>              env->cc_op = 1;
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 84f09b1..ba7d0f9 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -3977,7 +3977,6 @@ static ExitStatus op_stmh(DisasContext *s, DisasOps *o)
>  
>  static ExitStatus op_srst(DisasContext *s, DisasOps *o)
>  {
> -    potential_page_fault(s);
>      gen_helper_srst(o->in1, cpu_env, regs[0], o->in1, o->in2);
>      set_cc_static(s);
>      return_low128(o->in2);
> 

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

Re: [Qemu-devel] [PATCH 09/31] target/s390x: Use unwind data for helper_srst

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 3 ++-
>  target/s390x/translate.c  | 1 -
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 10/31] target/s390x: Use unwind data for helper_clst

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 5 +++--
 target/s390x/translate.c  | 1 -
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 3c28f3a..7c9e7c7 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -337,6 +337,7 @@ uint64_t HELPER(srst)(CPUS390XState *env, uint64_t r0, uint64_t end,
 /* unsigned string compare (c is string terminator) */
 uint64_t HELPER(clst)(CPUS390XState *env, uint64_t c, uint64_t s1, uint64_t s2)
 {
+    uintptr_t ra = GETPC();
     uint32_t len;
 
     c = c & 0xff;
@@ -346,8 +347,8 @@ uint64_t HELPER(clst)(CPUS390XState *env, uint64_t c, uint64_t s1, uint64_t 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.  */
     for (len = 0; len < 0x2000; ++len) {
-        uint8_t v1 = cpu_ldub_data(env, s1 + len);
-        uint8_t v2 = cpu_ldub_data(env, s2 + len);
+        uint8_t v1 = cpu_ldub_data_ra(env, s1 + len, ra);
+        uint8_t v2 = cpu_ldub_data_ra(env, s2 + len, ra);
         if (v1 == v2) {
             if (v1 == c) {
                 /* Equal.  CC=0, and don't advance the registers.  */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index ba7d0f9..735aa82 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1939,7 +1939,6 @@ static ExitStatus op_clm(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_clst(DisasContext *s, DisasOps *o)
 {
-    potential_page_fault(s);
     gen_helper_clst(o->in1, cpu_env, regs[0], o->in1, o->in2);
     set_cc_static(s);
     return_low128(o->in2);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 10/31] target/s390x: Use unwind data for helper_clst

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 5 +++--
>  target/s390x/translate.c  | 1 -
>  2 files changed, 3 insertions(+), 3 deletions(-)

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

Re: [Qemu-devel] [PATCH 10/31] target/s390x: Use unwind data for helper_clst

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 5 +++--
>  target/s390x/translate.c  | 1 -
>  2 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 11/31] target/s390x: Use unwind data for helper_mvpg

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/helper.h     | 2 +-
 target/s390x/mem_helper.c | 9 +++++----
 target/s390x/translate.c  | 3 +--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index d6cc513..2b4e7be 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -12,7 +12,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_4(mvpg, void, env, i64, 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_FLAGS_4(ex, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(stam, TCG_CALL_NO_WG, void, env, i32, i64, i32)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 7c9e7c7..9ef9f4a 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -373,11 +373,12 @@ uint64_t HELPER(clst)(CPUS390XState *env, uint64_t c, uint64_t s1, uint64_t s2)
 }
 
 /* move page */
-void HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t r2)
+uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t r2)
 {
-    /* XXX missing r0 handling */
-    env->cc_op = 0;
-    fast_memmove(env, r1, r2, TARGET_PAGE_SIZE, 0);
+    /* ??? missing r0 handling, which includes access keys, but more
+       importantly optional suppression of the exception!  */
+    fast_memmove(env, r1, r2, TARGET_PAGE_SIZE, GETPC());
+    return 0; /* data moved */
 }
 
 /* string copy (c is string terminator) */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 735aa82..a1edc79 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2921,8 +2921,7 @@ static ExitStatus op_mvcs(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_mvpg(DisasContext *s, DisasOps *o)
 {
-    potential_page_fault(s);
-    gen_helper_mvpg(cpu_env, regs[0], o->in1, o->in2);
+    gen_helper_mvpg(cc_op, cpu_env, regs[0], o->in1, o->in2);
     set_cc_static(s);
     return NO_EXIT;
 }
-- 
2.9.4

Re: [Qemu-devel] [PATCH 11/31] target/s390x: Use unwind data for helper_mvpg

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/helper.h     | 2 +-
>  target/s390x/mem_helper.c | 9 +++++----
>  target/s390x/translate.c  | 3 +--
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index d6cc513..2b4e7be 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -12,7 +12,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_4(mvpg, void, env, i64, 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_FLAGS_4(ex, TCG_CALL_NO_WG, void, env, i32, i64, i64)
>  DEF_HELPER_FLAGS_4(stam, TCG_CALL_NO_WG, void, env, i32, i64, i32)
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 7c9e7c7..9ef9f4a 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -373,11 +373,12 @@ uint64_t HELPER(clst)(CPUS390XState *env, uint64_t c, uint64_t s1, uint64_t s2)
>  }
>  
>  /* move page */
> -void HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t r2)
> +uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t r2)
>  {
> -    /* XXX missing r0 handling */
> -    env->cc_op = 0;
> -    fast_memmove(env, r1, r2, TARGET_PAGE_SIZE, 0);
> +    /* ??? missing r0 handling, which includes access keys, but more
> +       importantly optional suppression of the exception!  */
> +    fast_memmove(env, r1, r2, TARGET_PAGE_SIZE, GETPC());
> +    return 0; /* data moved */
>  }
>  
>  /* string copy (c is string terminator) */
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 735aa82..a1edc79 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -2921,8 +2921,7 @@ static ExitStatus op_mvcs(DisasContext *s, DisasOps *o)
>  
>  static ExitStatus op_mvpg(DisasContext *s, DisasOps *o)
>  {
> -    potential_page_fault(s);
> -    gen_helper_mvpg(cpu_env, regs[0], o->in1, o->in2);
> +    gen_helper_mvpg(cc_op, cpu_env, regs[0], o->in1, o->in2);
>      set_cc_static(s);
>      return NO_EXIT;
>  }

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

(but I'd maybe move this patch right after the helper_mvc patch in your
series, since it uses fast_memmove, too, so it's easier to review this
patch here in the right context)

Re: [Qemu-devel] [PATCH 11/31] target/s390x: Use unwind data for helper_mvpg

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/helper.h     | 2 +-
>  target/s390x/mem_helper.c | 9 +++++----
>  target/s390x/translate.c  | 3 +--
>  3 files changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 12/31] target/s390x: Use unwind data for helper_mvst

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 5 +++--
 target/s390x/translate.c  | 1 -
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 9ef9f4a..d1a7bcd 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -384,6 +384,7 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t r2)
 /* string copy (c is string terminator) */
 uint64_t HELPER(mvst)(CPUS390XState *env, uint64_t c, uint64_t d, uint64_t s)
 {
+    uintptr_t ra = GETPC();
     uint32_t len;
 
     c = c & 0xff;
@@ -393,8 +394,8 @@ uint64_t HELPER(mvst)(CPUS390XState *env, uint64_t c, uint64_t d, uint64_t 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.  */
     for (len = 0; len < 0x2000; ++len) {
-        uint8_t v = cpu_ldub_data(env, s + len);
-        cpu_stb_data(env, d + len, v);
+        uint8_t v = cpu_ldub_data_ra(env, s + len, ra);
+        cpu_stb_data_ra(env, d + len, v, ra);
         if (v == c) {
             /* Complete.  Set CC=1 and advance R1.  */
             env->cc_op = 1;
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index a1edc79..f9d05b6 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2928,7 +2928,6 @@ static ExitStatus op_mvpg(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_mvst(DisasContext *s, DisasOps *o)
 {
-    potential_page_fault(s);
     gen_helper_mvst(o->in1, cpu_env, regs[0], o->in1, o->in2);
     set_cc_static(s);
     return_low128(o->in2);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 12/31] target/s390x: Use unwind data for helper_mvst

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 5 +++--
>  target/s390x/translate.c  | 1 -
>  2 files changed, 3 insertions(+), 3 deletions(-)

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

Re: [Qemu-devel] [PATCH 12/31] target/s390x: Use unwind data for helper_mvst

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 5 +++--
>  target/s390x/translate.c  | 1 -
>  2 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 13/31] target/s390x: Use unwind data for helper_lam

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 3 ++-
 target/s390x/translate.c  | 1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index d1a7bcd..4ecec74 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -450,10 +450,11 @@ void HELPER(ex)(CPUS390XState *env, uint32_t ilen, uint64_t v1, uint64_t addr)
 /* load access registers r1 to r3 from memory at a2 */
 void HELPER(lam)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
 {
+    uintptr_t ra = GETPC();
     int i;
 
     for (i = r1;; i = (i + 1) % 16) {
-        env->aregs[i] = cpu_ldl_data(env, a2);
+        env->aregs[i] = cpu_ldl_data_ra(env, a2, ra);
         a2 += 4;
 
         if (i == r3) {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index f9d05b6..1fc58a1 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2626,7 +2626,6 @@ static ExitStatus op_lam(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));
-    potential_page_fault(s);
     gen_helper_lam(cpu_env, r1, o->in2, r3);
     tcg_temp_free_i32(r1);
     tcg_temp_free_i32(r3);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 13/31] target/s390x: Use unwind data for helper_lam

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 3 ++-
>  target/s390x/translate.c  | 1 -
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index d1a7bcd..4ecec74 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -450,10 +450,11 @@ void HELPER(ex)(CPUS390XState *env, uint32_t ilen, uint64_t v1, uint64_t addr)
>  /* load access registers r1 to r3 from memory at a2 */
>  void HELPER(lam)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
>  {
> +    uintptr_t ra = GETPC();
>      int i;
>  
>      for (i = r1;; i = (i + 1) % 16) {
> -        env->aregs[i] = cpu_ldl_data(env, a2);
> +        env->aregs[i] = cpu_ldl_data_ra(env, a2, ra);
>          a2 += 4;
>  
>          if (i == r3) {
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index f9d05b6..1fc58a1 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -2626,7 +2626,6 @@ static ExitStatus op_lam(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));
> -    potential_page_fault(s);
>      gen_helper_lam(cpu_env, r1, o->in2, r3);
>      tcg_temp_free_i32(r1);
>      tcg_temp_free_i32(r3);
> 

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

Re: [Qemu-devel] [PATCH 13/31] target/s390x: Use unwind data for helper_lam

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 3 ++-
>  target/s390x/translate.c  | 1 -
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 14/31] target/s390x: Use unwind data for helper_stam

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 3 ++-
 target/s390x/translate.c  | 1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 4ecec74..d6d5047 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -466,10 +466,11 @@ void HELPER(lam)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
 /* store access registers r1 to r3 in memory at a2 */
 void HELPER(stam)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
 {
+    uintptr_t ra = GETPC();
     int i;
 
     for (i = r1;; i = (i + 1) % 16) {
-        cpu_stl_data(env, a2, env->aregs[i]);
+        cpu_stl_data_ra(env, a2, env->aregs[i], ra);
         a2 += 4;
 
         if (i == r3) {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 1fc58a1..da7b5a6 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3867,7 +3867,6 @@ static ExitStatus op_stam(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));
-    potential_page_fault(s);
     gen_helper_stam(cpu_env, r1, o->in2, r3);
     tcg_temp_free_i32(r1);
     tcg_temp_free_i32(r3);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 14/31] target/s390x: Use unwind data for helper_stam

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 3 ++-
>  target/s390x/translate.c  | 1 -
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 4ecec74..d6d5047 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -466,10 +466,11 @@ void HELPER(lam)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
>  /* store access registers r1 to r3 in memory at a2 */
>  void HELPER(stam)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
>  {
> +    uintptr_t ra = GETPC();
>      int i;
>  
>      for (i = r1;; i = (i + 1) % 16) {
> -        cpu_stl_data(env, a2, env->aregs[i]);
> +        cpu_stl_data_ra(env, a2, env->aregs[i], ra);
>          a2 += 4;
>  
>          if (i == r3) {
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 1fc58a1..da7b5a6 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -3867,7 +3867,6 @@ static ExitStatus op_stam(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));
> -    potential_page_fault(s);
>      gen_helper_stam(cpu_env, r1, o->in2, r3);
>      tcg_temp_free_i32(r1);
>      tcg_temp_free_i32(r3);
> 

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

Re: [Qemu-devel] [PATCH 14/31] target/s390x: Use unwind data for helper_stam

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 3 ++-
>  target/s390x/translate.c  | 1 -
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 15/31] target/s390x: Use unwind data for helper_mvcl

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 7 ++++---
 target/s390x/translate.c  | 1 -
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index d6d5047..b764c48 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -482,6 +482,7 @@ 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)
 {
+    uintptr_t ra = GETPC();
     uint64_t destlen = env->regs[r1 + 1] & 0xffffff;
     uint64_t dest = get_address_31fix(env, r1);
     uint64_t srclen = env->regs[r2 + 1] & 0xffffff;
@@ -503,12 +504,12 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
     }
 
     for (; destlen && srclen; src++, dest++, destlen--, srclen--) {
-        v = cpu_ldub_data(env, src);
-        cpu_stb_data(env, dest, v);
+        v = cpu_ldub_data_ra(env, src, ra);
+        cpu_stb_data_ra(env, dest, v, ra);
     }
 
     for (; destlen; dest++, destlen--) {
-        cpu_stb_data(env, dest, pad);
+        cpu_stb_data_ra(env, dest, pad, ra);
     }
 
     env->regs[r1 + 1] = destlen;
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index da7b5a6..ad2e632 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2876,7 +2876,6 @@ static ExitStatus op_mvcl(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));
-    potential_page_fault(s);
     gen_helper_mvcl(cc_op, cpu_env, r1, r2);
     tcg_temp_free_i32(r1);
     tcg_temp_free_i32(r2);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 15/31] target/s390x: Use unwind data for helper_mvcl

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 7 ++++---
>  target/s390x/translate.c  | 1 -
>  2 files changed, 4 insertions(+), 4 deletions(-)

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

Re: [Qemu-devel] [PATCH 15/31] target/s390x: Use unwind data for helper_mvcl

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 7 ++++---
>  target/s390x/translate.c  | 1 -
>  2 files changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 16/31] target/s390x: Use unwind data for helper_mvcle

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 7 ++++---
 target/s390x/translate.c  | 1 -
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index b764c48..7a59be0 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -525,6 +525,7 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
 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 dest = env->regs[r1];
     uint64_t srclen = env->regs[r3 + 1];
@@ -553,12 +554,12 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     }
 
     for (; destlen && srclen; src++, dest++, destlen--, srclen--) {
-        v = cpu_ldub_data(env, src);
-        cpu_stb_data(env, dest, v);
+        v = cpu_ldub_data_ra(env, src, ra);
+        cpu_stb_data_ra(env, dest, v, ra);
     }
 
     for (; destlen; dest++, destlen--) {
-        cpu_stb_data(env, dest, pad);
+        cpu_stb_data_ra(env, dest, pad, ra);
     }
 
     env->regs[r1 + 1] = destlen;
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index ad2e632..c00c15e 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2887,7 +2887,6 @@ static ExitStatus op_mvcle(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));
-    potential_page_fault(s);
     gen_helper_mvcle(cc_op, cpu_env, r1, o->in2, r3);
     tcg_temp_free_i32(r1);
     tcg_temp_free_i32(r3);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 16/31] target/s390x: Use unwind data for helper_mvcle

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 7 ++++---
>  target/s390x/translate.c  | 1 -
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index b764c48..7a59be0 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -525,6 +525,7 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
>  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 dest = env->regs[r1];
>      uint64_t srclen = env->regs[r3 + 1];
> @@ -553,12 +554,12 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
>      }
>  
>      for (; destlen && srclen; src++, dest++, destlen--, srclen--) {
> -        v = cpu_ldub_data(env, src);
> -        cpu_stb_data(env, dest, v);
> +        v = cpu_ldub_data_ra(env, src, ra);
> +        cpu_stb_data_ra(env, dest, v, ra);
>      }
>  
>      for (; destlen; dest++, destlen--) {
> -        cpu_stb_data(env, dest, pad);
> +        cpu_stb_data_ra(env, dest, pad, ra);
>      }
>  
>      env->regs[r1 + 1] = destlen;
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index ad2e632..c00c15e 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -2887,7 +2887,6 @@ static ExitStatus op_mvcle(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));
> -    potential_page_fault(s);
>      gen_helper_mvcle(cc_op, cpu_env, r1, o->in2, r3);
>      tcg_temp_free_i32(r1);
>      tcg_temp_free_i32(r3);

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

Re: [Qemu-devel] [PATCH 16/31] target/s390x: Use unwind data for helper_mvcle

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 7 ++++---
>  target/s390x/translate.c  | 1 -
>  2 files changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 17/31] target/s390x: Use unwind data for helper_clcle

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 6 +++---
 target/s390x/translate.c  | 1 -
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 7a59be0..6e85406 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -576,12 +576,12 @@ uint32_t HELPER(mvcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
 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 dest = get_address_31fix(env, r1);
     uint64_t srclen = env->regs[r3 + 1];
     uint64_t src = get_address_31fix(env, r3);
     uint8_t pad = a2 & 0xff;
-    uint8_t v1 = 0, v2 = 0;
     uint32_t cc = 0;
 
     if (!(destlen || srclen)) {
@@ -593,8 +593,8 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
     }
 
     for (; destlen || srclen; src++, dest++, destlen--, srclen--) {
-        v1 = srclen ? cpu_ldub_data(env, src) : pad;
-        v2 = destlen ? cpu_ldub_data(env, dest) : pad;
+        uint8_t v1 = srclen ? cpu_ldub_data_ra(env, src, ra) : pad;
+        uint8_t v2 = destlen ? cpu_ldub_data_ra(env, dest, ra) : pad;
         if (v1 != v2) {
             cc = (v1 < v2) ? 1 : 2;
             break;
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index c00c15e..ad85a75 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1917,7 +1917,6 @@ static ExitStatus op_clcle(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));
-    potential_page_fault(s);
     gen_helper_clcle(cc_op, cpu_env, r1, o->in2, r3);
     tcg_temp_free_i32(r1);
     tcg_temp_free_i32(r3);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 17/31] target/s390x: Use unwind data for helper_clcle

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 6 +++---
>  target/s390x/translate.c  | 1 -
>  2 files changed, 3 insertions(+), 4 deletions(-)

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

Re: [Qemu-devel] [PATCH 17/31] target/s390x: Use unwind data for helper_clcle

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 6 +++---
>  target/s390x/translate.c  | 1 -
>  2 files changed, 3 insertions(+), 4 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 18/31] target/s390x: Use unwind data for helper_cksm

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 11 ++++++-----
 target/s390x/translate.c  |  1 -
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 6e85406..da81b38 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -614,6 +614,7 @@ uint32_t HELPER(clcle)(CPUS390XState *env, uint32_t r1, uint64_t a2,
 uint64_t HELPER(cksm)(CPUS390XState *env, uint64_t r1,
                       uint64_t src, uint64_t src_len)
 {
+    uintptr_t ra = GETPC();
     uint64_t max_len, len;
     uint64_t cksm = (uint32_t)r1;
 
@@ -623,21 +624,21 @@ uint64_t HELPER(cksm)(CPUS390XState *env, uint64_t r1,
 
     /* Process full words as available.  */
     for (len = 0; len + 4 <= max_len; len += 4, src += 4) {
-        cksm += (uint32_t)cpu_ldl_data(env, src);
+        cksm += (uint32_t)cpu_ldl_data_ra(env, src, ra);
     }
 
     switch (max_len - len) {
     case 1:
-        cksm += cpu_ldub_data(env, src) << 24;
+        cksm += cpu_ldub_data_ra(env, src, ra) << 24;
         len += 1;
         break;
     case 2:
-        cksm += cpu_lduw_data(env, src) << 16;
+        cksm += cpu_lduw_data_ra(env, src, ra) << 16;
         len += 2;
         break;
     case 3:
-        cksm += cpu_lduw_data(env, src) << 16;
-        cksm += cpu_ldub_data(env, src + 2) << 8;
+        cksm += cpu_lduw_data_ra(env, src, ra) << 16;
+        cksm += cpu_ldub_data_ra(env, src + 2, ra) << 8;
         len += 3;
         break;
     }
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index ad85a75..21c21a6 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1868,7 +1868,6 @@ static ExitStatus op_cksm(DisasContext *s, DisasOps *o)
     int r2 = get_field(s->fields, r2);
     TCGv_i64 len = tcg_temp_new_i64();
 
-    potential_page_fault(s);
     gen_helper_cksm(len, cpu_env, o->in1, o->in2, regs[r2 + 1]);
     set_cc_static(s);
     return_low128(o->out);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 18/31] target/s390x: Use unwind data for helper_cksm

[Thomas Huth]

On 23.05.2017 05:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 11 ++++++-----
>  target/s390x/translate.c  |  1 -
>  2 files changed, 6 insertions(+), 6 deletions(-)

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

Re: [Qemu-devel] [PATCH 18/31] target/s390x: Use unwind data for helper_cksm

[Aurelien Jarno]

On 2017-05-22 20:02, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 11 ++++++-----
>  target/s390x/translate.c  |  1 -
>  2 files changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 19/31] target/s390x: Use unwind data for helper_unpk

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 9 +++++----
 target/s390x/translate.c  | 1 -
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index da81b38..d37e691 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -660,6 +660,7 @@ uint64_t HELPER(cksm)(CPUS390XState *env, uint64_t r1,
 void HELPER(unpk)(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;
@@ -669,8 +670,8 @@ void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
     src += len_src;
 
     /* last byte is special, it only flips the nibbles */
-    b = cpu_ldub_data(env, src);
-    cpu_stb_data(env, dest, (b << 4) | (b >> 4));
+    b = cpu_ldub_data_ra(env, src, ra);
+    cpu_stb_data_ra(env, dest, (b << 4) | (b >> 4), ra);
     src--;
     len_src--;
 
@@ -680,7 +681,7 @@ void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
         uint8_t cur_byte = 0;
 
         if (len_src > 0) {
-            cur_byte = cpu_ldub_data(env, src);
+            cur_byte = cpu_ldub_data_ra(env, src, ra);
         }
 
         len_dest--;
@@ -699,7 +700,7 @@ void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
         /* zone bits */
         cur_byte |= 0xf0;
 
-        cpu_stb_data(env, dest, cur_byte);
+        cpu_stb_data_ra(env, dest, cur_byte, ra);
     }
 }
 
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 21c21a6..e8eefec 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4099,7 +4099,6 @@ static ExitStatus op_trt(DisasContext *s, DisasOps *o)
 static ExitStatus op_unpk(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
-    potential_page_fault(s);
     gen_helper_unpk(cpu_env, l, o->addr1, o->in2);
     tcg_temp_free_i32(l);
     return NO_EXIT;
-- 
2.9.4

Re: [Qemu-devel] [PATCH 19/31] target/s390x: Use unwind data for helper_unpk

[Thomas Huth]

On 23.05.2017 05:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 9 +++++----
>  target/s390x/translate.c  | 1 -
>  2 files changed, 5 insertions(+), 5 deletions(-)

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

Re: [Qemu-devel] [PATCH 19/31] target/s390x: Use unwind data for helper_unpk

[Aurelien Jarno]

On 2017-05-22 20:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 9 +++++----
>  target/s390x/translate.c  | 1 -
>  2 files changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 20/31] target/s390x: Use unwind data for helper_tr

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 10 +++++-----
 target/s390x/translate.c  |  1 -
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index d37e691..748a6e8 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -707,13 +707,13 @@ void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
 void HELPER(tr)(CPUS390XState *env, uint32_t len, uint64_t array,
                 uint64_t trans)
 {
-    int i;
+    uintptr_t ra = GETPC();
+    uint32_t i;
 
     for (i = 0; i <= len; i++) {
-        uint8_t byte = cpu_ldub_data(env, array + i);
-        uint8_t new_byte = cpu_ldub_data(env, trans + byte);
-
-        cpu_stb_data(env, array + i, new_byte);
+        uint8_t byte = cpu_ldub_data_ra(env, array + i, ra);
+        uint8_t new_byte = cpu_ldub_data_ra(env, trans + byte, ra);
+        cpu_stb_data_ra(env, array + i, new_byte, ra);
     }
 }
 
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index e8eefec..b506cee 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4070,7 +4070,6 @@ static ExitStatus op_tprot(DisasContext *s, DisasOps *o)
 static ExitStatus op_tr(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
-    potential_page_fault(s);
     gen_helper_tr(cpu_env, l, o->addr1, o->in2);
     tcg_temp_free_i32(l);
     set_cc_static(s);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 20/31] target/s390x: Use unwind data for helper_tr

[Thomas Huth]

On 23.05.2017 05:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 10 +++++-----
>  target/s390x/translate.c  |  1 -
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index d37e691..748a6e8 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -707,13 +707,13 @@ void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
>  void HELPER(tr)(CPUS390XState *env, uint32_t len, uint64_t array,
>                  uint64_t trans)
>  {
> -    int i;
> +    uintptr_t ra = GETPC();
> +    uint32_t i;
>  
>      for (i = 0; i <= len; i++) {
> -        uint8_t byte = cpu_ldub_data(env, array + i);
> -        uint8_t new_byte = cpu_ldub_data(env, trans + byte);
> -
> -        cpu_stb_data(env, array + i, new_byte);
> +        uint8_t byte = cpu_ldub_data_ra(env, array + i, ra);
> +        uint8_t new_byte = cpu_ldub_data_ra(env, trans + byte, ra);
> +        cpu_stb_data_ra(env, array + i, new_byte, ra);
>      }
>  }
>  
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index e8eefec..b506cee 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -4070,7 +4070,6 @@ static ExitStatus op_tprot(DisasContext *s, DisasOps *o)
>  static ExitStatus op_tr(DisasContext *s, DisasOps *o)
>  {
>      TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
> -    potential_page_fault(s);
>      gen_helper_tr(cpu_env, l, o->addr1, o->in2);
>      tcg_temp_free_i32(l);
>      set_cc_static(s);
> 

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

Re: [Qemu-devel] [PATCH 20/31] target/s390x: Use unwind data for helper_tr

[Aurelien Jarno]

On 2017-05-22 20:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 10 +++++-----
>  target/s390x/translate.c  |  1 -
>  2 files changed, 5 insertions(+), 6 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 21/31] target/s390x: Use unwind data for helper_tre

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 15 ++++++++-------
 target/s390x/translate.c  |  1 -
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 748a6e8..4c6c6ee 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -720,8 +720,10 @@ void HELPER(tr)(CPUS390XState *env, uint32_t len, uint64_t array,
 uint64_t HELPER(tre)(CPUS390XState *env, uint64_t array,
                      uint64_t len, uint64_t trans)
 {
+    uintptr_t ra = GETPC();
     uint8_t end = env->regs[0] & 0xff;
     uint64_t l = len;
+    uint32_t cc = 0;
     uint64_t i;
 
     if (!(env->psw.mask & PSW_MASK_64)) {
@@ -733,25 +735,24 @@ uint64_t HELPER(tre)(CPUS390XState *env, uint64_t array,
        amount of work we're willing to do.  For now, let's cap at 8k.  */
     if (l > 0x2000) {
         l = 0x2000;
-        env->cc_op = 3;
-    } else {
-        env->cc_op = 0;
+        cc = 3;
     }
 
     for (i = 0; i < l; i++) {
         uint8_t byte, new_byte;
 
-        byte = cpu_ldub_data(env, array + i);
+        byte = cpu_ldub_data_ra(env, array + i, ra);
 
         if (byte == end) {
-            env->cc_op = 1;
+            cc = 1;
             break;
         }
 
-        new_byte = cpu_ldub_data(env, trans + byte);
-        cpu_stb_data(env, array + i, new_byte);
+        new_byte = cpu_ldub_data_ra(env, trans + byte, ra);
+        cpu_stb_data_ra(env, array + i, new_byte, ra);
     }
 
+    env->cc_op = cc;
     env->retxl = len - i;
     return array + i;
 }
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index b506cee..ecef71c 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4078,7 +4078,6 @@ static ExitStatus op_tr(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_tre(DisasContext *s, DisasOps *o)
 {
-    potential_page_fault(s);
     gen_helper_tre(o->out, cpu_env, o->out, o->out2, o->in2);
     return_low128(o->out2);
     set_cc_static(s);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 21/31] target/s390x: Use unwind data for helper_tre

[Thomas Huth]

On 23.05.2017 05:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 15 ++++++++-------
>  target/s390x/translate.c  |  1 -
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 748a6e8..4c6c6ee 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -720,8 +720,10 @@ void HELPER(tr)(CPUS390XState *env, uint32_t len, uint64_t array,
>  uint64_t HELPER(tre)(CPUS390XState *env, uint64_t array,
>                       uint64_t len, uint64_t trans)
>  {
> +    uintptr_t ra = GETPC();
>      uint8_t end = env->regs[0] & 0xff;
>      uint64_t l = len;
> +    uint32_t cc = 0;
>      uint64_t i;
>  
>      if (!(env->psw.mask & PSW_MASK_64)) {
> @@ -733,25 +735,24 @@ uint64_t HELPER(tre)(CPUS390XState *env, uint64_t array,
>         amount of work we're willing to do.  For now, let's cap at 8k.  */
>      if (l > 0x2000) {
>          l = 0x2000;
> -        env->cc_op = 3;
> -    } else {
> -        env->cc_op = 0;
> +        cc = 3;
>      }
>  
>      for (i = 0; i < l; i++) {
>          uint8_t byte, new_byte;
>  
> -        byte = cpu_ldub_data(env, array + i);
> +        byte = cpu_ldub_data_ra(env, array + i, ra);
>  
>          if (byte == end) {
> -            env->cc_op = 1;
> +            cc = 1;
>              break;
>          }
>  
> -        new_byte = cpu_ldub_data(env, trans + byte);
> -        cpu_stb_data(env, array + i, new_byte);
> +        new_byte = cpu_ldub_data_ra(env, trans + byte, ra);
> +        cpu_stb_data_ra(env, array + i, new_byte, ra);
>      }
>  
> +    env->cc_op = cc;
>      env->retxl = len - i;
>      return array + i;
>  }

I think you should mention in the patch description that you've changed
the behavior of the CC settings here, too (it's not changed anymore in
case of exceptions).

Apart from that:

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

Re: [Qemu-devel] [PATCH 21/31] target/s390x: Use unwind data for helper_tre

[Aurelien Jarno]

On 2017-05-22 20:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 15 ++++++++-------
>  target/s390x/translate.c  |  1 -
>  2 files changed, 8 insertions(+), 8 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 22/31] target/s390x: Use unwind data for helper_trt

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 9 +++++----
 target/s390x/translate.c  | 1 -
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 4c6c6ee..902290c 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -760,16 +760,17 @@ uint64_t HELPER(tre)(CPUS390XState *env, uint64_t array,
 uint32_t HELPER(trt)(CPUS390XState *env, uint32_t len, uint64_t array,
                      uint64_t trans)
 {
+    uintptr_t ra = GETPC();
     uint32_t cc = 0;
-    int i;
+    uint32_t i;
 
     for (i = 0; i <= len; i++) {
-        uint8_t byte = cpu_ldub_data(env, array + i);
-        uint8_t sbyte = cpu_ldub_data(env, trans + byte);
+        uint8_t byte = cpu_ldub_data_ra(env, array + i, ra);
+        uint8_t sbyte = cpu_ldub_data_ra(env, trans + byte, ra);
 
         if (sbyte != 0) {
             env->regs[1] = array + i;
-            env->regs[2] = (env->regs[2] & ~0xff) | sbyte;
+            env->regs[2] = deposit64(env->regs[2], 0, 8, sbyte);
             cc = (i == len) ? 2 : 1;
             break;
         }
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index ecef71c..531b5f1 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4087,7 +4087,6 @@ static ExitStatus op_tre(DisasContext *s, DisasOps *o)
 static ExitStatus op_trt(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
-    potential_page_fault(s);
     gen_helper_trt(cc_op, cpu_env, l, o->addr1, o->in2);
     tcg_temp_free_i32(l);
     set_cc_static(s);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 22/31] target/s390x: Use unwind data for helper_trt

[Thomas Huth]

On 23.05.2017 05:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 9 +++++----
>  target/s390x/translate.c  | 1 -
>  2 files changed, 5 insertions(+), 5 deletions(-)

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

Re: [Qemu-devel] [PATCH 22/31] target/s390x: Use unwind data for helper_trt

[Aurelien Jarno]

On 2017-05-22 20:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 9 +++++----
>  target/s390x/translate.c  | 1 -
>  2 files changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 23/31] target/s390x: Use unwind data for helper_lctlg

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 8 ++++----
 target/s390x/translate.c  | 1 -
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 902290c..26960e4 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -821,20 +821,20 @@ void HELPER(cdsg)(CPUS390XState *env, uint64_t addr,
 #if !defined(CONFIG_USER_ONLY)
 void HELPER(lctlg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
 {
+    uintptr_t ra = GETPC();
     S390CPU *cpu = s390_env_get_cpu(env);
     bool PERchanged = false;
-    int i;
     uint64_t src = a2;
-    uint64_t val;
+    uint32_t i;
 
     for (i = r1;; i = (i + 1) % 16) {
-        val = cpu_ldq_data(env, src);
+        uint64_t val = cpu_ldq_data_ra(env, src, ra);
         if (env->cregs[i] != val && i >= 9 && i <= 11) {
             PERchanged = true;
         }
         env->cregs[i] = val;
         HELPER_LOG("load ctl %d from 0x%" PRIx64 " == 0x%" PRIx64 "\n",
-                   i, src, env->cregs[i]);
+                   i, src, val);
         src += sizeof(uint64_t);
 
         if (i == r3) {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 531b5f1..886991e 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2557,7 +2557,6 @@ static ExitStatus op_lctlg(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));
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_lctlg(cpu_env, r1, o->in2, r3);
     tcg_temp_free_i32(r1);
     tcg_temp_free_i32(r3);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 23/31] target/s390x: Use unwind data for helper_lctlg

[Thomas Huth]

On 23.05.2017 05:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 8 ++++----
>  target/s390x/translate.c  | 1 -
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 902290c..26960e4 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -821,20 +821,20 @@ void HELPER(cdsg)(CPUS390XState *env, uint64_t addr,
>  #if !defined(CONFIG_USER_ONLY)
>  void HELPER(lctlg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
>  {
> +    uintptr_t ra = GETPC();
>      S390CPU *cpu = s390_env_get_cpu(env);
>      bool PERchanged = false;
> -    int i;
>      uint64_t src = a2;
> -    uint64_t val;
> +    uint32_t i;
>  
>      for (i = r1;; i = (i + 1) % 16) {
> -        val = cpu_ldq_data(env, src);
> +        uint64_t val = cpu_ldq_data_ra(env, src, ra);
>          if (env->cregs[i] != val && i >= 9 && i <= 11) {
>              PERchanged = true;
>          }
>          env->cregs[i] = val;
>          HELPER_LOG("load ctl %d from 0x%" PRIx64 " == 0x%" PRIx64 "\n",
> -                   i, src, env->cregs[i]);
> +                   i, src, val);

I think you should mention in the patch description that you've fixed
the HELPER_LOG here, too. Apart from that:

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

Re: [Qemu-devel] [PATCH 23/31] target/s390x: Use unwind data for helper_lctlg

[Aurelien Jarno]

On 2017-05-22 20:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 8 ++++----
>  target/s390x/translate.c  | 1 -
>  2 files changed, 4 insertions(+), 5 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 24/31] target/s390x: Use unwind data for helper_lctl

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 9 +++++----
 target/s390x/translate.c  | 1 -
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 26960e4..b169e0e 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -851,18 +851,19 @@ void HELPER(lctlg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
 
 void HELPER(lctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
 {
+    uintptr_t ra = GETPC();
     S390CPU *cpu = s390_env_get_cpu(env);
     bool PERchanged = false;
-    int i;
     uint64_t src = a2;
-    uint32_t val;
+    uint32_t i;
 
     for (i = r1;; i = (i + 1) % 16) {
-        val = cpu_ldl_data(env, src);
+        uint32_t val = cpu_ldl_data_ra(env, src, ra);
         if ((uint32_t)env->cregs[i] != val && i >= 9 && i <= 11) {
             PERchanged = true;
         }
-        env->cregs[i] = (env->cregs[i] & 0xFFFFFFFF00000000ULL) | val;
+        env->cregs[i] = deposit64(env->cregs[i], 0, 32, val);
+        HELPER_LOG("load ctl %d from 0x%" PRIx64 " == 0x%x\n", i, src, val);
         src += sizeof(uint32_t);
 
         if (i == r3) {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 886991e..4d964a8 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2545,7 +2545,6 @@ static ExitStatus op_lctl(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));
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_lctl(cpu_env, r1, o->in2, r3);
     tcg_temp_free_i32(r1);
     tcg_temp_free_i32(r3);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 24/31] target/s390x: Use unwind data for helper_lctl

[Thomas Huth]

On 23.05.2017 05:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 9 +++++----
>  target/s390x/translate.c  | 1 -
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 26960e4..b169e0e 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -851,18 +851,19 @@ void HELPER(lctlg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
>  
>  void HELPER(lctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
>  {
> +    uintptr_t ra = GETPC();
>      S390CPU *cpu = s390_env_get_cpu(env);
>      bool PERchanged = false;
> -    int i;
>      uint64_t src = a2;
> -    uint32_t val;
> +    uint32_t i;
>  
>      for (i = r1;; i = (i + 1) % 16) {
> -        val = cpu_ldl_data(env, src);
> +        uint32_t val = cpu_ldl_data_ra(env, src, ra);
>          if ((uint32_t)env->cregs[i] != val && i >= 9 && i <= 11) {
>              PERchanged = true;
>          }
> -        env->cregs[i] = (env->cregs[i] & 0xFFFFFFFF00000000ULL) | val;
> +        env->cregs[i] = deposit64(env->cregs[i], 0, 32, val);
> +        HELPER_LOG("load ctl %d from 0x%" PRIx64 " == 0x%x\n", i, src, val);
>          src += sizeof(uint32_t);
>  
>          if (i == r3) {
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 886991e..4d964a8 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -2545,7 +2545,6 @@ static ExitStatus op_lctl(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));
>      check_privileged(s);
> -    potential_page_fault(s);
>      gen_helper_lctl(cpu_env, r1, o->in2, r3);
>      tcg_temp_free_i32(r1);
>      tcg_temp_free_i32(r3);

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

Re: [Qemu-devel] [PATCH 24/31] target/s390x: Use unwind data for helper_lctl

[Aurelien Jarno]

On 2017-05-22 20:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 9 +++++----
>  target/s390x/translate.c  | 1 -
>  2 files changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 25/31] target/s390x: Use unwind data for helper_stctl

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 10 ++++++----
 target/s390x/translate.c  |  2 --
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index b169e0e..1e31bd3 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -880,11 +880,12 @@ void HELPER(lctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
 
 void HELPER(stctg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
 {
-    int i;
+    uintptr_t ra = GETPC();
     uint64_t dest = a2;
+    uint32_t i;
 
     for (i = r1;; i = (i + 1) % 16) {
-        cpu_stq_data(env, dest, env->cregs[i]);
+        cpu_stq_data_ra(env, dest, env->cregs[i], ra);
         dest += sizeof(uint64_t);
 
         if (i == r3) {
@@ -895,11 +896,12 @@ void HELPER(stctg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
 
 void HELPER(stctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
 {
-    int i;
+    uintptr_t ra = GETPC();
     uint64_t dest = a2;
+    uint32_t i;
 
     for (i = r1;; i = (i + 1) % 16) {
-        cpu_stl_data(env, dest, env->cregs[i]);
+        cpu_stl_data_ra(env, dest, env->cregs[i], ra);
         dest += sizeof(uint32_t);
 
         if (i == r3) {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 4d964a8..3a2151f 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3617,7 +3617,6 @@ static ExitStatus op_stctg(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));
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_stctg(cpu_env, r1, o->in2, r3);
     tcg_temp_free_i32(r1);
     tcg_temp_free_i32(r3);
@@ -3629,7 +3628,6 @@ static ExitStatus op_stctl(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));
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_stctl(cpu_env, r1, o->in2, r3);
     tcg_temp_free_i32(r1);
     tcg_temp_free_i32(r3);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 25/31] target/s390x: Use unwind data for helper_stctl

[Thomas Huth]

On 23.05.2017 05:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 10 ++++++----
>  target/s390x/translate.c  |  2 --
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index b169e0e..1e31bd3 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -880,11 +880,12 @@ void HELPER(lctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
>  
>  void HELPER(stctg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
>  {
> -    int i;
> +    uintptr_t ra = GETPC();
>      uint64_t dest = a2;
> +    uint32_t i;
>  
>      for (i = r1;; i = (i + 1) % 16) {
> -        cpu_stq_data(env, dest, env->cregs[i]);
> +        cpu_stq_data_ra(env, dest, env->cregs[i], ra);
>          dest += sizeof(uint64_t);
>  
>          if (i == r3) {
> @@ -895,11 +896,12 @@ void HELPER(stctg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
>  
>  void HELPER(stctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
>  {
> -    int i;
> +    uintptr_t ra = GETPC();
>      uint64_t dest = a2;
> +    uint32_t i;
>  
>      for (i = r1;; i = (i + 1) % 16) {
> -        cpu_stl_data(env, dest, env->cregs[i]);
> +        cpu_stl_data_ra(env, dest, env->cregs[i], ra);
>          dest += sizeof(uint32_t);
>  
>          if (i == r3) {
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 4d964a8..3a2151f 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -3617,7 +3617,6 @@ static ExitStatus op_stctg(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));
>      check_privileged(s);
> -    potential_page_fault(s);
>      gen_helper_stctg(cpu_env, r1, o->in2, r3);
>      tcg_temp_free_i32(r1);
>      tcg_temp_free_i32(r3);
> @@ -3629,7 +3628,6 @@ static ExitStatus op_stctl(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));
>      check_privileged(s);
> -    potential_page_fault(s);
>      gen_helper_stctl(cpu_env, r1, o->in2, r3);
>      tcg_temp_free_i32(r1);
>      tcg_temp_free_i32(r3);
> 

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

Re: [Qemu-devel] [PATCH 25/31] target/s390x: Use unwind data for helper_stctl

[Aurelien Jarno]

On 2017-05-22 20:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 10 ++++++----
>  target/s390x/translate.c  |  2 --
>  2 files changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 26/31] target/s390x: Use unwind data for helper_testblock

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 3 +++
 target/s390x/translate.c  | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 1e31bd3..a874f8a 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -912,6 +912,7 @@ void HELPER(stctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
 
 uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t real_addr)
 {
+    uintptr_t ra = GETPC();
     CPUState *cs = CPU(s390_env_get_cpu(env));
     uint64_t abs_addr;
     int i;
@@ -920,12 +921,14 @@ uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t 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)) {
+        cpu_restore_state(cs, ra);
         program_interrupt(env, PGM_ADDRESSING, 4);
         return 1;
     }
 
     /* Check low-address protection */
     if ((env->cregs[0] & CR0_LOWPROT) && real_addr < 0x2000) {
+        cpu_restore_state(cs, ra);
         program_interrupt(env, PGM_PROTECTION, 4);
         return 1;
     }
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 3a2151f..694e099 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4047,7 +4047,6 @@ static ExitStatus op_tcxb(DisasContext *s, DisasOps *o)
 static ExitStatus op_testblock(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_testblock(cc_op, cpu_env, o->in2);
     set_cc_static(s);
     return NO_EXIT;
-- 
2.9.4

Re: [Qemu-devel] [PATCH 26/31] target/s390x: Use unwind data for helper_testblock

[Thomas Huth]

On 23.05.2017 05:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 3 +++
>  target/s390x/translate.c  | 1 -
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 1e31bd3..a874f8a 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -912,6 +912,7 @@ void HELPER(stctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
>  
>  uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t real_addr)
>  {
> +    uintptr_t ra = GETPC();
>      CPUState *cs = CPU(s390_env_get_cpu(env));
>      uint64_t abs_addr;
>      int i;
> @@ -920,12 +921,14 @@ uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t 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)) {
> +        cpu_restore_state(cs, ra);
>          program_interrupt(env, PGM_ADDRESSING, 4);
>          return 1;
>      }
>  
>      /* Check low-address protection */
>      if ((env->cregs[0] & CR0_LOWPROT) && real_addr < 0x2000) {
> +        cpu_restore_state(cs, ra);
>          program_interrupt(env, PGM_PROTECTION, 4);
>          return 1;
>      }
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 3a2151f..694e099 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -4047,7 +4047,6 @@ static ExitStatus op_tcxb(DisasContext *s, DisasOps *o)
>  static ExitStatus op_testblock(DisasContext *s, DisasOps *o)
>  {
>      check_privileged(s);
> -    potential_page_fault(s);
>      gen_helper_testblock(cc_op, cpu_env, o->in2);
>      set_cc_static(s);
>      return NO_EXIT;

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

Re: [Qemu-devel] [PATCH 26/31] target/s390x: Use unwind data for helper_testblock

[Aurelien Jarno]

On 2017-05-22 20:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 3 +++
>  target/s390x/translate.c  | 1 -
>  2 files changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 27/31] target/s390x: Use unwind data for helper_tprot

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 1 -
 target/s390x/translate.c  | 1 -
 2 files changed, 2 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index a874f8a..7f22bf0 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -943,7 +943,6 @@ uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t real_addr)
 uint32_t HELPER(tprot)(uint64_t a1, uint64_t a2)
 {
     /* XXX implement */
-
     return 0;
 }
 
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 694e099..282fa27 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4054,7 +4054,6 @@ static ExitStatus op_testblock(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_tprot(DisasContext *s, DisasOps *o)
 {
-    potential_page_fault(s);
     gen_helper_tprot(cc_op, o->addr1, o->in2);
     set_cc_static(s);
     return NO_EXIT;
-- 
2.9.4

Re: [Qemu-devel] [PATCH 27/31] target/s390x: Use unwind data for helper_tprot

[Thomas Huth]

On 23.05.2017 05:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 1 -
>  target/s390x/translate.c  | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index a874f8a..7f22bf0 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -943,7 +943,6 @@ uint32_t HELPER(testblock)(CPUS390XState *env, uint64_t real_addr)
>  uint32_t HELPER(tprot)(uint64_t a1, uint64_t a2)
>  {
>      /* XXX implement */
> -

Unnecessary white space change ;-)

>      return 0;
>  }
>  
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 694e099..282fa27 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -4054,7 +4054,6 @@ static ExitStatus op_testblock(DisasContext *s, DisasOps *o)
>  
>  static ExitStatus op_tprot(DisasContext *s, DisasOps *o)
>  {
> -    potential_page_fault(s);
>      gen_helper_tprot(cc_op, o->addr1, o->in2);
>      set_cc_static(s);
>      return NO_EXIT;
> 

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

Re: [Qemu-devel] [PATCH 27/31] target/s390x: Use unwind data for helper_tprot

[Aurelien Jarno]

On 2017-05-22 20:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 1 -
>  target/s390x/translate.c  | 1 -
>  2 files changed, 2 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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

[Qemu-devel] [PATCH 28/31] target/s390x: Use unwind data for helper_lra

[Richard Henderson]

Note that exception_index is not live during a TB,
so there is no point saving it around mmu_translate.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 4 +---
 target/s390x/translate.c  | 1 -
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 7f22bf0..81b27c0 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1181,17 +1181,16 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
 {
     CPUState *cs = CPU(s390_env_get_cpu(env));
     uint32_t cc = 0;
-    int old_exc = cs->exception_index;
     uint64_t asc = env->psw.mask & PSW_MASK_ASC;
     uint64_t ret;
     int flags;
 
     /* XXX incomplete - has more corner cases */
     if (!(env->psw.mask & PSW_MASK_64) && (addr >> 32)) {
+        cpu_restore_state(cs, GETPC());
         program_interrupt(env, PGM_SPECIAL_OP, 2);
     }
 
-    cs->exception_index = old_exc;
     if (mmu_translate(env, addr, 0, asc, &ret, &flags, true)) {
         cc = 3;
     }
@@ -1200,7 +1199,6 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
     } else {
         ret |= addr & ~TARGET_PAGE_MASK;
     }
-    cs->exception_index = old_exc;
 
     env->cc_op = cc;
     return ret;
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 282fa27..3e4b397 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2565,7 +2565,6 @@ static ExitStatus op_lctlg(DisasContext *s, DisasOps *o)
 static ExitStatus op_lra(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_lra(o->out, cpu_env, o->in2);
     set_cc_static(s);
     return NO_EXIT;
-- 
2.9.4

Re: [Qemu-devel] [PATCH 28/31] target/s390x: Use unwind data for helper_lra

[Aurelien Jarno]

On 2017-05-22 20:03, Richard Henderson wrote:
> Note that exception_index is not live during a TB,
> so there is no point saving it around mmu_translate.

What do you mean by "is not live"? Indeed cpu_loop_exit() is not called
so the TB is not terminated immediately. That said the while loop in
cpu_exec() will trigger the exception after the TB.

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

Re: [Qemu-devel] [PATCH 28/31] target/s390x: Use unwind data for helper_lra

[Richard Henderson]

On 05/23/2017 05:11 AM, Aurelien Jarno wrote:
> On 2017-05-22 20:03, Richard Henderson wrote:
>> Note that exception_index is not live during a TB,
>> so there is no point saving it around mmu_translate.
> 
> What do you mean by "is not live"? Indeed cpu_loop_exit() is not called
> so the TB is not terminated immediately. That said the while loop in
> cpu_exec() will trigger the exception after the TB.
> 

Ah, yes.  I'll undo that bit.


r~

[Qemu-devel] [PATCH 29/31] target/s390x: Use atomic operations for COMPARE SWAP PURGE

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/helper.h      |  2 +-
 target/s390x/insn-data.def |  2 +-
 target/s390x/mem_helper.c  | 32 ++++++++------------------------
 target/s390x/translate.c   | 42 ++++++++++++++++++++++++++++++++++++++----
 4 files changed, 48 insertions(+), 30 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 2b4e7be..a2e0bf2 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -107,13 +107,13 @@ DEF_HELPER_FLAGS_2(tprot, TCG_CALL_NO_RWG, i32, i64, i64)
 DEF_HELPER_FLAGS_2(iske, TCG_CALL_NO_RWG_SE, i64, env, i64)
 DEF_HELPER_FLAGS_3(sske, TCG_CALL_NO_RWG, void, env, i64, i64)
 DEF_HELPER_FLAGS_2(rrbe, TCG_CALL_NO_RWG, i32, env, i64)
-DEF_HELPER_3(csp, i32, env, i32, i64)
 DEF_HELPER_4(mvcs, i32, env, i64, i64, i64)
 DEF_HELPER_4(mvcp, i32, env, i64, i64, i64)
 DEF_HELPER_4(sigp, i32, env, i64, i32, i64)
 DEF_HELPER_FLAGS_2(sacf, TCG_CALL_NO_WG, void, env, i64)
 DEF_HELPER_FLAGS_3(ipte, TCG_CALL_NO_RWG, void, env, i64, i64)
 DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
+DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_2(lra, i64, env, i64)
 DEF_HELPER_FLAGS_2(lura, TCG_CALL_NO_WG, i64, env, i64)
 DEF_HELPER_FLAGS_2(lurag, TCG_CALL_NO_WG, i64, env, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 3c3541c..4c91f30 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -837,7 +837,7 @@
 
 #ifndef CONFIG_USER_ONLY
 /* COMPARE AND SWAP AND PURGE */
-    C(0xb250, CSP,     RRE,   Z,   0, ra2, 0, 0, csp, 0)
+    D(0xb250, CSP,     RRE,   Z,   r1_32u, ra2, r1_P, 0, csp, 0, MO_TEUL)
 /* DIAGNOSE (KVM hypercall) */
     C(0x8300, DIAG,    RSI,   Z,   0, 0, 0, 0, diag, 0)
 /* INSERT STORAGE KEY EXTENDED */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 81b27c0..4becc80 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1029,30 +1029,6 @@ uint32_t HELPER(rrbe)(CPUS390XState *env, uint64_t r2)
     return re >> 1;
 }
 
-/* compare and swap and purge */
-uint32_t HELPER(csp)(CPUS390XState *env, uint32_t r1, uint64_t r2)
-{
-    S390CPU *cpu = s390_env_get_cpu(env);
-    uint32_t cc;
-    uint32_t o1 = env->regs[r1];
-    uint64_t a2 = r2 & ~3ULL;
-    uint32_t o2 = cpu_ldl_data(env, a2);
-
-    if (o1 == o2) {
-        cpu_stl_data(env, a2, env->regs[(r1 + 1) & 15]);
-        if (r2 & 0x3) {
-            /* flush TLB / ALB */
-            tlb_flush(CPU(cpu));
-        }
-        cc = 0;
-    } else {
-        env->regs[r1] = (env->regs[r1] & 0xffffffff00000000ULL) | o2;
-        cc = 1;
-    }
-
-    return cc;
-}
-
 uint32_t HELPER(mvcs)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
 {
     int cc = 0, i;
@@ -1130,6 +1106,14 @@ void HELPER(ptlb)(CPUS390XState *env)
     tlb_flush(CPU(cpu));
 }
 
+/* flush global tlb */
+void HELPER(purge)(CPUS390XState *env)
+{
+    S390CPU *cpu = s390_env_get_cpu(env);
+
+    tlb_flush_all_cpus(CPU(cpu));
+}
+
 /* load using real address */
 uint64_t HELPER(lura)(CPUS390XState *env, uint64_t addr)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 3e4b397..ca5be7b 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2003,11 +2003,45 @@ static ExitStatus op_cdsg(DisasContext *s, DisasOps *o)
 #ifndef CONFIG_USER_ONLY
 static ExitStatus op_csp(DisasContext *s, DisasOps *o)
 {
-    TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
+    TCGMemOp mop = s->insn->data;
+    TCGv_i64 addr, old, cc;
+    TCGLabel *lab = gen_new_label();
+
+    /* Note that in1 = R1 (zero-extended expected value),
+       out = R1 (original reg), out2 = R1+1 (new value).  */
+
     check_privileged(s);
-    gen_helper_csp(cc_op, cpu_env, r1, o->in2);
-    tcg_temp_free_i32(r1);
-    set_cc_static(s);
+    addr = tcg_temp_new_i64();
+    old = tcg_temp_new_i64();
+    tcg_gen_andi_i64(addr, o->in2, -1ULL << (mop & MO_SIZE));
+    tcg_gen_atomic_cmpxchg_i64(old, addr, o->in1, o->out2,
+                               get_mem_index(s), mop | MO_ALIGN);
+    tcg_temp_free_i64(addr);
+
+    /* Are the memory and expected values (un)equal?  */
+    cc = tcg_temp_new_i64();
+    tcg_gen_setcond_i64(TCG_COND_NE, cc, o->in1, old);
+    tcg_gen_extrl_i64_i32(cc_op, cc);
+
+    /* Write back the output now, so that it happens before the
+       following branch, so that we don't need local temps.  */
+    if ((mop & MO_SIZE) == MO_32) {
+        tcg_gen_deposit_i64(o->out, o->out, old, 0, 32);
+    } else {
+        tcg_gen_mov_i64(o->out, old);
+    }
+    tcg_temp_free_i64(old);
+
+    /* If the comparison was equal, and the LSB of R2 was set,
+       then we need to flush the TLB (for all cpus).  */
+    tcg_gen_xori_i64(cc, cc, 1);
+    tcg_gen_and_i64(cc, cc, o->in2);
+    tcg_gen_brcondi_i64(TCG_COND_EQ, cc, 0, lab);
+    tcg_temp_free_i64(cc);
+
+    gen_helper_purge(cpu_env);
+    gen_set_label(lab);
+
     return NO_EXIT;
 }
 #endif
-- 
2.9.4

Re: [Qemu-devel] [PATCH 29/31] target/s390x: Use atomic operations for COMPARE SWAP PURGE

[Aurelien Jarno]

On 2017-05-22 20:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/helper.h      |  2 +-
>  target/s390x/insn-data.def |  2 +-
>  target/s390x/mem_helper.c  | 32 ++++++++------------------------
>  target/s390x/translate.c   | 42 ++++++++++++++++++++++++++++++++++++++----
>  4 files changed, 48 insertions(+), 30 deletions(-)
> 
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index 2b4e7be..a2e0bf2 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -107,13 +107,13 @@ DEF_HELPER_FLAGS_2(tprot, TCG_CALL_NO_RWG, i32, i64, i64)
>  DEF_HELPER_FLAGS_2(iske, TCG_CALL_NO_RWG_SE, i64, env, i64)
>  DEF_HELPER_FLAGS_3(sske, TCG_CALL_NO_RWG, void, env, i64, i64)
>  DEF_HELPER_FLAGS_2(rrbe, TCG_CALL_NO_RWG, i32, env, i64)
> -DEF_HELPER_3(csp, i32, env, i32, i64)
>  DEF_HELPER_4(mvcs, i32, env, i64, i64, i64)
>  DEF_HELPER_4(mvcp, i32, env, i64, i64, i64)
>  DEF_HELPER_4(sigp, i32, env, i64, i32, i64)
>  DEF_HELPER_FLAGS_2(sacf, TCG_CALL_NO_WG, void, env, i64)
>  DEF_HELPER_FLAGS_3(ipte, TCG_CALL_NO_RWG, void, env, i64, i64)
>  DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
> +DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
>  DEF_HELPER_2(lra, i64, env, i64)
>  DEF_HELPER_FLAGS_2(lura, TCG_CALL_NO_WG, i64, env, i64)
>  DEF_HELPER_FLAGS_2(lurag, TCG_CALL_NO_WG, i64, env, i64)
> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
> index 3c3541c..4c91f30 100644
> --- a/target/s390x/insn-data.def
> +++ b/target/s390x/insn-data.def
> @@ -837,7 +837,7 @@
>  
>  #ifndef CONFIG_USER_ONLY
>  /* COMPARE AND SWAP AND PURGE */
> -    C(0xb250, CSP,     RRE,   Z,   0, ra2, 0, 0, csp, 0)
> +    D(0xb250, CSP,     RRE,   Z,   r1_32u, ra2, r1_P, 0, csp, 0, MO_TEUL)
>  /* DIAGNOSE (KVM hypercall) */
>      C(0x8300, DIAG,    RSI,   Z,   0, 0, 0, 0, diag, 0)
>  /* INSERT STORAGE KEY EXTENDED */
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 81b27c0..4becc80 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -1029,30 +1029,6 @@ uint32_t HELPER(rrbe)(CPUS390XState *env, uint64_t r2)
>      return re >> 1;
>  }
>  
> -/* compare and swap and purge */
> -uint32_t HELPER(csp)(CPUS390XState *env, uint32_t r1, uint64_t r2)
> -{
> -    S390CPU *cpu = s390_env_get_cpu(env);
> -    uint32_t cc;
> -    uint32_t o1 = env->regs[r1];
> -    uint64_t a2 = r2 & ~3ULL;
> -    uint32_t o2 = cpu_ldl_data(env, a2);
> -
> -    if (o1 == o2) {
> -        cpu_stl_data(env, a2, env->regs[(r1 + 1) & 15]);
> -        if (r2 & 0x3) {
> -            /* flush TLB / ALB */
> -            tlb_flush(CPU(cpu));
> -        }
> -        cc = 0;
> -    } else {
> -        env->regs[r1] = (env->regs[r1] & 0xffffffff00000000ULL) | o2;
> -        cc = 1;
> -    }
> -
> -    return cc;
> -}
> -
>  uint32_t HELPER(mvcs)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
>  {
>      int cc = 0, i;
> @@ -1130,6 +1106,14 @@ void HELPER(ptlb)(CPUS390XState *env)
>      tlb_flush(CPU(cpu));
>  }
>  
> +/* flush global tlb */
> +void HELPER(purge)(CPUS390XState *env)
> +{
> +    S390CPU *cpu = s390_env_get_cpu(env);
> +
> +    tlb_flush_all_cpus(CPU(cpu));

From what I understand from the PoP, the instruction should not complete
before the TLB has been purged on all CPUs. Therefore I guess
tlb_flush_all_cpus_synced() should be used instead.

> +}
> +
>  /* load using real address */
>  uint64_t HELPER(lura)(CPUS390XState *env, uint64_t addr)
>  {
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 3e4b397..ca5be7b 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -2003,11 +2003,45 @@ static ExitStatus op_cdsg(DisasContext *s, DisasOps *o)
>  #ifndef CONFIG_USER_ONLY
>  static ExitStatus op_csp(DisasContext *s, DisasOps *o)
>  {
> -    TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
> +    TCGMemOp mop = s->insn->data;
> +    TCGv_i64 addr, old, cc;
> +    TCGLabel *lab = gen_new_label();
> +
> +    /* Note that in1 = R1 (zero-extended expected value),
> +       out = R1 (original reg), out2 = R1+1 (new value).  */
> +
>      check_privileged(s);
> -    gen_helper_csp(cc_op, cpu_env, r1, o->in2);
> -    tcg_temp_free_i32(r1);
> -    set_cc_static(s);
> +    addr = tcg_temp_new_i64();
> +    old = tcg_temp_new_i64();
> +    tcg_gen_andi_i64(addr, o->in2, -1ULL << (mop & MO_SIZE));
> +    tcg_gen_atomic_cmpxchg_i64(old, addr, o->in1, o->out2,

Here the prep generator took the 32-bit version of in1. I guess the same
should be done for out2.

> +                               get_mem_index(s), mop | MO_ALIGN);
> +    tcg_temp_free_i64(addr);
> +
> +    /* Are the memory and expected values (un)equal?  */
> +    cc = tcg_temp_new_i64();
> +    tcg_gen_setcond_i64(TCG_COND_NE, cc, o->in1, old);
> +    tcg_gen_extrl_i64_i32(cc_op, cc);
> +
> +    /* Write back the output now, so that it happens before the
> +       following branch, so that we don't need local temps.  */
> +    if ((mop & MO_SIZE) == MO_32) {
> +        tcg_gen_deposit_i64(o->out, o->out, old, 0, 32);
> +    } else {
> +        tcg_gen_mov_i64(o->out, old);
> +    }
> +    tcg_temp_free_i64(old);
> +
> +    /* If the comparison was equal, and the LSB of R2 was set,
> +       then we need to flush the TLB (for all cpus).  */
> +    tcg_gen_xori_i64(cc, cc, 1);
> +    tcg_gen_and_i64(cc, cc, o->in2);
> +    tcg_gen_brcondi_i64(TCG_COND_EQ, cc, 0, lab);
> +    tcg_temp_free_i64(cc);
> +
> +    gen_helper_purge(cpu_env);
> +    gen_set_label(lab);
> +
>      return NO_EXIT;
>  }
>  #endif
> -- 
> 2.9.4
> 
> 
> 

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

Re: [Qemu-devel] [PATCH 29/31] target/s390x: Use atomic operations for COMPARE SWAP PURGE

[Richard Henderson]

On 05/23/2017 05:28 AM, Aurelien Jarno wrote:
> On 2017-05-22 20:03, Richard Henderson wrote:
>> +/* flush global tlb */
>> +void HELPER(purge)(CPUS390XState *env)
>> +{
>> +    S390CPU *cpu = s390_env_get_cpu(env);
>> +
>> +    tlb_flush_all_cpus(CPU(cpu));
> 
>>From what I understand from the PoP, the instruction should not complete
> before the TLB has been purged on all CPUs. Therefore I guess
> tlb_flush_all_cpus_synced() should be used instead.
I don't read that from this:

# (1) all specified entries have been cleared
# from the ALB and TLB of this CPU and

# (2) all other
# CPUs in the configuration have completed any stor-
# age accesses, including the updating of the change
# and reference bits, by using the specified ALB and
# TLB entries.

It talks about referenced bits being updated -- presumably before the tlb entry 
is flushed.  But it doesn't say "all specified ALB and TLB entries of other 
CPUs in the configuration".

But if you still disagree, it's certainly an easy change as you note.


>> +    tcg_gen_atomic_cmpxchg_i64(old, addr, o->in1, o->out2,
> 
> Here the prep generator took the 32-bit version of in1. I guess the same
> should be done for out2.

No, in1 is zero-extended for its use ...

> 
>> +                               get_mem_index(s), mop | MO_ALIGN);
>> +    tcg_temp_free_i64(addr);
>> +
>> +    /* Are the memory and expected values (un)equal?  */
>> +    cc = tcg_temp_new_i64();
>> +    tcg_gen_setcond_i64(TCG_COND_NE, cc, o->in1, old);

... here.

For out2 above, cmpxchg acts as any other store wrt MO_TEUL, in that it ignores 
the unused upper bits.


r~

Re: [Qemu-devel] [PATCH 29/31] target/s390x: Use atomic operations for COMPARE SWAP PURGE

[Aurelien Jarno]

On 2017-05-23 09:31, Richard Henderson wrote:
> On 05/23/2017 05:28 AM, Aurelien Jarno wrote:
> > On 2017-05-22 20:03, Richard Henderson wrote:
> > > +/* flush global tlb */
> > > +void HELPER(purge)(CPUS390XState *env)
> > > +{
> > > +    S390CPU *cpu = s390_env_get_cpu(env);
> > > +
> > > +    tlb_flush_all_cpus(CPU(cpu));
> > 
> > > From what I understand from the PoP, the instruction should not complete
> > before the TLB has been purged on all CPUs. Therefore I guess
> > tlb_flush_all_cpus_synced() should be used instead.
> I don't read that from this:
> 
> # (1) all specified entries have been cleared
> # from the ALB and TLB of this CPU and
> 
> # (2) all other
> # CPUs in the configuration have completed any stor-
> # age accesses, including the updating of the change
> # and reference bits, by using the specified ALB and
> # TLB entries.
> 
> It talks about referenced bits being updated -- presumably before the tlb
> entry is flushed.  But it doesn't say "all specified ALB and TLB entries of
> other CPUs in the configuration".
> 
> But if you still disagree, it's certainly an easy change as you note.

Well i have to say it's not very clear. My point is that given the way
QEMU model things, if we want to ensure that all storage accesses using
the specified TLB entries are completed, we currently can only just make
sure that the all TLB entries have been flushed.
 
> > > +    tcg_gen_atomic_cmpxchg_i64(old, addr, o->in1, o->out2,
> > 
> > Here the prep generator took the 32-bit version of in1. I guess the same
> > should be done for out2.
> 
> No, in1 is zero-extended for its use ...
> 
> > 
> > > +                               get_mem_index(s), mop | MO_ALIGN);
> > > +    tcg_temp_free_i64(addr);
> > > +
> > > +    /* Are the memory and expected values (un)equal?  */
> > > +    cc = tcg_temp_new_i64();
> > > +    tcg_gen_setcond_i64(TCG_COND_NE, cc, o->in1, old);
> 
> ... here.
> 
> For out2 above, cmpxchg acts as any other store wrt MO_TEUL, in that it
> ignores the unused upper bits.

Indeed you are correct, I read it too fast.

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

[Qemu-devel] [PATCH 30/31] target/s390x: Implement CSPG

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/insn-data.def | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 4c91f30..8604847 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -838,6 +838,7 @@
 #ifndef CONFIG_USER_ONLY
 /* COMPARE AND SWAP AND PURGE */
     D(0xb250, CSP,     RRE,   Z,   r1_32u, ra2, r1_P, 0, csp, 0, MO_TEUL)
+    D(0xb98a, CSPG,    RRE,   Z,   r1_o, ra2, r1_P, 0, csp, 0, MO_TEQ)
 /* DIAGNOSE (KVM hypercall) */
     C(0x8300, DIAG,    RSI,   Z,   0, 0, 0, 0, diag, 0)
 /* INSERT STORAGE KEY EXTENDED */
-- 
2.9.4

Re: [Qemu-devel] [PATCH 30/31] target/s390x: Implement CSPG

[Aurelien Jarno]

On 2017-05-22 20:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/insn-data.def | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
> index 4c91f30..8604847 100644
> --- a/target/s390x/insn-data.def
> +++ b/target/s390x/insn-data.def
> @@ -838,6 +838,7 @@
>  #ifndef CONFIG_USER_ONLY
>  /* COMPARE AND SWAP AND PURGE */
>      D(0xb250, CSP,     RRE,   Z,   r1_32u, ra2, r1_P, 0, csp, 0, MO_TEUL)
> +    D(0xb98a, CSPG,    RRE,   Z,   r1_o, ra2, r1_P, 0, csp, 0, MO_TEQ)

CSPG is part of the of the DAT-enhancement facility. I called it DAT_ENH
in my local patches to match the name we have in the CPU features.

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

Re: [Qemu-devel] [PATCH 30/31] target/s390x: Implement CSPG

[Richard Henderson]

On 05/23/2017 04:12 AM, Aurelien Jarno wrote:
> On 2017-05-22 20:03, Richard Henderson wrote:
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>> ---
>>   target/s390x/insn-data.def | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
>> index 4c91f30..8604847 100644
>> --- a/target/s390x/insn-data.def
>> +++ b/target/s390x/insn-data.def
>> @@ -838,6 +838,7 @@
>>   #ifndef CONFIG_USER_ONLY
>>   /* COMPARE AND SWAP AND PURGE */
>>       D(0xb250, CSP,     RRE,   Z,   r1_32u, ra2, r1_P, 0, csp, 0, MO_TEUL)
>> +    D(0xb98a, CSPG,    RRE,   Z,   r1_o, ra2, r1_P, 0, csp, 0, MO_TEQ)
> 
> CSPG is part of the of the DAT-enhancement facility. I called it DAT_ENH
> in my local patches to match the name we have in the CPU features.

The translator needs a large overhaul to bring it into line with the (more 
recently added) facilities infrastructure.  We don't currently enforce anything.

But you're also right that I shouldn't just ignore the issue and leave it 
marked incorrectly.


r~

Re: [Qemu-devel] [PATCH 30/31] target/s390x: Implement CSPG

[Aurelien Jarno]

On 2017-05-23 09:33, Richard Henderson wrote:
> On 05/23/2017 04:12 AM, Aurelien Jarno wrote:
> > On 2017-05-22 20:03, Richard Henderson wrote:
> > > Signed-off-by: Richard Henderson <rth@twiddle.net>
> > > ---
> > >   target/s390x/insn-data.def | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
> > > index 4c91f30..8604847 100644
> > > --- a/target/s390x/insn-data.def
> > > +++ b/target/s390x/insn-data.def
> > > @@ -838,6 +838,7 @@
> > >   #ifndef CONFIG_USER_ONLY
> > >   /* COMPARE AND SWAP AND PURGE */
> > >       D(0xb250, CSP,     RRE,   Z,   r1_32u, ra2, r1_P, 0, csp, 0, MO_TEUL)
> > > +    D(0xb98a, CSPG,    RRE,   Z,   r1_o, ra2, r1_P, 0, csp, 0, MO_TEQ)
> > 
> > CSPG is part of the of the DAT-enhancement facility. I called it DAT_ENH
> > in my local patches to match the name we have in the CPU features.
> 
> The translator needs a large overhaul to bring it into line with the (more
> recently added) facilities infrastructure.  We don't currently enforce
> anything.

Agreed. That said it's actually useful to quickly check if all the
instructions of a given facility have been implemented and then flip the
corresponding facility bit.

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

[Qemu-devel] [PATCH 31/31] target/s390x: Use unwind data for helper_mvcs/mvcp

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/mem_helper.c | 8 ++++++--
 target/s390x/translate.c  | 2 --
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 4becc80..d8d29bd 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1031,6 +1031,7 @@ uint32_t HELPER(rrbe)(CPUS390XState *env, uint64_t r2)
 
 uint32_t HELPER(mvcs)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
 {
+    uintptr_t ra = GETPC();
     int cc = 0, i;
 
     HELPER_LOG("%s: %16" PRIx64 " %16" PRIx64 " %16" PRIx64 "\n",
@@ -1044,7 +1045,8 @@ uint32_t HELPER(mvcs)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
 
     /* XXX replace w/ memcpy */
     for (i = 0; i < l; i++) {
-        cpu_stb_secondary(env, a1 + i, cpu_ldub_primary(env, a2 + i));
+        uint8_t x = cpu_ldub_primary_ra(env, a2 + i, ra);
+        cpu_stb_secondary_ra(env, a1 + i, x, ra);
     }
 
     return cc;
@@ -1052,6 +1054,7 @@ uint32_t HELPER(mvcs)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
 
 uint32_t HELPER(mvcp)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
 {
+    uintptr_t ra = GETPC();
     int cc = 0, i;
 
     HELPER_LOG("%s: %16" PRIx64 " %16" PRIx64 " %16" PRIx64 "\n",
@@ -1065,7 +1068,8 @@ uint32_t HELPER(mvcp)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
 
     /* XXX replace w/ memcpy */
     for (i = 0; i < l; i++) {
-        cpu_stb_primary(env, a1 + i, cpu_ldub_secondary(env, a2 + i));
+        uint8_t x = cpu_ldub_secondary_ra(env, a2 + i, ra);
+        cpu_stb_primary_ra(env, a1 + i, x, ra);
     }
 
     return cc;
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index ca5be7b..67c85f0 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2928,7 +2928,6 @@ static ExitStatus op_mvcp(DisasContext *s, DisasOps *o)
 {
     int r1 = get_field(s->fields, l1);
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_mvcp(cc_op, cpu_env, regs[r1], o->addr1, o->in2);
     set_cc_static(s);
     return NO_EXIT;
@@ -2938,7 +2937,6 @@ static ExitStatus op_mvcs(DisasContext *s, DisasOps *o)
 {
     int r1 = get_field(s->fields, l1);
     check_privileged(s);
-    potential_page_fault(s);
     gen_helper_mvcs(cc_op, cpu_env, regs[r1], o->addr1, o->in2);
     set_cc_static(s);
     return NO_EXIT;
-- 
2.9.4

Re: [Qemu-devel] [PATCH 31/31] target/s390x: Use unwind data for helper_mvcs/mvcp

[Thomas Huth]

On 23.05.2017 05:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 8 ++++++--
>  target/s390x/translate.c  | 2 --
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 4becc80..d8d29bd 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -1031,6 +1031,7 @@ uint32_t HELPER(rrbe)(CPUS390XState *env, uint64_t r2)
>  
>  uint32_t HELPER(mvcs)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
>  {
> +    uintptr_t ra = GETPC();
>      int cc = 0, i;
>  
>      HELPER_LOG("%s: %16" PRIx64 " %16" PRIx64 " %16" PRIx64 "\n",
> @@ -1044,7 +1045,8 @@ uint32_t HELPER(mvcs)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
>  
>      /* XXX replace w/ memcpy */
>      for (i = 0; i < l; i++) {
> -        cpu_stb_secondary(env, a1 + i, cpu_ldub_primary(env, a2 + i));
> +        uint8_t x = cpu_ldub_primary_ra(env, a2 + i, ra);
> +        cpu_stb_secondary_ra(env, a1 + i, x, ra);
>      }
>  
>      return cc;
> @@ -1052,6 +1054,7 @@ uint32_t HELPER(mvcs)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
>  
>  uint32_t HELPER(mvcp)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
>  {
> +    uintptr_t ra = GETPC();
>      int cc = 0, i;
>  
>      HELPER_LOG("%s: %16" PRIx64 " %16" PRIx64 " %16" PRIx64 "\n",
> @@ -1065,7 +1068,8 @@ uint32_t HELPER(mvcp)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
>  
>      /* XXX replace w/ memcpy */
>      for (i = 0; i < l; i++) {
> -        cpu_stb_primary(env, a1 + i, cpu_ldub_secondary(env, a2 + i));
> +        uint8_t x = cpu_ldub_secondary_ra(env, a2 + i, ra);
> +        cpu_stb_primary_ra(env, a1 + i, x, ra);
>      }
>  
>      return cc;
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index ca5be7b..67c85f0 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -2928,7 +2928,6 @@ static ExitStatus op_mvcp(DisasContext *s, DisasOps *o)
>  {
>      int r1 = get_field(s->fields, l1);
>      check_privileged(s);
> -    potential_page_fault(s);
>      gen_helper_mvcp(cc_op, cpu_env, regs[r1], o->addr1, o->in2);
>      set_cc_static(s);
>      return NO_EXIT;
> @@ -2938,7 +2937,6 @@ static ExitStatus op_mvcs(DisasContext *s, DisasOps *o)
>  {
>      int r1 = get_field(s->fields, l1);
>      check_privileged(s);
> -    potential_page_fault(s);
>      gen_helper_mvcs(cc_op, cpu_env, regs[r1], o->addr1, o->in2);
>      set_cc_static(s);
>      return NO_EXIT;
> 

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

Re: [Qemu-devel] [PATCH 31/31] target/s390x: Use unwind data for helper_mvcs/mvcp

[Aurelien Jarno]

On 2017-05-22 20:03, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/mem_helper.c | 8 ++++++--
>  target/s390x/translate.c  | 2 --
>  2 files changed, 6 insertions(+), 4 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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