From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60593) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VTwLn-0000LO-Us for qemu-devel@nongnu.org; Wed, 09 Oct 2013 12:11:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VTwLf-000841-GW for qemu-devel@nongnu.org; Wed, 09 Oct 2013 12:11:03 -0400 Received: from mail-qc0-x235.google.com ([2607:f8b0:400d:c01::235]:58203) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VTwLf-00083r-Cc for qemu-devel@nongnu.org; Wed, 09 Oct 2013 12:10:55 -0400 Received: by mail-qc0-f181.google.com with SMTP id q4so750367qcx.12 for ; Wed, 09 Oct 2013 09:10:54 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <52558009.4000701@redhat.com> Date: Wed, 09 Oct 2013 18:10:49 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1380807975-13266-1-git-send-email-pbonzini@redhat.com> <20131003135404.GA1267@redhat.com> <524D9426.20707@redhat.com> <20131003165417.GA4616@redhat.com> <524DA0F6.3050007@redhat.com> <20131006182806.GA16531@redhat.com> <5251C96D.1020709@redhat.com> In-Reply-To: <5251C96D.1020709@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/4] qdev: switch reset to post-order, clean up PCI reset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: qemu-devel@nongnu.org, "Michael S. Tsirkin" Il 06/10/2013 22:34, Paolo Bonzini ha scritto: > Il 06/10/2013 20:28, Michael S. Tsirkin ha scritto: >>>>> For each PCI device I tried creating a VM with an instance of it (a few >>>>> devices at a time), and did VM resets. Earlier versions were tested by >>>>> the guy who reported the SCSI problems. >>>> >>>> x86 kvm only? >>> >>> Yes. >> >> Hmm, I'm not sure that's enough for this kind of change. > > I'll do more tests though, from looking at the source code, I'm not sure > what could happen depending on the host bridge. Did more tests, PPC g3beige and PPC64 mac99 both work. I also tested resetting the secondary bus of a PCI bridge (via setpci), and it also works as expected. Finally, I looked more at the history of the code to justify patch 2. Initially, zeroing the irq_state was added in commit 6eaa684 (Add pci_bus_reset() function., 2009-06-17) to deal with this issue: >> Shouldn't each device's reset function bring its line low, thus zeroing >> the irq_state naturally? >> >> If not, we have a bug somewhere. Note we have exactly the same issue >> with save/restore. >> > They should, but I haven't found one that does. More registers were then cleared by pci_device_reset in your commit c0b1905 (qemu/pci: reset device registers on bus reset, 2009-09-16). Deasserting interrupts explicitly came in later as part of PCI bus and FLR support in commit 4c92325 (pci: deassert intx on reset., 2011-01-20). That should be the point where the code starts following the invariant of patch 2. Paolo