From: Ethan Zhao <haifeng.zhao@intel.com> 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, ashok.raj@linux.intel.com, sathyanarayanan.kuppuswamy@intel.com, xerces.zhao@gmail.com, Ethan Zhao <haifeng.zhao@intel.com> Subject: [PATCH v8 0/6] Fix DPC hotplug race and enhance error handling Date: Wed, 7 Oct 2020 07:31:52 -0400 [thread overview] Message-ID: <20201007113158.48933-1-haifeng.zhao@intel.com> (raw) Hi,folks, This simple patch set fixed some serious security issues found when DPC error injection and NVMe SSD hotplug brute force test were doing -- race condition between DPC handler and pciehp, AER interrupt handlers, caused system hang and system with DPC feature couldn't recover to normal working state as expected (NVMe instance lost, mount operation hang, race PCIe access caused uncorrectable errors reported alternatively etc). The fundamental premise is that when due to error conditions (NON-FATAL/ FATAL) when events are processed by both DPC handler and hotplug handling of DLLSC/PDC both operating on the same device object ends up with crashes (from Ashok). Debug shows when port DPC feature was enabled and triggered by errors, DLLSC/PDC/DPC interrupts will be sent to pciehp and DPC driver almost at the same time, and no delay between them is required by specification. so DPC driver and pciehp drivers may handle these interrupts cocurrently, thus introduces the possibility of race condition, other details see every commit description part. With this patch set applied, stable 5.9-rc6 on ICS (Ice Lake SP platform, see https://en.wikichip.org/wiki/intel/microarchitectures/ice_lake_(server)) could pass the PCIe Gen4 NVMe SSD brute force hotplug test with any time interval between hot-remove and plug-in operation tens of times without any errors occur and system works normal. With this patch set applied, system with DPC feature could recover from NON-FATAL and FATAL errors injection test and works as expected. System works smoothly when errors happen while hotplug is doing, no uncorrectable errors found. 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 This patch set could be applied to stable 5.9-rc6/rc7/rc8 directly. Help to review and test. v2: changed according to review by Andy Shevchenko. v3: changed patch 4/5 to simpler coding. v4: move function pci_wait_port_outdpc() to DPC driver and its declaration to pci.h. (tip from Christoph Hellwig <hch@infradead.org>). v5: fix building issue reported by lkp@intel.com with some config. v6: move patch[3/5] as the first patch according to Lukas's suggestion. and rewrite the comment part of patch[3/5]. v7: change the patch[4/5], based on Bjorn's code and truth table. change the patch[5/5] about the debug output information. v8: according Bjorn's suggestion, put the pci_dev_set_io_state() simplification but no function code in one patch.(almost copy of Bjorn's code and truth table, understood). patch 5/6 re-based the function change code of pci_dev_set_io_state(). per Ashok's request, add more description to this cover-letter part. Thanks, Ethan Ethan Zhao (6): PCI/ERR: get device before call device driver to avoid NULL pointer dereference PCI/DPC: define a function to check and wait till port finish DPC handling PCI: pciehp: check and wait port status out of DPC before handling DLLSC and PDC PCI/ERR: simplify function pci_dev_set_io_state() with if PCI/ERR: only return true when dev io state is really changed PCI/ERR: don't mix io state not changed and no driver together drivers/pci/hotplug/pciehp_hpc.c | 4 ++- drivers/pci/pci.h | 55 +++++++++++++------------------- drivers/pci/pcie/dpc.c | 27 ++++++++++++++++ drivers/pci/pcie/err.c | 18 +++++++++-- 4 files changed, 69 insertions(+), 35 deletions(-) base-commit: 549738f15da0e5a00275977623be199fbbf7df50 -- 2.18.4
next reply other threads:[~2020-10-07 11:33 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-10-07 11:31 Ethan Zhao [this message] 2020-10-07 11:31 ` [PATCH v8 1/6] PCI/ERR: get device before call device driver to avoid NULL pointer dereference Ethan Zhao 2020-10-07 17:24 ` Kuppuswamy, Sathyanarayanan 2020-10-08 5:38 ` Ethan Zhao 2020-10-07 11:31 ` [PATCH v8 2/6] PCI/DPC: define a function to check and wait till port finish DPC handling Ethan Zhao 2020-10-07 17:28 ` Kuppuswamy, Sathyanarayanan 2020-10-08 5:49 ` Ethan Zhao 2020-10-09 3:16 ` Ethan Zhao 2020-10-07 11:31 ` [PATCH v8 3/6] PCI: pciehp: check and wait port status out of DPC before handling DLLSC and PDC Ethan Zhao 2020-10-07 11:31 ` [PATCH v8 4/6] PCI/ERR: simplify function pci_dev_set_io_state() with if Ethan Zhao 2020-10-07 11:31 ` [PATCH v8 5/6] PCI/ERR: only return true when dev io state is really changed Ethan Zhao 2020-10-07 11:31 ` [PATCH v8 6/6] PCI/ERR: don't mix io state not changed and no driver together Ethan Zhao
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=20201007113158.48933-1-haifeng.zhao@intel.com \ --to=haifeng.zhao@intel.com \ --cc=andriy.shevchenko@linux.intel.com \ --cc=ashok.raj@linux.intel.com \ --cc=bhelgaas@google.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=lukas@wunner.de \ --cc=mika.westerberg@linux.intel.com \ --cc=mr.nuke.me@gmail.com \ --cc=oohall@gmail.com \ --cc=ruscur@russell.cc \ --cc=sathyanarayanan.kuppuswamy@intel.com \ --cc=stuart.w.hayes@gmail.com \ --cc=xerces.zhao@gmail.com \ --subject='Re: [PATCH v8 0/6] Fix DPC hotplug race and enhance error handling' \ /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
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).