Re: [Qemu-devel] [PATCH 0/2] pci: exit path cleanup and fix

[Paolo Bonzini]

Il 04/07/2012 06:39, Alex Williamson ha scritto:
> 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(-)
> 
> 

The diffstat caught my eye. :)

Reviewed-by: Paolo Bonzini <address@hidden>