* [PATCH v4 0/5] virtio-crypto: Improve performance
@ 2022-04-24 10:41 ` zhenwei pi
0 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-04-24 10:41 UTC (permalink / raw)
To: arei.gonglei, mst, jasowang
Cc: helei.sig11, linux-kernel, zhenwei pi, virtualization,
linux-crypto, davem, herbert
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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v4 0/5] virtio-crypto: Improve performance
@ 2022-04-24 10:41 ` zhenwei pi
0 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-04-24 10:41 UTC (permalink / raw)
To: arei.gonglei, mst, jasowang
Cc: herbert, linux-kernel, virtualization, linux-crypto, helei.sig11,
davem, zhenwei pi
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v4 1/5] virtio-crypto: change code style
2022-04-24 10:41 ` zhenwei pi
@ 2022-04-24 10:41 ` zhenwei pi
-1 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-04-24 10:41 UTC (permalink / raw)
To: arei.gonglei, mst, jasowang
Cc: helei.sig11, linux-kernel, zhenwei pi, virtualization,
linux-crypto, davem, herbert
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 <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
.../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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v4 1/5] virtio-crypto: change code style
@ 2022-04-24 10:41 ` zhenwei pi
0 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-04-24 10:41 UTC (permalink / raw)
To: arei.gonglei, mst, jasowang
Cc: herbert, linux-kernel, virtualization, linux-crypto, helei.sig11,
davem, zhenwei pi
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 <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
.../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
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v4 2/5] virtio-crypto: use private buffer for control request
2022-04-24 10:41 ` zhenwei pi
@ 2022-04-24 10:41 ` zhenwei pi
-1 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-04-24 10:41 UTC (permalink / raw)
To: arei.gonglei, mst, jasowang
Cc: herbert, linux-kernel, virtualization, linux-crypto, helei.sig11,
davem, zhenwei pi
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 <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
.../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
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v4 2/5] virtio-crypto: use private buffer for control request
@ 2022-04-24 10:41 ` zhenwei pi
0 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-04-24 10:41 UTC (permalink / raw)
To: arei.gonglei, mst, jasowang
Cc: helei.sig11, linux-kernel, zhenwei pi, virtualization,
linux-crypto, davem, herbert
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 <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
.../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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v4 3/5] virtio-crypto: wait ctrl queue instead of busy polling
2022-04-24 10:41 ` zhenwei pi
@ 2022-04-24 10:41 ` zhenwei pi
-1 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-04-24 10:41 UTC (permalink / raw)
To: arei.gonglei, mst, jasowang
Cc: herbert, linux-kernel, virtualization, linux-crypto, helei.sig11,
davem, zhenwei pi
Originally, after submitting request into virtio crypto control
queue, the guest side polls the result from the virt queue. This
works like following:
CPU0 CPU1 ... CPUx CPUy
| | | |
\ \ / /
\--------spin_lock(&vcrypto->ctrl_lock)-------/
|
virtqueue add & kick
|
busy poll virtqueue
|
spin_unlock(&vcrypto->ctrl_lock)
...
There are two problems:
1, The queue depth is always 1, the performance of a virtio crypto
device gets limited. Multi user processes share a single control
queue, and hit spin lock race from control queue. Test on Intel
Platinum 8260, a single worker gets ~35K/s create/close session
operations, and 8 workers get ~40K/s operations with 800% CPU
utilization.
2, The control request is supposed to get handled immediately, but
in the current implementation of QEMU(v6.2), the vCPU thread kicks
another thread to do this work, the latency also gets unstable.
Tracking latency of virtio_crypto_alg_akcipher_close_session in 5s:
usecs : count distribution
0 -> 1 : 0 | |
2 -> 3 : 7 | |
4 -> 7 : 72 | |
8 -> 15 : 186485 |************************|
16 -> 31 : 687 | |
32 -> 63 : 5 | |
64 -> 127 : 3 | |
128 -> 255 : 1 | |
256 -> 511 : 0 | |
512 -> 1023 : 0 | |
1024 -> 2047 : 0 | |
2048 -> 4095 : 0 | |
4096 -> 8191 : 0 | |
8192 -> 16383 : 2 | |
This means that a CPU may hold vcrypto->ctrl_lock as long as 8192~16383us.
To improve the performance of control queue, a request on control queue
waits completion instead of busy polling to reduce lock racing, and gets
completed by control queue callback.
CPU0 CPU1 ... CPUx CPUy
| | | |
\ \ / /
\--------spin_lock(&vcrypto->ctrl_lock)-------/
|
virtqueue add & kick
|
---------spin_unlock(&vcrypto->ctrl_lock)------
/ / \ \
| | | |
wait wait wait wait
Test this patch, the guest side get ~200K/s operations with 300% CPU
utilization.
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
.../virtio/virtio_crypto_akcipher_algs.c | 29 ++---------
drivers/crypto/virtio/virtio_crypto_common.h | 4 ++
drivers/crypto/virtio/virtio_crypto_core.c | 52 ++++++++++++++++++-
.../virtio/virtio_crypto_skcipher_algs.c | 34 ++----------
4 files changed, 64 insertions(+), 55 deletions(-)
diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
index 509884e8b201..1e98502830cf 100644
--- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
@@ -103,7 +103,6 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher
struct scatterlist outhdr_sg, key_sg, inhdr_sg, *sgs[3];
struct virtio_crypto *vcrypto = ctx->vcrypto;
uint8_t *pkey;
- unsigned int inlen;
int err;
unsigned int num_out = 0, num_in = 0;
struct virtio_crypto_op_ctrl_req *ctrl;
@@ -135,18 +134,9 @@ 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) {
- spin_unlock(&vcrypto->ctrl_lock);
+ err = virtio_crypto_ctrl_vq_request(vcrypto, sgs, num_out, num_in, vc_ctrl_req);
+ if (err < 0)
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;
@@ -173,7 +163,7 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
struct scatterlist outhdr_sg, inhdr_sg, *sgs[2];
struct virtio_crypto_destroy_session_req *destroy_session;
struct virtio_crypto *vcrypto = ctx->vcrypto;
- unsigned int num_out = 0, num_in = 0, inlen;
+ unsigned int num_out = 0, num_in = 0;
int err;
struct virtio_crypto_op_ctrl_req *ctrl;
struct virtio_crypto_inhdr *ctrl_status;
@@ -205,18 +195,9 @@ 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) {
- spin_unlock(&vcrypto->ctrl_lock);
+ err = virtio_crypto_ctrl_vq_request(vcrypto, sgs, num_out, num_in, vc_ctrl_req);
+ if (err < 0)
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;
diff --git a/drivers/crypto/virtio/virtio_crypto_common.h b/drivers/crypto/virtio/virtio_crypto_common.h
index 2422237ec4e6..59a4c0259456 100644
--- a/drivers/crypto/virtio/virtio_crypto_common.h
+++ b/drivers/crypto/virtio/virtio_crypto_common.h
@@ -90,6 +90,7 @@ 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 completion compl;
};
struct virtio_crypto_request;
@@ -141,5 +142,8 @@ int virtio_crypto_skcipher_algs_register(struct virtio_crypto *vcrypto);
void virtio_crypto_skcipher_algs_unregister(struct virtio_crypto *vcrypto);
int virtio_crypto_akcipher_algs_register(struct virtio_crypto *vcrypto);
void virtio_crypto_akcipher_algs_unregister(struct virtio_crypto *vcrypto);
+int virtio_crypto_ctrl_vq_request(struct virtio_crypto *vcrypto, struct scatterlist *sgs[],
+ unsigned int out_sgs, unsigned int in_sgs,
+ struct virtio_crypto_ctrl_request *vc_ctrl_req);
#endif /* _VIRTIO_CRYPTO_COMMON_H */
diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c
index c6f482db0bc0..60490ffa3df1 100644
--- a/drivers/crypto/virtio/virtio_crypto_core.c
+++ b/drivers/crypto/virtio/virtio_crypto_core.c
@@ -22,6 +22,56 @@ virtcrypto_clear_request(struct virtio_crypto_request *vc_req)
}
}
+static void virtio_crypto_ctrlq_callback(struct virtio_crypto_ctrl_request *vc_ctrl_req)
+{
+ complete(&vc_ctrl_req->compl);
+}
+
+static void virtcrypto_ctrlq_callback(struct virtqueue *vq)
+{
+ struct virtio_crypto *vcrypto = vq->vdev->priv;
+ struct virtio_crypto_ctrl_request *vc_ctrl_req;
+ unsigned long flags;
+ unsigned int len;
+
+ spin_lock_irqsave(&vcrypto->ctrl_lock, flags);
+ do {
+ virtqueue_disable_cb(vq);
+ while ((vc_ctrl_req = virtqueue_get_buf(vq, &len)) != NULL) {
+ spin_unlock_irqrestore(&vcrypto->ctrl_lock, flags);
+ virtio_crypto_ctrlq_callback(vc_ctrl_req);
+ spin_lock_irqsave(&vcrypto->ctrl_lock, flags);
+ }
+ if (unlikely(virtqueue_is_broken(vq)))
+ break;
+ } while (!virtqueue_enable_cb(vq));
+ spin_unlock_irqrestore(&vcrypto->ctrl_lock, flags);
+}
+
+int virtio_crypto_ctrl_vq_request(struct virtio_crypto *vcrypto, struct scatterlist *sgs[],
+ unsigned int out_sgs, unsigned int in_sgs,
+ struct virtio_crypto_ctrl_request *vc_ctrl_req)
+{
+ int err;
+ unsigned long flags;
+
+ init_completion(&vc_ctrl_req->compl);
+
+ spin_lock_irqsave(&vcrypto->ctrl_lock, flags);
+ err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, out_sgs, in_sgs, vc_ctrl_req, GFP_ATOMIC);
+ if (err < 0) {
+ spin_unlock_irqrestore(&vcrypto->ctrl_lock, flags);
+ return err;
+ }
+
+ virtqueue_kick(vcrypto->ctrl_vq);
+ spin_unlock_irqrestore(&vcrypto->ctrl_lock, flags);
+
+ wait_for_completion(&vc_ctrl_req->compl);
+
+ return 0;
+}
+
static void virtcrypto_dataq_callback(struct virtqueue *vq)
{
struct virtio_crypto *vcrypto = vq->vdev->priv;
@@ -73,7 +123,7 @@ static int virtcrypto_find_vqs(struct virtio_crypto *vi)
goto err_names;
/* Parameters for control virtqueue */
- callbacks[total_vqs - 1] = NULL;
+ callbacks[total_vqs - 1] = virtcrypto_ctrlq_callback;
names[total_vqs - 1] = "controlq";
/* Allocate/initialize parameters for data virtqueues */
diff --git a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
index 6aaf0869b211..e553ccadbcbc 100644
--- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
@@ -118,7 +118,6 @@ static int virtio_crypto_alg_skcipher_init_session(
int encrypt)
{
struct scatterlist outhdr, key_sg, inhdr, *sgs[3];
- unsigned int tmp;
struct virtio_crypto *vcrypto = ctx->vcrypto;
int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT : VIRTIO_CRYPTO_OP_DECRYPT;
int err;
@@ -170,23 +169,9 @@ 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);
+ err = virtio_crypto_ctrl_vq_request(vcrypto, sgs, num_out, num_in, vc_ctrl_req);
+ if (err < 0)
goto out;
- }
- virtqueue_kick(vcrypto->ctrl_vq);
-
- /*
- * Trapping into the hypervisor, so the request should be
- * handled immediately.
- */
- 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) {
pr_err("virtio_crypto: Create session failed status: %u\n",
@@ -212,7 +197,6 @@ static int virtio_crypto_alg_skcipher_close_session(
int encrypt)
{
struct scatterlist outhdr, status_sg, *sgs[2];
- unsigned int tmp;
struct virtio_crypto_destroy_session_req *destroy_session;
struct virtio_crypto *vcrypto = ctx->vcrypto;
int err;
@@ -247,19 +231,9 @@ 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);
+ err = virtio_crypto_ctrl_vq_request(vcrypto, sgs, num_out, num_in, vc_ctrl_req);
+ if (err < 0)
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) {
pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n",
--
2.20.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v4 3/5] virtio-crypto: wait ctrl queue instead of busy polling
@ 2022-04-24 10:41 ` zhenwei pi
0 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-04-24 10:41 UTC (permalink / raw)
To: arei.gonglei, mst, jasowang
Cc: helei.sig11, linux-kernel, zhenwei pi, virtualization,
linux-crypto, davem, herbert
Originally, after submitting request into virtio crypto control
queue, the guest side polls the result from the virt queue. This
works like following:
CPU0 CPU1 ... CPUx CPUy
| | | |
\ \ / /
\--------spin_lock(&vcrypto->ctrl_lock)-------/
|
virtqueue add & kick
|
busy poll virtqueue
|
spin_unlock(&vcrypto->ctrl_lock)
...
There are two problems:
1, The queue depth is always 1, the performance of a virtio crypto
device gets limited. Multi user processes share a single control
queue, and hit spin lock race from control queue. Test on Intel
Platinum 8260, a single worker gets ~35K/s create/close session
operations, and 8 workers get ~40K/s operations with 800% CPU
utilization.
2, The control request is supposed to get handled immediately, but
in the current implementation of QEMU(v6.2), the vCPU thread kicks
another thread to do this work, the latency also gets unstable.
Tracking latency of virtio_crypto_alg_akcipher_close_session in 5s:
usecs : count distribution
0 -> 1 : 0 | |
2 -> 3 : 7 | |
4 -> 7 : 72 | |
8 -> 15 : 186485 |************************|
16 -> 31 : 687 | |
32 -> 63 : 5 | |
64 -> 127 : 3 | |
128 -> 255 : 1 | |
256 -> 511 : 0 | |
512 -> 1023 : 0 | |
1024 -> 2047 : 0 | |
2048 -> 4095 : 0 | |
4096 -> 8191 : 0 | |
8192 -> 16383 : 2 | |
This means that a CPU may hold vcrypto->ctrl_lock as long as 8192~16383us.
To improve the performance of control queue, a request on control queue
waits completion instead of busy polling to reduce lock racing, and gets
completed by control queue callback.
CPU0 CPU1 ... CPUx CPUy
| | | |
\ \ / /
\--------spin_lock(&vcrypto->ctrl_lock)-------/
|
virtqueue add & kick
|
---------spin_unlock(&vcrypto->ctrl_lock)------
/ / \ \
| | | |
wait wait wait wait
Test this patch, the guest side get ~200K/s operations with 300% CPU
utilization.
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
.../virtio/virtio_crypto_akcipher_algs.c | 29 ++---------
drivers/crypto/virtio/virtio_crypto_common.h | 4 ++
drivers/crypto/virtio/virtio_crypto_core.c | 52 ++++++++++++++++++-
.../virtio/virtio_crypto_skcipher_algs.c | 34 ++----------
4 files changed, 64 insertions(+), 55 deletions(-)
diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
index 509884e8b201..1e98502830cf 100644
--- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
@@ -103,7 +103,6 @@ static int virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher
struct scatterlist outhdr_sg, key_sg, inhdr_sg, *sgs[3];
struct virtio_crypto *vcrypto = ctx->vcrypto;
uint8_t *pkey;
- unsigned int inlen;
int err;
unsigned int num_out = 0, num_in = 0;
struct virtio_crypto_op_ctrl_req *ctrl;
@@ -135,18 +134,9 @@ 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) {
- spin_unlock(&vcrypto->ctrl_lock);
+ err = virtio_crypto_ctrl_vq_request(vcrypto, sgs, num_out, num_in, vc_ctrl_req);
+ if (err < 0)
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;
@@ -173,7 +163,7 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
struct scatterlist outhdr_sg, inhdr_sg, *sgs[2];
struct virtio_crypto_destroy_session_req *destroy_session;
struct virtio_crypto *vcrypto = ctx->vcrypto;
- unsigned int num_out = 0, num_in = 0, inlen;
+ unsigned int num_out = 0, num_in = 0;
int err;
struct virtio_crypto_op_ctrl_req *ctrl;
struct virtio_crypto_inhdr *ctrl_status;
@@ -205,18 +195,9 @@ 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) {
- spin_unlock(&vcrypto->ctrl_lock);
+ err = virtio_crypto_ctrl_vq_request(vcrypto, sgs, num_out, num_in, vc_ctrl_req);
+ if (err < 0)
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;
diff --git a/drivers/crypto/virtio/virtio_crypto_common.h b/drivers/crypto/virtio/virtio_crypto_common.h
index 2422237ec4e6..59a4c0259456 100644
--- a/drivers/crypto/virtio/virtio_crypto_common.h
+++ b/drivers/crypto/virtio/virtio_crypto_common.h
@@ -90,6 +90,7 @@ 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 completion compl;
};
struct virtio_crypto_request;
@@ -141,5 +142,8 @@ int virtio_crypto_skcipher_algs_register(struct virtio_crypto *vcrypto);
void virtio_crypto_skcipher_algs_unregister(struct virtio_crypto *vcrypto);
int virtio_crypto_akcipher_algs_register(struct virtio_crypto *vcrypto);
void virtio_crypto_akcipher_algs_unregister(struct virtio_crypto *vcrypto);
+int virtio_crypto_ctrl_vq_request(struct virtio_crypto *vcrypto, struct scatterlist *sgs[],
+ unsigned int out_sgs, unsigned int in_sgs,
+ struct virtio_crypto_ctrl_request *vc_ctrl_req);
#endif /* _VIRTIO_CRYPTO_COMMON_H */
diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c
index c6f482db0bc0..60490ffa3df1 100644
--- a/drivers/crypto/virtio/virtio_crypto_core.c
+++ b/drivers/crypto/virtio/virtio_crypto_core.c
@@ -22,6 +22,56 @@ virtcrypto_clear_request(struct virtio_crypto_request *vc_req)
}
}
+static void virtio_crypto_ctrlq_callback(struct virtio_crypto_ctrl_request *vc_ctrl_req)
+{
+ complete(&vc_ctrl_req->compl);
+}
+
+static void virtcrypto_ctrlq_callback(struct virtqueue *vq)
+{
+ struct virtio_crypto *vcrypto = vq->vdev->priv;
+ struct virtio_crypto_ctrl_request *vc_ctrl_req;
+ unsigned long flags;
+ unsigned int len;
+
+ spin_lock_irqsave(&vcrypto->ctrl_lock, flags);
+ do {
+ virtqueue_disable_cb(vq);
+ while ((vc_ctrl_req = virtqueue_get_buf(vq, &len)) != NULL) {
+ spin_unlock_irqrestore(&vcrypto->ctrl_lock, flags);
+ virtio_crypto_ctrlq_callback(vc_ctrl_req);
+ spin_lock_irqsave(&vcrypto->ctrl_lock, flags);
+ }
+ if (unlikely(virtqueue_is_broken(vq)))
+ break;
+ } while (!virtqueue_enable_cb(vq));
+ spin_unlock_irqrestore(&vcrypto->ctrl_lock, flags);
+}
+
+int virtio_crypto_ctrl_vq_request(struct virtio_crypto *vcrypto, struct scatterlist *sgs[],
+ unsigned int out_sgs, unsigned int in_sgs,
+ struct virtio_crypto_ctrl_request *vc_ctrl_req)
+{
+ int err;
+ unsigned long flags;
+
+ init_completion(&vc_ctrl_req->compl);
+
+ spin_lock_irqsave(&vcrypto->ctrl_lock, flags);
+ err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, out_sgs, in_sgs, vc_ctrl_req, GFP_ATOMIC);
+ if (err < 0) {
+ spin_unlock_irqrestore(&vcrypto->ctrl_lock, flags);
+ return err;
+ }
+
+ virtqueue_kick(vcrypto->ctrl_vq);
+ spin_unlock_irqrestore(&vcrypto->ctrl_lock, flags);
+
+ wait_for_completion(&vc_ctrl_req->compl);
+
+ return 0;
+}
+
static void virtcrypto_dataq_callback(struct virtqueue *vq)
{
struct virtio_crypto *vcrypto = vq->vdev->priv;
@@ -73,7 +123,7 @@ static int virtcrypto_find_vqs(struct virtio_crypto *vi)
goto err_names;
/* Parameters for control virtqueue */
- callbacks[total_vqs - 1] = NULL;
+ callbacks[total_vqs - 1] = virtcrypto_ctrlq_callback;
names[total_vqs - 1] = "controlq";
/* Allocate/initialize parameters for data virtqueues */
diff --git a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
index 6aaf0869b211..e553ccadbcbc 100644
--- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
@@ -118,7 +118,6 @@ static int virtio_crypto_alg_skcipher_init_session(
int encrypt)
{
struct scatterlist outhdr, key_sg, inhdr, *sgs[3];
- unsigned int tmp;
struct virtio_crypto *vcrypto = ctx->vcrypto;
int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT : VIRTIO_CRYPTO_OP_DECRYPT;
int err;
@@ -170,23 +169,9 @@ 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);
+ err = virtio_crypto_ctrl_vq_request(vcrypto, sgs, num_out, num_in, vc_ctrl_req);
+ if (err < 0)
goto out;
- }
- virtqueue_kick(vcrypto->ctrl_vq);
-
- /*
- * Trapping into the hypervisor, so the request should be
- * handled immediately.
- */
- 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) {
pr_err("virtio_crypto: Create session failed status: %u\n",
@@ -212,7 +197,6 @@ static int virtio_crypto_alg_skcipher_close_session(
int encrypt)
{
struct scatterlist outhdr, status_sg, *sgs[2];
- unsigned int tmp;
struct virtio_crypto_destroy_session_req *destroy_session;
struct virtio_crypto *vcrypto = ctx->vcrypto;
int err;
@@ -247,19 +231,9 @@ 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);
+ err = virtio_crypto_ctrl_vq_request(vcrypto, sgs, num_out, num_in, vc_ctrl_req);
+ if (err < 0)
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) {
pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n",
--
2.20.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v4 4/5] virtio-crypto: adjust dst_len at ops callback
2022-04-24 10:41 ` zhenwei pi
@ 2022-04-24 10:41 ` zhenwei pi
-1 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-04-24 10:41 UTC (permalink / raw)
To: arei.gonglei, mst, jasowang
Cc: herbert, linux-kernel, virtualization, linux-crypto, helei.sig11,
davem, zhenwei pi
From: lei he <helei.sig11@bytedance.com>
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 <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: lei he <helei.sig11@bytedance.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
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
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v4 4/5] virtio-crypto: adjust dst_len at ops callback
@ 2022-04-24 10:41 ` zhenwei pi
0 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-04-24 10:41 UTC (permalink / raw)
To: arei.gonglei, mst, jasowang
Cc: helei.sig11, linux-kernel, zhenwei pi, virtualization,
linux-crypto, davem, herbert
From: lei he <helei.sig11@bytedance.com>
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 <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: lei he <helei.sig11@bytedance.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v4 5/5] virtio-crypto: enable retry for virtio-crypto-dev
2022-04-24 10:41 ` zhenwei pi
@ 2022-04-24 10:41 ` zhenwei pi
-1 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-04-24 10:41 UTC (permalink / raw)
To: arei.gonglei, mst, jasowang
Cc: herbert, linux-kernel, virtualization, linux-crypto, helei.sig11,
davem, zhenwei pi
From: lei he <helei.sig11@bytedance.com>
Enable retry for virtio-crypto-dev, so that crypto-engine
can process cipher-requests parallelly.
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: lei he <helei.sig11@bytedance.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
drivers/crypto/virtio/virtio_crypto_core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c
index 60490ffa3df1..f67e0d4c1b0c 100644
--- a/drivers/crypto/virtio/virtio_crypto_core.c
+++ b/drivers/crypto/virtio/virtio_crypto_core.c
@@ -144,7 +144,8 @@ static int virtcrypto_find_vqs(struct virtio_crypto *vi)
spin_lock_init(&vi->data_vq[i].lock);
vi->data_vq[i].vq = vqs[i];
/* Initialize crypto engine */
- vi->data_vq[i].engine = crypto_engine_alloc_init(dev, 1);
+ vi->data_vq[i].engine = crypto_engine_alloc_init_and_set(dev, true, NULL, 1,
+ virtqueue_get_vring_size(vqs[i]));
if (!vi->data_vq[i].engine) {
ret = -ENOMEM;
goto err_engine;
--
2.20.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v4 5/5] virtio-crypto: enable retry for virtio-crypto-dev
@ 2022-04-24 10:41 ` zhenwei pi
0 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-04-24 10:41 UTC (permalink / raw)
To: arei.gonglei, mst, jasowang
Cc: helei.sig11, linux-kernel, zhenwei pi, virtualization,
linux-crypto, davem, herbert
From: lei he <helei.sig11@bytedance.com>
Enable retry for virtio-crypto-dev, so that crypto-engine
can process cipher-requests parallelly.
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: lei he <helei.sig11@bytedance.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
drivers/crypto/virtio/virtio_crypto_core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c
index 60490ffa3df1..f67e0d4c1b0c 100644
--- a/drivers/crypto/virtio/virtio_crypto_core.c
+++ b/drivers/crypto/virtio/virtio_crypto_core.c
@@ -144,7 +144,8 @@ static int virtcrypto_find_vqs(struct virtio_crypto *vi)
spin_lock_init(&vi->data_vq[i].lock);
vi->data_vq[i].vq = vqs[i];
/* Initialize crypto engine */
- vi->data_vq[i].engine = crypto_engine_alloc_init(dev, 1);
+ vi->data_vq[i].engine = crypto_engine_alloc_init_and_set(dev, true, NULL, 1,
+ virtqueue_get_vring_size(vqs[i]));
if (!vi->data_vq[i].engine) {
ret = -ENOMEM;
goto err_engine;
--
2.20.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v4 2/5] virtio-crypto: use private buffer for control request
2022-04-24 10:41 ` zhenwei pi
(?)
(?)
@ 2022-04-25 13:27 ` Dan Carpenter
-1 siblings, 0 replies; 29+ messages in thread
From: kernel test robot @ 2022-04-24 15:51 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 12288 bytes --]
CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220424104140.44841-3-pizhenwei@bytedance.com>
References: <20220424104140.44841-3-pizhenwei@bytedance.com>
TO: zhenwei pi <pizhenwei@bytedance.com>
TO: arei.gonglei(a)huawei.com
TO: mst(a)redhat.com
TO: jasowang(a)redhat.com
CC: herbert(a)gondor.apana.org.au
CC: linux-kernel(a)vger.kernel.org
CC: virtualization(a)lists.linux-foundation.org
CC: linux-crypto(a)vger.kernel.org
CC: helei.sig11(a)bytedance.com
CC: davem(a)davemloft.net
CC: zhenwei pi <pizhenwei@bytedance.com>
Hi zhenwei,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on herbert-cryptodev-2.6/master]
[also build test WARNING on linus/master v5.18-rc3 next-20220422]
[cannot apply to herbert-crypto-2.6/master linux/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
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
:::::: branch date: 5 hours ago
:::::: commit date: 5 hours ago
config: i386-randconfig-m021 (https://download.01.org/0day-ci/archive/20220424/202204242344.JepUMdzP-lkp(a)intel.com/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 <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
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 98
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));
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);
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 }
59ca6c93387d32 zhenwei pi 2022-03-02 238
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v4 2/5] virtio-crypto: use private buffer for control request
@ 2022-04-25 13:27 ` Dan Carpenter
0 siblings, 0 replies; 29+ messages in thread
From: Dan Carpenter @ 2022-04-25 13:27 UTC (permalink / raw)
To: kbuild, zhenwei pi, arei.gonglei, mst, jasowang
Cc: kbuild-all, lkp, linux-kernel, zhenwei pi, virtualization,
helei.sig11, linux-crypto, davem, herbert
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/202204242344.JepUMdzP-lkp@intel.com/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 <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v4 2/5] virtio-crypto: use private buffer for control request
@ 2022-04-25 13:27 ` Dan Carpenter
0 siblings, 0 replies; 29+ messages in thread
From: Dan Carpenter @ 2022-04-25 13:27 UTC (permalink / raw)
To: kbuild, zhenwei pi, arei.gonglei, mst, jasowang
Cc: lkp, kbuild-all, herbert, linux-kernel, virtualization,
linux-crypto, helei.sig11, davem, zhenwei pi
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/202204242344.JepUMdzP-lkp@intel.com/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 <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v4 2/5] virtio-crypto: use private buffer for control request
@ 2022-04-25 13:27 ` Dan Carpenter
0 siblings, 0 replies; 29+ messages in thread
From: Dan Carpenter @ 2022-04-25 13:27 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 11241 bytes --]
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/202204242344.JepUMdzP-lkp(a)intel.com/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 <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v4 1/5] virtio-crypto: change code style
2022-04-24 10:41 ` zhenwei pi
@ 2022-04-26 6:12 ` Jason Wang
-1 siblings, 0 replies; 29+ messages in thread
From: Jason Wang @ 2022-04-26 6:12 UTC (permalink / raw)
To: zhenwei pi
Cc: Gonglei (Arei),
mst, Herbert Xu, linux-kernel, virtualization, linux-crypto,
helei.sig11, davem
On Sun, Apr 24, 2022 at 6:45 PM zhenwei pi <pizhenwei@bytedance.com> 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 <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Gonglei <arei.gonglei@huawei.com>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
> .../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
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v4 1/5] virtio-crypto: change code style
@ 2022-04-26 6:12 ` Jason Wang
0 siblings, 0 replies; 29+ messages in thread
From: Jason Wang @ 2022-04-26 6:12 UTC (permalink / raw)
To: zhenwei pi
Cc: Herbert Xu, mst, linux-kernel, virtualization, linux-crypto,
davem, helei.sig11
On Sun, Apr 24, 2022 at 6:45 PM zhenwei pi <pizhenwei@bytedance.com> 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 <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Gonglei <arei.gonglei@huawei.com>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
> .../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
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Re: [PATCH v4 1/5] virtio-crypto: change code style
2022-04-26 6:12 ` Jason Wang
@ 2022-04-26 6:29 ` zhenwei pi
-1 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-04-26 6:29 UTC (permalink / raw)
To: Jason Wang, Gonglei (Arei)
Cc: mst, Herbert Xu, linux-kernel, virtualization, linux-crypto,
helei.sig11, davem
On 4/26/22 14:12, Jason Wang wrote:
> On Sun, Apr 24, 2022 at 6:45 PM zhenwei pi <pizhenwei@bytedance.com> 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
>
Thanks to Jason!
Hi, Lei
What's your opinion?
>>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: Jason Wang <jasowang@redhat.com>
>> Cc: Gonglei <arei.gonglei@huawei.com>
>> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
>> ---
>> .../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
>>
>
--
zhenwei pi
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Re: [PATCH v4 1/5] virtio-crypto: change code style
@ 2022-04-26 6:29 ` zhenwei pi
0 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-04-26 6:29 UTC (permalink / raw)
To: Jason Wang, Gonglei (Arei)
Cc: Herbert Xu, mst, linux-kernel, virtualization, linux-crypto,
davem, helei.sig11
On 4/26/22 14:12, Jason Wang wrote:
> On Sun, Apr 24, 2022 at 6:45 PM zhenwei pi <pizhenwei@bytedance.com> 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
>
Thanks to Jason!
Hi, Lei
What's your opinion?
>>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: Jason Wang <jasowang@redhat.com>
>> Cc: Gonglei <arei.gonglei@huawei.com>
>> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
>> ---
>> .../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
>>
>
--
zhenwei pi
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 29+ messages in thread
* PING: [PATCH v4 0/5] virtio-crypto: Improve performance
2022-04-24 10:41 ` zhenwei pi
@ 2022-05-05 2:35 ` zhenwei pi
-1 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-05-05 2:35 UTC (permalink / raw)
To: arei.gonglei, mst, jasowang
Cc: herbert, linux-kernel, virtualization, linux-crypto, helei.sig11, davem
Hi, Lei
Jason replied in another patch:
Still hundreds of lines of changes, I'd leave this change to other
maintainers to decide.
Quite frankly, the virtio crypto driver changed only a few in the past,
and the performance of control queue is not good enough. I am in doubt
about that this driver is not used widely. So I'd like to rework a lot,
it would be best to complete this work in 5.18 window.
This gets different point with Jason. I would appreciate it if you could
give me any hint.
On 4/24/22 18:41, zhenwei pi wrote:
> 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(-)
>
--
zhenwei pi
^ permalink raw reply [flat|nested] 29+ messages in thread
* PING: [PATCH v4 0/5] virtio-crypto: Improve performance
@ 2022-05-05 2:35 ` zhenwei pi
0 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-05-05 2:35 UTC (permalink / raw)
To: arei.gonglei, mst, jasowang
Cc: helei.sig11, linux-kernel, virtualization, linux-crypto, davem, herbert
Hi, Lei
Jason replied in another patch:
Still hundreds of lines of changes, I'd leave this change to other
maintainers to decide.
Quite frankly, the virtio crypto driver changed only a few in the past,
and the performance of control queue is not good enough. I am in doubt
about that this driver is not used widely. So I'd like to rework a lot,
it would be best to complete this work in 5.18 window.
This gets different point with Jason. I would appreciate it if you could
give me any hint.
On 4/24/22 18:41, zhenwei pi wrote:
> 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(-)
>
--
zhenwei pi
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: PING: [PATCH v4 0/5] virtio-crypto: Improve performance
2022-05-05 2:35 ` zhenwei pi
@ 2022-05-05 3:14 ` Gonglei (Arei) via Virtualization
-1 siblings, 0 replies; 29+ messages in thread
From: Gonglei (Arei) @ 2022-05-05 3:14 UTC (permalink / raw)
To: zhenwei pi, mst, jasowang
Cc: herbert, linux-kernel, virtualization, linux-crypto, helei.sig11, davem
> -----Original Message-----
> From: zhenwei pi [mailto:pizhenwei@bytedance.com]
> Sent: Thursday, May 5, 2022 10:35 AM
> To: Gonglei (Arei) <arei.gonglei@huawei.com>; mst@redhat.com;
> jasowang@redhat.com
> Cc: herbert@gondor.apana.org.au; linux-kernel@vger.kernel.org;
> virtualization@lists.linux-foundation.org; linux-crypto@vger.kernel.org;
> helei.sig11@bytedance.com; davem@davemloft.net
> Subject: PING: [PATCH v4 0/5] virtio-crypto: Improve performance
>
> Hi, Lei
>
> Jason replied in another patch:
> Still hundreds of lines of changes, I'd leave this change to other maintainers to
> decide.
>
> Quite frankly, the virtio crypto driver changed only a few in the past, and the
> performance of control queue is not good enough. I am in doubt about that this
> driver is not used widely. So I'd like to rework a lot, it would be best to complete
> this work in 5.18 window.
>
> This gets different point with Jason. I would appreciate it if you could give me
> any hint.
>
This is already in my todo list.
Regards,
-Gonglei
> On 4/24/22 18:41, zhenwei pi wrote:
> > 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(-)
> >
>
> --
> zhenwei pi
^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: PING: [PATCH v4 0/5] virtio-crypto: Improve performance
@ 2022-05-05 3:14 ` Gonglei (Arei) via Virtualization
0 siblings, 0 replies; 29+ messages in thread
From: Gonglei (Arei) via Virtualization @ 2022-05-05 3:14 UTC (permalink / raw)
To: zhenwei pi, mst, jasowang
Cc: helei.sig11, linux-kernel, virtualization, linux-crypto, davem, herbert
> -----Original Message-----
> From: zhenwei pi [mailto:pizhenwei@bytedance.com]
> Sent: Thursday, May 5, 2022 10:35 AM
> To: Gonglei (Arei) <arei.gonglei@huawei.com>; mst@redhat.com;
> jasowang@redhat.com
> Cc: herbert@gondor.apana.org.au; linux-kernel@vger.kernel.org;
> virtualization@lists.linux-foundation.org; linux-crypto@vger.kernel.org;
> helei.sig11@bytedance.com; davem@davemloft.net
> Subject: PING: [PATCH v4 0/5] virtio-crypto: Improve performance
>
> Hi, Lei
>
> Jason replied in another patch:
> Still hundreds of lines of changes, I'd leave this change to other maintainers to
> decide.
>
> Quite frankly, the virtio crypto driver changed only a few in the past, and the
> performance of control queue is not good enough. I am in doubt about that this
> driver is not used widely. So I'd like to rework a lot, it would be best to complete
> this work in 5.18 window.
>
> This gets different point with Jason. I would appreciate it if you could give me
> any hint.
>
This is already in my todo list.
Regards,
-Gonglei
> On 4/24/22 18:41, zhenwei pi wrote:
> > 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(-)
> >
>
> --
> zhenwei pi
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PING: [PATCH v4 0/5] virtio-crypto: Improve performance
2022-05-05 3:14 ` Gonglei (Arei) via Virtualization
@ 2022-05-05 4:57 ` Michael S. Tsirkin
-1 siblings, 0 replies; 29+ messages in thread
From: Michael S. Tsirkin @ 2022-05-05 4:57 UTC (permalink / raw)
To: Gonglei (Arei)
Cc: herbert, linux-kernel, zhenwei pi, virtualization, linux-crypto,
davem, helei.sig11
On Thu, May 05, 2022 at 03:14:40AM +0000, Gonglei (Arei) wrote:
>
>
> > -----Original Message-----
> > From: zhenwei pi [mailto:pizhenwei@bytedance.com]
> > Sent: Thursday, May 5, 2022 10:35 AM
> > To: Gonglei (Arei) <arei.gonglei@huawei.com>; mst@redhat.com;
> > jasowang@redhat.com
> > Cc: herbert@gondor.apana.org.au; linux-kernel@vger.kernel.org;
> > virtualization@lists.linux-foundation.org; linux-crypto@vger.kernel.org;
> > helei.sig11@bytedance.com; davem@davemloft.net
> > Subject: PING: [PATCH v4 0/5] virtio-crypto: Improve performance
> >
> > Hi, Lei
> >
> > Jason replied in another patch:
> > Still hundreds of lines of changes, I'd leave this change to other maintainers to
> > decide.
> >
> > Quite frankly, the virtio crypto driver changed only a few in the past, and the
> > performance of control queue is not good enough. I am in doubt about that this
> > driver is not used widely. So I'd like to rework a lot, it would be best to complete
> > this work in 5.18 window.
> >
> > This gets different point with Jason. I would appreciate it if you could give me
> > any hint.
> >
>
> This is already in my todo list.
>
> Regards,
> -Gonglei
It's been out a month though, not really acceptable latency for review.
So I would apply this for next, but you need to address Dan Captenter's
comment, and look for simular patterns elesewhere in your patch.
> > On 4/24/22 18:41, zhenwei pi wrote:
> > > 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(-)
> > >
> >
> > --
> > zhenwei pi
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: PING: [PATCH v4 0/5] virtio-crypto: Improve performance
@ 2022-05-05 4:57 ` Michael S. Tsirkin
0 siblings, 0 replies; 29+ messages in thread
From: Michael S. Tsirkin @ 2022-05-05 4:57 UTC (permalink / raw)
To: Gonglei (Arei)
Cc: zhenwei pi, jasowang, herbert, linux-kernel, virtualization,
linux-crypto, helei.sig11, davem
On Thu, May 05, 2022 at 03:14:40AM +0000, Gonglei (Arei) wrote:
>
>
> > -----Original Message-----
> > From: zhenwei pi [mailto:pizhenwei@bytedance.com]
> > Sent: Thursday, May 5, 2022 10:35 AM
> > To: Gonglei (Arei) <arei.gonglei@huawei.com>; mst@redhat.com;
> > jasowang@redhat.com
> > Cc: herbert@gondor.apana.org.au; linux-kernel@vger.kernel.org;
> > virtualization@lists.linux-foundation.org; linux-crypto@vger.kernel.org;
> > helei.sig11@bytedance.com; davem@davemloft.net
> > Subject: PING: [PATCH v4 0/5] virtio-crypto: Improve performance
> >
> > Hi, Lei
> >
> > Jason replied in another patch:
> > Still hundreds of lines of changes, I'd leave this change to other maintainers to
> > decide.
> >
> > Quite frankly, the virtio crypto driver changed only a few in the past, and the
> > performance of control queue is not good enough. I am in doubt about that this
> > driver is not used widely. So I'd like to rework a lot, it would be best to complete
> > this work in 5.18 window.
> >
> > This gets different point with Jason. I would appreciate it if you could give me
> > any hint.
> >
>
> This is already in my todo list.
>
> Regards,
> -Gonglei
It's been out a month though, not really acceptable latency for review.
So I would apply this for next, but you need to address Dan Captenter's
comment, and look for simular patterns elesewhere in your patch.
> > On 4/24/22 18:41, zhenwei pi wrote:
> > > 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(-)
> > >
> >
> > --
> > zhenwei pi
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Re: PING: [PATCH v4 0/5] virtio-crypto: Improve performance
2022-05-05 4:57 ` Michael S. Tsirkin
@ 2022-05-05 9:29 ` zhenwei pi
-1 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-05-05 9:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: jasowang, herbert, linux-kernel, Gonglei (Arei),
virtualization, linux-crypto, helei.sig11, davem
On 5/5/22 12:57, Michael S. Tsirkin wrote:
> On Thu, May 05, 2022 at 03:14:40AM +0000, Gonglei (Arei) wrote:
>>
>>
>>> -----Original Message-----
>>> From: zhenwei pi [mailto:pizhenwei@bytedance.com]
>>> Sent: Thursday, May 5, 2022 10:35 AM
>>> To: Gonglei (Arei) <arei.gonglei@huawei.com>; mst@redhat.com;
>>> jasowang@redhat.com
>>> Cc: herbert@gondor.apana.org.au; linux-kernel@vger.kernel.org;
>>> virtualization@lists.linux-foundation.org; linux-crypto@vger.kernel.org;
>>> helei.sig11@bytedance.com; davem@davemloft.net
>>> Subject: PING: [PATCH v4 0/5] virtio-crypto: Improve performance
>>>
>>> Hi, Lei
>>>
>>> Jason replied in another patch:
>>> Still hundreds of lines of changes, I'd leave this change to other maintainers to
>>> decide.
>>>
>>> Quite frankly, the virtio crypto driver changed only a few in the past, and the
>>> performance of control queue is not good enough. I am in doubt about that this
>>> driver is not used widely. So I'd like to rework a lot, it would be best to complete
>>> this work in 5.18 window.
>>>
>>> This gets different point with Jason. I would appreciate it if you could give me
>>> any hint.
>>>
>>
>> This is already in my todo list.
>>
>> Regards,
>> -Gonglei
>
> It's been out a month though, not really acceptable latency for review.
> So I would apply this for next, but you need to address Dan Captenter's
> comment, and look for simular patterns elesewhere in your patch.
>
I fixed this in the v5 series. Thanks!
--
zhenwei pi
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Re: PING: [PATCH v4 0/5] virtio-crypto: Improve performance
@ 2022-05-05 9:29 ` zhenwei pi
0 siblings, 0 replies; 29+ messages in thread
From: zhenwei pi @ 2022-05-05 9:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: herbert, linux-kernel, virtualization, linux-crypto, davem, helei.sig11
On 5/5/22 12:57, Michael S. Tsirkin wrote:
> On Thu, May 05, 2022 at 03:14:40AM +0000, Gonglei (Arei) wrote:
>>
>>
>>> -----Original Message-----
>>> From: zhenwei pi [mailto:pizhenwei@bytedance.com]
>>> Sent: Thursday, May 5, 2022 10:35 AM
>>> To: Gonglei (Arei) <arei.gonglei@huawei.com>; mst@redhat.com;
>>> jasowang@redhat.com
>>> Cc: herbert@gondor.apana.org.au; linux-kernel@vger.kernel.org;
>>> virtualization@lists.linux-foundation.org; linux-crypto@vger.kernel.org;
>>> helei.sig11@bytedance.com; davem@davemloft.net
>>> Subject: PING: [PATCH v4 0/5] virtio-crypto: Improve performance
>>>
>>> Hi, Lei
>>>
>>> Jason replied in another patch:
>>> Still hundreds of lines of changes, I'd leave this change to other maintainers to
>>> decide.
>>>
>>> Quite frankly, the virtio crypto driver changed only a few in the past, and the
>>> performance of control queue is not good enough. I am in doubt about that this
>>> driver is not used widely. So I'd like to rework a lot, it would be best to complete
>>> this work in 5.18 window.
>>>
>>> This gets different point with Jason. I would appreciate it if you could give me
>>> any hint.
>>>
>>
>> This is already in my todo list.
>>
>> Regards,
>> -Gonglei
>
> It's been out a month though, not really acceptable latency for review.
> So I would apply this for next, but you need to address Dan Captenter's
> comment, and look for simular patterns elesewhere in your patch.
>
I fixed this in the v5 series. Thanks!
--
zhenwei pi
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v4 2/5] virtio-crypto: use private buffer for control request
@ 2022-04-26 7:49 kernel test robot
0 siblings, 0 replies; 29+ messages in thread
From: kernel test robot @ 2022-04-26 7:49 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 39851 bytes --]
CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220424104140.44841-3-pizhenwei@bytedance.com>
References: <20220424104140.44841-3-pizhenwei@bytedance.com>
TO: zhenwei pi <pizhenwei@bytedance.com>
TO: arei.gonglei(a)huawei.com
TO: mst(a)redhat.com
TO: jasowang(a)redhat.com
CC: herbert(a)gondor.apana.org.au
CC: linux-kernel(a)vger.kernel.org
CC: virtualization(a)lists.linux-foundation.org
CC: linux-crypto(a)vger.kernel.org
CC: helei.sig11(a)bytedance.com
CC: davem(a)davemloft.net
CC: zhenwei pi <pizhenwei@bytedance.com>
Hi zhenwei,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on herbert-cryptodev-2.6/master]
[also build test WARNING on linus/master v5.18-rc4 next-20220422]
[cannot apply to herbert-crypto-2.6/master linux/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
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
:::::: branch date: 2 days ago
:::::: commit date: 2 days ago
config: arm-randconfig-c002-20220424 (https://download.01.org/0day-ci/archive/20220426/202204261507.YSlYncSr-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 1cddcfdc3c683b393df1a5c9063252eb60e52818)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
# https://github.com/intel-lab-lkp/linux/commit/286da9ed04239c7036f247b3781249d84a387e23
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review zhenwei-pi/virtio-crypto-Improve-performance/20220424-184732
git checkout 286da9ed04239c7036f247b3781249d84a387e23
# save the config file
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm clang-analyzer
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
clang-analyzer warnings: (new ones prefixed by >>)
^~~~~~~~~~~~~~~~~~~
include/linux/fortify-string.h:47:29: note: expanded from macro '__underlying_memset'
#define __underlying_memset __builtin_memset
^~~~~~~~~~~~~~~~
drivers/media/mc/mc-device.c:565:9: warning: Call to function 'sprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'sprintf_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
return sprintf(buf, "%.*s\n", (int)sizeof(mdev->model), mdev->model);
^~~~~~~
drivers/media/mc/mc-device.c:565:9: note: Call to function 'sprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'sprintf_s' in case of C11
return sprintf(buf, "%.*s\n", (int)sizeof(mdev->model), mdev->model);
^~~~~~~
Suppressed 38 warnings (38 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
15 warnings generated.
Suppressed 15 warnings (15 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
15 warnings generated.
Suppressed 15 warnings (15 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
50 warnings generated.
drivers/crypto/virtio/virtio_crypto_skcipher_algs.c:452:2: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
memcpy(iv, req->iv, ivsize);
^
include/linux/fortify-string.h:369:26: note: expanded from macro 'memcpy'
#define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/fortify-string.h:362:2: note: expanded from macro '__fortify_memcpy_chk'
__underlying_##op(p, q, __fortify_size); \
^~~~~~~~~~~~~~~~~
note: expanded from here
include/linux/fortify-string.h:45:29: note: expanded from macro '__underlying_memcpy'
#define __underlying_memcpy __builtin_memcpy
^~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_skcipher_algs.c:452:2: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
memcpy(iv, req->iv, ivsize);
^
include/linux/fortify-string.h:369:26: note: expanded from macro 'memcpy'
#define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/fortify-string.h:362:2: note: expanded from macro '__fortify_memcpy_chk'
__underlying_##op(p, q, __fortify_size); \
^~~~~~~~~~~~~~~~~
note: expanded from here
include/linux/fortify-string.h:45:29: note: expanded from macro '__underlying_memcpy'
#define __underlying_memcpy __builtin_memcpy
^~~~~~~~~~~~~~~~
Suppressed 49 warnings (49 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
56 warnings generated.
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:124:2: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
memcpy(&ctrl->header, header, sizeof(ctrl->header));
^
include/linux/fortify-string.h:369:26: note: expanded from macro 'memcpy'
#define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/fortify-string.h:362:2: note: expanded from macro '__fortify_memcpy_chk'
__underlying_##op(p, q, __fortify_size); \
^~~~~~~~~~~~~~~~~
note: expanded from here
include/linux/fortify-string.h:45:29: note: expanded from macro '__underlying_memcpy'
#define __underlying_memcpy __builtin_memcpy
^~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:124:2: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
memcpy(&ctrl->header, header, sizeof(ctrl->header));
^
include/linux/fortify-string.h:369:26: note: expanded from macro 'memcpy'
#define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/fortify-string.h:362:2: note: expanded from macro '__fortify_memcpy_chk'
__underlying_##op(p, q, __fortify_size); \
^~~~~~~~~~~~~~~~~
note: expanded from here
include/linux/fortify-string.h:45:29: note: expanded from macro '__underlying_memcpy'
#define __underlying_memcpy __builtin_memcpy
^~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:125:2: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
memcpy(&ctrl->u, para, sizeof(ctrl->u));
^
include/linux/fortify-string.h:369:26: note: expanded from macro 'memcpy'
#define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/fortify-string.h:362:2: note: expanded from macro '__fortify_memcpy_chk'
__underlying_##op(p, q, __fortify_size); \
^~~~~~~~~~~~~~~~~
note: expanded from here
include/linux/fortify-string.h:45:29: note: expanded from macro '__underlying_memcpy'
#define __underlying_memcpy __builtin_memcpy
^~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:125:2: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
memcpy(&ctrl->u, para, sizeof(ctrl->u));
^
include/linux/fortify-string.h:369:26: note: expanded from macro 'memcpy'
#define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/fortify-string.h:362:2: note: expanded from macro '__fortify_memcpy_chk'
__underlying_##op(p, q, __fortify_size); \
^~~~~~~~~~~~~~~~~
note: expanded from here
include/linux/fortify-string.h:45:29: note: expanded from macro '__underlying_memcpy'
#define __underlying_memcpy __builtin_memcpy
^~~~~~~~~~~~~~~~
>> drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:166:4: warning: Access to field 'status' results in a dereference of an undefined pointer value (loaded from variable 'input') [clang-analyzer-core.NullDereference]
le32_to_cpu(input->status));
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:480:9: note: Calling 'virtio_crypto_rsa_set_key'
return virtio_crypto_rsa_set_key(tfm, key, keylen, 0,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:405:6: note: 'private' is false
if (private) {
^~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:405:2: note: Taking false branch
if (private) {
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:413:6: note: Assuming 'ret' is 0
if (ret)
^~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:413:2: note: Taking false branch
if (ret)
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:417:6: note: Assuming field 'n' is non-null
if (!rsa_ctx->n)
^~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:417:2: note: Taking false branch
if (!rsa_ctx->n)
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:420:6: note: Assuming field 'vcrypto' is non-null
if (!ctx->vcrypto) {
^~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:420:2: note: Taking false branch
if (!ctx->vcrypto) {
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:445:9: note: Calling 'virtio_crypto_alg_akcipher_init_session'
return virtio_crypto_alg_akcipher_init_session(ctx, &header, ¶, key, keylen);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:110:2: note: 'input' declared without an initial value
struct virtio_crypto_session_input *input;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:113:9: note: Calling 'kmemdup'
pkey = kmemdup(key, keylen, GFP_ATOMIC);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/fortify-string.h:436:33: note: Left side of '&&' is false
if (__builtin_constant_p(size) && p_size < size)
^
include/linux/fortify-string.h:438:6: note: Assuming 'p_size' is >= 'size'
if (p_size < size)
^~~~~~~~~~~~~
include/linux/fortify-string.h:438:2: note: Taking false branch
if (p_size < size)
^
include/linux/fortify-string.h:440:2: note: Returning pointer, which participates in a condition later
return __real_kmemdup(p, size, gfp);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:113:9: note: Returning from 'kmemdup'
pkey = kmemdup(key, keylen, GFP_ATOMIC);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:114:6: note: Assuming 'pkey' is non-null
if (!pkey)
^~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:114:2: note: Taking false branch
if (!pkey)
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:118:6: note: Assuming 'vc_ctrl_req' is null
if (!vc_ctrl_req) {
^~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:118:2: note: Taking true branch
if (!vc_ctrl_req) {
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:120:3: note: Control jumps to line 161
goto out;
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:164:6: note: 'err' is < 0
if (err < 0)
^~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:164:2: note: Taking true branch
if (err < 0)
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:165:3: note: Loop condition is false. Exiting loop
pr_err("virtio_crypto: Create session failed status: %u\n",
^
include/linux/printk.h:489:2: note: expanded from macro 'pr_err'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^
include/linux/printk.h:446:26: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
^
include/linux/printk.h:417:3: note: expanded from macro 'printk_index_wrap'
__printk_index_emit(_fmt, NULL, NULL); \
^
include/linux/printk.h:392:34: note: expanded from macro '__printk_index_emit'
#define __printk_index_emit(...) do {} while (0)
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:166:4: note: Access to field 'status' results in a dereference of an undefined pointer value (loaded from variable 'input')
le32_to_cpu(input->status));
^
include/linux/printk.h:489:33: note: expanded from macro 'pr_err'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
include/linux/printk.h:446:60: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
^~~~~~~~~~~
include/linux/printk.h:418:19: note: expanded from macro 'printk_index_wrap'
_p_func(_fmt, ##__VA_ARGS__); \
^~~~~~~~~~~
include/linux/byteorder/generic.h:89:21: note: expanded from macro 'le32_to_cpu'
#define le32_to_cpu __le32_to_cpu
^
include/uapi/linux/byteorder/little_endian.h:35:50: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^~
>> drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:166:4: warning: Use of memory after it is freed [clang-analyzer-unix.Malloc]
le32_to_cpu(input->status));
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:480:9: note: Calling 'virtio_crypto_rsa_set_key'
return virtio_crypto_rsa_set_key(tfm, key, keylen, 0,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:405:6: note: 'private' is false
if (private) {
^~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:405:2: note: Taking false branch
if (private) {
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:413:6: note: Assuming 'ret' is 0
if (ret)
^~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:413:2: note: Taking false branch
if (ret)
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:417:6: note: Assuming field 'n' is non-null
if (!rsa_ctx->n)
^~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:417:2: note: Taking false branch
if (!rsa_ctx->n)
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:420:6: note: Assuming field 'vcrypto' is non-null
if (!ctx->vcrypto) {
^~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:420:2: note: Taking false branch
if (!ctx->vcrypto) {
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:445:9: note: Calling 'virtio_crypto_alg_akcipher_init_session'
return virtio_crypto_alg_akcipher_init_session(ctx, &header, ¶, key, keylen);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:114:6: note: Assuming 'pkey' is non-null
if (!pkey)
^~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:114:2: note: Taking false branch
if (!pkey)
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:118:6: note: Assuming 'vc_ctrl_req' is non-null
if (!vc_ctrl_req) {
^~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:118:2: note: Taking false branch
if (!vc_ctrl_req) {
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:140:6: note: Assuming 'err' is < 0
if (err < 0) {
^~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:140:2: note: Taking true branch
if (err < 0) {
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:142:3: note: Control jumps to line 161
goto out;
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:161:2: note: Memory is released
kfree(vc_ctrl_req);
^~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:164:6: note: 'err' is < 0
if (err < 0)
^~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:164:2: note: Taking true branch
if (err < 0)
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:165:3: note: Loop condition is false. Exiting loop
pr_err("virtio_crypto: Create session failed status: %u\n",
^
include/linux/printk.h:489:2: note: expanded from macro 'pr_err'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^
include/linux/printk.h:446:26: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
^
include/linux/printk.h:417:3: note: expanded from macro 'printk_index_wrap'
__printk_index_emit(_fmt, NULL, NULL); \
^
include/linux/printk.h:392:34: note: expanded from macro '__printk_index_emit'
#define __printk_index_emit(...) do {} while (0)
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:166:4: note: Use of memory after it is freed
le32_to_cpu(input->status));
^
include/linux/printk.h:489:33: note: expanded from macro 'pr_err'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
include/linux/printk.h:446:60: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
^~~~~~~~~~~
include/linux/printk.h:418:19: note: expanded from macro 'printk_index_wrap'
_p_func(_fmt, ##__VA_ARGS__); \
^~~~~~~~~~~
include/linux/byteorder/generic.h:89:21: note: expanded from macro 'le32_to_cpu'
#define le32_to_cpu __le32_to_cpu
^
include/uapi/linux/byteorder/little_endian.h:35:50: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:230:2: warning: 1st function call argument is an uninitialized value [clang-analyzer-core.CallAndMessage]
kfree(vc_ctrl_req);
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:510:2: note: Calling 'virtio_crypto_alg_akcipher_close_session'
virtio_crypto_alg_akcipher_close_session(ctx);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:180:2: note: 'vc_ctrl_req' declared without an initial value
struct virtio_crypto_ctrl_request *vc_ctrl_req;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:182:6: note: Assuming field 'session_valid' is false
if (!ctx->session_valid) {
^~~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:182:2: note: Taking true branch
if (!ctx->session_valid) {
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:184:3: note: Control jumps to line 230
goto out;
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:230:2: note: 1st function call argument is an uninitialized value
kfree(vc_ctrl_req);
^ ~~~~~~~~~~~
>> drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:233:4: warning: Access to field 'status' results in a dereference of an undefined pointer value (loaded from variable 'ctrl_status') [clang-analyzer-core.NullDereference]
ctrl_status->status, destroy_session->session_id);
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:510:2: note: Calling 'virtio_crypto_alg_akcipher_close_session'
virtio_crypto_alg_akcipher_close_session(ctx);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:179:2: note: 'ctrl_status' declared without an initial value
struct virtio_crypto_inhdr *ctrl_status;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:182:6: note: Assuming field 'session_valid' is true
if (!ctx->session_valid) {
^~~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:182:2: note: Taking false branch
if (!ctx->session_valid) {
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:188:6: note: Assuming 'vc_ctrl_req' is null
if (!vc_ctrl_req) {
^~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:188:2: note: Taking true branch
if (!vc_ctrl_req) {
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:190:3: note: Control jumps to line 230
goto out;
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:231:6: note: 'err' is < 0
if (err < 0) {
^~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:231:2: note: Taking true branch
if (err < 0) {
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:232:3: note: Loop condition is false. Exiting loop
pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n",
^
include/linux/printk.h:489:2: note: expanded from macro 'pr_err'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^
include/linux/printk.h:446:26: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
^
include/linux/printk.h:417:3: note: expanded from macro 'printk_index_wrap'
__printk_index_emit(_fmt, NULL, NULL); \
^
include/linux/printk.h:392:34: note: expanded from macro '__printk_index_emit'
#define __printk_index_emit(...) do {} while (0)
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:233:4: note: Access to field 'status' results in a dereference of an undefined pointer value (loaded from variable 'ctrl_status')
ctrl_status->status, destroy_session->session_id);
^
include/linux/printk.h:489:33: note: expanded from macro 'pr_err'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
include/linux/printk.h:446:60: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
^~~~~~~~~~~
include/linux/printk.h:418:19: note: expanded from macro 'printk_index_wrap'
_p_func(_fmt, ##__VA_ARGS__); \
^~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:233:4: warning: Use of memory after it is freed [clang-analyzer-unix.Malloc]
ctrl_status->status, destroy_session->session_id);
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:510:2: note: Calling 'virtio_crypto_alg_akcipher_close_session'
virtio_crypto_alg_akcipher_close_session(ctx);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:182:6: note: Assuming field 'session_valid' is true
if (!ctx->session_valid) {
^~~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:182:2: note: Taking false branch
if (!ctx->session_valid) {
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:188:6: note: Assuming 'vc_ctrl_req' is non-null
if (!vc_ctrl_req) {
^~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:188:2: note: Taking false branch
if (!vc_ctrl_req) {
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:210:6: note: Assuming 'err' is < 0
if (err < 0) {
^~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:210:2: note: Taking true branch
if (err < 0) {
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:212:3: note: Control jumps to line 230
goto out;
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:230:2: note: Memory is released
kfree(vc_ctrl_req);
^~~~~~~~~~~~~~~~~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:231:6: note: 'err' is < 0
if (err < 0) {
^~~
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:231:2: note: Taking true branch
if (err < 0) {
^
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:232:3: note: Loop condition is false. Exiting loop
pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n",
^
include/linux/printk.h:489:2: note: expanded from macro 'pr_err'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^
include/linux/printk.h:446:26: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
vim +166 drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
59ca6c93387d32 zhenwei pi 2022-03-02 98
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));
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);
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 }
59ca6c93387d32 zhenwei pi 2022-03-02 238
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2022-05-05 9:33 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-24 10:41 [PATCH v4 0/5] virtio-crypto: Improve performance zhenwei pi
2022-04-24 10:41 ` zhenwei pi
2022-04-24 10:41 ` [PATCH v4 1/5] virtio-crypto: change code style zhenwei pi
2022-04-24 10:41 ` zhenwei pi
2022-04-26 6:12 ` Jason Wang
2022-04-26 6:12 ` Jason Wang
2022-04-26 6:29 ` zhenwei pi
2022-04-26 6:29 ` zhenwei pi
2022-04-24 10:41 ` [PATCH v4 2/5] virtio-crypto: use private buffer for control request zhenwei pi
2022-04-24 10:41 ` zhenwei pi
2022-04-24 10:41 ` [PATCH v4 3/5] virtio-crypto: wait ctrl queue instead of busy polling zhenwei pi
2022-04-24 10:41 ` zhenwei pi
2022-04-24 10:41 ` [PATCH v4 4/5] virtio-crypto: adjust dst_len at ops callback zhenwei pi
2022-04-24 10:41 ` zhenwei pi
2022-04-24 10:41 ` [PATCH v4 5/5] virtio-crypto: enable retry for virtio-crypto-dev zhenwei pi
2022-04-24 10:41 ` zhenwei pi
2022-05-05 2:35 ` PING: [PATCH v4 0/5] virtio-crypto: Improve performance zhenwei pi
2022-05-05 2:35 ` zhenwei pi
2022-05-05 3:14 ` Gonglei (Arei)
2022-05-05 3:14 ` Gonglei (Arei) via Virtualization
2022-05-05 4:57 ` Michael S. Tsirkin
2022-05-05 4:57 ` Michael S. Tsirkin
2022-05-05 9:29 ` zhenwei pi
2022-05-05 9:29 ` zhenwei pi
2022-04-24 15:51 [PATCH v4 2/5] virtio-crypto: use private buffer for control request kernel test robot
2022-04-25 13:27 ` Dan Carpenter
2022-04-25 13:27 ` Dan Carpenter
2022-04-25 13:27 ` Dan Carpenter
2022-04-26 7:49 kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.