From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53945) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z9siU-0000K4-GY for qemu-devel@nongnu.org; Tue, 30 Jun 2015 06:24:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z9siQ-0005zn-BE for qemu-devel@nongnu.org; Tue, 30 Jun 2015 06:24:38 -0400 Received: from cantor2.suse.de ([195.135.220.15]:37555 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z9siQ-0005zI-4Q for qemu-devel@nongnu.org; Tue, 30 Jun 2015 06:24:34 -0400 Message-ID: <55926E56.7060505@suse.de> Date: Tue, 30 Jun 2015 12:24:22 +0200 From: =?windows-1252?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <7d54c33fa2a928fd317f3a95af61f854f43ba23c.1435195913.git.zhugh.fnst@cn.fujitsu.com> <558C33BE.8020603@redhat.com> <558C3A53.3000901@suse.de> <559237D6.8000602@cn.fujitsu.com> In-Reply-To: <559237D6.8000602@cn.fujitsu.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RESEND PATCH v8 2/4] hw: add a wrapper for registering reset handler List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhu Guihua , qemu-devel@nongnu.org Cc: chen.fan.fnst@cn.fujitsu.com, Paolo Bonzini , izumi.taku@jp.fujitsu.com, ehabkost@redhat.com, imammedo@redhat.com Am 30.06.2015 um 08:31 schrieb Zhu Guihua: > On 06/26/2015 01:28 AM, Andreas F=E4rber wrote: >> Am 25.06.2015 um 19:00 schrieb Paolo Bonzini: >>> On 25/06/2015 04:17, Zhu Guihua wrote: >>>> Add a wrapper to specify reset order when registering reset handler, >>>> instead of non-obvious initiazation code ordering. >>>> >>>> Signed-off-by: Zhu Guihua >>> I'm sorry, this is not really acceptable. The initialization code >>> ordering is good because it should be okay to run reset handlers in t= he >>> same order as code is run. If there are dependencies between reset >>> handlers, a random integer is not a maintainable way to maintain them= . >>> >>> Instead, you should have a single reset handler that calls the reset >>> handlers in the right order; for example a qdev bus such as icc_bus >>> always resets children before parents. >>> >>> Are you sure that you want to remove the icc_bus?... What are you >>> gaining exactly by doing so? >> >From my view we would be gaining by making the APIC an integral part >> (child<>) of the CPU in a follow-up step (there's a TODO to make thing= s >> link<>s). >> >> But either way the CPU's existing reset handler should be able to hand= le >> CPU/APIC interdependencies just fine, somehow. Which is what Eduardo >> said on v6 and v7. (Another alternative he raised was a machine init >> notifier, but I see no code for that after its mention on v7?) >=20 > According to Eduardo's suggestions on v7, the simpler way is to add a > ordering parameter > to qemu_register_reset(), so that we can ensure the order of apic reset > handler(apic reset > must be after the other devices' reset on x86). That is a very general statement. Surely the APIC does not need to be reset after *all* other devices but after some particular device(s). Which one is that if not the X86CPU? > This way will not influence the initialization code ordering expect > apic reset. And exactly that's the criticism: You're introducing a generic mechanism to address a single very specific problem. sPAPR already has the MachineClass::reset() callback to order CPU vs. device reset. So if you want a new mechanism you'll need to explain in detail why that is needed and not just say that it solves your issue. Regards, Andreas --=20 SUSE Linux GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Felix Imend=F6rffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB 21284 (AG N=FCrnberg)