From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58167) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SmOzQ-0002b9-HS for qemu-devel@nongnu.org; Wed, 04 Jul 2012 08:47:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SmOzL-0004xb-81 for qemu-devel@nongnu.org; Wed, 04 Jul 2012 08:47:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58350) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SmOzL-0004xT-0K for qemu-devel@nongnu.org; Wed, 04 Jul 2012 08:47:23 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q64ClLth031087 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 4 Jul 2012 08:47:21 -0400 Date: Wed, 4 Jul 2012 15:47:39 +0300 From: "Michael S. Tsirkin" Message-ID: <20120704124739.GA25976@redhat.com> References: <20120704041748.19100.95271.stgit@bling.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120704041748.19100.95271.stgit@bling.home> Subject: Re: [Qemu-devel] [PATCH 0/2] pci: exit path cleanup and fix List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: qemu-devel@nongnu.org On Tue, Jul 03, 2012 at 10:39:20PM -0600, Alex Williamson wrote: > We've currently got a bug in pci_unregister_device in the ordering of > calling the driver exit function and unregistering io regions. In > every driver memory regions are created in the init function and > destroyed in the exit function. By calling pci_unregister_io_regions > after the exit function, we're calling memory_region_del_subregion > with a pointer to a MemoryRegion that has already been destroyed. > > It's easy enough to change the ordering, but the exit function is > currently allowed to fail. Even if we wanted to restore the device > at that point, we've interrupted the mappings from the guest > perspective and it seems precarious at best whether an exit function > can fail and leave a usable device. Fortunately nobody has any > possibility of actually failing the exit path. Normally I'm a > proponent of error paths, but allowing an exit to fail is like > allowing free(3) to fail. Like, totally. > So, firt redefine that exit can't fail, then fix the ordering of > pci_unregister_device(). If anyone has plans for a failure case in > the exit path, please speak now. Thanks, > > Alex Applied, thanks! > --- > > Alex Williamson (2): > pci: Unregister BARs before device exit > pci: convert PCIUnregisterFunc to void > > > hw/ac97.c | 3 +-- > hw/e1000.c | 3 +-- > hw/eepro100.c | 3 +-- > hw/es1370.c | 3 +-- > hw/ide/cmd646.c | 4 +--- > hw/ide/ich.c | 4 +--- > hw/ide/piix.c | 4 +--- > hw/ide/via.c | 4 +--- > hw/intel-hda.c | 3 +-- > hw/ioh3420.c | 8 +++----- > hw/ivshmem.c | 4 +--- > hw/lsi53c895a.c | 4 +--- > hw/ne2000.c | 3 +-- > hw/pci.c | 11 +++++------ > hw/pci.h | 2 +- > hw/pci_bridge.c | 3 +-- > hw/pci_bridge.h | 2 +- > hw/pci_bridge_dev.c | 12 ++++-------- > hw/pcnet-pci.c | 3 +-- > hw/rtl8139.c | 3 +-- > hw/usb/hcd-uhci.c | 3 +-- > hw/virtio-pci.c | 23 +++++++++++------------ > hw/wdt_i6300esb.c | 4 +--- > hw/xio3130_downstream.c | 8 +++----- > hw/xio3130_upstream.c | 8 +++----- > 25 files changed, 48 insertions(+), 84 deletions(-)