All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC v4 6/6] i386/acpi-build: pxb-pcie start bus should always be 0 in separate pci domain
@ 2018-08-09  6:35 Zihan Yang
  2018-08-17 17:54 ` Marcel Apfelbaum
  0 siblings, 1 reply; 3+ messages in thread
From: Zihan Yang @ 2018-08-09  6:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Zihan Yang, Michael S. Tsirkin, Igor Mammedov, Marcel Apfelbaum,
	Paolo Bonzini, Richard Henderson, Eduardo Habkost

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

Signed-off-by: Zihan Yang <whois.zihan.yang@gmail.com>
---
 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);
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [RFC v4 6/6] i386/acpi-build: pxb-pcie start bus should always be 0 in separate pci domain
  2018-08-09  6:35 [Qemu-devel] [RFC v4 6/6] i386/acpi-build: pxb-pcie start bus should always be 0 in separate pci domain Zihan Yang
@ 2018-08-17 17:54 ` Marcel Apfelbaum
  2018-08-19  2:02   ` Zihan Yang
  0 siblings, 1 reply; 3+ messages in thread
From: Marcel Apfelbaum @ 2018-08-17 17:54 UTC (permalink / raw)
  To: Zihan Yang, qemu-devel
  Cc: Michael S. Tsirkin, Igor Mammedov, Paolo Bonzini,
	Richard Henderson, Eduardo Habkost



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 <whois.zihan.yang@gmail.com>
> ---
>   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);
>   

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [RFC v4 6/6] i386/acpi-build: pxb-pcie start bus should always be 0 in separate pci domain
  2018-08-17 17:54 ` Marcel Apfelbaum
@ 2018-08-19  2:02   ` Zihan Yang
  0 siblings, 0 replies; 3+ messages in thread
From: Zihan Yang @ 2018-08-19  2:02 UTC (permalink / raw)
  To: Marcel Apfelbaum; +Cc: qemu-devel, Michael S. Tsirkin

Marcel Apfelbaum <marcel.apfelbaum@gmail.com> 于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 <whois.zihan.yang@gmail.com>
> > ---
> >   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);
> >
>

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-08-19  2:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-09  6:35 [Qemu-devel] [RFC v4 6/6] i386/acpi-build: pxb-pcie start bus should always be 0 in separate pci domain Zihan Yang
2018-08-17 17:54 ` Marcel Apfelbaum
2018-08-19  2:02   ` Zihan Yang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.