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.si...@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.



> @@ -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


Reply via email to