Skip to content

Commit

Permalink
PCI: hotplug: Drop checking of PCI_BRIDGE_CONTROL in *_unconfigure_de…
Browse files Browse the repository at this point in the history
…vice()

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>
  • Loading branch information
Mika Westerberg authored and Bjorn Helgaas committed Dec 19, 2017
1 parent 1291a0d commit 0f4bd80
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 24 deletions.
12 changes: 0 additions & 12 deletions drivers/pci/hotplug/pciehp_pci.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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))
Expand Down
12 changes: 0 additions & 12 deletions drivers/pci/hotplug/shpchp_pci.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down

0 comments on commit 0f4bd80

Please sign in to comment.