From patchwork Fri Apr 13 21:16:51 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 10340769 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 369E3600D0 for ; Fri, 13 Apr 2018 21:16:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 225A728A14 for ; Fri, 13 Apr 2018 21:16:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 16DE028A18; Fri, 13 Apr 2018 21:16:57 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A7FB028A14 for ; Fri, 13 Apr 2018 21:16:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751946AbeDMVQz (ORCPT ); Fri, 13 Apr 2018 17:16:55 -0400 Received: from mail.kernel.org ([198.145.29.99]:45642 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750973AbeDMVQy (ORCPT ); Fri, 13 Apr 2018 17:16:54 -0400 Received: from localhost (unknown [69.71.5.252]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 57F79205F4; Fri, 13 Apr 2018 21:16:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 57F79205F4 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=helgaas@kernel.org Date: Fri, 13 Apr 2018 16:16:51 -0500 From: Bjorn Helgaas To: Srinath Mannam Cc: Bjorn Helgaas , Ray Jui , linux-pci@vger.kernel.org, Rajat Jain Subject: Re: Issue with Enable LTR while pcie_aspm off Message-ID: <20180413211651.GA80087@bhelgaas-glaptop.roam.corp.google.com> References: <20180413135659.GC46420@bhelgaas-glaptop.roam.corp.google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Apr 13, 2018 at 09:13:42PM +0530, Srinath Mannam wrote: > Hi Bjorn, > > Thank you very much for quick response. > > I am using samsung NVMe card as EP connected to our platform. As per > link capabilities device does not support L1.s. > But LTR is enabled in DEVCAP2. Interesting that the device advertises LTR support, but not L1.2 support. As far as I can tell, that's legal per spec, but I don't know what the LTR info would be used for. The only use I know about is comparison with LTR_L1.2_THRESHOLD, as in PCIe r4.0, sec 5.5.1. Can you attach lspci -vv output for the NVMe device and all the devices in the path leading to it? > In my observation after setting LTR without enabling ASPM (common > clock and link retraining) in software, device stopped responding. From sec 5.5.1, it seems clear that LTR would have to be enabled before L1.2 is enabled, because the decision to enter L1.2 depends on information from LTR messages. Since the device advertised LTR support, it seems like a hardware defect if enabling it breaks something. But I can imagine that if it doesn't advertise L1.2 support, nobody expected LTR to be used. Can you try the patch below? It should make it so we don't enable LTR unless we also support L1.2. Maybe that's enough to avoid the issue. Bjorn diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index f76eb7704f64..9e2212889e7e 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -400,6 +400,16 @@ static void pcie_get_aspm_reg(struct pci_dev *pdev, info->l1ss_cap = 0; return; } + + /* + * If we don't have LTR for the entire path from the Root Complex + * to this device, we can't use L1.2. PCIe r4.0, secs 5.5.4, 6.18. + */ + if (!pdev->ltr_path) { + info->l1ss_cap &= ~PCI_L1SS_CTL1_PCIPM_L1_2; + info->l1ss_cap &= ~PCI_L1SS_CTL1_ASPM_L1_2; + } + pci_read_config_dword(pdev, info->l1ss_cap_ptr + PCI_L1SS_CTL1, &info->l1ss_ctl1); pci_read_config_dword(pdev, info->l1ss_cap_ptr + PCI_L1SS_CTL2, diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index ac91b6fd0bcd..9a224612b3f8 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1954,12 +1954,29 @@ static void pci_configure_relaxed_ordering(struct pci_dev *dev) static void pci_configure_ltr(struct pci_dev *dev) { #ifdef CONFIG_PCIEASPM - u32 cap; + u32 cap, l1ss_cap, l1ss; struct pci_dev *bridge; if (!pci_is_pcie(dev)) return; + /* + * Per PCIe r4.0, sec 5.5.4, we must program LTR_L1.2_THRESHOLD if + * either L1.2 enable bit is set. That suggests that LTR must be + * enabled before L1.2. If the device (and the entire path leading + * to it) advertises L1.2 and LTR support, enable LTR. + */ + l1ss_cap = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_L1SS); + if (!l1ss_cap) + return; + + pci_read_config_dword(dev, l1ss_cap + PCI_L1SS_CAP, &l1ss); + if (!(l1ss & PCI_L1SS_CAP_L1_PM_SS)) + return; + + if (!(l1ss & (PCI_L1SS_CAP_PCIPM_L1_2 | PCI_L1SS_CAP_ASPM_L1_2))) + return; + pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap); if (!(cap & PCI_EXP_DEVCAP2_LTR)) return; @@ -1967,7 +1984,7 @@ static void pci_configure_ltr(struct pci_dev *dev) /* * Software must not enable LTR in an Endpoint unless the Root * Complex and all intermediate Switches indicate support for LTR. - * PCIe r3.1, sec 6.18. + * PCIe r4.0, sec 6.18. */ if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) dev->ltr_path = 1;