Re: [Qemu-devel] [PATCH v7 1/3] sysbus: register reset handler for main_system_bus when created

[Eduardo Habkost]

On Wed, Jun 03, 2015 at 02:26:27PM +0800, Zhu Guihua wrote:
> Hi Eduardo,
> 
> On 06/02/2015 11:53 PM, Eduardo Habkost wrote:
> >On Tue, Jun 02, 2015 at 05:23:55PM +0800, Zhu Guihua wrote:
> >>Since icc bus will be droped and apic reset should be after bus
> >>reset in x86, this patch moves reset handler for main_system_bus
> >>from vl.c to sysbus.c
> >So, you are changing reset ordering for all machines because of a
> >x86-specific requirement?
> >
> >Other machines can have different ordering requirements and may break if
> >you unconditionally change the reset order for all machines.
> >
> >Like I said on v6: if x86 has specific CPU reset ordering requirements,
> >we should be able to ensure the expected ordering in x86-specific code
> >(in pc.c?), not hardcode reset ordering for all machines.
> >
> >Maybe we can change the qemu_register_reset() API to accept an explicit
> >ordering parameter, so ordering requirements are visibly encoded in the
> >qemu_register_reset() calls instead of depending on non-obvious
> >initialization code ordering.
> >
> >While we don't have a better API, we can probably ensure the x86 reset
> >ordering without affecting the other machines by calling
> >qemu_register_reset() for the CPUs in a machine_init_done notifier
> >registered by PC.
> 
> I try to call qemu_register_reset() for the CPUs in
> pc_guest_info_machine_done (),
> the notifier is in hw/i386/pc.c. And I have tested this, it works well.

If you do that, please add a comment to pc_new_cpus() explaining why the
reset handler needs to be registered later at machine_done. You also
need to remove the reset handler registration from x86_cpu_realizefn()
and register the reset handler at pc_hot_add_cpu() too.

(If we add a ordering parameter to qemu_register_reset(), we will be
able to just register the reset handler at pc_new_cpu(), making
everything simpler).

-- 
Eduardo