[PATCH] fw/pci: do not automatically allocate IO region for PCIe bridges

[Marcel Apfelbaum]

PCIe downstream ports (Root Ports and switches Downstream Ports) appear
to firmware as PCI-PCI bridges and a 4K IO space is allocated for them
even if there is no device behind them requesting IO space,
all that for hotplug purpose.

However, PCIe devices can work without IO, so there is no need
to allocate IO space for hotplug.

Signed-off-by: Marcel Apfelbaum 
---
Notes:
 - This patch fixes a 15 PCIe Root ports limitation when used with
   virtio pci-express devices having no IO space requirements:
      <qemu cmd line> -M q35 `for i in {1..15}; do echo -device ioh3420,chassis=$i,id=b$i -device virtio-net-pci,bus=b$i,disable-legacy=on; done`
 - There is a patch on QEMU devel list that tackles the problem from another angle
   by allowing PCIe downstream ports to not forward IO requests.
       https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg04478.html
 - The mentioned patch is of course not enough and also requires management software intervention.

Thanks,
Marcel

src/fw/pciinit.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index 7b8aab7..4b37792 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -736,7 +736,9 @@ static int pci_bios_check_devices(struct pci_bus *busses)
             if (pci_region_align(&s->r[type]) > align)
                  align = pci_region_align(&s->r[type]);
             u64 sum = pci_region_sum(&s->r[type]);
-            if (!sum && hotplug_support)
+            int res_opt = (type == PCI_REGION_TYPE_IO) &&
+                          pci_find_capability(s->bus_dev, PCI_CAP_ID_EXP, 0);
+            if (!sum && hotplug_support && !res_opt)
                 sum = align; /* reserve min size for hot-plug */
             u64 size = ALIGN(sum, align);
             int is64 = pci_bios_bridge_region_is64(&s->r[type],

[PATCH] fw/pci: do not automatically allocate IO region for PCIe bridges

[Gerd Hoffmann]

Hi,
> However, PCIe devices can work without IO, so there is no need
> to allocate IO space for hotplug.

Makes sense.
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index 7b8aab7..4b37792 100644
> --- a/src/fw/pciinit.c
> +++ b/src/fw/pciinit.c
> @@ -736,7 +736,9 @@ static int pci_bios_check_devices(struct pci_bus *busses)
>              if (pci_region_align(&s->r[type]) > align)
>                   align = pci_region_align(&s->r[type]);
>              u64 sum = pci_region_sum(&s->r[type]);
>        if (!sum && hotplug_support)
>        int res_opt = (type == PCI_REGION_TYPE_IO) &&
>                      pci_find_capability(s->bus_dev, PCI_CAP_ID_EXP, 0);

I'd make the variable names longer and more descriptive.  Also move the
pcie check out of the loop.  Note that pci_bus_hotplug_support() looks
for the pcie capability too, so we probably should turn that into
something like pci_bridge_get_props(), so we have to look at the bridge
capabilities only once.

cheers,
  Gerd

[PATCH] fw/pci: do not automatically allocate IO region for PCIe bridges

[Marcel Apfelbaum]

On 12/07/2015 11:46 AM, Gerd Hoffmann wrote:
> Hi,
> > However, PCIe devices can work without IO, so there is no need
> > to allocate IO space for hotplug.
>
> Makes sense.
> > diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> > index 7b8aab7..4b37792 100644
> > --- a/src/fw/pciinit.c
> > +++ b/src/fw/pciinit.c
> > @@ -736,7 +736,9 @@ static int pci_bios_check_devices(struct pci_bus *busses)
> >               if (pci_region_align(&s->r[type]) > align)
> >                    align = pci_region_align(&s->r[type]);
> >               u64 sum = pci_region_sum(&s->r[type]);
> >        if (!sum && hotplug_support)
> >        int res_opt = (type == PCI_REGION_TYPE_IO) &&
> >                      pci_find_capability(s->bus_dev, PCI_CAP_ID_EXP, 0);
>
> I'd make the variable names longer and more descriptive.  Also move the
> pcie check out of the loop.  Note that pci_bus_hotplug_support() looks
> for the pcie capability too, so we probably should turn that into
> something like pci_bridge_get_props(), so we have to look at the bridge
> capabilities only once.

Hi Gerd,

Thanks for the review. I'll address it and post again.

Thanks,
Marcel
> cheers,
>    Gerd