[2/2] s390x: Add laa and laag instructions

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. I couldn't
really see a good way on making them fit into the current load/store mechanism
we have in place.

So the best thing I can think of right now for those instructions is to add
flags in the data field what type of operation we're looking at and just map
them all to the same op function.
---
 target-s390x/insn-data.def |  3 +++
 target-s390x/translate.c   | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

On 05/07/2015 06:12 PM, Alexander Graf wrote:
> +static ExitStatus op_laa(DisasContext *s, DisasOps *o)
> +{
> +    TCGv_i64 m2 = tcg_temp_new_i64();
> +
> +    /* XXX should be atomic */
> +    tcg_gen_qemu_ld32s(m2, o->in2, get_mem_index(s));
> +
> +    /* Set r1 to the unmodified contents of m2 */
> +    tcg_gen_mov_i64(o->out, m2);
> +
> +    /* m2 = r3 + m2 */
> +    tcg_gen_add_i64(m2, o->in1, m2);
> +    tcg_gen_qemu_st32(m2, o->in2, get_mem_index(s));
> +
> +    /* Set in2 to the input operand for cc calculation */
> +    tcg_gen_mov_i64(o->in2, o->out);
> +
> +    tcg_temp_free_i64(m2);
> +    return NO_EXIT;
> +}

I don't believe that this computes the right cc.
You need to place m2 into in2, not out.

That said, maybe

  tcg_temp_free_i64(o->in2);
  o->in2 = m2;

is the right way to perform that copy.  Since we
know that in2 was in2_a2, allocating a temp for an
address, we also know we can both (1) free it and
(2) put another temp in there that will be freed.

Don't worry about the atomic-ness of the operation
until some of the many patch sets going round on
that very subject are resolved.  This will be good
enough for system mode for now.

I think maybe the whole target ought to be cleaned
up for the new TCGMemOps.  For this, you could share
the code for LAA and LAAG with the TCGMemOp in insn->data.


r~

On 05/12/2015 05:53 AM, Richard Henderson wrote:
> On 05/07/2015 06:12 PM, Alexander Graf wrote:
>> +static ExitStatus op_laa(DisasContext *s, DisasOps *o)
>> +{
>> +    TCGv_i64 m2 = tcg_temp_new_i64();
>> +
>> +    /* XXX should be atomic */
>> +    tcg_gen_qemu_ld32s(m2, o->in2, get_mem_index(s));
>> +
>> +    /* Set r1 to the unmodified contents of m2 */
>> +    tcg_gen_mov_i64(o->out, m2);
>> +
>> +    /* m2 = r3 + m2 */
>> +    tcg_gen_add_i64(m2, o->in1, m2);
>> +    tcg_gen_qemu_st32(m2, o->in2, get_mem_index(s));
>> +
>> +    /* Set in2 to the input operand for cc calculation */
>> +    tcg_gen_mov_i64(o->in2, o->out);
>> +
>> +    tcg_temp_free_i64(m2);
>> +    return NO_EXIT;
>> +}
> I don't believe that this computes the right cc.
> You need to place m2 into in2, not out.
>
> That said, maybe
>
>    tcg_temp_free_i64(o->in2);
>    o->in2 = m2;
>
> is the right way to perform that copy.  Since we
> know that in2 was in2_a2, allocating a temp for an
> address, we also know we can both (1) free it and
> (2) put another temp in there that will be freed.

Well, alternatively I could try to make out be the memory value at the 
end of the op and invent new load/store helpers for atomic instructions. 
That way the op bit should shrink significantly and we get the correct 
operand in s->out automatically.


>
> Don't worry about the atomic-ness of the operation
> until some of the many patch sets going round on
> that very subject are resolved.  This will be good
> enough for system mode for now.
>
> I think maybe the whole target ought to be cleaned
> up for the new TCGMemOps.  For this, you could share
> the code for LAA and LAAG with the TCGMemOp in insn->data.

That's an interesting idea. Let me see what I can pull off.


Alex