Re: Re: Re: [PATCH v3 1/5] virtio-crypto: use private buffer for control request

From: zhenwei pi
Date: Sun Apr 24 2022 - 06:46:45 EST

Next message: Hyeonggon Yoo: "Re: [PATCH v3] mm: make minimum slab alignment a runtime property"
Previous message: zhenwei pi: "[PATCH v4 5/5] virtio-crypto: enable retry for virtio-crypto-dev"
In reply to: Jason Wang: "Re: Re: [PATCH v3 1/5] virtio-crypto: use private buffer for control request"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 4/24/22 14:21, Jason Wang wrote:

On Fri, Apr 22, 2022 at 5:12 PM zhenwei pi <pizhenwei@xxxxxxxxxxxxx> wrote:

On 4/22/22 15:41, Jason Wang wrote:

在 2022/4/21 18:40, 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, 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,
* 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@xxxxxxxxxx>
Cc: Jason Wang <jasowang@xxxxxxxxxx>
Cc: Gonglei <arei.gonglei@xxxxxxxxxx>
Signed-off-by: zhenwei pi <pizhenwei@xxxxxxxxxxxxx>
---
drivers/crypto/virtio/Makefile | 1 +
.../virtio/virtio_crypto_akcipher_algs.c | 90 ++++++------
drivers/crypto/virtio/virtio_crypto_common.c | 39 +++++

Any reason we can't use virtio_crypto_core.c?

Another patch in this series: [PATCH v3 3/5] virtio-crypto: move helpers
into virtio_crypto_common.c

Move virtcrypto_clear_request and virtcrypto_dataq_callback into
virtio_crypto_common.c to make code clear. 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

So I put this into a new file.

I don't see obvious differences but we can leave it to the
virtio-crypto maintainers to decide.

OK!

drivers/crypto/virtio/virtio_crypto_common.h | 19 ++-
.../virtio/virtio_crypto_skcipher_algs.c | 133 ++++++++----------
5 files changed, 156 insertions(+), 126 deletions(-)
create mode 100644 drivers/crypto/virtio/virtio_crypto_common.c

diff --git a/drivers/crypto/virtio/Makefile
b/drivers/crypto/virtio/Makefile
index bfa6cbae342e..49c1fa80e465 100644
--- a/drivers/crypto/virtio/Makefile
+++ b/drivers/crypto/virtio/Makefile
@@ -3,5 +3,6 @@ obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio_crypto.o
virtio_crypto-objs := \
virtio_crypto_skcipher_algs.o \
virtio_crypto_akcipher_algs.o \
+ virtio_crypto_common.o \
virtio_crypto_mgr.o \
virtio_crypto_core.o
diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
index f3ec9420215e..9561bc2df62b 100644
--- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
@@ -102,8 +102,8 @@ 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;
+ struct virtio_crypto_ctrl_request *vc_ctrl_req;
uint8_t *pkey;
- unsigned int inlen;
int err;
unsigned int num_out = 0, num_in = 0;
@@ -111,98 +111,91 @@ static int
virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher
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);
+ vc_ctrl_req = kzalloc(sizeof(*vc_ctrl_req), GFP_KERNEL);
+ if (!vc_ctrl_req) {
+ err = -ENOMEM;
+ goto out;
+ }
- sg_init_one(&outhdr_sg, &vcrypto->ctrl, sizeof(vcrypto->ctrl));
+ memcpy(&vc_ctrl_req->ctrl.header, header,
sizeof(vc_ctrl_req->ctrl.header));
+ memcpy(&vc_ctrl_req->ctrl.u, para, sizeof(vc_ctrl_req->ctrl.u));
+ sg_init_one(&outhdr_sg, &vc_ctrl_req->ctrl,
sizeof(vc_ctrl_req->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));
+ vc_ctrl_req->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR);

Nit: if there's no special reason, let's move this after the above
memcpys as what's done previously.

+ sg_init_one(&inhdr_sg, &vc_ctrl_req->input,
sizeof(vc_ctrl_req->input));
sgs[num_out + num_in++] = &inhdr_sg;
- err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in,
vcrypto, GFP_ATOMIC);
+ err = virtio_crypto_ctrl_vq_request(vcrypto, sgs, num_out,
num_in, vc_ctrl_req);

I'd split this into a separate patch.

OK!

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();
-
- if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) {
+ if (le32_to_cpu(vc_ctrl_req->input.status) != VIRTIO_CRYPTO_OK) {
+ pr_err("virtio_crypto: Create session failed status: %u\n",
+ le32_to_cpu(vc_ctrl_req->input.status));
err = -EINVAL;
goto out;
}

Do we need a warning for -ENOMEM?

Memory(especially small size) allocation is unlikely case, I also check
the virtio_net and virtio_blk, both handles -ENOMEM only without error
reporting.

Ok.

- ctx->session_id = le64_to_cpu(vcrypto->input.session_id);
+ ctx->session_id = le64_to_cpu(vc_ctrl_req->input.session_id);
ctx->session_valid = true;
err = 0;
out:
- spin_unlock(&vcrypto->ctrl_lock);
+ kfree(vc_ctrl_req);
kfree_sensitive(pkey);
- if (err < 0)
- pr_err("virtio_crypto: Create session failed status: %u\n",
- le32_to_cpu(vcrypto->input.status));
-
return err;
}
static int virtio_crypto_alg_akcipher_close_session(struct
virtio_crypto_akcipher_ctx *ctx)
{
struct scatterlist outhdr_sg, inhdr_sg, *sgs[2];
+ struct virtio_crypto_ctrl_request *vc_ctrl_req;
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;
- 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;
+ if (!ctx->session_valid)
+ return 0;
- destroy_session = &vcrypto->ctrl.u.destroy_session;
+ vc_ctrl_req = kzalloc(sizeof(*vc_ctrl_req), GFP_KERNEL);
+ if (!vc_ctrl_req)
+ return -ENOMEM;
+
+ vc_ctrl_req->ctrl.header.opcode =
cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION);
+ vc_ctrl_req->ctrl.header.queue_id = 0;
+
+ destroy_session = &vc_ctrl_req->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, &vc_ctrl_req->ctrl,
sizeof(vc_ctrl_req->ctrl));
sgs[num_out++] = &outhdr_sg;
- sg_init_one(&inhdr_sg, &vcrypto->ctrl_status.status,
sizeof(vcrypto->ctrl_status.status));
+ vc_ctrl_req->ctrl_status.status = VIRTIO_CRYPTO_ERR;

If no special reason, let's move this above:

vc_ctrl_req->ctrl.header.opcode =
cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION);

OK!

+ sg_init_one(&inhdr_sg, &vc_ctrl_req->ctrl_status.status,
+ sizeof(vc_ctrl_req->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);
+ 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();
-
- if (vcrypto->ctrl_status.status != VIRTIO_CRYPTO_OK) {
+ if (vc_ctrl_req->ctrl_status.status != VIRTIO_CRYPTO_OK) {
err = -EINVAL;
+ pr_err("virtio_crypto: Close session failed status: %u,
session_id: 0x%llx\n",
+ vc_ctrl_req->ctrl_status.status,
destroy_session->session_id);
goto out;
}
err = 0;
ctx->session_valid = false;
-
out:
- 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);
- }
+ kfree(vc_ctrl_req);
return err;
}
@@ -210,14 +203,11 @@ static int
virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
static int __virtio_crypto_akcipher_do_req(struct
virtio_crypto_akcipher_request *vc_akcipher_req,
struct akcipher_request *req, struct data_queue *data_vq)
{
- struct virtio_crypto_akcipher_ctx *ctx =
vc_akcipher_req->akcipher_ctx;
struct virtio_crypto_request *vc_req = &vc_akcipher_req->base;
- struct virtio_crypto *vcrypto = ctx->vcrypto;
struct virtio_crypto_op_data_req *req_data = vc_req->req_data;
struct scatterlist *sgs[4], outhdr_sg, inhdr_sg, srcdata_sg,
dstdata_sg;
void *src_buf = NULL, *dst_buf = NULL;
unsigned int num_out = 0, num_in = 0;
- int node = dev_to_node(&vcrypto->vdev->dev);
unsigned long flags;
int ret = -ENOMEM;
bool verify = vc_akcipher_req->opcode ==
VIRTIO_CRYPTO_AKCIPHER_VERIFY;
@@ -228,7 +218,7 @@ static int __virtio_crypto_akcipher_do_req(struct
virtio_crypto_akcipher_request
sgs[num_out++] = &outhdr_sg;
/* src data */
- src_buf = kcalloc_node(src_len, 1, GFP_KERNEL, node);
+ src_buf = kcalloc(src_len, 1, GFP_KERNEL);

This seems not a relevant change. If it is a must we need use a separate
for this and describe the rationale.

if (!src_buf)
goto err;
@@ -243,7 +233,7 @@ static int __virtio_crypto_akcipher_do_req(struct
virtio_crypto_akcipher_request
sgs[num_out++] = &srcdata_sg;
/* dst data */
- dst_buf = kcalloc_node(req->dst_len, 1, GFP_KERNEL, node);
+ dst_buf = kcalloc(req->dst_len, 1, GFP_KERNEL);

And this.

if (!dst_buf)
goto err;
diff --git a/drivers/crypto/virtio/virtio_crypto_common.c
b/drivers/crypto/virtio/virtio_crypto_common.c
new file mode 100644
index 000000000000..e65125a74db2
--- /dev/null
+++ b/drivers/crypto/virtio/virtio_crypto_common.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Common functions and helpers
+ *
+ * Authors: zhenwei pi <pizhenwei@xxxxxxxxxxxxx>
+ *
+ * Copyright 2022 Bytedance CO., LTD.
+ */
+
+#include "virtio_crypto_common.h"
+
+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 int inlen;
+ unsigned long flags;
+
+ 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);
+
+ /*
+ * Trapping into the hypervisor, so the request should be
+ * handled immediately.
+ */
+ while (!virtqueue_get_buf(vcrypto->ctrl_vq, &inlen) &&
+ !virtqueue_is_broken(vcrypto->ctrl_vq))
+ cpu_relax();
+
+ spin_unlock_irqrestore(&vcrypto->ctrl_lock, flags);
+
+ return 0;
+}
diff --git a/drivers/crypto/virtio/virtio_crypto_common.h
b/drivers/crypto/virtio/virtio_crypto_common.h
index e693d4ee83a6..d2a20fe6e13e 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,16 @@ struct virtio_crypto_sym_session_info {
__u64 session_id;
};
+/*
+ * Note: there are padding fields in request, clear them to zero
before sending to host,
+ * 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);
@@ -135,4 +141,7 @@ 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_skcipher_algs.c
b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
index a618c46a52b8..fef355ff461c 100644
--- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
@@ -118,11 +118,13 @@ 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;

Can we simply rename this to virtcrypto and then we can use the name
"vcrypto" for virtio_crypto_ctrl_request then we save tons lot changes?

It simplify the life of reviewers and backporting.

Thanks

This series focuses on performance improvment, and keeps the style with
the orignal style. What about fixing this in another series? (If so,
I'll fix this later)

Just in case we are at the same page, what I suggest can save hundreds
lines of codes:

E.g:

- vcrypto->ctrl.header.queue_id = 0;
+ header->queue_id = 0;

If we stick to the name of "vcrypto" we don't need this change at all.

It can simplify:

1) future context difference when backporting patches to -stable
2) reviewing

Thanks

Oh, sorry, I misunderstood this. I agree with this point, but I notice that 'vcrypto' is always a variable of 'type struct virtio_crypto *' in this driver, should we break this?

To simplify reviewing, I have another opinion in the v4 series, could you please review it?

int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT :
VIRTIO_CRYPTO_OP_DECRYPT;
int err;
unsigned int num_out = 0, num_in = 0;
+ struct virtio_crypto_ctrl_request *vc_ctrl_req;
+ struct virtio_crypto_ctrl_header *header;
+ struct virtio_crypto_sym_create_session_req *sym_create_session;
/*
* Avoid to do DMA from the stack, switch to using
@@ -133,26 +135,27 @@ 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 */
- vcrypto->ctrl.header.opcode =
- cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION);
- vcrypto->ctrl.header.algo = cpu_to_le32(alg);
+ header = &vc_ctrl_req->ctrl.header;
+ header->opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION);
+ header->algo = cpu_to_le32(alg);
/* Set the default dataqueue id to 0 */
- vcrypto->ctrl.header.queue_id = 0;
+ header->queue_id = 0;
- vcrypto->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 = &vc_ctrl_req->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 = 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, &vc_ctrl_req->ctrl, sizeof(vc_ctrl_req->ctrl));
sgs[num_out++] = &outhdr;
/* Set key */
@@ -160,45 +163,34 @@ 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));
+ vc_ctrl_req->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
+ sg_init_one(&inhdr, &vc_ctrl_req->input,
sizeof(vc_ctrl_req->input));
sgs[num_out + num_in++] = &inhdr;
- 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;
- }
- 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();
+ err = virtio_crypto_ctrl_vq_request(vcrypto, sgs, num_out,
num_in, vc_ctrl_req);
+ if (err < 0)
+ goto out;
- if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) {
- spin_unlock(&vcrypto->ctrl_lock);
+ if (le32_to_cpu(vc_ctrl_req->input.status) != VIRTIO_CRYPTO_OK) {
pr_err("virtio_crypto: Create session failed status: %u\n",
- le32_to_cpu(vcrypto->input.status));
- kfree_sensitive(cipher_key);
- return -EINVAL;
+ le32_to_cpu(vc_ctrl_req->input.status));
+ err = -EINVAL;
+ goto out;
}
if (encrypt)
ctx->enc_sess_info.session_id =
- le64_to_cpu(vcrypto->input.session_id);
+ le64_to_cpu(vc_ctrl_req->input.session_id);
else
ctx->dec_sess_info.session_id =
- le64_to_cpu(vcrypto->input.session_id);
-
- spin_unlock(&vcrypto->ctrl_lock);
+ le64_to_cpu(vc_ctrl_req->input.session_id);
+ err = 0;
+out:
+ kfree(vc_ctrl_req);
kfree_sensitive(cipher_key);
- return 0;
+
+ return err;
}
static int virtio_crypto_alg_skcipher_close_session(
@@ -206,21 +198,24 @@ 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;
unsigned int num_out = 0, num_in = 0;
+ struct virtio_crypto_ctrl_request *vc_ctrl_req;
+ struct virtio_crypto_ctrl_header *header;
+
+ vc_ctrl_req = kzalloc(sizeof(*vc_ctrl_req), GFP_KERNEL);
+ if (!vc_ctrl_req)
+ return -ENOMEM;
- spin_lock(&vcrypto->ctrl_lock);
- vcrypto->ctrl_status.status = VIRTIO_CRYPTO_ERR;
/* Pad ctrl header */
- vcrypto->ctrl.header.opcode =
- cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION);
+ header = &vc_ctrl_req->ctrl.header;
+ header->opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION);
/* Set the default virtqueue id to 0 */
- vcrypto->ctrl.header.queue_id = 0;
+ header->queue_id = 0;
- destroy_session = &vcrypto->ctrl.u.destroy_session;
+ destroy_session = &vc_ctrl_req->ctrl.u.destroy_session;
if (encrypt)
destroy_session->session_id =
@@ -229,37 +224,33 @@ static int
virtio_crypto_alg_skcipher_close_session(
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, &vc_ctrl_req->ctrl, sizeof(vc_ctrl_req->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));
+ vc_ctrl_req->ctrl_status.status = VIRTIO_CRYPTO_ERR;
+ sg_init_one(&status_sg, &vc_ctrl_req->ctrl_status.status,
+ sizeof(vc_ctrl_req->ctrl_status.status));
sgs[num_out + num_in++] = &status_sg;
- 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;
- }
- virtqueue_kick(vcrypto->ctrl_vq);
-
- while (!virtqueue_get_buf(vcrypto->ctrl_vq, &tmp) &&
- !virtqueue_is_broken(vcrypto->ctrl_vq))
- cpu_relax();
+ err = virtio_crypto_ctrl_vq_request(vcrypto, sgs, num_out,
num_in, vc_ctrl_req);
+ if (err < 0)
+ goto out;
- if (vcrypto->ctrl_status.status != VIRTIO_CRYPTO_OK) {
- spin_unlock(&vcrypto->ctrl_lock);
+ if (vc_ctrl_req->ctrl_status.status != VIRTIO_CRYPTO_OK) {
pr_err("virtio_crypto: Close session failed status: %u,
session_id: 0x%llx\n",
- vcrypto->ctrl_status.status,
+ vc_ctrl_req->ctrl_status.status,
destroy_session->session_id);
- return -EINVAL;
+ err = -EINVAL;
+ goto out;
}
- spin_unlock(&vcrypto->ctrl_lock);
- return 0;
+ err = 0;
+out:
+ kfree(vc_ctrl_req);
+
+ return err;
}
static int virtio_crypto_alg_skcipher_init_sessions(

--
zhenwei pi

--
zhenwei pi

Next message: Hyeonggon Yoo: "Re: [PATCH v3] mm: make minimum slab alignment a runtime property"
Previous message: zhenwei pi: "[PATCH v4 5/5] virtio-crypto: enable retry for virtio-crypto-dev"
In reply to: Jason Wang: "Re: Re: [PATCH v3 1/5] virtio-crypto: use private buffer for control request"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]