From patchwork Mon Mar 25 23:58:01 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Jiang X-Patchwork-Id: 13603199 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 31E4384D3F; Mon, 25 Mar 2024 23:59:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1711411158; cv=none; b=twuPSgNC2cFyvhwZRWp+eK0WiJke6LOG2/gysu4o5lBbHJ2XwZ48LhdMU2TLNngaXS01KpyGPAwCC27Jf/E7Z4Hn532Qdq51zwN8vCbJ9eqMGJgFPyJodqo8a+u0C8HTZSZC3f9kWoAIwiuaP18+uyER1TFfFR3WqMnu3f0mAMo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1711411158; c=relaxed/simple; bh=sJsKnYoV1Hlt1lUqxBSXv168upPf3QW9QRynQHgorbw=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=BE2+qAS4IKSmsJsEuv8gedskMw2qeOdCZ2OT9owGXIyBo4lxgpSrOtPHprLCNPVHodHU8OW/7XkEwiSvTBohTVcxk0uhnH4ShE3jzha2Xrq5Zr9xHY1Y8GX0Bu7aJtRTzqQ1MTY7cWy9/AsLyTsXG4+TppLMkXjde+Zlw4OVhWw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id AAD41C433C7; Mon, 25 Mar 2024 23:59:17 +0000 (UTC) From: Dave Jiang To: linux-cxl@vger.kernel.org, linux-pci@vger.kernel.org Cc: dan.j.williams@intel.com, ira.weiny@intel.com, vishal.l.verma@intel.com, alison.schofield@intel.com, Jonathan.Cameron@huawei.com, dave@stgolabs.net, bhelgaas@google.com, lukas@wunner.de Subject: [PATCH v2 1/3] PCI: Add check for CXL Secondary Bus Reset Date: Mon, 25 Mar 2024 16:58:01 -0700 Message-ID: <20240325235914.1897647-2-dave.jiang@intel.com> X-Mailer: git-send-email 2.44.0 In-Reply-To: <20240325235914.1897647-1-dave.jiang@intel.com> References: <20240325235914.1897647-1-dave.jiang@intel.com> Precedence: bulk X-Mailing-List: linux-cxl@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Per CXL spec r3.1 8.1.5.2, secondary bus reset is masked unless the "Unmask SBR" bit is set. Add a check to the PCI secondary bus reset path to fail the CXL SBR request if the "Unmask SBR" bit is clear in the CXL Port Control Extensions register by returning -ENOTTY. With the current behavior, the bus_reset would appear to have executed successfully, however the operation is actually masked if the "Unmask SBR" bit is set with the default value. The intention is to inform the user that SBR for the CXL device is masked and will not go through. The expectation is that if a user overrides the "Unmask SBR" via a user tool such as setpci then they can trigger a bus reset successfully. Link: https://lore.kernel.org/linux-cxl/20240220203956.GA1502351@bhelgaas/ Signed-off-by: Dave Jiang --- v2: - Rename is_cxl_device() to pci_is_cxl(). (Lukas) - Inverse xmas tree var declaration for is_cxl_port_sbr_masked(). (Lukas) - Return -ENOTTY on error of reset method. (Lukas) - Use pci_upstream_bridge() instead of dev->bus->self. (Lukas) - Additional explanation in commit log on behavior. (Lukas) --- drivers/cxl/cxlpci.h | 2 -- drivers/pci/pci.c | 45 +++++++++++++++++++++++++++++++++++ include/uapi/linux/pci_regs.h | 7 ++++++ 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h index 93992a1c8eec..55be4dccbed0 100644 --- a/drivers/cxl/cxlpci.h +++ b/drivers/cxl/cxlpci.h @@ -13,10 +13,8 @@ * "DVSEC" redundancies removed. When obvious, abbreviations may be used. */ #define PCI_DVSEC_HEADER1_LENGTH_MASK GENMASK(31, 20) -#define PCI_DVSEC_VENDOR_ID_CXL 0x1E98 /* CXL 2.0 8.1.3: PCIe DVSEC for CXL Device */ -#define CXL_DVSEC_PCIE_DEVICE 0 #define CXL_DVSEC_CAP_OFFSET 0xA #define CXL_DVSEC_MEM_CAPABLE BIT(2) #define CXL_DVSEC_HDM_COUNT_MASK GENMASK(5, 4) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index e5f243dd4288..259e5d6538bb 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4927,10 +4927,55 @@ static int pci_dev_reset_slot_function(struct pci_dev *dev, bool probe) return pci_reset_hotplug_slot(dev->slot->hotplug, probe); } +static bool pci_is_cxl(struct pci_dev *dev) +{ + return pci_find_dvsec_capability(dev, PCI_DVSEC_VENDOR_ID_CXL, + CXL_DVSEC_PCIE_DEVICE); +} + +static bool is_cxl_port_sbr_masked(struct pci_dev *dev) +{ + int dvsec; + u16 reg; + int rc; + + /* + * No DVSEC found, must not be CXL port. + */ + dvsec = pci_find_dvsec_capability(dev, PCI_DVSEC_VENDOR_ID_CXL, + CXL_DVSEC_PCIE_PORT); + if (!dvsec) + return false; + + rc = pci_read_config_word(dev, dvsec + CXL_DVSEC_PORT_CONTROL, ®); + if (rc) + return true; + + /* + * CXL spec r3.1 8.1.5.2 + * When 0, SBR bit in Bridge Control register of this Port has no effect. + * When 1, the Port shall generate hot reset when SBR bit in Bridge + * Control gets set to 1. + */ + if (reg & CXL_DVSEC_PORT_CONTROL_UNMASK_SBR) + return false; + + return true; +} + static int pci_reset_bus_function(struct pci_dev *dev, bool probe) { + struct pci_dev *bridge = pci_upstream_bridge(dev); int rc; + /* If it's a CXL port and the SBR control is masked, fail the SBR */ + if (pci_is_cxl(dev) && bridge && is_cxl_port_sbr_masked(bridge)) { + if (probe) + return 0; + + return -ENOTTY; + } + rc = pci_dev_reset_slot_function(dev, probe); if (rc != -ENOTTY) return rc; diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index a39193213ff2..5f2c66987299 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -1148,4 +1148,11 @@ #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_PROTOCOL 0x00ff0000 #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_NEXT_INDEX 0xff000000 +/* Compute Express Link (CXL) */ +#define PCI_DVSEC_VENDOR_ID_CXL 0x1e98 +#define CXL_DVSEC_PCIE_DEVICE 0 +#define CXL_DVSEC_PCIE_PORT 3 +#define CXL_DVSEC_PORT_CONTROL 0x0c +#define CXL_DVSEC_PORT_CONTROL_UNMASK_SBR 0x00000001 + #endif /* LINUX_PCI_REGS_H */ From patchwork Mon Mar 25 23:58:02 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Jiang X-Patchwork-Id: 13603200 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 044DD339BF; Mon, 25 Mar 2024 23:59:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1711411159; cv=none; b=s7XU3ra7IHAECAgocxVW8bQoIivd4lvvhsKrHMQCRNVaQMECZ48pvOvPva6JPQuPNKTMpCmlrGRWidu+TtH9J/R9RFXejO+rwC4tlBfk89JqAiWMKGQ9nLmRQ7S9ekMOzVxXZblnC+4Gie4p+IiJPYgTk5lASdv4sXgvVkJQYtg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1711411159; c=relaxed/simple; bh=0Rr0vRNDhPrYuFzBtu/cR/zTP20PLb4+1GfnCGCP0hE=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=gJ0ytB8jWIf1wR8WibRJLcFMMLw5rLVO/VqqkAMa9GFlObpCyCHILuYVWgrFOdOhhm/svjh6ncYZrtRt42zK/uDVWXJDWjHpj8NbRkpHM4esXI0OKfPGkIdVJSPsmc2am/EZghTIV0bgSoBUm2dO6lqUmFAYfPcsDLqVYmjYTXA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id B7232C433F1; Mon, 25 Mar 2024 23:59:18 +0000 (UTC) From: Dave Jiang To: linux-cxl@vger.kernel.org, linux-pci@vger.kernel.org Cc: dan.j.williams@intel.com, ira.weiny@intel.com, vishal.l.verma@intel.com, alison.schofield@intel.com, Jonathan.Cameron@huawei.com, dave@stgolabs.net, bhelgaas@google.com, lukas@wunner.de Subject: [PATCH v2 2/3] PCI: Create new reset method to force SBR for CXL Date: Mon, 25 Mar 2024 16:58:02 -0700 Message-ID: <20240325235914.1897647-3-dave.jiang@intel.com> X-Mailer: git-send-email 2.44.0 In-Reply-To: <20240325235914.1897647-1-dave.jiang@intel.com> References: <20240325235914.1897647-1-dave.jiang@intel.com> Precedence: bulk X-Mailing-List: linux-cxl@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 CXL spec r3.1 8.1.5.2 By default Secondary Bus Reset (SBR) is masked for CXL ports. Introduce a new PCI reset method "cxl_bus_force" to force SBR on CXL ports by setting the unmask SBR bit in the CXL DVSEC port control register before performing the bus reset and restore the original value of the bit post reset. The new reset method allows the user to intentionally perform SBR on a CXL device without needing to set the "Unmask SBR" bit via a user tool. Signed-off-by: Dave Jiang --- v2: - Use pci_upstream_bridge() instead of dev->bus->self. - Return -ENOTTY as error for reset function --- drivers/pci/pci.c | 52 +++++++++++++++++++++++++++++++++++++++++++-- include/linux/pci.h | 2 +- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 259e5d6538bb..cbcad8f0880d 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4933,6 +4933,12 @@ static bool pci_is_cxl(struct pci_dev *dev) CXL_DVSEC_PCIE_DEVICE); } +static int cxl_port_dvsec(struct pci_dev *dev) +{ + return pci_find_dvsec_capability(dev, PCI_DVSEC_VENDOR_ID_CXL, + CXL_DVSEC_PCIE_PORT); +} + static bool is_cxl_port_sbr_masked(struct pci_dev *dev) { int dvsec; @@ -4942,8 +4948,7 @@ static bool is_cxl_port_sbr_masked(struct pci_dev *dev) /* * No DVSEC found, must not be CXL port. */ - dvsec = pci_find_dvsec_capability(dev, PCI_DVSEC_VENDOR_ID_CXL, - CXL_DVSEC_PCIE_PORT); + dvsec = cxl_port_dvsec(dev); if (!dvsec) return false; @@ -4982,6 +4987,48 @@ static int pci_reset_bus_function(struct pci_dev *dev, bool probe) return pci_parent_bus_reset(dev, probe); } +static int cxl_reset_bus_function(struct pci_dev *dev, bool probe) +{ + struct pci_dev *bridge; + int dvsec; + int rc; + u16 reg, val; + + if (!pci_is_cxl(dev)) + return -ENOTTY; + + bridge = pci_upstream_bridge(dev); + if (!bridge) + return -ENOTTY; + + dvsec = cxl_port_dvsec(bridge); + if (!dvsec) + return -ENOTTY; + + if (probe) + return 0; + + rc = pci_read_config_word(bridge, dvsec + CXL_DVSEC_PORT_CONTROL, + ®); + if (rc) + return -ENOTTY; + + if (!(reg & CXL_DVSEC_PORT_CONTROL_UNMASK_SBR)) { + val = reg | CXL_DVSEC_PORT_CONTROL_UNMASK_SBR; + pci_write_config_word(bridge, + dvsec + CXL_DVSEC_PORT_CONTROL, val); + } else { + val = reg; + } + + rc = pci_reset_bus_function(dev, probe); + + if (reg != val) + pci_write_config_word(bridge, dvsec + CXL_DVSEC_PORT_CONTROL, reg); + + return rc; +} + void pci_dev_lock(struct pci_dev *dev) { /* block PM suspend, driver probe, etc. */ @@ -5066,6 +5113,7 @@ static const struct pci_reset_fn_method pci_reset_fn_methods[] = { { pci_af_flr, .name = "af_flr" }, { pci_pm_reset, .name = "pm" }, { pci_reset_bus_function, .name = "bus" }, + { cxl_reset_bus_function, .name = "cxl_bus_force" }, }; static ssize_t reset_method_show(struct device *dev, diff --git a/include/linux/pci.h b/include/linux/pci.h index 16493426a04f..235f37715a43 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -51,7 +51,7 @@ PCI_STATUS_PARITY) /* Number of reset methods used in pci_reset_fn_methods array in pci.c */ -#define PCI_NUM_RESET_METHODS 7 +#define PCI_NUM_RESET_METHODS 8 #define PCI_RESET_PROBE true #define PCI_RESET_DO_RESET false From patchwork Mon Mar 25 23:58:03 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Jiang X-Patchwork-Id: 13603201 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 65A6184D2A; Mon, 25 Mar 2024 23:59:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1711411160; cv=none; b=rU3z4vWvewahWP1O2iL9UDmrlrHkd31JG91PEEKR0ZEBrWtqzu4s46Sqn5NuJpTqlQ8FmeNTLezZ9y2rvQoIrSIDMsdk2j3pg9jP00Yed5oiHZaK5aOULwa5jZMjvA5H9t5d0PzvMglUQoXmaJMSdQbtz6PQ0rEO91BnIlAyH/g= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1711411160; c=relaxed/simple; bh=xAxMCxwjjX9S/PqJDyq6Rp0PSzq3bF+f+gEjXcKqCEs=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Yd8wWk8Fpr/nN8jlx0PDmeAH4HK1VduJzqkV5KsKM/rrh17TCmuTDXQ1oWyJkbkziIGzCOygAXdN6Ci6wk/r0L32hVYywgOa8ooRMgOLwkw37ouBly3ieJGi/565UTRMQHboLiLNFIO1ZM359c4U9ka7Jp2fgSr/Zev+L13Qzwk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id E1096C433F1; Mon, 25 Mar 2024 23:59:19 +0000 (UTC) From: Dave Jiang To: linux-cxl@vger.kernel.org, linux-pci@vger.kernel.org Cc: dan.j.williams@intel.com, ira.weiny@intel.com, vishal.l.verma@intel.com, alison.schofield@intel.com, Jonathan.Cameron@huawei.com, dave@stgolabs.net, bhelgaas@google.com, lukas@wunner.de Subject: [PATCH v2 3/3] cxl: Add post reset warning if reset is detected as Secondary Bus Reset (SBR) Date: Mon, 25 Mar 2024 16:58:03 -0700 Message-ID: <20240325235914.1897647-4-dave.jiang@intel.com> X-Mailer: git-send-email 2.44.0 In-Reply-To: <20240325235914.1897647-1-dave.jiang@intel.com> References: <20240325235914.1897647-1-dave.jiang@intel.com> Precedence: bulk X-Mailing-List: linux-cxl@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 SBR is equivalent to a device been hot removed and inserted again. Doing a SBR on a CXL type 3 device is problematic if the exported device memory is part of system memory that cannot be offlined. The event is equivalent to violently ripping out that range of memory from the kernel. While the hardware requires the "Unmask SBR" bit set in the Port Control Extensions register and the kernel currently does not unmask it, user can unmask this bit via setpci or similar tool. The driver does not have a way to detect whether a reset coming from the PCI subsystem is a Function Level Reset (FLR) or SBR. The only way to detect is to note if a decoder is marked as enabled in software but the decoder control register indicates it's not committed. A helper function is added to find discrepancy between the decoder software state versus the hardware register state. Suggested-by: Dan Williams Signed-off-by: Dave Jiang --- v2: - Fix typo in commit log --- drivers/cxl/core/port.c | 30 ++++++++++++++++++++++++++++++ drivers/cxl/cxl.h | 2 ++ drivers/cxl/pci.c | 19 +++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index 2b0cab556072..e33d047f992b 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -2222,6 +2222,36 @@ int cxl_endpoint_get_perf_coordinates(struct cxl_port *port, } EXPORT_SYMBOL_NS_GPL(cxl_endpoint_get_perf_coordinates, CXL); +static int decoder_hw_mismatch(struct device *dev, void *data) +{ + struct cxl_endpoint_decoder *cxled; + struct cxl_port *port = data; + struct cxl_decoder *cxld; + struct cxl_hdm *cxlhdm; + void __iomem *hdm; + u32 ctrl; + + if (!is_endpoint_decoder(dev)) + return 0; + + cxled = to_cxl_endpoint_decoder(dev); + if ((cxled->cxld.flags & CXL_DECODER_F_ENABLE) == 0) + return 0; + + cxld = &cxled->cxld; + cxlhdm = dev_get_drvdata(&port->dev); + hdm = cxlhdm->regs.hdm_decoder; + ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id)); + + return !FIELD_GET(CXL_HDM_DECODER0_CTRL_COMMITTED, ctrl); +} + +bool cxl_endpoint_decoder_reset_detected(struct cxl_port *port) +{ + return device_for_each_child(&port->dev, port, decoder_hw_mismatch); +} +EXPORT_SYMBOL_NS_GPL(cxl_endpoint_decoder_reset_detected, CXL); + /* for user tooling to ensure port disable work has completed */ static ssize_t flush_store(const struct bus_type *bus, const char *buf, size_t count) { diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 534e25e2f0a4..e3c237c50b59 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -895,6 +895,8 @@ void cxl_coordinates_combine(struct access_coordinate *out, struct access_coordinate *c1, struct access_coordinate *c2); +bool cxl_endpoint_decoder_reset_detected(struct cxl_port *port); + /* * Unit test builds overrides this to __weak, find the 'strong' version * of these symbols in tools/testing/cxl/. diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index 13450e75f5eb..78457542aeec 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -957,11 +957,30 @@ static void cxl_error_resume(struct pci_dev *pdev) dev->driver ? "successful" : "failed"); } +static void cxl_reset_done(struct pci_dev *pdev) +{ + struct cxl_dev_state *cxlds = pci_get_drvdata(pdev); + struct cxl_memdev *cxlmd = cxlds->cxlmd; + struct device *dev = &pdev->dev; + + /* + * FLR does not expect to touch the HDM decoders and related registers. + * SBR however will wipe all device configurations. + * Issue warning if there was active decoder before reset that no + * longer exists. + */ + if (cxl_endpoint_decoder_reset_detected(cxlmd->endpoint)) { + dev_warn(dev, "SBR happened without memory regions removal.\n"); + dev_warn(dev, "System may be unstable if regions hosted system memory.\n"); + } +} + static const struct pci_error_handlers cxl_error_handlers = { .error_detected = cxl_error_detected, .slot_reset = cxl_slot_reset, .resume = cxl_error_resume, .cor_error_detected = cxl_cor_error_detected, + .reset_done = cxl_reset_done, }; static struct pci_driver cxl_pci_driver = {