Re: [PATCH 27/35] target/arm: Report VNCR_EL2 based faults correctly

[Peter Maydell]

On Tue, 16 Jan 2024 at 15:29, Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Tue, 16 Jan 2024 14:59:15 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
>
> > On Tue, 16 Jan 2024 at 14:50, Jonathan Cameron
> > <Jonathan.Cameron@huawei.com> wrote:
> > >
> > > On Tue, 16 Jan 2024 13:20:33 +0000
> > > Peter Maydell <peter.maydell@linaro.org> wrote:
> > > > Bisecting to this patch is a bit weird because at this point
> > > > in the series emulation of FEAT_NV2 should be disabled and
> > > > the code being added should never be used. You could put
> > > > an assert(0) into the code in translate-a64.c before the
> > > > call to syn_data_abort_vncr() and in arm_deliver_fault()
> > > > assert(!is_vncr) to confirm that we're not somehow getting
> > > > into this code for some non-FEAT_NV2 situation, I guess.
> > >
> > > Not that, but surprisingly is_vncr == true.
> > > in arm_deliver_fault()
> > >
> > > Frigging that to be false gets me up and running. I'll see
> > > if I can figure out why it is set.
> >
> > I don't know if this is the cause, but looking again at the
> > line that sets is_vncr I see at least one obvious bug:
> >
> >     bool is_vncr = (mmu_idx != MMU_INST_FETCH) &&
> >         (env->exception.syndrome & ARM_EL_VNCR);
> >
> > is testing the wrong variable -- the first part
> > of the condition should be "access_type != MMU_INST_FETCH".
> >
> > If you fix that does the failure go away ?
> Ah - indeed that fixes it.
>
> I guess that makes sense. Presumably the bit is used for
> something else for instruction fetches.
>
> Thanks for your quick help!

Thanks for testing -- I'll send out a proper patch in
a bit.

-- PMM