Re: [Qemu-devel] [PATCH v5 for-2.3 19/28] hw/pci: implement iteration over multiple host bridges

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

On Tue, Mar 10, 2015 at 05:32:05PM +0200, Marcel Apfelbaum wrote:
> From: Marcel Apfelbaum <marcel.a@redhat.com>
> 
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>

This isn't a good way to split patches.
You add an API with no documentation,
it's use is also in a separate file.

This patch really can't reasonably be reviewed
in isolation.

In this case, I think a better way is to
scan from root complex down, and find PXBs.
This will scale nicely to multiple root complexes.
You can do this using PCI or QOM methods.



> ---
>  hw/pci/pci.c              | 8 ++++----
>  include/hw/pci/pci_host.h | 4 ++++
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 53598bd..f0cf752 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -69,7 +69,7 @@ static void pci_del_option_rom(PCIDevice *pdev);
>  static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
>  static uint16_t pci_default_sub_device_id = PCI_SUBDEVICE_ID_QEMU;
>  
> -static QLIST_HEAD(, PCIHostState) pci_host_bridges;
> +struct PCIHostQ pci_host_bridges = QLIST_HEAD_INITIALIZER(pci_host_bridges);
>  
>  static int pci_bar(PCIDevice *d, int reg)
>  {
> @@ -190,7 +190,7 @@ PCIBus *pci_find_primary_bus(void)
>      PCIBus *primary_bus = NULL;
>      PCIHostState *host;
>  
> -    QLIST_FOREACH(host, &pci_host_bridges, next) {
> +    HOST_BRIDGE_FOREACH(host) {
>          if (primary_bus) {
>              /* We have multiple root buses, refuse to select a primary */
>              return NULL;
> @@ -1264,7 +1264,7 @@ PciInfoList *qmp_query_pci(Error **errp)
>      PciInfoList *info, *head = NULL, *cur_item = NULL;
>      PCIHostState *host_bridge;
>  
> -    QLIST_FOREACH(host_bridge, &pci_host_bridges, next) {
> +    HOST_BRIDGE_FOREACH(host_bridge) {
>          info = g_malloc0(sizeof(*info));
>          info->value = qmp_query_pci_bus(host_bridge->bus,
>                                          pci_bus_num(host_bridge->bus));
> @@ -1802,7 +1802,7 @@ int pci_qdev_find_device(const char *id, PCIDevice **pdev)
>      PCIHostState *host_bridge;
>      int rc = -ENODEV;
>  
> -    QLIST_FOREACH(host_bridge, &pci_host_bridges, next) {
> +    HOST_BRIDGE_FOREACH(host_bridge) {
>          int tmp = pci_qdev_find_recursive(host_bridge->bus, id, pdev);
>          if (!tmp) {
>              rc = 0;
> diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
> index 3c72e26..ba5272f 100644
> --- a/include/hw/pci/pci_host.h
> +++ b/include/hw/pci/pci_host.h
> @@ -63,6 +63,10 @@ typedef struct PCIHostBridgeClass {
>      const char *(*root_bus_path)(PCIHostState *, PCIBus *);
>  } PCIHostBridgeClass;
>  
> +QLIST_HEAD(PCIHostQ, PCIHostState);
> +extern struct PCIHostQ pci_host_bridges;
> +#define HOST_BRIDGE_FOREACH(host) QLIST_FOREACH(host, &pci_host_bridges, next)
> +
>  /* common internal helpers for PCI/PCIe hosts, cut off overflows */
>  void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr,
>                                    uint32_t limit, uint32_t val, uint32_t len);
> -- 
> 2.1.0