On 11/7/2022 7:56 AM, Jonathan Cameron wrote:
On Wed, 21 Sep 2022 08:33:03 -0700
Dave Jiang <dave.ji...@intel.com> wrote:

The mock cxl mem devs needs a way to go into "locked" status to simulate
when the platform is rebooted. Add a sysfs mechanism so the device security
state is set to "locked" and the frozen state bits are cleared.

Signed-off-by: Dave Jiang <dave.ji...@intel.com>
Hi Dave

A few minor comments below.

---
  tools/testing/cxl/test/cxl.c |   52 ++++++++++++++++++++++++++++++++++++++++--
  1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 6dd286a52839..7f76f494a0d4 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -628,6 +628,45 @@ static void mock_companion(struct acpi_device *adev, 
struct device *dev)
  #define SZ_512G (SZ_64G * 8)
  #endif
+static ssize_t security_lock_show(struct device *dev,
+                                 struct device_attribute *attr, char *buf)
+{
+       struct cxl_mock_mem_pdata *mdata = dev_get_platdata(dev);
+
+       return sysfs_emit(buf, "%s\n", mdata->security_state & 
CXL_PMEM_SEC_STATE_LOCKED ?
+                         "locked" : "unlocked");

It's called lock. So 1 or 0 seems sufficient to me rather than needing strings.
Particularly when you use an int to lock it.

ok


+}
+
+static ssize_t security_lock_store(struct device *dev, struct device_attribute 
*attr,
+                                  const char *buf, size_t count)
+{
+       struct cxl_mock_mem_pdata *mdata = dev_get_platdata(dev);
+       u32 mask = CXL_PMEM_SEC_STATE_FROZEN | CXL_PMEM_SEC_STATE_USER_PLIMIT |
+                  CXL_PMEM_SEC_STATE_MASTER_PLIMIT;
+       int val;
+
+       if (kstrtoint(buf, 0, &val) < 0)
+               return -EINVAL;
+
+       if (val == 1) {
+               if (!(mdata->security_state & CXL_PMEM_SEC_STATE_USER_PASS_SET))
+                       return -ENXIO;
+               mdata->security_state |= CXL_PMEM_SEC_STATE_LOCKED;
+               mdata->security_state &= ~mask;
+       } else {
+               return -EINVAL;
+       }
+       return count;
+}
+
+static DEVICE_ATTR_RW(security_lock);
+
+static struct attribute *cxl_mock_mem_attrs[] = {
+       &dev_attr_security_lock.attr,
+       NULL
+};
+ATTRIBUTE_GROUPS(cxl_mock_mem);
+
  static __init int cxl_test_init(void)
  {
        struct cxl_mock_mem_pdata *mem_pdata;
@@ -757,6 +796,11 @@ static __init int cxl_test_init(void)
                        platform_device_put(pdev);
                        goto err_mem;
                }
+
+               rc = device_add_groups(&pdev->dev, cxl_mock_mem_groups);

Can we just set pdev->dev.groups? and avoid dynamic part of this or need to
remove them manually?   I can't immediately find an example of this for
a platform_device but it's done for lots of other types.

ok



+               if (rc)
+                       goto err_mem;
+
                cxl_mem[i] = pdev;
        }
@@ -811,8 +855,12 @@ static __exit void cxl_test_exit(void)
        int i;
platform_device_unregister(cxl_acpi);
-       for (i = ARRAY_SIZE(cxl_mem) - 1; i >= 0; i--)
-               platform_device_unregister(cxl_mem[i]);
+       for (i = ARRAY_SIZE(cxl_mem) - 1; i >= 0; i--) {
+               struct platform_device *pdev = cxl_mem[i];
+
+               device_remove_groups(&pdev->dev, cxl_mock_mem_groups);
+               platform_device_unregister(pdev);
+       }
        for (i = ARRAY_SIZE(cxl_switch_dport) - 1; i >= 0; i--)
                platform_device_unregister(cxl_switch_dport[i]);
        for (i = ARRAY_SIZE(cxl_switch_uport) - 1; i >= 0; i--)





Reply via email to