From patchwork Sat Jan 13 06:38:39 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 860303 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-pci-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zJVLs2y42z9s7F for ; Sat, 13 Jan 2018 17:38:45 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751124AbeAMGin (ORCPT ); Sat, 13 Jan 2018 01:38:43 -0500 Received: from mailout3.hostsharing.net ([176.9.242.54]:39189 "EHLO mailout3.hostsharing.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750926AbeAMGin (ORCPT ); Sat, 13 Jan 2018 01:38:43 -0500 Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.hostsharing.net", Issuer "COMODO RSA Domain Validation Secure Server CA" (not verified)) by mailout3.hostsharing.net (Postfix) with ESMTPS id 758EB101E1C1C; Sat, 13 Jan 2018 07:38:43 +0100 (CET) Received: from localhost (6-38-90-81.adsl.cmo.de [81.90.38.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id 46E5D60C4DEC; Sat, 13 Jan 2018 07:38:40 +0100 (CET) X-Mailbox-Line: From d0ac15a21de9e47157588a96b7d2f407295e358b Mon Sep 17 00:00:00 2001 Message-Id: From: Lukas Wunner Date: Sat, 13 Jan 2018 07:38:39 +0100 Subject: [PATCH] PCI: pciehp: Assume NoCompl+ for Thunderbolt ports To: Bjorn Helgaas Cc: linux-pci@vger.kernel.org, Sinan Kaya , Mika Westerberg , Yehezkel Bernat , Michael Jamet , Andreas Noever Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org Certain Thunderbolt 1 controllers claim to support Command Completed events (value of 0b in the No Command Completed Support field of the Slot Capabilities register) but in reality neither set the Command Completed bit in the Slot Status register nor signal a Command Completed interrupt. As a result, every call to pcie_write_cmd() takes 2 seconds (1 second for each invocation of pcie_wait_cmd()). pcie_write_cmd() is called on ->remove via pcie_disable_notification(), so after such a Thunderbolt device is unplugged, 2 seconds elapse until pciehp is unbound: [ 337.942727] pciehp 0000:0a:00.0:pcie204: Timeout on hotplug command 0x1038 (issued 21176 msec ago) [ 340.014735] pciehp 0000:0a:00.0:pcie204: Timeout on hotplug command 0x0000 (issued 2072 msec ago) That by itself has always been unpleasant, but the situation has become worse with commit cc27b735ad3a ("PCI/portdrv: Turn off PCIe services during shutdown"): Now pciehp is unbound on ->shutdown. Because Thunderbolt controllers typically have 4 hotplug ports, every reboot and shutdown is now delayed by 8 seconds, plus another 2 seconds for every attached Thunderbolt 1 device. By testing with my own equipment and by googling I was able to ascertain that at least the following chips are affected by this erratum: 8086:1513 CV82524 Thunderbolt Controller [Light Ridge 4C 2010] 8086:1547 DSL3510 Thunderbolt Controller [Cactus Ridge 4C 2012] 8086:1549 DSL2210 Thunderbolt Controller [Port Ridge 1C 2011] The following newer chips are not affected as they have NoCompl+ in the Slot Capabilities register: 8086:156d DSL5520 Thunderbolt 2 Bridge [Falcon Ridge 4C 2013] 8086:1578 DSL6540 Thunderbolt 3 Bridge [Alpine Ridge 4C 2015] Thunderbolt hotplug slots are not physical slots that one inserts cards into, but rather logical hotplug slots implemented in silicon. Devices appear beyond those logical slots once a PCI tunnel is established on top of the Thunderbolt Converged I/O switch. One would expect that commands written to the Slot Control register are executed pretty much immediately by the silicon, without requiring a delay, so the NoCompl+ on newer chips makes sense. It is unclear whether the older chips likewise do not require observing a delay or how long that delay needs to be. There is still no public spec or errata sheet for Thunderbolt controllers. I checked what macOS does: https://opensource.apple.com/source/IOPCIFamily/IOPCIFamily-320.1.1/IOPCIBridge.cpp.auto.html They do not enable Command Completed events: // data link change, hot plug, presence detect change kSlotControlEnables = ((1 << 12) | (1 << 5) | (1 << 3)) They also do not call IODelay() or check the Command Completed bit after writing to the Slot Control register: fBridgeDevice->configWrite16( fBridgeDevice->reserved->expressCapability + 0x18, ...); Do the same and assume NoCompl+ for any Thunderbolt hotplug port. I've stress-tested this by alternatingly binding and unbinding pciehp to a Port Ridge hotplug bridge in an endless loop for a few minutes (by enabling/disabling slot power via sysfs), which causes rapid writes to the Slot Control register. pciehp never missed a beat. Fixes: cc27b735ad3a ("PCI/portdrv: Turn off PCIe services during shutdown") Cc: Sinan Kaya Cc: Mika Westerberg Cc: Yehezkel Bernat Cc: Michael Jamet Cc: Andreas Noever Signed-off-by: Lukas Wunner Reviewed-by: Mika Westerberg Tested-by: Mika Westerberg --- drivers/pci/hotplug/pciehp_hpc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 7bab0606f1a9..1904f46a09e4 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -848,6 +848,10 @@ struct controller *pcie_init(struct pcie_device *dev) if (pdev->hotplug_user_indicators) slot_cap &= ~(PCI_EXP_SLTCAP_AIP | PCI_EXP_SLTCAP_PIP); + /* Thunderbolt 1 ports falsely advertise Command Completed support */ + if (pdev->is_thunderbolt) + slot_cap |= PCI_EXP_SLTCAP_NCCS; + ctrl->slot_cap = slot_cap; mutex_init(&ctrl->ctrl_lock); init_waitqueue_head(&ctrl->queue);