From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MGdlp-0000PT-29 for qemu-devel@nongnu.org; Tue, 16 Jun 2009 14:52:33 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MGdlk-0000Mn-Fe for qemu-devel@nongnu.org; Tue, 16 Jun 2009 14:52:32 -0400 Received: from [199.232.76.173] (port=43615 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MGdlk-0000Mg-4b for qemu-devel@nongnu.org; Tue, 16 Jun 2009 14:52:28 -0400 Received: from mail2.shareable.org ([80.68.89.115]:38150) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1MGdlj-0004JP-C5 for qemu-devel@nongnu.org; Tue, 16 Jun 2009 14:52:27 -0400 Date: Tue, 16 Jun 2009 19:52:18 +0100 From: Jamie Lokier Subject: Re: [Qemu-devel] Register uhci_reset() callback. Message-ID: <20090616185218.GK11893@shareable.org> References: <20090611084808.GA19508@redhat.com> <200906161754.59643.paul@codesourcery.com> <200906161900.11811.paul@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Avi Kivity , Paul Brook , Gleb Natapov , qemu-devel@nongnu.org Blue Swirl wrote: > On 6/16/09, Paul Brook wrote: > > > > Devices should not cause IRQ state changes on restore. Commit 3dcd219f > > > > is incorrect. > > > > > > I'm not so sure about this, but I can't think of a restore sequence > > > where the IRQ state would need to be changed if the IRQs tied together > > > are handled correctly. But surely if the devices states are restored > > > in strange order, the state changes could cause problems because the > > > device receiving the IRQ may still contain old state. > > > > > > It's precisely because devices are restored in unpredictable order that they > > should not be communicating with other devices (e.g. by modifying IRQ lines). > > > > Consider a system with a device (DEV) and a level triggered interrupt > > controller (PIC1) chained to an edge triggered interrupt controller (PIC2). > > > > (DEV) -> (PIC1) -> (PIC2) > > > > Before restore, DEV output is low, PIC1 has the interrupt unmasked (but low), > > PIC2 has no pending interrupts. > > > > We now restore a state where DEV output is high, PIC1 has masked the > > interrupt, and PIC2 has no pending interrupts. Devices are restored in he > > order PIC2, DEV, PIC1. > > > > If devices toggle their interrupts on restore then we get incorrect state > > after the restore: > > > > PIC2 is restored to the desired no-interrupts-pending state. > > DEV is restored. This raises the IRQ, which is passed to PIC1. PIC1 still has > > the old interrupt mask, so passes through to PIC2, which detects the edge > > event and marks the interrupt as pending. > > PIC1 is restored, updates the new mask and lowers its output. However this > > does not clear the internal PIC2 pending interrupt flag, so machine state will > > be wrong after resume. > > Yes, this is the "bad" restore scenario that I had in mind. I still > have a nagging feeling that there is a reverse scenario, where in > order to reach good state, you would have to call the IRQ function. > Anyway, 3dcd219f may not be correct in that case either. If any initialisation order might occur, then you really need all the states to be restored, including output levels, without any side effects, and then enable processing in all devices together. It's analogous to a distributed atomic transaction problem. So you need two phases: - Restoring: All devices states are restored, one by one including output levels of interrupt lines and GPIOs, but nothing actually _happens_ when those levels are set. - Running: All devices start running at the same instant from their restored state. They don't need to reexamine input levels, because their internal states are already consistent with the input levels. In the restoring phase, you might still use the normal functions to set output levels, but they would be prevented from being passed as changes to other devices. Anything else might be made to work with particular PICs etc., but two-phase restore is what you need to work with any wiring (including cycles) of arbitrary devices with arbitrary states. -- Jamie