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
