On Tue, Jun 27, 2023 at 10:15 AM Mark Cave-Ayland < mark.cave-ayl...@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 <npig...@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 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.) ;-) Best, Howard > > diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c > index 265c0bbd8d..e55f938da7 100644 > --- a/hw/misc/macio/macio.c > +++ b/hw/misc/macio/macio.c > @@ -26,6 +26,7 @@ > #include "qemu/osdep.h" > #include "qapi/error.h" > #include "qemu/module.h" > +#include "hw/misc/unimp.h" > #include "hw/misc/macio/cuda.h" > #include "hw/pci/pci.h" > #include "hw/ppc/mac_dbdma.h" > @@ -94,6 +95,7 @@ static bool macio_common_realize(PCIDevice *d, Error > **errp) > { > MacIOState *s = MACIO(d); > SysBusDevice *sbd; > + DeviceState *dev; > > if (!qdev_realize(DEVICE(&s->dbdma), BUS(&s->macio_bus), errp)) { > return false; > @@ -102,6 +104,14 @@ static bool macio_common_realize(PCIDevice *d, Error > **errp) > memory_region_add_subregion(&s->bar, 0x08000, > sysbus_mmio_get_region(sbd, 0)); > > + dev = qdev_new(TYPE_UNIMPLEMENTED_DEVICE); > + qdev_prop_set_string(dev, "name", "screamer"); > + qdev_prop_set_uint64(dev, "size", 0x1000); > + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > + sbd = SYS_BUS_DEVICE(dev); > + memory_region_add_subregion(&s->bar, 0x14000, > + sysbus_mmio_get_region(sbd, 0)); > + > qdev_prop_set_uint32(DEVICE(&s->escc), "disabled", 0); > qdev_prop_set_uint32(DEVICE(&s->escc), "frequency", ESCC_CLOCK); > qdev_prop_set_uint32(DEVICE(&s->escc), "it_shift", 4); > diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h > index 86df2c2b60..1894178a68 100644 > --- a/include/hw/misc/macio/macio.h > +++ b/include/hw/misc/macio/macio.h > @@ -109,6 +109,7 @@ struct MacIOState { > PMUState pmu; > DBDMAState dbdma; > ESCCState escc; > + MemoryRegion screamer; > uint64_t frequency; > }; > > > > ATB, > > Mark. > >