[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] target/arm: fix IL bit for data abort exceptions
From: |
Peter Maydell |
Subject: |
Re: [PATCH] target/arm: fix IL bit for data abort exceptions |
Date: |
Thu, 19 Dec 2019 12:43:45 +0000 |
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