On 9/27/24 7:50 AM, Daniel P. Berrangé wrote:
Markus: QAPI design Qs for you at the bottom
On Wed, Sep 25, 2024 at 10:19:33AM -0300, Daniel Henrique Barboza wrote:
On 9/19/24 9:22 AM, Daniel P. Berrangé wrote:
On Thu, Sep 19, 2024 at 08:20:56AM -0300, Daniel Henrique Barboza wrote:
Add a QMP command that shows all specific properties of the current
accelerator in use.
Why do we need to expose /everything/ ?
I wouldn't mind pick and choose advertised properties for the accelerators
like we do with other APIs.
This would mean that each arch should choose what to advertise or not, given
that
some accelerator properties might be relevant just for some archs. The API would
be implemented by each arch individually.
Well with qemu-system-any we might get multiple arches reporting
info in the same binary, so we'll need to fan out to fill in the
per-arch info, after doing a common base.
Hmmm, i wonder if qemu-system-any will support mixing KVM and TCG ?
ie KVM for the host native accelerator, combined with TCG for the
foreign archs ??? Hopefully not !
If you're talking about Phil's patches it seems that it'll be TCG only:
https://lore.kernel.org/qemu-devel/20240305220938.85410-1-phi...@linaro.org/
Patch 2 commit msg states:
------
Add the 'any'-architecture target.
- Only consider 64-bit targets
- Do not use any hardware accelerator (except qtest)
- For architecture constants, use:
. max of supported targets phys/virt address space
. max of supported targets MMU modes
. min of supported targets variable page bits
------
Thanks,
Daniel
This can be used as a complement of other APIs like query-machines and
query-cpu-model-expansion, allowing management to get a more complete
picture of the running QEMU process.
query-machines doesn't return every single QOM property, just
a hand selected set of information pieces.
The query-cpu-model-expansion does return everything, but I
consider that command to be bad design, as it doesn't distinguish
between hardware CPU features, and QEMU QOM properties
This is the output with a x86_64 TCG guest:
$ ./build/qemu-system-x86_64 -S -display none -accel tcg -qmp
tcp:localhost:1234,server,wait=off
$ ./scripts/qmp/qmp-shell localhost:1234
Welcome to the QMP low-level shell!
Connected to QEMU 9.1.50
(QEMU) query-accelerator
{"return": {"name": "tcg", "props": {"one-insn-per-tb": false, "thread": "multi", "tb-size": 0,
"split-wx": false, "type": "tcg-accel"}}}
And for a x86_64 KVM guest:
$ ./build/qemu-system-x86_64 -S -display none -accel kvm -qmp
tcp:localhost:1234,server,wait=off
$ ./scripts/qmp/qmp-shell localhost:1234
Welcome to the QMP low-level shell!
Connected to QEMU 9.1.50
(QEMU) query-accelerator
{"return": {"name": "KVM", "props": {"mem-container-smram[0]": "", "xen-gnttab-max-frames": 64, "device": "", "xen-version": 0, "mem-smram[0]": "",
"notify-window": 0, "dirty-ring-size": 0, "kvm-shadow-mem": -1, "type": "kvm-accel", "notify-vmexit": "run", "xen-evtchn-max-pirq": 256}}}
Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
---
hw/core/machine-qmp-cmds.c | 34 ++++++++++++++++++++++++++++++++++
qapi/machine.json | 27 +++++++++++++++++++++++++++
2 files changed, 61 insertions(+)
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 130217da8f..eac803bf36 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
+AccelInfo *qmp_query_accelerator(Error **errp)
+{
+ AccelState *accel = current_accel();
+ AccelClass *acc = ACCEL_GET_CLASS(accel);
+ AccelInfo *info = g_new0(AccelInfo, 1);
+ QDict *qdict_out = qdict_new();
+ ObjectPropertyIterator iter;
+ ObjectProperty *prop;
+
+ info->name = g_strdup(acc->name);
+
+ object_property_iter_init(&iter, OBJECT(accel));
+ while ((prop = object_property_iter_next(&iter))) {
+ QObject *value;
+
+ if (!prop->get) {
+ continue;
+ }
+
+ value = object_property_get_qobject(OBJECT(accel), prop->name,
+ &error_abort);
+ qdict_put_obj(qdict_out, prop->name, value);
+ }
I'm not at all convinced trhat we should be exposing every single
QOM property on the accelerator class as public QMP data
+
+ if (!qdict_size(qdict_out)) {
+ qobject_unref(qdict_out);
+ } else {
+ info->props = QOBJECT(qdict_out);
+ }
+
+ return info;
+}
diff --git a/qapi/machine.json b/qapi/machine.json
index a6b8795b09..d0d527d1eb 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1898,3 +1898,30 @@
{ 'command': 'x-query-interrupt-controllers',
'returns': 'HumanReadableText',
'features': [ 'unstable' ]}
+
+##
+# @AccelInfo:
+#
+# Information about the current accelerator.
+#
+# @name: the name of the current accelerator being used
+#
+# @props: a dictionary of the accelerator properties
+#
+# Since: 9.2
+##
+{ 'struct': 'AccelInfo',
+ 'data': { 'name': 'str',
+ '*props': 'any' } }
This is way too open ended. IMHO ideally we would never add more
instances of the 'any' type, as it has many downsides
- zero documentation about what is available
- no version info about when each prop was introduced
- no ability to tag fields as deprecated
For this new API, IMHO 'name' should be an enumeration of the
accelerator types, and thenm 'props' should be a discrinated
union of accelerator specific structs
We have accelerator properties that differs from arch to arch, e.g. x86 has
properties like
notify-vmexit, declared in kvm_arch_accel_class_init() from
target/i386/kvm/kvm.c, that no
other arch has access to. RISC-V has its own share of these properties too.
Is it possible to declare specific structs based on arch for the API? In a
quick glance
it seems like we're doing something like that with query-cpus-fast, where s390x
has
additional properties that are exposed.
To allow for qemu-system-any, which will eventually have multiple arches in
one, I guess we'll need multiple levels of nesting. Perhaps something like
this:
{ 'enum': 'AccelType',
'data': ['tcg', 'kvm', ....] }
{ 'union': 'AccelInfo',
'type': 'AccelType',
'data': {
'tcg': 'AccelInfoTCG',
'kvm': 'AccelInfoKVM',
} }
{ 'struct': 'AccelInfoTCGX86',
'data': {
'notify-vmexit': ...
} }
{ 'struct': 'AccelInfoTCGArch',
'data': {
'x86': 'AccelInfoTCGX86',
'riscv': 'AccelInfoTCGRiscV',
...etc...
}
{ 'struct': 'AccelInfoTCG',
'data': {
...non-arch specific fields...,
'arch': 'AccelInfoTCGArch',
} }
...equiv AccelInfoKVM* structs....
Markus: any other/better ideas ?
+
+##
+# @query-accelerator:
+#
+# Shows information about the accelerator in use.
+#
+# Returns: a CpuModelExpansionInfo describing the expanded CPU model
+#
+# Since: 9.2
+##
+{ 'command': 'query-accelerator',
+ 'returns': 'AccelInfo' }
--
With regards,
Daniel
