From: "Michael S. Tsirkin" <mst@redhat.com>
To: Lei He <helei.sig11@bytedance.com>
Cc: berrange@redhat.com, arei.gonglei@huawei.com,
pizhenwei@bytedance.com, qemu-devel@nongnu.org
Subject: Re: [PATCH v2 1/4] virtio-crypto: Support asynchronous mode
Date: Tue, 1 Nov 2022 15:51:36 -0400 [thread overview]
Message-ID: <20221101154944-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20221101063515-mutt-send-email-mst@kernel.org>
On Tue, Nov 01, 2022 at 06:37:26AM -0400, Michael S. Tsirkin wrote:
> On Sat, Oct 08, 2022 at 04:50:27PM +0800, Lei He wrote:
> > virtio-crypto: Modify the current interface of virtio-crypto
> > device to support asynchronous mode.
> >
> > Signed-off-by: lei he <helei.sig11@bytedance.com>
> > ---
> > backends/cryptodev-builtin.c | 69 ++++++---
> > backends/cryptodev-vhost-user.c | 51 +++++--
> > backends/cryptodev.c | 44 +++---
> > hw/virtio/virtio-crypto.c | 324 ++++++++++++++++++++++------------------
> > include/sysemu/cryptodev.h | 60 +++++---
> > 5 files changed, 336 insertions(+), 212 deletions(-)
> >
> > diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
> > index 125cbad1d3..cda6ca3b71 100644
> > --- a/backends/cryptodev-builtin.c
> > +++ b/backends/cryptodev-builtin.c
> > @@ -355,42 +355,62 @@ static int cryptodev_builtin_create_akcipher_session(
> > return index;
> > }
> >
> > -static int64_t cryptodev_builtin_create_session(
> > +static int cryptodev_builtin_create_session(
> > CryptoDevBackend *backend,
> > CryptoDevBackendSessionInfo *sess_info,
> > - uint32_t queue_index, Error **errp)
> > + uint32_t queue_index,
> > + CryptoDevCompletionFunc cb,
> > + void *opaque)
> > {
> > CryptoDevBackendBuiltin *builtin =
> > CRYPTODEV_BACKEND_BUILTIN(backend);
> > CryptoDevBackendSymSessionInfo *sym_sess_info;
> > CryptoDevBackendAsymSessionInfo *asym_sess_info;
> > + int ret, status;
> > + Error *local_error = NULL;
> >
> > switch (sess_info->op_code) {
> > case VIRTIO_CRYPTO_CIPHER_CREATE_SESSION:
> > sym_sess_info = &sess_info->u.sym_sess_info;
> > - return cryptodev_builtin_create_cipher_session(
> > - builtin, sym_sess_info, errp);
> > + ret = cryptodev_builtin_create_cipher_session(
> > + builtin, sym_sess_info, &local_error);
> > + break;
> >
> > case VIRTIO_CRYPTO_AKCIPHER_CREATE_SESSION:
> > asym_sess_info = &sess_info->u.asym_sess_info;
> > - return cryptodev_builtin_create_akcipher_session(
> > - builtin, asym_sess_info, errp);
> > + ret = cryptodev_builtin_create_akcipher_session(
> > + builtin, asym_sess_info, &local_error);
> > + break;
> >
> > case VIRTIO_CRYPTO_HASH_CREATE_SESSION:
> > case VIRTIO_CRYPTO_MAC_CREATE_SESSION:
> > default:
> > - error_setg(errp, "Unsupported opcode :%" PRIu32 "",
> > + error_setg(&local_error, "Unsupported opcode :%" PRIu32 "",
> > sess_info->op_code);
> > - return -1;
> > + return -VIRTIO_CRYPTO_NOTSUPP;
> > }
> >
> > - return -1;
> > + if (local_error) {
> > + error_report_err(local_error);
> > + }
> > + if (ret < 0) {
> > + status = -VIRTIO_CRYPTO_ERR;
> > + } else {
> > + sess_info->session_id = ret;
> > + status = VIRTIO_CRYPTO_OK;
> > + }
> > + if (cb) {
> > + cb(opaque, status);
> > + }
> > + return 0;
> > }
> >
> > static int cryptodev_builtin_close_session(
> > CryptoDevBackend *backend,
> > uint64_t session_id,
> > - uint32_t queue_index, Error **errp)
> > + uint32_t queue_index,
> > + CryptoDevCompletionFunc cb,
> > + void *opaque)
> > {
> > CryptoDevBackendBuiltin *builtin =
> > CRYPTODEV_BACKEND_BUILTIN(backend);
> > @@ -407,6 +427,9 @@ static int cryptodev_builtin_close_session(
> >
> > g_free(session);
> > builtin->sessions[session_id] = NULL;
> > + if (cb) {
> > + cb(opaque, VIRTIO_CRYPTO_OK);
> > + }
> > return 0;
> > }
> >
> > @@ -506,7 +529,9 @@ static int cryptodev_builtin_asym_operation(
> > static int cryptodev_builtin_operation(
> > CryptoDevBackend *backend,
> > CryptoDevBackendOpInfo *op_info,
> > - uint32_t queue_index, Error **errp)
> > + uint32_t queue_index,
> > + CryptoDevCompletionFunc cb,
> > + void *opaque)
> > {
> > CryptoDevBackendBuiltin *builtin =
> > CRYPTODEV_BACKEND_BUILTIN(backend);
> > @@ -514,11 +539,12 @@ static int cryptodev_builtin_operation(
> > CryptoDevBackendSymOpInfo *sym_op_info;
> > CryptoDevBackendAsymOpInfo *asym_op_info;
> > enum CryptoDevBackendAlgType algtype = op_info->algtype;
> > - int ret = -VIRTIO_CRYPTO_ERR;
> > + int status = -VIRTIO_CRYPTO_ERR;
> > + Error *local_error = NULL;
> >
> > if (op_info->session_id >= MAX_NUM_SESSIONS ||
> > builtin->sessions[op_info->session_id] == NULL) {
> > - error_setg(errp, "Cannot find a valid session id: %" PRIu64 "",
> > + error_setg(&local_error, "Cannot find a valid session id: %" PRIu64 "",
> > op_info->session_id);
> > return -VIRTIO_CRYPTO_INVSESS;
> > }
> > @@ -526,14 +552,21 @@ static int cryptodev_builtin_operation(
> > sess = builtin->sessions[op_info->session_id];
> > if (algtype == CRYPTODEV_BACKEND_ALG_SYM) {
> > sym_op_info = op_info->u.sym_op_info;
> > - ret = cryptodev_builtin_sym_operation(sess, sym_op_info, errp);
> > + status = cryptodev_builtin_sym_operation(sess, sym_op_info,
> > + &local_error);
> > } else if (algtype == CRYPTODEV_BACKEND_ALG_ASYM) {
> > asym_op_info = op_info->u.asym_op_info;
> > - ret = cryptodev_builtin_asym_operation(sess, op_info->op_code,
> > - asym_op_info, errp);
> > + status = cryptodev_builtin_asym_operation(sess, op_info->op_code,
> > + asym_op_info, &local_error);
> > }
> >
> > - return ret;
> > + if (local_error) {
> > + error_report_err(local_error);
> > + }
> > + if (cb) {
> > + cb(opaque, status);
> > + }
> > + return 0;
> > }
> >
> > static void cryptodev_builtin_cleanup(
> > @@ -548,7 +581,7 @@ static void cryptodev_builtin_cleanup(
> >
> > for (i = 0; i < MAX_NUM_SESSIONS; i++) {
> > if (builtin->sessions[i] != NULL) {
> > - cryptodev_builtin_close_session(backend, i, 0, &error_abort);
> > + cryptodev_builtin_close_session(backend, i, 0, NULL, NULL);
> > }
> > }
> >
> > diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c
> > index 5443a59153..351b2729e0 100644
> > --- a/backends/cryptodev-vhost-user.c
> > +++ b/backends/cryptodev-vhost-user.c
> > @@ -259,13 +259,18 @@ static int64_t cryptodev_vhost_user_sym_create_session(
> > return -1;
> > }
> >
> > -static int64_t cryptodev_vhost_user_create_session(
> > +static int cryptodev_vhost_user_create_session(
> > CryptoDevBackend *backend,
> > CryptoDevBackendSessionInfo *sess_info,
> > - uint32_t queue_index, Error **errp)
> > + uint32_t queue_index,
> > + CryptoDevCompletionFunc cb,
> > + void *opaque)
> > {
> > uint32_t op_code = sess_info->op_code;
> > CryptoDevBackendSymSessionInfo *sym_sess_info;
> > + int64_t ret;
> > + Error *local_error = NULL;
> > + int status;
> >
> > switch (op_code) {
> > case VIRTIO_CRYPTO_CIPHER_CREATE_SESSION:
> > @@ -273,27 +278,42 @@ static int64_t cryptodev_vhost_user_create_session(
> > case VIRTIO_CRYPTO_MAC_CREATE_SESSION:
> > case VIRTIO_CRYPTO_AEAD_CREATE_SESSION:
> > sym_sess_info = &sess_info->u.sym_sess_info;
> > - return cryptodev_vhost_user_sym_create_session(backend, sym_sess_info,
> > - queue_index, errp);
> > + ret = cryptodev_vhost_user_sym_create_session(backend, sym_sess_info,
> > + queue_index, &local_error);
> > + break;
> > +
> > default:
> > - error_setg(errp, "Unsupported opcode :%" PRIu32 "",
> > + error_setg(&local_error, "Unsupported opcode :%" PRIu32 "",
> > sess_info->op_code);
> > - return -1;
> > -
> > + return -VIRTIO_CRYPTO_NOTSUPP;
> > }
> >
> > - return -1;
> > + if (local_error) {
> > + error_report_err(local_error);
> > + }
> > + if (ret < 0) {
> > + status = -VIRTIO_CRYPTO_ERR;
> > + } else {
> > + sess_info->session_id = ret;
> > + status = VIRTIO_CRYPTO_OK;
> > + }
> > + if (cb) {
> > + cb(opaque, status);
> > + }
> > + return 0;
> > }
> >
> > static int cryptodev_vhost_user_close_session(
> > CryptoDevBackend *backend,
> > uint64_t session_id,
> > - uint32_t queue_index, Error **errp)
> > + uint32_t queue_index,
> > + CryptoDevCompletionFunc cb,
> > + void *opaque)
> > {
> > CryptoDevBackendClient *cc =
> > backend->conf.peers.ccs[queue_index];
> > CryptoDevBackendVhost *vhost_crypto;
> > - int ret;
> > + int ret = -1, status;
> >
> > vhost_crypto = cryptodev_vhost_user_get_vhost(cc, backend, queue_index);
> > if (vhost_crypto) {
> > @@ -301,12 +321,17 @@ static int cryptodev_vhost_user_close_session(
> > ret = dev->vhost_ops->vhost_crypto_close_session(dev,
> > session_id);
> > if (ret < 0) {
> > - return -1;
> > + status = -VIRTIO_CRYPTO_ERR;
> > } else {
> > - return 0;
> > + status = VIRTIO_CRYPTO_OK;
> > }
> > + } else {
> > + status = -VIRTIO_CRYPTO_NOTSUPP;
> > }
> > - return -1;
> > + if (cb) {
> > + cb(opaque, status);
> > + }
> > + return 0;
> > }
> >
> > static void cryptodev_vhost_user_cleanup(
> > diff --git a/backends/cryptodev.c b/backends/cryptodev.c
> > index 33eb4e1a70..54ee8c81f5 100644
> > --- a/backends/cryptodev.c
> > +++ b/backends/cryptodev.c
> > @@ -26,6 +26,7 @@
> > #include "qapi/error.h"
> > #include "qapi/visitor.h"
> > #include "qemu/config-file.h"
> > +#include "qemu/error-report.h"
> > #include "qom/object_interfaces.h"
> > #include "hw/virtio/virtio-crypto.h"
> >
> > @@ -72,69 +73,72 @@ void cryptodev_backend_cleanup(
> > }
> > }
> >
> > -int64_t cryptodev_backend_create_session(
> > +int cryptodev_backend_create_session(
> > CryptoDevBackend *backend,
> > CryptoDevBackendSessionInfo *sess_info,
> > - uint32_t queue_index, Error **errp)
> > + uint32_t queue_index,
> > + CryptoDevCompletionFunc cb,
> > + void *opaque)
> > {
> > CryptoDevBackendClass *bc =
> > CRYPTODEV_BACKEND_GET_CLASS(backend);
> >
> > if (bc->create_session) {
> > - return bc->create_session(backend, sess_info, queue_index, errp);
> > + return bc->create_session(backend, sess_info, queue_index, cb, opaque);
> > }
> > -
> > - return -1;
> > + return -VIRTIO_CRYPTO_NOTSUPP;
> > }
> >
> > int cryptodev_backend_close_session(
> > CryptoDevBackend *backend,
> > uint64_t session_id,
> > - uint32_t queue_index, Error **errp)
> > + uint32_t queue_index,
> > + CryptoDevCompletionFunc cb,
> > + void *opaque)
> > {
> > CryptoDevBackendClass *bc =
> > CRYPTODEV_BACKEND_GET_CLASS(backend);
> >
> > if (bc->close_session) {
> > - return bc->close_session(backend, session_id, queue_index, errp);
> > + return bc->close_session(backend, session_id, queue_index, cb, opaque);
> > }
> > -
> > - return -1;
> > + return -VIRTIO_CRYPTO_NOTSUPP;
> > }
> >
> > static int cryptodev_backend_operation(
> > CryptoDevBackend *backend,
> > CryptoDevBackendOpInfo *op_info,
> > - uint32_t queue_index, Error **errp)
> > + uint32_t queue_index,
> > + CryptoDevCompletionFunc cb,
> > + void *opaque)
> > {
> > CryptoDevBackendClass *bc =
> > CRYPTODEV_BACKEND_GET_CLASS(backend);
> >
> > if (bc->do_op) {
> > - return bc->do_op(backend, op_info, queue_index, errp);
> > + return bc->do_op(backend, op_info, queue_index, cb, opaque);
> > }
> > -
> > - return -VIRTIO_CRYPTO_ERR;
> > + return -VIRTIO_CRYPTO_NOTSUPP;
> > }
> >
> > int cryptodev_backend_crypto_operation(
> > CryptoDevBackend *backend,
> > - void *opaque,
> > - uint32_t queue_index, Error **errp)
> > + void *opaque1,
> > + uint32_t queue_index,
> > + CryptoDevCompletionFunc cb, void *opaque2)
> > {
> > - VirtIOCryptoReq *req = opaque;
> > + VirtIOCryptoReq *req = opaque1;
> > CryptoDevBackendOpInfo *op_info = &req->op_info;
> > enum CryptoDevBackendAlgType algtype = req->flags;
> >
> > if ((algtype != CRYPTODEV_BACKEND_ALG_SYM)
> > && (algtype != CRYPTODEV_BACKEND_ALG_ASYM)) {
> > - error_setg(errp, "Unsupported cryptodev alg type: %" PRIu32 "",
> > - algtype);
> > -
> > + error_report("Unsupported cryptodev alg type: %" PRIu32 "", algtype);
> > return -VIRTIO_CRYPTO_NOTSUPP;
> > }
> >
> > - return cryptodev_backend_operation(backend, op_info, queue_index, errp);
> > + return cryptodev_backend_operation(backend, op_info, queue_index,
> > + cb, opaque2);
> > }
> >
> > static void
> > diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
> > index df4bde210b..39c8f5914e 100644
> > --- a/hw/virtio/virtio-crypto.c
> > +++ b/hw/virtio/virtio-crypto.c
> > @@ -27,6 +27,39 @@
> >
> > #define VIRTIO_CRYPTO_VM_VERSION 1
> >
> > +typedef struct VirtIOCryptoSessionReq {
> > + VirtIODevice *vdev;
> > + VirtQueue *vq;
> > + VirtQueueElement *elem;
> > + CryptoDevBackendSessionInfo info;
> > + CryptoDevCompletionFunc cb;
> > +} VirtIOCryptoSessionReq;
> > +
> > +static void virtio_crypto_free_create_session_req(VirtIOCryptoSessionReq *sreq)
> > +{
> > + switch (sreq->info.op_code) {
> > + case VIRTIO_CRYPTO_CIPHER_CREATE_SESSION:
> > + g_free(sreq->info.u.sym_sess_info.cipher_key);
> > + g_free(sreq->info.u.sym_sess_info.auth_key);
> > + break;
> > +
> > + case VIRTIO_CRYPTO_AKCIPHER_CREATE_SESSION:
> > + g_free(sreq->info.u.asym_sess_info.key);
> > + break;
> > +
> > + case VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION:
> > + case VIRTIO_CRYPTO_HASH_DESTROY_SESSION:
> > + case VIRTIO_CRYPTO_MAC_DESTROY_SESSION:
> > + case VIRTIO_CRYPTO_AEAD_DESTROY_SESSION:
> > + case VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION:
> > + break;
> > +
> > + default:
> > + error_report("Unknown opcode: %u", sreq->info.op_code);
> > + }
> > + g_free(sreq);
> > +}
> > +
> > /*
> > * Transfer virtqueue index to crypto queue index.
> > * The control virtqueue is after the data virtqueues
> > @@ -75,27 +108,24 @@ virtio_crypto_cipher_session_helper(VirtIODevice *vdev,
> > return 0;
> > }
> >
> > -static int64_t
> > +static int
> > virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto,
> > struct virtio_crypto_sym_create_session_req *sess_req,
> > uint32_t queue_id,
> > uint32_t opcode,
> > - struct iovec *iov, unsigned int out_num)
> > + struct iovec *iov, unsigned int out_num,
> > + VirtIOCryptoSessionReq *sreq)
> > {
> > VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
> > - CryptoDevBackendSessionInfo info;
> > - CryptoDevBackendSymSessionInfo *sym_info;
> > - int64_t session_id;
> > + CryptoDevBackendSymSessionInfo *sym_info = &sreq->info.u.sym_sess_info;
> > int queue_index;
> > uint32_t op_type;
> > - Error *local_err = NULL;
> > int ret;
> >
> > - memset(&info, 0, sizeof(info));
> > op_type = ldl_le_p(&sess_req->op_type);
> > - info.op_code = opcode;
> > + sreq->info.op_code = opcode;
> >
> > - sym_info = &info.u.sym_sess_info;
> > + sym_info = &sreq->info.u.sym_sess_info;
> > sym_info->op_type = op_type;
> >
> > if (op_type == VIRTIO_CRYPTO_SYM_OP_CIPHER) {
> > @@ -103,7 +133,7 @@ virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto,
> > &sess_req->u.cipher.para,
> > &iov, &out_num);
> > if (ret < 0) {
> > - goto err;
> > + return ret;
> > }
> > } else if (op_type == VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING) {
> > size_t s;
> > @@ -112,7 +142,7 @@ virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto,
> > &sess_req->u.chain.para.cipher_param,
> > &iov, &out_num);
> > if (ret < 0) {
> > - goto err;
> > + return ret;
> > }
> > /* hash part */
> > sym_info->alg_chain_order = ldl_le_p(
> > @@ -129,8 +159,7 @@ virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto,
> > if (sym_info->auth_key_len > vcrypto->conf.max_auth_key_len) {
> > error_report("virtio-crypto length of auth key is too big: %u",
> > sym_info->auth_key_len);
> > - ret = -VIRTIO_CRYPTO_ERR;
> > - goto err;
> > + return -VIRTIO_CRYPTO_ERR;
> > }
> > /* get auth key */
> > if (sym_info->auth_key_len > 0) {
> > @@ -140,8 +169,7 @@ virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto,
> > if (unlikely(s != sym_info->auth_key_len)) {
> > virtio_error(vdev,
> > "virtio-crypto authenticated key incorrect");
> > - ret = -EFAULT;
> > - goto err;
> > + return -EFAULT;
> > }
> > iov_discard_front(&iov, &out_num, sym_info->auth_key_len);
> > }
> > @@ -153,49 +181,30 @@ virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto,
> > } else {
> > /* VIRTIO_CRYPTO_SYM_HASH_MODE_NESTED */
> > error_report("unsupported hash mode");
> > - ret = -VIRTIO_CRYPTO_NOTSUPP;
> > - goto err;
> > + return -VIRTIO_CRYPTO_NOTSUPP;
> > }
> > } else {
> > /* VIRTIO_CRYPTO_SYM_OP_NONE */
> > error_report("unsupported cipher op_type: VIRTIO_CRYPTO_SYM_OP_NONE");
> > - ret = -VIRTIO_CRYPTO_NOTSUPP;
> > - goto err;
> > + return -VIRTIO_CRYPTO_NOTSUPP;
> > }
> >
> > queue_index = virtio_crypto_vq2q(queue_id);
> > - session_id = cryptodev_backend_create_session(
> > - vcrypto->cryptodev,
> > - &info, queue_index, &local_err);
> > - if (session_id >= 0) {
> > - ret = session_id;
> > - } else {
> > - if (local_err) {
> > - error_report_err(local_err);
> > - }
> > - ret = -VIRTIO_CRYPTO_ERR;
> > - }
> > -
> > -err:
> > - g_free(sym_info->cipher_key);
> > - g_free(sym_info->auth_key);
> > - return ret;
> > + return cryptodev_backend_create_session(vcrypto->cryptodev, &sreq->info,
> > + queue_index, sreq->cb, sreq);
> > }
> >
> > -static int64_t
> > +static int
> > virtio_crypto_create_asym_session(VirtIOCrypto *vcrypto,
> > struct virtio_crypto_akcipher_create_session_req *sess_req,
> > uint32_t queue_id, uint32_t opcode,
> > - struct iovec *iov, unsigned int out_num)
> > + struct iovec *iov, unsigned int out_num,
> > + VirtIOCryptoSessionReq *sreq)
> > {
> > VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
> > - CryptoDevBackendSessionInfo info = {0};
> > - CryptoDevBackendAsymSessionInfo *asym_info;
> > - int64_t session_id;
> > + CryptoDevBackendAsymSessionInfo *asym_info = &sreq->info.u.asym_sess_info;
> > int queue_index;
> > uint32_t algo, keytype, keylen;
> > - g_autofree uint8_t *key = NULL;
> > - Error *local_err = NULL;
> >
> > algo = ldl_le_p(&sess_req->para.algo);
> > keytype = ldl_le_p(&sess_req->para.keytype);
> > @@ -208,20 +217,19 @@ virtio_crypto_create_asym_session(VirtIOCrypto *vcrypto,
> > }
> >
> > if (keylen) {
> > - key = g_malloc(keylen);
> > - if (iov_to_buf(iov, out_num, 0, key, keylen) != keylen) {
> > + asym_info->key = g_malloc(keylen);
> > + if (iov_to_buf(iov, out_num, 0, asym_info->key, keylen) != keylen) {
> > virtio_error(vdev, "virtio-crypto asym key incorrect");
> > return -EFAULT;
> > }
> > iov_discard_front(&iov, &out_num, keylen);
> > }
> >
> > - info.op_code = opcode;
> > - asym_info = &info.u.asym_sess_info;
> > + sreq->info.op_code = opcode;
> > + asym_info = &sreq->info.u.asym_sess_info;
> > asym_info->algo = algo;
> > asym_info->keytype = keytype;
> > asym_info->keylen = keylen;
> > - asym_info->key = key;
> > switch (asym_info->algo) {
> > case VIRTIO_CRYPTO_AKCIPHER_RSA:
> > asym_info->u.rsa.padding_algo =
> > @@ -237,45 +245,95 @@ virtio_crypto_create_asym_session(VirtIOCrypto *vcrypto,
> > }
> >
> > queue_index = virtio_crypto_vq2q(queue_id);
> > - session_id = cryptodev_backend_create_session(vcrypto->cryptodev, &info,
> > - queue_index, &local_err);
> > - if (session_id < 0) {
> > - if (local_err) {
> > - error_report_err(local_err);
> > - }
> > - return -VIRTIO_CRYPTO_ERR;
> > - }
> > -
> > - return session_id;
> > + return cryptodev_backend_create_session(vcrypto->cryptodev, &sreq->info,
> > + queue_index, sreq->cb, sreq);
> > }
> >
> > -static uint8_t
> > +static int
> > virtio_crypto_handle_close_session(VirtIOCrypto *vcrypto,
> > struct virtio_crypto_destroy_session_req *close_sess_req,
> > - uint32_t queue_id)
> > + uint32_t queue_id,
> > + VirtIOCryptoSessionReq *sreq)
> > {
> > - int ret;
> > uint64_t session_id;
> > - uint32_t status;
> > - Error *local_err = NULL;
> >
> > session_id = ldq_le_p(&close_sess_req->session_id);
> > DPRINTF("close session, id=%" PRIu64 "\n", session_id);
> >
> > - ret = cryptodev_backend_close_session(
> > - vcrypto->cryptodev, session_id, queue_id, &local_err);
> > - if (ret == 0) {
> > - status = VIRTIO_CRYPTO_OK;
> > + return cryptodev_backend_close_session(
> > + vcrypto->cryptodev, session_id, queue_id, sreq->cb, sreq);
> > +}
> > +
> > +static void virtio_crypto_create_session_completion(void *opaque, int ret)
> > +{
> > + VirtIOCryptoSessionReq *sreq = (VirtIOCryptoSessionReq *)opaque;
> > + VirtQueue *vq = sreq->vq;
> > + VirtQueueElement *elem = sreq->elem;
> > + VirtIODevice *vdev = sreq->vdev;
> > + struct virtio_crypto_session_input input;
> > + struct iovec *in_iov = elem->in_sg;
> > + unsigned in_num = elem->in_num;
> > + size_t s;
> > +
> > + memset(&input, 0, sizeof(input));
> > + /* Serious errors, need to reset virtio crypto device */
> > + if (ret == -EFAULT) {
> > + virtqueue_detach_element(vq, elem, 0);
> > + goto out;
> > + } else if (ret == -VIRTIO_CRYPTO_NOTSUPP) {
> > + stl_le_p(&input.status, VIRTIO_CRYPTO_NOTSUPP);
> > + } else if (ret == -VIRTIO_CRYPTO_KEY_REJECTED) {
> > + stl_le_p(&input.status, VIRTIO_CRYPTO_KEY_REJECTED);
> > + } else if (ret != VIRTIO_CRYPTO_OK) {
> > + stl_le_p(&input.status, VIRTIO_CRYPTO_ERR);
> > } else {
> > - if (local_err) {
> > - error_report_err(local_err);
> > - } else {
> > - error_report("destroy session failed");
> > - }
> > + /* Set the session id */
> > + stq_le_p(&input.session_id, sreq->info.session_id);
> > + stl_le_p(&input.status, VIRTIO_CRYPTO_OK);
> > + }
> > +
> > + s = iov_from_buf(in_iov, in_num, 0, &input, sizeof(input));
> > + if (unlikely(s != sizeof(input))) {
> > + virtio_error(vdev, "virtio-crypto input incorrect");
> > + virtqueue_detach_element(vq, elem, 0);
> > + goto out;
> > + }
> > + virtqueue_push(vq, elem, sizeof(input));
> > + virtio_notify(vdev, vq);
> > +
> > +out:
> > + g_free(elem);
> > + virtio_crypto_free_create_session_req(sreq);
> > +}
> > +
> > +static void virtio_crypto_destroy_session_completion(void *opaque, int ret)
> > +{
> > + VirtIOCryptoSessionReq *sreq = (VirtIOCryptoSessionReq *)opaque;
> > + VirtQueue *vq = sreq->vq;
> > + VirtQueueElement *elem = sreq->elem;
> > + VirtIODevice *vdev = sreq->vdev;
> > + struct iovec *in_iov = elem->in_sg;
> > + unsigned in_num = elem->in_num;
> > + uint8_t status;
> > + size_t s;
> > +
> > + if (ret < 0) {
> > status = VIRTIO_CRYPTO_ERR;
> > + } else {
> > + status = VIRTIO_CRYPTO_OK;
> > + }
> > + s = iov_from_buf(in_iov, in_num, 0, &status, sizeof(status));
> > + if (unlikely(s != sizeof(status))) {
> > + virtio_error(vdev, "virtio-crypto status incorrect");
> > + virtqueue_detach_element(vq, elem, 0);
> > + goto out;
> > }
> > + virtqueue_push(vq, elem, sizeof(status));
> > + virtio_notify(vdev, vq);
> >
> > - return status;
> > +out:
> > + g_free(elem);
> > + g_free(sreq);
> > }
> >
> > static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> > @@ -283,16 +341,16 @@ static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> > VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(vdev);
> > struct virtio_crypto_op_ctrl_req ctrl;
> > VirtQueueElement *elem;
> > - struct iovec *in_iov;
> > - struct iovec *out_iov;
> > - unsigned in_num;
> > + VirtIOCryptoSessionReq *sreq;
> > unsigned out_num;
> > + unsigned in_num;
> > uint32_t queue_id;
> > uint32_t opcode;
> > struct virtio_crypto_session_input input;
> > - int64_t session_id;
> > - uint8_t status;
> > size_t s;
> > + int ret;
> > + struct iovec *out_iov;
> > + struct iovec *in_iov;
> >
> > for (;;) {
> > g_autofree struct iovec *out_iov_copy = NULL;
> > @@ -327,44 +385,34 @@ static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> > opcode = ldl_le_p(&ctrl.header.opcode);
> > queue_id = ldl_le_p(&ctrl.header.queue_id);
> >
> > - memset(&input, 0, sizeof(input));
> > + sreq = g_new0(VirtIOCryptoSessionReq, 1);
> > + sreq->vdev = vdev;
> > + sreq->vq = vq;
> > + sreq->elem = elem;
> > +
> > switch (opcode) {
> > case VIRTIO_CRYPTO_CIPHER_CREATE_SESSION:
> > - session_id = virtio_crypto_create_sym_session(vcrypto,
> > - &ctrl.u.sym_create_session,
> > - queue_id, opcode,
> > - out_iov, out_num);
> > - goto check_session;
> > + sreq->cb = virtio_crypto_create_session_completion;
> > + ret = virtio_crypto_create_sym_session(vcrypto,
> > + &ctrl.u.sym_create_session,
> > + queue_id, opcode,
> > + out_iov, out_num,
> > + sreq);
> > + if (ret < 0) {
> > + virtio_crypto_create_session_completion(sreq, ret);
> > + }
> > + break;
> >
> > case VIRTIO_CRYPTO_AKCIPHER_CREATE_SESSION:
> > - session_id = virtio_crypto_create_asym_session(vcrypto,
> > + sreq->cb = virtio_crypto_create_session_completion;
> > + ret = virtio_crypto_create_asym_session(vcrypto,
> > &ctrl.u.akcipher_create_session,
> > queue_id, opcode,
> > - out_iov, out_num);
> > -
> > -check_session:
> > - /* Serious errors, need to reset virtio crypto device */
> > - if (session_id == -EFAULT) {
> > - virtqueue_detach_element(vq, elem, 0);
> > - break;
> > - } else if (session_id == -VIRTIO_CRYPTO_NOTSUPP) {
> > - stl_le_p(&input.status, VIRTIO_CRYPTO_NOTSUPP);
> > - } else if (session_id == -VIRTIO_CRYPTO_ERR) {
> > - stl_le_p(&input.status, VIRTIO_CRYPTO_ERR);
> > - } else {
> > - /* Set the session id */
> > - stq_le_p(&input.session_id, session_id);
> > - stl_le_p(&input.status, VIRTIO_CRYPTO_OK);
> > - }
> > -
> > - s = iov_from_buf(in_iov, in_num, 0, &input, sizeof(input));
> > - if (unlikely(s != sizeof(input))) {
> > - virtio_error(vdev, "virtio-crypto input incorrect");
> > - virtqueue_detach_element(vq, elem, 0);
> > - break;
> > + out_iov, out_num,
> > + sreq);
> > + if (ret < 0) {
> > + virtio_crypto_create_session_completion(sreq, ret);
> > }
> > - virtqueue_push(vq, elem, sizeof(input));
> > - virtio_notify(vdev, vq);
> > break;
> >
> > case VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION:
> > @@ -372,37 +420,36 @@ check_session:
> > case VIRTIO_CRYPTO_MAC_DESTROY_SESSION:
> > case VIRTIO_CRYPTO_AEAD_DESTROY_SESSION:
> > case VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION:
> > - status = virtio_crypto_handle_close_session(vcrypto,
> > - &ctrl.u.destroy_session, queue_id);
> > - /* The status only occupy one byte, we can directly use it */
> > - s = iov_from_buf(in_iov, in_num, 0, &status, sizeof(status));
> > - if (unlikely(s != sizeof(status))) {
> > - virtio_error(vdev, "virtio-crypto status incorrect");
> > - virtqueue_detach_element(vq, elem, 0);
> > - break;
> > + sreq->cb = virtio_crypto_destroy_session_completion;
> > + ret = virtio_crypto_handle_close_session(vcrypto,
> > + &ctrl.u.destroy_session, queue_id,
> > + sreq);
> > + if (ret < 0) {
> > + virtio_crypto_destroy_session_completion(sreq, ret);
> > }
> > - virtqueue_push(vq, elem, sizeof(status));
> > - virtio_notify(vdev, vq);
> > break;
> > +
> > case VIRTIO_CRYPTO_HASH_CREATE_SESSION:
> > case VIRTIO_CRYPTO_MAC_CREATE_SESSION:
> > case VIRTIO_CRYPTO_AEAD_CREATE_SESSION:
> > default:
> > + memset(&input, 0, sizeof(input));
> > error_report("virtio-crypto unsupported ctrl opcode: %d", opcode);
> > stl_le_p(&input.status, VIRTIO_CRYPTO_NOTSUPP);
> > s = iov_from_buf(in_iov, in_num, 0, &input, sizeof(input));
> > if (unlikely(s != sizeof(input))) {
> > virtio_error(vdev, "virtio-crypto input incorrect");
> > virtqueue_detach_element(vq, elem, 0);
> > - break;
> > + } else {
> > + virtqueue_push(vq, elem, sizeof(input));
> > + virtio_notify(vdev, vq);
> > }
> > - virtqueue_push(vq, elem, sizeof(input));
> > - virtio_notify(vdev, vq);
> > + g_free(sreq);
> > + g_free(elem);
> >
> > break;
> > } /* end switch case */
> >
> > - g_free(elem);
> > } /* end for loop */
> > }
> >
> > @@ -513,11 +560,13 @@ virtio_crypto_akcipher_input_data_helper(VirtIODevice *vdev,
> > req->in_len = sizeof(struct virtio_crypto_inhdr) + asym_op_info->dst_len;
> > }
> >
> > -
> > -static void virtio_crypto_req_complete(VirtIOCryptoReq *req, uint8_t status)
> > +static void virtio_crypto_req_complete(void *opaque, int ret)
> > {
> > + VirtIOCryptoReq *req = (VirtIOCryptoReq *)opaque;
> > VirtIOCrypto *vcrypto = req->vcrypto;
> > VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
> > + uint8_t status = -ret;
> > + g_autofree struct iovec *in_iov_copy = req->in_iov;
> >
> > if (req->flags == CRYPTODEV_BACKEND_ALG_SYM) {
> > virtio_crypto_sym_input_data_helper(vdev, req, status,
>
> I am not sure why is this g_autofree here.
> The variable is unused and clang gets confused (a bug in
> clang but oh well).
> Is the point to free when this goes out of scope?
> I suspect we should just g_free inside virtio_crypto_free_request
> or something like this.
> Pls send a fixup as otherwise I'll have to defer this until after
> the coming release.
Also to me it looks like not all paths call virtio_crypto_req_complete
so there might be a memory leak here.
Generally using g_autofree for something not allocated in the same
function is a hack.
I think I'll drop this patchset unless we can fix this quickly.
>
>
> > @@ -529,6 +578,7 @@ static void virtio_crypto_req_complete(VirtIOCryptoReq *req, uint8_t status)
> > stb_p(&req->in->status, status);
> > virtqueue_push(req->vq, &req->elem, req->in_len);
> > virtio_notify(vdev, req->vq);
> > + virtio_crypto_free_request(req);
> > }
> >
> > static VirtIOCryptoReq *
> > @@ -773,9 +823,7 @@ virtio_crypto_handle_request(VirtIOCryptoReq *request)
> > unsigned in_num;
> > unsigned out_num;
> > uint32_t opcode;
> > - uint8_t status = VIRTIO_CRYPTO_ERR;
> > CryptoDevBackendOpInfo *op_info = &request->op_info;
> > - Error *local_err = NULL;
> >
> > if (elem->out_num < 1 || elem->in_num < 1) {
> > virtio_error(vdev, "virtio-crypto dataq missing headers");
> > @@ -815,6 +863,8 @@ virtio_crypto_handle_request(VirtIOCryptoReq *request)
> > */
> > request->in_num = in_num;
> > request->in_iov = in_iov;
> > + /* now, we free the in_iov_copy inside virtio_crypto_req_complete */
> > + in_iov_copy = NULL;
> >
> > opcode = ldl_le_p(&req.header.opcode);
> > op_info->session_id = ldq_le_p(&req.header.session_id);
> > @@ -844,22 +894,11 @@ check_result:
> > return -1;
> > } else if (ret == -VIRTIO_CRYPTO_NOTSUPP) {
> > virtio_crypto_req_complete(request, VIRTIO_CRYPTO_NOTSUPP);
> > - virtio_crypto_free_request(request);
> > } else {
> > -
> > - /* Set request's parameter */
> > - ret = cryptodev_backend_crypto_operation(vcrypto->cryptodev,
> > - request, queue_index, &local_err);
> > - if (ret < 0) {
> > - status = -ret;
> > - if (local_err) {
> > - error_report_err(local_err);
> > - }
> > - } else { /* ret == VIRTIO_CRYPTO_OK */
> > - status = ret;
> > - }
> > - virtio_crypto_req_complete(request, status);
> > - virtio_crypto_free_request(request);
> > + cryptodev_backend_crypto_operation(vcrypto->cryptodev,
> > + request, queue_index,
> > + virtio_crypto_req_complete,
> > + request);
> > }
> > break;
> >
> > @@ -871,7 +910,6 @@ check_result:
> > error_report("virtio-crypto unsupported dataq opcode: %u",
> > opcode);
> > virtio_crypto_req_complete(request, VIRTIO_CRYPTO_NOTSUPP);
> > - virtio_crypto_free_request(request);
> > }
> >
> > return 0;
> > @@ -1011,7 +1049,7 @@ static void virtio_crypto_device_realize(DeviceState *dev, Error **errp)
> > vcrypto->vqs[i].vcrypto = vcrypto;
> > }
> >
> > - vcrypto->ctrl_vq = virtio_add_queue(vdev, 64, virtio_crypto_handle_ctrl);
> > + vcrypto->ctrl_vq = virtio_add_queue(vdev, 1024, virtio_crypto_handle_ctrl);
> > if (!cryptodev_backend_is_ready(vcrypto->cryptodev)) {
> > vcrypto->status &= ~VIRTIO_CRYPTO_S_HW_READY;
> > } else {
> > diff --git a/include/sysemu/cryptodev.h b/include/sysemu/cryptodev.h
> > index 37c3a360fd..32e9f4cf8a 100644
> > --- a/include/sysemu/cryptodev.h
> > +++ b/include/sysemu/cryptodev.h
> > @@ -113,6 +113,7 @@ typedef struct CryptoDevBackendSessionInfo {
> > CryptoDevBackendSymSessionInfo sym_sess_info;
> > CryptoDevBackendAsymSessionInfo asym_sess_info;
> > } u;
> > + uint64_t session_id;
> > } CryptoDevBackendSessionInfo;
> >
> > /**
> > @@ -188,21 +189,30 @@ typedef struct CryptoDevBackendOpInfo {
> > } u;
> > } CryptoDevBackendOpInfo;
> >
> > +typedef void (*CryptoDevCompletionFunc) (void *opaque, int ret);
> > struct CryptoDevBackendClass {
> > ObjectClass parent_class;
> >
> > void (*init)(CryptoDevBackend *backend, Error **errp);
> > void (*cleanup)(CryptoDevBackend *backend, Error **errp);
> >
> > - int64_t (*create_session)(CryptoDevBackend *backend,
> > - CryptoDevBackendSessionInfo *sess_info,
> > - uint32_t queue_index, Error **errp);
> > + int (*create_session)(CryptoDevBackend *backend,
> > + CryptoDevBackendSessionInfo *sess_info,
> > + uint32_t queue_index,
> > + CryptoDevCompletionFunc cb,
> > + void *opaque);
> > +
> > int (*close_session)(CryptoDevBackend *backend,
> > - uint64_t session_id,
> > - uint32_t queue_index, Error **errp);
> > + uint64_t session_id,
> > + uint32_t queue_index,
> > + CryptoDevCompletionFunc cb,
> > + void *opaque);
> > +
> > int (*do_op)(CryptoDevBackend *backend,
> > - CryptoDevBackendOpInfo *op_info,
> > - uint32_t queue_index, Error **errp);
> > + CryptoDevBackendOpInfo *op_info,
> > + uint32_t queue_index,
> > + CryptoDevCompletionFunc cb,
> > + void *opaque);
> > };
> >
> > typedef enum CryptoDevBackendOptionsType {
> > @@ -303,15 +313,20 @@ void cryptodev_backend_cleanup(
> > * @sess_info: parameters needed by session creating
> > * @queue_index: queue index of cryptodev backend client
> > * @errp: pointer to a NULL-initialized error object
> > + * @cb: callback when session create is compeleted
> > + * @opaque: parameter passed to callback
> > *
> > - * Create a session for symmetric/symmetric algorithms
> > + * Create a session for symmetric/asymmetric algorithms
> > *
> > - * Returns: session id on success, or -1 on error
> > + * Returns: 0 for success and cb will be called when creation is completed,
> > + * negative value for error, and cb will not be called.
> > */
> > -int64_t cryptodev_backend_create_session(
> > +int cryptodev_backend_create_session(
> > CryptoDevBackend *backend,
> > CryptoDevBackendSessionInfo *sess_info,
> > - uint32_t queue_index, Error **errp);
> > + uint32_t queue_index,
> > + CryptoDevCompletionFunc cb,
> > + void *opaque);
> >
> > /**
> > * cryptodev_backend_close_session:
> > @@ -319,34 +334,43 @@ int64_t cryptodev_backend_create_session(
> > * @session_id: the session id
> > * @queue_index: queue index of cryptodev backend client
> > * @errp: pointer to a NULL-initialized error object
> > + * @cb: callback when session create is compeleted
> > + * @opaque: parameter passed to callback
> > *
> > * Close a session for which was previously
> > * created by cryptodev_backend_create_session()
> > *
> > - * Returns: 0 on success, or Negative on error
> > + * Returns: 0 for success and cb will be called when creation is completed,
> > + * negative value for error, and cb will not be called.
> > */
> > int cryptodev_backend_close_session(
> > CryptoDevBackend *backend,
> > uint64_t session_id,
> > - uint32_t queue_index, Error **errp);
> > + uint32_t queue_index,
> > + CryptoDevCompletionFunc cb,
> > + void *opaque);
> >
> > /**
> > * cryptodev_backend_crypto_operation:
> > * @backend: the cryptodev backend object
> > - * @opaque: pointer to a VirtIOCryptoReq object
> > + * @opaque1: pointer to a VirtIOCryptoReq object
> > * @queue_index: queue index of cryptodev backend client
> > * @errp: pointer to a NULL-initialized error object
> > + * @cb: callbacks when operation is completed
> > + * @opaque2: parameter passed to cb
> > *
> > * Do crypto operation, such as encryption and
> > * decryption
> > *
> > - * Returns: VIRTIO_CRYPTO_OK on success,
> > - * or -VIRTIO_CRYPTO_* on error
> > + * Returns: 0 for success and cb will be called when creation is completed,
> > + * negative value for error, and cb will not be called.
> > */
> > int cryptodev_backend_crypto_operation(
> > CryptoDevBackend *backend,
> > - void *opaque,
> > - uint32_t queue_index, Error **errp);
> > + void *opaque1,
> > + uint32_t queue_index,
> > + CryptoDevCompletionFunc cb,
> > + void *opaque2);
> >
> > /**
> > * cryptodev_backend_set_used:
> > --
> > 2.11.0
next prev parent reply other threads:[~2022-11-01 19:52 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-08 8:50 [PATCH v2 0/4] Add a new backend for cryptodev Lei He
2022-10-08 8:50 ` [PATCH v2 1/4] virtio-crypto: Support asynchronous mode Lei He
2022-11-01 10:37 ` Michael S. Tsirkin
2022-11-01 19:51 ` Michael S. Tsirkin [this message]
2022-11-02 2:24 ` Lei He
2022-11-02 8:18 ` Lei He
2022-10-08 8:50 ` [PATCH v2 2/4] crypto: Support DER encodings Lei He
2022-10-11 9:31 ` Daniel P. Berrangé
2022-10-08 8:50 ` [PATCH v2 3/4] crypto: Support export akcipher to pkcs8 Lei He
2022-10-11 9:33 ` Daniel P. Berrangé
2022-10-08 8:50 ` [PATCH v2 4/4] cryptodev: Add a lkcf-backend for cryptodev Lei He
2022-10-24 8:55 ` [PATCH v2 0/4] Add a new backend " Lei He
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221101154944-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=arei.gonglei@huawei.com \
--cc=berrange@redhat.com \
--cc=helei.sig11@bytedance.com \
--cc=pizhenwei@bytedance.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.