[PATCH v3 0/7] cxl: Background cmds and device sanitation

[PATCH v3 0/7] cxl: Background cmds and device sanitation

[Davidlohr Bueso]

Hello,

This series adds support for the rest of the security related cxl operations
which are not strictly dependent on PMEM.

Patch 1 adds the required background cmd handling bits (polling and irq).
This is really the main motivation of the series as there are various
features that will need this regardless of sanitation.

Patch 2 adds a new sysfs 'security/state' file, which shows "disabled" if
no security features are available - which is more intuitive than no showing
it.

Patch 3 adds a way to check if the device is actively used by tracking regions
with target the memdevice to sanitize. I realize this might not be what is
desired and wanted to get the expectations for this. This is more of a RFC
patch.

Patches 4-7 add the Sanitation and Secure Erase support, per CXL3.0 specs.

These changes have been tested with both the mock device as well as with qemu[0,1].

Changes from v2 (https://lore.kernel.org/linux-cxl/20221206011501.464916-1-dave@stgolabs.net/):
- Redid the bacground cmd handling to use a syncronous approach instead.
- Added cxl_memdev_active_region() to check if the memdevice is being used
  (actively decoding any HPA ranges).
- Create a new security sysfs directory with a 'state' file which is always visible.
- The sysfs files' to trigger the security commands is only visible if
  cpu_cache_has_invalidate_memregion().
- Added a sanitize test for the mock device.

Please consider for v6.4.

Thanks!

[0]: https://github.com/davidlohr/qemu/commit/64a93a5b824b59d3b547f06f7fbb1269fb4790ce
[1]: https://lore.kernel.org/linux-cxl/20230224194443.1990440-1-dave@stgolabs.net

Davidlohr Bueso (7):
  cxl/mbox: Add background cmd handling machinery
  cxl/security: Add security state sysfs ABI
  cxl/region: Add cxl_memdev_active_region()
  cxl/mem: Support Sanitation
  cxl/test: Add "Sanitize" opcode support
  cxl/mem: Support Secure Erase
  cxl/test: Add "Secure Erase" opcode support

 Documentation/ABI/testing/sysfs-bus-cxl |  34 +++++++
 drivers/cxl/core/mbox.c                 | 117 +++++++++++++++++++++++
 drivers/cxl/core/memdev.c               | 119 ++++++++++++++++++++++++
 drivers/cxl/core/region.c               |  33 ++++++-
 drivers/cxl/cxl.h                       |  13 +++
 drivers/cxl/cxlmem.h                    |  14 +++
 drivers/cxl/pci.c                       | 100 +++++++++++++++++++-
 tools/testing/cxl/test/mem.c            |  52 +++++++++++
 8 files changed, 476 insertions(+), 6 deletions(-)

--
2.39.2

Re: [PATCH v3 0/7] cxl: Background cmds and device sanitation

[Davidlohr Bueso]

+Cc Dave (sorry).

On Fri, 24 Feb 2023, Davidlohr Bueso wrote:

>Hello,
>
>This series adds support for the rest of the security related cxl operations
>which are not strictly dependent on PMEM.
>
>Patch 1 adds the required background cmd handling bits (polling and irq).
>This is really the main motivation of the series as there are various
>features that will need this regardless of sanitation.
>
>Patch 2 adds a new sysfs 'security/state' file, which shows "disabled" if
>no security features are available - which is more intuitive than no showing
>it.
>
>Patch 3 adds a way to check if the device is actively used by tracking regions
>with target the memdevice to sanitize. I realize this might not be what is
>desired and wanted to get the expectations for this. This is more of a RFC
>patch.
>
>Patches 4-7 add the Sanitation and Secure Erase support, per CXL3.0 specs.
>
>These changes have been tested with both the mock device as well as with qemu[0,1].
>
>Changes from v2 (https://lore.kernel.org/linux-cxl/20221206011501.464916-1-dave@stgolabs.net/):
>- Redid the bacground cmd handling to use a syncronous approach instead.
>- Added cxl_memdev_active_region() to check if the memdevice is being used
>  (actively decoding any HPA ranges).
>- Create a new security sysfs directory with a 'state' file which is always visible.
>- The sysfs files' to trigger the security commands is only visible if
>  cpu_cache_has_invalidate_memregion().
>- Added a sanitize test for the mock device.
>
>Please consider for v6.4.
>
>Thanks!
>
>[0]: https://github.com/davidlohr/qemu/commit/64a93a5b824b59d3b547f06f7fbb1269fb4790ce
>[1]: https://lore.kernel.org/linux-cxl/20230224194443.1990440-1-dave@stgolabs.net
>
>Davidlohr Bueso (7):
>  cxl/mbox: Add background cmd handling machinery
>  cxl/security: Add security state sysfs ABI
>  cxl/region: Add cxl_memdev_active_region()
>  cxl/mem: Support Sanitation
>  cxl/test: Add "Sanitize" opcode support
>  cxl/mem: Support Secure Erase
>  cxl/test: Add "Secure Erase" opcode support
>
> Documentation/ABI/testing/sysfs-bus-cxl |  34 +++++++
> drivers/cxl/core/mbox.c                 | 117 +++++++++++++++++++++++
> drivers/cxl/core/memdev.c               | 119 ++++++++++++++++++++++++
> drivers/cxl/core/region.c               |  33 ++++++-
> drivers/cxl/cxl.h                       |  13 +++
> drivers/cxl/cxlmem.h                    |  14 +++
> drivers/cxl/pci.c                       | 100 +++++++++++++++++++-
> tools/testing/cxl/test/mem.c            |  52 +++++++++++
> 8 files changed, 476 insertions(+), 6 deletions(-)
>
>--
>2.39.2
>

[PATCH 1/7] cxl/mbox: Add background cmd handling machinery

[Davidlohr Bueso]

This adds support for handling background operations, as defined in
the CXL 3.0 spec. Commands that can take too long (over ~2 seconds)
can run in the background asynchronously (to the hardware). Currently
these are limited to Maintenance, transfer/activate Firmware, Scan
Media, Sanitize (aka overwrite), and VPPB bind/unbind.

The driver will deal with such commands synchronously, blocking
all other incoming commands for a specified period of time, allowing
time-slicing the command such that the caller can send incremental
requests to avoid monopolizing the driver/device. This approach
makes the code simpler, where any out of sync (timeout) between the
driver and hardware is just disregarded as an invalid state until
the next successful submission.

On devices where mbox interrupts are supported, this will still use
a poller that will wakeup in the specified wait intervals. The irq
handler will simply awake a blocked cmd, which is also safe vs a
task that is either waking (timing out) or already awoken.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 drivers/cxl/cxl.h    |   7 +++
 drivers/cxl/cxlmem.h |   6 +++
 drivers/cxl/pci.c    | 100 +++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 109 insertions(+), 4 deletions(-)

diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index d853a0238ad7..b834e55375e3 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -176,14 +176,21 @@ static inline int ways_to_eiw(unsigned int ways, u8 *eiw)
 /* CXL 2.0 8.2.8.4 Mailbox Registers */
 #define CXLDEV_MBOX_CAPS_OFFSET 0x00
 #define   CXLDEV_MBOX_CAP_PAYLOAD_SIZE_MASK GENMASK(4, 0)
+#define   CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK GENMASK(10, 7)
+#define   CXLDEV_MBOX_CAP_BG_CMD_IRQ BIT(6)
 #define CXLDEV_MBOX_CTRL_OFFSET 0x04
 #define   CXLDEV_MBOX_CTRL_DOORBELL BIT(0)
+#define   CXLDEV_MBOX_CTRL_BG_CMD_IRQ BIT(2)
 #define CXLDEV_MBOX_CMD_OFFSET 0x08
 #define   CXLDEV_MBOX_CMD_COMMAND_OPCODE_MASK GENMASK_ULL(15, 0)
 #define   CXLDEV_MBOX_CMD_PAYLOAD_LENGTH_MASK GENMASK_ULL(36, 16)
 #define CXLDEV_MBOX_STATUS_OFFSET 0x10
+#define   CXLDEV_MBOX_STATUS_BG_CMD BIT(0)
 #define   CXLDEV_MBOX_STATUS_RET_CODE_MASK GENMASK_ULL(47, 32)
 #define CXLDEV_MBOX_BG_CMD_STATUS_OFFSET 0x18
+#define   CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK GENMASK_ULL(15, 0)
+#define   CXLDEV_MBOX_BG_CMD_COMMAND_PCT_MASK GENMASK_ULL(22, 16)
+#define   CXLDEV_MBOX_BG_CMD_COMMAND_RC_MASK GENMASK_ULL(47, 32)
 #define CXLDEV_MBOX_PAYLOAD_OFFSET 0x20
 
 /*
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index ccbafc05a636..934076254d52 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -108,6 +108,9 @@ static inline struct cxl_ep *cxl_ep_load(struct cxl_port *port,
  *            variable sized output commands, it tells the exact number of bytes
  *            written.
  * @min_out: (input) internal command output payload size validation
+ * @poll_count: (input)  Number of timeouts to attempt.
+ * @poll_interval: (input) Number of ms between mailbox background command
+ *                 polling intervals timeouts.
  * @return_code: (output) Error code returned from hardware.
  *
  * This is the primary mechanism used to send commands to the hardware.
@@ -123,6 +126,8 @@ struct cxl_mbox_cmd {
 	size_t size_in;
 	size_t size_out;
 	size_t min_out;
+	int poll_count;
+	u64 poll_interval;
 	u16 return_code;
 };
 
@@ -322,6 +327,7 @@ enum cxl_opcode {
 	CXL_MBOX_OP_GET_SCAN_MEDIA_CAPS	= 0x4303,
 	CXL_MBOX_OP_SCAN_MEDIA		= 0x4304,
 	CXL_MBOX_OP_GET_SCAN_MEDIA	= 0x4305,
+	CXL_MBOX_OP_SANITIZE		= 0x4400,
 	CXL_MBOX_OP_GET_SECURITY_STATE	= 0x4500,
 	CXL_MBOX_OP_SET_PASSPHRASE	= 0x4501,
 	CXL_MBOX_OP_DISABLE_PASSPHRASE	= 0x4502,
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 60b23624d167..26b6105e2797 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -52,6 +52,8 @@ static unsigned short mbox_ready_timeout = 60;
 module_param(mbox_ready_timeout, ushort, 0644);
 MODULE_PARM_DESC(mbox_ready_timeout, "seconds to wait for mailbox ready");
 
+static DECLARE_WAIT_QUEUE_HEAD(mbox_wait);
+
 static int cxl_pci_mbox_wait_for_doorbell(struct cxl_dev_state *cxlds)
 {
 	const unsigned long start = jiffies;
@@ -85,6 +87,25 @@ static int cxl_pci_mbox_wait_for_doorbell(struct cxl_dev_state *cxlds)
 			    status & CXLMDEV_DEV_FATAL ? " fatal" : "",        \
 			    status & CXLMDEV_FW_HALT ? " firmware-halt" : "")
 
+static irqreturn_t cxl_mbox_irq(int irq, void *id)
+{
+	/* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
+	wake_up(&mbox_wait);
+	return IRQ_HANDLED;
+}
+
+static bool cxl_mbox_background_complete(struct cxl_dev_state *cxlds)
+{
+	u64 bgcmd_status_reg;
+	u32 pct;
+
+	bgcmd_status_reg = readq(cxlds->regs.mbox +
+				 CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
+	pct = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_PCT_MASK, bgcmd_status_reg);
+
+	return pct == 100;
+}
+
 /**
  * __cxl_pci_mbox_send_cmd() - Execute a mailbox command
  * @cxlds: The device state to communicate with.
@@ -178,6 +199,56 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
 	mbox_cmd->return_code =
 		FIELD_GET(CXLDEV_MBOX_STATUS_RET_CODE_MASK, status_reg);
 
+	/*
+	 * Handle the background command in a synchronous manner.
+	 *
+	 * All other mailbox commands will serialize/queue on the mbox_mutex,
+	 * which we currently hold. Furthermore this also guarantees that
+	 * cxl_mbox_background_complete() checks are safe amongst each other,
+	 * in that no new bg operation can occur in between.
+	 *
+	 * With the exception of special cases that merit monopolizing the
+	 * driver/device, bg operations are timesliced in accordance with
+	 * the nature of the command being sent.
+	 *
+	 * In the event of timeout, the mailbox state is indeterminate
+	 * until the next successful command submission and the driver
+	 * can get back in sync with the hardware state.
+	 */
+	if (mbox_cmd->return_code == CXL_MBOX_CMD_RC_BACKGROUND) {
+		u64 bg_status_reg;
+		const bool timeslice = mbox_cmd->opcode != CXL_MBOX_OP_SANITIZE;
+
+		dev_dbg(dev, "Mailbox background operation started\n");
+
+		while (1) {
+			if (wait_event_interruptible_timeout(
+				mbox_wait, cxl_mbox_background_complete(cxlds),
+				msecs_to_jiffies(mbox_cmd->poll_interval)) > 0)
+				break;
+
+			if (timeslice && !--mbox_cmd->poll_count)
+				break;
+		}
+
+		if (!cxl_mbox_background_complete(cxlds)) {
+			u64 md_status =
+				readq(cxlds->regs.memdev + CXLMDEV_STATUS_OFFSET);
+
+			cxl_cmd_err(cxlds->dev, mbox_cmd, md_status,
+				    "background timeout");
+			return -ETIMEDOUT;
+		}
+
+		bg_status_reg = readq(cxlds->regs.mbox +
+				      CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
+		mbox_cmd->return_code =
+			FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_RC_MASK,
+				  bg_status_reg);
+
+		dev_dbg(dev, "Mailbox background operation completed\n");
+	}
+
 	if (mbox_cmd->return_code != CXL_MBOX_CMD_RC_SUCCESS) {
 		dev_dbg(dev, "Mailbox operation had an error: %s\n",
 			cxl_mbox_cmd_rc2str(mbox_cmd));
@@ -222,8 +293,11 @@ static int cxl_pci_mbox_send(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *c
 static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds)
 {
 	const int cap = readl(cxlds->regs.mbox + CXLDEV_MBOX_CAPS_OFFSET);
+	struct device *dev = cxlds->dev;
+	struct pci_dev *pdev = to_pci_dev(dev);
 	unsigned long timeout;
 	u64 md_status;
+	int rc, irq;
 
 	timeout = jiffies + mbox_ready_timeout * HZ;
 	do {
@@ -272,6 +346,24 @@ static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds)
 	dev_dbg(cxlds->dev, "Mailbox payload sized %zu",
 		cxlds->payload_size);
 
+	if (!(cap & CXLDEV_MBOX_CAP_BG_CMD_IRQ)) {
+		dev_dbg(dev, "Only Mailbox polling is supported");
+		return 0;
+	}
+
+	irq = pci_irq_vector(pdev,
+			     FIELD_GET(CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK, cap));
+	if (irq < 0)
+		return irq;
+
+	rc = devm_request_irq(dev, irq, cxl_mbox_irq,
+			      IRQF_SHARED, "mailbox", cxlds);
+	if (rc)
+		return rc;
+
+	writel(CXLDEV_MBOX_CTRL_BG_CMD_IRQ,
+	       cxlds->regs.mbox + CXLDEV_MBOX_CTRL_OFFSET);
+
 	return 0;
 }
 
@@ -757,6 +849,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		dev_dbg(&pdev->dev, "Failed to map RAS capability.\n");
 
+	rc = cxl_alloc_irq_vectors(pdev);
+	if (rc)
+		return rc;
+
 	rc = cxl_pci_setup_mailbox(cxlds);
 	if (rc)
 		return rc;
@@ -777,10 +873,6 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		return rc;
 
-	rc = cxl_alloc_irq_vectors(pdev);
-	if (rc)
-		return rc;
-
 	cxlmd = devm_cxl_add_memdev(cxlds);
 	if (IS_ERR(cxlmd))
 		return PTR_ERR(cxlmd);
-- 
2.39.2

Re: [PATCH 1/7] cxl/mbox: Add background cmd handling machinery

[Dave Jiang]

On 2/24/23 12:46 PM, Davidlohr Bueso wrote:
> This adds support for handling background operations, as defined in
> the CXL 3.0 spec. Commands that can take too long (over ~2 seconds)
> can run in the background asynchronously (to the hardware). Currently
> these are limited to Maintenance, transfer/activate Firmware, Scan
> Media, Sanitize (aka overwrite), and VPPB bind/unbind.
> 
> The driver will deal with such commands synchronously, blocking
> all other incoming commands for a specified period of time, allowing
> time-slicing the command such that the caller can send incremental
> requests to avoid monopolizing the driver/device. This approach
> makes the code simpler, where any out of sync (timeout) between the
> driver and hardware is just disregarded as an invalid state until
> the next successful submission.
> 
> On devices where mbox interrupts are supported, this will still use
> a poller that will wakeup in the specified wait intervals. The irq
> handler will simply awake a blocked cmd, which is also safe vs a
> task that is either waking (timing out) or already awoken.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>

Just a minor comment inline. Otherwise
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>   drivers/cxl/cxl.h    |   7 +++
>   drivers/cxl/cxlmem.h |   6 +++
>   drivers/cxl/pci.c    | 100 +++++++++++++++++++++++++++++++++++++++++--
>   3 files changed, 109 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index d853a0238ad7..b834e55375e3 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -176,14 +176,21 @@ static inline int ways_to_eiw(unsigned int ways, u8 *eiw)
>   /* CXL 2.0 8.2.8.4 Mailbox Registers */
>   #define CXLDEV_MBOX_CAPS_OFFSET 0x00
>   #define   CXLDEV_MBOX_CAP_PAYLOAD_SIZE_MASK GENMASK(4, 0)
> +#define   CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK GENMASK(10, 7)
> +#define   CXLDEV_MBOX_CAP_BG_CMD_IRQ BIT(6)
>   #define CXLDEV_MBOX_CTRL_OFFSET 0x04
>   #define   CXLDEV_MBOX_CTRL_DOORBELL BIT(0)
> +#define   CXLDEV_MBOX_CTRL_BG_CMD_IRQ BIT(2)
>   #define CXLDEV_MBOX_CMD_OFFSET 0x08
>   #define   CXLDEV_MBOX_CMD_COMMAND_OPCODE_MASK GENMASK_ULL(15, 0)
>   #define   CXLDEV_MBOX_CMD_PAYLOAD_LENGTH_MASK GENMASK_ULL(36, 16)
>   #define CXLDEV_MBOX_STATUS_OFFSET 0x10
> +#define   CXLDEV_MBOX_STATUS_BG_CMD BIT(0)
>   #define   CXLDEV_MBOX_STATUS_RET_CODE_MASK GENMASK_ULL(47, 32)
>   #define CXLDEV_MBOX_BG_CMD_STATUS_OFFSET 0x18
> +#define   CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK GENMASK_ULL(15, 0)
> +#define   CXLDEV_MBOX_BG_CMD_COMMAND_PCT_MASK GENMASK_ULL(22, 16)
> +#define   CXLDEV_MBOX_BG_CMD_COMMAND_RC_MASK GENMASK_ULL(47, 32)
>   #define CXLDEV_MBOX_PAYLOAD_OFFSET 0x20
>   
>   /*
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index ccbafc05a636..934076254d52 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -108,6 +108,9 @@ static inline struct cxl_ep *cxl_ep_load(struct cxl_port *port,
>    *            variable sized output commands, it tells the exact number of bytes
>    *            written.
>    * @min_out: (input) internal command output payload size validation
> + * @poll_count: (input)  Number of timeouts to attempt.
> + * @poll_interval: (input) Number of ms between mailbox background command
> + *                 polling intervals timeouts.
>    * @return_code: (output) Error code returned from hardware.
>    *
>    * This is the primary mechanism used to send commands to the hardware.
> @@ -123,6 +126,8 @@ struct cxl_mbox_cmd {
>   	size_t size_in;
>   	size_t size_out;
>   	size_t min_out;
> +	int poll_count;
> +	u64 poll_interval;
>   	u16 return_code;
>   };
>   
> @@ -322,6 +327,7 @@ enum cxl_opcode {
>   	CXL_MBOX_OP_GET_SCAN_MEDIA_CAPS	= 0x4303,
>   	CXL_MBOX_OP_SCAN_MEDIA		= 0x4304,
>   	CXL_MBOX_OP_GET_SCAN_MEDIA	= 0x4305,
> +	CXL_MBOX_OP_SANITIZE		= 0x4400,
>   	CXL_MBOX_OP_GET_SECURITY_STATE	= 0x4500,
>   	CXL_MBOX_OP_SET_PASSPHRASE	= 0x4501,
>   	CXL_MBOX_OP_DISABLE_PASSPHRASE	= 0x4502,
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 60b23624d167..26b6105e2797 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -52,6 +52,8 @@ static unsigned short mbox_ready_timeout = 60;
>   module_param(mbox_ready_timeout, ushort, 0644);
>   MODULE_PARM_DESC(mbox_ready_timeout, "seconds to wait for mailbox ready");
>   
> +static DECLARE_WAIT_QUEUE_HEAD(mbox_wait);
> +
>   static int cxl_pci_mbox_wait_for_doorbell(struct cxl_dev_state *cxlds)
>   {
>   	const unsigned long start = jiffies;
> @@ -85,6 +87,25 @@ static int cxl_pci_mbox_wait_for_doorbell(struct cxl_dev_state *cxlds)
>   			    status & CXLMDEV_DEV_FATAL ? " fatal" : "",        \
>   			    status & CXLMDEV_FW_HALT ? " firmware-halt" : "")
>   
> +static irqreturn_t cxl_mbox_irq(int irq, void *id)
> +{
> +	/* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
> +	wake_up(&mbox_wait);
> +	return IRQ_HANDLED;
> +}
> +
> +static bool cxl_mbox_background_complete(struct cxl_dev_state *cxlds)
> +{
> +	u64 bgcmd_status_reg;
> +	u32 pct;
> +
> +	bgcmd_status_reg = readq(cxlds->regs.mbox +
> +				 CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
> +	pct = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_PCT_MASK, bgcmd_status_reg);
> +
> +	return pct == 100;
> +}
> +
>   /**
>    * __cxl_pci_mbox_send_cmd() - Execute a mailbox command
>    * @cxlds: The device state to communicate with.
> @@ -178,6 +199,56 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
>   	mbox_cmd->return_code =
>   		FIELD_GET(CXLDEV_MBOX_STATUS_RET_CODE_MASK, status_reg);
>   
> +	/*
> +	 * Handle the background command in a synchronous manner.
> +	 *
> +	 * All other mailbox commands will serialize/queue on the mbox_mutex,
> +	 * which we currently hold. Furthermore this also guarantees that
> +	 * cxl_mbox_background_complete() checks are safe amongst each other,
> +	 * in that no new bg operation can occur in between.
> +	 *
> +	 * With the exception of special cases that merit monopolizing the
> +	 * driver/device, bg operations are timesliced in accordance with
> +	 * the nature of the command being sent.
> +	 *
> +	 * In the event of timeout, the mailbox state is indeterminate
> +	 * until the next successful command submission and the driver
> +	 * can get back in sync with the hardware state.
> +	 */
> +	if (mbox_cmd->return_code == CXL_MBOX_CMD_RC_BACKGROUND) {
> +		u64 bg_status_reg;
> +		const bool timeslice = mbox_cmd->opcode != CXL_MBOX_OP_SANITIZE;
> +
> +		dev_dbg(dev, "Mailbox background operation started\n");
> +
> +		while (1) {
> +			if (wait_event_interruptible_timeout(
> +				mbox_wait, cxl_mbox_background_complete(cxlds),
> +				msecs_to_jiffies(mbox_cmd->poll_interval)) > 0)
> +				break;
> +
> +			if (timeslice && !--mbox_cmd->poll_count)
> +				break;
> +		}
> +
> +		if (!cxl_mbox_background_complete(cxlds)) {
> +			u64 md_status =
> +				readq(cxlds->regs.memdev + CXLMDEV_STATUS_OFFSET);
> +
> +			cxl_cmd_err(cxlds->dev, mbox_cmd, md_status,
> +				    "background timeout");
> +			return -ETIMEDOUT;
> +		}
> +
> +		bg_status_reg = readq(cxlds->regs.mbox +
> +				      CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
> +		mbox_cmd->return_code =
> +			FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_RC_MASK,
> +				  bg_status_reg);
> +
> +		dev_dbg(dev, "Mailbox background operation completed\n");

May be helpful to also emit the return_code in case of errors.

DJ

> +	}
> +
>   	if (mbox_cmd->return_code != CXL_MBOX_CMD_RC_SUCCESS) {
>   		dev_dbg(dev, "Mailbox operation had an error: %s\n",
>   			cxl_mbox_cmd_rc2str(mbox_cmd));
> @@ -222,8 +293,11 @@ static int cxl_pci_mbox_send(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *c
>   static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds)
>   {
>   	const int cap = readl(cxlds->regs.mbox + CXLDEV_MBOX_CAPS_OFFSET);
> +	struct device *dev = cxlds->dev;
> +	struct pci_dev *pdev = to_pci_dev(dev);
>   	unsigned long timeout;
>   	u64 md_status;
> +	int rc, irq;
>   
>   	timeout = jiffies + mbox_ready_timeout * HZ;
>   	do {
> @@ -272,6 +346,24 @@ static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds)
>   	dev_dbg(cxlds->dev, "Mailbox payload sized %zu",
>   		cxlds->payload_size);
>   
> +	if (!(cap & CXLDEV_MBOX_CAP_BG_CMD_IRQ)) {
> +		dev_dbg(dev, "Only Mailbox polling is supported");
> +		return 0;
> +	}
> +
> +	irq = pci_irq_vector(pdev,
> +			     FIELD_GET(CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK, cap));
> +	if (irq < 0)
> +		return irq;
> +
> +	rc = devm_request_irq(dev, irq, cxl_mbox_irq,
> +			      IRQF_SHARED, "mailbox", cxlds);
> +	if (rc)
> +		return rc;
> +
> +	writel(CXLDEV_MBOX_CTRL_BG_CMD_IRQ,
> +	       cxlds->regs.mbox + CXLDEV_MBOX_CTRL_OFFSET);
> +
>   	return 0;
>   }
>   
> @@ -757,6 +849,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	if (rc)
>   		dev_dbg(&pdev->dev, "Failed to map RAS capability.\n");
>   
> +	rc = cxl_alloc_irq_vectors(pdev);
> +	if (rc)
> +		return rc;
> +
>   	rc = cxl_pci_setup_mailbox(cxlds);
>   	if (rc)
>   		return rc;
> @@ -777,10 +873,6 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	if (rc)
>   		return rc;
>   
> -	rc = cxl_alloc_irq_vectors(pdev);
> -	if (rc)
> -		return rc;
> -
>   	cxlmd = devm_cxl_add_memdev(cxlds);
>   	if (IS_ERR(cxlmd))
>   		return PTR_ERR(cxlmd);

Re: [PATCH 1/7] cxl/mbox: Add background cmd handling machinery

[Davidlohr Bueso]

On Tue, 28 Feb 2023, Dave Jiang wrote:
>>+
>>+		dev_dbg(dev, "Mailbox background operation completed\n");
>
>May be helpful to also emit the return_code in case of errors.

Agreed, but that's given next:

>
>>+	}
>>+
>>	if (mbox_cmd->return_code != CXL_MBOX_CMD_RC_SUCCESS) {
>>		dev_dbg(dev, "Mailbox operation had an error: %s\n",
>>			cxl_mbox_cmd_rc2str(mbox_cmd));

here.

Thanks,
Davidlohr

Re: [PATCH 1/7] cxl/mbox: Add background cmd handling machinery

[Dave Jiang]

On 2/28/23 1:18 PM, Davidlohr Bueso wrote:
> On Tue, 28 Feb 2023, Dave Jiang wrote:
>>> +
>>> +        dev_dbg(dev, "Mailbox background operation completed\n");
>>
>> May be helpful to also emit the return_code in case of errors.
> 
> Agreed, but that's given next:
> 
>>
>>> +    }
>>> +
>>>     if (mbox_cmd->return_code != CXL_MBOX_CMD_RC_SUCCESS) {
>>>         dev_dbg(dev, "Mailbox operation had an error: %s\n",
>>>             cxl_mbox_cmd_rc2str(mbox_cmd));
> 
> here.

Ah ok.

> 
> Thanks,
> Davidlohr

RE: [PATCH 1/7] cxl/mbox: Add background cmd handling machinery

[Dan Williams]

Davidlohr Bueso wrote:

Hi Davidlohr, I am finally circling back to this, some comments below,
but I think this is looking good.

> This adds support for handling background operations, as defined in
> the CXL 3.0 spec. Commands that can take too long (over ~2 seconds)
> can run in the background asynchronously (to the hardware). Currently
> these are limited to Maintenance, transfer/activate Firmware, Scan
> Media, Sanitize (aka overwrite), and VPPB bind/unbind.
> 
> The driver will deal with such commands synchronously, blocking
> all other incoming commands for a specified period of time, allowing
> time-slicing the command such that the caller can send incremental
> requests to avoid monopolizing the driver/device. This approach
> makes the code simpler, where any out of sync (timeout) between the
> driver and hardware is just disregarded as an invalid state until
> the next successful submission.
> 
> On devices where mbox interrupts are supported, this will still use
> a poller that will wakeup in the specified wait intervals. The irq
> handler will simply awake a blocked cmd, which is also safe vs a
> task that is either waking (timing out) or already awoken.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>  drivers/cxl/cxl.h    |   7 +++
>  drivers/cxl/cxlmem.h |   6 +++
>  drivers/cxl/pci.c    | 100 +++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 109 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index d853a0238ad7..b834e55375e3 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -176,14 +176,21 @@ static inline int ways_to_eiw(unsigned int ways, u8 *eiw)
>  /* CXL 2.0 8.2.8.4 Mailbox Registers */
>  #define CXLDEV_MBOX_CAPS_OFFSET 0x00
>  #define   CXLDEV_MBOX_CAP_PAYLOAD_SIZE_MASK GENMASK(4, 0)
> +#define   CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK GENMASK(10, 7)
> +#define   CXLDEV_MBOX_CAP_BG_CMD_IRQ BIT(6)
>  #define CXLDEV_MBOX_CTRL_OFFSET 0x04
>  #define   CXLDEV_MBOX_CTRL_DOORBELL BIT(0)
> +#define   CXLDEV_MBOX_CTRL_BG_CMD_IRQ BIT(2)
>  #define CXLDEV_MBOX_CMD_OFFSET 0x08
>  #define   CXLDEV_MBOX_CMD_COMMAND_OPCODE_MASK GENMASK_ULL(15, 0)
>  #define   CXLDEV_MBOX_CMD_PAYLOAD_LENGTH_MASK GENMASK_ULL(36, 16)
>  #define CXLDEV_MBOX_STATUS_OFFSET 0x10
> +#define   CXLDEV_MBOX_STATUS_BG_CMD BIT(0)
>  #define   CXLDEV_MBOX_STATUS_RET_CODE_MASK GENMASK_ULL(47, 32)
>  #define CXLDEV_MBOX_BG_CMD_STATUS_OFFSET 0x18
> +#define   CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK GENMASK_ULL(15, 0)
> +#define   CXLDEV_MBOX_BG_CMD_COMMAND_PCT_MASK GENMASK_ULL(22, 16)
> +#define   CXLDEV_MBOX_BG_CMD_COMMAND_RC_MASK GENMASK_ULL(47, 32)
>  #define CXLDEV_MBOX_PAYLOAD_OFFSET 0x20
>  
>  /*
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index ccbafc05a636..934076254d52 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -108,6 +108,9 @@ static inline struct cxl_ep *cxl_ep_load(struct cxl_port *port,
>   *            variable sized output commands, it tells the exact number of bytes
>   *            written.
>   * @min_out: (input) internal command output payload size validation
> + * @poll_count: (input)  Number of timeouts to attempt.
> + * @poll_interval: (input) Number of ms between mailbox background command
> + *                 polling intervals timeouts.
>   * @return_code: (output) Error code returned from hardware.
>   *
>   * This is the primary mechanism used to send commands to the hardware.
> @@ -123,6 +126,8 @@ struct cxl_mbox_cmd {
>  	size_t size_in;
>  	size_t size_out;
>  	size_t min_out;
> +	int poll_count;
> +	u64 poll_interval;

An int will do, right? This value should likely never be above 1000.

>  	u16 return_code;
>  };
>  
> @@ -322,6 +327,7 @@ enum cxl_opcode {
>  	CXL_MBOX_OP_GET_SCAN_MEDIA_CAPS	= 0x4303,
>  	CXL_MBOX_OP_SCAN_MEDIA		= 0x4304,
>  	CXL_MBOX_OP_GET_SCAN_MEDIA	= 0x4305,
> +	CXL_MBOX_OP_SANITIZE		= 0x4400,
>  	CXL_MBOX_OP_GET_SECURITY_STATE	= 0x4500,
>  	CXL_MBOX_OP_SET_PASSPHRASE	= 0x4501,
>  	CXL_MBOX_OP_DISABLE_PASSPHRASE	= 0x4502,
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 60b23624d167..26b6105e2797 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -52,6 +52,8 @@ static unsigned short mbox_ready_timeout = 60;
>  module_param(mbox_ready_timeout, ushort, 0644);
>  MODULE_PARM_DESC(mbox_ready_timeout, "seconds to wait for mailbox ready");
>  
> +static DECLARE_WAIT_QUEUE_HEAD(mbox_wait);
> +
>  static int cxl_pci_mbox_wait_for_doorbell(struct cxl_dev_state *cxlds)
>  {
>  	const unsigned long start = jiffies;
> @@ -85,6 +87,25 @@ static int cxl_pci_mbox_wait_for_doorbell(struct cxl_dev_state *cxlds)
>  			    status & CXLMDEV_DEV_FATAL ? " fatal" : "",        \
>  			    status & CXLMDEV_FW_HALT ? " firmware-halt" : "")
>  
> +static irqreturn_t cxl_mbox_irq(int irq, void *id)
> +{
> +	/* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
> +	wake_up(&mbox_wait);
> +	return IRQ_HANDLED;
> +}
> +
> +static bool cxl_mbox_background_complete(struct cxl_dev_state *cxlds)
> +{
> +	u64 bgcmd_status_reg;
> +	u32 pct;
> +
> +	bgcmd_status_reg = readq(cxlds->regs.mbox +
> +				 CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
> +	pct = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_PCT_MASK, bgcmd_status_reg);
> +
> +	return pct == 100;
> +}
> +
>  /**
>   * __cxl_pci_mbox_send_cmd() - Execute a mailbox command
>   * @cxlds: The device state to communicate with.
> @@ -178,6 +199,56 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
>  	mbox_cmd->return_code =
>  		FIELD_GET(CXLDEV_MBOX_STATUS_RET_CODE_MASK, status_reg);
>  
> +	/*
> +	 * Handle the background command in a synchronous manner.
> +	 *
> +	 * All other mailbox commands will serialize/queue on the mbox_mutex,
> +	 * which we currently hold. Furthermore this also guarantees that
> +	 * cxl_mbox_background_complete() checks are safe amongst each other,
> +	 * in that no new bg operation can occur in between.
> +	 *
> +	 * With the exception of special cases that merit monopolizing the
> +	 * driver/device, bg operations are timesliced in accordance with
> +	 * the nature of the command being sent.
> +	 *
> +	 * In the event of timeout, the mailbox state is indeterminate
> +	 * until the next successful command submission and the driver
> +	 * can get back in sync with the hardware state.
> +	 */
> +	if (mbox_cmd->return_code == CXL_MBOX_CMD_RC_BACKGROUND) {
> +		u64 bg_status_reg;
> +		const bool timeslice = mbox_cmd->opcode != CXL_MBOX_OP_SANITIZE;

I understand that sanitize is special, but it feels out of place for
this to never give up polling when sanitize is in progress.

Yes, sanitize is a special case because the device is media disabled and
not much else can happen with it. However because of that I expect the
admin thread that submitted it may be different than the thread that
wants to check the status and re-enable the device.

cxl disable-memdev memX
echo 1 > /sys/bus/cxl/devices/memX/security/sanitize
<select()/poll() on /sys/bus/cxl/devices/memX/security/sanitize>
cxl enable-memdev memX

Per the spec other foreground commands can be allowed through while
sanitize is in flight and the device is expected to fail them. With
background commands I think the need to be careful and queue them up to
avoid clobbering the background status goes away with santize. If the
device has sanitize in progress the driver can just reject/fail new
commands with the "Background Operation" effect until it has had a
chance to reap the sanitize result.

> +
> +		dev_dbg(dev, "Mailbox background operation started\n");
> +
> +		while (1) {
> +			if (wait_event_interruptible_timeout(
> +				mbox_wait, cxl_mbox_background_complete(cxlds),
> +				msecs_to_jiffies(mbox_cmd->poll_interval)) > 0)
> +				break;

Given that this does not check for -EINTR what is the rationale for
making the wait interruptible?

> +
> +			if (timeslice && !--mbox_cmd->poll_count)
> +				break;

Per-above all commands that the kernel would synchronously wait for are
always timesliced, and sanitize has special behavior.

> +		}
> +
> +		if (!cxl_mbox_background_complete(cxlds)) {
> +			u64 md_status =
> +				readq(cxlds->regs.memdev + CXLMDEV_STATUS_OFFSET);
> +
> +			cxl_cmd_err(cxlds->dev, mbox_cmd, md_status,
> +				    "background timeout");
> +			return -ETIMEDOUT;
> +		}
> +
> +		bg_status_reg = readq(cxlds->regs.mbox +
> +				      CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
> +		mbox_cmd->return_code =
> +			FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_RC_MASK,
> +				  bg_status_reg);
> +
> +		dev_dbg(dev, "Mailbox background operation completed\n");

Might be nice to include the opcode here and the other dev_dbg() above.

> +	}
> +
>  	if (mbox_cmd->return_code != CXL_MBOX_CMD_RC_SUCCESS) {
>  		dev_dbg(dev, "Mailbox operation had an error: %s\n",
>  			cxl_mbox_cmd_rc2str(mbox_cmd));
> @@ -222,8 +293,11 @@ static int cxl_pci_mbox_send(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *c
>  static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds)
>  {
>  	const int cap = readl(cxlds->regs.mbox + CXLDEV_MBOX_CAPS_OFFSET);
> +	struct device *dev = cxlds->dev;
> +	struct pci_dev *pdev = to_pci_dev(dev);
>  	unsigned long timeout;
>  	u64 md_status;
> +	int rc, irq;
>  
>  	timeout = jiffies + mbox_ready_timeout * HZ;
>  	do {
> @@ -272,6 +346,24 @@ static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds)
>  	dev_dbg(cxlds->dev, "Mailbox payload sized %zu",
>  		cxlds->payload_size);
>  
> +	if (!(cap & CXLDEV_MBOX_CAP_BG_CMD_IRQ)) {
> +		dev_dbg(dev, "Only Mailbox polling is supported");
> +		return 0;
> +	}
> +
> +	irq = pci_irq_vector(pdev,
> +			     FIELD_GET(CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK, cap));
> +	if (irq < 0)
> +		return irq;
> +
> +	rc = devm_request_irq(dev, irq, cxl_mbox_irq,
> +			      IRQF_SHARED, "mailbox", cxlds);
> +	if (rc)
> +		return rc;
> +
> +	writel(CXLDEV_MBOX_CTRL_BG_CMD_IRQ,
> +	       cxlds->regs.mbox + CXLDEV_MBOX_CTRL_OFFSET);
> +
>  	return 0;
>  }
>  
> @@ -757,6 +849,10 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (rc)
>  		dev_dbg(&pdev->dev, "Failed to map RAS capability.\n");
>  
> +	rc = cxl_alloc_irq_vectors(pdev);
> +	if (rc)
> +		return rc;
> +
>  	rc = cxl_pci_setup_mailbox(cxlds);
>  	if (rc)
>  		return rc;
> @@ -777,10 +873,6 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (rc)
>  		return rc;
>  
> -	rc = cxl_alloc_irq_vectors(pdev);
> -	if (rc)
> -		return rc;
> -

This hunk wants to be its own lead-in patch so it can be bisected
independently of introducing background command support.

[PATCH 2/7] cxl/security: Add security state sysfs ABI

[Davidlohr Bueso]

This adds the sysfs memdev's security/ directory with
a single 'state' file, which is always visible. In the
case of unsupported security features, this will show
disabled.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 Documentation/ABI/testing/sysfs-bus-cxl |  8 ++++
 drivers/cxl/core/memdev.c               | 49 +++++++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index 3acf2f17a73f..e9c432a5a841 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -57,6 +57,14 @@ Description:
 		host PCI device for this memory device, emit the CPU node
 		affinity for this device.
 
+What:		/sys/bus/cxl/devices/memX/security/state
+Date:		February, 2023
+KernelVersion:	v6.4
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RO) The security state for that device. The following states
+		are available: frozen, locked, unlocked and disabled (which
+		is also the case for any unsupported security features).
 
 What:		/sys/bus/cxl/devices/*/devtype
 Date:		June, 2021
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 0af8856936dc..47cc625bb1b0 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /* Copyright(c) 2020 Intel Corporation. */
 
+#include <linux/memregion.h>
 #include <linux/device.h>
 #include <linux/slab.h>
 #include <linux/idr.h>
@@ -89,6 +90,43 @@ static ssize_t pmem_size_show(struct device *dev, struct device_attribute *attr,
 static struct device_attribute dev_attr_pmem_size =
 	__ATTR(size, 0444, pmem_size_show, NULL);
 
+static ssize_t security_state_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	u32 sec_out;
+	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
+	struct cxl_dev_state *cxlds = cxlmd->cxlds;
+	struct cxl_get_security_output {
+		__le32 flags;
+	} out;
+	struct cxl_mbox_cmd mbox_cmd = {
+		.opcode = CXL_MBOX_OP_GET_SECURITY_STATE,
+		.payload_out = &out,
+		.size_out = sizeof(out),
+	};
+
+	if (!cpu_cache_has_invalidate_memregion())
+		goto disabled;
+
+	if (cxl_internal_send_cmd(cxlds, &mbox_cmd) < 0)
+		goto disabled;
+
+	sec_out = le32_to_cpu(out.flags);
+	if (!(sec_out & CXL_PMEM_SEC_STATE_USER_PASS_SET))
+		goto disabled;
+	if (sec_out & CXL_PMEM_SEC_STATE_FROZEN)
+		return sysfs_emit(buf, "frozen\n");
+	if (sec_out & CXL_PMEM_SEC_STATE_LOCKED)
+		return sysfs_emit(buf, "locked\n");
+	else
+		return sysfs_emit(buf, "unlocked\n");
+disabled:
+	return sysfs_emit(buf, "disabled\n");
+}
+
+static struct device_attribute dev_attr_security_state =
+	__ATTR(state, 0444, security_state_show, NULL);
+
 static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)
 {
@@ -148,10 +186,21 @@ static struct attribute_group cxl_memdev_pmem_attribute_group = {
 	.attrs = cxl_memdev_pmem_attributes,
 };
 
+static struct attribute *cxl_memdev_security_attributes[] = {
+	&dev_attr_security_state.attr,
+	NULL,
+};
+
+static struct attribute_group cxl_memdev_security_attribute_group = {
+	.name = "security",
+	.attrs = cxl_memdev_security_attributes,
+};
+
 static const struct attribute_group *cxl_memdev_attribute_groups[] = {
 	&cxl_memdev_attribute_group,
 	&cxl_memdev_ram_attribute_group,
 	&cxl_memdev_pmem_attribute_group,
+	&cxl_memdev_security_attribute_group,
 	NULL,
 };
 
-- 
2.39.2

Re: [PATCH 2/7] cxl/security: Add security state sysfs ABI

[Dave Jiang]

On 2/24/23 12:46 PM, Davidlohr Bueso wrote:
> This adds the sysfs memdev's security/ directory with
> a single 'state' file, which is always visible. In the
> case of unsupported security features, this will show
> disabled.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>

I don't have strong opinion on whether the state attrib should be 
visible if there's no security support, but this deviates from the 
nvdimm security state behavior.

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>   Documentation/ABI/testing/sysfs-bus-cxl |  8 ++++
>   drivers/cxl/core/memdev.c               | 49 +++++++++++++++++++++++++
>   2 files changed, 57 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 3acf2f17a73f..e9c432a5a841 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -57,6 +57,14 @@ Description:
>   		host PCI device for this memory device, emit the CPU node
>   		affinity for this device.
>   
> +What:		/sys/bus/cxl/devices/memX/security/state
> +Date:		February, 2023
> +KernelVersion:	v6.4
> +Contact:	linux-cxl@vger.kernel.org
> +Description:
> +		(RO) The security state for that device. The following states
> +		are available: frozen, locked, unlocked and disabled (which
> +		is also the case for any unsupported security features).
>   
>   What:		/sys/bus/cxl/devices/*/devtype
>   Date:		June, 2021
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 0af8856936dc..47cc625bb1b0 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -1,6 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0-only
>   /* Copyright(c) 2020 Intel Corporation. */
>   
> +#include <linux/memregion.h>
>   #include <linux/device.h>
>   #include <linux/slab.h>
>   #include <linux/idr.h>
> @@ -89,6 +90,43 @@ static ssize_t pmem_size_show(struct device *dev, struct device_attribute *attr,
>   static struct device_attribute dev_attr_pmem_size =
>   	__ATTR(size, 0444, pmem_size_show, NULL);
>   
> +static ssize_t security_state_show(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	u32 sec_out;
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +	struct cxl_get_security_output {
> +		__le32 flags;
> +	} out;
> +	struct cxl_mbox_cmd mbox_cmd = {
> +		.opcode = CXL_MBOX_OP_GET_SECURITY_STATE,
> +		.payload_out = &out,
> +		.size_out = sizeof(out),
> +	};
> +
> +	if (!cpu_cache_has_invalidate_memregion())
> +		goto disabled;
> +
> +	if (cxl_internal_send_cmd(cxlds, &mbox_cmd) < 0)
> +		goto disabled;
> +
> +	sec_out = le32_to_cpu(out.flags);
> +	if (!(sec_out & CXL_PMEM_SEC_STATE_USER_PASS_SET))
> +		goto disabled;
> +	if (sec_out & CXL_PMEM_SEC_STATE_FROZEN)
> +		return sysfs_emit(buf, "frozen\n");
> +	if (sec_out & CXL_PMEM_SEC_STATE_LOCKED)
> +		return sysfs_emit(buf, "locked\n");
> +	else
> +		return sysfs_emit(buf, "unlocked\n");
> +disabled:
> +	return sysfs_emit(buf, "disabled\n");
> +}
> +
> +static struct device_attribute dev_attr_security_state =
> +	__ATTR(state, 0444, security_state_show, NULL);
> +
>   static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
>   			   char *buf)
>   {
> @@ -148,10 +186,21 @@ static struct attribute_group cxl_memdev_pmem_attribute_group = {
>   	.attrs = cxl_memdev_pmem_attributes,
>   };
>   
> +static struct attribute *cxl_memdev_security_attributes[] = {
> +	&dev_attr_security_state.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group cxl_memdev_security_attribute_group = {
> +	.name = "security",
> +	.attrs = cxl_memdev_security_attributes,
> +};
> +
>   static const struct attribute_group *cxl_memdev_attribute_groups[] = {
>   	&cxl_memdev_attribute_group,
>   	&cxl_memdev_ram_attribute_group,
>   	&cxl_memdev_pmem_attribute_group,
> +	&cxl_memdev_security_attribute_group,
>   	NULL,
>   };
>   

RE: [PATCH 2/7] cxl/security: Add security state sysfs ABI

[Dan Williams]

Davidlohr Bueso wrote:
> This adds the sysfs memdev's security/ directory with
> a single 'state' file, which is always visible. In the
> case of unsupported security features, this will show
> disabled.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>  Documentation/ABI/testing/sysfs-bus-cxl |  8 ++++
>  drivers/cxl/core/memdev.c               | 49 +++++++++++++++++++++++++
>  2 files changed, 57 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 3acf2f17a73f..e9c432a5a841 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -57,6 +57,14 @@ Description:
>  		host PCI device for this memory device, emit the CPU node
>  		affinity for this device.
>  
> +What:		/sys/bus/cxl/devices/memX/security/state
> +Date:		February, 2023
> +KernelVersion:	v6.4
> +Contact:	linux-cxl@vger.kernel.org
> +Description:
> +		(RO) The security state for that device. The following states
> +		are available: frozen, locked, unlocked and disabled (which
> +		is also the case for any unsupported security features).
>  
>  What:		/sys/bus/cxl/devices/*/devtype
>  Date:		June, 2021
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 0af8856936dc..47cc625bb1b0 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /* Copyright(c) 2020 Intel Corporation. */
>  
> +#include <linux/memregion.h>
>  #include <linux/device.h>
>  #include <linux/slab.h>
>  #include <linux/idr.h>
> @@ -89,6 +90,43 @@ static ssize_t pmem_size_show(struct device *dev, struct device_attribute *attr,
>  static struct device_attribute dev_attr_pmem_size =
>  	__ATTR(size, 0444, pmem_size_show, NULL);
>  
> +static ssize_t security_state_show(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	u32 sec_out;
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +	struct cxl_get_security_output {
> +		__le32 flags;
> +	} out;
> +	struct cxl_mbox_cmd mbox_cmd = {
> +		.opcode = CXL_MBOX_OP_GET_SECURITY_STATE,
> +		.payload_out = &out,
> +		.size_out = sizeof(out),
> +	};
> +
> +	if (!cpu_cache_has_invalidate_memregion())
> +		goto disabled;

I think this can go as security state can still be read even if
unlocking is not safely possible.

> +
> +	if (cxl_internal_send_cmd(cxlds, &mbox_cmd) < 0)
> +		goto disabled;

I would prefer to not have an any-user triggerable way to spam mailbox
commands. Security state should be read from a cached value that gets
updated when security operations are run.

> +
> +	sec_out = le32_to_cpu(out.flags);
> +	if (!(sec_out & CXL_PMEM_SEC_STATE_USER_PASS_SET))
> +		goto disabled;
> +	if (sec_out & CXL_PMEM_SEC_STATE_FROZEN)
> +		return sysfs_emit(buf, "frozen\n");
> +	if (sec_out & CXL_PMEM_SEC_STATE_LOCKED)
> +		return sysfs_emit(buf, "locked\n");
> +	else
> +		return sysfs_emit(buf, "unlocked\n");
> +disabled:
> +	return sysfs_emit(buf, "disabled\n");
> +}
> +
> +static struct device_attribute dev_attr_security_state =
> +	__ATTR(state, 0444, security_state_show, NULL);

This looks copied from pmem_size above, however that one is using
open-coded __ATTR() because the attribute name, "size", does not match
the prefix of the show() handler, "pmem_size_show()". In this case the
shorter DEVICE_ATTR_RO() helper can be used.

[PATCH 3/7] cxl/region: Add cxl_memdev_active_region()

[Davidlohr Bueso]

Track all regions associated to a memdev in order to
tell if the device might be in active use.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 drivers/cxl/core/memdev.c |  1 +
 drivers/cxl/core/region.c | 33 +++++++++++++++++++++++++++++++--
 drivers/cxl/cxl.h         |  6 ++++++
 drivers/cxl/cxlmem.h      |  4 ++++
 4 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 47cc625bb1b0..68c0ab06b999 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -306,6 +306,7 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
 	dev->type = &cxl_memdev_type;
 	device_set_pm_not_required(dev);
 	INIT_WORK(&cxlmd->detach_work, detach_memdev);
+	INIT_LIST_HEAD(&cxlmd->region_list);
 
 	cdev = &cxlmd->cdev;
 	cdev_init(cdev, fops);
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index f29028148806..cea9de6457b9 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -1730,7 +1730,10 @@ void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled)
 {
 	down_write(&cxl_region_rwsem);
 	cxled->mode = CXL_DECODER_DEAD;
-	cxl_region_detach(cxled);
+	if (!cxl_region_detach(cxled)) {
+		struct cxl_region *cxlr = cxled->cxld.region;
+		list_del(&cxlr->node);
+	}
 	up_write(&cxl_region_rwsem);
 }
 
@@ -1749,8 +1752,12 @@ static int attach_target(struct cxl_region *cxlr,
 
 	down_read(&cxl_dpa_rwsem);
 	rc = cxl_region_attach(cxlr, cxled, pos);
-	if (rc == 0)
+	if (rc == 0) {
+		struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
+
 		set_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags);
+		list_add_tail(&cxlr->node, &cxlmd->region_list);
+	}
 	up_read(&cxl_dpa_rwsem);
 	up_write(&cxl_region_rwsem);
 	return rc;
@@ -1778,6 +1785,8 @@ static int detach_target(struct cxl_region *cxlr, int pos)
 	}
 
 	rc = cxl_region_detach(p->targets[pos]);
+	if (rc == 0)
+		list_del(&cxlr->node);
 out:
 	up_write(&cxl_region_rwsem);
 	return rc;
@@ -2654,6 +2663,26 @@ int cxl_add_to_region(struct cxl_port *root, struct cxl_endpoint_decoder *cxled)
 }
 EXPORT_SYMBOL_NS_GPL(cxl_add_to_region, CXL);
 
+bool cxl_memdev_active_region(struct cxl_memdev *cxlmd)
+{
+	bool ret = false;
+	struct cxl_region *cxlr;
+
+	down_read(&cxl_region_rwsem);
+	list_for_each_entry(cxlr, &cxlmd->region_list, node) {
+		struct cxl_region_params *p = &cxlr->params;
+
+		if (p->state >= CXL_CONFIG_ACTIVE) {
+			ret = true;
+			break;
+		}
+	}
+	up_read(&cxl_region_rwsem);
+
+	return ret;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_memdev_active_region, CXL);
+
 static int cxl_region_invalidate_memregion(struct cxl_region *cxlr)
 {
 	if (!test_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags))
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index b834e55375e3..e211241b079b 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -502,6 +502,7 @@ struct cxl_region {
 	struct cxl_pmem_region *cxlr_pmem;
 	unsigned long flags;
 	struct cxl_region_params params;
+	struct list_head node;
 };
 
 struct cxl_nvdimm_bridge {
@@ -773,6 +774,7 @@ struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev);
 int cxl_add_to_region(struct cxl_port *root,
 		      struct cxl_endpoint_decoder *cxled);
 struct cxl_dax_region *to_cxl_dax_region(struct device *dev);
+bool cxl_memdev_active_region(struct cxl_memdev *cxlmd);
 #else
 static inline bool is_cxl_pmem_region(struct device *dev)
 {
@@ -791,6 +793,10 @@ static inline struct cxl_dax_region *to_cxl_dax_region(struct device *dev)
 {
 	return NULL;
 }
+static inline bool cxl_memdev_active_region(struct cxl_memdev *cxlmd)
+{
+	return false;
+}
 #endif
 
 /*
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 934076254d52..4e31f3234519 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -5,6 +5,7 @@
 #include <uapi/linux/cxl_mem.h>
 #include <linux/cdev.h>
 #include <linux/uuid.h>
+#include <linux/list.h>
 #include "cxl.h"
 
 /* CXL 2.0 8.2.8.5.1.1 Memory Device Status Register */
@@ -40,6 +41,8 @@
  * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem
  * @id: id number of this memdev instance.
  * @depth: endpoint port depth
+ * @region_list: List of regions that have as target the endpoint
+ *               decoder associated with this memdev
  */
 struct cxl_memdev {
 	struct device dev;
@@ -50,6 +53,7 @@ struct cxl_memdev {
 	struct cxl_nvdimm *cxl_nvd;
 	int id;
 	int depth;
+	struct list_head region_list;
 };
 
 static inline struct cxl_memdev *to_cxl_memdev(struct device *dev)
-- 
2.39.2

Re: [PATCH 3/7] cxl/region: Add cxl_memdev_active_region()

[Alison Schofield]

On Fri, Feb 24, 2023 at 11:46:48AM -0800, Davidlohr Bueso wrote:
> Track all regions associated to a memdev in order to
> tell if the device might be in active use.

Hi David,

I took a look here as I've been poked around in this space when looking
for 'region' names to associate with memdev poison trace events. [1]

How does the list created get used?

If we only need to know that this memdev is mapped in any region,
so don't touch it, we can look at it's port->commit_end. If that
commit_end >= 0, we can look at each endpoint to find the regions
it's mapped to.

Not sure if any of that is useful for what you need to do, but
it sounded familiar.

[1] https://lore.kernel.org/linux-cxl/62d24b380514c8c39b651aca79c81a424f0b5b37.1676685180.git.alison.schofield@intel.com/

Alison

> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>  drivers/cxl/core/memdev.c |  1 +
>  drivers/cxl/core/region.c | 33 +++++++++++++++++++++++++++++++--
>  drivers/cxl/cxl.h         |  6 ++++++
>  drivers/cxl/cxlmem.h      |  4 ++++
>  4 files changed, 42 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 47cc625bb1b0..68c0ab06b999 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -306,6 +306,7 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds,
>  	dev->type = &cxl_memdev_type;
>  	device_set_pm_not_required(dev);
>  	INIT_WORK(&cxlmd->detach_work, detach_memdev);
> +	INIT_LIST_HEAD(&cxlmd->region_list);
>  
>  	cdev = &cxlmd->cdev;
>  	cdev_init(cdev, fops);
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index f29028148806..cea9de6457b9 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1730,7 +1730,10 @@ void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled)
>  {
>  	down_write(&cxl_region_rwsem);
>  	cxled->mode = CXL_DECODER_DEAD;
> -	cxl_region_detach(cxled);
> +	if (!cxl_region_detach(cxled)) {
> +		struct cxl_region *cxlr = cxled->cxld.region;
> +		list_del(&cxlr->node);
> +	}
>  	up_write(&cxl_region_rwsem);
>  }
>  
> @@ -1749,8 +1752,12 @@ static int attach_target(struct cxl_region *cxlr,
>  
>  	down_read(&cxl_dpa_rwsem);
>  	rc = cxl_region_attach(cxlr, cxled, pos);
> -	if (rc == 0)
> +	if (rc == 0) {
> +		struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> +
>  		set_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags);
> +		list_add_tail(&cxlr->node, &cxlmd->region_list);
> +	}
>  	up_read(&cxl_dpa_rwsem);
>  	up_write(&cxl_region_rwsem);
>  	return rc;
> @@ -1778,6 +1785,8 @@ static int detach_target(struct cxl_region *cxlr, int pos)
>  	}
>  
>  	rc = cxl_region_detach(p->targets[pos]);
> +	if (rc == 0)
> +		list_del(&cxlr->node);
>  out:
>  	up_write(&cxl_region_rwsem);
>  	return rc;
> @@ -2654,6 +2663,26 @@ int cxl_add_to_region(struct cxl_port *root, struct cxl_endpoint_decoder *cxled)
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_add_to_region, CXL);
>  
> +bool cxl_memdev_active_region(struct cxl_memdev *cxlmd)
> +{
> +	bool ret = false;
> +	struct cxl_region *cxlr;
> +
> +	down_read(&cxl_region_rwsem);
> +	list_for_each_entry(cxlr, &cxlmd->region_list, node) {
> +		struct cxl_region_params *p = &cxlr->params;
> +
> +		if (p->state >= CXL_CONFIG_ACTIVE) {
> +			ret = true;
> +			break;
> +		}
> +	}
> +	up_read(&cxl_region_rwsem);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_memdev_active_region, CXL);
> +
>  static int cxl_region_invalidate_memregion(struct cxl_region *cxlr)
>  {
>  	if (!test_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags))
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index b834e55375e3..e211241b079b 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -502,6 +502,7 @@ struct cxl_region {
>  	struct cxl_pmem_region *cxlr_pmem;
>  	unsigned long flags;
>  	struct cxl_region_params params;
> +	struct list_head node;
>  };
>  
>  struct cxl_nvdimm_bridge {
> @@ -773,6 +774,7 @@ struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev);
>  int cxl_add_to_region(struct cxl_port *root,
>  		      struct cxl_endpoint_decoder *cxled);
>  struct cxl_dax_region *to_cxl_dax_region(struct device *dev);
> +bool cxl_memdev_active_region(struct cxl_memdev *cxlmd);
>  #else
>  static inline bool is_cxl_pmem_region(struct device *dev)
>  {
> @@ -791,6 +793,10 @@ static inline struct cxl_dax_region *to_cxl_dax_region(struct device *dev)
>  {
>  	return NULL;
>  }
> +static inline bool cxl_memdev_active_region(struct cxl_memdev *cxlmd)
> +{
> +	return false;
> +}
>  #endif
>  
>  /*
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 934076254d52..4e31f3234519 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -5,6 +5,7 @@
>  #include <uapi/linux/cxl_mem.h>
>  #include <linux/cdev.h>
>  #include <linux/uuid.h>
> +#include <linux/list.h>
>  #include "cxl.h"
>  
>  /* CXL 2.0 8.2.8.5.1.1 Memory Device Status Register */
> @@ -40,6 +41,8 @@
>   * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem
>   * @id: id number of this memdev instance.
>   * @depth: endpoint port depth
> + * @region_list: List of regions that have as target the endpoint
> + *               decoder associated with this memdev
>   */
>  struct cxl_memdev {
>  	struct device dev;
> @@ -50,6 +53,7 @@ struct cxl_memdev {
>  	struct cxl_nvdimm *cxl_nvd;
>  	int id;
>  	int depth;
> +	struct list_head region_list;
>  };
>  
>  static inline struct cxl_memdev *to_cxl_memdev(struct device *dev)
> -- 
> 2.39.2
> 

Re: [PATCH 3/7] cxl/region: Add cxl_memdev_active_region()

[Davidlohr Bueso]

Hi Alison,

On Sun, 26 Feb 2023, Alison Schofield wrote:

>I took a look here as I've been poked around in this space when looking
>for 'region' names to associate with memdev poison trace events. [1]

Thanks for the pointer.

>How does the list created get used?

It's used by the security commands, in the next patches in the series.
This came about because Dan requested some way of knowing wether the
memdev is being used (actively decoding HPA ranges).

>If we only need to know that this memdev is mapped in any region,
>so don't touch it, we can look at it's port->commit_end. If that
>commit_end >= 0, we can look at each endpoint to find the regions
>it's mapped to.

I think this is a lot better than using the list approach.

Thanks,
Davidlohr

Re: [PATCH 3/7] cxl/region: Add cxl_memdev_active_region()

[Fan Ni]

On Tue, Feb 28, 2023 at 12:26:23PM -0800, Davidlohr Bueso wrote:
> Hi Alison,
> 
> On Sun, 26 Feb 2023, Alison Schofield wrote:
> 
> > I took a look here as I've been poked around in this space when looking
> > for 'region' names to associate with memdev poison trace events. [1]
> 
> Thanks for the pointer.
> 
> > How does the list created get used?
> 
> It's used by the security commands, in the next patches in the series.
> This came about because Dan requested some way of knowing wether the
> memdev is being used (actively decoding HPA ranges).
> 
> > If we only need to know that this memdev is mapped in any region,
> > so don't touch it, we can look at it's port->commit_end. If that
> > commit_end >= 0, we can look at each endpoint to find the regions
> > it's mapped to.

commit_end will not be updated during commit if the decoder flag is
with CXL_DECODER_F_ENABLE, not sure whether it will cause issue or not
for your case. Can we use hdm_end instead?

Fan
> 
> I think this is a lot better than using the list approach.
> 
> Thanks,
> Davidlohr

Re: [PATCH 3/7] cxl/region: Add cxl_memdev_active_region()

[Dan Williams]

Davidlohr Bueso wrote:
[..]
> >If we only need to know that this memdev is mapped in any region,
> >so don't touch it, we can look at it's port->commit_end. If that
> >commit_end >= 0, we can look at each endpoint to find the regions
> >it's mapped to.
> 
> I think this is a lot better than using the list approach.

Seconded, thanks Alison!

[PATCH 4/7] cxl/mem: Support Sanitation

[Davidlohr Bueso]

Implement support for the non-pmem exclusive sanitize (aka overwrite),
per CXL specs. This is the baseline for the sanitize-on-release
functionality.

To properly support this feature, create a 'security/sanitize' sysfs
file that when read will list the current pmem security state and
when written to, perform the requested operation.

This operation can run in the background and the driver must wait
for completion (no timeout), where the poller will awake every
~10 seconds (this could be further based on the size of the device).

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 Documentation/ABI/testing/sysfs-bus-cxl | 14 ++++++
 drivers/cxl/core/mbox.c                 | 61 +++++++++++++++++++++++++
 drivers/cxl/core/memdev.c               | 39 ++++++++++++++++
 drivers/cxl/cxlmem.h                    |  2 +
 4 files changed, 116 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index e9c432a5a841..b315d78b7e91 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -66,6 +66,20 @@ Description:
 		are available: frozen, locked, unlocked and disabled (which
 		is also the case for any unsupported security features).
 
+What:          /sys/bus/cxl/devices/memX/security/sanitize
+Date:          February, 2023
+KernelVersion: v6.4
+Contact:       linux-cxl@vger.kernel.org
+Description:
+	       (WO) Write a boolean 'true' string value to this attribute to
+	       sanitize the device to securely re-purpose or decommission it.
+	       This is done by ensuring that all user data and meta-data,
+	       whether it resides in persistent capacity, volatile capacity,
+	       or the LSA, is made permanently unavailable by whatever means
+	       is appropriate for the media type. This causes all CPU caches
+	       to be flushed. If this sysfs entry is not present then the
+	       architecture does not support security features.
+
 What:		/sys/bus/cxl/devices/*/devtype
 Date:		June, 2021
 KernelVersion:	v5.14
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index f2addb457172..885de3506735 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /* Copyright(c) 2020 Intel Corporation. All rights reserved. */
 #include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/memregion.h>
 #include <linux/security.h>
 #include <linux/debugfs.h>
 #include <linux/ktime.h>
@@ -1021,6 +1022,66 @@ int cxl_dev_state_identify(struct cxl_dev_state *cxlds)
 }
 EXPORT_SYMBOL_NS_GPL(cxl_dev_state_identify, CXL);
 
+/**
+ * cxl_mem_sanitize() - Send sanitation (aka overwrite) command to the device.
+ * @cxlds: The device data for the operation
+ *
+ * Return: 0 if the command was executed successfully, regardless of
+ * whether or not the actual security operation is done in the background.
+ * Upon error, return the result of the mailbox command or -EINVAL if
+ * security requirements are not met. CPU caches are flushed before and
+ * after succesful completion of each command.
+ *
+ * See CXL 3.0 @8.2.9.8.5.1 Sanitize.
+ */
+int cxl_mem_sanitize(struct cxl_dev_state *cxlds)
+{
+	int rc;
+	u32 sec_out = 0;
+	struct cxl_get_security_output {
+		__le32 flags;
+	} out;
+	struct cxl_mbox_cmd sec_cmd = {
+		.opcode = CXL_MBOX_OP_GET_SECURITY_STATE,
+		.payload_out = &out,
+		.size_out = sizeof(out),
+	};
+	struct cxl_mbox_cmd mbox_cmd = {
+		.opcode = CXL_MBOX_OP_SANITIZE,
+		.poll_interval = 10000UL,
+	};
+
+	if (!cpu_cache_has_invalidate_memregion())
+		return -EINVAL;
+
+	rc = cxl_internal_send_cmd(cxlds, &sec_cmd);
+	if (rc < 0) {
+		dev_err(cxlds->dev, "Failed to get security state : %d", rc);
+		return rc;
+	}
+
+	/*
+	 * Prior to using these commands, any security applied to
+	 * the user data areas of the device shall be DISABLED (or
+	 * UNLOCKED for secure erase case).
+	 */
+	sec_out = le32_to_cpu(out.flags);
+	if (sec_out & CXL_PMEM_SEC_STATE_USER_PASS_SET)
+		return -EINVAL;
+
+	cpu_cache_invalidate_memregion(IORES_DESC_CXL);
+
+	rc = cxl_internal_send_cmd(cxlds, &mbox_cmd);
+	if (rc < 0) {
+		dev_err(cxlds->dev, "Failed to sanitize device : %d", rc);
+		return rc;
+	}
+
+	cpu_cache_invalidate_memregion(IORES_DESC_CXL);
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_mem_sanitize, CXL);
+
 static int add_dpa_res(struct device *dev, struct resource *parent,
 		       struct resource *res, resource_size_t start,
 		       resource_size_t size, const char *type)
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 68c0ab06b999..a1bb095d081c 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -127,6 +127,34 @@ static ssize_t security_state_show(struct device *dev,
 static struct device_attribute dev_attr_security_state =
 	__ATTR(state, 0444, security_state_show, NULL);
 
+static ssize_t security_sanitize_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t len)
+{
+	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
+	struct cxl_dev_state *cxlds = cxlmd->cxlds;
+	ssize_t rc;
+	bool sanitize;
+
+	rc = kstrtobool(buf, &sanitize);
+	if (rc)
+		return rc;
+
+	if (sanitize) {
+		if (cxl_memdev_active_region(cxlmd))
+			return -EBUSY;
+
+		rc = cxl_mem_sanitize(cxlds);
+	}
+
+	if (rc == 0)
+		rc = len;
+	return rc;
+}
+
+static struct device_attribute dev_attr_security_sanitize =
+	__ATTR(sanitize, 0200, NULL, security_sanitize_store);
+
 static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)
 {
@@ -188,11 +216,22 @@ static struct attribute_group cxl_memdev_pmem_attribute_group = {
 
 static struct attribute *cxl_memdev_security_attributes[] = {
 	&dev_attr_security_state.attr,
+	&dev_attr_security_sanitize.attr,
 	NULL,
 };
 
+static umode_t cxl_security_visible(struct kobject *kobj,
+				    struct attribute *a, int n)
+{
+	if (!cpu_cache_has_invalidate_memregion() &&
+	    a == &dev_attr_security_sanitize.attr)
+		return 0;
+	return a->mode;
+}
+
 static struct attribute_group cxl_memdev_security_attribute_group = {
 	.name = "security",
+	.is_visible = cxl_security_visible,
 	.attrs = cxl_memdev_security_attributes,
 };
 
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 4e31f3234519..0d2009b36933 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -631,6 +631,8 @@ static inline void cxl_mem_active_dec(void)
 }
 #endif
 
+int cxl_mem_sanitize(struct cxl_dev_state *cxlds);
+
 struct cxl_hdm {
 	struct cxl_component_regs regs;
 	unsigned int decoder_count;
-- 
2.39.2

Re: [PATCH 4/7] cxl/mem: Support Sanitation

[Dave Jiang]

On 2/24/23 12:46 PM, Davidlohr Bueso wrote:
> Implement support for the non-pmem exclusive sanitize (aka overwrite),
> per CXL specs. This is the baseline for the sanitize-on-release
> functionality.
> 
> To properly support this feature, create a 'security/sanitize' sysfs
> file that when read will list the current pmem security state and
> when written to, perform the requested operation.

I think this segment needs to be updated? The attrib is write only from 
the code below.

DJ

> 
> This operation can run in the background and the driver must wait
> for completion (no timeout), where the poller will awake every
> ~10 seconds (this could be further based on the size of the device).
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>   Documentation/ABI/testing/sysfs-bus-cxl | 14 ++++++
>   drivers/cxl/core/mbox.c                 | 61 +++++++++++++++++++++++++
>   drivers/cxl/core/memdev.c               | 39 ++++++++++++++++
>   drivers/cxl/cxlmem.h                    |  2 +
>   4 files changed, 116 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index e9c432a5a841..b315d78b7e91 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -66,6 +66,20 @@ Description:
>   		are available: frozen, locked, unlocked and disabled (which
>   		is also the case for any unsupported security features).
>   
> +What:          /sys/bus/cxl/devices/memX/security/sanitize
> +Date:          February, 2023
> +KernelVersion: v6.4
> +Contact:       linux-cxl@vger.kernel.org
> +Description:
> +	       (WO) Write a boolean 'true' string value to this attribute to
> +	       sanitize the device to securely re-purpose or decommission it.
> +	       This is done by ensuring that all user data and meta-data,
> +	       whether it resides in persistent capacity, volatile capacity,
> +	       or the LSA, is made permanently unavailable by whatever means
> +	       is appropriate for the media type. This causes all CPU caches
> +	       to be flushed. If this sysfs entry is not present then the
> +	       architecture does not support security features.
> +
>   What:		/sys/bus/cxl/devices/*/devtype
>   Date:		June, 2021
>   KernelVersion:	v5.14
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index f2addb457172..885de3506735 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -1,6 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0-only
>   /* Copyright(c) 2020 Intel Corporation. All rights reserved. */
>   #include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/memregion.h>
>   #include <linux/security.h>
>   #include <linux/debugfs.h>
>   #include <linux/ktime.h>
> @@ -1021,6 +1022,66 @@ int cxl_dev_state_identify(struct cxl_dev_state *cxlds)
>   }
>   EXPORT_SYMBOL_NS_GPL(cxl_dev_state_identify, CXL);
>   
> +/**
> + * cxl_mem_sanitize() - Send sanitation (aka overwrite) command to the device.
> + * @cxlds: The device data for the operation
> + *
> + * Return: 0 if the command was executed successfully, regardless of
> + * whether or not the actual security operation is done in the background.
> + * Upon error, return the result of the mailbox command or -EINVAL if
> + * security requirements are not met. CPU caches are flushed before and
> + * after succesful completion of each command.
> + *
> + * See CXL 3.0 @8.2.9.8.5.1 Sanitize.
> + */
> +int cxl_mem_sanitize(struct cxl_dev_state *cxlds)
> +{
> +	int rc;
> +	u32 sec_out = 0;
> +	struct cxl_get_security_output {
> +		__le32 flags;
> +	} out;
> +	struct cxl_mbox_cmd sec_cmd = {
> +		.opcode = CXL_MBOX_OP_GET_SECURITY_STATE,
> +		.payload_out = &out,
> +		.size_out = sizeof(out),
> +	};
> +	struct cxl_mbox_cmd mbox_cmd = {
> +		.opcode = CXL_MBOX_OP_SANITIZE,
> +		.poll_interval = 10000UL,
> +	};
> +
> +	if (!cpu_cache_has_invalidate_memregion())
> +		return -EINVAL;
> +
> +	rc = cxl_internal_send_cmd(cxlds, &sec_cmd);
> +	if (rc < 0) {
> +		dev_err(cxlds->dev, "Failed to get security state : %d", rc);
> +		return rc;
> +	}
> +
> +	/*
> +	 * Prior to using these commands, any security applied to
> +	 * the user data areas of the device shall be DISABLED (or
> +	 * UNLOCKED for secure erase case).
> +	 */
> +	sec_out = le32_to_cpu(out.flags);
> +	if (sec_out & CXL_PMEM_SEC_STATE_USER_PASS_SET)
> +		return -EINVAL;
> +
> +	cpu_cache_invalidate_memregion(IORES_DESC_CXL);
> +
> +	rc = cxl_internal_send_cmd(cxlds, &mbox_cmd);
> +	if (rc < 0) {
> +		dev_err(cxlds->dev, "Failed to sanitize device : %d", rc);
> +		return rc;
> +	}
> +
> +	cpu_cache_invalidate_memregion(IORES_DESC_CXL);
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_mem_sanitize, CXL);
> +
>   static int add_dpa_res(struct device *dev, struct resource *parent,
>   		       struct resource *res, resource_size_t start,
>   		       resource_size_t size, const char *type)
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 68c0ab06b999..a1bb095d081c 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -127,6 +127,34 @@ static ssize_t security_state_show(struct device *dev,
>   static struct device_attribute dev_attr_security_state =
>   	__ATTR(state, 0444, security_state_show, NULL);
>   
> +static ssize_t security_sanitize_store(struct device *dev,
> +				       struct device_attribute *attr,
> +				       const char *buf, size_t len)
> +{
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +	ssize_t rc;
> +	bool sanitize;
> +
> +	rc = kstrtobool(buf, &sanitize);
> +	if (rc)
> +		return rc;
> +
> +	if (sanitize) {
> +		if (cxl_memdev_active_region(cxlmd))
> +			return -EBUSY;
> +
> +		rc = cxl_mem_sanitize(cxlds);
> +	}
> +
> +	if (rc == 0)
> +		rc = len;
> +	return rc;
> +}
> +
> +static struct device_attribute dev_attr_security_sanitize =
> +	__ATTR(sanitize, 0200, NULL, security_sanitize_store);
> +
>   static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
>   			   char *buf)
>   {
> @@ -188,11 +216,22 @@ static struct attribute_group cxl_memdev_pmem_attribute_group = {
>   
>   static struct attribute *cxl_memdev_security_attributes[] = {
>   	&dev_attr_security_state.attr,
> +	&dev_attr_security_sanitize.attr,
>   	NULL,
>   };
>   
> +static umode_t cxl_security_visible(struct kobject *kobj,
> +				    struct attribute *a, int n)
> +{
> +	if (!cpu_cache_has_invalidate_memregion() &&
> +	    a == &dev_attr_security_sanitize.attr)
> +		return 0;
> +	return a->mode;
> +}
> +
>   static struct attribute_group cxl_memdev_security_attribute_group = {
>   	.name = "security",
> +	.is_visible = cxl_security_visible,
>   	.attrs = cxl_memdev_security_attributes,
>   };
>   
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 4e31f3234519..0d2009b36933 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -631,6 +631,8 @@ static inline void cxl_mem_active_dec(void)
>   }
>   #endif
>   
> +int cxl_mem_sanitize(struct cxl_dev_state *cxlds);
> +
>   struct cxl_hdm {
>   	struct cxl_component_regs regs;
>   	unsigned int decoder_count;

Re: [PATCH 4/7] cxl/mem: Support Sanitation

[Davidlohr Bueso]

On Tue, 28 Feb 2023, Dave Jiang wrote:

>On 2/24/23 12:46 PM, Davidlohr Bueso wrote:
>>Implement support for the non-pmem exclusive sanitize (aka overwrite),
>>per CXL specs. This is the baseline for the sanitize-on-release
>>functionality.
>>
>>To properly support this feature, create a 'security/sanitize' sysfs
>>file that when read will list the current pmem security state and
>>when written to, perform the requested operation.
>
>I think this segment needs to be updated? The attrib is write only
>from the code below.

Bleh, indeed both commands need the changelog updated, these are
intended to be write-only.

Thanks,
Davidlohr

RE: [PATCH 4/7] cxl/mem: Support Sanitation

[Dan Williams]

Hi Davidlohr,

Davidlohr Bueso wrote:
> Implement support for the non-pmem exclusive sanitize (aka overwrite),
> per CXL specs. This is the baseline for the sanitize-on-release
> functionality.

Wait, what sanitize-on-release functionality? The DCD facility to
sanitize-on-release (CXL 3.0 Table 8-126.  DC Region Configuration) is
indepdent of background command functionality, i.e. "Release Dynamic
Capacity" is a foreground operation.

> To properly support this feature, create a 'security/sanitize' sysfs
> file that when read will list the current pmem security state and
> when written to, perform the requested operation.

Sanitize (command set 0x44) is independent of the pmem security state
(command set 0x45), and I thought patch2 will list the current pmem
security state via security/state. 

> This operation can run in the background and the driver must wait
> for completion (no timeout), where the poller will awake every
> ~10 seconds (this could be further based on the size of the device).

Per the feedback on 1 this changes to be a facility that returns
immediately and signals completion via sysfs_notify_dirent() because if
it takes seconds and is hardware uninterruptible then that warrants
userspace being able to poll for completion if it wants.

> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>  Documentation/ABI/testing/sysfs-bus-cxl | 14 ++++++
>  drivers/cxl/core/mbox.c                 | 61 +++++++++++++++++++++++++
>  drivers/cxl/core/memdev.c               | 39 ++++++++++++++++
>  drivers/cxl/cxlmem.h                    |  2 +
>  4 files changed, 116 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index e9c432a5a841..b315d78b7e91 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -66,6 +66,20 @@ Description:
>  		are available: frozen, locked, unlocked and disabled (which
>  		is also the case for any unsupported security features).
>  
> +What:          /sys/bus/cxl/devices/memX/security/sanitize
> +Date:          February, 2023
> +KernelVersion: v6.4
> +Contact:       linux-cxl@vger.kernel.org
> +Description:
> +	       (WO) Write a boolean 'true' string value to this attribute to
> +	       sanitize the device to securely re-purpose or decommission it.
> +	       This is done by ensuring that all user data and meta-data,
> +	       whether it resides in persistent capacity, volatile capacity,
> +	       or the LSA, is made permanently unavailable by whatever means
> +	       is appropriate for the media type. This causes all CPU caches
> +	       to be flushed. If this sysfs entry is not present then the
> +	       architecture does not support security features.
> +
>  What:		/sys/bus/cxl/devices/*/devtype
>  Date:		June, 2021
>  KernelVersion:	v5.14
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index f2addb457172..885de3506735 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /* Copyright(c) 2020 Intel Corporation. All rights reserved. */
>  #include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/memregion.h>
>  #include <linux/security.h>
>  #include <linux/debugfs.h>
>  #include <linux/ktime.h>
> @@ -1021,6 +1022,66 @@ int cxl_dev_state_identify(struct cxl_dev_state *cxlds)
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_dev_state_identify, CXL);
>  
> +/**
> + * cxl_mem_sanitize() - Send sanitation (aka overwrite) command to the device.
> + * @cxlds: The device data for the operation
> + *
> + * Return: 0 if the command was executed successfully, regardless of
> + * whether or not the actual security operation is done in the background.
> + * Upon error, return the result of the mailbox command or -EINVAL if
> + * security requirements are not met. CPU caches are flushed before and
> + * after succesful completion of each command.
> + *
> + * See CXL 3.0 @8.2.9.8.5.1 Sanitize.
> + */
> +int cxl_mem_sanitize(struct cxl_dev_state *cxlds)
> +{
> +	int rc;
> +	u32 sec_out = 0;
> +	struct cxl_get_security_output {
> +		__le32 flags;
> +	} out;
> +	struct cxl_mbox_cmd sec_cmd = {
> +		.opcode = CXL_MBOX_OP_GET_SECURITY_STATE,
> +		.payload_out = &out,
> +		.size_out = sizeof(out),
> +	};
> +	struct cxl_mbox_cmd mbox_cmd = {
> +		.opcode = CXL_MBOX_OP_SANITIZE,
> +		.poll_interval = 10000UL,
> +	};
> +
> +	if (!cpu_cache_has_invalidate_memregion())
> +		return -EINVAL;

Given that the regions are already offline I think there is no
additional damage to allow santize to go through, because they could not
be dynamically re-enabled.

> +
> +	rc = cxl_internal_send_cmd(cxlds, &sec_cmd);
> +	if (rc < 0) {
> +		dev_err(cxlds->dev, "Failed to get security state : %d", rc);
> +		return rc;
> +	}
> +
> +	/*
> +	 * Prior to using these commands, any security applied to
> +	 * the user data areas of the device shall be DISABLED (or
> +	 * UNLOCKED for secure erase case).
> +	 */
> +	sec_out = le32_to_cpu(out.flags);
> +	if (sec_out & CXL_PMEM_SEC_STATE_USER_PASS_SET)
> +		return -EINVAL;
> +
> +	cpu_cache_invalidate_memregion(IORES_DESC_CXL);

Given that sanitize mandates bouncing the decoders the reconnect will
set CXL_REGION_F_INCOHERENT in attach_target(), I think it is safe to
assume that caches will be managed before the sanitized data will be
ingested by anything.

Specifically, I notice that attach_target() sets CXL_REGION_F_INCOHERENT
in the region creation and autodiscovery case.  The latter is a bug that
I'll fix up. It should only be set when the chance the HPA-to-DPA
mapping has changed, or the contents of the DPA has changed in a cache
incoherent manner. The fact that sanitize mandates what appears to be an
HPA-to-DPA change then no need to manage caches explicitly.

> +
> +	rc = cxl_internal_send_cmd(cxlds, &mbox_cmd);
> +	if (rc < 0) {
> +		dev_err(cxlds->dev, "Failed to sanitize device : %d", rc);
> +		return rc;
> +	}
> +
> +	cpu_cache_invalidate_memregion(IORES_DESC_CXL);
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_mem_sanitize, CXL);
> +
>  static int add_dpa_res(struct device *dev, struct resource *parent,
>  		       struct resource *res, resource_size_t start,
>  		       resource_size_t size, const char *type)
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 68c0ab06b999..a1bb095d081c 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -127,6 +127,34 @@ static ssize_t security_state_show(struct device *dev,
>  static struct device_attribute dev_attr_security_state =
>  	__ATTR(state, 0444, security_state_show, NULL);
>  
> +static ssize_t security_sanitize_store(struct device *dev,
> +				       struct device_attribute *attr,
> +				       const char *buf, size_t len)
> +{
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +	ssize_t rc;
> +	bool sanitize;
> +
> +	rc = kstrtobool(buf, &sanitize);
> +	if (rc)
> +		return rc;
> +
> +	if (sanitize) {
> +		if (cxl_memdev_active_region(cxlmd))
> +			return -EBUSY;
> +
> +		rc = cxl_mem_sanitize(cxlds);
> +	}
> +
> +	if (rc == 0)
> +		rc = len;
> +	return rc;
> +}
> +
> +static struct device_attribute dev_attr_security_sanitize =
> +	__ATTR(sanitize, 0200, NULL, security_sanitize_store);
> +

DEVICE_ATTR_RW() to poll(2) for completion.

>  static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
>  			   char *buf)
>  {
> @@ -188,11 +216,22 @@ static struct attribute_group cxl_memdev_pmem_attribute_group = {
>  
>  static struct attribute *cxl_memdev_security_attributes[] = {
>  	&dev_attr_security_state.attr,
> +	&dev_attr_security_sanitize.attr,
>  	NULL,
>  };
>  
> +static umode_t cxl_security_visible(struct kobject *kobj,
> +				    struct attribute *a, int n)
> +{
> +	if (!cpu_cache_has_invalidate_memregion() &&
> +	    a == &dev_attr_security_sanitize.attr)

No need to hide the attribute given the above discussion about
invalidation being done somewhere else.

> +		return 0;
> +	return a->mode;
> +}
> +
>  static struct attribute_group cxl_memdev_security_attribute_group = {
>  	.name = "security",
> +	.is_visible = cxl_security_visible,
>  	.attrs = cxl_memdev_security_attributes,
>  };
>  
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 4e31f3234519..0d2009b36933 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -631,6 +631,8 @@ static inline void cxl_mem_active_dec(void)
>  }
>  #endif
>  
> +int cxl_mem_sanitize(struct cxl_dev_state *cxlds);
> +
>  struct cxl_hdm {
>  	struct cxl_component_regs regs;
>  	unsigned int decoder_count;
> -- 
> 2.39.2
> 

Re: [PATCH 4/7] cxl/mem: Support Sanitation

[Davidlohr Bueso]

On Mon, 27 Mar 2023, Dan Williams wrote:

>Per the feedback on 1 this changes to be a facility that returns
>immediately and signals completion via sysfs_notify_dirent() because if
>it takes seconds and is hardware uninterruptible then that warrants
>userspace being able to poll for completion if it wants.

Perhaps I'm missing something here, but how can we signal completion
for sanitation if we return immediately (after timing out) and loose
context of the background command? Or are you referring to use the
Background Operation Status command (0002h), which afaict would be
the only way to do this, as nvdimm's overwrite_query. But as we've
discussed in the past, this also means that another bg command could
occur in between when the command finished and the query is done.

Thanks,
Davidlohr

Re: [PATCH 4/7] cxl/mem: Support Sanitation

[Dan Williams]

Davidlohr Bueso wrote:
> On Mon, 27 Mar 2023, Dan Williams wrote:
> 
> >Per the feedback on 1 this changes to be a facility that returns
> >immediately and signals completion via sysfs_notify_dirent() because if
> >it takes seconds and is hardware uninterruptible then that warrants
> >userspace being able to poll for completion if it wants.
> 
> Perhaps I'm missing something here, but how can we signal completion
> for sanitation if we return immediately (after timing out) and loose
> context of the background command? Or are you referring to use the
> Background Operation Status command (0002h), which afaict would be
> the only way to do this, as nvdimm's overwrite_query. But as we've
> discussed in the past, this also means that another bg command could
> occur in between when the command finished and the query is done.

The observation is that sanitation is special compared to other
background commands. It's so special that it gets to skip the normal
rules that background commands are parceled into tiny pieces to prevent
monopolization of the background command slot. Sanitization monopolizes
the device by definition.

Given that sanitation is going to take on the order of seconds, don't
trap the submitter in uninterruptible sleep. Instead, put the mailbox
into "santizing" mode, and return immediately after sanitation is
started.

In "sanitizing" mode. New background command submissions fail
immediately with EBUSY (as opposed to queuing because sanitization
destoys the device's context). The end of sanitization mode occurs when
the completion interrupt fires, or a kernel polling thread notices the
completion (in case the device does not implement background command
completion interrupts). When that happens the mailbox exits "sanitizing"
mode and reflects the completion status in the sysfs file that was used
trigger sanitization. When that state changes sysfs_notify_dirent()
tells userspace that the sysfs status has changed in case userspace
actually wanted to go to sleep while awaiting the completion rather than
live polling (but that sleep is interruptible and resumable if userspace
wants).

This is similar to what happens with address range scrubs in the ACPI
NFIT driver. See drivers/acpi/nfit/core.c::notify_ars_done().

Think of this like O_ASYNC semantics for sysfs files that trigger long
running operations.

[PATCH 5/7] cxl/test: Add "Sanitize" opcode support

[Davidlohr Bueso]

Add support to emulate a CXL mem device support the "Sanitize"
operation, without incurring in the background.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 tools/testing/cxl/test/mem.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
index 9263b04d35f7..d4466cb27947 100644
--- a/tools/testing/cxl/test/mem.c
+++ b/tools/testing/cxl/test/mem.c
@@ -497,6 +497,28 @@ static int mock_partition_info(struct cxl_dev_state *cxlds,
 	return 0;
 }
 
+static int mock_sanitize(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd)
+{
+	struct cxl_mockmem_data *mdata = dev_get_drvdata(cxlds->dev);
+
+	if (cmd->size_in != 0)
+		return -EINVAL;
+
+	if (cmd->size_out != 0)
+		return -EINVAL;
+
+	if (mdata->security_state & CXL_PMEM_SEC_STATE_USER_PASS_SET) {
+		cmd->return_code = CXL_MBOX_CMD_RC_SECURITY;
+		return -ENXIO;
+	}
+	if (mdata->security_state & CXL_PMEM_SEC_STATE_LOCKED) {
+		cmd->return_code = CXL_MBOX_CMD_RC_SECURITY;
+		return -ENXIO;
+	}
+
+	return 0; /* assume less than 2 secs, no bg */
+}
+
 static int mock_get_security_state(struct cxl_dev_state *cxlds,
 				   struct cxl_mbox_cmd *cmd)
 {
@@ -924,6 +946,9 @@ static int cxl_mock_mbox_send(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *
 	case CXL_MBOX_OP_GET_HEALTH_INFO:
 		rc = mock_health_info(cxlds, cmd);
 		break;
+	case CXL_MBOX_OP_SANITIZE:
+		rc = mock_sanitize(cxlds, cmd);
+		break;
 	case CXL_MBOX_OP_GET_SECURITY_STATE:
 		rc = mock_get_security_state(cxlds, cmd);
 		break;
-- 
2.39.2

Re: [PATCH 5/7] cxl/test: Add "Sanitize" opcode support

[Dave Jiang]

On 2/24/23 12:46 PM, Davidlohr Bueso wrote:
> Add support to emulate a CXL mem device support the "Sanitize"
> operation, without incurring in the background.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>   tools/testing/cxl/test/mem.c | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
> 
> diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
> index 9263b04d35f7..d4466cb27947 100644
> --- a/tools/testing/cxl/test/mem.c
> +++ b/tools/testing/cxl/test/mem.c
> @@ -497,6 +497,28 @@ static int mock_partition_info(struct cxl_dev_state *cxlds,
>   	return 0;
>   }
>   
> +static int mock_sanitize(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd)
> +{
> +	struct cxl_mockmem_data *mdata = dev_get_drvdata(cxlds->dev);
> +
> +	if (cmd->size_in != 0)
> +		return -EINVAL;
> +
> +	if (cmd->size_out != 0)
> +		return -EINVAL;
> +
> +	if (mdata->security_state & CXL_PMEM_SEC_STATE_USER_PASS_SET) {
> +		cmd->return_code = CXL_MBOX_CMD_RC_SECURITY;
> +		return -ENXIO;
> +	}
> +	if (mdata->security_state & CXL_PMEM_SEC_STATE_LOCKED) {
> +		cmd->return_code = CXL_MBOX_CMD_RC_SECURITY;
> +		return -ENXIO;
> +	}
> +
> +	return 0; /* assume less than 2 secs, no bg */
> +}
> +
>   static int mock_get_security_state(struct cxl_dev_state *cxlds,
>   				   struct cxl_mbox_cmd *cmd)
>   {
> @@ -924,6 +946,9 @@ static int cxl_mock_mbox_send(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *
>   	case CXL_MBOX_OP_GET_HEALTH_INFO:
>   		rc = mock_health_info(cxlds, cmd);
>   		break;
> +	case CXL_MBOX_OP_SANITIZE:
> +		rc = mock_sanitize(cxlds, cmd);
> +		break;
>   	case CXL_MBOX_OP_GET_SECURITY_STATE:
>   		rc = mock_get_security_state(cxlds, cmd);
>   		break;

[PATCH 6/7] cxl/mem: Support Secure Erase

[Davidlohr Bueso]

Implement support for the non-pmem exclusive secure erase, per
CXL specs.

To properly support this feature, create a 'security/erase' sysfs
file that when read will list the current pmem security state and
when written to, perform the requested operation.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 Documentation/ABI/testing/sysfs-bus-cxl | 12 ++++++
 drivers/cxl/core/mbox.c                 | 56 +++++++++++++++++++++++++
 drivers/cxl/core/memdev.c               | 32 +++++++++++++-
 drivers/cxl/cxlmem.h                    |  2 +
 4 files changed, 101 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index b315d78b7e91..91a74e27f248 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -80,6 +80,18 @@ Description:
 	       to be flushed. If this sysfs entry is not present then the
 	       architecture does not support security features.
 
+What:          /sys/bus/cxl/devices/memX/security/erase
+Date:          February, 2023
+KernelVersion: v6.4
+Contact:       linux-cxl@vger.kernel.org
+Description:
+	       (WO) Write a boolean 'true' string value to this attribute to
+	       secure erase the device to securely re-purpose or decommission
+	       it. This is done by hanging the media encryption keys for all
+	       user data areas of the device. This causes all CPU caches to
+	       be flushed. If this sysfs entry is not present then the
+	       architecture does not support security features.
+
 What:		/sys/bus/cxl/devices/*/devtype
 Date:		June, 2021
 KernelVersion:	v5.14
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 885de3506735..bf206fe26839 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -1082,6 +1082,62 @@ int cxl_mem_sanitize(struct cxl_dev_state *cxlds)
 }
 EXPORT_SYMBOL_NS_GPL(cxl_mem_sanitize, CXL);
 
+/**
+ * cxl_mem_secure_erase() - Send secure erase command to the device.
+ * @cxlds: The device data for the operation
+ *
+ * Return: 0 if the command was executed successfully.
+ * Upon error, return the result of the mailbox command or -EINVAL if
+ * security requirements are not met. CPU caches are flushed before and
+ * after succesful completion of each command.
+ *
+ * See CXL 3.0 @8.2.9.8.5.2 Secure Erase.
+ */
+int cxl_mem_secure_erase(struct cxl_dev_state *cxlds)
+{
+	int rc;
+	u32 sec_out = 0;
+	struct cxl_get_security_output {
+		__le32 flags;
+	} out;
+	struct cxl_mbox_cmd sec_cmd = {
+		.opcode = CXL_MBOX_OP_GET_SECURITY_STATE,
+		.payload_out = &out,
+		.size_out = sizeof(out),
+	};
+	struct cxl_mbox_cmd mbox_cmd = {
+		.opcode = CXL_MBOX_OP_SECURE_ERASE,
+	};
+
+	if (!cpu_cache_has_invalidate_memregion())
+		return -EINVAL;
+
+	rc = cxl_internal_send_cmd(cxlds, &sec_cmd);
+	if (rc < 0) {
+		dev_err(cxlds->dev, "Failed to get security state : %d", rc);
+		return rc;
+	}
+
+	sec_out = le32_to_cpu(out.flags);
+	if (sec_out & CXL_PMEM_SEC_STATE_USER_PASS_SET)
+		return -EINVAL;
+
+	if (sec_out & CXL_PMEM_SEC_STATE_LOCKED)
+		return -EINVAL;
+
+	cpu_cache_invalidate_memregion(IORES_DESC_CXL);
+
+	rc = cxl_internal_send_cmd(cxlds, &mbox_cmd);
+	if (rc < 0) {
+		dev_err(cxlds->dev, "Failed to secure erase device : %d", rc);
+		return rc;
+	}
+
+	cpu_cache_invalidate_memregion(IORES_DESC_CXL);
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_mem_secure_erase, CXL);
+
 static int add_dpa_res(struct device *dev, struct resource *parent,
 		       struct resource *res, resource_size_t start,
 		       resource_size_t size, const char *type)
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index a1bb095d081c..6334a0d1a925 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -155,6 +155,34 @@ static ssize_t security_sanitize_store(struct device *dev,
 static struct device_attribute dev_attr_security_sanitize =
 	__ATTR(sanitize, 0200, NULL, security_sanitize_store);
 
+static ssize_t security_erase_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t len)
+{
+	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
+	struct cxl_dev_state *cxlds = cxlmd->cxlds;
+	ssize_t rc;
+	bool erase;
+
+	rc = kstrtobool(buf, &erase);
+	if (rc)
+		return rc;
+
+	if (erase) {
+		if (cxl_memdev_active_region(cxlmd))
+			return -EBUSY;
+
+		rc = cxl_mem_secure_erase(cxlds);
+	}
+
+	if (rc == 0)
+		rc = len;
+	return rc;
+}
+
+static struct device_attribute dev_attr_security_erase =
+	__ATTR(sanitize, 0200, NULL, security_erase_store);
+
 static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)
 {
@@ -217,6 +245,7 @@ static struct attribute_group cxl_memdev_pmem_attribute_group = {
 static struct attribute *cxl_memdev_security_attributes[] = {
 	&dev_attr_security_state.attr,
 	&dev_attr_security_sanitize.attr,
+	&dev_attr_security_erase.attr,
 	NULL,
 };
 
@@ -224,7 +253,8 @@ static umode_t cxl_security_visible(struct kobject *kobj,
 				    struct attribute *a, int n)
 {
 	if (!cpu_cache_has_invalidate_memregion() &&
-	    a == &dev_attr_security_sanitize.attr)
+	    (a == &dev_attr_security_sanitize.attr ||
+	     a == &dev_attr_security_erase.attr))
 		return 0;
 	return a->mode;
 }
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 0d2009b36933..2cf9ec3242a6 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -332,6 +332,7 @@ enum cxl_opcode {
 	CXL_MBOX_OP_SCAN_MEDIA		= 0x4304,
 	CXL_MBOX_OP_GET_SCAN_MEDIA	= 0x4305,
 	CXL_MBOX_OP_SANITIZE		= 0x4400,
+	CXL_MBOX_OP_SECURE_ERASE	= 0x4401,
 	CXL_MBOX_OP_GET_SECURITY_STATE	= 0x4500,
 	CXL_MBOX_OP_SET_PASSPHRASE	= 0x4501,
 	CXL_MBOX_OP_DISABLE_PASSPHRASE	= 0x4502,
@@ -632,6 +633,7 @@ static inline void cxl_mem_active_dec(void)
 #endif
 
 int cxl_mem_sanitize(struct cxl_dev_state *cxlds);
+int cxl_mem_secure_erase(struct cxl_dev_state *cxlds);
 
 struct cxl_hdm {
 	struct cxl_component_regs regs;
-- 
2.39.2

Re: [PATCH 6/7] cxl/mem: Support Secure Erase

[Dave Jiang]

On 2/24/23 12:46 PM, Davidlohr Bueso wrote:
> Implement support for the non-pmem exclusive secure erase, per
> CXL specs.
> 
> To properly support this feature, create a 'security/erase' sysfs
> file that when read will list the current pmem security state and
> when written to, perform the requested operation.

Need update. WO attrib.

DJ

> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>   Documentation/ABI/testing/sysfs-bus-cxl | 12 ++++++
>   drivers/cxl/core/mbox.c                 | 56 +++++++++++++++++++++++++
>   drivers/cxl/core/memdev.c               | 32 +++++++++++++-
>   drivers/cxl/cxlmem.h                    |  2 +
>   4 files changed, 101 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index b315d78b7e91..91a74e27f248 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -80,6 +80,18 @@ Description:
>   	       to be flushed. If this sysfs entry is not present then the
>   	       architecture does not support security features.
>   
> +What:          /sys/bus/cxl/devices/memX/security/erase
> +Date:          February, 2023
> +KernelVersion: v6.4
> +Contact:       linux-cxl@vger.kernel.org
> +Description:
> +	       (WO) Write a boolean 'true' string value to this attribute to
> +	       secure erase the device to securely re-purpose or decommission
> +	       it. This is done by hanging the media encryption keys for all
> +	       user data areas of the device. This causes all CPU caches to
> +	       be flushed. If this sysfs entry is not present then the
> +	       architecture does not support security features.
> +
>   What:		/sys/bus/cxl/devices/*/devtype
>   Date:		June, 2021
>   KernelVersion:	v5.14
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 885de3506735..bf206fe26839 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -1082,6 +1082,62 @@ int cxl_mem_sanitize(struct cxl_dev_state *cxlds)
>   }
>   EXPORT_SYMBOL_NS_GPL(cxl_mem_sanitize, CXL);
>   
> +/**
> + * cxl_mem_secure_erase() - Send secure erase command to the device.
> + * @cxlds: The device data for the operation
> + *
> + * Return: 0 if the command was executed successfully.
> + * Upon error, return the result of the mailbox command or -EINVAL if
> + * security requirements are not met. CPU caches are flushed before and
> + * after succesful completion of each command.
> + *
> + * See CXL 3.0 @8.2.9.8.5.2 Secure Erase.
> + */
> +int cxl_mem_secure_erase(struct cxl_dev_state *cxlds)
> +{
> +	int rc;
> +	u32 sec_out = 0;
> +	struct cxl_get_security_output {
> +		__le32 flags;
> +	} out;
> +	struct cxl_mbox_cmd sec_cmd = {
> +		.opcode = CXL_MBOX_OP_GET_SECURITY_STATE,
> +		.payload_out = &out,
> +		.size_out = sizeof(out),
> +	};
> +	struct cxl_mbox_cmd mbox_cmd = {
> +		.opcode = CXL_MBOX_OP_SECURE_ERASE,
> +	};
> +
> +	if (!cpu_cache_has_invalidate_memregion())
> +		return -EINVAL;
> +
> +	rc = cxl_internal_send_cmd(cxlds, &sec_cmd);
> +	if (rc < 0) {
> +		dev_err(cxlds->dev, "Failed to get security state : %d", rc);
> +		return rc;
> +	}
> +
> +	sec_out = le32_to_cpu(out.flags);
> +	if (sec_out & CXL_PMEM_SEC_STATE_USER_PASS_SET)
> +		return -EINVAL;
> +
> +	if (sec_out & CXL_PMEM_SEC_STATE_LOCKED)
> +		return -EINVAL;
> +
> +	cpu_cache_invalidate_memregion(IORES_DESC_CXL);
> +
> +	rc = cxl_internal_send_cmd(cxlds, &mbox_cmd);
> +	if (rc < 0) {
> +		dev_err(cxlds->dev, "Failed to secure erase device : %d", rc);
> +		return rc;
> +	}
> +
> +	cpu_cache_invalidate_memregion(IORES_DESC_CXL);
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_mem_secure_erase, CXL);
> +
>   static int add_dpa_res(struct device *dev, struct resource *parent,
>   		       struct resource *res, resource_size_t start,
>   		       resource_size_t size, const char *type)
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index a1bb095d081c..6334a0d1a925 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -155,6 +155,34 @@ static ssize_t security_sanitize_store(struct device *dev,
>   static struct device_attribute dev_attr_security_sanitize =
>   	__ATTR(sanitize, 0200, NULL, security_sanitize_store);
>   
> +static ssize_t security_erase_store(struct device *dev,
> +				    struct device_attribute *attr,
> +				    const char *buf, size_t len)
> +{
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +	ssize_t rc;
> +	bool erase;
> +
> +	rc = kstrtobool(buf, &erase);
> +	if (rc)
> +		return rc;
> +
> +	if (erase) {
> +		if (cxl_memdev_active_region(cxlmd))
> +			return -EBUSY;
> +
> +		rc = cxl_mem_secure_erase(cxlds);
> +	}
> +
> +	if (rc == 0)
> +		rc = len;
> +	return rc;
> +}
> +
> +static struct device_attribute dev_attr_security_erase =
> +	__ATTR(sanitize, 0200, NULL, security_erase_store);
> +
>   static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
>   			   char *buf)
>   {
> @@ -217,6 +245,7 @@ static struct attribute_group cxl_memdev_pmem_attribute_group = {
>   static struct attribute *cxl_memdev_security_attributes[] = {
>   	&dev_attr_security_state.attr,
>   	&dev_attr_security_sanitize.attr,
> +	&dev_attr_security_erase.attr,
>   	NULL,
>   };
>   
> @@ -224,7 +253,8 @@ static umode_t cxl_security_visible(struct kobject *kobj,
>   				    struct attribute *a, int n)
>   {
>   	if (!cpu_cache_has_invalidate_memregion() &&
> -	    a == &dev_attr_security_sanitize.attr)
> +	    (a == &dev_attr_security_sanitize.attr ||
> +	     a == &dev_attr_security_erase.attr))
>   		return 0;
>   	return a->mode;
>   }
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 0d2009b36933..2cf9ec3242a6 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -332,6 +332,7 @@ enum cxl_opcode {
>   	CXL_MBOX_OP_SCAN_MEDIA		= 0x4304,
>   	CXL_MBOX_OP_GET_SCAN_MEDIA	= 0x4305,
>   	CXL_MBOX_OP_SANITIZE		= 0x4400,
> +	CXL_MBOX_OP_SECURE_ERASE	= 0x4401,
>   	CXL_MBOX_OP_GET_SECURITY_STATE	= 0x4500,
>   	CXL_MBOX_OP_SET_PASSPHRASE	= 0x4501,
>   	CXL_MBOX_OP_DISABLE_PASSPHRASE	= 0x4502,
> @@ -632,6 +633,7 @@ static inline void cxl_mem_active_dec(void)
>   #endif
>   
>   int cxl_mem_sanitize(struct cxl_dev_state *cxlds);
> +int cxl_mem_secure_erase(struct cxl_dev_state *cxlds);
>   
>   struct cxl_hdm {
>   	struct cxl_component_regs regs;

[PATCH 7/7] cxl/test: Add "Secure Erase" opcode support

[Davidlohr Bueso]

Add support to emulate a CXL mem device support the "Secure Erase"
operation.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 tools/testing/cxl/test/mem.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
index d4466cb27947..8a22a4e592c6 100644
--- a/tools/testing/cxl/test/mem.c
+++ b/tools/testing/cxl/test/mem.c
@@ -519,6 +519,30 @@ static int mock_sanitize(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd)
 	return 0; /* assume less than 2 secs, no bg */
 }
 
+static int mock_secure_erase(struct cxl_dev_state *cxlds,
+			     struct cxl_mbox_cmd *cmd)
+{
+	struct cxl_mockmem_data *mdata = dev_get_drvdata(cxlds->dev);
+
+	if (cmd->size_in != 0)
+		return -EINVAL;
+
+	if (cmd->size_out != 0)
+		return -EINVAL;
+
+	if (mdata->security_state & CXL_PMEM_SEC_STATE_USER_PASS_SET) {
+		cmd->return_code = CXL_MBOX_CMD_RC_SECURITY;
+		return -ENXIO;
+	}
+
+	if (mdata->security_state & CXL_PMEM_SEC_STATE_LOCKED) {
+		cmd->return_code = CXL_MBOX_CMD_RC_SECURITY;
+		return -ENXIO;
+	}
+
+	return 0;
+}
+
 static int mock_get_security_state(struct cxl_dev_state *cxlds,
 				   struct cxl_mbox_cmd *cmd)
 {
@@ -949,6 +973,9 @@ static int cxl_mock_mbox_send(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *
 	case CXL_MBOX_OP_SANITIZE:
 		rc = mock_sanitize(cxlds, cmd);
 		break;
+	case CXL_MBOX_OP_SECURE_ERASE:
+		rc = mock_secure_erase(cxlds, cmd);
+		break;
 	case CXL_MBOX_OP_GET_SECURITY_STATE:
 		rc = mock_get_security_state(cxlds, cmd);
 		break;
-- 
2.39.2

Re: [PATCH 7/7] cxl/test: Add "Secure Erase" opcode support

[Dave Jiang]

On 2/24/23 12:46 PM, Davidlohr Bueso wrote:
> Add support to emulate a CXL mem device support the "Secure Erase"
> operation.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>   tools/testing/cxl/test/mem.c | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
> 
> diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
> index d4466cb27947..8a22a4e592c6 100644
> --- a/tools/testing/cxl/test/mem.c
> +++ b/tools/testing/cxl/test/mem.c
> @@ -519,6 +519,30 @@ static int mock_sanitize(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd)
>   	return 0; /* assume less than 2 secs, no bg */
>   }
>   
> +static int mock_secure_erase(struct cxl_dev_state *cxlds,
> +			     struct cxl_mbox_cmd *cmd)
> +{
> +	struct cxl_mockmem_data *mdata = dev_get_drvdata(cxlds->dev);
> +
> +	if (cmd->size_in != 0)
> +		return -EINVAL;
> +
> +	if (cmd->size_out != 0)
> +		return -EINVAL;
> +
> +	if (mdata->security_state & CXL_PMEM_SEC_STATE_USER_PASS_SET) {
> +		cmd->return_code = CXL_MBOX_CMD_RC_SECURITY;
> +		return -ENXIO;
> +	}
> +
> +	if (mdata->security_state & CXL_PMEM_SEC_STATE_LOCKED) {
> +		cmd->return_code = CXL_MBOX_CMD_RC_SECURITY;
> +		return -ENXIO;
> +	}
> +
> +	return 0;
> +}
> +
>   static int mock_get_security_state(struct cxl_dev_state *cxlds,
>   				   struct cxl_mbox_cmd *cmd)
>   {
> @@ -949,6 +973,9 @@ static int cxl_mock_mbox_send(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *
>   	case CXL_MBOX_OP_SANITIZE:
>   		rc = mock_sanitize(cxlds, cmd);
>   		break;
> +	case CXL_MBOX_OP_SECURE_ERASE:
> +		rc = mock_secure_erase(cxlds, cmd);
> +		break;
>   	case CXL_MBOX_OP_GET_SECURITY_STATE:
>   		rc = mock_get_security_state(cxlds, cmd);
>   		break;

Re: [PATCH v3 0/7] cxl: Background cmds and device sanitation

[Davidlohr Bueso]

On Fri, 24 Feb 2023, Davidlohr Bueso wrote:

>Hello,
>
>This series adds support for the rest of the security related cxl operations
>which are not strictly dependent on PMEM.
>
>Patch 1 adds the required background cmd handling bits (polling and irq).
>This is really the main motivation of the series as there are various
>features that will need this regardless of sanitation.
>
>Patch 2 adds a new sysfs 'security/state' file, which shows "disabled" if
>no security features are available - which is more intuitive than no showing
>it.
>
>Patch 3 adds a way to check if the device is actively used by tracking regions
>with target the memdevice to sanitize. I realize this might not be what is
>desired and wanted to get the expectations for this. This is more of a RFC
>patch.
>
>Patches 4-7 add the Sanitation and Secure Erase support, per CXL3.0 specs.
>
>These changes have been tested with both the mock device as well as with qemu[0,1].
>
>Changes from v2 (https://lore.kernel.org/linux-cxl/20221206011501.464916-1-dave@stgolabs.net/):
>- Redid the bacground cmd handling to use a syncronous approach instead.
>- Added cxl_memdev_active_region() to check if the memdevice is being used
>  (actively decoding any HPA ranges).
>- Create a new security sysfs directory with a 'state' file which is always visible.
>- The sysfs files' to trigger the security commands is only visible if
>  cpu_cache_has_invalidate_memregion().
>- Added a sanitize test for the mock device.
>
>Please consider for v6.4.

Hi Dan, ping? I would like to see were we stand with this series and hopefully
still time for 6.4 material - but that's not important. The open points which
imo would best be answered before attempting a v4 based on the current input:

- Are you ok with the approach in patch 1?

- Are you ok with security directory always showing at least the state file? As
Dave mentions this differs somewhat with nfit, but for that matter so does the
whole security semantics (this is now a directory vs a single, multiplexed file).

- Regarding ensuring the device is not actively decoding, would Alison/Fan's suggestion
to use commit_end/hdm_end instead of my hacky patch 3 be feasible? Or did you
have something else in mind?

Thanks,
Davidlohr