Re: [Qemu-devel] [PATCH v2 00/38] Delay destruction of memory regions to instance_finalize

["Michael S. Tsirkin" <mst@redhat.com>]

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