From patchwork Sun Sep 27 05:28:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Zhao, Haifeng" X-Patchwork-Id: 11801773 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 587646CB for ; Sun, 27 Sep 2020 05:30:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 49C35239E5 for ; Sun, 27 Sep 2020 05:30:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730283AbgI0FaT (ORCPT ); Sun, 27 Sep 2020 01:30:19 -0400 Received: from mga07.intel.com ([134.134.136.100]:23634 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726478AbgI0FaT (ORCPT ); Sun, 27 Sep 2020 01:30:19 -0400 IronPort-SDR: iXRjaFxMCts+zb+4vi/rfN5RdHhcXNE5ZRIDhYaZsvS0FYdeu4t4ZtJ+pHsCXM2KVHP7xE1rBz 4iVNBIba/DJg== X-IronPort-AV: E=McAfee;i="6000,8403,9756"; a="225993130" X-IronPort-AV: E=Sophos;i="5.77,308,1596524400"; d="scan'208";a="225993130" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Sep 2020 22:30:18 -0700 IronPort-SDR: lwe9O8A/DcJFUChqzqo4COzMhjKnNj8pXL5qeArEGt4BbghcEeHLRf7jAXVfvDS7Einalw6MWK T2nkTDWvgufA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,308,1596524400"; d="scan'208";a="323909336" Received: from shskylake.sh.intel.com ([10.239.48.137]) by orsmga002.jf.intel.com with ESMTP; 26 Sep 2020 22:30:15 -0700 From: Ethan Zhao To: bhelgaas@google.com, oohall@gmail.com, ruscur@russell.cc, lukas@wunner.de, andriy.shevchenko@linux.intel.com, stuart.w.hayes@gmail.com, mr.nuke.me@gmail.com, mika.westerberg@linux.intel.com Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, pei.p.jia@intel.com, ashok.raj@linux.intel.com, sathyanarayanan.kuppuswamy@intel.com, Ethan Zhao Subject: [PATCH 1/5 V3] PCI: define a function to check and wait till port finish DPC handling Date: Sun, 27 Sep 2020 01:28:37 -0400 Message-Id: <20200927052841.12629-2-haifeng.zhao@intel.com> X-Mailer: git-send-email 2.18.4 In-Reply-To: <20200927052841.12629-1-haifeng.zhao@intel.com> References: <20200927052841.12629-1-haifeng.zhao@intel.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org Once root port DPC capability is enabled and triggered, at the beginning of DPC is triggered, the DPC status bits are set by hardware and then sends DPC/DLLSC/PDC interrupts to OS DPC and pciehp drivers, it will take the port and software DPC interrupt handler 10ms to 50ms (test data on ICS(Ice Lake SP platform, see https://en.wikichip.org/wiki/intel/microarchitectures/ice_lake_(server) & stable 5.9-rc6) to complete the DPC containment procedure till the DPC status is cleared at the end of the DPC interrupt handler. We use this function to check if the root port is in DPC handling status and wait till the hardware and software completed the procedure. Signed-off-by: Ethan Zhao Tested-by: Wen Jin Tested-by: Shanshan Zhang Reviewed-by: Andy Shevchenko --- changes: V2：align ICS code name to public doc. V3: no change. include/linux/pci.h | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/include/linux/pci.h b/include/linux/pci.h index 835530605c0d..5beb76c6ae26 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -38,6 +38,7 @@ #include #include #include +#include #include #include @@ -2427,4 +2428,34 @@ void pci_uevent_ers(struct pci_dev *pdev, enum pci_ers_result err_type); WARN_ONCE(condition, "%s %s: " fmt, \ dev_driver_string(&(pdev)->dev), pci_name(pdev), ##arg) +#ifdef CONFIG_PCIE_DPC +static inline bool pci_wait_port_outdpc(struct pci_dev *pdev) +{ + u16 cap = pdev->dpc_cap, status; + u16 loop = 0; + + if (!cap) { + pci_WARN_ONCE(pdev, !cap, "No DPC capability initiated\n"); + return false; + } + pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status); + pci_dbg(pdev, "DPC status %x, cap %x\n", status, cap); + while (status & PCI_EXP_DPC_STATUS_TRIGGER && loop < 100) { + msleep(10); + loop++; + pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status); + } + if (!(status & PCI_EXP_DPC_STATUS_TRIGGER)) { + pci_dbg(pdev, "Out of DPC %x, cost %d ms\n", status, loop*10); + return true; + } + pci_dbg(pdev, "Timeout to wait port out of DPC status\n"); + return false; +} +#else +static inline bool pci_wait_port_outdpc(struct pci_dev *pdev) +{ + return true; +} +#endif #endif /* LINUX_PCI_H */ From patchwork Sun Sep 27 05:28:38 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Zhao, Haifeng" X-Patchwork-Id: 11801775 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 785981580 for ; Sun, 27 Sep 2020 05:30:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6A7D7239E5 for ; Sun, 27 Sep 2020 05:30:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726478AbgI0FaW (ORCPT ); Sun, 27 Sep 2020 01:30:22 -0400 Received: from mga07.intel.com ([134.134.136.100]:23634 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726382AbgI0FaW (ORCPT ); Sun, 27 Sep 2020 01:30:22 -0400 IronPort-SDR: EF+MVEHadryAax0dkhrPvTKUEQCrN7cfkUO7Q19l+sv22vrsNV2QBqBG96iqcJp1jTo/io1Z4l 8w8s4GgU3ywA== X-IronPort-AV: E=McAfee;i="6000,8403,9756"; a="225993135" X-IronPort-AV: E=Sophos;i="5.77,308,1596524400"; d="scan'208";a="225993135" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Sep 2020 22:30:22 -0700 IronPort-SDR: ot33BQvV7wlSBLxEUOTNCJPdATDB5gRN+KDPZkFA+e2HydkfPdwgBWtapxakGE1YmnLVs2mb1S hxUcdoSuDRnw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,308,1596524400"; d="scan'208";a="323909364" Received: from shskylake.sh.intel.com ([10.239.48.137]) by orsmga002.jf.intel.com with ESMTP; 26 Sep 2020 22:30:18 -0700 From: Ethan Zhao To: bhelgaas@google.com, oohall@gmail.com, ruscur@russell.cc, lukas@wunner.de, andriy.shevchenko@linux.intel.com, stuart.w.hayes@gmail.com, mr.nuke.me@gmail.com, mika.westerberg@linux.intel.com Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, pei.p.jia@intel.com, ashok.raj@linux.intel.com, sathyanarayanan.kuppuswamy@intel.com, Ethan Zhao Subject: [PATCH 2/5 V3] PCI: pciehp: check and wait port status out of DPC before handling DLLSC and PDC Date: Sun, 27 Sep 2020 01:28:38 -0400 Message-Id: <20200927052841.12629-3-haifeng.zhao@intel.com> X-Mailer: git-send-email 2.18.4 In-Reply-To: <20200927052841.12629-1-haifeng.zhao@intel.com> References: <20200927052841.12629-1-haifeng.zhao@intel.com> Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org When root port has DPC capability and it is enabled, then triggered by errors, DPC DLLSC and PDC interrupts will be sent to DPC driver, pciehp driver at the same time. That will cause following result: 1. Link and device are recovered by hardware DPC and software DPC driver, device isn't removed, but the pciehp might treat it as device was hot removed. 2. Race condition happens bettween pciehp_unconfigure_device() called by pciehp_ist() in pciehp driver and pci_do_recovery() called by dpc_handler in DPC driver. no luck, there is no lock to protect pci_stop_and_remove_bus_device() against pci_walk_bus(), they hold different samphore and mutex, pci_stop_and_remove_bus_device holds pci_rescan_remove_lock, and pci_walk_bus() holds pci_bus_sem. This race condition is not purely code analysis, it could be triggered by following command series: # setpci -s 64:02.0 0x196.w=000a // 64:02.0 rootport has DPC capability # setpci -s 65:00.0 0x04.w=0544 // 65:00.0 NVMe SSD populated in port # mount /dev/nvme0n1p1 nvme One shot will cause system panic and NULL pointer reference happened. (tested on stable 5.8 & ICS(Ice Lake SP platform, see https://en.wikichip.org/wiki/intel/microarchitectures/ice_lake_(server)) Buffer I/O error on dev nvme0n1p1, logical block 3328, async page read BUG: kernel NULL pointer dereference, address: 0000000000000050 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page PGD 0 Oops: 0000 [#1] SMP NOPTI CPU: 12 PID: 513 Comm: irq/124-pcie-dp Not tainted 5.8.0 el8.x86_64+ #1 RIP: 0010:report_error_detected.cold.4+0x7d/0xe6 Code: b6 d0 e8 e8 fe 11 00 e8 16 c5 fb ff be 06 00 00 00 48 89 df e8 d3 65 ff ff b8 06 00 00 00 e9 75 fc ff ff 48 8b 43 68 45 31 c9 <48> 8b 50 50 48 83 3a 00 41 0f 94 c1 45 31 c0 48 85 d2 41 0f 94 c0 RSP: 0018:ff8e06cf8762fda8 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ff4e3eaacf42a000 RCX: ff4e3eb31f223c01 RDX: ff4e3eaacf42a140 RSI: ff4e3eb31f223c00 RDI: ff4e3eaacf42a138 RBP: ff8e06cf8762fdd0 R08: 00000000000000bf R09: 0000000000000000 R10: 000000eb8ebeab53 R11: ffffffff93453258 R12: 0000000000000002 R13: ff4e3eaacf42a130 R14: ff8e06cf8762fe2c R15: ff4e3eab44733828 FS: 0000000000000000(0000) GS:ff4e3eab1fd00000(0000) knl GS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000050 CR3: 0000000f8f80a004 CR4: 0000000000761ee0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 PKRU: 55555554 Call Trace: ? report_normal_detected+0x20/0x20 report_frozen_detected+0x16/0x20 pci_walk_bus+0x75/0x90 ? dpc_irq+0x90/0x90 pcie_do_recovery+0x157/0x201 ? irq_finalize_oneshot.part.47+0xe0/0xe0 dpc_handler+0x29/0x40 irq_thread_fn+0x24/0x60 irq_thread+0xea/0x170 ? irq_forced_thread_fn+0x80/0x80 ? irq_thread_check_affinity+0xf0/0xf0 kthread+0x124/0x140 ? kthread_park+0x90/0x90 ret_from_fork+0x1f/0x30 Modules linked in: nft_fib_inet......... CR2: 0000000000000050 With this patch, the handling flow of DPC containment and hotplug is partly ordered and serialized, let hardware DPC do the controller reset etc recovery action first, then DPC driver handling the call-back from device drivers, clear the DPC status, at the end, pciehp handle the DLLSC and PDC etc. Signed-off-by: Ethan Zhao Tested-by: Wen Jin Tested-by: Shanshan Zhang Reviewed-by: Andy Shevchenko --- Changes: V2: revise doc according to Andy's suggestion. V3: no change. drivers/pci/hotplug/pciehp_hpc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 53433b37e181..6f271160f18d 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -710,8 +710,10 @@ static irqreturn_t pciehp_ist(int irq, void *dev_id) down_read(&ctrl->reset_lock); if (events & DISABLE_SLOT) pciehp_handle_disable_request(ctrl); - else if (events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC)) + else if (events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC)) { + pci_wait_port_outdpc(pdev); pciehp_handle_presence_or_link_change(ctrl, events); + } up_read(&ctrl->reset_lock); ret = IRQ_HANDLED; From patchwork Sun Sep 27 05:28:39 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Zhao, Haifeng" X-Patchwork-Id: 11801779 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A34286CB for ; Sun, 27 Sep 2020 05:30:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 93E84239EE for ; Sun, 27 Sep 2020 05:30:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730307AbgI0Fa2 (ORCPT ); Sun, 27 Sep 2020 01:30:28 -0400 Received: from mga07.intel.com ([134.134.136.100]:23634 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726382AbgI0Fa0 (ORCPT ); Sun, 27 Sep 2020 01:30:26 -0400 IronPort-SDR: D7f/NyEJG6bYuqe03hmpi14w6Oz2k8K8Fl92Yn4Az8URFIN3Hr2ZzgGPItpJHaug55t5v8T21V iyQiVF0VSE+g== X-IronPort-AV: E=McAfee;i="6000,8403,9756"; a="225993140" X-IronPort-AV: E=Sophos;i="5.77,308,1596524400"; d="scan'208";a="225993140" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Sep 2020 22:30:25 -0700 IronPort-SDR: 17+Oq7Jbpafrap2OeZeeBWxz0VZjXPUqhb1F99EcaBT3YLAYDsi74znStTyZiMNIrZDyB8z/6h EpcMkl0cX/DQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,308,1596524400"; d="scan'208";a="323909387" Received: from shskylake.sh.intel.com ([10.239.48.137]) by orsmga002.jf.intel.com with ESMTP; 26 Sep 2020 22:30:22 -0700 From: Ethan Zhao To: bhelgaas@google.com, oohall@gmail.com, ruscur@russell.cc, lukas@wunner.de, andriy.shevchenko@linux.intel.com, stuart.w.hayes@gmail.com, mr.nuke.me@gmail.com, mika.westerberg@linux.intel.com Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, pei.p.jia@intel.com, ashok.raj@linux.intel.com, sathyanarayanan.kuppuswamy@intel.com, Ethan Zhao Subject: [PATCH 3/5 V3] PCI/ERR: get device before call device driver to avoid NULL pointer reference Date: Sun, 27 Sep 2020 01:28:39 -0400 Message-Id: <20200927052841.12629-4-haifeng.zhao@intel.com> X-Mailer: git-send-email 2.18.4 In-Reply-To: <20200927052841.12629-1-haifeng.zhao@intel.com> References: <20200927052841.12629-1-haifeng.zhao@intel.com> Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org During DPC error injection test we found there is race condition between pciehp and DPC driver, NULL pointer reference caused panic as following # setpci -s 64:02.0 0x196.w=000a // 64:02.0 is rootport has DPC capability # setpci -s 65:00.0 0x04.w=0544 // 65:00.0 is NVMe SSD populated in above port # mount /dev/nvme0n1p1 nvme (tested on stable 5.8 & ICS(Ice Lake SP platform, see https://en.wikichip.org/wiki/intel/microarchitectures/ice_lake_(server)) Buffer I/O error on dev nvme0n1p1, logical block 468843328, async page read BUG: kernel NULL pointer dereference, address: 0000000000000050 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page PGD 0 Oops: 0000 [#1] SMP NOPTI CPU: 12 PID: 513 Comm: irq/124-pcie-dp Not tainted 5.8.0-0.0.7.el8.x86_64+ #1 RIP: 0010:report_error_detected.cold.4+0x7d/0xe6 Code: b6 d0 e8 e8 fe 11 00 e8 16 c5 fb ff be 06 00 00 00 48 89 df e8 d3 65 ff ff b8 06 00 00 00 e9 75 fc ff ff 48 8b 43 68 45 31 c9 <48> 8b 50 50 48 83 3a 00 41 0f 94 c1 45 31 c0 48 85 d2 41 0f 94 c0 RSP: 0018:ff8e06cf8762fda8 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ff4e3eaacf42a000 RCX: ff4e3eb31f223c01 RDX: ff4e3eaacf42a140 RSI: ff4e3eb31f223c00 RDI: ff4e3eaacf42a138 RBP: ff8e06cf8762fdd0 R08: 00000000000000bf R09: 0000000000000000 R10: 000000eb8ebeab53 R11: ffffffff93453258 R12: 0000000000000002 R13: ff4e3eaacf42a130 R14: ff8e06cf8762fe2c R15: ff4e3eab44733828 FS: 0000000000000000(0000) GS:ff4e3eab1fd00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000050 CR3: 0000000f8f80a004 CR4: 0000000000761ee0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 PKRU: 55555554 Call Trace: ? report_normal_detected+0x20/0x20 report_frozen_detected+0x16/0x20 pci_walk_bus+0x75/0x90 ? dpc_irq+0x90/0x90 pcie_do_recovery+0x157/0x201 ? irq_finalize_oneshot.part.47+0xe0/0xe0 dpc_handler+0x29/0x40 irq_thread_fn+0x24/0x60 irq_thread+0xea/0x170 ? irq_forced_thread_fn+0x80/0x80 ? irq_thread_check_affinity+0xf0/0xf0 kthread+0x124/0x140 ? kthread_park+0x90/0x90 ret_from_fork+0x1f/0x30 Modules linked in: nft_fib_inet......... CR2: 0000000000000050 Though we partly close the race condition with patch 'PCI: pciehp: check and wait port status out of DPC before handling DLLSC and PDC', but there is no hardware spec or software sequence to guarantee the pcie_ist() run into pci_wait_port_outdpc() first or DPC triggered status bits being set first when errors triggered DPC containment procedure, so device still could be removed by function pci_stop_and_removed_bus_device() then freed by pci_dev_put() in pciehp driver first during pcie_do_recover()/ pci_walk_bus() is called by dpc_handler() in DPC driver. Maybe unify pci_bus_sem and pci_rescan_remove_lock to serialize the removal and walking operation is the right way, but here we use pci_dev_get() to increase the reference count of device before using the device to avoid it is freed in use. With this patch and patch 'PCI: pciehp: check and wait port status out of DPC before handling DLLSC and PDC', stable 5.9-rc6 could pass the error injection test and no panic happened. Brute DPC error injection script: for i in {0..100} do setpci -s 64:02.0 0x196.w=000a setpci -s 65:00.0 0x04.w=0544 mount /dev/nvme0n1p1 /root/nvme sleep 1 done Signed-off-by: Ethan Zhao Tested-by: Wen Jin Tested-by: Shanshan Zhang Reviewed-by: Andy Shevchenko --- Changes: V2: revise doc according to Andy's suggestion. V3: no change. drivers/pci/pcie/err.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c index c543f419d8f9..e35c4480c86b 100644 --- a/drivers/pci/pcie/err.c +++ b/drivers/pci/pcie/err.c @@ -52,6 +52,8 @@ static int report_error_detected(struct pci_dev *dev, pci_ers_result_t vote; const struct pci_error_handlers *err_handler; + if (!pci_dev_get(dev)) + return 0; device_lock(&dev->dev); if (!pci_dev_set_io_state(dev, state) || !dev->driver || @@ -76,6 +78,7 @@ static int report_error_detected(struct pci_dev *dev, pci_uevent_ers(dev, vote); *result = merge_result(*result, vote); device_unlock(&dev->dev); + pci_dev_put(dev); return 0; } @@ -94,6 +97,8 @@ static int report_mmio_enabled(struct pci_dev *dev, void *data) pci_ers_result_t vote, *result = data; const struct pci_error_handlers *err_handler; + if (!pci_dev_get(dev)) + return 0; device_lock(&dev->dev); if (!dev->driver || !dev->driver->err_handler || @@ -105,6 +110,7 @@ static int report_mmio_enabled(struct pci_dev *dev, void *data) *result = merge_result(*result, vote); out: device_unlock(&dev->dev); + pci_dev_put(dev); return 0; } @@ -113,6 +119,8 @@ static int report_slot_reset(struct pci_dev *dev, void *data) pci_ers_result_t vote, *result = data; const struct pci_error_handlers *err_handler; + if (!pci_dev_get(dev)) + return 0; device_lock(&dev->dev); if (!dev->driver || !dev->driver->err_handler || @@ -124,6 +132,7 @@ static int report_slot_reset(struct pci_dev *dev, void *data) *result = merge_result(*result, vote); out: device_unlock(&dev->dev); + pci_dev_put(dev); return 0; } @@ -131,6 +140,8 @@ static int report_resume(struct pci_dev *dev, void *data) { const struct pci_error_handlers *err_handler; + if (!pci_dev_get(dev)) + return 0; device_lock(&dev->dev); if (!pci_dev_set_io_state(dev, pci_channel_io_normal) || !dev->driver || @@ -143,6 +154,7 @@ static int report_resume(struct pci_dev *dev, void *data) out: pci_uevent_ers(dev, PCI_ERS_RESULT_RECOVERED); device_unlock(&dev->dev); + pci_dev_put(dev); return 0; } From patchwork Sun Sep 27 05:28:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Zhao, Haifeng" X-Patchwork-Id: 11801777 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id EB38D92C for ; Sun, 27 Sep 2020 05:30:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DBBF7239E5 for ; Sun, 27 Sep 2020 05:30:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730314AbgI0Fa3 (ORCPT ); Sun, 27 Sep 2020 01:30:29 -0400 Received: from mga07.intel.com ([134.134.136.100]:23634 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730311AbgI0Fa3 (ORCPT ); Sun, 27 Sep 2020 01:30:29 -0400 IronPort-SDR: iSwAXnDphXteXzDay/ftzkpOY9JqcWgXBwvA1PE3z2/P9zRE71KoxgEPrHw4ugSRVv6DABLroD jrS2KlKBfJwQ== X-IronPort-AV: E=McAfee;i="6000,8403,9756"; a="225993143" X-IronPort-AV: E=Sophos;i="5.77,308,1596524400"; d="scan'208";a="225993143" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Sep 2020 22:30:29 -0700 IronPort-SDR: z4V9w1T1LEzoevfb48WDI52acjviLoA++OhAR3+JBmlcV49l2+BS+kmC6bbY6I/G4wN3QFVfRI oHwxoGiXX2BQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,308,1596524400"; d="scan'208";a="323909397" Received: from shskylake.sh.intel.com ([10.239.48.137]) by orsmga002.jf.intel.com with ESMTP; 26 Sep 2020 22:30:26 -0700 From: Ethan Zhao To: bhelgaas@google.com, oohall@gmail.com, ruscur@russell.cc, lukas@wunner.de, andriy.shevchenko@linux.intel.com, stuart.w.hayes@gmail.com, mr.nuke.me@gmail.com, mika.westerberg@linux.intel.com Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, pei.p.jia@intel.com, ashok.raj@linux.intel.com, sathyanarayanan.kuppuswamy@intel.com, Ethan Zhao Subject: [PATCH 4/5 V3] PCI: only return true when dev io state is really changed Date: Sun, 27 Sep 2020 01:28:40 -0400 Message-Id: <20200927052841.12629-5-haifeng.zhao@intel.com> X-Mailer: git-send-email 2.18.4 In-Reply-To: <20200927052841.12629-1-haifeng.zhao@intel.com> References: <20200927052841.12629-1-haifeng.zhao@intel.com> Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org When uncorrectable error happens, AER driver and DPC driver interrupt handlers likely call pcie_do_recovery() ->pci_walk_bus() ->report_frozen_detected() with pci_channel_io_frozen the same time. If pci_dev_set_io_state() return true even if the original state is pci_channel_io_frozen, that will cause AER or DPC handler re-enter the error detecting and recovery procedure one after another. The result is the recovery flow mixed between AER and DPC. So simplify the pci_dev_set_io_state() function to only return true when dev->error_state is changed. Signed-off-by: Ethan Zhao Tested-by: Wen Jin Tested-by: Shanshan Zhang Reviewed-by: Andy Shevchenko Reviewed-by: Alexandru Gagniuc Reviewed-by: Joe Perches --- Changnes: V2: revise description and code according to suggestion from Andy. V3: Change code to simpler. drivers/pci/pci.h | 37 +++++-------------------------------- 1 file changed, 5 insertions(+), 32 deletions(-) diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index fa12f7cbc1a0..a2c1c7d5f494 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -359,39 +359,12 @@ struct pci_sriov { static inline bool pci_dev_set_io_state(struct pci_dev *dev, pci_channel_state_t new) { - bool changed = false; - device_lock_assert(&dev->dev); - switch (new) { - case pci_channel_io_perm_failure: - switch (dev->error_state) { - case pci_channel_io_frozen: - case pci_channel_io_normal: - case pci_channel_io_perm_failure: - changed = true; - break; - } - break; - case pci_channel_io_frozen: - switch (dev->error_state) { - case pci_channel_io_frozen: - case pci_channel_io_normal: - changed = true; - break; - } - break; - case pci_channel_io_normal: - switch (dev->error_state) { - case pci_channel_io_frozen: - case pci_channel_io_normal: - changed = true; - break; - } - break; - } - if (changed) - dev->error_state = new; - return changed; + if (dev->error_state == new) + return false; + + dev->error_state = new; + return true; } static inline int pci_dev_set_disconnected(struct pci_dev *dev, void *unused) From patchwork Sun Sep 27 05:28:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Zhao, Haifeng" X-Patchwork-Id: 11801781 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 84A466CB for ; Sun, 27 Sep 2020 05:30:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 74D90239E5 for ; Sun, 27 Sep 2020 05:30:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726840AbgI0Faj (ORCPT ); Sun, 27 Sep 2020 01:30:39 -0400 Received: from mga07.intel.com ([134.134.136.100]:23634 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730325AbgI0Fad (ORCPT ); Sun, 27 Sep 2020 01:30:33 -0400 IronPort-SDR: kz45VRzIvB/Yz+d0VE8vuWrubWlo/cubvB5VTu3F78PSblpVeaFqKhNvw+YFL55MOT0q9M2BxF ag5Yxxs3G8ow== X-IronPort-AV: E=McAfee;i="6000,8403,9756"; a="225993145" X-IronPort-AV: E=Sophos;i="5.77,308,1596524400"; d="scan'208";a="225993145" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Sep 2020 22:30:33 -0700 IronPort-SDR: wAmf8ckV5LcAVkHc6U//N7Z1GbAd4QYDLQ8AtknV2w5/bvFaeazLbdmuLnTb2+89YEZaZaBdkC yy7jwAlDxNgA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,308,1596524400"; d="scan'208";a="323909418" Received: from shskylake.sh.intel.com ([10.239.48.137]) by orsmga002.jf.intel.com with ESMTP; 26 Sep 2020 22:30:29 -0700 From: Ethan Zhao To: bhelgaas@google.com, oohall@gmail.com, ruscur@russell.cc, lukas@wunner.de, andriy.shevchenko@linux.intel.com, stuart.w.hayes@gmail.com, mr.nuke.me@gmail.com, mika.westerberg@linux.intel.com Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, pei.p.jia@intel.com, ashok.raj@linux.intel.com, sathyanarayanan.kuppuswamy@intel.com, Ethan Zhao Subject: [PATCH 5/5 V3] PCI/ERR: don't mix io state not changed and no driver together Date: Sun, 27 Sep 2020 01:28:41 -0400 Message-Id: <20200927052841.12629-6-haifeng.zhao@intel.com> X-Mailer: git-send-email 2.18.4 In-Reply-To: <20200927052841.12629-1-haifeng.zhao@intel.com> References: <20200927052841.12629-1-haifeng.zhao@intel.com> Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org When we see 'can't recover (no error_detected callback)' on console, Maybe the reason is io state is not changed by calling pci_dev_set_io_state(), that is confused. fix it. Signed-off-by: Ethan Zhao Tested-by: Wen Jin Tested-by: Shanshan Zhang --- Chagnes: V2: no change. V3: no change. drivers/pci/pcie/err.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c index e35c4480c86b..d85f27c90c26 100644 --- a/drivers/pci/pcie/err.c +++ b/drivers/pci/pcie/err.c @@ -55,8 +55,10 @@ static int report_error_detected(struct pci_dev *dev, if (!pci_dev_get(dev)) return 0; device_lock(&dev->dev); - if (!pci_dev_set_io_state(dev, state) || - !dev->driver || + if (!pci_dev_set_io_state(dev, state)) { + pci_dbg(dev, "Device might already being in error handling ...\n"); + vote = PCI_ERS_RESULT_NONE; + } else if (!dev->driver || !dev->driver->err_handler || !dev->driver->err_handler->error_detected) { /*