On Wed, 4 Nov 2020, Thomas Huth wrote:
> On 26/09/2020 16.02, Mark Cave-Ayland wrote:
>> Instead use qdev_prop_set_chr() to configure the ESCC serial chardevs at the
>> Mac Old World and New World machine level.
>>
>> Also remove the now obsolete comment referring to the use of serial_hd() and
>> the setting of user_creatable to false accordingly.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
>> ---
>> hw/misc/macio/macio.c | 4 ----
>> hw/ppc/mac_newworld.c | 6 ++++++
>> hw/ppc/mac_oldworld.c | 6 ++++++
>> 3 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
>> index 679722628e..51368884d0 100644
>> --- a/hw/misc/macio/macio.c
>> +++ b/hw/misc/macio/macio.c
>> @@ -109,8 +109,6 @@ static void macio_common_realize(PCIDevice *d, Error
>> **errp)
>> 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);
>> - qdev_prop_set_chr(DEVICE(&s->escc), "chrA", serial_hd(0));
>> - qdev_prop_set_chr(DEVICE(&s->escc), "chrB", serial_hd(1));
>> qdev_prop_set_uint32(DEVICE(&s->escc), "chnBtype", escc_serial);
>> qdev_prop_set_uint32(DEVICE(&s->escc), "chnAtype", escc_serial);
>> if (!qdev_realize(DEVICE(&s->escc), BUS(&s->macio_bus), errp)) {
>> @@ -458,8 +456,6 @@ static void macio_class_init(ObjectClass *klass, void
>> *data)
>> k->class_id = PCI_CLASS_OTHERS << 8;
>> device_class_set_props(dc, macio_properties);
>> set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>> - /* Reason: Uses serial_hds in macio_instance_init */
>> - dc->user_creatable = false;
>> }
>
> Hi Mark,
>
> the macio device can now be used to crash QEMU:
>
> $ ./qemu-system-ppc -M sam460ex -device macio-newworld
> Segmentation fault (core dumped)
>
> I guess we should either restore the user_creatable flag or add some sanity
> checks elsewhere?
Looks like it needs to check if pic_dev is set:
$ gdb --args ./qemu-system-ppc -M sam460ex -device macio-newworld
(gdb) r
Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
0x0000555555c3d65a in qdev_get_named_gpio_list (dev=0x0, name=0x0)
at ../hw/core/qdev.c:456
456 QLIST_FOREACH(ngl, &dev->gpios, node) {
(gdb) bt
#0 0x0000555555c3d65a in qdev_get_named_gpio_list (dev=0x0, name=0x0)
at ../hw/core/qdev.c:456
#1 0x0000555555c3e349 in qdev_get_gpio_in_named (dev=<optimized out>,
name=<optimized out>, n=36) at ../hw/core/qdev.c:532
#2 0x00005555559c690f in macio_newworld_realize (d=<optimized out>,
errp=0x7fffffffda40) at ../hw/misc/macio/macio.c:301
#3 0x0000555555946334 in pci_qdev_realize (qdev=0x555556b578e0,
errp=<optimized out>) at ../hw/pci/pci.c:2125
#4 0x0000555555c3f1ff in device_set_realized (obj=<optimized out>,
value=true, errp=0x7fffffffdb50) at ../hw/core/qdev.c:886
[...]
(gdb) up
#1 0x0000555555c3e349 in qdev_get_gpio_in_named (dev=<optimized out>,
name=<optimized out>, n=36) at ../hw/core/qdev.c:532
532 NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
(gdb)
#2 0x00005555559c690f in macio_newworld_realize (d=<optimized out>,
errp=0x7fffffffda40) at ../hw/misc/macio/macio.c:301
301 sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
(gdb) l 285
280 .read = timer_read,
281 .write = timer_write,
282 .endianness = DEVICE_LITTLE_ENDIAN,
283 };
284
285 static void macio_newworld_realize(PCIDevice *d, Error **errp)
286 {
287 MacIOState *s = MACIO(d);
288 NewWorldMacIOState *ns = NEWWORLD_MACIO(d);
289 DeviceState *pic_dev = DEVICE(ns->pic);
(gdb)
290 Error *err = NULL;
291 SysBusDevice *sysbus_dev;
292 MemoryRegion *timer_memory = NULL;
293
294 macio_common_realize(d, &err);
295 if (err) {
296 error_propagate(errp, err);
297 return;
298 }
299
(gdb)
300 sysbus_dev = SYS_BUS_DEVICE(&s->escc);
301 sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
302
NEWWORLD_ESCCB_IRQ));
303 sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev,
304
NEWWORLD_ESCCA_IRQ));
305
306 /* OpenPIC */
307 sysbus_dev = SYS_BUS_DEVICE(ns->pic);
308 memory_region_add_subregion(&s->bar, 0x40000,
309 sysbus_mmio_get_region(sysbus_dev,
0));
Maybe something like:
if (!pic_dev) {
error_setg(errp, "some meaningful error message");
return;
}
before the sysbus_connect_irq calls but unless the user can set this via
the command line somehow then keeping the user_creatable = false with
comment adjusted to say that this device needs to be connected by board
code is probably better.
Regards,
BALATON Zoltan
