PCI: hotplug: Drop checking of PCI_BRIDGE_CONTROL in *_unconfigure_device()
authorMika Westerberg <mika.westerberg@linux.intel.com>
Thu, 9 Nov 2017 11:15:08 +0000 (14:15 +0300)
committerBjorn Helgaas <bhelgaas@google.com>
Tue, 19 Dec 2017 05:05:18 +0000 (23:05 -0600)
When removing a bridge, pciehp_unconfigure_device() reads the
PCI_BRIDGE_CONTROL byte.  If this is a surprise hot-unplug, the device is
already gone and the read returns ~0, which pciehp_unconfigure_device()
interprets as having PCI_BRIDGE_CTL_VGA set.  This results in failure of
the remove operation:

  pciehp 0000:00:1c.0:pcie004: Slot(0): Link Down
  pciehp 0000:00:1c.0:pcie004: Slot(0): Card present
  pciehp 0000:00:1c.0:pcie004: Cannot remove display device 0000:01:00.0

Because of this the hierarchy is left untouched preventing further hotplug
operations.

Now, it is not clear why the check is there in the first place and why we
would like to prevent removing a bridge if it has PCI_BRIDGE_CTL_VGA set.
In case of PCIe surprise hot-unplug, it would not even be possible to
prevent the removal.

Given this and the issue described above, I think it makes sense to drop
the whole PCI_BRIDGE_CONTROL check from pciehp_unconfigure_device().  While
there do the same for shpchp_configure_device() based on the same reasoning
and the fact that the same bug might trigger in standard PCI hotplug as
well.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
[bhelgaas: changelog]
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
drivers/pci/hotplug/pciehp_pci.c
drivers/pci/hotplug/shpchp_pci.c

index 2a1ca02..acc360d 100644 (file)
@@ -79,7 +79,6 @@ int pciehp_configure_device(struct slot *p_slot)
 int pciehp_unconfigure_device(struct slot *p_slot)
 {
        int rc = 0;
-       u8 bctl = 0;
        u8 presence = 0;
        struct pci_dev *dev, *temp;
        struct pci_bus *parent = p_slot->ctrl->pcie->port->subordinate;
@@ -101,17 +100,6 @@ int pciehp_unconfigure_device(struct slot *p_slot)
        list_for_each_entry_safe_reverse(dev, temp, &parent->devices,
                                         bus_list) {
                pci_dev_get(dev);
-               if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE && presence) {
-                       pci_read_config_byte(dev, PCI_BRIDGE_CONTROL, &bctl);
-                       if (bctl & PCI_BRIDGE_CTL_VGA) {
-                               ctrl_err(ctrl,
-                                        "Cannot remove display device %s\n",
-                                        pci_name(dev));
-                               pci_dev_put(dev);
-                               rc = -EINVAL;
-                               break;
-                       }
-               }
                if (!presence) {
                        pci_dev_set_disconnected(dev, NULL);
                        if (pci_has_subordinate(dev))
index ea63db5..79f1682 100644 (file)
@@ -78,7 +78,6 @@ int shpchp_configure_device(struct slot *p_slot)
 int shpchp_unconfigure_device(struct slot *p_slot)
 {
        int rc = 0;
-       u8 bctl = 0;
        struct pci_bus *parent = p_slot->ctrl->pci_dev->subordinate;
        struct pci_dev *dev, *temp;
        struct controller *ctrl = p_slot->ctrl;
@@ -93,17 +92,6 @@ int shpchp_unconfigure_device(struct slot *p_slot)
                        continue;
 
                pci_dev_get(dev);
-               if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
-                       pci_read_config_byte(dev, PCI_BRIDGE_CONTROL, &bctl);
-                       if (bctl & PCI_BRIDGE_CTL_VGA) {
-                               ctrl_err(ctrl,
-                                        "Cannot remove display device %s\n",
-                                        pci_name(dev));
-                               pci_dev_put(dev);
-                               rc = -EINVAL;
-                               break;
-                       }
-               }
                pci_stop_and_remove_bus_device(dev);
                pci_dev_put(dev);
        }