* [for-8.0 v2 00/11] Refactor cryptodev
@ 2022-11-22 14:07 zhenwei pi
2022-11-22 14:07 ` [for-8.0 v2 01/11] cryptodev: Introduce cryptodev.json zhenwei pi
` (12 more replies)
0 siblings, 13 replies; 31+ messages in thread
From: zhenwei pi @ 2022-11-22 14:07 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini
Cc: qemu-devel, zhenwei pi
v1 -> v2:
- fix coding style and use 'g_strjoin()' instead of 'char services[128]'
(suggested by Dr. David Alan Gilbert)
- wrapper function 'cryptodev_backend_account' to record statistics, and
allocate sym_stat/asym_stat in cryptodev base class. see patch:
'cryptodev: Support statistics'.
- add more arguments into struct CryptoDevBackendOpInfo, then
cryptodev_backend_crypto_operation() uses *op_info only.
- support cryptodev QoS settings(BPS&OPS), both QEMU command line and QMP
command works fine.
- add myself as the maintainer for cryptodev.
v1:
- introduce cryptodev.json to describe the attributes of crypto device, then
drop duplicated type declare, remove some virtio related dependence.
- add statistics: OPS and bandwidth.
- add QMP command: query-cryptodev
- add HMP info command: cryptodev
- misc fix: detect akcipher capability instead of exposing akcipher service
unconditionally.
Zhenwei Pi (11):
cryptodev: Introduce cryptodev.json
cryptodev: Remove 'name' & 'model' fields
cryptodev: Introduce cryptodev alg type in QAPI
cryptodev: Introduce server type in QAPI
cryptodev: Introduce 'query-cryptodev' QMP command
cryptodev: Support statistics
cryptodev-builtin: Detect akcipher capability
hmp: add cryptodev info command
cryptodev: Use CryptoDevBackendOpInfo for operation
cryptodev: support QoS
MAINTAINERS: add myself as the maintainer for cryptodev
MAINTAINERS | 2 +
backends/cryptodev-builtin.c | 42 +++--
backends/cryptodev-lkcf.c | 19 +-
backends/cryptodev-vhost-user.c | 13 +-
backends/cryptodev-vhost.c | 4 +-
backends/cryptodev.c | 295 +++++++++++++++++++++++++++++---
hmp-commands-info.hx | 14 ++
hw/virtio/virtio-crypto.c | 48 ++++--
include/monitor/hmp.h | 1 +
include/sysemu/cryptodev.h | 94 +++++-----
monitor/hmp-cmds.c | 36 ++++
qapi/cryptodev.json | 144 ++++++++++++++++
qapi/meson.build | 1 +
qapi/qapi-schema.json | 1 +
qapi/qom.json | 8 +-
15 files changed, 604 insertions(+), 118 deletions(-)
create mode 100644 qapi/cryptodev.json
--
2.20.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* [for-8.0 v2 01/11] cryptodev: Introduce cryptodev.json
2022-11-22 14:07 [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
@ 2022-11-22 14:07 ` zhenwei pi
2023-01-16 10:58 ` Daniel P. Berrangé
2022-11-22 14:07 ` [for-8.0 v2 02/11] cryptodev: Remove 'name' & 'model' fields zhenwei pi
` (11 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: zhenwei pi @ 2022-11-22 14:07 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini
Cc: qemu-devel, zhenwei pi
Introduce QCryptodevBackendType in cryptodev.json, also apply this to
related codes. Then we can drop 'enum CryptoDevBackendOptionsType'.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
MAINTAINERS | 1 +
backends/cryptodev-builtin.c | 2 +-
backends/cryptodev-lkcf.c | 2 +-
backends/cryptodev-vhost-user.c | 4 ++--
backends/cryptodev-vhost.c | 4 ++--
include/sysemu/cryptodev.h | 11 ++---------
qapi/cryptodev.json | 20 ++++++++++++++++++++
qapi/meson.build | 1 +
qapi/qapi-schema.json | 1 +
9 files changed, 31 insertions(+), 15 deletions(-)
create mode 100644 qapi/cryptodev.json
diff --git a/MAINTAINERS b/MAINTAINERS
index caba73ec41..3f698cb0e9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2830,6 +2830,7 @@ M: Gonglei <arei.gonglei@huawei.com>
S: Maintained
F: include/sysemu/cryptodev*.h
F: backends/cryptodev*.c
+F: qapi/cryptodev.json
Python library
M: John Snow <jsnow@redhat.com>
diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
index cda6ca3b71..8c7c10847d 100644
--- a/backends/cryptodev-builtin.c
+++ b/backends/cryptodev-builtin.c
@@ -76,7 +76,7 @@ static void cryptodev_builtin_init(
"cryptodev-builtin", NULL);
cc->info_str = g_strdup_printf("cryptodev-builtin0");
cc->queue_index = 0;
- cc->type = CRYPTODEV_BACKEND_TYPE_BUILTIN;
+ cc->type = QCRYPTODEV_BACKEND_TYPE_BUILTIN;
backend->conf.peers.ccs[0] = cc;
backend->conf.crypto_services =
diff --git a/backends/cryptodev-lkcf.c b/backends/cryptodev-lkcf.c
index 133bd706a4..91e02c0df9 100644
--- a/backends/cryptodev-lkcf.c
+++ b/backends/cryptodev-lkcf.c
@@ -226,7 +226,7 @@ static void cryptodev_lkcf_init(CryptoDevBackend *backend, Error **errp)
cc = cryptodev_backend_new_client("cryptodev-lkcf", NULL);
cc->info_str = g_strdup_printf("cryptodev-lkcf0");
cc->queue_index = 0;
- cc->type = CRYPTODEV_BACKEND_TYPE_LKCF;
+ cc->type = QCRYPTODEV_BACKEND_TYPE_LKCF;
backend->conf.peers.ccs[0] = cc;
backend->conf.crypto_services =
diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c
index ab3028e045..c165a1b1d6 100644
--- a/backends/cryptodev-vhost-user.c
+++ b/backends/cryptodev-vhost-user.c
@@ -67,7 +67,7 @@ cryptodev_vhost_user_get_vhost(
{
CryptoDevBackendVhostUser *s =
CRYPTODEV_BACKEND_VHOST_USER(b);
- assert(cc->type == CRYPTODEV_BACKEND_TYPE_VHOST_USER);
+ assert(cc->type == QCRYPTODEV_BACKEND_TYPE_VHOST_USER);
assert(queue < MAX_CRYPTO_QUEUE_NUM);
return s->vhost_crypto[queue];
@@ -203,7 +203,7 @@ static void cryptodev_vhost_user_init(
cc->info_str = g_strdup_printf("cryptodev-vhost-user%zu to %s ",
i, chr->label);
cc->queue_index = i;
- cc->type = CRYPTODEV_BACKEND_TYPE_VHOST_USER;
+ cc->type = QCRYPTODEV_BACKEND_TYPE_VHOST_USER;
backend->conf.peers.ccs[i] = cc;
diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c
index bc13e466b4..0715014552 100644
--- a/backends/cryptodev-vhost.c
+++ b/backends/cryptodev-vhost.c
@@ -128,7 +128,7 @@ cryptodev_get_vhost(CryptoDevBackendClient *cc,
switch (cc->type) {
#if defined(CONFIG_VHOST_USER) && defined(CONFIG_LINUX)
- case CRYPTODEV_BACKEND_TYPE_VHOST_USER:
+ case QCRYPTODEV_BACKEND_TYPE_VHOST_USER:
vhost_crypto = cryptodev_vhost_user_get_vhost(cc, b, queue);
break;
#endif
@@ -196,7 +196,7 @@ int cryptodev_vhost_start(VirtIODevice *dev, int total_queues)
* because vhost user doesn't interrupt masking/unmasking
* properly.
*/
- if (cc->type == CRYPTODEV_BACKEND_TYPE_VHOST_USER) {
+ if (cc->type == QCRYPTODEV_BACKEND_TYPE_VHOST_USER) {
dev->use_guest_notifier_mask = false;
}
}
diff --git a/include/sysemu/cryptodev.h b/include/sysemu/cryptodev.h
index cf9b3f07fe..8d2adda974 100644
--- a/include/sysemu/cryptodev.h
+++ b/include/sysemu/cryptodev.h
@@ -25,6 +25,7 @@
#include "qemu/queue.h"
#include "qom/object.h"
+#include "qapi/qapi-types-cryptodev.h"
/**
* CryptoDevBackend:
@@ -215,16 +216,8 @@ struct CryptoDevBackendClass {
void *opaque);
};
-typedef enum CryptoDevBackendOptionsType {
- CRYPTODEV_BACKEND_TYPE_NONE = 0,
- CRYPTODEV_BACKEND_TYPE_BUILTIN = 1,
- CRYPTODEV_BACKEND_TYPE_VHOST_USER = 2,
- CRYPTODEV_BACKEND_TYPE_LKCF = 3,
- CRYPTODEV_BACKEND_TYPE__MAX,
-} CryptoDevBackendOptionsType;
-
struct CryptoDevBackendClient {
- CryptoDevBackendOptionsType type;
+ QCryptodevBackendType type;
char *model;
char *name;
char *info_str;
diff --git a/qapi/cryptodev.json b/qapi/cryptodev.json
new file mode 100644
index 0000000000..b65edbe183
--- /dev/null
+++ b/qapi/cryptodev.json
@@ -0,0 +1,20 @@
+# -*- Mode: Python -*-
+# vim: filetype=python
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
+
+##
+# @QCryptodevBackendType:
+#
+# The crypto device backend type
+#
+# @builtin: the QEMU builtin support
+# @vhost-user: vhost-user
+# @lkcf: Linux kernel cryptographic framework
+#
+# Since: 8.0
+##
+{ 'enum': 'QCryptodevBackendType',
+ 'prefix': 'QCRYPTODEV_BACKEND_TYPE',
+ 'data': ['builtin', 'vhost-user', 'lkcf']}
diff --git a/qapi/meson.build b/qapi/meson.build
index 9a36c15c04..b5069f34f8 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -56,6 +56,7 @@ if have_system
qapi_all_modules += [
'acpi',
'audio',
+ 'cryptodev',
'qdev',
'pci',
'rdma',
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index f000b90744..1e923945db 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -95,3 +95,4 @@
{ 'include': 'pci.json' }
{ 'include': 'stats.json' }
{ 'include': 'virtio.json' }
+{ 'include': 'cryptodev.json' }
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [for-8.0 v2 02/11] cryptodev: Remove 'name' & 'model' fields
2022-11-22 14:07 [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
2022-11-22 14:07 ` [for-8.0 v2 01/11] cryptodev: Introduce cryptodev.json zhenwei pi
@ 2022-11-22 14:07 ` zhenwei pi
2023-01-16 11:05 ` Daniel P. Berrangé
2022-11-22 14:07 ` [for-8.0 v2 03/11] cryptodev: Introduce cryptodev alg type in QAPI zhenwei pi
` (10 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: zhenwei pi @ 2022-11-22 14:07 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini
Cc: qemu-devel, zhenwei pi
We have already used qapi to generate crypto device types, this allows
to convert type to a string 'model', so the 'model' field is not
needed.
And the 'name' field is not used by any backend driver, drop it.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
backends/cryptodev-builtin.c | 3 +--
backends/cryptodev-lkcf.c | 2 +-
backends/cryptodev-vhost-user.c | 3 +--
backends/cryptodev.c | 11 +----------
include/sysemu/cryptodev.h | 12 +++---------
5 files changed, 7 insertions(+), 24 deletions(-)
diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
index 8c7c10847d..08895271eb 100644
--- a/backends/cryptodev-builtin.c
+++ b/backends/cryptodev-builtin.c
@@ -72,8 +72,7 @@ static void cryptodev_builtin_init(
return;
}
- cc = cryptodev_backend_new_client(
- "cryptodev-builtin", NULL);
+ cc = cryptodev_backend_new_client();
cc->info_str = g_strdup_printf("cryptodev-builtin0");
cc->queue_index = 0;
cc->type = QCRYPTODEV_BACKEND_TYPE_BUILTIN;
diff --git a/backends/cryptodev-lkcf.c b/backends/cryptodev-lkcf.c
index 91e02c0df9..de3d1867c5 100644
--- a/backends/cryptodev-lkcf.c
+++ b/backends/cryptodev-lkcf.c
@@ -223,7 +223,7 @@ static void cryptodev_lkcf_init(CryptoDevBackend *backend, Error **errp)
return;
}
- cc = cryptodev_backend_new_client("cryptodev-lkcf", NULL);
+ cc = cryptodev_backend_new_client();
cc->info_str = g_strdup_printf("cryptodev-lkcf0");
cc->queue_index = 0;
cc->type = QCRYPTODEV_BACKEND_TYPE_LKCF;
diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c
index c165a1b1d6..580bd1abb0 100644
--- a/backends/cryptodev-vhost-user.c
+++ b/backends/cryptodev-vhost-user.c
@@ -198,8 +198,7 @@ static void cryptodev_vhost_user_init(
s->opened = true;
for (i = 0; i < queues; i++) {
- cc = cryptodev_backend_new_client(
- "cryptodev-vhost-user", NULL);
+ cc = cryptodev_backend_new_client();
cc->info_str = g_strdup_printf("cryptodev-vhost-user%zu to %s ",
i, chr->label);
cc->queue_index = i;
diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index 54ee8c81f5..81941af816 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -34,18 +34,11 @@
static QTAILQ_HEAD(, CryptoDevBackendClient) crypto_clients;
-CryptoDevBackendClient *
-cryptodev_backend_new_client(const char *model,
- const char *name)
+CryptoDevBackendClient *cryptodev_backend_new_client(void)
{
CryptoDevBackendClient *cc;
cc = g_new0(CryptoDevBackendClient, 1);
- cc->model = g_strdup(model);
- if (name) {
- cc->name = g_strdup(name);
- }
-
QTAILQ_INSERT_TAIL(&crypto_clients, cc, next);
return cc;
@@ -55,8 +48,6 @@ void cryptodev_backend_free_client(
CryptoDevBackendClient *cc)
{
QTAILQ_REMOVE(&crypto_clients, cc, next);
- g_free(cc->name);
- g_free(cc->model);
g_free(cc->info_str);
g_free(cc);
}
diff --git a/include/sysemu/cryptodev.h b/include/sysemu/cryptodev.h
index 8d2adda974..af152d09db 100644
--- a/include/sysemu/cryptodev.h
+++ b/include/sysemu/cryptodev.h
@@ -218,8 +218,6 @@ struct CryptoDevBackendClass {
struct CryptoDevBackendClient {
QCryptodevBackendType type;
- char *model;
- char *name;
char *info_str;
unsigned int queue_index;
int vring_enable;
@@ -264,11 +262,8 @@ struct CryptoDevBackend {
/**
* cryptodev_backend_new_client:
- * @model: the cryptodev backend model
- * @name: the cryptodev backend name, can be NULL
*
- * Creates a new cryptodev backend client object
- * with the @name in the model @model.
+ * Creates a new cryptodev backend client object.
*
* The returned object must be released with
* cryptodev_backend_free_client() when no
@@ -276,9 +271,8 @@ struct CryptoDevBackend {
*
* Returns: a new cryptodev backend client object
*/
-CryptoDevBackendClient *
-cryptodev_backend_new_client(const char *model,
- const char *name);
+CryptoDevBackendClient *cryptodev_backend_new_client(void);
+
/**
* cryptodev_backend_free_client:
* @cc: the cryptodev backend client object
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [for-8.0 v2 03/11] cryptodev: Introduce cryptodev alg type in QAPI
2022-11-22 14:07 [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
2022-11-22 14:07 ` [for-8.0 v2 01/11] cryptodev: Introduce cryptodev.json zhenwei pi
2022-11-22 14:07 ` [for-8.0 v2 02/11] cryptodev: Remove 'name' & 'model' fields zhenwei pi
@ 2022-11-22 14:07 ` zhenwei pi
2023-01-16 11:08 ` Daniel P. Berrangé
2022-11-22 14:07 ` [for-8.0 v2 04/11] cryptodev: Introduce server " zhenwei pi
` (9 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: zhenwei pi @ 2022-11-22 14:07 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini
Cc: qemu-devel, zhenwei pi
Introduce cryptodev alg type in cryptodev.json, then apply this to
related codes, and drop 'enum CryptoDevBackendAlgType'.
There are two options:
1, { 'enum': 'QCryptodevBackendAlgType',
'prefix': 'CRYPTODEV_BACKEND_ALG',
'data': ['sym', 'asym']}
Then we can keep 'CRYPTODEV_BACKEND_ALG_SYM' and avoid lots of
changes.
2, changes in this patch(with prefix 'QCRYPTODEV_BACKEND_ALG').
To avoid breaking the rule of QAPI, use 2 here.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
backends/cryptodev-builtin.c | 6 +++---
backends/cryptodev-lkcf.c | 4 ++--
backends/cryptodev.c | 6 +++---
hw/virtio/virtio-crypto.c | 14 +++++++-------
include/sysemu/cryptodev.h | 8 +-------
qapi/cryptodev.json | 14 ++++++++++++++
6 files changed, 30 insertions(+), 22 deletions(-)
diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
index 08895271eb..5fb7b8f43f 100644
--- a/backends/cryptodev-builtin.c
+++ b/backends/cryptodev-builtin.c
@@ -537,7 +537,7 @@ static int cryptodev_builtin_operation(
CryptoDevBackendBuiltinSession *sess;
CryptoDevBackendSymOpInfo *sym_op_info;
CryptoDevBackendAsymOpInfo *asym_op_info;
- enum CryptoDevBackendAlgType algtype = op_info->algtype;
+ enum QCryptodevBackendAlgType algtype = op_info->algtype;
int status = -VIRTIO_CRYPTO_ERR;
Error *local_error = NULL;
@@ -549,11 +549,11 @@ static int cryptodev_builtin_operation(
}
sess = builtin->sessions[op_info->session_id];
- if (algtype == CRYPTODEV_BACKEND_ALG_SYM) {
+ if (algtype == QCRYPTODEV_BACKEND_ALG_SYM) {
sym_op_info = op_info->u.sym_op_info;
status = cryptodev_builtin_sym_operation(sess, sym_op_info,
&local_error);
- } else if (algtype == CRYPTODEV_BACKEND_ALG_ASYM) {
+ } else if (algtype == QCRYPTODEV_BACKEND_ALG_ASYM) {
asym_op_info = op_info->u.asym_op_info;
status = cryptodev_builtin_asym_operation(sess, op_info->op_code,
asym_op_info, &local_error);
diff --git a/backends/cryptodev-lkcf.c b/backends/cryptodev-lkcf.c
index de3d1867c5..919bf05b75 100644
--- a/backends/cryptodev-lkcf.c
+++ b/backends/cryptodev-lkcf.c
@@ -477,7 +477,7 @@ static int cryptodev_lkcf_operation(
CryptoDevBackendLKCF *lkcf =
CRYPTODEV_BACKEND_LKCF(backend);
CryptoDevBackendLKCFSession *sess;
- enum CryptoDevBackendAlgType algtype = op_info->algtype;
+ enum QCryptodevBackendAlgType algtype = op_info->algtype;
CryptoDevLKCFTask *task;
if (op_info->session_id >= MAX_SESSIONS ||
@@ -488,7 +488,7 @@ static int cryptodev_lkcf_operation(
}
sess = lkcf->sess[op_info->session_id];
- if (algtype != CRYPTODEV_BACKEND_ALG_ASYM) {
+ if (algtype != QCRYPTODEV_BACKEND_ALG_ASYM) {
error_report("algtype not supported: %u", algtype);
return -VIRTIO_CRYPTO_NOTSUPP;
}
diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index 81941af816..d3caded920 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -120,10 +120,10 @@ int cryptodev_backend_crypto_operation(
{
VirtIOCryptoReq *req = opaque1;
CryptoDevBackendOpInfo *op_info = &req->op_info;
- enum CryptoDevBackendAlgType algtype = req->flags;
+ enum QCryptodevBackendAlgType algtype = req->flags;
- if ((algtype != CRYPTODEV_BACKEND_ALG_SYM)
- && (algtype != CRYPTODEV_BACKEND_ALG_ASYM)) {
+ if ((algtype != QCRYPTODEV_BACKEND_ALG_SYM)
+ && (algtype != QCRYPTODEV_BACKEND_ALG_ASYM)) {
error_report("Unsupported cryptodev alg type: %" PRIu32 "", algtype);
return -VIRTIO_CRYPTO_NOTSUPP;
}
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 97da74e719..e0a7e2cbd8 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -462,7 +462,7 @@ static void virtio_crypto_init_request(VirtIOCrypto *vcrypto, VirtQueue *vq,
req->in_iov = NULL;
req->in_num = 0;
req->in_len = 0;
- req->flags = CRYPTODEV_BACKEND_ALG__MAX;
+ req->flags = QCRYPTODEV_BACKEND_ALG__MAX;
memset(&req->op_info, 0x00, sizeof(req->op_info));
}
@@ -472,7 +472,7 @@ static void virtio_crypto_free_request(VirtIOCryptoReq *req)
return;
}
- if (req->flags == CRYPTODEV_BACKEND_ALG_SYM) {
+ if (req->flags == QCRYPTODEV_BACKEND_ALG_SYM) {
size_t max_len;
CryptoDevBackendSymOpInfo *op_info = req->op_info.u.sym_op_info;
@@ -485,7 +485,7 @@ static void virtio_crypto_free_request(VirtIOCryptoReq *req)
/* Zeroize and free request data structure */
memset(op_info, 0, sizeof(*op_info) + max_len);
g_free(op_info);
- } else if (req->flags == CRYPTODEV_BACKEND_ALG_ASYM) {
+ } else if (req->flags == QCRYPTODEV_BACKEND_ALG_ASYM) {
CryptoDevBackendAsymOpInfo *op_info = req->op_info.u.asym_op_info;
if (op_info) {
g_free(op_info->src);
@@ -570,10 +570,10 @@ static void virtio_crypto_req_complete(void *opaque, int ret)
VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
uint8_t status = -ret;
- if (req->flags == CRYPTODEV_BACKEND_ALG_SYM) {
+ if (req->flags == QCRYPTODEV_BACKEND_ALG_SYM) {
virtio_crypto_sym_input_data_helper(vdev, req, status,
req->op_info.u.sym_op_info);
- } else if (req->flags == CRYPTODEV_BACKEND_ALG_ASYM) {
+ } else if (req->flags == QCRYPTODEV_BACKEND_ALG_ASYM) {
virtio_crypto_akcipher_input_data_helper(vdev, req, status,
req->op_info.u.asym_op_info);
}
@@ -875,7 +875,7 @@ virtio_crypto_handle_request(VirtIOCryptoReq *request)
switch (opcode) {
case VIRTIO_CRYPTO_CIPHER_ENCRYPT:
case VIRTIO_CRYPTO_CIPHER_DECRYPT:
- op_info->algtype = request->flags = CRYPTODEV_BACKEND_ALG_SYM;
+ op_info->algtype = request->flags = QCRYPTODEV_BACKEND_ALG_SYM;
ret = virtio_crypto_handle_sym_req(vcrypto,
&req.u.sym_req, op_info,
out_iov, out_num);
@@ -885,7 +885,7 @@ virtio_crypto_handle_request(VirtIOCryptoReq *request)
case VIRTIO_CRYPTO_AKCIPHER_DECRYPT:
case VIRTIO_CRYPTO_AKCIPHER_SIGN:
case VIRTIO_CRYPTO_AKCIPHER_VERIFY:
- op_info->algtype = request->flags = CRYPTODEV_BACKEND_ALG_ASYM;
+ op_info->algtype = request->flags = QCRYPTODEV_BACKEND_ALG_ASYM;
ret = virtio_crypto_handle_asym_req(vcrypto,
&req.u.akcipher_req, op_info,
out_iov, out_num);
diff --git a/include/sysemu/cryptodev.h b/include/sysemu/cryptodev.h
index af152d09db..f68a4baf13 100644
--- a/include/sysemu/cryptodev.h
+++ b/include/sysemu/cryptodev.h
@@ -49,12 +49,6 @@ typedef struct CryptoDevBackendPeers CryptoDevBackendPeers;
typedef struct CryptoDevBackendClient
CryptoDevBackendClient;
-enum CryptoDevBackendAlgType {
- CRYPTODEV_BACKEND_ALG_SYM,
- CRYPTODEV_BACKEND_ALG_ASYM,
- CRYPTODEV_BACKEND_ALG__MAX,
-};
-
/**
* CryptoDevBackendSymSessionInfo:
*
@@ -181,7 +175,7 @@ typedef struct CryptoDevBackendAsymOpInfo {
} CryptoDevBackendAsymOpInfo;
typedef struct CryptoDevBackendOpInfo {
- enum CryptoDevBackendAlgType algtype;
+ enum QCryptodevBackendAlgType algtype;
uint32_t op_code;
uint64_t session_id;
union {
diff --git a/qapi/cryptodev.json b/qapi/cryptodev.json
index b65edbe183..ebb6852035 100644
--- a/qapi/cryptodev.json
+++ b/qapi/cryptodev.json
@@ -4,6 +4,20 @@
# This work is licensed under the terms of the GNU GPL, version 2 or later.
# See the COPYING file in the top-level directory.
+##
+# @QCryptodevBackendAlgType:
+#
+# The supported algorithm types of a crypto device.
+#
+# @sym: symmetric encryption
+# @asym: asymmetric Encryption
+#
+# Since: 8.0
+##
+{ 'enum': 'QCryptodevBackendAlgType',
+ 'prefix': 'QCRYPTODEV_BACKEND_ALG',
+ 'data': ['sym', 'asym']}
+
##
# @QCryptodevBackendType:
#
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [for-8.0 v2 04/11] cryptodev: Introduce server type in QAPI
2022-11-22 14:07 [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
` (2 preceding siblings ...)
2022-11-22 14:07 ` [for-8.0 v2 03/11] cryptodev: Introduce cryptodev alg type in QAPI zhenwei pi
@ 2022-11-22 14:07 ` zhenwei pi
2023-01-16 11:09 ` Daniel P. Berrangé
2022-11-22 14:07 ` [for-8.0 v2 05/11] cryptodev: Introduce 'query-cryptodev' QMP command zhenwei pi
` (8 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: zhenwei pi @ 2022-11-22 14:07 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini
Cc: qemu-devel, zhenwei pi
Introduce cryptodev service type in cryptodev.json, then apply this
to related codes. Now we can remove VIRTIO_CRYPTO_SERVICE_xxx
dependence from QEMU cryptodev.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
backends/cryptodev-builtin.c | 8 ++++----
backends/cryptodev-lkcf.c | 2 +-
backends/cryptodev-vhost-user.c | 6 +++---
hw/virtio/virtio-crypto.c | 27 +++++++++++++++++++++++++--
qapi/cryptodev.json | 11 +++++++++++
5 files changed, 44 insertions(+), 10 deletions(-)
diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
index 5fb7b8f43f..4987abb7d6 100644
--- a/backends/cryptodev-builtin.c
+++ b/backends/cryptodev-builtin.c
@@ -79,10 +79,10 @@ static void cryptodev_builtin_init(
backend->conf.peers.ccs[0] = cc;
backend->conf.crypto_services =
- 1u << VIRTIO_CRYPTO_SERVICE_CIPHER |
- 1u << VIRTIO_CRYPTO_SERVICE_HASH |
- 1u << VIRTIO_CRYPTO_SERVICE_MAC |
- 1u << VIRTIO_CRYPTO_SERVICE_AKCIPHER;
+ 1u << QCRYPTODEV_BACKEND_SERVICE_CIPHER |
+ 1u << QCRYPTODEV_BACKEND_SERVICE_HASH |
+ 1u << QCRYPTODEV_BACKEND_SERVICE_MAC |
+ 1u << QCRYPTODEV_BACKEND_SERVICE_AKCIPHER;
backend->conf.cipher_algo_l = 1u << VIRTIO_CRYPTO_CIPHER_AES_CBC;
backend->conf.hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1;
backend->conf.akcipher_algo = 1u << VIRTIO_CRYPTO_AKCIPHER_RSA;
diff --git a/backends/cryptodev-lkcf.c b/backends/cryptodev-lkcf.c
index 919bf05b75..31ec712849 100644
--- a/backends/cryptodev-lkcf.c
+++ b/backends/cryptodev-lkcf.c
@@ -230,7 +230,7 @@ static void cryptodev_lkcf_init(CryptoDevBackend *backend, Error **errp)
backend->conf.peers.ccs[0] = cc;
backend->conf.crypto_services =
- 1u << VIRTIO_CRYPTO_SERVICE_AKCIPHER;
+ 1u << QCRYPTODEV_BACKEND_SERVICE_AKCIPHER;
backend->conf.akcipher_algo = 1u << VIRTIO_CRYPTO_AKCIPHER_RSA;
lkcf->running = true;
diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c
index 580bd1abb0..b1d9eb735f 100644
--- a/backends/cryptodev-vhost-user.c
+++ b/backends/cryptodev-vhost-user.c
@@ -221,9 +221,9 @@ static void cryptodev_vhost_user_init(
cryptodev_vhost_user_event, NULL, s, NULL, true);
backend->conf.crypto_services =
- 1u << VIRTIO_CRYPTO_SERVICE_CIPHER |
- 1u << VIRTIO_CRYPTO_SERVICE_HASH |
- 1u << VIRTIO_CRYPTO_SERVICE_MAC;
+ 1u << QCRYPTODEV_BACKEND_SERVICE_CIPHER |
+ 1u << QCRYPTODEV_BACKEND_SERVICE_HASH |
+ 1u << QCRYPTODEV_BACKEND_SERVICE_MAC;
backend->conf.cipher_algo_l = 1u << VIRTIO_CRYPTO_CIPHER_AES_CBC;
backend->conf.hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1;
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index e0a7e2cbd8..87d9582bc1 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -997,12 +997,35 @@ static void virtio_crypto_reset(VirtIODevice *vdev)
}
}
+static uint32_t virtio_crypto_init_services(uint32_t qservices)
+{
+ uint32_t vservices = 0;
+
+ if (qservices & (1 << QCRYPTODEV_BACKEND_SERVICE_CIPHER)) {
+ vservices |= (1 << VIRTIO_CRYPTO_SERVICE_CIPHER);
+ }
+ if (qservices & (1 << QCRYPTODEV_BACKEND_SERVICE_HASH)) {
+ vservices |= (1 << VIRTIO_CRYPTO_SERVICE_HASH);
+ }
+ if (qservices & (1 << QCRYPTODEV_BACKEND_SERVICE_MAC)) {
+ vservices |= (1 << VIRTIO_CRYPTO_SERVICE_MAC);
+ }
+ if (qservices & (1 << QCRYPTODEV_BACKEND_SERVICE_AEAD)) {
+ vservices |= (1 << VIRTIO_CRYPTO_SERVICE_AEAD);
+ }
+ if (qservices & (1 << QCRYPTODEV_BACKEND_SERVICE_AKCIPHER)) {
+ vservices |= (1 << VIRTIO_CRYPTO_SERVICE_AKCIPHER);
+ }
+
+ return vservices;
+}
+
static void virtio_crypto_init_config(VirtIODevice *vdev)
{
VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(vdev);
- vcrypto->conf.crypto_services =
- vcrypto->conf.cryptodev->conf.crypto_services;
+ vcrypto->conf.crypto_services = virtio_crypto_init_services(
+ vcrypto->conf.cryptodev->conf.crypto_services);
vcrypto->conf.cipher_algo_l =
vcrypto->conf.cryptodev->conf.cipher_algo_l;
vcrypto->conf.cipher_algo_h =
diff --git a/qapi/cryptodev.json b/qapi/cryptodev.json
index ebb6852035..8732a30524 100644
--- a/qapi/cryptodev.json
+++ b/qapi/cryptodev.json
@@ -18,6 +18,17 @@
'prefix': 'QCRYPTODEV_BACKEND_ALG',
'data': ['sym', 'asym']}
+##
+# @QCryptodevBackendServiceType:
+#
+# The supported service types of a crypto device.
+#
+# Since: 8.0
+##
+{ 'enum': 'QCryptodevBackendServiceType',
+ 'prefix': 'QCRYPTODEV_BACKEND_SERVICE',
+ 'data': ['cipher', 'hash', 'mac', 'aead', 'akcipher']}
+
##
# @QCryptodevBackendType:
#
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [for-8.0 v2 05/11] cryptodev: Introduce 'query-cryptodev' QMP command
2022-11-22 14:07 [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
` (3 preceding siblings ...)
2022-11-22 14:07 ` [for-8.0 v2 04/11] cryptodev: Introduce server " zhenwei pi
@ 2022-11-22 14:07 ` zhenwei pi
2023-01-16 11:18 ` Daniel P. Berrangé
2022-11-22 14:07 ` [for-8.0 v2 06/11] cryptodev: Support statistics zhenwei pi
` (7 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: zhenwei pi @ 2022-11-22 14:07 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini
Cc: qemu-devel, zhenwei pi
Now we have a QMP command to query crypto devices:
virsh qemu-monitor-command vm '{"execute": "query-cryptodev"}' | jq
{
"return": [
{
"service": [
"akcipher",
"mac",
"hash",
"cipher"
],
"id": "cryptodev1",
"client": [
{
"queue": 0,
"type": "builtin",
"info": "cryptodev-builtin0"
}
]
},
{
"service": [
"akcipher"
],
"id": "cryptodev0",
"client": [
{
"queue": 0,
"type": "lkcf",
"info": "cryptodev-lkcf0"
}
]
}
],
"id": "libvirt-415"
}
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
backends/cryptodev.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
qapi/cryptodev.json | 43 ++++++++++++++++++++++++++++++++++++++
2 files changed, 92 insertions(+)
diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index d3caded920..bf2f3234c9 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -24,6 +24,7 @@
#include "qemu/osdep.h"
#include "sysemu/cryptodev.h"
#include "qapi/error.h"
+#include "qapi/qapi-commands-cryptodev.h"
#include "qapi/visitor.h"
#include "qemu/config-file.h"
#include "qemu/error-report.h"
@@ -33,6 +34,54 @@
static QTAILQ_HEAD(, CryptoDevBackendClient) crypto_clients;
+static int qmp_query_cryptodev_foreach(Object *obj, void *data)
+{
+ CryptoDevBackend *backend;
+ CryptodevInfoList **infolist = data;
+ uint32_t services;
+
+ if (!object_dynamic_cast(obj, TYPE_CRYPTODEV_BACKEND)) {
+ return 0;
+ }
+
+ CryptodevInfo *info = g_new0(CryptodevInfo, 1);
+ info->id = g_strdup(object_get_canonical_path_component(obj));
+
+ backend = CRYPTODEV_BACKEND(obj);
+ services = backend->conf.crypto_services;
+ for (uint32_t i = 0; i < QCRYPTODEV_BACKEND_SERVICE__MAX; i++) {
+ if (services & (1 << i)) {
+ QAPI_LIST_PREPEND(info->service, i);
+ }
+ }
+
+ for (uint32_t i = 0; i < backend->conf.peers.queues; i++) {
+ CryptoDevBackendClient *cc = backend->conf.peers.ccs[i];
+ CryptodevBackendClient *client = g_new0(CryptodevBackendClient, 1);
+
+ client->queue = cc->queue_index;
+ client->type = cc->type;
+ if (cc->info_str) {
+ client->has_info = true;
+ client->info = strdup(cc->info_str);
+ }
+ QAPI_LIST_PREPEND(info->client, client);
+ }
+
+ QAPI_LIST_PREPEND(*infolist, info);
+
+ return 0;
+}
+
+CryptodevInfoList *qmp_query_cryptodev(Error **errp)
+{
+ CryptodevInfoList *list = NULL;
+ Object *objs = container_get(object_get_root(), "/objects");
+
+ object_child_foreach(objs, qmp_query_cryptodev_foreach, &list);
+
+ return list;
+}
CryptoDevBackendClient *cryptodev_backend_new_client(void)
{
diff --git a/qapi/cryptodev.json b/qapi/cryptodev.json
index 8732a30524..4cc4f4f0ed 100644
--- a/qapi/cryptodev.json
+++ b/qapi/cryptodev.json
@@ -43,3 +43,46 @@
{ 'enum': 'QCryptodevBackendType',
'prefix': 'QCRYPTODEV_BACKEND_TYPE',
'data': ['builtin', 'vhost-user', 'lkcf']}
+
+##
+# @CryptodevBackendClient:
+#
+# Information about a queue of crypto device.
+#
+# @type: the type of the crypto device
+#
+# @info: the additional infomation of the crypto device
+#
+# Since: 8.0
+##
+{ 'struct': 'CryptodevBackendClient',
+ 'data': { 'queue': 'int',
+ 'type': 'QCryptodevBackendType',
+ '*info': 'str' } }
+
+##
+# @CryptodevInfo:
+#
+# Information about a crypto device.
+#
+# @service: supported service types of a crypto device
+#
+# @client: the additional infomation of the crypto device
+#
+# Since: 8.0
+##
+{ 'struct': 'CryptodevInfo',
+ 'data': { 'id': 'str',
+ 'service': ['QCryptodevBackendServiceType'],
+ 'client': ['CryptodevBackendClient'] } }
+
+##
+# @query-cryptodev:
+#
+# Returns information about current crypto devices.
+#
+# Returns: a list of @CryptodevInfo
+#
+# Since: 8.0
+##
+{ 'command': 'query-cryptodev', 'returns': ['CryptodevInfo']}
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [for-8.0 v2 06/11] cryptodev: Support statistics
2022-11-22 14:07 [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
` (4 preceding siblings ...)
2022-11-22 14:07 ` [for-8.0 v2 05/11] cryptodev: Introduce 'query-cryptodev' QMP command zhenwei pi
@ 2022-11-22 14:07 ` zhenwei pi
2022-12-20 15:35 ` Michael S. Tsirkin
2023-01-16 11:22 ` Daniel P. Berrangé
2022-11-22 14:07 ` [for-8.0 v2 07/11] cryptodev-builtin: Detect akcipher capability zhenwei pi
` (6 subsequent siblings)
12 siblings, 2 replies; 31+ messages in thread
From: zhenwei pi @ 2022-11-22 14:07 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini
Cc: qemu-devel, zhenwei pi
Introduce cryptodev statistics in QAPI, and record OPS/Bandwidth for
each crypto device.
Example of this feature:
virsh qemu-monitor-command vm '{"execute": "query-cryptodev"}' | jq
{
"return": [
{
"service": [
"akcipher",
"mac",
"hash",
"cipher"
],
"asym-stat": {
"encrypt-ops": 0,
"verify-bytes": 0,
"sign-ops": 0,
"verify-ops": 0,
"sign-bytes": 0,
"decrypt-bytes": 0,
"decrypt-ops": 0,
"encrypt-bytes": 0
},
"sym-stat": {
"encrypt-ops": 40,
"decrypt-bytes": 5376,
"decrypt-ops": 40,
"encrypt-bytes": 5376
},
"id": "cryptodev1",
"client": [
{
"queue": 0,
"type": "builtin",
"info": "cryptodev-builtin0"
}
]
},
{
"service": [
"akcipher"
],
"asym-stat": {
"encrypt-ops": 54,
"verify-bytes": 8704,
"sign-ops": 17,
"verify-ops": 34,
"sign-bytes": 340,
"decrypt-bytes": 9215,
"decrypt-ops": 36,
"encrypt-bytes": 13294
},
"id": "cryptodev0",
"client": [
{
"queue": 0,
"type": "lkcf",
"info": "cryptodev-lkcf0"
}
]
}
],
"id": "libvirt-424"
}
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
backends/cryptodev.c | 81 +++++++++++++++++++++++++++++++++++---
include/sysemu/cryptodev.h | 30 ++++++++++++++
qapi/cryptodev.json | 58 ++++++++++++++++++++++++++-
3 files changed, 162 insertions(+), 7 deletions(-)
diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index bf2f3234c9..d623bf3bff 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -48,6 +48,18 @@ static int qmp_query_cryptodev_foreach(Object *obj, void *data)
info->id = g_strdup(object_get_canonical_path_component(obj));
backend = CRYPTODEV_BACKEND(obj);
+ if (backend->sym_stat) {
+ info->has_sym_stat = true;
+ info->sym_stat = g_memdup2(backend->sym_stat,
+ sizeof(QCryptodevBackendSymStat));
+ }
+
+ if (backend->asym_stat) {
+ info->has_asym_stat = true;
+ info->asym_stat = g_memdup2(backend->asym_stat,
+ sizeof(QCryptodevBackendAsymStat));
+ }
+
services = backend->conf.crypto_services;
for (uint32_t i = 0; i < QCRYPTODEV_BACKEND_SERVICE__MAX; i++) {
if (services & (1 << i)) {
@@ -111,6 +123,9 @@ void cryptodev_backend_cleanup(
if (bc->cleanup) {
bc->cleanup(backend, errp);
}
+
+ g_free(backend->sym_stat);
+ g_free(backend->asym_stat);
}
int cryptodev_backend_create_session(
@@ -161,6 +176,52 @@ static int cryptodev_backend_operation(
return -VIRTIO_CRYPTO_NOTSUPP;
}
+static int cryptodev_backend_account(CryptoDevBackend *backend,
+ CryptoDevBackendOpInfo *op_info)
+{
+ enum QCryptodevBackendAlgType algtype = op_info->algtype;
+ int len;
+
+ if (algtype == QCRYPTODEV_BACKEND_ALG_ASYM) {
+ CryptoDevBackendAsymOpInfo *asym_op_info = op_info->u.asym_op_info;
+ len = asym_op_info->src_len;
+ switch (op_info->op_code) {
+ case VIRTIO_CRYPTO_AKCIPHER_ENCRYPT:
+ QCryptodevAsymStatIncEncrypt(backend, len);
+ break;
+ case VIRTIO_CRYPTO_AKCIPHER_DECRYPT:
+ QCryptodevAsymStatIncDecrypt(backend, len);
+ break;
+ case VIRTIO_CRYPTO_AKCIPHER_SIGN:
+ QCryptodevAsymStatIncSign(backend, len);
+ break;
+ case VIRTIO_CRYPTO_AKCIPHER_VERIFY:
+ QCryptodevAsymStatIncVerify(backend, len);
+ break;
+ default:
+ return -VIRTIO_CRYPTO_NOTSUPP;
+ }
+ } else if (algtype == QCRYPTODEV_BACKEND_ALG_SYM) {
+ CryptoDevBackendSymOpInfo *sym_op_info = op_info->u.sym_op_info;
+ len = sym_op_info->src_len;
+ switch (op_info->op_code) {
+ case VIRTIO_CRYPTO_CIPHER_ENCRYPT:
+ QCryptodevSymStatIncEncrypt(backend, len);
+ break;
+ case VIRTIO_CRYPTO_CIPHER_DECRYPT:
+ QCryptodevSymStatIncDecrypt(backend, len);
+ break;
+ default:
+ return -VIRTIO_CRYPTO_NOTSUPP;
+ }
+ } else {
+ error_report("Unsupported cryptodev alg type: %" PRIu32 "", algtype);
+ return -VIRTIO_CRYPTO_NOTSUPP;
+ }
+
+ return len;
+}
+
int cryptodev_backend_crypto_operation(
CryptoDevBackend *backend,
void *opaque1,
@@ -169,14 +230,12 @@ int cryptodev_backend_crypto_operation(
{
VirtIOCryptoReq *req = opaque1;
CryptoDevBackendOpInfo *op_info = &req->op_info;
- enum QCryptodevBackendAlgType algtype = req->flags;
+ int ret;
- if ((algtype != QCRYPTODEV_BACKEND_ALG_SYM)
- && (algtype != QCRYPTODEV_BACKEND_ALG_ASYM)) {
- error_report("Unsupported cryptodev alg type: %" PRIu32 "", algtype);
- return -VIRTIO_CRYPTO_NOTSUPP;
+ ret = cryptodev_backend_account(backend, op_info);
+ if (ret < 0) {
+ return ret;
}
-
return cryptodev_backend_operation(backend, op_info, queue_index,
cb, opaque2);
}
@@ -214,10 +273,20 @@ cryptodev_backend_complete(UserCreatable *uc, Error **errp)
{
CryptoDevBackend *backend = CRYPTODEV_BACKEND(uc);
CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_GET_CLASS(uc);
+ uint32_t services;
if (bc->init) {
bc->init(backend, errp);
}
+
+ services = backend->conf.crypto_services;
+ if (services & (1 << QCRYPTODEV_BACKEND_SERVICE_CIPHER)) {
+ backend->sym_stat = g_new0(QCryptodevBackendSymStat, 1);
+ }
+
+ if (services & (1 << QCRYPTODEV_BACKEND_SERVICE_AKCIPHER)) {
+ backend->asym_stat = g_new0(QCryptodevBackendAsymStat, 1);
+ }
}
void cryptodev_backend_set_used(CryptoDevBackend *backend, bool used)
diff --git a/include/sysemu/cryptodev.h b/include/sysemu/cryptodev.h
index f68a4baf13..c154c52039 100644
--- a/include/sysemu/cryptodev.h
+++ b/include/sysemu/cryptodev.h
@@ -252,8 +252,38 @@ struct CryptoDevBackend {
/* Tag the cryptodev backend is used by virtio-crypto or not */
bool is_used;
CryptoDevBackendConf conf;
+ QCryptodevBackendSymStat *sym_stat;
+ QCryptodevBackendAsymStat *asym_stat;
};
+#define QCryptodevSymStatInc(be, op, bytes) do { \
+ be->sym_stat->op##_bytes += (bytes); \
+ be->sym_stat->op##_ops += 1; \
+} while (/*CONSTCOND*/0)
+
+#define QCryptodevSymStatIncEncrypt(be, bytes) \
+ QCryptodevSymStatInc(be, encrypt, bytes)
+
+#define QCryptodevSymStatIncDecrypt(be, bytes) \
+ QCryptodevSymStatInc(be, decrypt, bytes)
+
+#define QCryptodevAsymStatInc(be, op, bytes) do { \
+ be->asym_stat->op##_bytes += (bytes); \
+ be->asym_stat->op##_ops += 1; \
+} while (/*CONSTCOND*/0)
+
+#define QCryptodevAsymStatIncEncrypt(be, bytes) \
+ QCryptodevAsymStatInc(be, encrypt, bytes)
+
+#define QCryptodevAsymStatIncDecrypt(be, bytes) \
+ QCryptodevAsymStatInc(be, decrypt, bytes)
+
+#define QCryptodevAsymStatIncSign(be, bytes) \
+ QCryptodevAsymStatInc(be, sign, bytes)
+
+#define QCryptodevAsymStatIncVerify(be, bytes) \
+ QCryptodevAsymStatInc(be, verify, bytes)
+
/**
* cryptodev_backend_new_client:
*
diff --git a/qapi/cryptodev.json b/qapi/cryptodev.json
index 4cc4f4f0ed..f01f2d017a 100644
--- a/qapi/cryptodev.json
+++ b/qapi/cryptodev.json
@@ -60,6 +60,60 @@
'type': 'QCryptodevBackendType',
'*info': 'str' } }
+##
+# @QCryptodevBackendSymStat:
+#
+# The statistics of symmetric operation.
+#
+# @encrypt-ops: the operations of symmetric encryption
+#
+# @decrypt-ops: the operations of symmetric decryption
+#
+# @encrypt-bytes: the bytes of symmetric encryption
+#
+# @decrypt-bytes: the bytes of symmetric decryption
+#
+# Since: 8.0
+##
+{ 'struct': 'QCryptodevBackendSymStat',
+ 'data': { 'encrypt-ops': 'int',
+ 'decrypt-ops': 'int',
+ 'encrypt-bytes': 'int',
+ 'decrypt-bytes': 'int' } }
+
+##
+# @QCryptodevBackendAsymStat:
+#
+# The statistics of asymmetric operation.
+#
+# @encrypt-ops: the operations of asymmetric encryption
+#
+# @decrypt-ops: the operations of asymmetric decryption
+#
+# @sign-ops: the operations of asymmetric signature
+#
+# @verify-ops: the operations of asymmetric verification
+#
+# @encrypt-bytes: the bytes of asymmetric encryption
+#
+# @decrypt-bytes: the bytes of asymmetric decryption
+#
+# @sign-bytes: the bytes of asymmetric signature
+#
+# @verify-bytes: the bytes of asymmetric verification
+#
+# Since: 8.0
+##
+{ 'struct': 'QCryptodevBackendAsymStat',
+ 'data': { 'encrypt-ops': 'int',
+ 'decrypt-ops': 'int',
+ 'sign-ops': 'int',
+ 'verify-ops': 'int',
+ 'encrypt-bytes': 'int',
+ 'decrypt-bytes': 'int',
+ 'sign-bytes': 'int',
+ 'verify-bytes': 'int' } }
+
##
# @CryptodevInfo:
#
@@ -74,7 +128,9 @@
{ 'struct': 'CryptodevInfo',
'data': { 'id': 'str',
'service': ['QCryptodevBackendServiceType'],
- 'client': ['CryptodevBackendClient'] } }
+ 'client': ['CryptodevBackendClient'],
+ '*sym-stat': 'QCryptodevBackendSymStat',
+ '*asym-stat': 'QCryptodevBackendAsymStat' } }
##
# @query-cryptodev:
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [for-8.0 v2 07/11] cryptodev-builtin: Detect akcipher capability
2022-11-22 14:07 [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
` (5 preceding siblings ...)
2022-11-22 14:07 ` [for-8.0 v2 06/11] cryptodev: Support statistics zhenwei pi
@ 2022-11-22 14:07 ` zhenwei pi
2023-01-16 11:23 ` Daniel P. Berrangé
2022-11-22 14:07 ` [for-8.0 v2 08/11] hmp: add cryptodev info command zhenwei pi
` (5 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: zhenwei pi @ 2022-11-22 14:07 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini
Cc: qemu-devel, zhenwei pi
Rather than exposing akcipher service/RSA algorithm to virtio crypto
device unconditionally, detect akcipher capability from akcipher
crypto framework. This avoids unsuccessful requests.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
backends/cryptodev-builtin.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
index 4987abb7d6..94a02aeaf1 100644
--- a/backends/cryptodev-builtin.c
+++ b/backends/cryptodev-builtin.c
@@ -59,6 +59,19 @@ struct CryptoDevBackendBuiltin {
CryptoDevBackendBuiltinSession *sessions[MAX_NUM_SESSIONS];
};
+static void cryptodev_builtin_init_akcipher(CryptoDevBackend *backend)
+{
+ QCryptoAkCipherOptions opts;
+
+ opts.alg = QCRYPTO_AKCIPHER_ALG_RSA;
+ opts.u.rsa.padding_alg = QCRYPTO_RSA_PADDING_ALG_RAW;
+ if (qcrypto_akcipher_supports(&opts)) {
+ backend->conf.crypto_services |=
+ (1u << QCRYPTODEV_BACKEND_SERVICE_AKCIPHER);
+ backend->conf.akcipher_algo = 1u << VIRTIO_CRYPTO_AKCIPHER_RSA;
+ }
+}
+
static void cryptodev_builtin_init(
CryptoDevBackend *backend, Error **errp)
{
@@ -81,11 +94,9 @@ static void cryptodev_builtin_init(
backend->conf.crypto_services =
1u << QCRYPTODEV_BACKEND_SERVICE_CIPHER |
1u << QCRYPTODEV_BACKEND_SERVICE_HASH |
- 1u << QCRYPTODEV_BACKEND_SERVICE_MAC |
- 1u << QCRYPTODEV_BACKEND_SERVICE_AKCIPHER;
+ 1u << QCRYPTODEV_BACKEND_SERVICE_MAC;
backend->conf.cipher_algo_l = 1u << VIRTIO_CRYPTO_CIPHER_AES_CBC;
backend->conf.hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1;
- backend->conf.akcipher_algo = 1u << VIRTIO_CRYPTO_AKCIPHER_RSA;
/*
* Set the Maximum length of crypto request.
* Why this value? Just avoid to overflow when
@@ -94,6 +105,7 @@ static void cryptodev_builtin_init(
backend->conf.max_size = LONG_MAX - sizeof(CryptoDevBackendOpInfo);
backend->conf.max_cipher_key_len = CRYPTODEV_BUITLIN_MAX_CIPHER_KEY_LEN;
backend->conf.max_auth_key_len = CRYPTODEV_BUITLIN_MAX_AUTH_KEY_LEN;
+ cryptodev_builtin_init_akcipher(backend);
cryptodev_backend_set_ready(backend, true);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [for-8.0 v2 08/11] hmp: add cryptodev info command
2022-11-22 14:07 [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
` (6 preceding siblings ...)
2022-11-22 14:07 ` [for-8.0 v2 07/11] cryptodev-builtin: Detect akcipher capability zhenwei pi
@ 2022-11-22 14:07 ` zhenwei pi
2022-11-22 14:07 ` [for-8.0 v2 09/11] cryptodev: Use CryptoDevBackendOpInfo for operation zhenwei pi
` (4 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: zhenwei pi @ 2022-11-22 14:07 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini
Cc: qemu-devel, zhenwei pi
Example of this command:
# virsh qemu-monitor-command vm --hmp info cryptodev
cryptodev1: service=[akcipher|mac|hash|cipher]
queue 0: type=builtin
cryptodev0: service=[akcipher]
queue 0: type=lkcf
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
hmp-commands-info.hx | 14 ++++++++++++++
include/monitor/hmp.h | 1 +
monitor/hmp-cmds.c | 36 ++++++++++++++++++++++++++++++++++++
3 files changed, 51 insertions(+)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 754b1e8408..47d63d26db 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -993,3 +993,17 @@ SRST
``info virtio-queue-element`` *path* *queue* [*index*]
Display element of a given virtio queue
ERST
+
+ {
+ .name = "cryptodev",
+ .args_type = "",
+ .params = "",
+ .help = "show the crypto devices",
+ .cmd = hmp_info_cryptodev,
+ .flags = "p",
+ },
+
+SRST
+ ``info cryptodev``
+ Show the crypto devices.
+ERST
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index dfbc0c9a2f..b6b2b49202 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -143,5 +143,6 @@ void hmp_info_vcpu_dirty_limit(Monitor *mon, const QDict *qdict);
void hmp_human_readable_text_helper(Monitor *mon,
HumanReadableText *(*qmp_handler)(Error **));
void hmp_info_stats(Monitor *mon, const QDict *qdict);
+void hmp_info_cryptodev(Monitor *mon, const QDict *qdict);
#endif
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 01b789a79e..d5c9e8977d 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -33,6 +33,7 @@
#include "qapi/qapi-commands-block.h"
#include "qapi/qapi-commands-char.h"
#include "qapi/qapi-commands-control.h"
+#include "qapi/qapi-commands-cryptodev.h"
#include "qapi/qapi-commands-machine.h"
#include "qapi/qapi-commands-migration.h"
#include "qapi/qapi-commands-misc.h"
@@ -2761,3 +2762,38 @@ void hmp_virtio_queue_element(Monitor *mon, const QDict *qdict)
qapi_free_VirtioQueueElement(e);
}
+
+void hmp_info_cryptodev(Monitor *mon, const QDict *qdict)
+{
+ CryptodevInfoList *il;
+ QCryptodevBackendServiceTypeList *sl;
+ CryptodevBackendClientList *cl;
+
+ for (il = qmp_query_cryptodev(NULL); il; il = il->next) {
+ g_autofree char *services = NULL;
+ CryptodevInfo *info = il->value;
+ char *tmp_services;
+
+ /* build a string like 'service=[akcipher|mac|hash|cipher]' */
+ for (sl = info->service; sl; sl = sl->next) {
+ const char *service = QCryptodevBackendServiceType_str(sl->value);
+
+ if (!services) {
+ services = g_strdup(service);
+ } else {
+ tmp_services = g_strjoin("|", services, service, NULL);
+ g_free(services);
+ services = tmp_services;
+ }
+ }
+ monitor_printf(mon, "%s: service=[%s]\n", info->id, services);
+
+ for (cl = info->client; cl; cl = cl->next) {
+ CryptodevBackendClient *client = cl->value;
+ monitor_printf(mon, " queue %ld: type=%s\n", client->queue,
+ QCryptodevBackendType_str(client->type));
+ }
+ }
+
+ qapi_free_CryptodevInfoList(il);
+}
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [for-8.0 v2 09/11] cryptodev: Use CryptoDevBackendOpInfo for operation
2022-11-22 14:07 [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
` (7 preceding siblings ...)
2022-11-22 14:07 ` [for-8.0 v2 08/11] hmp: add cryptodev info command zhenwei pi
@ 2022-11-22 14:07 ` zhenwei pi
2022-11-22 14:07 ` [for-8.0 v2 10/11] cryptodev: support QoS zhenwei pi
` (3 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: zhenwei pi @ 2022-11-22 14:07 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini
Cc: qemu-devel, zhenwei pi
Move queue_index, CryptoDevCompletionFunc and opaque into struct
CryptoDevBackendOpInfo, then cryptodev_backend_crypto_operation()
needs an argument CryptoDevBackendOpInfo *op_info only. And remove
VirtIOCryptoReq from cryptodev.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
backends/cryptodev-builtin.c | 9 +++------
backends/cryptodev-lkcf.c | 9 +++------
backends/cryptodev.c | 16 ++++------------
hw/virtio/virtio-crypto.c | 7 ++++---
include/sysemu/cryptodev.h | 26 ++++++++++----------------
5 files changed, 24 insertions(+), 43 deletions(-)
diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
index 94a02aeaf1..68b1cbd440 100644
--- a/backends/cryptodev-builtin.c
+++ b/backends/cryptodev-builtin.c
@@ -539,10 +539,7 @@ static int cryptodev_builtin_asym_operation(
static int cryptodev_builtin_operation(
CryptoDevBackend *backend,
- CryptoDevBackendOpInfo *op_info,
- uint32_t queue_index,
- CryptoDevCompletionFunc cb,
- void *opaque)
+ CryptoDevBackendOpInfo *op_info)
{
CryptoDevBackendBuiltin *builtin =
CRYPTODEV_BACKEND_BUILTIN(backend);
@@ -574,8 +571,8 @@ static int cryptodev_builtin_operation(
if (local_error) {
error_report_err(local_error);
}
- if (cb) {
- cb(opaque, status);
+ if (op_info->cb) {
+ op_info->cb(op_info->opaque, status);
}
return 0;
}
diff --git a/backends/cryptodev-lkcf.c b/backends/cryptodev-lkcf.c
index 31ec712849..0e88f1ecac 100644
--- a/backends/cryptodev-lkcf.c
+++ b/backends/cryptodev-lkcf.c
@@ -469,10 +469,7 @@ static void *cryptodev_lkcf_worker(void *arg)
static int cryptodev_lkcf_operation(
CryptoDevBackend *backend,
- CryptoDevBackendOpInfo *op_info,
- uint32_t queue_index,
- CryptoDevCompletionFunc cb,
- void *opaque)
+ CryptoDevBackendOpInfo *op_info)
{
CryptoDevBackendLKCF *lkcf =
CRYPTODEV_BACKEND_LKCF(backend);
@@ -495,8 +492,8 @@ static int cryptodev_lkcf_operation(
task = g_new0(CryptoDevLKCFTask, 1);
task->op_info = op_info;
- task->cb = cb;
- task->opaque = opaque;
+ task->cb = op_info->cb;
+ task->opaque = op_info->opaque;
task->sess = sess;
task->lkcf = lkcf;
task->status = -VIRTIO_CRYPTO_ERR;
diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index d623bf3bff..72105df95a 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -162,16 +162,13 @@ int cryptodev_backend_close_session(
static int cryptodev_backend_operation(
CryptoDevBackend *backend,
- CryptoDevBackendOpInfo *op_info,
- uint32_t queue_index,
- CryptoDevCompletionFunc cb,
- void *opaque)
+ CryptoDevBackendOpInfo *op_info)
{
CryptoDevBackendClass *bc =
CRYPTODEV_BACKEND_GET_CLASS(backend);
if (bc->do_op) {
- return bc->do_op(backend, op_info, queue_index, cb, opaque);
+ return bc->do_op(backend, op_info);
}
return -VIRTIO_CRYPTO_NOTSUPP;
}
@@ -224,20 +221,15 @@ static int cryptodev_backend_account(CryptoDevBackend *backend,
int cryptodev_backend_crypto_operation(
CryptoDevBackend *backend,
- void *opaque1,
- uint32_t queue_index,
- CryptoDevCompletionFunc cb, void *opaque2)
+ CryptoDevBackendOpInfo *op_info)
{
- VirtIOCryptoReq *req = opaque1;
- CryptoDevBackendOpInfo *op_info = &req->op_info;
int ret;
ret = cryptodev_backend_account(backend, op_info);
if (ret < 0) {
return ret;
}
- return cryptodev_backend_operation(backend, op_info, queue_index,
- cb, opaque2);
+ return cryptodev_backend_operation(backend, op_info);
}
static void
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 87d9582bc1..e637fc6ab0 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -871,6 +871,9 @@ virtio_crypto_handle_request(VirtIOCryptoReq *request)
opcode = ldl_le_p(&req.header.opcode);
op_info->session_id = ldq_le_p(&req.header.session_id);
op_info->op_code = opcode;
+ op_info->queue_index = queue_index;
+ op_info->cb = virtio_crypto_req_complete;
+ op_info->opaque = request;
switch (opcode) {
case VIRTIO_CRYPTO_CIPHER_ENCRYPT:
@@ -898,9 +901,7 @@ check_result:
virtio_crypto_req_complete(request, -VIRTIO_CRYPTO_NOTSUPP);
} else {
ret = cryptodev_backend_crypto_operation(vcrypto->cryptodev,
- request, queue_index,
- virtio_crypto_req_complete,
- request);
+ op_info);
if (ret < 0) {
virtio_crypto_req_complete(request, ret);
}
diff --git a/include/sysemu/cryptodev.h b/include/sysemu/cryptodev.h
index c154c52039..ffbce1129c 100644
--- a/include/sysemu/cryptodev.h
+++ b/include/sysemu/cryptodev.h
@@ -174,9 +174,14 @@ typedef struct CryptoDevBackendAsymOpInfo {
uint8_t *dst;
} CryptoDevBackendAsymOpInfo;
+typedef void (*CryptoDevCompletionFunc) (void *opaque, int ret);
+
typedef struct CryptoDevBackendOpInfo {
enum QCryptodevBackendAlgType algtype;
uint32_t op_code;
+ uint32_t queue_index;
+ CryptoDevCompletionFunc cb;
+ void *opaque;
uint64_t session_id;
union {
CryptoDevBackendSymOpInfo *sym_op_info;
@@ -184,7 +189,6 @@ typedef struct CryptoDevBackendOpInfo {
} u;
} CryptoDevBackendOpInfo;
-typedef void (*CryptoDevCompletionFunc) (void *opaque, int ret);
struct CryptoDevBackendClass {
ObjectClass parent_class;
@@ -204,10 +208,7 @@ struct CryptoDevBackendClass {
void *opaque);
int (*do_op)(CryptoDevBackend *backend,
- CryptoDevBackendOpInfo *op_info,
- uint32_t queue_index,
- CryptoDevCompletionFunc cb,
- void *opaque);
+ CryptoDevBackendOpInfo *op_info);
};
struct CryptoDevBackendClient {
@@ -365,24 +366,17 @@ int cryptodev_backend_close_session(
/**
* cryptodev_backend_crypto_operation:
* @backend: the cryptodev backend object
- * @opaque1: pointer to a VirtIOCryptoReq object
- * @queue_index: queue index of cryptodev backend client
- * @errp: pointer to a NULL-initialized error object
- * @cb: callbacks when operation is completed
- * @opaque2: parameter passed to cb
+ * @op_info: pointer to a CryptoDevBackendOpInfo object
*
- * Do crypto operation, such as encryption and
- * decryption
+ * Do crypto operation, such as encryption, decryption, signature and
+ * verification
*
* Returns: 0 for success and cb will be called when creation is completed,
* negative value for error, and cb will not be called.
*/
int cryptodev_backend_crypto_operation(
CryptoDevBackend *backend,
- void *opaque1,
- uint32_t queue_index,
- CryptoDevCompletionFunc cb,
- void *opaque2);
+ CryptoDevBackendOpInfo *op_info);
/**
* cryptodev_backend_set_used:
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [for-8.0 v2 10/11] cryptodev: support QoS
2022-11-22 14:07 [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
` (8 preceding siblings ...)
2022-11-22 14:07 ` [for-8.0 v2 09/11] cryptodev: Use CryptoDevBackendOpInfo for operation zhenwei pi
@ 2022-11-22 14:07 ` zhenwei pi
2022-11-22 14:07 ` [for-8.0 v2 11/11] MAINTAINERS: add myself as the maintainer for cryptodev zhenwei pi
` (2 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: zhenwei pi @ 2022-11-22 14:07 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini
Cc: qemu-devel, zhenwei pi
Add 'throttle-bps' and 'throttle-ops' limitation to set QoS. The
two arguments work with both QEMU command line and QMP command.
Example of QEMU command line:
-object cryptodev-backend-builtin,id=cryptodev1,throttle-bps=1600,\
throttle-ops=100
Example of QMP command:
virsh qemu-monitor-command buster --hmp qom-set /objects/cryptodev1 \
throttle-ops 100
or cancel limitation:
virsh qemu-monitor-command buster --hmp qom-set /objects/cryptodev1 \
throttle-ops 0
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
backends/cryptodev.c | 140 +++++++++++++++++++++++++++++++++++++
include/sysemu/cryptodev.h | 7 ++
qapi/qom.json | 8 ++-
3 files changed, 154 insertions(+), 1 deletion(-)
diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index 72105df95a..5bbaa6eecc 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -28,6 +28,7 @@
#include "qapi/visitor.h"
#include "qemu/config-file.h"
#include "qemu/error-report.h"
+#include "qemu/main-loop.h"
#include "qom/object_interfaces.h"
#include "hw/virtio/virtio-crypto.h"
@@ -219,16 +220,54 @@ static int cryptodev_backend_account(CryptoDevBackend *backend,
return len;
}
+static void cryptodev_backend_throttle_timer_cb(void *opaque)
+{
+ CryptoDevBackend *backend = (CryptoDevBackend *)opaque;
+ CryptoDevBackendOpInfo *op_info, *tmpop;
+ int ret;
+
+ QTAILQ_FOREACH_SAFE(op_info, &backend->opinfos, next, tmpop) {
+ QTAILQ_REMOVE(&backend->opinfos, op_info, next);
+ ret = cryptodev_backend_account(backend, op_info);
+ if (ret < 0) {
+ op_info->cb(op_info->opaque, ret);
+ continue;
+ }
+
+ throttle_account(&backend->ts, true, ret);
+ cryptodev_backend_operation(backend, op_info);
+ if (throttle_enabled(&backend->tc) &&
+ throttle_schedule_timer(&backend->ts, &backend->tt, true)) {
+ break;
+ }
+ }
+}
+
int cryptodev_backend_crypto_operation(
CryptoDevBackend *backend,
CryptoDevBackendOpInfo *op_info)
{
int ret;
+ if (!throttle_enabled(&backend->tc)) {
+ ret = cryptodev_backend_account(backend, op_info);
+ if (ret < 0) {
+ return ret;
+ }
+ return cryptodev_backend_operation(backend, op_info);
+ }
+
+ if (throttle_schedule_timer(&backend->ts, &backend->tt, true) ||
+ !QTAILQ_EMPTY(&backend->opinfos)) {
+ QTAILQ_INSERT_TAIL(&backend->opinfos, op_info, next);
+ return 0;
+ }
+
ret = cryptodev_backend_account(backend, op_info);
if (ret < 0) {
return ret;
}
+ throttle_account(&backend->ts, true, ret);
return cryptodev_backend_operation(backend, op_info);
}
@@ -260,12 +299,98 @@ cryptodev_backend_set_queues(Object *obj, Visitor *v, const char *name,
backend->conf.peers.queues = value;
}
+static void cryptodev_backend_set_throttle(CryptoDevBackend *backend, int field,
+ uint64_t value, Error **errp)
+{
+ uint64_t orig = backend->tc.buckets[field].avg;
+ bool enabled = throttle_enabled(&backend->tc);
+
+ if (orig == value) {
+ return;
+ }
+
+ backend->tc.buckets[field].avg = value;
+ if (!throttle_enabled(&backend->tc)) {
+ throttle_timers_destroy(&backend->tt);
+ cryptodev_backend_throttle_timer_cb(backend); /* drain opinfos */
+ return;
+ }
+
+ if (!throttle_is_valid(&backend->tc, errp)) {
+ backend->tc.buckets[field].avg = orig; /* revert change */
+ return;
+ }
+
+ if (!enabled) {
+ throttle_init(&backend->ts);
+ throttle_timers_init(&backend->tt, qemu_get_aio_context(),
+ QEMU_CLOCK_REALTIME,
+ cryptodev_backend_throttle_timer_cb, /* FIXME */
+ cryptodev_backend_throttle_timer_cb, backend);
+ }
+
+ throttle_config(&backend->ts, QEMU_CLOCK_REALTIME, &backend->tc);
+}
+
+static void cryptodev_backend_get_bps(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+ CryptoDevBackend *backend = CRYPTODEV_BACKEND(obj);
+ uint64_t value = backend->tc.buckets[THROTTLE_BPS_TOTAL].avg;
+
+ visit_type_uint64(v, name, &value, errp);
+}
+
+static void cryptodev_backend_set_bps(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ CryptoDevBackend *backend = CRYPTODEV_BACKEND(obj);
+ uint64_t value;
+
+ if (!visit_type_uint64(v, name, &value, errp)) {
+ return;
+ }
+
+ cryptodev_backend_set_throttle(backend, THROTTLE_BPS_TOTAL, value, errp);
+}
+
+static void cryptodev_backend_get_ops(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ CryptoDevBackend *backend = CRYPTODEV_BACKEND(obj);
+ uint64_t value = backend->tc.buckets[THROTTLE_OPS_TOTAL].avg;
+
+ visit_type_uint64(v, name, &value, errp);
+}
+
+static void cryptodev_backend_set_ops(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+ CryptoDevBackend *backend = CRYPTODEV_BACKEND(obj);
+ uint64_t value;
+
+ if (!visit_type_uint64(v, name, &value, errp)) {
+ return;
+ }
+
+ cryptodev_backend_set_throttle(backend, THROTTLE_OPS_TOTAL, value, errp);
+}
+
static void
cryptodev_backend_complete(UserCreatable *uc, Error **errp)
{
CryptoDevBackend *backend = CRYPTODEV_BACKEND(uc);
CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_GET_CLASS(uc);
uint32_t services;
+ uint64_t value;
+
+ QTAILQ_INIT(&backend->opinfos);
+ value = backend->tc.buckets[THROTTLE_OPS_TOTAL].avg;
+ cryptodev_backend_set_throttle(backend, THROTTLE_OPS_TOTAL, value, errp);
+ value = backend->tc.buckets[THROTTLE_BPS_TOTAL].avg;
+ cryptodev_backend_set_throttle(backend, THROTTLE_BPS_TOTAL, value, errp);
if (bc->init) {
bc->init(backend, errp);
@@ -309,8 +434,12 @@ cryptodev_backend_can_be_deleted(UserCreatable *uc)
static void cryptodev_backend_instance_init(Object *obj)
{
+ CryptoDevBackend *backend = CRYPTODEV_BACKEND(obj);
+
/* Initialize devices' queues property to 1 */
object_property_set_int(obj, "queues", 1, NULL);
+
+ throttle_config_init(&backend->tc);
}
static void cryptodev_backend_finalize(Object *obj)
@@ -318,6 +447,9 @@ static void cryptodev_backend_finalize(Object *obj)
CryptoDevBackend *backend = CRYPTODEV_BACKEND(obj);
cryptodev_backend_cleanup(backend, NULL);
+ if (throttle_enabled(&backend->tc)) {
+ throttle_timers_destroy(&backend->tt);
+ }
}
static void
@@ -333,6 +465,14 @@ cryptodev_backend_class_init(ObjectClass *oc, void *data)
cryptodev_backend_get_queues,
cryptodev_backend_set_queues,
NULL, NULL);
+ object_class_property_add(oc, "throttle-bps", "uint64",
+ cryptodev_backend_get_bps,
+ cryptodev_backend_set_bps,
+ NULL, NULL);
+ object_class_property_add(oc, "throttle-ops", "uint64",
+ cryptodev_backend_get_ops,
+ cryptodev_backend_set_ops,
+ NULL, NULL);
}
static const TypeInfo cryptodev_backend_info = {
diff --git a/include/sysemu/cryptodev.h b/include/sysemu/cryptodev.h
index ffbce1129c..3382b45c1a 100644
--- a/include/sysemu/cryptodev.h
+++ b/include/sysemu/cryptodev.h
@@ -24,6 +24,7 @@
#define CRYPTODEV_H
#include "qemu/queue.h"
+#include "qemu/throttle.h"
#include "qom/object.h"
#include "qapi/qapi-types-cryptodev.h"
@@ -187,6 +188,7 @@ typedef struct CryptoDevBackendOpInfo {
CryptoDevBackendSymOpInfo *sym_op_info;
CryptoDevBackendAsymOpInfo *asym_op_info;
} u;
+ QTAILQ_ENTRY(CryptoDevBackendOpInfo) next;
} CryptoDevBackendOpInfo;
struct CryptoDevBackendClass {
@@ -255,6 +257,11 @@ struct CryptoDevBackend {
CryptoDevBackendConf conf;
QCryptodevBackendSymStat *sym_stat;
QCryptodevBackendAsymStat *asym_stat;
+
+ ThrottleState ts;
+ ThrottleTimers tt;
+ ThrottleConfig tc;
+ QTAILQ_HEAD(, CryptoDevBackendOpInfo) opinfos;
};
#define QCryptodevSymStatInc(be, op, bytes) do { \
diff --git a/qapi/qom.json b/qapi/qom.json
index 30e76653ad..a877b879b9 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -278,10 +278,16 @@
# cryptodev-backend and must be 1 for cryptodev-backend-builtin.
# (default: 1)
#
+# @throttle-bps: limit total bytes per second (Since 8.0)
+#
+# @throttle-ops: limit total operations per second (Since 8.0)
+#
# Since: 2.8
##
{ 'struct': 'CryptodevBackendProperties',
- 'data': { '*queues': 'uint32' } }
+ 'data': { '*queues': 'uint32',
+ '*throttle-bps': 'uint64',
+ '*throttle-ops': 'uint64' } }
##
# @CryptodevVhostUserProperties:
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [for-8.0 v2 11/11] MAINTAINERS: add myself as the maintainer for cryptodev
2022-11-22 14:07 [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
` (9 preceding siblings ...)
2022-11-22 14:07 ` [for-8.0 v2 10/11] cryptodev: support QoS zhenwei pi
@ 2022-11-22 14:07 ` zhenwei pi
2022-12-16 3:24 ` PING: [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
2022-12-20 15:36 ` Michael S. Tsirkin
12 siblings, 0 replies; 31+ messages in thread
From: zhenwei pi @ 2022-11-22 14:07 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini
Cc: qemu-devel, zhenwei pi
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 3f698cb0e9..d1814f87ef 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2827,6 +2827,7 @@ T: git https://gitlab.com/ehabkost/qemu.git machine-next
Cryptodev Backends
M: Gonglei <arei.gonglei@huawei.com>
+M: zhenwei pi <pizhenwei@bytedance.com>
S: Maintained
F: include/sysemu/cryptodev*.h
F: backends/cryptodev*.c
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* PING: [for-8.0 v2 00/11] Refactor cryptodev
2022-11-22 14:07 [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
` (10 preceding siblings ...)
2022-11-22 14:07 ` [for-8.0 v2 11/11] MAINTAINERS: add myself as the maintainer for cryptodev zhenwei pi
@ 2022-12-16 3:24 ` zhenwei pi
2022-12-20 15:36 ` Michael S. Tsirkin
12 siblings, 0 replies; 31+ messages in thread
From: zhenwei pi @ 2022-12-16 3:24 UTC (permalink / raw)
To: arei.gonglei, mst, eblake, armbru, michael.roth, pbonzini,
dgilbert
Cc: qemu-devel
Hi, Lei
Could you please review this series?
On 11/22/22 22:07, zhenwei pi wrote:
> v1 -> v2:
> - fix coding style and use 'g_strjoin()' instead of 'char services[128]'
> (suggested by Dr. David Alan Gilbert)
> - wrapper function 'cryptodev_backend_account' to record statistics, and
> allocate sym_stat/asym_stat in cryptodev base class. see patch:
> 'cryptodev: Support statistics'.
> - add more arguments into struct CryptoDevBackendOpInfo, then
> cryptodev_backend_crypto_operation() uses *op_info only.
> - support cryptodev QoS settings(BPS&OPS), both QEMU command line and QMP
> command works fine.
> - add myself as the maintainer for cryptodev.
>
> v1:
> - introduce cryptodev.json to describe the attributes of crypto device, then
> drop duplicated type declare, remove some virtio related dependence.
> - add statistics: OPS and bandwidth.
> - add QMP command: query-cryptodev
> - add HMP info command: cryptodev
> - misc fix: detect akcipher capability instead of exposing akcipher service
> unconditionally.
>
> Zhenwei Pi (11):
> cryptodev: Introduce cryptodev.json
> cryptodev: Remove 'name' & 'model' fields
> cryptodev: Introduce cryptodev alg type in QAPI
> cryptodev: Introduce server type in QAPI
> cryptodev: Introduce 'query-cryptodev' QMP command
> cryptodev: Support statistics
> cryptodev-builtin: Detect akcipher capability
> hmp: add cryptodev info command
> cryptodev: Use CryptoDevBackendOpInfo for operation
> cryptodev: support QoS
> MAINTAINERS: add myself as the maintainer for cryptodev
>
> MAINTAINERS | 2 +
> backends/cryptodev-builtin.c | 42 +++--
> backends/cryptodev-lkcf.c | 19 +-
> backends/cryptodev-vhost-user.c | 13 +-
> backends/cryptodev-vhost.c | 4 +-
> backends/cryptodev.c | 295 +++++++++++++++++++++++++++++---
> hmp-commands-info.hx | 14 ++
> hw/virtio/virtio-crypto.c | 48 ++++--
> include/monitor/hmp.h | 1 +
> include/sysemu/cryptodev.h | 94 +++++-----
> monitor/hmp-cmds.c | 36 ++++
> qapi/cryptodev.json | 144 ++++++++++++++++
> qapi/meson.build | 1 +
> qapi/qapi-schema.json | 1 +
> qapi/qom.json | 8 +-
> 15 files changed, 604 insertions(+), 118 deletions(-)
> create mode 100644 qapi/cryptodev.json
>
--
zhenwei pi
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [for-8.0 v2 06/11] cryptodev: Support statistics
2022-11-22 14:07 ` [for-8.0 v2 06/11] cryptodev: Support statistics zhenwei pi
@ 2022-12-20 15:35 ` Michael S. Tsirkin
2023-01-16 11:22 ` Daniel P. Berrangé
1 sibling, 0 replies; 31+ messages in thread
From: Michael S. Tsirkin @ 2022-12-20 15:35 UTC (permalink / raw)
To: zhenwei pi
Cc: arei.gonglei, dgilbert, eblake, armbru, michael.roth, pbonzini,
qemu-devel
On Tue, Nov 22, 2022 at 10:07:51PM +0800, zhenwei pi wrote:
> Introduce cryptodev statistics in QAPI, and record OPS/Bandwidth for
> each crypto device.
>
> Example of this feature:
> virsh qemu-monitor-command vm '{"execute": "query-cryptodev"}' | jq
> {
> "return": [
> {
> "service": [
> "akcipher",
> "mac",
> "hash",
> "cipher"
> ],
> "asym-stat": {
> "encrypt-ops": 0,
> "verify-bytes": 0,
> "sign-ops": 0,
> "verify-ops": 0,
> "sign-bytes": 0,
> "decrypt-bytes": 0,
> "decrypt-ops": 0,
> "encrypt-bytes": 0
> },
> "sym-stat": {
> "encrypt-ops": 40,
> "decrypt-bytes": 5376,
> "decrypt-ops": 40,
> "encrypt-bytes": 5376
> },
> "id": "cryptodev1",
> "client": [
> {
> "queue": 0,
> "type": "builtin",
> "info": "cryptodev-builtin0"
> }
> ]
> },
> {
> "service": [
> "akcipher"
> ],
> "asym-stat": {
> "encrypt-ops": 54,
> "verify-bytes": 8704,
> "sign-ops": 17,
> "verify-ops": 34,
> "sign-bytes": 340,
> "decrypt-bytes": 9215,
> "decrypt-ops": 36,
> "encrypt-bytes": 13294
> },
> "id": "cryptodev0",
> "client": [
> {
> "queue": 0,
> "type": "lkcf",
> "info": "cryptodev-lkcf0"
> }
> ]
> }
> ],
> "id": "libvirt-424"
> }
>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Needs ACK from QAPI maintainers.
> ---
> backends/cryptodev.c | 81 +++++++++++++++++++++++++++++++++++---
> include/sysemu/cryptodev.h | 30 ++++++++++++++
> qapi/cryptodev.json | 58 ++++++++++++++++++++++++++-
> 3 files changed, 162 insertions(+), 7 deletions(-)
>
> diff --git a/backends/cryptodev.c b/backends/cryptodev.c
> index bf2f3234c9..d623bf3bff 100644
> --- a/backends/cryptodev.c
> +++ b/backends/cryptodev.c
> @@ -48,6 +48,18 @@ static int qmp_query_cryptodev_foreach(Object *obj, void *data)
> info->id = g_strdup(object_get_canonical_path_component(obj));
>
> backend = CRYPTODEV_BACKEND(obj);
> + if (backend->sym_stat) {
> + info->has_sym_stat = true;
> + info->sym_stat = g_memdup2(backend->sym_stat,
> + sizeof(QCryptodevBackendSymStat));
> + }
> +
> + if (backend->asym_stat) {
> + info->has_asym_stat = true;
> + info->asym_stat = g_memdup2(backend->asym_stat,
> + sizeof(QCryptodevBackendAsymStat));
> + }
> +
> services = backend->conf.crypto_services;
> for (uint32_t i = 0; i < QCRYPTODEV_BACKEND_SERVICE__MAX; i++) {
> if (services & (1 << i)) {
> @@ -111,6 +123,9 @@ void cryptodev_backend_cleanup(
> if (bc->cleanup) {
> bc->cleanup(backend, errp);
> }
> +
> + g_free(backend->sym_stat);
> + g_free(backend->asym_stat);
> }
>
> int cryptodev_backend_create_session(
> @@ -161,6 +176,52 @@ static int cryptodev_backend_operation(
> return -VIRTIO_CRYPTO_NOTSUPP;
> }
>
> +static int cryptodev_backend_account(CryptoDevBackend *backend,
> + CryptoDevBackendOpInfo *op_info)
> +{
> + enum QCryptodevBackendAlgType algtype = op_info->algtype;
> + int len;
> +
> + if (algtype == QCRYPTODEV_BACKEND_ALG_ASYM) {
> + CryptoDevBackendAsymOpInfo *asym_op_info = op_info->u.asym_op_info;
> + len = asym_op_info->src_len;
> + switch (op_info->op_code) {
> + case VIRTIO_CRYPTO_AKCIPHER_ENCRYPT:
> + QCryptodevAsymStatIncEncrypt(backend, len);
> + break;
> + case VIRTIO_CRYPTO_AKCIPHER_DECRYPT:
> + QCryptodevAsymStatIncDecrypt(backend, len);
> + break;
> + case VIRTIO_CRYPTO_AKCIPHER_SIGN:
> + QCryptodevAsymStatIncSign(backend, len);
> + break;
> + case VIRTIO_CRYPTO_AKCIPHER_VERIFY:
> + QCryptodevAsymStatIncVerify(backend, len);
> + break;
> + default:
> + return -VIRTIO_CRYPTO_NOTSUPP;
> + }
> + } else if (algtype == QCRYPTODEV_BACKEND_ALG_SYM) {
> + CryptoDevBackendSymOpInfo *sym_op_info = op_info->u.sym_op_info;
> + len = sym_op_info->src_len;
> + switch (op_info->op_code) {
> + case VIRTIO_CRYPTO_CIPHER_ENCRYPT:
> + QCryptodevSymStatIncEncrypt(backend, len);
> + break;
> + case VIRTIO_CRYPTO_CIPHER_DECRYPT:
> + QCryptodevSymStatIncDecrypt(backend, len);
> + break;
> + default:
> + return -VIRTIO_CRYPTO_NOTSUPP;
> + }
> + } else {
> + error_report("Unsupported cryptodev alg type: %" PRIu32 "", algtype);
> + return -VIRTIO_CRYPTO_NOTSUPP;
> + }
> +
> + return len;
> +}
> +
> int cryptodev_backend_crypto_operation(
> CryptoDevBackend *backend,
> void *opaque1,
> @@ -169,14 +230,12 @@ int cryptodev_backend_crypto_operation(
> {
> VirtIOCryptoReq *req = opaque1;
> CryptoDevBackendOpInfo *op_info = &req->op_info;
> - enum QCryptodevBackendAlgType algtype = req->flags;
> + int ret;
>
> - if ((algtype != QCRYPTODEV_BACKEND_ALG_SYM)
> - && (algtype != QCRYPTODEV_BACKEND_ALG_ASYM)) {
> - error_report("Unsupported cryptodev alg type: %" PRIu32 "", algtype);
> - return -VIRTIO_CRYPTO_NOTSUPP;
> + ret = cryptodev_backend_account(backend, op_info);
> + if (ret < 0) {
> + return ret;
> }
> -
> return cryptodev_backend_operation(backend, op_info, queue_index,
> cb, opaque2);
> }
> @@ -214,10 +273,20 @@ cryptodev_backend_complete(UserCreatable *uc, Error **errp)
> {
> CryptoDevBackend *backend = CRYPTODEV_BACKEND(uc);
> CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_GET_CLASS(uc);
> + uint32_t services;
>
> if (bc->init) {
> bc->init(backend, errp);
> }
> +
> + services = backend->conf.crypto_services;
> + if (services & (1 << QCRYPTODEV_BACKEND_SERVICE_CIPHER)) {
> + backend->sym_stat = g_new0(QCryptodevBackendSymStat, 1);
> + }
> +
> + if (services & (1 << QCRYPTODEV_BACKEND_SERVICE_AKCIPHER)) {
> + backend->asym_stat = g_new0(QCryptodevBackendAsymStat, 1);
> + }
> }
>
> void cryptodev_backend_set_used(CryptoDevBackend *backend, bool used)
> diff --git a/include/sysemu/cryptodev.h b/include/sysemu/cryptodev.h
> index f68a4baf13..c154c52039 100644
> --- a/include/sysemu/cryptodev.h
> +++ b/include/sysemu/cryptodev.h
> @@ -252,8 +252,38 @@ struct CryptoDevBackend {
> /* Tag the cryptodev backend is used by virtio-crypto or not */
> bool is_used;
> CryptoDevBackendConf conf;
> + QCryptodevBackendSymStat *sym_stat;
> + QCryptodevBackendAsymStat *asym_stat;
> };
>
> +#define QCryptodevSymStatInc(be, op, bytes) do { \
> + be->sym_stat->op##_bytes += (bytes); \
> + be->sym_stat->op##_ops += 1; \
> +} while (/*CONSTCOND*/0)
> +
> +#define QCryptodevSymStatIncEncrypt(be, bytes) \
> + QCryptodevSymStatInc(be, encrypt, bytes)
> +
> +#define QCryptodevSymStatIncDecrypt(be, bytes) \
> + QCryptodevSymStatInc(be, decrypt, bytes)
> +
> +#define QCryptodevAsymStatInc(be, op, bytes) do { \
> + be->asym_stat->op##_bytes += (bytes); \
> + be->asym_stat->op##_ops += 1; \
> +} while (/*CONSTCOND*/0)
> +
> +#define QCryptodevAsymStatIncEncrypt(be, bytes) \
> + QCryptodevAsymStatInc(be, encrypt, bytes)
> +
> +#define QCryptodevAsymStatIncDecrypt(be, bytes) \
> + QCryptodevAsymStatInc(be, decrypt, bytes)
> +
> +#define QCryptodevAsymStatIncSign(be, bytes) \
> + QCryptodevAsymStatInc(be, sign, bytes)
> +
> +#define QCryptodevAsymStatIncVerify(be, bytes) \
> + QCryptodevAsymStatInc(be, verify, bytes)
> +
> /**
> * cryptodev_backend_new_client:
> *
> diff --git a/qapi/cryptodev.json b/qapi/cryptodev.json
> index 4cc4f4f0ed..f01f2d017a 100644
> --- a/qapi/cryptodev.json
> +++ b/qapi/cryptodev.json
> @@ -60,6 +60,60 @@
> 'type': 'QCryptodevBackendType',
> '*info': 'str' } }
>
> +##
> +# @QCryptodevBackendSymStat:
> +#
> +# The statistics of symmetric operation.
> +#
> +# @encrypt-ops: the operations of symmetric encryption
> +#
> +# @decrypt-ops: the operations of symmetric decryption
> +#
> +# @encrypt-bytes: the bytes of symmetric encryption
> +#
> +# @decrypt-bytes: the bytes of symmetric decryption
> +#
> +# Since: 8.0
> +##
> +{ 'struct': 'QCryptodevBackendSymStat',
> + 'data': { 'encrypt-ops': 'int',
> + 'decrypt-ops': 'int',
> + 'encrypt-bytes': 'int',
> + 'decrypt-bytes': 'int' } }
> +
> +##
> +# @QCryptodevBackendAsymStat:
> +#
> +# The statistics of asymmetric operation.
> +#
> +# @encrypt-ops: the operations of asymmetric encryption
> +#
> +# @decrypt-ops: the operations of asymmetric decryption
> +#
> +# @sign-ops: the operations of asymmetric signature
> +#
> +# @verify-ops: the operations of asymmetric verification
> +#
> +# @encrypt-bytes: the bytes of asymmetric encryption
> +#
> +# @decrypt-bytes: the bytes of asymmetric decryption
> +#
> +# @sign-bytes: the bytes of asymmetric signature
> +#
> +# @verify-bytes: the bytes of asymmetric verification
> +#
> +# Since: 8.0
> +##
> +{ 'struct': 'QCryptodevBackendAsymStat',
> + 'data': { 'encrypt-ops': 'int',
> + 'decrypt-ops': 'int',
> + 'sign-ops': 'int',
> + 'verify-ops': 'int',
> + 'encrypt-bytes': 'int',
> + 'decrypt-bytes': 'int',
> + 'sign-bytes': 'int',
> + 'verify-bytes': 'int' } }
> +
> ##
> # @CryptodevInfo:
> #
> @@ -74,7 +128,9 @@
> { 'struct': 'CryptodevInfo',
> 'data': { 'id': 'str',
> 'service': ['QCryptodevBackendServiceType'],
> - 'client': ['CryptodevBackendClient'] } }
> + 'client': ['CryptodevBackendClient'],
> + '*sym-stat': 'QCryptodevBackendSymStat',
> + '*asym-stat': 'QCryptodevBackendAsymStat' } }
>
> ##
> # @query-cryptodev:
> --
> 2.20.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [for-8.0 v2 00/11] Refactor cryptodev
2022-11-22 14:07 [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
` (11 preceding siblings ...)
2022-12-16 3:24 ` PING: [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
@ 2022-12-20 15:36 ` Michael S. Tsirkin
2022-12-22 2:04 ` zhenwei pi
2023-01-16 9:53 ` zhenwei pi
12 siblings, 2 replies; 31+ messages in thread
From: Michael S. Tsirkin @ 2022-12-20 15:36 UTC (permalink / raw)
To: zhenwei pi
Cc: arei.gonglei, dgilbert, eblake, armbru, michael.roth, pbonzini,
qemu-devel
On Tue, Nov 22, 2022 at 10:07:45PM +0800, zhenwei pi wrote:
> v1 -> v2:
> - fix coding style and use 'g_strjoin()' instead of 'char services[128]'
> (suggested by Dr. David Alan Gilbert)
> - wrapper function 'cryptodev_backend_account' to record statistics, and
> allocate sym_stat/asym_stat in cryptodev base class. see patch:
> 'cryptodev: Support statistics'.
> - add more arguments into struct CryptoDevBackendOpInfo, then
> cryptodev_backend_crypto_operation() uses *op_info only.
> - support cryptodev QoS settings(BPS&OPS), both QEMU command line and QMP
> command works fine.
> - add myself as the maintainer for cryptodev.
>
> v1:
> - introduce cryptodev.json to describe the attributes of crypto device, then
> drop duplicated type declare, remove some virtio related dependence.
> - add statistics: OPS and bandwidth.
> - add QMP command: query-cryptodev
> - add HMP info command: cryptodev
> - misc fix: detect akcipher capability instead of exposing akcipher service
> unconditionally.
Can we get ACK on QAPI things please?
Thanks!
> Zhenwei Pi (11):
> cryptodev: Introduce cryptodev.json
> cryptodev: Remove 'name' & 'model' fields
> cryptodev: Introduce cryptodev alg type in QAPI
> cryptodev: Introduce server type in QAPI
> cryptodev: Introduce 'query-cryptodev' QMP command
> cryptodev: Support statistics
> cryptodev-builtin: Detect akcipher capability
> hmp: add cryptodev info command
> cryptodev: Use CryptoDevBackendOpInfo for operation
> cryptodev: support QoS
> MAINTAINERS: add myself as the maintainer for cryptodev
>
> MAINTAINERS | 2 +
> backends/cryptodev-builtin.c | 42 +++--
> backends/cryptodev-lkcf.c | 19 +-
> backends/cryptodev-vhost-user.c | 13 +-
> backends/cryptodev-vhost.c | 4 +-
> backends/cryptodev.c | 295 +++++++++++++++++++++++++++++---
> hmp-commands-info.hx | 14 ++
> hw/virtio/virtio-crypto.c | 48 ++++--
> include/monitor/hmp.h | 1 +
> include/sysemu/cryptodev.h | 94 +++++-----
> monitor/hmp-cmds.c | 36 ++++
> qapi/cryptodev.json | 144 ++++++++++++++++
> qapi/meson.build | 1 +
> qapi/qapi-schema.json | 1 +
> qapi/qom.json | 8 +-
> 15 files changed, 604 insertions(+), 118 deletions(-)
> create mode 100644 qapi/cryptodev.json
>
> --
> 2.20.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: [for-8.0 v2 00/11] Refactor cryptodev
2022-12-20 15:36 ` Michael S. Tsirkin
@ 2022-12-22 2:04 ` zhenwei pi
2023-01-03 6:14 ` PING: " zhenwei pi
2023-01-16 9:53 ` zhenwei pi
1 sibling, 1 reply; 31+ messages in thread
From: zhenwei pi @ 2022-12-22 2:04 UTC (permalink / raw)
To: armbru, michael.roth
Cc: arei.gonglei, dgilbert, Michael S. Tsirkin, eblake, pbonzini,
qemu-devel
On 12/20/22 23:36, Michael S. Tsirkin wrote:
> On Tue, Nov 22, 2022 at 10:07:45PM +0800, zhenwei pi wrote:
>> v1 -> v2:
>> - fix coding style and use 'g_strjoin()' instead of 'char services[128]'
>> (suggested by Dr. David Alan Gilbert)
>> - wrapper function 'cryptodev_backend_account' to record statistics, and
>> allocate sym_stat/asym_stat in cryptodev base class. see patch:
>> 'cryptodev: Support statistics'.
>> - add more arguments into struct CryptoDevBackendOpInfo, then
>> cryptodev_backend_crypto_operation() uses *op_info only.
>> - support cryptodev QoS settings(BPS&OPS), both QEMU command line and QMP
>> command works fine.
>> - add myself as the maintainer for cryptodev.
>>
>> v1:
>> - introduce cryptodev.json to describe the attributes of crypto device, then
>> drop duplicated type declare, remove some virtio related dependence.
>> - add statistics: OPS and bandwidth.
>> - add QMP command: query-cryptodev
>> - add HMP info command: cryptodev
>> - misc fix: detect akcipher capability instead of exposing akcipher service
>> unconditionally.
>
>
> Can we get ACK on QAPI things please?
> Thanks!
>
Hi, Markus & Michael
Could you please review the changes of QAPI part?
>> Zhenwei Pi (11):
>> cryptodev: Introduce cryptodev.json
>> cryptodev: Remove 'name' & 'model' fields
>> cryptodev: Introduce cryptodev alg type in QAPI
>> cryptodev: Introduce server type in QAPI
>> cryptodev: Introduce 'query-cryptodev' QMP command
>> cryptodev: Support statistics
>> cryptodev-builtin: Detect akcipher capability
>> hmp: add cryptodev info command
>> cryptodev: Use CryptoDevBackendOpInfo for operation
>> cryptodev: support QoS
>> MAINTAINERS: add myself as the maintainer for cryptodev
>>
>> MAINTAINERS | 2 +
>> backends/cryptodev-builtin.c | 42 +++--
>> backends/cryptodev-lkcf.c | 19 +-
>> backends/cryptodev-vhost-user.c | 13 +-
>> backends/cryptodev-vhost.c | 4 +-
>> backends/cryptodev.c | 295 +++++++++++++++++++++++++++++---
>> hmp-commands-info.hx | 14 ++
>> hw/virtio/virtio-crypto.c | 48 ++++--
>> include/monitor/hmp.h | 1 +
>> include/sysemu/cryptodev.h | 94 +++++-----
>> monitor/hmp-cmds.c | 36 ++++
>> qapi/cryptodev.json | 144 ++++++++++++++++
>> qapi/meson.build | 1 +
>> qapi/qapi-schema.json | 1 +
>> qapi/qom.json | 8 +-
>> 15 files changed, 604 insertions(+), 118 deletions(-)
>> create mode 100644 qapi/cryptodev.json
>>
>> --
>> 2.20.1
>
--
zhenwei pi
^ permalink raw reply [flat|nested] 31+ messages in thread
* PING: [for-8.0 v2 00/11] Refactor cryptodev
2022-12-22 2:04 ` zhenwei pi
@ 2023-01-03 6:14 ` zhenwei pi
0 siblings, 0 replies; 31+ messages in thread
From: zhenwei pi @ 2023-01-03 6:14 UTC (permalink / raw)
To: armbru, michael.roth
Cc: arei.gonglei, dgilbert, Michael S. Tsirkin, eblake, pbonzini,
qemu-devel
Hi, Markus & Michael
Could you please take a look at the changes of QAPI part?
On 12/22/22 10:04, zhenwei pi wrote:
>
>
> On 12/20/22 23:36, Michael S. Tsirkin wrote:
>> On Tue, Nov 22, 2022 at 10:07:45PM +0800, zhenwei pi wrote:
>>> v1 -> v2:
>>> - fix coding style and use 'g_strjoin()' instead of 'char services[128]'
>>> (suggested by Dr. David Alan Gilbert)
>>> - wrapper function 'cryptodev_backend_account' to record statistics, and
>>> allocate sym_stat/asym_stat in cryptodev base class. see patch:
>>> 'cryptodev: Support statistics'.
>>> - add more arguments into struct CryptoDevBackendOpInfo, then
>>> cryptodev_backend_crypto_operation() uses *op_info only.
>>> - support cryptodev QoS settings(BPS&OPS), both QEMU command line and
>>> QMP
>>> command works fine.
>>> - add myself as the maintainer for cryptodev.
>>>
>>> v1:
>>> - introduce cryptodev.json to describe the attributes of crypto
>>> device, then
>>> drop duplicated type declare, remove some virtio related dependence.
>>> - add statistics: OPS and bandwidth.
>>> - add QMP command: query-cryptodev
>>> - add HMP info command: cryptodev
>>> - misc fix: detect akcipher capability instead of exposing akcipher
>>> service
>>> unconditionally.
>>
>>
>> Can we get ACK on QAPI things please?
>> Thanks!
>>
>
> Hi, Markus & Michael
>
> Could you please review the changes of QAPI part?
>
>>> Zhenwei Pi (11):
>>> cryptodev: Introduce cryptodev.json
>>> cryptodev: Remove 'name' & 'model' fields
>>> cryptodev: Introduce cryptodev alg type in QAPI
>>> cryptodev: Introduce server type in QAPI
>>> cryptodev: Introduce 'query-cryptodev' QMP command
>>> cryptodev: Support statistics
>>> cryptodev-builtin: Detect akcipher capability
>>> hmp: add cryptodev info command
>>> cryptodev: Use CryptoDevBackendOpInfo for operation
>>> cryptodev: support QoS
>>> MAINTAINERS: add myself as the maintainer for cryptodev
>>>
>>> MAINTAINERS | 2 +
>>> backends/cryptodev-builtin.c | 42 +++--
>>> backends/cryptodev-lkcf.c | 19 +-
>>> backends/cryptodev-vhost-user.c | 13 +-
>>> backends/cryptodev-vhost.c | 4 +-
>>> backends/cryptodev.c | 295 +++++++++++++++++++++++++++++---
>>> hmp-commands-info.hx | 14 ++
>>> hw/virtio/virtio-crypto.c | 48 ++++--
>>> include/monitor/hmp.h | 1 +
>>> include/sysemu/cryptodev.h | 94 +++++-----
>>> monitor/hmp-cmds.c | 36 ++++
>>> qapi/cryptodev.json | 144 ++++++++++++++++
>>> qapi/meson.build | 1 +
>>> qapi/qapi-schema.json | 1 +
>>> qapi/qom.json | 8 +-
>>> 15 files changed, 604 insertions(+), 118 deletions(-)
>>> create mode 100644 qapi/cryptodev.json
>>>
>>> --
>>> 2.20.1
>>
>
--
zhenwei pi
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: [for-8.0 v2 00/11] Refactor cryptodev
2022-12-20 15:36 ` Michael S. Tsirkin
2022-12-22 2:04 ` zhenwei pi
@ 2023-01-16 9:53 ` zhenwei pi
2023-01-16 11:27 ` Daniel P. Berrangé
1 sibling, 1 reply; 31+ messages in thread
From: zhenwei pi @ 2023-01-16 9:53 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: arei.gonglei, dgilbert, eblake, armbru, michael.roth, pbonzini,
qemu-devel
On 12/20/22 23:36, Michael S. Tsirkin wrote:
> On Tue, Nov 22, 2022 at 10:07:45PM +0800, zhenwei pi wrote:
>> v1 -> v2:
>> - fix coding style and use 'g_strjoin()' instead of 'char services[128]'
>> (suggested by Dr. David Alan Gilbert)
>> - wrapper function 'cryptodev_backend_account' to record statistics, and
>> allocate sym_stat/asym_stat in cryptodev base class. see patch:
>> 'cryptodev: Support statistics'.
>> - add more arguments into struct CryptoDevBackendOpInfo, then
>> cryptodev_backend_crypto_operation() uses *op_info only.
>> - support cryptodev QoS settings(BPS&OPS), both QEMU command line and QMP
>> command works fine.
>> - add myself as the maintainer for cryptodev.
>>
>> v1:
>> - introduce cryptodev.json to describe the attributes of crypto device, then
>> drop duplicated type declare, remove some virtio related dependence.
>> - add statistics: OPS and bandwidth.
>> - add QMP command: query-cryptodev
>> - add HMP info command: cryptodev
>> - misc fix: detect akcipher capability instead of exposing akcipher service
>> unconditionally.
>
>
> Can we get ACK on QAPI things please?
> Thanks!
>
Hi Michael
I pinged QAPI maintainers, but I got no ACK.
Could you please have a glance at this?
>> Zhenwei Pi (11):
>> cryptodev: Introduce cryptodev.json
>> cryptodev: Remove 'name' & 'model' fields
>> cryptodev: Introduce cryptodev alg type in QAPI
>> cryptodev: Introduce server type in QAPI
>> cryptodev: Introduce 'query-cryptodev' QMP command
>> cryptodev: Support statistics
>> cryptodev-builtin: Detect akcipher capability
>> hmp: add cryptodev info command
>> cryptodev: Use CryptoDevBackendOpInfo for operation
>> cryptodev: support QoS
>> MAINTAINERS: add myself as the maintainer for cryptodev
>>
>> MAINTAINERS | 2 +
>> backends/cryptodev-builtin.c | 42 +++--
>> backends/cryptodev-lkcf.c | 19 +-
>> backends/cryptodev-vhost-user.c | 13 +-
>> backends/cryptodev-vhost.c | 4 +-
>> backends/cryptodev.c | 295 +++++++++++++++++++++++++++++---
>> hmp-commands-info.hx | 14 ++
>> hw/virtio/virtio-crypto.c | 48 ++++--
>> include/monitor/hmp.h | 1 +
>> include/sysemu/cryptodev.h | 94 +++++-----
>> monitor/hmp-cmds.c | 36 ++++
>> qapi/cryptodev.json | 144 ++++++++++++++++
>> qapi/meson.build | 1 +
>> qapi/qapi-schema.json | 1 +
>> qapi/qom.json | 8 +-
>> 15 files changed, 604 insertions(+), 118 deletions(-)
>> create mode 100644 qapi/cryptodev.json
>>
>> --
>> 2.20.1
>
--
zhenwei pi
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [for-8.0 v2 01/11] cryptodev: Introduce cryptodev.json
2022-11-22 14:07 ` [for-8.0 v2 01/11] cryptodev: Introduce cryptodev.json zhenwei pi
@ 2023-01-16 10:58 ` Daniel P. Berrangé
0 siblings, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2023-01-16 10:58 UTC (permalink / raw)
To: zhenwei pi
Cc: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini, qemu-devel
On Tue, Nov 22, 2022 at 10:07:46PM +0800, zhenwei pi wrote:
> Introduce QCryptodevBackendType in cryptodev.json, also apply this to
> related codes. Then we can drop 'enum CryptoDevBackendOptionsType'.
>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
> MAINTAINERS | 1 +
> backends/cryptodev-builtin.c | 2 +-
> backends/cryptodev-lkcf.c | 2 +-
> backends/cryptodev-vhost-user.c | 4 ++--
> backends/cryptodev-vhost.c | 4 ++--
> include/sysemu/cryptodev.h | 11 ++---------
> qapi/cryptodev.json | 20 ++++++++++++++++++++
> qapi/meson.build | 1 +
> qapi/qapi-schema.json | 1 +
> 9 files changed, 31 insertions(+), 15 deletions(-)
> create mode 100644 qapi/cryptodev.json
> @@ -215,16 +216,8 @@ struct CryptoDevBackendClass {
> void *opaque);
> };
>
> -typedef enum CryptoDevBackendOptionsType {
> - CRYPTODEV_BACKEND_TYPE_NONE = 0,
> - CRYPTODEV_BACKEND_TYPE_BUILTIN = 1,
> - CRYPTODEV_BACKEND_TYPE_VHOST_USER = 2,
> - CRYPTODEV_BACKEND_TYPE_LKCF = 3,
> - CRYPTODEV_BACKEND_TYPE__MAX,
> -} CryptoDevBackendOptionsType;
Old code has a 'NONE' value as the default
> +##
> +# @QCryptodevBackendType:
> +#
> +# The crypto device backend type
> +#
> +# @builtin: the QEMU builtin support
> +# @vhost-user: vhost-user
> +# @lkcf: Linux kernel cryptographic framework
> +#
> +# Since: 8.0
> +##
> +{ 'enum': 'QCryptodevBackendType',
> + 'prefix': 'QCRYPTODEV_BACKEND_TYPE',
> + 'data': ['builtin', 'vhost-user', 'lkcf']}
...but new code has no 'none'. None the less I think that is
OK, as I'm not seeing anything that needed the 'none' type.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [for-8.0 v2 02/11] cryptodev: Remove 'name' & 'model' fields
2022-11-22 14:07 ` [for-8.0 v2 02/11] cryptodev: Remove 'name' & 'model' fields zhenwei pi
@ 2023-01-16 11:05 ` Daniel P. Berrangé
0 siblings, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2023-01-16 11:05 UTC (permalink / raw)
To: zhenwei pi
Cc: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini, qemu-devel
On Tue, Nov 22, 2022 at 10:07:47PM +0800, zhenwei pi wrote:
> We have already used qapi to generate crypto device types, this allows
> to convert type to a string 'model', so the 'model' field is not
> needed.
>
> And the 'name' field is not used by any backend driver, drop it.
>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
> backends/cryptodev-builtin.c | 3 +--
> backends/cryptodev-lkcf.c | 2 +-
> backends/cryptodev-vhost-user.c | 3 +--
> backends/cryptodev.c | 11 +----------
> include/sysemu/cryptodev.h | 12 +++---------
> 5 files changed, 7 insertions(+), 24 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [for-8.0 v2 03/11] cryptodev: Introduce cryptodev alg type in QAPI
2022-11-22 14:07 ` [for-8.0 v2 03/11] cryptodev: Introduce cryptodev alg type in QAPI zhenwei pi
@ 2023-01-16 11:08 ` Daniel P. Berrangé
0 siblings, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2023-01-16 11:08 UTC (permalink / raw)
To: zhenwei pi
Cc: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini, qemu-devel
On Tue, Nov 22, 2022 at 10:07:48PM +0800, zhenwei pi wrote:
> Introduce cryptodev alg type in cryptodev.json, then apply this to
> related codes, and drop 'enum CryptoDevBackendAlgType'.
>
> There are two options:
> 1, { 'enum': 'QCryptodevBackendAlgType',
> 'prefix': 'CRYPTODEV_BACKEND_ALG',
> 'data': ['sym', 'asym']}
> Then we can keep 'CRYPTODEV_BACKEND_ALG_SYM' and avoid lots of
> changes.
> 2, changes in this patch(with prefix 'QCRYPTODEV_BACKEND_ALG').
>
> To avoid breaking the rule of QAPI, use 2 here.
>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
> backends/cryptodev-builtin.c | 6 +++---
> backends/cryptodev-lkcf.c | 4 ++--
> backends/cryptodev.c | 6 +++---
> hw/virtio/virtio-crypto.c | 14 +++++++-------
> include/sysemu/cryptodev.h | 8 +-------
> qapi/cryptodev.json | 14 ++++++++++++++
> 6 files changed, 30 insertions(+), 22 deletions(-)
> diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
> index 08895271eb..5fb7b8f43f 100644
> --- a/backends/cryptodev-builtin.c
> +++ b/backends/cryptodev-builtin.c
> @@ -537,7 +537,7 @@ static int cryptodev_builtin_operation(
> CryptoDevBackendBuiltinSession *sess;
> CryptoDevBackendSymOpInfo *sym_op_info;
> CryptoDevBackendAsymOpInfo *asym_op_info;
> - enum CryptoDevBackendAlgType algtype = op_info->algtype;
> + enum QCryptodevBackendAlgType algtype = op_info->algtype;
QAPI generates a typedef for every enum with the same name as
the enum itself. IOW, this should change to merely
QCryptodevBackendAlgType algtype = op_info->algtype;
> diff --git a/backends/cryptodev-lkcf.c b/backends/cryptodev-lkcf.c
> index de3d1867c5..919bf05b75 100644
> --- a/backends/cryptodev-lkcf.c
> +++ b/backends/cryptodev-lkcf.c
> @@ -477,7 +477,7 @@ static int cryptodev_lkcf_operation(
> CryptoDevBackendLKCF *lkcf =
> CRYPTODEV_BACKEND_LKCF(backend);
> CryptoDevBackendLKCFSession *sess;
> - enum CryptoDevBackendAlgType algtype = op_info->algtype;
> + enum QCryptodevBackendAlgType algtype = op_info->algtype;
As above
> diff --git a/backends/cryptodev.c b/backends/cryptodev.c
> index 81941af816..d3caded920 100644
> --- a/backends/cryptodev.c
> +++ b/backends/cryptodev.c
> @@ -120,10 +120,10 @@ int cryptodev_backend_crypto_operation(
> {
> VirtIOCryptoReq *req = opaque1;
> CryptoDevBackendOpInfo *op_info = &req->op_info;
> - enum CryptoDevBackendAlgType algtype = req->flags;
> + enum QCryptodevBackendAlgType algtype = req->flags;
As above.
> diff --git a/include/sysemu/cryptodev.h b/include/sysemu/cryptodev.h
> index af152d09db..f68a4baf13 100644
> --- a/include/sysemu/cryptodev.h
> +++ b/include/sysemu/cryptodev.h
> @@ -49,12 +49,6 @@ typedef struct CryptoDevBackendPeers CryptoDevBackendPeers;
> typedef struct CryptoDevBackendClient
> CryptoDevBackendClient;
>
> -enum CryptoDevBackendAlgType {
> - CRYPTODEV_BACKEND_ALG_SYM,
> - CRYPTODEV_BACKEND_ALG_ASYM,
> - CRYPTODEV_BACKEND_ALG__MAX,
> -};
> -
> /**
> * CryptoDevBackendSymSessionInfo:
> *
> @@ -181,7 +175,7 @@ typedef struct CryptoDevBackendAsymOpInfo {
> } CryptoDevBackendAsymOpInfo;
>
> typedef struct CryptoDevBackendOpInfo {
> - enum CryptoDevBackendAlgType algtype;
> + enum QCryptodevBackendAlgType algtype;
As above, drop the 'enum' qualifier
> uint32_t op_code;
> uint64_t session_id;
> union {
With the redundant 'enum' qualifiers dropped, then you may add
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [for-8.0 v2 04/11] cryptodev: Introduce server type in QAPI
2022-11-22 14:07 ` [for-8.0 v2 04/11] cryptodev: Introduce server " zhenwei pi
@ 2023-01-16 11:09 ` Daniel P. Berrangé
0 siblings, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2023-01-16 11:09 UTC (permalink / raw)
To: zhenwei pi
Cc: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini, qemu-devel
On Tue, Nov 22, 2022 at 10:07:49PM +0800, zhenwei pi wrote:
> Introduce cryptodev service type in cryptodev.json, then apply this
> to related codes. Now we can remove VIRTIO_CRYPTO_SERVICE_xxx
> dependence from QEMU cryptodev.
>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
> backends/cryptodev-builtin.c | 8 ++++----
> backends/cryptodev-lkcf.c | 2 +-
> backends/cryptodev-vhost-user.c | 6 +++---
> hw/virtio/virtio-crypto.c | 27 +++++++++++++++++++++++++--
> qapi/cryptodev.json | 11 +++++++++++
> 5 files changed, 44 insertions(+), 10 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [for-8.0 v2 05/11] cryptodev: Introduce 'query-cryptodev' QMP command
2022-11-22 14:07 ` [for-8.0 v2 05/11] cryptodev: Introduce 'query-cryptodev' QMP command zhenwei pi
@ 2023-01-16 11:18 ` Daniel P. Berrangé
2023-01-18 10:25 ` Michael S. Tsirkin
0 siblings, 1 reply; 31+ messages in thread
From: Daniel P. Berrangé @ 2023-01-16 11:18 UTC (permalink / raw)
To: zhenwei pi
Cc: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini, qemu-devel
On Tue, Nov 22, 2022 at 10:07:50PM +0800, zhenwei pi wrote:
> Now we have a QMP command to query crypto devices:
> virsh qemu-monitor-command vm '{"execute": "query-cryptodev"}' | jq
> {
> "return": [
> {
> "service": [
> "akcipher",
> "mac",
> "hash",
> "cipher"
> ],
> "id": "cryptodev1",
> "client": [
> {
> "queue": 0,
> "type": "builtin",
> "info": "cryptodev-builtin0"
> }
> ]
> },
> {
> "service": [
> "akcipher"
> ],
> "id": "cryptodev0",
> "client": [
> {
> "queue": 0,
> "type": "lkcf",
> "info": "cryptodev-lkcf0"
> }
> ]
> }
> ],
> "id": "libvirt-415"
> }
>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
> backends/cryptodev.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
> qapi/cryptodev.json | 43 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 92 insertions(+)
>
> diff --git a/backends/cryptodev.c b/backends/cryptodev.c
> index d3caded920..bf2f3234c9 100644
> --- a/backends/cryptodev.c
> +++ b/backends/cryptodev.c
> @@ -24,6 +24,7 @@
> #include "qemu/osdep.h"
> #include "sysemu/cryptodev.h"
> #include "qapi/error.h"
> +#include "qapi/qapi-commands-cryptodev.h"
> #include "qapi/visitor.h"
> #include "qemu/config-file.h"
> #include "qemu/error-report.h"
> @@ -33,6 +34,54 @@
>
> static QTAILQ_HEAD(, CryptoDevBackendClient) crypto_clients;
>
> +static int qmp_query_cryptodev_foreach(Object *obj, void *data)
> +{
> + CryptoDevBackend *backend;
> + CryptodevInfoList **infolist = data;
> + uint32_t services;
> +
> + if (!object_dynamic_cast(obj, TYPE_CRYPTODEV_BACKEND)) {
> + return 0;
> + }
> +
> + CryptodevInfo *info = g_new0(CryptodevInfo, 1);
> + info->id = g_strdup(object_get_canonical_path_component(obj));
> +
> + backend = CRYPTODEV_BACKEND(obj);
> + services = backend->conf.crypto_services;
> + for (uint32_t i = 0; i < QCRYPTODEV_BACKEND_SERVICE__MAX; i++) {
QEMU coding style doesn't declare types inside the for() control
conditions. I'd suggest 'size_t i', and put it at top of this
function.
> + if (services & (1 << i)) {
> + QAPI_LIST_PREPEND(info->service, i);
> + }
> + }
> +
> + for (uint32_t i = 0; i < backend->conf.peers.queues; i++) {
> + CryptoDevBackendClient *cc = backend->conf.peers.ccs[i];
> + CryptodevBackendClient *client = g_new0(CryptodevBackendClient, 1);
> +
> + client->queue = cc->queue_index;
> + client->type = cc->type;
> + if (cc->info_str) {
> + client->has_info = true;
> + client->info = strdup(cc->info_str);
This will need rebasing, because the 'has_XXXX' fields have gone
away for all pointer types.
> + }
> + QAPI_LIST_PREPEND(info->client, client);
> + }
> +
> + QAPI_LIST_PREPEND(*infolist, info);
> +
> + return 0;
> +}
> +
> +CryptodevInfoList *qmp_query_cryptodev(Error **errp)
> +{
> + CryptodevInfoList *list = NULL;
> + Object *objs = container_get(object_get_root(), "/objects");
> +
> + object_child_foreach(objs, qmp_query_cryptodev_foreach, &list);
> +
> + return list;
> +}
>
> CryptoDevBackendClient *cryptodev_backend_new_client(void)
> {
> diff --git a/qapi/cryptodev.json b/qapi/cryptodev.json
> index 8732a30524..4cc4f4f0ed 100644
> --- a/qapi/cryptodev.json
> +++ b/qapi/cryptodev.json
> @@ -43,3 +43,46 @@
> { 'enum': 'QCryptodevBackendType',
> 'prefix': 'QCRYPTODEV_BACKEND_TYPE',
> 'data': ['builtin', 'vhost-user', 'lkcf']}
> +
> +##
> +# @CryptodevBackendClient:
> +#
> +# Information about a queue of crypto device.
> +#
> +# @type: the type of the crypto device
> +#
> +# @info: the additional infomation of the crypto device
> +#
> +# Since: 8.0
> +##
> +{ 'struct': 'CryptodevBackendClient',
> + 'data': { 'queue': 'int',
> + 'type': 'QCryptodevBackendType',
> + '*info': 'str' } }
'queue' field is not documented
I'm not too sure about the approach of exposing 'info'.
It looks like this is either a plain static string whose
value is implicitly determined by 'type', for the 'builtin'
and 'lkcf' backend types, or it is a printf() formattted
string for the 'vhost-user' type, which references the
chardev.
Exposing printf() formatted output is often an anti-pattern
for QAPI design. For example, if it is important for users
to know the chardev assocaited with the vhost-user backend,
then 'info' should be a union that is discriminated by
'type'. The 'vhost-user' branch of the enum should then
identify the chardev 'id' directly.
> +##
> +# @CryptodevInfo:
> +#
> +# Information about a crypto device.
> +#
> +# @service: supported service types of a crypto device
> +#
> +# @client: the additional infomation of the crypto device
> +#
> +# Since: 8.0
> +##
> +{ 'struct': 'CryptodevInfo',
> + 'data': { 'id': 'str',
> + 'service': ['QCryptodevBackendServiceType'],
> + 'client': ['CryptodevBackendClient'] } }
'id' field is not documented.
> +
> +##
> +# @query-cryptodev:
> +#
> +# Returns information about current crypto devices.
> +#
> +# Returns: a list of @CryptodevInfo
> +#
> +# Since: 8.0
> +##
> +{ 'command': 'query-cryptodev', 'returns': ['CryptodevInfo']}
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [for-8.0 v2 06/11] cryptodev: Support statistics
2022-11-22 14:07 ` [for-8.0 v2 06/11] cryptodev: Support statistics zhenwei pi
2022-12-20 15:35 ` Michael S. Tsirkin
@ 2023-01-16 11:22 ` Daniel P. Berrangé
1 sibling, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2023-01-16 11:22 UTC (permalink / raw)
To: zhenwei pi
Cc: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini, qemu-devel
On Tue, Nov 22, 2022 at 10:07:51PM +0800, zhenwei pi wrote:
> Introduce cryptodev statistics in QAPI, and record OPS/Bandwidth for
> each crypto device.
>
> Example of this feature:
> virsh qemu-monitor-command vm '{"execute": "query-cryptodev"}' | jq
> {
> "return": [
> {
> "service": [
> "akcipher",
> "mac",
> "hash",
> "cipher"
> ],
> "asym-stat": {
> "encrypt-ops": 0,
> "verify-bytes": 0,
> "sign-ops": 0,
> "verify-ops": 0,
> "sign-bytes": 0,
> "decrypt-bytes": 0,
> "decrypt-ops": 0,
> "encrypt-bytes": 0
> },
> "sym-stat": {
> "encrypt-ops": 40,
> "decrypt-bytes": 5376,
> "decrypt-ops": 40,
> "encrypt-bytes": 5376
> },
> "id": "cryptodev1",
> "client": [
> {
> "queue": 0,
> "type": "builtin",
> "info": "cryptodev-builtin0"
> }
> ]
> },
> {
> "service": [
> "akcipher"
> ],
> "asym-stat": {
> "encrypt-ops": 54,
> "verify-bytes": 8704,
> "sign-ops": 17,
> "verify-ops": 34,
> "sign-bytes": 340,
> "decrypt-bytes": 9215,
> "decrypt-ops": 36,
> "encrypt-bytes": 13294
> },
> "id": "cryptodev0",
> "client": [
> {
> "queue": 0,
> "type": "lkcf",
> "info": "cryptodev-lkcf0"
> }
> ]
> }
> ],
> "id": "libvirt-424"
> }
In 7.1, Paolo introduced the 'query-stats' command which was designed
as a pretty flexible/extensible mechanism for statistic reporting for
anything in QEMU. I think there's a decent argument to be made for
wiring cryptodev upto query-stats, rather than putting stats intot he
query-cryptodev command. This gives separation between querying
configuration info (which is what query-cryptodev does prior to this
patch) and querying runtime performance info.
>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
> backends/cryptodev.c | 81 +++++++++++++++++++++++++++++++++++---
> include/sysemu/cryptodev.h | 30 ++++++++++++++
> qapi/cryptodev.json | 58 ++++++++++++++++++++++++++-
> 3 files changed, 162 insertions(+), 7 deletions(-)
>
> diff --git a/backends/cryptodev.c b/backends/cryptodev.c
> index bf2f3234c9..d623bf3bff 100644
> --- a/backends/cryptodev.c
> +++ b/backends/cryptodev.c
> @@ -48,6 +48,18 @@ static int qmp_query_cryptodev_foreach(Object *obj, void *data)
> info->id = g_strdup(object_get_canonical_path_component(obj));
>
> backend = CRYPTODEV_BACKEND(obj);
> + if (backend->sym_stat) {
> + info->has_sym_stat = true;
> + info->sym_stat = g_memdup2(backend->sym_stat,
> + sizeof(QCryptodevBackendSymStat));
> + }
> +
> + if (backend->asym_stat) {
> + info->has_asym_stat = true;
> + info->asym_stat = g_memdup2(backend->asym_stat,
> + sizeof(QCryptodevBackendAsymStat));
> + }
> +
> services = backend->conf.crypto_services;
> for (uint32_t i = 0; i < QCRYPTODEV_BACKEND_SERVICE__MAX; i++) {
> if (services & (1 << i)) {
> @@ -111,6 +123,9 @@ void cryptodev_backend_cleanup(
> if (bc->cleanup) {
> bc->cleanup(backend, errp);
> }
> +
> + g_free(backend->sym_stat);
> + g_free(backend->asym_stat);
> }
>
> int cryptodev_backend_create_session(
> @@ -161,6 +176,52 @@ static int cryptodev_backend_operation(
> return -VIRTIO_CRYPTO_NOTSUPP;
> }
>
> +static int cryptodev_backend_account(CryptoDevBackend *backend,
> + CryptoDevBackendOpInfo *op_info)
> +{
> + enum QCryptodevBackendAlgType algtype = op_info->algtype;
> + int len;
> +
> + if (algtype == QCRYPTODEV_BACKEND_ALG_ASYM) {
> + CryptoDevBackendAsymOpInfo *asym_op_info = op_info->u.asym_op_info;
> + len = asym_op_info->src_len;
> + switch (op_info->op_code) {
> + case VIRTIO_CRYPTO_AKCIPHER_ENCRYPT:
> + QCryptodevAsymStatIncEncrypt(backend, len);
> + break;
> + case VIRTIO_CRYPTO_AKCIPHER_DECRYPT:
> + QCryptodevAsymStatIncDecrypt(backend, len);
> + break;
> + case VIRTIO_CRYPTO_AKCIPHER_SIGN:
> + QCryptodevAsymStatIncSign(backend, len);
> + break;
> + case VIRTIO_CRYPTO_AKCIPHER_VERIFY:
> + QCryptodevAsymStatIncVerify(backend, len);
> + break;
> + default:
> + return -VIRTIO_CRYPTO_NOTSUPP;
> + }
> + } else if (algtype == QCRYPTODEV_BACKEND_ALG_SYM) {
> + CryptoDevBackendSymOpInfo *sym_op_info = op_info->u.sym_op_info;
> + len = sym_op_info->src_len;
> + switch (op_info->op_code) {
> + case VIRTIO_CRYPTO_CIPHER_ENCRYPT:
> + QCryptodevSymStatIncEncrypt(backend, len);
> + break;
> + case VIRTIO_CRYPTO_CIPHER_DECRYPT:
> + QCryptodevSymStatIncDecrypt(backend, len);
> + break;
> + default:
> + return -VIRTIO_CRYPTO_NOTSUPP;
> + }
> + } else {
> + error_report("Unsupported cryptodev alg type: %" PRIu32 "", algtype);
> + return -VIRTIO_CRYPTO_NOTSUPP;
> + }
> +
> + return len;
> +}
> +
> int cryptodev_backend_crypto_operation(
> CryptoDevBackend *backend,
> void *opaque1,
> @@ -169,14 +230,12 @@ int cryptodev_backend_crypto_operation(
> {
> VirtIOCryptoReq *req = opaque1;
> CryptoDevBackendOpInfo *op_info = &req->op_info;
> - enum QCryptodevBackendAlgType algtype = req->flags;
> + int ret;
>
> - if ((algtype != QCRYPTODEV_BACKEND_ALG_SYM)
> - && (algtype != QCRYPTODEV_BACKEND_ALG_ASYM)) {
> - error_report("Unsupported cryptodev alg type: %" PRIu32 "", algtype);
> - return -VIRTIO_CRYPTO_NOTSUPP;
> + ret = cryptodev_backend_account(backend, op_info);
> + if (ret < 0) {
> + return ret;
> }
> -
> return cryptodev_backend_operation(backend, op_info, queue_index,
> cb, opaque2);
> }
> @@ -214,10 +273,20 @@ cryptodev_backend_complete(UserCreatable *uc, Error **errp)
> {
> CryptoDevBackend *backend = CRYPTODEV_BACKEND(uc);
> CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_GET_CLASS(uc);
> + uint32_t services;
>
> if (bc->init) {
> bc->init(backend, errp);
> }
> +
> + services = backend->conf.crypto_services;
> + if (services & (1 << QCRYPTODEV_BACKEND_SERVICE_CIPHER)) {
> + backend->sym_stat = g_new0(QCryptodevBackendSymStat, 1);
> + }
> +
> + if (services & (1 << QCRYPTODEV_BACKEND_SERVICE_AKCIPHER)) {
> + backend->asym_stat = g_new0(QCryptodevBackendAsymStat, 1);
> + }
> }
>
> void cryptodev_backend_set_used(CryptoDevBackend *backend, bool used)
> diff --git a/include/sysemu/cryptodev.h b/include/sysemu/cryptodev.h
> index f68a4baf13..c154c52039 100644
> --- a/include/sysemu/cryptodev.h
> +++ b/include/sysemu/cryptodev.h
> @@ -252,8 +252,38 @@ struct CryptoDevBackend {
> /* Tag the cryptodev backend is used by virtio-crypto or not */
> bool is_used;
> CryptoDevBackendConf conf;
> + QCryptodevBackendSymStat *sym_stat;
> + QCryptodevBackendAsymStat *asym_stat;
> };
>
> +#define QCryptodevSymStatInc(be, op, bytes) do { \
> + be->sym_stat->op##_bytes += (bytes); \
> + be->sym_stat->op##_ops += 1; \
> +} while (/*CONSTCOND*/0)
> +
> +#define QCryptodevSymStatIncEncrypt(be, bytes) \
> + QCryptodevSymStatInc(be, encrypt, bytes)
> +
> +#define QCryptodevSymStatIncDecrypt(be, bytes) \
> + QCryptodevSymStatInc(be, decrypt, bytes)
> +
> +#define QCryptodevAsymStatInc(be, op, bytes) do { \
> + be->asym_stat->op##_bytes += (bytes); \
> + be->asym_stat->op##_ops += 1; \
> +} while (/*CONSTCOND*/0)
> +
> +#define QCryptodevAsymStatIncEncrypt(be, bytes) \
> + QCryptodevAsymStatInc(be, encrypt, bytes)
> +
> +#define QCryptodevAsymStatIncDecrypt(be, bytes) \
> + QCryptodevAsymStatInc(be, decrypt, bytes)
> +
> +#define QCryptodevAsymStatIncSign(be, bytes) \
> + QCryptodevAsymStatInc(be, sign, bytes)
> +
> +#define QCryptodevAsymStatIncVerify(be, bytes) \
> + QCryptodevAsymStatInc(be, verify, bytes)
> +
> /**
> * cryptodev_backend_new_client:
> *
> diff --git a/qapi/cryptodev.json b/qapi/cryptodev.json
> index 4cc4f4f0ed..f01f2d017a 100644
> --- a/qapi/cryptodev.json
> +++ b/qapi/cryptodev.json
> @@ -60,6 +60,60 @@
> 'type': 'QCryptodevBackendType',
> '*info': 'str' } }
>
> +##
> +# @QCryptodevBackendSymStat:
> +#
> +# The statistics of symmetric operation.
> +#
> +# @encrypt-ops: the operations of symmetric encryption
> +#
> +# @decrypt-ops: the operations of symmetric decryption
> +#
> +# @encrypt-bytes: the bytes of symmetric encryption
> +#
> +# @decrypt-bytes: the bytes of symmetric decryption
> +#
> +# Since: 8.0
> +##
> +{ 'struct': 'QCryptodevBackendSymStat',
> + 'data': { 'encrypt-ops': 'int',
> + 'decrypt-ops': 'int',
> + 'encrypt-bytes': 'int',
> + 'decrypt-bytes': 'int' } }
> +
> +##
> +# @QCryptodevBackendAsymStat:
> +#
> +# The statistics of asymmetric operation.
> +#
> +# @encrypt-ops: the operations of asymmetric encryption
> +#
> +# @decrypt-ops: the operations of asymmetric decryption
> +#
> +# @sign-ops: the operations of asymmetric signature
> +#
> +# @verify-ops: the operations of asymmetric verification
> +#
> +# @encrypt-bytes: the bytes of asymmetric encryption
> +#
> +# @decrypt-bytes: the bytes of asymmetric decryption
> +#
> +# @sign-bytes: the bytes of asymmetric signature
> +#
> +# @verify-bytes: the bytes of asymmetric verification
> +#
> +# Since: 8.0
> +##
> +{ 'struct': 'QCryptodevBackendAsymStat',
> + 'data': { 'encrypt-ops': 'int',
> + 'decrypt-ops': 'int',
> + 'sign-ops': 'int',
> + 'verify-ops': 'int',
> + 'encrypt-bytes': 'int',
> + 'decrypt-bytes': 'int',
> + 'sign-bytes': 'int',
> + 'verify-bytes': 'int' } }
> +
> ##
> # @CryptodevInfo:
> #
> @@ -74,7 +128,9 @@
> { 'struct': 'CryptodevInfo',
> 'data': { 'id': 'str',
> 'service': ['QCryptodevBackendServiceType'],
> - 'client': ['CryptodevBackendClient'] } }
> + 'client': ['CryptodevBackendClient'],
> + '*sym-stat': 'QCryptodevBackendSymStat',
> + '*asym-stat': 'QCryptodevBackendAsymStat' } }
>
> ##
> # @query-cryptodev:
> --
> 2.20.1
>
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [for-8.0 v2 07/11] cryptodev-builtin: Detect akcipher capability
2022-11-22 14:07 ` [for-8.0 v2 07/11] cryptodev-builtin: Detect akcipher capability zhenwei pi
@ 2023-01-16 11:23 ` Daniel P. Berrangé
0 siblings, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2023-01-16 11:23 UTC (permalink / raw)
To: zhenwei pi
Cc: arei.gonglei, mst, dgilbert, eblake, armbru, michael.roth,
pbonzini, qemu-devel
On Tue, Nov 22, 2022 at 10:07:52PM +0800, zhenwei pi wrote:
> Rather than exposing akcipher service/RSA algorithm to virtio crypto
> device unconditionally, detect akcipher capability from akcipher
> crypto framework. This avoids unsuccessful requests.
>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
> backends/cryptodev-builtin.c | 18 +++++++++++++++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: [for-8.0 v2 00/11] Refactor cryptodev
2023-01-16 9:53 ` zhenwei pi
@ 2023-01-16 11:27 ` Daniel P. Berrangé
2023-01-17 1:52 ` zhenwei pi
0 siblings, 1 reply; 31+ messages in thread
From: Daniel P. Berrangé @ 2023-01-16 11:27 UTC (permalink / raw)
To: zhenwei pi
Cc: Michael S. Tsirkin, arei.gonglei, dgilbert, eblake, armbru,
michael.roth, pbonzini, qemu-devel
On Mon, Jan 16, 2023 at 05:53:07PM +0800, zhenwei pi wrote:
> On 12/20/22 23:36, Michael S. Tsirkin wrote:
> > On Tue, Nov 22, 2022 at 10:07:45PM +0800, zhenwei pi wrote:
> > > v1 -> v2:
> > > - fix coding style and use 'g_strjoin()' instead of 'char services[128]'
> > > (suggested by Dr. David Alan Gilbert)
> > > - wrapper function 'cryptodev_backend_account' to record statistics, and
> > > allocate sym_stat/asym_stat in cryptodev base class. see patch:
> > > 'cryptodev: Support statistics'.
> > > - add more arguments into struct CryptoDevBackendOpInfo, then
> > > cryptodev_backend_crypto_operation() uses *op_info only.
> > > - support cryptodev QoS settings(BPS&OPS), both QEMU command line and QMP
> > > command works fine.
> > > - add myself as the maintainer for cryptodev.
> > >
> > > v1:
> > > - introduce cryptodev.json to describe the attributes of crypto device, then
> > > drop duplicated type declare, remove some virtio related dependence.
> > > - add statistics: OPS and bandwidth.
> > > - add QMP command: query-cryptodev
> > > - add HMP info command: cryptodev
> > > - misc fix: detect akcipher capability instead of exposing akcipher service
> > > unconditionally.
> >
> >
> > Can we get ACK on QAPI things please?
> > Thanks!
> >
>
> Hi Michael
>
> I pinged QAPI maintainers, but I got no ACK.
> Could you please have a glance at this?
I am /not/ a QAPI maintainer, but I've given some review of the patches
that touch QAPI.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: Re: Re: [for-8.0 v2 00/11] Refactor cryptodev
2023-01-16 11:27 ` Daniel P. Berrangé
@ 2023-01-17 1:52 ` zhenwei pi
0 siblings, 0 replies; 31+ messages in thread
From: zhenwei pi @ 2023-01-17 1:52 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Michael S. Tsirkin, arei.gonglei, dgilbert, eblake, armbru,
michael.roth, pbonzini, qemu-devel
On 1/16/23 19:27, Daniel P. Berrangé wrote:
> On Mon, Jan 16, 2023 at 05:53:07PM +0800, zhenwei pi wrote:
>> On 12/20/22 23:36, Michael S. Tsirkin wrote:
>>> On Tue, Nov 22, 2022 at 10:07:45PM +0800, zhenwei pi wrote:
>>>> v1 -> v2:
>>>> - fix coding style and use 'g_strjoin()' instead of 'char services[128]'
>>>> (suggested by Dr. David Alan Gilbert)
>>>> - wrapper function 'cryptodev_backend_account' to record statistics, and
>>>> allocate sym_stat/asym_stat in cryptodev base class. see patch:
>>>> 'cryptodev: Support statistics'.
>>>> - add more arguments into struct CryptoDevBackendOpInfo, then
>>>> cryptodev_backend_crypto_operation() uses *op_info only.
>>>> - support cryptodev QoS settings(BPS&OPS), both QEMU command line and QMP
>>>> command works fine.
>>>> - add myself as the maintainer for cryptodev.
>>>>
>>>> v1:
>>>> - introduce cryptodev.json to describe the attributes of crypto device, then
>>>> drop duplicated type declare, remove some virtio related dependence.
>>>> - add statistics: OPS and bandwidth.
>>>> - add QMP command: query-cryptodev
>>>> - add HMP info command: cryptodev
>>>> - misc fix: detect akcipher capability instead of exposing akcipher service
>>>> unconditionally.
>>>
>>>
>>> Can we get ACK on QAPI things please?
>>> Thanks!
>>>
>>
>> Hi Michael
>>
>> I pinged QAPI maintainers, but I got no ACK.
>> Could you please have a glance at this?
>
> I am /not/ a QAPI maintainer, but I've given some review of the patches
> that touch QAPI.
>
A million thanks to you!
> With regards,
> Daniel
--
zhenwei pi
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [for-8.0 v2 05/11] cryptodev: Introduce 'query-cryptodev' QMP command
2023-01-16 11:18 ` Daniel P. Berrangé
@ 2023-01-18 10:25 ` Michael S. Tsirkin
2023-01-18 10:29 ` Daniel P. Berrangé
0 siblings, 1 reply; 31+ messages in thread
From: Michael S. Tsirkin @ 2023-01-18 10:25 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: zhenwei pi, arei.gonglei, dgilbert, eblake, armbru, michael.roth,
pbonzini, qemu-devel
On Mon, Jan 16, 2023 at 11:18:19AM +0000, Daniel P. Berrangé wrote:
> > + for (uint32_t i = 0; i < QCRYPTODEV_BACKEND_SERVICE__MAX; i++) {
>
> QEMU coding style doesn't declare types inside the for() control
> conditions. I'd suggest 'size_t i', and put it at top of this
> function.
It's actually kind of vague:
Mixed declarations (interleaving statements and declarations within
blocks) are generally not allowed; declarations should be at the beginning
of blocks.
for loop starts a block, does it not?
It's in C99 but we use designated initializers widely so we already
depend on that.
--
MST
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [for-8.0 v2 05/11] cryptodev: Introduce 'query-cryptodev' QMP command
2023-01-18 10:25 ` Michael S. Tsirkin
@ 2023-01-18 10:29 ` Daniel P. Berrangé
2023-01-18 10:58 ` Thomas Huth
0 siblings, 1 reply; 31+ messages in thread
From: Daniel P. Berrangé @ 2023-01-18 10:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: zhenwei pi, arei.gonglei, dgilbert, eblake, armbru, michael.roth,
pbonzini, qemu-devel
On Wed, Jan 18, 2023 at 05:25:37AM -0500, Michael S. Tsirkin wrote:
> On Mon, Jan 16, 2023 at 11:18:19AM +0000, Daniel P. Berrangé wrote:
> > > + for (uint32_t i = 0; i < QCRYPTODEV_BACKEND_SERVICE__MAX; i++) {
> >
> > QEMU coding style doesn't declare types inside the for() control
> > conditions. I'd suggest 'size_t i', and put it at top of this
> > function.
>
> It's actually kind of vague:
>
> Mixed declarations (interleaving statements and declarations within
> blocks) are generally not allowed; declarations should be at the beginning
> of blocks.
>
> for loop starts a block, does it not?
I wasn't refering to the specific docs per-se, but rather that no
code does this at all in QEMU. It is effectively our style, even
if not documented as such
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [for-8.0 v2 05/11] cryptodev: Introduce 'query-cryptodev' QMP command
2023-01-18 10:29 ` Daniel P. Berrangé
@ 2023-01-18 10:58 ` Thomas Huth
2023-01-18 11:01 ` Daniel P. Berrangé
0 siblings, 1 reply; 31+ messages in thread
From: Thomas Huth @ 2023-01-18 10:58 UTC (permalink / raw)
To: Daniel P. Berrangé, Michael S. Tsirkin
Cc: zhenwei pi, arei.gonglei, dgilbert, eblake, armbru, michael.roth,
pbonzini, qemu-devel
On 18/01/2023 11.29, Daniel P. Berrangé wrote:
> On Wed, Jan 18, 2023 at 05:25:37AM -0500, Michael S. Tsirkin wrote:
>> On Mon, Jan 16, 2023 at 11:18:19AM +0000, Daniel P. Berrangé wrote:
>>>> + for (uint32_t i = 0; i < QCRYPTODEV_BACKEND_SERVICE__MAX; i++) {
>>>
>>> QEMU coding style doesn't declare types inside the for() control
>>> conditions. I'd suggest 'size_t i', and put it at top of this
>>> function.
>>
>> It's actually kind of vague:
>>
>> Mixed declarations (interleaving statements and declarations within
>> blocks) are generally not allowed; declarations should be at the beginning
>> of blocks.
>>
>> for loop starts a block, does it not?
>
> I wasn't refering to the specific docs per-se, but rather that no
> code does this at all in QEMU. It is effectively our style, even
> if not documented as such
$ grep -r 'for (int ' * | wc -l
381
... we're using it in many places already, and I think it should be OK since
we started using gnu99 and later as a base standard. Just my 0.02 cents.
Thomas
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [for-8.0 v2 05/11] cryptodev: Introduce 'query-cryptodev' QMP command
2023-01-18 10:58 ` Thomas Huth
@ 2023-01-18 11:01 ` Daniel P. Berrangé
0 siblings, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2023-01-18 11:01 UTC (permalink / raw)
To: Thomas Huth
Cc: Michael S. Tsirkin, zhenwei pi, arei.gonglei, dgilbert, eblake,
armbru, michael.roth, pbonzini, qemu-devel
On Wed, Jan 18, 2023 at 11:58:19AM +0100, Thomas Huth wrote:
> On 18/01/2023 11.29, Daniel P. Berrangé wrote:
> > On Wed, Jan 18, 2023 at 05:25:37AM -0500, Michael S. Tsirkin wrote:
> > > On Mon, Jan 16, 2023 at 11:18:19AM +0000, Daniel P. Berrangé wrote:
> > > > > + for (uint32_t i = 0; i < QCRYPTODEV_BACKEND_SERVICE__MAX; i++) {
> > > >
> > > > QEMU coding style doesn't declare types inside the for() control
> > > > conditions. I'd suggest 'size_t i', and put it at top of this
> > > > function.
> > >
> > > It's actually kind of vague:
> > >
> > > Mixed declarations (interleaving statements and declarations within
> > > blocks) are generally not allowed; declarations should be at the beginning
> > > of blocks.
> > >
> > > for loop starts a block, does it not?
> >
> > I wasn't refering to the specific docs per-se, but rather that no
> > code does this at all in QEMU. It is effectively our style, even
> > if not documented as such
>
> $ grep -r 'for (int ' * | wc -l
> 381
>
> ... we're using it in many places already, and I think it should be OK since
> we started using gnu99 and later as a base standard. Just my 0.02 cents.
Sigh, my bad grepping skills, i missed the space between for and (.
I withdraw my objection.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2023-01-18 11:01 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-22 14:07 [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
2022-11-22 14:07 ` [for-8.0 v2 01/11] cryptodev: Introduce cryptodev.json zhenwei pi
2023-01-16 10:58 ` Daniel P. Berrangé
2022-11-22 14:07 ` [for-8.0 v2 02/11] cryptodev: Remove 'name' & 'model' fields zhenwei pi
2023-01-16 11:05 ` Daniel P. Berrangé
2022-11-22 14:07 ` [for-8.0 v2 03/11] cryptodev: Introduce cryptodev alg type in QAPI zhenwei pi
2023-01-16 11:08 ` Daniel P. Berrangé
2022-11-22 14:07 ` [for-8.0 v2 04/11] cryptodev: Introduce server " zhenwei pi
2023-01-16 11:09 ` Daniel P. Berrangé
2022-11-22 14:07 ` [for-8.0 v2 05/11] cryptodev: Introduce 'query-cryptodev' QMP command zhenwei pi
2023-01-16 11:18 ` Daniel P. Berrangé
2023-01-18 10:25 ` Michael S. Tsirkin
2023-01-18 10:29 ` Daniel P. Berrangé
2023-01-18 10:58 ` Thomas Huth
2023-01-18 11:01 ` Daniel P. Berrangé
2022-11-22 14:07 ` [for-8.0 v2 06/11] cryptodev: Support statistics zhenwei pi
2022-12-20 15:35 ` Michael S. Tsirkin
2023-01-16 11:22 ` Daniel P. Berrangé
2022-11-22 14:07 ` [for-8.0 v2 07/11] cryptodev-builtin: Detect akcipher capability zhenwei pi
2023-01-16 11:23 ` Daniel P. Berrangé
2022-11-22 14:07 ` [for-8.0 v2 08/11] hmp: add cryptodev info command zhenwei pi
2022-11-22 14:07 ` [for-8.0 v2 09/11] cryptodev: Use CryptoDevBackendOpInfo for operation zhenwei pi
2022-11-22 14:07 ` [for-8.0 v2 10/11] cryptodev: support QoS zhenwei pi
2022-11-22 14:07 ` [for-8.0 v2 11/11] MAINTAINERS: add myself as the maintainer for cryptodev zhenwei pi
2022-12-16 3:24 ` PING: [for-8.0 v2 00/11] Refactor cryptodev zhenwei pi
2022-12-20 15:36 ` Michael S. Tsirkin
2022-12-22 2:04 ` zhenwei pi
2023-01-03 6:14 ` PING: " zhenwei pi
2023-01-16 9:53 ` zhenwei pi
2023-01-16 11:27 ` Daniel P. Berrangé
2023-01-17 1:52 ` zhenwei pi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).