mbox series

[RFC,00/15] Introduce security commands for CXL pmem device

Message ID 165791918718.2491387.4203738301057301285.stgit@djiang5-desk3.ch.intel.com
Headers show
Series Introduce security commands for CXL pmem device | expand

Message

Dave Jiang July 15, 2022, 9:08 p.m. UTC
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.

---

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

--

Comments

Davidlohr Bueso July 15, 2022, 9:29 p.m. UTC | #1
On Fri, 15 Jul 2022, Dave Jiang wrote:

>This series is seeking comments on the implementation. It has not been fully
>tested yet.

Sorry if this is already somewhere, but how exactly does one test the mock device?
Dave Jiang July 19, 2022, 6:53 p.m. UTC | #2
On 7/15/2022 2:29 PM, Davidlohr Bueso wrote:
> On Fri, 15 Jul 2022, Dave Jiang wrote:
>
>> This series is seeking comments on the implementation. It has not 
>> been fully
>> tested yet.
>
> Sorry if this is already somewhere, but how exactly does one test the 
> mock device?
So you can do "make M=tools/testing/cxl" to build cxl_test drivers. It's 
similar to ndctl_test and the ndctl README has some instruction on how 
to build and load. Probably should add some information for cxl_test in 
that file. The run_qemu tool from Vishal also provides support for this 
if you add the --cxl-test switch.
Jonathan Cameron Aug. 3, 2022, 5:03 p.m. UTC | #3
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
> 
> --
>
Dave Jiang Aug. 8, 2022, 10:18 p.m. UTC | #4
On 8/3/2022 10:03 AM, Jonathan Cameron wrote:
> 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?

Hi Jonathan. Thanks for reviewing the patches. When I posted the series, 
I haven't tested the code. I just wanted to make sure there are no 
objections to the direction of this enabling going with reusing the 
nvdimm security ops. Once I address Davidlohr and your comments and get 
it fully tested, I'll release v2 w/o RFC.


>
> 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
>>
>> --
>>
>