diff mbox

[v4,05/13] virtio-crypto: add virtio crypto device emulation

Message ID 1475051152-400276-6-git-send-email-arei.gonglei@huawei.com
State New
Headers show

Commit Message

Gonglei (Arei) Sept. 28, 2016, 8:25 a.m. UTC 
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/virtio/Makefile.objs                     |   1 +
 hw/virtio/virtio-crypto.c                   | 199 ++++++++++++++++++++++++++++
 include/hw/virtio/virtio-crypto.h           |  74 +++++++++++
 include/standard-headers/linux/virtio_ids.h |   2 +-
 4 files changed, 275 insertions(+), 1 deletion(-)
 create mode 100644 hw/virtio/virtio-crypto.c
 create mode 100644 include/hw/virtio/virtio-crypto.h

Comments

Stefan Hajnoczi Oct. 4, 2016, 9:38 a.m. UTC | #1 
On Wed, Sep 28, 2016 at 04:25:44PM +0800, Gonglei wrote:
> 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

This patch contains functions and struct fields that are unused.  It
would make code review easier if you add them when they are needed
throughout the patch series.

> +static void virtio_crypto_device_realize(DeviceState *dev, Error **errp)
> +{
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(dev);
> +    int i;
> +
> +    vcrypto->cryptodev = vcrypto->conf.cryptodev;
> +    if (vcrypto->cryptodev == NULL) {
> +        error_setg(errp, "'cryptodev' parameter expects a valid object");
> +        return;
> +    }
> +
> +    vcrypto->max_queues = MAX(vcrypto->cryptodev->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);

The error message is off by 1:

must be a positive integer less than VIRTIO_QUEUE_MAX

> +        return;
> +    }
> +
> +    virtio_init(vdev, "virtio-crypto", VIRTIO_ID_CRYPTO, vcrypto->config_size);
> +    vcrypto->curr_queues = 1;
> +
> +    for (i = 0; i < vcrypto->max_queues; i++) {
> +        virtio_add_queue(vdev, 1024, virtio_crypto_handle_dataq);
> +    }
> +
> +    vcrypto->ctrl_vq = virtio_add_queue(vdev, 64, virtio_crypto_handle_ctrl);
> +    if (!vcrypto->cryptodev->ready) {
> +        vcrypto->status &= ~VIRTIO_CRYPTO_S_HW_READY;
> +    } else {
> +        vcrypto->status |= VIRTIO_CRYPTO_S_HW_READY;
> +    }
> +    register_savevm(dev, "virtio-crypto", -1, 1, virtio_crypto_save,
> +                    virtio_crypto_load, vcrypto);

Please use VMSTATE_VIRTIO_DEVICE() instead of calling register_savevm().

> +#ifdef DEBUG_VIRTIO_CRYPTO
> +#define DPRINTF(fmt, ...) \
> +do { printf("virtio_crypto: " fmt , ## __VA_ARGS__); } while (0)
> +#else
> +#define DPRINTF(fmt, ...) do { } while (0)
> +#endif

Please use tracing (see docs/tracing.txt) or if you really want to use
debug printfs then define DPRINTF() in a way that prevents bitrot:

#define DEBUG_VIRTIO_CRYPTO 0
#define DPRINTF(fmt, ...) \
do { \
    if (DEBUG_VIRTIO_CRYPTO) { \
        fprintf(stderr, "virtio_crypto: " fmt, ##__VA_ARGS__); \
    } \
} while (0)
Gonglei (Arei) Oct. 5, 2016, 3:26 a.m. UTC | #2 
> From: Stefan Hajnoczi [mailto:stefanha@redhat.com]
> Sent: Tuesday, October 04, 2016 5:38 PM
> Subject: Re: [PATCH v4 05/13] virtio-crypto: add virtio crypto device emulation
> 
> On Wed, Sep 28, 2016 at 04:25:44PM +0800, Gonglei wrote:
> > 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
> 
> This patch contains functions and struct fields that are unused.  It
> would make code review easier if you add them when they are needed
> throughout the patch series.
> 
OK, will remove them.

> > +static void virtio_crypto_device_realize(DeviceState *dev, Error **errp)
> > +{
> > +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> > +    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(dev);
> > +    int i;
> > +
> > +    vcrypto->cryptodev = vcrypto->conf.cryptodev;
> > +    if (vcrypto->cryptodev == NULL) {
> > +        error_setg(errp, "'cryptodev' parameter expects a valid object");
> > +        return;
> > +    }
> > +
> > +    vcrypto->max_queues = MAX(vcrypto->cryptodev->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);
> 
> The error message is off by 1:
> 
> must be a positive integer less than VIRTIO_QUEUE_MAX
> 
Yes.

> > +        return;
> > +    }
> > +
> > +    virtio_init(vdev, "virtio-crypto", VIRTIO_ID_CRYPTO,
> vcrypto->config_size);
> > +    vcrypto->curr_queues = 1;
> > +
> > +    for (i = 0; i < vcrypto->max_queues; i++) {
> > +        virtio_add_queue(vdev, 1024, virtio_crypto_handle_dataq);
> > +    }
> > +
> > +    vcrypto->ctrl_vq = virtio_add_queue(vdev, 64,
> virtio_crypto_handle_ctrl);
> > +    if (!vcrypto->cryptodev->ready) {
> > +        vcrypto->status &= ~VIRTIO_CRYPTO_S_HW_READY;
> > +    } else {
> > +        vcrypto->status |= VIRTIO_CRYPTO_S_HW_READY;
> > +    }
> > +    register_savevm(dev, "virtio-crypto", -1, 1, virtio_crypto_save,
> > +                    virtio_crypto_load, vcrypto);
> 
> Please use VMSTATE_VIRTIO_DEVICE() instead of calling register_savevm().
> 
OK.

> > +#ifdef DEBUG_VIRTIO_CRYPTO
> > +#define DPRINTF(fmt, ...) \
> > +do { printf("virtio_crypto: " fmt , ## __VA_ARGS__); } while (0)
> > +#else
> > +#define DPRINTF(fmt, ...) do { } while (0)
> > +#endif
> 
> Please use tracing (see docs/tracing.txt) or if you really want to use

I planned to polish tracing log in my TODO list. So, currently I'd like to use
DPRINTF().

> debug printfs then define DPRINTF() in a way that prevents bitrot:
> 
> #define DEBUG_VIRTIO_CRYPTO 0
> #define DPRINTF(fmt, ...) \
> do { \
>     if (DEBUG_VIRTIO_CRYPTO) { \
>         fprintf(stderr, "virtio_crypto: " fmt, ##__VA_ARGS__); \
>     } \
> } while (0)

Make sense.


Regards,
-Gonglei
diff mbox

Patch

diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index e716308..968f392 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-y += virtio-crypto.o
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
new file mode 100644
index 0000000..e639fb3
--- /dev/null
+++ b/hw/virtio/virtio-crypto.c
@@ -0,0 +1,199 @@ 
+/*
+ * 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.
+ */
+#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"
+#include "standard-headers/linux/virtio_ids.h"
+
+
+static void virtio_crypto_process(VirtIOCrypto *vcrypto)
+{
+}
+
+static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+{
+
+}
+
+static void virtio_crypto_handle_dataq(VirtIODevice *vdev, VirtQueue *vq)
+{
+
+}
+
+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_reset(VirtIODevice *vdev)
+{
+    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(vdev);
+    /* multiqueue is disabled by default */
+    vcrypto->curr_queues = 1;
+    if (!vcrypto->cryptodev->ready) {
+        vcrypto->status &= ~VIRTIO_CRYPTO_S_HW_READY;
+    } else {
+        vcrypto->status |= VIRTIO_CRYPTO_S_HW_READY;
+    }
+}
+
+static void virtio_crypto_device_realize(DeviceState *dev, Error **errp)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(dev);
+    int i;
+
+    vcrypto->cryptodev = vcrypto->conf.cryptodev;
+    if (vcrypto->cryptodev == NULL) {
+        error_setg(errp, "'cryptodev' parameter expects a valid object");
+        return;
+    }
+
+    vcrypto->max_queues = MAX(vcrypto->cryptodev->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->curr_queues = 1;
+
+    for (i = 0; i < vcrypto->max_queues; i++) {
+        virtio_add_queue(vdev, 1024, virtio_crypto_handle_dataq);
+    }
+
+    vcrypto->ctrl_vq = virtio_add_queue(vdev, 64, virtio_crypto_handle_ctrl);
+    if (!vcrypto->cryptodev->ready) {
+        vcrypto->status &= ~VIRTIO_CRYPTO_S_HW_READY;
+    } else {
+        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);
+
+    unregister_savevm(dev, "virtio-crypto", vcrypto);
+
+    virtio_cleanup(vdev);
+}
+
+static Property virtio_crypto_properties[] = {
+    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->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);
+
+    object_property_add_link(obj, "cryptodev",
+                             TYPE_QCRYPTO_CRYPTODEV_BACKEND,
+                             (Object **)&vcrypto->conf.cryptodev,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE, NULL);
+}
+
+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/virtio/virtio-crypto.h b/include/hw/virtio/virtio-crypto.h
new file mode 100644
index 0000000..484062c
--- /dev/null
+++ b/include/hw/virtio/virtio-crypto.h
@@ -0,0 +1,74 @@ 
+/*
+ * 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 "sysemu/cryptodev.h"
+
+
+/* #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)
+
+
+typedef struct VirtIOCryptoConf {
+    QCryptoCryptoDevBackend *cryptodev;
+} VirtIOCryptoConf;
+
+struct VirtIOCrypto;
+
+typedef struct VirtIOCryptoReq {
+    VirtQueueElement elem;
+    /* flags of operation, such as type of algorithm */
+    uint32_t flags;
+    /* address of in data (Device to Driver) */
+    void *idata_hva;
+    VirtQueue *vq;
+    struct VirtIOCrypto *vcrypto;
+    union {
+        QCryptoCryptoDevBackendSymOpInfo *sym_op_info;
+    } u;
+} VirtIOCryptoReq;
+
+typedef struct VirtIOCrypto {
+    VirtIODevice parent_obj;
+
+    VirtQueue *ctrl_vq;
+
+    VirtIOCryptoConf conf;
+    QCryptoCryptoDevBackend *cryptodev;
+
+    uint32_t max_queues;
+    uint32_t status;
+
+    int multiqueue;
+    uint32_t curr_queues;
+    size_t config_size;
+} VirtIOCrypto;
+
+#endif /* _QEMU_VIRTIO_CRYPTO_H */
diff --git a/include/standard-headers/linux/virtio_ids.h b/include/standard-headers/linux/virtio_ids.h
index 3228d58..fe74e42 100644
--- a/include/standard-headers/linux/virtio_ids.h
+++ b/include/standard-headers/linux/virtio_ids.h
@@ -42,5 +42,5 @@ 
 #define VIRTIO_ID_GPU          16 /* virtio GPU */
 #define VIRTIO_ID_INPUT        18 /* virtio input */
 #define VIRTIO_ID_VSOCK        19 /* virtio vsock transport */
-
+#define VIRTIO_ID_CRYPTO       20 /* virtio crypto */
 #endif /* _LINUX_VIRTIO_IDS_H */