| Message ID | 20180805182832.3012-3-pavel.zbitskiy@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Some improvements in z/Arch instructions support | expand |
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.zbitskiy@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 <david@redhat.com>
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); } tcg_temp_free_i32(t_r3);
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 FC=1: 3 FC=2: 4 For SC these numbers are: SC=0: 0 SC=1: 1 SC=2: 2 SC=3: 3 SC=4: 4 Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com> --- target/s390x/mem_helper.c | 2 +- target/s390x/translate.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)