| Message ID | 165791933557.2491387.2141316283759403219.stgit@djiang5-desk3.ch.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce security commands for CXL pmem device | expand |
On Fri, 15 Jul 2022, Dave Jiang wrote: >However, the spec leaves a gap WRT master passphrase usages. The spec does >not define any ways to retrieve the status of if the support of master >passphrase is available for the device, nor does the commands that utilize >master passphrase will return a specific error that indicates master >passphrase is not supported. If using a device does not support master >passphrase and a command is issued with a master passphrase, the error >message returned by the device will be ambiguos. In general I think that the 2.0 spec is brief at *best* wrt to these topics. Even if a lot is redundant, there should be an explicit equivalent to the theory of operation found in https://pmem.io/documents/NVDIMM_DSM_Interface-V1.8.pdf Thanks, Davidlohr
On 7/17/2022 11:36 PM, Davidlohr Bueso wrote: > On Fri, 15 Jul 2022, Dave Jiang wrote: > >> However, the spec leaves a gap WRT master passphrase usages. The spec >> does >> not define any ways to retrieve the status of if the support of master >> passphrase is available for the device, nor does the commands that >> utilize >> master passphrase will return a specific error that indicates master >> passphrase is not supported. If using a device does not support master >> passphrase and a command is issued with a master passphrase, the error >> message returned by the device will be ambiguos. > > In general I think that the 2.0 spec is brief at *best* wrt to these > topics. > Even if a lot is redundant, there should be an explicit equivalent to the > theory of operation found in > https://pmem.io/documents/NVDIMM_DSM_Interface-V1.8.pdf I totally agree. > > Thanks, > Davidlohr
On Fri, 15 Jul 2022 14:08:55 -0700 Dave Jiang <dave.jiang@intel.com> wrote: > Create callback function to support the nvdimm_security_ops ->change_key() > callback. Translate the operation to send "Set Passphrase" security command > for CXL memory device. The operation supports setting a passphrase for the > CXL persistent memory device. It also supports the changing of the > currently set passphrase. The operation allows manipulation of a user > passphrase or a master passphrase. > > See CXL 2.0 spec section 8.2.9.5.6.2 for reference. > > However, the spec leaves a gap WRT master passphrase usages. The spec does > not define any ways to retrieve the status of if the support of master > passphrase is available for the device, nor does the commands that utilize > master passphrase will return a specific error that indicates master > passphrase is not supported. If using a device does not support master > passphrase and a command is issued with a master passphrase, the error > message returned by the device will be ambiguos. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> A couple of trivial comments all of which I'm fine with you ignoring if you like Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/cxl/cxlmem.h | 14 ++++++++++++++ > drivers/cxl/security.c | 27 +++++++++++++++++++++++++++ > 2 files changed, 41 insertions(+) > > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > index 35de2889aac3..1e76d22f4fd2 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -251,6 +251,7 @@ enum cxl_opcode { > CXL_MBOX_OP_SCAN_MEDIA = 0x4304, > CXL_MBOX_OP_GET_SCAN_MEDIA = 0x4305, > CXL_MBOX_OP_GET_SECURITY_STATE = 0x4500, > + CXL_MBOX_OP_SET_PASSPHRASE = 0x4501, > CXL_MBOX_OP_MAX = 0x10000 > }; > > @@ -350,6 +351,19 @@ struct cxl_mem_command { > #define CXL_PMEM_SEC_STATE_USER_PLIMIT 0x10 > #define CXL_PMEM_SEC_STATE_MASTER_PLIMIT 0x20 > > +/* set passphrase input payload */ > +struct cxl_set_pass { > + u8 type; > + u8 reserved[31]; > + u8 old_pass[NVDIMM_PASSPHRASE_LEN]; Obviously same length, but maybe a comment to that effect as the is a CXL structure using an NVIDMM define. > + u8 new_pass[NVDIMM_PASSPHRASE_LEN]; > +} __packed; > + > +enum { > + CXL_PMEM_SEC_PASS_MASTER = 0, > + CXL_PMEM_SEC_PASS_USER, > +}; > + > int cxl_mbox_send_cmd(struct cxl_dev_state *cxlds, u16 opcode, void *in, > size_t in_size, void *out, size_t out_size); > int cxl_dev_state_identify(struct cxl_dev_state *cxlds); > diff --git a/drivers/cxl/security.c b/drivers/cxl/security.c > index 5b830ae621db..76ec5087f966 100644 > --- a/drivers/cxl/security.c > +++ b/drivers/cxl/security.c > @@ -50,8 +50,35 @@ static unsigned long cxl_pmem_get_security_flags(struct nvdimm *nvdimm, > return security_flags; > } > > +static int cxl_pmem_security_change_key(struct nvdimm *nvdimm, > + const struct nvdimm_key_data *old_data, > + const struct nvdimm_key_data *new_data, > + enum nvdimm_passphrase_type ptype) > +{ > + struct cxl_nvdimm *cxl_nvd = nvdimm_provider_data(nvdimm); > + struct cxl_memdev *cxlmd = cxl_nvd->cxlmd; > + struct cxl_dev_state *cxlds = cxlmd->cxlds; > + struct cxl_set_pass *set_pass; > + int rc; > + > + set_pass = kzalloc(sizeof(*set_pass), GFP_KERNEL); It's not huge. Maybe just have it on the stack? I'm fine either way. > + if (!set_pass) > + return -ENOMEM; > + > + set_pass->type = ptype == NVDIMM_MASTER ? > + CXL_PMEM_SEC_PASS_MASTER : CXL_PMEM_SEC_PASS_USER; > + memcpy(set_pass->old_pass, old_data->data, NVDIMM_PASSPHRASE_LEN); > + memcpy(set_pass->new_pass, new_data->data, NVDIMM_PASSPHRASE_LEN); > + > + rc = cxl_mbox_send_cmd(cxlds, CXL_MBOX_OP_SET_PASSPHRASE, > + set_pass, sizeof(*set_pass), NULL, 0); > + kfree(set_pass); > + return rc; > +} > + > static const struct nvdimm_security_ops __cxl_security_ops = { > .get_flags = cxl_pmem_get_security_flags, > + .change_key = cxl_pmem_security_change_key, > }; > > const struct nvdimm_security_ops *cxl_security_ops = &__cxl_security_ops; > >
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index 35de2889aac3..1e76d22f4fd2 100644 --- a/drivers/cxl/cxlmem.h +++ b/drivers/cxl/cxlmem.h @@ -251,6 +251,7 @@ enum cxl_opcode { CXL_MBOX_OP_SCAN_MEDIA = 0x4304, CXL_MBOX_OP_GET_SCAN_MEDIA = 0x4305, CXL_MBOX_OP_GET_SECURITY_STATE = 0x4500, + CXL_MBOX_OP_SET_PASSPHRASE = 0x4501, CXL_MBOX_OP_MAX = 0x10000 }; @@ -350,6 +351,19 @@ struct cxl_mem_command { #define CXL_PMEM_SEC_STATE_USER_PLIMIT 0x10 #define CXL_PMEM_SEC_STATE_MASTER_PLIMIT 0x20 +/* set passphrase input payload */ +struct cxl_set_pass { + u8 type; + u8 reserved[31]; + u8 old_pass[NVDIMM_PASSPHRASE_LEN]; + u8 new_pass[NVDIMM_PASSPHRASE_LEN]; +} __packed; + +enum { + CXL_PMEM_SEC_PASS_MASTER = 0, + CXL_PMEM_SEC_PASS_USER, +}; + int cxl_mbox_send_cmd(struct cxl_dev_state *cxlds, u16 opcode, void *in, size_t in_size, void *out, size_t out_size); int cxl_dev_state_identify(struct cxl_dev_state *cxlds); diff --git a/drivers/cxl/security.c b/drivers/cxl/security.c index 5b830ae621db..76ec5087f966 100644 --- a/drivers/cxl/security.c +++ b/drivers/cxl/security.c @@ -50,8 +50,35 @@ static unsigned long cxl_pmem_get_security_flags(struct nvdimm *nvdimm, return security_flags; } +static int cxl_pmem_security_change_key(struct nvdimm *nvdimm, + const struct nvdimm_key_data *old_data, + const struct nvdimm_key_data *new_data, + enum nvdimm_passphrase_type ptype) +{ + struct cxl_nvdimm *cxl_nvd = nvdimm_provider_data(nvdimm); + struct cxl_memdev *cxlmd = cxl_nvd->cxlmd; + struct cxl_dev_state *cxlds = cxlmd->cxlds; + struct cxl_set_pass *set_pass; + int rc; + + set_pass = kzalloc(sizeof(*set_pass), GFP_KERNEL); + if (!set_pass) + return -ENOMEM; + + set_pass->type = ptype == NVDIMM_MASTER ? + CXL_PMEM_SEC_PASS_MASTER : CXL_PMEM_SEC_PASS_USER; + memcpy(set_pass->old_pass, old_data->data, NVDIMM_PASSPHRASE_LEN); + memcpy(set_pass->new_pass, new_data->data, NVDIMM_PASSPHRASE_LEN); + + rc = cxl_mbox_send_cmd(cxlds, CXL_MBOX_OP_SET_PASSPHRASE, + set_pass, sizeof(*set_pass), NULL, 0); + kfree(set_pass); + return rc; +} + static const struct nvdimm_security_ops __cxl_security_ops = { .get_flags = cxl_pmem_get_security_flags, + .change_key = cxl_pmem_security_change_key, }; const struct nvdimm_security_ops *cxl_security_ops = &__cxl_security_ops;
Create callback function to support the nvdimm_security_ops ->change_key() callback. Translate the operation to send "Set Passphrase" security command for CXL memory device. The operation supports setting a passphrase for the CXL persistent memory device. It also supports the changing of the currently set passphrase. The operation allows manipulation of a user passphrase or a master passphrase. See CXL 2.0 spec section 8.2.9.5.6.2 for reference. However, the spec leaves a gap WRT master passphrase usages. The spec does not define any ways to retrieve the status of if the support of master passphrase is available for the device, nor does the commands that utilize master passphrase will return a specific error that indicates master passphrase is not supported. If using a device does not support master passphrase and a command is issued with a master passphrase, the error message returned by the device will be ambiguos. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- drivers/cxl/cxlmem.h | 14 ++++++++++++++ drivers/cxl/security.c | 27 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+)