* [PATCH 1/7] cxl/mbox: Allow for IRQ_NONE case in the isr
2023-06-12 18:10 [PATCH v6 0/7] cxl: Support device sanitation Davidlohr Bueso
@ 2023-06-12 18:10 ` Davidlohr Bueso
2023-06-13 12:49 ` Jonathan Cameron
2023-06-13 18:11 ` Dave Jiang
2023-06-12 18:10 ` [PATCH 2/7] cxl/mem: Introduce security state sysfs file Davidlohr Bueso
` (7 subsequent siblings)
8 siblings, 2 replies; 30+ messages in thread
From: Davidlohr Bueso @ 2023-06-12 18:10 UTC (permalink / raw)
To: dan.j.williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, dave, linux-cxl, Jonathan Cameron
For cases when the mailbox background operation is not complete,
do not "handle" the interrupt, as it was not from this device.
And furthermore there are no racy scenarios such as the hw being
out of sync with the driver and starting a new background op
behind its back.
Reported-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Fixes: ccadf1310fb (cxl/mbox: Add background cmd handling machinery)
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
drivers/cxl/pci.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index a78e40e6d0e0..4b2575502f49 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -118,9 +118,11 @@ static irqreturn_t cxl_pci_mbox_irq(int irq, void *id)
struct cxl_dev_id *dev_id = id;
struct cxl_dev_state *cxlds = dev_id->cxlds;
+ if (!cxl_mbox_background_complete(cxlds))
+ return IRQ_NONE;
+
/* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
- if (cxl_mbox_background_complete(cxlds))
- rcuwait_wake_up(&cxlds->mbox_wait);
+ rcuwait_wake_up(&cxlds->mbox_wait);
return IRQ_HANDLED;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/7] cxl/mbox: Allow for IRQ_NONE case in the isr
2023-06-12 18:10 ` [PATCH 1/7] cxl/mbox: Allow for IRQ_NONE case in the isr Davidlohr Bueso
@ 2023-06-13 12:49 ` Jonathan Cameron
2023-06-13 18:11 ` Dave Jiang
1 sibling, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2023-06-13 12:49 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: dan.j.williams, dave.jiang, vishal.l.verma, fan.ni, a.manzanares,
linux-cxl
On Mon, 12 Jun 2023 11:10:32 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:
> For cases when the mailbox background operation is not complete,
> do not "handle" the interrupt, as it was not from this device.
> And furthermore there are no racy scenarios such as the hw being
> out of sync with the driver and starting a new background op
> behind its back.
>
> Reported-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Fixes: ccadf1310fb (cxl/mbox: Add background cmd handling machinery)
Fixes is a bit strong for something that shouldn't be seen anyway.
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
Otherwise LGTM
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> drivers/cxl/pci.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index a78e40e6d0e0..4b2575502f49 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -118,9 +118,11 @@ static irqreturn_t cxl_pci_mbox_irq(int irq, void *id)
> struct cxl_dev_id *dev_id = id;
> struct cxl_dev_state *cxlds = dev_id->cxlds;
>
> + if (!cxl_mbox_background_complete(cxlds))
> + return IRQ_NONE;
> +
> /* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
> - if (cxl_mbox_background_complete(cxlds))
> - rcuwait_wake_up(&cxlds->mbox_wait);
> + rcuwait_wake_up(&cxlds->mbox_wait);
>
> return IRQ_HANDLED;
> }
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/7] cxl/mbox: Allow for IRQ_NONE case in the isr
2023-06-12 18:10 ` [PATCH 1/7] cxl/mbox: Allow for IRQ_NONE case in the isr Davidlohr Bueso
2023-06-13 12:49 ` Jonathan Cameron
@ 2023-06-13 18:11 ` Dave Jiang
1 sibling, 0 replies; 30+ messages in thread
From: Dave Jiang @ 2023-06-13 18:11 UTC (permalink / raw)
To: Davidlohr Bueso, dan.j.williams
Cc: vishal.l.verma, Jonathan.Cameron, fan.ni, a.manzanares, linux-cxl
On 6/12/23 11:10, Davidlohr Bueso wrote:
> For cases when the mailbox background operation is not complete,
> do not "handle" the interrupt, as it was not from this device.
> And furthermore there are no racy scenarios such as the hw being
> out of sync with the driver and starting a new background op
> behind its back.
>
> Reported-by: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Fixes: ccadf1310fb (cxl/mbox: Add background cmd handling machinery)
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/cxl/pci.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index a78e40e6d0e0..4b2575502f49 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -118,9 +118,11 @@ static irqreturn_t cxl_pci_mbox_irq(int irq, void *id)
> struct cxl_dev_id *dev_id = id;
> struct cxl_dev_state *cxlds = dev_id->cxlds;
>
> + if (!cxl_mbox_background_complete(cxlds))
> + return IRQ_NONE;
> +
> /* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
> - if (cxl_mbox_background_complete(cxlds))
> - rcuwait_wake_up(&cxlds->mbox_wait);
> + rcuwait_wake_up(&cxlds->mbox_wait);
>
> return IRQ_HANDLED;
> }
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2/7] cxl/mem: Introduce security state sysfs file
2023-06-12 18:10 [PATCH v6 0/7] cxl: Support device sanitation Davidlohr Bueso
2023-06-12 18:10 ` [PATCH 1/7] cxl/mbox: Allow for IRQ_NONE case in the isr Davidlohr Bueso
@ 2023-06-12 18:10 ` Davidlohr Bueso
2023-06-13 18:12 ` Dave Jiang
2023-06-12 18:10 ` [PATCH 3/7] cxl/mbox: Add sanitation handling machinery Davidlohr Bueso
` (6 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Davidlohr Bueso @ 2023-06-12 18:10 UTC (permalink / raw)
To: dan.j.williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, dave, linux-cxl
Add a read-only sysfs file to display the security state
of a device (currently only pmem):
/sys/bus/cxl/devices/memX/security/state
This introduces a cxl_security_state structure that is
to be the placeholder for common CXL security features.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Fan Ni <fan.ni@samsung.com>
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
Documentation/ABI/testing/sysfs-bus-cxl | 10 ++++++++
drivers/cxl/core/memdev.c | 33 +++++++++++++++++++++++++
drivers/cxl/cxlmem.h | 10 ++++++++
drivers/cxl/security.c | 3 +++
4 files changed, 56 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index 48ac0d911801..721a44d8a482 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -58,6 +58,16 @@ Description:
affinity for this device.
+What: /sys/bus/cxl/devices/memX/security/state
+Date: June, 2023
+KernelVersion: v6.5
+Contact: linux-cxl@vger.kernel.org
+Description:
+ (RO) Reading this file will display the CXL security state for
+ that device. Such states can be: 'disabled', or those available
+ only for persistent memory: 'locked', 'unlocked' or 'frozen'.
+
+
What: /sys/bus/cxl/devices/*/devtype
Date: June, 2021
KernelVersion: v5.14
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 057a43267290..1bbb7e39fc93 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -107,6 +107,28 @@ static ssize_t numa_node_show(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_RO(numa_node);
+static ssize_t security_state_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
+ struct cxl_dev_state *cxlds = cxlmd->cxlds;
+ unsigned long state = cxlds->security.state;
+
+ if (!(state & CXL_PMEM_SEC_STATE_USER_PASS_SET))
+ return sysfs_emit(buf, "disabled\n");
+ if (state & CXL_PMEM_SEC_STATE_FROZEN ||
+ state & CXL_PMEM_SEC_STATE_MASTER_PLIMIT ||
+ state & CXL_PMEM_SEC_STATE_USER_PLIMIT)
+ return sysfs_emit(buf, "frozen\n");
+ if (state & CXL_PMEM_SEC_STATE_LOCKED)
+ return sysfs_emit(buf, "locked\n");
+ else
+ return sysfs_emit(buf, "unlocked\n");
+}
+static struct device_attribute dev_attr_security_state =
+ __ATTR(state, 0444, security_state_show, NULL);
+
static int cxl_get_poison_by_memdev(struct cxl_memdev *cxlmd)
{
struct cxl_dev_state *cxlds = cxlmd->cxlds;
@@ -352,6 +374,11 @@ static struct attribute *cxl_memdev_ram_attributes[] = {
NULL,
};
+static struct attribute *cxl_memdev_security_attributes[] = {
+ &dev_attr_security_state.attr,
+ NULL,
+};
+
static umode_t cxl_memdev_visible(struct kobject *kobj, struct attribute *a,
int n)
{
@@ -375,10 +402,16 @@ static struct attribute_group cxl_memdev_pmem_attribute_group = {
.attrs = cxl_memdev_pmem_attributes,
};
+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,
};
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 1d8e81c87c6a..091f1200736b 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -260,6 +260,15 @@ struct cxl_poison_state {
struct mutex lock; /* Protect reads of poison list */
};
+/**
+ * struct cxl_security_state - Device security state
+ *
+ * @state: state of last security operation
+ */
+struct cxl_security_state {
+ unsigned long state;
+};
+
/**
* struct cxl_dev_state - The driver device state
*
@@ -336,6 +345,7 @@ struct cxl_dev_state {
struct cxl_event_state event;
struct cxl_poison_state poison;
+ struct cxl_security_state security;
struct rcuwait mbox_wait;
int (*mbox_send)(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd);
diff --git a/drivers/cxl/security.c b/drivers/cxl/security.c
index 4ad4bda2d18e..9da6785dfd31 100644
--- a/drivers/cxl/security.c
+++ b/drivers/cxl/security.c
@@ -34,6 +34,9 @@ static unsigned long cxl_pmem_get_security_flags(struct nvdimm *nvdimm,
return 0;
sec_out = le32_to_cpu(out.flags);
+ /* cache security state */
+ cxlds->security.state = sec_out;
+
if (ptype == NVDIMM_MASTER) {
if (sec_out & CXL_PMEM_SEC_STATE_MASTER_PASS_SET)
set_bit(NVDIMM_SECURITY_UNLOCKED, &security_flags);
--
2.41.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 2/7] cxl/mem: Introduce security state sysfs file
2023-06-12 18:10 ` [PATCH 2/7] cxl/mem: Introduce security state sysfs file Davidlohr Bueso
@ 2023-06-13 18:12 ` Dave Jiang
0 siblings, 0 replies; 30+ messages in thread
From: Dave Jiang @ 2023-06-13 18:12 UTC (permalink / raw)
To: Davidlohr Bueso, dan.j.williams
Cc: vishal.l.verma, Jonathan.Cameron, fan.ni, a.manzanares, linux-cxl
On 6/12/23 11:10, Davidlohr Bueso wrote:
> Add a read-only sysfs file to display the security state
> of a device (currently only pmem):
>
> /sys/bus/cxl/devices/memX/security/state
>
> This introduces a cxl_security_state structure that is
> to be the placeholder for common CXL security features.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Fan Ni <fan.ni@samsung.com>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> Documentation/ABI/testing/sysfs-bus-cxl | 10 ++++++++
> drivers/cxl/core/memdev.c | 33 +++++++++++++++++++++++++
> drivers/cxl/cxlmem.h | 10 ++++++++
> drivers/cxl/security.c | 3 +++
> 4 files changed, 56 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 48ac0d911801..721a44d8a482 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -58,6 +58,16 @@ Description:
> affinity for this device.
>
>
> +What: /sys/bus/cxl/devices/memX/security/state
> +Date: June, 2023
> +KernelVersion: v6.5
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RO) Reading this file will display the CXL security state for
> + that device. Such states can be: 'disabled', or those available
> + only for persistent memory: 'locked', 'unlocked' or 'frozen'.
> +
> +
> What: /sys/bus/cxl/devices/*/devtype
> Date: June, 2021
> KernelVersion: v5.14
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 057a43267290..1bbb7e39fc93 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -107,6 +107,28 @@ static ssize_t numa_node_show(struct device *dev, struct device_attribute *attr,
> }
> static DEVICE_ATTR_RO(numa_node);
>
> +static ssize_t security_state_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> + struct cxl_dev_state *cxlds = cxlmd->cxlds;
> + unsigned long state = cxlds->security.state;
> +
> + if (!(state & CXL_PMEM_SEC_STATE_USER_PASS_SET))
> + return sysfs_emit(buf, "disabled\n");
> + if (state & CXL_PMEM_SEC_STATE_FROZEN ||
> + state & CXL_PMEM_SEC_STATE_MASTER_PLIMIT ||
> + state & CXL_PMEM_SEC_STATE_USER_PLIMIT)
> + return sysfs_emit(buf, "frozen\n");
> + if (state & CXL_PMEM_SEC_STATE_LOCKED)
> + return sysfs_emit(buf, "locked\n");
> + else
> + return sysfs_emit(buf, "unlocked\n");
> +}
> +static struct device_attribute dev_attr_security_state =
> + __ATTR(state, 0444, security_state_show, NULL);
> +
> static int cxl_get_poison_by_memdev(struct cxl_memdev *cxlmd)
> {
> struct cxl_dev_state *cxlds = cxlmd->cxlds;
> @@ -352,6 +374,11 @@ static struct attribute *cxl_memdev_ram_attributes[] = {
> NULL,
> };
>
> +static struct attribute *cxl_memdev_security_attributes[] = {
> + &dev_attr_security_state.attr,
> + NULL,
> +};
> +
> static umode_t cxl_memdev_visible(struct kobject *kobj, struct attribute *a,
> int n)
> {
> @@ -375,10 +402,16 @@ static struct attribute_group cxl_memdev_pmem_attribute_group = {
> .attrs = cxl_memdev_pmem_attributes,
> };
>
> +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,
> };
>
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 1d8e81c87c6a..091f1200736b 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -260,6 +260,15 @@ struct cxl_poison_state {
> struct mutex lock; /* Protect reads of poison list */
> };
>
> +/**
> + * struct cxl_security_state - Device security state
> + *
> + * @state: state of last security operation
> + */
> +struct cxl_security_state {
> + unsigned long state;
> +};
> +
> /**
> * struct cxl_dev_state - The driver device state
> *
> @@ -336,6 +345,7 @@ struct cxl_dev_state {
>
> struct cxl_event_state event;
> struct cxl_poison_state poison;
> + struct cxl_security_state security;
>
> struct rcuwait mbox_wait;
> int (*mbox_send)(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd);
> diff --git a/drivers/cxl/security.c b/drivers/cxl/security.c
> index 4ad4bda2d18e..9da6785dfd31 100644
> --- a/drivers/cxl/security.c
> +++ b/drivers/cxl/security.c
> @@ -34,6 +34,9 @@ static unsigned long cxl_pmem_get_security_flags(struct nvdimm *nvdimm,
> return 0;
>
> sec_out = le32_to_cpu(out.flags);
> + /* cache security state */
> + cxlds->security.state = sec_out;
> +
> if (ptype == NVDIMM_MASTER) {
> if (sec_out & CXL_PMEM_SEC_STATE_MASTER_PASS_SET)
> set_bit(NVDIMM_SECURITY_UNLOCKED, &security_flags);
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 3/7] cxl/mbox: Add sanitation handling machinery
2023-06-12 18:10 [PATCH v6 0/7] cxl: Support device sanitation Davidlohr Bueso
2023-06-12 18:10 ` [PATCH 1/7] cxl/mbox: Allow for IRQ_NONE case in the isr Davidlohr Bueso
2023-06-12 18:10 ` [PATCH 2/7] cxl/mem: Introduce security state sysfs file Davidlohr Bueso
@ 2023-06-12 18:10 ` Davidlohr Bueso
2023-06-13 16:07 ` Jonathan Cameron
` (2 more replies)
2023-06-12 18:10 ` [PATCH 4/7] cxl/mem: Wire up Sanitation support Davidlohr Bueso
` (5 subsequent siblings)
8 siblings, 3 replies; 30+ messages in thread
From: Davidlohr Bueso @ 2023-06-12 18:10 UTC (permalink / raw)
To: dan.j.williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, dave, linux-cxl
Sanitation is by definition a device-monopolizing operation, and thus
the timeslicing rules for other background commands do not apply.
As such handle this special case asynchronously and return immediately.
Subsequent changes will allow completion to be pollable from userspace
via a sysfs file interface.
For devices that don't support interrupts for notifying background
command completion, self-poll with the caveat that the poller can
be out of sync with the ready hardware, and therefore care must be
taken to not allow any new commands to go through until the poller
sees the hw completion. The poller takes the mbox_mutex to stabilize
the flagging, minimizing any runtime overhead in the send path to
check for 'sanitize_tmo' for uncommon poll scenarios.
The irq case is much simpler as hardware will serialize/error
appropriately.
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
drivers/cxl/core/memdev.c | 10 +++++
drivers/cxl/cxlmem.h | 7 ++++
drivers/cxl/pci.c | 77 +++++++++++++++++++++++++++++++++++++--
3 files changed, 91 insertions(+), 3 deletions(-)
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 1bbb7e39fc93..834f418b6bcb 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -460,11 +460,21 @@ void clear_exclusive_cxl_commands(struct cxl_dev_state *cxlds, unsigned long *cm
}
EXPORT_SYMBOL_NS_GPL(clear_exclusive_cxl_commands, CXL);
+static void cxl_memdev_security_shutdown(struct device *dev)
+{
+ struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
+ struct cxl_dev_state *cxlds = cxlmd->cxlds;
+
+ if (cxlds->security.poll)
+ cancel_delayed_work_sync(&cxlds->security.poll_dwork);
+}
+
static void cxl_memdev_shutdown(struct device *dev)
{
struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
down_write(&cxl_memdev_rwsem);
+ cxl_memdev_security_shutdown(dev);
cxlmd->cxlds = NULL;
up_write(&cxl_memdev_rwsem);
}
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 091f1200736b..3a9df1044144 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -264,9 +264,15 @@ struct cxl_poison_state {
* struct cxl_security_state - Device security state
*
* @state: state of last security operation
+ * @poll: polling for sanitation is enabled, device has no mbox irq support
+ * @poll_tmo_secs: polling timeout
+ * @poll_dwork: polling work item
*/
struct cxl_security_state {
unsigned long state;
+ bool poll;
+ int poll_tmo_secs;
+ struct delayed_work poll_dwork;
};
/**
@@ -379,6 +385,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 4b2575502f49..c92eab55a5a7 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -115,18 +115,52 @@ static bool cxl_mbox_background_complete(struct cxl_dev_state *cxlds)
static irqreturn_t cxl_pci_mbox_irq(int irq, void *id)
{
+ u64 reg;
+ u16 opcode;
struct cxl_dev_id *dev_id = id;
struct cxl_dev_state *cxlds = dev_id->cxlds;
if (!cxl_mbox_background_complete(cxlds))
return IRQ_NONE;
- /* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
- rcuwait_wake_up(&cxlds->mbox_wait);
+ reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
+ opcode = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK, reg);
+ if (opcode == CXL_MBOX_OP_SANITIZE) {
+ dev_dbg(cxlds->dev, "Sanitation operation ended\n");
+ } else {
+ /* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
+ rcuwait_wake_up(&cxlds->mbox_wait);
+ }
return IRQ_HANDLED;
}
+/*
+ * Sanitation operation polling mode.
+ */
+static void cxl_mbox_sanitize_work(struct work_struct *work)
+{
+ struct cxl_dev_state *cxlds;
+
+ cxlds = container_of(work,
+ struct cxl_dev_state, security.poll_dwork.work);
+
+ mutex_lock(&cxlds->mbox_mutex);
+ if (cxl_mbox_background_complete(cxlds)) {
+ cxlds->security.poll_tmo_secs = 0;
+ put_device(cxlds->dev);
+
+ dev_dbg(cxlds->dev, "Sanitation operation ended\n");
+ } else {
+ int timeout = cxlds->security.poll_tmo_secs + 10;
+
+ cxlds->security.poll_tmo_secs = min(15 * 60, timeout);
+ queue_delayed_work(system_wq, &cxlds->security.poll_dwork,
+ timeout * HZ);
+ }
+ mutex_unlock(&cxlds->mbox_mutex);
+}
+
/**
* __cxl_pci_mbox_send_cmd() - Execute a mailbox command
* @cxlds: The device state to communicate with.
@@ -187,6 +221,16 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
return -EBUSY;
}
+ /*
+ * With sanitize polling, hardware might be done and the poller still
+ * not be in sync. Ensure no new command comes in until so. Keep the
+ * hardware semantics and only allow device health status.
+ */
+ if (unlikely(cxlds->security.poll_tmo_secs > 0)) {
+ if (mbox_cmd->opcode != CXL_MBOX_OP_GET_HEALTH_INFO)
+ return -EBUSY;
+ }
+
cmd_reg = FIELD_PREP(CXLDEV_MBOX_CMD_COMMAND_OPCODE_MASK,
mbox_cmd->opcode);
if (mbox_cmd->size_in) {
@@ -235,11 +279,34 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
*/
if (mbox_cmd->return_code == CXL_MBOX_CMD_RC_BACKGROUND) {
u64 bg_status_reg;
- int i, timeout = mbox_cmd->poll_interval_ms;
+ int i, timeout;
+
+ /*
++ * Sanitation is a special case which monopolizes the device
+ * and cannot be timesliced. Handle asynchronously instead,
+ * and allow userspace to poll(2) for completion.
+ */
+ if (mbox_cmd->opcode == CXL_MBOX_OP_SANITIZE) {
+ if (cxlds->security.poll_tmo_secs != -1) {
+ /* hold the device throughout */
+ get_device(cxlds->dev);
+
+ /* give first timeout a second */
+ timeout = 1;
+ cxlds->security.poll_tmo_secs = timeout;
+ queue_delayed_work(system_wq,
+ &cxlds->security.poll_dwork,
+ timeout * HZ);
+ }
+
+ dev_dbg(dev, "Sanitation operation started\n");
+ goto success;
+ }
dev_dbg(dev, "Mailbox background operation (0x%04x) started\n",
mbox_cmd->opcode);
+ timeout = mbox_cmd->poll_interval_ms;
for (i = 0; i < mbox_cmd->poll_count; i++) {
if (rcuwait_wait_event_timeout(&cxlds->mbox_wait,
cxl_mbox_background_complete(cxlds),
@@ -270,6 +337,7 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
return 0; /* completed but caller must check return_code */
}
+success:
/* #7 */
cmd_reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_CMD_OFFSET);
out_len = FIELD_GET(CXLDEV_MBOX_CMD_PAYLOAD_LENGTH_MASK, cmd_reg);
@@ -382,6 +450,9 @@ static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds)
}
mbox_poll:
+ cxlds->security.poll = true;
+ INIT_DELAYED_WORK(&cxlds->security.poll_dwork, cxl_mbox_sanitize_work);
+
dev_dbg(cxlds->dev, "Mailbox interrupts are unsupported");
return 0;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 3/7] cxl/mbox: Add sanitation handling machinery
2023-06-12 18:10 ` [PATCH 3/7] cxl/mbox: Add sanitation handling machinery Davidlohr Bueso
@ 2023-06-13 16:07 ` Jonathan Cameron
2023-06-13 16:28 ` Davidlohr Bueso
2023-06-25 22:13 ` Dan Williams
2023-06-25 22:18 ` Dan Williams
2 siblings, 1 reply; 30+ messages in thread
From: Jonathan Cameron @ 2023-06-13 16:07 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: dan.j.williams, dave.jiang, vishal.l.verma, fan.ni, a.manzanares,
linux-cxl
On Mon, 12 Jun 2023 11:10:34 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:
> Sanitation is by definition a device-monopolizing operation, and thus
> the timeslicing rules for other background commands do not apply.
> As such handle this special case asynchronously and return immediately.
> Subsequent changes will allow completion to be pollable from userspace
> via a sysfs file interface.
>
> For devices that don't support interrupts for notifying background
> command completion, self-poll with the caveat that the poller can
> be out of sync with the ready hardware, and therefore care must be
> taken to not allow any new commands to go through until the poller
> sees the hw completion. The poller takes the mbox_mutex to stabilize
> the flagging, minimizing any runtime overhead in the send path to
> check for 'sanitize_tmo' for uncommon poll scenarios.
>
> The irq case is much simpler as hardware will serialize/error
> appropriately.
>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
Not updated the check against -1 for whether to poll or not.
Which I'm guessing is the bug I'm seeing whilst testing this on qemu
> ---
> drivers/cxl/core/memdev.c | 10 +++++
> drivers/cxl/cxlmem.h | 7 ++++
> drivers/cxl/pci.c | 77 +++++++++++++++++++++++++++++++++++++--
> 3 files changed, 91 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 1bbb7e39fc93..834f418b6bcb 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -460,11 +460,21 @@ void clear_exclusive_cxl_commands(struct cxl_dev_state *cxlds, unsigned long *cm
> }
> EXPORT_SYMBOL_NS_GPL(clear_exclusive_cxl_commands, CXL);
>
> +static void cxl_memdev_security_shutdown(struct device *dev)
> +{
> + struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> + struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +
> + if (cxlds->security.poll)
> + cancel_delayed_work_sync(&cxlds->security.poll_dwork);
> +}
> +
> static void cxl_memdev_shutdown(struct device *dev)
> {
> struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
>
> down_write(&cxl_memdev_rwsem);
> + cxl_memdev_security_shutdown(dev);
> cxlmd->cxlds = NULL;
> up_write(&cxl_memdev_rwsem);
> }
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 091f1200736b..3a9df1044144 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -264,9 +264,15 @@ struct cxl_poison_state {
> * struct cxl_security_state - Device security state
> *
> * @state: state of last security operation
> + * @poll: polling for sanitation is enabled, device has no mbox irq support
> + * @poll_tmo_secs: polling timeout
> + * @poll_dwork: polling work item
> */
> struct cxl_security_state {
> unsigned long state;
> + bool poll;
> + int poll_tmo_secs;
> + struct delayed_work poll_dwork;
> };
>
> /**
> @@ -379,6 +385,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 4b2575502f49..c92eab55a5a7 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -115,18 +115,52 @@ static bool cxl_mbox_background_complete(struct cxl_dev_state *cxlds)
>
> static irqreturn_t cxl_pci_mbox_irq(int irq, void *id)
> {
> + u64 reg;
> + u16 opcode;
> struct cxl_dev_id *dev_id = id;
> struct cxl_dev_state *cxlds = dev_id->cxlds;
>
> if (!cxl_mbox_background_complete(cxlds))
> return IRQ_NONE;
>
> - /* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
> - rcuwait_wake_up(&cxlds->mbox_wait);
> + reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
> + opcode = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK, reg);
> + if (opcode == CXL_MBOX_OP_SANITIZE) {
> + dev_dbg(cxlds->dev, "Sanitation operation ended\n");
> + } else {
> + /* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
> + rcuwait_wake_up(&cxlds->mbox_wait);
> + }
>
> return IRQ_HANDLED;
> }
>
> +/*
> + * Sanitation operation polling mode.
> + */
> +static void cxl_mbox_sanitize_work(struct work_struct *work)
> +{
> + struct cxl_dev_state *cxlds;
> +
> + cxlds = container_of(work,
> + struct cxl_dev_state, security.poll_dwork.work);
> +
> + mutex_lock(&cxlds->mbox_mutex);
> + if (cxl_mbox_background_complete(cxlds)) {
> + cxlds->security.poll_tmo_secs = 0;
> + put_device(cxlds->dev);
> +
> + dev_dbg(cxlds->dev, "Sanitation operation ended\n");
> + } else {
> + int timeout = cxlds->security.poll_tmo_secs + 10;
> +
> + cxlds->security.poll_tmo_secs = min(15 * 60, timeout);
> + queue_delayed_work(system_wq, &cxlds->security.poll_dwork,
> + timeout * HZ);
> + }
> + mutex_unlock(&cxlds->mbox_mutex);
> +}
> +
> /**
> * __cxl_pci_mbox_send_cmd() - Execute a mailbox command
> * @cxlds: The device state to communicate with.
> @@ -187,6 +221,16 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
> return -EBUSY;
> }
>
> + /*
> + * With sanitize polling, hardware might be done and the poller still
> + * not be in sync. Ensure no new command comes in until so. Keep the
> + * hardware semantics and only allow device health status.
> + */
> + if (unlikely(cxlds->security.poll_tmo_secs > 0)) {
> + if (mbox_cmd->opcode != CXL_MBOX_OP_GET_HEALTH_INFO)
> + return -EBUSY;
> + }
> +
> cmd_reg = FIELD_PREP(CXLDEV_MBOX_CMD_COMMAND_OPCODE_MASK,
> mbox_cmd->opcode);
> if (mbox_cmd->size_in) {
> @@ -235,11 +279,34 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
> */
> if (mbox_cmd->return_code == CXL_MBOX_CMD_RC_BACKGROUND) {
> u64 bg_status_reg;
> - int i, timeout = mbox_cmd->poll_interval_ms;
> + int i, timeout;
> +
> + /*
> ++ * Sanitation is a special case which monopolizes the device
> + * and cannot be timesliced. Handle asynchronously instead,
> + * and allow userspace to poll(2) for completion.
> + */
> + if (mbox_cmd->opcode == CXL_MBOX_OP_SANITIZE) {
> + if (cxlds->security.poll_tmo_secs != -1) {
Should be checking your new poll boolean.
Jonathan
> + /* hold the device throughout */
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/7] cxl/mbox: Add sanitation handling machinery
2023-06-13 16:07 ` Jonathan Cameron
@ 2023-06-13 16:28 ` Davidlohr Bueso
2023-06-14 8:36 ` Jonathan Cameron
0 siblings, 1 reply; 30+ messages in thread
From: Davidlohr Bueso @ 2023-06-13 16:28 UTC (permalink / raw)
To: Jonathan Cameron
Cc: dan.j.williams, dave.jiang, vishal.l.verma, fan.ni, a.manzanares,
linux-cxl
On Tue, 13 Jun 2023, Jonathan Cameron wrote:
>> + if (mbox_cmd->opcode == CXL_MBOX_OP_SANITIZE) {
>> + if (cxlds->security.poll_tmo_secs != -1) {
>Should be checking your new poll boolean.
Yes, again sorry for the oversight.
Thanks,
Davidlohr
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/7] cxl/mbox: Add sanitation handling machinery
2023-06-13 16:28 ` Davidlohr Bueso
@ 2023-06-14 8:36 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2023-06-14 8:36 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: dan.j.williams, dave.jiang, vishal.l.verma, fan.ni, a.manzanares,
linux-cxl
On Tue, 13 Jun 2023 09:28:54 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:
> On Tue, 13 Jun 2023, Jonathan Cameron wrote:
>
> >> + if (mbox_cmd->opcode == CXL_MBOX_OP_SANITIZE) {
> >> + if (cxlds->security.poll_tmo_secs != -1) {
> >Should be checking your new poll boolean.
>
> Yes, again sorry for the oversight.
Other than that, LGTM and with the fix
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Thanks,
> Davidlohr
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH 3/7] cxl/mbox: Add sanitation handling machinery
2023-06-12 18:10 ` [PATCH 3/7] cxl/mbox: Add sanitation handling machinery Davidlohr Bueso
2023-06-13 16:07 ` Jonathan Cameron
@ 2023-06-25 22:13 ` Dan Williams
2023-06-26 18:17 ` Davidlohr Bueso
2023-06-25 22:18 ` Dan Williams
2 siblings, 1 reply; 30+ messages in thread
From: Dan Williams @ 2023-06-25 22:13 UTC (permalink / raw)
To: Davidlohr Bueso, dan.j.williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, dave, linux-cxl
Davidlohr Bueso wrote:
> Sanitation is by definition a device-monopolizing operation, and thus
> the timeslicing rules for other background commands do not apply.
> As such handle this special case asynchronously and return immediately.
> Subsequent changes will allow completion to be pollable from userspace
> via a sysfs file interface.
>
> For devices that don't support interrupts for notifying background
> command completion, self-poll with the caveat that the poller can
> be out of sync with the ready hardware, and therefore care must be
> taken to not allow any new commands to go through until the poller
> sees the hw completion. The poller takes the mbox_mutex to stabilize
> the flagging, minimizing any runtime overhead in the send path to
> check for 'sanitize_tmo' for uncommon poll scenarios.
>
> The irq case is much simpler as hardware will serialize/error
> appropriately.
Some minor things to fixup below, if this is all I find I can likely
handle this on applying:
>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
> drivers/cxl/core/memdev.c | 10 +++++
> drivers/cxl/cxlmem.h | 7 ++++
> drivers/cxl/pci.c | 77 +++++++++++++++++++++++++++++++++++++--
> 3 files changed, 91 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 1bbb7e39fc93..834f418b6bcb 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -460,11 +460,21 @@ void clear_exclusive_cxl_commands(struct cxl_dev_state *cxlds, unsigned long *cm
> }
> EXPORT_SYMBOL_NS_GPL(clear_exclusive_cxl_commands, CXL);
>
> +static void cxl_memdev_security_shutdown(struct device *dev)
> +{
> + struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> + struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +
> + if (cxlds->security.poll)
> + cancel_delayed_work_sync(&cxlds->security.poll_dwork);
> +}
> +
> static void cxl_memdev_shutdown(struct device *dev)
> {
> struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
>
> down_write(&cxl_memdev_rwsem);
> + cxl_memdev_security_shutdown(dev);
> cxlmd->cxlds = NULL;
> up_write(&cxl_memdev_rwsem);
> }
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 091f1200736b..3a9df1044144 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -264,9 +264,15 @@ struct cxl_poison_state {
> * struct cxl_security_state - Device security state
> *
> * @state: state of last security operation
> + * @poll: polling for sanitation is enabled, device has no mbox irq support
> + * @poll_tmo_secs: polling timeout
> + * @poll_dwork: polling work item
> */
> struct cxl_security_state {
> unsigned long state;
> + bool poll;
> + int poll_tmo_secs;
> + struct delayed_work poll_dwork;
> };
>
> /**
> @@ -379,6 +385,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 4b2575502f49..c92eab55a5a7 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -115,18 +115,52 @@ static bool cxl_mbox_background_complete(struct cxl_dev_state *cxlds)
>
> static irqreturn_t cxl_pci_mbox_irq(int irq, void *id)
> {
> + u64 reg;
> + u16 opcode;
> struct cxl_dev_id *dev_id = id;
> struct cxl_dev_state *cxlds = dev_id->cxlds;
>
> if (!cxl_mbox_background_complete(cxlds))
> return IRQ_NONE;
>
> - /* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
> - rcuwait_wake_up(&cxlds->mbox_wait);
> + reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
> + opcode = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK, reg);
> + if (opcode == CXL_MBOX_OP_SANITIZE) {
> + dev_dbg(cxlds->dev, "Sanitation operation ended\n");
> + } else {
> + /* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
> + rcuwait_wake_up(&cxlds->mbox_wait);
Just a question, is there any harm in awaking this even though nothing
is waiting? I.e. just wondering if this has functional purpose or is
just for cleanliness?
> + }
>
> return IRQ_HANDLED;
> }
>
> +/*
> + * Sanitation operation polling mode.
> + */
> +static void cxl_mbox_sanitize_work(struct work_struct *work)
> +{
> + struct cxl_dev_state *cxlds;
> +
> + cxlds = container_of(work,
> + struct cxl_dev_state, security.poll_dwork.work);
> +
> + mutex_lock(&cxlds->mbox_mutex);
> + if (cxl_mbox_background_complete(cxlds)) {
> + cxlds->security.poll_tmo_secs = 0;
> + put_device(cxlds->dev);
> +
> + dev_dbg(cxlds->dev, "Sanitation operation ended\n");
> + } else {
> + int timeout = cxlds->security.poll_tmo_secs + 10;
> +
> + cxlds->security.poll_tmo_secs = min(15 * 60, timeout);
> + queue_delayed_work(system_wq, &cxlds->security.poll_dwork,
> + timeout * HZ);
> + }
> + mutex_unlock(&cxlds->mbox_mutex);
> +}
> +
> /**
> * __cxl_pci_mbox_send_cmd() - Execute a mailbox command
> * @cxlds: The device state to communicate with.
> @@ -187,6 +221,16 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
> return -EBUSY;
> }
>
> + /*
> + * With sanitize polling, hardware might be done and the poller still
> + * not be in sync. Ensure no new command comes in until so. Keep the
> + * hardware semantics and only allow device health status.
> + */
> + if (unlikely(cxlds->security.poll_tmo_secs > 0)) {
CPUs and compilers do a decent job at likely/unlikely branch prediction,
and given mailbox operations are a slow path I can not imagine this
unlikely() annotation makes any measurable difference.
> + if (mbox_cmd->opcode != CXL_MBOX_OP_GET_HEALTH_INFO)
> + return -EBUSY;
> + }
> +
> cmd_reg = FIELD_PREP(CXLDEV_MBOX_CMD_COMMAND_OPCODE_MASK,
> mbox_cmd->opcode);
> if (mbox_cmd->size_in) {
> @@ -235,11 +279,34 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
> */
> if (mbox_cmd->return_code == CXL_MBOX_CMD_RC_BACKGROUND) {
> u64 bg_status_reg;
> - int i, timeout = mbox_cmd->poll_interval_ms;
> + int i, timeout;
> +
> + /*
> ++ * Sanitation is a special case which monopolizes the device
^ extra '+' character?
> + * and cannot be timesliced. Handle asynchronously instead,
> + * and allow userspace to poll(2) for completion.
> + */
> + if (mbox_cmd->opcode == CXL_MBOX_OP_SANITIZE) {
> + if (cxlds->security.poll_tmo_secs != -1) {
> + /* hold the device throughout */
> + get_device(cxlds->dev);
> +
> + /* give first timeout a second */
> + timeout = 1;
> + cxlds->security.poll_tmo_secs = timeout;
> + queue_delayed_work(system_wq,
> + &cxlds->security.poll_dwork,
> + timeout * HZ);
> + }
> +
> + dev_dbg(dev, "Sanitation operation started\n");
> + goto success;
> + }
>
> dev_dbg(dev, "Mailbox background operation (0x%04x) started\n",
> mbox_cmd->opcode);
>
> + timeout = mbox_cmd->poll_interval_ms;
> for (i = 0; i < mbox_cmd->poll_count; i++) {
> if (rcuwait_wait_event_timeout(&cxlds->mbox_wait,
> cxl_mbox_background_complete(cxlds),
> @@ -270,6 +337,7 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
> return 0; /* completed but caller must check return_code */
> }
>
> +success:
> /* #7 */
> cmd_reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_CMD_OFFSET);
> out_len = FIELD_GET(CXLDEV_MBOX_CMD_PAYLOAD_LENGTH_MASK, cmd_reg);
> @@ -382,6 +450,9 @@ static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds)
> }
>
> mbox_poll:
> + cxlds->security.poll = true;
> + INIT_DELAYED_WORK(&cxlds->security.poll_dwork, cxl_mbox_sanitize_work);
> +
> dev_dbg(cxlds->dev, "Mailbox interrupts are unsupported");
> return 0;
> }
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/7] cxl/mbox: Add sanitation handling machinery
2023-06-25 22:13 ` Dan Williams
@ 2023-06-26 18:17 ` Davidlohr Bueso
0 siblings, 0 replies; 30+ messages in thread
From: Davidlohr Bueso @ 2023-06-26 18:17 UTC (permalink / raw)
To: Dan Williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, linux-cxl
On Sun, 25 Jun 2023, Dan Williams wrote:
>Davidlohr Bueso wrote:
>> static irqreturn_t cxl_pci_mbox_irq(int irq, void *id)
>> {
>> + u64 reg;
>> + u16 opcode;
>> struct cxl_dev_id *dev_id = id;
>> struct cxl_dev_state *cxlds = dev_id->cxlds;
>>
>> if (!cxl_mbox_background_complete(cxlds))
>> return IRQ_NONE;
>>
>> - /* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
>> - rcuwait_wake_up(&cxlds->mbox_wait);
>> + reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
>> + opcode = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK, reg);
>> + if (opcode == CXL_MBOX_OP_SANITIZE) {
>> + dev_dbg(cxlds->dev, "Sanitation operation ended\n");
>> + } else {
>> + /* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
>> + rcuwait_wake_up(&cxlds->mbox_wait);
>
>Just a question, is there any harm in awaking this even though nothing
>is waiting? I.e. just wondering if this has functional purpose or is
>just for cleanliness?
No, there is no harm in calling the wakeup if nothing is blocked.
rcuwait will check for nil task pointer before calling wake_up_process().
In such cases it will be a nop.
...
>>
>> + /*
>> + * With sanitize polling, hardware might be done and the poller still
>> + * not be in sync. Ensure no new command comes in until so. Keep the
>> + * hardware semantics and only allow device health status.
>> + */
>> + if (unlikely(cxlds->security.poll_tmo_secs > 0)) {
>
>CPUs and compilers do a decent job at likely/unlikely branch prediction,
>and given mailbox operations are a slow path I can not imagine this
>unlikely() annotation makes any measurable difference.
So this was more about documenting the rare case more than an actual performance
optimization. Either way is fine I guess.
Thanks,
Davidlohr
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [PATCH 3/7] cxl/mbox: Add sanitation handling machinery
2023-06-12 18:10 ` [PATCH 3/7] cxl/mbox: Add sanitation handling machinery Davidlohr Bueso
2023-06-13 16:07 ` Jonathan Cameron
2023-06-25 22:13 ` Dan Williams
@ 2023-06-25 22:18 ` Dan Williams
2 siblings, 0 replies; 30+ messages in thread
From: Dan Williams @ 2023-06-25 22:18 UTC (permalink / raw)
To: Davidlohr Bueso, dan.j.williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, dave, linux-cxl
Davidlohr Bueso wrote:
> Sanitation is by definition a device-monopolizing operation, and thus
> the timeslicing rules for other background commands do not apply.
> As such handle this special case asynchronously and return immediately.
> Subsequent changes will allow completion to be pollable from userspace
> via a sysfs file interface.
>
> For devices that don't support interrupts for notifying background
> command completion, self-poll with the caveat that the poller can
> be out of sync with the ready hardware, and therefore care must be
> taken to not allow any new commands to go through until the poller
> sees the hw completion. The poller takes the mbox_mutex to stabilize
> the flagging, minimizing any runtime overhead in the send path to
> check for 'sanitize_tmo' for uncommon poll scenarios.
>
> The irq case is much simpler as hardware will serialize/error
> appropriately.
I noticed that this series bounces back and forth between "sanitation" and
"sanitization". I think everywhere it mention "sanitation" it means
"sanitization", right?
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 4/7] cxl/mem: Wire up Sanitation support
2023-06-12 18:10 [PATCH v6 0/7] cxl: Support device sanitation Davidlohr Bueso
` (2 preceding siblings ...)
2023-06-12 18:10 ` [PATCH 3/7] cxl/mbox: Add sanitation handling machinery Davidlohr Bueso
@ 2023-06-12 18:10 ` Davidlohr Bueso
2023-06-25 22:34 ` Dan Williams
2023-06-12 18:10 ` [PATCH 5/7] cxl/test: Add Sanitize opcode support Davidlohr Bueso
` (4 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Davidlohr Bueso @ 2023-06-12 18:10 UTC (permalink / raw)
To: dan.j.williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, dave, linux-cxl
Implement support for CXL 3.0 8.2.9.8.5.1 Sanitize. This is done by
adding a security/sanitize' memdev sysfs file to trigger the operation
and extend the status file to make it poll(2)-capable for completion.
Unlike all other background commands, this is the only operation that
is special and monopolizes the device for long periods of time.
In addition to the traditional pmem security requirements, all regions
must also be offline in order to perform the operation. This permits
avoiding explicit global CPU cache management, relying instead on
attach_target() setting CXL_REGION_F_INCOHERENT upon reconnect.
The expectation is that userspace can use it such as:
cxl disable-memdev memX
echo 1 > /sys/bus/cxl/devices/memX/security/sanitize
cxl wait-sanitize memX
cxl enable-memdev memX
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
Documentation/ABI/testing/sysfs-bus-cxl | 21 +++++++-
drivers/cxl/core/mbox.c | 55 ++++++++++++++++++++
drivers/cxl/core/memdev.c | 67 +++++++++++++++++++++++++
drivers/cxl/cxlmem.h | 4 ++
drivers/cxl/pci.c | 6 +++
5 files changed, 151 insertions(+), 2 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index 721a44d8a482..5753cba98692 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -64,8 +64,25 @@ KernelVersion: v6.5
Contact: linux-cxl@vger.kernel.org
Description:
(RO) Reading this file will display the CXL security state for
- that device. Such states can be: 'disabled', or those available
- only for persistent memory: 'locked', 'unlocked' or 'frozen'.
+ that device. Such states can be: 'disabled', 'sanitize', when
+ a sanitation is currently underway; or those available only
+ for persistent memory: 'locked', 'unlocked' or 'frozen'. This
+ sysfs entry is select/poll capable from userspace to notify
+ upon completion of a sanitize operation.
+
+
+What: /sys/bus/cxl/devices/memX/security/sanitize
+Date: June, 2023
+KernelVersion: v6.5
+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 functionality requires
+ the device to be not be actively decoding any HPA ranges.
What: /sys/bus/cxl/devices/*/devtype
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 5993261e3e08..51c64829f20a 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -1075,6 +1075,61 @@ int cxl_dev_state_identify(struct cxl_dev_state *cxlds)
}
EXPORT_SYMBOL_NS_GPL(cxl_dev_state_identify, CXL);
+/**
+ * cxl_mem_sanitize() - Send a sanitation command to the device.
+ * @cxlds: The device data for the operation
+ * @cmd: The specific sanitation command opcode
+ *
+ * Return: 0 if the command was executed successfully, regardless of
+ * whether or not the actual security operation is done in the background,
+ * such as for the Sanitize case.
+ * Error return values can be the result of the mailbox command, -EINVAL
+ * when security requirements are not met or invalid contexts.
+ *
+ * See CXL 3.0 @8.2.9.8.5.1 Sanitize and @8.2.9.8.5.2 Secure Erase.
+ */
+int cxl_mem_sanitize(struct cxl_dev_state *cxlds, u16 cmd)
+{
+ 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 = cmd };
+
+ if (cmd != CXL_MBOX_OP_SANITIZE)
+ 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;
+
+ rc = cxl_internal_send_cmd(cxlds, &mbox_cmd);
+ if (rc < 0) {
+ dev_err(cxlds->dev, "Failed to sanitize device : %d", rc);
+ return rc;
+ }
+
+ 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 834f418b6bcb..bdd1edfd62e8 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/io-64-nonatomic-lo-hi.h>
#include <linux/device.h>
#include <linux/slab.h>
#include <linux/idr.h>
@@ -114,6 +115,12 @@ static ssize_t security_state_show(struct device *dev,
struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
struct cxl_dev_state *cxlds = cxlmd->cxlds;
unsigned long state = cxlds->security.state;
+ u64 reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
+ u32 pct = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_PCT_MASK, reg);
+ u16 cmd = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK, reg);
+
+ if (cmd == CXL_MBOX_OP_SANITIZE && pct != 100)
+ return sysfs_emit(buf, "sanitize\n");
if (!(state & CXL_PMEM_SEC_STATE_USER_PASS_SET))
return sysfs_emit(buf, "disabled\n");
@@ -129,6 +136,33 @@ 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;
+ struct cxl_port *port = dev_get_drvdata(&cxlmd->dev);
+ ssize_t rc;
+ bool sanitize;
+
+ if (kstrtobool(buf, &sanitize) || !sanitize)
+ return -EINVAL;
+
+ if (!port || !is_cxl_endpoint(port))
+ return -EINVAL;
+
+ /* ensure no regions are mapped to this memdev */
+ if (port->commit_end != -1)
+ return -EBUSY;
+
+ rc = cxl_mem_sanitize(cxlds, CXL_MBOX_OP_SANITIZE);
+
+ return rc ? rc : len;
+}
+static struct device_attribute dev_attr_security_sanitize =
+ __ATTR(sanitize, 0200, NULL, security_sanitize_store);
+
static int cxl_get_poison_by_memdev(struct cxl_memdev *cxlmd)
{
struct cxl_dev_state *cxlds = cxlmd->cxlds;
@@ -376,6 +410,7 @@ static struct attribute *cxl_memdev_ram_attributes[] = {
static struct attribute *cxl_memdev_security_attributes[] = {
&dev_attr_security_state.attr,
+ &dev_attr_security_sanitize.attr,
NULL,
};
@@ -594,6 +629,34 @@ static const struct file_operations cxl_memdev_fops = {
.llseek = noop_llseek,
};
+static void put_sanitize(void *data)
+{
+ struct cxl_dev_state *cxlds = data;
+
+ sysfs_put(cxlds->security.sanitize_node);
+}
+
+static int cxl_memdev_security_init(struct cxl_memdev *cxlmd)
+{
+ struct cxl_dev_state *cxlds = cxlmd->cxlds;
+ struct device *dev = &cxlmd->dev;
+ struct kernfs_node *sec;
+
+ sec = sysfs_get_dirent(dev->kobj.sd, "security");
+ if (!sec) {
+ dev_err(dev, "sysfs_get_dirent 'security' failed\n");
+ return -ENODEV;
+ }
+ cxlds->security.sanitize_node = sysfs_get_dirent(sec, "state");
+ sysfs_put(sec);
+ if (!cxlds->security.sanitize_node) {
+ dev_err(dev, "sysfs_get_dirent 'state' failed\n");
+ return -ENODEV;
+ }
+
+ return devm_add_action_or_reset(cxlds->dev, put_sanitize, cxlds);
+ }
+
struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
{
struct cxl_memdev *cxlmd;
@@ -622,6 +685,10 @@ struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
if (rc)
goto err;
+ rc = cxl_memdev_security_init(cxlmd);
+ if (rc)
+ goto err;
+
rc = devm_add_action_or_reset(cxlds->dev, cxl_memdev_unregister, cxlmd);
if (rc)
return ERR_PTR(rc);
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 3a9df1044144..177a76578a94 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -267,12 +267,14 @@ struct cxl_poison_state {
* @poll: polling for sanitation is enabled, device has no mbox irq support
* @poll_tmo_secs: polling timeout
* @poll_dwork: polling work item
+ * @sanitize_node: sanitation sysfs file to notify
*/
struct cxl_security_state {
unsigned long state;
bool poll;
int poll_tmo_secs;
struct delayed_work poll_dwork;
+ struct kernfs_node *sanitize_node;
};
/**
@@ -746,6 +748,8 @@ static inline void cxl_mem_active_dec(void)
}
#endif
+int cxl_mem_sanitize(struct cxl_dev_state *cxlds, u16 cmd);
+
struct cxl_hdm {
struct cxl_component_regs regs;
unsigned int decoder_count;
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index c92eab55a5a7..d1df23c19245 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -126,6 +126,9 @@ static irqreturn_t cxl_pci_mbox_irq(int irq, void *id)
reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
opcode = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK, reg);
if (opcode == CXL_MBOX_OP_SANITIZE) {
+ if (cxlds->security.sanitize_node)
+ sysfs_notify_dirent(cxlds->security.sanitize_node);
+
dev_dbg(cxlds->dev, "Sanitation operation ended\n");
} else {
/* short-circuit the wait in __cxl_pci_mbox_send_cmd() */
@@ -150,6 +153,9 @@ static void cxl_mbox_sanitize_work(struct work_struct *work)
cxlds->security.poll_tmo_secs = 0;
put_device(cxlds->dev);
+ if (cxlds->security.sanitize_node)
+ sysfs_notify_dirent(cxlds->security.sanitize_node);
+
dev_dbg(cxlds->dev, "Sanitation operation ended\n");
} else {
int timeout = cxlds->security.poll_tmo_secs + 10;
--
2.41.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* RE: [PATCH 4/7] cxl/mem: Wire up Sanitation support
2023-06-12 18:10 ` [PATCH 4/7] cxl/mem: Wire up Sanitation support Davidlohr Bueso
@ 2023-06-25 22:34 ` Dan Williams
0 siblings, 0 replies; 30+ messages in thread
From: Dan Williams @ 2023-06-25 22:34 UTC (permalink / raw)
To: Davidlohr Bueso, dan.j.williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, dave, linux-cxl
Davidlohr Bueso wrote:
> Implement support for CXL 3.0 8.2.9.8.5.1 Sanitize. This is done by
> adding a security/sanitize' memdev sysfs file to trigger the operation
> and extend the status file to make it poll(2)-capable for completion.
> Unlike all other background commands, this is the only operation that
> is special and monopolizes the device for long periods of time.
>
> In addition to the traditional pmem security requirements, all regions
> must also be offline in order to perform the operation. This permits
> avoiding explicit global CPU cache management, relying instead on
> attach_target() setting CXL_REGION_F_INCOHERENT upon reconnect.
CXL_REGION_F_INCOHERENT is going away, but the sentiment still holds. I
will update this to:
"This permits avoiding explicit global CPU cache management, relying
instead on the implict cache management when a region transitions
between CXL_CONFIG_ACTIVE and CXL_CONFIG_COMMIT."
>
> The expectation is that userspace can use it such as:
>
> cxl disable-memdev memX
> echo 1 > /sys/bus/cxl/devices/memX/security/sanitize
I assume this will become 'cxl sanitize-memdev' and handle all the busy
reporting etc for the user?
> cxl wait-sanitize memX
> cxl enable-memdev memX
>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
> Documentation/ABI/testing/sysfs-bus-cxl | 21 +++++++-
> drivers/cxl/core/mbox.c | 55 ++++++++++++++++++++
> drivers/cxl/core/memdev.c | 67 +++++++++++++++++++++++++
> drivers/cxl/cxlmem.h | 4 ++
> drivers/cxl/pci.c | 6 +++
> 5 files changed, 151 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 721a44d8a482..5753cba98692 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -64,8 +64,25 @@ KernelVersion: v6.5
> Contact: linux-cxl@vger.kernel.org
> Description:
> (RO) Reading this file will display the CXL security state for
> - that device. Such states can be: 'disabled', or those available
> - only for persistent memory: 'locked', 'unlocked' or 'frozen'.
> + that device. Such states can be: 'disabled', 'sanitize', when
> + a sanitation is currently underway; or those available only
> + for persistent memory: 'locked', 'unlocked' or 'frozen'. This
> + sysfs entry is select/poll capable from userspace to notify
> + upon completion of a sanitize operation.
> +
> +
> +What: /sys/bus/cxl/devices/memX/security/sanitize
> +Date: June, 2023
> +KernelVersion: v6.5
> +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 functionality requires
> + the device to be not be actively decoding any HPA ranges.
I notice this attribute is unconditionally available. It would be nice
to hide it on devices that do not support the optional sanitize command.
This is a minor fixup that just needs to be in place before v6.5-final.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 5/7] cxl/test: Add Sanitize opcode support
2023-06-12 18:10 [PATCH v6 0/7] cxl: Support device sanitation Davidlohr Bueso
` (3 preceding siblings ...)
2023-06-12 18:10 ` [PATCH 4/7] cxl/mem: Wire up Sanitation support Davidlohr Bueso
@ 2023-06-12 18:10 ` Davidlohr Bueso
2023-06-12 18:10 ` [PATCH 6/7] cxl/mem: Support Secure Erase Davidlohr Bueso
` (3 subsequent siblings)
8 siblings, 0 replies; 30+ messages in thread
From: Davidlohr Bueso @ 2023-06-12 18:10 UTC (permalink / raw)
To: dan.j.williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, dave, linux-cxl
Add support to emulate the "Sanitize" operation, without
incurring in the background.
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
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 34b48027b3de..faa484ea5b0b 100644
--- a/tools/testing/cxl/test/mem.c
+++ b/tools/testing/cxl/test/mem.c
@@ -535,6 +535,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)
{
@@ -1153,6 +1175,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.41.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 6/7] cxl/mem: Support Secure Erase
2023-06-12 18:10 [PATCH v6 0/7] cxl: Support device sanitation Davidlohr Bueso
` (4 preceding siblings ...)
2023-06-12 18:10 ` [PATCH 5/7] cxl/test: Add Sanitize opcode support Davidlohr Bueso
@ 2023-06-12 18:10 ` Davidlohr Bueso
2023-06-12 18:10 ` [PATCH 7/7] cxl/test: Add Secure Erase opcode support Davidlohr Bueso
` (2 subsequent siblings)
8 siblings, 0 replies; 30+ messages in thread
From: Davidlohr Bueso @ 2023-06-12 18:10 UTC (permalink / raw)
To: dan.j.williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, dave, linux-cxl
Implement support for the non-pmem exclusive secure erase, per
CXL specs. Create a write-only 'security/erase' sysfs file to
perform the requested operation.
As with the sanitation this requires the device being offline
and thus no active HPA-DPA decoding.
The expectation is that userspace can use it such as:
cxl disable-memdev memX
echo 1 > /sys/bus/cxl/devices/memX/security/erase
cxl enable-memdev memX
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Fan Ni <fan.ni@samsung.com>
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
Documentation/ABI/testing/sysfs-bus-cxl | 10 +++++++++
drivers/cxl/core/mbox.c | 6 +++++-
drivers/cxl/core/memdev.c | 28 +++++++++++++++++++++++++
drivers/cxl/cxlmem.h | 1 +
4 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index 5753cba98692..f224c1215f22 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -85,6 +85,16 @@ Description:
the device to be not be actively decoding any HPA ranges.
+What /sys/bus/cxl/devices/memX/security/erase
+Date: June, 2023
+KernelVersion: v6.5
+Contact: linux-cxl@vger.kernel.org
+Description:
+ (WO) Write a boolean 'true' string value to this attribute to
+ secure erase user data by changing the media encryption keys for
+ all user data areas of the device.
+
+
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 51c64829f20a..6622eac66bf1 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -1102,7 +1102,7 @@ int cxl_mem_sanitize(struct cxl_dev_state *cxlds, u16 cmd)
};
struct cxl_mbox_cmd mbox_cmd = { .opcode = cmd };
- if (cmd != CXL_MBOX_OP_SANITIZE)
+ if (cmd != CXL_MBOX_OP_SANITIZE && cmd != CXL_MBOX_OP_SECURE_ERASE)
return -EINVAL;
rc = cxl_internal_send_cmd(cxlds, &sec_cmd);
@@ -1120,6 +1120,10 @@ int cxl_mem_sanitize(struct cxl_dev_state *cxlds, u16 cmd)
if (sec_out & CXL_PMEM_SEC_STATE_USER_PASS_SET)
return -EINVAL;
+ if (cmd == CXL_MBOX_OP_SECURE_ERASE &&
+ sec_out & CXL_PMEM_SEC_STATE_LOCKED)
+ return -EINVAL;
+
rc = cxl_internal_send_cmd(cxlds, &mbox_cmd);
if (rc < 0) {
dev_err(cxlds->dev, "Failed to sanitize device : %d", rc);
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index bdd1edfd62e8..ed8de7efddef 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -163,6 +163,33 @@ 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;
+ struct cxl_port *port = dev_get_drvdata(&cxlmd->dev);
+ ssize_t rc;
+ bool erase;
+
+ if (kstrtobool(buf, &erase) || !erase)
+ return -EINVAL;
+
+ if (!port || !is_cxl_endpoint(port))
+ return -EINVAL;
+
+ /* ensure no regions are mapped to this memdev */
+ if (port->commit_end != -1)
+ return -EBUSY;
+
+ rc = cxl_mem_sanitize(cxlds, CXL_MBOX_OP_SECURE_ERASE);
+
+ return rc ? rc : len;
+}
+static struct device_attribute dev_attr_security_erase =
+ __ATTR(erase, 0200, NULL, security_erase_store);
+
static int cxl_get_poison_by_memdev(struct cxl_memdev *cxlmd)
{
struct cxl_dev_state *cxlds = cxlmd->cxlds;
@@ -411,6 +438,7 @@ static struct attribute *cxl_memdev_ram_attributes[] = {
static struct attribute *cxl_memdev_security_attributes[] = {
&dev_attr_security_state.attr,
&dev_attr_security_sanitize.attr,
+ &dev_attr_security_erase.attr,
NULL,
};
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 177a76578a94..e57db7adb2a3 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -388,6 +388,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,
--
2.41.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 7/7] cxl/test: Add Secure Erase opcode support
2023-06-12 18:10 [PATCH v6 0/7] cxl: Support device sanitation Davidlohr Bueso
` (5 preceding siblings ...)
2023-06-12 18:10 ` [PATCH 6/7] cxl/mem: Support Secure Erase Davidlohr Bueso
@ 2023-06-12 18:10 ` Davidlohr Bueso
2023-06-13 15:26 ` [PATCH v6 0/7] cxl: Support device sanitation Jonathan Cameron
2023-06-25 22:44 ` Dan Williams
8 siblings, 0 replies; 30+ messages in thread
From: Davidlohr Bueso @ 2023-06-12 18:10 UTC (permalink / raw)
To: dan.j.williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, dave, linux-cxl
Add support to emulate the CXL the "Secure Erase" operation.
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
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 faa484ea5b0b..97de0d3b2fd0 100644
--- a/tools/testing/cxl/test/mem.c
+++ b/tools/testing/cxl/test/mem.c
@@ -557,6 +557,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)
{
@@ -1178,6 +1202,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.41.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v6 0/7] cxl: Support device sanitation
2023-06-12 18:10 [PATCH v6 0/7] cxl: Support device sanitation Davidlohr Bueso
` (6 preceding siblings ...)
2023-06-12 18:10 ` [PATCH 7/7] cxl/test: Add Secure Erase opcode support Davidlohr Bueso
@ 2023-06-13 15:26 ` Jonathan Cameron
2023-06-13 15:51 ` Jonathan Cameron
2023-06-25 22:44 ` Dan Williams
8 siblings, 1 reply; 30+ messages in thread
From: Jonathan Cameron @ 2023-06-13 15:26 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: dan.j.williams, dave.jiang, vishal.l.verma, fan.ni, a.manzanares,
linux-cxl
Hi Davidlohr,
> Testing.
> ========
>
> o There are the mock device tests for Sanitize and Secure Erase.
>
> o The latest (v2) qemu bg/sanitize support series is posted here:
> https://lore.kernel.org/linux-cxl/20230418172337.19207-1-dave@stgolabs.net/
That doesn't seem to have support for reading back the security state so
the stuff this set adds fails before it gets going.
Am I missing another series?
Jonathan
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v6 0/7] cxl: Support device sanitation
2023-06-13 15:26 ` [PATCH v6 0/7] cxl: Support device sanitation Jonathan Cameron
@ 2023-06-13 15:51 ` Jonathan Cameron
2023-06-13 16:25 ` Davidlohr Bueso
0 siblings, 1 reply; 30+ messages in thread
From: Jonathan Cameron @ 2023-06-13 15:51 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: dan.j.williams, dave.jiang, vishal.l.verma, fan.ni, a.manzanares,
linux-cxl
On Tue, 13 Jun 2023 16:26:11 +0100
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> Hi Davidlohr,
>
> > Testing.
> > ========
> >
> > o There are the mock device tests for Sanitize and Secure Erase.
> >
> > o The latest (v2) qemu bg/sanitize support series is posted here:
> > https://lore.kernel.org/linux-cxl/20230418172337.19207-1-dave@stgolabs.net/
>
> That doesn't seem to have support for reading back the security state so
> the stuff this set adds fails before it gets going.
> Am I missing another series?
>
I hacked in enough to get things to carry on...
Following might be something I've broken locally but on basis
it might not... Note my QEMU is odd right now as I'm in middle
of a big refactor of the CCI handling, but this should still not happen
even if I happen to have broken QEMU side of things in some weird way.
My base is more or less mainline + background set as queued on CXL
tree plus this set.
However similar traces to below happen when I poke a 1 into
sanitize
This appears to be:
WARN_ON_ONCE(timer->function != delayed_work_timer_fn);
then
WARN_ON_ONCE(!list_empty(&work->entry));
triggering.
------------[ cut here ]------------
WARNING: CPU: 3 PID: 617 at kernel/workqueue.c:1663 __queue_delayed_work+0xb8/0xe8
Modules linked in: cxl_pmem cxl_mem cxl_port cxl_pmu cxl_acpi cxl_pci cxl_core
CPU: 3 PID: 617 Comm: bash Not tainted 6.4.0-rc6+ #790
Hardware name: QEMU QEMU Virtual Machine, BIOS unknown unknown
pstate: 014000c5 (nzcv daIF +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
pc : __queue_delayed_work+0xb8/0xe8
lr : queue_delayed_work_on+0x70/0x98
sp : ffff800008a23b10
x29: ffff800008a23b10 x28: ffff0000f6de5880 x27: ffff0000c14d9c80
x26: ffff8000085510a8 x25: ffff0000c0a490d0 x24: ffff0000c14d9cd8
x23: ffff800008a23da8 x22: fffffffffffffff2 x21: 0000000000000000
x20: 0000000000000001 x19: 0000000000000000 x18: 0000000000000000
x17: 0000000000000000 x16: ffffba73a9885008 x15: 0000aaaad5a0b6b0
x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
x11: 0000000000000000 x10: 0000000000000000 x9 : ffffba73a9885078
x8 : ffff800008a23c98 x7 : 0000000000000000 x6 : 0000000000000000
x5 : ffffba73a9884ea8 x4 : 0000000000000100 x3 : 00000000000000fa
x2 : ffff0000c14d9e98 x1 : ffff0000c0028600 x0 : ffff0000c14d9eb8
Call trace:
__queue_delayed_work+0xb8/0xe8
queue_delayed_work_on+0x70/0x98
cxl_pci_mbox_send+0x404/0x580 [cxl_pci]
cxl_internal_send_cmd+0x48/0x110 [cxl_core]
cxl_mem_sanitize+0xbc/0x140 [cxl_core]
security_sanitize_store+0x98/0xf0 [cxl_core]
dev_attr_store+0x20/0x40
sysfs_kf_write+0x4c/0x68
kernfs_fop_write_iter+0x128/0x200
vfs_write+0x1ac/0x2e8
ksys_write+0x74/0x110
__arm64_sys_write+0x24/0x38
invoke_syscall.constprop.0+0x58/0xf8
do_el0_svc+0x60/0x168
el0_svc+0x38/0xf0
el0t_64_sync_handler+0xf4/0x120
el0t_64_sync+0x190/0x198
---[ end trace 0000000000000000 ]---
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v6 0/7] cxl: Support device sanitation
2023-06-13 15:51 ` Jonathan Cameron
@ 2023-06-13 16:25 ` Davidlohr Bueso
0 siblings, 0 replies; 30+ messages in thread
From: Davidlohr Bueso @ 2023-06-13 16:25 UTC (permalink / raw)
To: Jonathan Cameron
Cc: dan.j.williams, dave.jiang, vishal.l.verma, fan.ni, a.manzanares,
linux-cxl
On Tue, 13 Jun 2023, Jonathan Cameron wrote:
>On Tue, 13 Jun 2023 16:26:11 +0100
>Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
>
>> Hi Davidlohr,
>>
>> > Testing.
>> > ========
>> >
>> > o There are the mock device tests for Sanitize and Secure Erase.
>> >
>> > o The latest (v2) qemu bg/sanitize support series is posted here:
>> > https://lore.kernel.org/linux-cxl/20230418172337.19207-1-dave@stgolabs.net/
>>
>> That doesn't seem to have support for reading back the security state so
>> the stuff this set adds fails before it gets going.
>> Am I missing another series?
Correct, when testing I simply comment out the security state call in cxl_mem_sanitize().
>>
>I hacked in enough to get things to carry on...
>
>Following might be something I've broken locally but on basis
>it might not... Note my QEMU is odd right now as I'm in middle
>of a big refactor of the CCI handling, but this should still not happen
>even if I happen to have broken QEMU side of things in some weird way.
>
>My base is more or less mainline + background set as queued on CXL
>tree plus this set.
>
>However similar traces to below happen when I poke a 1 into
>sanitize
>
> This appears to be:
>
> WARN_ON_ONCE(timer->function != delayed_work_timer_fn);
>then
> WARN_ON_ONCE(!list_empty(&work->entry));
>
>triggering.
>
Oh apologies, my bad. I actually should have tested this last iteration :/
But with the below I replicated the testing without anything falling out
(also with polling).
I'll wait a bit to see if Dan has any other comments regarding the series
before sending in another iteration - otherwise maybe this could be folded
in to avoid more email overhead?
Thanks,
Davidlohr
----8<-------
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index d1df23c19245..008e1c267ce2 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -288,12 +288,12 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_dev_state *cxlds,
int i, timeout;
/*
-+ * Sanitation is a special case which monopolizes the device
+ * Sanitation is a special case which monopolizes the device
* and cannot be timesliced. Handle asynchronously instead,
* and allow userspace to poll(2) for completion.
*/
if (mbox_cmd->opcode == CXL_MBOX_OP_SANITIZE) {
- if (cxlds->security.poll_tmo_secs != -1) {
+ if (cxlds->security.poll) {
/* hold the device throughout */
get_device(cxlds->dev);
^ permalink raw reply related [flat|nested] 30+ messages in thread
* RE: [PATCH v6 0/7] cxl: Support device sanitation
2023-06-12 18:10 [PATCH v6 0/7] cxl: Support device sanitation Davidlohr Bueso
` (7 preceding siblings ...)
2023-06-13 15:26 ` [PATCH v6 0/7] cxl: Support device sanitation Jonathan Cameron
@ 2023-06-25 22:44 ` Dan Williams
2023-06-26 21:32 ` Davidlohr Bueso
8 siblings, 1 reply; 30+ messages in thread
From: Dan Williams @ 2023-06-25 22:44 UTC (permalink / raw)
To: Davidlohr Bueso, dan.j.williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, dave, linux-cxl
Davidlohr Bueso wrote:
> Hi,
>
> Changes from v5 (https://lore.kernel.org/linux-cxl/20230526033344.17167-1-dave@stgolabs.net/):
> o Added patch 1 which fixes bogus irq handled scenarios when it's not our interrupt.
> This should be picked up regardless of the rest of the series (Jonathan)
> o Added security.poll boolean instead of using the timeout member (Dave, Jonathan).
> o Do not explicitly init security.state (Dave).
> o Misc cleanups (Jonathan).
> o Updated changelog in patch 4.
> o Picked up tags.
>
> This adds the sanitation part of the background command handling. Some noteworthy items:
>
> o Treating Sanitation as such a special beast can make the code a bit invasive,
> but couldn't find a decent alternative. For example I realize that this is really
> ad-hoc code in __cxl_pci_mbox_send_cmd(). A lot of this also comes from the fact
> that polling for sanitize is supported, so sw still needs to keep up and serialize.
>
> o Nothing depends explicitly on CPU cacheline management
>
> o All sysfs files/attributes in the security directory are visible.
>
> o Continue to use __ATTR() macros for sysfs attributes instead of the requested
> DEVICE_ATTR_*() ones because of the naming the security directory, otherwise
> names don't match.
>
> Patch 1 fixes mbox isr.
> Patch 2: adds a new security/state file.
> Patch 3 paves the required sanitation handling code before actually using it.
> Patch 4,5 wires up sanitation + unit test.
> Patch 6,7 wires up secure erase + unit test.
>
> Testing.
> ========
>
> o There are the mock device tests for Sanitize and Secure Erase.
>
> o The latest (v2) qemu bg/sanitize support series is posted here:
> https://lore.kernel.org/linux-cxl/20230418172337.19207-1-dave@stgolabs.net/
>
> (1) Window where driver is out of sync with hw (Sanitation async polling).
>
> [root@fedora ~]# echo 1 > /sys/bus/cxl/devices/mem0/security/sanitize
> [ 159.297482] cxl_pci:__cxl_pci_mbox_send_cmd:243: cxl_pci 0000:37:00.0: Sending command: 0x4400
> [ 159.298648] cxl_pci:cxl_pci_mbox_wait_for_doorbell:73: cxl_pci 0000:37:00.0: Doorbell wait took 0ms
> [ 159.299908] cxl_pci:__cxl_pci_mbox_send_cmd:295: cxl_pci 0000:37:00.0: Sanitation operation started
> >>>> qemu informs sanitation is done <<<<<
> [root@fedora ~]# echo 1 > /sys/bus/cxl/devices/mem0/security/sanitize
> [ 165.897345] cxl_pci 0000:37:00.0: Failed to sanitize device : -16
> [ 171.692050] cxl_pci:cxl_mbox_sanitize_work:147: cxl_pci 0000:37:00.0: Sanitation operation ended
> [root@fedora ~]# echo 1 > /sys/bus/cxl/devices/mem0/security/sanitize
> [ 173.373337] cxl_pci:__cxl_pci_mbox_send_cmd:243: cxl_pci 0000:37:00.0: Sending command: 0x4400
> [ 173.374498] cxl_pci:cxl_pci_mbox_wait_for_doorbell:73: cxl_pci 0000:37:00.0: Doorbell wait took 0ms
> [ 173.375727] cxl_pci:__cxl_pci_mbox_send_cmd:295: cxl_pci 0000:37:00.0: Sanitation operation started
>
> (2) Perform sanitation of more than one memdev at a time (Sanitation async polling).
>
> [root@fedora ~]# echo 1 > /sys/bus/cxl/devices/mem1/security/sanitize
> [ 351.287129] cxl_pci:__cxl_pci_mbox_send_cmd:243: cxl_pci 0000:36:00.0: Sending command: 0x4400
> [ 351.288403] cxl_pci:cxl_pci_mbox_wait_for_doorbell:73: cxl_pci 0000:36:00.0: Doorbell wait took 0ms
> [ 351.289706] cxl_pci:__cxl_pci_mbox_send_cmd:295: cxl_pci 0000:36:00.0: Sanitation operation started
> [root@fedora ~]# echo 1 > /sys/bus/cxl/devices/mem0/security/sanitize
> [ 353.058614] cxl_pci:__cxl_pci_mbox_send_cmd:243: cxl_pci 0000:37:00.0: Sending command: 0x4400
> [ 353.059854] cxl_pci:cxl_pci_mbox_wait_for_doorbell:73: cxl_pci 0000:37:00.0: Doorbell wait took 0ms
> [ 353.061126] cxl_pci:__cxl_pci_mbox_send_cmd:295: cxl_pci 0000:37:00.0: Sanitation operation started
> >>>> qemu informs sanitation is done <<<<<
> >>>> qemu informs sanitation is done <<<<<
> [ 363.692138] cxl_pci:cxl_mbox_sanitize_work:147: cxl_pci 0000:36:00.0: Sanitation operation ended
> [ 365.227416] cxl_pci:cxl_mbox_sanitize_work:147: cxl_pci 0000:37:00.0: Sanitation operation ended
>
> (3) Perform sanitation of more than one memdev at a time (Sanitation async irq).
>
> [root@fedora ~]# echo 1 > /sys/bus/cxl/devices/mem1/security/sanitize
> [ 193.729821] cxl_pci:__cxl_pci_mbox_send_cmd:243: cxl_pci 0000:c1:00.0: Sending command: 0x4400
> [ 193.731071] cxl_pci:cxl_pci_mbox_wait_for_doorbell:73: cxl_pci 0000:c1:00.0: Doorbell wait took 0ms
> [ 193.732360] cxl_pci:__cxl_pci_mbox_send_cmd:295: cxl_pci 0000:c1:00.0: Sanitation operation started
> [root@fedora ~]# echo 1 > /sys/bus/cxl/devices/mem0/security/sanitize
> [ 197.001466] cxl_pci:__cxl_pci_mbox_send_cmd:243: cxl_pci 0000:36:00.0: Sending command: 0x4400
> [ 197.002694] cxl_pci:cxl_pci_mbox_wait_for_doorbell:73: cxl_pci 0000:36:00.0: Doorbell wait took 0ms
> [ 197.003956] cxl_pci:__cxl_pci_mbox_send_cmd:295: cxl_pci 0000:36:00.0: Sanitation operation started
> >>>> qemu says sanitation is done <<<<
> [ 197.731473] cxl_pci:cxl_pci_mbox_irq:119: cxl_pci 0000:c1:00.0: Sanitation operation ended
> >>>> qemu says sanitation is done <<<<
> [ 201.003258] cxl_pci:cxl_pci_mbox_irq:119: cxl_pci 0000:36:00.0: Sanitation operation ended
>
> (4) Forbid new sanitation while one is in progress (Sanitation async irq).
>
> [root@fedora ~]# cat /sys/bus/cxl/devices/mem0/security/state
> disabled
> [root@fedora ~]# echo 1 > /sys/bus/cxl/devices/mem0/security/sanitize
> [ 39.284258] cxl_pci:__cxl_pci_mbox_send_cmd:243: cxl_pci 0000:36:00.0: Sending command: 0x4400
> [ 39.285459] cxl_pci:cxl_pci_mbox_wait_for_doorbell:73: cxl_pci 0000:36:00.0: Doorbell wait took 0ms
> [ 39.286723] cxl_pci:__cxl_pci_mbox_send_cmd:295: cxl_pci 0000:36:00.0: Sanitation operation started
> [root@fedora ~]# cat /sys/bus/cxl/devices/mem0/security/state
> sanitize
> [root@fedora ~]# echo 1 > /sys/bus/cxl/devices/mem0/security/sanitize
> [ 42.697129] cxl_pci:__cxl_pci_mbox_send_cmd:243: cxl_pci 0000:36:00.0: Sending command: 0x4400
> [ 42.698323] cxl_pci:cxl_pci_mbox_wait_for_doorbell:73: cxl_pci 0000:36:00.0: Doorbell wait took 0ms
> [ 42.699525] cxl_pci:__cxl_pci_mbox_send_cmd:335: cxl_pci 0000:36:00.0: Mailbox operation had an error: ongoing background operation
> [ 42.701119] cxl_pci 0000:36:00.0: Failed to sanitize device : -6
> >>>> qemu says sanitation is done <<<<
> [ 43.285334] cxl_pci:cxl_pci_mbox_irq:119: cxl_pci 0000:36:00.0: Sanitation operation ended
>
>
> Applies against 'for-6.5/cxl-background' from cxl.git.
>
> Please consider for v6.5.
This looks good to me after s/([sS])anitation/\1anitization/ throughout
and some othe minor fixups that I mentioned.
Merged for v6.5.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v6 0/7] cxl: Support device sanitation
2023-06-25 22:44 ` Dan Williams
@ 2023-06-26 21:32 ` Davidlohr Bueso
2023-06-26 22:47 ` Dan Williams
0 siblings, 1 reply; 30+ messages in thread
From: Davidlohr Bueso @ 2023-06-26 21:32 UTC (permalink / raw)
To: Dan Williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, linux-cxl
On Sun, 25 Jun 2023, Dan Williams wrote:
>This looks good to me after s/([sS])anitation/\1anitization/ throughout
>and some othe minor fixups that I mentioned.
>
>Merged for v6.5.
fyi per https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/commit/?h=pending&id=0c36b6ad436a38b167af16e6c690c890b8b2df62
this seems to be missing this fix:
https://lore.kernel.org/linux-cxl/ispowzuk5fg2u7zbpenocvyihh4t3kywtszcbg245unypzduex@jenknfy62rjl/
Thanks,
Davidlohr
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v6 0/7] cxl: Support device sanitation
2023-06-26 21:32 ` Davidlohr Bueso
@ 2023-06-26 22:47 ` Dan Williams
2023-06-27 8:02 ` [PATCH] cxl/pci: Use correct flag for sanitize polling Davidlohr Bueso
0 siblings, 1 reply; 30+ messages in thread
From: Dan Williams @ 2023-06-26 22:47 UTC (permalink / raw)
To: Davidlohr Bueso, Dan Williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, linux-cxl
Davidlohr Bueso wrote:
> On Sun, 25 Jun 2023, Dan Williams wrote:
>
> >This looks good to me after s/([sS])anitation/\1anitization/ throughout
> >and some othe minor fixups that I mentioned.
> >
> >Merged for v6.5.
>
> fyi per https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git/commit/?h=pending&id=0c36b6ad436a38b167af16e6c690c890b8b2df62
> this seems to be missing this fix:
>
> https://lore.kernel.org/linux-cxl/ispowzuk5fg2u7zbpenocvyihh4t3kywtszcbg245unypzduex@jenknfy62rjl/
Oh, I missed that. Can you send it as a follow-on? Also make sure you
have "core.abbrev = 12" in your git config if you include a Fixes: tag.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] cxl/pci: Use correct flag for sanitize polling
2023-06-26 22:47 ` Dan Williams
@ 2023-06-27 8:02 ` Davidlohr Bueso
2023-06-27 23:01 ` Dan Williams
0 siblings, 1 reply; 30+ messages in thread
From: Davidlohr Bueso @ 2023-06-27 8:02 UTC (permalink / raw)
To: Dan Williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, linux-cxl
This is a bogus value, left behind from a previous version.
Fixes: 0c36b6ad436a ("cxl/mbox: Add sanitization handling machinery")
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
drivers/cxl/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 48f88d96029d..1cb1494c28fe 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -295,7 +295,7 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_memdev_state *mds,
* and allow userspace to poll(2) for completion.
*/
if (mbox_cmd->opcode == CXL_MBOX_OP_SANITIZE) {
- if (mds->security.poll_tmo_secs != -1) {
+ if (mds->security.poll) {
/* hold the device throughout */
get_device(cxlds->dev);
--
2.41.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* RE: [PATCH] cxl/pci: Use correct flag for sanitize polling
2023-06-27 8:02 ` [PATCH] cxl/pci: Use correct flag for sanitize polling Davidlohr Bueso
@ 2023-06-27 23:01 ` Dan Williams
0 siblings, 0 replies; 30+ messages in thread
From: Dan Williams @ 2023-06-27 23:01 UTC (permalink / raw)
To: Davidlohr Bueso, Dan Williams
Cc: dave.jiang, vishal.l.verma, Jonathan.Cameron, fan.ni,
a.manzanares, linux-cxl
Davidlohr Bueso wrote:
> This is a bogus value, left behind from a previous version.
>
> Fixes: 0c36b6ad436a ("cxl/mbox: Add sanitization handling machinery")
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
> drivers/cxl/pci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 48f88d96029d..1cb1494c28fe 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -295,7 +295,7 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_memdev_state *mds,
> * and allow userspace to poll(2) for completion.
> */
> if (mbox_cmd->opcode == CXL_MBOX_OP_SANITIZE) {
> - if (mds->security.poll_tmo_secs != -1) {
> + if (mds->security.poll) {
> /* hold the device throughout */
> get_device(cxlds->dev);
>
> --
> 2.41.0
I had to pick this up manually because 'git am' complains:
error: corrupt patch at line 10
Just letting you know in case you can spot the issue because I am still
scratching my head. Here are the steps I took to apply it manually:
---
$ b4 shazam -sl -P _ 7q3vcjqidtmxmys4n34g6b3mygvhaen7yikzxanpz56lw43fz7@7subbtbfkmyx
Grabbing thread from lore.kernel.org/all/7q3vcjqidtmxmys4n34g6b3mygvhaen7yikzxanpz56lw43fz7@7subbtbfkmyx/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Added from v6: 1 patches
Analyzing 18 messages in the thread
Checking attestation on all messages, may take a moment...
---
✓ [PATCH] cxl/pci: Use correct flag for sanitize polling
+ Link: https://lore.kernel.org/r/7q3vcjqidtmxmys4n34g6b3mygvhaen7yikzxanpz56lw43fz7@7subbtbfkmyx
+ Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
✓ Signed: DKIM/stgolabs.net
---
Total patches: 1 (cherrypicked: <7q3vcjqidtmxmys4n34g6b3mygvhaen7yikzxanpz56lw43fz7@7subbtbfkmyx>)
---
Applying: cxl/pci: Use correct flag for sanitize polling
Patch failed at 0001 cxl/pci: Use correct flag for sanitize polling
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
error: corrupt patch at line 10
hint: Use 'git am --show-current-patch=diff' to see the failed patch
$ b4 am -sl -P _ 7q3vcjqidtmxmys4n34g6b3mygvhaen7yikzxanpz56lw43fz7@7subbtbfkmyx
Analyzing 1 messages in the thread
Checking attestation on all messages, may take a moment...
---
✓ [PATCH] cxl/pci: Use correct flag for sanitize polling
+ Link: https://lore.kernel.org/r/7q3vcjqidtmxmys4n34g6b3mygvhaen7yikzxanpz56lw43fz7@7subbtbfkmyx
+ Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
✓ Signed: DKIM/stgolabs.net
---
Total patches: 1 (cherrypicked: <7q3vcjqidtmxmys4n34g6b3mygvhaen7yikzxanpz56lw43fz7@7subbtbfkmyx>)
---
Link: https://lore.kernel.org/r/7q3vcjqidtmxmys4n34g6b3mygvhaen7yikzxanpz56lw43fz7@7subbtbfkmyx
Base: not specified
git am ./20230627_dave_cxl_pci_use_correct_flag_for_sanitize_polling.mbx
$ cat ./20230627_dave_cxl_pci_use_correct_flag_for_sanitize_polling.mbx | patch -p1
patching file drivers/cxl/pci.c
$ git add drivers/cxl/pci.c
$ git am --continue
Applying: cxl/pci: Use correct flag for sanitize polling
$ git show
commit 71baec7b8500c92f9723f39d06a7ae465483da1f (HEAD -> for-6.5/cxl)
Author: Davidlohr Bueso <dave@stgolabs.net>
Date: Tue Jun 27 01:02:02 2023 -0700
cxl/pci: Use correct flag for sanitize polling
This is a bogus value, left behind from a previous version.
Fixes: 0c36b6ad436a ("cxl/mbox: Add sanitization handling machinery")
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
Link: https://lore.kernel.org/r/7q3vcjqidtmxmys4n34g6b3mygvhaen7yikzxanpz56lw43fz7@7subbtbfkmyx
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 48f88d96029d..1cb1494c28fe 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -295,7 +295,7 @@ static int __cxl_pci_mbox_send_cmd(struct cxl_memdev_state *mds,
* and allow userspace to poll(2) for completion.
*/
if (mbox_cmd->opcode == CXL_MBOX_OP_SANITIZE) {
- if (mds->security.poll_tmo_secs != -1) {
+ if (mds->security.poll) {
/* hold the device throughout */
get_device(cxlds->dev);
^ permalink raw reply related [flat|nested] 30+ messages in thread