diff mbox series

[v6,03/10] cxl/memdev: Warn of poison inject or clear to a mapped region

Message ID f911ca5277c9d0f9757b72d7e6842871bfff4fa2.1681874357.git.alison.schofield@intel.com
State Accepted
Commit 0a105ab28a4de44eb738ce64e9ac74946aa5133b
Headers show
Series cxl: CXL Inject & Clear Poison | expand

Commit Message

Alison Schofield April 19, 2023, 3:26 a.m. UTC 
From: Alison Schofield <alison.schofield@intel.com>

Inject and clear poison capabilities and intended for debug usage only.
In order to be useful in debug environments, the driver needs to allow
inject and clear operations on DPAs mapped in regions.

dev_warn_once() when either operation occurs.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 drivers/cxl/core/memdev.c | 58 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

Comments

Jonathan Cameron April 23, 2023, 3:35 p.m. UTC | #1 
On Tue, 18 Apr 2023 20:26:27 -0700
alison.schofield@intel.com wrote:

> From: Alison Schofield <alison.schofield@intel.com>
> 
> Inject and clear poison capabilities and intended for debug usage only.
> In order to be useful in debug environments, the driver needs to allow
> inject and clear operations on DPAs mapped in regions.
> 
> dev_warn_once() when either operation occurs.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

The warn for the clear says it's injecting poison.  Other than that.
(I wondered why I got two copies of a dev_warn_once() message in my
test kernel log).

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/core/memdev.c | 58 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 3b48b6eec007..23ac8d322b37 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -160,6 +160,50 @@ int cxl_trigger_poison_list(struct cxl_memdev *cxlmd)
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_trigger_poison_list, CXL);
>  
> +struct cxl_dpa_to_region_context {
> +	struct cxl_region *cxlr;
> +	u64 dpa;
> +};
> +
> +static int __cxl_dpa_to_region(struct device *dev, void *arg)
> +{
> +	struct cxl_dpa_to_region_context *ctx = arg;
> +	struct cxl_endpoint_decoder *cxled;
> +	u64 dpa = ctx->dpa;
> +
> +	if (!is_endpoint_decoder(dev))
> +		return 0;
> +
> +	cxled = to_cxl_endpoint_decoder(dev);
> +	if (!cxled->dpa_res || !resource_size(cxled->dpa_res))
> +		return 0;
> +
> +	if (dpa > cxled->dpa_res->end || dpa < cxled->dpa_res->start)
> +		return 0;
> +
> +	dev_dbg(dev, "dpa:0x%llx mapped in region:%s\n", dpa,
> +		dev_name(&cxled->cxld.region->dev));
> +
> +	ctx->cxlr = cxled->cxld.region;
> +
> +	return 1;
> +}
> +
> +static struct cxl_region *cxl_dpa_to_region(struct cxl_memdev *cxlmd, u64 dpa)
> +{
> +	struct cxl_dpa_to_region_context ctx;
> +	struct cxl_port *port;
> +
> +	ctx = (struct cxl_dpa_to_region_context) {
> +		.dpa = dpa,
> +	};
> +	port = dev_get_drvdata(&cxlmd->dev);
> +	if (port && is_cxl_endpoint(port) && port->commit_end != -1)
> +		device_for_each_child(&port->dev, &ctx, __cxl_dpa_to_region);
> +
> +	return ctx.cxlr;
> +}
> +
>  static int cxl_validate_poison_dpa(struct cxl_memdev *cxlmd, u64 dpa)
>  {
>  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> @@ -189,6 +233,7 @@ int cxl_inject_poison(struct cxl_memdev *cxlmd, u64 dpa)
>  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  	struct cxl_mbox_inject_poison inject;
>  	struct cxl_mbox_cmd mbox_cmd;
> +	struct cxl_region *cxlr;
>  	int rc;
>  
>  	if (!IS_ENABLED(CONFIG_DEBUG_FS))
> @@ -209,6 +254,13 @@ int cxl_inject_poison(struct cxl_memdev *cxlmd, u64 dpa)
>  		.payload_in = &inject,
>  	};
>  	rc = cxl_internal_send_cmd(cxlds, &mbox_cmd);
> +	if (rc)
> +		goto out;
> +
> +	cxlr = cxl_dpa_to_region(cxlmd, dpa);
> +	if (cxlr)
> +		dev_warn_once(cxlds->dev, "poison inject dpa:0x%llx region: %s\n",
> +			      dpa, dev_name(&cxlr->dev));
>  out:
>  	up_read(&cxl_dpa_rwsem);
>  
> @@ -221,6 +273,7 @@ int cxl_clear_poison(struct cxl_memdev *cxlmd, u64 dpa)
>  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  	struct cxl_mbox_clear_poison clear;
>  	struct cxl_mbox_cmd mbox_cmd;
> +	struct cxl_region *cxlr;
>  	int rc;
>  
>  	if (!IS_ENABLED(CONFIG_DEBUG_FS))
> @@ -252,6 +305,11 @@ int cxl_clear_poison(struct cxl_memdev *cxlmd, u64 dpa)
>  	rc = cxl_internal_send_cmd(cxlds, &mbox_cmd);
>  	if (rc)
>  		goto out;
> +
> +	cxlr = cxl_dpa_to_region(cxlmd, dpa);
> +	if (cxlr)
> +		dev_warn_once(cxlds->dev, "poison inject dpa:0x%llx region: %s\n",

Ah. This had me confused. I got two warnings from a dev_warn_once.

Should be mentioning clearing for this one not injecting ;)

> +			      dpa, dev_name(&cxlr->dev));
>  out:
>  	up_read(&cxl_dpa_rwsem);
>
Dan Williams April 23, 2023, 6:36 p.m. UTC | #2 
Jonathan Cameron wrote:
> On Tue, 18 Apr 2023 20:26:27 -0700
> alison.schofield@intel.com wrote:
> 
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > Inject and clear poison capabilities and intended for debug usage only.
> > In order to be useful in debug environments, the driver needs to allow
> > inject and clear operations on DPAs mapped in regions.
> > 
> > dev_warn_once() when either operation occurs.
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> 
> The warn for the clear says it's injecting poison.  Other than that.
> (I wondered why I got two copies of a dev_warn_once() message in my
> test kernel log).
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
[..]
> > @@ -252,6 +305,11 @@ int cxl_clear_poison(struct cxl_memdev *cxlmd, u64 dpa)
> >  	rc = cxl_internal_send_cmd(cxlds, &mbox_cmd);
> >  	if (rc)
> >  		goto out;
> > +
> > +	cxlr = cxl_dpa_to_region(cxlmd, dpa);
> > +	if (cxlr)
> > +		dev_warn_once(cxlds->dev, "poison inject dpa:0x%llx region: %s\n",
> 
> Ah. This had me confused. I got two warnings from a dev_warn_once.
> 
> Should be mentioning clearing for this one not injecting ;)

Will fix that up on applying. I also notice that it is spelling out "0x"
when "%#" will do for the number format.
diff mbox series

Patch

diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 3b48b6eec007..23ac8d322b37 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -160,6 +160,50 @@  int cxl_trigger_poison_list(struct cxl_memdev *cxlmd)
 }
 EXPORT_SYMBOL_NS_GPL(cxl_trigger_poison_list, CXL);
 
+struct cxl_dpa_to_region_context {
+	struct cxl_region *cxlr;
+	u64 dpa;
+};
+
+static int __cxl_dpa_to_region(struct device *dev, void *arg)
+{
+	struct cxl_dpa_to_region_context *ctx = arg;
+	struct cxl_endpoint_decoder *cxled;
+	u64 dpa = ctx->dpa;
+
+	if (!is_endpoint_decoder(dev))
+		return 0;
+
+	cxled = to_cxl_endpoint_decoder(dev);
+	if (!cxled->dpa_res || !resource_size(cxled->dpa_res))
+		return 0;
+
+	if (dpa > cxled->dpa_res->end || dpa < cxled->dpa_res->start)
+		return 0;
+
+	dev_dbg(dev, "dpa:0x%llx mapped in region:%s\n", dpa,
+		dev_name(&cxled->cxld.region->dev));
+
+	ctx->cxlr = cxled->cxld.region;
+
+	return 1;
+}
+
+static struct cxl_region *cxl_dpa_to_region(struct cxl_memdev *cxlmd, u64 dpa)
+{
+	struct cxl_dpa_to_region_context ctx;
+	struct cxl_port *port;
+
+	ctx = (struct cxl_dpa_to_region_context) {
+		.dpa = dpa,
+	};
+	port = dev_get_drvdata(&cxlmd->dev);
+	if (port && is_cxl_endpoint(port) && port->commit_end != -1)
+		device_for_each_child(&port->dev, &ctx, __cxl_dpa_to_region);
+
+	return ctx.cxlr;
+}
+
 static int cxl_validate_poison_dpa(struct cxl_memdev *cxlmd, u64 dpa)
 {
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
@@ -189,6 +233,7 @@  int cxl_inject_poison(struct cxl_memdev *cxlmd, u64 dpa)
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 	struct cxl_mbox_inject_poison inject;
 	struct cxl_mbox_cmd mbox_cmd;
+	struct cxl_region *cxlr;
 	int rc;
 
 	if (!IS_ENABLED(CONFIG_DEBUG_FS))
@@ -209,6 +254,13 @@  int cxl_inject_poison(struct cxl_memdev *cxlmd, u64 dpa)
 		.payload_in = &inject,
 	};
 	rc = cxl_internal_send_cmd(cxlds, &mbox_cmd);
+	if (rc)
+		goto out;
+
+	cxlr = cxl_dpa_to_region(cxlmd, dpa);
+	if (cxlr)
+		dev_warn_once(cxlds->dev, "poison inject dpa:0x%llx region: %s\n",
+			      dpa, dev_name(&cxlr->dev));
 out:
 	up_read(&cxl_dpa_rwsem);
 
@@ -221,6 +273,7 @@  int cxl_clear_poison(struct cxl_memdev *cxlmd, u64 dpa)
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 	struct cxl_mbox_clear_poison clear;
 	struct cxl_mbox_cmd mbox_cmd;
+	struct cxl_region *cxlr;
 	int rc;
 
 	if (!IS_ENABLED(CONFIG_DEBUG_FS))
@@ -252,6 +305,11 @@  int cxl_clear_poison(struct cxl_memdev *cxlmd, u64 dpa)
 	rc = cxl_internal_send_cmd(cxlds, &mbox_cmd);
 	if (rc)
 		goto out;
+
+	cxlr = cxl_dpa_to_region(cxlmd, dpa);
+	if (cxlr)
+		dev_warn_once(cxlds->dev, "poison inject dpa:0x%llx region: %s\n",
+			      dpa, dev_name(&cxlr->dev));
 out:
 	up_read(&cxl_dpa_rwsem);