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

[Zihan Yang]

Marcel Apfelbaum <address@hidden> 于2018年8月18日周六 上午1:54写道：
>
>
>
> 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)

I'm not sure about it either, for the same reason in previous patch. I thought
we should start from bus 0 always. I think I'll bring start_bus back
in next version.

> 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);
> >
>