linux-kernel - Re: [PATCH v3] PCI: Only enable IO window if supported

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAE9FiQVGhfbFW5-5deYcU2bDWE9k0-4+V+cepwdqZ661sLWOFA@mail.gmail.com>
Date:	Wed, 5 Aug 2015 18:38:31 -0700
From:	Yinghai Lu <yinghai@...nel.org>
To:	Guenter Roeck <linux@...ck-us.net>
Cc:	Bjorn Helgaas <bhelgaas@...gle.com>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Lorenzo Pieralisi <lorenzo.pieralisi@....com>
Subject: Re: [PATCH v3] PCI: Only enable IO window if supported

On Wed, Aug 5, 2015 at 6:14 PM, Yinghai Lu <yinghai@...nel.org> wrote:
>
> It only can avoid warning with bridge, and still have warning on
> devices under the bridge.
>
> also would have problem on transparent bridges, like
>
> BRIDGE_A ---- BRIDGE_AA----DEVICE_AA
>                    |
>                    \-- BRIDGE_AB ---DEVICE_AB
>
> if only BRIDGE_A and BRIDGE_AA are transparent, and BRIDGE_AB is not.
>
> and if pci_bridge_supports_io() return false for BRIDGE_A.
>
> We will have all three bridges have PCI_BUS_FLAGS_SUPPORTS_IO cleared.
> so all will not been sized and will not get assigned io port resource.
>
> later DEVICE_AA could root bus io port as parent, and get io resource assigned.
> but DEVICE_AB will not get assigned.
>
> so we should get rid of pci_bridge_supports_io(), and just check root
> resource IO port.

I would suggest this version instead:

Subject: [PATCH] PCI: Only try to assign io port only if root bus support that

The PCI subsystem always assumes that I/O is supported on root bus and
tries to assign an I/O window to each child bus even if that is not the
case.

This may result in messages such as:

  pcieport 0000:02:00.0: res[7]=[io  0x1000-0x0fff] get_res_add_size
add_size 1000
  pcieport 0000:02:00.0: BAR 7: no space for [io  size 0x1000]
  pcieport 0000:02:00.0: BAR 7: failed to assign [io  size 0x1000]

for each bridge port, even if root does not support I/O in the first place.

To avoid this message, check if root bus supports I/O, then child bus
will inherit the setting from parent bus, and later during sizing and
assigning, check that bus flags and skip those resources.

[bhelgaas: reverse sense of new pci_bus_flags_t value]
[yinghai: only check root bus io port, and use flag on sizing and assigning]
Cc: Guenter Roeck <linux@...ck-us.net>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>
CC: Lorenzo Pieralisi <lorenzo.pieralisi@....com>

---
 drivers/pci/probe.c     |    5 +++++
 drivers/pci/setup-bus.c |    6 +++++-
 include/linux/pci.h     |    1 +
 3 files changed, 11 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ linux-2.6/drivers/pci/probe.c
@@ -340,6 +340,9 @@ static void pci_read_bridge_io(struct pc
     struct pci_bus_region region;
     struct resource *res;

+    if (!(child->bus_flags & PCI_BUS_FLAGS_ROOT_SUPPORTS_IO))
+        return;
+
     io_mask = PCI_IO_RANGE_MASK;
     io_granularity = 0x1000;
     if (dev->io_window_1k) {
@@ -2070,6 +2073,8 @@ struct pci_bus *pci_create_root_bus(stru
         } else
             bus_addr[0] = '\0';
         dev_info(&b->dev, "root bus resource %pR%s\n", res, bus_addr);
+        if (resource_type(res) == IORESOURCE_IO)
+            b->bus_flags |= PCI_BUS_FLAGS_ROOT_SUPPORTS_IO;
     }

     down_write(&pci_bus_sem);
Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -156,6 +156,10 @@ static void pdev_sort_resources(struct p
         if (r->flags & IORESOURCE_PCI_FIXED)
             continue;

+        if ((r->flags & IORESOURCE_IO) &&
+            !(dev->bus->bus_flags & PCI_BUS_FLAGS_ROOT_SUPPORTS_IO))
+            continue;
+
         if (!(r->flags) || r->parent)
             continue;

@@ -909,7 +913,7 @@ static void pbus_size_io(struct pci_bus
     resource_size_t children_add_size = 0;
     resource_size_t min_align, align;

-    if (!b_res)
+    if (!b_res || !(bus->bus_flags & PCI_BUS_FLAGS_ROOT_SUPPORTS_IO))
         return;

     min_align = window_alignment(bus, IORESOURCE_IO);
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -193,6 +193,7 @@ typedef unsigned short __bitwise pci_bus
 enum pci_bus_flags {
     PCI_BUS_FLAGS_NO_MSI   = (__force pci_bus_flags_t) 1,
     PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2,
+    PCI_BUS_FLAGS_ROOT_SUPPORTS_IO = (__force pci_bus_flags_t) 4,
 };

 /* These values come from the PCI Express Spec */

View attachment "pci_root_bus_no_ioport.patch" of type "text/x-patch" (3188 bytes)