On 8/4/2022 6:19 AM, Jonathan Cameron wrote:
On Fri, 15 Jul 2022 14:09:36 -0700
Dave Jiang <[email protected]> wrote:
Create callback function to support the nvdimm_security_ops() ->unlock()
callback. Translate the operation to send "Unlock" security command for CXL
mem device.
When the mem device is unlocked, arch_invalidate_nvdimm_cache() is called
in order to invalidate all CPU caches before attempting to access the mem
device.
See CXL 2.0 spec section 8.2.9.5.6.4 for reference.
Signed-off-by: Dave Jiang <[email protected]>
Hi Dave,
One trivial thing inline.
Thanks,
Jonathan
---
drivers/cxl/cxlmem.h | 1 +
drivers/cxl/security.c | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index ced85be291f3..ae8ccd484491 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -253,6 +253,7 @@ enum cxl_opcode {
CXL_MBOX_OP_GET_SECURITY_STATE = 0x4500,
CXL_MBOX_OP_SET_PASSPHRASE = 0x4501,
CXL_MBOX_OP_DISABLE_PASSPHRASE = 0x4502,
+ CXL_MBOX_OP_UNLOCK = 0x4503,
CXL_MBOX_OP_FREEZE_SECURITY = 0x4504,
CXL_MBOX_OP_MAX = 0x10000
};
diff --git a/drivers/cxl/security.c b/drivers/cxl/security.c
index 6399266a5908..d15520f280f0 100644
--- a/drivers/cxl/security.c
+++ b/drivers/cxl/security.c
@@ -114,11 +114,32 @@ static int cxl_pmem_security_freeze(struct nvdimm *nvdimm)
return cxl_mbox_send_cmd(cxlds, CXL_MBOX_OP_FREEZE_SECURITY, NULL, 0,
NULL, 0);
}
+static int cxl_pmem_security_unlock(struct nvdimm *nvdimm,
+ const struct nvdimm_key_data *key_data)
+{
+ struct cxl_nvdimm *cxl_nvd = nvdimm_provider_data(nvdimm);
+ struct cxl_memdev *cxlmd = cxl_nvd->cxlmd;
+ struct cxl_dev_state *cxlds = cxlmd->cxlds;
+ u8 pass[NVDIMM_PASSPHRASE_LEN];
+ int rc;
+
+ memcpy(pass, key_data->data, NVDIMM_PASSPHRASE_LEN);
Why do we need a local copy? I'd have thought we could just
pass keydata->data in as the payload for cxl_mbox_send_cmd()
There might be some value in making it easier to check by
having a structure defined for this payload (obviously trivial)
but given we are using an array of length defined by a non CXL
define, I'm not sure there is any point in the copy.
We end up hitting a compile warning if we just directly pass in because
key_data->data has const qualifier.
tools/testing/cxl/../../../drivers/cxl/security.c: In function
‘cxl_pmem_security_unlock’:
tools/testing/cxl/../../../drivers/cxl/security.c:116:40: warning: passing
argument 3 of ‘cxl_mbox_send_cmd’ discards ‘const’ qualifier from pointer
target type [-Wdiscarded-qualifiers]
116 | key_data->data, NVDIMM_PASSPHRASE_LEN,
NULL, 0);
| ~~~~~~~~^~~~~~
In file included from tools/testing/cxl/../../../drivers/cxl/security.c:8:
tools/testing/cxl/../../../drivers/cxl/cxlmem.h:408:70: note: expected ‘void *’
but argument is of type ‘const u8 *’ {aka ‘const unsigned char *’}
408 | int cxl_mbox_send_cmd(struct cxl_dev_state *cxlds, u16 opcode, void *in,
| ~~~~~~^~
+ rc = cxl_mbox_send_cmd(cxlds, CXL_MBOX_OP_UNLOCK,
+ pass, NVDIMM_PASSPHRASE_LEN, NULL, 0);
+ if (rc < 0)
+ return rc;
+
+ /* DIMM unlocked, invalidate all CPU caches before we read it */
+ arch_invalidate_nvdimm_cache();
+ return 0;
+}
+
static const struct nvdimm_security_ops __cxl_security_ops = {
.get_flags = cxl_pmem_get_security_flags,
.change_key = cxl_pmem_security_change_key,
.disable = cxl_pmem_security_disable,
.freeze = cxl_pmem_security_freeze,
+ .unlock = cxl_pmem_security_unlock,
};
const struct nvdimm_security_ops *cxl_security_ops = &__cxl_security_ops;
