[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in
From: |
Mark Cave-Ayland |
Subject: |
Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via |
Date: |
Mon, 9 Mar 2020 16:16:49 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 09/03/2020 14:14, Markus Armbruster wrote:
> Pan Nengyuan <address@hidden> writes:
>
>> On 3/9/2020 8:34 PM, Markus Armbruster wrote:
>>> Peter Maydell <address@hidden> writes:
>>>
>>>> On Mon, 9 Mar 2020 at 10:02, Pan Nengyuan <address@hidden> wrote:
>>>>> On 3/9/2020 5:21 PM, Peter Maydell wrote:
>>>>>> Could you explain more? My thought is that we should be using
>>>>>> sysbus_init_child_obj() and we should be doing it in the init method.
>>>>>> Why does that break the tests ? It's the same thing various other
>>>>>> devices do.
>>>>>
>>>>> device-introspect-test do the follow check for each device type:
>>>>>
>>>>> qtree_start = qtest_hmp(qts, "info qtree");
>>>>> ...
>>>>> qtest_qmp(qts, "{'execute': 'device-list-properties','arguments':
>>>>> {'typename': %s}}", type);
>>>>> ...
>>>>> qtree_end = qtest_hmp(qts, "info qtree");
>>>>> g_assert_cmpstr(qtree_start, ==, qtree_end);
>>>>>
>>>>> If we do qdev_set_parent_bus in init, it will check fail when type =
>>>>> 'mac_via'.
>>>>> mac_via_init() is called by q800_init(). But it will not be called in
>>>>> qtest(-machine none) in the step qtree_start.
>>>>> And after we call 'device-list-properties', mac_via_init() was called and
>>>>> set dev parent bus. We can find these
>>>>> devices in the qtree_end. So it break the test on the assert.
>>>>
>>>> Markus, do you know what's happening here? Why is
>>>> trying to use sysbus_init_child_obj() breaking the
>>>> device-introspect-test for this particular device,
>>>> but fine for the other places where we use it?
>>>> (Maybe we're accidentally leaking a reference to
>>>> something so the sub-device stays on the sysbus
>>>> when it should have removed itself when the
>>>> device was deinited ?)
>>>
>>> Pan Nengyuan, please provide the exact patch that fails for you.
>>
>> As the follow patch:
>>
>> >From 9b4f35e294597410cc03b967c127242ce099692e Mon Sep 17 00:00:00 2001
>> From: Pan Nengyuan <address@hidden>
>> Date: Wed, 4 Mar 2020 11:29:28 +0800
>> Subject: [PATCH] mac_via: fix incorrect creation of mos6522 device in mac_via
>>
>> This patch fix a bug in mac_via where it failed to actually realize devices
>> it was using.
>> And move the init codes which inits the mos6522 objects and properties on
>> them from realize()
>> into init(). However, we keep qdev_set_parent_bus in realize(), otherwise it
>> will cause
>> device-introspect-test test fail. Then do the realize mos6522 device in the
>> mac_vir_realize.
>>
>> Signed-off-by: Pan Nengyuan <address@hidden>
>> ---
>> hw/misc/mac_via.c | 40 ++++++++++++++++++++++++++--------------
>> 1 file changed, 26 insertions(+), 14 deletions(-)
>>
>> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
>> index b7d0012794..4c5c432140 100644
>> --- a/hw/misc/mac_via.c
>> +++ b/hw/misc/mac_via.c
>> @@ -868,24 +868,21 @@ static void mac_via_reset(DeviceState *dev)
>> static void mac_via_realize(DeviceState *dev, Error **errp)
>> {
>> MacVIAState *m = MAC_VIA(dev);
>> - MOS6522State *ms;
>> struct tm tm;
>> int ret;
>> + Error *err = NULL;
>>
>> - /* Init VIAs 1 and 2 */
>> - sysbus_init_child_obj(OBJECT(dev), "via1", &m->mos6522_via1,
>> - sizeof(m->mos6522_via1), TYPE_MOS6522_Q800_VIA1);
>> -
>> - sysbus_init_child_obj(OBJECT(dev), "via2", &m->mos6522_via2,
>> - sizeof(m->mos6522_via2), TYPE_MOS6522_Q800_VIA2);
>> + object_property_set_bool(OBJECT(&m->mos6522_via1), true, "realized",
>> &err);
>> + if (err != NULL) {
>> + error_propagate(errp, err);
>> + return;
>> + }
>>
>> - /* Pass through mos6522 output IRQs */
>> - ms = MOS6522(&m->mos6522_via1);
>> - object_property_add_alias(OBJECT(dev), "irq[0]", OBJECT(ms),
>> - SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
>> - ms = MOS6522(&m->mos6522_via2);
>> - object_property_add_alias(OBJECT(dev), "irq[1]", OBJECT(ms),
>> - SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
>> + object_property_set_bool(OBJECT(&m->mos6522_via2), true, "realized",
>> &err);
>> + if (err != NULL) {
>> + error_propagate(errp, err);
>> + return;
>> + }
>>
>> /* Pass through mos6522 input IRQs */
>> qdev_pass_gpios(DEVICE(&m->mos6522_via1), dev, "via1-irq");
>> @@ -932,6 +929,7 @@ static void mac_via_init(Object *obj)
>> {
>> SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>> MacVIAState *m = MAC_VIA(obj);
>> + MOS6522State *ms;
>>
>> /* MMIO */
>> memory_region_init(&m->mmio, obj, "mac-via", 2 * VIA_SIZE);
>> @@ -948,6 +946,20 @@ static void mac_via_init(Object *obj)
>> /* ADB */
>> qbus_create_inplace((BusState *)&m->adb_bus, sizeof(m->adb_bus),
>> TYPE_ADB_BUS, DEVICE(obj), "adb.0");
>> +
>> + /* Init VIAs 1 and 2 */
>> + sysbus_init_child_obj(OBJECT(m), "via1", &m->mos6522_via1,
>> + sizeof(m->mos6522_via1), TYPE_MOS6522_Q800_VIA1);
>> + sysbus_init_child_obj(OBJECT(dev), "via2", &m->mos6522_via2,
>> + sizeof(m->mos6522_via2), TYPE_MOS6522_Q800_VIA2);
>> +
>> + /* Pass through mos6522 output IRQs */
>> + ms = MOS6522(&m->mos6522_via1);
>> + object_property_add_alias(OBJECT(m), "irq[0]", OBJECT(ms),
>> + SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
>> + ms = MOS6522(&m->mos6522_via2);
>> + object_property_add_alias(OBJECT(m), "irq[1]", OBJECT(ms),
>> + SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
>> }
>>
>> static void postload_update_cb(void *opaque, int running, RunState state)
>> --
>> 2.18.2
>
> In file included from /work/armbru/qemu/include/hw/vmstate-if.h:12,
> from /work/armbru/qemu/include/migration/vmstate.h:30,
> from /work/armbru/qemu/hw/misc/mac_via.c:20:
> /work/armbru/qemu/hw/misc/mac_via.c: In function ‘mac_via_init’:
> /work/armbru/qemu/hw/misc/mac_via.c:953:34: error: ‘dev’ undeclared (first
> use in this function); did you mean ‘div’?
> 953 | sysbus_init_child_obj(OBJECT(dev), "via2", &m->mos6522_via2,
> | ^~~
>
> Try again?
Looks like a simple copy/paste error: I think that line should read OBJECT(m)
like
the one above it?
ATB,
Mark.
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, (continued)
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Pan Nengyuan, 2020/03/05
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Pan Nengyuan, 2020/03/05
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Peter Maydell, 2020/03/08
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Pan Nengyuan, 2020/03/08
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Peter Maydell, 2020/03/09
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Pan Nengyuan, 2020/03/09
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Peter Maydell, 2020/03/09
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Markus Armbruster, 2020/03/09
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Pan Nengyuan, 2020/03/09
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Markus Armbruster, 2020/03/09
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via,
Mark Cave-Ayland <=
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Pan Nengyuan, 2020/03/09
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Markus Armbruster, 2020/03/10
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Peter Maydell, 2020/03/10
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, BALATON Zoltan, 2020/03/10
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Mark Cave-Ayland, 2020/03/14
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Paolo Bonzini, 2020/03/14
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Markus Armbruster, 2020/03/15
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Paolo Bonzini, 2020/03/15
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Markus Armbruster, 2020/03/16
- Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via, Paolo Bonzini, 2020/03/16