From patchwork Thu Nov 20 22:33:22 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rajat Jain X-Patchwork-Id: 5351221 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 2D622C11AC for ; Thu, 20 Nov 2014 22:33:13 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 2B6A4201B4 for ; Thu, 20 Nov 2014 22:33:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 08D6E2016C for ; Thu, 20 Nov 2014 22:33:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756397AbaKTWdI (ORCPT ); Thu, 20 Nov 2014 17:33:08 -0500 Received: from mail-pa0-f52.google.com ([209.85.220.52]:58776 "EHLO mail-pa0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756099AbaKTWdH (ORCPT ); Thu, 20 Nov 2014 17:33:07 -0500 Received: by mail-pa0-f52.google.com with SMTP id eu11so3447142pac.11 for ; Thu, 20 Nov 2014 14:33:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:cc:subject :content-type:content-transfer-encoding; bh=cVoL/PCVBiqYazVc5R0CVurE7D0xVpP8oXpakUHgtXs=; b=asyv0uzBDMx2bKBFOzG51+uxsbC4togG2DeiLA5Hnq+x5AGYo59gAzra+Pzoo/0n9K rdvXv6gmI9bvhPZN6XoirteZgjpIlCV8p82VDfyp4WWuAGzHi/EpAWxWZ6xIRehOiEx+ BgPBw+03LtiwVCASuqhbh7CGaulVnVhDgM+vwee8xAs7w/m8XxcrDtHNmJ5Vuqsptnmm UMG7a7vE22KwLceTTw2lCyT/il0Bq4OfEC4e3XF4xgmD8heWxVO4zzW5Q/1qIASk0UC2 FykRJKGZoAgW/s+7YG/Tdf7ZYP+ICM9EgwJSHXBNx/EXLbDdyTnLWVECi/P2vQYhLlq7 UW4w== X-Received: by 10.66.234.100 with SMTP id ud4mr1420725pac.36.1416522787206; Thu, 20 Nov 2014 14:33:07 -0800 (PST) Received: from [192.168.95.133] ([66.129.239.13]) by mx.google.com with ESMTPSA id sk3sm2976497pac.13.2014.11.20.14.33.06 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 20 Nov 2014 14:33:06 -0800 (PST) Message-ID: <546E6C32.6090909@gmail.com> Date: Thu, 20 Nov 2014 14:33:22 -0800 From: Rajat Jain User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130803 Thunderbird/17.0.8 MIME-Version: 1.0 To: Bjorn Helgaas , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org CC: Rajat Jain , Guenter Roeck , Rajat Jain Subject: [PATCH] PCI: pciehp: Check link state before accessing device during removal Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 Signed-off-by: Rajat Jain Signed-off-by: Guenter Roeck --- 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;