From patchwork Thu May 2 16:57:30 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Jiang X-Patchwork-Id: 13651912 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 E2C7916C875; Thu, 2 May 2024 16:58:54 +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=1714669134; cv=none; b=SO8ADSHOGkO6yKaGh/Cd8PwLN1/MVrOwGzi1lq9/fvnw6r9cNpzn1Rqcd+9HO7O6bJo+2gHx9toZ42+syp0mH9zhVbgvI9+5C0QfIJogzecUw20R/JHNe8RyO0J+AcHwGP/bIg5K8dudzkXR3ERVT1PvlEtZfJJEQfXzGl5IUpA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714669134; c=relaxed/simple; bh=Z031GXMbUsB3e7fIFQi3am6h1ECiZzo9fWUhh+4xnzM=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=iYvRDmt5orNoctZzgga4JoIQZ+lJ60T5cOa68j9xuiU1iEmSYSS6F3nP/4mr1+pgxZbEUIGKHwJKdnFOI92emWPikJrc39QF0blob+b4ZYgzM5xu3v4PLoH64zIXTOgcFmPUDkn5Ddw8pLKg/4pG9SjsdjdfaszTyNhTBBGyoTc= 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 B18B0C4AF19; Thu, 2 May 2024 16:58:54 +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, Bjorn Helgaas , Kuppuswamy Sathyanarayanan Subject: [PATCH v6 1/5] PCI/cxl: Move PCI CXL vendor Id to a common location from CXL subsystem Date: Thu, 2 May 2024 09:57:30 -0700 Message-ID: <20240502165851.1948523-2-dave.jiang@intel.com> X-Mailer: git-send-email 2.44.0 In-Reply-To: <20240502165851.1948523-1-dave.jiang@intel.com> References: <20240502165851.1948523-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 Move PCI_DVSEC_VENDOR_ID_CXL in CXL private code to PCI_VENDOR_ID_CXL in pci_ids.h in order to be utilized in PCI subsystem. When uplevelling PCI_DVSEC_VENDOR_ID_CXL to a common locatoin Bjorn suggested making it a proper PCI_VENDOR_ID_* define in include/linux/pci_ids.h. While it is not in the PCI IDs database it is a reserved value and Linux treats it as a 'vendor id' for all intents and purposes [1]. Link: https://lore.kernel.org/linux-cxl/20240402172323.GA1818777@bhelgaas/ Suggested-by: Bjorn Helgaas Reviewed-by: Kuppuswamy Sathyanarayanan Reviewed-by: Dan Williams Signed-off-by: Dave Jiang --- drivers/cxl/core/pci.c | 6 +++--- drivers/cxl/core/regs.c | 2 +- drivers/cxl/cxlpci.h | 1 - drivers/cxl/pci.c | 2 +- drivers/perf/cxl_pmu.c | 2 +- include/linux/pci_ids.h | 2 ++ 6 files changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index 0df09bd79408..c496a9710d62 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -525,7 +525,7 @@ static int cxl_cdat_get_length(struct device *dev, __le32 response[2]; int rc; - rc = pci_doe(doe_mb, PCI_DVSEC_VENDOR_ID_CXL, + rc = pci_doe(doe_mb, PCI_VENDOR_ID_CXL, CXL_DOE_PROTOCOL_TABLE_ACCESS, &request, sizeof(request), &response, sizeof(response)); @@ -555,7 +555,7 @@ static int cxl_cdat_read_table(struct device *dev, __le32 request = CDAT_DOE_REQ(entry_handle); int rc; - rc = pci_doe(doe_mb, PCI_DVSEC_VENDOR_ID_CXL, + rc = pci_doe(doe_mb, PCI_VENDOR_ID_CXL, CXL_DOE_PROTOCOL_TABLE_ACCESS, &request, sizeof(request), rsp, sizeof(*rsp) + remaining); @@ -640,7 +640,7 @@ void read_cdat_data(struct cxl_port *port) if (!pdev) return; - doe_mb = pci_find_doe_mailbox(pdev, PCI_DVSEC_VENDOR_ID_CXL, + doe_mb = pci_find_doe_mailbox(pdev, PCI_VENDOR_ID_CXL, CXL_DOE_PROTOCOL_TABLE_ACCESS); if (!doe_mb) { dev_dbg(dev, "No CDAT mailbox\n"); diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c index 3c42f984eeaf..e1082e749c69 100644 --- a/drivers/cxl/core/regs.c +++ b/drivers/cxl/core/regs.c @@ -314,7 +314,7 @@ int cxl_find_regblock_instance(struct pci_dev *pdev, enum cxl_regloc_type type, .resource = CXL_RESOURCE_NONE, }; - regloc = pci_find_dvsec_capability(pdev, PCI_DVSEC_VENDOR_ID_CXL, + regloc = pci_find_dvsec_capability(pdev, PCI_VENDOR_ID_CXL, CXL_DVSEC_REG_LOCATOR); if (!regloc) return -ENXIO; diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h index 93992a1c8eec..4da07727ab9c 100644 --- a/drivers/cxl/cxlpci.h +++ b/drivers/cxl/cxlpci.h @@ -13,7 +13,6 @@ * "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 diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index 2ff361e756d6..110478573296 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -817,7 +817,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) cxlds->rcd = is_cxl_restricted(pdev); cxlds->serial = pci_get_dsn(pdev); cxlds->cxl_dvsec = pci_find_dvsec_capability( - pdev, PCI_DVSEC_VENDOR_ID_CXL, CXL_DVSEC_PCIE_DEVICE); + pdev, PCI_VENDOR_ID_CXL, CXL_DVSEC_PCIE_DEVICE); if (!cxlds->cxl_dvsec) dev_warn(&pdev->dev, "Device DVSEC not present, skip CXL.mem init\n"); diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c index 308c9969642e..a1b742b1a735 100644 --- a/drivers/perf/cxl_pmu.c +++ b/drivers/perf/cxl_pmu.c @@ -345,7 +345,7 @@ static ssize_t cxl_pmu_event_sysfs_show(struct device *dev, /* For CXL spec defined events */ #define CXL_PMU_EVENT_CXL_ATTR(_name, _gid, _msk) \ - CXL_PMU_EVENT_ATTR(_name, PCI_DVSEC_VENDOR_ID_CXL, _gid, _msk) + CXL_PMU_EVENT_ATTR(_name, PCI_VENDOR_ID_CXL, _gid, _msk) static struct attribute *cxl_pmu_event_attrs[] = { CXL_PMU_EVENT_CXL_ATTR(clock_ticks, CXL_PMU_GID_CLOCK_TICKS, BIT(0)), diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index a0c75e467df3..7dfbf6d96b3d 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -2607,6 +2607,8 @@ #define PCI_VENDOR_ID_ALIBABA 0x1ded +#define PCI_VENDOR_ID_CXL 0x1e98 + #define PCI_VENDOR_ID_TEHUTI 0x1fc9 #define PCI_DEVICE_ID_TEHUTI_3009 0x3009 #define PCI_DEVICE_ID_TEHUTI_3010 0x3010 From patchwork Thu May 2 16:57:31 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Jiang X-Patchwork-Id: 13651913 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 7536A16F91B; Thu, 2 May 2024 16:58:56 +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=1714669136; cv=none; b=o0m8D1wDzY8Qcm5O3aa21DGB750XDzpbM+YvkrIckFZjJtoMGCYGwFWs/unoQIzABDns2r1wEYMXvqpjz/1G6c5x0GY+sCvboP1amsAcipvGNeE9/hVToakSGNHEXbOXBewQwQ0ZU4o35eSvuSd+ol8yNhkfc3GwledGxj3yATE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714669136; c=relaxed/simple; bh=Q5sy09O8iDSYIAN3/meQJU6BSUfriC2mE8pwdT3SBRA=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=PUw20/UZCqcz7COHGuc+/L3JMvuvzY406p3oqRAsaIrIo2G1/d3lRB+fGrO4aXGuPAEjkR0NjPzgdgPniMtJk+a3fRX3+9eYSw/nTNoS+lIHp16xISbNHBLmHkybyWhiJNgqDbcbiIeagzKgqahjmibOgayLnm1/0f4jfQWRRk4= 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 EBCE3C4AF14; Thu, 2 May 2024 16:58:55 +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 v6 2/5] PCI: Add locking of upstream bridge for pci_reset_function() Date: Thu, 2 May 2024 09:57:31 -0700 Message-ID: <20240502165851.1948523-3-dave.jiang@intel.com> X-Mailer: git-send-email 2.44.0 In-Reply-To: <20240502165851.1948523-1-dave.jiang@intel.com> References: <20240502165851.1948523-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 Fix a long standing locking gap for missing pci_cfg_access_lock() while manipulating bridge reset registers and configuration during pci_reset_bus_function(). Add calling of pci_dev_lock() against the bridge device before locking the device. The locking is conditional depending on whether the trigger device has an upstream bridge. If the device is a root port then there would be no upstream bridge and thus the locking of the bridge is unnecessary. As part of calling pci_dev_lock(), pci_cfg_access_lock() happens and blocks the writing of PCI config space by user space. Add lockdep assertion via pci_dev->cfg_access_lock in order to verify pci_dev->block_cfg_access is set. Co-developed-by: Dan Williams Signed-off-by: Dan Williams Signed-off-by: Dave Jiang --- drivers/pci/access.c | 4 ++++ drivers/pci/pci.c | 13 +++++++++++++ drivers/pci/probe.c | 3 +++ include/linux/lockdep.h | 5 +++++ include/linux/pci.h | 2 ++ 5 files changed, 27 insertions(+) diff --git a/drivers/pci/access.c b/drivers/pci/access.c index 6449056b57dd..36f10c7f9ef5 100644 --- a/drivers/pci/access.c +++ b/drivers/pci/access.c @@ -275,6 +275,8 @@ void pci_cfg_access_lock(struct pci_dev *dev) { might_sleep(); + lock_map_acquire(&dev->cfg_access_lock); + raw_spin_lock_irq(&pci_lock); if (dev->block_cfg_access) pci_wait_cfg(dev); @@ -329,6 +331,8 @@ void pci_cfg_access_unlock(struct pci_dev *dev) raw_spin_unlock_irqrestore(&pci_lock, flags); wake_up_all(&pci_cfg_wait); + + lock_map_release(&dev->cfg_access_lock); } EXPORT_SYMBOL_GPL(pci_cfg_access_unlock); diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index e5f243dd4288..482372f5d268 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4879,6 +4879,7 @@ void __weak pcibios_reset_secondary_bus(struct pci_dev *dev) */ int pci_bridge_secondary_bus_reset(struct pci_dev *dev) { + lock_map_assert_held(&dev->cfg_access_lock); pcibios_reset_secondary_bus(dev); return pci_bridge_wait_for_secondary_bus(dev, "bus reset"); @@ -5245,11 +5246,20 @@ void pci_init_reset_methods(struct pci_dev *dev) */ int pci_reset_function(struct pci_dev *dev) { + struct pci_dev *bridge; int rc; if (!pci_reset_supported(dev)) return -ENOTTY; + bridge = pci_upstream_bridge(dev); + /* + * If there's no upstream bridge, then no locking is needed since there is no + * upstream bridge configuration to hold consistent. + */ + if (bridge) + pci_dev_lock(bridge); + pci_dev_lock(dev); pci_dev_save_and_disable(dev); @@ -5258,6 +5268,9 @@ int pci_reset_function(struct pci_dev *dev) pci_dev_restore(dev); pci_dev_unlock(dev); + if (bridge) + pci_dev_unlock(bridge); + return rc; } EXPORT_SYMBOL_GPL(pci_reset_function); diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 1325fbae2f28..a3da776bf986 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -2543,6 +2543,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus) dev->dev.dma_mask = &dev->dma_mask; dev->dev.dma_parms = &dev->dma_parms; dev->dev.coherent_dma_mask = 0xffffffffull; + lockdep_register_key(&dev->cfg_access_key); + lockdep_init_map(&dev->cfg_access_lock, dev_name(&dev->dev), + &dev->cfg_access_key, 0); dma_set_max_seg_size(&dev->dev, 65536); dma_set_seg_boundary(&dev->dev, 0xffffffff); diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index 08b0d1d9d78b..5e51b0de4c4b 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -297,6 +297,9 @@ extern void lock_unpin_lock(struct lockdep_map *lock, struct pin_cookie); .wait_type_inner = _wait_type, \ .lock_type = LD_LOCK_WAIT_OVERRIDE, } +#define lock_map_assert_held(l) \ + lockdep_assert(lock_is_held(l) != LOCK_STATE_NOT_HELD) + #else /* !CONFIG_LOCKDEP */ static inline void lockdep_init_task(struct task_struct *task) @@ -388,6 +391,8 @@ extern int lockdep_is_held(const void *); #define DEFINE_WAIT_OVERRIDE_MAP(_name, _wait_type) \ struct lockdep_map __maybe_unused _name = {} +#define lock_map_assert_held(l) do { (void)(l); } while (0) + #endif /* !LOCKDEP */ #ifdef CONFIG_PROVE_LOCKING diff --git a/include/linux/pci.h b/include/linux/pci.h index 16493426a04f..e4e7b175af54 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -413,6 +413,8 @@ struct pci_dev { struct resource driver_exclusive_resource; /* driver exclusive resource ranges */ bool match_driver; /* Skip attaching driver */ + struct lock_class_key cfg_access_key; + struct lockdep_map cfg_access_lock; unsigned int transparent:1; /* Subtractive decode bridge */ unsigned int io_window:1; /* Bridge has I/O window */ From patchwork Thu May 2 16:57:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Jiang X-Patchwork-Id: 13651914 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 65F9316D327; Thu, 2 May 2024 16:58:57 +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=1714669137; cv=none; b=UrwfCTRHUKcYI+YEiBREtQ5hx0g7zWF+vPsaXQEtgUYiGc0IctypqABzS1wgQfhy2y2NdIjgRC/56o+Mi/ytSHZY2mc158QNjVl1Cjz6kMjrvAGqUsRnf9jIIUutsjbbmHYZVGAiiUKdRc/+piK73P0iE7ePeTVGkecHS0CGWXY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714669137; c=relaxed/simple; bh=UUspGaHr11cDIRq6tNNiyeKC0U4Ywq9+MmlcVcW1PWk=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Feh9TKy6Sqyrrd7u5+B1OgEtpK+HqnBu2qLCTAuPSWg+YMABRpYCzomQtdaygx8dz/DFI7RC7eJwfVjCr/tm+oFwGWa4LzWOqsPLD3W71+5ycEJoMaB9X+6X8gn7g37c8JLlTUExg5Kk9m+YIA0UN1EqVJw9mslN2UiHCbix25g= 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 F073CC4AF14; Thu, 2 May 2024 16:58:56 +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, Kuppuswamy Sathyanarayanan Subject: [PATCH v6 3/5] PCI: Add check for CXL Secondary Bus Reset Date: Thu, 2 May 2024 09:57:32 -0700 Message-ID: <20240502165851.1948523-4-dave.jiang@intel.com> X-Mailer: git-send-email 2.44.0 In-Reply-To: <20240502165851.1948523-1-dave.jiang@intel.com> References: <20240502165851.1948523-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 (SBR) 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. Otherwise when the "Unmask SBR" bit is set to 0 (default), the bus_reset would appear to have executed successfully. However the operation is actually masked. The intention is to inform the user that SBR for the CXL device is masked and will not go through. If the "Unmask SBR" bit is set to 1, then the bus reset will execute successfully. Add the locking of the upstream bridge in pci_reset_function() to ensure the locking order of locking the bridge then locking the device. The bridge configuration needs to be consistent for a CXL device. This should not impact PCI devices. Link: https://lore.kernel.org/linux-cxl/20240220203956.GA1502351@bhelgaas/ Reviewed-by: Jonathan Cameron Reviewed-by: Kuppuswamy Sathyanarayanan Reviewed-by: Dan Williams Signed-off-by: Dave Jiang --- v6: - Split out upstream bridge locking. (Dan) --- drivers/pci/pci.c | 45 +++++++++++++++++++++++++++++++++++ include/uapi/linux/pci_regs.h | 5 ++++ 2 files changed, 50 insertions(+) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 482372f5d268..3484bf283707 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4928,10 +4928,55 @@ static int pci_dev_reset_slot_function(struct pci_dev *dev, bool probe) return pci_reset_hotplug_slot(dev->slot->hotplug, probe); } +static u16 cxl_port_dvsec(struct pci_dev *dev) +{ + return pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL, + PCI_DVSEC_CXL_PORT); +} + +static bool cxl_sbr_masked(struct pci_dev *dev) +{ + u16 dvsec, reg; + int rc; + + /* + * No DVSEC found, either is not a CXL port, or not connected in which + * case mask state is a nop (CXL r3.1 sec 9.12.3 "Enumerating CXL RPs + * and DSPs" + */ + dvsec = cxl_port_dvsec(dev); + if (!dvsec) + return false; + + rc = pci_read_config_word(dev, dvsec + PCI_DVSEC_CXL_PORT_CTL, ®); + if (rc || PCI_POSSIBLE_ERROR(reg)) + return false; + + /* + * 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 & PCI_DVSEC_CXL_PORT_CTL_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 (bridge && cxl_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..d61fa43662e3 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -1148,4 +1148,9 @@ #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_CXL_PORT 3 +#define PCI_DVSEC_CXL_PORT_CTL 0x0c +#define PCI_DVSEC_CXL_PORT_CTL_UNMASK_SBR 0x00000001 + #endif /* LINUX_PCI_REGS_H */ From patchwork Thu May 2 16:57:33 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Jiang X-Patchwork-Id: 13651915 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 941CC16D4C0; Thu, 2 May 2024 16:58:58 +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=1714669138; cv=none; b=JrzjvTPcYUwHRntc8B6d9ZVeUPVxhtdv+QRvGvhBE9jR/1yDe1gYK30bPtp97qExmvrG0O1zgTpe+y09xFyCDRFASPyWl4WKWa7YaudPbZyqWNmXf33f3NVJll3JncHwWlAXGLKPac0TSsVADqvddr41VnAvgkTed6QnWIkYYGk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714669138; c=relaxed/simple; bh=E8TSeNpSssxlD5NtqvS08OfVEpGPBjORPSCWK19j++Y=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=GsuopuQIrl8NHqDQTeDXWYm7Gh6yZzZnOutzBOpwetRmmk3L2/rMj5vUGzN7MTubHftPKoMm7ZhWSpOig/Kkd1NU+UJsY+xxvg2BX94rVsLGYNbpAECnDK3PEtg3dWJBDCUEG2ML8HK2RoPY+kzKRkdeW/QpnQUj8zOdzIrNIpI= 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 125F4C113CC; Thu, 2 May 2024 16:58:58 +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 v6 4/5] PCI: Create new reset method to force SBR for CXL Date: Thu, 2 May 2024 09:57:33 -0700 Message-ID: <20240502165851.1948523-5-dave.jiang@intel.com> X-Mailer: git-send-email 2.44.0 In-Reply-To: <20240502165851.1948523-1-dave.jiang@intel.com> References: <20240502165851.1948523-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" 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. Reviewed-by: Jonathan Cameron Reviewed-by: Dan Williams Signed-off-by: Dave Jiang --- drivers/pci/pci.c | 38 ++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 2 +- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 3484bf283707..0ec2b04dbceb 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4983,6 +4983,43 @@ 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; + u16 dvsec, reg, val; + int rc; + + 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 + PCI_DVSEC_CXL_PORT_CTL, ®); + if (rc) + return -ENOTTY; + + if (!(reg & PCI_DVSEC_CXL_PORT_CTL_UNMASK_SBR)) { + val = reg | PCI_DVSEC_CXL_PORT_CTL_UNMASK_SBR; + pci_write_config_word(bridge, dvsec + PCI_DVSEC_CXL_PORT_CTL, + val); + } else { + val = reg; + } + + rc = pci_reset_bus_function(dev, probe); + + if (reg != val) + pci_write_config_word(bridge, dvsec + PCI_DVSEC_CXL_PORT_CTL, reg); + + return rc; +} + void pci_dev_lock(struct pci_dev *dev) { /* block PM suspend, driver probe, etc. */ @@ -5067,6 +5104,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" }, }; static ssize_t reset_method_show(struct device *dev, diff --git a/include/linux/pci.h b/include/linux/pci.h index e4e7b175af54..b06c1c0ec9bd 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 Thu May 2 16:57:34 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Jiang X-Patchwork-Id: 13651916 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 97DB916FF4E; Thu, 2 May 2024 16:58:59 +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=1714669139; cv=none; b=GWjqRCjlStpnjwExKdRDmEf3+xhDpOneXnXWPaxdW8viy/cphZZwiPI0Aq6H397XY0ra6Jux42p0krDSmhbZ1s1ziu+bXN7fEnXzdW0ifrHnMCI+mzzDQbo2cSb0Re6IDVM/FjtABiediDMqvAAjOEr7RfsCE5dlVGpn0l1YslA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714669139; c=relaxed/simple; bh=bKVW0zAlC/I6CSUrA7QG6wc9a6veievHXR8yN1d6gUQ=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=LFpHZf5ccQsIs+u4XrfE4Ry891jMgQHLuJvSrEZ0DyRrfNxGTaRppHAcljaAe9rDmm5+YGenbav5XRl+FW+eIiOWxJ2RNaod1feGexh6YmcyA3ciUJFZKq2JzOaD40V20UEofMIkprDEKHRcuRlF0wq9UQ406nQrx/MNe4c/R9Y= 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 26446C113CC; Thu, 2 May 2024 16:58:59 +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 v6 5/5] cxl: Add post reset warning if reset results in loss of previously committed HDM decoders Date: Thu, 2 May 2024 09:57:34 -0700 Message-ID: <20240502165851.1948523-6-dave.jiang@intel.com> X-Mailer: git-send-email 2.44.0 In-Reply-To: <20240502165851.1948523-1-dave.jiang@intel.com> References: <20240502165851.1948523-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 Reviewed-by: Jonathan Cameron Reviewed-by: Dan Williams Signed-off-by: Dave Jiang --- drivers/cxl/core/pci.c | 29 +++++++++++++++++++++++++++++ drivers/cxl/cxl.h | 2 ++ drivers/cxl/pci.c | 22 ++++++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index c496a9710d62..8567dd11eaac 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -1045,3 +1045,32 @@ long cxl_pci_get_latency(struct pci_dev *pdev) return cxl_flit_size(pdev) * MEGA / bw; } + +static int __cxl_endpoint_decoder_reset_detected(struct device *dev, void *data) +{ + 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; + + cxld = to_cxl_decoder(dev); + if ((cxld->flags & CXL_DECODER_F_ENABLE) == 0) + return 0; + + 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, + __cxl_endpoint_decoder_reset_detected); +} +EXPORT_SYMBOL_NS_GPL(cxl_endpoint_decoder_reset_detected, CXL); diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 036d17db68e0..72fa47740768 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -891,6 +891,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 110478573296..dccd71840c5b 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -957,11 +957,33 @@ 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. + */ + guard(device)(&cxlmd->dev); + if (cxlmd->endpoint && + cxl_endpoint_decoder_reset_detected(cxlmd->endpoint)) { + dev_crit(dev, "SBR happened without memory regions removal.\n"); + dev_crit(dev, "System may be unstable if regions hosted system memory.\n"); + add_taint(TAINT_USER, LOCKDEP_STILL_OK); + } +} + 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 = {