All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation
@ 2016-09-13  3:52 Gonglei
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto legacy hardware Gonglei
                   ` (15 more replies)
  0 siblings, 16 replies; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

Changes since v1:
 - rmmove mixed endian-ness handler for virtio-crypto device, just
   use little-endian. [mst]
 - add sg list support according virtio-crypto spec v10 (will be posted soon).
 - fix a memory leak in session handler.
 - add a feature page link in qemu.org (http://qemu-project.org/Features/VirtioCrypto)
 - fix some trivial problems, sush as 's/Since 2.7/Since 2.8/g' in qapi-schema.json
 - rebase the latest qemu master tree.

Please review, thanks!

This patch series realize the framework and emulation of a new
virtio crypto device, which is similar with virtio net device.
 
 - I introduce the cryptodev backend as the client of virtio crypto device
   which can be realized by different methods, such as cryptodev-linux in my series,
   vhost-crypto kernel module, vhost-user etc.
 - The patch set abides by the virtio crypto speccification.
 - The virtio crypto support symmetric algorithms (including CIPHER and algorithm chainning)
   at present, except HASH, MAC and AEAD services.
 - unsupport hot plug/unplug cryptodev client at this moment.

Cryptodev-linux is a device that allows access to Linux kernel cryptographic drivers;
thus allowing of userspace applications to take advantage of hardware accelerators.
It can be found here:

 http://cryptodev-linux.org/

(please use the latest version)

To use the cryptodev-linux as the client, the cryptodev.ko should be insert on the host.

 # enter cryptodev-linux module root directory, then
 make;make install

then configuring QEMU shows:

 [...]
 jemalloc support  no
 avx2 optimization no
 cryptodev-linux support yes

QEMU can then be started using the following parameters:

qemu-system-x86_64 \
    [...] \
        -cryptodev type=cryptodev-linux,id=cryptodev0 \
        -device virtio-crypto-pci,id=crypto0,cryptodev=cryptodev0 \
    [...]

The front-end linux kernel driver (Experimental at present) is publicly accessible from:
 
   https://github.com/gongleiarei/virtio-crypto-linux-driver.git

After insmod virtio-crypto.ko, you also can use cryptodev-linux test the crypto function
in the guest. For example:

linux-guest:/home/gonglei/cryptodev-linux/tests # ./cipher -
requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver virtio_crypto_aes_cbc
AES Test passed
requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver virtio_crypto_aes_cbc
requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver virtio_crypto_aes_cbc
Test passed

QEMU code also can be accessible from:

 https://github.com/gongleiarei/qemu.git 

 branch virtio-crypto

For more information, please see:
 http://qemu-project.org/Features/VirtioCrypto


Gonglei (15):
  crypto: introduce cryptodev backend and crypto legacy hardware
  crypto: introduce crypto queue handler
  crypto: add cryptoLegacyHW stuff
  crypto: add symetric algorithms support
  crypto: add cryptodev-linux as a cryptodev backend
  crypto: add internal handle logic layer
  virtio-crypto: introduce virtio-crypto.h
  virtio-crypto-pci: add virtio crypto pci support
  virtio-crypto: add virtio crypto realization
  virtio-crypto: set capacity of crypto legacy hardware
  virtio-crypto: add control queue handler
  virtio-crypto: add destroy session logic
  virtio-crypto: get correct input data address for each request
  virtio-crypto: add data virtqueue processing handler
  virtio-crypto: support scatter gather list

 configure                                      |   16 +
 crypto/Makefile.objs                           |    3 +
 crypto/crypto-queue.c                          |  206 +++++
 crypto/crypto.c                                |  378 +++++++++
 crypto/cryptodev-linux.c                       |  419 ++++++++++
 hw/core/qdev-properties-system.c               |   86 ++
 hw/virtio/Makefile.objs                        |    3 +-
 hw/virtio/virtio-crypto-pci.c                  |   71 ++
 hw/virtio/virtio-crypto.c                      | 1013 ++++++++++++++++++++++++
 hw/virtio/virtio-pci.h                         |   15 +
 include/crypto/crypto-clients.h                |   39 +
 include/crypto/crypto-queue.h                  |   69 ++
 include/crypto/crypto.h                        |  192 +++++
 include/hw/qdev-properties.h                   |    3 +
 include/hw/virtio/virtio-crypto.h              |   96 +++
 include/qemu/typedefs.h                        |    1 +
 include/standard-headers/linux/virtio_crypto.h |  466 +++++++++++
 include/sysemu/sysemu.h                        |    1 +
 qapi-schema.json                               |   61 ++
 qemu-options.hx                                |   19 +
 vl.c                                           |   13 +
 21 files changed, 3169 insertions(+), 1 deletion(-)
 create mode 100644 crypto/crypto-queue.c
 create mode 100644 crypto/crypto.c
 create mode 100644 crypto/cryptodev-linux.c
 create mode 100644 hw/virtio/virtio-crypto-pci.c
 create mode 100644 hw/virtio/virtio-crypto.c
 create mode 100644 include/crypto/crypto-clients.h
 create mode 100644 include/crypto/crypto-queue.h
 create mode 100644 include/crypto/crypto.h
 create mode 100644 include/hw/virtio/virtio-crypto.h
 create mode 100644 include/standard-headers/linux/virtio_crypto.h

-- 
1.7.12.4

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [Qemu-devel] [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto legacy hardware
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
@ 2016-09-13  3:52 ` Gonglei
  2016-09-13  9:13   ` Daniel P. Berrange
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 02/15] crypto: introduce crypto queue handler Gonglei
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

cryptodev backend is used to realize the active work for
virtual crypto device. CryptoLegacyHW device is a cryptographic
hardware device seen by the virtual machine.
The relationship between cryptodev backend and legacy hadware
as follow:
 crypto_legacy_hw device (1)--->(n) cryptodev client backend

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 crypto/Makefile.objs    |   1 +
 crypto/crypto.c         | 171 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/crypto/crypto.h |  66 +++++++++++++++++++
 include/qemu/typedefs.h |   1 +
 include/sysemu/sysemu.h |   1 +
 qapi-schema.json        |  46 +++++++++++++
 qemu-options.hx         |   3 +
 vl.c                    |  13 ++++
 8 files changed, 302 insertions(+)
 create mode 100644 crypto/crypto.c
 create mode 100644 include/crypto/crypto.h

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index a36d2d9..2a63cb8 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -26,6 +26,7 @@ crypto-obj-y += xts.o
 crypto-obj-y += block.o
 crypto-obj-y += block-qcow.o
 crypto-obj-y += block-luks.o
+crypto-obj-y += crypto.o
 
 # Let the userspace emulators avoid linking gnutls/etc
 crypto-aes-obj-y = aes.o
diff --git a/crypto/crypto.c b/crypto/crypto.c
new file mode 100644
index 0000000..fbc6497
--- /dev/null
+++ b/crypto/crypto.c
@@ -0,0 +1,171 @@
+/*
+ * QEMU Crypto Device Implement
+ *
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *    Gonglei <arei.gonglei@huawei.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu/osdep.h"
+#include "sysemu/sysemu.h"
+#include "qapi/error.h"
+#include "qemu/iov.h"
+#include "qapi-visit.h"
+#include "qapi/opts-visitor.h"
+
+#include "crypto/crypto.h"
+#include "qemu/config-file.h"
+#include "monitor/monitor.h"
+
+
+static QTAILQ_HEAD(, CryptoClientState) crypto_clients;
+
+QemuOptsList qemu_cryptodev_opts = {
+    .name = "cryptodev",
+    .implied_opt_name = "type",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_cryptodev_opts.head),
+    .desc = {
+        /*
+         * no elements => accept any params
+         * validation will happen later
+         */
+        { /* end of list */ }
+    },
+};
+
+static int
+crypto_init_cryptodev(void *dummy, QemuOpts *opts, Error **errp)
+{
+    Error *local_err = NULL;
+    int ret;
+
+    ret = crypto_client_init(opts, &local_err);
+    if (local_err) {
+        error_report_err(local_err);
+        return -1;
+    }
+
+    return ret;
+}
+
+int crypto_init_clients(void)
+{
+    QTAILQ_INIT(&crypto_clients);
+
+    if (qemu_opts_foreach(qemu_find_opts("cryptodev"),
+                          crypto_init_cryptodev, NULL, NULL)) {
+        return -1;
+    }
+
+    return 0;
+}
+
+static int (* const crypto_client_init_fun[CRYPTO_CLIENT_OPTIONS_KIND__MAX])(
+    const CryptoClientOptions *opts,
+    const char *name,
+    CryptoClientState *peer, Error **errp);
+
+static int crypto_client_init1(const void *object, Error **errp)
+{
+    const CryptoClientOptions *opts;
+    const char *name;
+
+    const Cryptodev *cryptodev = object;
+    opts = cryptodev->opts;
+    name = cryptodev->id;
+
+    if (opts->type == CRYPTO_CLIENT_OPTIONS_KIND_LEGACY_HW ||
+        !crypto_client_init_fun[opts->type]) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
+                   "a cryptodev backend type");
+        return -1;
+    }
+
+    if (crypto_client_init_fun[opts->type](opts, name, NULL, errp) < 0) {
+        if (errp && !*errp) {
+            error_setg(errp, QERR_DEVICE_INIT_FAILED,
+                       CryptoClientOptionsKind_lookup[opts->type]);
+        }
+        return -1;
+    }
+    return 0;
+}
+
+int crypto_client_init(QemuOpts *opts, Error **errp)
+{
+    void *object = NULL;
+    Error *err = NULL;
+    int ret = -1;
+    Visitor *v = opts_visitor_new(opts);
+
+    visit_type_Cryptodev(v, NULL, (Cryptodev **)&object, &err);
+    if (!err) {
+        ret = crypto_client_init1(object, &err);
+    }
+
+    qapi_free_Cryptodev(object);
+    error_propagate(errp, err);
+    return ret;
+}
+
+static void crypto_client_destructor(CryptoClientState *cc)
+{
+    g_free(cc);
+}
+
+static void crypto_client_setup(CryptoClientState *cc,
+                                  CryptoClientInfo *info,
+                                  CryptoClientState *peer,
+                                  const char *model,
+                                  const char *name,
+                                  CryptoClientDestructor *destructor)
+{
+    cc->info = info;
+    cc->model = g_strdup(model);
+    if (name) {
+        cc->name = g_strdup(name);
+    }
+
+    if (peer) {
+        assert(!peer->peer);
+        cc->peer = peer;
+        peer->peer = cc;
+    }
+    QTAILQ_INSERT_TAIL(&crypto_clients, cc, next);
+    cc->destructor = destructor;
+}
+
+CryptoClientState *new_crypto_client(CryptoClientInfo *info,
+                                    CryptoClientState *peer,
+                                    const char *model,
+                                    const char *name)
+{
+    CryptoClientState *cc;
+
+    assert(info->size >= sizeof(CryptoClientState));
+
+    /* allocate the memroy of CryptoClientState's parent */
+    cc = g_malloc0(info->size);
+    crypto_client_setup(cc, info, peer, model, name,
+                          crypto_client_destructor);
+
+    return cc;
+}
diff --git a/include/crypto/crypto.h b/include/crypto/crypto.h
new file mode 100644
index 0000000..f93f6f9
--- /dev/null
+++ b/include/crypto/crypto.h
@@ -0,0 +1,66 @@
+/*
+ * QEMU Crypto Device Implement
+ *
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *    Gonglei <arei.gonglei@huawei.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#ifndef QCRYPTO_CRYPTO_H__
+#define QCRYPTO_CRYPTO_H__
+
+#include "qemu/queue.h"
+#include "qapi-types.h"
+
+typedef void (CryptoPoll)(CryptoClientState *, bool);
+typedef void (CryptoCleanup) (CryptoClientState *);
+typedef void (CryptoClientDestructor)(CryptoClientState *);
+typedef void (CryptoHWStatusChanged)(CryptoClientState *);
+
+typedef struct CryptoClientInfo {
+    CryptoClientOptionsKind type;
+    size_t size;
+
+    CryptoCleanup *cleanup;
+    CryptoPoll *poll;
+    CryptoHWStatusChanged *hw_status_changed;
+} CryptoClientInfo;
+
+struct CryptoClientState {
+    CryptoClientInfo *info;
+    int ready;
+    QTAILQ_ENTRY(CryptoClientState) next;
+    CryptoClientState *peer;
+    char *model;
+    char *name;
+    char info_str[256];
+    CryptoClientDestructor *destructor;
+};
+
+int crypto_client_init(QemuOpts *opts, Error **errp);
+int crypto_init_clients(void);
+
+CryptoClientState *new_crypto_client(CryptoClientInfo *info,
+                                    CryptoClientState *peer,
+                                    const char *model,
+                                    const char *name);
+
+#endif /* QCRYPTO_CRYPTO_H__ */
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index b113fcf..82843a9 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -94,5 +94,6 @@ typedef struct SSIBus SSIBus;
 typedef struct uWireSlave uWireSlave;
 typedef struct VirtIODevice VirtIODevice;
 typedef struct Visitor Visitor;
+typedef struct CryptoClientState CryptoClientState;
 
 #endif /* QEMU_TYPEDEFS_H */
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index ee7c760..58e0d88 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -244,5 +244,6 @@ extern QemuOptsList qemu_netdev_opts;
 extern QemuOptsList qemu_net_opts;
 extern QemuOptsList qemu_global_opts;
 extern QemuOptsList qemu_mon_opts;
+extern QemuOptsList qemu_cryptodev_opts;
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index c4f3674..46f7993 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4582,3 +4582,49 @@
 # Since: 2.7
 ##
 { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
+
+##
+# @CryptoLegacyHWOptions
+#
+# Create a new cryptographic hardware device.
+#
+# @cryptodev: #optional id of -cryptodev to connect to
+#
+# @model: #optional device model (Only virtio at present)
+#
+# @vectors: #optional number of MSI-x vectors, 0 to disable MSI-X
+#
+# Since 2.8
+##
+{ 'struct': 'CryptoLegacyHWOptions',
+  'data': {
+    '*cryptodev':  'str',
+    '*model':   'str',
+    '*vectors': 'uint32' } }
+
+##
+# @CryptoClientOptions
+#
+# A discriminated record of crypto device traits.
+#
+# Since 2.8
+#
+##
+{ 'union': 'CryptoClientOptions',
+  'data': {'legacy-hw':   'CryptoLegacyHWOptions'} }
+
+##
+# @Cryptodev
+#
+# Captures the configuration of a crypto device.
+#
+# @id: identifier for monitor commands.
+#
+# @opts: device type specific properties
+#
+# Since 2.8
+##
+{ 'struct': 'Cryptodev',
+  'data': {
+    'id':   'str',
+    'opts': 'CryptoClientOptions' } }
diff --git a/qemu-options.hx b/qemu-options.hx
index a71aaf8..a64ce25 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3968,6 +3968,9 @@ contents of @code{iv.b64} to the second secret
 
 ETEXI
 
+DEF("cryptodev", HAS_ARG, QEMU_OPTION_cryptodev,
+    "",
+    QEMU_ARCH_ALL)
 
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
diff --git a/vl.c b/vl.c
index ee557a1..24e86ea 100644
--- a/vl.c
+++ b/vl.c
@@ -119,6 +119,7 @@ int main(int argc, char **argv)
 #include "qapi-event.h"
 #include "exec/semihost.h"
 #include "crypto/init.h"
+#include "crypto/crypto.h"
 #include "sysemu/replay.h"
 #include "qapi/qmp/qerror.h"
 
@@ -3015,6 +3016,7 @@ int main(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_icount_opts);
     qemu_add_opts(&qemu_semihosting_config_opts);
     qemu_add_opts(&qemu_fw_cfg_opts);
+    qemu_add_opts(&qemu_cryptodev_opts);
     module_call_init(MODULE_INIT_OPTS);
 
     runstate_init();
@@ -3983,6 +3985,13 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
                 break;
+            case QEMU_OPTION_cryptodev:
+                opts = qemu_opts_parse_noisily(qemu_find_opts("cryptodev"),
+                                               optarg, false);
+                if (!opts) {
+                    exit(1);
+                }
+                break;
             default:
                 os_parse_cmd_args(popt->index, optarg);
             }
@@ -4372,6 +4381,10 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    if (crypto_init_clients() < 0) {
+        exit(1);
+    }
+
     if (qemu_opts_foreach(qemu_find_opts("object"),
                           user_creatable_add_opts_foreach,
                           object_create_delayed, NULL)) {
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [Qemu-devel] [PATCH v2 02/15] crypto: introduce crypto queue handler
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto legacy hardware Gonglei
@ 2016-09-13  3:52 ` Gonglei
  2016-09-13  9:20   ` Daniel P. Berrange
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 03/15] crypto: add cryptoLegacyHW stuff Gonglei
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

crypto queue is a gallery used for executing crypto
operation, which supports both synchronization and
asynchronization. The thoughts stolen from net/queue.c

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 crypto/Makefile.objs          |   1 +
 crypto/crypto-queue.c         | 206 ++++++++++++++++++++++++++++++++++++++++++
 crypto/crypto.c               |  28 ++++++
 include/crypto/crypto-queue.h |  69 ++++++++++++++
 include/crypto/crypto.h       |  12 +++
 5 files changed, 316 insertions(+)
 create mode 100644 crypto/crypto-queue.c
 create mode 100644 include/crypto/crypto-queue.h

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 2a63cb8..652b429 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -27,6 +27,7 @@ crypto-obj-y += block.o
 crypto-obj-y += block-qcow.o
 crypto-obj-y += block-luks.o
 crypto-obj-y += crypto.o
+crypto-obj-y += crypto-queue.o
 
 # Let the userspace emulators avoid linking gnutls/etc
 crypto-aes-obj-y = aes.o
diff --git a/crypto/crypto-queue.c b/crypto/crypto-queue.c
new file mode 100644
index 0000000..3e91be9
--- /dev/null
+++ b/crypto/crypto-queue.c
@@ -0,0 +1,206 @@
+/*
+ * Queue management for crypto device (based on net/qeueu.c)
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *    Gonglei <arei.gonglei@huawei.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "crypto/crypto-queue.h"
+#include "crypto/crypto.h"
+#include "qemu/queue.h"
+
+
+/* The delivery handler may only return zero if it will call
+ * qemu_crypto_queue_flush() when it determines that it is once again able
+ * to deliver packets. It must also call qemu_crypto_queue_purge() in its
+ * cleanup path.
+ *
+ * If a sent callback is provided to send(), the caller must handle a
+ * zero return from the delivery handler by not sending any more packets
+ * until we have invoked the callback. Only in that case will we queue
+ * the packet.
+ *
+ * If a sent callback isn't provided, we just drop the packet to avoid
+ * unbounded queueing.
+ */
+
+struct CryptoPacket {
+    QTAILQ_ENTRY(CryptoPacket) entry;
+    CryptoClientState *sender;
+    unsigned flags; /* algorithms' type etc. */
+    CryptoPacketSent *sent_cb; /* callback after packet sent */
+    void *opaque; /* header struct pointer of operation */
+    uint8_t data[0];
+};
+
+struct CryptoQueue {
+    void *opaque;
+    uint32_t nq_maxlen;
+    uint32_t nq_count;
+    CryptoQueueDeliverFunc *deliver;
+
+    QTAILQ_HEAD(packets, CryptoPacket) packets;
+
+    unsigned delivering:1;
+};
+
+CryptoQueue *
+qemu_new_crypto_queue(CryptoQueueDeliverFunc *deliver, void *opaque)
+{
+    CryptoQueue *queue;
+
+    queue = g_new0(CryptoQueue, 1);
+
+    queue->opaque = opaque;
+    queue->nq_maxlen = 10000;
+    queue->nq_count = 0;
+    queue->deliver = deliver;
+
+    QTAILQ_INIT(&queue->packets);
+
+    queue->delivering = 0;
+
+    return queue;
+}
+
+void qemu_del_crypto_queue(CryptoQueue *queue)
+{
+    CryptoPacket *packet, *next;
+
+    QTAILQ_FOREACH_SAFE(packet, &queue->packets, entry, next) {
+        QTAILQ_REMOVE(&queue->packets, packet, entry);
+        g_free(packet->opaque);
+        g_free(packet);
+    }
+
+    g_free(queue);
+}
+
+void qemu_crypto_queue_cache(CryptoQueue *queue,
+                               unsigned flags,
+                               CryptoClientState *sender,
+                               void *opaque,
+                               CryptoPacketSent *sent_cb)
+{
+    CryptoPacket *packet;
+
+    if (queue->nq_count >= queue->nq_maxlen && !sent_cb) {
+        return; /* drop if queue full and no callback */
+    }
+
+    packet = g_malloc(sizeof(CryptoPacket));
+    packet->sender = sender;
+    packet->sent_cb = sent_cb;
+    packet->flags = flags,
+    packet->opaque = opaque;
+
+    queue->nq_count++;
+    QTAILQ_INSERT_TAIL(&queue->packets, packet, entry);
+}
+
+static ssize_t qemu_crypto_queue_deliver(CryptoQueue *queue,
+                                         unsigned flags,
+                                         CryptoClientState *sender,
+                                         void *opaque)
+{
+    ssize_t ret = -1;
+
+    queue->delivering = 1;
+    ret = queue->deliver(sender, flags, opaque, queue->opaque);
+    queue->delivering = 0;
+
+    return ret;
+}
+
+int qemu_crypto_queue_send(CryptoQueue *queue,
+                            unsigned flags,
+                            CryptoClientState *sender,
+                            void *opaque,
+                            CryptoPacketSent *sent_cb)
+{
+    int ret;
+
+    if (queue->delivering) {
+        qemu_crypto_queue_cache(queue, flags, sender,
+                                opaque, sent_cb);
+        return 0;
+    }
+
+    ret = qemu_crypto_queue_deliver(queue, flags, sender, opaque);
+    if (ret == 0) {
+        qemu_crypto_queue_cache(queue, flags, sender,
+                                opaque, sent_cb);
+        return 0;
+    }
+
+    qemu_crypto_queue_flush(queue);
+
+    return ret;
+}
+
+void qemu_crypto_queue_purge(CryptoQueue *queue, CryptoClientState *from)
+{
+    CryptoPacket *packet, *next;
+
+    QTAILQ_FOREACH_SAFE(packet, &queue->packets, entry, next) {
+        if (packet->sender == from) {
+            QTAILQ_REMOVE(&queue->packets, packet, entry);
+            queue->nq_count--;
+            if (packet->sent_cb) {
+                packet->sent_cb(packet->sender, 0);
+            }
+            g_free(packet->opaque);
+            g_free(packet);
+        }
+    }
+}
+
+bool qemu_crypto_queue_flush(CryptoQueue *queue)
+{
+    while (!QTAILQ_EMPTY(&queue->packets)) {
+        CryptoPacket *packet;
+        int ret;
+
+        packet = QTAILQ_FIRST(&queue->packets);
+        QTAILQ_REMOVE(&queue->packets, packet, entry);
+        queue->nq_count--;
+
+        ret = qemu_crypto_queue_deliver(queue, packet->flags,
+                                        packet->sender, packet->opaque);
+        if (ret == 0) {
+            queue->nq_count++;
+            QTAILQ_INSERT_HEAD(&queue->packets, packet, entry);
+            return false;
+        }
+
+        if (packet->sent_cb) {
+            packet->sent_cb(packet->sender, ret);
+        }
+
+        g_free(packet->opaque);
+        g_free(packet);
+    }
+    return true;
+}
diff --git a/crypto/crypto.c b/crypto/crypto.c
index fbc6497..a0e4a34 100644
--- a/crypto/crypto.c
+++ b/crypto/crypto.c
@@ -151,6 +151,8 @@ static void crypto_client_setup(CryptoClientState *cc,
     }
     QTAILQ_INSERT_TAIL(&crypto_clients, cc, next);
     cc->destructor = destructor;
+    cc->incoming_queue =
+                qemu_new_crypto_queue(qemu_deliver_crypto_packet, cc);
 }
 
 CryptoClientState *new_crypto_client(CryptoClientInfo *info,
@@ -169,3 +171,29 @@ CryptoClientState *new_crypto_client(CryptoClientInfo *info,
 
     return cc;
 }
+
+int qemu_deliver_crypto_packet(CryptoClientState *sender,
+                              unsigned flags,
+                              void *header_opqaue,
+                              void *opaque)
+{
+    return 0;
+}
+
+int qemu_send_crypto_packet_async(CryptoClientState *sender,
+                                unsigned flags,
+                                void *opaque,
+                                CryptoPacketSent *sent_cb)
+{
+    CryptoQueue *queue;
+
+    if (!sender->ready) {
+        /* we assume that all packets are sent */
+        return 1;
+    }
+
+    queue = sender->peer->incoming_queue;
+
+    return qemu_crypto_queue_send(queue, flags, sender,
+                                  opaque, sent_cb);
+}
diff --git a/include/crypto/crypto-queue.h b/include/crypto/crypto-queue.h
new file mode 100644
index 0000000..6fba64d
--- /dev/null
+++ b/include/crypto/crypto-queue.h
@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *    Gonglei <arei.gonglei@huawei.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef QEMU_CRYPTO_QUEUE_H
+#define QEMU_CRYPTO_QUEUE_H
+
+#include "qemu-common.h"
+
+typedef struct CryptoPacket CryptoPacket;
+typedef struct CryptoQueue CryptoQueue;
+typedef struct CryptoPacketBuf CryptoPacketBuf;
+
+typedef void (CryptoPacketSent) (CryptoClientState *, int);
+
+
+/* Returns:
+ *   >0 - success
+ *    0 - queue packet for future redelivery
+ *   <0 - failure (discard packet)
+ */
+typedef int (CryptoQueueDeliverFunc)(CryptoClientState *sender,
+                                     unsigned flags,
+                                     void *header_opaque,
+                                     void *opaque);
+
+CryptoQueue *
+qemu_new_crypto_queue(CryptoQueueDeliverFunc *deliver, void *opaque);
+
+void qemu_crypto_queue_cache(CryptoQueue *queue,
+                               unsigned flags,
+                               CryptoClientState *sender,
+                               void *opaque,
+                               CryptoPacketSent *sent_cb);
+
+void qemu_del_crypto_queue(CryptoQueue *queue);
+
+int qemu_crypto_queue_send(CryptoQueue *queue,
+                                unsigned flags,
+                                CryptoClientState *sender,
+                                void *opaque,
+                                CryptoPacketSent *sent_cb);
+
+void qemu_crypto_queue_purge(CryptoQueue *queue, CryptoClientState *from);
+bool qemu_crypto_queue_flush(CryptoQueue *queue);
+
+#endif /* QEMU_CRYPTO_QUEUE_H */
diff --git a/include/crypto/crypto.h b/include/crypto/crypto.h
index f93f6f9..46b3b9e 100644
--- a/include/crypto/crypto.h
+++ b/include/crypto/crypto.h
@@ -29,6 +29,8 @@
 
 #include "qemu/queue.h"
 #include "qapi-types.h"
+#include "crypto/crypto-queue.h"
+
 
 typedef void (CryptoPoll)(CryptoClientState *, bool);
 typedef void (CryptoCleanup) (CryptoClientState *);
@@ -52,6 +54,8 @@ struct CryptoClientState {
     char *model;
     char *name;
     char info_str[256];
+    CryptoQueue *incoming_queue;
+    unsigned int queue_index;
     CryptoClientDestructor *destructor;
 };
 
@@ -62,5 +66,13 @@ CryptoClientState *new_crypto_client(CryptoClientInfo *info,
                                     CryptoClientState *peer,
                                     const char *model,
                                     const char *name);
+int qemu_deliver_crypto_packet(CryptoClientState *sender,
+                              unsigned flags,
+                              void *header_opqaue,
+                              void *opaque);
+int qemu_send_crypto_packet_async(CryptoClientState *sender,
+                                unsigned flags,
+                                void *opaque,
+                                CryptoPacketSent *sent_cb);
 
 #endif /* QCRYPTO_CRYPTO_H__ */
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [Qemu-devel] [PATCH v2 03/15] crypto: add cryptoLegacyHW stuff
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto legacy hardware Gonglei
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 02/15] crypto: introduce crypto queue handler Gonglei
@ 2016-09-13  3:52 ` Gonglei
  2016-09-13  9:22   ` Daniel P. Berrange
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 04/15] crypto: add symetric algorithms support Gonglei
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

In previous patch, we define CryptoLegacyHWOptions in
qapi-schema.json. we introduce the new/delete funciton
about crypto legacy hardware device.

Note: legacy cryptographic device support muliple queue.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 crypto/crypto.c         | 74 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/crypto/crypto.h | 29 +++++++++++++++++++
 2 files changed, 103 insertions(+)

diff --git a/crypto/crypto.c b/crypto/crypto.c
index a0e4a34..3f760fd 100644
--- a/crypto/crypto.c
+++ b/crypto/crypto.c
@@ -197,3 +197,77 @@ int qemu_send_crypto_packet_async(CryptoClientState *sender,
     return qemu_crypto_queue_send(queue, flags, sender,
                                   opaque, sent_cb);
 }
+
+CryptoLegacyHWState *
+qemu_new_crypto_legacy_hw(CryptoClientInfo *info,
+                           CryptoLegacyHWConf *conf,
+                           const char *model,
+                           const char *name,
+                           void *opaque)
+{
+    CryptoLegacyHWState *crypto;
+    CryptoClientState **peers = conf->peers.ccs;
+    int i, queues = MAX(1, conf->peers.queues);
+
+    assert(info->type == CRYPTO_CLIENT_OPTIONS_KIND_LEGACY_HW);
+    assert(info->size >= sizeof(CryptoLegacyHWState));
+
+    crypto = g_malloc0(info->size + sizeof(CryptoClientState) * queues);
+    crypto->ccs = (void *)crypto + info->size;
+    crypto->opaque = opaque;
+    crypto->conf = conf;
+
+    for (i = 0; i < queues; i++) {
+        crypto_client_setup(&crypto->ccs[i], info, peers[i], model, name,
+                              NULL);
+        crypto->ccs[i].queue_index = i;
+        crypto->ccs[i].ready = true;
+    }
+
+    return crypto;
+}
+
+static void qemu_cleanup_crypto_client(CryptoClientState *cc)
+{
+    QTAILQ_REMOVE(&crypto_clients, cc, next);
+
+    if (cc->info->cleanup) {
+        cc->info->cleanup(cc);
+    }
+}
+
+static void qemu_free_crypto_client(CryptoClientState *cc)
+{
+    if (cc->incoming_queue) {
+        qemu_del_crypto_queue(cc->incoming_queue);
+    }
+    if (cc->peer) {
+        cc->peer->peer = NULL;
+    }
+    g_free(cc->model);
+    g_free(cc->name);
+
+    if (cc->destructor) {
+        cc->destructor(cc);
+    }
+}
+
+CryptoClientState *
+qemu_get_crypto_subqueue(CryptoLegacyHWState *crypto, int queue_index)
+{
+    return crypto->ccs + queue_index;
+}
+
+void qemu_del_crypto_legacy_hw(CryptoLegacyHWState *crypto)
+{
+    int i, queues = MAX(crypto->conf->peers.queues, 1);
+
+    for (i = queues - 1; i >= 0; i--) {
+        CryptoClientState *cc = qemu_get_crypto_subqueue(crypto, i);
+
+        qemu_cleanup_crypto_client(cc);
+        qemu_free_crypto_client(cc);
+    }
+
+    g_free(crypto);
+}
diff --git a/include/crypto/crypto.h b/include/crypto/crypto.h
index 46b3b9e..4f0efb7 100644
--- a/include/crypto/crypto.h
+++ b/include/crypto/crypto.h
@@ -31,6 +31,7 @@
 #include "qapi-types.h"
 #include "crypto/crypto-queue.h"
 
+#define MAX_CRYPTO_QUEUE_NUM  64
 
 typedef void (CryptoPoll)(CryptoClientState *, bool);
 typedef void (CryptoCleanup) (CryptoClientState *);
@@ -59,6 +60,24 @@ struct CryptoClientState {
     CryptoClientDestructor *destructor;
 };
 
+/* qdev crypto legacy hardware properties */
+
+typedef struct CryptoLegacyHWPeers {
+    CryptoClientState *ccs[MAX_CRYPTO_QUEUE_NUM];
+    int32_t queues;
+} CryptoLegacyHWPeers;
+
+typedef struct CryptoLegacyHWConf {
+    CryptoLegacyHWPeers peers;
+} CryptoLegacyHWConf;
+
+typedef struct CryptoLegacyHWState {
+    CryptoClientState *ccs;
+    void *opaque;
+    CryptoLegacyHWConf *conf;
+    bool peer_deleted;
+} CryptoLegacyHWState;
+
 int crypto_client_init(QemuOpts *opts, Error **errp);
 int crypto_init_clients(void);
 
@@ -74,5 +93,15 @@ int qemu_send_crypto_packet_async(CryptoClientState *sender,
                                 unsigned flags,
                                 void *opaque,
                                 CryptoPacketSent *sent_cb);
+CryptoLegacyHWState *
+qemu_new_crypto_legacy_hw(CryptoClientInfo *info,
+                           CryptoLegacyHWConf *conf,
+                           const char *model,
+                           const char *name,
+                           void *opaque);
+void qemu_del_crypto_legacy_hw(CryptoLegacyHWState *crypto);
+
+CryptoClientState *
+qemu_get_crypto_subqueue(CryptoLegacyHWState *crypto, int queue_index);
 
 #endif /* QCRYPTO_CRYPTO_H__ */
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [Qemu-devel] [PATCH v2 04/15] crypto: add symetric algorithms support
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
                   ` (2 preceding siblings ...)
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 03/15] crypto: add cryptoLegacyHW stuff Gonglei
@ 2016-09-13  3:52 ` Gonglei
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 05/15] crypto: add cryptodev-linux as a cryptodev backend Gonglei
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

This patch include three parts, the first is define the
session structure and the second is the opertion structure,
whose properties are needed to finish the symetric algorithms.
The third part defines some function pointers.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 crypto/crypto.c         | 16 +++++++++-
 include/crypto/crypto.h | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/crypto/crypto.c b/crypto/crypto.c
index 3f760fd..958a959 100644
--- a/crypto/crypto.c
+++ b/crypto/crypto.c
@@ -177,7 +177,21 @@ int qemu_deliver_crypto_packet(CryptoClientState *sender,
                               void *header_opqaue,
                               void *opaque)
 {
-    return 0;
+    CryptoClientState *cc = opaque;
+    int ret = -1;
+
+    if (!cc->ready) {
+        return 1;
+    }
+
+    if (flags == QEMU_CRYPTO_PACKET_FLAG_SYM) {
+        CryptoSymOpInfo *op_info = header_opqaue;
+        if (cc->info->do_sym_op) {
+            ret = cc->info->do_sym_op(cc, op_info);
+        }
+    }
+
+    return ret;
 }
 
 int qemu_send_crypto_packet_async(CryptoClientState *sender,
diff --git a/include/crypto/crypto.h b/include/crypto/crypto.h
index 4f0efb7..95cca23 100644
--- a/include/crypto/crypto.h
+++ b/include/crypto/crypto.h
@@ -33,10 +33,50 @@
 
 #define MAX_CRYPTO_QUEUE_NUM  64
 
+#define  QEMU_CRYPTO_PACKET_FLAG_NONE (0)
+#define  QEMU_CRYPTO_PACKET_FLAG_SYM (1 << 0)
+
+typedef struct CryptoSymSessionInfo {
+    uint8_t op_code;
+    uint8_t op_type;
+    uint8_t direction;
+    uint32_t cipher_alg;
+    uint32_t key_len;
+    uint8_t *cipher_key;
+
+    uint32_t hash_alg;
+    uint8_t hash_mode;
+    uint32_t hash_result_len;
+    uint8_t alg_chain_order;
+    uint32_t auth_key_len;
+    uint32_t add_len;
+    uint8_t *auth_key;
+} CryptoSymSessionInfo;
+
+typedef struct CryptoSymOpInfo {
+    uint64_t session_id;
+    uint8_t op_type; /* cipher or algo chainning */
+    uint8_t *src;
+    uint8_t *dst;
+    uint8_t *iv;
+    uint8_t *aad_data; /* additional auth data */
+    uint32_t aad_len;
+    uint32_t iv_len;
+    uint32_t src_len;
+    /* the dst_len is equal to src_len + hash_result_len
+     * if hash alg configured */
+    uint32_t dst_len;
+    uint8_t data[0];
+} CryptoSymOpInfo;
+
 typedef void (CryptoPoll)(CryptoClientState *, bool);
 typedef void (CryptoCleanup) (CryptoClientState *);
 typedef void (CryptoClientDestructor)(CryptoClientState *);
 typedef void (CryptoHWStatusChanged)(CryptoClientState *);
+typedef int (CryptoCreateSymSession)(CryptoClientState *,
+                              CryptoSymSessionInfo *, uint64_t *);
+typedef int (CryptoCloseSession)(CryptoClientState *, uint64_t);
+typedef int (CryptoDoSymOp)(CryptoClientState *, CryptoSymOpInfo *);
 
 typedef struct CryptoClientInfo {
     CryptoClientOptionsKind type;
@@ -45,6 +85,9 @@ typedef struct CryptoClientInfo {
     CryptoCleanup *cleanup;
     CryptoPoll *poll;
     CryptoHWStatusChanged *hw_status_changed;
+    CryptoCreateSymSession *create_session;
+    CryptoCloseSession *close_session;
+    CryptoDoSymOp *do_sym_op;
 } CryptoClientInfo;
 
 struct CryptoClientState {
@@ -57,6 +100,21 @@ struct CryptoClientState {
     char info_str[256];
     CryptoQueue *incoming_queue;
     unsigned int queue_index;
+
+    /* Supported service mask */
+    uint32_t crypto_services;
+
+    /* Detailed algorithms mask */
+    uint32_t cipher_algo_l;
+    uint32_t cipher_algo_h;
+    uint32_t hash_algo;
+    uint32_t mac_algo_l;
+    uint32_t mac_algo_h;
+    uint32_t asym_algo;
+    uint32_t kdf_algo;
+    uint32_t aead_algo;
+    uint32_t primitive_algo;
+
     CryptoClientDestructor *destructor;
 };
 
@@ -69,6 +127,20 @@ typedef struct CryptoLegacyHWPeers {
 
 typedef struct CryptoLegacyHWConf {
     CryptoLegacyHWPeers peers;
+
+    /* Supported service mask */
+    uint32_t crypto_services;
+
+    /* Detailed algorithms mask */
+    uint32_t cipher_algo_l;
+    uint32_t cipher_algo_h;
+    uint32_t hash_algo;
+    uint32_t mac_algo_l;
+    uint32_t mac_algo_h;
+    uint32_t asym_algo;
+    uint32_t kdf_algo;
+    uint32_t aead_algo;
+    uint32_t primitive_algo;
 } CryptoLegacyHWConf;
 
 typedef struct CryptoLegacyHWState {
@@ -104,4 +176,17 @@ void qemu_del_crypto_legacy_hw(CryptoLegacyHWState *crypto);
 CryptoClientState *
 qemu_get_crypto_subqueue(CryptoLegacyHWState *crypto, int queue_index);
 
+CryptoLegacyHWState *qemu_get_crypto_legacy_hw(CryptoClientState *cc);
+
+void *qemu_get_crypto_legacy_hw_opaque(CryptoClientState *cc);
+
+int qemu_find_crypto_clients_except(const char *id, CryptoClientState **ccs,
+                                 CryptoClientOptionsKind type, int max);
+
+int qemu_crypto_create_session(CryptoClientState *cc,
+                               CryptoSymSessionInfo *info,
+                               uint64_t *session_id);
+int qemu_crypto_close_session(CryptoClientState *cc,
+                               uint64_t session_id);
+
 #endif /* QCRYPTO_CRYPTO_H__ */
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [Qemu-devel] [PATCH v2 05/15] crypto: add cryptodev-linux as a cryptodev backend
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
                   ` (3 preceding siblings ...)
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 04/15] crypto: add symetric algorithms support Gonglei
@ 2016-09-13  3:52 ` Gonglei
  2016-09-13  9:23   ` Daniel P. Berrange
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 06/15] crypto: add internal handle logic layer Gonglei
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

Cryptodev-linux is a device that allows access to Linux
kernel cryptographic drivers; thus allowing of userspace
applications to take advantage of hardware accelerators.
Cryptodev-linux is implemented as a standalone module
that requires no dependencies other than a stock linux kernel.

The Cryptodev-linux project website is:
  http://cryptodev-linux.org/

Meanwile, I introdue the virtio_crypto.h which follows
virtio-crypto specification bacause cryptodev-linux.c include it.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 configure                                      |  16 +
 crypto/Makefile.objs                           |   1 +
 crypto/crypto.c                                |   8 +-
 crypto/cryptodev-linux.c                       | 419 +++++++++++++++++++++++
 include/crypto/crypto-clients.h                |  39 +++
 include/standard-headers/linux/virtio_crypto.h | 448 +++++++++++++++++++++++++
 qapi-schema.json                               |  17 +-
 qemu-options.hx                                |  18 +-
 8 files changed, 963 insertions(+), 3 deletions(-)
 create mode 100644 crypto/cryptodev-linux.c
 create mode 100644 include/crypto/crypto-clients.h
 create mode 100644 include/standard-headers/linux/virtio_crypto.h

diff --git a/configure b/configure
index 331c36f..5982ff8 100755
--- a/configure
+++ b/configure
@@ -321,6 +321,7 @@ vhdx=""
 numa=""
 tcmalloc="no"
 jemalloc="no"
+cryptodev_linux="no"
 
 # parse CC options first
 for opt do
@@ -3622,6 +3623,16 @@ EOF
 fi
 
 ##########################################
+# cryptodev-linux header probe
+cat > $TMPC << EOF
+#include <crypto/cryptodev.h>
+int main(void) { return 0; }
+EOF
+if compile_prog "" "" ; then
+    cryptodev_linux="yes"
+fi
+
+##########################################
 # signalfd probe
 signalfd="no"
 cat > $TMPC << EOF
@@ -4927,6 +4938,7 @@ echo "NUMA host support $numa"
 echo "tcmalloc support  $tcmalloc"
 echo "jemalloc support  $jemalloc"
 echo "avx2 optimization $avx2_opt"
+echo "cryptodev-linux support $cryptodev_linux"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -5331,6 +5343,10 @@ if test "$avx2_opt" = "yes" ; then
   echo "CONFIG_AVX2_OPT=y" >> $config_host_mak
 fi
 
+if test "$cryptodev_linux" = "yes" ; then
+  echo "CONFIG_CRYPTODEV_LINUX=y" >> $config_host_mak
+fi
+
 if test "$lzo" = "yes" ; then
   echo "CONFIG_LZO=y" >> $config_host_mak
 fi
diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 652b429..1f2ec24 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -28,6 +28,7 @@ crypto-obj-y += block-qcow.o
 crypto-obj-y += block-luks.o
 crypto-obj-y += crypto.o
 crypto-obj-y += crypto-queue.o
+crypto-obj-$(CONFIG_CRYPTODEV_LINUX) += cryptodev-linux.o
 
 # Let the userspace emulators avoid linking gnutls/etc
 crypto-aes-obj-y = aes.o
diff --git a/crypto/crypto.c b/crypto/crypto.c
index 958a959..1d3d1d3 100644
--- a/crypto/crypto.c
+++ b/crypto/crypto.c
@@ -34,6 +34,7 @@
 #include "crypto/crypto.h"
 #include "qemu/config-file.h"
 #include "monitor/monitor.h"
+#include "crypto/crypto-clients.h"
 
 
 static QTAILQ_HEAD(, CryptoClientState) crypto_clients;
@@ -81,7 +82,12 @@ int crypto_init_clients(void)
 static int (* const crypto_client_init_fun[CRYPTO_CLIENT_OPTIONS_KIND__MAX])(
     const CryptoClientOptions *opts,
     const char *name,
-    CryptoClientState *peer, Error **errp);
+    CryptoClientState *peer, Error **errp) = {
+#ifdef CONFIG_CRYPTODEV_LINUX
+        [CRYPTO_CLIENT_OPTIONS_KIND_CRYPTODEV_LINUX] =
+                                             crypto_init_cryptodev_linux,
+#endif
+};
 
 static int crypto_client_init1(const void *object, Error **errp)
 {
diff --git a/crypto/cryptodev-linux.c b/crypto/cryptodev-linux.c
new file mode 100644
index 0000000..ae2dcdb
--- /dev/null
+++ b/crypto/cryptodev-linux.c
@@ -0,0 +1,419 @@
+/*
+ * cryptodev-linux backend support
+ *
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *    Gonglei <arei.gonglei@huawei.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include <sys/ioctl.h>
+#include <fcntl.h>
+#include <crypto/cryptodev.h>
+
+#include "qemu/osdep.h"
+#include "sysemu/sysemu.h"
+#include "qapi/error.h"
+#include "qemu/iov.h"
+#include "qapi-visit.h"
+#include "qapi/opts-visitor.h"
+#include "qemu/error-report.h"
+
+#include "crypto/crypto.h"
+#include "crypto/crypto-clients.h"
+#include "standard-headers/linux/virtio_crypto.h"
+
+
+#define CRYPTO_CHARDEV_PATH "/dev/crypto"
+
+
+typedef struct CryptodevLinuxSession {
+    struct session_op *sess;
+    uint8_t direction; /* encryption or decryption */
+    uint8_t type; /* cipher? hash? aead? */
+    QTAILQ_ENTRY(CryptodevLinuxSession) next;
+} CryptodevLinuxSession;
+
+typedef struct CryptodevLinuxState {
+    CryptoClientState cc;
+    int fd;
+    bool read_poll;
+    bool write_poll;
+    bool enabled;
+    QTAILQ_HEAD(, CryptodevLinuxSession) sessions;
+} CryptodevLinuxState;
+
+static int
+cryptodev_linux_open(int *gfd, Error **errp)
+{
+    int fd = -1;
+
+    /* Open the crypto device */
+    fd = open(CRYPTO_CHARDEV_PATH, O_RDWR, 0);
+    if (fd < 0) {
+        error_setg_errno(errp, errno, "Cannot open %s",
+                         CRYPTO_CHARDEV_PATH);
+        return -1;
+    }
+
+    /* Set close-on-exec (not really neede here) */
+    if (fcntl(fd, F_SETFD, 1) == -1) {
+        perror("fcntl(F_SETFD)");
+        error_setg_errno(errp, errno,
+                        "Failed to fcntl(F_SETFD) %s",
+                         CRYPTO_CHARDEV_PATH);
+        close(fd);
+        return -1;
+    }
+
+    *gfd = fd;
+
+    return 0;
+}
+
+static int
+cryptodev_linux_handle_cipher_sess(CryptodevLinuxState *s,
+                              CryptoSymSessionInfo *session_info,
+                              struct session_op *sess,
+                              uint8_t *direction)
+{
+    switch (session_info->cipher_alg) {
+    case VIRTIO_CRYPTO_CIPHER_AES_CBC:
+        sess->cipher = CRYPTO_AES_CBC;
+        break;
+    default:
+        error_report("Unsupported cipher alg: %u",
+                     session_info->cipher_alg);
+        return -1;
+    }
+    /* Get crypto session for assinged algorithm */
+    sess->keylen = session_info->key_len;
+    sess->key = session_info->cipher_key;
+    *direction = session_info->direction;
+    return 0;
+}
+
+static int
+cryptodev_linux_handle_hash_sess(CryptodevLinuxState *s,
+                              CryptoSymSessionInfo *session_info,
+                              struct session_op *sess)
+{
+    switch (session_info->hash_alg) {
+    case VIRTIO_CRYPTO_HASH_SHA1:
+        sess->mac = CRYPTO_SHA1_HMAC;
+        break;
+    default:
+        error_report("Unsupported hash alg: %u",
+                     session_info->hash_alg);
+        return -1;
+    }
+
+    sess->mackeylen = session_info->auth_key_len;
+    sess->mackey = session_info->auth_key;
+    return 0;
+}
+
+static int
+cryptodev_linux_handle_chaining_sess(CryptodevLinuxState *s,
+                              CryptoSymSessionInfo *session_info,
+                              struct session_op *sess,
+                              uint8_t *direction)
+{
+    int ret;
+
+    ret = cryptodev_linux_handle_cipher_sess(s, session_info,
+                                             sess, direction);
+    if (ret == 0) {
+        ret = cryptodev_linux_handle_hash_sess(s,
+                                         session_info, sess);
+    }
+    return ret;
+}
+
+static int
+cryptodev_linux_create_session(CryptoClientState *cc,
+                               CryptoSymSessionInfo *session_info,
+                               uint64_t *session_id)
+{
+    CryptodevLinuxState *s = DO_UPCAST(CryptodevLinuxState, cc, cc);
+    int fd = s->fd;
+    int ret = -1;
+    CryptodevLinuxSession *session = NULL;
+    uint8_t direction = 0;
+    struct session_op *sess;
+#ifdef CIOCGSESSINFO
+    struct session_info_op siop;
+#endif
+
+    sess = g_new0(struct session_op, 1);
+    /* Setup session Parameters */
+    switch (session_info->op_type) {
+    case VIRTIO_CRYPTO_SYM_OP_CIPHER:
+        ret = cryptodev_linux_handle_cipher_sess(s, session_info,
+                                               sess, &direction);
+        if (ret < 0) {
+            goto err;
+        }
+        break;
+    case VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING:
+        ret = cryptodev_linux_handle_chaining_sess(s, session_info,
+                                          sess, &direction);
+        if (ret < 0) {
+            goto err;
+        }
+        break;
+    default:
+        error_report("Unsupported type: %u", session_info->op_type);
+        goto err;
+    }
+
+    if (ioctl(fd, CIOCGSESSION, sess)) {
+        perror("ioctl(CIOCGSESSION)");
+        ret = -1;
+        goto err;
+    }
+
+#ifdef CIOCGSESSINFO
+    siop.ses = sess->ses;
+    if (ioctl(fd, CIOCGSESSINFO, &siop)) {
+        perror("ioctl(CIOCGSESSINFO)");
+        ret = -1;
+        goto err;
+    }
+    printf("got %s with driver %s\n",
+          siop.cipher_info.cra_name, siop.cipher_info.cra_driver_name);
+#endif
+
+    *session_id = sess->ses;
+
+    session = g_new0(CryptodevLinuxSession, 1);
+    session->sess = sess;
+    session->type = session_info->op_type;
+    switch (direction) {
+    case VIRTIO_CRYPTO_OP_ENCRYPT:
+        session->direction = COP_ENCRYPT;
+        break;
+    case VIRTIO_CRYPTO_OP_DECRYPT:
+        session->direction = COP_DECRYPT;
+        break;
+    default:
+        error_report("Unsupported direction: %u", direction);
+        goto err;
+    }
+
+    QTAILQ_INSERT_TAIL(&s->sessions, session, next);
+
+    return 0;
+err:
+    g_free(sess);
+    g_free(session);
+    return ret;
+}
+
+static CryptodevLinuxSession *
+cryptodev_linux_find_session(CryptodevLinuxState *s,
+                             uint64_t session_id)
+{
+    CryptodevLinuxSession *session, *tmp;
+
+    QTAILQ_FOREACH_SAFE(session, &s->sessions, next, tmp) {
+        if (session->sess->ses == session_id) {
+            return session;
+        }
+    }
+
+    return NULL;
+}
+
+static int
+cryptodev_linux_close_session(CryptoClientState *cc,
+                               uint64_t session_id)
+{
+    CryptodevLinuxState *s = DO_UPCAST(CryptodevLinuxState, cc, cc);
+    int fd = s->fd;
+    CryptodevLinuxSession *session;
+
+    session = cryptodev_linux_find_session(s, session_id);
+    if (session == NULL) {
+        error_report("Cannot find the session: %" PRIu64 "",
+                      session_id);
+        return -1;
+    }
+
+    if (ioctl(fd, CIOCFSESSION, &session_id)) {
+        perror("ioctl(CIOCFSESSION)");
+        return -1;
+    }
+
+    QTAILQ_REMOVE(&s->sessions, session, next);
+    g_free(session->sess);
+    g_free(session);
+    return 0;
+}
+
+static int
+cryptodev_linux_handle_cipher_op(CryptoSymOpInfo *op_info,
+                                   CryptodevLinuxSession *session,
+                                   int fd)
+{
+    struct crypt_op cryp;
+
+    cryp.ses = op_info->session_id;
+    cryp.len = op_info->src_len;
+    cryp.src = op_info->src;
+    cryp.dst = op_info->dst;
+    cryp.iv = op_info->iv;
+    cryp.op = session->direction;
+
+    if (ioctl(fd, CIOCCRYPT, &cryp)) {
+        perror("ioctl(CIOCCRYPT)");
+        return -1;
+    }
+
+    return 1;
+}
+
+static int
+cryptodev_linux_handle_chaining_op(CryptoSymOpInfo *op_info,
+                                   CryptodevLinuxSession *session,
+                                   int fd)
+{
+    struct crypt_auth_op cao;
+
+    cao.ses = op_info->session_id;
+    cao.len = op_info->src_len;
+    cao.src = op_info->src;
+    cao.dst = op_info->dst;
+    cao.iv = op_info->iv;
+    cao.op = session->direction;
+
+    if (op_info->aad_len > 0) {
+        cao.auth_len = op_info->aad_len;
+        cao.auth_src = op_info->aad_data;
+    }
+
+    /* We only support TLS mode at present, the hash result is
+       stored at the end of cipher text, the frontend driver
+       should allocate enough memory. */
+    cao.flags = COP_FLAG_AEAD_TLS_TYPE;
+
+    if (ioctl(fd, CIOCAUTHCRYPT, &cao)) {
+        perror("ioctl(CIOCCRYPT)");
+        return -1;
+    }
+
+    return 1;
+}
+
+
+static int
+cryptodev_linux_do_sym_op(CryptoClientState *cc,
+                              CryptoSymOpInfo *op_info)
+{
+    CryptodevLinuxState *s = DO_UPCAST(CryptodevLinuxState, cc, cc);
+    CryptodevLinuxSession *session;
+
+    session = cryptodev_linux_find_session(s, op_info->session_id);
+    if (session == NULL) {
+        error_report("Cannot find the session: %" PRIu64 "",
+                      op_info->session_id);
+        return -VIRTIO_CRYPTO_OP_INVSESS;
+    }
+
+    switch (session->type) {
+    case VIRTIO_CRYPTO_SYM_OP_CIPHER:
+        return cryptodev_linux_handle_cipher_op(op_info, session, s->fd);
+    case VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING:
+        return cryptodev_linux_handle_chaining_op(op_info, session, s->fd);
+    default:
+        error_report("Unsupported type: %u", session->type);
+        return -1;
+    }
+}
+
+static void
+cryptodev_linux_cleanup(CryptoClientState *cc)
+{
+    CryptodevLinuxState *s = DO_UPCAST(CryptodevLinuxState, cc, cc);
+    CryptodevLinuxSession *session;
+    uint64_t session_id;
+
+    QTAILQ_FOREACH(session, &s->sessions, next) {
+        session_id = session->sess->ses;
+        if (ioctl(s->fd, CIOCFSESSION, &session_id)) {
+            perror("ioctl(CIOCFSESSION)");
+        }
+        g_free(session->sess);
+        g_free(session);
+    }
+
+    close(s->fd);
+    s->fd = -1;
+    s->enabled = false;
+}
+
+static void
+cryptodev_linux_poll(CryptoClientState *cc, bool enable)
+{
+
+}
+
+static CryptoClientInfo crypto_cryptodev_info = {
+    .type = CRYPTO_CLIENT_OPTIONS_KIND_CRYPTODEV_LINUX,
+    .size = sizeof(CryptodevLinuxState),
+    .create_session = cryptodev_linux_create_session,
+    .close_session = cryptodev_linux_close_session,
+    .do_sym_op = cryptodev_linux_do_sym_op,
+    .cleanup = cryptodev_linux_cleanup,
+    .poll = cryptodev_linux_poll,
+};
+
+int crypto_init_cryptodev_linux(const CryptoClientOptions *opts,
+                                const char *name,
+                                CryptoClientState *peer, Error **errp)
+{
+    const CryptodevLinuxOptions *cryptodev;
+    int fd, ret;
+    CryptoClientState *cc;
+    CryptodevLinuxState *s;
+
+    assert(opts->type == CRYPTO_CLIENT_OPTIONS_KIND_CRYPTODEV_LINUX);
+
+    cryptodev = opts->u.cryptodev_linux.data;
+    if (cryptodev->has_fd) {
+        if (cryptodev->fd < 0) {
+            error_setg(errp, "Invaild fd: %" PRId64 "", cryptodev->fd);
+            return -1;
+        } else {
+            fd = cryptodev->fd;
+        }
+    } else {
+        ret = cryptodev_linux_open(&fd, errp);
+        if (ret < 0) {
+            return -1;
+        }
+    }
+
+    cc = new_crypto_client(&crypto_cryptodev_info, peer,
+                           "cryptodev-linux", name);
+
+    cc->crypto_services = 1u << VIRTIO_CRYPTO_SERVICE_CIPHER |
+                         1u << VIRTIO_CRYPTO_SERVICE_HASH |
+                         1u << VIRTIO_CRYPTO_SERVICE_AEAD;
+    cc->cipher_algo_l = 1u << VIRTIO_CRYPTO_CIPHER_AES_CBC;
+    cc->hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1;
+
+    /* the cryptodev backend is ready for work */
+    cc->ready = true;
+    s = DO_UPCAST(CryptodevLinuxState, cc, cc);
+
+    s->fd = fd;
+    s->enabled = true;
+    QTAILQ_INIT(&s->sessions);
+
+    return 0;
+}
diff --git a/include/crypto/crypto-clients.h b/include/crypto/crypto-clients.h
new file mode 100644
index 0000000..dea5748
--- /dev/null
+++ b/include/crypto/crypto-clients.h
@@ -0,0 +1,39 @@
+/*
+ * Crypto clients header
+ *
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *    Gonglei <arei.gonglei@huawei.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#ifndef QEMU_CRYPTO_CLIENTS_H
+#define QEMU_CRYPTO_CLIENTS_H
+
+#include "crypto/crypto.h"
+#include "qapi-types.h"
+
+#ifdef CONFIG_CRYPTODEV_LINUX
+int crypto_init_cryptodev_linux(const CryptoClientOptions *opts,
+                                const char *name,
+                                CryptoClientState *peer, Error **errp);
+#endif
+
+#endif /* QEMU_CRYPTO_CLIENTS_H */
diff --git a/include/standard-headers/linux/virtio_crypto.h b/include/standard-headers/linux/virtio_crypto.h
new file mode 100644
index 0000000..9de5bdb
--- /dev/null
+++ b/include/standard-headers/linux/virtio_crypto.h
@@ -0,0 +1,448 @@
+#ifndef _VIRTIO_CRYPTO_H
+#define _VIRTIO_CRYPTO_H
+
+#include "standard-headers/linux/types.h"
+#include "standard-headers/linux/virtio_config.h"
+#include "standard-headers/linux/virtio_types.h"
+
+
+#define VIRTIO_CRYPTO_SERVICE_CIPHER (0)
+#define VIRTIO_CRYPTO_SERVICE_HASH (1)
+#define VIRTIO_CRYPTO_SERVICE_MAC  (2)
+#define VIRTIO_CRYPTO_SERVICE_AEAD (3)
+
+#define VIRTIO_CRYPTO_OPCODE(service, op)   ((service << 8) | (op))
+
+struct virtio_crypto_ctrl_header {
+#define VIRTIO_CRYPTO_CIPHER_CREATE_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x02)
+#define VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x03)
+#define VIRTIO_CRYPTO_HASH_CREATE_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x02)
+#define VIRTIO_CRYPTO_HASH_DESTROY_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x03)
+#define VIRTIO_CRYPTO_MAC_CREATE_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x02)
+#define VIRTIO_CRYPTO_MAC_DESTROY_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x03)
+#define VIRTIO_CRYPTO_AEAD_CREATE_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x02)
+#define VIRTIO_CRYPTO_AEAD_DESTROY_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x03)
+    __virtio32 opcode;
+    __virtio32 algo;
+    __virtio32 flag;
+    /* data virtqueue id */
+    __virtio32 queue_id;
+};
+
+struct virtio_crypto_cipher_session_para {
+#define VIRTIO_CRYPTO_NO_CIPHER                 0
+#define VIRTIO_CRYPTO_CIPHER_ARC4               1
+#define VIRTIO_CRYPTO_CIPHER_AES_ECB            2
+#define VIRTIO_CRYPTO_CIPHER_AES_CBC            3
+#define VIRTIO_CRYPTO_CIPHER_AES_CTR            4
+#define VIRTIO_CRYPTO_CIPHER_DES_ECB            5
+#define VIRTIO_CRYPTO_CIPHER_DES_CBC            6
+#define VIRTIO_CRYPTO_CIPHER_3DES_ECB           7
+#define VIRTIO_CRYPTO_CIPHER_3DES_CBC           8
+#define VIRTIO_CRYPTO_CIPHER_3DES_CTR           9
+#define VIRTIO_CRYPTO_CIPHER_KASUMI_F8          10
+#define VIRTIO_CRYPTO_CIPHER_SNOW3G_UEA2        11
+#define VIRTIO_CRYPTO_CIPHER_AES_F8             12
+#define VIRTIO_CRYPTO_CIPHER_AES_XTS            13
+#define VIRTIO_CRYPTO_CIPHER_ZUC_EEA3           14
+    __virtio32 algo;
+    /* length of key */
+    __virtio32 keylen;
+
+#define VIRTIO_CRYPTO_OP_ENCRYPT  1
+#define VIRTIO_CRYPTO_OP_DECRYPT  2
+    /* encrypt or decrypt */
+    __virtio32 op;
+    __virtio32 padding;
+};
+
+struct virtio_crypto_session_input {
+    /* Device-writable part */
+    __virtio64 session_id;
+    __virtio32 status;
+    __virtio32 padding;
+};
+
+struct virtio_crypto_cipher_session_output {
+    __virtio64 key_addr; /* guest key physical address */
+};
+
+struct virtio_crypto_cipher_session_req {
+    struct virtio_crypto_cipher_session_para para;
+    struct virtio_crypto_cipher_session_output out;
+    struct virtio_crypto_session_input input;
+};
+
+struct virtio_crypto_hash_session_para {
+#define VIRTIO_CRYPTO_NO_HASH            0
+#define VIRTIO_CRYPTO_HASH_MD5           1
+#define VIRTIO_CRYPTO_HASH_SHA1          2
+#define VIRTIO_CRYPTO_HASH_SHA_224       3
+#define VIRTIO_CRYPTO_HASH_SHA_256       4
+#define VIRTIO_CRYPTO_HASH_SHA_384       5
+#define VIRTIO_CRYPTO_HASH_SHA_512       6
+#define VIRTIO_CRYPTO_HASH_SHA3_224      7
+#define VIRTIO_CRYPTO_HASH_SHA3_256      8
+#define VIRTIO_CRYPTO_HASH_SHA3_384      9
+#define VIRTIO_CRYPTO_HASH_SHA3_512      10
+#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE128      11
+#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE256      12
+    __virtio32 algo;
+    /* hash result length */
+    __virtio32 hash_result_len;
+};
+
+struct virtio_crypto_hash_create_session_req {
+    struct virtio_crypto_hash_session_para para;
+    struct virtio_crypto_session_input input;
+};
+
+struct virtio_crypto_mac_session_para {
+#define VIRTIO_CRYPTO_NO_MAC                       0
+#define VIRTIO_CRYPTO_MAC_HMAC_MD5                 1
+#define VIRTIO_CRYPTO_MAC_HMAC_SHA1                2
+#define VIRTIO_CRYPTO_MAC_HMAC_SHA_224             3
+#define VIRTIO_CRYPTO_MAC_HMAC_SHA_256             4
+#define VIRTIO_CRYPTO_MAC_HMAC_SHA_384             5
+#define VIRTIO_CRYPTO_MAC_HMAC_SHA_512             6
+#define VIRTIO_CRYPTO_MAC_CMAC_3DES                25
+#define VIRTIO_CRYPTO_MAC_CMAC_AES                 26
+#define VIRTIO_CRYPTO_MAC_KASUMI_F9                27
+#define VIRTIO_CRYPTO_MAC_SNOW3G_UIA2              28
+#define VIRTIO_CRYPTO_MAC_GMAC_AES                 41
+#define VIRTIO_CRYPTO_MAC_GMAC_TWOFISH             42
+#define VIRTIO_CRYPTO_MAC_CBCMAC_AES               49
+#define VIRTIO_CRYPTO_MAC_CBCMAC_KASUMI_F9         50
+#define VIRTIO_CRYPTO_MAC_XCBC_AES                 53
+    __virtio32 algo;
+    /* hash result length */
+    __virtio32 hash_result_len;
+    /* length of authenticated key */
+    __virtio32 auth_key_len;
+    __virtio32 padding;
+};
+
+struct virtio_crypto_mac_session_output {
+    __virtio64 auth_key_addr; /* guest key physical address */
+};
+
+struct virtio_crypto_mac_create_session_req {
+    struct virtio_crypto_mac_session_para para;
+    struct virtio_crypto_mac_session_output out;
+    struct virtio_crypto_session_input input;
+};
+
+struct virtio_crypto_aead_session_para {
+#define VIRTIO_CRYPTO_NO_AEAD     0
+#define VIRTIO_CRYPTO_AEAD_GCM    1
+#define VIRTIO_CRYPTO_AEAD_CCM    2
+#define VIRTIO_CRYPTO_AEAD_CHACHA20_POLY1305  3
+    __virtio32 algo;
+    /* length of key */
+    __virtio32 key_len;
+    /* digest result length */
+    __virtio32 digest_result_len;
+    /* length of the additional authenticated data (AAD) in bytes */
+    __virtio32 aad_len;
+    /* encrypt or decrypt, See above VIRTIO_CRYPTO_OP_* */
+    __virtio32 op;
+    __virtio32 padding;
+};
+
+struct virtio_crypto_aead_session_output {
+    __virtio64 key_addr; /* guest key physical address */
+};
+
+struct virtio_crypto_aead_create_session_req {
+    struct virtio_crypto_aead_session_para para;
+    struct virtio_crypto_aead_session_output out;
+    struct virtio_crypto_session_input input;
+};
+
+struct virtio_crypto_alg_chain_session_para {
+#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_HASH_THEN_CIPHER  1
+#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_CIPHER_THEN_HASH  2
+    __virtio32 alg_chain_order;
+/* Plain hash */
+#define VIRTIO_CRYPTO_SYM_HASH_MODE_PLAIN    1
+/* Authenticated hash (mac) */
+#define VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH     2
+/* Nested hash */
+#define VIRTIO_CRYPTO_SYM_HASH_MODE_NESTED   3
+    __virtio32 hash_mode;
+    struct virtio_crypto_cipher_session_para cipher_param;
+    union {
+        struct virtio_crypto_hash_session_para hash_param;
+        struct virtio_crypto_mac_session_para mac_param;
+    } u;
+    /* length of the additional authenticated data (AAD) in bytes */
+    __virtio32 aad_len;
+    __virtio32 padding;
+};
+
+struct virtio_crypto_alg_chain_session_output {
+    struct virtio_crypto_cipher_session_output cipher;
+    struct virtio_crypto_mac_session_output mac;
+};
+
+struct virtio_crypto_alg_chain_session_req {
+    struct virtio_crypto_alg_chain_session_para para;
+    struct virtio_crypto_alg_chain_session_output out;
+    struct virtio_crypto_session_input input;
+};
+
+struct virtio_crypto_sym_create_session_req {
+    union {
+        struct virtio_crypto_cipher_session_req cipher;
+        struct virtio_crypto_alg_chain_session_req chain;
+    } u;
+
+    /* Device-readable part */
+
+/* No operation */
+#define VIRTIO_CRYPTO_SYM_OP_NONE  0
+/* Cipher only operation on the data */
+#define VIRTIO_CRYPTO_SYM_OP_CIPHER  1
+/* Chain any cipher with any hash or mac operation. The order
+   depends on the value of alg_chain_order param */
+#define VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING  2
+    __virtio32 op_type;
+    __virtio32 padding;
+};
+
+struct virtio_crypto_destroy_session_req {
+    /* Device-readable part */
+    __virtio64  session_id;
+    /* Device-writable part */
+    __virtio32  status;
+    __virtio32  padding;
+};
+
+/* The request of the control viritqueue's packet */
+struct virtio_crypto_op_ctrl_req {
+    struct virtio_crypto_ctrl_header header;
+
+    union {
+        struct virtio_crypto_sym_create_session_req   sym_create_session;
+        struct virtio_crypto_hash_create_session_req  hash_create_session;
+        struct virtio_crypto_mac_create_session_req   mac_create_session;
+        struct virtio_crypto_aead_create_session_req  aead_create_session;
+        struct virtio_crypto_destroy_session_req      destroy_session;
+    } u;
+};
+
+struct virtio_crypto_op_header {
+#define VIRTIO_CRYPTO_CIPHER_ENCRYPT \
+    VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x00)
+#define VIRTIO_CRYPTO_CIPHER_DECRYPT \
+    VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x01)
+#define VIRTIO_CRYPTO_HASH \
+    VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x00)
+#define VIRTIO_CRYPTO_MAC \
+    VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x00)
+#define VIRTIO_CRYPTO_AEAD_ENCRYPT \
+    VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x00)
+#define VIRTIO_CRYPTO_AEAD_DECRYPT \
+    VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x01)
+    __virtio32 opcode;
+    /* algo should be service-specific algorithms */
+    __virtio32 algo;
+    /* session_id should be service-specific algorithms */
+    __virtio64 session_id;
+    /* control flag to control the request */
+    __virtio32 flag;
+    __virtio32 padding;
+};
+
+struct virtio_crypto_sym_input {
+    /* destination data guest address, it's useless for plain HASH and MAC */
+    __virtio64 dst_data_addr;
+    /* digest result guest address, it's useless for plain cipher algos */
+    __virtio64 digest_result_addr;
+
+    __virtio32 status;
+    __virtio32 padding;
+};
+
+struct virtio_crypto_cipher_para {
+    __virtio32 iv_len;
+    /* length of source data */
+    __virtio32 src_data_len;
+    /* length of dst data */
+    __virtio32 dst_data_len;
+    __virtio32 padding;
+};
+
+struct virtio_crypto_cipher_input {
+    struct virtio_crypto_sym_input input;
+};
+
+struct virtio_crypto_cipher_output {
+    __virtio64 iv_addr; /* iv guest address */
+    __virtio64 src_data_addr; /* source data guest address */
+};
+
+struct virtio_crypto_hash_input {
+    struct virtio_crypto_sym_input input;
+};
+
+struct virtio_crypto_hash_output {
+    /* source data guest address */
+    __virtio64 src_data_addr;
+    /* length of source data */
+    __virtio32 src_data_len;
+    __virtio32 padding;
+};
+
+struct virtio_crypto_mac_input {
+    struct virtio_crypto_sym_input input;
+};
+
+struct virtio_crypto_mac_output {
+    struct virtio_crypto_hash_output hash_output;
+};
+
+struct virtio_crypto_aead_para {
+    __virtio32 iv_len;
+    /* length of additional auth data */
+    __virtio32 aad_len;
+    /* length of source data */
+    __virtio32 src_data_len;
+    /* length of dst data */
+    __virtio32 dst_data_len;
+};
+
+struct virtio_crypto_aead_input {
+    struct virtio_crypto_sym_input input;
+};
+
+struct virtio_crypto_aead_output {
+    __virtio64 iv_addr; /* iv guest address */
+    __virtio64 src_data_addr; /* source data guest address */
+    __virtio64 add_data_addr; /* additional auth data guest address */
+};
+
+struct virtio_crypto_cipher_data_req {
+    /* Device-readable part */
+    struct virtio_crypto_cipher_para para;
+    struct virtio_crypto_cipher_output odata;
+    /* Device-writable part */
+    struct virtio_crypto_cipher_input idata;
+};
+
+struct virtio_crypto_hash_data_req {
+    /* Device-readable part */
+    struct virtio_crypto_hash_output odata;
+    /* Device-writable part */
+    struct virtio_crypto_hash_input idata;
+};
+
+struct virtio_crypto_mac_data_req {
+    /* Device-readable part */
+    struct virtio_crypto_mac_output odata;
+    /* Device-writable part */
+    struct virtio_crypto_mac_input idata;
+};
+
+struct virtio_crypto_alg_chain_data_para {
+    struct virtio_crypto_cipher_para cipher;
+};
+
+struct virtio_crypto_alg_chain_data_output {
+    struct virtio_crypto_cipher_output cipher;
+
+    /* Device-readable part */
+    __virtio64 aad_data_addr; /* additional auth data guest address */
+    __virtio32 aad_len; /* length of additional auth data */
+    __virtio32 padding;
+};
+
+struct virtio_crypto_alg_chain_data_input {
+    struct virtio_crypto_sym_input input;
+};
+
+struct virtio_crypto_alg_chain_data_req {
+    /* Device-readable part */
+    struct virtio_crypto_alg_chain_data_para para;
+    struct virtio_crypto_alg_chain_data_output odata;
+    /* Device-writable part */
+    struct virtio_crypto_alg_chain_data_input idata;
+};
+
+struct virtio_crypto_sym_data_req {
+    union {
+        struct virtio_crypto_cipher_data_req cipher;
+        struct virtio_crypto_alg_chain_data_req chain;
+    } u;
+
+    /* Device-readable part */
+
+    /* See above VIRTIO_CRYPTO_SYM_OP_* */
+    __virtio32 op_type;
+    __virtio32 padding;
+};
+
+struct virtio_crypto_aead_data_req {
+    /* Device-readable part */
+    struct virtio_crypto_aead_para para;
+    struct virtio_crypto_aead_output odata;
+    /* Device-writable part */
+    struct virtio_crypto_aead_input idata;
+};
+
+/* The request of the data viritqueue's packet */
+struct virtio_crypto_op_data_req {
+    struct virtio_crypto_op_header header;
+
+    union {
+        struct virtio_crypto_sym_data_req  sym_req;
+        struct virtio_crypto_hash_data_req hash_req;
+        struct virtio_crypto_mac_data_req mac_req;
+        struct virtio_crypto_aead_data_req aead_req;
+    } u;
+};
+
+#define VIRTIO_CRYPTO_OP_OK        0
+#define VIRTIO_CRYPTO_OP_ERR       1
+#define VIRTIO_CRYPTO_OP_BADMSG    2
+#define VIRTIO_CRYPTO_OP_NOTSUPP   3
+#define VIRTIO_CRYPTO_OP_INVSESS   4 /* Invaild session id */
+
+/* The accelerator hardware is ready */
+#define VIRTIO_CRYPTO_S_HW_READY  (1 << 0)
+#define VIRTIO_CRYPTO_S_STARTED  (1 << 1)
+
+struct virtio_crypto_config {
+    /* See VIRTIO_CRYPTO_OP_* above */
+    __virtio32  status;
+
+    /*
+     * Maximum number of data queue legal values are between 1 and 0x8000
+     */
+    __virtio32  max_dataqueues;
+
+    /* Specifies the services mask which the devcie support,
+       see VIRTIO_CRYPTO_SERVICE_* above */
+    __virtio32 crypto_services;
+
+    /* Detailed algorithms mask */
+    __virtio32 cipher_algo_l;
+    __virtio32 cipher_algo_h;
+    __virtio32 hash_algo;
+    __virtio32 mac_algo_l;
+    __virtio32 mac_algo_h;
+    __virtio32 asym_algo;
+    __virtio32 kdf_algo;
+    __virtio32 aead_algo;
+    __virtio32 primitive_algo;
+};
+
+#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 46f7993..55ed256 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4603,6 +4603,20 @@
     '*vectors': 'uint32' } }
 
 ##
+# @CryptodevLinuxOptions
+#
+# Cryptodev-linux cryptodev backend. Cryptodev-linux is a device that
+# allows access to Linux kernel cryptographic drivers; thus allowing
+# of userspace applications to take advantage of hardware accelerators.
+#
+# @fd: #optional file descriptor of an already opened cryptodev chardev
+#
+# Since 2.7
+##
+{ 'struct': 'CryptodevLinuxOptions',
+  'data': {
+    '*fd':    'int' } }
+##
 # @CryptoClientOptions
 #
 # A discriminated record of crypto device traits.
@@ -4611,7 +4625,8 @@
 #
 ##
 { 'union': 'CryptoClientOptions',
-  'data': {'legacy-hw':   'CryptoLegacyHWOptions'} }
+  'data': {'legacy-hw':   'CryptoLegacyHWOptions',
+           'cryptodev-linux': 'CryptodevLinuxOptions'} }
 
 ##
 # @Cryptodev
diff --git a/qemu-options.hx b/qemu-options.hx
index a64ce25..0f7c662 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3969,9 +3969,25 @@ contents of @code{iv.b64} to the second secret
 ETEXI
 
 DEF("cryptodev", HAS_ARG, QEMU_OPTION_cryptodev,
-    "",
+    "-cryptodev cryptodev-linux,id=str[,fd=h]\n"
+    "           configure a cryptodev-linux cryptodev backend with ID 'str'\n"
+    "           use 'fd=h' to connect to an already opened '/dev/crypto' fd\n",
     QEMU_ARCH_ALL)
 
+STEXI
+@item -cryptodev cryptodev-linux,id=@var{id}[,fd=@var{h}]
+configure a cryptodev backend with ID @var{id}.
+
+Example:
+@example
+#launch a QEMU instance with one virtio-crypto device,
+#which connected a cryptodev-linux backend device.
+qemu-system-x86_64 linux.img -cryptodev cryptodev-linux,id=cryptodev0 \
+                  -device virtio-crypto-pci,cryptodev=cryptodev0
+
+@end example
+ETEXI
+
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
 @end table
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [Qemu-devel] [PATCH v2 06/15] crypto: add internal handle logic layer
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
                   ` (4 preceding siblings ...)
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 05/15] crypto: add cryptodev-linux as a cryptodev backend Gonglei
@ 2016-09-13  3:52 ` Gonglei
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 07/15] virtio-crypto: introduce virtio-crypto.h Gonglei
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

This patch add some transfering layer or functions,
such as session creation, closing etc, which will be
used by subsequent virtio crypto device patches.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 crypto/crypto.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/crypto/crypto.c b/crypto/crypto.c
index 1d3d1d3..184f837 100644
--- a/crypto/crypto.c
+++ b/crypto/crypto.c
@@ -291,3 +291,71 @@ void qemu_del_crypto_legacy_hw(CryptoLegacyHWState *crypto)
 
     g_free(crypto);
 }
+
+CryptoLegacyHWState *qemu_get_crypto_legacy_hw(CryptoClientState *cc)
+{
+    CryptoClientState *cc0 = cc - cc->queue_index;
+
+    return (CryptoLegacyHWState *)((void *)cc0 - cc->info->size);
+}
+
+void *qemu_get_crypto_legacy_hw_opaque(CryptoClientState *cc)
+{
+    CryptoLegacyHWState *crypto = qemu_get_crypto_legacy_hw(cc);
+
+    return crypto->opaque;
+}
+
+int qemu_find_crypto_clients_except(const char *id, CryptoClientState **ccs,
+                                 CryptoClientOptionsKind type, int max)
+{
+    CryptoClientState *cc;
+    int ret = 0;
+
+    QTAILQ_FOREACH(cc, &crypto_clients, next) {
+        if (cc->info->type == type) {
+            continue;
+        }
+        if (!id || !strcmp(cc->name, id)) {
+            if (ret < max) {
+                ccs[ret] = cc;
+            }
+            ret++;
+        }
+    }
+
+    return ret;
+}
+
+int qemu_crypto_create_session(CryptoClientState *cc,
+                               CryptoSymSessionInfo *info,
+                               uint64_t *session_id)
+{
+    int ret = -1;
+    CryptoClientState *peer = cc->peer;
+
+    if (!peer || !peer->ready) {
+        return ret;
+    }
+
+    if (peer->info->create_session) {
+        ret = peer->info->create_session(peer, info, session_id);
+    }
+    return ret;
+}
+
+int qemu_crypto_close_session(CryptoClientState *cc,
+                               uint64_t session_id)
+{
+    int ret = -1;
+    CryptoClientState *peer = cc->peer;
+
+    if (!peer || !peer->ready) {
+        return ret;
+    }
+
+    if (peer->info->close_session) {
+        ret = peer->info->close_session(peer, session_id);
+    }
+    return ret;
+}
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [Qemu-devel] [PATCH v2 07/15] virtio-crypto: introduce virtio-crypto.h
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
                   ` (5 preceding siblings ...)
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 06/15] crypto: add internal handle logic layer Gonglei
@ 2016-09-13  3:52 ` Gonglei
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 08/15] virtio-crypto-pci: add virtio crypto pci support Gonglei
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

This patch introduces the header of virtio crypto emulation.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 include/hw/virtio/virtio-crypto.h | 84 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)
 create mode 100644 include/hw/virtio/virtio-crypto.h

diff --git a/include/hw/virtio/virtio-crypto.h b/include/hw/virtio/virtio-crypto.h
new file mode 100644
index 0000000..243ed71
--- /dev/null
+++ b/include/hw/virtio/virtio-crypto.h
@@ -0,0 +1,84 @@
+/*
+ * Virtio crypto Support
+ *
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *    Gonglei <arei.gonglei@huawei.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#ifndef _QEMU_VIRTIO_CRYPTO_H
+#define _QEMU_VIRTIO_CRYPTO_H
+
+#include "standard-headers/linux/virtio_crypto.h"
+#include "hw/virtio/virtio.h"
+#include "sysemu/iothread.h"
+#include "crypto/crypto.h"
+
+#define VIRTIO_ID_CRYPTO 20
+
+/* #define DEBUG_VIRTIO_CRYPTO */
+
+#ifdef DEBUG_VIRTIO_CRYPTO
+#define DPRINTF(fmt, ...) \
+do { printf("virtio_crypto: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) do { } while (0)
+#endif
+
+#define TYPE_VIRTIO_CRYPTO "virtio-crypto-device"
+#define VIRTIO_CRYPTO(obj) \
+        OBJECT_CHECK(VirtIOCrypto, (obj), TYPE_VIRTIO_CRYPTO)
+#define VIRTIO_CRYPTO_GET_PARENT_CLASS(obj) \
+        OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_CRYPTO)
+
+
+/* Limit the number of packets that can be sent via a single flush
+ * of the TX queue.  This gives us a guaranteed exit condition and
+ * ensures fairness in the io path.  256 conveniently matches the
+ * length of the TX queue and shows a good balance of performance
+ * and latency. */
+#define VIRTIO_CRYPTO_TX_BURST 256
+
+typedef struct VirtIOCryptoConf {
+    int32_t txburst;
+} VirtIOCryptoConf;
+
+struct VirtIOCrypto;
+
+typedef struct VirtIOCryptoQueue {
+    VirtQueue *dataq;
+    QEMUBH *tx_bh;
+    int tx_waiting;
+    struct {
+        VirtQueueElement *elem;
+        uint32_t flags;
+        CryptoSymOpInfo *op_info;
+        void *idata_hva;
+    } async_tx;
+    struct VirtIOCrypto *vcrypto;
+} VirtIOCryptoQueue;
+
+typedef struct VirtIOCrypto {
+    VirtIODevice parent_obj;
+
+    VirtIOCryptoQueue *vqs;
+    VirtQueue *ctrl_vq;
+    CryptoLegacyHWState *crypto;
+    CryptoLegacyHWConf legacy_conf;
+
+    VirtIOCryptoConf conf;
+    int32_t tx_burst;
+    uint32_t max_queues;
+    uint32_t status;
+
+    int multiqueue;
+    uint32_t curr_queues;
+    size_t config_size;
+} VirtIOCrypto;
+
+#endif /* _QEMU_VIRTIO_CRYPTO_H */
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [Qemu-devel] [PATCH v2 08/15] virtio-crypto-pci: add virtio crypto pci support
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
                   ` (6 preceding siblings ...)
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 07/15] virtio-crypto: introduce virtio-crypto.h Gonglei
@ 2016-09-13  3:52 ` Gonglei
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 09/15] virtio-crypto: add virtio crypto realization Gonglei
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

This patch adds virtio-crypto-pci, which is the pci proxy for the virtio
crypto device.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/virtio/Makefile.objs       |  2 +-
 hw/virtio/virtio-crypto-pci.c | 71 +++++++++++++++++++++++++++++++++++++++++++
 hw/virtio/virtio-pci.h        | 15 +++++++++
 3 files changed, 87 insertions(+), 1 deletion(-)
 create mode 100644 hw/virtio/virtio-crypto-pci.c

diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index e716308..d29966e 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -5,5 +5,5 @@ common-obj-y += virtio-mmio.o
 
 obj-y += virtio.o virtio-balloon.o 
 obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
-
 obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
+obj-$(CONFIG_VIRTIO_PCI) += virtio-crypto-pci.o
diff --git a/hw/virtio/virtio-crypto-pci.c b/hw/virtio/virtio-crypto-pci.c
new file mode 100644
index 0000000..4436955
--- /dev/null
+++ b/hw/virtio/virtio-crypto-pci.c
@@ -0,0 +1,71 @@
+/*
+ * Virtio crypto device
+ *
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *    Gonglei <arei.gonglei@huawei.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ *
+ */
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/virtio-pci.h"
+#include "hw/virtio/virtio-crypto.h"
+
+static Property virtio_crypto_pci_properties[] = {
+    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
+                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
+    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_crypto_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
+{
+    VirtIOCryptoPCI *vcrypto = VIRTIO_CRYPTO_PCI(vpci_dev);
+    DeviceState *vdev = DEVICE(&vcrypto->vdev);
+
+    qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
+    virtio_pci_force_virtio_1(vpci_dev);
+    object_property_set_bool(OBJECT(vdev), true, "realized", errp);
+}
+
+static void virtio_crypto_pci_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
+    PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
+
+    k->realize = virtio_crypto_pci_realize;
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+    dc->props = virtio_crypto_pci_properties;
+
+    pcidev_k->class_id = PCI_CLASS_OTHERS;
+}
+
+static void virtio_crypto_initfn(Object *obj)
+{
+    VirtIOCryptoPCI *dev = VIRTIO_CRYPTO_PCI(obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_CRYPTO);
+}
+
+static const TypeInfo virtio_crypto_pci_info = {
+    .name          = TYPE_VIRTIO_CRYPTO_PCI,
+    .parent        = TYPE_VIRTIO_PCI,
+    .instance_size = sizeof(VirtIOCryptoPCI),
+    .instance_init = virtio_crypto_initfn,
+    .class_init    = virtio_crypto_pci_class_init,
+};
+
+static void virtio_crypto_pci_register_types(void)
+{
+    type_register_static(&virtio_crypto_pci_info);
+}
+type_init(virtio_crypto_pci_register_types)
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 0698157..6a31fde 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -25,6 +25,8 @@
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio-input.h"
 #include "hw/virtio/virtio-gpu.h"
+#include "hw/virtio/virtio-crypto.h"
+
 #ifdef CONFIG_VIRTFS
 #include "hw/9pfs/virtio-9p.h"
 #endif
@@ -48,6 +50,7 @@ typedef struct VirtIOInputHIDPCI VirtIOInputHIDPCI;
 typedef struct VirtIOInputHostPCI VirtIOInputHostPCI;
 typedef struct VirtIOGPUPCI VirtIOGPUPCI;
 typedef struct VHostVSockPCI VHostVSockPCI;
+typedef struct VirtIOCryptoPCI VirtIOCryptoPCI;
 
 /* virtio-pci-bus */
 
@@ -347,6 +350,18 @@ struct VHostVSockPCI {
 };
 #endif
 
+/*
+ * virtio-crypto-pci: This extends VirtioPCIProxy.
+ */
+#define TYPE_VIRTIO_CRYPTO_PCI "virtio-crypto-pci"
+#define VIRTIO_CRYPTO_PCI(obj) \
+        OBJECT_CHECK(VirtIOCryptoPCI, (obj), TYPE_VIRTIO_CRYPTO_PCI)
+
+struct VirtIOCryptoPCI {
+    VirtIOPCIProxy parent_obj;
+    VirtIOCrypto vdev;
+};
+
 /* Virtio ABI version, if we increment this, we break the guest driver. */
 #define VIRTIO_PCI_ABI_VERSION          0
 
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [Qemu-devel] [PATCH v2 09/15] virtio-crypto: add virtio crypto realization
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
                   ` (7 preceding siblings ...)
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 08/15] virtio-crypto-pci: add virtio crypto pci support Gonglei
@ 2016-09-13  3:52 ` Gonglei
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 10/15] virtio-crypto: set capacity of crypto legacy hardware Gonglei
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

Introduce the virtio crypto realization, I'll
finish the core code in the following patches. The
thoughts came from virtio net realization.

For more information see:
http://qemu-project.org/Features/VirtioCrypto

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/core/qdev-properties-system.c |  86 ++++++++++++++
 hw/virtio/Makefile.objs          |   1 +
 hw/virtio/virtio-crypto.c        | 250 +++++++++++++++++++++++++++++++++++++++
 include/hw/qdev-properties.h     |   3 +
 4 files changed, 340 insertions(+)
 create mode 100644 hw/virtio/virtio-crypto.c

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index e55afe6..93afd64 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -22,6 +22,8 @@
 #include "qapi/visitor.h"
 #include "sysemu/char.h"
 #include "sysemu/iothread.h"
+#include "crypto/crypto.h"
+
 
 static void get_pointer(Object *obj, Visitor *v, Property *prop,
                         char *(*print)(void *ptr),
@@ -430,3 +432,87 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
     }
     nd->instantiated = 1;
 }
+
+/* --- cryptodev device --- */
+static void get_cryptodev(Object *obj, Visitor *v, const char *name,
+                          void *opaque, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    CryptoLegacyHWPeers *peers_ptr = qdev_get_prop_ptr(dev, prop);
+    char *p = g_strdup(peers_ptr->ccs[0] ? peers_ptr->ccs[0]->name : "");
+    visit_type_str(v, name, &p, errp);
+    g_free(p);
+}
+
+static void set_cryptodev(Object *obj, Visitor *v, const char *name,
+                          void *opaque, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    CryptoLegacyHWPeers *peers_ptr = qdev_get_prop_ptr(dev, prop);
+    CryptoClientState **ccs = peers_ptr->ccs;
+    CryptoClientState *peers[MAX_CRYPTO_QUEUE_NUM];
+    Error *local_err = NULL;
+    int queues, err = 0, i = 0;
+    char *str;
+
+    if (dev->realized) {
+        qdev_prop_set_after_realize(dev, name, errp);
+        return;
+    }
+
+    visit_type_str(v, name, &str, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    queues = qemu_find_crypto_clients_except(str, peers,
+                                          CRYPTO_CLIENT_OPTIONS_KIND_LEGACY_HW,
+                                          MAX_CRYPTO_QUEUE_NUM);
+    if (queues == 0) {
+        err = -ENOENT;
+        goto out;
+    }
+
+    if (queues > MAX_CRYPTO_QUEUE_NUM) {
+        error_setg(errp,
+          "queues of backend '%s'(%d) exceeds QEMU limitation(%d)",
+                   str, queues, MAX_CRYPTO_QUEUE_NUM);
+        goto out;
+    }
+
+    for (i = 0; i < queues; i++) {
+        if (peers[i] == NULL) {
+            err = -ENOENT;
+            goto out;
+        }
+
+        if (peers[i]->peer) {
+            err = -EEXIST;
+            goto out;
+        }
+
+        if (ccs[i]) {
+            err = -EINVAL;
+            goto out;
+        }
+
+        ccs[i] = peers[i];
+        ccs[i]->queue_index = i;
+    }
+
+    peers_ptr->queues = queues;
+
+out:
+    error_set_from_qdev_prop_error(errp, err, dev, prop, str);
+    g_free(str);
+}
+
+PropertyInfo qdev_prop_cryptodev = {
+    .name  = "str",
+    .description = "ID of a cryptodev to use as a backend",
+    .get   = get_cryptodev,
+    .set   = set_cryptodev,
+};
diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index d29966e..00731f6 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -7,3 +7,4 @@ obj-y += virtio.o virtio-balloon.o
 obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
 obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
 obj-$(CONFIG_VIRTIO_PCI) += virtio-crypto-pci.o
+obj-y += virtio-crypto.o
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
new file mode 100644
index 0000000..23c5041
--- /dev/null
+++ b/hw/virtio/virtio-crypto.c
@@ -0,0 +1,250 @@
+/*
+ * Virtio crypto Support
+ *
+ * Based on virtio-net.c
+ *
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ *    Gonglei <arei.gonglei@huawei.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "qemu/iov.h"
+#include "hw/qdev.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-crypto.h"
+#include "hw/virtio/virtio-access.h"
+
+static void virtio_crypto_process(VirtIOCrypto *vcrypto)
+{
+}
+
+static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+{
+}
+
+static void virtio_crypto_handle_dataq_bh(VirtIODevice *vdev, VirtQueue *vq)
+{
+}
+
+static void virtio_crypto_dataq_bh(void *opaque)
+{
+}
+
+static void virtio_crypto_add_queue(VirtIOCrypto *vcrypto, int index)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
+
+    vcrypto->vqs[index].dataq = virtio_add_queue(vdev, 1024,
+                                         virtio_crypto_handle_dataq_bh);
+    vcrypto->vqs[index].tx_bh = qemu_bh_new(virtio_crypto_dataq_bh,
+                                              &vcrypto->vqs[index]);
+
+    vcrypto->vqs[index].tx_waiting = 0;
+    vcrypto->vqs[index].vcrypto = vcrypto;
+}
+
+static void virtio_crypto_del_queue(VirtIOCrypto *vcrypto, int index)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
+    VirtIOCryptoQueue *q = &vcrypto->vqs[index];
+
+    virtio_del_queue(vdev, index);
+    qemu_bh_delete(q->tx_bh);
+}
+
+static uint64_t virtio_crypto_get_features(VirtIODevice *vdev,
+                                           uint64_t features,
+                                           Error **errp)
+{
+    return features;
+}
+
+static void virtio_crypto_set_features(VirtIODevice *vdev, uint64_t features)
+{
+
+}
+
+static void virtio_crypto_save(QEMUFile *f, void *opaque)
+{
+    VirtIODevice *vdev = opaque;
+
+    virtio_save(vdev, f);
+}
+
+static int virtio_crypto_load(QEMUFile *f, void *opaque, int version_id)
+{
+    VirtIOCrypto *vcrypto = opaque;
+    int ret;
+
+    if (version_id != 1) {
+        return -EINVAL;
+    }
+    ret = virtio_load(VIRTIO_DEVICE(vcrypto), f, version_id);
+    if (ret != 0) {
+        return ret;
+    }
+
+    /* We may have an element ready but couldn't process it due to a quota
+     * limit.  Make sure to try again after live migration when the quota may
+     * have been reset.
+     */
+    virtio_crypto_process(vcrypto);
+
+    return 0;
+}
+
+static void virtio_crypto_set_status(VirtIODevice *vdev, uint8_t status)
+{
+}
+
+static void virtio_crypto_reset(VirtIODevice *vdev)
+{
+    /*
+     * This should cancel pending requests, but can't do nicely until there
+     * are per-device request lists.
+     */
+}
+
+static void virtio_crypto_set_hw_status(CryptoClientState *cc)
+{
+    VirtIOCrypto *vcrypto = qemu_get_crypto_legacy_hw_opaque(cc);
+    VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
+    uint16_t old_status = vcrypto->status;
+
+    if (!cc->ready) {
+        vcrypto->status &= ~VIRTIO_CRYPTO_S_HW_READY;
+    } else {
+        vcrypto->status |= VIRTIO_CRYPTO_S_HW_READY;
+    }
+    if (vcrypto->status != old_status) {
+        virtio_notify_config(vdev);
+    }
+
+    virtio_crypto_set_status(vdev, vdev->status);
+}
+
+static CryptoClientInfo crypto_virtio_info = {
+    .type = CRYPTO_CLIENT_OPTIONS_KIND_LEGACY_HW,
+    .size = sizeof(CryptoLegacyHWState),
+    .hw_status_changed = virtio_crypto_set_hw_status,
+};
+
+static void virtio_crypto_device_realize(DeviceState *dev, Error **errp)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(dev);
+    int i;
+
+    vcrypto->max_queues = MAX(vcrypto->legacy_conf.peers.queues, 1);
+    if (vcrypto->max_queues + 1 > VIRTIO_QUEUE_MAX) {
+        error_setg(errp, "Invalid number of queues (= %" PRIu16 "), "
+                   "must be a postive integer less than %d.",
+                   vcrypto->max_queues, VIRTIO_QUEUE_MAX - 1);
+        return;
+    }
+
+    virtio_init(vdev, "virtio-crypto", VIRTIO_ID_CRYPTO, vcrypto->config_size);
+    vcrypto->vqs = g_new0(VirtIOCryptoQueue, vcrypto->max_queues);
+    vcrypto->curr_queues = 1;
+
+    for (i = 0; i < vcrypto->max_queues; i++) {
+        virtio_crypto_add_queue(vcrypto, i);
+    }
+
+    vcrypto->ctrl_vq = virtio_add_queue(vdev, 64, virtio_crypto_handle_ctrl);
+    vcrypto->tx_burst = vcrypto->conf.txburst;
+    vcrypto->crypto = qemu_new_crypto_legacy_hw(&crypto_virtio_info,
+                                             &vcrypto->legacy_conf,
+                                             object_get_typename(OBJECT(dev)),
+                                             dev->id, vcrypto);
+    vcrypto->status = VIRTIO_CRYPTO_S_HW_READY;
+    register_savevm(dev, "virtio-crypto", -1, 1, virtio_crypto_save,
+                    virtio_crypto_load, vcrypto);
+}
+
+static void virtio_crypto_device_unrealize(DeviceState *dev, Error **errp)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(dev);
+    int i, max_queues;
+
+    unregister_savevm(dev, "virtio-crypto", vcrypto);
+
+    max_queues = vcrypto->multiqueue ? vcrypto->max_queues : 1;
+    for (i = 0; i < max_queues; i++) {
+        virtio_crypto_del_queue(vcrypto, i);
+    }
+    g_free(vcrypto->vqs);
+    qemu_del_crypto_legacy_hw(vcrypto->crypto);
+
+    virtio_cleanup(vdev);
+}
+
+static Property virtio_crypto_properties[] = {
+    DEFINE_PROP_CRYPTODEV("cryptodev", VirtIOCrypto, legacy_conf.peers),
+    DEFINE_PROP_INT32("x-txburst", VirtIOCrypto, conf.txburst,
+                      VIRTIO_CRYPTO_TX_BURST),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_crypto_get_config(VirtIODevice *vdev, uint8_t *config)
+{
+
+}
+
+static void virtio_crypto_set_config(VirtIODevice *vdev, const uint8_t *config)
+{
+
+}
+
+static void virtio_crypto_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+
+    dc->props = virtio_crypto_properties;
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+    vdc->realize = virtio_crypto_device_realize;
+    vdc->unrealize = virtio_crypto_device_unrealize;
+    vdc->get_config = virtio_crypto_get_config;
+    vdc->set_config = virtio_crypto_set_config;
+    vdc->get_features = virtio_crypto_get_features;
+    vdc->set_features = virtio_crypto_set_features;
+    vdc->set_status = virtio_crypto_set_status;
+    vdc->reset = virtio_crypto_reset;
+}
+
+static void virtio_crypto_instance_init(Object *obj)
+{
+    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(obj);
+
+    /*
+     * The default config_size is sizeof(struct virtio_crypto_config).
+     * Can be overriden with virtio_crypto_set_config_size.
+     */
+    vcrypto->config_size = sizeof(struct virtio_crypto_config);
+}
+
+
+static const TypeInfo virtio_crypto_info = {
+    .name = TYPE_VIRTIO_CRYPTO,
+    .parent = TYPE_VIRTIO_DEVICE,
+    .instance_size = sizeof(VirtIOCrypto),
+    .instance_init = virtio_crypto_instance_init,
+    .class_init = virtio_crypto_class_init,
+};
+
+static void virtio_register_types(void)
+{
+    type_register_static(&virtio_crypto_info);
+}
+
+type_init(virtio_register_types)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 2a9d2f9..7e7facd 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -30,6 +30,7 @@ extern PropertyInfo qdev_prop_pci_devfn;
 extern PropertyInfo qdev_prop_blocksize;
 extern PropertyInfo qdev_prop_pci_host_devaddr;
 extern PropertyInfo qdev_prop_arraylen;
+extern PropertyInfo qdev_prop_cryptodev;
 
 #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
         .name      = (_name),                                    \
@@ -171,6 +172,8 @@ extern PropertyInfo qdev_prop_arraylen;
     DEFINE_PROP_DEFAULT(_n, _s, _f, 0, qdev_prop_blocksize, uint16_t)
 #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
     DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
+#define DEFINE_PROP_CRYPTODEV(_n, _s, _f)             \
+            DEFINE_PROP(_n, _s, _f, qdev_prop_cryptodev, CryptoLegacyHWPeers)
 
 #define DEFINE_PROP_END_OF_LIST()               \
     {}
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [Qemu-devel] [PATCH v2 10/15] virtio-crypto: set capacity of crypto legacy hardware
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
                   ` (8 preceding siblings ...)
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 09/15] virtio-crypto: add virtio crypto realization Gonglei
@ 2016-09-13  3:52 ` Gonglei
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 11/15] virtio-crypto: add control queue handler Gonglei
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

Set the crypto legacy hardware's capacity according to the
backend peer cryptodev's capacity. We only support only one
queue at present.

Virtio crypto device is a kind of crypto legacy hardware.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 crypto/crypto.c           | 17 +++++++++++++++++
 hw/virtio/virtio-crypto.c | 18 +++++++++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/crypto/crypto.c b/crypto/crypto.c
index 184f837..ba99d7c 100644
--- a/crypto/crypto.c
+++ b/crypto/crypto.c
@@ -228,6 +228,7 @@ qemu_new_crypto_legacy_hw(CryptoClientInfo *info,
     CryptoLegacyHWState *crypto;
     CryptoClientState **peers = conf->peers.ccs;
     int i, queues = MAX(1, conf->peers.queues);
+    int has_set = 0;
 
     assert(info->type == CRYPTO_CLIENT_OPTIONS_KIND_LEGACY_HW);
     assert(info->size >= sizeof(CryptoLegacyHWState));
@@ -242,6 +243,22 @@ qemu_new_crypto_legacy_hw(CryptoClientInfo *info,
                               NULL);
         crypto->ccs[i].queue_index = i;
         crypto->ccs[i].ready = true;
+        /* The mask bits of crypto_services and algos in
+           CryptoLegacyHWConf is set only once */
+        if (has_set == 0 && peers[i]) {
+            conf->crypto_services = peers[i]->crypto_services;
+            conf->cipher_algo_l = peers[i]->cipher_algo_l;
+            conf->cipher_algo_h = peers[i]->cipher_algo_h;
+            conf->hash_algo = peers[i]->hash_algo;
+            conf->mac_algo_l = peers[i]->mac_algo_l;
+            conf->mac_algo_h = peers[i]->mac_algo_h;
+            conf->asym_algo = peers[i]->asym_algo;
+            conf->kdf_algo = peers[i]->kdf_algo;
+            conf->aead_algo = peers[i]->aead_algo;
+            conf->primitive_algo = peers[i]->primitive_algo;
+
+            has_set = 1;
+        }
     }
 
     return crypto;
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 23c5041..b5a108f 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -197,7 +197,23 @@ static Property virtio_crypto_properties[] = {
 
 static void virtio_crypto_get_config(VirtIODevice *vdev, uint8_t *config)
 {
-
+    VirtIOCrypto *c = VIRTIO_CRYPTO(vdev);
+    struct virtio_crypto_config crypto_cfg;
+
+    crypto_cfg.status = c->status;
+    crypto_cfg.max_dataqueues = c->max_queues;
+    crypto_cfg.crypto_services = c->legacy_conf.crypto_services;
+    crypto_cfg.cipher_algo_l = c->legacy_conf.cipher_algo_l;
+    crypto_cfg.cipher_algo_h = c->legacy_conf.cipher_algo_h;
+    crypto_cfg.hash_algo = c->legacy_conf.hash_algo;
+    crypto_cfg.mac_algo_l = c->legacy_conf.mac_algo_l;
+    crypto_cfg.mac_algo_h = c->legacy_conf.mac_algo_h;
+    crypto_cfg.asym_algo = c->legacy_conf.asym_algo;
+    crypto_cfg.kdf_algo = c->legacy_conf.kdf_algo;
+    crypto_cfg.aead_algo = c->legacy_conf.aead_algo;
+    crypto_cfg.primitive_algo = c->legacy_conf.primitive_algo;
+
+    memcpy(config, &crypto_cfg, c->config_size;
 }
 
 static void virtio_crypto_set_config(VirtIODevice *vdev, const uint8_t *config)
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [Qemu-devel] [PATCH v2 11/15] virtio-crypto: add control queue handler
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
                   ` (9 preceding siblings ...)
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 10/15] virtio-crypto: set capacity of crypto legacy hardware Gonglei
@ 2016-09-13  3:52 ` Gonglei
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 12/15] virtio-crypto: add destroy session logic Gonglei
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

Realize the Symmetric algos session creation handler,
including plain cipher and chainning algorithms.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/virtio/virtio-crypto.c | 175 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 174 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index b5a108f..1090946 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -26,8 +26,181 @@ static void virtio_crypto_process(VirtIOCrypto *vcrypto)
 {
 }
 
+static inline int virtio_crypto_vq2q(int queue_index)
+{
+    return queue_index;
+}
+
+static void
+virtio_crypto_cipher_session_helper(VirtIODevice *vdev,
+           CryptoSymSessionInfo *info,
+           struct virtio_crypto_cipher_session_para *cipher_para,
+           struct virtio_crypto_cipher_session_output *cipher_out)
+{
+    hwaddr key_gpa;
+    void *key_hva;
+    hwaddr len;
+
+    info->cipher_alg = cipher_para->algo;
+    info->key_len = cipher_para->keylen;
+    info->direction = cipher_para->op;
+    len = info->key_len;
+    /* get cipher key */
+    if (len > 0) {
+        DPRINTF("keylen=%" PRIu32 "\n", info->key_len);
+        key_gpa = cipher_out->key_addr;
+
+        key_hva = cpu_physical_memory_map(key_gpa, &len, 0);
+
+        info->cipher_key = g_malloc(info->key_len);
+        memcpy(info->cipher_key, key_hva, info->key_len);
+        cpu_physical_memory_unmap(key_hva, len, 0, len);
+    }
+}
+
+static int64_t
+virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto,
+               struct virtio_crypto_sym_create_session_req *sess_req,
+               uint32_t queue_id,
+               uint64_t *session_id)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
+    CryptoSymSessionInfo info;
+    int ret;
+    CryptoClientState *cc;
+    int queue_index;;
+    uint32_t op_type;
+    hwaddr auth_key_gpa;
+    void *auth_key_hva;
+    struct virtio_crypto_session_input *input;
+    hwaddr len;
+
+    memset(&info, 0, sizeof(info));
+    op_type = sess_req->op_type;
+    info.op_type = op_type;
+
+    if (op_type == VIRTIO_CRYPTO_SYM_OP_CIPHER) {
+        virtio_crypto_cipher_session_helper(vdev, &info,
+                           &sess_req->u.cipher.para,
+                           &sess_req->u.cipher.out);
+        input = &sess_req->u.cipher.input;
+    } else if (op_type == VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING) {
+        /* cipher part */
+        virtio_crypto_cipher_session_helper(vdev, &info,
+                           &sess_req->u.chain.para.cipher_param,
+                           &sess_req->u.chain.out.cipher);
+        input = &sess_req->u.chain.input;
+        /* hash part */
+        info.alg_chain_order = sess_req->u.chain.para.alg_chain_order;
+        info.add_len = sess_req->u.chain.para.aad_len;
+        info.hash_mode = sess_req->u.chain.para.hash_mode;
+        if (info.hash_mode == VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH) {
+            info.hash_alg = sess_req->u.chain.para.u.mac_param.algo;
+            len = info.auth_key_len =
+                       sess_req->u.chain.para.u.mac_param.auth_key_len;
+            info.hash_result_len =
+                    sess_req->u.chain.para.u.mac_param.hash_result_len;
+            /* get auth key */
+            if (len > 0) {
+                DPRINTF("keylen=%" PRIu32 "\n", info.auth_key_len);
+                auth_key_gpa = sess_req->u.chain.out.mac.auth_key_addr;
+                auth_key_hva = cpu_physical_memory_map(auth_key_gpa,
+                               &len, false);
+                info.auth_key = g_malloc(len);
+                memcpy(info.auth_key, auth_key_hva, len);
+                cpu_physical_memory_unmap(auth_key_hva, len, false, len);
+            }
+        } else if (info.hash_mode == VIRTIO_CRYPTO_SYM_HASH_MODE_PLAIN) {
+            info.hash_alg = sess_req->u.chain.para.u.hash_param.algo;
+            info.hash_result_len =
+                   sess_req->u.chain.para.u.hash_param.hash_result_len;
+        } else {
+            /* VIRTIO_CRYPTO_SYM_HASH_MODE_NESTED */
+            error_report("unsupported hash mode");
+            goto err;
+        }
+    } else {
+        /* VIRTIO_CRYPTO_SYM_OP_NONE */
+        error_report("unsupported cipher type");
+        goto err;
+    }
+
+    queue_index = virtio_crypto_vq2q(queue_id);
+    cc = qemu_get_crypto_subqueue(vcrypto->crypto, queue_index);
+    ret = qemu_crypto_create_session(cc, &info, session_id);
+    if (ret == 0) {
+        DPRINTF("create session_id=%" PRIu64 "\n", *session_id);
+        /* Set the result, notify the frontend driver soon */
+        input->status = VIRTIO_CRYPTO_OP_OK;
+        input->session_id = *session_id;
+
+        g_free(info.cipher_key);
+        g_free(info.auth_key);
+        return 0;
+    }
+
+err:
+    g_free(info.cipher_key);
+    g_free(info.auth_key);
+    input->status = VIRTIO_CRYPTO_OP_ERR;
+    return -1;
+}
+
 static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
+    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(vdev);
+    struct virtio_crypto_op_ctrl_req ctrl;
+    VirtQueueElement *elem;
+    size_t s;
+    struct iovec *iov;
+    unsigned int iov_cnt;
+    uint32_t queue_id;
+    uint32_t opcode;
+    uint64_t session_id = 0;
+
+    for (;;) {
+        elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+        if (!elem) {
+            break;
+        }
+        if (elem->in_num < 1 ||
+            iov_size(elem->in_sg, elem->in_num) < sizeof(ctrl)) {
+            error_report("virtio-crypto ctrl missing headers");
+            exit(1);
+        }
+
+        iov_cnt = elem->in_num;
+        iov = elem->in_sg;
+        s = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl));
+        assert(s == sizeof(ctrl));
+        opcode = ctrl.header.opcode;
+        queue_id = ctrl.header.queue_id;
+
+        switch (opcode) {
+        case VIRTIO_CRYPTO_CIPHER_CREATE_SESSION:
+            virtio_crypto_create_sym_session(vcrypto,
+                             &ctrl.u.sym_create_session,
+                             queue_id,
+                             &session_id);
+
+            break;
+        case VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION:
+        case VIRTIO_CRYPTO_HASH_CREATE_SESSION:
+        case VIRTIO_CRYPTO_HASH_DESTROY_SESSION:
+        case VIRTIO_CRYPTO_MAC_CREATE_SESSION:
+        case VIRTIO_CRYPTO_MAC_DESTROY_SESSION:
+        case VIRTIO_CRYPTO_AEAD_CREATE_SESSION:
+        case VIRTIO_CRYPTO_AEAD_DESTROY_SESSION:
+        default:
+            error_report("virtio-crypto unsupported ctrl opcode: %u",
+                         opcode);
+            exit(1);
+        }
+
+        virtqueue_push(vq, elem, sizeof(ctrl));
+        virtio_notify(vdev, vq);
+        g_free(elem);
+    }
 }
 
 static void virtio_crypto_handle_dataq_bh(VirtIODevice *vdev, VirtQueue *vq)
@@ -213,7 +386,7 @@ static void virtio_crypto_get_config(VirtIODevice *vdev, uint8_t *config)
     crypto_cfg.aead_algo = c->legacy_conf.aead_algo;
     crypto_cfg.primitive_algo = c->legacy_conf.primitive_algo;
 
-    memcpy(config, &crypto_cfg, c->config_size;
+    memcpy(config, &crypto_cfg, c->config_size);
 }
 
 static void virtio_crypto_set_config(VirtIODevice *vdev, const uint8_t *config)
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [Qemu-devel] [PATCH v2 12/15] virtio-crypto: add destroy session logic
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
                   ` (10 preceding siblings ...)
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 11/15] virtio-crypto: add control queue handler Gonglei
@ 2016-09-13  3:52 ` Gonglei
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 13/15] virtio-crypto: get correct input data address for each request Gonglei
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/virtio/virtio-crypto.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 1090946..0d7ab63 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -146,6 +146,32 @@ err:
     return -1;
 }
 
+static void
+virtio_crypto_handle_close_session(VirtIOCrypto *vcrypto,
+         struct virtio_crypto_destroy_session_req *close_sess_req,
+         uint32_t queue_id)
+{
+    int ret;
+    CryptoClientState *cc = vcrypto->crypto->ccs;
+    uint64_t session_id;
+    uint32_t status;
+    int queue_index = virtio_crypto_vq2q(queue_id);
+
+    session_id = close_sess_req->session_id;
+    DPRINTF("close session, id=%" PRIu64 "\n", session_id);
+    cc = qemu_get_crypto_subqueue(vcrypto->crypto, queue_index);
+    ret = qemu_crypto_close_session(cc, session_id);
+    if (ret == 0) {
+        status = VIRTIO_CRYPTO_OP_OK;
+    } else {
+        error_report("destroy session failed");
+        status = VIRTIO_CRYPTO_OP_ERR;
+    }
+
+    /* Set the result, notify the frontend driver soon */
+    close_sess_req->status = status;
+}
+
 static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(vdev);
@@ -185,12 +211,15 @@ static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 
             break;
         case VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION:
-        case VIRTIO_CRYPTO_HASH_CREATE_SESSION:
         case VIRTIO_CRYPTO_HASH_DESTROY_SESSION:
-        case VIRTIO_CRYPTO_MAC_CREATE_SESSION:
         case VIRTIO_CRYPTO_MAC_DESTROY_SESSION:
-        case VIRTIO_CRYPTO_AEAD_CREATE_SESSION:
         case VIRTIO_CRYPTO_AEAD_DESTROY_SESSION:
+            virtio_crypto_handle_close_session(vcrypto,
+                   &ctrl.u.destroy_session, queue_id);
+            break;
+        case VIRTIO_CRYPTO_HASH_CREATE_SESSION:
+        case VIRTIO_CRYPTO_MAC_CREATE_SESSION:
+        case VIRTIO_CRYPTO_AEAD_CREATE_SESSION:
         default:
             error_report("virtio-crypto unsupported ctrl opcode: %u",
                          opcode);
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [Qemu-devel] [PATCH v2 13/15] virtio-crypto: get correct input data address for each request
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
                   ` (11 preceding siblings ...)
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 12/15] virtio-crypto: add destroy session logic Gonglei
@ 2016-09-13  3:52 ` Gonglei
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 14/15] virtio-crypto: add data virtqueue processing handler Gonglei
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

We MUST use the original hva for input data, but not the
copyed address, otherwise the guest can't get the results.
Fix a non-initial problem in an exception case as well.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/virtio/virtio-crypto.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 0d7ab63..96c5a2a 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -62,7 +62,8 @@ static int64_t
 virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto,
                struct virtio_crypto_sym_create_session_req *sess_req,
                uint32_t queue_id,
-               uint64_t *session_id)
+               uint64_t *session_id,
+               VirtQueueElement *elem)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
     CryptoSymSessionInfo info;
@@ -74,6 +75,8 @@ virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto,
     void *auth_key_hva;
     struct virtio_crypto_session_input *input;
     hwaddr len;
+    size_t input_offset;
+    struct iovec *iov = elem->in_sg;
 
     memset(&info, 0, sizeof(info));
     op_type = sess_req->op_type;
@@ -83,13 +86,19 @@ virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto,
         virtio_crypto_cipher_session_helper(vdev, &info,
                            &sess_req->u.cipher.para,
                            &sess_req->u.cipher.out);
-        input = &sess_req->u.cipher.input;
+        /* calculate the offset of input data */
+        input_offset = offsetof(struct virtio_crypto_op_ctrl_req,
+                          u.sym_create_session.u.cipher.input);
+        input = (void *)iov[0].iov_base + input_offset;
     } else if (op_type == VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING) {
         /* cipher part */
         virtio_crypto_cipher_session_helper(vdev, &info,
                            &sess_req->u.chain.para.cipher_param,
                            &sess_req->u.chain.out.cipher);
-        input = &sess_req->u.chain.input;
+        /* calculate the offset of input data */
+        input_offset = offsetof(struct virtio_crypto_op_ctrl_req,
+                                u.sym_create_session.u.chain.input);
+        input = (void *)iov[0].iov_base + input_offset;
         /* hash part */
         info.alg_chain_order = sess_req->u.chain.para.alg_chain_order;
         info.add_len = sess_req->u.chain.para.aad_len;
@@ -120,6 +129,10 @@ virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto,
             goto err;
         }
     } else {
+        /* calculate the offset of input data */
+        input_offset = offsetof(struct virtio_crypto_op_ctrl_req,
+                                u.sym_create_session.u.cipher.input);
+        input = (void *)iov[0].iov_base + input_offset;
         /* VIRTIO_CRYPTO_SYM_OP_NONE */
         error_report("unsupported cipher type");
         goto err;
@@ -149,13 +162,17 @@ err:
 static void
 virtio_crypto_handle_close_session(VirtIOCrypto *vcrypto,
          struct virtio_crypto_destroy_session_req *close_sess_req,
-         uint32_t queue_id)
+         uint32_t queue_id,
+         VirtQueueElement *elem)
 {
     int ret;
     CryptoClientState *cc = vcrypto->crypto->ccs;
     uint64_t session_id;
     uint32_t status;
     int queue_index = virtio_crypto_vq2q(queue_id);
+    struct iovec *iov = elem->in_sg;
+    size_t status_offset;
+    void *in_status_ptr;
 
     session_id = close_sess_req->session_id;
     DPRINTF("close session, id=%" PRIu64 "\n", session_id);
@@ -168,8 +185,12 @@ virtio_crypto_handle_close_session(VirtIOCrypto *vcrypto,
         status = VIRTIO_CRYPTO_OP_ERR;
     }
 
+    /* calculate the offset of status bits */
+    status_offset = offsetof(struct virtio_crypto_op_ctrl_req,
+                             u.destroy_session.status);
+    in_status_ptr = (void *)iov[0].iov_base + status_offset;
     /* Set the result, notify the frontend driver soon */
-    close_sess_req->status = status;
+    memcpy(in_status_ptr, &status, sizeof(status));
 }
 
 static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
@@ -207,7 +228,8 @@ static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
             virtio_crypto_create_sym_session(vcrypto,
                              &ctrl.u.sym_create_session,
                              queue_id,
-                             &session_id);
+                             &session_id,
+                             elem);
 
             break;
         case VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION:
@@ -215,7 +237,8 @@ static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
         case VIRTIO_CRYPTO_MAC_DESTROY_SESSION:
         case VIRTIO_CRYPTO_AEAD_DESTROY_SESSION:
             virtio_crypto_handle_close_session(vcrypto,
-                   &ctrl.u.destroy_session, queue_id);
+                   &ctrl.u.destroy_session, queue_id,
+                   elem);
             break;
         case VIRTIO_CRYPTO_HASH_CREATE_SESSION:
         case VIRTIO_CRYPTO_MAC_CREATE_SESSION:
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [Qemu-devel] [PATCH v2 14/15] virtio-crypto: add data virtqueue processing handler
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
                   ` (12 preceding siblings ...)
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 13/15] virtio-crypto: get correct input data address for each request Gonglei
@ 2016-09-13  3:52 ` Gonglei
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 15/15] virtio-crypto: support scatter gather list Gonglei
  2016-09-13  8:57 ` [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Daniel P. Berrange
  15 siblings, 0 replies; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

At present, we only support cipher and algorithm chainning.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/virtio/virtio-crypto.c | 364 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 364 insertions(+)

diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 96c5a2a..16b1a2b 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -22,6 +22,8 @@
 #include "hw/virtio/virtio-crypto.h"
 #include "hw/virtio/virtio-access.h"
 
+static int32_t virtio_crypto_flush_dataq(VirtIOCryptoQueue *q);
+
 static void virtio_crypto_process(VirtIOCrypto *vcrypto)
 {
 }
@@ -31,6 +33,14 @@ static inline int virtio_crypto_vq2q(int queue_index)
     return queue_index;
 }
 
+static VirtIOCryptoQueue *
+virtio_crypto_get_subqueue(CryptoClientState *cc)
+{
+    VirtIOCrypto *vcrypto = qemu_get_crypto_legacy_hw_opaque(cc);
+
+    return &vcrypto->vqs[cc->queue_index];
+}
+
 static void
 virtio_crypto_cipher_session_helper(VirtIODevice *vdev,
            CryptoSymSessionInfo *info,
@@ -255,12 +265,366 @@ static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
     }
 }
 
+static CryptoSymOpInfo *
+virtio_crypto_cipher_op_helper(VirtIODevice *vdev,
+           struct virtio_crypto_cipher_para *para,
+           struct virtio_crypto_cipher_output *out,
+           uint32_t aad_len,
+           uint64_t aad_data_addr)
+{
+    CryptoSymOpInfo *op_info;
+    uint32_t src_len, dst_len;
+    uint32_t iv_len;
+    size_t max_len, curr_size = 0;
+    hwaddr iv_gpa, src_gpa;
+    void *iv_hva, *src_hva, *aad_hva;
+    hwaddr len;
+
+    iv_len = para->iv_len;
+    src_len = para->src_data_len;
+    dst_len = para->dst_data_len;
+
+    max_len = iv_len + aad_len + src_len + dst_len;
+    op_info = g_malloc0(sizeof(CryptoSymOpInfo) + max_len);
+    op_info->iv_len = iv_len;
+    op_info->src_len = src_len;
+    op_info->dst_len = dst_len;
+    op_info->aad_len = aad_len;
+    /* handle the initilization vector */
+    if (op_info->iv_len > 0) {
+        len = op_info->iv_len;
+        DPRINTF("iv_len=%" PRIu32 "\n", len);
+        op_info->iv = op_info->data + curr_size;
+
+        iv_gpa = out->iv_addr;
+        iv_hva = cpu_physical_memory_map(iv_gpa, &len, false);
+        memcpy(op_info->iv, iv_hva, len);
+        cpu_physical_memory_unmap(iv_hva, len, false, len);
+        curr_size += len;
+    }
+
+    /* handle additional authentication data if exist */
+    if (op_info->aad_len > 0) {
+        len = op_info->aad_len;
+        DPRINTF("aad_len=%" PRIu32 "\n", len);
+        op_info->aad_data = op_info->data + curr_size;
+
+        aad_hva = cpu_physical_memory_map(aad_data_addr, &len, false);
+        memcpy(op_info->aad_data, aad_hva, len);
+        cpu_physical_memory_unmap(aad_hva, len, false, len);
+        curr_size += len;
+    }
+
+    /* handle the source data */
+    if (op_info->src_len > 0) {
+        len = op_info->src_len;
+        DPRINTF("src_len=%" PRIu32 "\n", len);
+        op_info->src = op_info->data + curr_size;
+
+        src_gpa = out->src_data_addr;
+        src_hva = cpu_physical_memory_map(src_gpa, &len, false);
+        memcpy(op_info->src, src_hva, len);
+        cpu_physical_memory_unmap(src_hva, len, false, len);
+        curr_size += len;
+    }
+    op_info->dst = op_info->data + curr_size;
+    DPRINTF("dst_len=%" PRIu32 "\n", op_info->dst_len);
+
+    return op_info;
+}
+
+static void
+virtio_crypto_sym_input_data_helper(VirtIODevice *vdev,
+                void *idata_hva,
+                uint32_t status,
+                CryptoSymOpInfo *sym_op_info)
+{
+    struct virtio_crypto_sym_input *idata = idata_hva;
+    hwaddr dst_gpa, len;
+    void *dst_hva;
+
+    idata->status = status;
+    if (status != VIRTIO_CRYPTO_OP_OK) {
+        return;
+    }
+
+    /* save the cipher result */
+    dst_gpa = idata->dst_data_addr;
+    /* Note: length of dest_data is equal to length of src_data for cipher */
+    len = sym_op_info->src_len;
+    dst_hva = cpu_physical_memory_map(dst_gpa, &len, true);
+    memcpy(dst_hva, sym_op_info->dst, len);
+    cpu_physical_memory_unmap(dst_hva, len, true, len);
+
+    if (sym_op_info->op_type ==
+                      VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING) {
+        hwaddr digest_gpa;
+        void *digest_hva;
+
+        /* save the digest result */
+        digest_gpa = idata->digest_result_addr;
+        len = sym_op_info->dst_len - sym_op_info->src_len;
+        digest_hva = cpu_physical_memory_map(digest_gpa, &len, true);
+        memcpy(digest_hva, sym_op_info->dst + sym_op_info->src_len, len);
+        cpu_physical_memory_unmap(digest_hva, len, true, len);
+    }
+}
+
+static void virtio_crypto_tx_complete(CryptoClientState *cc,
+                                      int ret)
+{
+    VirtIOCrypto *vcrypto = qemu_get_crypto_legacy_hw_opaque(cc);
+    VirtIOCryptoQueue *q = virtio_crypto_get_subqueue(cc);
+    VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
+    uint32_t flags = q->async_tx.flags;
+
+    if (flags == QEMU_CRYPTO_PACKET_FLAG_SYM) {
+        CryptoSymOpInfo *sym_op_info = q->async_tx.op_info;
+
+        if (ret > 0) {
+            virtio_crypto_sym_input_data_helper(vdev,
+                                    q->async_tx.idata_hva,
+                                    VIRTIO_CRYPTO_OP_OK,
+                                    sym_op_info);
+        } else if (ret == -1 || ret == 0) {
+            virtio_crypto_sym_input_data_helper(vdev,
+                                    q->async_tx.idata_hva,
+                                    VIRTIO_CRYPTO_OP_ERR,
+                                    sym_op_info);
+        } else if (ret == -VIRTIO_CRYPTO_OP_BADMSG) {
+            virtio_crypto_sym_input_data_helper(vdev,
+                                    q->async_tx.idata_hva,
+                                    VIRTIO_CRYPTO_OP_BADMSG,
+                                    sym_op_info);
+        } else if (ret == -VIRTIO_CRYPTO_OP_INVSESS) {
+            virtio_crypto_sym_input_data_helper(vdev,
+                                    q->async_tx.idata_hva,
+                                    VIRTIO_CRYPTO_OP_INVSESS,
+                                    sym_op_info);
+        }
+    }
+
+    virtqueue_push(q->dataq, q->async_tx.elem,
+                sizeof(struct virtio_crypto_op_data_req));
+    virtio_notify(vdev, q->dataq);
+
+    g_free(q->async_tx.elem);
+    q->async_tx.elem = NULL;
+
+    virtio_queue_set_notification(q->dataq, 1);
+    virtio_crypto_flush_dataq(q);
+}
+
+static void
+virtio_crypto_handle_sym_req(VirtIOCrypto *vcrypto,
+               struct virtio_crypto_sym_data_req *req,
+               CryptoSymOpInfo **sym_op_info,
+               void **idata_hva,
+               VirtQueueElement *elem)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
+    uint32_t op_type;
+    void *idata;
+    size_t idata_offset;
+    struct iovec *iov = elem->in_sg;
+    CryptoSymOpInfo *op_info;
+
+    op_type = req->op_type;
+
+    if (op_type == VIRTIO_CRYPTO_SYM_OP_CIPHER) {
+        op_info = virtio_crypto_cipher_op_helper(vdev, &req->u.cipher.para,
+                                              &req->u.cipher.odata, 0, 0);
+        op_info->op_type = op_type;
+        /* calculate the offset of input data */
+        idata_offset = offsetof(struct virtio_crypto_op_data_req,
+                                u.sym_req.u.cipher.idata.input);
+        idata = (void *)iov[0].iov_base + idata_offset;
+
+    } else if (op_type == VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING) {
+        uint32_t aad_len;
+        uint64_t aad_data_addr;
+
+        aad_len = req->u.chain.odata.aad_len;
+        aad_data_addr = req->u.chain.odata.aad_data_addr;
+        /* cipher part */
+        op_info = virtio_crypto_cipher_op_helper(vdev, &req->u.cipher.para,
+                                              &req->u.cipher.odata, aad_len,
+                                              aad_data_addr);
+        op_info->op_type = op_type;
+
+        /* calculate the offset of input data */
+        idata_offset = offsetof(struct virtio_crypto_op_data_req,
+                                u.sym_req.u.chain.idata.input);
+        idata = (void *)iov[0].iov_base + idata_offset;
+    } else {
+        /* VIRTIO_CRYPTO_SYM_OP_NONE */
+        error_report("unsupported cipher type");
+        exit(1);
+    }
+
+    *sym_op_info = op_info;
+    *idata_hva = idata;
+}
+
+static int32_t virtio_crypto_flush_dataq(VirtIOCryptoQueue *q)
+{
+    VirtIOCrypto *vcrypto = q->vcrypto;
+    VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
+    VirtQueueElement *elem;
+    int32_t num_packets = 0;
+    int queue_index = virtio_crypto_vq2q(virtio_get_queue_index(q->dataq));
+    struct virtio_crypto_op_data_req req;
+    size_t s;
+    int ret;
+    struct iovec *iov;
+    unsigned int iov_cnt;
+    uint32_t opcode;
+    uint64_t session_id;
+    CryptoSymOpInfo *sym_op_info = NULL;
+    void *idata_hva = NULL;
+
+    if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+        return num_packets;
+    }
+
+    if (q->async_tx.elem) {
+        virtio_queue_set_notification(q->dataq, 0);
+        return num_packets;
+    }
+
+    for (;;) {
+        elem = virtqueue_pop(q->dataq, sizeof(VirtQueueElement));
+        if (!elem) {
+            break;
+        }
+
+        if (elem->in_num < 1 ||
+            iov_size(elem->in_sg, elem->in_num) < sizeof(req)) {
+            error_report("virtio-crypto dataq missing headers");
+            exit(1);
+        }
+
+        iov_cnt = elem->in_num;
+        iov = elem->in_sg;
+
+        s = iov_to_buf(iov, iov_cnt, 0, &req, sizeof(req));
+        assert(s == sizeof(req));
+        opcode = req.header.opcode;
+        session_id = req.header.session_id;
+
+        switch (opcode) {
+        case VIRTIO_CRYPTO_CIPHER_ENCRYPT:
+        case VIRTIO_CRYPTO_CIPHER_DECRYPT:
+            virtio_crypto_handle_sym_req(vcrypto,
+                             &req.u.sym_req,
+                             &sym_op_info,
+                             &idata_hva,
+                             elem);
+            sym_op_info->session_id = session_id;
+            ret = qemu_send_crypto_packet_async(
+                 qemu_get_crypto_subqueue(vcrypto->crypto, queue_index),
+                                      QEMU_CRYPTO_PACKET_FLAG_SYM,
+                                      sym_op_info, virtio_crypto_tx_complete);
+            if (ret == 0) {
+                virtio_queue_set_notification(q->dataq, 0);
+                q->async_tx.elem = elem;
+                q->async_tx.flags = QEMU_CRYPTO_PACKET_FLAG_SYM;
+                q->async_tx.idata_hva = idata_hva;
+                q->async_tx.op_info = sym_op_info;
+                return -EBUSY;
+            } else if (ret < 0) {
+                virtio_crypto_sym_input_data_helper(vdev, idata_hva,
+                                VIRTIO_CRYPTO_OP_ERR,
+                                sym_op_info);
+                goto drop;
+            } else { /* ret > 0 */
+                virtio_crypto_sym_input_data_helper(vdev, idata_hva,
+                                VIRTIO_CRYPTO_OP_OK,
+                                sym_op_info);
+            }
+            break;
+        case VIRTIO_CRYPTO_HASH:
+        case VIRTIO_CRYPTO_MAC:
+        case VIRTIO_CRYPTO_AEAD_ENCRYPT:
+        case VIRTIO_CRYPTO_AEAD_DECRYPT:
+        default:
+            error_report("virtio-crypto unsupported dataq opcode: %u",
+                         opcode);
+            exit(1);
+        }
+
+drop:
+        virtqueue_push(q->dataq, elem, sizeof(req));
+        virtio_notify(vdev, q->dataq);
+
+        if (++num_packets >= vcrypto->tx_burst) {
+            break;
+        }
+    }
+    return num_packets;
+}
+
 static void virtio_crypto_handle_dataq_bh(VirtIODevice *vdev, VirtQueue *vq)
 {
+    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(vdev);
+    int queue_index = virtio_crypto_vq2q(virtio_get_queue_index(vq));
+    VirtIOCryptoQueue *q = &vcrypto->vqs[queue_index];
+
+    if (unlikely(q->tx_waiting)) {
+        return;
+    }
+    q->tx_waiting = 1;
+    /* This happens when device was stopped but VCPU wasn't. */
+    if (!vdev->vm_running) {
+        return;
+    }
+    virtio_queue_set_notification(vq, 0);
+    qemu_bh_schedule(q->tx_bh);
 }
 
 static void virtio_crypto_dataq_bh(void *opaque)
 {
+    VirtIOCryptoQueue *q = opaque;
+    VirtIOCrypto *vcrypto = q->vcrypto;
+    VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
+    int32_t ret;
+
+    /* This happens when device was stopped but BH wasn't. */
+    if (!vdev->vm_running) {
+        /* Make sure tx waiting is set, so we'll run when restarted. */
+        assert(q->tx_waiting);
+        return;
+    }
+
+    q->tx_waiting = 0;
+
+    /* Just in case the driver is not ready on more */
+    if (unlikely(!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK))) {
+        return;
+    }
+
+    ret = virtio_crypto_flush_dataq(q);
+    if (ret == -EBUSY) {
+        return; /* Notification re-enable handled by tx_complete */
+    }
+
+    /* If we flush a full burst of packets, assume there are
+     * more coming and immediately reschedule */
+    if (ret >= vcrypto->tx_burst) {
+        qemu_bh_schedule(q->tx_bh);
+        q->tx_waiting = 1;
+        return;
+    }
+
+    /* If less than a full burst, re-enable notification and flush
+     * anything that may have come in while we weren't looking.  If
+     * we find something, assume the guest is still active and reschedule */
+    virtio_queue_set_notification(q->dataq, 1);
+    if (virtio_crypto_flush_dataq(q) > 0) {
+        virtio_queue_set_notification(q->dataq, 0);
+        qemu_bh_schedule(q->tx_bh);
+        q->tx_waiting = 1;
+    }
 }
 
 static void virtio_crypto_add_queue(VirtIOCrypto *vcrypto, int index)
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [Qemu-devel] [PATCH v2 15/15] virtio-crypto: support scatter gather list
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
                   ` (13 preceding siblings ...)
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 14/15] virtio-crypto: add data virtqueue processing handler Gonglei
@ 2016-09-13  3:52 ` Gonglei
  2016-09-13  8:57 ` [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Daniel P. Berrange
  15 siblings, 0 replies; 40+ messages in thread
From: Gonglei @ 2016-09-13  3:52 UTC (permalink / raw)
  To: qemu-devel, virtio-dev
  Cc: peter.huangpeng, luonengjun, mst, stefanha, pbonzini, berrange,
	weidong.huang, mike.caraman, agraf, xin.zeng, claudio.fontana,
	nmorey, vincent.jardin, Gonglei

Now, we can support scatter gather list on source
data, destination data, and associated anthentication
data.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/virtio/virtio-crypto.c                      | 224 +++++++++++++++++++++----
 include/hw/virtio/virtio-crypto.h              |  12 ++
 include/standard-headers/linux/virtio_crypto.h |  46 +++--
 3 files changed, 235 insertions(+), 47 deletions(-)

diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 16b1a2b..661223b 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -265,25 +265,177 @@ static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
     }
 }
 
+static void virtio_crypto_map_iovec(unsigned int *p_num_sg, hwaddr *addr,
+                               struct iovec *iov,
+                               unsigned int max_num_sg,
+                               hwaddr pa, size_t sz,
+                               bool is_write)
+{
+    unsigned num_sg = *p_num_sg;
+    assert(num_sg <= max_num_sg);
+
+    if (!sz) {
+        error_report("virtio-crypto: zero sized buffers are not allowed");
+        exit(1);
+    }
+
+    while (sz) {
+        hwaddr len = sz;
+
+        if (num_sg == max_num_sg) {
+            error_report("virtio-crypto: too many entries "
+                        "in the scatter gather list");
+            exit(1);
+        }
+
+        iov[num_sg].iov_base = cpu_physical_memory_map(pa, &len, is_write);
+        iov[num_sg].iov_len = len;
+        addr[num_sg] = pa;
+
+        sz -= len;
+        pa += len;
+        num_sg++;
+    }
+    *p_num_sg = num_sg;
+}
+
+static void virtio_crypto_unmap_iovec(VirtIOCryptoBuffer *buf,
+                               unsigned int len,
+                               bool is_write)
+{
+    unsigned int offset;
+    int i;
+
+    if (is_write) {
+        offset = 0;
+        for (i = 0; i < buf->num; i++) {
+            size_t size = MIN(len - offset, buf->sg[i].iov_len);
+
+            cpu_physical_memory_unmap(buf->sg[i].iov_base,
+                                      buf->sg[i].iov_len,
+                                      1, size);
+
+            offset += size;
+        }
+    } else {
+        for (i = 0; i < buf->num; i++) {
+            cpu_physical_memory_unmap(buf->sg[i].iov_base,
+                                      buf->sg[i].iov_len,
+                                      0, buf->sg[i].iov_len);
+        }
+    }
+}
+
+static void *virtio_crypto_read_next_iovec(VirtIODevice *vdev,
+                                struct virtio_crypto_iovec *iovec,
+                                bool is_write,
+                                struct iovec *iov,
+                                unsigned int *num)
+{
+    struct virtio_crypto_iovec *iovec_hva;
+    hwaddr pa;
+    hwaddr len;
+
+    /* If this descriptor says it doesn't chain, we're done. */
+    if (!(iovec->flags & VIRTIO_CRYPTO_IOVEC_F_NEXT)) {
+        return NULL;
+    }
+
+    pa = iovec->next_iovec;
+    len = sizeof(*iovec_hva);
+    iovec_hva = cpu_physical_memory_map(pa, &len, is_write);
+    assert(len == sizeof(*iovec_hva));
+
+    iov[*num].iov_base = iovec_hva;
+    iov[*num].iov_len = len;
+    (*num)++;
+
+    return iovec_hva;
+}
+
+static void *virtio_crypto_alloc_buf(unsigned num)
+{
+    VirtIOCryptoBuffer *buf;
+    size_t addr_ofs = QEMU_ALIGN_UP(sizeof(*buf), __alignof__(buf->addr[0]));
+    size_t addr_end = addr_ofs + num * sizeof(buf->addr[0]);
+    size_t sg_ofs = QEMU_ALIGN_UP(addr_end, __alignof__(buf->sg[0]));
+    size_t sg_end = sg_ofs + num * sizeof(buf->sg[0]);
+
+    buf = g_malloc(sg_end);
+    buf->num = num;
+
+    buf->addr = (void *)buf + addr_ofs;
+    buf->sg = (void *)buf + sg_ofs;
+    return buf;
+}
+
+static void *virtio_crypto_iovec_read(VirtIODevice *vdev,
+                      struct virtio_crypto_iovec *iovec,
+                      bool is_write)
+{
+
+    VirtIOCryptoBuffer *buf;
+    hwaddr addr[VIRTIO_CRYPTO_SG_MAX];
+    struct iovec iov[VIRTIO_CRYPTO_SG_MAX];
+    unsigned int num = 0;
+    /* Save virtio_crypto_iov structure's hva information in sg_list */
+    struct iovec vc_iov[VIRTIO_CRYPTO_SG_MAX];
+    unsigned int vc_num = 0;
+    unsigned int i;
+
+    struct virtio_crypto_iovec *p_iovec = iovec;
+
+    /* Collect all the sgs */
+    do {
+        virtio_crypto_map_iovec(&num, addr, iov,
+                           VIRTIO_CRYPTO_SG_MAX,
+                           p_iovec->addr, p_iovec->len,
+                           is_write);
+    } while ((p_iovec = virtio_crypto_read_next_iovec(vdev,
+                p_iovec, false, vc_iov, &vc_num))
+                != NULL);
+
+    /* Now copy what we have collected and mapped */
+    buf = virtio_crypto_alloc_buf(num);
+    for (i = 0; i < num; i++) {
+        buf->addr[i] = addr[i];
+        buf->sg[i] = iov[i];
+    }
+    /* Unmap all virtio_crypto_iov structure if exists */
+    for (i = 0; i < vc_num; i++) {
+        cpu_physical_memory_unmap(vc_iov[i].iov_base,
+                                  vc_iov[i].iov_len,
+                                  false, vc_iov[i].iov_len);
+    }
+
+    return buf;
+}
+
 static CryptoSymOpInfo *
 virtio_crypto_cipher_op_helper(VirtIODevice *vdev,
            struct virtio_crypto_cipher_para *para,
            struct virtio_crypto_cipher_output *out,
-           uint32_t aad_len,
-           uint64_t aad_data_addr)
+           struct virtio_crypto_iovec *add_data)
 {
     CryptoSymOpInfo *op_info;
     uint32_t src_len, dst_len;
     uint32_t iv_len;
     size_t max_len, curr_size = 0;
-    hwaddr iv_gpa, src_gpa;
-    void *iv_hva, *src_hva, *aad_hva;
+    hwaddr iv_gpa;
+    void *iv_hva;
     hwaddr len;
+    uint32_t aad_len = 0;
+    VirtIOCryptoBuffer *buf;
+    size_t s;
 
     iv_len = para->iv_len;
     src_len = para->src_data_len;
     dst_len = para->dst_data_len;
 
+    if (add_data) {
+        aad_len = add_data->len;
+    }
+
     max_len = iv_len + aad_len + src_len + dst_len;
     op_info = g_malloc0(sizeof(CryptoSymOpInfo) + max_len);
     op_info->iv_len = iv_len;
@@ -305,27 +457,32 @@ virtio_crypto_cipher_op_helper(VirtIODevice *vdev,
 
     /* handle additional authentication data if exist */
     if (op_info->aad_len > 0) {
-        len = op_info->aad_len;
         DPRINTF("aad_len=%" PRIu32 "\n", len);
         op_info->aad_data = op_info->data + curr_size;
 
-        aad_hva = cpu_physical_memory_map(aad_data_addr, &len, false);
-        memcpy(op_info->aad_data, aad_hva, len);
-        cpu_physical_memory_unmap(aad_hva, len, false, len);
-        curr_size += len;
+        buf = virtio_crypto_iovec_read(vdev, add_data, false);
+        s = iov_to_buf(buf->sg, buf->num, 0, op_info->aad_data,
+                       op_info->aad_len);
+        assert(s == op_info->aad_len);
+
+        virtio_crypto_unmap_iovec(buf, op_info->aad_len, false);
+        g_free(buf);
+        curr_size += op_info->aad_len;
     }
 
     /* handle the source data */
     if (op_info->src_len > 0) {
-        len = op_info->src_len;
-        DPRINTF("src_len=%" PRIu32 "\n", len);
+        DPRINTF("src_len=%" PRIu32 "\n", op_info->src_len);
         op_info->src = op_info->data + curr_size;
 
-        src_gpa = out->src_data_addr;
-        src_hva = cpu_physical_memory_map(src_gpa, &len, false);
-        memcpy(op_info->src, src_hva, len);
-        cpu_physical_memory_unmap(src_hva, len, false, len);
-        curr_size += len;
+        buf = virtio_crypto_iovec_read(vdev, &out->src_data, false);
+        s = iov_to_buf(buf->sg, buf->num, 0, op_info->src, op_info->src_len);
+        assert(s == op_info->src_len);
+
+        virtio_crypto_unmap_iovec(buf, op_info->src_len, false);
+        g_free(buf);
+
+        curr_size += op_info->src_len;
     }
     op_info->dst = op_info->data + curr_size;
     DPRINTF("dst_len=%" PRIu32 "\n", op_info->dst_len);
@@ -340,21 +497,24 @@ virtio_crypto_sym_input_data_helper(VirtIODevice *vdev,
                 CryptoSymOpInfo *sym_op_info)
 {
     struct virtio_crypto_sym_input *idata = idata_hva;
-    hwaddr dst_gpa, len;
-    void *dst_hva;
+    hwaddr len;
+    VirtIOCryptoBuffer *buf;
+    size_t s;
 
     idata->status = status;
     if (status != VIRTIO_CRYPTO_OP_OK) {
         return;
     }
 
-    /* save the cipher result */
-    dst_gpa = idata->dst_data_addr;
+    buf = virtio_crypto_iovec_read(vdev, &idata->dst_data, true);
     /* Note: length of dest_data is equal to length of src_data for cipher */
     len = sym_op_info->src_len;
-    dst_hva = cpu_physical_memory_map(dst_gpa, &len, true);
-    memcpy(dst_hva, sym_op_info->dst, len);
-    cpu_physical_memory_unmap(dst_hva, len, true, len);
+    /* save the cipher result */
+    s = iov_from_buf(buf->sg, buf->num, 0, sym_op_info->dst, len);
+    assert(s == len);
+
+    virtio_crypto_unmap_iovec(buf, len, false);
+    g_free(buf);
 
     if (sym_op_info->op_type ==
                       VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING) {
@@ -363,8 +523,12 @@ virtio_crypto_sym_input_data_helper(VirtIODevice *vdev,
 
         /* save the digest result */
         digest_gpa = idata->digest_result_addr;
-        len = sym_op_info->dst_len - sym_op_info->src_len;
+        len = idata->digest_result_len;
+        if (len != sym_op_info->dst_len - sym_op_info->src_len) {
+            len = sym_op_info->dst_len - sym_op_info->src_len;
+        }
         digest_hva = cpu_physical_memory_map(digest_gpa, &len, true);
+        /* find the digest result, then copy it into guest's memory */
         memcpy(digest_hva, sym_op_info->dst + sym_op_info->src_len, len);
         cpu_physical_memory_unmap(digest_hva, len, true, len);
     }
@@ -433,23 +597,17 @@ virtio_crypto_handle_sym_req(VirtIOCrypto *vcrypto,
 
     if (op_type == VIRTIO_CRYPTO_SYM_OP_CIPHER) {
         op_info = virtio_crypto_cipher_op_helper(vdev, &req->u.cipher.para,
-                                              &req->u.cipher.odata, 0, 0);
+                                              &req->u.cipher.odata, NULL);
         op_info->op_type = op_type;
         /* calculate the offset of input data */
         idata_offset = offsetof(struct virtio_crypto_op_data_req,
                                 u.sym_req.u.cipher.idata.input);
         idata = (void *)iov[0].iov_base + idata_offset;
-
     } else if (op_type == VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING) {
-        uint32_t aad_len;
-        uint64_t aad_data_addr;
-
-        aad_len = req->u.chain.odata.aad_len;
-        aad_data_addr = req->u.chain.odata.aad_data_addr;
         /* cipher part */
         op_info = virtio_crypto_cipher_op_helper(vdev, &req->u.cipher.para,
-                                              &req->u.cipher.odata, aad_len,
-                                              aad_data_addr);
+                                              &req->u.cipher.odata,
+                                              &req->u.chain.odata.add_data);
         op_info->op_type = op_type;
 
         /* calculate the offset of input data */
diff --git a/include/hw/virtio/virtio-crypto.h b/include/hw/virtio/virtio-crypto.h
index 243ed71..4f70eab 100644
--- a/include/hw/virtio/virtio-crypto.h
+++ b/include/hw/virtio/virtio-crypto.h
@@ -44,6 +44,18 @@ do { printf("virtio_crypto: " fmt , ## __VA_ARGS__); } while (0)
  * and latency. */
 #define VIRTIO_CRYPTO_TX_BURST 256
 
+/* Max entries of scatter gather list in one virtio-crypto buffer */
+#define VIRTIO_CRYPTO_SG_MAX 256
+
+typedef struct VirtIOCryptoBuffer {
+    unsigned int num;
+    /* Guest physical address */
+    hwaddr *addr;
+    /* Store host virtual address and length */
+    struct iovec *sg;
+    uint8_t data[0];
+} VirtIOCryptoBuffer;
+
 typedef struct VirtIOCryptoConf {
     int32_t txburst;
 } VirtIOCryptoConf;
diff --git a/include/standard-headers/linux/virtio_crypto.h b/include/standard-headers/linux/virtio_crypto.h
index 9de5bdb..9648984 100644
--- a/include/standard-headers/linux/virtio_crypto.h
+++ b/include/standard-headers/linux/virtio_crypto.h
@@ -6,6 +6,21 @@
 #include "standard-headers/linux/virtio_types.h"
 
 
+struct virtio_crypto_iovec {
+    /* Guest physical address */
+    __virtio64 addr;
+    /* Length of guest physical address */
+    __virtio32 len;
+
+/* This marks a buffer as continuing via the next field */
+#define VIRTIO_CRYPTO_IOVEC_F_NEXT 1
+    /* The flags as indicated above. */
+    __virtio32 flags;
+    /* Pointer to next struct virtio_crypto_iovec if flags & NEXT */
+    __virtio64 next_iovec;
+};
+
+
 #define VIRTIO_CRYPTO_SERVICE_CIPHER (0)
 #define VIRTIO_CRYPTO_SERVICE_HASH (1)
 #define VIRTIO_CRYPTO_SERVICE_MAC  (2)
@@ -263,13 +278,14 @@ struct virtio_crypto_op_header {
 };
 
 struct virtio_crypto_sym_input {
-    /* destination data guest address, it's useless for plain HASH and MAC */
-    __virtio64 dst_data_addr;
+    /* destination data, it's useless for plain HASH and MAC */
+    struct virtio_crypto_iovec dst_data;
     /* digest result guest address, it's useless for plain cipher algos */
     __virtio64 digest_result_addr;
+    /* digest result length which is the same with session para above */
+    __virtio32 digest_result_len;
 
     __virtio32 status;
-    __virtio32 padding;
 };
 
 struct virtio_crypto_cipher_para {
@@ -286,8 +302,10 @@ struct virtio_crypto_cipher_input {
 };
 
 struct virtio_crypto_cipher_output {
-    __virtio64 iv_addr; /* iv guest address */
-    __virtio64 src_data_addr; /* source data guest address */
+    /* iv guest address */
+    __virtio64 iv_addr;
+    /* source data */
+    struct virtio_crypto_iovec src_data;
 };
 
 struct virtio_crypto_hash_input {
@@ -295,10 +313,8 @@ struct virtio_crypto_hash_input {
 };
 
 struct virtio_crypto_hash_output {
-    /* source data guest address */
-    __virtio64 src_data_addr;
-    /* length of source data */
-    __virtio32 src_data_len;
+    /* source data */
+    struct virtio_crypto_iovec src_data;
     __virtio32 padding;
 };
 
@@ -326,8 +342,10 @@ struct virtio_crypto_aead_input {
 
 struct virtio_crypto_aead_output {
     __virtio64 iv_addr; /* iv guest address */
-    __virtio64 src_data_addr; /* source data guest address */
-    __virtio64 add_data_addr; /* additional auth data guest address */
+    /* source data */
+    struct virtio_crypto_iovec src_data;
+    /* additional auth data guest address */
+    struct virtio_crypto_iovec add_data;
 };
 
 struct virtio_crypto_cipher_data_req {
@@ -357,12 +375,12 @@ struct virtio_crypto_alg_chain_data_para {
 };
 
 struct virtio_crypto_alg_chain_data_output {
+    /* Device-writable part */
     struct virtio_crypto_cipher_output cipher;
 
     /* Device-readable part */
-    __virtio64 aad_data_addr; /* additional auth data guest address */
-    __virtio32 aad_len; /* length of additional auth data */
-    __virtio32 padding;
+    /* additional auth data guest address */
+    struct virtio_crypto_iovec add_data;
 };
 
 struct virtio_crypto_alg_chain_data_input {
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation
  2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
                   ` (14 preceding siblings ...)
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 15/15] virtio-crypto: support scatter gather list Gonglei
@ 2016-09-13  8:57 ` Daniel P. Berrange
  2016-09-13  9:45   ` Gonglei (Arei)
  15 siblings, 1 reply; 40+ messages in thread
From: Daniel P. Berrange @ 2016-09-13  8:57 UTC (permalink / raw)
  To: Gonglei
  Cc: qemu-devel, virtio-dev, peter.huangpeng, luonengjun, mst,
	stefanha, pbonzini, weidong.huang, mike.caraman, agraf, xin.zeng,
	claudio.fontana, nmorey, vincent.jardin

On Tue, Sep 13, 2016 at 11:52:06AM +0800, Gonglei wrote:
> Changes since v1:
>  - rmmove mixed endian-ness handler for virtio-crypto device, just
>    use little-endian. [mst]
>  - add sg list support according virtio-crypto spec v10 (will be posted soon).
>  - fix a memory leak in session handler.
>  - add a feature page link in qemu.org (http://qemu-project.org/Features/VirtioCrypto)
>  - fix some trivial problems, sush as 's/Since 2.7/Since 2.8/g' in qapi-schema.json
>  - rebase the latest qemu master tree.
> 
> Please review, thanks!
> 
> This patch series realize the framework and emulation of a new
> virtio crypto device, which is similar with virtio net device.
>  
>  - I introduce the cryptodev backend as the client of virtio crypto device
>    which can be realized by different methods, such as cryptodev-linux in my series,
>    vhost-crypto kernel module, vhost-user etc.
>  - The patch set abides by the virtio crypto speccification.
>  - The virtio crypto support symmetric algorithms (including CIPHER and algorithm chainning)
>    at present, except HASH, MAC and AEAD services.
>  - unsupport hot plug/unplug cryptodev client at this moment.
> 
> Cryptodev-linux is a device that allows access to Linux kernel cryptographic drivers;
> thus allowing of userspace applications to take advantage of hardware accelerators.
> It can be found here:
> 
>  http://cryptodev-linux.org/
> 
> (please use the latest version)
> 
> To use the cryptodev-linux as the client, the cryptodev.ko should be insert on the host.
> 
>  # enter cryptodev-linux module root directory, then
>  make;make install


The cryptodev kernel module is not upstream and shows no sign of
ever being likely to be accepted & merged upstream. On that basis,
support for it in QEMU has a firm NACK from me, as trying to support
out of tree kernel modules long term never ends well. This is
particularly bad because it appears to be the only impl backend
you've provided in this series.

IMHO, the first default backend should ought to use the internal
QEMU crypto APIs for its impl, which delegate to nettle/gcrypt,
which in turn can take care of using the kernel for acceleration
if they so choose.

> then configuring QEMU shows:
> 
>  [...]
>  jemalloc support  no
>  avx2 optimization no
>  cryptodev-linux support yes
> 
> QEMU can then be started using the following parameters:
> 
> qemu-system-x86_64 \
>     [...] \
>         -cryptodev type=cryptodev-linux,id=cryptodev0 \
>         -device virtio-crypto-pci,id=crypto0,cryptodev=cryptodev0 \
>     [...]
> 
> The front-end linux kernel driver (Experimental at present) is publicly accessible from:
>  
>    https://github.com/gongleiarei/virtio-crypto-linux-driver.git
> 
> After insmod virtio-crypto.ko, you also can use cryptodev-linux test the crypto function
> in the guest. For example:


> 
> linux-guest:/home/gonglei/cryptodev-linux/tests # ./cipher -
> requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver virtio_crypto_aes_cbc
> AES Test passed
> requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver virtio_crypto_aes_cbc
> requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver virtio_crypto_aes_cbc
> Test passed
> 
> QEMU code also can be accessible from:
> 
>  https://github.com/gongleiarei/qemu.git 
> 
>  branch virtio-crypto
> 
> For more information, please see:
>  http://qemu-project.org/Features/VirtioCrypto
> 
> 
> Gonglei (15):
>   crypto: introduce cryptodev backend and crypto legacy hardware
>   crypto: introduce crypto queue handler
>   crypto: add cryptoLegacyHW stuff
>   crypto: add symetric algorithms support
>   crypto: add cryptodev-linux as a cryptodev backend
>   crypto: add internal handle logic layer
>   virtio-crypto: introduce virtio-crypto.h
>   virtio-crypto-pci: add virtio crypto pci support
>   virtio-crypto: add virtio crypto realization
>   virtio-crypto: set capacity of crypto legacy hardware
>   virtio-crypto: add control queue handler
>   virtio-crypto: add destroy session logic
>   virtio-crypto: get correct input data address for each request
>   virtio-crypto: add data virtqueue processing handler
>   virtio-crypto: support scatter gather list
> 
>  configure                                      |   16 +
>  crypto/Makefile.objs                           |    3 +
>  crypto/crypto-queue.c                          |  206 +++++
>  crypto/crypto.c                                |  378 +++++++++

As a general point, please don't take the plain 'crypto'
namespace - that's way too generic a term.

I'd expect to see 'cryptodev.c' and 'cryptodev-queue.c'
really, since these files appear specific to the cryptodev

>  crypto/cryptodev-linux.c                       |  419 ++++++++++
>  hw/core/qdev-properties-system.c               |   86 ++
>  hw/virtio/Makefile.objs                        |    3 +-
>  hw/virtio/virtio-crypto-pci.c                  |   71 ++
>  hw/virtio/virtio-crypto.c                      | 1013 ++++++++++++++++++++++++
>  hw/virtio/virtio-pci.h                         |   15 +
>  include/crypto/crypto-clients.h                |   39 +

I'd expect header file names to match the name of the
.c file containing the impl. You've not added any
crypto-clients.c file - if the code is in crypto.c
then the crypto-clients.h content should be in
crypto.h too.

>  include/crypto/crypto-queue.h                  |   69 ++
>  include/crypto/crypto.h                        |  192 +++++

Again, cryptodev.h and crypto-queue.h are preferrable


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto legacy hardware
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto legacy hardware Gonglei
@ 2016-09-13  9:13   ` Daniel P. Berrange
  2016-09-13  9:55     ` Gonglei (Arei)
  2016-09-13 10:50     ` Paolo Bonzini
  0 siblings, 2 replies; 40+ messages in thread
From: Daniel P. Berrange @ 2016-09-13  9:13 UTC (permalink / raw)
  To: Gonglei
  Cc: qemu-devel, virtio-dev, peter.huangpeng, luonengjun, mst,
	stefanha, pbonzini, weidong.huang, mike.caraman, agraf, xin.zeng,
	claudio.fontana, nmorey, vincent.jardin

On Tue, Sep 13, 2016 at 11:52:07AM +0800, Gonglei wrote:
> cryptodev backend is used to realize the active work for
> virtual crypto device. CryptoLegacyHW device is a cryptographic
> hardware device seen by the virtual machine.
> The relationship between cryptodev backend and legacy hadware
> as follow:
>  crypto_legacy_hw device (1)--->(n) cryptodev client backend
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>

> diff --git a/crypto/crypto.c b/crypto/crypto.c
> new file mode 100644
> index 0000000..fbc6497
> --- /dev/null
> +++ b/crypto/crypto.c
> @@ -0,0 +1,171 @@
> +/*
> + * QEMU Crypto Device Implement
> + *
> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> + *
> + * Authors:
> + *    Gonglei <arei.gonglei@huawei.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */

New files are expected to be submitted under the GPLv2+ license,
unless they're header files imported from an external project,
which this is not.

The same license mistake is made across other files / patches
in this series, so I won't repeat the comment - just fix all
of them to be GPLv2+ please.

> +#include "qemu/osdep.h"
> +#include "sysemu/sysemu.h"
> +#include "qapi/error.h"
> +#include "qemu/iov.h"
> +#include "qapi-visit.h"
> +#include "qapi/opts-visitor.h"
> +
> +#include "crypto/crypto.h"
> +#include "qemu/config-file.h"
> +#include "monitor/monitor.h"
> +
> +
> +static QTAILQ_HEAD(, CryptoClientState) crypto_clients;
> +
> +QemuOptsList qemu_cryptodev_opts = {
> +    .name = "cryptodev",
> +    .implied_opt_name = "type",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_cryptodev_opts.head),
> +    .desc = {
> +        /*
> +         * no elements => accept any params
> +         * validation will happen later
> +         */
> +        { /* end of list */ }
> +    },
> +};

No new code should be adding more command line options for
device backends.

Your backend impl should be using QOM to define a object,
which can be created via the -object command line arg,
or object_add monitor command.

If you're not familiar with this, take a look at the
crypto/secret.c file which is a pretty simple example of
how to use QOM to define a new user creatable object.

I'm going to skip reviewing any of the .c code in the
crypto/ dir for now, since that will all change quite a
bit when you switch over to QOM.

> diff --git a/include/crypto/crypto.h b/include/crypto/crypto.h
> new file mode 100644
> index 0000000..f93f6f9
> --- /dev/null
> +++ b/include/crypto/crypto.h

> +#ifndef QCRYPTO_CRYPTO_H__
> +#define QCRYPTO_CRYPTO_H__
> +
> +#include "qemu/queue.h"
> +#include "qapi-types.h"
> +
> +typedef void (CryptoPoll)(CryptoClientState *, bool);
> +typedef void (CryptoCleanup) (CryptoClientState *);
> +typedef void (CryptoClientDestructor)(CryptoClientState *);
> +typedef void (CryptoHWStatusChanged)(CryptoClientState *);
> +
> +typedef struct CryptoClientInfo {
> +    CryptoClientOptionsKind type;
> +    size_t size;
> +
> +    CryptoCleanup *cleanup;
> +    CryptoPoll *poll;
> +    CryptoHWStatusChanged *hw_status_changed;
> +} CryptoClientInfo;
> +
> +struct CryptoClientState {
> +    CryptoClientInfo *info;
> +    int ready;
> +    QTAILQ_ENTRY(CryptoClientState) next;
> +    CryptoClientState *peer;
> +    char *model;
> +    char *name;
> +    char info_str[256];
> +    CryptoClientDestructor *destructor;
> +};
> +
> +int crypto_client_init(QemuOpts *opts, Error **errp);
> +int crypto_init_clients(void);
> +
> +CryptoClientState *new_crypto_client(CryptoClientInfo *info,
> +                                    CryptoClientState *peer,
> +                                    const char *model,
> +                                    const char *name);
> +
> +#endif /* QCRYPTO_CRYPTO_H__ */

For all files in the crypto sub-directory I've been trying
to stick to the strict convention that all methods must
follow the naming scheme  qcrypto_[filename]_XX
eg if you rename this file to cryptodev as requested,
your methods should all follow the naming convention
of 'qcrypto_cryptdev_XXX'.

Similarly all structs would be QCryptoCryptoDevXXX

Finally the .h file should contain full inline documentation.
At start there should be a general description of the file
and its purpose, along with example usages. Then there should
be formal documentation for every single method in the .h
file describing the method semantics, parameters and return
values. See  include/crypto/cipher.h for an example.

> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index b113fcf..82843a9 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -94,5 +94,6 @@ typedef struct SSIBus SSIBus;
>  typedef struct uWireSlave uWireSlave;
>  typedef struct VirtIODevice VirtIODevice;
>  typedef struct Visitor Visitor;
> +typedef struct CryptoClientState CryptoClientState;

Don't add to this file - anything that wants to see
the CryptoClientState typedef should explicitly include
the crypto/cryptodev.h file or whatever the master
definition lives.

> diff --git a/qapi-schema.json b/qapi-schema.json
> index c4f3674..46f7993 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4582,3 +4582,49 @@
>  # Since: 2.7
>  ##
>  { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
> +
> +##
> +# @CryptoLegacyHWOptions
> +#
> +# Create a new cryptographic hardware device.
> +#
> +# @cryptodev: #optional id of -cryptodev to connect to
> +#
> +# @model: #optional device model (Only virtio at present)
> +#
> +# @vectors: #optional number of MSI-x vectors, 0 to disable MSI-X
> +#
> +# Since 2.8
> +##
> +{ 'struct': 'CryptoLegacyHWOptions',
> +  'data': {
> +    '*cryptodev':  'str',
> +    '*model':   'str',
> +    '*vectors': 'uint32' } }
> +
> +##
> +# @CryptoClientOptions
> +#
> +# A discriminated record of crypto device traits.
> +#
> +# Since 2.8
> +#
> +##
> +{ 'union': 'CryptoClientOptions',
> +  'data': {'legacy-hw':   'CryptoLegacyHWOptions'} }
> +
> +##
> +# @Cryptodev
> +#
> +# Captures the configuration of a crypto device.
> +#
> +# @id: identifier for monitor commands.
> +#
> +# @opts: device type specific properties
> +#
> +# Since 2.8
> +##
> +{ 'struct': 'Cryptodev',
> +  'data': {
> +    'id':   'str',
> +    'opts': 'CryptoClientOptions' } }

All crypto related QAPI additions should be in qapi/crypto.json


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 02/15] crypto: introduce crypto queue handler
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 02/15] crypto: introduce crypto queue handler Gonglei
@ 2016-09-13  9:20   ` Daniel P. Berrange
  2016-09-13  9:58     ` Gonglei (Arei)
  2016-09-13 10:58     ` Paolo Bonzini
  0 siblings, 2 replies; 40+ messages in thread
From: Daniel P. Berrange @ 2016-09-13  9:20 UTC (permalink / raw)
  To: Gonglei
  Cc: qemu-devel, virtio-dev, peter.huangpeng, luonengjun, mst,
	stefanha, pbonzini, weidong.huang, mike.caraman, agraf, xin.zeng,
	claudio.fontana, nmorey, vincent.jardin

On Tue, Sep 13, 2016 at 11:52:08AM +0800, Gonglei wrote:
> crypto queue is a gallery used for executing crypto
> operation, which supports both synchronization and
> asynchronization. The thoughts stolen from net/queue.c
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  crypto/Makefile.objs          |   1 +
>  crypto/crypto-queue.c         | 206 ++++++++++++++++++++++++++++++++++++++++++
>  crypto/crypto.c               |  28 ++++++
>  include/crypto/crypto-queue.h |  69 ++++++++++++++
>  include/crypto/crypto.h       |  12 +++
>  5 files changed, 316 insertions(+)
>  create mode 100644 crypto/crypto-queue.c
>  create mode 100644 include/crypto/crypto-queue.h
> 

> diff --git a/include/crypto/crypto-queue.h b/include/crypto/crypto-queue.h
> new file mode 100644
> index 0000000..6fba64d
> --- /dev/null
> +++ b/include/crypto/crypto-queue.h
> @@ -0,0 +1,69 @@
> +/*
> + * Copyright (c) 2003-2008 Fabrice Bellard
> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> + *
> + * Authors:
> + *    Gonglei <arei.gonglei@huawei.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */

Again, wrong license header.

> +#ifndef QEMU_CRYPTO_QUEUE_H
> +#define QEMU_CRYPTO_QUEUE_H
> +
> +#include "qemu-common.h"
> +
> +typedef struct CryptoPacket CryptoPacket;
> +typedef struct CryptoQueue CryptoQueue;
> +typedef struct CryptoPacketBuf CryptoPacketBuf;
> +
> +typedef void (CryptoPacketSent) (CryptoClientState *, int);

As previously, I'd expect naming of

 QCryptoCryptodevPacket
 QCryptoCryptodevPacketBuf
 QCryptoCryptodevQueue

> +
> +
> +/* Returns:
> + *   >0 - success
> + *    0 - queue packet for future redelivery
> + *   <0 - failure (discard packet)
> + */
> +typedef int (CryptoQueueDeliverFunc)(CryptoClientState *sender,
> +                                     unsigned flags,
> +                                     void *header_opaque,
> +                                     void *opaque);
> +
> +CryptoQueue *
> +qemu_new_crypto_queue(CryptoQueueDeliverFunc *deliver, void *opaque);
> +
> +void qemu_crypto_queue_cache(CryptoQueue *queue,
> +                               unsigned flags,
> +                               CryptoClientState *sender,
> +                               void *opaque,
> +                               CryptoPacketSent *sent_cb);
> +
> +void qemu_del_crypto_queue(CryptoQueue *queue);
> +
> +int qemu_crypto_queue_send(CryptoQueue *queue,
> +                                unsigned flags,
> +                                CryptoClientState *sender,
> +                                void *opaque,
> +                                CryptoPacketSent *sent_cb);
> +
> +void qemu_crypto_queue_purge(CryptoQueue *queue, CryptoClientState *from);
> +bool qemu_crypto_queue_flush(CryptoQueue *queue);

And naming of

  qcrypto_cryptodev_queue_purge
  qcrypto_cryptodev_queue_flush
  etc.

Also missing docs for this file

> +#endif /* QEMU_CRYPTO_QUEUE_H */
> diff --git a/include/crypto/crypto.h b/include/crypto/crypto.h
> index f93f6f9..46b3b9e 100644
> --- a/include/crypto/crypto.h
> +++ b/include/crypto/crypto.h
> @@ -29,6 +29,8 @@
>  
>  #include "qemu/queue.h"
>  #include "qapi-types.h"
> +#include "crypto/crypto-queue.h"
> +
>  
>  typedef void (CryptoPoll)(CryptoClientState *, bool);
>  typedef void (CryptoCleanup) (CryptoClientState *);
> @@ -52,6 +54,8 @@ struct CryptoClientState {
>      char *model;
>      char *name;
>      char info_str[256];
> +    CryptoQueue *incoming_queue;
> +    unsigned int queue_index;
>      CryptoClientDestructor *destructor;
>  };
>  
> @@ -62,5 +66,13 @@ CryptoClientState *new_crypto_client(CryptoClientInfo *info,
>                                      CryptoClientState *peer,
>                                      const char *model,
>                                      const char *name);
> +int qemu_deliver_crypto_packet(CryptoClientState *sender,
> +                              unsigned flags,
> +                              void *header_opqaue,
> +                              void *opaque);
> +int qemu_send_crypto_packet_async(CryptoClientState *sender,
> +                                unsigned flags,
> +                                void *opaque,
> +                                CryptoPacketSent *sent_cb);

Missing docs for these API additions.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 03/15] crypto: add cryptoLegacyHW stuff
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 03/15] crypto: add cryptoLegacyHW stuff Gonglei
@ 2016-09-13  9:22   ` Daniel P. Berrange
  2016-09-13 10:05     ` Gonglei (Arei)
  0 siblings, 1 reply; 40+ messages in thread
From: Daniel P. Berrange @ 2016-09-13  9:22 UTC (permalink / raw)
  To: Gonglei
  Cc: qemu-devel, virtio-dev, peter.huangpeng, luonengjun, mst,
	stefanha, pbonzini, weidong.huang, mike.caraman, agraf, xin.zeng,
	claudio.fontana, nmorey, vincent.jardin

On Tue, Sep 13, 2016 at 11:52:09AM +0800, Gonglei wrote:
> In previous patch, we define CryptoLegacyHWOptions in
> qapi-schema.json. we introduce the new/delete funciton
> about crypto legacy hardware device.

Isn't virtio-crypto / cryptodev an entirely new specification ?
I'm surprised to be seeing something completely new described
as "legacy". Can you explain this in more detail.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 05/15] crypto: add cryptodev-linux as a cryptodev backend
  2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 05/15] crypto: add cryptodev-linux as a cryptodev backend Gonglei
@ 2016-09-13  9:23   ` Daniel P. Berrange
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel P. Berrange @ 2016-09-13  9:23 UTC (permalink / raw)
  To: Gonglei
  Cc: qemu-devel, virtio-dev, peter.huangpeng, luonengjun, mst,
	stefanha, pbonzini, weidong.huang, mike.caraman, agraf, xin.zeng,
	claudio.fontana, nmorey, vincent.jardin

On Tue, Sep 13, 2016 at 11:52:11AM +0800, Gonglei wrote:
> Cryptodev-linux is a device that allows access to Linux
> kernel cryptographic drivers; thus allowing of userspace
> applications to take advantage of hardware accelerators.
> Cryptodev-linux is implemented as a standalone module
> that requires no dependencies other than a stock linux kernel.
> 
> The Cryptodev-linux project website is:
>   http://cryptodev-linux.org/

As mentioned in the cover letter, NACK to supporting a module
that is out of tree from Linux, and shows no sign of ever being
accepted in to tree.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation
  2016-09-13  8:57 ` [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Daniel P. Berrange
@ 2016-09-13  9:45   ` Gonglei (Arei)
  2016-09-13  9:54     ` Daniel P. Berrange
  0 siblings, 1 reply; 40+ messages in thread
From: Gonglei (Arei) @ 2016-09-13  9:45 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: qemu-devel, virtio-dev, Huangpeng (Peter),
	Luonengjun, mst, stefanha, pbonzini, Huangweidong (C),
	mike.caraman, agraf, xin.zeng, Claudio Fontana, nmorey,
	vincent.jardin

Hi Daniel,

Thanks for your comments fristly, please see my embedded reply.

Regards,
-Gonglei


> -----Original Message-----
> From: Daniel P. Berrange [mailto:berrange@redhat.com]
> Sent: Tuesday, September 13, 2016 4:58 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org; Huangpeng
> (Peter); Luonengjun; mst@redhat.com; stefanha@redhat.com;
> pbonzini@redhat.com; Huangweidong (C); mike.caraman@nxp.com;
> agraf@suse.de; xin.zeng@intel.com; Claudio Fontana; nmorey@kalray.eu;
> vincent.jardin@6wind.com
> Subject: Re: [PATCH v2 00/15] virtio-crypto: introduce framework and device
> emulation
> 
> On Tue, Sep 13, 2016 at 11:52:06AM +0800, Gonglei wrote:
> > Changes since v1:
> >  - rmmove mixed endian-ness handler for virtio-crypto device, just
> >    use little-endian. [mst]
> >  - add sg list support according virtio-crypto spec v10 (will be posted soon).
> >  - fix a memory leak in session handler.
> >  - add a feature page link in qemu.org
> (http://qemu-project.org/Features/VirtioCrypto)
> >  - fix some trivial problems, sush as 's/Since 2.7/Since 2.8/g' in
> qapi-schema.json
> >  - rebase the latest qemu master tree.
> >
> > Please review, thanks!
> >
> > This patch series realize the framework and emulation of a new
> > virtio crypto device, which is similar with virtio net device.
> >
> >  - I introduce the cryptodev backend as the client of virtio crypto device
> >    which can be realized by different methods, such as cryptodev-linux in my
> series,
> >    vhost-crypto kernel module, vhost-user etc.
> >  - The patch set abides by the virtio crypto speccification.
> >  - The virtio crypto support symmetric algorithms (including CIPHER and
> algorithm chainning)
> >    at present, except HASH, MAC and AEAD services.
> >  - unsupport hot plug/unplug cryptodev client at this moment.
> >
> > Cryptodev-linux is a device that allows access to Linux kernel cryptographic
> drivers;
> > thus allowing of userspace applications to take advantage of hardware
> accelerators.
> > It can be found here:
> >
> >  http://cryptodev-linux.org/
> >
> > (please use the latest version)
> >
> > To use the cryptodev-linux as the client, the cryptodev.ko should be insert on
> the host.
> >
> >  # enter cryptodev-linux module root directory, then
> >  make;make install
> 
> 
> The cryptodev kernel module is not upstream and shows no sign of
> ever being likely to be accepted & merged upstream. On that basis,
> support for it in QEMU has a firm NACK from me, as trying to support
> out of tree kernel modules long term never ends well. This is
> particularly bad because it appears to be the only impl backend
> you've provided in this series.
> 

OK, I agree with you :)  But if we support multiple backends, can
we keep cryptodev-linux module as one option?

> IMHO, the first default backend should ought to use the internal
> QEMU crypto APIs for its impl, which delegate to nettle/gcrypt,
> which in turn can take care of using the kernel for acceleration
> if they so choose.
> 
Will work on this later.

> > then configuring QEMU shows:
> >
> >  [...]
> >  jemalloc support  no
> >  avx2 optimization no
> >  cryptodev-linux support yes
> >
> > QEMU can then be started using the following parameters:
> >
> > qemu-system-x86_64 \
> >     [...] \
> >         -cryptodev type=cryptodev-linux,id=cryptodev0 \
> >         -device virtio-crypto-pci,id=crypto0,cryptodev=cryptodev0 \
> >     [...]
> >
> > The front-end linux kernel driver (Experimental at present) is publicly
> accessible from:
> >
> >    https://github.com/gongleiarei/virtio-crypto-linux-driver.git
> >
> > After insmod virtio-crypto.ko, you also can use cryptodev-linux test the crypto
> function
> > in the guest. For example:
> 
> 
> >
> > linux-guest:/home/gonglei/cryptodev-linux/tests # ./cipher -
> > requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver
> virtio_crypto_aes_cbc
> > AES Test passed
> > requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver
> virtio_crypto_aes_cbc
> > requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver
> virtio_crypto_aes_cbc
> > Test passed
> >
> > QEMU code also can be accessible from:
> >
> >  https://github.com/gongleiarei/qemu.git
> >
> >  branch virtio-crypto
> >
> > For more information, please see:
> >  http://qemu-project.org/Features/VirtioCrypto
> >
> >
> > Gonglei (15):
> >   crypto: introduce cryptodev backend and crypto legacy hardware
> >   crypto: introduce crypto queue handler
> >   crypto: add cryptoLegacyHW stuff
> >   crypto: add symetric algorithms support
> >   crypto: add cryptodev-linux as a cryptodev backend
> >   crypto: add internal handle logic layer
> >   virtio-crypto: introduce virtio-crypto.h
> >   virtio-crypto-pci: add virtio crypto pci support
> >   virtio-crypto: add virtio crypto realization
> >   virtio-crypto: set capacity of crypto legacy hardware
> >   virtio-crypto: add control queue handler
> >   virtio-crypto: add destroy session logic
> >   virtio-crypto: get correct input data address for each request
> >   virtio-crypto: add data virtqueue processing handler
> >   virtio-crypto: support scatter gather list
> >
> >  configure                                      |   16 +
> >  crypto/Makefile.objs                           |    3 +
> >  crypto/crypto-queue.c                          |  206 +++++
> >  crypto/crypto.c                                |  378 +++++++++
> 
> As a general point, please don't take the plain 'crypto'
> namespace - that's way too generic a term.
> 
Ok, I'll use 'cryptodev' as the prefix in the following series.

> I'd expect to see 'cryptodev.c' and 'cryptodev-queue.c'
> really, since these files appear specific to the cryptodev
> 
OK, it makes senses.

> >  crypto/cryptodev-linux.c                       |  419 ++++++++++
> >  hw/core/qdev-properties-system.c               |   86 ++
> >  hw/virtio/Makefile.objs                        |    3 +-
> >  hw/virtio/virtio-crypto-pci.c                  |   71 ++
> >  hw/virtio/virtio-crypto.c                      | 1013
> ++++++++++++++++++++++++
> >  hw/virtio/virtio-pci.h                         |   15 +
> >  include/crypto/crypto-clients.h                |   39 +
> 
> I'd expect header file names to match the name of the
> .c file containing the impl. You've not added any
> crypto-clients.c file - if the code is in crypto.c
> then the crypto-clients.h content should be in
> crypto.h too.
> 
Agree.

> >  include/crypto/crypto-queue.h                  |   69 ++
> >  include/crypto/crypto.h                        |  192 +++++
> 
> Again, cryptodev.h and crypto-queue.h are preferrable
> 
OK.

> 
> Regards,
> Daniel
> --
> |: http://berrange.com      -o-
> http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-
> http://virt-manager.org :|
> |: http://autobuild.org       -o-
> http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-
> http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation
  2016-09-13  9:45   ` Gonglei (Arei)
@ 2016-09-13  9:54     ` Daniel P. Berrange
  2016-09-13 10:08       ` Gonglei (Arei)
  2016-09-13 10:58       ` Paolo Bonzini
  0 siblings, 2 replies; 40+ messages in thread
From: Daniel P. Berrange @ 2016-09-13  9:54 UTC (permalink / raw)
  To: Gonglei (Arei)
  Cc: qemu-devel, virtio-dev, Huangpeng (Peter),
	Luonengjun, mst, stefanha, pbonzini, Huangweidong (C),
	mike.caraman, agraf, xin.zeng, Claudio Fontana, nmorey,
	vincent.jardin

On Tue, Sep 13, 2016 at 09:45:05AM +0000, Gonglei (Arei) wrote:
> Hi Daniel,
> 
> Thanks for your comments fristly, please see my embedded reply.
> 
> Regards,
> -Gonglei
> 
> 
> > -----Original Message-----
> > From: Daniel P. Berrange [mailto:berrange@redhat.com]
> > Sent: Tuesday, September 13, 2016 4:58 PM
> > To: Gonglei (Arei)
> > Cc: qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org; Huangpeng
> > (Peter); Luonengjun; mst@redhat.com; stefanha@redhat.com;
> > pbonzini@redhat.com; Huangweidong (C); mike.caraman@nxp.com;
> > agraf@suse.de; xin.zeng@intel.com; Claudio Fontana; nmorey@kalray.eu;
> > vincent.jardin@6wind.com
> > Subject: Re: [PATCH v2 00/15] virtio-crypto: introduce framework and device
> > emulation
> > 
> > On Tue, Sep 13, 2016 at 11:52:06AM +0800, Gonglei wrote:
> > > Changes since v1:
> > >  - rmmove mixed endian-ness handler for virtio-crypto device, just
> > >    use little-endian. [mst]
> > >  - add sg list support according virtio-crypto spec v10 (will be posted soon).
> > >  - fix a memory leak in session handler.
> > >  - add a feature page link in qemu.org
> > (http://qemu-project.org/Features/VirtioCrypto)
> > >  - fix some trivial problems, sush as 's/Since 2.7/Since 2.8/g' in
> > qapi-schema.json
> > >  - rebase the latest qemu master tree.
> > >
> > > Please review, thanks!
> > >
> > > This patch series realize the framework and emulation of a new
> > > virtio crypto device, which is similar with virtio net device.
> > >
> > >  - I introduce the cryptodev backend as the client of virtio crypto device
> > >    which can be realized by different methods, such as cryptodev-linux in my
> > series,
> > >    vhost-crypto kernel module, vhost-user etc.
> > >  - The patch set abides by the virtio crypto speccification.
> > >  - The virtio crypto support symmetric algorithms (including CIPHER and
> > algorithm chainning)
> > >    at present, except HASH, MAC and AEAD services.
> > >  - unsupport hot plug/unplug cryptodev client at this moment.
> > >
> > > Cryptodev-linux is a device that allows access to Linux kernel cryptographic
> > drivers;
> > > thus allowing of userspace applications to take advantage of hardware
> > accelerators.
> > > It can be found here:
> > >
> > >  http://cryptodev-linux.org/
> > >
> > > (please use the latest version)
> > >
> > > To use the cryptodev-linux as the client, the cryptodev.ko should be insert on
> > the host.
> > >
> > >  # enter cryptodev-linux module root directory, then
> > >  make;make install
> > 
> > 
> > The cryptodev kernel module is not upstream and shows no sign of
> > ever being likely to be accepted & merged upstream. On that basis,
> > support for it in QEMU has a firm NACK from me, as trying to support
> > out of tree kernel modules long term never ends well. This is
> > particularly bad because it appears to be the only impl backend
> > you've provided in this series.
> > 
> 
> OK, I agree with you :)  But if we support multiple backends, can
> we keep cryptodev-linux module as one option?

I'm personally against any support for out of tree kernel modules
in QEMU, regardless of whether QEMU also implements alternative
backends, unless there is a strong sign that the module in question
is on the verge of being accepted into mainline Linux. That does
not seem to be the case there - mainline settled on AF_ALG as the
only supported approach AFAICT.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto legacy hardware
  2016-09-13  9:13   ` Daniel P. Berrange
@ 2016-09-13  9:55     ` Gonglei (Arei)
  2016-09-13  9:59       ` Daniel P. Berrange
  2016-09-13 10:50     ` Paolo Bonzini
  1 sibling, 1 reply; 40+ messages in thread
From: Gonglei (Arei) @ 2016-09-13  9:55 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: qemu-devel, virtio-dev, Huangpeng (Peter),
	Luonengjun, mst, stefanha, pbonzini, Huangweidong (C),
	mike.caraman, agraf, xin.zeng, Claudio Fontana, nmorey,
	vincent.jardin


>
> From: Daniel P. Berrange [mailto:berrange@redhat.com]
> Sent: Tuesday, September 13, 2016 5:14 PM
> Subject: Re: [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto
> legacy hardware
> 
> On Tue, Sep 13, 2016 at 11:52:07AM +0800, Gonglei wrote:
> > cryptodev backend is used to realize the active work for
> > virtual crypto device. CryptoLegacyHW device is a cryptographic
> > hardware device seen by the virtual machine.
> > The relationship between cryptodev backend and legacy hadware
> > as follow:
> >  crypto_legacy_hw device (1)--->(n) cryptodev client backend
> >
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> 
> > diff --git a/crypto/crypto.c b/crypto/crypto.c
> > new file mode 100644
> > index 0000000..fbc6497
> > --- /dev/null
> > +++ b/crypto/crypto.c
> > @@ -0,0 +1,171 @@
> > +/*
> > + * QEMU Crypto Device Implement
> > + *
> > + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> > + *
> > + * Authors:
> > + *    Gonglei <arei.gonglei@huawei.com>
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> > + * of this software and associated documentation files (the "Software"), to
> deal
> > + * in the Software without restriction, including without limitation the rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> > + * copies of the Software, and to permit persons to whom the Software is
> > + * furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING FROM,
> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> OTHER DEALINGS IN
> > + * THE SOFTWARE.
> > + */
> 
> New files are expected to be submitted under the GPLv2+ license,
> unless they're header files imported from an external project,
> which this is not.
> 
> The same license mistake is made across other files / patches
> in this series, so I won't repeat the comment - just fix all
> of them to be GPLv2+ please.
> 
> > +#include "qemu/osdep.h"
> > +#include "sysemu/sysemu.h"
> > +#include "qapi/error.h"
> > +#include "qemu/iov.h"
> > +#include "qapi-visit.h"
> > +#include "qapi/opts-visitor.h"
> > +
> > +#include "crypto/crypto.h"
> > +#include "qemu/config-file.h"
> > +#include "monitor/monitor.h"
> > +
> > +
> > +static QTAILQ_HEAD(, CryptoClientState) crypto_clients;
> > +
> > +QemuOptsList qemu_cryptodev_opts = {
> > +    .name = "cryptodev",
> > +    .implied_opt_name = "type",
> > +    .head = QTAILQ_HEAD_INITIALIZER(qemu_cryptodev_opts.head),
> > +    .desc = {
> > +        /*
> > +         * no elements => accept any params
> > +         * validation will happen later
> > +         */
> > +        { /* end of list */ }
> > +    },
> > +};
> 
> No new code should be adding more command line options for
> device backends.
> 
> Your backend impl should be using QOM to define a object,
> which can be created via the -object command line arg,
> or object_add monitor command.
> 
Any backgrounds about this rule? Maybe I missed something.

> If you're not familiar with this, take a look at the
> crypto/secret.c file which is a pretty simple example of
> how to use QOM to define a new user creatable object.
> 
OK, thanks.

> I'm going to skip reviewing any of the .c code in the
> crypto/ dir for now, since that will all change quite a
> bit when you switch over to QOM.
> 
OK.

> > diff --git a/include/crypto/crypto.h b/include/crypto/crypto.h
> > new file mode 100644
> > index 0000000..f93f6f9
> > --- /dev/null
> > +++ b/include/crypto/crypto.h
> 
> > +#ifndef QCRYPTO_CRYPTO_H__
> > +#define QCRYPTO_CRYPTO_H__
> > +
> > +#include "qemu/queue.h"
> > +#include "qapi-types.h"
> > +
> > +typedef void (CryptoPoll)(CryptoClientState *, bool);
> > +typedef void (CryptoCleanup) (CryptoClientState *);
> > +typedef void (CryptoClientDestructor)(CryptoClientState *);
> > +typedef void (CryptoHWStatusChanged)(CryptoClientState *);
> > +
> > +typedef struct CryptoClientInfo {
> > +    CryptoClientOptionsKind type;
> > +    size_t size;
> > +
> > +    CryptoCleanup *cleanup;
> > +    CryptoPoll *poll;
> > +    CryptoHWStatusChanged *hw_status_changed;
> > +} CryptoClientInfo;
> > +
> > +struct CryptoClientState {
> > +    CryptoClientInfo *info;
> > +    int ready;
> > +    QTAILQ_ENTRY(CryptoClientState) next;
> > +    CryptoClientState *peer;
> > +    char *model;
> > +    char *name;
> > +    char info_str[256];
> > +    CryptoClientDestructor *destructor;
> > +};
> > +
> > +int crypto_client_init(QemuOpts *opts, Error **errp);
> > +int crypto_init_clients(void);
> > +
> > +CryptoClientState *new_crypto_client(CryptoClientInfo *info,
> > +                                    CryptoClientState *peer,
> > +                                    const char *model,
> > +                                    const char *name);
> > +
> > +#endif /* QCRYPTO_CRYPTO_H__ */
> 
> For all files in the crypto sub-directory I've been trying
> to stick to the strict convention that all methods must
> follow the naming scheme  qcrypto_[filename]_XX
> eg if you rename this file to cryptodev as requested,
> your methods should all follow the naming convention
> of 'qcrypto_cryptdev_XXX'.
> 
> Similarly all structs would be QCryptoCryptoDevXXX
> 
OK.

> Finally the .h file should contain full inline documentation.
> At start there should be a general description of the file
> and its purpose, along with example usages. Then there should
> be formal documentation for every single method in the .h
> file describing the method semantics, parameters and return
> values. See  include/crypto/cipher.h for an example.
> 
OK, make senses to me.

> > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> > index b113fcf..82843a9 100644
> > --- a/include/qemu/typedefs.h
> > +++ b/include/qemu/typedefs.h
> > @@ -94,5 +94,6 @@ typedef struct SSIBus SSIBus;
> >  typedef struct uWireSlave uWireSlave;
> >  typedef struct VirtIODevice VirtIODevice;
> >  typedef struct Visitor Visitor;
> > +typedef struct CryptoClientState CryptoClientState;
> 
> Don't add to this file - anything that wants to see
> the CryptoClientState typedef should explicitly include
> the crypto/cryptodev.h file or whatever the master
> definition lives.
> 
OK.

> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index c4f3674..46f7993 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -4582,3 +4582,49 @@
> >  # Since: 2.7
> >  ##
> >  { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
> > +
> > +##
> > +# @CryptoLegacyHWOptions
> > +#
> > +# Create a new cryptographic hardware device.
> > +#
> > +# @cryptodev: #optional id of -cryptodev to connect to
> > +#
> > +# @model: #optional device model (Only virtio at present)
> > +#
> > +# @vectors: #optional number of MSI-x vectors, 0 to disable MSI-X
> > +#
> > +# Since 2.8
> > +##
> > +{ 'struct': 'CryptoLegacyHWOptions',
> > +  'data': {
> > +    '*cryptodev':  'str',
> > +    '*model':   'str',
> > +    '*vectors': 'uint32' } }
> > +
> > +##
> > +# @CryptoClientOptions
> > +#
> > +# A discriminated record of crypto device traits.
> > +#
> > +# Since 2.8
> > +#
> > +##
> > +{ 'union': 'CryptoClientOptions',
> > +  'data': {'legacy-hw':   'CryptoLegacyHWOptions'} }
> > +
> > +##
> > +# @Cryptodev
> > +#
> > +# Captures the configuration of a crypto device.
> > +#
> > +# @id: identifier for monitor commands.
> > +#
> > +# @opts: device type specific properties
> > +#
> > +# Since 2.8
> > +##
> > +{ 'struct': 'Cryptodev',
> > +  'data': {
> > +    'id':   'str',
> > +    'opts': 'CryptoClientOptions' } }
> 
> All crypto related QAPI additions should be in qapi/crypto.json
> 
OK.

> 
> Regards,
> Daniel
> --
> |: http://berrange.com      -o-
> http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-
> http://virt-manager.org :|
> |: http://autobuild.org       -o-
> http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-
> http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 02/15] crypto: introduce crypto queue handler
  2016-09-13  9:20   ` Daniel P. Berrange
@ 2016-09-13  9:58     ` Gonglei (Arei)
  2016-09-13 10:58     ` Paolo Bonzini
  1 sibling, 0 replies; 40+ messages in thread
From: Gonglei (Arei) @ 2016-09-13  9:58 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: qemu-devel, virtio-dev, Huangpeng (Peter),
	Luonengjun, mst, stefanha, pbonzini, Huangweidong (C),
	mike.caraman, agraf, xin.zeng, Claudio Fontana, nmorey,
	vincent.jardin

Hi,

All comments are accepted :)


Regards,
-Gonglei


> -----Original Message-----
> From: Daniel P. Berrange [mailto:berrange@redhat.com]
> Sent: Tuesday, September 13, 2016 5:21 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org; Huangpeng
> (Peter); Luonengjun; mst@redhat.com; stefanha@redhat.com;
> pbonzini@redhat.com; Huangweidong (C); mike.caraman@nxp.com;
> agraf@suse.de; xin.zeng@intel.com; Claudio Fontana; nmorey@kalray.eu;
> vincent.jardin@6wind.com
> Subject: Re: [PATCH v2 02/15] crypto: introduce crypto queue handler
> 
> On Tue, Sep 13, 2016 at 11:52:08AM +0800, Gonglei wrote:
> > crypto queue is a gallery used for executing crypto
> > operation, which supports both synchronization and
> > asynchronization. The thoughts stolen from net/queue.c
> >
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> >  crypto/Makefile.objs          |   1 +
> >  crypto/crypto-queue.c         | 206
> ++++++++++++++++++++++++++++++++++++++++++
> >  crypto/crypto.c               |  28 ++++++
> >  include/crypto/crypto-queue.h |  69 ++++++++++++++
> >  include/crypto/crypto.h       |  12 +++
> >  5 files changed, 316 insertions(+)
> >  create mode 100644 crypto/crypto-queue.c
> >  create mode 100644 include/crypto/crypto-queue.h
> >
> 
> > diff --git a/include/crypto/crypto-queue.h b/include/crypto/crypto-queue.h
> > new file mode 100644
> > index 0000000..6fba64d
> > --- /dev/null
> > +++ b/include/crypto/crypto-queue.h
> > @@ -0,0 +1,69 @@
> > +/*
> > + * Copyright (c) 2003-2008 Fabrice Bellard
> > + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> > + *
> > + * Authors:
> > + *    Gonglei <arei.gonglei@huawei.com>
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> > + * of this software and associated documentation files (the "Software"), to
> deal
> > + * in the Software without restriction, including without limitation the rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> > + * copies of the Software, and to permit persons to whom the Software is
> > + * furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING FROM,
> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> OTHER DEALINGS IN
> > + * THE SOFTWARE.
> > + */
> 
> Again, wrong license header.
> 
> > +#ifndef QEMU_CRYPTO_QUEUE_H
> > +#define QEMU_CRYPTO_QUEUE_H
> > +
> > +#include "qemu-common.h"
> > +
> > +typedef struct CryptoPacket CryptoPacket;
> > +typedef struct CryptoQueue CryptoQueue;
> > +typedef struct CryptoPacketBuf CryptoPacketBuf;
> > +
> > +typedef void (CryptoPacketSent) (CryptoClientState *, int);
> 
> As previously, I'd expect naming of
> 
>  QCryptoCryptodevPacket
>  QCryptoCryptodevPacketBuf
>  QCryptoCryptodevQueue
> 
> > +
> > +
> > +/* Returns:
> > + *   >0 - success
> > + *    0 - queue packet for future redelivery
> > + *   <0 - failure (discard packet)
> > + */
> > +typedef int (CryptoQueueDeliverFunc)(CryptoClientState *sender,
> > +                                     unsigned flags,
> > +                                     void *header_opaque,
> > +                                     void *opaque);
> > +
> > +CryptoQueue *
> > +qemu_new_crypto_queue(CryptoQueueDeliverFunc *deliver, void *opaque);
> > +
> > +void qemu_crypto_queue_cache(CryptoQueue *queue,
> > +                               unsigned flags,
> > +                               CryptoClientState *sender,
> > +                               void *opaque,
> > +                               CryptoPacketSent *sent_cb);
> > +
> > +void qemu_del_crypto_queue(CryptoQueue *queue);
> > +
> > +int qemu_crypto_queue_send(CryptoQueue *queue,
> > +                                unsigned flags,
> > +                                CryptoClientState *sender,
> > +                                void *opaque,
> > +                                CryptoPacketSent *sent_cb);
> > +
> > +void qemu_crypto_queue_purge(CryptoQueue *queue, CryptoClientState
> *from);
> > +bool qemu_crypto_queue_flush(CryptoQueue *queue);
> 
> And naming of
> 
>   qcrypto_cryptodev_queue_purge
>   qcrypto_cryptodev_queue_flush
>   etc.
> 
> Also missing docs for this file
> 
> > +#endif /* QEMU_CRYPTO_QUEUE_H */
> > diff --git a/include/crypto/crypto.h b/include/crypto/crypto.h
> > index f93f6f9..46b3b9e 100644
> > --- a/include/crypto/crypto.h
> > +++ b/include/crypto/crypto.h
> > @@ -29,6 +29,8 @@
> >
> >  #include "qemu/queue.h"
> >  #include "qapi-types.h"
> > +#include "crypto/crypto-queue.h"
> > +
> >
> >  typedef void (CryptoPoll)(CryptoClientState *, bool);
> >  typedef void (CryptoCleanup) (CryptoClientState *);
> > @@ -52,6 +54,8 @@ struct CryptoClientState {
> >      char *model;
> >      char *name;
> >      char info_str[256];
> > +    CryptoQueue *incoming_queue;
> > +    unsigned int queue_index;
> >      CryptoClientDestructor *destructor;
> >  };
> >
> > @@ -62,5 +66,13 @@ CryptoClientState
> *new_crypto_client(CryptoClientInfo *info,
> >                                      CryptoClientState *peer,
> >                                      const char *model,
> >                                      const char *name);
> > +int qemu_deliver_crypto_packet(CryptoClientState *sender,
> > +                              unsigned flags,
> > +                              void *header_opqaue,
> > +                              void *opaque);
> > +int qemu_send_crypto_packet_async(CryptoClientState *sender,
> > +                                unsigned flags,
> > +                                void *opaque,
> > +                                CryptoPacketSent *sent_cb);
> 
> Missing docs for these API additions.
> 
> 
> Regards,
> Daniel
> --
> |: http://berrange.com      -o-
> http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-
> http://virt-manager.org :|
> |: http://autobuild.org       -o-
> http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-
> http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto legacy hardware
  2016-09-13  9:55     ` Gonglei (Arei)
@ 2016-09-13  9:59       ` Daniel P. Berrange
  2016-09-13 10:06         ` Gonglei (Arei)
  0 siblings, 1 reply; 40+ messages in thread
From: Daniel P. Berrange @ 2016-09-13  9:59 UTC (permalink / raw)
  To: Gonglei (Arei)
  Cc: qemu-devel, virtio-dev, Huangpeng (Peter),
	Luonengjun, mst, stefanha, pbonzini, Huangweidong (C),
	mike.caraman, agraf, xin.zeng, Claudio Fontana, nmorey,
	vincent.jardin

On Tue, Sep 13, 2016 at 09:55:27AM +0000, Gonglei (Arei) wrote:
> 
> >
> > From: Daniel P. Berrange [mailto:berrange@redhat.com]
> > Sent: Tuesday, September 13, 2016 5:14 PM
> > Subject: Re: [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto
> > legacy hardware
> > 
> > On Tue, Sep 13, 2016 at 11:52:07AM +0800, Gonglei wrote:
> > > cryptodev backend is used to realize the active work for
> > > virtual crypto device. CryptoLegacyHW device is a cryptographic
> > > hardware device seen by the virtual machine.
> > > The relationship between cryptodev backend and legacy hadware
> > > as follow:
> > >  crypto_legacy_hw device (1)--->(n) cryptodev client backend
> > >
> > > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > 
> > > diff --git a/crypto/crypto.c b/crypto/crypto.c
> > > new file mode 100644
> > > index 0000000..fbc6497
> > > --- /dev/null
> > > +++ b/crypto/crypto.c
> > > @@ -0,0 +1,171 @@
> > > +/*
> > > + * QEMU Crypto Device Implement
> > > + *
> > > + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> > > + *
> > > + * Authors:
> > > + *    Gonglei <arei.gonglei@huawei.com>
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person obtaining a
> > copy
> > > + * of this software and associated documentation files (the "Software"), to
> > deal
> > > + * in the Software without restriction, including without limitation the rights
> > > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> > > + * copies of the Software, and to permit persons to whom the Software is
> > > + * furnished to do so, subject to the following conditions:
> > > + *
> > > + * The above copyright notice and this permission notice shall be included in
> > > + * all copies or substantial portions of the Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> > EVENT SHALL
> > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> > DAMAGES OR OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > ARISING FROM,
> > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > OTHER DEALINGS IN
> > > + * THE SOFTWARE.
> > > + */
> > 
> > New files are expected to be submitted under the GPLv2+ license,
> > unless they're header files imported from an external project,
> > which this is not.
> > 
> > The same license mistake is made across other files / patches
> > in this series, so I won't repeat the comment - just fix all
> > of them to be GPLv2+ please.
> > 
> > > +#include "qemu/osdep.h"
> > > +#include "sysemu/sysemu.h"
> > > +#include "qapi/error.h"
> > > +#include "qemu/iov.h"
> > > +#include "qapi-visit.h"
> > > +#include "qapi/opts-visitor.h"
> > > +
> > > +#include "crypto/crypto.h"
> > > +#include "qemu/config-file.h"
> > > +#include "monitor/monitor.h"
> > > +
> > > +
> > > +static QTAILQ_HEAD(, CryptoClientState) crypto_clients;
> > > +
> > > +QemuOptsList qemu_cryptodev_opts = {
> > > +    .name = "cryptodev",
> > > +    .implied_opt_name = "type",
> > > +    .head = QTAILQ_HEAD_INITIALIZER(qemu_cryptodev_opts.head),
> > > +    .desc = {
> > > +        /*
> > > +         * no elements => accept any params
> > > +         * validation will happen later
> > > +         */
> > > +        { /* end of list */ }
> > > +    },
> > > +};
> > 
> > No new code should be adding more command line options for
> > device backends.
> > 
> > Your backend impl should be using QOM to define a object,
> > which can be created via the -object command line arg,
> > or object_add monitor command.
> > 
> Any backgrounds about this rule? Maybe I missed something.

I don't think it has been documented anywhere explicitly.
The general idea of using QOM though is that it avoids the
need for you to re-invent the wheel for command line parsing
and gives you QMP/HMP based hotplug for free at the same
time. It was first used for the RNG and hostmem backends,
and more recently my tls-creds and secrets objects.

A very very very long term goal would be for exinsting backends
to be converted to be QOM based too, but for now we're just aiming
for any new backends to use QOM from the start.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 03/15] crypto: add cryptoLegacyHW stuff
  2016-09-13  9:22   ` Daniel P. Berrange
@ 2016-09-13 10:05     ` Gonglei (Arei)
  2016-09-13 10:08       ` Daniel P. Berrange
  0 siblings, 1 reply; 40+ messages in thread
From: Gonglei (Arei) @ 2016-09-13 10:05 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: qemu-devel, virtio-dev, Huangpeng (Peter),
	Luonengjun, mst, stefanha, pbonzini, Huangweidong (C),
	mike.caraman, agraf, xin.zeng, Claudio Fontana, nmorey,
	vincent.jardin



> -----Original Message-----
> From: Daniel P. Berrange [mailto:berrange@redhat.com]
> Sent: Tuesday, September 13, 2016 5:23 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org; Huangpeng
> (Peter); Luonengjun; mst@redhat.com; stefanha@redhat.com;
> pbonzini@redhat.com; Huangweidong (C); mike.caraman@nxp.com;
> agraf@suse.de; xin.zeng@intel.com; Claudio Fontana; nmorey@kalray.eu;
> vincent.jardin@6wind.com
> Subject: Re: [PATCH v2 03/15] crypto: add cryptoLegacyHW stuff
> 
> On Tue, Sep 13, 2016 at 11:52:09AM +0800, Gonglei wrote:
> > In previous patch, we define CryptoLegacyHWOptions in
> > qapi-schema.json. we introduce the new/delete funciton
> > about crypto legacy hardware device.
> 
> Isn't virtio-crypto / cryptodev an entirely new specification ?

Yes, it's an complete new device.

> I'm surprised to be seeing something completely new described
> as "legacy". Can you explain this in more detail.
> 
Because there are two kind of crypto device, one is virtio-crypto which is
a virtual crypto device (And virtio-crypto device is a real crypto device seen by the guest). 
The other is cryptodev backend device.  I have no good idea to name the kind
of virtio-crypto device, so named to legacy crypto device refer to Net stuff.

Do you have any suggestion? Thanks!

Regards,
-Gonglei

> Regards,
> Daniel
> --
> |: http://berrange.com      -o-
> http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-
> http://virt-manager.org :|
> |: http://autobuild.org       -o-
> http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-
> http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto legacy hardware
  2016-09-13  9:59       ` Daniel P. Berrange
@ 2016-09-13 10:06         ` Gonglei (Arei)
  0 siblings, 0 replies; 40+ messages in thread
From: Gonglei (Arei) @ 2016-09-13 10:06 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: qemu-devel, virtio-dev, Huangpeng (Peter),
	Luonengjun, mst, stefanha, pbonzini, Huangweidong (C),
	mike.caraman, agraf, xin.zeng, Claudio Fontana, nmorey,
	vincent.jardin


> -----Original Message-----
> From: Daniel P. Berrange [mailto:berrange@redhat.com]
> Sent: Tuesday, September 13, 2016 5:59 PM
> Subject: Re: [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto
> legacy hardware
> 
> On Tue, Sep 13, 2016 at 09:55:27AM +0000, Gonglei (Arei) wrote:
> >
> > >
> > > From: Daniel P. Berrange [mailto:berrange@redhat.com]
> > > Sent: Tuesday, September 13, 2016 5:14 PM
> > > Subject: Re: [PATCH v2 01/15] crypto: introduce cryptodev backend and
> crypto
> > > legacy hardware
> > >
> > > On Tue, Sep 13, 2016 at 11:52:07AM +0800, Gonglei wrote:
> > > > cryptodev backend is used to realize the active work for
> > > > virtual crypto device. CryptoLegacyHW device is a cryptographic
> > > > hardware device seen by the virtual machine.
> > > > The relationship between cryptodev backend and legacy hadware
> > > > as follow:
> > > >  crypto_legacy_hw device (1)--->(n) cryptodev client backend
> > > >
> > > > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > >
> > > > diff --git a/crypto/crypto.c b/crypto/crypto.c
> > > > new file mode 100644
> > > > index 0000000..fbc6497
> > > > --- /dev/null
> > > > +++ b/crypto/crypto.c
> > > > @@ -0,0 +1,171 @@
> > > > +/*
> > > > + * QEMU Crypto Device Implement
> > > > + *
> > > > + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> > > > + *
> > > > + * Authors:
> > > > + *    Gonglei <arei.gonglei@huawei.com>
> > > > + *
> > > > + * Permission is hereby granted, free of charge, to any person obtaining
> a
> > > copy
> > > > + * of this software and associated documentation files (the "Software"),
> to
> > > deal
> > > > + * in the Software without restriction, including without limitation the
> rights
> > > > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> > > > + * copies of the Software, and to permit persons to whom the Software is
> > > > + * furnished to do so, subject to the following conditions:
> > > > + *
> > > > + * The above copyright notice and this permission notice shall be included
> in
> > > > + * all copies or substantial portions of the Software.
> > > > + *
> > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
> KIND,
> > > EXPRESS OR
> > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > MERCHANTABILITY,
> > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
> NO
> > > EVENT SHALL
> > > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> > > DAMAGES OR OTHER
> > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> OTHERWISE,
> > > ARISING FROM,
> > > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > > OTHER DEALINGS IN
> > > > + * THE SOFTWARE.
> > > > + */
> > >
> > > New files are expected to be submitted under the GPLv2+ license,
> > > unless they're header files imported from an external project,
> > > which this is not.
> > >
> > > The same license mistake is made across other files / patches
> > > in this series, so I won't repeat the comment - just fix all
> > > of them to be GPLv2+ please.
> > >
> > > > +#include "qemu/osdep.h"
> > > > +#include "sysemu/sysemu.h"
> > > > +#include "qapi/error.h"
> > > > +#include "qemu/iov.h"
> > > > +#include "qapi-visit.h"
> > > > +#include "qapi/opts-visitor.h"
> > > > +
> > > > +#include "crypto/crypto.h"
> > > > +#include "qemu/config-file.h"
> > > > +#include "monitor/monitor.h"
> > > > +
> > > > +
> > > > +static QTAILQ_HEAD(, CryptoClientState) crypto_clients;
> > > > +
> > > > +QemuOptsList qemu_cryptodev_opts = {
> > > > +    .name = "cryptodev",
> > > > +    .implied_opt_name = "type",
> > > > +    .head = QTAILQ_HEAD_INITIALIZER(qemu_cryptodev_opts.head),
> > > > +    .desc = {
> > > > +        /*
> > > > +         * no elements => accept any params
> > > > +         * validation will happen later
> > > > +         */
> > > > +        { /* end of list */ }
> > > > +    },
> > > > +};
> > >
> > > No new code should be adding more command line options for
> > > device backends.
> > >
> > > Your backend impl should be using QOM to define a object,
> > > which can be created via the -object command line arg,
> > > or object_add monitor command.
> > >
> > Any backgrounds about this rule? Maybe I missed something.
> 
> I don't think it has been documented anywhere explicitly.
> The general idea of using QOM though is that it avoids the
> need for you to re-invent the wheel for command line parsing
> and gives you QMP/HMP based hotplug for free at the same
> time. It was first used for the RNG and hostmem backends,
> and more recently my tls-creds and secrets objects.
> 
> A very very very long term goal would be for exinsting backends
> to be converted to be QOM based too, but for now we're just aiming
> for any new backends to use QOM from the start.
> 
OK, I see. Thanks!


Regards,
-Gonglei

> Regards,
> Daniel
> --
> |: http://berrange.com      -o-
> http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-
> http://virt-manager.org :|
> |: http://autobuild.org       -o-
> http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-
> http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation
  2016-09-13  9:54     ` Daniel P. Berrange
@ 2016-09-13 10:08       ` Gonglei (Arei)
  2016-09-13 10:58       ` Paolo Bonzini
  1 sibling, 0 replies; 40+ messages in thread
From: Gonglei (Arei) @ 2016-09-13 10:08 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: qemu-devel, virtio-dev, Huangpeng (Peter),
	Luonengjun, mst, stefanha, pbonzini, Huangweidong (C),
	mike.caraman, agraf, xin.zeng, Claudio Fontana, nmorey,
	vincent.jardin



> -----Original Message-----
> From: Daniel P. Berrange [mailto:berrange@redhat.com]
> Sent: Tuesday, September 13, 2016 5:54 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org; Huangpeng
> (Peter); Luonengjun; mst@redhat.com; stefanha@redhat.com;
> pbonzini@redhat.com; Huangweidong (C); mike.caraman@nxp.com;
> agraf@suse.de; xin.zeng@intel.com; Claudio Fontana; nmorey@kalray.eu;
> vincent.jardin@6wind.com
> Subject: Re: [PATCH v2 00/15] virtio-crypto: introduce framework and device
> emulation
> 
> On Tue, Sep 13, 2016 at 09:45:05AM +0000, Gonglei (Arei) wrote:
> > Hi Daniel,
> >
> > Thanks for your comments fristly, please see my embedded reply.
> >
> > Regards,
> > -Gonglei
> >
> >
> > > -----Original Message-----
> > > From: Daniel P. Berrange [mailto:berrange@redhat.com]
> > > Sent: Tuesday, September 13, 2016 4:58 PM
> > > To: Gonglei (Arei)
> > > Cc: qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org; Huangpeng
> > > (Peter); Luonengjun; mst@redhat.com; stefanha@redhat.com;
> > > pbonzini@redhat.com; Huangweidong (C); mike.caraman@nxp.com;
> > > agraf@suse.de; xin.zeng@intel.com; Claudio Fontana; nmorey@kalray.eu;
> > > vincent.jardin@6wind.com
> > > Subject: Re: [PATCH v2 00/15] virtio-crypto: introduce framework and device
> > > emulation
> > >
> > > On Tue, Sep 13, 2016 at 11:52:06AM +0800, Gonglei wrote:
> > > > Changes since v1:
> > > >  - rmmove mixed endian-ness handler for virtio-crypto device, just
> > > >    use little-endian. [mst]
> > > >  - add sg list support according virtio-crypto spec v10 (will be posted
> soon).
> > > >  - fix a memory leak in session handler.
> > > >  - add a feature page link in qemu.org
> > > (http://qemu-project.org/Features/VirtioCrypto)
> > > >  - fix some trivial problems, sush as 's/Since 2.7/Since 2.8/g' in
> > > qapi-schema.json
> > > >  - rebase the latest qemu master tree.
> > > >
> > > > Please review, thanks!
> > > >
> > > > This patch series realize the framework and emulation of a new
> > > > virtio crypto device, which is similar with virtio net device.
> > > >
> > > >  - I introduce the cryptodev backend as the client of virtio crypto device
> > > >    which can be realized by different methods, such as cryptodev-linux in
> my
> > > series,
> > > >    vhost-crypto kernel module, vhost-user etc.
> > > >  - The patch set abides by the virtio crypto speccification.
> > > >  - The virtio crypto support symmetric algorithms (including CIPHER and
> > > algorithm chainning)
> > > >    at present, except HASH, MAC and AEAD services.
> > > >  - unsupport hot plug/unplug cryptodev client at this moment.
> > > >
> > > > Cryptodev-linux is a device that allows access to Linux kernel
> cryptographic
> > > drivers;
> > > > thus allowing of userspace applications to take advantage of hardware
> > > accelerators.
> > > > It can be found here:
> > > >
> > > >  http://cryptodev-linux.org/
> > > >
> > > > (please use the latest version)
> > > >
> > > > To use the cryptodev-linux as the client, the cryptodev.ko should be insert
> on
> > > the host.
> > > >
> > > >  # enter cryptodev-linux module root directory, then
> > > >  make;make install
> > >
> > >
> > > The cryptodev kernel module is not upstream and shows no sign of
> > > ever being likely to be accepted & merged upstream. On that basis,
> > > support for it in QEMU has a firm NACK from me, as trying to support
> > > out of tree kernel modules long term never ends well. This is
> > > particularly bad because it appears to be the only impl backend
> > > you've provided in this series.
> > >
> >
> > OK, I agree with you :)  But if we support multiple backends, can
> > we keep cryptodev-linux module as one option?
> 
> I'm personally against any support for out of tree kernel modules
> in QEMU, regardless of whether QEMU also implements alternative
> backends, unless there is a strong sign that the module in question
> is on the verge of being accepted into mainline Linux. That does
> not seem to be the case there - mainline settled on AF_ALG as the
> only supported approach AFAICT.
> 
OK.


Regards,
-Gonglei

> Regards,
> Daniel
> --
> |: http://berrange.com      -o-
> http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-
> http://virt-manager.org :|
> |: http://autobuild.org       -o-
> http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-
> http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 03/15] crypto: add cryptoLegacyHW stuff
  2016-09-13 10:05     ` Gonglei (Arei)
@ 2016-09-13 10:08       ` Daniel P. Berrange
  2016-09-13 10:14         ` Gonglei (Arei)
  0 siblings, 1 reply; 40+ messages in thread
From: Daniel P. Berrange @ 2016-09-13 10:08 UTC (permalink / raw)
  To: Gonglei (Arei)
  Cc: qemu-devel, virtio-dev, Huangpeng (Peter),
	Luonengjun, mst, stefanha, pbonzini, Huangweidong (C),
	mike.caraman, agraf, xin.zeng, Claudio Fontana, nmorey,
	vincent.jardin

On Tue, Sep 13, 2016 at 10:05:01AM +0000, Gonglei (Arei) wrote:
> 
> 
> > -----Original Message-----
> > From: Daniel P. Berrange [mailto:berrange@redhat.com]
> > Sent: Tuesday, September 13, 2016 5:23 PM
> > To: Gonglei (Arei)
> > Cc: qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org; Huangpeng
> > (Peter); Luonengjun; mst@redhat.com; stefanha@redhat.com;
> > pbonzini@redhat.com; Huangweidong (C); mike.caraman@nxp.com;
> > agraf@suse.de; xin.zeng@intel.com; Claudio Fontana; nmorey@kalray.eu;
> > vincent.jardin@6wind.com
> > Subject: Re: [PATCH v2 03/15] crypto: add cryptoLegacyHW stuff
> > 
> > On Tue, Sep 13, 2016 at 11:52:09AM +0800, Gonglei wrote:
> > > In previous patch, we define CryptoLegacyHWOptions in
> > > qapi-schema.json. we introduce the new/delete funciton
> > > about crypto legacy hardware device.
> > 
> > Isn't virtio-crypto / cryptodev an entirely new specification ?
> 
> Yes, it's an complete new device.
> 
> > I'm surprised to be seeing something completely new described
> > as "legacy". Can you explain this in more detail.
> > 
> Because there are two kind of crypto device, one is virtio-crypto which is
> a virtual crypto device (And virtio-crypto device is a real crypto device seen by the guest). 
> The other is cryptodev backend device.  I have no good idea to name the kind
> of virtio-crypto device, so named to legacy crypto device refer to Net stuff.
> 
> Do you have any suggestion? Thanks!

How about just calling it 'Backend' instead of 'Legacy' then ?

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 03/15] crypto: add cryptoLegacyHW stuff
  2016-09-13 10:08       ` Daniel P. Berrange
@ 2016-09-13 10:14         ` Gonglei (Arei)
  0 siblings, 0 replies; 40+ messages in thread
From: Gonglei (Arei) @ 2016-09-13 10:14 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: qemu-devel, virtio-dev, Huangpeng (Peter),
	Luonengjun, mst, stefanha, pbonzini, Huangweidong (C),
	mike.caraman, agraf, xin.zeng, Claudio Fontana, nmorey,
	vincent.jardin


> -----Original Message-----
> From: Daniel P. Berrange [mailto:berrange@redhat.com]
> Sent: Tuesday, September 13, 2016 6:09 PM
> To: Gonglei (Arei)
> Subject: Re: [PATCH v2 03/15] crypto: add cryptoLegacyHW stuff
> 
> On Tue, Sep 13, 2016 at 10:05:01AM +0000, Gonglei (Arei) wrote:
> >
> >
> > > -----Original Message-----
> > > From: Daniel P. Berrange [mailto:berrange@redhat.com]
> > > Sent: Tuesday, September 13, 2016 5:23 PM
> > > To: Gonglei (Arei)
> > > Cc: qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org; Huangpeng
> > > (Peter); Luonengjun; mst@redhat.com; stefanha@redhat.com;
> > > pbonzini@redhat.com; Huangweidong (C); mike.caraman@nxp.com;
> > > agraf@suse.de; xin.zeng@intel.com; Claudio Fontana; nmorey@kalray.eu;
> > > vincent.jardin@6wind.com
> > > Subject: Re: [PATCH v2 03/15] crypto: add cryptoLegacyHW stuff
> > >
> > > On Tue, Sep 13, 2016 at 11:52:09AM +0800, Gonglei wrote:
> > > > In previous patch, we define CryptoLegacyHWOptions in
> > > > qapi-schema.json. we introduce the new/delete funciton
> > > > about crypto legacy hardware device.
> > >
> > > Isn't virtio-crypto / cryptodev an entirely new specification ?
> >
> > Yes, it's an complete new device.
> >
> > > I'm surprised to be seeing something completely new described
> > > as "legacy". Can you explain this in more detail.
> > >
> > Because there are two kind of crypto device, one is virtio-crypto which is
> > a virtual crypto device (And virtio-crypto device is a real crypto device seen by
> the guest).
> > The other is cryptodev backend device.  I have no good idea to name the
> kind
> > of virtio-crypto device, so named to legacy crypto device refer to Net stuff.
> >
> > Do you have any suggestion? Thanks!
> 
> How about just calling it 'Backend' instead of 'Legacy' then ?
> 
It sounds reasonable I think. Because the virtio specification usually say
The backend device.... and The frontend driver....

So let's:

s/cryptoLegacyHW/cryptoBackendHW/g


Regards,
-Gonglei


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto legacy hardware
  2016-09-13  9:13   ` Daniel P. Berrange
  2016-09-13  9:55     ` Gonglei (Arei)
@ 2016-09-13 10:50     ` Paolo Bonzini
  2016-09-13 11:04       ` Daniel P. Berrange
  1 sibling, 1 reply; 40+ messages in thread
From: Paolo Bonzini @ 2016-09-13 10:50 UTC (permalink / raw)
  To: Daniel P. Berrange, Gonglei
  Cc: qemu-devel, virtio-dev, peter.huangpeng, luonengjun, mst,
	stefanha, weidong.huang, mike.caraman, agraf, xin.zeng,
	claudio.fontana, nmorey, vincent.jardin



On 13/09/2016 11:13, Daniel P. Berrange wrote:
> > + * Permission is hereby granted, free of charge, to any person obtaining a copy
> > + * of this software and associated documentation files (the "Software"), to deal
> > + * in the Software without restriction, including without limitation the rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> > + * copies of the Software, and to permit persons to whom the Software is
> > + * furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> > + * THE SOFTWARE.
> > + */
>
> New files are expected to be submitted under the GPLv2+ license,
> unless they're header files imported from an external project,
> which this is not.

This is not true.  New files are expected to be submitted under a
GPLv2+-compatible license, which this one is.

Paolo

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 02/15] crypto: introduce crypto queue handler
  2016-09-13  9:20   ` Daniel P. Berrange
  2016-09-13  9:58     ` Gonglei (Arei)
@ 2016-09-13 10:58     ` Paolo Bonzini
  2016-09-13 11:52       ` [Qemu-devel] [virtio-dev] " Ola Liljedahl
  2016-09-14  1:03       ` Gonglei (Arei)
  1 sibling, 2 replies; 40+ messages in thread
From: Paolo Bonzini @ 2016-09-13 10:58 UTC (permalink / raw)
  To: Daniel P. Berrange, Gonglei
  Cc: qemu-devel, virtio-dev, peter.huangpeng, luonengjun, mst,
	stefanha, weidong.huang, mike.caraman, agraf, xin.zeng,
	claudio.fontana, nmorey, vincent.jardin



On 13/09/2016 11:20, Daniel P. Berrange wrote:
>> > +typedef struct CryptoPacket CryptoPacket;
>> > +typedef struct CryptoQueue CryptoQueue;
>> > +typedef struct CryptoPacketBuf CryptoPacketBuf;
>> > +
>> > +typedef void (CryptoPacketSent) (CryptoClientState *, int);
> As previously, I'd expect naming of
> 
>  QCryptoCryptodevPacket
>  QCryptoCryptodevPacketBuf
>  QCryptoCryptodevQueue
> 

Gonglei,

you are copying a lot of code from network backends.

I am not sure why you would need a queue for virtio-crypto rather than a
direct connection between frontend and backend (and the backend would be
QEMU crypto APIs, like Daniel suggested).

Paolo

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation
  2016-09-13  9:54     ` Daniel P. Berrange
  2016-09-13 10:08       ` Gonglei (Arei)
@ 2016-09-13 10:58       ` Paolo Bonzini
  2016-09-13 11:07         ` Daniel P. Berrange
  1 sibling, 1 reply; 40+ messages in thread
From: Paolo Bonzini @ 2016-09-13 10:58 UTC (permalink / raw)
  To: Daniel P. Berrange, Gonglei (Arei)
  Cc: qemu-devel, virtio-dev, Huangpeng (Peter),
	Luonengjun, mst, stefanha, Huangweidong (C),
	mike.caraman, agraf, xin.zeng, Claudio Fontana, nmorey,
	vincent.jardin



On 13/09/2016 11:54, Daniel P. Berrange wrote:
> > OK, I agree with you :)  But if we support multiple backends, can
> > we keep cryptodev-linux module as one option?
>
> I'm personally against any support for out of tree kernel modules
> in QEMU, regardless of whether QEMU also implements alternative
> backends, unless there is a strong sign that the module in question
> is on the verge of being accepted into mainline Linux. That does
> not seem to be the case there - mainline settled on AF_ALG as the
> only supported approach AFAICT.

Is there any reason to embed knowledge of AF_ALG directly in QEMU,
rather than delegating that to gcrypt/nettle?

Paolo

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto legacy hardware
  2016-09-13 10:50     ` Paolo Bonzini
@ 2016-09-13 11:04       ` Daniel P. Berrange
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel P. Berrange @ 2016-09-13 11:04 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Gonglei, qemu-devel, virtio-dev, peter.huangpeng, luonengjun,
	mst, stefanha, weidong.huang, mike.caraman, agraf, xin.zeng,
	claudio.fontana, nmorey, vincent.jardin

On Tue, Sep 13, 2016 at 12:50:53PM +0200, Paolo Bonzini wrote:
> 
> 
> On 13/09/2016 11:13, Daniel P. Berrange wrote:
> > > + * Permission is hereby granted, free of charge, to any person obtaining a copy
> > > + * of this software and associated documentation files (the "Software"), to deal
> > > + * in the Software without restriction, including without limitation the rights
> > > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> > > + * copies of the Software, and to permit persons to whom the Software is
> > > + * furnished to do so, subject to the following conditions:
> > > + *
> > > + * The above copyright notice and this permission notice shall be included in
> > > + * all copies or substantial portions of the Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> > > + * THE SOFTWARE.
> > > + */
> >
> > New files are expected to be submitted under the GPLv2+ license,
> > unless they're header files imported from an external project,
> > which this is not.
> 
> This is not true.  New files are expected to be submitted under a
> GPLv2+-compatible license, which this one is.

If you want to copy code snippets from existing files which are GPLv2+
licensed, into this new file, then that'd be a license violation as
this new file is granting broader permissions which the original code
did not grant. So I still believe this should be GPLv2+ unless there
is a compelling reason why it cannot be.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation
  2016-09-13 10:58       ` Paolo Bonzini
@ 2016-09-13 11:07         ` Daniel P. Berrange
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel P. Berrange @ 2016-09-13 11:07 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Gonglei (Arei), qemu-devel, virtio-dev, Huangpeng (Peter),
	Luonengjun, mst, stefanha, Huangweidong (C),
	mike.caraman, agraf, xin.zeng, Claudio Fontana, nmorey,
	vincent.jardin

On Tue, Sep 13, 2016 at 12:58:59PM +0200, Paolo Bonzini wrote:
> 
> 
> On 13/09/2016 11:54, Daniel P. Berrange wrote:
> > > OK, I agree with you :)  But if we support multiple backends, can
> > > we keep cryptodev-linux module as one option?
> >
> > I'm personally against any support for out of tree kernel modules
> > in QEMU, regardless of whether QEMU also implements alternative
> > backends, unless there is a strong sign that the module in question
> > is on the verge of being accepted into mainline Linux. That does
> > not seem to be the case there - mainline settled on AF_ALG as the
> > only supported approach AFAICT.
> 
> Is there any reason to embed knowledge of AF_ALG directly in QEMU,
> rather than delegating that to gcrypt/nettle?

Actually looking at the code again, neither of those libraries will
ever delegate to the kernel. So if we did want AF_ALG, we would have
to provide a backend in QEMU to use that.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [virtio-dev] Re: [PATCH v2 02/15] crypto: introduce crypto queue handler
  2016-09-13 10:58     ` Paolo Bonzini
@ 2016-09-13 11:52       ` Ola Liljedahl
  2016-09-14  1:07         ` Gonglei (Arei)
  2016-09-14  1:03       ` Gonglei (Arei)
  1 sibling, 1 reply; 40+ messages in thread
From: Ola Liljedahl @ 2016-09-13 11:52 UTC (permalink / raw)
  To: Paolo Bonzini, Daniel P. Berrange, Gonglei
  Cc: qemu-devel, virtio-dev, peter.huangpeng, luonengjun, mst,
	stefanha, weidong.huang, mike.caraman, agraf, xin.zeng,
	claudio.fontana, nmorey, vincent.jardin





On 13/09/2016, 12:58, "virtio-dev@lists.oasis-open.org on behalf of Paolo
Bonzini" <virtio-dev@lists.oasis-open.org on behalf of
pbonzini@redhat.com> wrote:

>
>
>On 13/09/2016 11:20, Daniel P. Berrange wrote:
>>> > +typedef struct CryptoPacket CryptoPacket;
>>> > +typedef struct CryptoQueue CryptoQueue;
>>> > +typedef struct CryptoPacketBuf CryptoPacketBuf;
>>> > +
>>> > +typedef void (CryptoPacketSent) (CryptoClientState *, int);
>> As previously, I'd expect naming of
>>
>>  QCryptoCryptodevPacket
>>  QCryptoCryptodevPacketBuf
>>  QCryptoCryptodevQueue
>>
>
>Gonglei,
>
>you are copying a lot of code from network backends.
>
>I am not sure why you would need a queue for virtio-crypto rather than a
>direct connection between frontend and backend (and the backend would be
>QEMU crypto APIs, like Daniel suggested).
What about backends implemented directly in HW? Bypass the middle man.
Make crypto offload meaningful for small size blocks.

‹ Ola

>
>Paolo
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
>For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [virtio-dev] Re: [PATCH v2 02/15] crypto: introduce crypto queue handler
  2016-09-13 10:58     ` Paolo Bonzini
  2016-09-13 11:52       ` [Qemu-devel] [virtio-dev] " Ola Liljedahl
@ 2016-09-14  1:03       ` Gonglei (Arei)
  1 sibling, 0 replies; 40+ messages in thread
From: Gonglei (Arei) @ 2016-09-14  1:03 UTC (permalink / raw)
  To: Paolo Bonzini, Daniel P. Berrange
  Cc: qemu-devel, virtio-dev, Huangpeng (Peter),
	Luonengjun, mst, stefanha, Huangweidong (C),
	mike.caraman, agraf, xin.zeng, Claudio Fontana, nmorey,
	vincent.jardin

Hi Paolo,


> -----Original Message-----
> From: virtio-dev@lists.oasis-open.org [mailto:virtio-dev@lists.oasis-open.org]
> On Behalf Of Paolo Bonzini
> Sent: Tuesday, September 13, 2016 6:58 PM
> To: Daniel P. Berrange; Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org; Huangpeng
> (Peter); Luonengjun; mst@redhat.com; stefanha@redhat.com; Huangweidong
> (C); mike.caraman@nxp.com; agraf@suse.de; xin.zeng@intel.com; Claudio
> Fontana; nmorey@kalray.eu; vincent.jardin@6wind.com
> Subject: [virtio-dev] Re: [PATCH v2 02/15] crypto: introduce crypto queue
> handler
> 
> 
> 
> On 13/09/2016 11:20, Daniel P. Berrange wrote:
> >> > +typedef struct CryptoPacket CryptoPacket;
> >> > +typedef struct CryptoQueue CryptoQueue;
> >> > +typedef struct CryptoPacketBuf CryptoPacketBuf;
> >> > +
> >> > +typedef void (CryptoPacketSent) (CryptoClientState *, int);
> > As previously, I'd expect naming of
> >
> >  QCryptoCryptodevPacket
> >  QCryptoCryptodevPacketBuf
> >  QCryptoCryptodevQueue
> >
> 
> Gonglei,
> 
> you are copying a lot of code from network backends.
> 
> I am not sure why you would need a queue for virtio-crypto rather than a
> direct connection between frontend and backend (and the backend would be
> QEMU crypto APIs, like Daniel suggested).
> 
My initial idea is support asynchronous crypto operation, so I used a queue to
cache the crypto packets like network did. Now I think again, either synchronous or asynchronous
operation is directly depend on the backend cryptodevs' capacity, we don't need
to use a queue to do that, but provide interfaces which include sync and async operations.

I'll drop the middle queue stuff. Thanks!

Regards,
-Gonglei

> Paolo
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [virtio-dev] Re: [PATCH v2 02/15] crypto: introduce crypto queue handler
  2016-09-13 11:52       ` [Qemu-devel] [virtio-dev] " Ola Liljedahl
@ 2016-09-14  1:07         ` Gonglei (Arei)
  2016-09-14  8:24           ` Ola Liljedahl
  0 siblings, 1 reply; 40+ messages in thread
From: Gonglei (Arei) @ 2016-09-14  1:07 UTC (permalink / raw)
  To: Ola Liljedahl, Paolo Bonzini, Daniel P. Berrange
  Cc: qemu-devel, virtio-dev, Huangpeng (Peter),
	Luonengjun, mst, stefanha, Huangweidong (C),
	mike.caraman, agraf, xin.zeng, Claudio Fontana, nmorey,
	vincent.jardin

Hi Ola,



> -----Original Message-----
> From: Ola Liljedahl [mailto:Ola.Liljedahl@arm.com]
> Sent: Tuesday, September 13, 2016 7:53 PM
> To: Paolo Bonzini; Daniel P. Berrange; Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org; Huangpeng
> (Peter); Luonengjun; mst@redhat.com; stefanha@redhat.com; Huangweidong
> (C); mike.caraman@nxp.com; agraf@suse.de; xin.zeng@intel.com; Claudio
> Fontana; nmorey@kalray.eu; vincent.jardin@6wind.com
> Subject: Re: [virtio-dev] Re: [PATCH v2 02/15] crypto: introduce crypto queue
> handler
> 
> 
> 
> 
> 
> On 13/09/2016, 12:58, "virtio-dev@lists.oasis-open.org on behalf of Paolo
> Bonzini" <virtio-dev@lists.oasis-open.org on behalf of
> pbonzini@redhat.com> wrote:
> 
> >
> >
> >On 13/09/2016 11:20, Daniel P. Berrange wrote:
> >>> > +typedef struct CryptoPacket CryptoPacket;
> >>> > +typedef struct CryptoQueue CryptoQueue;
> >>> > +typedef struct CryptoPacketBuf CryptoPacketBuf;
> >>> > +
> >>> > +typedef void (CryptoPacketSent) (CryptoClientState *, int);
> >> As previously, I'd expect naming of
> >>
> >>  QCryptoCryptodevPacket
> >>  QCryptoCryptodevPacketBuf
> >>  QCryptoCryptodevQueue
> >>
> >
> >Gonglei,
> >
> >you are copying a lot of code from network backends.
> >
> >I am not sure why you would need a queue for virtio-crypto rather than a
> >direct connection between frontend and backend (and the backend would be
> >QEMU crypto APIs, like Daniel suggested).
>
> What about backends implemented directly in HW? Bypass the middle man.
> Make crypto offload meaningful for small size blocks.
> 
As I said in other reply, I only provide interfaces, you can realize them according
to different backend cryptodevs.

Regards,
-Gonglei

> < Ola
> 
> >
> >Paolo
> >
> >---------------------------------------------------------------------
> >To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> >For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> >
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended recipient,
> please notify the sender immediately and do not disclose the contents to any
> other person, use it for any purpose, or store or copy the information in any
> medium. Thank you.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [Qemu-devel] [virtio-dev] Re: [PATCH v2 02/15] crypto: introduce crypto queue handler
  2016-09-14  1:07         ` Gonglei (Arei)
@ 2016-09-14  8:24           ` Ola Liljedahl
  0 siblings, 0 replies; 40+ messages in thread
From: Ola Liljedahl @ 2016-09-14  8:24 UTC (permalink / raw)
  To: Gonglei (Arei), Paolo Bonzini, Daniel P. Berrange
  Cc: qemu-devel, virtio-dev, Huangpeng (Peter),
	Luonengjun, mst, stefanha, Huangweidong (C),
	mike.caraman, agraf, xin.zeng, Claudio Fontana, nmorey,
	vincent.jardin


On 14/09/2016, 03:07, "Gonglei (Arei)" <arei.gonglei@huawei.com> wrote:

>Hi Ola,
>
>
>
>> -----Original Message-----
>> From: Ola Liljedahl [mailto:Ola.Liljedahl@arm.com]
>> Sent: Tuesday, September 13, 2016 7:53 PM
>> To: Paolo Bonzini; Daniel P. Berrange; Gonglei (Arei)
>> Cc: qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org; Huangpeng
>> (Peter); Luonengjun; mst@redhat.com; stefanha@redhat.com; Huangweidong
>> (C); mike.caraman@nxp.com; agraf@suse.de; xin.zeng@intel.com; Claudio
>> Fontana; nmorey@kalray.eu; vincent.jardin@6wind.com
>> Subject: Re: [virtio-dev] Re: [PATCH v2 02/15] crypto: introduce crypto
>>queue
>> handler
>>
>>
>>
>>
>>
>> On 13/09/2016, 12:58, "virtio-dev@lists.oasis-open.org on behalf of
>>Paolo
>> Bonzini" <virtio-dev@lists.oasis-open.org on behalf of
>> pbonzini@redhat.com> wrote:
>>
>> >
>> >
>> >On 13/09/2016 11:20, Daniel P. Berrange wrote:
>> >>> > +typedef struct CryptoPacket CryptoPacket;
>> >>> > +typedef struct CryptoQueue CryptoQueue;
>> >>> > +typedef struct CryptoPacketBuf CryptoPacketBuf;
>> >>> > +
>> >>> > +typedef void (CryptoPacketSent) (CryptoClientState *, int);
>> >> As previously, I'd expect naming of
>> >>
>> >>  QCryptoCryptodevPacket
>> >>  QCryptoCryptodevPacketBuf
>> >>  QCryptoCryptodevQueue
>> >>
>> >
>> >Gonglei,
>> >
>> >you are copying a lot of code from network backends.
>> >
>> >I am not sure why you would need a queue for virtio-crypto rather than
>>a
>> >direct connection between frontend and backend (and the backend would
>>be
>> >QEMU crypto APIs, like Daniel suggested).
>>
>> What about backends implemented directly in HW? Bypass the middle man.
>> Make crypto offload meaningful for small size blocks.
>>
>As I said in other reply, I only provide interfaces, you can realize them
>according
>to different backend cryptodevs.
Interfaces imposes contraints on performance and implementation. A poor
interface could make certain implementations unnecessarily difficult.
Since these virtio-* interfaces will live for a long time, we should
ensure that they do not impose unnecessary constraints. For functionality
like crypto and IPsec, I can see the benefit of implementing the backend
in HW (for the datapath).

>
>Regards,
>-Gonglei
>
>> < Ola
>>
>> >
>> >Paolo
>> >
>> >---------------------------------------------------------------------
>> >To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
>> >For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>> >
>>
>> IMPORTANT NOTICE: The contents of this email and any attachments are
>> confidential and may also be privileged. If you are not the intended
>>recipient,
>> please notify the sender immediately and do not disclose the contents
>>to any
>> other person, use it for any purpose, or store or copy the information
>>in any
>> medium. Thank you.
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2016-09-14  8:24 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-13  3:52 [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Gonglei
2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto legacy hardware Gonglei
2016-09-13  9:13   ` Daniel P. Berrange
2016-09-13  9:55     ` Gonglei (Arei)
2016-09-13  9:59       ` Daniel P. Berrange
2016-09-13 10:06         ` Gonglei (Arei)
2016-09-13 10:50     ` Paolo Bonzini
2016-09-13 11:04       ` Daniel P. Berrange
2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 02/15] crypto: introduce crypto queue handler Gonglei
2016-09-13  9:20   ` Daniel P. Berrange
2016-09-13  9:58     ` Gonglei (Arei)
2016-09-13 10:58     ` Paolo Bonzini
2016-09-13 11:52       ` [Qemu-devel] [virtio-dev] " Ola Liljedahl
2016-09-14  1:07         ` Gonglei (Arei)
2016-09-14  8:24           ` Ola Liljedahl
2016-09-14  1:03       ` Gonglei (Arei)
2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 03/15] crypto: add cryptoLegacyHW stuff Gonglei
2016-09-13  9:22   ` Daniel P. Berrange
2016-09-13 10:05     ` Gonglei (Arei)
2016-09-13 10:08       ` Daniel P. Berrange
2016-09-13 10:14         ` Gonglei (Arei)
2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 04/15] crypto: add symetric algorithms support Gonglei
2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 05/15] crypto: add cryptodev-linux as a cryptodev backend Gonglei
2016-09-13  9:23   ` Daniel P. Berrange
2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 06/15] crypto: add internal handle logic layer Gonglei
2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 07/15] virtio-crypto: introduce virtio-crypto.h Gonglei
2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 08/15] virtio-crypto-pci: add virtio crypto pci support Gonglei
2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 09/15] virtio-crypto: add virtio crypto realization Gonglei
2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 10/15] virtio-crypto: set capacity of crypto legacy hardware Gonglei
2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 11/15] virtio-crypto: add control queue handler Gonglei
2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 12/15] virtio-crypto: add destroy session logic Gonglei
2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 13/15] virtio-crypto: get correct input data address for each request Gonglei
2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 14/15] virtio-crypto: add data virtqueue processing handler Gonglei
2016-09-13  3:52 ` [Qemu-devel] [PATCH v2 15/15] virtio-crypto: support scatter gather list Gonglei
2016-09-13  8:57 ` [Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation Daniel P. Berrange
2016-09-13  9:45   ` Gonglei (Arei)
2016-09-13  9:54     ` Daniel P. Berrange
2016-09-13 10:08       ` Gonglei (Arei)
2016-09-13 10:58       ` Paolo Bonzini
2016-09-13 11:07         ` Daniel P. Berrange

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.