Re: [PATCH RFC 00/15] Introduce security commands for CXL pmem device

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

On Fri, 15 Jul 2022 14:08:32 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> This series is seeking comments on the implementation. It has not been fully
> tested yet.
> 
> This series adds the support for "Persistent Memory Data-at-rest Security"
> block of command set for the CXL Memory Devices. The enabling is done through
> the nvdimm_security_ops as the operations are very similar to the same
> operations that the persistent memory devices through NFIT provider support.
> This enabling does not include the security pass-through commands nor the
> Santize commands.
> 
> Under the nvdimm_security_ops, this patch series will enable get_flags(),
> freeze(), change_key(), unlock(), disable(), and erase(). The disable() API
> does not support disabling of the master passphrase. To maintain established
> user ABI through the sysfs attribute "security", the "disable" command is
> left untouched and a new "disable_master" command is introduced with a new
> disable_master() API call for the nvdimm_security_ops().
> 
> This series does not include plumbing to directly handle the security commands
> through cxl control util. The enabled security commands will still go through
> ndctl tool with this enabling.
> 
> For calls such as unlock() and erase(), the CPU caches must be invalidated
> post operation. Currently, the implementation resides in
> drivers/acpi/nfit/intel.c with a comment that it should be implemented
> cross arch when more than just NFIT based device needs this operation.
> With the coming of CXL persistent memory devices this is now needed.
> Introduce ARCH_HAS_NVDIMM_INVAL_CACHE and implement similar to
> ARCH_HAS_PMEM_API where the arch can opt in with implementation.
> Currently only add x86_64 implementation where wbinvd_on_all_cpus()
> is called.
> 
Hi Dave,

Just curious.  What was reasoning behind this being a RFC?
What do you particular want comments on?

Thanks,

Jonathan

> ---
> 
> Dave Jiang (15):
>       cxl/pmem: Introduce nvdimm_security_ops with ->get_flags() operation
>       tools/testing/cxl: Create context for cxl mock device
>       tools/testing/cxl: Add "Get Security State" opcode support
>       cxl/pmem: Add "Set Passphrase" security command support
>       tools/testing/cxl: Add "Set Passphrase" opcode support
>       cxl/pmem: Add Disable Passphrase security command support
>       tools/testing/cxl: Add "Disable" security opcode support
>       cxl/pmem: Add "Freeze Security State" security command support
>       tools/testing/cxl: Add "Freeze Security State" security opcode support
>       x86: add an arch helper function to invalidate all cache for nvdimm
>       cxl/pmem: Add "Unlock" security command support
>       tools/testing/cxl: Add "Unlock" security opcode support
>       cxl/pmem: Add "Passphrase Secure Erase" security command support
>       tools/testing/cxl: Add "passphrase secure erase" opcode support
>       nvdimm/cxl/pmem: Add support for master passphrase disable security command
> 
> 
>  arch/x86/Kconfig             |   1 +
>  arch/x86/mm/pat/set_memory.c |   8 +
>  drivers/acpi/nfit/intel.c    |  28 +--
>  drivers/cxl/Kconfig          |  16 ++
>  drivers/cxl/Makefile         |   1 +
>  drivers/cxl/cxlmem.h         |  41 +++++
>  drivers/cxl/pmem.c           |  10 +-
>  drivers/cxl/security.c       | 182 ++++++++++++++++++
>  drivers/nvdimm/security.c    |  33 +++-
>  include/linux/libnvdimm.h    |  10 +
>  lib/Kconfig                  |   3 +
>  tools/testing/cxl/Kbuild     |   1 +
>  tools/testing/cxl/test/mem.c | 348 ++++++++++++++++++++++++++++++++++-
>  13 files changed, 644 insertions(+), 38 deletions(-)
>  create mode 100644 drivers/cxl/security.c
> 
> --
>