Re: [PATCH 0/4] target/ppc: Catch invalid real address accesses

[Nicholas Piggin]

On Thu Jun 29, 2023 at 7:05 PM AEST, Cédric Le Goater wrote:
> On 6/29/23 10:29, Mark Cave-Ayland wrote:
> > On 28/06/2023 08:17, Cédric Le Goater wrote:
> > 
> >>>>>> qemu-system-ppc.exe -M mac99,via=pmu -cdrom C:\mac-iso\9.2.2.iso -boot 
> >>>>>> d -L pc-bios
> >>>>>> crashes Mac OS with an address error. (with unpatched and patched 
> >>>>>> builds).
> >>>>>
> >>>>> Same on Linux. I get an invalid opcode. QEMU 7.2 work fine though.
> >>>>>
> >>>>> C.
> >>>>
> >>>> That certainly shouldn't happen, and if it worked in 7.2 then there's 
> >>>> definitely a regression which has crept in there somewhere. I'll try and 
> >>>> bisect this at some point soon, but feel free to try and beat me ;)
> >>>
> >>> bisect points to :
> >>>
> >>> commit e506ad6a05c806bbef460a7d014a184ff8d707a6
> >>> Author: Richard Henderson <richard.henderson@linaro.org>
> >>> Date:   Mon Mar 6 04:30:11 2023 +0300
> >>>
> >>>      accel/tcg: Pass last not end to tb_invalidate_phys_range
> >>>      Pass the address of the last byte to be changed, rather than
> >>>      the first address past the last byte.  This avoids overflow
> >>>      when the last page of the address space is involved.
> >>>      Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >>>      Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >>>
> >>>   include/exec/exec-all.h   |  2 +-
> >>>   accel/tcg/tb-maint.c      | 31 ++++++++++++++++---------------
> >>>   accel/tcg/translate-all.c |  2 +-
> >>>   accel/tcg/user-exec.c     |  2 +-
> >>>   softmmu/physmem.c         |  2 +-
> >>>   5 files changed, 20 insertions(+), 19 deletions(-)
> >>>
> >>>
> >>> I think the instruction is fnmadds. Needs more digging.
> >>
> >> the invalid opcode is just a symptom of something bad happening.
> >>
> >> C.
> > 
> > Indeed, it appears to be a copy/paste error within that commit. I've just 
> > posted a proposed fix for this: 
> > 20230629082522.606219-1-mark.cave-ayland@ilande.co.uk/T/">https://lore.kernel.org/qemu-devel/20230629082522.606219-1-mark.cave-ayland@ilande.co.uk/T/.
>
> Looks good. I could boot macos 9.2.1 from an iso and 9.2.2 from disk.
>
> > Nick: you may wish to try your series again with this fix applied to see if 
> > there are still problems with the CPUs used in the Mac machines.
>
> Looks good too, with these :
>
>   [PATCH v2 1/4] target/ppc: Machine check on invalid real address
>   [PATCH v2 2/4] target/ppc: Move common check in machine check
>   [PATCH v2 3/4] target/ppc: Make checkstop actually stop the system

In the v2 series I removed the machine check for 970 btw so there
might still be invalid memory access.

Thanks,
Nick