public inbox for linux-cxl@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/1] cxl: Media Operations
@ 2026-03-23 20:40 Davidlohr Bueso
  2026-03-23 20:40 ` [PATCH 1/1] cxl/mbox: Support Media Operation Davidlohr Bueso
  2026-03-24 15:32 ` [RFC PATCH 0/1] cxl: Media Operations Jonathan Cameron
  0 siblings, 2 replies; 10+ messages in thread
From: Davidlohr Bueso @ 2026-03-23 20:40 UTC (permalink / raw)
  To: dave.jiang
  Cc: jonathan.cameron, alison.schofield, ira.weiny, dan.j.williams,
	dongjoo.seo1, anisa.su, linux-cxl, dave

Hello,

This implements support for Media Operation cmd. It is marked RFC because of these
noteworthy points:

 (i) Sysfs interfaces. These changes implement 'security/zero' and multiplex
     'security/sanitize' to allow dpa, len pairs for the range (no impact for
     current usage as whole-device). This breaks the 1 value per file "rule",
     and would be a first for drivers/cxl/*, so am I open to suggestions.     
  
 (ii) Background cmd handling semantics. Media operations are synchronous,
      which for raged zeroing operations makes sense and for range sanitize
      differs from the current async handling for whole-device, so user response
      differs significantly, something I am not crazy about. Another downside
      of this is that it exposes lock hold times to be user-controllable, but is
      no different than what we do for fw. Fundamentally, users have the
      responsibility to break up the work into sane ranges. Further, this is
      without considering abortable bg cmds[0], which, per-spec, are unsupported
      today for media operations.

Tested with qemu + fixlets (https://lore.kernel.org/all/20260319184256.3762391-1-dave@stgolabs.net/)

Applies against latest 'next' from cxl.git.

[0] https://lore.kernel.org/all/20250617193611.564668-1-dave@stgolabs.net/

Thanks!

Davidlohr Bueso (1):
  cxl/mbox: Support Media Operation

 Documentation/ABI/testing/sysfs-bus-cxl |  31 +++-
 drivers/cxl/core/mbox.c                 | 235 ++++++++++++++++++++++++
 drivers/cxl/core/memdev.c               |  45 ++++-
 drivers/cxl/cxlmem.h                    |  59 ++++++
 drivers/cxl/pci.c                       |   4 +
 5 files changed, 369 insertions(+), 5 deletions(-)

--
2.39.5


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/1] cxl/mbox: Support Media Operation
  2026-03-23 20:40 [RFC PATCH 0/1] cxl: Media Operations Davidlohr Bueso
@ 2026-03-23 20:40 ` Davidlohr Bueso
  2026-03-24 16:18   ` Jonathan Cameron
                     ` (2 more replies)
  2026-03-24 15:32 ` [RFC PATCH 0/1] cxl: Media Operations Jonathan Cameron
  1 sibling, 3 replies; 10+ messages in thread
From: Davidlohr Bueso @ 2026-03-23 20:40 UTC (permalink / raw)
  To: dave.jiang
  Cc: jonathan.cameron, alison.schofield, ira.weiny, dan.j.williams,
	dongjoo.seo1, anisa.su, linux-cxl, dave

Add support for the Media Operation command (opcode 4402h) which
enables targeted sanitize and zero operations on specific DPA
ranges, as defined in CXL 4.0 Section 8.2.10.9.5.3.

Run Discovery during probe to learn the device's DPA range granularity
and which operations are supported, and expose to sysfs accordingly:

  o 'security/sanitize' is multiplexed to allow single ranged input.
  o 'security/zero' is added, also with single ranged input.

Operations are only allowed on DPA ranges that do not overlap any
hardware-committed endpoint decoder to ensure not corrupting active
mappings. Background operations are handled synchronously.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 Documentation/ABI/testing/sysfs-bus-cxl |  31 +++-
 drivers/cxl/core/mbox.c                 | 235 ++++++++++++++++++++++++
 drivers/cxl/core/memdev.c               |  45 ++++-
 drivers/cxl/cxlmem.h                    |  59 ++++++
 drivers/cxl/pci.c                       |   4 +
 5 files changed, 369 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index c80a1b5a03db..da8f8dc92a5a 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -117,10 +117,12 @@ Contact:	linux-cxl@vger.kernel.org
 Description:
 		(RO) Reading this file will display the CXL security state for
 		that device. Such states can be: 'disabled', 'sanitize', when
-		a sanitization is currently underway; or those available only
-		for persistent memory: 'locked', 'unlocked' or 'frozen'. This
-		sysfs entry is select/poll capable from userspace to notify
-		upon completion of a sanitize operation.
+		a whole-device sanitization is currently underway; or those
+		available only for persistent memory: 'locked', 'unlocked' or
+		'frozen'. This sysfs entry is select/poll capable from
+		userspace to notify upon completion of a sanitize operation.
+		Targeted range sanitize via Media Operation does not affect
+		this state as it completes synchronously.
 
 
 What:           /sys/bus/cxl/devices/memX/security/sanitize
@@ -141,6 +143,13 @@ Description:
 		states. If this file is not present, then there is no hardware
 		support for the operation.
 
+		If the device supports the Media Operation command with the
+		sanitize operation, a DPA range may be written as 'start
+		length' in bytes to sanitize a targeted region of media.
+		The start address and length must be aligned to the device's
+		media operation granularity. The sanitize method applied to
+		the range is the same as for whole-device sanitize.
+
 
 What            /sys/bus/cxl/devices/memX/security/erase
 Date:           June, 2023
@@ -158,6 +167,20 @@ Description:
 		support for the operation.
 
 
+What:		/sys/bus/cxl/devices/memX/security/zero
+Date:		March, 2026
+KernelVersion:	v7.1
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(WO) Write a DPA range as 'start length' in bytes to this
+		attribute to zero a specific region of device media via the
+		Media Operation command. After completion, reads from the
+		specified range will return zero. The start address and
+		length must be aligned to the device's media operation
+		granularity. If this file is not present, then the device
+		does not support the Media Operation zero command.
+
+
 What:		/sys/bus/cxl/devices/memX/firmware/
 Date:		April, 2023
 KernelVersion:	v6.5
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index aaa5c6277ebf..b7047cb9abed 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -160,6 +160,10 @@ static void cxl_set_security_cmd_enabled(struct cxl_security_state *security,
 		set_bit(CXL_SEC_ENABLED_PASSPHRASE_SECURE_ERASE,
 			security->enabled_cmds);
 		break;
+	case CXL_MBOX_OP_MEDIA_OPERATION:
+		set_bit(CXL_SEC_ENABLED_MEDIA_OPERATIONS,
+			security->enabled_cmds);
+		break;
 	default:
 		break;
 	}
@@ -893,6 +897,102 @@ int cxl_enumerate_cmds(struct cxl_memdev_state *mds)
 }
 EXPORT_SYMBOL_NS_GPL(cxl_enumerate_cmds, "CXL");
 
+/**
+ * cxl_media_op_discover() - Discover supported media operation
+ * @mds: The device data for the operation
+ *
+ * Discover any available Media Operations.
+ *
+ * Return: 0 on success or if Media Operation is not supported,
+ * negative error code on failure.
+ */
+#define CXL_MEDIA_OP_MAX_OPS 2
+
+int cxl_media_op_discover(struct cxl_memdev_state *mds)
+{
+	size_t out_size, in_size;
+	void *payload_in;
+	struct cxl_mbox_cmd mbox_cmd;
+	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
+	struct cxl_mbox_media_op_input *payload;
+	struct cxl_mbox_media_op_discovery_args *args;
+	struct cxl_mbox_media_op_discovery_out *disc_out;
+	int rc, i;
+	u16 num_returned;
+
+	if (!test_bit(CXL_SEC_ENABLED_MEDIA_OPERATIONS,
+		      mds->security.enabled_cmds))
+		return 0;
+
+	in_size = sizeof(*payload) + sizeof(*args);
+	payload_in = kzalloc(in_size, GFP_KERNEL);
+	if (!payload_in)
+		return -ENOMEM;
+
+	payload = payload_in;
+	payload->class = CXL_MEDIA_OP_CLASS_GENERAL;
+	payload->subclass = CXL_MEDIA_OP_SUBCLASS_DISCOVERY;
+	payload->dpa_range_count = 0;
+
+	args = payload_in + sizeof(*payload);
+	args->start_index = 0;
+	args->num_ops = cpu_to_le16(CXL_MEDIA_OP_MAX_OPS);
+
+	out_size = sizeof(*disc_out) +
+		   CXL_MEDIA_OP_MAX_OPS * sizeof(disc_out->ops[0]);
+	disc_out = kzalloc(out_size, GFP_KERNEL);
+	if (!disc_out) {
+		kfree(payload_in);
+		return -ENOMEM;
+	}
+
+	mbox_cmd = (struct cxl_mbox_cmd) {
+		.opcode = CXL_MBOX_OP_MEDIA_OPERATION,
+		.payload_in = payload_in,
+		.size_in = in_size,
+		.payload_out = disc_out,
+		.size_out = out_size,
+		.min_out = sizeof(*disc_out),
+		.poll_count = 1,
+		.poll_interval_ms = 1000,
+	};
+
+	rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
+	if (rc < 0) {
+		dev_dbg(cxl_mbox->host,
+			"Media Operation Discovery failed: %d\n", rc);
+		goto out;
+	}
+
+	mds->media_op.granularity = le64_to_cpu(disc_out->granularity);
+	num_returned = min_t(u16, le16_to_cpu(disc_out->num_returned),
+			     CXL_MEDIA_OP_MAX_OPS);
+
+	for (i = 0; i < num_returned; i++) {
+		u8 cls = disc_out->ops[i].class;
+		u8 sub = disc_out->ops[i].subclass;
+
+		if (cls == CXL_MEDIA_OP_CLASS_SANITIZE &&
+		    sub == CXL_MEDIA_OP_SUBCLASS_SANITIZE)
+			mds->media_op.sanitize_supported = true;
+		if (cls == CXL_MEDIA_OP_CLASS_SANITIZE &&
+		    sub == CXL_MEDIA_OP_SUBCLASS_ZERO)
+			mds->media_op.zero_supported = true;
+	}
+
+	dev_dbg(cxl_mbox->host,
+		"Media Operation: granularity=%llu sanitize=%d zero=%d\n",
+		mds->media_op.granularity,
+		mds->media_op.sanitize_supported,
+		mds->media_op.zero_supported);
+
+out:
+	kfree(disc_out);
+	kfree(payload_in);
+	return rc;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_media_op_discover, "CXL");
+
 void cxl_event_trace_record(struct cxl_memdev *cxlmd,
 			    enum cxl_event_log_type type,
 			    enum cxl_event_type event_type,
@@ -1308,6 +1408,141 @@ int cxl_mem_sanitize(struct cxl_memdev *cxlmd, u16 cmd)
 	return -EBUSY;
 }
 
+static int __cxl_mem_media_op(struct cxl_memdev_state *mds, u8 class,
+			      u8 subclass, u64 dpa_start, u64 dpa_length)
+{
+	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
+	struct cxl_mbox_media_op_input *payload;
+	struct cxl_mbox_cmd mbox_cmd;
+	u64 granularity;
+	size_t size;
+	int rc;
+
+	if (!test_bit(CXL_SEC_ENABLED_MEDIA_OPERATIONS,
+		      mds->security.enabled_cmds))
+		return -EOPNOTSUPP;
+
+	/* ensure the specific operation was reported by Discovery */
+	if (class == CXL_MEDIA_OP_CLASS_SANITIZE &&
+	    subclass == CXL_MEDIA_OP_SUBCLASS_SANITIZE &&
+	    !mds->media_op.sanitize_supported)
+		return -EOPNOTSUPP;
+	if (class == CXL_MEDIA_OP_CLASS_SANITIZE &&
+	    subclass == CXL_MEDIA_OP_SUBCLASS_ZERO &&
+	    !mds->media_op.zero_supported)
+		return -EOPNOTSUPP;
+
+	granularity = mds->media_op.granularity;
+	if (!granularity)
+		return -EINVAL;
+
+	if (!IS_ALIGNED(dpa_start, granularity) ||
+	    !IS_ALIGNED(dpa_length, granularity) ||
+	    dpa_length == 0) {
+		dev_dbg(cxl_mbox->host,
+			"DPA range not aligned to %llu-byte granularity\n",
+			granularity);
+		return -EINVAL;
+	}
+
+	size = sizeof(*payload) + sizeof(payload->dpa_range_list[0]);
+	payload = kzalloc(size, GFP_KERNEL);
+	if (!payload)
+		return -ENOMEM;
+
+	payload->class = class;
+	payload->subclass = subclass;
+	payload->dpa_range_count = cpu_to_le32(1);
+	payload->dpa_range_list[0].starting_dpa = cpu_to_le64(dpa_start);
+	payload->dpa_range_list[0].length = cpu_to_le64(dpa_length);
+
+	mbox_cmd = (struct cxl_mbox_cmd) {
+		.opcode = CXL_MBOX_OP_MEDIA_OPERATION,
+		.payload_in = payload,
+		.size_in = size,
+		.poll_count = 60,
+		.poll_interval_ms = 1000,
+	};
+
+	rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
+	kfree(payload);
+
+	if (rc < 0) {
+		dev_dbg(cxl_mbox->host,
+			"Media Operation (class=%u sub=%u) failed: %d\n",
+			class, subclass, rc);
+		return rc;
+	}
+
+	return 0;
+}
+
+static int __cxl_dpa_overlap_committed(struct device *dev, void *arg)
+{
+	const struct resource *range = arg;
+	struct cxl_endpoint_decoder *cxled;
+
+	if (!is_endpoint_decoder(dev))
+		return 0;
+
+	cxled = to_cxl_endpoint_decoder(dev);
+	if (!(cxled->cxld.flags & CXL_DECODER_F_ENABLE))
+		return 0;
+	if (!cxled->dpa_res || !resource_size(cxled->dpa_res))
+		return 0;
+
+	return resource_overlaps(cxled->dpa_res, range);
+}
+
+static bool cxl_media_op_dpa_overlap(struct cxl_memdev *cxlmd,
+				     u64 dpa_start, u64 dpa_length)
+{
+	struct cxl_port *endpoint = cxlmd->endpoint;
+	struct resource range = DEFINE_RES_MEM(dpa_start, dpa_length);
+
+	if (!cxlmd->dev.driver || !is_cxl_endpoint(endpoint) ||
+	    cxl_num_decoders_committed(endpoint) == 0)
+		return false;
+
+	return device_for_each_child(&endpoint->dev, &range,
+				     __cxl_dpa_overlap_committed);
+}
+
+/**
+ * cxl_mem_media_op() - Send a Media Operation command to the device.
+ * @cxlmd: The device for the operation
+ * @class: Media operation class
+ * @subclass: Media operation subclass
+ * @dpa_start: Starting DPA in bytes
+ * @dpa_length: Length of the DPA range in bytes
+ *
+ * Send a Media Operation command with a single DPA range.
+ *
+ * Requires corresponding decoder containing the range be offline to
+ * prevent corrupting any actively mapped memory.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int cxl_mem_media_op(struct cxl_memdev *cxlmd, u8 class, u8 subclass,
+		     u64 dpa_start, u64 dpa_length)
+{
+	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
+
+	if (!dpa_length)
+		return -EINVAL;
+
+	guard(device)(&cxlmd->dev);
+	guard(rwsem_read)(&cxl_rwsem.region);
+	guard(rwsem_read)(&cxl_rwsem.dpa);
+
+	/* reject if the DPA range overlaps a committed decoder range */
+	if (cxl_media_op_dpa_overlap(cxlmd, dpa_start, dpa_length))
+		return -EBUSY;
+
+	return __cxl_mem_media_op(mds, class, subclass, dpa_start, dpa_length);
+}
+EXPORT_SYMBOL_NS_GPL(cxl_mem_media_op, "CXL");
+
 static void add_part(struct cxl_dpa_info *info, u64 start, u64 size, enum cxl_partition_mode mode)
 {
 	int i = info->nr_partitions;
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 99e422594885..a78a0469fb41 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -164,9 +164,20 @@ static ssize_t security_sanitize_store(struct device *dev,
 				       const char *buf, size_t len)
 {
 	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
+	u64 dpa_start, dpa_length;
 	bool sanitize;
 	ssize_t rc;
 
+	/* try "start length" first for ranged sanitize */
+	if (sscanf(buf, "%llx %llx", &dpa_start, &dpa_length) == 2) {
+		rc = cxl_mem_media_op(cxlmd, CXL_MEDIA_OP_CLASS_SANITIZE,
+				      CXL_MEDIA_OP_SUBCLASS_SANITIZE,
+				      dpa_start, dpa_length);
+		if (rc)
+			return rc;
+		return len;
+	}
+
 	if (kstrtobool(buf, &sanitize) || !sanitize)
 		return -EINVAL;
 
@@ -199,6 +210,28 @@ static ssize_t security_erase_store(struct device *dev,
 static struct device_attribute dev_attr_security_erase =
 	__ATTR(erase, 0200, NULL, security_erase_store);
 
+static ssize_t security_zero_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t len)
+{
+	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
+	u64 dpa_start, dpa_length;
+	ssize_t rc;
+
+	if (sscanf(buf, "%llx %llx", &dpa_start, &dpa_length) != 2)
+		return -EINVAL;
+
+	rc = cxl_mem_media_op(cxlmd, CXL_MEDIA_OP_CLASS_SANITIZE,
+			      CXL_MEDIA_OP_SUBCLASS_ZERO,
+			      dpa_start, dpa_length);
+	if (rc)
+		return rc;
+
+	return len;
+}
+static struct device_attribute dev_attr_security_zero =
+	__ATTR(zero, 0200, NULL, security_zero_store);
+
 bool cxl_memdev_has_poison_cmd(struct cxl_memdev *cxlmd,
 			       enum poison_cmd_enabled_bits cmd)
 {
@@ -476,6 +509,7 @@ static struct attribute *cxl_memdev_security_attributes[] = {
 	&dev_attr_security_state.attr,
 	&dev_attr_security_sanitize.attr,
 	&dev_attr_security_erase.attr,
+	&dev_attr_security_zero.attr,
 	NULL,
 };
 
@@ -538,13 +572,22 @@ static umode_t cxl_memdev_security_visible(struct kobject *kobj,
 	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
 
 	if (a == &dev_attr_security_sanitize.attr &&
-	    !test_bit(CXL_SEC_ENABLED_SANITIZE, mds->security.enabled_cmds))
+	    !test_bit(CXL_SEC_ENABLED_SANITIZE, mds->security.enabled_cmds) &&
+	    !(test_bit(CXL_SEC_ENABLED_MEDIA_OPERATIONS,
+		       mds->security.enabled_cmds) &&
+	      mds->media_op.sanitize_supported))
 		return 0;
 
 	if (a == &dev_attr_security_erase.attr &&
 	    !test_bit(CXL_SEC_ENABLED_SECURE_ERASE, mds->security.enabled_cmds))
 		return 0;
 
+	if (a == &dev_attr_security_zero.attr &&
+	    (!test_bit(CXL_SEC_ENABLED_MEDIA_OPERATIONS,
+		       mds->security.enabled_cmds) ||
+	     !mds->media_op.zero_supported))
+		return 0;
+
 	return a->mode;
 }
 
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 02054f233fc5..cde613b25581 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -251,6 +251,7 @@ enum security_cmd_enabled_bits {
 	CXL_SEC_ENABLED_UNLOCK,
 	CXL_SEC_ENABLED_FREEZE_SECURITY,
 	CXL_SEC_ENABLED_PASSPHRASE_SECURE_ERASE,
+	CXL_SEC_ENABLED_MEDIA_OPERATIONS,
 	CXL_SEC_ENABLED_MAX
 };
 
@@ -352,6 +353,59 @@ struct cxl_fw_state {
 	int next_slot;
 };
 
+/* Media Operation classes and subclasses (CXL 4.0 Table 8-331) */
+#define CXL_MEDIA_OP_CLASS_GENERAL	0x00
+#define CXL_MEDIA_OP_CLASS_SANITIZE	0x01
+
+#define CXL_MEDIA_OP_SUBCLASS_DISCOVERY	0x00
+#define CXL_MEDIA_OP_SUBCLASS_SANITIZE	0x00
+#define CXL_MEDIA_OP_SUBCLASS_ZERO	0x01
+
+/* Media Operation DPA Range (CXL 4.0 Table 8-330) */
+struct cxl_media_op_dpa_range {
+	__le64 starting_dpa;
+	__le64 length;
+} __packed;
+
+/* Media Operation Input Payload (CXL 4.0 Table 8-329) */
+struct cxl_mbox_media_op_input {
+	u8 class;
+	u8 subclass;
+	u8 rsvd[2];
+	__le32 dpa_range_count;
+	struct cxl_media_op_dpa_range dpa_range_list[];
+} __packed;
+
+/* Discovery operation-specific arguments (CXL 4.0 Table 8-332) */
+struct cxl_mbox_media_op_discovery_args {
+	__le16 start_index;
+	__le16 num_ops;
+} __packed;
+
+/* Discovery output payload (CXL 4.0 Table 8-333) */
+struct cxl_mbox_media_op_discovery_out {
+	__le64 granularity;
+	__le16 total_supported;
+	__le16 num_returned;
+	struct {
+		u8 class;
+		u8 subclass;
+	} __packed ops[];
+} __packed;
+
+/**
+ * struct cxl_media_op_state - Media Operation state
+ *
+ * @granularity: DPA range granularity (bytes) from Discovery
+ * @sanitize_supported: device supports ranged sanitize
+ * @zero_supported: device supports ranged zero
+ */
+struct cxl_media_op_state {
+	u64 granularity;
+	bool sanitize_supported;
+	bool zero_supported;
+};
+
 /**
  * struct cxl_security_state - Device security state
  *
@@ -429,6 +483,7 @@ struct cxl_memdev_state {
 	struct cxl_poison_state poison;
 	struct cxl_security_state security;
 	struct cxl_fw_state fw;
+	struct cxl_media_op_state media_op;
 	struct notifier_block mce_notifier;
 };
 
@@ -479,6 +534,7 @@ enum cxl_opcode {
 	CXL_MBOX_OP_GET_SCAN_MEDIA	= 0x4305,
 	CXL_MBOX_OP_SANITIZE		= 0x4400,
 	CXL_MBOX_OP_SECURE_ERASE	= 0x4401,
+	CXL_MBOX_OP_MEDIA_OPERATION	= 0x4402,
 	CXL_MBOX_OP_GET_SECURITY_STATE	= 0x4500,
 	CXL_MBOX_OP_SET_PASSPHRASE	= 0x4501,
 	CXL_MBOX_OP_DISABLE_PASSPHRASE	= 0x4502,
@@ -829,6 +885,9 @@ static inline void cxl_mem_active_dec(void)
 #endif
 
 int cxl_mem_sanitize(struct cxl_memdev *cxlmd, u16 cmd);
+int cxl_media_op_discover(struct cxl_memdev_state *mds);
+int cxl_mem_media_op(struct cxl_memdev *cxlmd, u8 class, u8 subclass,
+		     u64 dpa_start, u64 dpa_length);
 
 /**
  * struct cxl_hdm - HDM Decoder registers and cached / decoded capabilities
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index bace662dc988..95bf773aab14 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -878,6 +878,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		dev_dbg(&pdev->dev, "No CXL Features discovered\n");
 
+	rc = cxl_media_op_discover(mds);
+	if (rc)
+		dev_dbg(&pdev->dev, "No Media Operation discovered\n");
+
 	cxlmd = devm_cxl_add_memdev(cxlds, NULL);
 	if (IS_ERR(cxlmd))
 		return PTR_ERR(cxlmd);
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 0/1] cxl: Media Operations
  2026-03-23 20:40 [RFC PATCH 0/1] cxl: Media Operations Davidlohr Bueso
  2026-03-23 20:40 ` [PATCH 1/1] cxl/mbox: Support Media Operation Davidlohr Bueso
@ 2026-03-24 15:32 ` Jonathan Cameron
  2026-03-24 18:07   ` Davidlohr Bueso
  1 sibling, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2026-03-24 15:32 UTC (permalink / raw)
  To: Davidlohr Bueso
  Cc: dave.jiang, alison.schofield, ira.weiny, dan.j.williams,
	dongjoo.seo1, anisa.su, linux-cxl

On Mon, 23 Mar 2026 13:40:12 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:

> Hello,
> 
> This implements support for Media Operation cmd. It is marked RFC because of these
> noteworthy points:
> 
>  (i) Sysfs interfaces. These changes implement 'security/zero' and multiplex
>      'security/sanitize' to allow dpa, len pairs for the range (no impact for
>      current usage as whole-device). This breaks the 1 value per file "rule",
>      and would be a first for drivers/cxl/*, so am I open to suggestions.     

Value is a vague sort of term.  It's one thing (an range) so seems fine to me.

>   
>  (ii) Background cmd handling semantics. Media operations are synchronous,
>       which for raged zeroing operations makes sense and for range sanitize

ranged

>       differs from the current async handling for whole-device, so user response
>       differs significantly, something I am not crazy about. Another downside
>       of this is that it exposes lock hold times to be user-controllable, but is
>       no different than what we do for fw. Fundamentally, users have the
>       responsibility to break up the work into sane ranges.

We could apply a limit on this and make that discoverable via another sysfs attribute.
So let userspace request in say 1Gig chunks.  Would have to check that the device
supports that granularity though.
I don't think there is anything in the spec today to let us query how long
something takes (which would be a better way to bound this).


> Further, this is
>       without considering abortable bg cmds[0], which, per-spec, are unsupported
>       today for media operations.
> 
> Tested with qemu + fixlets (https://lore.kernel.org/all/20260319184256.3762391-1-dave@stgolabs.net/)

I think those all landed upstream today and are getting pulled into qemu stable as well.

> 
> Applies against latest 'next' from cxl.git.
> 
> [0] https://lore.kernel.org/all/20250617193611.564668-1-dave@stgolabs.net/
> 
> Thanks!
> 
> Davidlohr Bueso (1):
>   cxl/mbox: Support Media Operation
> 
>  Documentation/ABI/testing/sysfs-bus-cxl |  31 +++-
>  drivers/cxl/core/mbox.c                 | 235 ++++++++++++++++++++++++
>  drivers/cxl/core/memdev.c               |  45 ++++-
>  drivers/cxl/cxlmem.h                    |  59 ++++++
>  drivers/cxl/pci.c                       |   4 +
>  5 files changed, 369 insertions(+), 5 deletions(-)
> 
> --
> 2.39.5
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] cxl/mbox: Support Media Operation
  2026-03-23 20:40 ` [PATCH 1/1] cxl/mbox: Support Media Operation Davidlohr Bueso
@ 2026-03-24 16:18   ` Jonathan Cameron
  2026-03-24 18:40     ` Davidlohr Bueso
  2026-03-25 13:47   ` kernel test robot
  2026-03-25 15:55   ` kernel test robot
  2 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2026-03-24 16:18 UTC (permalink / raw)
  To: Davidlohr Bueso
  Cc: dave.jiang, alison.schofield, ira.weiny, dan.j.williams,
	dongjoo.seo1, anisa.su, linux-cxl

On Mon, 23 Mar 2026 13:40:13 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:

> Add support for the Media Operation command (opcode 4402h) which
> enables targeted sanitize and zero operations on specific DPA
> ranges, as defined in CXL 4.0 Section 8.2.10.9.5.3.
> 
> Run Discovery during probe to learn the device's DPA range granularity
> and which operations are supported, and expose to sysfs accordingly:
> 
>   o 'security/sanitize' is multiplexed to allow single ranged input.
>   o 'security/zero' is added, also with single ranged input.
> 
> Operations are only allowed on DPA ranges that do not overlap any
> hardware-committed endpoint decoder to ensure not corrupting active
> mappings. Background operations are handled synchronously.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
Hi Davidlohr,

Various comments inline,

Thanks,

Jonathan

> ---
>  Documentation/ABI/testing/sysfs-bus-cxl |  31 +++-
>  drivers/cxl/core/mbox.c                 | 235 ++++++++++++++++++++++++
>  drivers/cxl/core/memdev.c               |  45 ++++-
>  drivers/cxl/cxlmem.h                    |  59 ++++++
>  drivers/cxl/pci.c                       |   4 +
>  5 files changed, 369 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index c80a1b5a03db..da8f8dc92a5a 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -117,10 +117,12 @@ Contact:	linux-cxl@vger.kernel.org
>  Description:
>  		(RO) Reading this file will display the CXL security state for
>  		that device. Such states can be: 'disabled', 'sanitize', when
> -		a sanitization is currently underway; or those available only
> -		for persistent memory: 'locked', 'unlocked' or 'frozen'. This
> -		sysfs entry is select/poll capable from userspace to notify
> -		upon completion of a sanitize operation.
> +		a whole-device sanitization is currently underway; or those
> +		available only for persistent memory: 'locked', 'unlocked' or
> +		'frozen'. This sysfs entry is select/poll capable from
> +		userspace to notify upon completion of a sanitize operation.
> +		Targeted range sanitize via Media Operation does not affect
> +		this state as it completes synchronously.
>  
>  
>  What:           /sys/bus/cxl/devices/memX/security/sanitize
> @@ -141,6 +143,13 @@ Description:
>  		states. If this file is not present, then there is no hardware
>  		support for the operation.
>  
> +		If the device supports the Media Operation command with the
> +		sanitize operation, a DPA range may be written as 'start
> +		length' in bytes to sanitize a targeted region of media.
> +		The start address and length must be aligned to the device's
> +		media operation granularity. The sanitize method applied to
> +		the range is the same as for whole-device sanitize.
> +
>  
>  What            /sys/bus/cxl/devices/memX/security/erase
>  Date:           June, 2023
> @@ -158,6 +167,20 @@ Description:
>  		support for the operation.
>  
>  
> +What:		/sys/bus/cxl/devices/memX/security/zero
> +Date:		March, 2026
> +KernelVersion:	v7.1
> +Contact:	linux-cxl@vger.kernel.org
> +Description:
> +		(WO) Write a DPA range as 'start length' in bytes to this
> +		attribute to zero a specific region of device media via the
> +		Media Operation command. After completion, reads from the
> +		specified range will return zero.

This is only true if we've flushed any caches on the CPU Side of the link.
Will happen anyway I think as part of making the memory available.
Perhaps not worth mentioning that complexity here.

> The start address and
> +		length must be aligned to the device's media operation
> +		granularity. If this file is not present, then the device
> +		does not support the Media Operation zero command.
> +
> +
>  What:		/sys/bus/cxl/devices/memX/firmware/
>  Date:		April, 2023
>  KernelVersion:	v6.5
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index aaa5c6277ebf..b7047cb9abed 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c

>  
> +/**
> + * cxl_media_op_discover() - Discover supported media operation
> + * @mds: The device data for the operation
> + *
> + * Discover any available Media Operations.
> + *
> + * Return: 0 on success or if Media Operation is not supported,
> + * negative error code on failure.
> + */
> +#define CXL_MEDIA_OP_MAX_OPS 2
> +
> +int cxl_media_op_discover(struct cxl_memdev_state *mds)
> +{
> +	size_t out_size, in_size;
> +	void *payload_in;
> +	struct cxl_mbox_cmd mbox_cmd;
> +	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
> +	struct cxl_mbox_media_op_input *payload;
> +	struct cxl_mbox_media_op_discovery_args *args;

Ah. I'd forgotten this spec oddity of two different types
of trailing member, or technically a flex array in the middle
but one that is defined to be zero length if the structure
after it is present.   Maybe cleanest option is
a separate cxl_mbox_media_op_discover_input structure?  Probably
with a comment to say there is a zero length array in the middle.

> +	struct cxl_mbox_media_op_discovery_out *disc_out;
> +	int rc, i;
> +	u16 num_returned;
> +
> +	if (!test_bit(CXL_SEC_ENABLED_MEDIA_OPERATIONS,
> +		      mds->security.enabled_cmds))
> +		return 0;
> +
> +	in_size = sizeof(*payload) + sizeof(*args);
> +	payload_in = kzalloc(in_size, GFP_KERNEL);

With a discovery specific input structure can use kzalloc_obj() and
__free.

> +	if (!payload_in)
> +		return -ENOMEM;
> +
> +	payload = payload_in;
> +	payload->class = CXL_MEDIA_OP_CLASS_GENERAL;
> +	payload->subclass = CXL_MEDIA_OP_SUBCLASS_DISCOVERY;
> +	payload->dpa_range_count = 0;
> +
> +	args = payload_in + sizeof(*payload);
> +	args->start_index = 0;
> +	args->num_ops = cpu_to_le16(CXL_MEDIA_OP_MAX_OPS);
> +
> +	out_size = sizeof(*disc_out) +
> +		   CXL_MEDIA_OP_MAX_OPS * sizeof(disc_out->ops[0]);

kzalloc_flex() and __free() again.


> +	disc_out = kzalloc(out_size, GFP_KERNEL);
> +	if (!disc_out) {
> +		kfree(payload_in);

If you don't switch to __free(), use a second label in error path
to just free this.

> +		return -ENOMEM;
> +	}
> +
> +	mbox_cmd = (struct cxl_mbox_cmd) {
> +		.opcode = CXL_MBOX_OP_MEDIA_OPERATION,
> +		.payload_in = payload_in,
> +		.size_in = in_size,
> +		.payload_out = disc_out,
> +		.size_out = out_size,
> +		.min_out = sizeof(*disc_out),
> +		.poll_count = 1,
> +		.poll_interval_ms = 1000,
> +	};
> +
> +	rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
> +	if (rc < 0) {
> +		dev_dbg(cxl_mbox->host,
> +			"Media Operation Discovery failed: %d\n", rc);
> +		goto out;
> +	}
> +
> +	mds->media_op.granularity = le64_to_cpu(disc_out->granularity);
> +	num_returned = min_t(u16, le16_to_cpu(disc_out->num_returned),
> +			     CXL_MEDIA_OP_MAX_OPS);

If it claims to have returned more than we asked for, why not error out?
I might be missing something.

> +
> +	for (i = 0; i < num_returned; i++) {
> +		u8 cls = disc_out->ops[i].class;
> +		u8 sub = disc_out->ops[i].subclass;
> +
> +		if (cls == CXL_MEDIA_OP_CLASS_SANITIZE &&
> +		    sub == CXL_MEDIA_OP_SUBCLASS_SANITIZE)
> +			mds->media_op.sanitize_supported = true;
> +		if (cls == CXL_MEDIA_OP_CLASS_SANITIZE &&
> +		    sub == CXL_MEDIA_OP_SUBCLASS_ZERO)
> +			mds->media_op.zero_supported = true;
> +	}
> +
> +	dev_dbg(cxl_mbox->host,
> +		"Media Operation: granularity=%llu sanitize=%d zero=%d\n",
> +		mds->media_op.granularity,
> +		mds->media_op.sanitize_supported,
> +		mds->media_op.zero_supported);
> +
> +out:
> +	kfree(disc_out);
> +	kfree(payload_in);
> +	return rc;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_media_op_discover, "CXL");
> +
>  void cxl_event_trace_record(struct cxl_memdev *cxlmd,
>  			    enum cxl_event_log_type type,
>  			    enum cxl_event_type event_type,
> @@ -1308,6 +1408,141 @@ int cxl_mem_sanitize(struct cxl_memdev *cxlmd, u16 cmd)
>  	return -EBUSY;
>  }
>  
> +static int __cxl_mem_media_op(struct cxl_memdev_state *mds, u8 class,
> +			      u8 subclass, u64 dpa_start, u64 dpa_length)
> +{
> +	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
> +	struct cxl_mbox_media_op_input *payload;
> +	struct cxl_mbox_cmd mbox_cmd;
> +	u64 granularity;
> +	size_t size;
> +	int rc;
> +
> +	if (!test_bit(CXL_SEC_ENABLED_MEDIA_OPERATIONS,
> +		      mds->security.enabled_cmds))
> +		return -EOPNOTSUPP;
> +
> +	/* ensure the specific operation was reported by Discovery */
> +	if (class == CXL_MEDIA_OP_CLASS_SANITIZE &&
> +	    subclass == CXL_MEDIA_OP_SUBCLASS_SANITIZE &&
> +	    !mds->media_op.sanitize_supported)
> +		return -EOPNOTSUPP;
> +	if (class == CXL_MEDIA_OP_CLASS_SANITIZE &&
> +	    subclass == CXL_MEDIA_OP_SUBCLASS_ZERO &&
> +	    !mds->media_op.zero_supported)
> +		return -EOPNOTSUPP;

Maybe nest multiple ifs.
	if (class == CXL_MEDIA_OP_CLASS_SANITIZE) {
		if (subclass == CXL_MEDIA_OP_SUBCLASS_SANITIZE &&
		    !mds->media_op.sanitize_supported)
`			return -EOPNOTSUPP;
		if (subclass = CXL_MEDIA_OP_SUBCLASS_ZERO &&
		    !mds->media_op.zero_supported)
			return -EOPNOTSUPP;
	}
Doesn't matter much whilst we have only two, but if we get lots it
may make them easier to find in the code.

...

> +
> +	size = sizeof(*payload) + sizeof(payload->dpa_range_list[0]);
> +	payload = kzalloc(size, GFP_KERNEL);
	payload = kzalloc_flex(*payload, dpa_range_list, 1);

I think... (not gotten used to these yet so I'm not 100% sure on syntax).

> +	if (!payload)
> +		return -ENOMEM;
> +
> +	payload->class = class;
> +	payload->subclass = subclass;
> +	payload->dpa_range_count = cpu_to_le32(1);
> +	payload->dpa_range_list[0].starting_dpa = cpu_to_le64(dpa_start);
> +	payload->dpa_range_list[0].length = cpu_to_le64(dpa_length);
> +
> +	mbox_cmd = (struct cxl_mbox_cmd) {
> +		.opcode = CXL_MBOX_OP_MEDIA_OPERATION,
> +		.payload_in = payload,
> +		.size_in = size,
> +		.poll_count = 60,
> +		.poll_interval_ms = 1000,
> +	};
> +
> +	rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
> +	kfree(payload);
Apply some __free() magic.
> +
> +	if (rc < 0) {
> +		dev_dbg(cxl_mbox->host,
> +			"Media Operation (class=%u sub=%u) failed: %d\n",
> +			class, subclass, rc);
> +		return rc;
> +	}
> +
> +	return 0;
> +}

> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 99e422594885..a78a0469fb41 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c

> @@ -538,13 +572,22 @@ static umode_t cxl_memdev_security_visible(struct kobject *kobj,
>  	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
>  
>  	if (a == &dev_attr_security_sanitize.attr &&
> -	    !test_bit(CXL_SEC_ENABLED_SANITIZE, mds->security.enabled_cmds))
> +	    !test_bit(CXL_SEC_ENABLED_SANITIZE, mds->security.enabled_cmds) &&
> +	    !(test_bit(CXL_SEC_ENABLED_MEDIA_OPERATIONS,
> +		       mds->security.enabled_cmds) &&

I'd put that on one slightly long line for better readability.

Maybe add a comment on the dual use of the sanitize attribute.
That had me confused for a bit.


> +	      mds->media_op.sanitize_supported))
>  		return 0;
>  
>  	if (a == &dev_attr_security_erase.attr &&
>  	    !test_bit(CXL_SEC_ENABLED_SECURE_ERASE, mds->security.enabled_cmds))
>  		return 0;
>  
> +	if (a == &dev_attr_security_zero.attr &&
> +	    (!test_bit(CXL_SEC_ENABLED_MEDIA_OPERATIONS,
> +		       mds->security.enabled_cmds) ||

I'd put the two lines above on line line.  Bit log but more redable.

> +	     !mds->media_op.zero_supported))
> +		return 0;
> +
>  	return a->mode;
>  }
>  
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 02054f233fc5..cde613b25581 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h

> +/* Media Operation classes and subclasses (CXL 4.0 Table 8-331) */
> +#define CXL_MEDIA_OP_CLASS_GENERAL	0x00
> +#define CXL_MEDIA_OP_CLASS_SANITIZE	0x01
> +
> +#define CXL_MEDIA_OP_SUBCLASS_DISCOVERY	0x00
Maybe worth associating these more explicitly with which class they are
in?  Bit tricky to do without long names, but perhaps worth doing even
so.

> +#define CXL_MEDIA_OP_SUBCLASS_SANITIZE	0x00
> +#define CXL_MEDIA_OP_SUBCLASS_ZERO	0x01
> +
> +/* Media Operation DPA Range (CXL 4.0 Table 8-330) */
> +struct cxl_media_op_dpa_range {
> +	__le64 starting_dpa;
> +	__le64 length;
> +} __packed;
> +
> +/* Media Operation Input Payload (CXL 4.0 Table 8-329) */
> +struct cxl_mbox_media_op_input {
> +	u8 class;
> +	u8 subclass;
> +	u8 rsvd[2];
> +	__le32 dpa_range_count;
> +	struct cxl_media_op_dpa_range dpa_range_list[];

__counted_by_le(dpa_range_count)

> +} __packed;
> +
> +/* Discovery operation-specific arguments (CXL 4.0 Table 8-332) */
> +struct cxl_mbox_media_op_discovery_args {
> +	__le16 start_index;
> +	__le16 num_ops;
> +} __packed;
> +
> +/* Discovery output payload (CXL 4.0 Table 8-333) */
> +struct cxl_mbox_media_op_discovery_out {
> +	__le64 granularity;
> +	__le16 total_supported;
> +	__le16 num_returned;
> +	struct {
> +		u8 class;
> +		u8 subclass;
> +	} __packed ops[];  

__counted_by_le(num_returned) ?

> +} __packed;



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 0/1] cxl: Media Operations
  2026-03-24 15:32 ` [RFC PATCH 0/1] cxl: Media Operations Jonathan Cameron
@ 2026-03-24 18:07   ` Davidlohr Bueso
  2026-03-25 12:06     ` Jonathan Cameron
  0 siblings, 1 reply; 10+ messages in thread
From: Davidlohr Bueso @ 2026-03-24 18:07 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: dave.jiang, alison.schofield, ira.weiny, dan.j.williams,
	dongjoo.seo1, anisa.su, linux-cxl

On Tue, 24 Mar 2026, Jonathan Cameron wrote:

>On Mon, 23 Mar 2026 13:40:12 -0700
>Davidlohr Bueso <dave@stgolabs.net> wrote:
>
>> Hello,
>>
>> This implements support for Media Operation cmd. It is marked RFC because of these
>> noteworthy points:
>>
>>  (i) Sysfs interfaces. These changes implement 'security/zero' and multiplex
>>      'security/sanitize' to allow dpa, len pairs for the range (no impact for
>>      current usage as whole-device). This breaks the 1 value per file "rule",
>>      and would be a first for drivers/cxl/*, so am I open to suggestions.
>
>Value is a vague sort of term.  It's one thing (an range) so seems fine to me.

I agree.

>
>>
>>  (ii) Background cmd handling semantics. Media operations are synchronous,
>>       which for raged zeroing operations makes sense and for range sanitize
>
>ranged
>
>>       differs from the current async handling for whole-device, so user response
>>       differs significantly, something I am not crazy about. Another downside
>>       of this is that it exposes lock hold times to be user-controllable, but is
>>       no different than what we do for fw. Fundamentally, users have the
>>       responsibility to break up the work into sane ranges.
>
>We could apply a limit on this and make that discoverable via another sysfs attribute.
>So let userspace request in say 1Gig chunks.  Would have to check that the device
>supports that granularity though.

If we agree that this will be performed synchronously, then yes, adding a size cap
would be necessary. I don't love having it in ie: security/range_limit(?), but that
could be created at discovery time when we know the granularity. Another way would
be to avoid the extra file and just print the limit in the error message - but I
guess dev_dbg() would be needed to not spam the dmesg. In any case with a cap,
while no guarantees, this also reduces the chances of the user triggering driver timeouts.

>I don't think there is anything in the spec today to let us query how long
>something takes (which would be a better way to bound this).

Correct, that is just something Scan Media allows for - and yes, that's exactly
the sane way to do the work breakup.

Thanks,
Davidlohr

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] cxl/mbox: Support Media Operation
  2026-03-24 16:18   ` Jonathan Cameron
@ 2026-03-24 18:40     ` Davidlohr Bueso
  0 siblings, 0 replies; 10+ messages in thread
From: Davidlohr Bueso @ 2026-03-24 18:40 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: dave.jiang, alison.schofield, ira.weiny, dan.j.williams,
	dongjoo.seo1, anisa.su, linux-cxl

On Tue, 24 Mar 2026, Jonathan Cameron wrote:

>On Mon, 23 Mar 2026 13:40:13 -0700
>Davidlohr Bueso <dave@stgolabs.net> wrote:
>> +What:		/sys/bus/cxl/devices/memX/security/zero
>> +Date:		March, 2026
>> +KernelVersion:	v7.1
>> +Contact:	linux-cxl@vger.kernel.org
>> +Description:
>> +		(WO) Write a DPA range as 'start length' in bytes to this
>> +		attribute to zero a specific region of device media via the
>> +		Media Operation command. After completion, reads from the
>> +		specified range will return zero.
>
>This is only true if we've flushed any caches on the CPU Side of the link.
>Will happen anyway I think as part of making the memory available.

Correct, same as for current sanitize/secure erase, described in the sysfs doc.

>Perhaps not worth mentioning that complexity here.

Still important, I will mention this in the changelog.

>
>> The start address and
>> +		length must be aligned to the device's media operation
>> +		granularity. If this file is not present, then the device
>> +		does not support the Media Operation zero command.
>> +
>> +
>>  What:		/sys/bus/cxl/devices/memX/firmware/
>>  Date:		April, 2023
>>  KernelVersion:	v6.5
>> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
>> index aaa5c6277ebf..b7047cb9abed 100644
>> --- a/drivers/cxl/core/mbox.c
>> +++ b/drivers/cxl/core/mbox.c
>
>>
>> +/**
>> + * cxl_media_op_discover() - Discover supported media operation
>> + * @mds: The device data for the operation
>> + *
>> + * Discover any available Media Operations.
>> + *
>> + * Return: 0 on success or if Media Operation is not supported,
>> + * negative error code on failure.
>> + */
>> +#define CXL_MEDIA_OP_MAX_OPS 2
>> +
>> +int cxl_media_op_discover(struct cxl_memdev_state *mds)
>> +{
>> +	size_t out_size, in_size;
>> +	void *payload_in;
>> +	struct cxl_mbox_cmd mbox_cmd;
>> +	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
>> +	struct cxl_mbox_media_op_input *payload;
>> +	struct cxl_mbox_media_op_discovery_args *args;
>
>Ah. I'd forgotten this spec oddity of two different types
>of trailing member, or technically a flex array in the middle
>but one that is defined to be zero length if the structure
>after it is present.   Maybe cleanest option is
>a separate cxl_mbox_media_op_discover_input structure?  Probably
>with a comment to say there is a zero length array in the middle.

Yeah this is a pita.

...

>> +	mbox_cmd = (struct cxl_mbox_cmd) {
>> +		.opcode = CXL_MBOX_OP_MEDIA_OPERATION,
>> +		.payload_in = payload_in,
>> +		.size_in = in_size,
>> +		.payload_out = disc_out,
>> +		.size_out = out_size,
>> +		.min_out = sizeof(*disc_out),
>> +		.poll_count = 1,
>> +		.poll_interval_ms = 1000,
>> +	};
>> +
>> +	rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
>> +	if (rc < 0) {
>> +		dev_dbg(cxl_mbox->host,
>> +			"Media Operation Discovery failed: %d\n", rc);
>> +		goto out;
>> +	}
>> +
>> +	mds->media_op.granularity = le64_to_cpu(disc_out->granularity);
>> +	num_returned = min_t(u16, le16_to_cpu(disc_out->num_returned),
>> +			     CXL_MEDIA_OP_MAX_OPS);
>
>If it claims to have returned more than we asked for, why not error out?
>I might be missing something.

Made sense at the time, reconsidering I tend to agree.

...

>> +static int __cxl_mem_media_op(struct cxl_memdev_state *mds, u8 class,
>> +			      u8 subclass, u64 dpa_start, u64 dpa_length)
>> +{
>> +	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
>> +	struct cxl_mbox_media_op_input *payload;
>> +	struct cxl_mbox_cmd mbox_cmd;
>> +	u64 granularity;
>> +	size_t size;
>> +	int rc;
>> +
>> +	if (!test_bit(CXL_SEC_ENABLED_MEDIA_OPERATIONS,
>> +		      mds->security.enabled_cmds))
>> +		return -EOPNOTSUPP;
>> +
>> +	/* ensure the specific operation was reported by Discovery */
>> +	if (class == CXL_MEDIA_OP_CLASS_SANITIZE &&
>> +	    subclass == CXL_MEDIA_OP_SUBCLASS_SANITIZE &&
>> +	    !mds->media_op.sanitize_supported)
>> +		return -EOPNOTSUPP;
>> +	if (class == CXL_MEDIA_OP_CLASS_SANITIZE &&
>> +	    subclass == CXL_MEDIA_OP_SUBCLASS_ZERO &&
>> +	    !mds->media_op.zero_supported)
>> +		return -EOPNOTSUPP;
>
>Maybe nest multiple ifs.
>	if (class == CXL_MEDIA_OP_CLASS_SANITIZE) {
>		if (subclass == CXL_MEDIA_OP_SUBCLASS_SANITIZE &&
>		    !mds->media_op.sanitize_supported)
>`			return -EOPNOTSUPP;
>		if (subclass = CXL_MEDIA_OP_SUBCLASS_ZERO &&
>		    !mds->media_op.zero_supported)
>			return -EOPNOTSUPP;
>	}
>Doesn't matter much whilst we have only two, but if we get lots it
>may make them easier to find in the code.

I will use a switch.

... and ack to the rest of the feedback.

Thanks,
Davidlohr

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 0/1] cxl: Media Operations
  2026-03-24 18:07   ` Davidlohr Bueso
@ 2026-03-25 12:06     ` Jonathan Cameron
  2026-03-27 15:17       ` John Groves
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2026-03-25 12:06 UTC (permalink / raw)
  To: Davidlohr Bueso
  Cc: dave.jiang, alison.schofield, ira.weiny, dan.j.williams,
	dongjoo.seo1, anisa.su, linux-cxl, John Groves, John Groves

On Tue, 24 Mar 2026 11:07:54 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:

> On Tue, 24 Mar 2026, Jonathan Cameron wrote:
> 
> >On Mon, 23 Mar 2026 13:40:12 -0700
> >Davidlohr Bueso <dave@stgolabs.net> wrote:
> >  
> >> Hello,
> >>
> >> This implements support for Media Operation cmd. It is marked RFC because of these
> >> noteworthy points:
> >>
> >>  (i) Sysfs interfaces. These changes implement 'security/zero' and multiplex
> >>      'security/sanitize' to allow dpa, len pairs for the range (no impact for
> >>      current usage as whole-device). This breaks the 1 value per file "rule",
> >>      and would be a first for drivers/cxl/*, so am I open to suggestions.  
> >
> >Value is a vague sort of term.  It's one thing (an range) so seems fine to me.  
> 
> I agree.
> 
> >  
> >>
> >>  (ii) Background cmd handling semantics. Media operations are synchronous,
> >>       which for raged zeroing operations makes sense and for range sanitize  
> >
> >ranged
> >  
> >>       differs from the current async handling for whole-device, so user response
> >>       differs significantly, something I am not crazy about. Another downside
> >>       of this is that it exposes lock hold times to be user-controllable, but is
> >>       no different than what we do for fw. Fundamentally, users have the
> >>       responsibility to break up the work into sane ranges.  
> >
> >We could apply a limit on this and make that discoverable via another sysfs attribute.
> >So let userspace request in say 1Gig chunks.  Would have to check that the device
> >supports that granularity though.  
> 
> If we agree that this will be performed synchronously, then yes, adding a size cap
> would be necessary. I don't love having it in ie: security/range_limit(?), but that
> could be created at discovery time when we know the granularity. Another way would
> be to avoid the extra file and just print the limit in the error message - but I
> guess dev_dbg() would be needed to not spam the dmesg. In any case with a cap,
> while no guarantees, this also reduces the chances of the user triggering driver timeouts.
> 
> >I don't think there is anything in the spec today to let us query how long
> >something takes (which would be a better way to bound this).  
> 
> Correct, that is just something Scan Media allows for - and yes, that's exactly
> the sane way to do the work breakup.
> 
Yo John,

I'm sure you are keeping a list of potential improvements for the appropriate group
that I believe you have some leadership role in ;)

I'd like devices to provide info to allow us to establish an upper bound on how
long a zero / sanitize media operation would take.  This one doesn't need to be
code first so we can consider this a potential feature request.

Jonathan

> Thanks,
> Davidlohr


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] cxl/mbox: Support Media Operation
  2026-03-23 20:40 ` [PATCH 1/1] cxl/mbox: Support Media Operation Davidlohr Bueso
  2026-03-24 16:18   ` Jonathan Cameron
@ 2026-03-25 13:47   ` kernel test robot
  2026-03-25 15:55   ` kernel test robot
  2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2026-03-25 13:47 UTC (permalink / raw)
  To: Davidlohr Bueso, dave.jiang
  Cc: oe-kbuild-all, jonathan.cameron, alison.schofield, ira.weiny,
	dan.j.williams, dongjoo.seo1, anisa.su, linux-cxl, dave

Hi Davidlohr,

kernel test robot noticed the following build warnings:

[auto build test WARNING on cxl/next]
[also build test WARNING on next-20260324]
[cannot apply to linus/master cxl/pending v7.0-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Davidlohr-Bueso/cxl-mbox-Support-Media-Operation/20260325-172343
base:   https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git next
patch link:    https://lore.kernel.org/r/20260323204013.4010634-2-dave%40stgolabs.net
patch subject: [PATCH 1/1] cxl/mbox: Support Media Operation
config: x86_64-rhel-9.4-ltp (https://download.01.org/0day-ci/archive/20260325/202603251458.SwLIShRj-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260325/202603251458.SwLIShRj-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603251458.SwLIShRj-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: drivers/cxl/core/mbox.c:908 expecting prototype for cxl_media_op_discover(). Prototype was for CXL_MEDIA_OP_MAX_OPS() instead

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] cxl/mbox: Support Media Operation
  2026-03-23 20:40 ` [PATCH 1/1] cxl/mbox: Support Media Operation Davidlohr Bueso
  2026-03-24 16:18   ` Jonathan Cameron
  2026-03-25 13:47   ` kernel test robot
@ 2026-03-25 15:55   ` kernel test robot
  2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2026-03-25 15:55 UTC (permalink / raw)
  To: Davidlohr Bueso, dave.jiang
  Cc: oe-kbuild-all, jonathan.cameron, alison.schofield, ira.weiny,
	dan.j.williams, dongjoo.seo1, anisa.su, linux-cxl, dave

Hi Davidlohr,

kernel test robot noticed the following build warnings:

[auto build test WARNING on cxl/next]
[also build test WARNING on next-20260324]
[cannot apply to linus/master cxl/pending v7.0-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Davidlohr-Bueso/cxl-mbox-Support-Media-Operation/20260325-172343
base:   https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git next
patch link:    https://lore.kernel.org/r/20260323204013.4010634-2-dave%40stgolabs.net
patch subject: [PATCH 1/1] cxl/mbox: Support Media Operation
config: openrisc-randconfig-r071-20260325 (https://download.01.org/0day-ci/archive/20260325/202603252348.nPuXnlRG-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 14.3.0
smatch: v0.5.0-9004-gb810ac53
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260325/202603252348.nPuXnlRG-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603252348.nPuXnlRG-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: drivers/cxl/core/mbox.c:908 expecting prototype for cxl_media_op_discover(). Prototype was for CXL_MEDIA_OP_MAX_OPS() instead

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 0/1] cxl: Media Operations
  2026-03-25 12:06     ` Jonathan Cameron
@ 2026-03-27 15:17       ` John Groves
  0 siblings, 0 replies; 10+ messages in thread
From: John Groves @ 2026-03-27 15:17 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Davidlohr Bueso, dave.jiang, alison.schofield, ira.weiny,
	dan.j.williams, dongjoo.seo1, anisa.su, linux-cxl, John Groves

On 26/03/25 12:06PM, Jonathan Cameron wrote:
> On Tue, 24 Mar 2026 11:07:54 -0700
> Davidlohr Bueso <dave@stgolabs.net> wrote:
> 
> > On Tue, 24 Mar 2026, Jonathan Cameron wrote:
> > 
> > >On Mon, 23 Mar 2026 13:40:12 -0700
> > >Davidlohr Bueso <dave@stgolabs.net> wrote:
> > >  
> > >> Hello,
> > >>
> > >> This implements support for Media Operation cmd. It is marked RFC because of these
> > >> noteworthy points:
> > >>
> > >>  (i) Sysfs interfaces. These changes implement 'security/zero' and multiplex
> > >>      'security/sanitize' to allow dpa, len pairs for the range (no impact for
> > >>      current usage as whole-device). This breaks the 1 value per file "rule",
> > >>      and would be a first for drivers/cxl/*, so am I open to suggestions.  
> > >
> > >Value is a vague sort of term.  It's one thing (an range) so seems fine to me.  
> > 
> > I agree.
> > 
> > >  
> > >>
> > >>  (ii) Background cmd handling semantics. Media operations are synchronous,
> > >>       which for raged zeroing operations makes sense and for range sanitize  
> > >
> > >ranged
> > >  
> > >>       differs from the current async handling for whole-device, so user response
> > >>       differs significantly, something I am not crazy about. Another downside
> > >>       of this is that it exposes lock hold times to be user-controllable, but is
> > >>       no different than what we do for fw. Fundamentally, users have the
> > >>       responsibility to break up the work into sane ranges.  
> > >
> > >We could apply a limit on this and make that discoverable via another sysfs attribute.
> > >So let userspace request in say 1Gig chunks.  Would have to check that the device
> > >supports that granularity though.  
> > 
> > If we agree that this will be performed synchronously, then yes, adding a size cap
> > would be necessary. I don't love having it in ie: security/range_limit(?), but that
> > could be created at discovery time when we know the granularity. Another way would
> > be to avoid the extra file and just print the limit in the error message - but I
> > guess dev_dbg() would be needed to not spam the dmesg. In any case with a cap,
> > while no guarantees, this also reduces the chances of the user triggering driver timeouts.
> > 
> > >I don't think there is anything in the spec today to let us query how long
> > >something takes (which would be a better way to bound this).  
> > 
> > Correct, that is just something Scan Media allows for - and yes, that's exactly
> > the sane way to do the work breakup.
> > 
> Yo John,
> 
> I'm sure you are keeping a list of potential improvements for the appropriate group
> that I believe you have some leadership role in ;)
> 
> I'd like devices to provide info to allow us to establish an upper bound on how
> long a zero / sanitize media operation would take.  This one doesn't need to be
> code first so we can consider this a potential feature request.
> 
> Jonathan
> 
> > Thanks,
> > Davidlohr
> 

Acknowledged. Are you thinking of a one-off for sanitize, or is there
a class of commands that might ought to provide such an upper-bound?

I'd be happy to try to push something through, but the solution ought
to come from somebody who experiences the inconvenience directtly :D

Regards,
John


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-03-27 15:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-23 20:40 [RFC PATCH 0/1] cxl: Media Operations Davidlohr Bueso
2026-03-23 20:40 ` [PATCH 1/1] cxl/mbox: Support Media Operation Davidlohr Bueso
2026-03-24 16:18   ` Jonathan Cameron
2026-03-24 18:40     ` Davidlohr Bueso
2026-03-25 13:47   ` kernel test robot
2026-03-25 15:55   ` kernel test robot
2026-03-24 15:32 ` [RFC PATCH 0/1] cxl: Media Operations Jonathan Cameron
2026-03-24 18:07   ` Davidlohr Bueso
2026-03-25 12:06     ` Jonathan Cameron
2026-03-27 15:17       ` John Groves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox