qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: mst@redhat.com
Cc: qemu-devel@nongnu.org
Subject: [Qemu-devel] [PATCH 0/2] pci: exit path cleanup and fix
Date: Tue, 03 Jul 2012 22:39:20 -0600	[thread overview]
Message-ID: <20120704041748.19100.95271.stgit@bling.home> (raw)

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.

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

---

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(-)

             reply	other threads:[~2012-07-04  4:39 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-04  4:39 Alex Williamson [this message]
2012-07-04  4:39 ` [Qemu-devel] [PATCH 1/2] pci: convert PCIUnregisterFunc to void Alex Williamson
2012-07-04  4:39 ` [Qemu-devel] [PATCH 2/2] pci: Unregister BARs before device exit Alex Williamson
2012-07-04  8:22 ` [Qemu-devel] [PATCH 0/2] pci: exit path cleanup and fix Paolo Bonzini
2012-07-04 12:47 ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120704041748.19100.95271.stgit@bling.home \
    --to=alex.williamson@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).