[Qemu-devel] Re: [PATCH 00/10] pci: pci to pci bridge clean up and enhancement

[Michael S. Tsirkin]

On Thu, Jun 17, 2010 at 03:15:42PM +0900, Isaku Yamahata wrote:
> This patch series cleans up pci to pci bridge layer by introducing
> pci bridge layer. and some bug fixes.
> Although pci bridge implementation would belong to pci.c,
> I split it out into pci_bridge.c because pci.c is already big enough.
> 
> This might seem over engineering, but it's also a preparation for
> pci express root/upstream/downstream port emulators.
> Those express ports are similar, but different from each other.
> So new pci bridge layer helps here.
> Once this patch series is merged, the express ports patch will follow.

A separate patchset with just bugfixes and cleanups, would be easier to
merge.


For example, forcing all devices to call pci_reset_default
in their reset routines does not look like a good cleanup:
the less boilerplate, the better IMO. New APIs seem undocumented
and it is not obvious what they do. For example, what does
qdev_reset do?  Adding more callbacks
also does not make me very happy, they are hard to follow
and to debug. Maybe it would be better to look at the bridge layer when
we see how it helps pci express. But it would be even better IMO to avoid
layers, and just export some common functions that can be reused,
without forcing all devices to go through them.

> Isaku Yamahata (10):
>   pci_bridge: split out pci bridge code into pci_bridge.c from pci.c
>   qdev: export qdev_reset() for later use.
>   pci: fix pci_bus_reset() with 64bit BAR and several clean ups.
>   pci_bridge: introduce pci bridge layer.
>   pci bridge: add helper function for ssvid capability.
>   pci: eliminate work around in pci_device_reset().
>   pci: fix pci domain registering.
>   pci: remove PCIDeviceInfo::header_type
>   pci: set PCI multi-function bit appropriately.
>   pci: don't overwrite multi functio bit in pci header type.
> 
>  Makefile.objs     |    2 +-
>  hw/ac97.c         |    1 -
>  hw/acpi_piix4.c   |    1 -
>  hw/apb_pci.c      |   43 ++++++++-----
>  hw/dec_pci.c      |   31 ++++++---
>  hw/e1000.c        |    1 +
>  hw/grackle_pci.c  |    1 -
>  hw/ide/cmd646.c   |    1 -
>  hw/ide/piix.c     |    1 -
>  hw/lsi53c895a.c   |    2 +
>  hw/macio.c        |    1 -
>  hw/ne2000.c       |    1 -
>  hw/openpic.c      |    1 -
>  hw/pci.c          |  194 
> +++++++++++++++++++++++------------------------------
>  hw/pci.h          |   22 +++++-
>  hw/pci_bridge.c   |  188 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/pci_bridge.h   |   71 +++++++++++++++++++
>  hw/pcnet.c        |    2 +-
>  hw/piix4.c        |    3 +-
>  hw/piix_pci.c     |    5 +-
>  hw/prep_pci.c     |    1 -
>  hw/qdev.c         |   13 +++-
>  hw/qdev.h         |    1 +
>  hw/rtl8139.c      |    3 +-
>  hw/sun4u.c        |    1 -
>  hw/unin_pci.c     |    4 -
>  hw/usb-uhci.c     |    1 -
>  hw/vga-pci.c      |    1 -
>  hw/virtio-pci.c   |    2 +-
>  hw/vmware_vga.c   |    1 -
>  hw/wdt_i6300esb.c |    1 -
>  qemu-common.h     |    1 +
>  32 files changed, 430 insertions(+), 172 deletions(-)
>  create mode 100644 hw/pci_bridge.c
>  create mode 100644 hw/pci_bridge.h