Re: [PATCH 2/7] cxl/mem: Introduce security state sysfs file

[Dave Jiang <dave.jiang@intel.com>]

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);