| Message ID | 166377438336.430546.14222889528313880160.stgit@djiang5-desk3.ch.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce security commands for CXL pmem device | expand |
On Wed, 21 Sep 2022 08:33:03 -0700 Dave Jiang <dave.jiang@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.jiang@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. > +} > + > +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. > + 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--) > >
On 11/7/2022 7:56 AM, Jonathan Cameron wrote: > On Wed, 21 Sep 2022 08:33:03 -0700 > Dave Jiang <dave.jiang@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.jiang@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--) >> >> > >
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"); +} + +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); + 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--)
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.jiang@intel.com> --- tools/testing/cxl/test/cxl.c | 52 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-)