From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43258) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sx7ld-00058V-Ok for qemu-devel@nongnu.org; Thu, 02 Aug 2012 22:37:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Sx7lc-0007Ti-0N for qemu-devel@nongnu.org; Thu, 02 Aug 2012 22:37:33 -0400 Received: from ozlabs.org ([203.10.76.45]:44722) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sx7lb-0007Tc-Bl for qemu-devel@nongnu.org; Thu, 02 Aug 2012 22:37:31 -0400 Date: Fri, 3 Aug 2012 12:37:26 +1000 From: David Gibson Message-ID: <20120803023726.GH12733@truffala.fritz.box> References: <1343873409-8571-1-git-send-email-david@gibson.dropbear.id.au> <1343873409-8571-3-git-send-email-david@gibson.dropbear.id.au> <501AA071.3030406@suse.de> <87vch1i1va.fsf@codemonkey.ws> <501AC915.5080004@suse.de> <87lihx84m4.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <87lihx84m4.fsf@codemonkey.ws> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/2] pseries: Use new hook to correct reset sequence List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: agraf@suse.de, Igor Mammedov , Andreas =?iso-8859-1?Q?F=E4rber?= , qemu-devel@nongnu.org On Thu, Aug 02, 2012 at 02:40:19PM -0500, Anthony Liguori wrote: > Andreas F=E4rber writes: >=20 > > Am 02.08.2012 20:29, schrieb Anthony Liguori: > >> Andreas F=E4rber writes: > >>=20 > >>> Anthony was favoring moving reset code out of machines and expresse= d > >>> dislike for looping through CPUs, which my above patch took into > >>> account. The ordering issue between CPU and devices is still unsolv= ed there. > >>> > >>> Some on-list comments from Anthony would be nice, since we are movi= ng > >>> into opposing directions here - having the sPAPR machine be more in > >>> control vs. moving code away from the PC machine into target-i386 C= PU > >>> and/or common CPU code. > >>=20 > >> I already commented on the first patch because I had a feeling you'd > >> post something like this ;-) > > > > I was not cc'ed. :( > > > >> Regarding reset: > >>=20 > >> 1) Devices should implement DeviceState::reset() > >>=20 > >> 2) If a device doesn't implement ::reset(), it should call > >> qemu_register_reset() > >>=20 > >> 3) Reset should propagate through the device model, starting with th= e > >> top-level machine which is logically what's plugged into the wall an= d > >> is the source of power in the first place. > > > > So you changed your opinion over night? >=20 > No. >=20 > > I wanted to keep the reset callbacks in the machine. You applied a pa= tch > > breaking that pattern and argued you wanted to move reset code *out* = of > > the machine. Now you say the machine should *propagate* reset. Sorry, > > that's unlogical to me... >=20 > You're not listening carefully. Just a friendly piece of advise-- > instead of sending knee-jerk emails, spend some time going back and > re-reading these discussions. >=20 > This has been discussed literally to death now for years. >=20 > Reset propagates. There is unanimous consensus that this is the Right > Way to model reset. There is also wide consensus that reset typically > will propagate through the composition tree although in some cases, > reset actually propagates through the bus (this mostly affects devices > that are children of /peripheral paths though). >=20 > The "root" of the composition tree is the machine. The machine in the > abstract sense, not the QEMUMachine sense. QEMUMachine::init() should > eventually become trivial--just create a handful of devices that > represent the core components of the machine with everything else being > created through composition. So what code controls the order in which "the machine in the abstract sense" initiates the reset at the top-leve? > Open coded logic in QEMUMachine::init is always bad. Handling reset fo= r > a specific device in QEMUMachine::init is bad. That goes against the > idea of making QEMUMachine::init trivial. >=20 > However, reset does logically start at QEMUMachine. That doesn't mean > that QEMUMachine should be explicitly resetting devices in a specific > order. This is why I was quick to comment on David's patch because the > argument about having a controller that determines reset ordering was > silly. While this does exist on some architectures, Some platforms; architecture does not imply a particular platform - this is one of the more subtle and pervasive x86-isms around. > it's not at all > typical. So? If it sometimes exists, we need to support that model. The argument that "real" hardware never has reset order dependencies is simply incorrect. > Reset should flow with QEMUMachine::reset just playing the > role of deciding whether it starts propagating from. >=20 > The only machines that can have complex reset logic are ones that can > afford to take an extremely long time to startup--typically doing a > tremendous amount of self-checks in the process. These are not common > among the types of machines QEMU simulates. "having at least one order dependency in reset" !=3D "complex and slow reset logic". --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other= _ | _way_ _around_! http://www.ozlabs.org/~dgibson