Re: [PATCH v2 5/6] macio: don't reference serial_hd() directly within the device

[Mark Cave-Ayland]

On 04/11/2020 12:47, 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-ayland@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?

(goes and looks)

Ah okay it appears to be because the object property link to the PIC is missing, 
which is to be expected as it is only present on the Mac machines.

With the latest round of QOM updates I can see the solution but it's probably a bit 
much now that we've reached rc-0. The easiest thing for the moment is to switch 
user_creatable back to false if this is causing an issue.

Just out of interest how did you find this? My new workflow involves a local "make 
check" with all ppc targets and a pass through Travis-CI and it didn't show up there 
for me (or indeed Peter's pre-merge tests).


ATB,

Mark.