From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49608) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sx1Fw-0001UO-JG for qemu-devel@nongnu.org; Thu, 02 Aug 2012 15:40:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Sx1Fv-0001qy-66 for qemu-devel@nongnu.org; Thu, 02 Aug 2012 15:40:24 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:47325) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sx1Fu-0001qr-Vc for qemu-devel@nongnu.org; Thu, 02 Aug 2012 15:40:23 -0400 Received: by obbta14 with SMTP id ta14so13487450obb.4 for ; Thu, 02 Aug 2012 12:40:22 -0700 (PDT) From: Anthony Liguori In-Reply-To: <501AC915.5080004@suse.de> 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> Date: Thu, 02 Aug 2012 14:40:19 -0500 Message-ID: <87lihx84m4.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 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: Andreas =?utf-8?Q?F=C3=A4rber?= Cc: agraf@suse.de, Igor Mammedov , qemu-devel@nongnu.org, David Gibson Andreas F=C3=A4rber writes: > Am 02.08.2012 20:29, schrieb Anthony Liguori: >> Andreas F=C3=A4rber writes: >>=20 >>> Anthony was favoring moving reset code out of machines and expressed >>> dislike for looping through CPUs, which my above patch took into >>> account. The ordering issue between CPU and devices is still unsolved t= here. >>> >>> Some on-list comments from Anthony would be nice, since we are moving >>> into opposing directions here - having the sPAPR machine be more in >>> control vs. moving code away from the PC machine into target-i386 CPU >>> 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 the >> top-level machine which is logically what's plugged into the wall and >> is the source of power in the first place. > > So you changed your opinion over night? No. > I wanted to keep the reset callbacks in the machine. You applied a patch > 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... 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. This has been discussed literally to death now for years. 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). 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. Open coded logic in QEMUMachine::init is always bad. Handling reset for a specific device in QEMUMachine::init is bad. That goes against the idea of making QEMUMachine::init trivial. 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, it's not at all typical. Reset should flow with QEMUMachine::reset just playing the role of deciding whether it starts propagating from. 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. Regards, Anthony Liguori > > If the machine should propagate reset then the disputed i386 patch is > doing The Wrong Thing. > > If reset code should vanish from machine code AFAP then this patch is > doing The Wrong Thing. > > No? > > Regards, > Andreas > > --=20 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3= =BCrnberg