| Message ID | 20230421092321.12741-5-dave@stgolabs.net |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cxl: Background cmds and device sanitation | expand |
Hi Davidlohr,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.3-rc7 next-20230420]
[cannot apply to cxl/next cxl/pending]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Davidlohr-Bueso/cxl-pci-Allocate-irq-vectors-earlier-in-pci-probe/20230421-175725
base: linus/master
patch link: https://lore.kernel.org/r/20230421092321.12741-5-dave%40stgolabs.net
patch subject: [PATCH 4/7] cxl/mem: Wire up Sanitation support
config: powerpc-buildonly-randconfig-r006-20230421 (https://download.01.org/0day-ci/archive/20230422/202304220358.PInFhQmP-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 437b7602e4a998220871de78afcb020b9c14a661)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc cross compiling tool for clang build
# apt-get install binutils-powerpc-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/b5227afb40297993eba355a804720c834da3fe2a
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Davidlohr-Bueso/cxl-pci-Allocate-irq-vectors-earlier-in-pci-probe/20230421-175725
git checkout b5227afb40297993eba355a804720c834da3fe2a
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/cxl/core/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304220358.PInFhQmP-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/cxl/core/memdev.c:97:12: error: call to undeclared function 'readq'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
u64 reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
^
1 error generated.
vim +/readq +97 drivers/cxl/core/memdev.c
88
89 static struct device_attribute dev_attr_pmem_size =
90 __ATTR(size, 0444, pmem_size_show, NULL);
91
92 static ssize_t security_sanitize_show(struct device *dev,
93 struct device_attribute *attr, char *buf)
94 {
95 struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
96 struct cxl_dev_state *cxlds = cxlmd->cxlds;
> 97 u64 reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
98 u32 pct = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_PCT_MASK, reg);
99 u16 cmd = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK, reg);
100
101 if (cmd == CXL_MBOX_OP_SANITIZE && pct != 100)
102 return sysfs_emit(buf, "sanitize\n");
103 else
104 return sysfs_emit(buf, "disabled\n");
105 }
106
Hi Davidlohr,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.3-rc7 next-20230420]
[cannot apply to cxl/next cxl/pending]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Davidlohr-Bueso/cxl-pci-Allocate-irq-vectors-earlier-in-pci-probe/20230421-175725
base: linus/master
patch link: https://lore.kernel.org/r/20230421092321.12741-5-dave%40stgolabs.net
patch subject: [PATCH 4/7] cxl/mem: Wire up Sanitation support
config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20230422/202304220436.O3l806d0-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/b5227afb40297993eba355a804720c834da3fe2a
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Davidlohr-Bueso/cxl-pci-Allocate-irq-vectors-earlier-in-pci-probe/20230421-175725
git checkout b5227afb40297993eba355a804720c834da3fe2a
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/cxl/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304220436.O3l806d0-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/cxl/core/memdev.c:97:12: error: implicit declaration of function 'readq' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
u64 reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
^
1 error generated.
vim +/readq +97 drivers/cxl/core/memdev.c
88
89 static struct device_attribute dev_attr_pmem_size =
90 __ATTR(size, 0444, pmem_size_show, NULL);
91
92 static ssize_t security_sanitize_show(struct device *dev,
93 struct device_attribute *attr, char *buf)
94 {
95 struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
96 struct cxl_dev_state *cxlds = cxlmd->cxlds;
> 97 u64 reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_BG_CMD_STATUS_OFFSET);
98 u32 pct = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_PCT_MASK, reg);
99 u16 cmd = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK, reg);
100
101 if (cmd == CXL_MBOX_OP_SANITIZE && pct != 100)
102 return sysfs_emit(buf, "sanitize\n");
103 else
104 return sysfs_emit(buf, "disabled\n");
105 }
106
On Fri, 21 Apr 2023 02:23:18 -0700 Davidlohr Bueso <dave@stgolabs.net> wrote: > Implement support for CXL 3.0 8.2.9.8.5.1 Sanitize. This is done by > adding a security/sanitize' memdev sysfs file, which is poll(2)-capable > for completion. Unlike all other background commands, this is the > only operation that is special and monopolizes the device for long > periods of time. > > In addition to the traditional pmem security requirements, all regions > must also be offline in order to perform the operation. > This permits > avoiding explicit global CPU cache management, relying instead on > attach_target() setting CXL_REGION_F_INCOHERENT upon reconnect. > > The expectation is that userspace can use it such as: > > cxl disable-memdev memX > echo 1 > /sys/bus/cxl/devices/memX/security/sanitize > cxl wait-sanitize memX > cxl enable-memdev memX > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > --- > Documentation/ABI/testing/sysfs-bus-cxl | 19 ++++++ > drivers/cxl/core/mbox.c | 56 ++++++++++++++++ > drivers/cxl/core/memdev.c | 86 +++++++++++++++++++++++++ > drivers/cxl/cxlmem.h | 4 ++ > drivers/cxl/pci.c | 5 ++ > 5 files changed, 170 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl > index 3acf2f17a73f..2e98ec9220ca 100644 > --- a/Documentation/ABI/testing/sysfs-bus-cxl > +++ b/Documentation/ABI/testing/sysfs-bus-cxl > @@ -58,6 +58,25 @@ Description: > affinity for this device. > > > +What: /sys/bus/cxl/devices/memX/security/sanitize > +Date: May, 2023 > +KernelVersion: v6.5 > +Contact: linux-cxl@vger.kernel.org > +Description: > + (RW) Write a boolean 'true' string value to this attribute to > + sanitize the device to securely re-purpose or decommission it. > + This is done by ensuring that all user data and meta-data, > + whether it resides in persistent capacity, volatile capacity, > + or the LSA, is made permanently unavailable by whatever means > + is appropriate for the media type. This functionality requires > + the device to be not be actively decoding any HPA ranges. > + > + Reading this file shows either "disabled" when not running, or > + "sanitize" during the duration of the sanitize operation. This > + sysfs entry is select/poll capable from userspace to notify upon > + completion. A sysfs attribute that reads different from what is written is not very intuitive. The one file one thing rule suggests to me that you should have a separate santize_status or similar. Or just have this read true when in progress making it a self resetting toggle that returns -EBUSY if anyone tries to unset it. > + > + > What: /sys/bus/cxl/devices/*/devtype > Date: June, 2021 > KernelVersion: v5.14 > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > index cde7270c6037..28daf7dcdec4 100644 > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > @@ -1021,6 +1021,62 @@ int cxl_dev_state_identify(struct cxl_dev_state *cxlds) > } > EXPORT_SYMBOL_NS_GPL(cxl_dev_state_identify, CXL); > > +/** > + * cxl_mem_sanitize() - Send a sanitation command to the device. > + * @cxlds: The device data for the operation > + * @cmd: The specific sanitation command opcode > + * > + * Return: 0 if the command was executed successfully, regardless of > + * whether or not the actual security operation is done in the background, > + * such as for the Sanitize case. > + * Error return values can be the result of the mailbox command, -EINVAL > + * when security requirements are not met or invalid contexts, or -EBUSY > + * if the device is not offline. What does offline mean for the device? Perhaps a tighter definition needed. > + * > + * See CXL 3.0 @8.2.9.8.5.1 Sanitize and @8.2.9.8.5.2 Secure Erase. This @ syntax would be fine but it's inconsistent with other references in this file. > + */ > +int cxl_mem_sanitize(struct cxl_dev_state *cxlds, u16 cmd) > +{ > + int rc; > + u32 sec_out = 0; > + struct cxl_get_security_output { > + __le32 flags; > + } out; > + struct cxl_mbox_cmd sec_cmd = { > + .opcode = CXL_MBOX_OP_GET_SECURITY_STATE, > + .payload_out = &out, > + .size_out = sizeof(out), > + }; > + struct cxl_mbox_cmd mbox_cmd = { .opcode = cmd }; > + > + if (cmd != CXL_MBOX_OP_SANITIZE) > + return -EINVAL; > + > + rc = cxl_internal_send_cmd(cxlds, &sec_cmd); > + if (rc < 0) { > + dev_err(cxlds->dev, "Failed to get security state : %d", rc); > + return rc; > + } > + > + /* > + * Prior to using these commands, any security applied to > + * the user data areas of the device shall be DISABLED (or > + * UNLOCKED for secure erase case). > + */ > + sec_out = le32_to_cpu(out.flags); > + if (sec_out & CXL_PMEM_SEC_STATE_USER_PASS_SET) > + return -EINVAL; > + > + rc = cxl_internal_send_cmd(cxlds, &mbox_cmd); > + if (rc < 0) { > + dev_err(cxlds->dev, "Failed to sanitize device : %d", rc); > + return rc; > + } > + > + return 0; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_mem_sanitize, CXL); > + > static int add_dpa_res(struct device *dev, struct resource *parent, > struct resource *res, resource_size_t start, > resource_size_t size, const char *type) > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c > index 28a05f2fe32d..70e7158826c9 100644 > --- a/drivers/cxl/core/memdev.c > +++ b/drivers/cxl/core/memdev.c > @@ -89,6 +89,55 @@ 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_sanitize_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; > + u64 reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_BG_CMD_STATUS_OFFSET); > + u32 pct = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_PCT_MASK, reg); > + u16 cmd = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK, reg); > + > + if (cmd == CXL_MBOX_OP_SANITIZE && pct != 100) > + return sysfs_emit(buf, "sanitize\n"); > + else > + return sysfs_emit(buf, "disabled\n"); As above. I don't like inconsistency of read and write values. > +} > + > +static ssize_t security_sanitize_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t len) > +{ > + struct cxl_memdev *cxlmd = to_cxl_memdev(dev); > + struct cxl_dev_state *cxlds = cxlmd->cxlds; > + ssize_t rc; > + bool sanitize; > + > + rc = kstrtobool(buf, &sanitize); > + if (rc) > + return rc; > + > + if (sanitize) { I'd short cut the false case if (!sanitize) return len; ... > + struct cxl_port *port = dev_get_drvdata(&cxlmd->dev); > + > + if (!port || !is_cxl_endpoint(port)) > + return -EINVAL; > + /* ensure no regions are mapped to this memdev */ > + if (port->commit_end != -1) > + return -EBUSY; > + > + rc = cxl_mem_sanitize(cxlds, CXL_MBOX_OP_SANITIZE); if (rc) return rc; } return len; Simple flow is easier for reviewers to follow. > + } > + > + if (rc == 0) > + rc = len; > + return rc; > +} > + > @@ -324,11 +384,19 @@ static const struct file_operations cxl_memdev_fops = { > .llseek = noop_llseek, > }; > > +static void put_sanitize(void *data) > +{ > + struct cxl_dev_state *cxlds = data; > + > + sysfs_put(cxlds->sec.sanitize_state); > +} > + > struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds) > { > struct cxl_memdev *cxlmd; > struct device *dev; > struct cdev *cdev; > + struct kernfs_node *sec; > int rc; > > cxlmd = cxl_memdev_alloc(cxlds, &cxl_memdev_fops); > @@ -355,6 +423,24 @@ struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds) > rc = devm_add_action_or_reset(cxlds->dev, cxl_memdev_unregister, cxlmd); > if (rc) > return ERR_PTR(rc); > + > + sec = sysfs_get_dirent(dev->kobj.sd, "security"); > + if (!sec) { > + dev_err(dev, "sysfs_get_dirent 'security' failed\n"); > + rc = -ENODEV; > + goto err; At this stage the devm action is registered to unwind anything above here, so just return ERR_PTR(-ENODEV); > + } > + cxlds->sec.sanitize_state = sysfs_get_dirent(sec, "sanitize"); > + sysfs_put(sec); > + if (!cxlds->sec.sanitize_state) { > + dev_err(dev, "sysfs_get_dirent 'sanitize' failed\n"); > + rc = -ENODEV; > + goto err; return ERR_PTR(-ENODDEV); > + } > + rc = devm_add_action_or_reset(cxlds->dev, put_sanitize, cxlds); > + if (rc) > + return ERR_PTR(rc); > + > return cxlmd; > > err: > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > index 17e3ab3c641a..9bd33cfdc0ec 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -223,10 +223,12 @@ struct cxl_event_state { > /** > * struct cxl_security_state - Device security state > * > + * @sanitize_state: sanitation sysfs file to notify > * @sanitize_dwork: self-polling work item for sanitation > * @sanitize_tmo: self-polling timeout > */ > struct cxl_security_state { > + struct kernfs_node *sanitize_state; > /* below only used if device mbox irqs are not supported */ > struct delayed_work sanitize_dwork; > int sanitize_tmo; > @@ -642,6 +644,8 @@ static inline void cxl_mem_active_dec(void) > } > #endif > > +int cxl_mem_sanitize(struct cxl_dev_state *cxlds, u16 cmd); > + > struct cxl_hdm { > struct cxl_component_regs regs; > unsigned int decoder_count; > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index bdee5273af5a..2bc3b595f270 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -113,6 +113,9 @@ static irqreturn_t cxl_pci_mbox_irq(int irq, void *id) > opcode = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK, reg); > > if (opcode == CXL_MBOX_OP_SANITIZE) { > + if (cxlds->sec.sanitize_state) > + sysfs_notify_dirent(cxlds->sec.sanitize_state); > + > dev_dbg(cxlds->dev, "Sanitation operation ended\n"); > } else { > /* short-circuit the wait in __cxl_pci_mbox_send_cmd() */ > @@ -138,6 +141,8 @@ static void cxl_mbox_sanitize_work(struct work_struct *work) > if (cxl_mbox_background_complete(cxlds)) { > cxlds->sec.sanitize_tmo = 0; > put_device(cxlds->dev); > + if (cxlds->sec.sanitize_state) > + sysfs_notify_dirent(cxlds->sec.sanitize_state); > > dev_dbg(cxlds->dev, "Sanitation operation ended\n"); > } else {
On Thu, 11 May 2023, Jonathan Cameron wrote: >> +What: /sys/bus/cxl/devices/memX/security/sanitize >> +Date: May, 2023 >> +KernelVersion: v6.5 >> +Contact: linux-cxl@vger.kernel.org >> +Description: >> + (RW) Write a boolean 'true' string value to this attribute to >> + sanitize the device to securely re-purpose or decommission it. >> + This is done by ensuring that all user data and meta-data, >> + whether it resides in persistent capacity, volatile capacity, >> + or the LSA, is made permanently unavailable by whatever means >> + is appropriate for the media type. This functionality requires >> + the device to be not be actively decoding any HPA ranges. >> + >> + Reading this file shows either "disabled" when not running, or >> + "sanitize" during the duration of the sanitize operation. This >> + sysfs entry is select/poll capable from userspace to notify upon >> + completion. > >A sysfs attribute that reads different from what is written is not very intuitive. >The one file one thing rule suggests to me that you should have a separate >santize_status or similar. Or just have this read true when in progress making >it a self resetting toggle that returns -EBUSY if anyone tries to unset it. So the plan is to also to have the (cached) pmem security status (read-only): /sys/bus/cxl/devices/memX/security/status sanitize could nicely be incorporated there and just read/poll that file for all things security. So security/sanitize file goes to being write-only, just like its secure erase counter part. >> + >> + >> What: /sys/bus/cxl/devices/*/devtype >> Date: June, 2021 >> KernelVersion: v5.14 >> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c >> index cde7270c6037..28daf7dcdec4 100644 >> --- a/drivers/cxl/core/mbox.c >> +++ b/drivers/cxl/core/mbox.c >> @@ -1021,6 +1021,62 @@ int cxl_dev_state_identify(struct cxl_dev_state *cxlds) >> } >> EXPORT_SYMBOL_NS_GPL(cxl_dev_state_identify, CXL); >> >> +/** >> + * cxl_mem_sanitize() - Send a sanitation command to the device. >> + * @cxlds: The device data for the operation >> + * @cmd: The specific sanitation command opcode >> + * >> + * Return: 0 if the command was executed successfully, regardless of >> + * whether or not the actual security operation is done in the background, >> + * such as for the Sanitize case. >> + * Error return values can be the result of the mailbox command, -EINVAL >> + * when security requirements are not met or invalid contexts, or -EBUSY >> + * if the device is not offline. > >What does offline mean for the device? Perhaps a tighter definition needed. I can expand. But overall, with Alison's poison work being picked up, now we can add a cxl_memdev_active() helper to ensure no regions are mapped to this memdev. Thanks, Davidlohr
On Thu, 11 May 2023 10:23:31 -0700 Davidlohr Bueso <dave@stgolabs.net> wrote: > On Thu, 11 May 2023, Jonathan Cameron wrote: > > >> +What: /sys/bus/cxl/devices/memX/security/sanitize > >> +Date: May, 2023 > >> +KernelVersion: v6.5 > >> +Contact: linux-cxl@vger.kernel.org > >> +Description: > >> + (RW) Write a boolean 'true' string value to this attribute to > >> + sanitize the device to securely re-purpose or decommission it. > >> + This is done by ensuring that all user data and meta-data, > >> + whether it resides in persistent capacity, volatile capacity, > >> + or the LSA, is made permanently unavailable by whatever means > >> + is appropriate for the media type. This functionality requires > >> + the device to be not be actively decoding any HPA ranges. > >> + > >> + Reading this file shows either "disabled" when not running, or > >> + "sanitize" during the duration of the sanitize operation. This > >> + sysfs entry is select/poll capable from userspace to notify upon > >> + completion. > > > >A sysfs attribute that reads different from what is written is not very intuitive. > >The one file one thing rule suggests to me that you should have a separate > >santize_status or similar. Or just have this read true when in progress making > >it a self resetting toggle that returns -EBUSY if anyone tries to unset it. > > So the plan is to also to have the (cached) pmem security status (read-only): > /sys/bus/cxl/devices/memX/security/status > > sanitize could nicely be incorporated there and just read/poll that file for all > things security. So security/sanitize file goes to being write-only, just like > its secure erase counter part. That works nicely. Good plan. > > >> + > >> + > >> What: /sys/bus/cxl/devices/*/devtype > >> Date: June, 2021 > >> KernelVersion: v5.14 > >> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > >> index cde7270c6037..28daf7dcdec4 100644 > >> --- a/drivers/cxl/core/mbox.c > >> +++ b/drivers/cxl/core/mbox.c > >> @@ -1021,6 +1021,62 @@ int cxl_dev_state_identify(struct cxl_dev_state *cxlds) > >> } > >> EXPORT_SYMBOL_NS_GPL(cxl_dev_state_identify, CXL); > >> > >> +/** > >> + * cxl_mem_sanitize() - Send a sanitation command to the device. > >> + * @cxlds: The device data for the operation > >> + * @cmd: The specific sanitation command opcode > >> + * > >> + * Return: 0 if the command was executed successfully, regardless of > >> + * whether or not the actual security operation is done in the background, > >> + * such as for the Sanitize case. > >> + * Error return values can be the result of the mailbox command, -EINVAL > >> + * when security requirements are not met or invalid contexts, or -EBUSY > >> + * if the device is not offline. > > > >What does offline mean for the device? Perhaps a tighter definition needed. > > I can expand. But overall, with Alison's poison work being picked up, now we > can add a cxl_memdev_active() helper to ensure no regions are mapped to this > memdev. Ok. > > Thanks, > Davidlohr
diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl index 3acf2f17a73f..2e98ec9220ca 100644 --- a/Documentation/ABI/testing/sysfs-bus-cxl +++ b/Documentation/ABI/testing/sysfs-bus-cxl @@ -58,6 +58,25 @@ Description: affinity for this device. +What: /sys/bus/cxl/devices/memX/security/sanitize +Date: May, 2023 +KernelVersion: v6.5 +Contact: linux-cxl@vger.kernel.org +Description: + (RW) Write a boolean 'true' string value to this attribute to + sanitize the device to securely re-purpose or decommission it. + This is done by ensuring that all user data and meta-data, + whether it resides in persistent capacity, volatile capacity, + or the LSA, is made permanently unavailable by whatever means + is appropriate for the media type. This functionality requires + the device to be not be actively decoding any HPA ranges. + + Reading this file shows either "disabled" when not running, or + "sanitize" during the duration of the sanitize operation. This + sysfs entry is select/poll capable from userspace to notify upon + completion. + + What: /sys/bus/cxl/devices/*/devtype Date: June, 2021 KernelVersion: v5.14 diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c index cde7270c6037..28daf7dcdec4 100644 --- a/drivers/cxl/core/mbox.c +++ b/drivers/cxl/core/mbox.c @@ -1021,6 +1021,62 @@ int cxl_dev_state_identify(struct cxl_dev_state *cxlds) } EXPORT_SYMBOL_NS_GPL(cxl_dev_state_identify, CXL); +/** + * cxl_mem_sanitize() - Send a sanitation command to the device. + * @cxlds: The device data for the operation + * @cmd: The specific sanitation command opcode + * + * Return: 0 if the command was executed successfully, regardless of + * whether or not the actual security operation is done in the background, + * such as for the Sanitize case. + * Error return values can be the result of the mailbox command, -EINVAL + * when security requirements are not met or invalid contexts, or -EBUSY + * if the device is not offline. + * + * See CXL 3.0 @8.2.9.8.5.1 Sanitize and @8.2.9.8.5.2 Secure Erase. + */ +int cxl_mem_sanitize(struct cxl_dev_state *cxlds, u16 cmd) +{ + int rc; + u32 sec_out = 0; + struct cxl_get_security_output { + __le32 flags; + } out; + struct cxl_mbox_cmd sec_cmd = { + .opcode = CXL_MBOX_OP_GET_SECURITY_STATE, + .payload_out = &out, + .size_out = sizeof(out), + }; + struct cxl_mbox_cmd mbox_cmd = { .opcode = cmd }; + + if (cmd != CXL_MBOX_OP_SANITIZE) + return -EINVAL; + + rc = cxl_internal_send_cmd(cxlds, &sec_cmd); + if (rc < 0) { + dev_err(cxlds->dev, "Failed to get security state : %d", rc); + return rc; + } + + /* + * Prior to using these commands, any security applied to + * the user data areas of the device shall be DISABLED (or + * UNLOCKED for secure erase case). + */ + sec_out = le32_to_cpu(out.flags); + if (sec_out & CXL_PMEM_SEC_STATE_USER_PASS_SET) + return -EINVAL; + + rc = cxl_internal_send_cmd(cxlds, &mbox_cmd); + if (rc < 0) { + dev_err(cxlds->dev, "Failed to sanitize device : %d", rc); + return rc; + } + + return 0; +} +EXPORT_SYMBOL_NS_GPL(cxl_mem_sanitize, CXL); + static int add_dpa_res(struct device *dev, struct resource *parent, struct resource *res, resource_size_t start, resource_size_t size, const char *type) diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c index 28a05f2fe32d..70e7158826c9 100644 --- a/drivers/cxl/core/memdev.c +++ b/drivers/cxl/core/memdev.c @@ -89,6 +89,55 @@ 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_sanitize_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; + u64 reg = readq(cxlds->regs.mbox + CXLDEV_MBOX_BG_CMD_STATUS_OFFSET); + u32 pct = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_PCT_MASK, reg); + u16 cmd = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK, reg); + + if (cmd == CXL_MBOX_OP_SANITIZE && pct != 100) + return sysfs_emit(buf, "sanitize\n"); + else + return sysfs_emit(buf, "disabled\n"); +} + +static ssize_t security_sanitize_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + struct cxl_memdev *cxlmd = to_cxl_memdev(dev); + struct cxl_dev_state *cxlds = cxlmd->cxlds; + ssize_t rc; + bool sanitize; + + rc = kstrtobool(buf, &sanitize); + if (rc) + return rc; + + if (sanitize) { + struct cxl_port *port = dev_get_drvdata(&cxlmd->dev); + + if (!port || !is_cxl_endpoint(port)) + return -EINVAL; + /* ensure no regions are mapped to this memdev */ + if (port->commit_end != -1) + return -EBUSY; + + rc = cxl_mem_sanitize(cxlds, CXL_MBOX_OP_SANITIZE); + } + + if (rc == 0) + rc = len; + return rc; +} + +static struct device_attribute dev_attr_security_sanitize = + __ATTR(sanitize, 0644, + security_sanitize_show, security_sanitize_store); + static ssize_t serial_show(struct device *dev, struct device_attribute *attr, char *buf) { @@ -148,10 +197,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_sanitize.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, }; @@ -324,11 +384,19 @@ static const struct file_operations cxl_memdev_fops = { .llseek = noop_llseek, }; +static void put_sanitize(void *data) +{ + struct cxl_dev_state *cxlds = data; + + sysfs_put(cxlds->sec.sanitize_state); +} + struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds) { struct cxl_memdev *cxlmd; struct device *dev; struct cdev *cdev; + struct kernfs_node *sec; int rc; cxlmd = cxl_memdev_alloc(cxlds, &cxl_memdev_fops); @@ -355,6 +423,24 @@ struct cxl_memdev *devm_cxl_add_memdev(struct cxl_dev_state *cxlds) rc = devm_add_action_or_reset(cxlds->dev, cxl_memdev_unregister, cxlmd); if (rc) return ERR_PTR(rc); + + sec = sysfs_get_dirent(dev->kobj.sd, "security"); + if (!sec) { + dev_err(dev, "sysfs_get_dirent 'security' failed\n"); + rc = -ENODEV; + goto err; + } + cxlds->sec.sanitize_state = sysfs_get_dirent(sec, "sanitize"); + sysfs_put(sec); + if (!cxlds->sec.sanitize_state) { + dev_err(dev, "sysfs_get_dirent 'sanitize' failed\n"); + rc = -ENODEV; + goto err; + } + rc = devm_add_action_or_reset(cxlds->dev, put_sanitize, cxlds); + if (rc) + return ERR_PTR(rc); + return cxlmd; err: diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index 17e3ab3c641a..9bd33cfdc0ec 100644 --- a/drivers/cxl/cxlmem.h +++ b/drivers/cxl/cxlmem.h @@ -223,10 +223,12 @@ struct cxl_event_state { /** * struct cxl_security_state - Device security state * + * @sanitize_state: sanitation sysfs file to notify * @sanitize_dwork: self-polling work item for sanitation * @sanitize_tmo: self-polling timeout */ struct cxl_security_state { + struct kernfs_node *sanitize_state; /* below only used if device mbox irqs are not supported */ struct delayed_work sanitize_dwork; int sanitize_tmo; @@ -642,6 +644,8 @@ static inline void cxl_mem_active_dec(void) } #endif +int cxl_mem_sanitize(struct cxl_dev_state *cxlds, u16 cmd); + struct cxl_hdm { struct cxl_component_regs regs; unsigned int decoder_count; diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index bdee5273af5a..2bc3b595f270 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -113,6 +113,9 @@ static irqreturn_t cxl_pci_mbox_irq(int irq, void *id) opcode = FIELD_GET(CXLDEV_MBOX_BG_CMD_COMMAND_OPCODE_MASK, reg); if (opcode == CXL_MBOX_OP_SANITIZE) { + if (cxlds->sec.sanitize_state) + sysfs_notify_dirent(cxlds->sec.sanitize_state); + dev_dbg(cxlds->dev, "Sanitation operation ended\n"); } else { /* short-circuit the wait in __cxl_pci_mbox_send_cmd() */ @@ -138,6 +141,8 @@ static void cxl_mbox_sanitize_work(struct work_struct *work) if (cxl_mbox_background_complete(cxlds)) { cxlds->sec.sanitize_tmo = 0; put_device(cxlds->dev); + if (cxlds->sec.sanitize_state) + sysfs_notify_dirent(cxlds->sec.sanitize_state); dev_dbg(cxlds->dev, "Sanitation operation ended\n"); } else {
Implement support for CXL 3.0 8.2.9.8.5.1 Sanitize. This is done by adding a security/sanitize' memdev sysfs file, which is poll(2)-capable for completion. Unlike all other background commands, this is the only operation that is special and monopolizes the device for long periods of time. In addition to the traditional pmem security requirements, all regions must also be offline in order to perform the operation. This permits avoiding explicit global CPU cache management, relying instead on attach_target() setting CXL_REGION_F_INCOHERENT upon reconnect. The expectation is that userspace can use it such as: cxl disable-memdev memX echo 1 > /sys/bus/cxl/devices/memX/security/sanitize cxl wait-sanitize memX cxl enable-memdev memX Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> --- Documentation/ABI/testing/sysfs-bus-cxl | 19 ++++++ drivers/cxl/core/mbox.c | 56 ++++++++++++++++ drivers/cxl/core/memdev.c | 86 +++++++++++++++++++++++++ drivers/cxl/cxlmem.h | 4 ++ drivers/cxl/pci.c | 5 ++ 5 files changed, 170 insertions(+)