[Qemu-devel] Re: [PATCH v4 00/23] pci: various pci clean up and pci express support.

[Michael S. Tsirkin]

On Mon, Oct 05, 2009 at 07:06:40PM +0900, Isaku Yamahata wrote:
> Thanks to Michael's review, patches have been much improved
> and became much readable.
> I believe I covered almost all the points from him except two.
> The rest issues are PRI macros and writing to PCI config space.
> 
> - PRI macros
>   I think that PRI is already widely used in qemu, so I decided to use it.
>   However Michael dislike those macros wanting to convert uintN_t to 
>   unsigned int/long/long long.

Not all over.  Only where we do not care about width, such as in address
for pci config, which is what this patch was touching.

>   Since my main motivation is pci express, this isn't essential for me.
>   So I don't insist on uintN_t as long as the patches are accepted.
>   However I also prefer consistent coding style, so I'd like to hear
>   other's opinion.
> 
> - writing to PCI config space
>   I eventually introduced some helper functions and a structure
>   which can be used by each pci device.
>   Maybe there would be a discussion on them.
> 
> 
> This patch set is preliminary for q35 based chipset support.
> This patch sets does various pci related clean ups and 
> 64bit bar, pciemmconfig and pci bridge support.
> 
> I'm aware that some of patches are already conflicting with
> other's patch. It is appriciated to merge some first part of
> patches which doesn't conflict.
> 
> thanks,
> 
> changes from V3:
> - indentation
> - reworked helper function to update pci configuration space
> - dropped pci_conf_init[blw]()
> - pci: use PCI_SLOT() and PCI_FUNC(). two more use.
>   I kept the acked-by hoping Michael won't be against it.
> - pci 64bit bar
>   - comment
>   - s/PCI_BASE_ADDRESS_SPACE_MEM_64/PCI_BASE_ADDRESS_MEM_TYPE_64/g
>   - s/pci_bar_is_64big/pci_bar_is_mem64/g
> - pci bridge
>   - refined pci_bar_config_offset(). 
> - bridge/qdevfy
>   - pass -1 for vmstate_register()
>   - use pci_create_simple()
>   - some clean up
> - pci/monitor: print out bridge's filtering values and so on. 
>   - fixed io range calculation
>   - unified case
>   - indentation
> 
> changes from V2:
> - bug fix pointed out by Gerd Hoffmann
>   pci_find_devie()
>   pci: make pci configuration transaction more accurate.
> 
> changes from V1:
> - introduced pcibus_t
> - dropped pci_config_[sg]et_xxx()
> - many code clean up
> - reworked pci_host.h
> - reworked pci bus search to introduce tree representation for pci bus
> - dropped some non essential patches
> 
> Isaku Yamahata (23):
>   pci: fix PCI_DPRINTF() wrt variadic macro.
>   pci: use appropriate PRIs in PCI_DPRINTF() for portability.
>   pci: introduce constant PCI_NUM_PINS for the number of interrupt
>     pins, 4.
>   pci: use the symbolic constant, PCI_ROM_ADDRESS_ENABLE instead of 1.
>   pci: use PCI_SLOT() and PCI_FUNC().
>   pci: define a constant to represent a unmapped bar and use it.
>   pci: helper functions to access PCIDevice::config
>   pci: use helper functions to access pci config space.
>   pci: introduce pcibus_t to represent pci bus address/size instead of
>     uint32_t
>   pci: introduce FMT_PCIBUS for printf format for pcibus_t.
>   pci: typedef pcibus_t as uint64_t instead of uint32_t.
>   pci: 64bit bar support.
>   pci: make pci configuration transaction more accurate.
>   pci: factor out the logic to get pci device from address.
>   pci_host.h: split non-inline static function in pci_host.h into
>     pci_host.c
>   pci: pcie host and mmcfg support.
>   pci: fix pci_default_write_config()
>   pci: add helper functions for pci config write function.
>   pci: use helper function in pci_default_write_config()
>   pci: make bar update function aware of pci bridge.
>   pci/brdige: qdevfy and initialize secondary bus and subordinate bus.
>   pci: initialize wmask according to pci header type.
>   pci/monitor: print out bridge's filtering values and so on.
> 
>  Makefile.target               |    8 +-
>  hw/ac97.c                     |    2 +-
>  hw/apb_pci.c                  |   31 +-
>  hw/cirrus_vga.c               |    8 +-
>  hw/e1000.c                    |   12 +-
>  hw/eepro100.c                 |   10 +-
>  hw/es1370.c                   |    2 +-
>  hw/grackle_pci.c              |   24 +-
>  hw/gt64xxx.c                  |   11 +-
>  hw/hw.h                       |   12 +
>  hw/ide/pci.c                  |    4 +-
>  hw/lsi53c895a.c               |    6 +-
>  hw/macio.c                    |    2 +-
>  hw/msix.c                     |    2 +-
>  hw/msix.h                     |    3 +-
>  hw/ne2000.c                   |    2 +-
>  hw/openpic.c                  |    2 +-
>  hw/pci-hotplug.c              |    6 +-
>  hw/pci.c                      |  845 
> ++++++++++++++++++++++++++++++++---------
>  hw/pci.h                      |  187 +++++++++-
>  hw/{pci_host.h => pci_host.c} |   61 +++-
>  hw/pci_host.h                 |  110 ++----
>  hw/pcnet.c                    |    9 +-
>  hw/piix_pci.c                 |   16 +-
>  hw/ppc4xx_pci.c               |   18 +-
>  hw/ppce500_pci.c              |   17 +-
>  hw/prep_pci.c                 |   14 +-
>  hw/rtl8139.c                  |    4 +-
>  hw/sun4u.c                    |    2 +-
>  hw/unin_pci.c                 |   47 +--
>  hw/usb-ohci.c                 |    2 +-
>  hw/usb-uhci.c                 |    2 +-
>  hw/vga-pci.c                  |    2 +-
>  hw/virtio-pci.c               |    2 +-
>  hw/vmware_vga.c               |    4 +-
>  hw/wdt_i6300esb.c             |    5 +-
>  36 files changed, 1031 insertions(+), 463 deletions(-)
>  copy hw/{pci_host.h => pci_host.c} (69%)