| Message ID | 20230224194652.1990604-3-dave@stgolabs.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: Background cmds and device sanitation | expand |
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, > }; >
Davidlohr Bueso wrote: > This adds the sysfs memdev's security/ directory with > a single 'state' file, which is always visible. In the > case of unsupported security features, this will show > disabled. > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > --- > Documentation/ABI/testing/sysfs-bus-cxl | 8 ++++ > drivers/cxl/core/memdev.c | 49 +++++++++++++++++++++++++ > 2 files changed, 57 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl > index 3acf2f17a73f..e9c432a5a841 100644 > --- a/Documentation/ABI/testing/sysfs-bus-cxl > +++ b/Documentation/ABI/testing/sysfs-bus-cxl > @@ -57,6 +57,14 @@ Description: > host PCI device for this memory device, emit the CPU node > affinity for this device. > > +What: /sys/bus/cxl/devices/memX/security/state > +Date: February, 2023 > +KernelVersion: v6.4 > +Contact: linux-cxl@vger.kernel.org > +Description: > + (RO) The security state for that device. The following states > + are available: frozen, locked, unlocked and disabled (which > + is also the case for any unsupported security features). > > What: /sys/bus/cxl/devices/*/devtype > Date: June, 2021 > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c > index 0af8856936dc..47cc625bb1b0 100644 > --- a/drivers/cxl/core/memdev.c > +++ b/drivers/cxl/core/memdev.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* Copyright(c) 2020 Intel Corporation. */ > > +#include <linux/memregion.h> > #include <linux/device.h> > #include <linux/slab.h> > #include <linux/idr.h> > @@ -89,6 +90,43 @@ static ssize_t pmem_size_show(struct device *dev, struct device_attribute *attr, > static struct device_attribute dev_attr_pmem_size = > __ATTR(size, 0444, pmem_size_show, NULL); > > +static ssize_t security_state_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + u32 sec_out; > + struct cxl_memdev *cxlmd = to_cxl_memdev(dev); > + struct cxl_dev_state *cxlds = cxlmd->cxlds; > + struct cxl_get_security_output { > + __le32 flags; > + } out; > + struct cxl_mbox_cmd mbox_cmd = { > + .opcode = CXL_MBOX_OP_GET_SECURITY_STATE, > + .payload_out = &out, > + .size_out = sizeof(out), > + }; > + > + if (!cpu_cache_has_invalidate_memregion()) > + goto disabled; I think this can go as security state can still be read even if unlocking is not safely possible. > + > + if (cxl_internal_send_cmd(cxlds, &mbox_cmd) < 0) > + goto disabled; I would prefer to not have an any-user triggerable way to spam mailbox commands. Security state should be read from a cached value that gets updated when security operations are run. > + > + sec_out = le32_to_cpu(out.flags); > + if (!(sec_out & CXL_PMEM_SEC_STATE_USER_PASS_SET)) > + goto disabled; > + if (sec_out & CXL_PMEM_SEC_STATE_FROZEN) > + return sysfs_emit(buf, "frozen\n"); > + if (sec_out & CXL_PMEM_SEC_STATE_LOCKED) > + return sysfs_emit(buf, "locked\n"); > + else > + return sysfs_emit(buf, "unlocked\n"); > +disabled: > + return sysfs_emit(buf, "disabled\n"); > +} > + > +static struct device_attribute dev_attr_security_state = > + __ATTR(state, 0444, security_state_show, NULL); This looks copied from pmem_size above, however that one is using open-coded __ATTR() because the attribute name, "size", does not match the prefix of the show() handler, "pmem_size_show()". In this case the shorter DEVICE_ATTR_RO() helper can be used.
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, };
This adds the sysfs memdev's security/ directory with a single 'state' file, which is always visible. In the case of unsupported security features, this will show disabled. Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> --- Documentation/ABI/testing/sysfs-bus-cxl | 8 ++++ drivers/cxl/core/memdev.c | 49 +++++++++++++++++++++++++ 2 files changed, 57 insertions(+)