On Tue, Sep 03, 2013 at 02:32:51PM +0200, Paolo Bonzini wrote: > QOM splits the destruction of a device in two phases: > > - unrealize, also known as "exit" from qdev times, should isolate > the device from the guest. After unrealize returns, the guest > should not be able to issue new requests. > > - instance_finalize will reclaim the memory. This is only called > after all requests terminate and drop the references on the > device. > > Though overlooked, this is important even now: QEMU's little secret is > that devices already do access memory out of the iothread mutex (with > address_space_map/unmap and AIO), and this can be MMIO memory too > through a bounce buffer. This series prepares things so that, once > we'll put the memory_region_ref/unref infrastructure to complete use, > things will just work. > > Of course this split will be particularly important for devices that > will be able to do unlocked MMIO. > > This series changes all PCI devices (the sole to support hotplug _and_ > use MemoryRegions) to do memory_region_del_subregion at unrealize time, > and memory_region_destroy at instance_finalize time. As it is mostly > a PCI patch, it should go through mst's tree. > > Paolo
So lots of devices modify guest state until you stop them, and ATM we stop them through the exit function. For this reason I think you can't just change all devices to do all cleanup in finalize, you need to split it up. Sents comments on e.g. e1000 but it applies widely. > Paolo Bonzini (38): > qdev: document assumption that unrealize is followed by finalize > pci: split exit and finalize > ac97: use instance_finalize instead of exit > es1370: use instance_finalize instead of exit > hda: reclaim memory in instance_finalize instead of exit > serial: reclaim memory in instance_finalize instead of exit > tpci200: use instance_finalize instead of exit > pci-assign: reclaim memory in instance_finalize instead of exit > ahci: reclaim memory in instance_finalize instead of exit > msix: split msix_free from msix_uninit > cmd646: use instance_finalize instead of exit > ide/piix: use instance_finalize instead of exit > ide/via: use instance_finalize instead of exit > ivshmem: reclaim memory in instance_finalize instead of exit > pci-testdev: use instance_finalize instead of exit > vfio: reclaim memory in instance_finalize instead of exit > e1000: use instance_finalize instead of exit > eepro100: use instance_finalize instead of exit > ne2000: use instance_finalize instead of exit > pcnet: use instance_finalize instead of exit > rtl8139: use instance_finalize instead of exit > vmxnet3: reclaim memory in instance_finalize instead of exit > shpc: split shpc_free from shpc_cleanup > pci_bridge: split pci_bridge_free from pci_bridge_exitfn > pcie_aer: pcie_aer_exit really frees stuff > pci_bridge: reclaim memory in instance_finalize instead of exit > ioh4320: reclaim memory in instance_finalize instead of exit > xio3130-downstream: reclaim memory in instance_finalize instead of > exit > xio3130-upstream: reclaim memory in instance_finalize instead of exit > pcie: do not recreate mmcfg I/O region, use an alias instead > esp: use instance_finalize instead of exit > lsi: use instance_finalize instead of exit > pvscsi: reclaim memory in instance_finalize instead of exit > usb-uhci: use instance_finalize instead of exit > virtio-pci: reclaim memory in instance_finalize instead of exit > wdt_i6300esb: use instance_finalize instead of exit > xen_pt: reclaim memory in instance_finalize instead of exit > tpm: move add/del_subregion to realize/unrealize > > hw/audio/ac97.c | 5 ++-- > hw/audio/es1370.c | 5 ++-- > hw/audio/intel-hda.c | 8 ++++++ > hw/char/serial-pci.c | 24 ++++++++++++++++++ > hw/char/tpci200.c | 5 ++-- > hw/i386/kvm/pci-assign.c | 8 ++++++ > hw/ide/ahci.c | 2 +- > hw/ide/ahci.h | 2 +- > hw/ide/cmd646.c | 6 ++--- > hw/ide/ich.c | 12 ++++++--- > hw/ide/piix.c | 9 ++++--- > hw/ide/via.c | 6 ++--- > hw/misc/ivshmem.c | 13 +++++++--- > hw/misc/pci-testdev.c | 6 ++--- > hw/misc/vfio.c | 52 > +++++++++++++++++++++++++++++++++++--- > hw/net/e1000.c | 6 ++--- > hw/net/eepro100.c | 5 ++-- > hw/net/ne2000.c | 5 ++-- > hw/net/pcnet-pci.c | 6 ++--- > hw/net/rtl8139.c | 6 ++--- > hw/net/vmxnet3.c | 14 ++++++++-- > hw/pci-bridge/i82801b11.c | 1 + > hw/pci-bridge/ioh3420.c | 11 +++++++- > hw/pci-bridge/pci_bridge_dev.c | 13 +++++++++- > hw/pci-bridge/xio3130_downstream.c | 11 +++++++- > hw/pci-bridge/xio3130_upstream.c | 11 +++++++- > hw/pci/msix.c | 22 +++++++++++----- > hw/pci/pci.c | 15 ++++++++--- > hw/pci/pci_bridge.c | 5 ++++ > hw/pci/pcie_aer.c | 3 ++- > hw/pci/pcie_host.c | 23 ++++++++++++----- > hw/pci/shpc.c | 8 +++++- > hw/scsi/esp-pci.c | 6 ++--- > hw/scsi/lsi53c895a.c | 6 ++--- > hw/scsi/vmw_pvscsi.c | 12 ++++++++- > hw/tpm/tpm_tis.c | 17 +++++++++---- > hw/usb/hcd-uhci.c | 5 ++-- > hw/virtio/virtio-pci.c | 10 +++++++- > hw/watchdog/wdt_i6300esb.c | 5 ++-- > hw/xen/xen_pt.c | 10 ++++++++ > hw/xen/xen_pt_config_init.c | 3 --- > hw/xen/xen_pt_msi.c | 8 +++++- > include/hw/pci/msix.h | 1 + > include/hw/pci/pci_bridge.h | 1 + > include/hw/pci/pcie_aer.h | 2 +- > include/hw/pci/pcie_host.h | 1 + > include/hw/pci/shpc.h | 1 + > include/hw/qdev-core.h | 4 +++ > 48 files changed, 329 insertions(+), 91 deletions(-) > > -- > 1.8.3.1