From: Eric Bischoff <ebischoff@nerim.net>

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Eric Bischoff <ebischoff@nerim.net>
Message-Id: <20170228120134.7921-1-ebischoff@suse.com>
[rth: Combine the two via insn->data; free the address temps.]
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target/s390x/insn-data.def |  4 +++-
 target/s390x/translate.c   | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 43c5707..0909060 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -504,7 +504,9 @@
     C(0xb9e2, LOCGR,   RRF_c, LOC, r1, r2, r1, 0, loc, 0)
     C(0xebf2, LOC,     RSY_b, LOC, r1, m2_32u, new, r1_32, loc, 0)
     C(0xebe2, LOCG,    RSY_b, LOC, r1, m2_64, r1, 0, loc, 0)
-/* LOAD PAIR DISJOINT TODO */
+/* LOAD PAIR DISJOINT */
+    D(0xc804, LPD,     SSF,   ILA, 0, 0, new_P, r3_P32, lpd, 0, MO_TEUL)
+    D(0xc805, LPDG,    SSF,   ILA, 0, 0, new_P, r3_P64, lpd, 0, MO_TEQ)
 /* LOAD POSITIVE */
     C(0x1000, LPR,     RR_a,  Z,   0, r2_32s, new, r1_32, abs, abs32)
     C(0xb900, LPGR,    RRE,   Z,   0, r2, r1, 0, abs, abs64)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 2b66a4e..8de0177 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2559,6 +2559,7 @@ static ExitStatus op_lctlg(DisasContext *s, DisasOps *o)
     tcg_temp_free_i32(r3);
     return NO_EXIT;
 }
+
 static ExitStatus op_lra(DisasContext *s, DisasOps *o)
 {
     check_privileged(s);
@@ -2759,6 +2760,31 @@ static ExitStatus op_lm64(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_lpd(DisasContext *s, DisasOps *o)
+{
+    TCGv_i64 a1, a2;
+    TCGMemOp mop = s->insn->data;
+
+    /* In a parallel context, stop the world and single step.  */
+    if (parallel_cpus) {
+        potential_page_fault(s);
+        gen_helper_exit_atomic(cpu_env);
+        return EXIT_NORETURN;
+    }
+
+    /* In a serial context, perform the two loads ... */
+    a1 = get_address(s, 0, get_field(s->fields, b1), get_field(s->fields, d1));
+    a2 = get_address(s, 0, get_field(s->fields, b2), get_field(s->fields, d2));
+    tcg_gen_qemu_ld_i64(o->out, a1, get_mem_index(s), mop | MO_ALIGN);
+    tcg_gen_qemu_ld_i64(o->out2, a2, get_mem_index(s), mop | MO_ALIGN);
+    tcg_temp_free_i64(a1);
+    tcg_temp_free_i64(a2);
+
+    /* ... and indicate that we performed them while interlocked.  */
+    gen_op_movi_cc(s, 0);
+    return NO_EXIT;
+}
+
 #ifndef CONFIG_USER_ONLY
 static ExitStatus op_lura(DisasContext *s, DisasOps *o)
 {
@@ -4430,6 +4456,22 @@ static void wout_r1_D32(DisasContext *s, DisasFields *f, DisasOps *o)
 }
 #define SPEC_wout_r1_D32 SPEC_r1_even
 
+static void wout_r3_P32(DisasContext *s, DisasFields *f, DisasOps *o)
+{
+    int r3 = get_field(f, r3);
+    store_reg32_i64(r3, o->out);
+    store_reg32_i64(r3 + 1, o->out2);
+}
+#define SPEC_wout_r3_P32 SPEC_r3_even
+
+static void wout_r3_P64(DisasContext *s, DisasFields *f, DisasOps *o)
+{
+    int r3 = get_field(f, r3);
+    store_reg(r3, o->out);
+    store_reg(r3 + 1, o->out2);
+}
+#define SPEC_wout_r3_P64 SPEC_r3_even
+
 static void wout_e1(DisasContext *s, DisasFields *f, DisasOps *o)
 {
     store_freg32_i64(get_field(f, r1), o->out);
-- 
2.9.3

On 2017-05-09 11:07, Richard Henderson wrote:
> From: Eric Bischoff <ebischoff@nerim.net>
> 
> Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
> Signed-off-by: Eric Bischoff <ebischoff@nerim.net>
> Message-Id: <20170228120134.7921-1-ebischoff@suse.com>
> [rth: Combine the two via insn->data; free the address temps.]
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/s390x/insn-data.def |  4 +++-
>  target/s390x/translate.c   | 42 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+), 1 deletion(-)

[snip]

> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 2b66a4e..8de0177 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -2559,6 +2559,7 @@ static ExitStatus op_lctlg(DisasContext *s, DisasOps *o)
>      tcg_temp_free_i32(r3);
>      return NO_EXIT;
>  }
> +
>  static ExitStatus op_lra(DisasContext *s, DisasOps *o)
>  {
>      check_privileged(s);
> @@ -2759,6 +2760,31 @@ static ExitStatus op_lm64(DisasContext *s, DisasOps *o)
>      return NO_EXIT;
>  }
>  
> +static ExitStatus op_lpd(DisasContext *s, DisasOps *o)
> +{
> +    TCGv_i64 a1, a2;
> +    TCGMemOp mop = s->insn->data;
> +
> +    /* In a parallel context, stop the world and single step.  */
> +    if (parallel_cpus) {
> +        potential_page_fault(s);
> +        gen_helper_exit_atomic(cpu_env);
> +        return EXIT_NORETURN;
> +    }

One small additional comment about this patch I haven't spotted at the
first review. The exit_atomic helper is properly restoring the CPU state
passing the return address to cpu_loop_exit_atomic, so I believe the
potential_page_fault call is not necessary. That said, it doesn't hurt
either.

Aurelien

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

Le mercredi 10 mai 2017, 12:16:20 Aurelien Jarno a écrit :
> > +    /* In a parallel context, stop the world and single step.  */
> > +    if (parallel_cpus) {
> > +        potential_page_fault(s);
> > +        gen_helper_exit_atomic(cpu_env);
> > +        return EXIT_NORETURN;
> > +    }
> 
> One small additional comment about this patch I haven't spotted at the
> first review. The exit_atomic helper is properly restoring the CPU state
> passing the return address to cpu_loop_exit_atomic, so I believe the
> potential_page_fault call is not necessary. That said, it doesn't hurt
> either.

Merci pour la relecture Aurélien.

Richard, what do we do? We remove the potential_page_fault(s); or not?


-- 
Éric Bischoff

On 05/10/2017 10:13 AM, Éric Bischoff wrote:
> Le mercredi 10 mai 2017, 12:16:20 Aurelien Jarno a écrit :
>>> +    /* In a parallel context, stop the world and single step.  */
>>> +    if (parallel_cpus) {
>>> +        potential_page_fault(s);
>>> +        gen_helper_exit_atomic(cpu_env);
>>> +        return EXIT_NORETURN;
>>> +    }
>>
>> One small additional comment about this patch I haven't spotted at the
>> first review. The exit_atomic helper is properly restoring the CPU state
>> passing the return address to cpu_loop_exit_atomic, so I believe the
>> potential_page_fault call is not necessary. That said, it doesn't hurt
>> either.
> 
> Merci pour la relecture Aurélien.
> 
> Richard, what do we do? We remove the potential_page_fault(s); or not?

I'm thinking of using gen_exception(EXCP_ATOMIC) instead.
The unwind associated with the regular helper_exit_atomic
has more overhead than potential_page_fault().


r~

On 2017-05-10 10:43, Richard Henderson wrote:
> On 05/10/2017 10:13 AM, Éric Bischoff wrote:
> > Le mercredi 10 mai 2017, 12:16:20 Aurelien Jarno a écrit :
> > > > +    /* In a parallel context, stop the world and single step.  */
> > > > +    if (parallel_cpus) {
> > > > +        potential_page_fault(s);
> > > > +        gen_helper_exit_atomic(cpu_env);
> > > > +        return EXIT_NORETURN;
> > > > +    }
> > > 
> > > One small additional comment about this patch I haven't spotted at the
> > > first review. The exit_atomic helper is properly restoring the CPU state
> > > passing the return address to cpu_loop_exit_atomic, so I believe the
> > > potential_page_fault call is not necessary. That said, it doesn't hurt
> > > either.
> > 
> > Merci pour la relecture Aurélien.
> > 
> > Richard, what do we do? We remove the potential_page_fault(s); or not?
> 
> I'm thinking of using gen_exception(EXCP_ATOMIC) instead.
> The unwind associated with the regular helper_exit_atomic
> has more overhead than potential_page_fault().

That was just a remark to optimize the code a bit. That said I think the
current code can go like that, it is not wrong.

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