* [PATCH v4 01/12] cryptodev: Introduce cryptodev.json
2023-01-29 2:57 [PATCH v4 00/12] Refactor cryptodev zhenwei pi
@ 2023-01-29 2:57 ` zhenwei pi
2023-01-29 2:57 ` [PATCH v4 02/12] cryptodev: Remove 'name' & 'model' fields zhenwei pi
` (11 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: zhenwei pi @ 2023-01-29 2:57 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, pbonzini, berrange
Cc: armbru, qemu-devel, zhenwei pi
Introduce QCryptodevBackendType in cryptodev.json, also apply this to
related codes. Then we can drop 'enum CryptoDevBackendOptionsType'.
Note that `CRYPTODEV_BACKEND_TYPE_NONE` is *NOT* used by anywhere, so
drop it(no 'none' enum in QCryptodevBackendType).
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 c581c11a64..9f6c54b145 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2860,6 +2860,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 572f87b3be..a2b5a2cb3b 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 fbdb442fdf..1c37ae7491 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.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 02/12] cryptodev: Remove 'name' & 'model' fields
2023-01-29 2:57 [PATCH v4 00/12] Refactor cryptodev zhenwei pi
2023-01-29 2:57 ` [PATCH v4 01/12] cryptodev: Introduce cryptodev.json zhenwei pi
@ 2023-01-29 2:57 ` zhenwei pi
2023-01-29 2:57 ` [PATCH v4 03/12] cryptodev: Introduce cryptodev alg type in QAPI zhenwei pi
` (10 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: zhenwei pi @ 2023-01-29 2:57 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, pbonzini, berrange
Cc: armbru, 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.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 03/12] cryptodev: Introduce cryptodev alg type in QAPI
2023-01-29 2:57 [PATCH v4 00/12] Refactor cryptodev zhenwei pi
2023-01-29 2:57 ` [PATCH v4 01/12] cryptodev: Introduce cryptodev.json zhenwei pi
2023-01-29 2:57 ` [PATCH v4 02/12] cryptodev: Remove 'name' & 'model' fields zhenwei pi
@ 2023-01-29 2:57 ` zhenwei pi
2023-01-29 2:57 ` [PATCH v4 04/12] cryptodev: Introduce server " zhenwei pi
` (9 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: zhenwei pi @ 2023-01-29 2:57 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, pbonzini, berrange
Cc: armbru, 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.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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..e70dcd5dad 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;
+ 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..53a932b58d 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;
+ 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..c2a053db0e 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;
+ 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 516425e26a..0d1be0ada9 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..16f01dd48a 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;
+ 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.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 04/12] cryptodev: Introduce server type in QAPI
2023-01-29 2:57 [PATCH v4 00/12] Refactor cryptodev zhenwei pi
` (2 preceding siblings ...)
2023-01-29 2:57 ` [PATCH v4 03/12] cryptodev: Introduce cryptodev alg type in QAPI zhenwei pi
@ 2023-01-29 2:57 ` zhenwei pi
2023-01-29 2:57 ` [PATCH v4 05/12] cryptodev: Introduce 'query-cryptodev' QMP command zhenwei pi
` (8 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: zhenwei pi @ 2023-01-29 2:57 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, pbonzini, berrange
Cc: armbru, 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.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 e70dcd5dad..c0fbb650d7 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 53a932b58d..edec99f104 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 0d1be0ada9..e4f0de4d1c 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.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 05/12] cryptodev: Introduce 'query-cryptodev' QMP command
2023-01-29 2:57 [PATCH v4 00/12] Refactor cryptodev zhenwei pi
` (3 preceding siblings ...)
2023-01-29 2:57 ` [PATCH v4 04/12] cryptodev: Introduce server " zhenwei pi
@ 2023-01-29 2:57 ` zhenwei pi
2023-01-29 2:57 ` [PATCH v4 06/12] cryptodev-builtin: Detect akcipher capability zhenwei pi
` (7 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: zhenwei pi @ 2023-01-29 2:57 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, pbonzini, berrange
Cc: armbru, 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"
}
]
},
{
"service": [
"akcipher"
],
"id": "cryptodev0",
"client": [
{
"queue": 0,
"type": "lkcf"
}
]
}
],
"id": "libvirt-417"
}
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
backends/cryptodev.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
qapi/cryptodev.json | 44 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 89 insertions(+)
diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index c2a053db0e..3a45d19823 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,50 @@
static QTAILQ_HEAD(, CryptoDevBackendClient) crypto_clients;
+static int qmp_query_cryptodev_foreach(Object *obj, void *data)
+{
+ CryptoDevBackend *backend;
+ QCryptodevInfoList **infolist = data;
+ uint32_t services, i;
+
+ if (!object_dynamic_cast(obj, TYPE_CRYPTODEV_BACKEND)) {
+ return 0;
+ }
+
+ QCryptodevInfo *info = g_new0(QCryptodevInfo, 1);
+ info->id = g_strdup(object_get_canonical_path_component(obj));
+
+ backend = CRYPTODEV_BACKEND(obj);
+ services = backend->conf.crypto_services;
+ for (i = 0; i < QCRYPTODEV_BACKEND_SERVICE__MAX; i++) {
+ if (services & (1 << i)) {
+ QAPI_LIST_PREPEND(info->service, i);
+ }
+ }
+
+ for (i = 0; i < backend->conf.peers.queues; i++) {
+ CryptoDevBackendClient *cc = backend->conf.peers.ccs[i];
+ QCryptodevBackendClient *client = g_new0(QCryptodevBackendClient, 1);
+
+ client->queue = cc->queue_index;
+ client->type = cc->type;
+ QAPI_LIST_PREPEND(info->client, client);
+ }
+
+ QAPI_LIST_PREPEND(*infolist, info);
+
+ return 0;
+}
+
+QCryptodevInfoList *qmp_query_cryptodev(Error **errp)
+{
+ QCryptodevInfoList *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..f33f96a692 100644
--- a/qapi/cryptodev.json
+++ b/qapi/cryptodev.json
@@ -43,3 +43,47 @@
{ 'enum': 'QCryptodevBackendType',
'prefix': 'QCRYPTODEV_BACKEND_TYPE',
'data': ['builtin', 'vhost-user', 'lkcf']}
+
+##
+# @QCryptodevBackendClient:
+#
+# Information about a queue of crypto device.
+#
+# @queue: the queue index of the crypto device
+#
+# @type: the type of the crypto device
+#
+# Since: 8.0
+##
+{ 'struct': 'QCryptodevBackendClient',
+ 'data': { 'queue': 'uint32',
+ 'type': 'QCryptodevBackendType' } }
+
+##
+# @QCryptodevInfo:
+#
+# Information about a crypto device.
+#
+# @id: the id of the crypto device
+#
+# @service: supported service types of a crypto device
+#
+# @client: the additional infomation of the crypto device
+#
+# Since: 8.0
+##
+{ 'struct': 'QCryptodevInfo',
+ 'data': { 'id': 'str',
+ 'service': ['QCryptodevBackendServiceType'],
+ 'client': ['QCryptodevBackendClient'] } }
+
+##
+# @query-cryptodev:
+#
+# Returns information about current crypto devices.
+#
+# Returns: a list of @QCryptodevInfo
+#
+# Since: 8.0
+##
+{ 'command': 'query-cryptodev', 'returns': ['QCryptodevInfo']}
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 06/12] cryptodev-builtin: Detect akcipher capability
2023-01-29 2:57 [PATCH v4 00/12] Refactor cryptodev zhenwei pi
` (4 preceding siblings ...)
2023-01-29 2:57 ` [PATCH v4 05/12] cryptodev: Introduce 'query-cryptodev' QMP command zhenwei pi
@ 2023-01-29 2:57 ` zhenwei pi
2023-01-29 2:57 ` [PATCH v4 07/12] hmp: add cryptodev info command zhenwei pi
` (6 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: zhenwei pi @ 2023-01-29 2:57 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, pbonzini, berrange
Cc: armbru, 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.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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 c0fbb650d7..c45b5906c5 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.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 07/12] hmp: add cryptodev info command
2023-01-29 2:57 [PATCH v4 00/12] Refactor cryptodev zhenwei pi
` (5 preceding siblings ...)
2023-01-29 2:57 ` [PATCH v4 06/12] cryptodev-builtin: Detect akcipher capability zhenwei pi
@ 2023-01-29 2:57 ` zhenwei pi
2023-01-29 2:57 ` [PATCH v4 08/12] cryptodev: Use CryptoDevBackendOpInfo for operation zhenwei pi
` (5 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: zhenwei pi @ 2023-01-29 2:57 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, pbonzini, berrange
Cc: armbru, 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 | 37 +++++++++++++++++++++++++++++++++++++
3 files changed, 52 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 1b3bdcb446..391a097ffd 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -151,5 +151,6 @@ void hmp_human_readable_text_helper(Monitor *mon,
HumanReadableText *(*qmp_handler)(Error **));
void hmp_info_stats(Monitor *mon, const QDict *qdict);
void hmp_pcie_aer_inject_error(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 1dba973092..cda52c2526 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"
@@ -2280,3 +2281,39 @@ void hmp_virtio_queue_element(Monitor *mon, const QDict *qdict)
qapi_free_VirtioQueueElement(e);
}
+
+void hmp_info_cryptodev(Monitor *mon, const QDict *qdict)
+{
+ QCryptodevInfoList *il;
+ QCryptodevBackendServiceTypeList *sl;
+ QCryptodevBackendClientList *cl;
+
+ for (il = qmp_query_cryptodev(NULL); il; il = il->next) {
+ g_autofree char *services = NULL;
+ QCryptodevInfo *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) {
+ QCryptodevBackendClient *client = cl->value;
+ monitor_printf(mon, " queue %" PRIu32 ": type=%s\n",
+ client->queue,
+ QCryptodevBackendType_str(client->type));
+ }
+ }
+
+ qapi_free_QCryptodevInfoList(il);
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 08/12] cryptodev: Use CryptoDevBackendOpInfo for operation
2023-01-29 2:57 [PATCH v4 00/12] Refactor cryptodev zhenwei pi
` (6 preceding siblings ...)
2023-01-29 2:57 ` [PATCH v4 07/12] hmp: add cryptodev info command zhenwei pi
@ 2023-01-29 2:57 ` zhenwei pi
2023-01-29 2:57 ` [PATCH v4 09/12] cryptodev: Account statistics zhenwei pi
` (4 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: zhenwei pi @ 2023-01-29 2:57 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, pbonzini, berrange
Cc: armbru, 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. It's also possible to hide
VirtIOCryptoReq into virtio-crypto.c in the next step. (In theory,
VirtIOCryptoReq is a private structure used by virtio-crypto only)
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
backends/cryptodev-builtin.c | 9 +++------
backends/cryptodev-lkcf.c | 9 +++------
backends/cryptodev.c | 18 +++++-------------
hw/virtio/virtio-crypto.c | 7 ++++---
include/sysemu/cryptodev.h | 26 ++++++++++----------------
5 files changed, 25 insertions(+), 44 deletions(-)
diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
index c45b5906c5..39d0455280 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 edec99f104..45aba1ff67 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 3a45d19823..ba7b0bc770 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -143,29 +143,22 @@ 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;
}
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;
- QCryptodevBackendAlgType algtype = req->flags;
+ QCryptodevBackendAlgType algtype = op_info->algtype;
if ((algtype != QCRYPTODEV_BACKEND_ALG_SYM)
&& (algtype != QCRYPTODEV_BACKEND_ALG_ASYM)) {
@@ -173,8 +166,7 @@ int cryptodev_backend_crypto_operation(
return -VIRTIO_CRYPTO_NOTSUPP;
}
- 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 e4f0de4d1c..802e1b9659 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 16f01dd48a..048a627035 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 {
QCryptodevBackendAlgType algtype;
uint32_t op_code;
+ uint32_t queue_index;
+ CryptoDevCompletionFunc cb;
+ void *opaque; /* argument for cb */
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 {
@@ -335,24 +336,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.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 09/12] cryptodev: Account statistics
2023-01-29 2:57 [PATCH v4 00/12] Refactor cryptodev zhenwei pi
` (7 preceding siblings ...)
2023-01-29 2:57 ` [PATCH v4 08/12] cryptodev: Use CryptoDevBackendOpInfo for operation zhenwei pi
@ 2023-01-29 2:57 ` zhenwei pi
2023-01-29 2:57 ` [PATCH v4 10/12] cryptodev: support QoS zhenwei pi
` (3 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: zhenwei pi @ 2023-01-29 2:57 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, pbonzini, berrange
Cc: armbru, qemu-devel, zhenwei pi
Account OPS/BPS for crypto device, this will be used for 'query-stats'
QEMU monitor command and QoS in the next step.
Note that a crypto device may support symmetric mode, asymmetric mode,
both symmetric and asymmetric mode. So we use two structure to
describe the statistics of a crypto device.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
backends/cryptodev.c | 68 +++++++++++++++++++++++++++++++++++---
include/sysemu/cryptodev.h | 31 +++++++++++++++++
qapi/cryptodev.json | 54 ++++++++++++++++++++++++++++++
3 files changed, 148 insertions(+), 5 deletions(-)
diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index ba7b0bc770..cc824e9665 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -107,6 +107,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(
@@ -154,16 +157,61 @@ 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,
CryptoDevBackendOpInfo *op_info)
{
- QCryptodevBackendAlgType algtype = op_info->algtype;
+ 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);
@@ -202,10 +250,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 048a627035..15e8c04dcf 100644
--- a/include/sysemu/cryptodev.h
+++ b/include/sysemu/cryptodev.h
@@ -253,8 +253,39 @@ 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 f33f96a692..54d7f9cb58 100644
--- a/qapi/cryptodev.json
+++ b/qapi/cryptodev.json
@@ -87,3 +87,57 @@
# Since: 8.0
##
{ 'command': 'query-cryptodev', 'returns': ['QCryptodevInfo']}
+
+##
+# @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' } }
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 10/12] cryptodev: support QoS
2023-01-29 2:57 [PATCH v4 00/12] Refactor cryptodev zhenwei pi
` (8 preceding siblings ...)
2023-01-29 2:57 ` [PATCH v4 09/12] cryptodev: Account statistics zhenwei pi
@ 2023-01-29 2:57 ` zhenwei pi
2023-01-29 2:57 ` [PATCH v4 11/12] cryptodev: Support query-stats QMP command zhenwei pi
` (2 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: zhenwei pi @ 2023-01-29 2:57 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, pbonzini, berrange
Cc: armbru, 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 | 138 +++++++++++++++++++++++++++++++++++++
include/sysemu/cryptodev.h | 7 ++
qapi/qom.json | 8 ++-
3 files changed, 152 insertions(+), 1 deletion(-)
diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index cc824e9665..09ffdd345f 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"
@@ -203,17 +204,53 @@ 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)) {
+ goto do_account;
+ }
+
+ if (throttle_schedule_timer(&backend->ts, &backend->tt, true) ||
+ !QTAILQ_EMPTY(&backend->opinfos)) {
+ QTAILQ_INSERT_TAIL(&backend->opinfos, op_info, next);
+ return 0;
+ }
+
+do_account:
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);
}
@@ -245,12 +282,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);
@@ -294,8 +417,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)
@@ -303,6 +430,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
@@ -318,6 +448,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 15e8c04dcf..113eec4caf 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.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v4 11/12] cryptodev: Support query-stats QMP command
2023-01-29 2:57 [PATCH v4 00/12] Refactor cryptodev zhenwei pi
` (9 preceding siblings ...)
2023-01-29 2:57 ` [PATCH v4 10/12] cryptodev: support QoS zhenwei pi
@ 2023-01-29 2:57 ` zhenwei pi
2023-02-28 12:56 ` Michael S. Tsirkin
2023-01-29 2:57 ` [PATCH v4 12/12] MAINTAINERS: add myself as the maintainer for cryptodev zhenwei pi
2023-02-06 0:23 ` PING: [PATCH v4 00/12] Refactor cryptodev zhenwei pi
12 siblings, 1 reply; 21+ messages in thread
From: zhenwei pi @ 2023-01-29 2:57 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, pbonzini, berrange
Cc: armbru, qemu-devel, zhenwei pi
Now we can use "query-stats" QMP command to query statistics of
crypto devices. (Originally this was designed to show statistics
by '{"execute": "query-cryptodev"}'. Daniel Berrangé suggested that
querying configuration info by "query-cryptodev", and querying
runtime performance info by "query-stats". This makes sense!)
Example:
~# virsh qemu-monitor-command vm '{"execute": "query-stats", \
"arguments": {"target": "cryptodev"} }' | jq
{
"return": [
{
"provider": "cryptodev",
"stats": [
{
"name": "asym-verify-bytes",
"value": 7680
},
...
{
"name": "asym-decrypt-ops",
"value": 32
},
{
"name": "asym-encrypt-ops",
"value": 48
}
],
"qom-path": "/objects/cryptodev0" # support asym only
},
{
"provider": "cryptodev",
"stats": [
{
"name": "asym-verify-bytes",
"value": 0
},
...
{
"name": "sym-decrypt-bytes",
"value": 5376
},
...
],
"qom-path": "/objects/cryptodev1" # support asym/sym
}
],
"id": "libvirt-422"
}
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
backends/cryptodev.c | 141 +++++++++++++++++++++++++++++++++++++++++++
monitor/hmp-cmds.c | 5 ++
monitor/qmp-cmds.c | 2 +
qapi/stats.json | 10 ++-
4 files changed, 156 insertions(+), 2 deletions(-)
diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index 09ffdd345f..9d52220772 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -22,9 +22,11 @@
*/
#include "qemu/osdep.h"
+#include "monitor/stats.h"
#include "sysemu/cryptodev.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-cryptodev.h"
+#include "qapi/qapi-types-stats.h"
#include "qapi/visitor.h"
#include "qemu/config-file.h"
#include "qemu/error-report.h"
@@ -32,6 +34,14 @@
#include "qom/object_interfaces.h"
#include "hw/virtio/virtio-crypto.h"
+typedef struct StatsArgs {
+ union StatsResultsType {
+ StatsResultList **stats;
+ StatsSchemaList **schema;
+ } result;
+ strList *names;
+ Error **errp;
+} StatsArgs;
static QTAILQ_HEAD(, CryptoDevBackendClient) crypto_clients;
@@ -435,6 +445,134 @@ static void cryptodev_backend_finalize(Object *obj)
}
}
+static StatsList *cryptodev_backend_stats_add(const char *name, int64_t *val,
+ StatsList *stats_list)
+{
+ Stats *stats = g_new0(Stats, 1);
+
+ stats->name = g_strdup(name);
+ stats->value = g_new0(StatsValue, 1);
+ stats->value->type = QTYPE_QNUM;
+ stats->value->u.scalar = *val;
+
+ QAPI_LIST_PREPEND(stats_list, stats);
+ return stats_list;
+}
+
+static int cryptodev_backend_stats_query(Object *obj, void *data)
+{
+ StatsArgs *stats_args = data;
+ StatsResultList **stats_results = stats_args->result.stats;
+ StatsList *stats_list = NULL;
+ StatsResult *entry;
+ CryptoDevBackend *backend;
+ QCryptodevBackendSymStat *sym_stat;
+ QCryptodevBackendAsymStat *asym_stat;
+
+ if (!object_dynamic_cast(obj, TYPE_CRYPTODEV_BACKEND)) {
+ return 0;
+ }
+
+ backend = CRYPTODEV_BACKEND(obj);
+ sym_stat = backend->sym_stat;
+ if (sym_stat) {
+ stats_list = cryptodev_backend_stats_add("sym-encrypt-ops",
+ &sym_stat->encrypt_ops, stats_list);
+ stats_list = cryptodev_backend_stats_add("sym-decrypt-ops",
+ &sym_stat->decrypt_ops, stats_list);
+ stats_list = cryptodev_backend_stats_add("sym-encrypt-bytes",
+ &sym_stat->encrypt_bytes, stats_list);
+ stats_list = cryptodev_backend_stats_add("sym-decrypt-bytes",
+ &sym_stat->decrypt_bytes, stats_list);
+ }
+
+ asym_stat = backend->asym_stat;
+ if (asym_stat) {
+ stats_list = cryptodev_backend_stats_add("asym-encrypt-ops",
+ &asym_stat->encrypt_ops, stats_list);
+ stats_list = cryptodev_backend_stats_add("asym-decrypt-ops",
+ &asym_stat->decrypt_ops, stats_list);
+ stats_list = cryptodev_backend_stats_add("asym-sign-ops",
+ &asym_stat->sign_ops, stats_list);
+ stats_list = cryptodev_backend_stats_add("asym-verify-ops",
+ &asym_stat->verify_ops, stats_list);
+ stats_list = cryptodev_backend_stats_add("asym-encrypt-bytes",
+ &asym_stat->encrypt_bytes, stats_list);
+ stats_list = cryptodev_backend_stats_add("asym-decrypt-bytes",
+ &asym_stat->decrypt_bytes, stats_list);
+ stats_list = cryptodev_backend_stats_add("asym-sign-bytes",
+ &asym_stat->sign_bytes, stats_list);
+ stats_list = cryptodev_backend_stats_add("asym-verify-bytes",
+ &asym_stat->verify_bytes, stats_list);
+ }
+
+ entry = g_new0(StatsResult, 1);
+ entry->provider = STATS_PROVIDER_CRYPTODEV;
+ entry->qom_path = g_strdup(object_get_canonical_path(obj));
+ entry->stats = stats_list;
+ QAPI_LIST_PREPEND(*stats_results, entry);
+
+ return 0;
+}
+
+static void cryptodev_backend_stats_cb(StatsResultList **result,
+ StatsTarget target,
+ strList *names, strList *targets,
+ Error **errp)
+{
+ switch (target) {
+ case STATS_TARGET_CRYPTODEV:
+ {
+ Object *objs = container_get(object_get_root(), "/objects");
+ StatsArgs stats_args;
+ stats_args.result.stats = result;
+ stats_args.names = names;
+ stats_args.errp = errp;
+
+ object_child_foreach(objs, cryptodev_backend_stats_query, &stats_args);
+ break;
+ }
+ default:
+ break;
+ }
+}
+
+static StatsSchemaValueList *cryptodev_backend_schemas_add(const char *name,
+ StatsSchemaValueList *list)
+{
+ StatsSchemaValueList *schema_entry = g_new0(StatsSchemaValueList, 1);
+
+ schema_entry->value = g_new0(StatsSchemaValue, 1);
+ schema_entry->value->type = STATS_TYPE_CUMULATIVE;
+ schema_entry->value->name = g_strdup(name);
+ schema_entry->next = list;
+
+ return schema_entry;
+}
+
+static void cryptodev_backend_schemas_cb(StatsSchemaList **result,
+ Error **errp)
+{
+ StatsSchemaValueList *stats_list = NULL;
+ const char *sym_stats[] = {"sym-encrypt-ops", "sym-decrypt-ops",
+ "sym-encrypt-bytes", "sym-decrypt-bytes"};
+ const char *asym_stats[] = {"asym-encrypt-ops", "asym-decrypt-ops",
+ "asym-sign-ops", "asym-verify-ops",
+ "asym-encrypt-bytes", "asym-decrypt-bytes",
+ "asym-sign-bytes", "asym-verify-bytes"};
+
+ for (int i = 0; i < ARRAY_SIZE(sym_stats); i++) {
+ stats_list = cryptodev_backend_schemas_add(sym_stats[i], stats_list);
+ }
+
+ for (int i = 0; i < ARRAY_SIZE(asym_stats); i++) {
+ stats_list = cryptodev_backend_schemas_add(asym_stats[i], stats_list);
+ }
+
+ add_stats_schema(result, STATS_PROVIDER_CRYPTODEV, STATS_TARGET_CRYPTODEV,
+ stats_list);
+}
+
static void
cryptodev_backend_class_init(ObjectClass *oc, void *data)
{
@@ -456,6 +594,9 @@ cryptodev_backend_class_init(ObjectClass *oc, void *data)
cryptodev_backend_get_ops,
cryptodev_backend_set_ops,
NULL, NULL);
+
+ add_stats_callbacks(STATS_PROVIDER_CRYPTODEV, cryptodev_backend_stats_cb,
+ cryptodev_backend_schemas_cb);
}
static const TypeInfo cryptodev_backend_info = {
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index cda52c2526..dda69a1098 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1883,6 +1883,8 @@ static StatsFilter *stats_filter(StatsTarget target, const char *names,
filter->u.vcpu.vcpus = vcpu_list;
break;
}
+ case STATS_TARGET_CRYPTODEV:
+ break;
default:
break;
}
@@ -1954,6 +1956,9 @@ void hmp_info_stats(Monitor *mon, const QDict *qdict)
int cpu_index = monitor_get_cpu_index(mon);
filter = stats_filter(target, names, cpu_index, provider);
break;
+ case STATS_TARGET_CRYPTODEV:
+ filter = stats_filter(target, names, -1, provider);
+ break;
default:
abort();
}
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index bf22a8c5a6..dd31936f6a 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -385,6 +385,8 @@ static bool invoke_stats_cb(StatsCallbacks *entry,
targets = filter->u.vcpu.vcpus;
}
break;
+ case STATS_TARGET_CRYPTODEV:
+ break;
default:
abort();
}
diff --git a/qapi/stats.json b/qapi/stats.json
index 57db5b1c74..f9dec18066 100644
--- a/qapi/stats.json
+++ b/qapi/stats.json
@@ -50,10 +50,14 @@
#
# Enumeration of statistics providers.
#
+# @kvm: since 7.1
+#
+# @cryptodev: since 8.0
+#
# Since: 7.1
##
{ 'enum': 'StatsProvider',
- 'data': [ 'kvm' ] }
+ 'data': [ 'kvm', 'cryptodev' ] }
##
# @StatsTarget:
@@ -65,10 +69,12 @@
#
# @vcpu: statistics that apply to a single virtual CPU.
#
+# @cryptodev: statistics that apply to a crypto device.
+#
# Since: 7.1
##
{ 'enum': 'StatsTarget',
- 'data': [ 'vm', 'vcpu' ] }
+ 'data': [ 'vm', 'vcpu', 'cryptodev' ] }
##
# @StatsRequest:
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v4 11/12] cryptodev: Support query-stats QMP command
2023-01-29 2:57 ` [PATCH v4 11/12] cryptodev: Support query-stats QMP command zhenwei pi
@ 2023-02-28 12:56 ` Michael S. Tsirkin
2023-02-28 13:17 ` Daniel P. Berrangé
0 siblings, 1 reply; 21+ messages in thread
From: Michael S. Tsirkin @ 2023-02-28 12:56 UTC (permalink / raw)
To: zhenwei pi; +Cc: arei.gonglei, dgilbert, pbonzini, berrange, armbru, qemu-devel
On Sun, Jan 29, 2023 at 10:57:46AM +0800, zhenwei pi wrote:
> Now we can use "query-stats" QMP command to query statistics of
> crypto devices. (Originally this was designed to show statistics
> by '{"execute": "query-cryptodev"}'. Daniel Berrangé suggested that
> querying configuration info by "query-cryptodev", and querying
> runtime performance info by "query-stats". This makes sense!)
>
> Example:
> ~# virsh qemu-monitor-command vm '{"execute": "query-stats", \
> "arguments": {"target": "cryptodev"} }' | jq
> {
> "return": [
> {
> "provider": "cryptodev",
> "stats": [
> {
> "name": "asym-verify-bytes",
> "value": 7680
> },
> ...
> {
> "name": "asym-decrypt-ops",
> "value": 32
> },
> {
> "name": "asym-encrypt-ops",
> "value": 48
> }
> ],
> "qom-path": "/objects/cryptodev0" # support asym only
> },
> {
> "provider": "cryptodev",
> "stats": [
> {
> "name": "asym-verify-bytes",
> "value": 0
> },
> ...
> {
> "name": "sym-decrypt-bytes",
> "value": 5376
> },
> ...
> ],
> "qom-path": "/objects/cryptodev1" # support asym/sym
> }
> ],
> "id": "libvirt-422"
> }
>
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
I assume since this has been out a long time and no
comments by maintainers it's ok from QAPI POV.
> ---
> backends/cryptodev.c | 141 +++++++++++++++++++++++++++++++++++++++++++
> monitor/hmp-cmds.c | 5 ++
> monitor/qmp-cmds.c | 2 +
> qapi/stats.json | 10 ++-
> 4 files changed, 156 insertions(+), 2 deletions(-)
>
> diff --git a/backends/cryptodev.c b/backends/cryptodev.c
> index 09ffdd345f..9d52220772 100644
> --- a/backends/cryptodev.c
> +++ b/backends/cryptodev.c
> @@ -22,9 +22,11 @@
> */
>
> #include "qemu/osdep.h"
> +#include "monitor/stats.h"
> #include "sysemu/cryptodev.h"
> #include "qapi/error.h"
> #include "qapi/qapi-commands-cryptodev.h"
> +#include "qapi/qapi-types-stats.h"
> #include "qapi/visitor.h"
> #include "qemu/config-file.h"
> #include "qemu/error-report.h"
> @@ -32,6 +34,14 @@
> #include "qom/object_interfaces.h"
> #include "hw/virtio/virtio-crypto.h"
>
> +typedef struct StatsArgs {
> + union StatsResultsType {
> + StatsResultList **stats;
> + StatsSchemaList **schema;
> + } result;
> + strList *names;
> + Error **errp;
> +} StatsArgs;
>
> static QTAILQ_HEAD(, CryptoDevBackendClient) crypto_clients;
>
> @@ -435,6 +445,134 @@ static void cryptodev_backend_finalize(Object *obj)
> }
> }
>
> +static StatsList *cryptodev_backend_stats_add(const char *name, int64_t *val,
> + StatsList *stats_list)
> +{
> + Stats *stats = g_new0(Stats, 1);
> +
> + stats->name = g_strdup(name);
> + stats->value = g_new0(StatsValue, 1);
> + stats->value->type = QTYPE_QNUM;
> + stats->value->u.scalar = *val;
> +
> + QAPI_LIST_PREPEND(stats_list, stats);
> + return stats_list;
> +}
> +
> +static int cryptodev_backend_stats_query(Object *obj, void *data)
> +{
> + StatsArgs *stats_args = data;
> + StatsResultList **stats_results = stats_args->result.stats;
> + StatsList *stats_list = NULL;
> + StatsResult *entry;
> + CryptoDevBackend *backend;
> + QCryptodevBackendSymStat *sym_stat;
> + QCryptodevBackendAsymStat *asym_stat;
> +
> + if (!object_dynamic_cast(obj, TYPE_CRYPTODEV_BACKEND)) {
> + return 0;
> + }
> +
> + backend = CRYPTODEV_BACKEND(obj);
> + sym_stat = backend->sym_stat;
> + if (sym_stat) {
> + stats_list = cryptodev_backend_stats_add("sym-encrypt-ops",
> + &sym_stat->encrypt_ops, stats_list);
> + stats_list = cryptodev_backend_stats_add("sym-decrypt-ops",
> + &sym_stat->decrypt_ops, stats_list);
> + stats_list = cryptodev_backend_stats_add("sym-encrypt-bytes",
> + &sym_stat->encrypt_bytes, stats_list);
> + stats_list = cryptodev_backend_stats_add("sym-decrypt-bytes",
> + &sym_stat->decrypt_bytes, stats_list);
> + }
> +
> + asym_stat = backend->asym_stat;
> + if (asym_stat) {
> + stats_list = cryptodev_backend_stats_add("asym-encrypt-ops",
> + &asym_stat->encrypt_ops, stats_list);
> + stats_list = cryptodev_backend_stats_add("asym-decrypt-ops",
> + &asym_stat->decrypt_ops, stats_list);
> + stats_list = cryptodev_backend_stats_add("asym-sign-ops",
> + &asym_stat->sign_ops, stats_list);
> + stats_list = cryptodev_backend_stats_add("asym-verify-ops",
> + &asym_stat->verify_ops, stats_list);
> + stats_list = cryptodev_backend_stats_add("asym-encrypt-bytes",
> + &asym_stat->encrypt_bytes, stats_list);
> + stats_list = cryptodev_backend_stats_add("asym-decrypt-bytes",
> + &asym_stat->decrypt_bytes, stats_list);
> + stats_list = cryptodev_backend_stats_add("asym-sign-bytes",
> + &asym_stat->sign_bytes, stats_list);
> + stats_list = cryptodev_backend_stats_add("asym-verify-bytes",
> + &asym_stat->verify_bytes, stats_list);
> + }
> +
> + entry = g_new0(StatsResult, 1);
> + entry->provider = STATS_PROVIDER_CRYPTODEV;
> + entry->qom_path = g_strdup(object_get_canonical_path(obj));
> + entry->stats = stats_list;
> + QAPI_LIST_PREPEND(*stats_results, entry);
> +
> + return 0;
> +}
> +
> +static void cryptodev_backend_stats_cb(StatsResultList **result,
> + StatsTarget target,
> + strList *names, strList *targets,
> + Error **errp)
> +{
> + switch (target) {
> + case STATS_TARGET_CRYPTODEV:
> + {
> + Object *objs = container_get(object_get_root(), "/objects");
> + StatsArgs stats_args;
> + stats_args.result.stats = result;
> + stats_args.names = names;
> + stats_args.errp = errp;
> +
> + object_child_foreach(objs, cryptodev_backend_stats_query, &stats_args);
> + break;
> + }
> + default:
> + break;
> + }
> +}
> +
> +static StatsSchemaValueList *cryptodev_backend_schemas_add(const char *name,
> + StatsSchemaValueList *list)
> +{
> + StatsSchemaValueList *schema_entry = g_new0(StatsSchemaValueList, 1);
> +
> + schema_entry->value = g_new0(StatsSchemaValue, 1);
> + schema_entry->value->type = STATS_TYPE_CUMULATIVE;
> + schema_entry->value->name = g_strdup(name);
> + schema_entry->next = list;
> +
> + return schema_entry;
> +}
> +
> +static void cryptodev_backend_schemas_cb(StatsSchemaList **result,
> + Error **errp)
> +{
> + StatsSchemaValueList *stats_list = NULL;
> + const char *sym_stats[] = {"sym-encrypt-ops", "sym-decrypt-ops",
> + "sym-encrypt-bytes", "sym-decrypt-bytes"};
> + const char *asym_stats[] = {"asym-encrypt-ops", "asym-decrypt-ops",
> + "asym-sign-ops", "asym-verify-ops",
> + "asym-encrypt-bytes", "asym-decrypt-bytes",
> + "asym-sign-bytes", "asym-verify-bytes"};
> +
> + for (int i = 0; i < ARRAY_SIZE(sym_stats); i++) {
> + stats_list = cryptodev_backend_schemas_add(sym_stats[i], stats_list);
> + }
> +
> + for (int i = 0; i < ARRAY_SIZE(asym_stats); i++) {
> + stats_list = cryptodev_backend_schemas_add(asym_stats[i], stats_list);
> + }
> +
> + add_stats_schema(result, STATS_PROVIDER_CRYPTODEV, STATS_TARGET_CRYPTODEV,
> + stats_list);
> +}
> +
> static void
> cryptodev_backend_class_init(ObjectClass *oc, void *data)
> {
> @@ -456,6 +594,9 @@ cryptodev_backend_class_init(ObjectClass *oc, void *data)
> cryptodev_backend_get_ops,
> cryptodev_backend_set_ops,
> NULL, NULL);
> +
> + add_stats_callbacks(STATS_PROVIDER_CRYPTODEV, cryptodev_backend_stats_cb,
> + cryptodev_backend_schemas_cb);
> }
>
> static const TypeInfo cryptodev_backend_info = {
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index cda52c2526..dda69a1098 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1883,6 +1883,8 @@ static StatsFilter *stats_filter(StatsTarget target, const char *names,
> filter->u.vcpu.vcpus = vcpu_list;
> break;
> }
> + case STATS_TARGET_CRYPTODEV:
> + break;
> default:
> break;
> }
> @@ -1954,6 +1956,9 @@ void hmp_info_stats(Monitor *mon, const QDict *qdict)
> int cpu_index = monitor_get_cpu_index(mon);
> filter = stats_filter(target, names, cpu_index, provider);
> break;
> + case STATS_TARGET_CRYPTODEV:
> + filter = stats_filter(target, names, -1, provider);
> + break;
> default:
> abort();
> }
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index bf22a8c5a6..dd31936f6a 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -385,6 +385,8 @@ static bool invoke_stats_cb(StatsCallbacks *entry,
> targets = filter->u.vcpu.vcpus;
> }
> break;
> + case STATS_TARGET_CRYPTODEV:
> + break;
> default:
> abort();
> }
> diff --git a/qapi/stats.json b/qapi/stats.json
> index 57db5b1c74..f9dec18066 100644
> --- a/qapi/stats.json
> +++ b/qapi/stats.json
> @@ -50,10 +50,14 @@
> #
> # Enumeration of statistics providers.
> #
> +# @kvm: since 7.1
> +#
> +# @cryptodev: since 8.0
> +#
> # Since: 7.1
> ##
> { 'enum': 'StatsProvider',
> - 'data': [ 'kvm' ] }
> + 'data': [ 'kvm', 'cryptodev' ] }
>
> ##
> # @StatsTarget:
> @@ -65,10 +69,12 @@
> #
> # @vcpu: statistics that apply to a single virtual CPU.
> #
> +# @cryptodev: statistics that apply to a crypto device.
> +#
> # Since: 7.1
> ##
> { 'enum': 'StatsTarget',
> - 'data': [ 'vm', 'vcpu' ] }
> + 'data': [ 'vm', 'vcpu', 'cryptodev' ] }
>
> ##
> # @StatsRequest:
> --
> 2.34.1
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v4 11/12] cryptodev: Support query-stats QMP command
2023-02-28 12:56 ` Michael S. Tsirkin
@ 2023-02-28 13:17 ` Daniel P. Berrangé
2023-02-28 14:06 ` Markus Armbruster
2023-02-28 14:13 ` Michael S. Tsirkin
0 siblings, 2 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2023-02-28 13:17 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: zhenwei pi, arei.gonglei, dgilbert, pbonzini, armbru, qemu-devel
On Tue, Feb 28, 2023 at 07:56:14AM -0500, Michael S. Tsirkin wrote:
> On Sun, Jan 29, 2023 at 10:57:46AM +0800, zhenwei pi wrote:
> > Now we can use "query-stats" QMP command to query statistics of
> > crypto devices. (Originally this was designed to show statistics
> > by '{"execute": "query-cryptodev"}'. Daniel Berrangé suggested that
> > querying configuration info by "query-cryptodev", and querying
> > runtime performance info by "query-stats". This makes sense!)
> >
> > Example:
> > ~# virsh qemu-monitor-command vm '{"execute": "query-stats", \
> > "arguments": {"target": "cryptodev"} }' | jq
> > {
> > "return": [
> > {
> > "provider": "cryptodev",
> > "stats": [
> > {
> > "name": "asym-verify-bytes",
> > "value": 7680
> > },
> > ...
> > {
> > "name": "asym-decrypt-ops",
> > "value": 32
> > },
> > {
> > "name": "asym-encrypt-ops",
> > "value": 48
> > }
> > ],
> > "qom-path": "/objects/cryptodev0" # support asym only
> > },
> > {
> > "provider": "cryptodev",
> > "stats": [
> > {
> > "name": "asym-verify-bytes",
> > "value": 0
> > },
> > ...
> > {
> > "name": "sym-decrypt-bytes",
> > "value": 5376
> > },
> > ...
> > ],
> > "qom-path": "/objects/cryptodev1" # support asym/sym
> > }
> > ],
> > "id": "libvirt-422"
> > }
> >
> > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
>
> I assume since this has been out a long time and no
> comments by maintainers it's ok from QAPI POV.
I'm not the QAPI maintainer, but I think this worked out
pretty nicely compared to the previous versions of the
series which didn't use query-stats.. just a minor point
below.
>
> > ---
> > backends/cryptodev.c | 141 +++++++++++++++++++++++++++++++++++++++++++
> > monitor/hmp-cmds.c | 5 ++
> > monitor/qmp-cmds.c | 2 +
> > qapi/stats.json | 10 ++-
> > 4 files changed, 156 insertions(+), 2 deletions(-)
> >
> > diff --git a/backends/cryptodev.c b/backends/cryptodev.c
> > index 09ffdd345f..9d52220772 100644
> > --- a/backends/cryptodev.c
> > +++ b/backends/cryptodev.c
> > @@ -22,9 +22,11 @@
> > */
> >
> > #include "qemu/osdep.h"
> > +#include "monitor/stats.h"
> > #include "sysemu/cryptodev.h"
> > #include "qapi/error.h"
> > #include "qapi/qapi-commands-cryptodev.h"
> > +#include "qapi/qapi-types-stats.h"
> > #include "qapi/visitor.h"
> > #include "qemu/config-file.h"
> > #include "qemu/error-report.h"
> > @@ -32,6 +34,14 @@
> > #include "qom/object_interfaces.h"
> > #include "hw/virtio/virtio-crypto.h"
> >
> > +typedef struct StatsArgs {
> > + union StatsResultsType {
> > + StatsResultList **stats;
> > + StatsSchemaList **schema;
> > + } result;
> > + strList *names;
> > + Error **errp;
> > +} StatsArgs;
> >
> > static QTAILQ_HEAD(, CryptoDevBackendClient) crypto_clients;
> >
> > @@ -435,6 +445,134 @@ static void cryptodev_backend_finalize(Object *obj)
> > }
> > }
> >
> > +static StatsList *cryptodev_backend_stats_add(const char *name, int64_t *val,
> > + StatsList *stats_list)
> > +{
> > + Stats *stats = g_new0(Stats, 1);
> > +
> > + stats->name = g_strdup(name);
> > + stats->value = g_new0(StatsValue, 1);
> > + stats->value->type = QTYPE_QNUM;
> > + stats->value->u.scalar = *val;
> > +
> > + QAPI_LIST_PREPEND(stats_list, stats);
> > + return stats_list;
> > +}
> > +
> > +static int cryptodev_backend_stats_query(Object *obj, void *data)
> > +{
> > + StatsArgs *stats_args = data;
> > + StatsResultList **stats_results = stats_args->result.stats;
> > + StatsList *stats_list = NULL;
> > + StatsResult *entry;
> > + CryptoDevBackend *backend;
> > + QCryptodevBackendSymStat *sym_stat;
> > + QCryptodevBackendAsymStat *asym_stat;
> > +
> > + if (!object_dynamic_cast(obj, TYPE_CRYPTODEV_BACKEND)) {
> > + return 0;
> > + }
> > +
> > + backend = CRYPTODEV_BACKEND(obj);
> > + sym_stat = backend->sym_stat;
> > + if (sym_stat) {
> > + stats_list = cryptodev_backend_stats_add("sym-encrypt-ops",
> > + &sym_stat->encrypt_ops, stats_list);
> > + stats_list = cryptodev_backend_stats_add("sym-decrypt-ops",
> > + &sym_stat->decrypt_ops, stats_list);
> > + stats_list = cryptodev_backend_stats_add("sym-encrypt-bytes",
> > + &sym_stat->encrypt_bytes, stats_list);
> > + stats_list = cryptodev_backend_stats_add("sym-decrypt-bytes",
> > + &sym_stat->decrypt_bytes, stats_list);
> > + }
> > +
> > + asym_stat = backend->asym_stat;
> > + if (asym_stat) {
> > + stats_list = cryptodev_backend_stats_add("asym-encrypt-ops",
> > + &asym_stat->encrypt_ops, stats_list);
> > + stats_list = cryptodev_backend_stats_add("asym-decrypt-ops",
> > + &asym_stat->decrypt_ops, stats_list);
> > + stats_list = cryptodev_backend_stats_add("asym-sign-ops",
> > + &asym_stat->sign_ops, stats_list);
> > + stats_list = cryptodev_backend_stats_add("asym-verify-ops",
> > + &asym_stat->verify_ops, stats_list);
> > + stats_list = cryptodev_backend_stats_add("asym-encrypt-bytes",
> > + &asym_stat->encrypt_bytes, stats_list);
> > + stats_list = cryptodev_backend_stats_add("asym-decrypt-bytes",
> > + &asym_stat->decrypt_bytes, stats_list);
> > + stats_list = cryptodev_backend_stats_add("asym-sign-bytes",
> > + &asym_stat->sign_bytes, stats_list);
> > + stats_list = cryptodev_backend_stats_add("asym-verify-bytes",
> > + &asym_stat->verify_bytes, stats_list);
Perhaps there's benefit in having constants defined for these strings,
since they become part of our API, and the same strings are repeated
again further down.....
> > + }
> > +
> > + entry = g_new0(StatsResult, 1);
> > + entry->provider = STATS_PROVIDER_CRYPTODEV;
> > + entry->qom_path = g_strdup(object_get_canonical_path(obj));
> > + entry->stats = stats_list;
> > + QAPI_LIST_PREPEND(*stats_results, entry);
> > +
> > + return 0;
> > +}
...snip...
> > +
> > +static void cryptodev_backend_schemas_cb(StatsSchemaList **result,
> > + Error **errp)
> > +{
> > + StatsSchemaValueList *stats_list = NULL;
> > + const char *sym_stats[] = {"sym-encrypt-ops", "sym-decrypt-ops",
> > + "sym-encrypt-bytes", "sym-decrypt-bytes"};
> > + const char *asym_stats[] = {"asym-encrypt-ops", "asym-decrypt-ops",
> > + "asym-sign-ops", "asym-verify-ops",
> > + "asym-encrypt-bytes", "asym-decrypt-bytes",
> > + "asym-sign-bytes", "asym-verify-bytes"};
... here's the repeated use of the constant strings from earlier
> > +
> > + for (int i = 0; i < ARRAY_SIZE(sym_stats); i++) {
> > + stats_list = cryptodev_backend_schemas_add(sym_stats[i], stats_list);
> > + }
> > +
> > + for (int i = 0; i < ARRAY_SIZE(asym_stats); i++) {
> > + stats_list = cryptodev_backend_schemas_add(asym_stats[i], stats_list);
> > + }
> > +
> > + add_stats_schema(result, STATS_PROVIDER_CRYPTODEV, STATS_TARGET_CRYPTODEV,
> > + stats_list);
> > +}
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] 21+ messages in thread* Re: [PATCH v4 11/12] cryptodev: Support query-stats QMP command
2023-02-28 13:17 ` Daniel P. Berrangé
@ 2023-02-28 14:06 ` Markus Armbruster
2023-02-28 14:13 ` Michael S. Tsirkin
1 sibling, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2023-02-28 14:06 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Michael S. Tsirkin, zhenwei pi, arei.gonglei, dgilbert, pbonzini,
qemu-devel
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Tue, Feb 28, 2023 at 07:56:14AM -0500, Michael S. Tsirkin wrote:
>> I assume since this has been out a long time and no
>> comments by maintainers it's ok from QAPI POV.
>
> I'm not the QAPI maintainer, but I think this worked out
> pretty nicely compared to the previous versions of the
> series which didn't use query-stats.. just a minor point
> below.
I'm sorry, but I can't keep up with the amount of patches touching the
QAPI schema. Happy to rely on Daniel's judgement!
[...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 11/12] cryptodev: Support query-stats QMP command
2023-02-28 13:17 ` Daniel P. Berrangé
2023-02-28 14:06 ` Markus Armbruster
@ 2023-02-28 14:13 ` Michael S. Tsirkin
2023-02-28 14:21 ` Daniel P. Berrangé
1 sibling, 1 reply; 21+ messages in thread
From: Michael S. Tsirkin @ 2023-02-28 14:13 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: zhenwei pi, arei.gonglei, dgilbert, pbonzini, armbru, qemu-devel
On Tue, Feb 28, 2023 at 01:17:52PM +0000, Daniel P. Berrangé wrote:
> On Tue, Feb 28, 2023 at 07:56:14AM -0500, Michael S. Tsirkin wrote:
> > On Sun, Jan 29, 2023 at 10:57:46AM +0800, zhenwei pi wrote:
> > > Now we can use "query-stats" QMP command to query statistics of
> > > crypto devices. (Originally this was designed to show statistics
> > > by '{"execute": "query-cryptodev"}'. Daniel Berrangé suggested that
> > > querying configuration info by "query-cryptodev", and querying
> > > runtime performance info by "query-stats". This makes sense!)
> > >
> > > Example:
> > > ~# virsh qemu-monitor-command vm '{"execute": "query-stats", \
> > > "arguments": {"target": "cryptodev"} }' | jq
> > > {
> > > "return": [
> > > {
> > > "provider": "cryptodev",
> > > "stats": [
> > > {
> > > "name": "asym-verify-bytes",
> > > "value": 7680
> > > },
> > > ...
> > > {
> > > "name": "asym-decrypt-ops",
> > > "value": 32
> > > },
> > > {
> > > "name": "asym-encrypt-ops",
> > > "value": 48
> > > }
> > > ],
> > > "qom-path": "/objects/cryptodev0" # support asym only
> > > },
> > > {
> > > "provider": "cryptodev",
> > > "stats": [
> > > {
> > > "name": "asym-verify-bytes",
> > > "value": 0
> > > },
> > > ...
> > > {
> > > "name": "sym-decrypt-bytes",
> > > "value": 5376
> > > },
> > > ...
> > > ],
> > > "qom-path": "/objects/cryptodev1" # support asym/sym
> > > }
> > > ],
> > > "id": "libvirt-422"
> > > }
> > >
> > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > > Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> >
> > I assume since this has been out a long time and no
> > comments by maintainers it's ok from QAPI POV.
>
> I'm not the QAPI maintainer, but I think this worked out
> pretty nicely compared to the previous versions of the
> series which didn't use query-stats.. just a minor point
> below.
Hmm applied already ... is this ok to fix with patch on top
or do I have to revert?
> >
> > > ---
> > > backends/cryptodev.c | 141 +++++++++++++++++++++++++++++++++++++++++++
> > > monitor/hmp-cmds.c | 5 ++
> > > monitor/qmp-cmds.c | 2 +
> > > qapi/stats.json | 10 ++-
> > > 4 files changed, 156 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/backends/cryptodev.c b/backends/cryptodev.c
> > > index 09ffdd345f..9d52220772 100644
> > > --- a/backends/cryptodev.c
> > > +++ b/backends/cryptodev.c
> > > @@ -22,9 +22,11 @@
> > > */
> > >
> > > #include "qemu/osdep.h"
> > > +#include "monitor/stats.h"
> > > #include "sysemu/cryptodev.h"
> > > #include "qapi/error.h"
> > > #include "qapi/qapi-commands-cryptodev.h"
> > > +#include "qapi/qapi-types-stats.h"
> > > #include "qapi/visitor.h"
> > > #include "qemu/config-file.h"
> > > #include "qemu/error-report.h"
> > > @@ -32,6 +34,14 @@
> > > #include "qom/object_interfaces.h"
> > > #include "hw/virtio/virtio-crypto.h"
> > >
> > > +typedef struct StatsArgs {
> > > + union StatsResultsType {
> > > + StatsResultList **stats;
> > > + StatsSchemaList **schema;
> > > + } result;
> > > + strList *names;
> > > + Error **errp;
> > > +} StatsArgs;
> > >
> > > static QTAILQ_HEAD(, CryptoDevBackendClient) crypto_clients;
> > >
> > > @@ -435,6 +445,134 @@ static void cryptodev_backend_finalize(Object *obj)
> > > }
> > > }
> > >
> > > +static StatsList *cryptodev_backend_stats_add(const char *name, int64_t *val,
> > > + StatsList *stats_list)
> > > +{
> > > + Stats *stats = g_new0(Stats, 1);
> > > +
> > > + stats->name = g_strdup(name);
> > > + stats->value = g_new0(StatsValue, 1);
> > > + stats->value->type = QTYPE_QNUM;
> > > + stats->value->u.scalar = *val;
> > > +
> > > + QAPI_LIST_PREPEND(stats_list, stats);
> > > + return stats_list;
> > > +}
> > > +
> > > +static int cryptodev_backend_stats_query(Object *obj, void *data)
> > > +{
> > > + StatsArgs *stats_args = data;
> > > + StatsResultList **stats_results = stats_args->result.stats;
> > > + StatsList *stats_list = NULL;
> > > + StatsResult *entry;
> > > + CryptoDevBackend *backend;
> > > + QCryptodevBackendSymStat *sym_stat;
> > > + QCryptodevBackendAsymStat *asym_stat;
> > > +
> > > + if (!object_dynamic_cast(obj, TYPE_CRYPTODEV_BACKEND)) {
> > > + return 0;
> > > + }
> > > +
> > > + backend = CRYPTODEV_BACKEND(obj);
> > > + sym_stat = backend->sym_stat;
> > > + if (sym_stat) {
> > > + stats_list = cryptodev_backend_stats_add("sym-encrypt-ops",
> > > + &sym_stat->encrypt_ops, stats_list);
> > > + stats_list = cryptodev_backend_stats_add("sym-decrypt-ops",
> > > + &sym_stat->decrypt_ops, stats_list);
> > > + stats_list = cryptodev_backend_stats_add("sym-encrypt-bytes",
> > > + &sym_stat->encrypt_bytes, stats_list);
> > > + stats_list = cryptodev_backend_stats_add("sym-decrypt-bytes",
> > > + &sym_stat->decrypt_bytes, stats_list);
> > > + }
> > > +
> > > + asym_stat = backend->asym_stat;
> > > + if (asym_stat) {
> > > + stats_list = cryptodev_backend_stats_add("asym-encrypt-ops",
> > > + &asym_stat->encrypt_ops, stats_list);
> > > + stats_list = cryptodev_backend_stats_add("asym-decrypt-ops",
> > > + &asym_stat->decrypt_ops, stats_list);
> > > + stats_list = cryptodev_backend_stats_add("asym-sign-ops",
> > > + &asym_stat->sign_ops, stats_list);
> > > + stats_list = cryptodev_backend_stats_add("asym-verify-ops",
> > > + &asym_stat->verify_ops, stats_list);
> > > + stats_list = cryptodev_backend_stats_add("asym-encrypt-bytes",
> > > + &asym_stat->encrypt_bytes, stats_list);
> > > + stats_list = cryptodev_backend_stats_add("asym-decrypt-bytes",
> > > + &asym_stat->decrypt_bytes, stats_list);
> > > + stats_list = cryptodev_backend_stats_add("asym-sign-bytes",
> > > + &asym_stat->sign_bytes, stats_list);
> > > + stats_list = cryptodev_backend_stats_add("asym-verify-bytes",
> > > + &asym_stat->verify_bytes, stats_list);
>
> Perhaps there's benefit in having constants defined for these strings,
> since they become part of our API, and the same strings are repeated
> again further down.....
>
> > > + }
> > > +
> > > + entry = g_new0(StatsResult, 1);
> > > + entry->provider = STATS_PROVIDER_CRYPTODEV;
> > > + entry->qom_path = g_strdup(object_get_canonical_path(obj));
> > > + entry->stats = stats_list;
> > > + QAPI_LIST_PREPEND(*stats_results, entry);
> > > +
> > > + return 0;
> > > +}
>
> ...snip...
>
> > > +
> > > +static void cryptodev_backend_schemas_cb(StatsSchemaList **result,
> > > + Error **errp)
> > > +{
> > > + StatsSchemaValueList *stats_list = NULL;
> > > + const char *sym_stats[] = {"sym-encrypt-ops", "sym-decrypt-ops",
> > > + "sym-encrypt-bytes", "sym-decrypt-bytes"};
> > > + const char *asym_stats[] = {"asym-encrypt-ops", "asym-decrypt-ops",
> > > + "asym-sign-ops", "asym-verify-ops",
> > > + "asym-encrypt-bytes", "asym-decrypt-bytes",
> > > + "asym-sign-bytes", "asym-verify-bytes"};
>
> ... here's the repeated use of the constant strings from earlier
>
> > > +
> > > + for (int i = 0; i < ARRAY_SIZE(sym_stats); i++) {
> > > + stats_list = cryptodev_backend_schemas_add(sym_stats[i], stats_list);
> > > + }
> > > +
> > > + for (int i = 0; i < ARRAY_SIZE(asym_stats); i++) {
> > > + stats_list = cryptodev_backend_schemas_add(asym_stats[i], stats_list);
> > > + }
> > > +
> > > + add_stats_schema(result, STATS_PROVIDER_CRYPTODEV, STATS_TARGET_CRYPTODEV,
> > > + stats_list);
> > > +}
>
> 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] 21+ messages in thread* Re: [PATCH v4 11/12] cryptodev: Support query-stats QMP command
2023-02-28 14:13 ` Michael S. Tsirkin
@ 2023-02-28 14:21 ` Daniel P. Berrangé
2023-02-28 14:58 ` Michael S. Tsirkin
0 siblings, 1 reply; 21+ messages in thread
From: Daniel P. Berrangé @ 2023-02-28 14:21 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: zhenwei pi, arei.gonglei, dgilbert, pbonzini, armbru, qemu-devel
On Tue, Feb 28, 2023 at 09:13:30AM -0500, Michael S. Tsirkin wrote:
> On Tue, Feb 28, 2023 at 01:17:52PM +0000, Daniel P. Berrangé wrote:
> > On Tue, Feb 28, 2023 at 07:56:14AM -0500, Michael S. Tsirkin wrote:
> > > On Sun, Jan 29, 2023 at 10:57:46AM +0800, zhenwei pi wrote:
> > > > Now we can use "query-stats" QMP command to query statistics of
> > > > crypto devices. (Originally this was designed to show statistics
> > > > by '{"execute": "query-cryptodev"}'. Daniel Berrangé suggested that
> > > > querying configuration info by "query-cryptodev", and querying
> > > > runtime performance info by "query-stats". This makes sense!)
> > > >
> > > > Example:
> > > > ~# virsh qemu-monitor-command vm '{"execute": "query-stats", \
> > > > "arguments": {"target": "cryptodev"} }' | jq
> > > > {
> > > > "return": [
> > > > {
> > > > "provider": "cryptodev",
> > > > "stats": [
> > > > {
> > > > "name": "asym-verify-bytes",
> > > > "value": 7680
> > > > },
> > > > ...
> > > > {
> > > > "name": "asym-decrypt-ops",
> > > > "value": 32
> > > > },
> > > > {
> > > > "name": "asym-encrypt-ops",
> > > > "value": 48
> > > > }
> > > > ],
> > > > "qom-path": "/objects/cryptodev0" # support asym only
> > > > },
> > > > {
> > > > "provider": "cryptodev",
> > > > "stats": [
> > > > {
> > > > "name": "asym-verify-bytes",
> > > > "value": 0
> > > > },
> > > > ...
> > > > {
> > > > "name": "sym-decrypt-bytes",
> > > > "value": 5376
> > > > },
> > > > ...
> > > > ],
> > > > "qom-path": "/objects/cryptodev1" # support asym/sym
> > > > }
> > > > ],
> > > > "id": "libvirt-422"
> > > > }
> > > >
> > > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > > > Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> > >
> > > I assume since this has been out a long time and no
> > > comments by maintainers it's ok from QAPI POV.
> >
> > I'm not the QAPI maintainer, but I think this worked out
> > pretty nicely compared to the previous versions of the
> > series which didn't use query-stats.. just a minor point
> > below.
>
> Hmm applied already ... is this ok to fix with patch on top
> or do I have to revert?
Not a big deal. It'd be fine as a followup improvement if
zhenwei wants to submit a later cleanup.
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] 21+ messages in thread* Re: [PATCH v4 11/12] cryptodev: Support query-stats QMP command
2023-02-28 14:21 ` Daniel P. Berrangé
@ 2023-02-28 14:58 ` Michael S. Tsirkin
2023-03-01 2:53 ` zhenwei pi
0 siblings, 1 reply; 21+ messages in thread
From: Michael S. Tsirkin @ 2023-02-28 14:58 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: zhenwei pi, arei.gonglei, dgilbert, pbonzini, armbru, qemu-devel
On Tue, Feb 28, 2023 at 02:21:17PM +0000, Daniel P. Berrangé wrote:
> On Tue, Feb 28, 2023 at 09:13:30AM -0500, Michael S. Tsirkin wrote:
> > On Tue, Feb 28, 2023 at 01:17:52PM +0000, Daniel P. Berrangé wrote:
> > > On Tue, Feb 28, 2023 at 07:56:14AM -0500, Michael S. Tsirkin wrote:
> > > > On Sun, Jan 29, 2023 at 10:57:46AM +0800, zhenwei pi wrote:
> > > > > Now we can use "query-stats" QMP command to query statistics of
> > > > > crypto devices. (Originally this was designed to show statistics
> > > > > by '{"execute": "query-cryptodev"}'. Daniel Berrangé suggested that
> > > > > querying configuration info by "query-cryptodev", and querying
> > > > > runtime performance info by "query-stats". This makes sense!)
> > > > >
> > > > > Example:
> > > > > ~# virsh qemu-monitor-command vm '{"execute": "query-stats", \
> > > > > "arguments": {"target": "cryptodev"} }' | jq
> > > > > {
> > > > > "return": [
> > > > > {
> > > > > "provider": "cryptodev",
> > > > > "stats": [
> > > > > {
> > > > > "name": "asym-verify-bytes",
> > > > > "value": 7680
> > > > > },
> > > > > ...
> > > > > {
> > > > > "name": "asym-decrypt-ops",
> > > > > "value": 32
> > > > > },
> > > > > {
> > > > > "name": "asym-encrypt-ops",
> > > > > "value": 48
> > > > > }
> > > > > ],
> > > > > "qom-path": "/objects/cryptodev0" # support asym only
> > > > > },
> > > > > {
> > > > > "provider": "cryptodev",
> > > > > "stats": [
> > > > > {
> > > > > "name": "asym-verify-bytes",
> > > > > "value": 0
> > > > > },
> > > > > ...
> > > > > {
> > > > > "name": "sym-decrypt-bytes",
> > > > > "value": 5376
> > > > > },
> > > > > ...
> > > > > ],
> > > > > "qom-path": "/objects/cryptodev1" # support asym/sym
> > > > > }
> > > > > ],
> > > > > "id": "libvirt-422"
> > > > > }
> > > > >
> > > > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > > > > Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> > > >
> > > > I assume since this has been out a long time and no
> > > > comments by maintainers it's ok from QAPI POV.
> > >
> > > I'm not the QAPI maintainer, but I think this worked out
> > > pretty nicely compared to the previous versions of the
> > > series which didn't use query-stats.. just a minor point
> > > below.
> >
> > Hmm applied already ... is this ok to fix with patch on top
> > or do I have to revert?
>
> Not a big deal. It'd be fine as a followup improvement if
> zhenwei wants to submit a later cleanup.
>
> With regards,
> Daniel
zhenwei can you confirm pls?
> --
> |: 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] 21+ messages in thread* Re: Re: [PATCH v4 11/12] cryptodev: Support query-stats QMP command
2023-02-28 14:58 ` Michael S. Tsirkin
@ 2023-03-01 2:53 ` zhenwei pi
0 siblings, 0 replies; 21+ messages in thread
From: zhenwei pi @ 2023-03-01 2:53 UTC (permalink / raw)
To: Michael S. Tsirkin, Daniel P. Berrangé
Cc: arei.gonglei, dgilbert, pbonzini, armbru, qemu-devel
On 2/28/23 22:58, Michael S. Tsirkin wrote:
> On Tue, Feb 28, 2023 at 02:21:17PM +0000, Daniel P. Berrangé wrote:
>> On Tue, Feb 28, 2023 at 09:13:30AM -0500, Michael S. Tsirkin wrote:
>>> On Tue, Feb 28, 2023 at 01:17:52PM +0000, Daniel P. Berrangé wrote:
>>>> On Tue, Feb 28, 2023 at 07:56:14AM -0500, Michael S. Tsirkin wrote:
>>>>> On Sun, Jan 29, 2023 at 10:57:46AM +0800, zhenwei pi wrote:
>>>>>> Now we can use "query-stats" QMP command to query statistics of
>>>>>> crypto devices. (Originally this was designed to show statistics
>>>>>> by '{"execute": "query-cryptodev"}'. Daniel Berrangé suggested that
>>>>>> querying configuration info by "query-cryptodev", and querying
>>>>>> runtime performance info by "query-stats". This makes sense!)
>>>>>>
>>>>>> Example:
>>>>>> ~# virsh qemu-monitor-command vm '{"execute": "query-stats", \
>>>>>> "arguments": {"target": "cryptodev"} }' | jq
>>>>>> {
>>>>>> "return": [
>>>>>> {
>>>>>> "provider": "cryptodev",
>>>>>> "stats": [
>>>>>> {
>>>>>> "name": "asym-verify-bytes",
>>>>>> "value": 7680
>>>>>> },
>>>>>> ...
>>>>>> {
>>>>>> "name": "asym-decrypt-ops",
>>>>>> "value": 32
>>>>>> },
>>>>>> {
>>>>>> "name": "asym-encrypt-ops",
>>>>>> "value": 48
>>>>>> }
>>>>>> ],
>>>>>> "qom-path": "/objects/cryptodev0" # support asym only
>>>>>> },
>>>>>> {
>>>>>> "provider": "cryptodev",
>>>>>> "stats": [
>>>>>> {
>>>>>> "name": "asym-verify-bytes",
>>>>>> "value": 0
>>>>>> },
>>>>>> ...
>>>>>> {
>>>>>> "name": "sym-decrypt-bytes",
>>>>>> "value": 5376
>>>>>> },
>>>>>> ...
>>>>>> ],
>>>>>> "qom-path": "/objects/cryptodev1" # support asym/sym
>>>>>> }
>>>>>> ],
>>>>>> "id": "libvirt-422"
>>>>>> }
>>>>>>
>>>>>> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
>>>>>> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
>>>>>
>>>>> I assume since this has been out a long time and no
>>>>> comments by maintainers it's ok from QAPI POV.
>>>>
>>>> I'm not the QAPI maintainer, but I think this worked out
>>>> pretty nicely compared to the previous versions of the
>>>> series which didn't use query-stats.. just a minor point
>>>> below.
>>>
>>> Hmm applied already ... is this ok to fix with patch on top
>>> or do I have to revert?
>>
>> Not a big deal. It'd be fine as a followup improvement if
>> zhenwei wants to submit a later cleanup.
>>
>> With regards,
>> Daniel
>
> zhenwei can you confirm pls?
>
Hi,
I noticed that Markus has already separated HMP commands from monitor/*
into subsystems, so I need rework this part of this series.
I sent a new version which includes: 1, HMP part; 2, suggested by 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 :|
>
--
zhenwei pi
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 12/12] MAINTAINERS: add myself as the maintainer for cryptodev
2023-01-29 2:57 [PATCH v4 00/12] Refactor cryptodev zhenwei pi
` (10 preceding siblings ...)
2023-01-29 2:57 ` [PATCH v4 11/12] cryptodev: Support query-stats QMP command zhenwei pi
@ 2023-01-29 2:57 ` zhenwei pi
2023-02-06 0:23 ` PING: [PATCH v4 00/12] Refactor cryptodev zhenwei pi
12 siblings, 0 replies; 21+ messages in thread
From: zhenwei pi @ 2023-01-29 2:57 UTC (permalink / raw)
To: arei.gonglei, mst, dgilbert, pbonzini, berrange
Cc: armbru, qemu-devel, zhenwei pi
I developed the akcipher service, QoS setting, QMP/HMP commands and
statistics accounting for crypto device. Making myself as the
maintainer for QEMU's cryptodev.
Cc: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 9f6c54b145..e21a6ee470 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2857,6 +2857,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.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* PING: [PATCH v4 00/12] Refactor cryptodev
2023-01-29 2:57 [PATCH v4 00/12] Refactor cryptodev zhenwei pi
` (11 preceding siblings ...)
2023-01-29 2:57 ` [PATCH v4 12/12] MAINTAINERS: add myself as the maintainer for cryptodev zhenwei pi
@ 2023-02-06 0:23 ` zhenwei pi
12 siblings, 0 replies; 21+ messages in thread
From: zhenwei pi @ 2023-02-06 0:23 UTC (permalink / raw)
To: mst; +Cc: armbru, qemu-devel, arei.gonglei, dgilbert, berrange, pbonzini
Hi Michael
Please correct me if I miss anything...
On 1/29/23 10:57, zhenwei pi wrote:
> v4 -> v5:
> - suggested by MST, use 'PRIu32' instead of '%u' to print a uint32_t value
> - correct *QCryptodevBackendClient* and *QCryptodevInfo* in qapi/cryptodev.json
>
> v3 -> v4:
> - a small change in '0005-cryptodev-Introduce-query-cryptodev-QMP-command.patch':
> use 'uint32' instead of 'int' to describe CryptodevBackendClient:queue
> - fix compling warning(gcc)/error(clang-11) on 32 bit platform in
> '0007-hmp-add-cryptodev-info-command.patch':
> use 'printf("%u", client->queue)' instead of 'printf("%ld", client->queue)'
>
> v2 -> v3:
> - rebase code against the lastest commist: fb7e7990342e59cf67d
> - document the missing fields in qapi/cryptodev.json
> - rework statistics part: use 'query-stats' command instead of
> 'query-cryptodev'(cryptodev: Support query-stats QMP command)
>
> 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 (12):
> 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-builtin: Detect akcipher capability
> hmp: add cryptodev info command
> cryptodev: Use CryptoDevBackendOpInfo for operation
> cryptodev: Account statistics
> cryptodev: support QoS
> cryptodev: Support query-stats QMP command
> 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 | 419 ++++++++++++++++++++++++++++++--
> hmp-commands-info.hx | 14 ++
> hw/virtio/virtio-crypto.c | 48 +++-
> include/monitor/hmp.h | 1 +
> include/sysemu/cryptodev.h | 95 ++++----
> monitor/hmp-cmds.c | 42 ++++
> monitor/qmp-cmds.c | 2 +
> qapi/cryptodev.json | 143 +++++++++++
> qapi/meson.build | 1 +
> qapi/qapi-schema.json | 1 +
> qapi/qom.json | 8 +-
> qapi/stats.json | 10 +-
> 17 files changed, 744 insertions(+), 120 deletions(-)
> create mode 100644 qapi/cryptodev.json
>
--
zhenwei pi
^ permalink raw reply [flat|nested] 21+ messages in thread