[v2,for-2.12,01/10] s390x/tcg: ASI/ASGI are atomic with interlocked-acccess facility 1

Message ID 20171207165355.7559-2-david@redhat.com
State New
Headers show
Series
  • s390x/tcg: facilitites and instructions
Related show

Commit Message

David Hildenbrand Dec. 7, 2017, 4:53 p.m.
The semantics of these operations changed. Let's implement them just
like LOAD AND ADD, so they are atomic.

This fixes random crashes when booting a Linux kernel compiled for
z196+ with SMP + MTTCG.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/insn-data.def |  4 ++--
 target/s390x/translate.c   | 11 +++++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

Comments

Richard Henderson Dec. 7, 2017, 11:41 p.m. | #1
On 12/07/2017 08:53 AM, David Hildenbrand wrote:
> +static ExitStatus op_asi(DisasContext *s, DisasOps *o)
> +{
> +    o->in1 = tcg_temp_new_i64();
> +    /* Perform the atomic addition in memory. */
> +    tcg_gen_atomic_fetch_add_i64(o->in1, o->addr1, o->in2, get_mem_index(s),
> +                                 s->insn->data);
> +    /* However, we need to recompute the addition for setting CC.  */
> +    tcg_gen_add_i64(o->out, o->in1, o->in2);
> +    return NO_EXIT;
> +}

Is it worth conditionalizing the atomic operation on having
interlocked-access-facility-1 enabled?


r~
David Hildenbrand Dec. 8, 2017, 12:51 p.m. | #2
On 08.12.2017 00:41, Richard Henderson wrote:
> On 12/07/2017 08:53 AM, David Hildenbrand wrote:
>> +static ExitStatus op_asi(DisasContext *s, DisasOps *o)
>> +{
>> +    o->in1 = tcg_temp_new_i64();
>> +    /* Perform the atomic addition in memory. */
>> +    tcg_gen_atomic_fetch_add_i64(o->in1, o->addr1, o->in2, get_mem_index(s),
>> +                                 s->insn->data);
>> +    /* However, we need to recompute the addition for setting CC.  */
>> +    tcg_gen_add_i64(o->out, o->in1, o->in2);
>> +    return NO_EXIT;
>> +}
> 
> Is it worth conditionalizing the atomic operation on having
> interlocked-access-facility-1 enabled?
> 

Should be easily doable with a few LOC. Will give it a shot.

> 
> r~
>

Patch

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 43ab1963c8..57f2e5133f 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -39,10 +39,10 @@ 
     C(0xb9d8, AHHLR,   RRF_a, HW,  r2_sr32, r3, new, r1_32h, add, adds32)
 /* ADD IMMEDIATE */
     C(0xc209, AFI,     RIL_a, EI,  r1, i2, new, r1_32, add, adds32)
-    C(0xeb6a, ASI,     SIY,   GIE, m1_32s, i2, new, m1_32, add, adds32)
+    D(0xeb6a, ASI,     SIY,   GIE, la1, i2, new, 0, asi, adds32, MO_TESL)
     C(0xecd8, AHIK,    RIE_d, DO,  r3, i2, new, r1_32, add, adds32)
     C(0xc208, AGFI,    RIL_a, EI,  r1, i2, r1, 0, add, adds64)
-    C(0xeb7a, AGSI,    SIY,   GIE, m1_64, i2, new, m1_64, add, adds64)
+    D(0xeb7a, AGSI,    SIY,   GIE, la1, i2, new, 0, asi, adds64, MO_TEQ)
     C(0xecd9, AGHIK,   RIE_d, DO,  r3, i2, r1, 0, add, adds64)
 /* ADD IMMEDIATE HIGH */
     C(0xcc08, AIH,     RIL_a, HW,  r1_sr32, i2, new, r1_32h, add, adds32)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 5e051fdd03..79d2ee650c 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1364,6 +1364,17 @@  static ExitStatus op_addc(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_asi(DisasContext *s, DisasOps *o)
+{
+    o->in1 = tcg_temp_new_i64();
+    /* Perform the atomic addition in memory. */
+    tcg_gen_atomic_fetch_add_i64(o->in1, o->addr1, o->in2, get_mem_index(s),
+                                 s->insn->data);
+    /* However, we need to recompute the addition for setting CC.  */
+    tcg_gen_add_i64(o->out, o->in1, o->in2);
+    return NO_EXIT;
+}
+
 static ExitStatus op_aeb(DisasContext *s, DisasOps *o)
 {
     gen_helper_aeb(o->out, cpu_env, o->in1, o->in2);