| Message ID | 150032210775.102848.1585950131204303137.stgit@sosxen.amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Herbert Xu |
| Headers | show |
On 7/17/2017 3:08 PM, Gary R Hook wrote: > Version 5 CCPs have differing requirements for XTS-AES: key components > are stored in a 512-bit vector. The context must be little-endian > justified. AES-256 is supported now, so propagate the cipher size to > the command descriptor. > > Signed-off-by: Gary R Hook <gary.hook@amd.com> > --- > drivers/crypto/ccp/ccp-crypto-aes-xts.c | 79 ++++++++++++++++--------------- > drivers/crypto/ccp/ccp-dev-v5.c | 2 + > drivers/crypto/ccp/ccp-dev.h | 2 + > drivers/crypto/ccp/ccp-ops.c | 56 ++++++++++++++++++---- > 4 files changed, 89 insertions(+), 50 deletions(-) > > diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c b/drivers/crypto/ccp/ccp-crypto-aes-xts.c > index 58a4244b4752..8d248b198e22 100644 > --- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c > +++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c > @@ -1,7 +1,7 @@ > /* > * AMD Cryptographic Coprocessor (CCP) AES XTS crypto API support > * > - * Copyright (C) 2013 Advanced Micro Devices, Inc. > + * Copyright (C) 2013,2017 Advanced Micro Devices, Inc. > * > * Author: Tom Lendacky <thomas.lendacky@amd.com> > * > @@ -37,46 +37,26 @@ struct ccp_unit_size_map { > u32 value; > }; > > -static struct ccp_unit_size_map unit_size_map[] = { > +static struct ccp_unit_size_map xts_unit_sizes[] = { > { > - .size = 4096, > - .value = CCP_XTS_AES_UNIT_SIZE_4096, > - }, > - { > - .size = 2048, > - .value = CCP_XTS_AES_UNIT_SIZE_2048, > - }, > - { > - .size = 1024, > - .value = CCP_XTS_AES_UNIT_SIZE_1024, > + .size = 16, > + .value = CCP_XTS_AES_UNIT_SIZE_16, > }, > { > - .size = 512, > + .size = 512, > .value = CCP_XTS_AES_UNIT_SIZE_512, > }, > { > - .size = 256, > - .value = CCP_XTS_AES_UNIT_SIZE__LAST, > - }, > - { > - .size = 128, > - .value = CCP_XTS_AES_UNIT_SIZE__LAST, > - }, > - { > - .size = 64, > - .value = CCP_XTS_AES_UNIT_SIZE__LAST, > - }, > - { > - .size = 32, > - .value = CCP_XTS_AES_UNIT_SIZE__LAST, > + .size = 1024, > + .value = CCP_XTS_AES_UNIT_SIZE_1024, > }, > { > - .size = 16, > - .value = CCP_XTS_AES_UNIT_SIZE_16, > + .size = 2048, > + .value = CCP_XTS_AES_UNIT_SIZE_2048, > }, > { > - .size = 1, > - .value = CCP_XTS_AES_UNIT_SIZE__LAST, > + .size = 4096, > + .value = CCP_XTS_AES_UNIT_SIZE_4096, > }, > }; Because of the way the unit size check is performed, you can't delete the intermediate size checks. Those must remain so that unit sizes that aren't supported by the CCP are sent to the fallback mechanism. Also, re-arranging the order should be a separate patch if that doesn't really fix anything. > > @@ -97,14 +77,20 @@ static int ccp_aes_xts_setkey(struct crypto_ablkcipher *tfm, const u8 *key, > unsigned int key_len) > { > struct ccp_ctx *ctx = crypto_tfm_ctx(crypto_ablkcipher_tfm(tfm)); > + unsigned int ccpversion = ccp_version(); > > /* Only support 128-bit AES key with a 128-bit Tweak key, > * otherwise use the fallback > */ > + Remove the addition of the blank line and update the above comment to indicate the new supported key size added below. > switch (key_len) { > case AES_KEYSIZE_128 * 2: > memcpy(ctx->u.aes.key, key, key_len); > break; > + case AES_KEYSIZE_256 * 2: > + if (ccpversion > CCP_VERSION(3, 0)) > + memcpy(ctx->u.aes.key, key, key_len); > + break; Isn't u.aes.key defined with a maximum buffer size of AES_MAX_KEY_SIZE (which is 32)? I think this will cause a buffer overrun on memcpy. > } > ctx->u.aes.key_len = key_len / 2; > sg_init_one(&ctx->u.aes.key_sg, ctx->u.aes.key, key_len); > @@ -117,7 +103,10 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req, > { > struct ccp_ctx *ctx = crypto_tfm_ctx(req->base.tfm); > struct ccp_aes_req_ctx *rctx = ablkcipher_request_ctx(req); > + unsigned int ccpversion = ccp_version(); > + unsigned int fallback = 0; > unsigned int unit; > + u32 block_size = 0; > u32 unit_size; > int ret; > > @@ -131,17 +120,29 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req, > return -EINVAL; > > unit_size = CCP_XTS_AES_UNIT_SIZE__LAST; > - if (req->nbytes <= unit_size_map[0].size) { This check can't be deleted. It was added specifically to catch cases where the size was greater than 4096. > - for (unit = 0; unit < ARRAY_SIZE(unit_size_map); unit++) { > - if (!(req->nbytes & (unit_size_map[unit].size - 1))) { > - unit_size = unit_size_map[unit].value; > - break; > - } > + for (unit = ARRAY_SIZE(xts_unit_sizes); unit-- > 0; ) { > + if (!(req->nbytes & (xts_unit_sizes[unit].size - 1))) { > + unit_size = unit; > + block_size = xts_unit_sizes[unit].size; > + break; > } > } > > - if ((unit_size == CCP_XTS_AES_UNIT_SIZE__LAST) || > - (ctx->u.aes.key_len != AES_KEYSIZE_128)) { > + /* The CCP has restrictions on block sizes. Also, a version 3 device > + * only supports AES-128 operations; version 5 CCPs support both > + * AES-128 and -256 operations. > + */ > + if (unit_size == CCP_XTS_AES_UNIT_SIZE__LAST) > + fallback = 1; > + if ((ccpversion < CCP_VERSION(5, 0)) && > + (ctx->u.aes.key_len != AES_KEYSIZE_128)) > + fallback = 1; > + if ((ctx->u.aes.key_len != AES_KEYSIZE_128) && > + (ctx->u.aes.key_len != AES_KEYSIZE_256)) > + fallback = 1; > + if (req->nbytes != block_size) > + fallback = 1; I don't believe this is correct. Everything should fall out properly for fallback based on the unit size and key size checks above. Thanks, Tom > + if (fallback) { > SKCIPHER_REQUEST_ON_STACK(subreq, ctx->u.aes.tfm_skcipher); > > /* Use the fallback to process the request for any > diff --git a/drivers/crypto/ccp/ccp-dev-v5.c b/drivers/crypto/ccp/ccp-dev-v5.c > index b3526336d608..11febe2bd07c 100644 > --- a/drivers/crypto/ccp/ccp-dev-v5.c > +++ b/drivers/crypto/ccp/ccp-dev-v5.c > @@ -144,6 +144,7 @@ union ccp_function { > #define CCP_AES_ENCRYPT(p) ((p)->aes.encrypt) > #define CCP_AES_MODE(p) ((p)->aes.mode) > #define CCP_AES_TYPE(p) ((p)->aes.type) > +#define CCP_XTS_TYPE(p) ((p)->aes_xts.type) > #define CCP_XTS_SIZE(p) ((p)->aes_xts.size) > #define CCP_XTS_ENCRYPT(p) ((p)->aes_xts.encrypt) > #define CCP_DES3_SIZE(p) ((p)->des3.size) > @@ -344,6 +345,7 @@ static int ccp5_perform_xts_aes(struct ccp_op *op) > CCP5_CMD_PROT(&desc) = 0; > > function.raw = 0; > + CCP_XTS_TYPE(&function) = op->u.xts.type; > CCP_XTS_ENCRYPT(&function) = op->u.xts.action; > CCP_XTS_SIZE(&function) = op->u.xts.unit_size; > CCP5_CMD_FUNCTION(&desc) = function.raw; > diff --git a/drivers/crypto/ccp/ccp-dev.h b/drivers/crypto/ccp/ccp-dev.h > index 9320931d89da..3d51180199ac 100644 > --- a/drivers/crypto/ccp/ccp-dev.h > +++ b/drivers/crypto/ccp/ccp-dev.h > @@ -194,6 +194,7 @@ > #define CCP_AES_CTX_SB_COUNT 1 > > #define CCP_XTS_AES_KEY_SB_COUNT 1 > +#define CCP5_XTS_AES_KEY_SB_COUNT 2 > #define CCP_XTS_AES_CTX_SB_COUNT 1 > > #define CCP_DES3_KEY_SB_COUNT 1 > @@ -497,6 +498,7 @@ struct ccp_aes_op { > }; > > struct ccp_xts_aes_op { > + enum ccp_aes_type type; > enum ccp_aes_action action; > enum ccp_xts_aes_unit_size unit_size; > }; > diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c > index e23d138fc1ce..d1be07884a9b 100644 > --- a/drivers/crypto/ccp/ccp-ops.c > +++ b/drivers/crypto/ccp/ccp-ops.c > @@ -1038,6 +1038,8 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, > struct ccp_op op; > unsigned int unit_size, dm_offset; > bool in_place = false; > + unsigned int sb_count = 0; > + enum ccp_aes_type aestype; > int ret; > > switch (xts->unit_size) { > @@ -1056,12 +1058,15 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, > case CCP_XTS_AES_UNIT_SIZE_4096: > unit_size = 4096; > break; > - > default: > return -EINVAL; > } > > - if (xts->key_len != AES_KEYSIZE_128) > + if (xts->key_len == AES_KEYSIZE_128) > + aestype = CCP_AES_TYPE_128; > + else if (xts->key_len == AES_KEYSIZE_256) > + aestype = CCP_AES_TYPE_256; > + else > return -EINVAL; > > if (!xts->final && (xts->src_len & (AES_BLOCK_SIZE - 1))) > @@ -1084,22 +1089,50 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, > op.sb_ctx = cmd_q->sb_ctx; > op.init = 1; > op.u.xts.action = xts->action; > + op.u.xts.type = aestype; > op.u.xts.unit_size = xts->unit_size; > > - /* All supported key sizes fit in a single (32-byte) SB entry > - * and must be in little endian format. Use the 256-bit byte > - * swap passthru option to convert from big endian to little > - * endian. > + /* A version 3 device only supports 128-bit keys, which fits into a > + * single SB entry. A version 5 device uses a 512-bit vector, so two > + * SB entries. > */ > + if (cmd_q->ccp->vdata->version == CCP_VERSION(3, 0)) > + sb_count = CCP_XTS_AES_KEY_SB_COUNT; > + else > + sb_count = CCP5_XTS_AES_KEY_SB_COUNT; > ret = ccp_init_dm_workarea(&key, cmd_q, > - CCP_XTS_AES_KEY_SB_COUNT * CCP_SB_BYTES, > + sb_count * CCP_SB_BYTES, > DMA_TO_DEVICE); > if (ret) > return ret; > > - dm_offset = CCP_SB_BYTES - AES_KEYSIZE_128; > - ccp_set_dm_area(&key, dm_offset, xts->key, 0, xts->key_len); > - ccp_set_dm_area(&key, 0, xts->key, dm_offset, xts->key_len); > + if (cmd_q->ccp->vdata->version == CCP_VERSION(3, 0)) { > + /* All supported key sizes * must be in little endian format. > + * Use the 256-bit byte swap passthru option to convert from > + * big endian to little endian. > + */ > + dm_offset = CCP_SB_BYTES - AES_KEYSIZE_128; > + ccp_set_dm_area(&key, dm_offset, xts->key, 0, xts->key_len); > + ccp_set_dm_area(&key, 0, xts->key, xts->key_len, xts->key_len); > + } else { > + /* The AES key is at the little end and the tweak key is > + * at the big end. Since the byteswap operation is only > + * 256-bit, load the buffer according to the way things > + * will end up. > + */ > + unsigned int pad; > + > + op.sb_key = cmd_q->ccp->vdata->perform->sballoc(cmd_q, > + sb_count); > + if (!op.sb_key) > + return -EIO; > + > + dm_offset = CCP_SB_BYTES; > + pad = dm_offset - xts->key_len; > + ccp_set_dm_area(&key, pad, xts->key, 0, xts->key_len); > + ccp_set_dm_area(&key, dm_offset + pad, xts->key, xts->key_len, > + xts->key_len); > + } > ret = ccp_copy_to_sb(cmd_q, &key, op.jobid, op.sb_key, > CCP_PASSTHRU_BYTESWAP_256BIT); > if (ret) { > @@ -1179,7 +1212,6 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, > e_dst: > if (!in_place) > ccp_free_data(&dst, cmd_q); > - > e_src: > ccp_free_data(&src, cmd_q); > > @@ -1188,6 +1220,8 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, > > e_key: > ccp_dm_free(&key); > + if (cmd_q->ccp->vdata->version > CCP_VERSION(3, 0)) > + cmd_q->ccp->vdata->perform->sbfree(cmd_q, op.sb_key, sb_count); > > return ret; > } >
Am Montag, 17. Juli 2017, 22:08:27 CEST schrieb Gary R Hook: Hi Gary, > Version 5 CCPs have differing requirements for XTS-AES: key components > are stored in a 512-bit vector. The context must be little-endian > justified. AES-256 is supported now, so propagate the cipher size to > the command descriptor. > > Signed-off-by: Gary R Hook <gary.hook@amd.com> > --- > drivers/crypto/ccp/ccp-crypto-aes-xts.c | 79 > ++++++++++++++++--------------- drivers/crypto/ccp/ccp-dev-v5.c | > 2 + > drivers/crypto/ccp/ccp-dev.h | 2 + > drivers/crypto/ccp/ccp-ops.c | 56 ++++++++++++++++++---- > 4 files changed, 89 insertions(+), 50 deletions(-) > > diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c > b/drivers/crypto/ccp/ccp-crypto-aes-xts.c index 58a4244b4752..8d248b198e22 > 100644 > --- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c > +++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c > @@ -1,7 +1,7 @@ > /* > * AMD Cryptographic Coprocessor (CCP) AES XTS crypto API support > * > - * Copyright (C) 2013 Advanced Micro Devices, Inc. > + * Copyright (C) 2013,2017 Advanced Micro Devices, Inc. > * > * Author: Tom Lendacky <thomas.lendacky@amd.com> > * > @@ -37,46 +37,26 @@ struct ccp_unit_size_map { > u32 value; > }; > > -static struct ccp_unit_size_map unit_size_map[] = { > +static struct ccp_unit_size_map xts_unit_sizes[] = { > { > - .size = 4096, > - .value = CCP_XTS_AES_UNIT_SIZE_4096, > - }, > - { > - .size = 2048, > - .value = CCP_XTS_AES_UNIT_SIZE_2048, > - }, > - { > - .size = 1024, > - .value = CCP_XTS_AES_UNIT_SIZE_1024, > + .size = 16, > + .value = CCP_XTS_AES_UNIT_SIZE_16, > }, > { > - .size = 512, > + .size = 512, > .value = CCP_XTS_AES_UNIT_SIZE_512, > }, > { > - .size = 256, > - .value = CCP_XTS_AES_UNIT_SIZE__LAST, > - }, > - { > - .size = 128, > - .value = CCP_XTS_AES_UNIT_SIZE__LAST, > - }, > - { > - .size = 64, > - .value = CCP_XTS_AES_UNIT_SIZE__LAST, > - }, > - { > - .size = 32, > - .value = CCP_XTS_AES_UNIT_SIZE__LAST, > + .size = 1024, > + .value = CCP_XTS_AES_UNIT_SIZE_1024, > }, > { > - .size = 16, > - .value = CCP_XTS_AES_UNIT_SIZE_16, > + .size = 2048, > + .value = CCP_XTS_AES_UNIT_SIZE_2048, > }, > { > - .size = 1, > - .value = CCP_XTS_AES_UNIT_SIZE__LAST, > + .size = 4096, > + .value = CCP_XTS_AES_UNIT_SIZE_4096, > }, > }; > > @@ -97,14 +77,20 @@ static int ccp_aes_xts_setkey(struct crypto_ablkcipher > *tfm, const u8 *key, unsigned int key_len) > { > struct ccp_ctx *ctx = crypto_tfm_ctx(crypto_ablkcipher_tfm(tfm)); > + unsigned int ccpversion = ccp_version(); > > /* Only support 128-bit AES key with a 128-bit Tweak key, > * otherwise use the fallback > */ > + Can you please add xts_check_key here? > switch (key_len) { > case AES_KEYSIZE_128 * 2: > memcpy(ctx->u.aes.key, key, key_len); > break; > + case AES_KEYSIZE_256 * 2: > + if (ccpversion > CCP_VERSION(3, 0)) > + memcpy(ctx->u.aes.key, key, key_len); > + break; > } > ctx->u.aes.key_len = key_len / 2; > sg_init_one(&ctx->u.aes.key_sg, ctx->u.aes.key, key_len); > @@ -117,7 +103,10 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request > *req, { > struct ccp_ctx *ctx = crypto_tfm_ctx(req->base.tfm); > struct ccp_aes_req_ctx *rctx = ablkcipher_request_ctx(req); > + unsigned int ccpversion = ccp_version(); > + unsigned int fallback = 0; > unsigned int unit; > + u32 block_size = 0; > u32 unit_size; > int ret; > > @@ -131,17 +120,29 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request > *req, return -EINVAL; > > unit_size = CCP_XTS_AES_UNIT_SIZE__LAST; > - if (req->nbytes <= unit_size_map[0].size) { > - for (unit = 0; unit < ARRAY_SIZE(unit_size_map); unit++) { > - if (!(req->nbytes & (unit_size_map[unit].size - 1))) { > - unit_size = unit_size_map[unit].value; > - break; > - } > + for (unit = ARRAY_SIZE(xts_unit_sizes); unit-- > 0; ) { > + if (!(req->nbytes & (xts_unit_sizes[unit].size - 1))) { > + unit_size = unit; > + block_size = xts_unit_sizes[unit].size; > + break; > } > } > > - if ((unit_size == CCP_XTS_AES_UNIT_SIZE__LAST) || > - (ctx->u.aes.key_len != AES_KEYSIZE_128)) { > + /* The CCP has restrictions on block sizes. Also, a version 3 device > + * only supports AES-128 operations; version 5 CCPs support both > + * AES-128 and -256 operations. > + */ > + if (unit_size == CCP_XTS_AES_UNIT_SIZE__LAST) > + fallback = 1; > + if ((ccpversion < CCP_VERSION(5, 0)) && > + (ctx->u.aes.key_len != AES_KEYSIZE_128)) > + fallback = 1; > + if ((ctx->u.aes.key_len != AES_KEYSIZE_128) && > + (ctx->u.aes.key_len != AES_KEYSIZE_256)) > + fallback = 1; > + if (req->nbytes != block_size) > + fallback = 1; > + if (fallback) { > SKCIPHER_REQUEST_ON_STACK(subreq, ctx->u.aes.tfm_skcipher); > > /* Use the fallback to process the request for any > diff --git a/drivers/crypto/ccp/ccp-dev-v5.c > b/drivers/crypto/ccp/ccp-dev-v5.c index b3526336d608..11febe2bd07c 100644 > --- a/drivers/crypto/ccp/ccp-dev-v5.c > +++ b/drivers/crypto/ccp/ccp-dev-v5.c > @@ -144,6 +144,7 @@ union ccp_function { > #define CCP_AES_ENCRYPT(p) ((p)->aes.encrypt) > #define CCP_AES_MODE(p) ((p)->aes.mode) > #define CCP_AES_TYPE(p) ((p)->aes.type) > +#define CCP_XTS_TYPE(p) ((p)->aes_xts.type) > #define CCP_XTS_SIZE(p) ((p)->aes_xts.size) > #define CCP_XTS_ENCRYPT(p) ((p)->aes_xts.encrypt) > #define CCP_DES3_SIZE(p) ((p)->des3.size) > @@ -344,6 +345,7 @@ static int ccp5_perform_xts_aes(struct ccp_op *op) > CCP5_CMD_PROT(&desc) = 0; > > function.raw = 0; > + CCP_XTS_TYPE(&function) = op->u.xts.type; > CCP_XTS_ENCRYPT(&function) = op->u.xts.action; > CCP_XTS_SIZE(&function) = op->u.xts.unit_size; > CCP5_CMD_FUNCTION(&desc) = function.raw; > diff --git a/drivers/crypto/ccp/ccp-dev.h b/drivers/crypto/ccp/ccp-dev.h > index 9320931d89da..3d51180199ac 100644 > --- a/drivers/crypto/ccp/ccp-dev.h > +++ b/drivers/crypto/ccp/ccp-dev.h > @@ -194,6 +194,7 @@ > #define CCP_AES_CTX_SB_COUNT 1 > > #define CCP_XTS_AES_KEY_SB_COUNT 1 > +#define CCP5_XTS_AES_KEY_SB_COUNT 2 > #define CCP_XTS_AES_CTX_SB_COUNT 1 > > #define CCP_DES3_KEY_SB_COUNT 1 > @@ -497,6 +498,7 @@ struct ccp_aes_op { > }; > > struct ccp_xts_aes_op { > + enum ccp_aes_type type; > enum ccp_aes_action action; > enum ccp_xts_aes_unit_size unit_size; > }; > diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c > index e23d138fc1ce..d1be07884a9b 100644 > --- a/drivers/crypto/ccp/ccp-ops.c > +++ b/drivers/crypto/ccp/ccp-ops.c > @@ -1038,6 +1038,8 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue > *cmd_q, struct ccp_op op; > unsigned int unit_size, dm_offset; > bool in_place = false; > + unsigned int sb_count = 0; > + enum ccp_aes_type aestype; > int ret; > > switch (xts->unit_size) { > @@ -1056,12 +1058,15 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue > *cmd_q, case CCP_XTS_AES_UNIT_SIZE_4096: > unit_size = 4096; > break; > - > default: > return -EINVAL; > } > > - if (xts->key_len != AES_KEYSIZE_128) > + if (xts->key_len == AES_KEYSIZE_128) > + aestype = CCP_AES_TYPE_128; > + else if (xts->key_len == AES_KEYSIZE_256) > + aestype = CCP_AES_TYPE_256; > + else > return -EINVAL; > > if (!xts->final && (xts->src_len & (AES_BLOCK_SIZE - 1))) > @@ -1084,22 +1089,50 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue > *cmd_q, op.sb_ctx = cmd_q->sb_ctx; > op.init = 1; > op.u.xts.action = xts->action; > + op.u.xts.type = aestype; > op.u.xts.unit_size = xts->unit_size; > > - /* All supported key sizes fit in a single (32-byte) SB entry > - * and must be in little endian format. Use the 256-bit byte > - * swap passthru option to convert from big endian to little > - * endian. > + /* A version 3 device only supports 128-bit keys, which fits into a > + * single SB entry. A version 5 device uses a 512-bit vector, so two > + * SB entries. > */ > + if (cmd_q->ccp->vdata->version == CCP_VERSION(3, 0)) > + sb_count = CCP_XTS_AES_KEY_SB_COUNT; > + else > + sb_count = CCP5_XTS_AES_KEY_SB_COUNT; > ret = ccp_init_dm_workarea(&key, cmd_q, > - CCP_XTS_AES_KEY_SB_COUNT * CCP_SB_BYTES, > + sb_count * CCP_SB_BYTES, > DMA_TO_DEVICE); > if (ret) > return ret; > > - dm_offset = CCP_SB_BYTES - AES_KEYSIZE_128; > - ccp_set_dm_area(&key, dm_offset, xts->key, 0, xts->key_len); > - ccp_set_dm_area(&key, 0, xts->key, dm_offset, xts->key_len); > + if (cmd_q->ccp->vdata->version == CCP_VERSION(3, 0)) { > + /* All supported key sizes * must be in little endian format. > + * Use the 256-bit byte swap passthru option to convert from > + * big endian to little endian. > + */ > + dm_offset = CCP_SB_BYTES - AES_KEYSIZE_128; > + ccp_set_dm_area(&key, dm_offset, xts->key, 0, xts->key_len); > + ccp_set_dm_area(&key, 0, xts->key, xts->key_len, xts->key_len); > + } else { > + /* The AES key is at the little end and the tweak key is > + * at the big end. Since the byteswap operation is only > + * 256-bit, load the buffer according to the way things > + * will end up. > + */ > + unsigned int pad; > + > + op.sb_key = cmd_q->ccp->vdata->perform->sballoc(cmd_q, > + sb_count); > + if (!op.sb_key) > + return -EIO; > + > + dm_offset = CCP_SB_BYTES; > + pad = dm_offset - xts->key_len; > + ccp_set_dm_area(&key, pad, xts->key, 0, xts->key_len); > + ccp_set_dm_area(&key, dm_offset + pad, xts->key, xts->key_len, > + xts->key_len); > + } > ret = ccp_copy_to_sb(cmd_q, &key, op.jobid, op.sb_key, > CCP_PASSTHRU_BYTESWAP_256BIT); > if (ret) { > @@ -1179,7 +1212,6 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue > *cmd_q, e_dst: > if (!in_place) > ccp_free_data(&dst, cmd_q); > - > e_src: > ccp_free_data(&src, cmd_q); > > @@ -1188,6 +1220,8 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue > *cmd_q, > > e_key: > ccp_dm_free(&key); > + if (cmd_q->ccp->vdata->version > CCP_VERSION(3, 0)) > + cmd_q->ccp->vdata->perform->sbfree(cmd_q, op.sb_key, sb_count); > > return ret; > } Ciao Stephan
On 07/18/2017 01:28 AM, Stephan Müller wrote: > Am Montag, 17. Juli 2017, 22:08:27 CEST schrieb Gary R Hook: > > Hi Gary, > >> Version 5 CCPs have differing requirements for XTS-AES: key components >> are stored in a 512-bit vector. The context must be little-endian >> justified. AES-256 is supported now, so propagate the cipher size to >> the command descriptor. >> >> Signed-off-by: Gary R Hook <gary.hook@amd.com> >> --- >> drivers/crypto/ccp/ccp-crypto-aes-xts.c | 79 ...<snip>... >> @@ -97,14 +77,20 @@ static int ccp_aes_xts_setkey(struct crypto_ablkcipher >> *tfm, const u8 *key, unsigned int key_len) >> { >> struct ccp_ctx *ctx = crypto_tfm_ctx(crypto_ablkcipher_tfm(tfm)); >> + unsigned int ccpversion = ccp_version(); >> >> /* Only support 128-bit AES key with a 128-bit Tweak key, >> * otherwise use the fallback >> */ >> + > > Can you please add xts_check_key here? Certainly!
On 07/17/2017 04:48 PM, Lendacky, Thomas wrote: > On 7/17/2017 3:08 PM, Gary R Hook wrote: >> Version 5 CCPs have differing requirements for XTS-AES: key components >> are stored in a 512-bit vector. The context must be little-endian >> justified. AES-256 is supported now, so propagate the cipher size to >> the command descriptor. >> >> Signed-off-by: Gary R Hook <gary.hook@amd.com> Look for a version 2 >> --- >> drivers/crypto/ccp/ccp-crypto-aes-xts.c | 79 ++++++++++++++++--------------- >> drivers/crypto/ccp/ccp-dev-v5.c | 2 + >> drivers/crypto/ccp/ccp-dev.h | 2 + >> drivers/crypto/ccp/ccp-ops.c | 56 ++++++++++++++++++---- >> 4 files changed, 89 insertions(+), 50 deletions(-) >> >> diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c b/drivers/crypto/ccp/ccp-crypto-aes-xts.c >> index 58a4244b4752..8d248b198e22 100644 >> --- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c >> +++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c >> @@ -1,7 +1,7 @@ >> /* >> * AMD Cryptographic Coprocessor (CCP) AES XTS crypto API support >> * >> - * Copyright (C) 2013 Advanced Micro Devices, Inc. >> + * Copyright (C) 2013,2017 Advanced Micro Devices, Inc. >> * >> * Author: Tom Lendacky <thomas.lendacky@amd.com> >> * >> @@ -37,46 +37,26 @@ struct ccp_unit_size_map { >> u32 value; >> }; >> >> -static struct ccp_unit_size_map unit_size_map[] = { >> +static struct ccp_unit_size_map xts_unit_sizes[] = { >> { >> - .size = 4096, >> - .value = CCP_XTS_AES_UNIT_SIZE_4096, >> - }, >> - { >> - .size = 2048, >> - .value = CCP_XTS_AES_UNIT_SIZE_2048, >> - }, >> - { >> - .size = 1024, >> - .value = CCP_XTS_AES_UNIT_SIZE_1024, >> + .size = 16, >> + .value = CCP_XTS_AES_UNIT_SIZE_16, >> }, >> { >> - .size = 512, >> + .size = 512, >> .value = CCP_XTS_AES_UNIT_SIZE_512, >> }, >> { >> - .size = 256, >> - .value = CCP_XTS_AES_UNIT_SIZE__LAST, >> - }, >> - { >> - .size = 128, >> - .value = CCP_XTS_AES_UNIT_SIZE__LAST, >> - }, >> - { >> - .size = 64, >> - .value = CCP_XTS_AES_UNIT_SIZE__LAST, >> - }, >> - { >> - .size = 32, >> - .value = CCP_XTS_AES_UNIT_SIZE__LAST, >> + .size = 1024, >> + .value = CCP_XTS_AES_UNIT_SIZE_1024, >> }, >> { >> - .size = 16, >> - .value = CCP_XTS_AES_UNIT_SIZE_16, >> + .size = 2048, >> + .value = CCP_XTS_AES_UNIT_SIZE_2048, >> }, >> { >> - .size = 1, >> - .value = CCP_XTS_AES_UNIT_SIZE__LAST, >> + .size = 4096, >> + .value = CCP_XTS_AES_UNIT_SIZE_4096, >> }, >> }; > > Because of the way the unit size check is performed, you can't delete > the intermediate size checks. Those must remain so that unit sizes > that aren't supported by the CCP are sent to the fallback mechanism. Given the limitations of the CCP (w.r.t. XTS-AES) I thought it more clear to look for only those unit-sizes that are supported, and to use the enumerated value as the index. > Also, re-arranging the order should be a separate patch if that doesn't > really fix anything. Yes, agreed. >> >> @@ -97,14 +77,20 @@ static int ccp_aes_xts_setkey(struct crypto_ablkcipher *tfm, const u8 *key, >> unsigned int key_len) >> { >> struct ccp_ctx *ctx = crypto_tfm_ctx(crypto_ablkcipher_tfm(tfm)); >> + unsigned int ccpversion = ccp_version(); >> >> /* Only support 128-bit AES key with a 128-bit Tweak key, >> * otherwise use the fallback >> */ >> + > > Remove the addition of the blank line and update the above comment to > indicate the new supported key size added below. Yes. > >> switch (key_len) { >> case AES_KEYSIZE_128 * 2: >> memcpy(ctx->u.aes.key, key, key_len); >> break; >> + case AES_KEYSIZE_256 * 2: >> + if (ccpversion > CCP_VERSION(3, 0)) >> + memcpy(ctx->u.aes.key, key, key_len); >> + break; > > Isn't u.aes.key defined with a maximum buffer size of AES_MAX_KEY_SIZE > (which is 32)? I think this will cause a buffer overrun on memcpy. Yes, the structure member needs to be made larger. > >> } >> ctx->u.aes.key_len = key_len / 2; >> sg_init_one(&ctx->u.aes.key_sg, ctx->u.aes.key, key_len); >> @@ -117,7 +103,10 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req, >> { >> struct ccp_ctx *ctx = crypto_tfm_ctx(req->base.tfm); >> struct ccp_aes_req_ctx *rctx = ablkcipher_request_ctx(req); >> + unsigned int ccpversion = ccp_version(); >> + unsigned int fallback = 0; >> unsigned int unit; >> + u32 block_size = 0; >> u32 unit_size; >> int ret; >> >> @@ -131,17 +120,29 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req, >> return -EINVAL; >> >> unit_size = CCP_XTS_AES_UNIT_SIZE__LAST; >> - if (req->nbytes <= unit_size_map[0].size) { > > This check can't be deleted. It was added specifically to catch cases > where the size was greater than 4096. My version two approach will address this. > >> - for (unit = 0; unit < ARRAY_SIZE(unit_size_map); unit++) { >> - if (!(req->nbytes & (unit_size_map[unit].size - 1))) { >> - unit_size = unit_size_map[unit].value; >> - break; >> - } >> + for (unit = ARRAY_SIZE(xts_unit_sizes); unit-- > 0; ) { >> + if (!(req->nbytes & (xts_unit_sizes[unit].size - 1))) { >> + unit_size = unit; >> + block_size = xts_unit_sizes[unit].size; >> + break; >> } >> } >> >> - if ((unit_size == CCP_XTS_AES_UNIT_SIZE__LAST) || >> - (ctx->u.aes.key_len != AES_KEYSIZE_128)) { >> + /* The CCP has restrictions on block sizes. Also, a version 3 device >> + * only supports AES-128 operations; version 5 CCPs support both >> + * AES-128 and -256 operations. >> + */ >> + if (unit_size == CCP_XTS_AES_UNIT_SIZE__LAST) >> + fallback = 1; >> + if ((ccpversion < CCP_VERSION(5, 0)) && >> + (ctx->u.aes.key_len != AES_KEYSIZE_128)) >> + fallback = 1; >> + if ((ctx->u.aes.key_len != AES_KEYSIZE_128) && >> + (ctx->u.aes.key_len != AES_KEYSIZE_256)) >> + fallback = 1; >> + if (req->nbytes != block_size) >> + fallback = 1; > > I don't believe this is correct. Everything should fall out properly > for fallback based on the unit size and key size checks above. The changes for a version 5 device require additional checks. Also the next patch version will correct/clarify the logic by which I arrive at the fallback choice.
On 07/17/2017 04:48 PM, Lendacky, Thomas wrote: > On 7/17/2017 3:08 PM, Gary R Hook wrote: >> Version 5 CCPs have differing requirements for XTS-AES: key components >> are stored in a 512-bit vector. The context must be little-endian >> justified. AES-256 is supported now, so propagate the cipher size to >> the command descriptor. >> >> Signed-off-by: Gary R Hook <gary.hook@amd.com> Herbert, I see that this patch (and others that add function or fix bugs) has been added to cryptodev. I've not seen anything CCP-related pushed to Linux in a while, however. Is there something more I need to do to for these recent patches, to get them promoted to the mainline kernel..? Thanks much! Gary >> --- >> drivers/crypto/ccp/ccp-crypto-aes-xts.c | 79 ++++++++++++++++--------------- >> drivers/crypto/ccp/ccp-dev-v5.c | 2 + >> drivers/crypto/ccp/ccp-dev.h | 2 + >> drivers/crypto/ccp/ccp-ops.c | 56 ++++++++++++++++++---- >> 4 files changed, 89 insertions(+), 50 deletions(-) >> >> diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c b/drivers/crypto/ccp/ccp-crypto-aes-xts.c >> index 58a4244b4752..8d248b198e22 100644 >> --- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c >> +++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c >> @@ -1,7 +1,7 @@ >> /* >> * AMD Cryptographic Coprocessor (CCP) AES XTS crypto API support >> * >> - * Copyright (C) 2013 Advanced Micro Devices, Inc. >> + * Copyright (C) 2013,2017 Advanced Micro Devices, Inc. >> * >> * Author: Tom Lendacky <thomas.lendacky@amd.com> >> * >> @@ -37,46 +37,26 @@ struct ccp_unit_size_map { >> u32 value; >> }; >> >> -static struct ccp_unit_size_map unit_size_map[] = { >> +static struct ccp_unit_size_map xts_unit_sizes[] = { >> { >> - .size = 4096, >> - .value = CCP_XTS_AES_UNIT_SIZE_4096, >> - }, >> - { >> - .size = 2048, >> - .value = CCP_XTS_AES_UNIT_SIZE_2048, >> - }, >> - { >> - .size = 1024, >> - .value = CCP_XTS_AES_UNIT_SIZE_1024, >> + .size = 16, >> + .value = CCP_XTS_AES_UNIT_SIZE_16, >> }, >> { >> - .size = 512, >> + .size = 512, >> .value = CCP_XTS_AES_UNIT_SIZE_512, >> }, >> { >> - .size = 256, >> - .value = CCP_XTS_AES_UNIT_SIZE__LAST, >> - }, >> - { >> - .size = 128, >> - .value = CCP_XTS_AES_UNIT_SIZE__LAST, >> - }, >> - { >> - .size = 64, >> - .value = CCP_XTS_AES_UNIT_SIZE__LAST, >> - }, >> - { >> - .size = 32, >> - .value = CCP_XTS_AES_UNIT_SIZE__LAST, >> + .size = 1024, >> + .value = CCP_XTS_AES_UNIT_SIZE_1024, >> }, >> { >> - .size = 16, >> - .value = CCP_XTS_AES_UNIT_SIZE_16, >> + .size = 2048, >> + .value = CCP_XTS_AES_UNIT_SIZE_2048, >> }, >> { >> - .size = 1, >> - .value = CCP_XTS_AES_UNIT_SIZE__LAST, >> + .size = 4096, >> + .value = CCP_XTS_AES_UNIT_SIZE_4096, >> }, >> }; > > Because of the way the unit size check is performed, you can't delete > the intermediate size checks. Those must remain so that unit sizes > that aren't supported by the CCP are sent to the fallback mechanism. > > Also, re-arranging the order should be a separate patch if that doesn't > really fix anything. > >> >> @@ -97,14 +77,20 @@ static int ccp_aes_xts_setkey(struct crypto_ablkcipher *tfm, const u8 *key, >> unsigned int key_len) >> { >> struct ccp_ctx *ctx = crypto_tfm_ctx(crypto_ablkcipher_tfm(tfm)); >> + unsigned int ccpversion = ccp_version(); >> >> /* Only support 128-bit AES key with a 128-bit Tweak key, >> * otherwise use the fallback >> */ >> + > > Remove the addition of the blank line and update the above comment to > indicate the new supported key size added below. > >> switch (key_len) { >> case AES_KEYSIZE_128 * 2: >> memcpy(ctx->u.aes.key, key, key_len); >> break; >> + case AES_KEYSIZE_256 * 2: >> + if (ccpversion > CCP_VERSION(3, 0)) >> + memcpy(ctx->u.aes.key, key, key_len); >> + break; > > Isn't u.aes.key defined with a maximum buffer size of AES_MAX_KEY_SIZE > (which is 32)? I think this will cause a buffer overrun on memcpy. > >> } >> ctx->u.aes.key_len = key_len / 2; >> sg_init_one(&ctx->u.aes.key_sg, ctx->u.aes.key, key_len); >> @@ -117,7 +103,10 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req, >> { >> struct ccp_ctx *ctx = crypto_tfm_ctx(req->base.tfm); >> struct ccp_aes_req_ctx *rctx = ablkcipher_request_ctx(req); >> + unsigned int ccpversion = ccp_version(); >> + unsigned int fallback = 0; >> unsigned int unit; >> + u32 block_size = 0; >> u32 unit_size; >> int ret; >> >> @@ -131,17 +120,29 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req, >> return -EINVAL; >> >> unit_size = CCP_XTS_AES_UNIT_SIZE__LAST; >> - if (req->nbytes <= unit_size_map[0].size) { > > This check can't be deleted. It was added specifically to catch cases > where the size was greater than 4096. > >> - for (unit = 0; unit < ARRAY_SIZE(unit_size_map); unit++) { >> - if (!(req->nbytes & (unit_size_map[unit].size - 1))) { >> - unit_size = unit_size_map[unit].value; >> - break; >> - } >> + for (unit = ARRAY_SIZE(xts_unit_sizes); unit-- > 0; ) { >> + if (!(req->nbytes & (xts_unit_sizes[unit].size - 1))) { >> + unit_size = unit; >> + block_size = xts_unit_sizes[unit].size; >> + break; >> } >> } >> >> - if ((unit_size == CCP_XTS_AES_UNIT_SIZE__LAST) || >> - (ctx->u.aes.key_len != AES_KEYSIZE_128)) { >> + /* The CCP has restrictions on block sizes. Also, a version 3 device >> + * only supports AES-128 operations; version 5 CCPs support both >> + * AES-128 and -256 operations. >> + */ >> + if (unit_size == CCP_XTS_AES_UNIT_SIZE__LAST) >> + fallback = 1; >> + if ((ccpversion < CCP_VERSION(5, 0)) && >> + (ctx->u.aes.key_len != AES_KEYSIZE_128)) >> + fallback = 1; >> + if ((ctx->u.aes.key_len != AES_KEYSIZE_128) && >> + (ctx->u.aes.key_len != AES_KEYSIZE_256)) >> + fallback = 1; >> + if (req->nbytes != block_size) >> + fallback = 1; > > I don't believe this is correct. Everything should fall out properly > for fallback based on the unit size and key size checks above. > > Thanks, > Tom > >> + if (fallback) { >> SKCIPHER_REQUEST_ON_STACK(subreq, ctx->u.aes.tfm_skcipher); >> >> /* Use the fallback to process the request for any >> diff --git a/drivers/crypto/ccp/ccp-dev-v5.c b/drivers/crypto/ccp/ccp-dev-v5.c >> index b3526336d608..11febe2bd07c 100644 >> --- a/drivers/crypto/ccp/ccp-dev-v5.c >> +++ b/drivers/crypto/ccp/ccp-dev-v5.c >> @@ -144,6 +144,7 @@ union ccp_function { >> #define CCP_AES_ENCRYPT(p) ((p)->aes.encrypt) >> #define CCP_AES_MODE(p) ((p)->aes.mode) >> #define CCP_AES_TYPE(p) ((p)->aes.type) >> +#define CCP_XTS_TYPE(p) ((p)->aes_xts.type) >> #define CCP_XTS_SIZE(p) ((p)->aes_xts.size) >> #define CCP_XTS_ENCRYPT(p) ((p)->aes_xts.encrypt) >> #define CCP_DES3_SIZE(p) ((p)->des3.size) >> @@ -344,6 +345,7 @@ static int ccp5_perform_xts_aes(struct ccp_op *op) >> CCP5_CMD_PROT(&desc) = 0; >> >> function.raw = 0; >> + CCP_XTS_TYPE(&function) = op->u.xts.type; >> CCP_XTS_ENCRYPT(&function) = op->u.xts.action; >> CCP_XTS_SIZE(&function) = op->u.xts.unit_size; >> CCP5_CMD_FUNCTION(&desc) = function.raw; >> diff --git a/drivers/crypto/ccp/ccp-dev.h b/drivers/crypto/ccp/ccp-dev.h >> index 9320931d89da..3d51180199ac 100644 >> --- a/drivers/crypto/ccp/ccp-dev.h >> +++ b/drivers/crypto/ccp/ccp-dev.h >> @@ -194,6 +194,7 @@ >> #define CCP_AES_CTX_SB_COUNT 1 >> >> #define CCP_XTS_AES_KEY_SB_COUNT 1 >> +#define CCP5_XTS_AES_KEY_SB_COUNT 2 >> #define CCP_XTS_AES_CTX_SB_COUNT 1 >> >> #define CCP_DES3_KEY_SB_COUNT 1 >> @@ -497,6 +498,7 @@ struct ccp_aes_op { >> }; >> >> struct ccp_xts_aes_op { >> + enum ccp_aes_type type; >> enum ccp_aes_action action; >> enum ccp_xts_aes_unit_size unit_size; >> }; >> diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c >> index e23d138fc1ce..d1be07884a9b 100644 >> --- a/drivers/crypto/ccp/ccp-ops.c >> +++ b/drivers/crypto/ccp/ccp-ops.c >> @@ -1038,6 +1038,8 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, >> struct ccp_op op; >> unsigned int unit_size, dm_offset; >> bool in_place = false; >> + unsigned int sb_count = 0; >> + enum ccp_aes_type aestype; >> int ret; >> >> switch (xts->unit_size) { >> @@ -1056,12 +1058,15 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, >> case CCP_XTS_AES_UNIT_SIZE_4096: >> unit_size = 4096; >> break; >> - >> default: >> return -EINVAL; >> } >> >> - if (xts->key_len != AES_KEYSIZE_128) >> + if (xts->key_len == AES_KEYSIZE_128) >> + aestype = CCP_AES_TYPE_128; >> + else if (xts->key_len == AES_KEYSIZE_256) >> + aestype = CCP_AES_TYPE_256; >> + else >> return -EINVAL; >> >> if (!xts->final && (xts->src_len & (AES_BLOCK_SIZE - 1))) >> @@ -1084,22 +1089,50 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, >> op.sb_ctx = cmd_q->sb_ctx; >> op.init = 1; >> op.u.xts.action = xts->action; >> + op.u.xts.type = aestype; >> op.u.xts.unit_size = xts->unit_size; >> >> - /* All supported key sizes fit in a single (32-byte) SB entry >> - * and must be in little endian format. Use the 256-bit byte >> - * swap passthru option to convert from big endian to little >> - * endian. >> + /* A version 3 device only supports 128-bit keys, which fits into a >> + * single SB entry. A version 5 device uses a 512-bit vector, so two >> + * SB entries. >> */ >> + if (cmd_q->ccp->vdata->version == CCP_VERSION(3, 0)) >> + sb_count = CCP_XTS_AES_KEY_SB_COUNT; >> + else >> + sb_count = CCP5_XTS_AES_KEY_SB_COUNT; >> ret = ccp_init_dm_workarea(&key, cmd_q, >> - CCP_XTS_AES_KEY_SB_COUNT * CCP_SB_BYTES, >> + sb_count * CCP_SB_BYTES, >> DMA_TO_DEVICE); >> if (ret) >> return ret; >> >> - dm_offset = CCP_SB_BYTES - AES_KEYSIZE_128; >> - ccp_set_dm_area(&key, dm_offset, xts->key, 0, xts->key_len); >> - ccp_set_dm_area(&key, 0, xts->key, dm_offset, xts->key_len); >> + if (cmd_q->ccp->vdata->version == CCP_VERSION(3, 0)) { >> + /* All supported key sizes * must be in little endian format. >> + * Use the 256-bit byte swap passthru option to convert from >> + * big endian to little endian. >> + */ >> + dm_offset = CCP_SB_BYTES - AES_KEYSIZE_128; >> + ccp_set_dm_area(&key, dm_offset, xts->key, 0, xts->key_len); >> + ccp_set_dm_area(&key, 0, xts->key, xts->key_len, xts->key_len); >> + } else { >> + /* The AES key is at the little end and the tweak key is >> + * at the big end. Since the byteswap operation is only >> + * 256-bit, load the buffer according to the way things >> + * will end up. >> + */ >> + unsigned int pad; >> + >> + op.sb_key = cmd_q->ccp->vdata->perform->sballoc(cmd_q, >> + sb_count); >> + if (!op.sb_key) >> + return -EIO; >> + >> + dm_offset = CCP_SB_BYTES; >> + pad = dm_offset - xts->key_len; >> + ccp_set_dm_area(&key, pad, xts->key, 0, xts->key_len); >> + ccp_set_dm_area(&key, dm_offset + pad, xts->key, xts->key_len, >> + xts->key_len); >> + } >> ret = ccp_copy_to_sb(cmd_q, &key, op.jobid, op.sb_key, >> CCP_PASSTHRU_BYTESWAP_256BIT); >> if (ret) { >> @@ -1179,7 +1212,6 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, >> e_dst: >> if (!in_place) >> ccp_free_data(&dst, cmd_q); >> - >> e_src: >> ccp_free_data(&src, cmd_q); >> >> @@ -1188,6 +1220,8 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, >> >> e_key: >> ccp_dm_free(&key); >> + if (cmd_q->ccp->vdata->version > CCP_VERSION(3, 0)) >> + cmd_q->ccp->vdata->perform->sbfree(cmd_q, op.sb_key, sb_count); >> >> return ret; >> } >>
On 08/18/2017 11:19 AM, Gary R Hook wrote: > On 07/17/2017 04:48 PM, Lendacky, Thomas wrote: >> On 7/17/2017 3:08 PM, Gary R Hook wrote: >>> Version 5 CCPs have differing requirements for XTS-AES: key components >>> are stored in a 512-bit vector. The context must be little-endian >>> justified. AES-256 is supported now, so propagate the cipher size to >>> the command descriptor. >>> >>> Signed-off-by: Gary R Hook <gary.hook@amd.com> > > Herbert, > > I see that this patch (and others that add function or fix bugs) has > been added to > cryptodev. I've not seen anything CCP-related pushed to Linux in a > while, however. > > Is there something more I need to do to for these recent patches, to get > them promoted > to the mainline kernel..? Please ignore. I realized (too late) that the referenced items missed the merge window for 4.13. > > Thanks much! > Gary > > >>> --- >>> drivers/crypto/ccp/ccp-crypto-aes-xts.c | 79 ++++++++++++++++--------------- >>> drivers/crypto/ccp/ccp-dev-v5.c | 2 + >>> drivers/crypto/ccp/ccp-dev.h | 2 + >>> drivers/crypto/ccp/ccp-ops.c | 56 ++++++++++++++++++---- >>> 4 files changed, 89 insertions(+), 50 deletions(-) >>> >>> diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c b/drivers/crypto/ccp/ccp-crypto-aes-xts.c >>> index 58a4244b4752..8d248b198e22 100644 >>> --- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c >>> +++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c >>> @@ -1,7 +1,7 @@ >>> /* >>> * AMD Cryptographic Coprocessor (CCP) AES XTS crypto API support >>> * >>> - * Copyright (C) 2013 Advanced Micro Devices, Inc. >>> + * Copyright (C) 2013,2017 Advanced Micro Devices, Inc. >>> * >>> * Author: Tom Lendacky <thomas.lendacky@amd.com> >>> * >>> @@ -37,46 +37,26 @@ struct ccp_unit_size_map { >>> u32 value; >>> }; >>> >>> -static struct ccp_unit_size_map unit_size_map[] = { >>> +static struct ccp_unit_size_map xts_unit_sizes[] = { >>> { >>> - .size = 4096, >>> - .value = CCP_XTS_AES_UNIT_SIZE_4096, >>> - }, >>> - { >>> - .size = 2048, >>> - .value = CCP_XTS_AES_UNIT_SIZE_2048, >>> - }, >>> - { >>> - .size = 1024, >>> - .value = CCP_XTS_AES_UNIT_SIZE_1024, >>> + .size = 16, >>> + .value = CCP_XTS_AES_UNIT_SIZE_16, >>> }, >>> { >>> - .size = 512, >>> + .size = 512, >>> .value = CCP_XTS_AES_UNIT_SIZE_512, >>> }, >>> { >>> - .size = 256, >>> - .value = CCP_XTS_AES_UNIT_SIZE__LAST, >>> - }, >>> - { >>> - .size = 128, >>> - .value = CCP_XTS_AES_UNIT_SIZE__LAST, >>> - }, >>> - { >>> - .size = 64, >>> - .value = CCP_XTS_AES_UNIT_SIZE__LAST, >>> - }, >>> - { >>> - .size = 32, >>> - .value = CCP_XTS_AES_UNIT_SIZE__LAST, >>> + .size = 1024, >>> + .value = CCP_XTS_AES_UNIT_SIZE_1024, >>> }, >>> { >>> - .size = 16, >>> - .value = CCP_XTS_AES_UNIT_SIZE_16, >>> + .size = 2048, >>> + .value = CCP_XTS_AES_UNIT_SIZE_2048, >>> }, >>> { >>> - .size = 1, >>> - .value = CCP_XTS_AES_UNIT_SIZE__LAST, >>> + .size = 4096, >>> + .value = CCP_XTS_AES_UNIT_SIZE_4096, >>> }, >>> }; >> >> Because of the way the unit size check is performed, you can't delete >> the intermediate size checks. Those must remain so that unit sizes >> that aren't supported by the CCP are sent to the fallback mechanism. >> >> Also, re-arranging the order should be a separate patch if that doesn't >> really fix anything. >> >>> >>> @@ -97,14 +77,20 @@ static int ccp_aes_xts_setkey(struct crypto_ablkcipher *tfm, const u8 *key, >>> unsigned int key_len) >>> { >>> struct ccp_ctx *ctx = crypto_tfm_ctx(crypto_ablkcipher_tfm(tfm)); >>> + unsigned int ccpversion = ccp_version(); >>> >>> /* Only support 128-bit AES key with a 128-bit Tweak key, >>> * otherwise use the fallback >>> */ >>> + >> >> Remove the addition of the blank line and update the above comment to >> indicate the new supported key size added below. >> >>> switch (key_len) { >>> case AES_KEYSIZE_128 * 2: >>> memcpy(ctx->u.aes.key, key, key_len); >>> break; >>> + case AES_KEYSIZE_256 * 2: >>> + if (ccpversion > CCP_VERSION(3, 0)) >>> + memcpy(ctx->u.aes.key, key, key_len); >>> + break; >> >> Isn't u.aes.key defined with a maximum buffer size of AES_MAX_KEY_SIZE >> (which is 32)? I think this will cause a buffer overrun on memcpy. >> >>> } >>> ctx->u.aes.key_len = key_len / 2; >>> sg_init_one(&ctx->u.aes.key_sg, ctx->u.aes.key, key_len); >>> @@ -117,7 +103,10 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req, >>> { >>> struct ccp_ctx *ctx = crypto_tfm_ctx(req->base.tfm); >>> struct ccp_aes_req_ctx *rctx = ablkcipher_request_ctx(req); >>> + unsigned int ccpversion = ccp_version(); >>> + unsigned int fallback = 0; >>> unsigned int unit; >>> + u32 block_size = 0; >>> u32 unit_size; >>> int ret; >>> >>> @@ -131,17 +120,29 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req, >>> return -EINVAL; >>> >>> unit_size = CCP_XTS_AES_UNIT_SIZE__LAST; >>> - if (req->nbytes <= unit_size_map[0].size) { >> >> This check can't be deleted. It was added specifically to catch cases >> where the size was greater than 4096. >> >>> - for (unit = 0; unit < ARRAY_SIZE(unit_size_map); unit++) { >>> - if (!(req->nbytes & (unit_size_map[unit].size - 1))) { >>> - unit_size = unit_size_map[unit].value; >>> - break; >>> - } >>> + for (unit = ARRAY_SIZE(xts_unit_sizes); unit-- > 0; ) { >>> + if (!(req->nbytes & (xts_unit_sizes[unit].size - 1))) { >>> + unit_size = unit; >>> + block_size = xts_unit_sizes[unit].size; >>> + break; >>> } >>> } >>> >>> - if ((unit_size == CCP_XTS_AES_UNIT_SIZE__LAST) || >>> - (ctx->u.aes.key_len != AES_KEYSIZE_128)) { >>> + /* The CCP has restrictions on block sizes. Also, a version 3 device >>> + * only supports AES-128 operations; version 5 CCPs support both >>> + * AES-128 and -256 operations. >>> + */ >>> + if (unit_size == CCP_XTS_AES_UNIT_SIZE__LAST) >>> + fallback = 1; >>> + if ((ccpversion < CCP_VERSION(5, 0)) && >>> + (ctx->u.aes.key_len != AES_KEYSIZE_128)) >>> + fallback = 1; >>> + if ((ctx->u.aes.key_len != AES_KEYSIZE_128) && >>> + (ctx->u.aes.key_len != AES_KEYSIZE_256)) >>> + fallback = 1; >>> + if (req->nbytes != block_size) >>> + fallback = 1; >> >> I don't believe this is correct. Everything should fall out properly >> for fallback based on the unit size and key size checks above. >> >> Thanks, >> Tom >> >>> + if (fallback) { >>> SKCIPHER_REQUEST_ON_STACK(subreq, ctx->u.aes.tfm_skcipher); >>> >>> /* Use the fallback to process the request for any >>> diff --git a/drivers/crypto/ccp/ccp-dev-v5.c b/drivers/crypto/ccp/ccp-dev-v5.c >>> index b3526336d608..11febe2bd07c 100644 >>> --- a/drivers/crypto/ccp/ccp-dev-v5.c >>> +++ b/drivers/crypto/ccp/ccp-dev-v5.c >>> @@ -144,6 +144,7 @@ union ccp_function { >>> #define CCP_AES_ENCRYPT(p) ((p)->aes.encrypt) >>> #define CCP_AES_MODE(p) ((p)->aes.mode) >>> #define CCP_AES_TYPE(p) ((p)->aes.type) >>> +#define CCP_XTS_TYPE(p) ((p)->aes_xts.type) >>> #define CCP_XTS_SIZE(p) ((p)->aes_xts.size) >>> #define CCP_XTS_ENCRYPT(p) ((p)->aes_xts.encrypt) >>> #define CCP_DES3_SIZE(p) ((p)->des3.size) >>> @@ -344,6 +345,7 @@ static int ccp5_perform_xts_aes(struct ccp_op *op) >>> CCP5_CMD_PROT(&desc) = 0; >>> >>> function.raw = 0; >>> + CCP_XTS_TYPE(&function) = op->u.xts.type; >>> CCP_XTS_ENCRYPT(&function) = op->u.xts.action; >>> CCP_XTS_SIZE(&function) = op->u.xts.unit_size; >>> CCP5_CMD_FUNCTION(&desc) = function.raw; >>> diff --git a/drivers/crypto/ccp/ccp-dev.h b/drivers/crypto/ccp/ccp-dev.h >>> index 9320931d89da..3d51180199ac 100644 >>> --- a/drivers/crypto/ccp/ccp-dev.h >>> +++ b/drivers/crypto/ccp/ccp-dev.h >>> @@ -194,6 +194,7 @@ >>> #define CCP_AES_CTX_SB_COUNT 1 >>> >>> #define CCP_XTS_AES_KEY_SB_COUNT 1 >>> +#define CCP5_XTS_AES_KEY_SB_COUNT 2 >>> #define CCP_XTS_AES_CTX_SB_COUNT 1 >>> >>> #define CCP_DES3_KEY_SB_COUNT 1 >>> @@ -497,6 +498,7 @@ struct ccp_aes_op { >>> }; >>> >>> struct ccp_xts_aes_op { >>> + enum ccp_aes_type type; >>> enum ccp_aes_action action; >>> enum ccp_xts_aes_unit_size unit_size; >>> }; >>> diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c >>> index e23d138fc1ce..d1be07884a9b 100644 >>> --- a/drivers/crypto/ccp/ccp-ops.c >>> +++ b/drivers/crypto/ccp/ccp-ops.c >>> @@ -1038,6 +1038,8 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, >>> struct ccp_op op; >>> unsigned int unit_size, dm_offset; >>> bool in_place = false; >>> + unsigned int sb_count = 0; >>> + enum ccp_aes_type aestype; >>> int ret; >>> >>> switch (xts->unit_size) { >>> @@ -1056,12 +1058,15 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, >>> case CCP_XTS_AES_UNIT_SIZE_4096: >>> unit_size = 4096; >>> break; >>> - >>> default: >>> return -EINVAL; >>> } >>> >>> - if (xts->key_len != AES_KEYSIZE_128) >>> + if (xts->key_len == AES_KEYSIZE_128) >>> + aestype = CCP_AES_TYPE_128; >>> + else if (xts->key_len == AES_KEYSIZE_256) >>> + aestype = CCP_AES_TYPE_256; >>> + else >>> return -EINVAL; >>> >>> if (!xts->final && (xts->src_len & (AES_BLOCK_SIZE - 1))) >>> @@ -1084,22 +1089,50 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, >>> op.sb_ctx = cmd_q->sb_ctx; >>> op.init = 1; >>> op.u.xts.action = xts->action; >>> + op.u.xts.type = aestype; >>> op.u.xts.unit_size = xts->unit_size; >>> >>> - /* All supported key sizes fit in a single (32-byte) SB entry >>> - * and must be in little endian format. Use the 256-bit byte >>> - * swap passthru option to convert from big endian to little >>> - * endian. >>> + /* A version 3 device only supports 128-bit keys, which fits into a >>> + * single SB entry. A version 5 device uses a 512-bit vector, so two >>> + * SB entries. >>> */ >>> + if (cmd_q->ccp->vdata->version == CCP_VERSION(3, 0)) >>> + sb_count = CCP_XTS_AES_KEY_SB_COUNT; >>> + else >>> + sb_count = CCP5_XTS_AES_KEY_SB_COUNT; >>> ret = ccp_init_dm_workarea(&key, cmd_q, >>> - CCP_XTS_AES_KEY_SB_COUNT * CCP_SB_BYTES, >>> + sb_count * CCP_SB_BYTES, >>> DMA_TO_DEVICE); >>> if (ret) >>> return ret; >>> >>> - dm_offset = CCP_SB_BYTES - AES_KEYSIZE_128; >>> - ccp_set_dm_area(&key, dm_offset, xts->key, 0, xts->key_len); >>> - ccp_set_dm_area(&key, 0, xts->key, dm_offset, xts->key_len); >>> + if (cmd_q->ccp->vdata->version == CCP_VERSION(3, 0)) { >>> + /* All supported key sizes * must be in little endian format. >>> + * Use the 256-bit byte swap passthru option to convert from >>> + * big endian to little endian. >>> + */ >>> + dm_offset = CCP_SB_BYTES - AES_KEYSIZE_128; >>> + ccp_set_dm_area(&key, dm_offset, xts->key, 0, xts->key_len); >>> + ccp_set_dm_area(&key, 0, xts->key, xts->key_len, xts->key_len); >>> + } else { >>> + /* The AES key is at the little end and the tweak key is >>> + * at the big end. Since the byteswap operation is only >>> + * 256-bit, load the buffer according to the way things >>> + * will end up. >>> + */ >>> + unsigned int pad; >>> + >>> + op.sb_key = cmd_q->ccp->vdata->perform->sballoc(cmd_q, >>> + sb_count); >>> + if (!op.sb_key) >>> + return -EIO; >>> + >>> + dm_offset = CCP_SB_BYTES; >>> + pad = dm_offset - xts->key_len; >>> + ccp_set_dm_area(&key, pad, xts->key, 0, xts->key_len); >>> + ccp_set_dm_area(&key, dm_offset + pad, xts->key, xts->key_len, >>> + xts->key_len); >>> + } >>> ret = ccp_copy_to_sb(cmd_q, &key, op.jobid, op.sb_key, >>> CCP_PASSTHRU_BYTESWAP_256BIT); >>> if (ret) { >>> @@ -1179,7 +1212,6 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, >>> e_dst: >>> if (!in_place) >>> ccp_free_data(&dst, cmd_q); >>> - >>> e_src: >>> ccp_free_data(&src, cmd_q); >>> >>> @@ -1188,6 +1220,8 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, >>> >>> e_key: >>> ccp_dm_free(&key); >>> + if (cmd_q->ccp->vdata->version > CCP_VERSION(3, 0)) >>> + cmd_q->ccp->vdata->perform->sbfree(cmd_q, op.sb_key, sb_count); >>> >>> return ret; >>> } >>>
>On Tue, Jul 25, 2017 at 02:12:11PM -0500, Gary R Hook wrote: > > Version 5 CCPs have some new requirements for XTS-AES: the type field > > must be specified, and the key requires 512 bits, with each part > > occupying 256 bits and padded with zeroes. > > > > cc: <stable@vger.kernel.org> # 4.9.x+ > > > > Signed-off-by: Gary R Hook <ghook@amd.com> > > Patch applied. Thanks. Herbert, *ping* This is a bug fix, Could we push this into 4.13, please? Gary
On Fri, Aug 18, 2017 at 11:41:04AM -0500, Gary R Hook wrote: > >On Tue, Jul 25, 2017 at 02:12:11PM -0500, Gary R Hook wrote: > >> Version 5 CCPs have some new requirements for XTS-AES: the type field > >> must be specified, and the key requires 512 bits, with each part > >> occupying 256 bits and padded with zeroes. > >> > >> cc: <stable@vger.kernel.org> # 4.9.x+ > >> > >> Signed-off-by: Gary R Hook <ghook@amd.com> > > > >Patch applied. Thanks. > > Herbert, > > *ping* > > This is a bug fix, Could we push this into 4.13, please? I don't think this one is critical enough to go in right now. Is there any reason why it can't wait until the next merge window? Thanks,
On 08/18/2017 11:02 PM, Herbert Xu wrote: > On Fri, Aug 18, 2017 at 11:41:04AM -0500, Gary R Hook wrote: >>> On Tue, Jul 25, 2017 at 02:12:11PM -0500, Gary R Hook wrote: >>>> Version 5 CCPs have some new requirements for XTS-AES: the type field >>>> must be specified, and the key requires 512 bits, with each part >>>> occupying 256 bits and padded with zeroes. >>>> >>>> cc: <stable@vger.kernel.org> # 4.9.x+ >>>> >>>> Signed-off-by: Gary R Hook <ghook@amd.com> >>> >>> Patch applied. Thanks. >> >> Herbert, >> >> *ping* >> >> This is a bug fix, Could we push this into 4.13, please? > > I don't think this one is critical enough to go in right now. > Is there any reason why it can't wait until the next merge window? I am happy to defer to your expertise in this area. My concern was whether the patch had been overlooked. I see now that it had not. Thank you, sir.
diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c b/drivers/crypto/ccp/ccp-crypto-aes-xts.c index 58a4244b4752..8d248b198e22 100644 --- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c +++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c @@ -1,7 +1,7 @@ /* * AMD Cryptographic Coprocessor (CCP) AES XTS crypto API support * - * Copyright (C) 2013 Advanced Micro Devices, Inc. + * Copyright (C) 2013,2017 Advanced Micro Devices, Inc. * * Author: Tom Lendacky <thomas.lendacky@amd.com> * @@ -37,46 +37,26 @@ struct ccp_unit_size_map { u32 value; }; -static struct ccp_unit_size_map unit_size_map[] = { +static struct ccp_unit_size_map xts_unit_sizes[] = { { - .size = 4096, - .value = CCP_XTS_AES_UNIT_SIZE_4096, - }, - { - .size = 2048, - .value = CCP_XTS_AES_UNIT_SIZE_2048, - }, - { - .size = 1024, - .value = CCP_XTS_AES_UNIT_SIZE_1024, + .size = 16, + .value = CCP_XTS_AES_UNIT_SIZE_16, }, { - .size = 512, + .size = 512, .value = CCP_XTS_AES_UNIT_SIZE_512, }, { - .size = 256, - .value = CCP_XTS_AES_UNIT_SIZE__LAST, - }, - { - .size = 128, - .value = CCP_XTS_AES_UNIT_SIZE__LAST, - }, - { - .size = 64, - .value = CCP_XTS_AES_UNIT_SIZE__LAST, - }, - { - .size = 32, - .value = CCP_XTS_AES_UNIT_SIZE__LAST, + .size = 1024, + .value = CCP_XTS_AES_UNIT_SIZE_1024, }, { - .size = 16, - .value = CCP_XTS_AES_UNIT_SIZE_16, + .size = 2048, + .value = CCP_XTS_AES_UNIT_SIZE_2048, }, { - .size = 1, - .value = CCP_XTS_AES_UNIT_SIZE__LAST, + .size = 4096, + .value = CCP_XTS_AES_UNIT_SIZE_4096, }, }; @@ -97,14 +77,20 @@ static int ccp_aes_xts_setkey(struct crypto_ablkcipher *tfm, const u8 *key, unsigned int key_len) { struct ccp_ctx *ctx = crypto_tfm_ctx(crypto_ablkcipher_tfm(tfm)); + unsigned int ccpversion = ccp_version(); /* Only support 128-bit AES key with a 128-bit Tweak key, * otherwise use the fallback */ + switch (key_len) { case AES_KEYSIZE_128 * 2: memcpy(ctx->u.aes.key, key, key_len); break; + case AES_KEYSIZE_256 * 2: + if (ccpversion > CCP_VERSION(3, 0)) + memcpy(ctx->u.aes.key, key, key_len); + break; } ctx->u.aes.key_len = key_len / 2; sg_init_one(&ctx->u.aes.key_sg, ctx->u.aes.key, key_len); @@ -117,7 +103,10 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req, { struct ccp_ctx *ctx = crypto_tfm_ctx(req->base.tfm); struct ccp_aes_req_ctx *rctx = ablkcipher_request_ctx(req); + unsigned int ccpversion = ccp_version(); + unsigned int fallback = 0; unsigned int unit; + u32 block_size = 0; u32 unit_size; int ret; @@ -131,17 +120,29 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req, return -EINVAL; unit_size = CCP_XTS_AES_UNIT_SIZE__LAST; - if (req->nbytes <= unit_size_map[0].size) { - for (unit = 0; unit < ARRAY_SIZE(unit_size_map); unit++) { - if (!(req->nbytes & (unit_size_map[unit].size - 1))) { - unit_size = unit_size_map[unit].value; - break; - } + for (unit = ARRAY_SIZE(xts_unit_sizes); unit-- > 0; ) { + if (!(req->nbytes & (xts_unit_sizes[unit].size - 1))) { + unit_size = unit; + block_size = xts_unit_sizes[unit].size; + break; } } - if ((unit_size == CCP_XTS_AES_UNIT_SIZE__LAST) || - (ctx->u.aes.key_len != AES_KEYSIZE_128)) { + /* The CCP has restrictions on block sizes. Also, a version 3 device + * only supports AES-128 operations; version 5 CCPs support both + * AES-128 and -256 operations. + */ + if (unit_size == CCP_XTS_AES_UNIT_SIZE__LAST) + fallback = 1; + if ((ccpversion < CCP_VERSION(5, 0)) && + (ctx->u.aes.key_len != AES_KEYSIZE_128)) + fallback = 1; + if ((ctx->u.aes.key_len != AES_KEYSIZE_128) && + (ctx->u.aes.key_len != AES_KEYSIZE_256)) + fallback = 1; + if (req->nbytes != block_size) + fallback = 1; + if (fallback) { SKCIPHER_REQUEST_ON_STACK(subreq, ctx->u.aes.tfm_skcipher); /* Use the fallback to process the request for any diff --git a/drivers/crypto/ccp/ccp-dev-v5.c b/drivers/crypto/ccp/ccp-dev-v5.c index b3526336d608..11febe2bd07c 100644 --- a/drivers/crypto/ccp/ccp-dev-v5.c +++ b/drivers/crypto/ccp/ccp-dev-v5.c @@ -144,6 +144,7 @@ union ccp_function { #define CCP_AES_ENCRYPT(p) ((p)->aes.encrypt) #define CCP_AES_MODE(p) ((p)->aes.mode) #define CCP_AES_TYPE(p) ((p)->aes.type) +#define CCP_XTS_TYPE(p) ((p)->aes_xts.type) #define CCP_XTS_SIZE(p) ((p)->aes_xts.size) #define CCP_XTS_ENCRYPT(p) ((p)->aes_xts.encrypt) #define CCP_DES3_SIZE(p) ((p)->des3.size) @@ -344,6 +345,7 @@ static int ccp5_perform_xts_aes(struct ccp_op *op) CCP5_CMD_PROT(&desc) = 0; function.raw = 0; + CCP_XTS_TYPE(&function) = op->u.xts.type; CCP_XTS_ENCRYPT(&function) = op->u.xts.action; CCP_XTS_SIZE(&function) = op->u.xts.unit_size; CCP5_CMD_FUNCTION(&desc) = function.raw; diff --git a/drivers/crypto/ccp/ccp-dev.h b/drivers/crypto/ccp/ccp-dev.h index 9320931d89da..3d51180199ac 100644 --- a/drivers/crypto/ccp/ccp-dev.h +++ b/drivers/crypto/ccp/ccp-dev.h @@ -194,6 +194,7 @@ #define CCP_AES_CTX_SB_COUNT 1 #define CCP_XTS_AES_KEY_SB_COUNT 1 +#define CCP5_XTS_AES_KEY_SB_COUNT 2 #define CCP_XTS_AES_CTX_SB_COUNT 1 #define CCP_DES3_KEY_SB_COUNT 1 @@ -497,6 +498,7 @@ struct ccp_aes_op { }; struct ccp_xts_aes_op { + enum ccp_aes_type type; enum ccp_aes_action action; enum ccp_xts_aes_unit_size unit_size; }; diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c index e23d138fc1ce..d1be07884a9b 100644 --- a/drivers/crypto/ccp/ccp-ops.c +++ b/drivers/crypto/ccp/ccp-ops.c @@ -1038,6 +1038,8 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, struct ccp_op op; unsigned int unit_size, dm_offset; bool in_place = false; + unsigned int sb_count = 0; + enum ccp_aes_type aestype; int ret; switch (xts->unit_size) { @@ -1056,12 +1058,15 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, case CCP_XTS_AES_UNIT_SIZE_4096: unit_size = 4096; break; - default: return -EINVAL; } - if (xts->key_len != AES_KEYSIZE_128) + if (xts->key_len == AES_KEYSIZE_128) + aestype = CCP_AES_TYPE_128; + else if (xts->key_len == AES_KEYSIZE_256) + aestype = CCP_AES_TYPE_256; + else return -EINVAL; if (!xts->final && (xts->src_len & (AES_BLOCK_SIZE - 1))) @@ -1084,22 +1089,50 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, op.sb_ctx = cmd_q->sb_ctx; op.init = 1; op.u.xts.action = xts->action; + op.u.xts.type = aestype; op.u.xts.unit_size = xts->unit_size; - /* All supported key sizes fit in a single (32-byte) SB entry - * and must be in little endian format. Use the 256-bit byte - * swap passthru option to convert from big endian to little - * endian. + /* A version 3 device only supports 128-bit keys, which fits into a + * single SB entry. A version 5 device uses a 512-bit vector, so two + * SB entries. */ + if (cmd_q->ccp->vdata->version == CCP_VERSION(3, 0)) + sb_count = CCP_XTS_AES_KEY_SB_COUNT; + else + sb_count = CCP5_XTS_AES_KEY_SB_COUNT; ret = ccp_init_dm_workarea(&key, cmd_q, - CCP_XTS_AES_KEY_SB_COUNT * CCP_SB_BYTES, + sb_count * CCP_SB_BYTES, DMA_TO_DEVICE); if (ret) return ret; - dm_offset = CCP_SB_BYTES - AES_KEYSIZE_128; - ccp_set_dm_area(&key, dm_offset, xts->key, 0, xts->key_len); - ccp_set_dm_area(&key, 0, xts->key, dm_offset, xts->key_len); + if (cmd_q->ccp->vdata->version == CCP_VERSION(3, 0)) { + /* All supported key sizes * must be in little endian format. + * Use the 256-bit byte swap passthru option to convert from + * big endian to little endian. + */ + dm_offset = CCP_SB_BYTES - AES_KEYSIZE_128; + ccp_set_dm_area(&key, dm_offset, xts->key, 0, xts->key_len); + ccp_set_dm_area(&key, 0, xts->key, xts->key_len, xts->key_len); + } else { + /* The AES key is at the little end and the tweak key is + * at the big end. Since the byteswap operation is only + * 256-bit, load the buffer according to the way things + * will end up. + */ + unsigned int pad; + + op.sb_key = cmd_q->ccp->vdata->perform->sballoc(cmd_q, + sb_count); + if (!op.sb_key) + return -EIO; + + dm_offset = CCP_SB_BYTES; + pad = dm_offset - xts->key_len; + ccp_set_dm_area(&key, pad, xts->key, 0, xts->key_len); + ccp_set_dm_area(&key, dm_offset + pad, xts->key, xts->key_len, + xts->key_len); + } ret = ccp_copy_to_sb(cmd_q, &key, op.jobid, op.sb_key, CCP_PASSTHRU_BYTESWAP_256BIT); if (ret) { @@ -1179,7 +1212,6 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, e_dst: if (!in_place) ccp_free_data(&dst, cmd_q); - e_src: ccp_free_data(&src, cmd_q); @@ -1188,6 +1220,8 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q, e_key: ccp_dm_free(&key); + if (cmd_q->ccp->vdata->version > CCP_VERSION(3, 0)) + cmd_q->ccp->vdata->perform->sbfree(cmd_q, op.sb_key, sb_count); return ret; }
Version 5 CCPs have differing requirements for XTS-AES: key components are stored in a 512-bit vector. The context must be little-endian justified. AES-256 is supported now, so propagate the cipher size to the command descriptor. Signed-off-by: Gary R Hook <gary.hook@amd.com> --- drivers/crypto/ccp/ccp-crypto-aes-xts.c | 79 ++++++++++++++++--------------- drivers/crypto/ccp/ccp-dev-v5.c | 2 + drivers/crypto/ccp/ccp-dev.h | 2 + drivers/crypto/ccp/ccp-ops.c | 56 ++++++++++++++++++---- 4 files changed, 89 insertions(+), 50 deletions(-)