Re: [Qemu-devel] [PATCH] Fix duplicate device reset
Hi, here is the patch. Can you please give it a try?
From 41039df3174fa46477c4faf93d13eab360dccc22 Mon Sep 17 00:00:00 2001
Message-Id:
41039df3174fa46477c4faf93d13eab360dccc22.1312196365.git.yamah...@valinux.co.jp
From: Isaku Yamahata yamah...@valinux.co.jp
Date: Mon, 1 Aug 2011 19:56:42 +0900
Subject: [PATCH] qdev: Fix duplicate reset
qbus_reset_all_fn was registered twice, so a lot of device reset
functions were also called twice when QEMU started.
Which was introduced by 80376c3fc2c38fdd45354e4b0eb45031f35587ed
This patch fixes it by making the main_sytem_bus creation not register
reset handler.
Cc: Stefan Weil w...@mail.berlios.de
Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp
---
hw/qdev.c | 14 --
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index b4ea8e1..6819537 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -36,6 +36,7 @@ static bool qdev_hot_removed = false;
/* This is a nasty hack to allow passing a NULL bus to qdev_create. */
static BusState *main_system_bus;
+static void main_system_bus_create(void);
DeviceInfo *device_info_list;
@@ -328,8 +329,7 @@ static int qdev_reset_one(DeviceState *dev, void *opaque)
BusState *sysbus_get_default(void)
{
if (!main_system_bus) {
-main_system_bus = qbus_create(system_bus_info, NULL,
- main-system-bus);
+main_system_bus_create();
}
return main_system_bus;
}
@@ -784,6 +784,16 @@ BusState *qbus_create(BusInfo *info, DeviceState *parent,
const char *name)
return bus;
}
+static void main_system_bus_create(void)
+{
+/* assign main_system_bus before qbus_create_inplace()
+ * in order to make if (bus != main_system_bus) work */
+main_system_bus = qemu_mallocz(system_bus_info.size);
+main_system_bus-qdev_allocated = 1;
+qbus_create_inplace(main_system_bus, system_bus_info, NULL,
+main-system-bus);
+}
+
void qbus_free(BusState *bus)
{
DeviceState *dev;
--
1.7.1.1
--
yamahata
Re: [Qemu-devel] [PATCH] Fix duplicate device reset
Am 01.08.2011 13:00, schrieb Isaku Yamahata: Hi, here is the patch. Can you please give it a try? From 41039df3174fa46477c4faf93d13eab360dccc22 Mon Sep 17 00:00:00 2001 Message-Id: 41039df3174fa46477c4faf93d13eab360dccc22.1312196365.git.yamah...@valinux.co.jp From: Isaku Yamahata yamah...@valinux.co.jp Date: Mon, 1 Aug 2011 19:56:42 +0900 Subject: [PATCH] qdev: Fix duplicate reset qbus_reset_all_fn was registered twice, so a lot of device reset functions were also called twice when QEMU started. Which was introduced by 80376c3fc2c38fdd45354e4b0eb45031f35587ed This patch fixes it by making the main_sytem_bus creation not register main_system_bus reset handler. Cc: Stefan Weil w...@mail.berlios.de Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp --- hw/qdev.c | 14 -- 1 files changed, 12 insertions(+), 2 deletions(-) Thanks. I tested your patch with i386-softmmu (bios only) and with mipsel-softmmu (debian boot / malta). All registered reset functions were called only once, and qbus_reset_all_fn was the last one called. I noticed that there are two functions named piix3_reset. One might be renamed to piix3_ide_reset, but this is not related to your patch. There are also two functions piix4_reset. Tested-by: Stefan Weil w...@mail.berlios.de
Re: [Qemu-devel] [PATCH] Fix duplicate device reset
Am 19.07.2011 04:39, schrieb Isaku Yamahata: Thank you for addressing this. Similar patches were proposed and weren't merged unfortunately. The reason why the qdev_register_reset() in vl.c is to keep the reset order. The reset for main_system_bus shouldn't registered by qbus_create_inplace(). But the check, bus != main_system_bus, doesn't work as intended because main_system_bus is NULL in early qdev creation. So there are possible ways for the fix. - Don't care the reset order your patch + remove if (bus != main_system_bus) in qbus_create_inplace() - keep the reset order - instantiate main_system_bus early. So the check, bus != main_system_bus in qbus_create_inplace(), will work. or - fix the check, bus != main_system_bus in qbus_create_inplace(), somehow thanks, Hi, my patch does not remove sysbus_get_default(), so the reset order is kept because main_system_bus is instantiated by this call. Cheers, Stefan
Re: [Qemu-devel] [PATCH] Fix duplicate device reset
On Tue, Jul 19, 2011 at 07:56:41AM +0200, Stefan Weil wrote: Am 19.07.2011 04:39, schrieb Isaku Yamahata: Thank you for addressing this. Similar patches were proposed and weren't merged unfortunately. The reason why the qdev_register_reset() in vl.c is to keep the reset order. The reset for main_system_bus shouldn't registered by qbus_create_inplace(). But the check, bus != main_system_bus, doesn't work as intended because main_system_bus is NULL in early qdev creation. So there are possible ways for the fix. - Don't care the reset order your patch + remove if (bus != main_system_bus) in qbus_create_inplace() - keep the reset order - instantiate main_system_bus early. So the check, bus != main_system_bus in qbus_create_inplace(), will work. or - fix the check, bus != main_system_bus in qbus_create_inplace(), somehow thanks, Hi, my patch does not remove sysbus_get_default(), so the reset order is kept because main_system_bus is instantiated by this call. Yes, your patch doesn't change the order from the existing code. I think it's not intended one. During machine creation, someone may call sysbus_get_default(). So the reset for main_system_bus may not be the lastly registered. The changeset of 80376c3f tries to keep the reset order, but failed. That's the issue. -- yamahata
[Qemu-devel] [PATCH] Fix duplicate device reset
qbus_reset_all_fn was registered twice, so a lot of device reset functions were also called twice when QEMU started. It is sufficient to call sysbus_get_default() which will register qbus_reset_all_fn. Signed-off-by: Stefan Weil w...@mail.berlios.de --- vl.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/vl.c b/vl.c index fcd7395..fb2f6db 100644 --- a/vl.c +++ b/vl.c @@ -3301,7 +3301,7 @@ int main(int argc, char **argv, char **envp) /* TODO: once all bus devices are qdevified, this should be done * when bus is created by qdev.c */ -qemu_register_reset(qbus_reset_all_fn, sysbus_get_default()); +sysbus_get_default(); qemu_run_machine_init_done_notifiers(); qemu_system_reset(VMRESET_SILENT); -- 1.7.2.5
Re: [Qemu-devel] [PATCH] Fix duplicate device reset
Thank you for addressing this. Similar patches were proposed and weren't merged unfortunately. The reason why the qdev_register_reset() in vl.c is to keep the reset order. The reset for main_system_bus shouldn't registered by qbus_create_inplace(). But the check, bus != main_system_bus, doesn't work as intended because main_system_bus is NULL in early qdev creation. So there are possible ways for the fix. - Don't care the reset order your patch + remove if (bus != main_system_bus) in qbus_create_inplace() - keep the reset order - instantiate main_system_bus early. So the check, bus != main_system_bus in qbus_create_inplace(), will work. or - fix the check, bus != main_system_bus in qbus_create_inplace(), somehow thanks, On Mon, Jul 18, 2011 at 10:22:26PM +0200, Stefan Weil wrote: qbus_reset_all_fn was registered twice, so a lot of device reset functions were also called twice when QEMU started. It is sufficient to call sysbus_get_default() which will register qbus_reset_all_fn. Signed-off-by: Stefan Weil w...@mail.berlios.de --- vl.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/vl.c b/vl.c index fcd7395..fb2f6db 100644 --- a/vl.c +++ b/vl.c @@ -3301,7 +3301,7 @@ int main(int argc, char **argv, char **envp) /* TODO: once all bus devices are qdevified, this should be done * when bus is created by qdev.c */ -qemu_register_reset(qbus_reset_all_fn, sysbus_get_default()); +sysbus_get_default(); qemu_run_machine_init_done_notifiers(); qemu_system_reset(VMRESET_SILENT); -- 1.7.2.5 -- yamahata
