[3/3] cxl/core: Add sysfs attribute get_poison for list retrieval

Commit Message

Alison Schofield June 15, 2022, 12:10 a.m. UTC

From: Alison Schofield <[e-mail]>

The sysfs attribute, get_poison, allows user space to request the
retrieval of a CXL devices poison list for its persistent memory.

From Documentation/ABI/.../sysfs-bus-cxl
        (WO) When a '1' is written to this attribute the memdev
        driver retrieves the poison list from the device. The list
        includes addresses that are poisoned or would result in
        poison if accessed, and the source of the poison. This
        attribute is only visible for devices supporting the
        capability. The retrieved errors are logged as kernel
        trace events with the label: cxl_poison_list.

Signed-off-by: Alison Schofield <[e-mail]>
---
 Documentation/ABI/testing/sysfs-bus-cxl | 13 ++++++++++
 drivers/cxl/core/memdev.c               | 32 +++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

Comments

Ira Weiny June 15, 2022, 3:30 a.m. UTC | #1

On Tue, Jun 14, 2022 at 05:10:28PM -0700, Alison Schofield wrote:
> From: Alison Schofield <[e-mail]>
> 
> The sysfs attribute, get_poison, allows user space to request the
> retrieval of a CXL devices poison list for its persistent memory.
> 
> From Documentation/ABI/.../sysfs-bus-cxl
>         (WO) When a '1' is written to this attribute the memdev
>         driver retrieves the poison list from the device. The list
>         includes addresses that are poisoned or would result in
>         poison if accessed, and the source of the poison. This
>         attribute is only visible for devices supporting the
>         capability. The retrieved errors are logged as kernel
>         trace events with the label: cxl_poison_list.
> 
> Signed-off-by: Alison Schofield <[e-mail]>

Reviewed-by: Ira Weiny <[e-mail]>

> ---
>  Documentation/ABI/testing/sysfs-bus-cxl | 13 ++++++++++
>  drivers/cxl/core/memdev.c               | 32 +++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 7c2b846521f3..9d0c3988fdd2 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -163,3 +163,16 @@ Description:
>  		memory (type-3). The 'target_type' attribute indicates the
>  		current setting which may dynamically change based on what
>  		memory regions are activated in this decode hierarchy.
> +
> +What:		/sys/bus/cxl/devices/memX/get_poison
> +Date:		June, 2022
> +KernelVersion:	v5.20
> +Contact:	[e-mail]
> +Description:
> +		(WO) When a '1' is written to this attribute the memdev
> +		driver retrieves the poison list from the device. The list
> +		includes addresses that are poisoned or would result in
> +		poison if accessed, and the source of the poison. This
> +		attribute is only visible for devices supporting the
> +		capability. The retrieved errors are logged as kernel
> +		trace events with the label: cxl_poison_list.
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index f7cdcd33504a..5ef9ffaa934a 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -106,12 +106,34 @@ static ssize_t numa_node_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RO(numa_node);
>  
> +static ssize_t get_poison_store(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t len)
> +
> +{
> +	int rc;
> +
> +	if (!sysfs_streq(buf, "1")) {
> +		dev_err(dev, "%s: unknown value: %s\n", attr->attr.name, buf);
> +		return -EINVAL;
> +	}
> +
> +	rc = cxl_mem_get_poison_list(dev);
> +	if (rc) {
> +		dev_err(dev, "Failed to retrieve poison list %d\n", rc);
> +		return rc;
> +	}
> +	return len;
> +}
> +static DEVICE_ATTR_WO(get_poison);
> +
>  static struct attribute *cxl_memdev_attributes[] = {
>  	&dev_attr_serial.attr,
>  	&dev_attr_firmware_version.attr,
>  	&dev_attr_payload_max.attr,
>  	&dev_attr_label_storage_size.attr,
>  	&dev_attr_numa_node.attr,
> +	&dev_attr_get_poison.attr,
>  	NULL,
>  };
>  
> @@ -130,6 +152,16 @@ static umode_t cxl_memdev_visible(struct kobject *kobj, struct attribute *a,
>  {
>  	if (!IS_ENABLED(CONFIG_NUMA) && a == &dev_attr_numa_node.attr)
>  		return 0;
> +
> +	if (a == &dev_attr_get_poison.attr) {
> +		struct device *dev = container_of(kobj, struct device, kobj);
> +		struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> +		struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +
> +		if (!test_bit(CXL_MEM_COMMAND_ID_GET_POISON,
> +			      cxlds->enabled_cmds))
> +			return 0;
> +	}
>  	return a->mode;
>  }
>  
> -- 
> 2.31.1
>

Jonathan Cameron June 16, 2022, 3:04 p.m. UTC | #2

On Tue, 14 Jun 2022 17:10:28 -0700
[e-mail] wrote:

> From: Alison Schofield <[e-mail]>
> 
> The sysfs attribute, get_poison, allows user space to request the
> retrieval of a CXL devices poison list for its persistent memory.
> 
> From Documentation/ABI/.../sysfs-bus-cxl
>         (WO) When a '1' is written to this attribute the memdev
>         driver retrieves the poison list from the device. The list
>         includes addresses that are poisoned or would result in
>         poison if accessed, and the source of the poison. This
>         attribute is only visible for devices supporting the
>         capability. The retrieved errors are logged as kernel
>         trace events with the label: cxl_poison_list.
> 
> Signed-off-by: Alison Schofield <[e-mail]>

Hi Alison,

I'm planning to throw together QEMU support for this and test
it. In meantime a few quick comments / suggestions inline.

Thanks,

Jonathan

> ---
>  Documentation/ABI/testing/sysfs-bus-cxl | 13 ++++++++++
>  drivers/cxl/core/memdev.c               | 32 +++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 7c2b846521f3..9d0c3988fdd2 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -163,3 +163,16 @@ Description:
>  		memory (type-3). The 'target_type' attribute indicates the
>  		current setting which may dynamically change based on what
>  		memory regions are activated in this decode hierarchy.
> +
> +What:		/sys/bus/cxl/devices/memX/get_poison
> +Date:		June, 2022
> +KernelVersion:	v5.20
> +Contact:	[e-mail]
> +Description:
> +		(WO) When a '1' is written to this attribute the memdev
> +		driver retrieves the poison list from the device. The list
> +		includes addresses that are poisoned or would result in
> +		poison if accessed, and the source of the poison. This
> +		attribute is only visible for devices supporting the
> +		capability. The retrieved errors are logged as kernel
> +		trace events with the label: cxl_poison_list.
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index f7cdcd33504a..5ef9ffaa934a 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -106,12 +106,34 @@ static ssize_t numa_node_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RO(numa_node);
>  
> +static ssize_t get_poison_store(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t len)
> +
> +{
> +	int rc;
> +
> +	if (!sysfs_streq(buf, "1")) {

Maybe kstrtobool?  If you do then fine to leave the documentation claiming
it's tighter as that'll tell people who actually read it to expect to
write a 1.

> +		dev_err(dev, "%s: unknown value: %s\n", attr->attr.name, buf);

Feels noisy when I'd expect -EINVAL to be enough info to indicate an invalid
parameter.

> +		return -EINVAL;
> +	}
> +
> +	rc = cxl_mem_get_poison_list(dev);
> +	if (rc) {
> +		dev_err(dev, "Failed to retrieve poison list %d\n", rc);

Here I'd expect the error code to returned on the write to probably be enough
info so not sure this error print is useful either.

> +		return rc;
> +	}
> +	return len;
> +}
> +static DEVICE_ATTR_WO(get_poison);
> +
>  static struct attribute *cxl_memdev_attributes[] = {
>  	&dev_attr_serial.attr,
>  	&dev_attr_firmware_version.attr,
>  	&dev_attr_payload_max.attr,
>  	&dev_attr_label_storage_size.attr,
>  	&dev_attr_numa_node.attr,
> +	&dev_attr_get_poison.attr,
>  	NULL,
>  };
>  
> @@ -130,6 +152,16 @@ static umode_t cxl_memdev_visible(struct kobject *kobj, struct attribute *a,
>  {
>  	if (!IS_ENABLED(CONFIG_NUMA) && a == &dev_attr_numa_node.attr)
>  		return 0;
> +
> +	if (a == &dev_attr_get_poison.attr) {
> +		struct device *dev = container_of(kobj, struct device, kobj);
> +		struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> +		struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +
> +		if (!test_bit(CXL_MEM_COMMAND_ID_GET_POISON,
> +			      cxlds->enabled_cmds))
			      to_cxl_memdev(dev)->enabled_cmds))
and drop the local variable is shorter and I don't htink it loses
any readability.

> +			return 0;
> +	}
>  	return a->mode;
>  }
>

Alison Schofield June 16, 2022, 8:39 p.m. UTC | #3

On Thu, Jun 16, 2022 at 08:04:12AM -0700, Jonathan Cameron wrote:
> On Tue, 14 Jun 2022 17:10:28 -0700
> [e-mail] wrote:
> 
> > From: Alison Schofield <[e-mail]>
> > 
> > The sysfs attribute, get_poison, allows user space to request the
> > retrieval of a CXL devices poison list for its persistent memory.
> > 
> > From Documentation/ABI/.../sysfs-bus-cxl
> >         (WO) When a '1' is written to this attribute the memdev
> >         driver retrieves the poison list from the device. The list
> >         includes addresses that are poisoned or would result in
> >         poison if accessed, and the source of the poison. This
> >         attribute is only visible for devices supporting the
> >         capability. The retrieved errors are logged as kernel
> >         trace events with the label: cxl_poison_list.
> > 
> > Signed-off-by: Alison Schofield <[e-mail]>
> 
> Hi Alison,
> 
> I'm planning to throw together QEMU support for this and test
> it. In meantime a few quick comments / suggestions inline.

Thanks Jonathan.
I've tested with a test patch that returns contrived output payloads,
and will look fwd to trying out w qemu,

> 
> Thanks,
> 
> Jonathan
> 
> > ---
snip
> > +
> > +	if (!sysfs_streq(buf, "1")) {
> 
> Maybe kstrtobool?  If you do then fine to leave the documentation claiming
> it's tighter as that'll tell people who actually read it to expect to
> write a 1.
> 
Got it.

> > +		dev_err(dev, "%s: unknown value: %s\n", attr->attr.name, buf);
> 
> Feels noisy when I'd expect -EINVAL to be enough info to indicate an invalid
> parameter.
> 
Got it.

> > +		return -EINVAL;
> > +	}
> > +
> > +	rc = cxl_mem_get_poison_list(dev);
> > +	if (rc) {
> > +		dev_err(dev, "Failed to retrieve poison list %d\n", rc);
> 
> Here I'd expect the error code to returned on the write to probably be enough
> info so not sure this error print is useful either.
> 
Got it.

> > +
snip
> > +	if (a == &dev_attr_get_poison.attr) {
> > +		struct device *dev = container_of(kobj, struct device, kobj);
> > +		struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> > +		struct cxl_dev_state *cxlds = cxlmd->cxlds;
> > +
> > +		if (!test_bit(CXL_MEM_COMMAND_ID_GET_POISON,
> > +			      cxlds->enabled_cmds))
> 			      to_cxl_memdev(dev)->enabled_cmds))
> and drop the local variable is shorter and I don't htink it loses
> any readability.
> 
Got it.

Thanks Jonathan!

Dan Williams June 17, 2022, 6:42 p.m. UTC | #4

alison.schofield@ wrote:
> From: Alison Schofield <[e-mail]>
> 
> The sysfs attribute, get_poison, allows user space to request the
> retrieval of a CXL devices poison list for its persistent memory.

If the device supports get poison list for volatile memory, just grab
that too. With the "to be released soon" region patches userspace can
trivially translate DPA addresses to media type.

> 
> From Documentation/ABI/.../sysfs-bus-cxl
>         (WO) When a '1' is written to this attribute the memdev
>         driver retrieves the poison list from the device. The list
>         includes addresses that are poisoned or would result in
>         poison if accessed, and the source of the poison. This
>         attribute is only visible for devices supporting the
>         capability. The retrieved errors are logged as kernel
>         trace events with the label: cxl_poison_list.
> 
> Signed-off-by: Alison Schofield <[e-mail]>
> ---
>  Documentation/ABI/testing/sysfs-bus-cxl | 13 ++++++++++
>  drivers/cxl/core/memdev.c               | 32 +++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 7c2b846521f3..9d0c3988fdd2 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -163,3 +163,16 @@ Description:
>  		memory (type-3). The 'target_type' attribute indicates the
>  		current setting which may dynamically change based on what
>  		memory regions are activated in this decode hierarchy.
> +
> +What:		/sys/bus/cxl/devices/memX/get_poison
> +Date:		June, 2022
> +KernelVersion:	v5.20
> +Contact:	[e-mail]
> +Description:
> +		(WO) When a '1' is written to this attribute the memdev
> +		driver retrieves the poison list from the device. The list
> +		includes addresses that are poisoned or would result in
> +		poison if accessed, and the source of the poison. This
> +		attribute is only visible for devices supporting the
> +		capability. The retrieved errors are logged as kernel
> +		trace events with the label: cxl_poison_list.
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index f7cdcd33504a..5ef9ffaa934a 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -106,12 +106,34 @@ static ssize_t numa_node_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RO(numa_node);
>  
> +static ssize_t get_poison_store(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t len)
> +
> +{
> +	int rc;
> +
> +	if (!sysfs_streq(buf, "1")) {

kstrtobool()?

> +		dev_err(dev, "%s: unknown value: %s\n", attr->attr.name, buf);

dev_err() is overkill for sysfs errors. dev_dbg() can be nice for errors
that trigger deep within the kernel in response to a sysfs write. In
this case EINVAL return is sufficient.

> +		return -EINVAL;
> +	}
> +
> +	rc = cxl_mem_get_poison_list(dev);
> +	if (rc) {
> +		dev_err(dev, "Failed to retrieve poison list %d\n", rc);

Too chatty, dev_dbg() or delete.

> +		return rc;
> +	}
> +	return len;
> +}
> +static DEVICE_ATTR_WO(get_poison);
> +
>  static struct attribute *cxl_memdev_attributes[] = {
>  	&dev_attr_serial.attr,
>  	&dev_attr_firmware_version.attr,
>  	&dev_attr_payload_max.attr,
>  	&dev_attr_label_storage_size.attr,
>  	&dev_attr_numa_node.attr,
> +	&dev_attr_get_poison.attr,
>  	NULL,
>  };
>  
> @@ -130,6 +152,16 @@ static umode_t cxl_memdev_visible(struct kobject *kobj, struct attribute *a,
>  {
>  	if (!IS_ENABLED(CONFIG_NUMA) && a == &dev_attr_numa_node.attr)
>  		return 0;
> +
> +	if (a == &dev_attr_get_poison.attr) {
> +		struct device *dev = container_of(kobj, struct device, kobj);

Use the kobj_to_dev() helper.

> +		struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> +		struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +
> +		if (!test_bit(CXL_MEM_COMMAND_ID_GET_POISON,
> +			      cxlds->enabled_cmds))
> +			return 0;
> +	}
>  	return a->mode;
>  }
>  
> -- 
> 2.31.1
>

Alison Schofield June 18, 2022, 12:21 a.m. UTC | #5

On Fri, Jun 17, 2022 at 11:42:11AM -0700, Dan Williams wrote:
> alison.schofield@ wrote:
> > From: Alison Schofield <[e-mail]>
> > 
> > The sysfs attribute, get_poison, allows user space to request the
> > retrieval of a CXL devices poison list for its persistent memory.
> 
> If the device supports get poison list for volatile memory, just grab
> that too. With the "to be released soon" region patches userspace can
> trivially translate DPA addresses to media type.
> 

Dan,

The only way I know to discover if the device supports poison list for
volatile is to do the get_poison_list on the volatile range and see
what happens. Am I missing a capability setting somewhere?

Here's a blanket "Got it, Thanks!" for all the other pieces.

Alison


> > 
snip

Dan Williams June 18, 2022, 1:08 a.m. UTC | #6

Alison Schofield wrote:
> On Fri, Jun 17, 2022 at 11:42:11AM -0700, Dan Williams wrote:
> > alison.schofield@ wrote:
> > > From: Alison Schofield <[e-mail]>
> > > 
> > > The sysfs attribute, get_poison, allows user space to request the
> > > retrieval of a CXL devices poison list for its persistent memory.
> > 
> > If the device supports get poison list for volatile memory, just grab
> > that too. With the "to be released soon" region patches userspace can
> > trivially translate DPA addresses to media type.
> > 
> 
> Dan,
> 
> The only way I know to discover if the device supports poison list for
> volatile is to do the get_poison_list on the volatile range and see
> what happens. Am I missing a capability setting somewhere?

If someone executes "echo 1 > trace_poison_list" I expect that the
driver does:

get_poison_list(volatile_range);
get_poison_list(pmem_range);

...and if scanning the volatile partition ends in error then that just
means no error records appear. When the error is "Invalid Physical
Address" the driver can just remember that's a permanent error and never
try again. So it's more like:

if (volatile_range_valid) {
	if (get_poison_list(volatile_range) == INVALID_PHYS_ADDR)
		volatile_range_valid = false;
}
get_poison_list(pmem_range);

...but that's probably overkill since get_poison_list() is cheap. Just
treat it like the zero error records case.

In the to be released region provisioning patches there is a DPA
resource tree partitioned by DPA mode type, so the poison list code
probably wants to do something like:

down_read(&cxl_dpa_rwsem);
for (p = cxlds->dpa_res.child; p; p = p->sibling)
	get_poison_list(p->start, resource_size(p));
up_read(&cxl_dpa_rwsem);

Alison Schofield June 18, 2022, 1:35 a.m. UTC | #7

On Fri, Jun 17, 2022 at 06:08:52PM -0700, Dan Williams wrote:
> Alison Schofield wrote:
> > On Fri, Jun 17, 2022 at 11:42:11AM -0700, Dan Williams wrote:
> > > alison.schofield@ wrote:
> > > > From: Alison Schofield <[e-mail]>
> > > > 
> > > > The sysfs attribute, get_poison, allows user space to request the
> > > > retrieval of a CXL devices poison list for its persistent memory.
> > > 
> > > If the device supports get poison list for volatile memory, just grab
> > > that too. With the "to be released soon" region patches userspace can
> > > trivially translate DPA addresses to media type.
> > > 
> > 
> > Dan,
> > 
> > The only way I know to discover if the device supports poison list for
> > volatile is to do the get_poison_list on the volatile range and see
> > what happens. Am I missing a capability setting somewhere?
> 
> If someone executes "echo 1 > trace_poison_list" I expect that the
> driver does:
> 
> get_poison_list(volatile_range);
> get_poison_list(pmem_range);
> 
> ...and if scanning the volatile partition ends in error then that just
> means no error records appear. When the error is "Invalid Physical
> Address" the driver can just remember that's a permanent error and never
> try again. So it's more like:
> 
> if (volatile_range_valid) {
> 	if (get_poison_list(volatile_range) == INVALID_PHYS_ADDR)
> 		volatile_range_valid = false;
> }
> get_poison_list(pmem_range);
> 
> ...but that's probably overkill since get_poison_list() is cheap. Just
> treat it like the zero error records case.

Got it!

> 
> In the to be released region provisioning patches there is a DPA
> resource tree partitioned by DPA mode type, so the poison list code
> probably wants to do something like:
> 
> down_read(&cxl_dpa_rwsem);
> for (p = cxlds->dpa_res.child; p; p = p->sibling)
> 	get_poison_list(p->start, resource_size(p));
> up_read(&cxl_dpa_rwsem);

Great ending to the week! This is going to make collecting the
poison per region much simpler than I was imagining :)

Patch

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index 7c2b846521f3..9d0c3988fdd2 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -163,3 +163,16 @@  Description:
 		memory (type-3). The 'target_type' attribute indicates the
 		current setting which may dynamically change based on what
 		memory regions are activated in this decode hierarchy.
+
+What:		/sys/bus/cxl/devices/memX/get_poison
+Date:		June, 2022
+KernelVersion:	v5.20
+Contact:	[e-mail]
+Description:
+		(WO) When a '1' is written to this attribute the memdev
+		driver retrieves the poison list from the device. The list
+		includes addresses that are poisoned or would result in
+		poison if accessed, and the source of the poison. This
+		attribute is only visible for devices supporting the
+		capability. The retrieved errors are logged as kernel
+		trace events with the label: cxl_poison_list.
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index f7cdcd33504a..5ef9ffaa934a 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -106,12 +106,34 @@  static ssize_t numa_node_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(numa_node);
 
+static ssize_t get_poison_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t len)
+
+{
+	int rc;
+
+	if (!sysfs_streq(buf, "1")) {
+		dev_err(dev, "%s: unknown value: %s\n", attr->attr.name, buf);
+		return -EINVAL;
+	}
+
+	rc = cxl_mem_get_poison_list(dev);
+	if (rc) {
+		dev_err(dev, "Failed to retrieve poison list %d\n", rc);
+		return rc;
+	}
+	return len;
+}
+static DEVICE_ATTR_WO(get_poison);
+
 static struct attribute *cxl_memdev_attributes[] = {
 	&dev_attr_serial.attr,
 	&dev_attr_firmware_version.attr,
 	&dev_attr_payload_max.attr,
 	&dev_attr_label_storage_size.attr,
 	&dev_attr_numa_node.attr,
+	&dev_attr_get_poison.attr,
 	NULL,
 };
 
@@ -130,6 +152,16 @@  static umode_t cxl_memdev_visible(struct kobject *kobj, struct attribute *a,
 {
 	if (!IS_ENABLED(CONFIG_NUMA) && a == &dev_attr_numa_node.attr)
 		return 0;
+
+	if (a == &dev_attr_get_poison.attr) {
+		struct device *dev = container_of(kobj, struct device, kobj);
+		struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
+		struct cxl_dev_state *cxlds = cxlmd->cxlds;
+
+		if (!test_bit(CXL_MEM_COMMAND_ID_GET_POISON,
+			      cxlds->enabled_cmds))
+			return 0;
+	}
 	return a->mode;
 }