On Wed Jun 28, 2023 at 3:40 AM AEST, BALATON Zoltan wrote: > On Mon, 26 Jun 2023, Nicholas Piggin wrote: > > On Tue Jun 20, 2023 at 8:47 PM AEST, BALATON Zoltan wrote: > >> On Tue, 20 Jun 2023, Nicholas Piggin wrote: > >>> On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote: > >>>> From: Nicholas Piggin <npig...@gmail.com> > >>>> > >>>> Unlike sc, for scv a facility unavailable interrupt must be generated > >>>> if FSCR[SCV]=0 so we can't raise the exception with nip set to next > >>>> instruction but we can move advancing nip if the FSCR check passes to > >>>> helper_scv so the exception handler does not need to change it. > >>>> > >>>> [balaton: added commit message] > >>>> Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu> > >>> > >>> Ah you sent it, fine, thank you. But actually now I look again, > >>> now we're off by one in the other direction for the dumps. > >> > >> This is mentioned in the commit message for the patch changing sc. I think > >> we should not patch nip in the dump so we actually dump what the CPU > >> should have and match the ISA docs. > >> > >>> So... probably your way is still better because it matches the > >>> interrupt semantics of the ISA when executing the instruction, > >>> but it needs this patch: > >> > >> OK so then I'm confused why we need nip - 4 in dumps? > > > > Sorry I missed your reply here. We want nip - 4 in dumps so the > > address of the syscall is the sc instruction itself, not the > > random one after it. > > Although that's how it was in QEMU before
Current upstream QEMU dumps syscall address of sc instruction. After patch 8 and 9, it will dump the address of the instruction after it. > that's not how it is on real > hardware so I don't think we should keep this and just log what a real CPU > would have and people should know how to interpret that after consulting > the ISA docs. I did get the feeling it was nicer your way, OTOH there really is not anything in the ISA that requires a particular implementation. QEMU is a real implementation of the ISA anyway. You could argue it's more consistent for QEMU to keep env->nip as the address of instruction that caused the interrupt, and then the sc fixup is restricted to setting SRR0. I'm on the fence about it now. Thanks, Nick