Messages in this thread | | | Date | Fri, 22 Apr 2022 17:08:13 +0800 | Subject | Re: Re: [PATCH v3 1/5] virtio-crypto: use private buffer for control request | From | zhenwei pi <> |
| |
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@redhat.com> >> Cc: Jason Wang <jasowang@redhat.com> >> Cc: Gonglei <arei.gonglei@huawei.com> >> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> >> --- >> 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.
> >> 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.
> >> - 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@bytedance.com> >> + * >> + * 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) > >> 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
| |