Linux CXL
 help / color / mirror / Atom feed
From: Dave Jiang <dave.jiang@intel.com>
To: linux-cxl@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
Subject: [PATCH] cxl: Add post reset warning if the reset is detected as Secondary Bus Reset (SBR)
Date: Thu, 15 Feb 2024 16:23:07 -0700	[thread overview]
Message-ID: <20240215232307.2793530-1-dave.jiang@intel.com> (raw)

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 there are active decoders before the reset and check
if the range register memory active bit remains set after reset.

A helper function to check is added to detect if the range register memory
active bit is set. A locked helper for cxl_num_decoders_committed() is also
added to allow pci code to call the cxl_num_decoders_committed() while
holding the cxl_region_rwsem.

Add a err_handler->reset_prepare() to detect whether there are active
decoders.  Add a err_handler->reset_done() to check if there was active
memory before the reset and it is no longer active after the reset. A
warning is emitted in the case of active memory has been offlined.

Suggested-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/pci.c  | 14 ++++++++++++++
 drivers/cxl/core/port.c | 11 +++++++++++
 drivers/cxl/cxl.h       |  2 ++
 drivers/cxl/cxlmem.h    |  1 +
 drivers/cxl/pci.c       | 31 +++++++++++++++++++++++++++++++
 5 files changed, 59 insertions(+)

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 6c9c8d92f8f7..73b5cd54e761 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -322,6 +322,20 @@ static int devm_cxl_enable_hdm(struct device *host, struct cxl_hdm *cxlhdm)
 	return devm_add_action_or_reset(host, disable_hdm, cxlhdm);
 }
 
+bool cxl_dvsec_rr_active(struct device *dev, int d)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	u32 temp;
+	int rc;
+
+	rc = pci_read_config_dword(pdev, d + CXL_DVSEC_RANGE_SIZE_LOW(0), &temp);
+	if (rc)
+		return false;
+
+	return FIELD_GET(CXL_DVSEC_MEM_ACTIVE, temp);
+}
+EXPORT_SYMBOL_NS_GPL(cxl_dvsec_rr_active, CXL);
+
 int cxl_dvsec_rr_decode(struct device *dev, int d,
 			struct cxl_endpoint_dvsec_info *info)
 {
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index e59d9d37aa65..81d9f57d2e84 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -45,6 +45,17 @@ int cxl_num_decoders_committed(struct cxl_port *port)
 	return port->commit_end + 1;
 }
 
+int cxl_num_decoders_committed_locked(struct cxl_port *port)
+{
+	int decoders;
+
+	guard(rwsem_read)(&cxl_region_rwsem);
+	decoders = cxl_num_decoders_committed(port);
+
+	return decoders;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_num_decoders_committed_locked, CXL);
+
 static ssize_t devtype_show(struct device *dev, struct device_attribute *attr,
 			    char *buf)
 {
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index b6017c0c57b4..530c7e693096 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -720,6 +720,7 @@ static inline bool is_cxl_root(struct cxl_port *port)
 }
 
 int cxl_num_decoders_committed(struct cxl_port *port);
+int cxl_num_decoders_committed_locked(struct cxl_port *port);
 bool is_cxl_port(const struct device *dev);
 struct cxl_port *to_cxl_port(const struct device *dev);
 struct pci_bus;
@@ -800,6 +801,7 @@ int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
 int devm_cxl_add_passthrough_decoder(struct cxl_port *port);
 int cxl_dvsec_rr_decode(struct device *dev, int dvsec,
 			struct cxl_endpoint_dvsec_info *info);
+bool cxl_dvsec_rr_active(struct device *dev, int d);
 
 bool is_cxl_region(struct device *dev);
 
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 5303d6942b88..9f1814005322 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -440,6 +440,7 @@ struct cxl_dev_state {
 	struct resource ram_res;
 	u64 serial;
 	enum cxl_devtype type;
+	bool active_rr_prereset;
 };
 
 /**
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 233e7c42c161..5a5fda7134f6 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -957,11 +957,42 @@ static void cxl_error_resume(struct pci_dev *pdev)
 		 dev->driver ? "successful" : "failed");
 }
 
+static void cxl_reset_prepare(struct pci_dev *pdev)
+{
+	struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
+	struct cxl_memdev *cxlmd = cxlds->cxlmd;
+
+	if (cxl_num_decoders_committed_locked(cxlmd->endpoint))
+		cxlds->active_rr_prereset = true;
+}
+
+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 = &cxlmd->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 configuration before reset that no
+	 * longer exists.
+	 */
+	if (cxlds->active_rr_prereset &&
+	    !cxl_dvsec_rr_active(&pdev->dev, cxlds->cxl_dvsec)) {
+		dev_warn(dev, "SBR happened without memory regions removal.\n");
+		dev_warn(dev, "System may be unstable if regions hosted system memory.\n");
+	}
+	cxlds->active_rr_prereset = false;
+}
+
 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_prepare	= cxl_reset_prepare,
+	.reset_done	= cxl_reset_done,
 };
 
 static struct pci_driver cxl_pci_driver = {
-- 
2.43.0


             reply	other threads:[~2024-02-15 23:23 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-15 23:23 Dave Jiang [this message]
2024-02-18 19:36 ` [PATCH] cxl: Add post reset warning if the reset is detected as Secondary Bus Reset (SBR) Ira Weiny
2024-02-19 14:20 ` Jonathan Cameron
2024-02-20 18:20   ` Dan Williams
2024-02-21 16:35     ` Dave Jiang
2024-02-21 19:45       ` Dan Williams
2024-02-20 20:39   ` Bjorn Helgaas
2024-02-20 21:00     ` Dan Williams

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=20240215232307.2793530-1-dave.jiang@intel.com \
    --to=dave.jiang@intel.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=alison.schofield@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave@stgolabs.net \
    --cc=ira.weiny@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=vishal.l.verma@intel.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox