[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: sysbus failed assert for xen_sysdev
From: |
Paul Durrant |
Subject: |
RE: sysbus failed assert for xen_sysdev |
Date: |
Tue, 23 Jun 2020 12:46:41 +0100 |
> -----Original Message-----
> From: Markus Armbruster <armbru@redhat.com>
> Sent: 23 June 2020 09:41
> To: Jason Andryuk <jandryuk@gmail.com>
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>; Anthony PERARD
> <anthony.perard@citrix.com>; xen-
> devel <xen-devel@lists.xenproject.org>; Paul Durrant <paul@xen.org>; QEMU
> <qemu-devel@nongnu.org>
> Subject: Re: sysbus failed assert for xen_sysdev
>
> Jason Andryuk <jandryuk@gmail.com> writes:
>
> > On Mon, Jun 22, 2020 at 5:17 PM Mark Cave-Ayland
> > <mark.cave-ayland@ilande.co.uk> wrote:
> >>
> >> On 22/06/2020 21:33, Jason Andryuk wrote:
> >>
> >> > Hi,
> >> >
> >> > Running qemu devel for a Xen VM is failing an assert after the recent
> >> > "qdev: Rework how we plug into the parent bus" sysbus changes.
> >> >
> >> > qemu-system-i386: hw/core/qdev.c:102: qdev_set_parent_bus: Assertion
> >> > `dc->bus_type && object_dynamic_cast(OBJECT(bus), dc->bus_type)'
> >> > failed.
> >> >
> >> > dc->bus_type is "xen-sysbus" and it's the
> >> > `object_dynamic_cast(OBJECT(bus), dc->bus_type)` portion that fails
> >> > the assert. bus seems to be "main-system-bus", I think:
> >> > (gdb) p *bus
> >> > $3 = {obj = {class = 0x55555636d780, free = 0x7ffff7c40db0 <g_free>,
> >> > properties = 0x5555563f7180, ref = 3, parent = 0x5555563fe980}, parent
> >> > = 0x0, name = 0x5555563fec60 "main-system-bus", ...
> >> >
> >> > The call comes from hw/xen/xen-legacy-backend.c:706
> >> > sysbus_realize_and_unref(SYS_BUS_DEVICE(xen_sysdev), &error_fatal);
> >> >
> >> > Any pointers on what needs to be fixed?
> >>
> >> Hi Jason,
> >>
> >> My understanding is that the assert() is telling you that you're plugging a
> >> TYPE_SYS_BUS_DEVICE into a bus that isn't derived from TYPE_SYSTEM_BUS.
> >> TYPE_SYS_BUS_DEVICE into a bus that isn't derived from TYPE_SYSTEM_BUS. A
> >> quick look
>
> Correct. Let's review the assertion:
>
> assert(dc->bus_type && object_dynamic_cast(OBJECT(bus), dc->bus_type));
>
> Context: we're supposted to plug @dev into @bus, and @dc is @dev's
> DeviceClass.
>
> The assertion checks that
>
> 1. @dev plugs into a bus: dc->bus_type
>
> 2. @bus is an instance of the type of bus @dev plugs into:
> object_dynamic_cast(OBJECT(bus), dc->bus_type)
>
> >> at the file in question suggests that you could try changing the parent
> >> class of
> >> TYPE_XENSYSBUS from TYPE_BUS to TYPE_SYSTEM_BUS to see if that helps?
> >
> > Hi, Mark.
> >
> > Thanks, but unfortunately changing xensysbus_info .parent does not
> > stop the assert. But it kinda pointed me in the right direction.
> >
> > xen-sysdev overrode the bus_type which was breaking sysbus_realize.
> > So drop that:
> >
> > --- a/hw/xen/xen-legacy-backend.c
> > +++ b/hw/xen/xen-legacy-backend.c
> > @@ -824,7 +825,7 @@ static void xen_sysdev_class_init(ObjectClass
> > *klass, void *data)
> > DeviceClass *dc = DEVICE_CLASS(klass);
> >
> > device_class_set_props(dc, xen_sysdev_properties);
> > - dc->bus_type = TYPE_XENSYSBUS;
> > + //dc->bus_type = TYPE_XENSYSBUS;
> > }
>
> Uff!
>
> Let me explain how things are supposed to work.
>
> Say we have FOO bus (QOM type TYPE_FOO_BUS), with FOO devices plugging
> into it (abstract QOM type TYPE_FOO_DEVICE). One of them is SOME_FOO
> (concrete QOM type TYPE_SOME_FOO). Code ties them together like this:
>
> static const TypeInfo pci_bus_info = {
> .name = TYPE_PCI_BUS,
> .parent = TYPE_BUS,
> ...
> };
>
> static const TypeInfo foo_device_info = {
> .name = TYPE_FOO_DEVICE,
> .parent = TYPE_DEVICE,
> .abstract = true,
> .class_init = foo_device_class_init,
> ...
> };
>
> static void foo_device_class_init(ObjectClass *oc, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(oc);
>
> dc->bus_type = TYPE_FOO_BUS;
> ...
> }
>
> static const TypeInfo some_foo_info = {
> .name = TYPE_SOME_FOO,
> .parent = TYPE_FOO_DEVICE,
> ...
> };
>
> When you plug an instance of TYPE_SOME_FOO into a bus, the assertion
> checks that the bus is an instance of TYPE_FOO_BUS.
>
> Note that subtypes of TYPE_FOO_DEVICE do not mess with dc->bus_type!
>
> TYPE_XENSYSDEV does mess with it:
>
> static void xen_sysdev_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> device_class_set_props(dc, xen_sysdev_properties);
> dc->bus_type = TYPE_XENSYSBUS;
> }
>
> static const TypeInfo xensysdev_info = {
> .name = TYPE_XENSYSDEV,
> .parent = TYPE_SYS_BUS_DEVICE,
> .instance_size = sizeof(SysBusDevice),
> .class_init = xen_sysdev_class_init,
> };
>
> On the one hand, xensysdev_info.parent claims TYPE_XENSYSDEV is a
> TYPE_SYS_BUS_DEVICE (and therefore should plug into a TYPE_SYSTEM_BUS).
> On the other hand, its dc->bus_type is a TYPE_XENSYSBUS, which is *not*
> a subtype of TYPE_SYSTEM_BUS:
>
> static const TypeInfo xensysbus_info = {
> .name = TYPE_XENSYSBUS,
> ---> .parent = TYPE_BUS,
> .class_init = xen_sysbus_class_init,
> .interfaces = (InterfaceInfo[]) {
> { TYPE_HOTPLUG_HANDLER },
> { }
> }
> };
>
> This is an inconsistent mess.
>
> Are TYPE_XENSYSDEV and TYPE_XENSYSBUS related to TYPE_SYS_BUS_DEVICE and
> TYPE_SYSTEM_BUS?
>
> If no, then xensysbus_info.parent should not be TYPE_SYS_BUS_DEVICE, and
> you must not pass instances of one kind to functions expecting the other
> kind.
>
> If yes, how? If the former are specializations of the latter, consider
> making the former subtypes of the latter. Both of them. Then a
> TYPE_XENSYSDEV device can plug into a TYPE_XENSYSBUS bus, but not into a
> TYPE_SYSTEM_BUS bus.
>
> A TYPE_SYS_BUS_DEVICE could still plug into TYPE_XENSYSBUS, because the
> latter is also an instance of TYPE_SYSTEM_BUS.
>
> Questions?
>
> >
> > static const TypeInfo xensysdev_info = {
> >
> > Then I had a different instance of the failed assert trying to attach
> > xen-console-0 to xen-sysbus. So I made this change:
> > --- a/hw/xen/xen-legacy-backend.c
> > +++ b/hw/xen/xen-legacy-backend.c
> > @@ -789,6 +789,7 @@ static void xendev_class_init(ObjectClass *klass,
> > void *data)
> > set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> > /* xen-backend devices can be plugged/unplugged dynamically */
> > dc->user_creatable = true;
> > + dc->bus_type = TYPE_XENSYSBUS;
> > }
> >
> > static const TypeInfo xendev_type_info = {
> >
> > Then it gets farther... until
> > qemu-system-i386: hw/core/qdev.c:439: qdev_assert_realized_properly:
> > Assertion `dev->realized' failed.
> >
> > dev->id is NULL. The failing device is:
> > (gdb) p *dev.parent_obj.class.type
> > $12 = {name = 0x555556207770 "cfi.pflash01",
> >
Having commented out the call to xen_be_init() entirely (and xen_bus_init() for
good measure) I also get this assertion failure, so
I don't think is related.
Paul
- sysbus failed assert for xen_sysdev, Jason Andryuk, 2020/06/22
- Re: sysbus failed assert for xen_sysdev, Mark Cave-Ayland, 2020/06/22
- Re: sysbus failed assert for xen_sysdev, Jason Andryuk, 2020/06/22
- Re: sysbus failed assert for xen_sysdev, Markus Armbruster, 2020/06/23
- RE: sysbus failed assert for xen_sysdev,
Paul Durrant <=
- Re: sysbus failed assert for xen_sysdev, Jason Andryuk, 2020/06/23
- RE: sysbus failed assert for xen_sysdev, Paul Durrant, 2020/06/24
- Re: sysbus failed assert for xen_sysdev, Jason Andryuk, 2020/06/24
- RE: sysbus failed assert for xen_sysdev, Paul Durrant, 2020/06/24
- Re: sysbus failed assert for xen_sysdev, Jason Andryuk, 2020/06/23
- RE: sysbus failed assert for xen_sysdev, Paul Durrant, 2020/06/23
- Re: sysbus failed assert for xen_sysdev, Jason Andryuk, 2020/06/23