| Message ID | 20200609122339.937862-10-armbru@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixes around device realization | expand |
Peter, forgot to cc: you. May I have your blessings for this version? Markus Armbruster <armbru@redhat.com> writes: > cuda_init() creates a "mos6522-cuda" device, but it's never realized. > Affects machines mac99 with via=cuda (default) and g3beige. > > pmu_init() creates a "mos6522-pmu" device, but it's never realized. > Affects machine mac99 with via=pmu and via=pmu-adb, > > In theory, a device becomes real only on realize. In practice, the > transition from unreal to real is a fuzzy one. The work to make a > device real can be spread between realize methods (fine), > instance_init methods (wrong), and board code wiring up the device > (fine as long as it effectively happens on realize). Depending on > what exactly is done where, a device can work even when we neglect > to realize it. > > These two appear to work. Nevertheless, it's a clear misuse of the > interface. Even when it works today (more or less by chance), it can > break tomorrow. > > Fix by realizing them in cuda_realize() and pmu_realize(), > respectively. > > Fixes: 6dca62a0000f95e0b7020aa00d0ca9b2c421f341 > Cc: Laurent Vivier <laurent@vivier.eu> > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > hw/misc/macio/cuda.c | 15 ++++++++++----- > hw/misc/macio/pmu.c | 15 ++++++++++----- > 2 files changed, 20 insertions(+), 10 deletions(-) > > diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c > index e0cc0aac5d..3cb10c743c 100644 > --- a/hw/misc/macio/cuda.c > +++ b/hw/misc/macio/cuda.c > @@ -33,6 +33,7 @@ > #include "hw/misc/macio/cuda.h" > #include "qemu/timer.h" > #include "sysemu/runstate.h" > +#include "qapi/error.h" > #include "qemu/cutils.h" > #include "qemu/log.h" > #include "qemu/module.h" > @@ -522,16 +523,20 @@ static void cuda_reset(DeviceState *dev) > static void cuda_realize(DeviceState *dev, Error **errp) > { > CUDAState *s = CUDA(dev); > + Error *err = NULL; > SysBusDevice *sbd; > - MOS6522State *ms; > - DeviceState *d; > struct tm tm; > > + object_property_set_bool(OBJECT(&s->mos6522_cuda), true, "realized", > + &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > + > /* Pass IRQ from 6522 */ > - d = DEVICE(&s->mos6522_cuda); > - ms = MOS6522(d); > sbd = SYS_BUS_DEVICE(s); > - sysbus_pass_irq(sbd, SYS_BUS_DEVICE(ms)); > + sysbus_pass_irq(sbd, SYS_BUS_DEVICE(&s->mos6522_cuda)); > > qemu_get_timedate(&tm, 0); > s->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET; > diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c > index 9a9cd427e1..0895b78b59 100644 > --- a/hw/misc/macio/pmu.c > +++ b/hw/misc/macio/pmu.c > @@ -40,6 +40,7 @@ > #include "hw/misc/macio/pmu.h" > #include "qemu/timer.h" > #include "sysemu/runstate.h" > +#include "qapi/error.h" > #include "qemu/cutils.h" > #include "qemu/log.h" > #include "qemu/module.h" > @@ -739,16 +740,20 @@ static void pmu_reset(DeviceState *dev) > static void pmu_realize(DeviceState *dev, Error **errp) > { > PMUState *s = VIA_PMU(dev); > + Error *err = NULL; > SysBusDevice *sbd; > - MOS6522State *ms; > - DeviceState *d; > struct tm tm; > > + object_property_set_bool(OBJECT(&s->mos6522_pmu), true, "realized", > + &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > + > /* Pass IRQ from 6522 */ > - d = DEVICE(&s->mos6522_pmu); > - ms = MOS6522(d); > sbd = SYS_BUS_DEVICE(s); > - sysbus_pass_irq(sbd, SYS_BUS_DEVICE(ms)); > + sysbus_pass_irq(sbd, SYS_BUS_DEVICE(&s->mos6522_pmu)); > > qemu_get_timedate(&tm, 0); > s->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET;
On 6/15/20 7:26 AM, Markus Armbruster wrote: > Peter, forgot to cc: you. May I have your blessings for this version? This file is maintained by Mark: ./scripts/get_maintainer.pl -f hw/misc/macio/cuda.c Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> (odd fixer:Old World (g3beige)) David Gibson <david@gibson.dropbear.id.au> (reviewer:Old World (g3beige)) qemu-ppc@nongnu.org (open list:Old World (g3beige)) Mark only check qemu-devel during week-end. Too bad you forgot to Cc him, has he did a review pass over the last WE. > > Markus Armbruster <armbru@redhat.com> writes: > >> cuda_init() creates a "mos6522-cuda" device, but it's never realized. >> Affects machines mac99 with via=cuda (default) and g3beige. >> >> pmu_init() creates a "mos6522-pmu" device, but it's never realized. >> Affects machine mac99 with via=pmu and via=pmu-adb, >> >> In theory, a device becomes real only on realize. In practice, the >> transition from unreal to real is a fuzzy one. The work to make a >> device real can be spread between realize methods (fine), >> instance_init methods (wrong), and board code wiring up the device >> (fine as long as it effectively happens on realize). Depending on >> what exactly is done where, a device can work even when we neglect >> to realize it. >> >> These two appear to work. Nevertheless, it's a clear misuse of the >> interface. Even when it works today (more or less by chance), it can >> break tomorrow. >> >> Fix by realizing them in cuda_realize() and pmu_realize(), >> respectively. >> >> Fixes: 6dca62a0000f95e0b7020aa00d0ca9b2c421f341 >> Cc: Laurent Vivier <laurent@vivier.eu> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> hw/misc/macio/cuda.c | 15 ++++++++++----- >> hw/misc/macio/pmu.c | 15 ++++++++++----- >> 2 files changed, 20 insertions(+), 10 deletions(-) >> >> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c >> index e0cc0aac5d..3cb10c743c 100644 >> --- a/hw/misc/macio/cuda.c >> +++ b/hw/misc/macio/cuda.c >> @@ -33,6 +33,7 @@ >> #include "hw/misc/macio/cuda.h" >> #include "qemu/timer.h" >> #include "sysemu/runstate.h" >> +#include "qapi/error.h" >> #include "qemu/cutils.h" >> #include "qemu/log.h" >> #include "qemu/module.h" >> @@ -522,16 +523,20 @@ static void cuda_reset(DeviceState *dev) >> static void cuda_realize(DeviceState *dev, Error **errp) >> { >> CUDAState *s = CUDA(dev); >> + Error *err = NULL; >> SysBusDevice *sbd; >> - MOS6522State *ms; >> - DeviceState *d; >> struct tm tm; >> >> + object_property_set_bool(OBJECT(&s->mos6522_cuda), true, "realized", >> + &err); >> + if (err) { >> + error_propagate(errp, err); >> + return; >> + } >> + >> /* Pass IRQ from 6522 */ >> - d = DEVICE(&s->mos6522_cuda); >> - ms = MOS6522(d); >> sbd = SYS_BUS_DEVICE(s); >> - sysbus_pass_irq(sbd, SYS_BUS_DEVICE(ms)); >> + sysbus_pass_irq(sbd, SYS_BUS_DEVICE(&s->mos6522_cuda)); >> >> qemu_get_timedate(&tm, 0); >> s->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET; >> diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c >> index 9a9cd427e1..0895b78b59 100644 >> --- a/hw/misc/macio/pmu.c >> +++ b/hw/misc/macio/pmu.c >> @@ -40,6 +40,7 @@ >> #include "hw/misc/macio/pmu.h" >> #include "qemu/timer.h" >> #include "sysemu/runstate.h" >> +#include "qapi/error.h" >> #include "qemu/cutils.h" >> #include "qemu/log.h" >> #include "qemu/module.h" >> @@ -739,16 +740,20 @@ static void pmu_reset(DeviceState *dev) >> static void pmu_realize(DeviceState *dev, Error **errp) >> { >> PMUState *s = VIA_PMU(dev); >> + Error *err = NULL; >> SysBusDevice *sbd; >> - MOS6522State *ms; >> - DeviceState *d; >> struct tm tm; >> >> + object_property_set_bool(OBJECT(&s->mos6522_pmu), true, "realized", >> + &err); >> + if (err) { >> + error_propagate(errp, err); >> + return; >> + } >> + >> /* Pass IRQ from 6522 */ >> - d = DEVICE(&s->mos6522_pmu); >> - ms = MOS6522(d); >> sbd = SYS_BUS_DEVICE(s); >> - sysbus_pass_irq(sbd, SYS_BUS_DEVICE(ms)); >> + sysbus_pass_irq(sbd, SYS_BUS_DEVICE(&s->mos6522_pmu)); >> >> qemu_get_timedate(&tm, 0); >> s->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET; > >
diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c index e0cc0aac5d..3cb10c743c 100644 --- a/hw/misc/macio/cuda.c +++ b/hw/misc/macio/cuda.c @@ -33,6 +33,7 @@ #include "hw/misc/macio/cuda.h" #include "qemu/timer.h" #include "sysemu/runstate.h" +#include "qapi/error.h" #include "qemu/cutils.h" #include "qemu/log.h" #include "qemu/module.h" @@ -522,16 +523,20 @@ static void cuda_reset(DeviceState *dev) static void cuda_realize(DeviceState *dev, Error **errp) { CUDAState *s = CUDA(dev); + Error *err = NULL; SysBusDevice *sbd; - MOS6522State *ms; - DeviceState *d; struct tm tm; + object_property_set_bool(OBJECT(&s->mos6522_cuda), true, "realized", + &err); + if (err) { + error_propagate(errp, err); + return; + } + /* Pass IRQ from 6522 */ - d = DEVICE(&s->mos6522_cuda); - ms = MOS6522(d); sbd = SYS_BUS_DEVICE(s); - sysbus_pass_irq(sbd, SYS_BUS_DEVICE(ms)); + sysbus_pass_irq(sbd, SYS_BUS_DEVICE(&s->mos6522_cuda)); qemu_get_timedate(&tm, 0); s->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET; diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c index 9a9cd427e1..0895b78b59 100644 --- a/hw/misc/macio/pmu.c +++ b/hw/misc/macio/pmu.c @@ -40,6 +40,7 @@ #include "hw/misc/macio/pmu.h" #include "qemu/timer.h" #include "sysemu/runstate.h" +#include "qapi/error.h" #include "qemu/cutils.h" #include "qemu/log.h" #include "qemu/module.h" @@ -739,16 +740,20 @@ static void pmu_reset(DeviceState *dev) static void pmu_realize(DeviceState *dev, Error **errp) { PMUState *s = VIA_PMU(dev); + Error *err = NULL; SysBusDevice *sbd; - MOS6522State *ms; - DeviceState *d; struct tm tm; + object_property_set_bool(OBJECT(&s->mos6522_pmu), true, "realized", + &err); + if (err) { + error_propagate(errp, err); + return; + } + /* Pass IRQ from 6522 */ - d = DEVICE(&s->mos6522_pmu); - ms = MOS6522(d); sbd = SYS_BUS_DEVICE(s); - sysbus_pass_irq(sbd, SYS_BUS_DEVICE(ms)); + sysbus_pass_irq(sbd, SYS_BUS_DEVICE(&s->mos6522_pmu)); qemu_get_timedate(&tm, 0); s->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET;
cuda_init() creates a "mos6522-cuda" device, but it's never realized. Affects machines mac99 with via=cuda (default) and g3beige. pmu_init() creates a "mos6522-pmu" device, but it's never realized. Affects machine mac99 with via=pmu and via=pmu-adb, In theory, a device becomes real only on realize. In practice, the transition from unreal to real is a fuzzy one. The work to make a device real can be spread between realize methods (fine), instance_init methods (wrong), and board code wiring up the device (fine as long as it effectively happens on realize). Depending on what exactly is done where, a device can work even when we neglect to realize it. These two appear to work. Nevertheless, it's a clear misuse of the interface. Even when it works today (more or less by chance), it can break tomorrow. Fix by realizing them in cuda_realize() and pmu_realize(), respectively. Fixes: 6dca62a0000f95e0b7020aa00d0ca9b2c421f341 Cc: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Markus Armbruster <armbru@redhat.com> --- hw/misc/macio/cuda.c | 15 ++++++++++----- hw/misc/macio/pmu.c | 15 ++++++++++----- 2 files changed, 20 insertions(+), 10 deletions(-)