Re: [PATCH 4/7] cxl/mem: Wire up Sanitation support

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

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