Hi, Lei
I'd like to move helper and callback functions(Eg, virtcrypto_clear_request
and virtcrypto_ctrlq_callback) from xx_core.c to xx_common.c,
then the xx_core.c supports:
- probe/remove/irq affinity seting for a virtio device
- basic virtio related operations
xx_common.c supports:
- common helpers/functions for algos
Do you have any suggestion about this?
v3 -> v4:
- Don't create new file virtio_common.c, the new functions are added
into virtio_crypto_core.c
- Split the first patch into two parts:
1, change code style,
2, use private buffer instead of shared buffer
- Remove relevant change.
- Other minor changes.
v2 -> v3:
- Jason suggested that spliting the first patch into two part:
1, using private buffer
2, remove the busy polling
Rework as Jason's suggestion, this makes the smaller change in
each one and clear.
v1 -> v2:
- Use kfree instead of kfree_sensitive for insensitive buffer.
- Several coding style fix.
- Use memory from current node, instead of memory close to device
- Add more message in commit, also explain why removing per-device
request buffer.
- Add necessary comment in code to explain why using kzalloc to
allocate struct virtio_crypto_ctrl_request.
v1:
The main point of this series is to improve the performance for
virtio crypto:
- Use wait mechanism instead of busy polling for ctrl queue, this
reduces CPU and lock racing, it's possiable to create/destroy session
parallelly, QPS increases from ~40K/s to ~200K/s.
- Enable retry on crypto engine to improve performance for data queue,
this allows the larger depth instead of 1.
- Fix dst data length in akcipher service.
- Other style fix.
lei he (2):
virtio-crypto: adjust dst_len at ops callback
virtio-crypto: enable retry for virtio-crypto-dev
zhenwei pi (3):
virtio-crypto: change code style
virtio-crypto: use private buffer for control request
virtio-crypto: wait ctrl queue instead of busy polling
.../virtio/virtio_crypto_akcipher_algs.c | 83 ++++++-----
drivers/crypto/virtio/virtio_crypto_common.h | 21 ++-
drivers/crypto/virtio/virtio_crypto_core.c | 55 ++++++-
.../virtio/virtio_crypto_skcipher_algs.c | 140 ++++++++----------
4 files changed, 180 insertions(+), 119 deletions(-)
--
2.20.1
Originally, all of the control requests share a single buffer(
ctrl & input & ctrl_status fields in struct virtio_crypto), this
allows queue depth 1 only, the performance of control queue gets
limited by this design.
In this patch, each request allocates request buffer dynamically, and
free buffer after request, so the scope protected by ctrl_lock also
get optimized here.
It's possible to optimize control queue depth in the next step.
A necessary comment is already in code, still describe it again:
/*
* Note: there are padding fields in request, clear them to zero before
* sending to host to avoid to divulge any information.
* Ex, virtio_crypto_ctrl_request::ctrl::u::destroy_session::padding[48]
*/
So use kzalloc to allocate buffer of struct virtio_crypto_ctrl_request.
Cc: Michael S. Tsirkin <[email protected]>
Cc: Jason Wang <[email protected]>
Cc: Gonglei <[email protected]>
Signed-off-by: zhenwei pi <[email protected]>
---
.../virtio/virtio_crypto_akcipher_algs.c | 41 +++++++++++----
drivers/crypto/virtio/virtio_crypto_common.h | 17 +++++--
.../virtio/virtio_crypto_skcipher_algs.c | 50 ++++++++++++-------
3 files changed, 75 insertions(+), 33 deletions(-)
diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
index 20901a263fc8..509884e8b201 100644
--- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
@@ -108,16 +108,22 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher
unsigned int num_out = 0, num_in = 0;
struct virtio_crypto_op_ctrl_req *ctrl;
struct virtio_crypto_session_input *input;
+ struct virtio_crypto_ctrl_request *vc_ctrl_req;
pkey = kmemdup(key, keylen, GFP_ATOMIC);
if (!pkey)
return -ENOMEM;
- spin_lock(&vcrypto->ctrl_lock);
- ctrl = &vcrypto->ctrl;
+ vc_ctrl_req = kzalloc(sizeof(*vc_ctrl_req), GFP_KERNEL);
+ if (!vc_ctrl_req) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ ctrl = &vc_ctrl_req->ctrl;
memcpy(&ctrl->header, header, sizeof(ctrl->header));
memcpy(&ctrl->u, para, sizeof(ctrl->u));
- input = &vcrypto->input;
+ input = &vc_ctrl_req->input;
input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
sg_init_one(&outhdr_sg, ctrl, sizeof(*ctrl));
@@ -129,14 +135,18 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher
sg_init_one(&inhdr_sg, input, sizeof(*input));
sgs[num_out + num_in++] = &inhdr_sg;
+ spin_lock(&vcrypto->ctrl_lock);
err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC);
- if (err < 0)
+ if (err < 0) {
+ spin_unlock(&vcrypto->ctrl_lock);
goto out;
+ }
virtqueue_kick(vcrypto->ctrl_vq);
while (!virtqueue_get_buf(vcrypto->ctrl_vq, &inlen) &&
!virtqueue_is_broken(vcrypto->ctrl_vq))
cpu_relax();
+ spin_unlock(&vcrypto->ctrl_lock);
if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) {
err = -EINVAL;
@@ -148,7 +158,7 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher
err = 0;
out:
- spin_unlock(&vcrypto->ctrl_lock);
+ kfree(vc_ctrl_req);
kfree_sensitive(pkey);
if (err < 0)
@@ -167,15 +177,22 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
int err;
struct virtio_crypto_op_ctrl_req *ctrl;
struct virtio_crypto_inhdr *ctrl_status;
+ struct virtio_crypto_ctrl_request *vc_ctrl_req;
- spin_lock(&vcrypto->ctrl_lock);
if (!ctx->session_valid) {
err = 0;
goto out;
}
- ctrl_status = &vcrypto->ctrl_status;
+
+ vc_ctrl_req = kzalloc(sizeof(*vc_ctrl_req), GFP_KERNEL);
+ if (!vc_ctrl_req) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ ctrl_status = &vc_ctrl_req->ctrl_status;
ctrl_status->status = VIRTIO_CRYPTO_ERR;
- ctrl = &vcrypto->ctrl;
+ ctrl = &vc_ctrl_req->ctrl;
ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION);
ctrl->header.queue_id = 0;
@@ -188,14 +205,18 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
sg_init_one(&inhdr_sg, &ctrl_status->status, sizeof(ctrl_status->status));
sgs[num_out + num_in++] = &inhdr_sg;
+ spin_lock(&vcrypto->ctrl_lock);
err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC);
- if (err < 0)
+ if (err < 0) {
+ spin_unlock(&vcrypto->ctrl_lock);
goto out;
+ }
virtqueue_kick(vcrypto->ctrl_vq);
while (!virtqueue_get_buf(vcrypto->ctrl_vq, &inlen) &&
!virtqueue_is_broken(vcrypto->ctrl_vq))
cpu_relax();
+ spin_unlock(&vcrypto->ctrl_lock);
if (ctrl_status->status != VIRTIO_CRYPTO_OK) {
err = -EINVAL;
@@ -206,7 +227,7 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
ctx->session_valid = false;
out:
- spin_unlock(&vcrypto->ctrl_lock);
+ kfree(vc_ctrl_req);
if (err < 0) {
pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n",
ctrl_status->status, destroy_session->session_id);
diff --git a/drivers/crypto/virtio/virtio_crypto_common.h b/drivers/crypto/virtio/virtio_crypto_common.h
index e693d4ee83a6..2422237ec4e6 100644
--- a/drivers/crypto/virtio/virtio_crypto_common.h
+++ b/drivers/crypto/virtio/virtio_crypto_common.h
@@ -13,6 +13,7 @@
#include <crypto/aead.h>
#include <crypto/aes.h>
#include <crypto/engine.h>
+#include <uapi/linux/virtio_crypto.h>
/* Internal representation of a data virtqueue */
@@ -65,11 +66,6 @@ struct virtio_crypto {
/* Maximum size of per request */
u64 max_size;
- /* Control VQ buffers: protected by the ctrl_lock */
- struct virtio_crypto_op_ctrl_req ctrl;
- struct virtio_crypto_session_input input;
- struct virtio_crypto_inhdr ctrl_status;
-
unsigned long status;
atomic_t ref_count;
struct list_head list;
@@ -85,6 +81,17 @@ struct virtio_crypto_sym_session_info {
__u64 session_id;
};
+/*
+ * Note: there are padding fields in request, clear them to zero before
+ * sending to host to avoid to divulge any information.
+ * Ex, virtio_crypto_ctrl_request::ctrl::u::destroy_session::padding[48]
+ */
+struct virtio_crypto_ctrl_request {
+ struct virtio_crypto_op_ctrl_req ctrl;
+ struct virtio_crypto_session_input input;
+ struct virtio_crypto_inhdr ctrl_status;
+};
+
struct virtio_crypto_request;
typedef void (*virtio_crypto_data_callback)
(struct virtio_crypto_request *vc_req, int len);
diff --git a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
index e3c5bc8d6112..6aaf0869b211 100644
--- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
@@ -126,6 +126,7 @@ static int virtio_crypto_alg_skcipher_init_session(
struct virtio_crypto_op_ctrl_req *ctrl;
struct virtio_crypto_session_input *input;
struct virtio_crypto_sym_create_session_req *sym_create_session;
+ struct virtio_crypto_ctrl_request *vc_ctrl_req;
/*
* Avoid to do DMA from the stack, switch to using
@@ -136,15 +137,20 @@ static int virtio_crypto_alg_skcipher_init_session(
if (!cipher_key)
return -ENOMEM;
- spin_lock(&vcrypto->ctrl_lock);
+ vc_ctrl_req = kzalloc(sizeof(*vc_ctrl_req), GFP_KERNEL);
+ if (!vc_ctrl_req) {
+ err = -ENOMEM;
+ goto out;
+ }
+
/* Pad ctrl header */
- ctrl = &vcrypto->ctrl;
+ ctrl = &vc_ctrl_req->ctrl;
ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION);
ctrl->header.algo = cpu_to_le32(alg);
/* Set the default dataqueue id to 0 */
ctrl->header.queue_id = 0;
- input = &vcrypto->input;
+ input = &vc_ctrl_req->input;
input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
/* Pad cipher's parameters */
sym_create_session = &ctrl->u.sym_create_session;
@@ -164,12 +170,12 @@ static int virtio_crypto_alg_skcipher_init_session(
sg_init_one(&inhdr, input, sizeof(*input));
sgs[num_out + num_in++] = &inhdr;
+ spin_lock(&vcrypto->ctrl_lock);
err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out,
num_in, vcrypto, GFP_ATOMIC);
if (err < 0) {
spin_unlock(&vcrypto->ctrl_lock);
- kfree_sensitive(cipher_key);
- return err;
+ goto out;
}
virtqueue_kick(vcrypto->ctrl_vq);
@@ -180,13 +186,13 @@ static int virtio_crypto_alg_skcipher_init_session(
while (!virtqueue_get_buf(vcrypto->ctrl_vq, &tmp) &&
!virtqueue_is_broken(vcrypto->ctrl_vq))
cpu_relax();
+ spin_unlock(&vcrypto->ctrl_lock);
if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) {
- spin_unlock(&vcrypto->ctrl_lock);
pr_err("virtio_crypto: Create session failed status: %u\n",
le32_to_cpu(input->status));
- kfree_sensitive(cipher_key);
- return -EINVAL;
+ err = -EINVAL;
+ goto out;
}
if (encrypt)
@@ -194,10 +200,11 @@ static int virtio_crypto_alg_skcipher_init_session(
else
ctx->dec_sess_info.session_id = le64_to_cpu(input->session_id);
- spin_unlock(&vcrypto->ctrl_lock);
-
+ err = 0;
+out:
+ kfree(vc_ctrl_req);
kfree_sensitive(cipher_key);
- return 0;
+ return err;
}
static int virtio_crypto_alg_skcipher_close_session(
@@ -212,12 +219,16 @@ static int virtio_crypto_alg_skcipher_close_session(
unsigned int num_out = 0, num_in = 0;
struct virtio_crypto_op_ctrl_req *ctrl;
struct virtio_crypto_inhdr *ctrl_status;
+ struct virtio_crypto_ctrl_request *vc_ctrl_req;
- spin_lock(&vcrypto->ctrl_lock);
- ctrl_status = &vcrypto->ctrl_status;
+ vc_ctrl_req = kzalloc(sizeof(*vc_ctrl_req), GFP_KERNEL);
+ if (!vc_ctrl_req)
+ return -ENOMEM;
+
+ ctrl_status = &vc_ctrl_req->ctrl_status;
ctrl_status->status = VIRTIO_CRYPTO_ERR;
/* Pad ctrl header */
- ctrl = &vcrypto->ctrl;
+ ctrl = &vc_ctrl_req->ctrl;
ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION);
/* Set the default virtqueue id to 0 */
ctrl->header.queue_id = 0;
@@ -236,28 +247,31 @@ static int virtio_crypto_alg_skcipher_close_session(
sg_init_one(&status_sg, &ctrl_status->status, sizeof(ctrl_status->status));
sgs[num_out + num_in++] = &status_sg;
+ spin_lock(&vcrypto->ctrl_lock);
err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out,
num_in, vcrypto, GFP_ATOMIC);
if (err < 0) {
spin_unlock(&vcrypto->ctrl_lock);
- return err;
+ goto out;
}
virtqueue_kick(vcrypto->ctrl_vq);
while (!virtqueue_get_buf(vcrypto->ctrl_vq, &tmp) &&
!virtqueue_is_broken(vcrypto->ctrl_vq))
cpu_relax();
+ spin_unlock(&vcrypto->ctrl_lock);
if (ctrl_status->status != VIRTIO_CRYPTO_OK) {
- spin_unlock(&vcrypto->ctrl_lock);
pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n",
ctrl_status->status, destroy_session->session_id);
return -EINVAL;
}
- spin_unlock(&vcrypto->ctrl_lock);
- return 0;
+ err = 0;
+out:
+ kfree(vc_ctrl_req);
+ return err;
}
static int virtio_crypto_alg_skcipher_init_sessions(
--
2.20.1
From: lei he <[email protected]>
For some akcipher operations(eg, decryption of pkcs1pad(rsa)),
the length of returned result maybe less than akcipher_req->dst_len,
we need to recalculate the actual dst_len through the virt-queue
protocol.
Cc: Michael S. Tsirkin <[email protected]>
Cc: Jason Wang <[email protected]>
Cc: Gonglei <[email protected]>
Signed-off-by: lei he <[email protected]>
Signed-off-by: zhenwei pi <[email protected]>
---
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
index 1e98502830cf..1892901d2a71 100644
--- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
@@ -90,9 +90,12 @@ static void virtio_crypto_dataq_akcipher_callback(struct virtio_crypto_request *
}
akcipher_req = vc_akcipher_req->akcipher_req;
- if (vc_akcipher_req->opcode != VIRTIO_CRYPTO_AKCIPHER_VERIFY)
+ if (vc_akcipher_req->opcode != VIRTIO_CRYPTO_AKCIPHER_VERIFY) {
+ /* actuall length maybe less than dst buffer */
+ akcipher_req->dst_len = len - sizeof(vc_req->status);
sg_copy_from_buffer(akcipher_req->dst, sg_nents(akcipher_req->dst),
vc_akcipher_req->dst_buf, akcipher_req->dst_len);
+ }
virtio_crypto_akcipher_finalize_req(vc_akcipher_req, akcipher_req, error);
}
--
2.20.1
Use temporary variable to make code easy to read and maintain.
/* Pad cipher's parameters */
vcrypto->ctrl.u.sym_create_session.op_type =
cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
vcrypto->ctrl.u.sym_create_session.u.cipher.para.algo =
vcrypto->ctrl.header.algo;
vcrypto->ctrl.u.sym_create_session.u.cipher.para.keylen =
cpu_to_le32(keylen);
vcrypto->ctrl.u.sym_create_session.u.cipher.para.op =
cpu_to_le32(op);
-->
sym_create_session = &ctrl->u.sym_create_session;
sym_create_session->op_type = cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
sym_create_session->u.cipher.para.algo = ctrl->header.algo;
sym_create_session->u.cipher.para.keylen = cpu_to_le32(keylen);
sym_create_session->u.cipher.para.op = cpu_to_le32(op);
The new style shows more obviously:
- the variable we want to operate.
- an assignment statement in a single line.
Cc: Michael S. Tsirkin <[email protected]>
Cc: Jason Wang <[email protected]>
Cc: Gonglei <[email protected]>
Signed-off-by: zhenwei pi <[email protected]>
---
.../virtio/virtio_crypto_akcipher_algs.c | 40 ++++++-----
.../virtio/virtio_crypto_skcipher_algs.c | 72 +++++++++----------
2 files changed, 59 insertions(+), 53 deletions(-)
diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
index f3ec9420215e..20901a263fc8 100644
--- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
@@ -106,23 +106,27 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher
unsigned int inlen;
int err;
unsigned int num_out = 0, num_in = 0;
+ struct virtio_crypto_op_ctrl_req *ctrl;
+ struct virtio_crypto_session_input *input;
pkey = kmemdup(key, keylen, GFP_ATOMIC);
if (!pkey)
return -ENOMEM;
spin_lock(&vcrypto->ctrl_lock);
- memcpy(&vcrypto->ctrl.header, header, sizeof(vcrypto->ctrl.header));
- memcpy(&vcrypto->ctrl.u, para, sizeof(vcrypto->ctrl.u));
- vcrypto->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
+ ctrl = &vcrypto->ctrl;
+ memcpy(&ctrl->header, header, sizeof(ctrl->header));
+ memcpy(&ctrl->u, para, sizeof(ctrl->u));
+ input = &vcrypto->input;
+ input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
- sg_init_one(&outhdr_sg, &vcrypto->ctrl, sizeof(vcrypto->ctrl));
+ sg_init_one(&outhdr_sg, ctrl, sizeof(*ctrl));
sgs[num_out++] = &outhdr_sg;
sg_init_one(&key_sg, pkey, keylen);
sgs[num_out++] = &key_sg;
- sg_init_one(&inhdr_sg, &vcrypto->input, sizeof(vcrypto->input));
+ sg_init_one(&inhdr_sg, input, sizeof(*input));
sgs[num_out + num_in++] = &inhdr_sg;
err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC);
@@ -134,12 +138,12 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher
!virtqueue_is_broken(vcrypto->ctrl_vq))
cpu_relax();
- if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) {
+ if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) {
err = -EINVAL;
goto out;
}
- ctx->session_id = le64_to_cpu(vcrypto->input.session_id);
+ ctx->session_id = le64_to_cpu(input->session_id);
ctx->session_valid = true;
err = 0;
@@ -149,7 +153,7 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher
if (err < 0)
pr_err("virtio_crypto: Create session failed status: %u\n",
- le32_to_cpu(vcrypto->input.status));
+ le32_to_cpu(input->status));
return err;
}
@@ -161,23 +165,27 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
struct virtio_crypto *vcrypto = ctx->vcrypto;
unsigned int num_out = 0, num_in = 0, inlen;
int err;
+ struct virtio_crypto_op_ctrl_req *ctrl;
+ struct virtio_crypto_inhdr *ctrl_status;
spin_lock(&vcrypto->ctrl_lock);
if (!ctx->session_valid) {
err = 0;
goto out;
}
- vcrypto->ctrl_status.status = VIRTIO_CRYPTO_ERR;
- vcrypto->ctrl.header.opcode = cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION);
- vcrypto->ctrl.header.queue_id = 0;
+ ctrl_status = &vcrypto->ctrl_status;
+ ctrl_status->status = VIRTIO_CRYPTO_ERR;
+ ctrl = &vcrypto->ctrl;
+ ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION);
+ ctrl->header.queue_id = 0;
- destroy_session = &vcrypto->ctrl.u.destroy_session;
+ destroy_session = &ctrl->u.destroy_session;
destroy_session->session_id = cpu_to_le64(ctx->session_id);
- sg_init_one(&outhdr_sg, &vcrypto->ctrl, sizeof(vcrypto->ctrl));
+ sg_init_one(&outhdr_sg, ctrl, sizeof(*ctrl));
sgs[num_out++] = &outhdr_sg;
- sg_init_one(&inhdr_sg, &vcrypto->ctrl_status.status, sizeof(vcrypto->ctrl_status.status));
+ sg_init_one(&inhdr_sg, &ctrl_status->status, sizeof(ctrl_status->status));
sgs[num_out + num_in++] = &inhdr_sg;
err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC);
@@ -189,7 +197,7 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
!virtqueue_is_broken(vcrypto->ctrl_vq))
cpu_relax();
- if (vcrypto->ctrl_status.status != VIRTIO_CRYPTO_OK) {
+ if (ctrl_status->status != VIRTIO_CRYPTO_OK) {
err = -EINVAL;
goto out;
}
@@ -201,7 +209,7 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
spin_unlock(&vcrypto->ctrl_lock);
if (err < 0) {
pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n",
- vcrypto->ctrl_status.status, destroy_session->session_id);
+ ctrl_status->status, destroy_session->session_id);
}
return err;
diff --git a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
index a618c46a52b8..e3c5bc8d6112 100644
--- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
@@ -123,6 +123,9 @@ static int virtio_crypto_alg_skcipher_init_session(
int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT : VIRTIO_CRYPTO_OP_DECRYPT;
int err;
unsigned int num_out = 0, num_in = 0;
+ struct virtio_crypto_op_ctrl_req *ctrl;
+ struct virtio_crypto_session_input *input;
+ struct virtio_crypto_sym_create_session_req *sym_create_session;
/*
* Avoid to do DMA from the stack, switch to using
@@ -135,24 +138,22 @@ static int virtio_crypto_alg_skcipher_init_session(
spin_lock(&vcrypto->ctrl_lock);
/* Pad ctrl header */
- vcrypto->ctrl.header.opcode =
- cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION);
- vcrypto->ctrl.header.algo = cpu_to_le32(alg);
+ ctrl = &vcrypto->ctrl;
+ ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION);
+ ctrl->header.algo = cpu_to_le32(alg);
/* Set the default dataqueue id to 0 */
- vcrypto->ctrl.header.queue_id = 0;
+ ctrl->header.queue_id = 0;
- vcrypto->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
+ input = &vcrypto->input;
+ input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
/* Pad cipher's parameters */
- vcrypto->ctrl.u.sym_create_session.op_type =
- cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
- vcrypto->ctrl.u.sym_create_session.u.cipher.para.algo =
- vcrypto->ctrl.header.algo;
- vcrypto->ctrl.u.sym_create_session.u.cipher.para.keylen =
- cpu_to_le32(keylen);
- vcrypto->ctrl.u.sym_create_session.u.cipher.para.op =
- cpu_to_le32(op);
-
- sg_init_one(&outhdr, &vcrypto->ctrl, sizeof(vcrypto->ctrl));
+ sym_create_session = &ctrl->u.sym_create_session;
+ sym_create_session->op_type = cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
+ sym_create_session->u.cipher.para.algo = ctrl->header.algo;
+ sym_create_session->u.cipher.para.keylen = cpu_to_le32(keylen);
+ sym_create_session->u.cipher.para.op = cpu_to_le32(op);
+
+ sg_init_one(&outhdr, ctrl, sizeof(*ctrl));
sgs[num_out++] = &outhdr;
/* Set key */
@@ -160,7 +161,7 @@ static int virtio_crypto_alg_skcipher_init_session(
sgs[num_out++] = &key_sg;
/* Return status and session id back */
- sg_init_one(&inhdr, &vcrypto->input, sizeof(vcrypto->input));
+ sg_init_one(&inhdr, input, sizeof(*input));
sgs[num_out + num_in++] = &inhdr;
err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out,
@@ -180,20 +181,18 @@ static int virtio_crypto_alg_skcipher_init_session(
!virtqueue_is_broken(vcrypto->ctrl_vq))
cpu_relax();
- if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) {
+ if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) {
spin_unlock(&vcrypto->ctrl_lock);
pr_err("virtio_crypto: Create session failed status: %u\n",
- le32_to_cpu(vcrypto->input.status));
+ le32_to_cpu(input->status));
kfree_sensitive(cipher_key);
return -EINVAL;
}
if (encrypt)
- ctx->enc_sess_info.session_id =
- le64_to_cpu(vcrypto->input.session_id);
+ ctx->enc_sess_info.session_id = le64_to_cpu(input->session_id);
else
- ctx->dec_sess_info.session_id =
- le64_to_cpu(vcrypto->input.session_id);
+ ctx->dec_sess_info.session_id = le64_to_cpu(input->session_id);
spin_unlock(&vcrypto->ctrl_lock);
@@ -211,30 +210,30 @@ static int virtio_crypto_alg_skcipher_close_session(
struct virtio_crypto *vcrypto = ctx->vcrypto;
int err;
unsigned int num_out = 0, num_in = 0;
+ struct virtio_crypto_op_ctrl_req *ctrl;
+ struct virtio_crypto_inhdr *ctrl_status;
spin_lock(&vcrypto->ctrl_lock);
- vcrypto->ctrl_status.status = VIRTIO_CRYPTO_ERR;
+ ctrl_status = &vcrypto->ctrl_status;
+ ctrl_status->status = VIRTIO_CRYPTO_ERR;
/* Pad ctrl header */
- vcrypto->ctrl.header.opcode =
- cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION);
+ ctrl = &vcrypto->ctrl;
+ ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION);
/* Set the default virtqueue id to 0 */
- vcrypto->ctrl.header.queue_id = 0;
+ ctrl->header.queue_id = 0;
- destroy_session = &vcrypto->ctrl.u.destroy_session;
+ destroy_session = &ctrl->u.destroy_session;
if (encrypt)
- destroy_session->session_id =
- cpu_to_le64(ctx->enc_sess_info.session_id);
+ destroy_session->session_id = cpu_to_le64(ctx->enc_sess_info.session_id);
else
- destroy_session->session_id =
- cpu_to_le64(ctx->dec_sess_info.session_id);
+ destroy_session->session_id = cpu_to_le64(ctx->dec_sess_info.session_id);
- sg_init_one(&outhdr, &vcrypto->ctrl, sizeof(vcrypto->ctrl));
+ sg_init_one(&outhdr, ctrl, sizeof(*ctrl));
sgs[num_out++] = &outhdr;
/* Return status and session id back */
- sg_init_one(&status_sg, &vcrypto->ctrl_status.status,
- sizeof(vcrypto->ctrl_status.status));
+ sg_init_one(&status_sg, &ctrl_status->status, sizeof(ctrl_status->status));
sgs[num_out + num_in++] = &status_sg;
err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out,
@@ -249,11 +248,10 @@ static int virtio_crypto_alg_skcipher_close_session(
!virtqueue_is_broken(vcrypto->ctrl_vq))
cpu_relax();
- if (vcrypto->ctrl_status.status != VIRTIO_CRYPTO_OK) {
+ if (ctrl_status->status != VIRTIO_CRYPTO_OK) {
spin_unlock(&vcrypto->ctrl_lock);
pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n",
- vcrypto->ctrl_status.status,
- destroy_session->session_id);
+ ctrl_status->status, destroy_session->session_id);
return -EINVAL;
}
--
2.20.1
Hi zhenwei,
url: https://github.com/intel-lab-lkp/linux/commits/zhenwei-pi/virtio-crypto-Improve-performance/20220424-184732
base: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: i386-randconfig-m021 (https://download.01.org/0day-ci/archive/20220424/[email protected]/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
smatch warnings:
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:165 virtio_crypto_alg_akcipher_init_session() error: potentially dereferencing uninitialized 'input'.
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:230 virtio_crypto_alg_akcipher_close_session() error: uninitialized symbol 'vc_ctrl_req'.
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:232 virtio_crypto_alg_akcipher_close_session() error: potentially dereferencing uninitialized 'ctrl_status'.
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:232 virtio_crypto_alg_akcipher_close_session() error: potentially dereferencing uninitialized 'destroy_session'.
vim +/input +165 drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
59ca6c93387d32 zhenwei pi 2022-03-02 99 static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher_ctx *ctx,
59ca6c93387d32 zhenwei pi 2022-03-02 100 struct virtio_crypto_ctrl_header *header, void *para,
59ca6c93387d32 zhenwei pi 2022-03-02 101 const uint8_t *key, unsigned int keylen)
59ca6c93387d32 zhenwei pi 2022-03-02 102 {
59ca6c93387d32 zhenwei pi 2022-03-02 103 struct scatterlist outhdr_sg, key_sg, inhdr_sg, *sgs[3];
59ca6c93387d32 zhenwei pi 2022-03-02 104 struct virtio_crypto *vcrypto = ctx->vcrypto;
59ca6c93387d32 zhenwei pi 2022-03-02 105 uint8_t *pkey;
59ca6c93387d32 zhenwei pi 2022-03-02 106 unsigned int inlen;
59ca6c93387d32 zhenwei pi 2022-03-02 107 int err;
59ca6c93387d32 zhenwei pi 2022-03-02 108 unsigned int num_out = 0, num_in = 0;
bb26cab9a7c25d zhenwei pi 2022-04-24 109 struct virtio_crypto_op_ctrl_req *ctrl;
bb26cab9a7c25d zhenwei pi 2022-04-24 110 struct virtio_crypto_session_input *input;
286da9ed04239c zhenwei pi 2022-04-24 111 struct virtio_crypto_ctrl_request *vc_ctrl_req;
59ca6c93387d32 zhenwei pi 2022-03-02 112
59ca6c93387d32 zhenwei pi 2022-03-02 113 pkey = kmemdup(key, keylen, GFP_ATOMIC);
59ca6c93387d32 zhenwei pi 2022-03-02 114 if (!pkey)
59ca6c93387d32 zhenwei pi 2022-03-02 115 return -ENOMEM;
59ca6c93387d32 zhenwei pi 2022-03-02 116
286da9ed04239c zhenwei pi 2022-04-24 117 vc_ctrl_req = kzalloc(sizeof(*vc_ctrl_req), GFP_KERNEL);
286da9ed04239c zhenwei pi 2022-04-24 118 if (!vc_ctrl_req) {
286da9ed04239c zhenwei pi 2022-04-24 119 err = -ENOMEM;
286da9ed04239c zhenwei pi 2022-04-24 120 goto out;
286da9ed04239c zhenwei pi 2022-04-24 121 }
286da9ed04239c zhenwei pi 2022-04-24 122
286da9ed04239c zhenwei pi 2022-04-24 123 ctrl = &vc_ctrl_req->ctrl;
bb26cab9a7c25d zhenwei pi 2022-04-24 124 memcpy(&ctrl->header, header, sizeof(ctrl->header));
bb26cab9a7c25d zhenwei pi 2022-04-24 125 memcpy(&ctrl->u, para, sizeof(ctrl->u));
286da9ed04239c zhenwei pi 2022-04-24 126 input = &vc_ctrl_req->input;
bb26cab9a7c25d zhenwei pi 2022-04-24 127 input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
59ca6c93387d32 zhenwei pi 2022-03-02 128
bb26cab9a7c25d zhenwei pi 2022-04-24 129 sg_init_one(&outhdr_sg, ctrl, sizeof(*ctrl));
59ca6c93387d32 zhenwei pi 2022-03-02 130 sgs[num_out++] = &outhdr_sg;
59ca6c93387d32 zhenwei pi 2022-03-02 131
59ca6c93387d32 zhenwei pi 2022-03-02 132 sg_init_one(&key_sg, pkey, keylen);
59ca6c93387d32 zhenwei pi 2022-03-02 133 sgs[num_out++] = &key_sg;
59ca6c93387d32 zhenwei pi 2022-03-02 134
bb26cab9a7c25d zhenwei pi 2022-04-24 135 sg_init_one(&inhdr_sg, input, sizeof(*input));
59ca6c93387d32 zhenwei pi 2022-03-02 136 sgs[num_out + num_in++] = &inhdr_sg;
59ca6c93387d32 zhenwei pi 2022-03-02 137
286da9ed04239c zhenwei pi 2022-04-24 138 spin_lock(&vcrypto->ctrl_lock);
59ca6c93387d32 zhenwei pi 2022-03-02 139 err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC);
286da9ed04239c zhenwei pi 2022-04-24 140 if (err < 0) {
286da9ed04239c zhenwei pi 2022-04-24 141 spin_unlock(&vcrypto->ctrl_lock);
59ca6c93387d32 zhenwei pi 2022-03-02 142 goto out;
286da9ed04239c zhenwei pi 2022-04-24 143 }
59ca6c93387d32 zhenwei pi 2022-03-02 144
59ca6c93387d32 zhenwei pi 2022-03-02 145 virtqueue_kick(vcrypto->ctrl_vq);
59ca6c93387d32 zhenwei pi 2022-03-02 146 while (!virtqueue_get_buf(vcrypto->ctrl_vq, &inlen) &&
59ca6c93387d32 zhenwei pi 2022-03-02 147 !virtqueue_is_broken(vcrypto->ctrl_vq))
59ca6c93387d32 zhenwei pi 2022-03-02 148 cpu_relax();
286da9ed04239c zhenwei pi 2022-04-24 149 spin_unlock(&vcrypto->ctrl_lock);
59ca6c93387d32 zhenwei pi 2022-03-02 150
bb26cab9a7c25d zhenwei pi 2022-04-24 151 if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) {
59ca6c93387d32 zhenwei pi 2022-03-02 152 err = -EINVAL;
59ca6c93387d32 zhenwei pi 2022-03-02 153 goto out;
59ca6c93387d32 zhenwei pi 2022-03-02 154 }
59ca6c93387d32 zhenwei pi 2022-03-02 155
bb26cab9a7c25d zhenwei pi 2022-04-24 156 ctx->session_id = le64_to_cpu(input->session_id);
59ca6c93387d32 zhenwei pi 2022-03-02 157 ctx->session_valid = true;
59ca6c93387d32 zhenwei pi 2022-03-02 158 err = 0;
59ca6c93387d32 zhenwei pi 2022-03-02 159
59ca6c93387d32 zhenwei pi 2022-03-02 160 out:
286da9ed04239c zhenwei pi 2022-04-24 161 kfree(vc_ctrl_req);
59ca6c93387d32 zhenwei pi 2022-03-02 162 kfree_sensitive(pkey);
59ca6c93387d32 zhenwei pi 2022-03-02 163
59ca6c93387d32 zhenwei pi 2022-03-02 164 if (err < 0)
59ca6c93387d32 zhenwei pi 2022-03-02 @165 pr_err("virtio_crypto: Create session failed status: %u\n",
bb26cab9a7c25d zhenwei pi 2022-04-24 166 le32_to_cpu(input->status));
goto out is always suspicious. "input" is not initialized.
59ca6c93387d32 zhenwei pi 2022-03-02 167
59ca6c93387d32 zhenwei pi 2022-03-02 168 return err;
59ca6c93387d32 zhenwei pi 2022-03-02 169 }
59ca6c93387d32 zhenwei pi 2022-03-02 170
59ca6c93387d32 zhenwei pi 2022-03-02 171 static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akcipher_ctx *ctx)
59ca6c93387d32 zhenwei pi 2022-03-02 172 {
59ca6c93387d32 zhenwei pi 2022-03-02 173 struct scatterlist outhdr_sg, inhdr_sg, *sgs[2];
59ca6c93387d32 zhenwei pi 2022-03-02 174 struct virtio_crypto_destroy_session_req *destroy_session;
59ca6c93387d32 zhenwei pi 2022-03-02 175 struct virtio_crypto *vcrypto = ctx->vcrypto;
59ca6c93387d32 zhenwei pi 2022-03-02 176 unsigned int num_out = 0, num_in = 0, inlen;
59ca6c93387d32 zhenwei pi 2022-03-02 177 int err;
bb26cab9a7c25d zhenwei pi 2022-04-24 178 struct virtio_crypto_op_ctrl_req *ctrl;
bb26cab9a7c25d zhenwei pi 2022-04-24 179 struct virtio_crypto_inhdr *ctrl_status;
286da9ed04239c zhenwei pi 2022-04-24 180 struct virtio_crypto_ctrl_request *vc_ctrl_req;
59ca6c93387d32 zhenwei pi 2022-03-02 181
59ca6c93387d32 zhenwei pi 2022-03-02 182 if (!ctx->session_valid) {
59ca6c93387d32 zhenwei pi 2022-03-02 183 err = 0;
59ca6c93387d32 zhenwei pi 2022-03-02 184 goto out;
59ca6c93387d32 zhenwei pi 2022-03-02 185 }
286da9ed04239c zhenwei pi 2022-04-24 186
286da9ed04239c zhenwei pi 2022-04-24 187 vc_ctrl_req = kzalloc(sizeof(*vc_ctrl_req), GFP_KERNEL);
286da9ed04239c zhenwei pi 2022-04-24 188 if (!vc_ctrl_req) {
286da9ed04239c zhenwei pi 2022-04-24 189 err = -ENOMEM;
286da9ed04239c zhenwei pi 2022-04-24 190 goto out;
286da9ed04239c zhenwei pi 2022-04-24 191 }
286da9ed04239c zhenwei pi 2022-04-24 192
286da9ed04239c zhenwei pi 2022-04-24 193 ctrl_status = &vc_ctrl_req->ctrl_status;
bb26cab9a7c25d zhenwei pi 2022-04-24 194 ctrl_status->status = VIRTIO_CRYPTO_ERR;
286da9ed04239c zhenwei pi 2022-04-24 195 ctrl = &vc_ctrl_req->ctrl;
bb26cab9a7c25d zhenwei pi 2022-04-24 196 ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION);
bb26cab9a7c25d zhenwei pi 2022-04-24 197 ctrl->header.queue_id = 0;
59ca6c93387d32 zhenwei pi 2022-03-02 198
bb26cab9a7c25d zhenwei pi 2022-04-24 199 destroy_session = &ctrl->u.destroy_session;
59ca6c93387d32 zhenwei pi 2022-03-02 200 destroy_session->session_id = cpu_to_le64(ctx->session_id);
59ca6c93387d32 zhenwei pi 2022-03-02 201
bb26cab9a7c25d zhenwei pi 2022-04-24 202 sg_init_one(&outhdr_sg, ctrl, sizeof(*ctrl));
59ca6c93387d32 zhenwei pi 2022-03-02 203 sgs[num_out++] = &outhdr_sg;
59ca6c93387d32 zhenwei pi 2022-03-02 204
bb26cab9a7c25d zhenwei pi 2022-04-24 205 sg_init_one(&inhdr_sg, &ctrl_status->status, sizeof(ctrl_status->status));
59ca6c93387d32 zhenwei pi 2022-03-02 206 sgs[num_out + num_in++] = &inhdr_sg;
59ca6c93387d32 zhenwei pi 2022-03-02 207
286da9ed04239c zhenwei pi 2022-04-24 208 spin_lock(&vcrypto->ctrl_lock);
59ca6c93387d32 zhenwei pi 2022-03-02 209 err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC);
286da9ed04239c zhenwei pi 2022-04-24 210 if (err < 0) {
286da9ed04239c zhenwei pi 2022-04-24 211 spin_unlock(&vcrypto->ctrl_lock);
59ca6c93387d32 zhenwei pi 2022-03-02 212 goto out;
286da9ed04239c zhenwei pi 2022-04-24 213 }
59ca6c93387d32 zhenwei pi 2022-03-02 214
59ca6c93387d32 zhenwei pi 2022-03-02 215 virtqueue_kick(vcrypto->ctrl_vq);
59ca6c93387d32 zhenwei pi 2022-03-02 216 while (!virtqueue_get_buf(vcrypto->ctrl_vq, &inlen) &&
59ca6c93387d32 zhenwei pi 2022-03-02 217 !virtqueue_is_broken(vcrypto->ctrl_vq))
59ca6c93387d32 zhenwei pi 2022-03-02 218 cpu_relax();
286da9ed04239c zhenwei pi 2022-04-24 219 spin_unlock(&vcrypto->ctrl_lock);
59ca6c93387d32 zhenwei pi 2022-03-02 220
bb26cab9a7c25d zhenwei pi 2022-04-24 221 if (ctrl_status->status != VIRTIO_CRYPTO_OK) {
59ca6c93387d32 zhenwei pi 2022-03-02 222 err = -EINVAL;
59ca6c93387d32 zhenwei pi 2022-03-02 223 goto out;
59ca6c93387d32 zhenwei pi 2022-03-02 224 }
59ca6c93387d32 zhenwei pi 2022-03-02 225
59ca6c93387d32 zhenwei pi 2022-03-02 226 err = 0;
59ca6c93387d32 zhenwei pi 2022-03-02 227 ctx->session_valid = false;
59ca6c93387d32 zhenwei pi 2022-03-02 228
59ca6c93387d32 zhenwei pi 2022-03-02 229 out:
286da9ed04239c zhenwei pi 2022-04-24 @230 kfree(vc_ctrl_req);
59ca6c93387d32 zhenwei pi 2022-03-02 231 if (err < 0) {
59ca6c93387d32 zhenwei pi 2022-03-02 @232 pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n",
bb26cab9a7c25d zhenwei pi 2022-04-24 233 ctrl_status->status, destroy_session->session_id);
More canonical goto out bugs.
59ca6c93387d32 zhenwei pi 2022-03-02 234 }
59ca6c93387d32 zhenwei pi 2022-03-02 235
59ca6c93387d32 zhenwei pi 2022-03-02 236 return err;
59ca6c93387d32 zhenwei pi 2022-03-02 237 }
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On Sun, Apr 24, 2022 at 6:45 PM zhenwei pi <[email protected]> wrote:
>
> Use temporary variable to make code easy to read and maintain.
> /* Pad cipher's parameters */
> vcrypto->ctrl.u.sym_create_session.op_type =
> cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
> vcrypto->ctrl.u.sym_create_session.u.cipher.para.algo =
> vcrypto->ctrl.header.algo;
> vcrypto->ctrl.u.sym_create_session.u.cipher.para.keylen =
> cpu_to_le32(keylen);
> vcrypto->ctrl.u.sym_create_session.u.cipher.para.op =
> cpu_to_le32(op);
> -->
> sym_create_session = &ctrl->u.sym_create_session;
> sym_create_session->op_type = cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
> sym_create_session->u.cipher.para.algo = ctrl->header.algo;
> sym_create_session->u.cipher.para.keylen = cpu_to_le32(keylen);
> sym_create_session->u.cipher.para.op = cpu_to_le32(op);
>
> The new style shows more obviously:
> - the variable we want to operate.
> - an assignment statement in a single line.
Still hundreds of lines of changes, I'd leave this change to other
mainters to dedice.
Thanks
>
> Cc: Michael S. Tsirkin <[email protected]>
> Cc: Jason Wang <[email protected]>
> Cc: Gonglei <[email protected]>
> Signed-off-by: zhenwei pi <[email protected]>
> ---
> .../virtio/virtio_crypto_akcipher_algs.c | 40 ++++++-----
> .../virtio/virtio_crypto_skcipher_algs.c | 72 +++++++++----------
> 2 files changed, 59 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
> index f3ec9420215e..20901a263fc8 100644
> --- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
> +++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
> @@ -106,23 +106,27 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher
> unsigned int inlen;
> int err;
> unsigned int num_out = 0, num_in = 0;
> + struct virtio_crypto_op_ctrl_req *ctrl;
> + struct virtio_crypto_session_input *input;
>
> pkey = kmemdup(key, keylen, GFP_ATOMIC);
> if (!pkey)
> return -ENOMEM;
>
> spin_lock(&vcrypto->ctrl_lock);
> - memcpy(&vcrypto->ctrl.header, header, sizeof(vcrypto->ctrl.header));
> - memcpy(&vcrypto->ctrl.u, para, sizeof(vcrypto->ctrl.u));
> - vcrypto->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
> + ctrl = &vcrypto->ctrl;
> + memcpy(&ctrl->header, header, sizeof(ctrl->header));
> + memcpy(&ctrl->u, para, sizeof(ctrl->u));
> + input = &vcrypto->input;
> + input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
>
> - sg_init_one(&outhdr_sg, &vcrypto->ctrl, sizeof(vcrypto->ctrl));
> + sg_init_one(&outhdr_sg, ctrl, sizeof(*ctrl));
> sgs[num_out++] = &outhdr_sg;
>
> sg_init_one(&key_sg, pkey, keylen);
> sgs[num_out++] = &key_sg;
>
> - sg_init_one(&inhdr_sg, &vcrypto->input, sizeof(vcrypto->input));
> + sg_init_one(&inhdr_sg, input, sizeof(*input));
> sgs[num_out + num_in++] = &inhdr_sg;
>
> err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC);
> @@ -134,12 +138,12 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher
> !virtqueue_is_broken(vcrypto->ctrl_vq))
> cpu_relax();
>
> - if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) {
> + if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) {
> err = -EINVAL;
> goto out;
> }
>
> - ctx->session_id = le64_to_cpu(vcrypto->input.session_id);
> + ctx->session_id = le64_to_cpu(input->session_id);
> ctx->session_valid = true;
> err = 0;
>
> @@ -149,7 +153,7 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher
>
> if (err < 0)
> pr_err("virtio_crypto: Create session failed status: %u\n",
> - le32_to_cpu(vcrypto->input.status));
> + le32_to_cpu(input->status));
>
> return err;
> }
> @@ -161,23 +165,27 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
> struct virtio_crypto *vcrypto = ctx->vcrypto;
> unsigned int num_out = 0, num_in = 0, inlen;
> int err;
> + struct virtio_crypto_op_ctrl_req *ctrl;
> + struct virtio_crypto_inhdr *ctrl_status;
>
> spin_lock(&vcrypto->ctrl_lock);
> if (!ctx->session_valid) {
> err = 0;
> goto out;
> }
> - vcrypto->ctrl_status.status = VIRTIO_CRYPTO_ERR;
> - vcrypto->ctrl.header.opcode = cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION);
> - vcrypto->ctrl.header.queue_id = 0;
> + ctrl_status = &vcrypto->ctrl_status;
> + ctrl_status->status = VIRTIO_CRYPTO_ERR;
> + ctrl = &vcrypto->ctrl;
> + ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION);
> + ctrl->header.queue_id = 0;
>
> - destroy_session = &vcrypto->ctrl.u.destroy_session;
> + destroy_session = &ctrl->u.destroy_session;
> destroy_session->session_id = cpu_to_le64(ctx->session_id);
>
> - sg_init_one(&outhdr_sg, &vcrypto->ctrl, sizeof(vcrypto->ctrl));
> + sg_init_one(&outhdr_sg, ctrl, sizeof(*ctrl));
> sgs[num_out++] = &outhdr_sg;
>
> - sg_init_one(&inhdr_sg, &vcrypto->ctrl_status.status, sizeof(vcrypto->ctrl_status.status));
> + sg_init_one(&inhdr_sg, &ctrl_status->status, sizeof(ctrl_status->status));
> sgs[num_out + num_in++] = &inhdr_sg;
>
> err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC);
> @@ -189,7 +197,7 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
> !virtqueue_is_broken(vcrypto->ctrl_vq))
> cpu_relax();
>
> - if (vcrypto->ctrl_status.status != VIRTIO_CRYPTO_OK) {
> + if (ctrl_status->status != VIRTIO_CRYPTO_OK) {
> err = -EINVAL;
> goto out;
> }
> @@ -201,7 +209,7 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
> spin_unlock(&vcrypto->ctrl_lock);
> if (err < 0) {
> pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n",
> - vcrypto->ctrl_status.status, destroy_session->session_id);
> + ctrl_status->status, destroy_session->session_id);
> }
>
> return err;
> diff --git a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
> index a618c46a52b8..e3c5bc8d6112 100644
> --- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
> +++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
> @@ -123,6 +123,9 @@ static int virtio_crypto_alg_skcipher_init_session(
> int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT : VIRTIO_CRYPTO_OP_DECRYPT;
> int err;
> unsigned int num_out = 0, num_in = 0;
> + struct virtio_crypto_op_ctrl_req *ctrl;
> + struct virtio_crypto_session_input *input;
> + struct virtio_crypto_sym_create_session_req *sym_create_session;
>
> /*
> * Avoid to do DMA from the stack, switch to using
> @@ -135,24 +138,22 @@ static int virtio_crypto_alg_skcipher_init_session(
>
> spin_lock(&vcrypto->ctrl_lock);
> /* Pad ctrl header */
> - vcrypto->ctrl.header.opcode =
> - cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION);
> - vcrypto->ctrl.header.algo = cpu_to_le32(alg);
> + ctrl = &vcrypto->ctrl;
> + ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION);
> + ctrl->header.algo = cpu_to_le32(alg);
> /* Set the default dataqueue id to 0 */
> - vcrypto->ctrl.header.queue_id = 0;
> + ctrl->header.queue_id = 0;
>
> - vcrypto->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
> + input = &vcrypto->input;
> + input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
> /* Pad cipher's parameters */
> - vcrypto->ctrl.u.sym_create_session.op_type =
> - cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
> - vcrypto->ctrl.u.sym_create_session.u.cipher.para.algo =
> - vcrypto->ctrl.header.algo;
> - vcrypto->ctrl.u.sym_create_session.u.cipher.para.keylen =
> - cpu_to_le32(keylen);
> - vcrypto->ctrl.u.sym_create_session.u.cipher.para.op =
> - cpu_to_le32(op);
> -
> - sg_init_one(&outhdr, &vcrypto->ctrl, sizeof(vcrypto->ctrl));
> + sym_create_session = &ctrl->u.sym_create_session;
> + sym_create_session->op_type = cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
> + sym_create_session->u.cipher.para.algo = ctrl->header.algo;
> + sym_create_session->u.cipher.para.keylen = cpu_to_le32(keylen);
> + sym_create_session->u.cipher.para.op = cpu_to_le32(op);
> +
> + sg_init_one(&outhdr, ctrl, sizeof(*ctrl));
> sgs[num_out++] = &outhdr;
>
> /* Set key */
> @@ -160,7 +161,7 @@ static int virtio_crypto_alg_skcipher_init_session(
> sgs[num_out++] = &key_sg;
>
> /* Return status and session id back */
> - sg_init_one(&inhdr, &vcrypto->input, sizeof(vcrypto->input));
> + sg_init_one(&inhdr, input, sizeof(*input));
> sgs[num_out + num_in++] = &inhdr;
>
> err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out,
> @@ -180,20 +181,18 @@ static int virtio_crypto_alg_skcipher_init_session(
> !virtqueue_is_broken(vcrypto->ctrl_vq))
> cpu_relax();
>
> - if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) {
> + if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) {
> spin_unlock(&vcrypto->ctrl_lock);
> pr_err("virtio_crypto: Create session failed status: %u\n",
> - le32_to_cpu(vcrypto->input.status));
> + le32_to_cpu(input->status));
> kfree_sensitive(cipher_key);
> return -EINVAL;
> }
>
> if (encrypt)
> - ctx->enc_sess_info.session_id =
> - le64_to_cpu(vcrypto->input.session_id);
> + ctx->enc_sess_info.session_id = le64_to_cpu(input->session_id);
> else
> - ctx->dec_sess_info.session_id =
> - le64_to_cpu(vcrypto->input.session_id);
> + ctx->dec_sess_info.session_id = le64_to_cpu(input->session_id);
>
> spin_unlock(&vcrypto->ctrl_lock);
>
> @@ -211,30 +210,30 @@ static int virtio_crypto_alg_skcipher_close_session(
> struct virtio_crypto *vcrypto = ctx->vcrypto;
> int err;
> unsigned int num_out = 0, num_in = 0;
> + struct virtio_crypto_op_ctrl_req *ctrl;
> + struct virtio_crypto_inhdr *ctrl_status;
>
> spin_lock(&vcrypto->ctrl_lock);
> - vcrypto->ctrl_status.status = VIRTIO_CRYPTO_ERR;
> + ctrl_status = &vcrypto->ctrl_status;
> + ctrl_status->status = VIRTIO_CRYPTO_ERR;
> /* Pad ctrl header */
> - vcrypto->ctrl.header.opcode =
> - cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION);
> + ctrl = &vcrypto->ctrl;
> + ctrl->header.opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION);
> /* Set the default virtqueue id to 0 */
> - vcrypto->ctrl.header.queue_id = 0;
> + ctrl->header.queue_id = 0;
>
> - destroy_session = &vcrypto->ctrl.u.destroy_session;
> + destroy_session = &ctrl->u.destroy_session;
>
> if (encrypt)
> - destroy_session->session_id =
> - cpu_to_le64(ctx->enc_sess_info.session_id);
> + destroy_session->session_id = cpu_to_le64(ctx->enc_sess_info.session_id);
> else
> - destroy_session->session_id =
> - cpu_to_le64(ctx->dec_sess_info.session_id);
> + destroy_session->session_id = cpu_to_le64(ctx->dec_sess_info.session_id);
>
> - sg_init_one(&outhdr, &vcrypto->ctrl, sizeof(vcrypto->ctrl));
> + sg_init_one(&outhdr, ctrl, sizeof(*ctrl));
> sgs[num_out++] = &outhdr;
>
> /* Return status and session id back */
> - sg_init_one(&status_sg, &vcrypto->ctrl_status.status,
> - sizeof(vcrypto->ctrl_status.status));
> + sg_init_one(&status_sg, &ctrl_status->status, sizeof(ctrl_status->status));
> sgs[num_out + num_in++] = &status_sg;
>
> err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out,
> @@ -249,11 +248,10 @@ static int virtio_crypto_alg_skcipher_close_session(
> !virtqueue_is_broken(vcrypto->ctrl_vq))
> cpu_relax();
>
> - if (vcrypto->ctrl_status.status != VIRTIO_CRYPTO_OK) {
> + if (ctrl_status->status != VIRTIO_CRYPTO_OK) {
> spin_unlock(&vcrypto->ctrl_lock);
> pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n",
> - vcrypto->ctrl_status.status,
> - destroy_session->session_id);
> + ctrl_status->status, destroy_session->session_id);
>
> return -EINVAL;
> }
> --
> 2.20.1
>