Re: [PATCH] target/arm: fix IL bit for data abort exceptions

[Peter Maydell]

On Wed, 18 Dec 2019 at 01:03, Richard Henderson
<address@hidden> wrote:
>
> On 12/17/19 11:02 AM, Jeff Kubascik wrote:
> > diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
> > index 5feb312941..e63f8bda29 100644
> > --- a/target/arm/tlb_helper.c
> > +++ b/target/arm/tlb_helper.c
> > @@ -44,7 +44,7 @@ static inline uint32_t merge_syn_data_abort(uint32_t 
> > template_syn,
> >          syn = syn_data_abort_with_iss(same_el,
> >                                        0, 0, 0, 0, 0,
> >                                        ea, 0, s1ptw, is_write, fsc,
> > -                                      false);
> > +                                      true);
> >          /* Merge the runtime syndrome with the template syndrome.  */
> >          syn |= template_syn;
>
> This doesn't look correct.  Surely the IL bit should come from template_syn?

Yes. In translate.c we put it into the syndrome information by
passing true/false to syn_data_abort_with_iss() depending on
whether the issinfo passed in to disas_set_da_iss() has the
ISSIs16Bit flag set.

I think this is a regression introduced in commit 46beb58efbb8a2a32
when we converted the Thumb decoder over to decodetree. Before that
16 bit Thumb insns were in a different place in the old decoder and
the 16-bit Thumb path passed ISSIs16Bit in with its issflags.
(We should cc: address@hidden on the fix for this.)

> > diff --git a/target/arm/translate.c b/target/arm/translate.c
> > index 2b6c1f91bf..300480f1b7 100644
> > --- a/target/arm/translate.c
> > +++ b/target/arm/translate.c
> > @@ -8555,7 +8555,7 @@ static ISSInfo make_issinfo(DisasContext *s, int rd, 
> > bool p, bool w)
> >
> >      /* ISS not valid if writeback */
> >      if (p && !w) {
> > -        ret = rd;
> > +        ret = rd | (s->is_16bit ? ISSIs16Bit : 0);
> >      } else {
> >          ret = ISSInvalid;
> >      }

Rather than setting an is_16bit flag, we could just use
"dc->base.pc_next - dc->pc_curr == 2", couldn't we?

thanks
-- PMM