All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions
@ 2015-05-12 20:20 Alexander Graf
  2015-05-12 20:27 ` Richard Henderson
  0 siblings, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2015-05-12 20:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: rth

We're currently missing the laa and laag instructions in our emulation.
In fact, we're missing the complete "interlocked-access facility 1" which
is part of zEC12. However, I really only needed the laa instruction for now.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

This really should implement all the other atomic load&modify instructions,
but I'd like to make sure we have a smart scheme to implement them first.

v1 -> v2:

  - move atomic specific bits into load/store helpers, leave actual op
    as normal op we can reuse
---
 target-s390x/insn-data.def |  3 +++
 target-s390x/translate.c   | 48 ++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/target-s390x/insn-data.def b/target-s390x/insn-data.def
index 48e979e..cd95035 100644
--- a/target-s390x/insn-data.def
+++ b/target-s390x/insn-data.def
@@ -359,6 +359,9 @@
     C(0xe371, LAY,     RXY_a, LD,  0, a2, 0, r1, mov2, 0)
 /* LOAD ADDRESS RELATIVE LONG */
     C(0xc000, LARL,    RIL_b, Z,   0, ri2, 0, r1, mov2, 0)
+/* LOAD AND ADD */
+    C(0xebf8, LAA,     RSY_a, ILA, r3_32s, m2_32s_atomic, new, m2_32_r1_atomic, add, adds32)
+    C(0xebe8, LAAG,    RSY_a, ILA, r3, m2_64_atomic, new, m2_64_r1_atomic, add, adds64)
 /* LOAD AND TEST */
     C(0x1200, LTR,     RR_a,  Z,   0, r2_o, 0, cond_r1r2_32, mov2, s32)
     C(0xb902, LTGR,    RRE,   Z,   0, r2_o, 0, r1, mov2, s64)
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index fa3e334..73f2de3 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -1118,6 +1118,7 @@ typedef enum DisasFacility {
     FAC_PC,                 /* population count */
     FAC_SCF,                /* store clock fast */
     FAC_SFLE,               /* store facility list extended */
+    FAC_ILA,                /* interlocked access facility 1 */
 } DisasFacility;
 
 struct DisasInsn {
@@ -1297,6 +1298,12 @@ static ExitStatus help_branch(DisasContext *s, DisasCompare *c,
     return ret;
 }
 
+static TCGv_i64 get_a2(DisasContext *s, DisasFields *f)
+{
+    int x2 = have_field(f, x2) ? get_field(f, x2) : 0;
+    return get_address(s, x2, get_field(f, b2), get_field(f, d2));
+}
+
 /* ====================================================================== */
 /* The operations.  These perform the bulk of the work for any insn,
    usually after the operands have been loaded and output initialized.  */
@@ -4065,6 +4072,30 @@ static void wout_m2_32(DisasContext *s, DisasFields *f, DisasOps *o)
 }
 #define SPEC_wout_m2_32 0
 
+static void wout_m2_32_r1_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
+{
+    TCGv_i64 a2 = get_a2(s, f);
+
+    /* XXX release reservation */
+    tcg_gen_qemu_st32(o->out, a2, get_mem_index(s));
+    store_reg32_i64(get_field(f, r1), o->in2);
+
+    tcg_temp_free_i64(a2);
+}
+#define SPEC_wout_m2_32_r1_atomic 0
+
+static void wout_m2_64_r1_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
+{
+    TCGv_i64 a2 = get_a2(s, f);
+
+    /* XXX release reservation */
+    tcg_gen_qemu_st64(o->out, a2, get_mem_index(s));
+    store_reg(get_field(f, r1), o->in2);
+
+    tcg_temp_free_i64(a2);
+}
+#define SPEC_wout_m2_64_r1_atomic 0
+
 /* ====================================================================== */
 /* The "INput 1" generators.  These load the first operand to an insn.  */
 
@@ -4393,8 +4424,7 @@ static void in2_ra2(DisasContext *s, DisasFields *f, DisasOps *o)
 
 static void in2_a2(DisasContext *s, DisasFields *f, DisasOps *o)
 {
-    int x2 = have_field(f, x2) ? get_field(f, x2) : 0;
-    o->in2 = get_address(s, x2, get_field(f, b2), get_field(f, d2));
+    o->in2 = get_a2(s, f);
 }
 #define SPEC_in2_a2 0
 
@@ -4486,6 +4516,20 @@ static void in2_mri2_64(DisasContext *s, DisasFields *f, DisasOps *o)
 }
 #define SPEC_in2_mri2_64 0
 
+static void in2_m2_32s_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
+{
+    /* XXX should reserve the address */
+    in2_m2_32s(s, f, o);
+}
+#define SPEC_in2_m2_32s_atomic 0
+
+static void in2_m2_64_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
+{
+    /* XXX should reserve the address */
+    in2_m2_64(s, f, o);
+}
+#define SPEC_in2_m2_64_atomic 0
+
 static void in2_i2(DisasContext *s, DisasFields *f, DisasOps *o)
 {
     o->in2 = tcg_const_i64(get_field(f, i2));
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions
  2015-05-12 20:20 [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions Alexander Graf
@ 2015-05-12 20:27 ` Richard Henderson
  2015-05-12 20:42   ` Alexander Graf
  2015-05-12 20:46   ` Alexander Graf
  0 siblings, 2 replies; 6+ messages in thread
From: Richard Henderson @ 2015-05-12 20:27 UTC (permalink / raw)
  To: Alexander Graf, qemu-devel

On 05/12/2015 01:20 PM, Alexander Graf wrote:
> +static void in2_m2_32s_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
> +{
> +    /* XXX should reserve the address */
> +    in2_m2_32s(s, f, o);
> +}
> +#define SPEC_in2_m2_32s_atomic 0
> +
> +static void in2_m2_64_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
> +{
> +    /* XXX should reserve the address */
> +    in2_m2_64(s, f, o);
> +}
> +#define SPEC_in2_m2_64_atomic 0
> +

I think these should save the address in o->addr1 so that you don't have to
recompute it in the wout functions.

Otherwise I think this is a great scheme, making it easy to implement this
whole family of instructions.


r~

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions
  2015-05-12 20:27 ` Richard Henderson
@ 2015-05-12 20:42   ` Alexander Graf
  2015-05-12 21:15     ` Richard Henderson
  2015-05-12 20:46   ` Alexander Graf
  1 sibling, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2015-05-12 20:42 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

On 05/12/2015 10:27 PM, Richard Henderson wrote:
> On 05/12/2015 01:20 PM, Alexander Graf wrote:
>> +static void in2_m2_32s_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
>> +{
>> +    /* XXX should reserve the address */
>> +    in2_m2_32s(s, f, o);
>> +}
>> +#define SPEC_in2_m2_32s_atomic 0
>> +
>> +static void in2_m2_64_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
>> +{
>> +    /* XXX should reserve the address */
>> +    in2_m2_64(s, f, o);
>> +}
>> +#define SPEC_in2_m2_64_atomic 0
>> +
> I think these should save the address in o->addr1 so that you don't have to
> recompute it in the wout functions.

But wouldn't it really be "addr2"? This is the address source for the 
second argument after all.


Alex

>
> Otherwise I think this is a great scheme, making it easy to implement this
> whole family of instructions.
>
>
> r~

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions
  2015-05-12 20:27 ` Richard Henderson
  2015-05-12 20:42   ` Alexander Graf
@ 2015-05-12 20:46   ` Alexander Graf
  2015-05-12 21:19     ` Richard Henderson
  1 sibling, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2015-05-12 20:46 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

On 05/12/2015 10:27 PM, Richard Henderson wrote:
> On 05/12/2015 01:20 PM, Alexander Graf wrote:
>> +static void in2_m2_32s_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
>> +{
>> +    /* XXX should reserve the address */
>> +    in2_m2_32s(s, f, o);
>> +}
>> +#define SPEC_in2_m2_32s_atomic 0
>> +
>> +static void in2_m2_64_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
>> +{
>> +    /* XXX should reserve the address */
>> +    in2_m2_64(s, f, o);
>> +}
>> +#define SPEC_in2_m2_64_atomic 0
>> +
> I think these should save the address in o->addr1 so that you don't have to
> recompute it in the wout functions.

But I suppose you basically mean something like this?


Alex

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 73f2de3..8a30c8f 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -4074,25 +4074,17 @@ static void wout_m2_32(DisasContext *s, 
DisasFields *f, DisasOps *o)

  static void wout_m2_32_r1_atomic(DisasContext *s, DisasFields *f, 
DisasOps *o)
  {
-    TCGv_i64 a2 = get_a2(s, f);
-
      /* XXX release reservation */
-    tcg_gen_qemu_st32(o->out, a2, get_mem_index(s));
+    tcg_gen_qemu_st32(o->out, o->addr1, get_mem_index(s));
      store_reg32_i64(get_field(f, r1), o->in2);
-
-    tcg_temp_free_i64(a2);
  }
  #define SPEC_wout_m2_32_r1_atomic 0

  static void wout_m2_64_r1_atomic(DisasContext *s, DisasFields *f, 
DisasOps *o)
  {
-    TCGv_i64 a2 = get_a2(s, f);
-
      /* XXX release reservation */
-    tcg_gen_qemu_st64(o->out, a2, get_mem_index(s));
+    tcg_gen_qemu_st64(o->out, o->addr1, get_mem_index(s));
      store_reg(get_field(f, r1), o->in2);
-
-    tcg_temp_free_i64(a2);
  }
  #define SPEC_wout_m2_64_r1_atomic 0

@@ -4519,14 +4511,18 @@ static void in2_mri2_64(DisasContext *s, 
DisasFields *f, DisasOps *o)
  static void in2_m2_32s_atomic(DisasContext *s, DisasFields *f, 
DisasOps *o)
  {
      /* XXX should reserve the address */
-    in2_m2_32s(s, f, o);
+    o->addr1 = get_a2(s, f);
+    o->in2 = tcg_temp_new_i64();
+    tcg_gen_qemu_ld32s(o->in2, o->addr1, get_mem_index(s));
  }
  #define SPEC_in2_m2_32s_atomic 0

  static void in2_m2_64_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
  {
      /* XXX should reserve the address */
-    in2_m2_64(s, f, o);
+    o->addr1 = get_a2(s, f);
+    o->in2 = tcg_temp_new_i64();
+    tcg_gen_qemu_ld64(o->in2, o->addr1, get_mem_index(s));
  }
  #define SPEC_in2_m2_64_atomic 0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions
  2015-05-12 20:42   ` Alexander Graf
@ 2015-05-12 21:15     ` Richard Henderson
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2015-05-12 21:15 UTC (permalink / raw)
  To: Alexander Graf, qemu-devel

On 05/12/2015 01:42 PM, Alexander Graf wrote:
> 
> But wouldn't it really be "addr2"? This is the address source for the second
> argument after all.

Yes, but we already abuse the name.


r~

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions
  2015-05-12 20:46   ` Alexander Graf
@ 2015-05-12 21:19     ` Richard Henderson
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2015-05-12 21:19 UTC (permalink / raw)
  To: Alexander Graf, qemu-devel

On 05/12/2015 01:46 PM, Alexander Graf wrote:
> On 05/12/2015 10:27 PM, Richard Henderson wrote:
>> On 05/12/2015 01:20 PM, Alexander Graf wrote:
>>> +static void in2_m2_32s_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
>>> +{
>>> +    /* XXX should reserve the address */
>>> +    in2_m2_32s(s, f, o);
>>> +}
>>> +#define SPEC_in2_m2_32s_atomic 0
>>> +
>>> +static void in2_m2_64_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
>>> +{
>>> +    /* XXX should reserve the address */
>>> +    in2_m2_64(s, f, o);
>>> +}
>>> +#define SPEC_in2_m2_64_atomic 0
>>> +
>> I think these should save the address in o->addr1 so that you don't have to
>> recompute it in the wout functions.
> 
> But I suppose you basically mean something like this?

Yes.

There's an in1_la2 that does what your get_a2 does.
Again, slightly misnamed, but that's the beauty of these atomic ops -- same
fields have gotten renamed.


r~

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-05-12 21:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-12 20:20 [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions Alexander Graf
2015-05-12 20:27 ` Richard Henderson
2015-05-12 20:42   ` Alexander Graf
2015-05-12 21:15     ` Richard Henderson
2015-05-12 20:46   ` Alexander Graf
2015-05-12 21:19     ` Richard Henderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.