From: Rajat Jain <rajatxjain@gmail.com>
To: Bjorn Helgaas <bhelgaas@google.com>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Rajat Jain <rajatjain@juniper.net>,
Guenter Roeck <groeck@juniper.net>,
Rajat Jain <rajatxjain@gmail.com>
Subject: [PATCH] PCI: pciehp: Check link state before accessing device during removal
Date: Thu, 20 Nov 2014 14:33:22 -0800 [thread overview]
Message-ID: <546E6C32.6090909@gmail.com> (raw)
While removing a card, we can't assume the presence to mean that the
access to card is OK. That is because the cause of removal may be a
link down event, and the card may still be physically present. Thus,
instead of presence, use the link state to decide whether or not it is
OK to access the card devices.
Here are the problem symptoms:
During the removal of a card due to link down, sometimes the following
error is seen (because pciehp_unconfigure_device() reads 0xFF from
bridge control register as the link is down, which cause it to assume
that the VGA bit is set):
pciehp 0000:21:05.0:pcie24: pcie_isr: intr_loc 100
pciehp 0000:21:05.0:pcie24: Data Link Layer State change
pciehp 0000:21:05.0:pcie24: slot(5): Link Down event
pciehp 0000:21:05.0:pcie24: Disabling domain:bus:device=0000:60:00
pciehp 0000:21:05.0:pcie24: pciehp_unconfigure_device: domain:bus:dev = 0000:60:00
pciehp 0000:21:05.0:pcie24: Cannot remove display device 0000:60:00.0
Ofcourse, when the link comes back up, the device addition fails too:
pciehp 0000:21:05.0:pcie24: pcie_isr: intr_loc 100
pciehp 0000:21:05.0:pcie24: Data Link Layer State change
pciehp 0000:21:05.0:pcie24: pciehp_check_link_active: lnk_status = 6011
pciehp 0000:21:05.0:pcie24: slot(5): Link Up event
pciehp 0000:21:05.0:pcie24: Enabling domain:bus:device=0000:60:00
pciehp 0000:21:05.0:pcie24: pciehp_check_link_active: lnk_status = 6011
pciehp 0000:21:05.0:pcie24: pciehp_check_link_status: lnk_status = 6011
pciehp 0000:21:05.0:pcie24: Device 0000:60:00.0 already exists at 0000:60:00, cannot hot-add
pciehp 0000:21:05.0:pcie24: Cannot add device at 0000:60:00
The problem is not seen with this patch applied. The device removal and
insertion works as expected.
Signed-off-by: Rajat Jain <rajatxjain@gmail.com>
Signed-off-by: Rajat Jain <rajatjain@juniper.net>
Signed-off-by: Guenter Roeck <groeck@juniper.net>
---
drivers/pci/hotplug/pciehp_pci.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
index 9e69403..8572af6 100644
--- a/drivers/pci/hotplug/pciehp_pci.c
+++ b/drivers/pci/hotplug/pciehp_pci.c
@@ -77,7 +77,7 @@ int pciehp_unconfigure_device(struct slot *p_slot)
{
int rc = 0;
u8 bctl = 0;
- u8 presence = 0;
+ bool link_active = false;
struct pci_dev *dev, *temp;
struct pci_bus *parent = p_slot->ctrl->pcie->port->subordinate;
u16 command;
@@ -85,7 +85,7 @@ int pciehp_unconfigure_device(struct slot *p_slot)
ctrl_dbg(ctrl, "%s: domain:bus:dev = %04x:%02x:00\n",
__func__, pci_domain_nr(parent), parent->number);
- pciehp_get_adapter_status(p_slot, &presence);
+ link_active = pciehp_check_link_active(p_slot->ctrl);
pci_lock_rescan_remove();
@@ -98,7 +98,7 @@ 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) {
+ if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE && link_active) {
pci_read_config_byte(dev, PCI_BRIDGE_CONTROL, &bctl);
if (bctl & PCI_BRIDGE_CTL_VGA) {
ctrl_err(ctrl,
@@ -114,7 +114,7 @@ int pciehp_unconfigure_device(struct slot *p_slot)
* Ensure that no new Requests will be generated from
* the device.
*/
- if (presence) {
+ if (link_active) {
pci_read_config_word(dev, PCI_COMMAND, &command);
command &= ~(PCI_COMMAND_MASTER | PCI_COMMAND_SERR);
command |= PCI_COMMAND_INTX_DISABLE;
--
1.7.9.5
reply other threads:[~2014-11-20 22:33 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=546E6C32.6090909@gmail.com \
--to=rajatxjain@gmail.com \
--cc=bhelgaas@google.com \
--cc=groeck@juniper.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=rajatjain@juniper.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).