Re: [PATCH v5 05/12] cxl/mem: Add debugfs attributes for poison inject and clear

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Sun, 26 Mar 2023 22:03:11 -0700
alison.schofield@intel.com wrote:

> From: Alison Schofield <alison.schofield@intel.com>
> 
> Inject and Clear Poison commands are optionally supported by CXL
> memdev devices and are intended for use in debug environments only.
> Add debugfs attributes for user access.
> 
> Documentation/ABI/testing/debugfs-cxl describes the usage.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  Documentation/ABI/testing/debugfs-cxl | 36 +++++++++++++++++++++++++++
>  drivers/cxl/mem.c                     | 26 +++++++++++++++++++
>  2 files changed, 62 insertions(+)
>  create mode 100644 Documentation/ABI/testing/debugfs-cxl
> 
> diff --git a/Documentation/ABI/testing/debugfs-cxl b/Documentation/ABI/testing/debugfs-cxl
> new file mode 100644
> index 000000000000..3f1624f95f11
> --- /dev/null
> +++ b/Documentation/ABI/testing/debugfs-cxl
> @@ -0,0 +1,36 @@
> +What:		/sys/kernel/debug/cxl/memX/inject_poison
> +Date:		April, 2023
> +KernelVersion:	v6.4
> +Contact:	linux-cxl@vger.kernel.org
> +Description:
> +		(WO) When a Device Physical Address (DPA) is written to this
> +		attribute, the memdev driver sends an inject poison command to
> +		the device for the specified address. The DPA must be 64-byte
> +		aligned and the length of the injected poison is 64-bytes. If
> +		successful, the device returns poison when the address is
> +		accessed through the CXL.mem bus. Injecting poison adds the
> +		address to the device's Poison List and the error source is set
> +		to Injected. In addition, the device adds a poison creation
> +		event to its internal Informational Event log, updates the
> +		Event Status register, and if configured, interrupts the host.
> +		It is not an error to inject poison into an address that
> +		already has poison present and no error is returned. The
> +		inject_poison attribute is only visible for devices supporting
> +		the capability.
> +
> +
> +What:		/sys/kernel/debug/memX/clear_poison
> +Date:		April, 2023
> +KernelVersion:	v6.4
> +Contact:	linux-cxl@vger.kernel.org
> +Description:
> +		(WO) When a Device Physical Address (DPA) is written to this
> +		attribute, the memdev driver sends a clear poison command to
> +		the device for the specified address. Clearing poison removes
> +		the address from the device's Poison List and writes 0 (zero)
> +		for 64 bytes starting at address. It is not an error to clear
> +		poison from an address that does not have poison set, and if
> +		poison was not set, the address is not overwritten. If the
As in earlier patch. I don't think this statement is true.  The 64 bytes
at the address are always overwritten, whether or not there is poison.

Otherwise makes sense to hide these in debugfs given they are definitely for debug.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> +		device cannot clear poison from the address, -ENXIO is returned.
> +		The clear_poison attribute is only visible for devices
> +		supporting the capability.
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 576f5b691589..ff67ecf07001 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -94,6 +94,22 @@ static int devm_cxl_add_endpoint(struct device *host, struct cxl_memdev *cxlmd,
>  	return 0;
>  }
>  
> +static int cxl_debugfs_poison_inject(void *data, u64 dpa)
> +{
> +	return cxl_inject_poison(data, dpa);
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_inject_fops, NULL,
> +			 cxl_debugfs_poison_inject, "%llx\n");
> +
> +static int cxl_debugfs_poison_clear(void *data, u64 dpa)
> +{
> +	return cxl_clear_poison(data, dpa);
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL,
> +			 cxl_debugfs_poison_clear, "%llx\n");
> +
>  static int cxl_mem_probe(struct device *dev)
>  {
>  	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> @@ -117,6 +133,16 @@ static int cxl_mem_probe(struct device *dev)
>  
>  	dentry = cxl_debugfs_create_dir(dev_name(dev));
>  	debugfs_create_devm_seqfile(dev, "dpamem", dentry, cxl_mem_dpa_show);
> +
> +	if (test_bit(CXL_MEM_COMMAND_ID_INJECT_POISON,
> +		     to_cxl_memdev(dev)->cxlds->enabled_cmds))
> +		debugfs_create_file("inject_poison", 0200, dentry, dev,
> +				    &cxl_poison_inject_fops);
> +	if (test_bit(CXL_MEM_COMMAND_ID_CLEAR_POISON,
> +		     to_cxl_memdev(dev)->cxlds->enabled_cmds))
> +		debugfs_create_file("clear_poison", 0200, dentry, dev,
> +				    &cxl_poison_clear_fops);
> +
>  	rc = devm_add_action_or_reset(dev, remove_debugfs, dentry);
>  	if (rc)
>  		return rc;