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

[Howard Spoelstra <hsp.cat7@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5951 bytes --]

On Tue, Jun 27, 2023 at 1:24 PM Mark Cave-Ayland <
mark.cave-ayland@ilande.co.uk> wrote:

> On 27/06/2023 11:28, Howard Spoelstra wrote:
>
> > On Tue, Jun 27, 2023 at 10:15 AM Mark Cave-Ayland <
> mark.cave-ayland@ilande.co.uk
> > <mailto:mark.cave-ayland@ilande.co.uk>> wrote:
> >
> >     On 26/06/2023 14:35, Cédric Le Goater wrote:
> >
> >      > On 6/23/23 14:37, Cédric Le Goater wrote:
> >      >> On 6/23/23 11:10, Peter Maydell wrote:
> >      >>> On Fri, 23 Jun 2023 at 09:21, Nicholas Piggin <
> npiggin@gmail.com
> >     <mailto:npiggin@gmail.com>> wrote:
> >      >>>>
> >      >>>> ppc has always silently ignored access to real (physical)
> addresses
> >      >>>> with nothing behind it, which can make debugging difficult at
> times.
> >      >>>>
> >      >>>> It looks like the way to handle this is implement the
> transaction
> >      >>>> failed call, which most target architectures do. Notably not
> x86
> >      >>>> though, I wonder why?
> >      >>>
> >      >>> Much of this is historical legacy. QEMU originally had no
> >      >>> concept of "the system outside the CPU returns some kind
> >      >>> of bus error and the CPU raises an exception for it".
> >      >>> This is turn is (I think) because the x86 PC doesn't do
> >      >>> that: you always get back some kind of response, I think
> >      >>> -1 on reads and writes ignored. We added the
> do_transaction_failed
> >      >>> hook largely because we wanted it to give more accurate
> >      >>> emulation of this kind of thing on Arm, but as usual with new
> >      >>> facilities we left the other architectures to do it themselves
> >      >>> if they wanted -- by default the behaviour remained the same.
> >      >>> Some architectures have picked it up; some haven't.
> >      >>>
> >      >>> The main reason it's a bit of a pain to turn the correct
> >      >>> handling on is because often boards don't actually implement
> >      >>> all the devices they're supposed to. For a pile of legacy Arm
> >      >>> boards, especially where we didn't have good test images,
> >      >>> we use the machine flag ignore_memory_transaction_failures to
> >      >>> retain the legacy behaviour. (This isn't great because it's
> >      >>> pretty much going to mean we have that flag set on those
> >      >>> boards forever because nobody is going to care enough to
> >      >>> investigate and test.)
> >      >>>
> >      >>>> Other question is, sometimes I guess it's nice to avoid
> crashing in
> >      >>>> order to try to quickly get past some unimplemented MMIO.
> Maybe a
> >      >>>> command line option or something could turn it off? It should
> >      >>>> probably be a QEMU-wide option if so, so that shouldn't hold
> this
> >      >>>> series up, I can propose a option for that if anybody is
> worried
> >      >>>> about it.
> >      >>>
> >      >>> I would not recommend going any further than maybe setting the
> >      >>> ignore_memory_transaction_failures flag for boards you don't
> >      >>> care about. (But in an ideal world, don't set it and deal with
> >      >>> any bug reports by implementing stub versions of missing
> devices.
> >      >>> Depends how confident you are in your test coverage.)
> >      >>
> >      >> It seems it broke the "mac99" and  powernv10 machines, using the
> >      >> qemu-ppc-boot images which are mostly buildroot. See below for
> logs.
> >      >>
> >      >> Adding Mark for further testing on Mac OS.
> >      >
> >      >
> >      > Mac OS 9.2 fails to boot with a popup saying :
> >      >          Sorry, a system error occured.
> >      >          "Sound Manager"
> >      >            address error
> >      >          To temporarily turn off extensions, restart and
> >      >          hold down the shift key
> >      >
> >      >
> >      > Darwin and Mac OSX look OK.
> >
> >     My guess would be that MacOS 9.2 is trying to access the sound chip
> registers which
> >     isn't implemented in QEMU for the moment (I have a separate screamer
> branch
> >     available, but it's not ready for primetime yet). In theory they
> shouldn't be
> >     accessed at all because the sound device isn't present in the
> OpenBIOS device tree,
> >     but this is all fairly old stuff.
> >
> >     Does implementing the sound registers using a dummy device help at
> all?
> >
> >
> > My uneducated guess is that you stumbled on a longstanding, but
> intermittently
> > occurring, issue specific to Mac OS 9.2 related to sound support over
> USB in Apple
> > monitors.
>
> I'm not sure I understand this: are there non-standard command line
> options being
> used here other than "qemu-system-ppc -M mac99 -cdrom macos92.iso -boot d"?
>


It must be my windows host ;-)

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).

qemu-system-ppc.exe -M mac99 -hda C:\mac-hd\9.2.2-clean.img -boot c -L
pc-bios sometimes crashes with an illegal instruction.

qemu-system-ppc.exe -M mac99,via=pmu -hda C:\mac-hd\9.2.2-clean.img -boot c
-L pc-bios sometimes crashes with Sound manager address error.
(with both patched and non-patched versions).

Best,
Howard



>
> > I believe It is not fixed by the patch set from the 23 of june, I still
> get system
> > errors when running Mac OS 9.2 with the mac99 machine after applying
> them.
> > Mac OS 9.2 has required mac99,via=pmu for a long time now to always boot
> > successfully. (while 9.0.4 requires mac99 to boot, due to an undiagnosed
> OHCI USB
> > problem with the specific drivers that ship with it.)  ;-)
>
> I always test MacOS 9.2 boot both with and without via=pmu for my OpenBIOS
> tests, so
> I'd expect this to work unless a regression has slipped in?
>
>
> ATB,
>
> Mark.
>
>

[-- Attachment #2: Type: text/html, Size: 8101 bytes --]