* [PATCH v4 00/12] Refactor cryptodev
@ 2023-01-29 2:57 zhenwei pi
2023-01-29 2:57 ` [PATCH v4 01/12] cryptodev: Introduce cryptodev.json zhenwei pi
` (12 more replies)
0 siblings, 13 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
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
--
2.34.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [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
* [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
* 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
end of thread, other threads:[~2023-03-01 2:55 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH v4 03/12] cryptodev: Introduce cryptodev alg type in QAPI zhenwei pi
2023-01-29 2:57 ` [PATCH v4 04/12] cryptodev: Introduce server " zhenwei pi
2023-01-29 2:57 ` [PATCH v4 05/12] cryptodev: Introduce 'query-cryptodev' QMP command zhenwei pi
2023-01-29 2:57 ` [PATCH v4 06/12] cryptodev-builtin: Detect akcipher capability zhenwei pi
2023-01-29 2:57 ` [PATCH v4 07/12] hmp: add cryptodev info command zhenwei pi
2023-01-29 2:57 ` [PATCH v4 08/12] cryptodev: Use CryptoDevBackendOpInfo for operation zhenwei pi
2023-01-29 2:57 ` [PATCH v4 09/12] cryptodev: Account statistics zhenwei pi
2023-01-29 2:57 ` [PATCH v4 10/12] cryptodev: support QoS zhenwei pi
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é
2023-02-28 14:06 ` Markus Armbruster
2023-02-28 14:13 ` Michael S. Tsirkin
2023-02-28 14:21 ` Daniel P. Berrangé
2023-02-28 14:58 ` Michael S. Tsirkin
2023-03-01 2:53 ` zhenwei pi
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
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.