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

[Michael S. Tsirkin]

On Tue, Mar 10, 2015 at 05:32:05PM +0200, Marcel Apfelbaum wrote:
> From: Marcel Apfelbaum
>
> Signed-off-by: Marcel Apfelbaum

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