Re: [Qemu-devel] [RFC v4 6/6] i386/acpi-build: pxb-pcie start bus should always be 0 in separate pci domain

[Marcel Apfelbaum]

On 08/09/2018 09:35 AM, Zihan Yang wrote:
> The bus_nr indicates the bus number of pxb-pcie under pcie.0, but since pxb
> host can be put into different pci domain, the start bus should always be 0

I am  not sure about this limitation. Do you know where is coming from?
Why do we must start a different pci_domain from 0? (Maybe we do, I just 
want to be sure)

Thanks,
Marcel

> Signed-off-by: Zihan Yang <address@hidden>
> ---
>   hw/i386/acpi-build.c                | 14 ++------------
>   hw/pci-bridge/pci_expander_bridge.c | 10 +++++-----
>   2 files changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index a7d9af2..0911008 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -91,7 +91,6 @@ typedef struct AcpiMcfgInfo {
>       uint64_t mcfg_base;
>       uint32_t mcfg_size;
>       uint32_t domain_nr;
> -    uint8_t bus_nr; // start bus number
>       struct AcpiMcfgInfo *next;
>   } AcpiMcfgInfo;
>   
> @@ -2470,8 +2469,8 @@ build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
>       while (info) {
>           mcfg[0].allocation[count].address = cpu_to_le64(info->mcfg_base);
>           mcfg[0].allocation[count].pci_segment = cpu_to_le16(info->domain_nr);
> -        mcfg[0].allocation[count].start_bus_number = info->bus_nr;
> -        mcfg[0].allocation[count++].end_bus_number = info->bus_nr + \
> +        mcfg[0].allocation[count].start_bus_number = 0;
> +        mcfg[0].allocation[count++].end_bus_number = \
>                                       PCIE_MMCFG_BUS(info->mcfg_size - 1);
>           info = info->next;
>       }
> @@ -2694,15 +2693,6 @@ static AcpiMcfgInfo *acpi_get_mcfg(void)
>           mcfg->mcfg_size = qnum_get_uint(qobject_to(QNum, o));
>           qobject_unref(o);
>   
> -        o = object_property_get_qobject(obj, PROP_PXB_BUS_NR, NULL);
> -        if (!o) {
> -            /* we are in q35 host again */
> -            mcfg->bus_nr = 0;
> -        } else {
> -            mcfg->bus_nr = qnum_get_uint(qobject_to(QNum, o));
> -            qobject_unref(o);
> -        }
> -
>           pci_host = OBJECT(QTAILQ_NEXT(PCI_HOST_BRIDGE(pci_host), next));
>       }
>   
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 686b026..53f5776 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -91,7 +91,7 @@ static int pxb_bus_num(PCIBus *bus)
>   {
>       PXBDev *pxb = convert_to_pxb(bus->parent_dev);
>   
> -    return pxb->bus_nr;
> +    return pxb->domain_nr ? 0 : pxb->bus_nr;
>   }
>   
>   static int pxb_domain_num(PCIBus *bus)
> @@ -145,8 +145,8 @@ static void pxb_pcie_foreach(gpointer data, gpointer 
> user_data)
>   
>       if (pxb->domain_nr > 0) {
>           /* only reserve what users ask for to reduce memory cost. Plus one
> -         * as the interval [bus_nr, max_bus] has (max_bus-bus_nr+1) buses */
> -        pxb_mcfg_hole_size += ((pxb->max_bus - pxb->bus_nr + 1ULL) * MiB);
> +         * as the interval [0, max_bus] has (max_bus+1) buses */
> +        pxb_mcfg_hole_size += ((pxb->max_bus + 1ULL) * MiB);
>       }
>   }
>   
> @@ -385,9 +385,9 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>           /* will be overwritten by firmware, but kept for readability */
>           qdev_prop_set_uint64(ds, PCIE_HOST_MCFG_BASE,
>               pxb->domain_nr ? pxb_pcie_mcfg_base : 
> MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT);
> -        /* +1 because [bus_nr, max_bus] has (max_bus-bus_nr+1) buses */
> +        /* +1 because [0, max_bus] has (max_bus+1) buses */
>           qdev_prop_set_uint64(ds, PCIE_HOST_MCFG_SIZE,
> -            pxb->domain_nr ? (pxb->max_bus - pxb->bus_nr + 1ULL) * MiB : 0);
> +            pxb->domain_nr ? (pxb->max_bus + 1ULL) * MiB : 0);
>           if (pxb->domain_nr)
>               pxb_pcie_mcfg_base += ((pxb->max_bus + 1ULL) * MiB);
>