On 05.08.2018 20:28, Pavel Zbitskiy wrote:
> CSST is defined as:
>
> C(0xc802, CSST, SSF, CASS, la1, a2, 0, 0, csst, 0)
>
> It means that the first parameter is handled by in1_la1().
> in1_la1() fills addr1 field, and not in1.
>
> Furthermore, when extract32() is used for the alignment check, the
> third parameter should specify the number of trailing bits that must
> be 0. For FC these numbers are:
>
> FC=0: 2
-> word, 4 bytes -> 2 bit
> FC=1: 3
-> double word, 8 bytes -> 3 bit
> FC=2: 4
-> quad word, 16 bytes -> 4 bit
>
> For SC these numbers are:
>
> SC=0: 0
> SC=1: 1
> SC=2: 2
> SC=3: 3
> SC=4: 4
Right, corresponds to the size.
>
> Signed-off-by: Pavel Zbitskiy <pavel.zbits...@gmail.com>
> ---
> target/s390x/mem_helper.c | 2 +-
> target/s390x/translate.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index e21a47fb4d..c94dbf3fcb 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -1442,7 +1442,7 @@ static uint32_t do_csst(CPUS390XState *env, uint32_t
> r3, uint64_t a1,
> }
>
> /* Sanity check the alignments. */
> - if (extract32(a1, 0, 4 << fc) || extract32(a2, 0, 1 << sc)) {
> + if (extract32(a1, 0, fc + 2) || extract32(a2, 0, sc)) {
> goto spec_exception;
> }
>
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index efdc88e227..f318fb6e4e 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -2050,9 +2050,9 @@ static DisasJumpType op_csst(DisasContext *s, DisasOps
> *o)
> TCGv_i32 t_r3 = tcg_const_i32(r3);
>
> if (tb_cflags(s->base.tb) & CF_PARALLEL) {
> - gen_helper_csst_parallel(cc_op, cpu_env, t_r3, o->in1, o->in2);
> + gen_helper_csst_parallel(cc_op, cpu_env, t_r3, o->addr1, o->in2);
> } else {
> - gen_helper_csst(cc_op, cpu_env, t_r3, o->in1, o->in2);
> + gen_helper_csst(cc_op, cpu_env, t_r3, o->addr1, o->in2);
Indeed, only addr1 is filled.
(did this ever work?)
> }
> tcg_temp_free_i32(t_r3);
>
>
Are you running some test case or how did you find this? (PoP review?)
Haven't tested it yet, but looks sane to me
Reviewed-by: David Hildenbrand <da...@redhat.com>
--
Thanks,
David / dhildenb
