| Message ID | 1311020546-9769-1-git-send-email-weil@mail.berlios.de |
|---|---|
| State | Superseded |
| Headers | show |
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 <weil@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
>
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
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.
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);
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 <weil@mail.berlios.de> --- vl.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)