On Tue, 2023-07-04 at 09:47 +0200, David Hildenbrand wrote:
> On 03.07.23 17:50, Ilya Leoshkevich wrote:
> > When a DAT error occurs, LRA is supposed to write the error
> > information
> > to the bottom 32 bits of R1, and leave the top 32 bits of R1 alone.
> >
> > Fix by passing the original value of R1 into helper and copying the
> > top 32 bits to the return value.
> >
> > Fixes: d8fe4a9c284f ("target-s390: Convert LRA")
> > Cc: qemu-sta...@nongnu.org
> > Signed-off-by: Ilya Leoshkevich <i...@linux.ibm.com>
> > ---
> > target/s390x/helper.h | 2 +-
> > target/s390x/tcg/mem_helper.c | 4 ++--
> > target/s390x/tcg/translate.c | 2 +-
> > 3 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> > index 6bc01df73d7..05102578fc9 100644
> > --- a/target/s390x/helper.h
> > +++ b/target/s390x/helper.h
> > @@ -355,7 +355,7 @@ DEF_HELPER_FLAGS_4(idte, TCG_CALL_NO_RWG, void,
> > env, i64, i64, i32)
> > DEF_HELPER_FLAGS_4(ipte, TCG_CALL_NO_RWG, void, env, i64, i64,
> > i32)
> > DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
> > DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
> > -DEF_HELPER_2(lra, i64, env, i64)
> > +DEF_HELPER_3(lra, i64, env, i64, i64)
> > DEF_HELPER_1(per_check_exception, void, env)
> > DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64,
> > i64)
> > DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
> > diff --git a/target/s390x/tcg/mem_helper.c
> > b/target/s390x/tcg/mem_helper.c
> > index 84ad85212c9..94d93d7ea78 100644
> > --- a/target/s390x/tcg/mem_helper.c
> > +++ b/target/s390x/tcg/mem_helper.c
> > @@ -2356,7 +2356,7 @@ void HELPER(purge)(CPUS390XState *env)
> > }
> >
> > /* load real address */
> > -uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
> > +uint64_t HELPER(lra)(CPUS390XState *env, uint64_t r1, uint64_t
> > addr)
> > {
> > uint64_t asc = env->psw.mask & PSW_MASK_ASC;
> > uint64_t ret, tec;
> > @@ -2370,7 +2370,7 @@ uint64_t HELPER(lra)(CPUS390XState *env,
> > uint64_t addr)
> > exc = mmu_translate(env, addr, MMU_S390_LRA, asc, &ret,
> > &flags, &tec);
> > if (exc) {
> > cc = 3;
> > - ret = exc | 0x80000000;
> > + ret = (r1 & 0xFFFFFFFF00000000) | exc | 0x80000000;
>
> ull missing for large constant?
Will do.
Just for my understanding, why is this necessary?
The current code base tends towards using ULL, but it's a little bit
inconsistent:
$ git grep -i 0xfffffffff | wc -l
2338
$ git grep -i 0xfffffffff | grep -i -v ul | wc -l
95
>
> > } else {
> > cc = 0;
> > ret |= addr & ~TARGET_PAGE_MASK;
> > diff --git a/target/s390x/tcg/translate.c
> > b/target/s390x/tcg/translate.c
> > index 0cef6efbef4..a6079ab7b4f 100644
> > --- a/target/s390x/tcg/translate.c
> > +++ b/target/s390x/tcg/translate.c
> > @@ -2932,7 +2932,7 @@ static DisasJumpType op_lctlg(DisasContext
> > *s, DisasOps *o)
> >
> > static DisasJumpType op_lra(DisasContext *s, DisasOps *o)
> > {
> > - gen_helper_lra(o->out, cpu_env, o->in2);
> > + gen_helper_lra(o->out, cpu_env, o->out, o->in2);
> > set_cc_static(s);
> > return DISAS_NEXT;
> > }
>
> Can't we use something like in1_r1 + wout_r1_32 instead ? *maybe*
> cleaner :)
>
The problem is that we want all 64 bits for the non-error case.
