From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55261) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XReic-00008K-SE for qemu-devel@nongnu.org; Wed, 10 Sep 2014 06:01:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XReiT-0008Me-QW for qemu-devel@nongnu.org; Wed, 10 Sep 2014 06:01:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:15679) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XReiT-0008MU-He for qemu-devel@nongnu.org; Wed, 10 Sep 2014 06:01:33 -0400 Date: Wed, 10 Sep 2014 14:04:51 +0300 From: "Michael S. Tsirkin" Message-ID: <20140910110451.GA11512@redhat.com> References: <1410265809-27247-1-git-send-email-pbonzini@redhat.com> <1410265809-27247-10-git-send-email-pbonzini@redhat.com> <20140909135424.GA13212@redhat.com> <540F35E3.7060207@redhat.com> <20140909205122.GB15637@redhat.com> <54100E16.1040306@redhat.com> <20140910105051.GE7902@redhat.com> <541020CA.5000901@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <541020CA.5000901@redhat.com> Subject: Re: [Qemu-devel] [PATCH 09/10] piix: do not raise irq while loading vmstate List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: amit.shah@redhat.com, quintela@redhat.com, dgilbert@redhat.com, Pavel.Dovgaluk@ispras.ru, qemu-devel@nongnu.org On Wed, Sep 10, 2014 at 11:58:34AM +0200, Paolo Bonzini wrote: > Il 10/09/2014 12:50, Michael S. Tsirkin ha scritto: > > OK, got this, thanks for the explanation! > > So the reason i8259 might be out of sync is > > because it's not yet deserialized. > > Yes, especially the IMR/IRR/ISR fields. > > > I think it's a good idea to put (at least the > > last part) in the commit log. > > Like this: > > This patch disables raising an irq while loading the state of PCI bridge. > Because the i8259 has not been deserialized yet, raising an interrupt > could bring the system out-of-sync with the migration source. For example, > the migration source could have masked the interrupt in the i8259. On the > destination, the i8259 device model would not know that yet and would > trigger an interrupt in the CPU. > > This patch eliminates raising an irq and just restores the calculated > state fields in post_load function. Interrupt state will be deserialized > separately through the IRR field of the i8259. Yes, thanks! Except imho it's a bit better to s/raising/setting/ in the last paragraph. > > Also it's updating irq state, not just raising irq, > > that might be problematic, right? > > Well, the i8259 is in the reset state so ISR=IRR=0, aka all IRQ lines > are known to be low. By luck, yes. > But yes, in general it's the update that is > problematic.