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

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

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

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

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

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