[PATCH v2 0/4] Add a new backend for cryptodev

[PATCH v2 0/4] Add a new backend for cryptodev

[Lei He]

v1 --> v2:
- Fix compile errors when neither 'nettle' nor 'gcrypt' are enabled.
- Trivial changes to error codes when neither 'nettle' nor 'gcrypt' are
enabled.

This patch adds a new backend called LKCF to cryptodev, LKCF stands
for Linux Kernel Cryptography Framework. If a cryptographic
accelerator that supports LKCF is installed on the the host (you can
see which algorithms are supported in host's LKCF by executing
'cat /proc/crypto'), then RSA operations can be offloaded.
More background info can refer to: https://lwn.net/Articles/895399/,
'keyctl[5]' in the picture.

This patch:
1. Modified some interfaces of cryptodev and cryptodev-backend to
support asynchronous requests.
2. Extended the DER encoder in crypto, so that we can export the
RSA private key into PKCS#8 format and upload it to host kernel.
3. Added a new backend for cryptodev.

I tested the backend with a QAT card, the qps of RSA-2048-decryption
is about 25k/s, and the main-loop becomes the bottleneck. The qps
using OpenSSL directly is about 6k/s (with 6 vCPUs). We will support 
IO-thread for cryptodev in another series later.


Lei He (4):
  virtio-crypto: Support asynchronous mode
  crypto: Support DER encodings
  crypto: Support export akcipher to pkcs8
  cryptodev: Add a lkcf-backend for cryptodev

 backends/cryptodev-builtin.c    |  69 +++--
 backends/cryptodev-lkcf.c       | 645 ++++++++++++++++++++++++++++++++++++++++
 backends/cryptodev-vhost-user.c |  51 +++-
 backends/cryptodev.c            |  44 +--
 backends/meson.build            |   3 +
 crypto/akcipher.c               |  18 ++
 crypto/der.c                    | 307 +++++++++++++++++--
 crypto/der.h                    | 211 ++++++++++++-
 crypto/rsakey.c                 |  42 +++
 crypto/rsakey.h                 |  11 +-
 hw/virtio/virtio-crypto.c       | 324 +++++++++++---------
 include/crypto/akcipher.h       |  21 ++
 include/sysemu/cryptodev.h      |  61 ++--
 qapi/qom.json                   |   2 +
 tests/unit/test-crypto-der.c    | 126 ++++++--
 15 files changed, 1675 insertions(+), 260 deletions(-)
 create mode 100644 backends/cryptodev-lkcf.c

-- 
2.11.0

[PATCH v2 1/4] virtio-crypto: Support asynchronous mode

[Lei He]

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

Re: [PATCH v2 1/4] virtio-crypto: Support asynchronous mode

[Michael S. Tsirkin]

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.



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

Re: [PATCH v2 1/4] virtio-crypto: Support asynchronous mode

[Michael S. Tsirkin]

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

Re: Re: [PATCH v2 1/4] virtio-crypto: Support asynchronous mode

[Lei He]

On 2022/11/2 03:51, Michael S. Tsirkin wrote:
> 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.
> 

  Thanks for reminding, I will prioritize fixing this today.


Best regards,
Lei He
--
helei.sig11@bytedance.com

Re: Re: [PATCH v2 1/4] virtio-crypto: Support asynchronous mode

[Lei He]

On 2022/11/2 03:51, Michael S. Tsirkin wrote:
> 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.

virtio_crypto_sym_input_data_helper may call
iov_discard_front(&req->in_iov...), so I saved req->in_iov and try to
free it with g_autofree. Anyway, g_free has now been used instead
of g_autofree.

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

Fixed.

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

V3 is sent and thanks a lot for the review. If there are any other
issues, I will respond as soon as possible.

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

Best regards,
Lei He
--
helei.sig11@bytedance.com

[PATCH v2 2/4] crypto: Support DER encodings

[Lei He]

Add encoding interfaces for DER encoding:
1. support decoding of 'bit string', 'octet string', 'object id'
and 'context specific tag' for DER encoder.
2. implemented a simple DER encoder.
3. add more testsuits for DER encoder.

Signed-off-by: lei he <helei.sig11@bytedance.com>
---
 crypto/der.c                 | 307 +++++++++++++++++++++++++++++++++++++++----
 crypto/der.h                 | 211 ++++++++++++++++++++++++++++-
 tests/unit/test-crypto-der.c | 126 ++++++++++++++----
 3 files changed, 597 insertions(+), 47 deletions(-)

diff --git a/crypto/der.c b/crypto/der.c
index f877390bbb..dab3fe4f24 100644
--- a/crypto/der.c
+++ b/crypto/der.c
@@ -22,20 +22,93 @@
 #include "qemu/osdep.h"
 #include "crypto/der.h"
 
+typedef struct QCryptoDerEncodeNode {
+    uint8_t tag;
+    struct QCryptoDerEncodeNode *parent;
+    struct QCryptoDerEncodeNode *next;
+    /* for constructed type, data is null */
+    const uint8_t *data;
+    size_t dlen;
+} QCryptoDerEncodeNode;
+
+typedef struct QCryptoEncodeContext {
+    QCryptoDerEncodeNode root;
+    QCryptoDerEncodeNode *current_parent;
+    QCryptoDerEncodeNode *tail;
+} QCryptoEncodeContext;
+
 enum QCryptoDERTypeTag {
     QCRYPTO_DER_TYPE_TAG_BOOL = 0x1,
     QCRYPTO_DER_TYPE_TAG_INT = 0x2,
     QCRYPTO_DER_TYPE_TAG_BIT_STR = 0x3,
     QCRYPTO_DER_TYPE_TAG_OCT_STR = 0x4,
-    QCRYPTO_DER_TYPE_TAG_OCT_NULL = 0x5,
-    QCRYPTO_DER_TYPE_TAG_OCT_OID = 0x6,
+    QCRYPTO_DER_TYPE_TAG_NULL = 0x5,
+    QCRYPTO_DER_TYPE_TAG_OID = 0x6,
     QCRYPTO_DER_TYPE_TAG_SEQ = 0x10,
     QCRYPTO_DER_TYPE_TAG_SET = 0x11,
 };
 
-#define QCRYPTO_DER_CONSTRUCTED_MASK 0x20
+enum QCryptoDERTagClass {
+    QCRYPTO_DER_TAG_CLASS_UNIV = 0x0,
+    QCRYPTO_DER_TAG_CLASS_APPL = 0x1,
+    QCRYPTO_DER_TAG_CLASS_CONT = 0x2,
+    QCRYPTO_DER_TAG_CLASS_PRIV = 0x3,
+};
+
+enum QCryptoDERTagEnc {
+    QCRYPTO_DER_TAG_ENC_PRIM = 0x0,
+    QCRYPTO_DER_TAG_ENC_CONS = 0x1,
+};
+
+#define QCRYPTO_DER_TAG_ENC_MASK 0x20
+#define QCRYPTO_DER_TAG_ENC_SHIFT 5
+
+#define QCRYPTO_DER_TAG_CLASS_MASK 0xc0
+#define QCRYPTO_DER_TAG_CLASS_SHIFT 6
+
+#define QCRYPTO_DER_TAG_VAL_MASK 0x1f
 #define QCRYPTO_DER_SHORT_LEN_MASK 0x80
 
+#define QCRYPTO_DER_TAG(class, enc, val)           \
+    (((class) << QCRYPTO_DER_TAG_CLASS_SHIFT) |    \
+     ((enc) << QCRYPTO_DER_TAG_ENC_SHIFT) | (val))
+
+/**
+ * qcrypto_der_encode_length:
+ * @src_len: the length of source data
+ * @dst: distination to save the encoded 'length', if dst is NULL, only compute
+ * the expected buffer size in bytes.
+ * @dst_len: output parameter, indicates how many bytes wrote.
+ *
+ * Encode the 'length' part of TLV tuple.
+ */
+static void qcrypto_der_encode_length(size_t src_len,
+                                      uint8_t *dst, size_t *dst_len)
+{
+    size_t max_length = 0xFF;
+    uint8_t length_bytes = 0, header_byte;
+
+    if (src_len < QCRYPTO_DER_SHORT_LEN_MASK) {
+        header_byte = src_len;
+        *dst_len = 1;
+    } else {
+        for (length_bytes = 1; max_length < src_len; length_bytes++) {
+            max_length = (max_length << 8) + max_length;
+        }
+        header_byte = length_bytes;
+        header_byte |= QCRYPTO_DER_SHORT_LEN_MASK;
+        *dst_len = length_bytes + 1;
+    }
+    if (!dst) {
+        return;
+    }
+    *dst++ = header_byte;
+    /* Bigendian length bytes */
+    for (; length_bytes > 0; length_bytes--) {
+        *dst++ = ((src_len >> (length_bytes - 1) * 8) & 0xFF);
+    }
+}
+
 static uint8_t qcrypto_der_peek_byte(const uint8_t **data, size_t *dlen)
 {
     return **data;
@@ -150,40 +223,230 @@ static int qcrypto_der_extract_data(const uint8_t **data, size_t *dlen,
     return qcrypto_der_extract_definite_data(data, dlen, cb, ctx, errp);
 }
 
-int qcrypto_der_decode_int(const uint8_t **data, size_t *dlen,
-                           QCryptoDERDecodeCb cb, void *ctx, Error **errp)
+static int qcrypto_der_decode_tlv(const uint8_t expected_tag,
+                                  const uint8_t **data, size_t *dlen,
+                                  QCryptoDERDecodeCb cb,
+                                  void *ctx, Error **errp)
 {
+    const uint8_t *saved_data = *data;
+    size_t saved_dlen = *dlen;
     uint8_t tag;
+    int data_length;
+
     if (*dlen < 1) {
         error_setg(errp, "Need more data");
         return -1;
     }
     tag = qcrypto_der_cut_byte(data, dlen);
+    if (tag != expected_tag) {
+        error_setg(errp, "Unexpected tag: expected: %u, actual: %u",
+                   expected_tag, tag);
+        goto error;
+    }
 
-    /* INTEGER must encoded in primitive-form */
-    if (tag != QCRYPTO_DER_TYPE_TAG_INT) {
-        error_setg(errp, "Invalid integer type tag: %u", tag);
-        return -1;
+    data_length = qcrypto_der_extract_data(data, dlen, cb, ctx, errp);
+    if (data_length < 0) {
+        goto error;
     }
+    return data_length;
 
-    return qcrypto_der_extract_data(data, dlen, cb, ctx, errp);
+error:
+    *data = saved_data;
+    *dlen = saved_dlen;
+    return -1;
+}
+
+int qcrypto_der_decode_int(const uint8_t **data, size_t *dlen,
+                           QCryptoDERDecodeCb cb, void *ctx, Error **errp)
+{
+    const uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                        QCRYPTO_DER_TAG_ENC_PRIM,
+                                        QCRYPTO_DER_TYPE_TAG_INT);
+    return qcrypto_der_decode_tlv(tag, data, dlen, cb, ctx, errp);
 }
 
 int qcrypto_der_decode_seq(const uint8_t **data, size_t *dlen,
                            QCryptoDERDecodeCb cb, void *ctx, Error **errp)
 {
-    uint8_t tag;
-    if (*dlen < 1) {
-        error_setg(errp, "Need more data");
-        return -1;
-    }
-    tag = qcrypto_der_cut_byte(data, dlen);
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                  QCRYPTO_DER_TAG_ENC_CONS,
+                                  QCRYPTO_DER_TYPE_TAG_SEQ);
+    return qcrypto_der_decode_tlv(tag, data, dlen, cb, ctx, errp);
+}
 
-    /* SEQUENCE must use constructed form */
-    if (tag != (QCRYPTO_DER_TYPE_TAG_SEQ | QCRYPTO_DER_CONSTRUCTED_MASK)) {
-        error_setg(errp, "Invalid type sequence tag: %u", tag);
-        return -1;
-    }
+int qcrypto_der_decode_octet_str(const uint8_t **data, size_t *dlen,
+                                 QCryptoDERDecodeCb cb, void *ctx, Error **errp)
+{
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                  QCRYPTO_DER_TAG_ENC_PRIM,
+                                  QCRYPTO_DER_TYPE_TAG_OCT_STR);
+    return qcrypto_der_decode_tlv(tag, data, dlen, cb, ctx, errp);
+}
+
+int qcrypto_der_decode_bit_str(const uint8_t **data, size_t *dlen,
+                               QCryptoDERDecodeCb cb, void *ctx, Error **errp)
+{
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                  QCRYPTO_DER_TAG_ENC_PRIM,
+                                  QCRYPTO_DER_TYPE_TAG_BIT_STR);
+    return qcrypto_der_decode_tlv(tag, data, dlen, cb, ctx, errp);
+}
+
+int qcrypto_der_decode_oid(const uint8_t **data, size_t *dlen,
+                           QCryptoDERDecodeCb cb, void *ctx, Error **errp)
+{
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                  QCRYPTO_DER_TAG_ENC_PRIM,
+                                  QCRYPTO_DER_TYPE_TAG_OID);
+    return qcrypto_der_decode_tlv(tag, data, dlen, cb, ctx, errp);
+}
+
+int qcrypto_der_decode_ctx_tag(const uint8_t **data, size_t *dlen, int tag_id,
+                               QCryptoDERDecodeCb cb, void *ctx, Error **errp)
+{
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_CONT,
+                                  QCRYPTO_DER_TAG_ENC_CONS,
+                                  tag_id);
+    return qcrypto_der_decode_tlv(tag, data, dlen, cb, ctx, errp);
+}
 
-    return qcrypto_der_extract_data(data, dlen, cb, ctx, errp);
+static void qcrypto_der_encode_prim(QCryptoEncodeContext *ctx, uint8_t tag,
+                                    const uint8_t *data, size_t dlen)
+{
+    QCryptoDerEncodeNode *node = g_new0(QCryptoDerEncodeNode, 1);
+    size_t nbytes_len;
+
+    node->tag = tag;
+    node->data = data;
+    node->dlen = dlen;
+    node->parent = ctx->current_parent;
+
+    qcrypto_der_encode_length(dlen, NULL, &nbytes_len);
+    /* 1 byte for Tag, nbyte_len for Length, and dlen for Value */
+    node->parent->dlen += 1 + nbytes_len + dlen;
+
+    ctx->tail->next = node;
+    ctx->tail = node;
+}
+
+QCryptoEncodeContext *qcrypto_der_encode_ctx_new(void)
+{
+    QCryptoEncodeContext *ctx = g_new0(QCryptoEncodeContext, 1);
+    ctx->current_parent = &ctx->root;
+    ctx->tail = &ctx->root;
+    return ctx;
+}
+
+static void qcrypto_der_encode_cons_begin(QCryptoEncodeContext *ctx,
+                                          uint8_t tag)
+{
+    QCryptoDerEncodeNode *node = g_new0(QCryptoDerEncodeNode, 1);
+
+    node->tag = tag;
+    node->parent = ctx->current_parent;
+    ctx->current_parent = node;
+    ctx->tail->next = node;
+    ctx->tail = node;
+}
+
+static void qcrypto_der_encode_cons_end(QCryptoEncodeContext *ctx)
+{
+    QCryptoDerEncodeNode *cons_node = ctx->current_parent;
+    size_t nbytes_len;
+
+    qcrypto_der_encode_length(cons_node->dlen, NULL, &nbytes_len);
+    /* 1 byte for Tag, nbyte_len for Length, and dlen for Value */
+    cons_node->parent->dlen += 1 + nbytes_len + cons_node->dlen;
+    ctx->current_parent = cons_node->parent;
+}
+
+void qcrypto_der_encode_seq_begin(QCryptoEncodeContext *ctx)
+{
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                  QCRYPTO_DER_TAG_ENC_CONS,
+                                  QCRYPTO_DER_TYPE_TAG_SEQ);
+    qcrypto_der_encode_cons_begin(ctx, tag);
+}
+
+void qcrypto_der_encode_seq_end(QCryptoEncodeContext *ctx)
+{
+    qcrypto_der_encode_cons_end(ctx);
+}
+
+void qcrypto_der_encode_oid(QCryptoEncodeContext *ctx,
+                            const uint8_t *src, size_t src_len)
+{
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                  QCRYPTO_DER_TAG_ENC_PRIM,
+                                  QCRYPTO_DER_TYPE_TAG_OID);
+    qcrypto_der_encode_prim(ctx, tag, src, src_len);
+}
+
+void qcrypto_der_encode_int(QCryptoEncodeContext *ctx,
+                            const uint8_t *src, size_t src_len)
+{
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                  QCRYPTO_DER_TAG_ENC_PRIM,
+                                  QCRYPTO_DER_TYPE_TAG_INT);
+    qcrypto_der_encode_prim(ctx, tag, src, src_len);
+}
+
+void qcrypto_der_encode_null(QCryptoEncodeContext *ctx)
+{
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                  QCRYPTO_DER_TAG_ENC_PRIM,
+                                  QCRYPTO_DER_TYPE_TAG_NULL);
+    qcrypto_der_encode_prim(ctx, tag, NULL, 0);
+}
+
+void qcrypto_der_encode_octet_str(QCryptoEncodeContext *ctx,
+                                  const uint8_t *src, size_t src_len)
+{
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                  QCRYPTO_DER_TAG_ENC_PRIM,
+                                  QCRYPTO_DER_TYPE_TAG_OCT_STR);
+    qcrypto_der_encode_prim(ctx, tag, src, src_len);
+}
+
+void qcrypto_der_encode_octet_str_begin(QCryptoEncodeContext *ctx)
+{
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                  QCRYPTO_DER_TAG_ENC_PRIM,
+                                  QCRYPTO_DER_TYPE_TAG_OCT_STR);
+    qcrypto_der_encode_cons_begin(ctx, tag);
+}
+
+void qcrypto_der_encode_octet_str_end(QCryptoEncodeContext *ctx)
+{
+    qcrypto_der_encode_cons_end(ctx);
+}
+
+size_t qcrypto_der_encode_ctx_buffer_len(QCryptoEncodeContext *ctx)
+{
+    return ctx->root.dlen;
+}
+
+void qcrypto_der_encode_ctx_flush_and_free(QCryptoEncodeContext *ctx,
+                                           uint8_t *dst)
+{
+    QCryptoDerEncodeNode *node, *prev;
+    size_t len;
+
+    for (prev = &ctx->root;
+         (node = prev->next) && (prev->next = node->next, 1);) {
+        /* Tag */
+        *dst++ = node->tag;
+
+        /* Length */
+        qcrypto_der_encode_length(node->dlen, dst, &len);
+        dst += len;
+
+        /* Value */
+        if (node->data) {
+            memcpy(dst, node->data, node->dlen);
+            dst += node->dlen;
+        }
+        g_free(node);
+    }
+    g_free(ctx);
 }
diff --git a/crypto/der.h b/crypto/der.h
index e3d3aeacdc..0e895bbeec 100644
--- a/crypto/der.h
+++ b/crypto/der.h
@@ -22,6 +22,11 @@
 
 #include "qapi/error.h"
 
+typedef struct QCryptoEncodeContext QCryptoEncodeContext;
+
+/* rsaEncryption: 1.2.840.113549.1.1.1 */
+#define QCRYPTO_OID_rsaEncryption "\x2A\x86\x48\x86\xF7\x0D\x01\x01\x01"
+
 /* Simple decoder used to parse DER encoded rsa keys. */
 
 /**
@@ -47,14 +52,13 @@ typedef int (*QCryptoDERDecodeCb) (void *opaque, const uint8_t *value,
  * will be set to the rest length of data, if cb is not NULL, must
  * return 0 to make decode success, at last, the length of the data
  * part of the decoded INTEGER will be returned. Otherwise, -1 is
- * returned.
+ * returned and the valued of *data and *dlen keep unchanged.
  */
 int qcrypto_der_decode_int(const uint8_t **data,
                            size_t *dlen,
                            QCryptoDERDecodeCb cb,
                            void *opaque,
                            Error **errp);
-
 /**
  * qcrypto_der_decode_seq:
  *
@@ -70,7 +74,7 @@ int qcrypto_der_decode_int(const uint8_t **data,
  * will be set to the rest length of data, if cb is not NULL, must
  * return 0 to make decode success, at last, the length of the data
  * part of the decoded SEQUENCE will be returned. Otherwise, -1 is
- * returned.
+ * returned and the valued of *data and *dlen keep unchanged.
  */
 int qcrypto_der_decode_seq(const uint8_t **data,
                            size_t *dlen,
@@ -78,4 +82,205 @@ int qcrypto_der_decode_seq(const uint8_t **data,
                            void *opaque,
                            Error **errp);
 
+/**
+ * qcrypto_der_decode_oid:
+ *
+ * Decode OID from DER-encoded data, similar with der_decode_int.
+ *
+ * @data: pointer to address of input data
+ * @dlen: pointer to length of input data
+ * @cb: callback invoked when decode succeed, if cb equals NULL, no
+ * callback will be invoked
+ * @opaque: parameter passed to cb
+ *
+ * Returns: On success, *data points to rest data, and *dlen
+ * will be set to the rest length of data, if cb is not NULL, must
+ * return 0 to make decode success, at last, the length of the data
+ * part of the decoded OID will be returned. Otherwise, -1 is
+ * returned and the valued of *data and *dlen keep unchanged.
+ */
+int qcrypto_der_decode_oid(const uint8_t **data,
+                           size_t *dlen,
+                           QCryptoDERDecodeCb cb,
+                           void *opaque,
+                           Error **errp);
+
+/**
+ * qcrypto_der_decode_octet_str:
+ *
+ * Decode OCTET STRING from DER-encoded data, similar with der_decode_int.
+ *
+ * @data: pointer to address of input data
+ * @dlen: pointer to length of input data
+ * @cb: callback invoked when decode succeed, if cb equals NULL, no
+ * callback will be invoked
+ * @opaque: parameter passed to cb
+ *
+ * Returns: On success, *data points to rest data, and *dlen
+ * will be set to the rest length of data, if cb is not NULL, must
+ * return 0 to make decode success, at last, the length of the data
+ * part of the decoded OCTET STRING will be returned. Otherwise, -1 is
+ * returned and the valued of *data and *dlen keep unchanged.
+ */
+int qcrypto_der_decode_octet_str(const uint8_t **data,
+                                 size_t *dlen,
+                                 QCryptoDERDecodeCb cb,
+                                 void *opaque,
+                                 Error **errp);
+
+/**
+ * qcrypto_der_decode_bit_str:
+ *
+ * Decode BIT STRING from DER-encoded data, similar with der_decode_int.
+ *
+ * @data: pointer to address of input data
+ * @dlen: pointer to length of input data
+ * @cb: callback invoked when decode succeed, if cb equals NULL, no
+ * callback will be invoked
+ * @opaque: parameter passed to cb
+ *
+ * Returns: On success, *data points to rest data, and *dlen
+ * will be set to the rest length of data, if cb is not NULL, must
+ * return 0 to make decode success, at last, the length of the data
+ * part of the decoded BIT STRING will be returned. Otherwise, -1 is
+ * returned and the valued of *data and *dlen keep unchanged.
+ */
+int qcrypto_der_decode_bit_str(const uint8_t **data,
+                               size_t *dlen,
+                               QCryptoDERDecodeCb cb,
+                               void *opaque,
+                               Error **errp);
+
+
+/**
+ * qcrypto_der_decode_ctx_tag:
+ *
+ * Decode context specific tag
+ *
+ * @data: pointer to address of input data
+ * @dlen: pointer to length of input data
+ * @tag: expected value of context specific tag
+ * @cb: callback invoked when decode succeed, if cb equals NULL, no
+ * callback will be invoked
+ * @opaque: parameter passed to cb
+ *
+ * Returns: On success, *data points to rest data, and *dlen
+ * will be set to the rest length of data, if cb is not NULL, must
+ * return 0 to make decode success, at last, the length of the data
+ * part of the decoded BIT STRING will be returned. Otherwise, -1 is
+ * returned and the valued of *data and *dlen keep unchanged.
+ */
+int qcrypto_der_decode_ctx_tag(const uint8_t **data,
+                               size_t *dlen, int tag_id,
+                               QCryptoDERDecodeCb cb,
+                               void *opaque,
+                               Error **errp);
+
+/**
+ * qcrypto_der_encode_ctx_new:
+ *
+ * Allocate a context used for der encoding.
+ */
+QCryptoEncodeContext *qcrypto_der_encode_ctx_new(void);
+
+/**
+ * qcrypto_der_encode_seq_begin:
+ * @ctx: the encode context.
+ *
+ * Start encoding a SEQUENCE for ctx.
+ *
+ */
+void qcrypto_der_encode_seq_begin(QCryptoEncodeContext *ctx);
+
+/**
+ * qcrypto_der_encode_seq_begin:
+ * @ctx: the encode context.
+ *
+ * Finish uencoding a SEQUENCE for ctx.
+ *
+ */
+void qcrypto_der_encode_seq_end(QCryptoEncodeContext *ctx);
+
+
+/**
+ * qcrypto_der_encode_oid:
+ * @ctx: the encode context.
+ * @src: the source data of oid, note it should be already encoded, this
+ * function only add tag and length part for it.
+ *
+ * Encode an oid into ctx.
+ */
+void qcrypto_der_encode_oid(QCryptoEncodeContext *ctx,
+                            const uint8_t *src, size_t src_len);
+
+/**
+ * qcrypto_der_encode_int:
+ * @ctx: the encode context.
+ * @src: the source data of integer, note it should be already encoded, this
+ * function only add tag and length part for it.
+ *
+ * Encode an integer into ctx.
+ */
+void qcrypto_der_encode_int(QCryptoEncodeContext *ctx,
+                            const uint8_t *src, size_t src_len);
+
+/**
+ * qcrypto_der_encode_null:
+ * @ctx: the encode context.
+ *
+ * Encode a null into ctx.
+ */
+void qcrypto_der_encode_null(QCryptoEncodeContext *ctx);
+
+/**
+ * qcrypto_der_encode_octet_str:
+ * @ctx: the encode context.
+ * @src: the source data of the octet string.
+ *
+ * Encode a octet string into ctx.
+ */
+void qcrypto_der_encode_octet_str(QCryptoEncodeContext *ctx,
+                                  const uint8_t *src, size_t src_len);
+
+/**
+ * qcrypto_der_encode_octet_str_begin:
+ * @ctx: the encode context.
+ *
+ * Start encoding a octet string, All fields between
+ * qcrypto_der_encode_octet_str_begin and qcrypto_der_encode_octet_str_end
+ * are encoded as an octet string. This is useful when we need to encode a
+ * encoded SEQUNCE as OCTET STRING.
+ */
+void qcrypto_der_encode_octet_str_begin(QCryptoEncodeContext *ctx);
+
+/**
+ * qcrypto_der_encode_octet_str_end:
+ * @ctx: the encode context.
+ *
+ * Finish encoding a octet string, All fields between
+ * qcrypto_der_encode_octet_str_begin and qcrypto_der_encode_octet_str_end
+ * are encoded as an octet string. This is useful when we need to encode a
+ * encoded SEQUNCE as OCTET STRING.
+ */
+void qcrypto_der_encode_octet_str_end(QCryptoEncodeContext *ctx);
+
+/**
+ * qcrypto_der_encode_ctx_buffer_len:
+ * @ctx: the encode context.
+ *
+ * Compute the expected buffer size to save all encoded things.
+ */
+size_t qcrypto_der_encode_ctx_buffer_len(QCryptoEncodeContext *ctx);
+
+/**
+ * qcrypto_der_encode_ctx_flush_and_free:
+ * @ctx: the encode context.
+ * @dst: the distination to save the encoded data, the length of dst should
+ * not less than qcrypto_der_encode_cxt_buffer_len
+ *
+ * Flush all encoded data into dst, then free ctx.
+ */
+void qcrypto_der_encode_ctx_flush_and_free(QCryptoEncodeContext *ctx,
+                                           uint8_t *dst);
+
 #endif  /* QCRYPTO_ASN1_DECODER_H */
diff --git a/tests/unit/test-crypto-der.c b/tests/unit/test-crypto-der.c
index aed0f28d68..d218a7f170 100644
--- a/tests/unit/test-crypto-der.c
+++ b/tests/unit/test-crypto-der.c
@@ -147,13 +147,58 @@ static const uint8_t test_rsa2048_priv_key[] =
     "\x4e\x2f\x4c\xf9\xab\x97\x38\xe4\x20\x32\x32\x96\xc8\x9e\x79\xd3"
     "\x12";
 
+static const uint8_t test_ecdsa_p192_priv_key[] =
+    "\x30\x53"               /* SEQUENCE, offset 0, length 83 */
+    "\x02\x01\x01"           /* INTEGER, offset 2, length 1 */
+    "\x04\x18"               /* OCTET STRING, offset 5, length 24 */
+    "\xcb\xc8\x86\x0e\x66\x3c\xf7\x5a\x44\x13\xb8\xef\xea\x1d\x7b\xa6"
+    "\x1c\xda\xf4\x1b\xc7\x67\x6b\x35"
+    "\xa1\x34"               /* CONTEXT SPECIFIC 1, offset 31, length 52 */
+    "\x03\x32"               /* BIT STRING, offset 33, length 50 */
+    "\x00\x04\xc4\x16\xb3\xff\xac\xd5\x87\x98\xf7\xd9\x45\xfe\xd3\x5c"
+    "\x17\x9d\xb2\x36\x22\xcc\x07\xb3\x6d\x3c\x4e\x04\x5f\xeb\xb6\x52"
+    "\x58\xfb\x36\x10\x52\xb7\x01\x62\x0e\x94\x51\x1d\xe2\xef\x10\x82"
+    "\x88\x78";
+
+static const uint8_t test_ecdsa_p256_priv_key[] =
+    "\x30\x77"              /* SEQUENCE, offset 0, length 119 */
+    "\x02\x01\x01"          /* INTEGER, offset 2, length 1 */
+    "\x04\x20"              /* OCTET STRING, offset 5, length 32 */
+    "\xf6\x92\xdd\x29\x1c\x6e\xef\xb6\xb2\x73\x9f\x40\x1b\xb3\x2a\x28"
+    "\xd2\x37\xd6\x4a\x5b\xe4\x40\x4c\x6a\x95\x99\xfa\xf7\x92\x49\xbe"
+    "\xa0\x0a"              /* CONTEXT SPECIFIC 0, offset 39, length 10 */
+    "\x06\x08"              /* OID, offset 41, length 8 */
+    "\x2a\x86\x48\xce\x3d\x03\x01\x07"
+    "\xa1\x44"              /* CONTEXT SPECIFIC 1, offset 51, length 68 */
+    "\x03\x42"              /* BIT STRING, offset 53, length 66 */
+    "\x00\x04\xed\x42\x9c\x67\x79\xbe\x46\x83\x88\x3e\x8c\xc1\x33\xf3"
+    "\xc3\xf6\x2c\xf3\x13\x6a\x00\xc2\xc9\x3e\x87\x7f\x86\x39\xe6\xae"
+    "\xe3\xb9\xba\x2f\x58\x63\x32\x62\x62\x54\x07\x27\xf9\x5a\x3a\xc7"
+    "\x3a\x6b\x5b\xbc\x0d\x33\xba\xbb\xd4\xa3\xff\x4f\x9e\xdd\xf5\x59"
+    "\xc0\xf6";
+
 #define MAX_CHECKER_COUNT 32
 
+static int qcrypto_wrapped_decode_ctx_tag0(const uint8_t **data, size_t *dlen,
+                                           QCryptoDERDecodeCb cb, void *opaque,
+                                           Error **errp)
+{
+   return qcrypto_der_decode_ctx_tag(data, dlen, 0, cb, opaque, errp);
+}
+
+static int qcrypto_wrapped_decode_ctx_tag1(const uint8_t **data, size_t *dlen,
+                                           QCryptoDERDecodeCb cb, void *opaque,
+                                           Error **errp)
+{
+   return qcrypto_der_decode_ctx_tag(data, dlen, 1, cb, opaque, errp);
+}
+
 typedef struct QCryptoAns1DecoderResultChecker QCryptoAns1DecoderResultChecker;
 struct QCryptoAns1DecoderResultChecker {
     int (*action) (const uint8_t **data, size_t *dlen,
                    QCryptoDERDecodeCb cb, void *opaque, Error **errp);
     QCryptoDERDecodeCb cb;
+    bool constructed;
     const uint8_t *exp_value;
     size_t exp_vlen;
 };
@@ -204,7 +249,7 @@ static void test_ans1(const void *opaque)
         g_assert(checker->action(&c->data, &c->dlen, checker_callback,
                                  (void *)checker, &error_abort)
             == checker->exp_vlen);
-        if (checker->action == qcrypto_der_decode_seq) {
+        if (checker->constructed) {
             ++seq_depth;
             ctx[seq_depth].data = checker->exp_value;
             ctx[seq_depth].dlen = checker->exp_vlen;
@@ -225,25 +270,25 @@ static QCryptoAns1DecoderTestData test_data[] = {
     .test_data = test_rsa512_priv_key,
     .test_data_len = sizeof(test_rsa512_priv_key) - 1,
     .checker = {
-        { qcrypto_der_decode_seq, checker_callback,
+        { qcrypto_der_decode_seq, checker_callback, true,
           test_rsa512_priv_key + 4, 313 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 4 + 2, 1 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 7 + 2, 65 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 74 + 2, 3 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 79 + 2, 64 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 145 + 2, 33 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 180 + 2, 33 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 215 + 2, 32 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 249 + 2, 32 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 283 + 2, 32 },
     },
 },
@@ -252,29 +297,66 @@ static QCryptoAns1DecoderTestData test_data[] = {
     .test_data = test_rsa2048_priv_key,
     .test_data_len = sizeof(test_rsa2048_priv_key) - 1,
     .checker = {
-        { qcrypto_der_decode_seq, checker_callback,
+        { qcrypto_der_decode_seq, checker_callback, true,
           test_rsa2048_priv_key + 4, 1190 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 4 + 2, 1 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 7 + 4, 257 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 268 + 2, 3 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 273 + 4, 257 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 534 + 3, 129 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 666 + 3, 129 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 798 + 3, 129 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 930 + 3, 129 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 1062 + 3, 129 },
     },
 },
-
+{
+    .path = "/crypto/der/parse-ecdsa-p192-priv-key",
+    .test_data = test_ecdsa_p192_priv_key,
+    .test_data_len = sizeof(test_ecdsa_p192_priv_key) - 1,
+    .checker = {
+        { qcrypto_der_decode_seq, checker_callback, true,
+          test_ecdsa_p192_priv_key + 2, 83 },
+        { qcrypto_der_decode_int, checker_callback, false,
+          test_ecdsa_p192_priv_key + 2 + 2, 1 },
+        { qcrypto_der_decode_octet_str, checker_callback, false,
+          test_ecdsa_p192_priv_key + 5 + 2, 24 },
+        { qcrypto_wrapped_decode_ctx_tag1, checker_callback, true,
+          test_ecdsa_p192_priv_key + 31 + 2, 52 },
+        { qcrypto_der_decode_bit_str , checker_callback, false,
+          test_ecdsa_p192_priv_key + 33 + 2, 50 },
+    },
+},
+{
+    .path = "/crypto/der/parse-ecdsa-p256-priv-key",
+    .test_data = test_ecdsa_p256_priv_key,
+    .test_data_len = sizeof(test_ecdsa_p256_priv_key) - 1,
+    .checker = {
+        { qcrypto_der_decode_seq, checker_callback, true,
+          test_ecdsa_p256_priv_key + 2, 119 },
+        { qcrypto_der_decode_int, checker_callback, false,
+          test_ecdsa_p256_priv_key + 2 + 2, 1 },
+        { qcrypto_der_decode_octet_str, checker_callback, false,
+          test_ecdsa_p256_priv_key + 5 + 2, 32 },
+        { qcrypto_wrapped_decode_ctx_tag0, checker_callback, true,
+          test_ecdsa_p256_priv_key + 39 + 2, 10 },
+        { qcrypto_der_decode_oid, checker_callback, false,
+          test_ecdsa_p256_priv_key + 41 + 2, 8 },
+        { qcrypto_wrapped_decode_ctx_tag1, checker_callback, true,
+          test_ecdsa_p256_priv_key + 51 + 2, 68 },
+        { qcrypto_der_decode_bit_str , checker_callback, false,
+          test_ecdsa_p256_priv_key + 53 + 2, 66 },
+    },
+},
 };
 
 int main(int argc, char **argv)
-- 
2.11.0

Re: [PATCH v2 2/4] crypto: Support DER encodings

[Daniel P. Berrangé]

On Sat, Oct 08, 2022 at 04:50:28PM +0800, Lei He wrote:
> Add encoding interfaces for DER encoding:
> 1. support decoding of 'bit string', 'octet string', 'object id'
> and 'context specific tag' for DER encoder.
> 2. implemented a simple DER encoder.
> 3. add more testsuits for DER encoder.
> 
> Signed-off-by: lei he <helei.sig11@bytedance.com>
> ---
>  crypto/der.c                 | 307 +++++++++++++++++++++++++++++++++++++++----
>  crypto/der.h                 | 211 ++++++++++++++++++++++++++++-
>  tests/unit/test-crypto-der.c | 126 ++++++++++++++----
>  3 files changed, 597 insertions(+), 47 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 3/4] crypto: Support export akcipher to pkcs8

[Lei He]

crypto: support export RSA private keys with PKCS#8 standard.
So that users can upload this private key to linux kernel.

Signed-off-by: lei he <helei.sig11@bytedance.com>
---
 crypto/akcipher.c         | 18 ++++++++++++++++++
 crypto/rsakey.c           | 42 ++++++++++++++++++++++++++++++++++++++++++
 crypto/rsakey.h           | 11 ++++++++++-
 include/crypto/akcipher.h | 21 +++++++++++++++++++++
 4 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/crypto/akcipher.c b/crypto/akcipher.c
index ad88379c1e..e4bbc6e5f1 100644
--- a/crypto/akcipher.c
+++ b/crypto/akcipher.c
@@ -22,6 +22,8 @@
 #include "qemu/osdep.h"
 #include "crypto/akcipher.h"
 #include "akcipherpriv.h"
+#include "der.h"
+#include "rsakey.h"
 
 #if defined(CONFIG_GCRYPT)
 #include "akcipher-gcrypt.c.inc"
@@ -106,3 +108,19 @@ void qcrypto_akcipher_free(QCryptoAkCipher *akcipher)
 
     drv->free(akcipher);
 }
+
+int qcrypto_akcipher_export_p8info(const QCryptoAkCipherOptions *opts,
+                                   uint8_t *key, size_t keylen,
+                                   uint8_t **dst, size_t *dst_len,
+                                   Error **errp)
+{
+    switch (opts->alg) {
+    case QCRYPTO_AKCIPHER_ALG_RSA:
+        qcrypto_akcipher_rsakey_export_p8info(key, keylen, dst, dst_len);
+        return 0;
+
+    default:
+        error_setg(errp, "Unsupported algorithm: %u", opts->alg);
+        return -1;
+    }
+}
diff --git a/crypto/rsakey.c b/crypto/rsakey.c
index cc40e072f0..7d6f273aef 100644
--- a/crypto/rsakey.c
+++ b/crypto/rsakey.c
@@ -19,6 +19,8 @@
  *
  */
 
+#include "qemu/osdep.h"
+#include "der.h"
 #include "rsakey.h"
 
 void qcrypto_akcipher_rsakey_free(QCryptoAkCipherRSAKey *rsa_key)
@@ -37,6 +39,46 @@ void qcrypto_akcipher_rsakey_free(QCryptoAkCipherRSAKey *rsa_key)
     g_free(rsa_key);
 }
 
+/**
+ * PKCS#8 private key info for RSA
+ *
+ * PrivateKeyInfo ::= SEQUENCE {
+ * version         INTEGER,
+ * privateKeyAlgorithm PrivateKeyAlgorithmIdentifier,
+ * privateKey      OCTET STRING,
+ * attributes      [0] IMPLICIT Attributes OPTIONAL
+ * }
+ */
+void qcrypto_akcipher_rsakey_export_p8info(const uint8_t *key,
+                                           size_t keylen,
+                                           uint8_t **dst,
+                                           size_t *dlen)
+{
+    QCryptoEncodeContext *ctx = qcrypto_der_encode_ctx_new();
+    uint8_t version = 0;
+
+    qcrypto_der_encode_seq_begin(ctx);
+
+    /* version */
+    qcrypto_der_encode_int(ctx, &version, sizeof(version));
+
+    /* algorithm identifier */
+    qcrypto_der_encode_seq_begin(ctx);
+    qcrypto_der_encode_oid(ctx, (uint8_t *)QCRYPTO_OID_rsaEncryption,
+                           sizeof(QCRYPTO_OID_rsaEncryption) - 1);
+    qcrypto_der_encode_null(ctx);
+    qcrypto_der_encode_seq_end(ctx);
+
+    /* RSA private key */
+    qcrypto_der_encode_octet_str(ctx, key, keylen);
+
+    qcrypto_der_encode_seq_end(ctx);
+
+    *dlen = qcrypto_der_encode_ctx_buffer_len(ctx);
+    *dst = g_malloc(*dlen);
+    qcrypto_der_encode_ctx_flush_and_free(ctx, *dst);
+}
+
 #if defined(CONFIG_NETTLE) && defined(CONFIG_HOGWEED)
 #include "rsakey-nettle.c.inc"
 #else
diff --git a/crypto/rsakey.h b/crypto/rsakey.h
index 974b76f659..00b3eccec7 100644
--- a/crypto/rsakey.h
+++ b/crypto/rsakey.h
@@ -22,7 +22,6 @@
 #ifndef QCRYPTO_RSAKEY_H
 #define QCRYPTO_RSAKEY_H
 
-#include "qemu/osdep.h"
 #include "qemu/host-utils.h"
 #include "crypto/akcipher.h"
 
@@ -84,6 +83,16 @@ QCryptoAkCipherRSAKey *qcrypto_akcipher_rsakey_parse(
     QCryptoAkCipherKeyType type,
     const uint8_t *key, size_t keylen, Error **errp);
 
+/**
+ * qcrypto_akcipher_rsakey_export_as_p8info:
+ *
+ * Export RSA private key to PKCS#8 private key info.
+ */
+void qcrypto_akcipher_rsakey_export_p8info(const uint8_t *key,
+                                           size_t keylen,
+                                           uint8_t **dst,
+                                           size_t *dlen);
+
 void qcrypto_akcipher_rsakey_free(QCryptoAkCipherRSAKey *key);
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoAkCipherRSAKey,
diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
index 51f5fa2774..214e58ca47 100644
--- a/include/crypto/akcipher.h
+++ b/include/crypto/akcipher.h
@@ -153,6 +153,27 @@ int qcrypto_akcipher_max_dgst_len(QCryptoAkCipher *akcipher);
  */
 void qcrypto_akcipher_free(QCryptoAkCipher *akcipher);
 
+/**
+ * qcrypto_akcipher_export_p8info:
+ * @opts: the options of the akcipher to be exported.
+ * @key: the original key of the akcipher to be exported.
+ * @keylen: length of the 'key'
+ * @dst: output parameter, if export succeed, *dst is set to the
+ * PKCS#8 encoded private key, caller MUST free this key with
+ * g_free after use.
+ * @dst_len: output parameter, indicates the length of PKCS#8 encoded
+ * key.
+ *
+ * Export the akcipher into DER encoded pkcs#8 private key info, expects
+ * |key| stores a valid asymmetric PRIVATE key.
+ *
+ * Returns: 0 for succeed, otherwise -1 is returned.
+ */
+int qcrypto_akcipher_export_p8info(const QCryptoAkCipherOptions *opts,
+                                   uint8_t *key, size_t keylen,
+                                   uint8_t **dst, size_t *dst_len,
+                                   Error **errp);
+
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoAkCipher, qcrypto_akcipher_free)
 
 #endif /* QCRYPTO_AKCIPHER_H */
-- 
2.11.0

Re: [PATCH v2 3/4] crypto: Support export akcipher to pkcs8

[Daniel P. Berrangé]

On Sat, Oct 08, 2022 at 04:50:29PM +0800, Lei He wrote:
> crypto: support export RSA private keys with PKCS#8 standard.
> So that users can upload this private key to linux kernel.
> 
> Signed-off-by: lei he <helei.sig11@bytedance.com>
> ---
>  crypto/akcipher.c         | 18 ++++++++++++++++++
>  crypto/rsakey.c           | 42 ++++++++++++++++++++++++++++++++++++++++++
>  crypto/rsakey.h           | 11 ++++++++++-
>  include/crypto/akcipher.h | 21 +++++++++++++++++++++
>  4 files changed, 91 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 4/4] cryptodev: Add a lkcf-backend for cryptodev

[Lei He]

cryptodev: Added a new type of backend named lkcf-backend for
cryptodev. This backend upload asymmetric keys to linux kernel,
and let kernel do the accelerations if possible.
The lkcf stands for Linux Kernel Cryptography Framework.

Signed-off-by: lei he <helei.sig11@bytedance.com>
---
 backends/cryptodev-lkcf.c  | 645 +++++++++++++++++++++++++++++++++++++++++++++
 backends/meson.build       |   3 +
 include/sysemu/cryptodev.h |   1 +
 qapi/qom.json              |   2 +
 4 files changed, 651 insertions(+)
 create mode 100644 backends/cryptodev-lkcf.c

diff --git a/backends/cryptodev-lkcf.c b/backends/cryptodev-lkcf.c
new file mode 100644
index 0000000000..133bd706a4
--- /dev/null
+++ b/backends/cryptodev-lkcf.c
@@ -0,0 +1,645 @@
+/*
+ * QEMU Cryptodev backend for QEMU cipher APIs
+ *
+ * Copyright (c) 2022 Bytedance.Inc
+ *
+ * Authors:
+ *    lei he <helei.sig11@bytedance.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "crypto/cipher.h"
+#include "crypto/akcipher.h"
+#include "qapi/error.h"
+#include "qemu/main-loop.h"
+#include "qemu/thread.h"
+#include "qemu/error-report.h"
+#include "qemu/queue.h"
+#include "qom/object.h"
+#include "sysemu/cryptodev.h"
+#include "standard-headers/linux/virtio_crypto.h"
+
+#include <keyutils.h>
+#include <sys/eventfd.h>
+
+/**
+ * @TYPE_CRYPTODEV_BACKEND_LKCF:
+ * name of backend that uses linux kernel crypto framework
+ */
+#define TYPE_CRYPTODEV_BACKEND_LKCF "cryptodev-backend-lkcf"
+
+OBJECT_DECLARE_SIMPLE_TYPE(CryptoDevBackendLKCF, CRYPTODEV_BACKEND_LKCF)
+
+#define INVALID_KEY_ID -1
+#define MAX_SESSIONS 256
+#define NR_WORKER_THREAD 64
+
+#define KCTL_KEY_TYPE_PKEY "asymmetric"
+/**
+ * Here the key is uploaded to the thread-keyring of worker thread, at least
+ * util linux-6.0:
+ * 1. process keyring seems to behave unexpectedly if main-thread does not
+ * create the keyring before creating any other thread.
+ * 2. at present, the guest kernel never perform multiple operations on a
+ * session.
+ * 3. it can reduce the load of the main-loop because the key passed by the
+ * guest kernel has been already checked.
+ */
+#define KCTL_KEY_RING KEY_SPEC_THREAD_KEYRING
+
+typedef struct CryptoDevBackendLKCFSession {
+    uint8_t *key;
+    size_t keylen;
+    QCryptoAkCipherKeyType keytype;
+    QCryptoAkCipherOptions akcipher_opts;
+} CryptoDevBackendLKCFSession;
+
+typedef struct CryptoDevBackendLKCF CryptoDevBackendLKCF;
+typedef struct CryptoDevLKCFTask CryptoDevLKCFTask;
+struct CryptoDevLKCFTask {
+    CryptoDevBackendLKCFSession *sess;
+    CryptoDevBackendOpInfo *op_info;
+    CryptoDevCompletionFunc cb;
+    void *opaque;
+    int status;
+    CryptoDevBackendLKCF *lkcf;
+    QSIMPLEQ_ENTRY(CryptoDevLKCFTask) queue;
+};
+
+typedef struct CryptoDevBackendLKCF {
+    CryptoDevBackend parent_obj;
+    CryptoDevBackendLKCFSession *sess[MAX_SESSIONS];
+    QSIMPLEQ_HEAD(, CryptoDevLKCFTask) requests;
+    QSIMPLEQ_HEAD(, CryptoDevLKCFTask) responses;
+    QemuMutex mutex;
+    QemuCond cond;
+    QemuMutex rsp_mutex;
+
+    /**
+     * There is no async interface for asymmetric keys like AF_ALG sockets,
+     * we don't seem to have better way than create a lots of thread.
+     */
+    QemuThread worker_threads[NR_WORKER_THREAD];
+    bool running;
+    int eventfd;
+} CryptoDevBackendLKCF;
+
+static void *cryptodev_lkcf_worker(void *arg);
+static int cryptodev_lkcf_close_session(CryptoDevBackend *backend,
+                                        uint64_t session_id,
+                                        uint32_t queue_index,
+                                        CryptoDevCompletionFunc cb,
+                                        void *opaque);
+
+static void cryptodev_lkcf_handle_response(void *opaque)
+{
+    CryptoDevBackendLKCF *lkcf = (CryptoDevBackendLKCF *)opaque;
+    QSIMPLEQ_HEAD(, CryptoDevLKCFTask) responses;
+    CryptoDevLKCFTask *task, *next;
+    eventfd_t nevent;
+
+    QSIMPLEQ_INIT(&responses);
+    eventfd_read(lkcf->eventfd, &nevent);
+
+    qemu_mutex_lock(&lkcf->rsp_mutex);
+    QSIMPLEQ_PREPEND(&responses, &lkcf->responses);
+    qemu_mutex_unlock(&lkcf->rsp_mutex);
+
+    QSIMPLEQ_FOREACH_SAFE(task, &responses, queue, next) {
+        if (task->cb) {
+            task->cb(task->opaque, task->status);
+        }
+        g_free(task);
+    }
+}
+
+static int cryptodev_lkcf_set_op_desc(QCryptoAkCipherOptions *opts,
+                                      char *key_desc,
+                                      size_t desc_len,
+                                      Error **errp)
+{
+    QCryptoAkCipherOptionsRSA *rsa_opt;
+    if (opts->alg != QCRYPTO_AKCIPHER_ALG_RSA) {
+        error_setg(errp, "Unsupported alg: %u", opts->alg);
+        return -1;
+    }
+
+    rsa_opt = &opts->u.rsa;
+    if (rsa_opt->padding_alg == QCRYPTO_RSA_PADDING_ALG_PKCS1) {
+        snprintf(key_desc, desc_len, "enc=%s hash=%s",
+                 QCryptoRSAPaddingAlgorithm_str(rsa_opt->padding_alg),
+                 QCryptoHashAlgorithm_str(rsa_opt->hash_alg));
+
+    } else {
+        snprintf(key_desc, desc_len, "enc=%s",
+                 QCryptoRSAPaddingAlgorithm_str(rsa_opt->padding_alg));
+    }
+    return 0;
+}
+
+static int cryptodev_lkcf_set_rsa_opt(int virtio_padding_alg,
+                                      int virtio_hash_alg,
+                                      QCryptoAkCipherOptionsRSA *opt,
+                                      Error **errp)
+{
+    if (virtio_padding_alg == VIRTIO_CRYPTO_RSA_PKCS1_PADDING) {
+        opt->padding_alg = QCRYPTO_RSA_PADDING_ALG_PKCS1;
+
+        switch (virtio_hash_alg) {
+        case VIRTIO_CRYPTO_RSA_MD5:
+            opt->hash_alg = QCRYPTO_HASH_ALG_MD5;
+            break;
+
+        case VIRTIO_CRYPTO_RSA_SHA1:
+            opt->hash_alg = QCRYPTO_HASH_ALG_SHA1;
+            break;
+
+        case VIRTIO_CRYPTO_RSA_SHA256:
+            opt->hash_alg = QCRYPTO_HASH_ALG_SHA256;
+            break;
+
+        case VIRTIO_CRYPTO_RSA_SHA512:
+            opt->hash_alg = QCRYPTO_HASH_ALG_SHA512;
+            break;
+
+        default:
+            error_setg(errp, "Unsupported rsa hash algo: %d", virtio_hash_alg);
+            return -1;
+        }
+        return 0;
+    }
+
+    if (virtio_padding_alg == VIRTIO_CRYPTO_RSA_RAW_PADDING) {
+        opt->padding_alg = QCRYPTO_RSA_PADDING_ALG_RAW;
+        return 0;
+    }
+
+    error_setg(errp, "Unsupported rsa padding algo: %u", virtio_padding_alg);
+    return -1;
+}
+
+static int cryptodev_lkcf_get_unused_session_index(CryptoDevBackendLKCF *lkcf)
+{
+    size_t i;
+
+    for (i = 0; i < MAX_SESSIONS; i++) {
+        if (lkcf->sess[i] == NULL) {
+            return i;
+        }
+    }
+    return -1;
+}
+
+static void cryptodev_lkcf_init(CryptoDevBackend *backend, Error **errp)
+{
+    /* Only support one queue */
+    int queues = backend->conf.peers.queues, i;
+    CryptoDevBackendClient *cc;
+    CryptoDevBackendLKCF *lkcf =
+        CRYPTODEV_BACKEND_LKCF(backend);
+
+    if (queues != 1) {
+        error_setg(errp,
+                   "Only support one queue in cryptodev-builtin backend");
+        return;
+    }
+    lkcf->eventfd = eventfd(0, 0);
+    if (lkcf->eventfd < 0) {
+        error_setg(errp, "Failed to create eventfd: %d", errno);
+        return;
+    }
+
+    cc = cryptodev_backend_new_client("cryptodev-lkcf", NULL);
+    cc->info_str = g_strdup_printf("cryptodev-lkcf0");
+    cc->queue_index = 0;
+    cc->type = CRYPTODEV_BACKEND_TYPE_LKCF;
+    backend->conf.peers.ccs[0] = cc;
+
+    backend->conf.crypto_services =
+        1u << VIRTIO_CRYPTO_SERVICE_AKCIPHER;
+    backend->conf.akcipher_algo = 1u << VIRTIO_CRYPTO_AKCIPHER_RSA;
+    lkcf->running = true;
+
+    QSIMPLEQ_INIT(&lkcf->requests);
+    QSIMPLEQ_INIT(&lkcf->responses);
+    qemu_mutex_init(&lkcf->mutex);
+    qemu_mutex_init(&lkcf->rsp_mutex);
+    qemu_cond_init(&lkcf->cond);
+    for (i = 0; i < NR_WORKER_THREAD; i++) {
+        qemu_thread_create(&lkcf->worker_threads[i], "lkcf-worker",
+                           cryptodev_lkcf_worker, lkcf, 0);
+    }
+    qemu_set_fd_handler(
+        lkcf->eventfd, cryptodev_lkcf_handle_response, NULL, lkcf);
+    cryptodev_backend_set_ready(backend, true);
+}
+
+static void cryptodev_lkcf_cleanup(CryptoDevBackend *backend, Error **errp)
+{
+    CryptoDevBackendLKCF *lkcf = CRYPTODEV_BACKEND_LKCF(backend);
+    size_t i;
+    int queues = backend->conf.peers.queues;
+    CryptoDevBackendClient *cc;
+    CryptoDevLKCFTask *task, *next;
+
+    qemu_mutex_lock(&lkcf->mutex);
+    lkcf->running = false;
+    qemu_mutex_unlock(&lkcf->mutex);
+    qemu_cond_broadcast(&lkcf->cond);
+
+    close(lkcf->eventfd);
+    for (i = 0; i < NR_WORKER_THREAD; i++) {
+        qemu_thread_join(&lkcf->worker_threads[i]);
+    }
+
+    QSIMPLEQ_FOREACH_SAFE(task, &lkcf->requests, queue, next) {
+        if (task->cb) {
+            task->cb(task->opaque, task->status);
+        }
+        g_free(task);
+    }
+
+    QSIMPLEQ_FOREACH_SAFE(task, &lkcf->responses, queue, next) {
+        if (task->cb) {
+            task->cb(task->opaque, task->status);
+        }
+        g_free(task);
+    }
+
+    qemu_mutex_destroy(&lkcf->mutex);
+    qemu_cond_destroy(&lkcf->cond);
+    qemu_mutex_destroy(&lkcf->rsp_mutex);
+
+    for (i = 0; i < MAX_SESSIONS; i++) {
+        if (lkcf->sess[i] != NULL) {
+            cryptodev_lkcf_close_session(backend, i, 0, NULL, NULL);
+        }
+    }
+
+    for (i = 0; i < queues; i++) {
+        cc = backend->conf.peers.ccs[i];
+        if (cc) {
+            cryptodev_backend_free_client(cc);
+            backend->conf.peers.ccs[i] = NULL;
+        }
+    }
+
+    cryptodev_backend_set_ready(backend, false);
+}
+
+static void cryptodev_lkcf_execute_task(CryptoDevLKCFTask *task)
+{
+    CryptoDevBackendLKCFSession *session = task->sess;
+    CryptoDevBackendAsymOpInfo *asym_op_info;
+    bool kick = false;
+    int ret, status, op_code = task->op_info->op_code;
+    size_t p8info_len;
+    g_autofree uint8_t *p8info = NULL;
+    Error *local_error = NULL;
+    key_serial_t key_id = INVALID_KEY_ID;
+    char op_desc[64];
+    g_autoptr(QCryptoAkCipher) akcipher = NULL;
+
+    /**
+     * We only offload private key session:
+     * 1. currently, the Linux kernel can only accept public key wrapped
+     * with X.509 certificates, but unfortunately the cost of making a
+     * ceritificate with public key is too expensive.
+     * 2. generally, public key related compution is fast, just compute it with
+     * thread-pool.
+     */
+    if (session->keytype == QCRYPTO_AKCIPHER_KEY_TYPE_PRIVATE) {
+        if (qcrypto_akcipher_export_p8info(&session->akcipher_opts,
+                                           session->key, session->keylen,
+                                           &p8info, &p8info_len,
+                                           &local_error) != 0 ||
+            cryptodev_lkcf_set_op_desc(&session->akcipher_opts, op_desc,
+                                       sizeof(op_desc), &local_error) != 0) {
+            error_report_err(local_error);
+        } else {
+            key_id = add_key(KCTL_KEY_TYPE_PKEY, "lkcf-backend-priv-key",
+                             p8info, p8info_len, KCTL_KEY_RING);
+        }
+    }
+
+    if (key_id < 0) {
+        if (!qcrypto_akcipher_supports(&session->akcipher_opts)) {
+            status = -VIRTIO_CRYPTO_NOTSUPP;
+            goto out;
+        }
+        akcipher = qcrypto_akcipher_new(&session->akcipher_opts,
+                                        session->keytype,
+                                        session->key, session->keylen,
+                                        &local_error);
+        if (!akcipher) {
+            status = -VIRTIO_CRYPTO_ERR;
+            goto out;
+        }
+    }
+
+    asym_op_info = task->op_info->u.asym_op_info;
+    switch (op_code) {
+    case VIRTIO_CRYPTO_AKCIPHER_ENCRYPT:
+        if (key_id >= 0) {
+            ret = keyctl_pkey_encrypt(key_id, op_desc,
+                asym_op_info->src, asym_op_info->src_len,
+                asym_op_info->dst, asym_op_info->dst_len);
+        } else {
+            ret = qcrypto_akcipher_encrypt(akcipher,
+                asym_op_info->src, asym_op_info->src_len,
+                asym_op_info->dst, asym_op_info->dst_len, &local_error);
+        }
+        break;
+
+    case VIRTIO_CRYPTO_AKCIPHER_DECRYPT:
+        if (key_id >= 0) {
+            ret = keyctl_pkey_decrypt(key_id, op_desc,
+                asym_op_info->src, asym_op_info->src_len,
+                asym_op_info->dst, asym_op_info->dst_len);
+        } else {
+            ret = qcrypto_akcipher_decrypt(akcipher,
+                asym_op_info->src, asym_op_info->src_len,
+                asym_op_info->dst, asym_op_info->dst_len, &local_error);
+        }
+        break;
+
+    case VIRTIO_CRYPTO_AKCIPHER_SIGN:
+        if (key_id >= 0) {
+            ret = keyctl_pkey_sign(key_id, op_desc,
+                asym_op_info->src, asym_op_info->src_len,
+                asym_op_info->dst, asym_op_info->dst_len);
+        } else {
+            ret = qcrypto_akcipher_sign(akcipher,
+                asym_op_info->src, asym_op_info->src_len,
+                asym_op_info->dst, asym_op_info->dst_len, &local_error);
+        }
+        break;
+
+    case VIRTIO_CRYPTO_AKCIPHER_VERIFY:
+        if (key_id >= 0) {
+            ret = keyctl_pkey_verify(key_id, op_desc,
+                asym_op_info->src, asym_op_info->src_len,
+                asym_op_info->dst, asym_op_info->dst_len);
+        } else {
+            ret = qcrypto_akcipher_verify(akcipher,
+                asym_op_info->src, asym_op_info->src_len,
+                asym_op_info->dst, asym_op_info->dst_len, &local_error);
+        }
+        break;
+
+    default:
+        error_setg(&local_error, "Unknown opcode: %u", op_code);
+        status = -VIRTIO_CRYPTO_ERR;
+        goto out;
+    }
+
+    if (ret < 0) {
+        if (!local_error) {
+            if (errno != EKEYREJECTED) {
+                error_report("Failed do operation with keyctl: %d", errno);
+            }
+        } else {
+            error_report_err(local_error);
+        }
+        status = op_code == VIRTIO_CRYPTO_AKCIPHER_VERIFY ?
+            -VIRTIO_CRYPTO_KEY_REJECTED : -VIRTIO_CRYPTO_ERR;
+    } else {
+        status = VIRTIO_CRYPTO_OK;
+        asym_op_info->dst_len = ret;
+    }
+
+out:
+    if (key_id >= 0) {
+        keyctl_unlink(key_id, KCTL_KEY_RING);
+    }
+    task->status = status;
+
+    qemu_mutex_lock(&task->lkcf->rsp_mutex);
+    if (QSIMPLEQ_EMPTY(&task->lkcf->responses)) {
+        kick = true;
+    }
+    QSIMPLEQ_INSERT_TAIL(&task->lkcf->responses, task, queue);
+    qemu_mutex_unlock(&task->lkcf->rsp_mutex);
+
+    if (kick) {
+        eventfd_write(task->lkcf->eventfd, 1);
+    }
+}
+
+static void *cryptodev_lkcf_worker(void *arg)
+{
+    CryptoDevBackendLKCF *backend = (CryptoDevBackendLKCF *)arg;
+    CryptoDevLKCFTask *task;
+
+    for (;;) {
+        task = NULL;
+        qemu_mutex_lock(&backend->mutex);
+        while (backend->running && QSIMPLEQ_EMPTY(&backend->requests)) {
+            qemu_cond_wait(&backend->cond, &backend->mutex);
+        }
+        if (backend->running) {
+            task = QSIMPLEQ_FIRST(&backend->requests);
+            QSIMPLEQ_REMOVE_HEAD(&backend->requests, queue);
+        }
+        qemu_mutex_unlock(&backend->mutex);
+
+        /* stopped */
+        if (!task) {
+            break;
+        }
+        cryptodev_lkcf_execute_task(task);
+   }
+
+   return NULL;
+}
+
+static int cryptodev_lkcf_operation(
+    CryptoDevBackend *backend,
+    CryptoDevBackendOpInfo *op_info,
+    uint32_t queue_index,
+    CryptoDevCompletionFunc cb,
+    void *opaque)
+{
+    CryptoDevBackendLKCF *lkcf =
+        CRYPTODEV_BACKEND_LKCF(backend);
+    CryptoDevBackendLKCFSession *sess;
+    enum CryptoDevBackendAlgType algtype = op_info->algtype;
+    CryptoDevLKCFTask *task;
+
+    if (op_info->session_id >= MAX_SESSIONS ||
+        lkcf->sess[op_info->session_id] == NULL) {
+        error_report("Cannot find a valid session id: %" PRIu64 "",
+                     op_info->session_id);
+        return -VIRTIO_CRYPTO_INVSESS;
+    }
+
+    sess = lkcf->sess[op_info->session_id];
+    if (algtype != CRYPTODEV_BACKEND_ALG_ASYM) {
+        error_report("algtype not supported: %u", algtype);
+        return -VIRTIO_CRYPTO_NOTSUPP;
+    }
+
+    task = g_new0(CryptoDevLKCFTask, 1);
+    task->op_info = op_info;
+    task->cb = cb;
+    task->opaque = opaque;
+    task->sess = sess;
+    task->lkcf = lkcf;
+    task->status = -VIRTIO_CRYPTO_ERR;
+
+    qemu_mutex_lock(&lkcf->mutex);
+    QSIMPLEQ_INSERT_TAIL(&lkcf->requests, task, queue);
+    qemu_mutex_unlock(&lkcf->mutex);
+    qemu_cond_signal(&lkcf->cond);
+
+    return VIRTIO_CRYPTO_OK;
+}
+
+static int cryptodev_lkcf_create_asym_session(
+    CryptoDevBackendLKCF *lkcf,
+    CryptoDevBackendAsymSessionInfo *sess_info,
+    uint64_t *session_id)
+{
+    Error *local_error = NULL;
+    int index;
+    g_autofree CryptoDevBackendLKCFSession *sess =
+        g_new0(CryptoDevBackendLKCFSession, 1);
+
+    switch (sess_info->algo) {
+    case VIRTIO_CRYPTO_AKCIPHER_RSA:
+        sess->akcipher_opts.alg = QCRYPTO_AKCIPHER_ALG_RSA;
+        if (cryptodev_lkcf_set_rsa_opt(
+            sess_info->u.rsa.padding_algo, sess_info->u.rsa.hash_algo,
+            &sess->akcipher_opts.u.rsa, &local_error) != 0) {
+            error_report_err(local_error);
+            return -VIRTIO_CRYPTO_ERR;
+        }
+        break;
+
+    default:
+        error_report("Unsupported asym alg %u", sess_info->algo);
+        return -VIRTIO_CRYPTO_NOTSUPP;
+    }
+
+    switch (sess_info->keytype) {
+    case VIRTIO_CRYPTO_AKCIPHER_KEY_TYPE_PUBLIC:
+        sess->keytype = QCRYPTO_AKCIPHER_KEY_TYPE_PUBLIC;
+        break;
+
+    case VIRTIO_CRYPTO_AKCIPHER_KEY_TYPE_PRIVATE:
+        sess->keytype = QCRYPTO_AKCIPHER_KEY_TYPE_PRIVATE;
+        break;
+
+    default:
+        error_report("Unknown akcipher keytype: %u", sess_info->keytype);
+        return -VIRTIO_CRYPTO_ERR;
+    }
+
+    index = cryptodev_lkcf_get_unused_session_index(lkcf);
+    if (index < 0) {
+        error_report("Total number of sessions created exceeds %u",
+                     MAX_SESSIONS);
+        return -VIRTIO_CRYPTO_ERR;
+    }
+
+    sess->keylen = sess_info->keylen;
+    sess->key = g_malloc(sess_info->keylen);
+    memcpy(sess->key, sess_info->key, sess_info->keylen);
+
+    lkcf->sess[index] = g_steal_pointer(&sess);
+    *session_id = index;
+
+    return VIRTIO_CRYPTO_OK;
+}
+
+static int cryptodev_lkcf_create_session(
+    CryptoDevBackend *backend,
+    CryptoDevBackendSessionInfo *sess_info,
+    uint32_t queue_index,
+    CryptoDevCompletionFunc cb,
+    void *opaque)
+{
+    CryptoDevBackendAsymSessionInfo *asym_sess_info;
+    CryptoDevBackendLKCF *lkcf =
+        CRYPTODEV_BACKEND_LKCF(backend);
+    int ret;
+
+    switch (sess_info->op_code) {
+    case VIRTIO_CRYPTO_AKCIPHER_CREATE_SESSION:
+        asym_sess_info = &sess_info->u.asym_sess_info;
+        ret = cryptodev_lkcf_create_asym_session(
+            lkcf, asym_sess_info, &sess_info->session_id);
+        break;
+
+    default:
+        ret = -VIRTIO_CRYPTO_NOTSUPP;
+        error_report("Unsupported opcode: %" PRIu32 "",
+                     sess_info->op_code);
+        break;
+    }
+    if (cb) {
+        cb(opaque, ret);
+    }
+    return 0;
+}
+
+static int cryptodev_lkcf_close_session(CryptoDevBackend *backend,
+                                        uint64_t session_id,
+                                        uint32_t queue_index,
+                                        CryptoDevCompletionFunc cb,
+                                        void *opaque)
+{
+    CryptoDevBackendLKCF *lkcf = CRYPTODEV_BACKEND_LKCF(backend);
+    CryptoDevBackendLKCFSession *session;
+
+    assert(session_id < MAX_SESSIONS && lkcf->sess[session_id]);
+    session = lkcf->sess[session_id];
+    lkcf->sess[session_id] = NULL;
+
+    g_free(session->key);
+    g_free(session);
+
+    if (cb) {
+        cb(opaque, VIRTIO_CRYPTO_OK);
+    }
+    return 0;
+}
+
+static void cryptodev_lkcf_class_init(ObjectClass *oc, void *data)
+{
+    CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_CLASS(oc);
+
+    bc->init = cryptodev_lkcf_init;
+    bc->cleanup = cryptodev_lkcf_cleanup;
+    bc->create_session = cryptodev_lkcf_create_session;
+    bc->close_session = cryptodev_lkcf_close_session;
+    bc->do_op = cryptodev_lkcf_operation;
+}
+
+static const TypeInfo cryptodev_builtin_info = {
+    .name = TYPE_CRYPTODEV_BACKEND_LKCF,
+    .parent = TYPE_CRYPTODEV_BACKEND,
+    .class_init = cryptodev_lkcf_class_init,
+    .instance_size = sizeof(CryptoDevBackendLKCF),
+};
+
+static void cryptodev_lkcf_register_types(void)
+{
+    type_register_static(&cryptodev_builtin_info);
+}
+
+type_init(cryptodev_lkcf_register_types);
diff --git a/backends/meson.build b/backends/meson.build
index b1884a88ec..954e658b25 100644
--- a/backends/meson.build
+++ b/backends/meson.build
@@ -12,6 +12,9 @@ softmmu_ss.add([files(
 softmmu_ss.add(when: 'CONFIG_POSIX', if_true: files('rng-random.c'))
 softmmu_ss.add(when: 'CONFIG_POSIX', if_true: files('hostmem-file.c'))
 softmmu_ss.add(when: 'CONFIG_LINUX', if_true: files('hostmem-memfd.c'))
+if keyutils.found()
+    softmmu_ss.add(keyutils, files('cryptodev-lkcf.c'))
+endif
 if have_vhost_user
   softmmu_ss.add(when: 'CONFIG_VIRTIO', if_true: files('vhost-user.c'))
 endif
diff --git a/include/sysemu/cryptodev.h b/include/sysemu/cryptodev.h
index 32e9f4cf8a..cf9b3f07fe 100644
--- a/include/sysemu/cryptodev.h
+++ b/include/sysemu/cryptodev.h
@@ -219,6 +219,7 @@ typedef enum CryptoDevBackendOptionsType {
     CRYPTODEV_BACKEND_TYPE_NONE = 0,
     CRYPTODEV_BACKEND_TYPE_BUILTIN = 1,
     CRYPTODEV_BACKEND_TYPE_VHOST_USER = 2,
+    CRYPTODEV_BACKEND_TYPE_LKCF = 3,
     CRYPTODEV_BACKEND_TYPE__MAX,
 } CryptoDevBackendOptionsType;
 
diff --git a/qapi/qom.json b/qapi/qom.json
index 80dd419b39..cb83497c73 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -850,6 +850,7 @@
     'colo-compare',
     'cryptodev-backend',
     'cryptodev-backend-builtin',
+    'cryptodev-backend-lkcf',
     { 'name': 'cryptodev-vhost-user',
       'if': 'CONFIG_VHOST_CRYPTO' },
     'dbus-vmstate',
@@ -917,6 +918,7 @@
       'colo-compare':               'ColoCompareProperties',
       'cryptodev-backend':          'CryptodevBackendProperties',
       'cryptodev-backend-builtin':  'CryptodevBackendProperties',
+      'cryptodev-backend-lkcf':     'CryptodevBackendProperties',
       'cryptodev-vhost-user':       { 'type': 'CryptodevVhostUserProperties',
                                       'if': 'CONFIG_VHOST_CRYPTO' },
       'dbus-vmstate':               'DBusVMStateProperties',
-- 
2.11.0

Re: [PATCH v2 0/4] Add a new backend for cryptodev

[Lei He]

On 2022/10/8 16:50, Lei He wrote:
> v1 --> v2:
> - Fix compile errors when neither 'nettle' nor 'gcrypt' are enabled.
> - Trivial changes to error codes when neither 'nettle' nor 'gcrypt' are
> enabled.
> 

Hi, lei:
   Daniel has reviewed the crypto part of this patch(thanks a lot for 
Daniel), can you help to have a glance at rest codes? Thanks.

Best regards,
Lei He
--
helei.sig11@bytedance.com