The following series adds support for XS-AES on version 5 CCPs,
both 128- and 256-bit, and enhances/clarifies/simplifies some
crypto layer code.
Changes since v1:
- rework the validation of the unit-size; move to a separate patch
- expand the key buffer to accommodate 256-bit keys
- use xts_check_key() in the crypto layer
---
Gary R Hook (4):
crypto: ccp - Add a call to xts_check_key()
crypto: ccp - Enable XTS-AES-128 support on all CCPs
crypto: ccp - Rework the unit-size check for XTS-AES
crypto: ccp - Add XTS-AES-256 support for CCP version 5
drivers/crypto/ccp/ccp-crypto-aes-xts.c | 98 +++++++++++++++++--------------
drivers/crypto/ccp/ccp-crypto.h | 2 -
drivers/crypto/ccp/ccp-dev-v5.c | 2 +
drivers/crypto/ccp/ccp-dev.h | 2 +
drivers/crypto/ccp/ccp-ops.c | 55 +++++++++++++++--
5 files changed, 106 insertions(+), 53 deletions(-)
Vet the key using the available standard function
Signed-off-by: Gary R Hook <[email protected]>
---
drivers/crypto/ccp/ccp-crypto-aes-xts.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 58a4244b4752..4a313f62dbea 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -15,6 +15,7 @@
#include <linux/delay.h>
#include <linux/scatterlist.h>
#include <crypto/aes.h>
+#include <crypto/xts.h>
#include <crypto/internal/skcipher.h>
#include <crypto/scatterwalk.h>
@@ -96,7 +97,13 @@ static int ccp_aes_xts_complete(struct crypto_async_request *async_req, int ret)
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));
+ struct crypto_tfm *xfm = crypto_ablkcipher_tfm(tfm);
+ struct ccp_ctx *ctx = crypto_tfm_ctx(xfm);
+ int ret;
+
+ ret = xts_check_key(xfm, key, key_len);
+ if (ret)
+ return ret;
/* Only support 128-bit AES key with a 128-bit Tweak key,
* otherwise use the fallback
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.
Signed-off-by: Gary R Hook <[email protected]>
---
drivers/crypto/ccp/ccp-dev-v5.c | 2 ++
drivers/crypto/ccp/ccp-dev.h | 2 ++
drivers/crypto/ccp/ccp-ops.c | 52 ++++++++++++++++++++++++++++++++-------
3 files changed, 47 insertions(+), 9 deletions(-)
diff --git a/drivers/crypto/ccp/ccp-dev-v5.c b/drivers/crypto/ccp/ccp-dev-v5.c
index b3526336d608..0fb4519c5194 100644
--- a/drivers/crypto/ccp/ccp-dev-v5.c
+++ b/drivers/crypto/ccp/ccp-dev-v5.c
@@ -145,6 +145,7 @@ union ccp_function {
#define CCP_AES_MODE(p) ((p)->aes.mode)
#define CCP_AES_TYPE(p) ((p)->aes.type)
#define CCP_XTS_SIZE(p) ((p)->aes_xts.size)
+#define CCP_XTS_TYPE(p) ((p)->aes_xts.type)
#define CCP_XTS_ENCRYPT(p) ((p)->aes_xts.encrypt)
#define CCP_DES3_SIZE(p) ((p)->des3.size)
#define CCP_DES3_ENCRYPT(p) ((p)->des3.encrypt)
@@ -346,6 +347,7 @@ static int ccp5_perform_xts_aes(struct ccp_op *op)
function.raw = 0;
CCP_XTS_ENCRYPT(&function) = op->u.xts.action;
CCP_XTS_SIZE(&function) = op->u.xts.unit_size;
+ CCP_XTS_TYPE(&function) = op->u.xts.type;
CCP5_CMD_FUNCTION(&desc) = function.raw;
CCP5_CMD_LEN(&desc) = op->src.u.dma.length;
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..8113355151d2 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) {
@@ -1061,9 +1063,12 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q,
return -EINVAL;
}
- if (xts->key_len != AES_KEYSIZE_128)
+ if (xts->key_len == AES_KEYSIZE_128)
+ aestype = CCP_AES_TYPE_128;
+ else
return -EINVAL;
+
if (!xts->final && (xts->src_len & (AES_BLOCK_SIZE - 1)))
return -EINVAL;
@@ -1086,20 +1091,47 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q,
op.u.xts.action = xts->action;
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) {
@@ -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;
}
The CCP supports a limited set of unit-size values. Change the check
for this parameter such that acceptable values match the enumeration.
Then clarify the conditions under which we must use the fallback
implementation.
Signed-off-by: Gary R Hook <[email protected]>
---
drivers/crypto/ccp/ccp-crypto-aes-xts.c | 75 ++++++++++++++-----------------
1 file changed, 35 insertions(+), 40 deletions(-)
diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 4a313f62dbea..3c37794ffe2d 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -1,8 +1,9 @@
/*
* 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: Gary R Hook <[email protected]>
* Author: Tom Lendacky <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
@@ -38,46 +39,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,
},
};
@@ -124,7 +105,9 @@ 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 fallback = 0;
unsigned int unit;
+ u32 block_size;
u32 unit_size;
int ret;
@@ -137,18 +120,30 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req,
if (!req->info)
return -EINVAL;
+ /* Check conditions under which the CCP can fulfill a request. The
+ * device can handle input plaintext of a length that is a multiple
+ * of the unit_size, bug the crypto implementation only supports
+ * the unit_size being equal to the input length. This limits the
+ * number of scenarios we can handle. Also validate the key length.
+ */
+ block_size = 0;
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 = 0; unit < ARRAY_SIZE(xts_unit_sizes); unit++) {
+ if (req->nbytes == xts_unit_sizes[unit].size) {
+ 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 (ctx->u.aes.key_len != AES_KEYSIZE_128)
+ fallback = 1;
+ if (fallback) {
SKCIPHER_REQUEST_ON_STACK(subreq, ctx->u.aes.tfm_skcipher);
/* Use the fallback to process the request for any
Signed-off-by: Gary R Hook <[email protected]>
---
drivers/crypto/ccp/ccp-crypto-aes-xts.c | 16 +++++++++++++---
drivers/crypto/ccp/ccp-crypto.h | 2 +-
drivers/crypto/ccp/ccp-ops.c | 3 +++
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
index 3c37794ffe2d..4a3fe4d5ac71 100644
--- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
+++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
@@ -80,19 +80,24 @@ static int ccp_aes_xts_setkey(struct crypto_ablkcipher *tfm, const u8 *key,
{
struct crypto_tfm *xfm = crypto_ablkcipher_tfm(tfm);
struct ccp_ctx *ctx = crypto_tfm_ctx(xfm);
+ unsigned int ccpversion = ccp_version();
int ret;
ret = xts_check_key(xfm, key, key_len);
if (ret)
return ret;
- /* Only support 128-bit AES key with a 128-bit Tweak key,
- * otherwise use the fallback
+ /* Version 3 devices support 128-bit keys; version 5 devices can
+ * accommodate 128- and 256-bit keys.
*/
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);
@@ -105,6 +110,7 @@ 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;
@@ -141,7 +147,11 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req,
*/
if (unit_size == CCP_XTS_AES_UNIT_SIZE__LAST)
fallback = 1;
- if (ctx->u.aes.key_len != AES_KEYSIZE_128)
+ 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 (fallback) {
SKCIPHER_REQUEST_ON_STACK(subreq, ctx->u.aes.tfm_skcipher);
diff --git a/drivers/crypto/ccp/ccp-crypto.h b/drivers/crypto/ccp/ccp-crypto.h
index 156b8233853f..880f8acdd0cd 100644
--- a/drivers/crypto/ccp/ccp-crypto.h
+++ b/drivers/crypto/ccp/ccp-crypto.h
@@ -91,7 +91,7 @@ struct ccp_aes_ctx {
struct scatterlist key_sg;
unsigned int key_len;
- u8 key[AES_MAX_KEY_SIZE];
+ u8 key[AES_MAX_KEY_SIZE * 2];
u8 nonce[CTR_RFC3686_NONCE_SIZE];
diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c
index 8113355151d2..fbd024f6e898 100644
--- a/drivers/crypto/ccp/ccp-ops.c
+++ b/drivers/crypto/ccp/ccp-ops.c
@@ -1065,6 +1065,8 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q,
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;
@@ -1089,6 +1091,7 @@ 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;
/* A version 3 device only supports 128-bit keys, which fits into a
On 7/21/2017 2:04 PM, 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.
This appears to be a fix and not a feature. You need to send this as
a separate patch through the fix process and back through to the stable
releases.
>
> Signed-off-by: Gary R Hook <[email protected]>
> ---
> drivers/crypto/ccp/ccp-dev-v5.c | 2 ++
> drivers/crypto/ccp/ccp-dev.h | 2 ++
> drivers/crypto/ccp/ccp-ops.c | 52 ++++++++++++++++++++++++++++++++-------
> 3 files changed, 47 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/crypto/ccp/ccp-dev-v5.c b/drivers/crypto/ccp/ccp-dev-v5.c
> index b3526336d608..0fb4519c5194 100644
> --- a/drivers/crypto/ccp/ccp-dev-v5.c
> +++ b/drivers/crypto/ccp/ccp-dev-v5.c
> @@ -145,6 +145,7 @@ union ccp_function {
> #define CCP_AES_MODE(p) ((p)->aes.mode)
> #define CCP_AES_TYPE(p) ((p)->aes.type)
> #define CCP_XTS_SIZE(p) ((p)->aes_xts.size)
> +#define CCP_XTS_TYPE(p) ((p)->aes_xts.type)
> #define CCP_XTS_ENCRYPT(p) ((p)->aes_xts.encrypt)
> #define CCP_DES3_SIZE(p) ((p)->des3.size)
> #define CCP_DES3_ENCRYPT(p) ((p)->des3.encrypt)
> @@ -346,6 +347,7 @@ static int ccp5_perform_xts_aes(struct ccp_op *op)
> function.raw = 0;
> CCP_XTS_ENCRYPT(&function) = op->u.xts.action;
> CCP_XTS_SIZE(&function) = op->u.xts.unit_size;
> + CCP_XTS_TYPE(&function) = op->u.xts.type;
> CCP5_CMD_FUNCTION(&desc) = function.raw;
>
> CCP5_CMD_LEN(&desc) = op->src.u.dma.length;
> 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..8113355151d2 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;
No need to initialize to zero here.
> + enum ccp_aes_type aestype;
> int ret;
>
> switch (xts->unit_size) {
> @@ -1061,9 +1063,12 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q,
> return -EINVAL;
> }
>
> - if (xts->key_len != AES_KEYSIZE_128)
> + if (xts->key_len == AES_KEYSIZE_128)
> + aestype = CCP_AES_TYPE_128;
> + else
> return -EINVAL;
>
> +
Remove extra blank line.
> if (!xts->final && (xts->src_len & (AES_BLOCK_SIZE - 1)))
> return -EINVAL;
>
> @@ -1086,20 +1091,47 @@ static int ccp_run_xts_aes_cmd(struct ccp_cmd_queue *cmd_q,
> op.u.xts.action = xts->action;
> 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.
> + */
The second half of your patch description better describes what you're
doing here... that is, you need two SB entries with the key in one and
the tweak in the other with appropriate padding.
> + unsigned int pad;
> +
> + op.sb_key = cmd_q->ccp->vdata->perform->sballoc(cmd_q,
> + sb_count);
> + if (!op.sb_key)
> + return -EIO;
So now you're always allocating SB entries. Doesn't the initialization
function already pre-allocate space for 2 SB entries for keys? I don't
think you need to do this allocation.
> +
> + 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) {
> @@ -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);
This can be removed if you end up using the pre-allocated entries.
Thanks,
Tom
>
> return ret;
> }
>
On 7/21/2017 2:05 PM, Gary R Hook wrote:
> The CCP supports a limited set of unit-size values. Change the check
> for this parameter such that acceptable values match the enumeration.
> Then clarify the conditions under which we must use the fallback
> implementation.
>
> Signed-off-by: Gary R Hook <[email protected]>
> ---
> drivers/crypto/ccp/ccp-crypto-aes-xts.c | 75 ++++++++++++++-----------------
> 1 file changed, 35 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/crypto/ccp/ccp-crypto-aes-xts.c b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
> index 4a313f62dbea..3c37794ffe2d 100644
> --- a/drivers/crypto/ccp/ccp-crypto-aes-xts.c
> +++ b/drivers/crypto/ccp/ccp-crypto-aes-xts.c
> @@ -1,8 +1,9 @@
> /*
> * 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: Gary R Hook <[email protected]>
> * Author: Tom Lendacky <[email protected]>
> *
> * This program is free software; you can redistribute it and/or modify
> @@ -38,46 +39,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,
> },
> };
>
> @@ -124,7 +105,9 @@ 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 fallback = 0;
> unsigned int unit;
> + u32 block_size;
I don't see this variable used anywhere. It should be deleted.
> u32 unit_size;
> int ret;
>
> @@ -137,18 +120,30 @@ static int ccp_aes_xts_crypt(struct ablkcipher_request *req,
> if (!req->info)
> return -EINVAL;
>
> + /* Check conditions under which the CCP can fulfill a request. The
> + * device can handle input plaintext of a length that is a multiple
> + * of the unit_size, bug the crypto implementation only supports
> + * the unit_size being equal to the input length. This limits the
> + * number of scenarios we can handle. Also validate the key length.
Remove the "Also validate the key length." since that happens below and
is covered by a different comment.
> + */
> + block_size = 0;
> 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 = 0; unit < ARRAY_SIZE(xts_unit_sizes); unit++) {
> + if (req->nbytes == xts_unit_sizes[unit].size) {
> + 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.
The 256-bit XTS support isn't here yet, so shouldn't mention it now.
> + */
> + if (unit_size == CCP_XTS_AES_UNIT_SIZE__LAST)
> + fallback = 1;
> + if (ctx->u.aes.key_len != AES_KEYSIZE_128)
> + fallback = 1;
> + if (fallback) {
It appears these changes are not necessary... It does the same thing as
the previous check. Just make the changes when the XTS-256 support is
added in the next patch.
Thanks,
Tom
> SKCIPHER_REQUEST_ON_STACK(subreq, ctx->u.aes.tfm_skcipher);
>
> /* Use the fallback to process the request for any
>