* [PATCH 01/10] qapi: expose rtc-reset-reinjection command unconditionally
2025-05-08 13:58 [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Daniel P. Berrangé
@ 2025-05-08 13:58 ` Daniel P. Berrangé
2025-05-10 9:57 ` Markus Armbruster
2025-05-08 13:58 ` [PATCH 02/10] qapi: expand docs for SEV commands Daniel P. Berrangé
` (11 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-08 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini,
Markus Armbruster, Pierrick Bouvier, Richard Henderson,
Daniel P. Berrangé
This removes the TARGET_I386 condition from the rtc-reset-reinjection
command. This requires providing a QMP command stub for non-i386 target.
This in turn requires moving the command out of misc-target.json, since
that will trigger symbol poisoning errors when built from target
independent code.
Rather than putting the command into misc.json, it is proposed to create
misc-$TARGET.json files to hold commands whose impl is conceptually
only applicable to a single target. This gives an obvious docs hint to
consumers that the command is only useful in relation a specific target,
while misc.json is for commands applicable to 2 or more targets.
The current impl of qmp_rtc_reset_reinject() is a no-op if the i386
RTC is disabled in Kconfig, or if the running machine type lack any
RTC device. Thus the stub impl for non-i386 targets retains this
no-op behaviour, instead of reporting a Error which is the more usual
choice for commands invoked against unsupported configurations.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hw/i386/monitor.c | 2 +-
qapi/meson.build | 1 +
qapi/misc-i386.json | 24 ++++++++++++++++++++++++
qapi/misc-target.json | 17 -----------------
qapi/qapi-schema.json | 1 +
stubs/meson.build | 1 +
stubs/monitor-i386-rtc.c | 10 ++++++++++
7 files changed, 38 insertions(+), 18 deletions(-)
create mode 100644 qapi/misc-i386.json
create mode 100644 stubs/monitor-i386-rtc.c
diff --git a/hw/i386/monitor.c b/hw/i386/monitor.c
index 1921e4d52e..79df96562f 100644
--- a/hw/i386/monitor.c
+++ b/hw/i386/monitor.c
@@ -26,7 +26,7 @@
#include "monitor/monitor.h"
#include "qobject/qdict.h"
#include "qapi/error.h"
-#include "qapi/qapi-commands-misc-target.h"
+#include "qapi/qapi-commands-misc-i386.h"
#include "hw/i386/x86.h"
#include "hw/rtc/mc146818rtc.h"
diff --git a/qapi/meson.build b/qapi/meson.build
index eadde4db30..3a9bd06104 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -64,6 +64,7 @@ if have_system
'qdev',
'pci',
'rocker',
+ 'misc-i386',
'tpm',
'uefi',
]
diff --git a/qapi/misc-i386.json b/qapi/misc-i386.json
new file mode 100644
index 0000000000..d5bfd91405
--- /dev/null
+++ b/qapi/misc-i386.json
@@ -0,0 +1,24 @@
+# -*- Mode: Python -*-
+# vim: filetype=python
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+##
+# @rtc-reset-reinjection:
+#
+# This command will reset the RTC interrupt reinjection backlog. Can
+# be used if another mechanism to synchronize guest time is in effect,
+# for example QEMU guest agent's guest-set-time command.
+#
+# Use of this command is only applicable for x86 machines with an RTC,
+# and on other machines will silently return without performing any
+# action.
+#
+# Since: 2.1
+#
+# .. qmp-example::
+#
+# -> { "execute": "rtc-reset-reinjection" }
+# <- { "return": {} }
+##
+{ 'command': 'rtc-reset-reinjection' }
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 42e4a7417d..5d0ffb0164 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -2,23 +2,6 @@
# vim: filetype=python
#
-##
-# @rtc-reset-reinjection:
-#
-# This command will reset the RTC interrupt reinjection backlog. Can
-# be used if another mechanism to synchronize guest time is in effect,
-# for example QEMU guest agent's guest-set-time command.
-#
-# Since: 2.1
-#
-# .. qmp-example::
-#
-# -> { "execute": "rtc-reset-reinjection" }
-# <- { "return": {} }
-##
-{ 'command': 'rtc-reset-reinjection',
- 'if': 'TARGET_I386' }
-
##
# @SevState:
#
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 7bc600bb76..96f6aa4413 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -61,6 +61,7 @@
{ 'include': 'replay.json' }
{ 'include': 'yank.json' }
{ 'include': 'misc.json' }
+{ 'include': 'misc-i386.json' }
{ 'include': 'misc-target.json' }
{ 'include': 'audio.json' }
{ 'include': 'acpi.json' }
diff --git a/stubs/meson.build b/stubs/meson.build
index 63392f5e78..9907b54c1e 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -77,6 +77,7 @@ if have_system
stub_ss.add(files('target-monitor-defs.c'))
stub_ss.add(files('win32-kbd-hook.c'))
stub_ss.add(files('xen-hw-stub.c'))
+ stub_ss.add(files('monitor-i386-rtc.c'))
endif
if have_system or have_user
diff --git a/stubs/monitor-i386-rtc.c b/stubs/monitor-i386-rtc.c
new file mode 100644
index 0000000000..ee2e60d95b
--- /dev/null
+++ b/stubs/monitor-i386-rtc.c
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-misc-i386.h"
+
+void qmp_rtc_reset_reinjection(Error **errp)
+{
+ /* Nothing to do since non-x86 machines lack an RTC */
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 01/10] qapi: expose rtc-reset-reinjection command unconditionally
2025-05-08 13:58 ` [PATCH 01/10] qapi: expose rtc-reset-reinjection command unconditionally Daniel P. Berrangé
@ 2025-05-10 9:57 ` Markus Armbruster
2025-05-12 18:33 ` Daniel P. Berrangé
0 siblings, 1 reply; 41+ messages in thread
From: Markus Armbruster @ 2025-05-10 9:57 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini, Pierrick Bouvier, Richard Henderson
Daniel P. Berrangé <berrange@redhat.com> writes:
> This removes the TARGET_I386 condition from the rtc-reset-reinjection
> command. This requires providing a QMP command stub for non-i386 target.
> This in turn requires moving the command out of misc-target.json, since
> that will trigger symbol poisoning errors when built from target
> independent code.
>
> Rather than putting the command into misc.json, it is proposed to create
> misc-$TARGET.json files to hold commands whose impl is conceptually
> only applicable to a single target. This gives an obvious docs hint to
> consumers that the command is only useful in relation a specific target,
> while misc.json is for commands applicable to 2 or more targets.
Starting with this patch, the series structures the manual like this:
= Machines
... contents of machine.json ...
== Specific to S390
... contents of machine-s390.json ...
and
= Miscellanea
... contents of misc.json ...
== Specific to ARM
... contents of misc-arm.json ...
== Specific to i386
... contents of misc-i386.json ...
Except it doesn't add == subsection headers, but that's detail. The
text I show for them here is crap.
Possible alternative: collect the target-specific stuff in one place
rather than two:
= Targets
== ARM
== i386
== S390
Again the header text is crap.
Is separating the current contents of misc-<target>.json from
machine-<target>.json useful?
> The current impl of qmp_rtc_reset_reinject() is a no-op if the i386
> RTC is disabled in Kconfig, or if the running machine type lack any
> RTC device. Thus the stub impl for non-i386 targets retains this
> no-op behaviour, instead of reporting a Error which is the more usual
> choice for commands invoked against unsupported configurations.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> hw/i386/monitor.c | 2 +-
> qapi/meson.build | 1 +
> qapi/misc-i386.json | 24 ++++++++++++++++++++++++
> qapi/misc-target.json | 17 -----------------
> qapi/qapi-schema.json | 1 +
> stubs/meson.build | 1 +
> stubs/monitor-i386-rtc.c | 10 ++++++++++
> 7 files changed, 38 insertions(+), 18 deletions(-)
> create mode 100644 qapi/misc-i386.json
> create mode 100644 stubs/monitor-i386-rtc.c
>
> diff --git a/hw/i386/monitor.c b/hw/i386/monitor.c
> index 1921e4d52e..79df96562f 100644
> --- a/hw/i386/monitor.c
> +++ b/hw/i386/monitor.c
> @@ -26,7 +26,7 @@
> #include "monitor/monitor.h"
> #include "qobject/qdict.h"
> #include "qapi/error.h"
> -#include "qapi/qapi-commands-misc-target.h"
> +#include "qapi/qapi-commands-misc-i386.h"
> #include "hw/i386/x86.h"
> #include "hw/rtc/mc146818rtc.h"
>
> diff --git a/qapi/meson.build b/qapi/meson.build
> index eadde4db30..3a9bd06104 100644
> --- a/qapi/meson.build
> +++ b/qapi/meson.build
> @@ -64,6 +64,7 @@ if have_system
> 'qdev',
> 'pci',
> 'rocker',
> + 'misc-i386',
> 'tpm',
> 'uefi',
> ]
> diff --git a/qapi/misc-i386.json b/qapi/misc-i386.json
> new file mode 100644
> index 0000000000..d5bfd91405
> --- /dev/null
> +++ b/qapi/misc-i386.json
> @@ -0,0 +1,24 @@
> +# -*- Mode: Python -*-
> +# vim: filetype=python
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
Might be cleaner to add this to all qapi/*.json first, and in a separate
patch.
> +
> +##
> +# @rtc-reset-reinjection:
> +#
> +# This command will reset the RTC interrupt reinjection backlog. Can
> +# be used if another mechanism to synchronize guest time is in effect,
> +# for example QEMU guest agent's guest-set-time command.
> +#
> +# Use of this command is only applicable for x86 machines with an RTC,
> +# and on other machines will silently return without performing any
> +# action.
This paragraph replaces ...
> +#
> +# Since: 2.1
> +#
> +# .. qmp-example::
> +#
> +# -> { "execute": "rtc-reset-reinjection" }
> +# <- { "return": {} }
> +##
> +{ 'command': 'rtc-reset-reinjection' }
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index 42e4a7417d..5d0ffb0164 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -2,23 +2,6 @@
> # vim: filetype=python
> #
>
> -##
> -# @rtc-reset-reinjection:
> -#
> -# This command will reset the RTC interrupt reinjection backlog. Can
> -# be used if another mechanism to synchronize guest time is in effect,
> -# for example QEMU guest agent's guest-set-time command.
> -#
> -# Since: 2.1
> -#
> -# .. qmp-example::
> -#
> -# -> { "execute": "rtc-reset-reinjection" }
> -# <- { "return": {} }
> -##
> -{ 'command': 'rtc-reset-reinjection',
> - 'if': 'TARGET_I386' }
... the conditional.
Before, attempting to execute the command fails with CommandNotFound.
Afterwards it succeeds without doing anything. I think it should fail
instead. CommandNotFound would be a lie, so change it to GenericError.
"Specific to target" is no longer machine-readable. Should
machine-readability be (or become) desirable, we could expose it via
suitable QAPI features, e.g. 'features': ['target-i386'].
> -
> ##
> # @SevState:
> #
> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> index 7bc600bb76..96f6aa4413 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -61,6 +61,7 @@
> { 'include': 'replay.json' }
> { 'include': 'yank.json' }
> { 'include': 'misc.json' }
> +{ 'include': 'misc-i386.json' }
> { 'include': 'misc-target.json' }
> { 'include': 'audio.json' }
> { 'include': 'acpi.json' }
> diff --git a/stubs/meson.build b/stubs/meson.build
> index 63392f5e78..9907b54c1e 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -77,6 +77,7 @@ if have_system
> stub_ss.add(files('target-monitor-defs.c'))
> stub_ss.add(files('win32-kbd-hook.c'))
> stub_ss.add(files('xen-hw-stub.c'))
> + stub_ss.add(files('monitor-i386-rtc.c'))
> endif
>
> if have_system or have_user
> diff --git a/stubs/monitor-i386-rtc.c b/stubs/monitor-i386-rtc.c
> new file mode 100644
> index 0000000000..ee2e60d95b
> --- /dev/null
> +++ b/stubs/monitor-i386-rtc.c
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-misc-i386.h"
> +
> +void qmp_rtc_reset_reinjection(Error **errp)
> +{
> + /* Nothing to do since non-x86 machines lack an RTC */
> +}
I think I'd create one stub file per qapi/<foo>-<target>.json.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 01/10] qapi: expose rtc-reset-reinjection command unconditionally
2025-05-10 9:57 ` Markus Armbruster
@ 2025-05-12 18:33 ` Daniel P. Berrangé
2025-05-13 0:54 ` Pierrick Bouvier
2025-05-13 7:55 ` Markus Armbruster
0 siblings, 2 replies; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-12 18:33 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini, Pierrick Bouvier, Richard Henderson
On Sat, May 10, 2025 at 11:57:10AM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
> > This removes the TARGET_I386 condition from the rtc-reset-reinjection
> > command. This requires providing a QMP command stub for non-i386 target.
> > This in turn requires moving the command out of misc-target.json, since
> > that will trigger symbol poisoning errors when built from target
> > independent code.
> >
> > Rather than putting the command into misc.json, it is proposed to create
> > misc-$TARGET.json files to hold commands whose impl is conceptually
> > only applicable to a single target. This gives an obvious docs hint to
> > consumers that the command is only useful in relation a specific target,
> > while misc.json is for commands applicable to 2 or more targets.
>
> Starting with this patch, the series structures the manual like this:
>
> = Machines
> ... contents of machine.json ...
> == Specific to S390
> ... contents of machine-s390.json ...
>
> and
>
> = Miscellanea
> ... contents of misc.json ...
> == Specific to ARM
> ... contents of misc-arm.json ...
> == Specific to i386
> ... contents of misc-i386.json ...
>
> Except it doesn't add == subsection headers, but that's detail. The
> text I show for them here is crap.
>
> Possible alternative: collect the target-specific stuff in one place
> rather than two:
>
> = Targets
> == ARM
> == i386
> == S390
>
> Again the header text is crap.
>
> Is separating the current contents of misc-<target>.json from
> machine-<target>.json useful?
I'm fairly confused what the intended difference is between
stuff in 'misc.json' and stuff in 'machine.json' already,
and just preserved that split rather than try to think about
it in more detail.
> > diff --git a/qapi/misc-i386.json b/qapi/misc-i386.json
> > new file mode 100644
> > index 0000000000..d5bfd91405
> > --- /dev/null
> > +++ b/qapi/misc-i386.json
> > @@ -0,0 +1,24 @@
> > +# -*- Mode: Python -*-
> > +# vim: filetype=python
> > +#
> > +# SPDX-License-Identifier: GPL-2.0-or-later
>
> Might be cleaner to add this to all qapi/*.json first, and in a separate
> patch.
Adding SPDX-License-Identifier to existing files is something we're
not generally doing, only newly created files are getting it.
> > +
> > +##
> > +# @rtc-reset-reinjection:
> > +#
> > +# This command will reset the RTC interrupt reinjection backlog. Can
> > +# be used if another mechanism to synchronize guest time is in effect,
> > +# for example QEMU guest agent's guest-set-time command.
> > +#
> > +# Use of this command is only applicable for x86 machines with an RTC,
> > +# and on other machines will silently return without performing any
> > +# action.
>
> This paragraph replaces ...
>
> > +#
> > +# Since: 2.1
> > +#
> > +# .. qmp-example::
> > +#
> > +# -> { "execute": "rtc-reset-reinjection" }
> > +# <- { "return": {} }
> > +##
> > +{ 'command': 'rtc-reset-reinjection' }
> > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > index 42e4a7417d..5d0ffb0164 100644
> > --- a/qapi/misc-target.json
> > +++ b/qapi/misc-target.json
> > @@ -2,23 +2,6 @@
> > # vim: filetype=python
> > #
> >
> > -##
> > -# @rtc-reset-reinjection:
> > -#
> > -# This command will reset the RTC interrupt reinjection backlog. Can
> > -# be used if another mechanism to synchronize guest time is in effect,
> > -# for example QEMU guest agent's guest-set-time command.
> > -#
> > -# Since: 2.1
> > -#
> > -# .. qmp-example::
> > -#
> > -# -> { "execute": "rtc-reset-reinjection" }
> > -# <- { "return": {} }
> > -##
> > -{ 'command': 'rtc-reset-reinjection',
> > - 'if': 'TARGET_I386' }
>
> ... the conditional.
>
> Before, attempting to execute the command fails with CommandNotFound.
>
> Afterwards it succeeds without doing anything. I think it should fail
> instead. CommandNotFound would be a lie, so change it to GenericError.
See also my comment in the commit message that this has different
behaviour on x86 vs non-x86, when no RTC is present - x86 treats
"no RTC" as a no-op, but non-x86 treats it as an error.
I don't mind if we preserve this inconsistency though.
> > diff --git a/stubs/monitor-i386-rtc.c b/stubs/monitor-i386-rtc.c
> > new file mode 100644
> > index 0000000000..ee2e60d95b
> > --- /dev/null
> > +++ b/stubs/monitor-i386-rtc.c
> > @@ -0,0 +1,10 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qapi/error.h"
> > +#include "qapi/qapi-commands-misc-i386.h"
> > +
> > +void qmp_rtc_reset_reinjection(Error **errp)
> > +{
> > + /* Nothing to do since non-x86 machines lack an RTC */
> > +}
>
> I think I'd create one stub file per qapi/<foo>-<target>.json.
That's what I started doing but I forgot that doesn't work out well
with the linker.
The linker's granularity for dropping stubs is per-file, not per-symbol.
If /any/ method in a stub/nnn.c file is needed, the linker will pull in
all methods. This results in duplicate symbol errors if only a subset
of stub methods were actually needed.
This forces us to have a separate stub file per build configuration
scenario that can affect use of stubs.
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] 41+ messages in thread* Re: [PATCH 01/10] qapi: expose rtc-reset-reinjection command unconditionally
2025-05-12 18:33 ` Daniel P. Berrangé
@ 2025-05-13 0:54 ` Pierrick Bouvier
2025-05-13 1:09 ` Pierrick Bouvier
2025-05-13 7:55 ` Markus Armbruster
1 sibling, 1 reply; 41+ messages in thread
From: Pierrick Bouvier @ 2025-05-13 0:54 UTC (permalink / raw)
To: Daniel P. Berrangé, Markus Armbruster
Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini, Richard Henderson
On 5/12/25 11:33 AM, Daniel P. Berrangé wrote:
>>> diff --git a/stubs/monitor-i386-rtc.c b/stubs/monitor-i386-rtc.c
>>> new file mode 100644
>>> index 0000000000..ee2e60d95b
>>> --- /dev/null
>>> +++ b/stubs/monitor-i386-rtc.c
>>> @@ -0,0 +1,10 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "qapi/error.h"
>>> +#include "qapi/qapi-commands-misc-i386.h"
>>> +
>>> +void qmp_rtc_reset_reinjection(Error **errp)
>>> +{
>>> + /* Nothing to do since non-x86 machines lack an RTC */
>>> +}
>>
>> I think I'd create one stub file per qapi/<foo>-<target>.json.
>
> That's what I started doing but I forgot that doesn't work out well
> with the linker.
>
> The linker's granularity for dropping stubs is per-file, not per-symbol.
>
> If /any/ method in a stub/nnn.c file is needed, the linker will pull in
> all methods. This results in duplicate symbol errors if only a subset
> of stub methods were actually needed.
>
Richard mentioned this behaviour as well, but I could not see where it
is implemented in our build system.
The only thing possibly related is the undefsyms mechanic, but from what
I understand, it's only for ensuring modules can be linked.
> This forces us to have a separate stub file per build configuration
> scenario that can affect use of stubs.
>
> With regards,
> Daniel
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 01/10] qapi: expose rtc-reset-reinjection command unconditionally
2025-05-13 0:54 ` Pierrick Bouvier
@ 2025-05-13 1:09 ` Pierrick Bouvier
0 siblings, 0 replies; 41+ messages in thread
From: Pierrick Bouvier @ 2025-05-13 1:09 UTC (permalink / raw)
To: Daniel P. Berrangé, Markus Armbruster
Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini, Richard Henderson
On 5/12/25 5:54 PM, Pierrick Bouvier wrote:
> On 5/12/25 11:33 AM, Daniel P. Berrangé wrote:
>>>> diff --git a/stubs/monitor-i386-rtc.c b/stubs/monitor-i386-rtc.c
>>>> new file mode 100644
>>>> index 0000000000..ee2e60d95b
>>>> --- /dev/null
>>>> +++ b/stubs/monitor-i386-rtc.c
>>>> @@ -0,0 +1,10 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>>> +
>>>> +#include "qemu/osdep.h"
>>>> +#include "qapi/error.h"
>>>> +#include "qapi/qapi-commands-misc-i386.h"
>>>> +
>>>> +void qmp_rtc_reset_reinjection(Error **errp)
>>>> +{
>>>> + /* Nothing to do since non-x86 machines lack an RTC */
>>>> +}
>>>
>>> I think I'd create one stub file per qapi/<foo>-<target>.json.
>>
>> That's what I started doing but I forgot that doesn't work out well
>> with the linker.
>>
>> The linker's granularity for dropping stubs is per-file, not per-symbol.
>>
>> If /any/ method in a stub/nnn.c file is needed, the linker will pull in
>> all methods. This results in duplicate symbol errors if only a subset
>> of stub methods were actually needed.
>>
>
> Richard mentioned this behaviour as well, but I could not see where it
> is implemented in our build system.
>
> The only thing possibly related is the undefsyms mechanic, but from what
> I understand, it's only for ensuring modules can be linked.
>
Answer: it's an implicit behaviour, due to the fact libqemuutil is built
as a (real) static library, so the linker only adds code as needed.
>> This forces us to have a separate stub file per build configuration
>> scenario that can affect use of stubs.
>>
>> With regards,
>> Daniel
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 01/10] qapi: expose rtc-reset-reinjection command unconditionally
2025-05-12 18:33 ` Daniel P. Berrangé
2025-05-13 0:54 ` Pierrick Bouvier
@ 2025-05-13 7:55 ` Markus Armbruster
1 sibling, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2025-05-13 7:55 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini, Pierrick Bouvier, Richard Henderson
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Sat, May 10, 2025 at 11:57:10AM +0200, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>>
>> > This removes the TARGET_I386 condition from the rtc-reset-reinjection
>> > command. This requires providing a QMP command stub for non-i386 target.
>> > This in turn requires moving the command out of misc-target.json, since
>> > that will trigger symbol poisoning errors when built from target
>> > independent code.
>> >
>> > Rather than putting the command into misc.json, it is proposed to create
>> > misc-$TARGET.json files to hold commands whose impl is conceptually
>> > only applicable to a single target. This gives an obvious docs hint to
>> > consumers that the command is only useful in relation a specific target,
>> > while misc.json is for commands applicable to 2 or more targets.
>>
>> Starting with this patch, the series structures the manual like this:
>>
>> = Machines
>> ... contents of machine.json ...
>> == Specific to S390
>> ... contents of machine-s390.json ...
>>
>> and
>>
>> = Miscellanea
>> ... contents of misc.json ...
>> == Specific to ARM
>> ... contents of misc-arm.json ...
>> == Specific to i386
>> ... contents of misc-i386.json ...
>>
>> Except it doesn't add == subsection headers, but that's detail. The
>> text I show for them here is crap.
>>
>> Possible alternative: collect the target-specific stuff in one place
>> rather than two:
>>
>> = Targets
>> == ARM
>> == i386
>> == S390
>>
>> Again the header text is crap.
>>
>> Is separating the current contents of misc-<target>.json from
>> machine-<target>.json useful?
>
> I'm fairly confused what the intended difference is between
> stuff in 'misc.json' and stuff in 'machine.json' already,
> and just preserved that split rather than try to think about
> it in more detail.
Fair enough, we can always rearrange things on top.
>> > The current impl of qmp_rtc_reset_reinject() is a no-op if the i386
>> > RTC is disabled in Kconfig, or if the running machine type lack any
>> > RTC device. Thus the stub impl for non-i386 targets retains this
>> > no-op behaviour, instead of reporting a Error which is the more usual
>> > choice for commands invoked against unsupported configurations.
>> >
>> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
[...]
>> > diff --git a/qapi/misc-i386.json b/qapi/misc-i386.json
>> > new file mode 100644
>> > index 0000000000..d5bfd91405
>> > --- /dev/null
>> > +++ b/qapi/misc-i386.json
>> > @@ -0,0 +1,24 @@
>> > +# -*- Mode: Python -*-
>> > +# vim: filetype=python
>> > +#
>> > +# SPDX-License-Identifier: GPL-2.0-or-later
>>
>> Might be cleaner to add this to all qapi/*.json first, and in a separate
>> patch.
>
> Adding SPDX-License-Identifier to existing files is something we're
> not generally doing, only newly created files are getting it.
I see.
Code motion between files where the license isn't obviously the same
gives me pause.
>> > +
>> > +##
>> > +# @rtc-reset-reinjection:
>> > +#
>> > +# This command will reset the RTC interrupt reinjection backlog. Can
>> > +# be used if another mechanism to synchronize guest time is in effect,
>> > +# for example QEMU guest agent's guest-set-time command.
>> > +#
>> > +# Use of this command is only applicable for x86 machines with an RTC,
>> > +# and on other machines will silently return without performing any
>> > +# action.
>>
>> This paragraph replaces ...
>>
>> > +#
>> > +# Since: 2.1
>> > +#
>> > +# .. qmp-example::
>> > +#
>> > +# -> { "execute": "rtc-reset-reinjection" }
>> > +# <- { "return": {} }
>> > +##
>> > +{ 'command': 'rtc-reset-reinjection' }
>> > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
>> > index 42e4a7417d..5d0ffb0164 100644
>> > --- a/qapi/misc-target.json
>> > +++ b/qapi/misc-target.json
>> > @@ -2,23 +2,6 @@
>> > # vim: filetype=python
>> > #
>> >
>> > -##
>> > -# @rtc-reset-reinjection:
>> > -#
>> > -# This command will reset the RTC interrupt reinjection backlog. Can
>> > -# be used if another mechanism to synchronize guest time is in effect,
>> > -# for example QEMU guest agent's guest-set-time command.
>> > -#
>> > -# Since: 2.1
>> > -#
>> > -# .. qmp-example::
>> > -#
>> > -# -> { "execute": "rtc-reset-reinjection" }
>> > -# <- { "return": {} }
>> > -##
>> > -{ 'command': 'rtc-reset-reinjection',
>> > - 'if': 'TARGET_I386' }
>>
>> ... the conditional.
>>
>> Before, attempting to execute the command fails with CommandNotFound.
>>
>> Afterwards it succeeds without doing anything. I think it should fail
>> instead. CommandNotFound would be a lie, so change it to GenericError.
>
> See also my comment in the commit message that this has different
> behaviour on x86 vs non-x86, when no RTC is present - x86 treats
> "no RTC" as a no-op, but non-x86 treats it as an error.
>
> I don't mind if we preserve this inconsistency though.
I see.
The command is a mess. It goes back to commit f2ae8abf1fa (mc146818rtc:
add rtc-reset-reinjection QMP command). Here's the commit message's
rationale:
It is necessary to reset RTC interrupt reinjection backlog if
guest time is synchronized via a different mechanism, such as
QGA's guest-set-time command.
Failing to do so causes both corrections to be applied (summed),
resulting in an incorrect guest time.
Plausible enough, but why is the solution limited to target i386
machines with an MC146818 RTC? I think I found the answer in recent
commit d0be0ac2c37 (hw/i386: move rtc-reset-reinjection command out of
hw/rtc):
The rtc-reset-reinjection QMP command is specific to x86, other boards do not
have the ACK tracking functionality that is needed for RTC interrupt
reinjection. Therefore the QMP command is only included in x86, but
qmp_rtc_reset_reinjection() is implemented by hw/rtc/mc146818rtc.c
and requires tracking of all created RTC devices. Move the implementation
to hw/i386, so that 1) it is available even if no RTC device exist
2) the only RTC that exists is easily found in x86ms->rtc.
So, the time sync problem is serious enough for us to provide a
solution, but it's only serious enough for i386 machines that use the
RTC most common there. This makes no sense to me.
Code:
void qmp_rtc_reset_reinjection(Error **errp)
{
X86MachineState *x86ms = X86_MACHINE(qdev_get_machine());
#ifdef CONFIG_MC146818RTC
if (x86ms->rtc) {
rtc_reset_reinjection(MC146818_RTC(x86ms->rtc));
}
#else
assert(!x86ms->rtc);
#endif
}
Behavior:
* target other than i386: CommandNotFound
* else if the machine's ->rtc is null: silently do nothing
* else if CONFIG_MC146818RTC: works as advertized as long ->rtc is an
MC146818, else crash
* else: crash
WTF!?!
>> > diff --git a/stubs/monitor-i386-rtc.c b/stubs/monitor-i386-rtc.c
>> > new file mode 100644
>> > index 0000000000..ee2e60d95b
>> > --- /dev/null
>> > +++ b/stubs/monitor-i386-rtc.c
>> > @@ -0,0 +1,10 @@
>> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> > +
>> > +#include "qemu/osdep.h"
>> > +#include "qapi/error.h"
>> > +#include "qapi/qapi-commands-misc-i386.h"
>> > +
>> > +void qmp_rtc_reset_reinjection(Error **errp)
>> > +{
>> > + /* Nothing to do since non-x86 machines lack an RTC */
The comment is wrong: pretty much all machines do have a real time
clock.
>> > +}
>>
>> I think I'd create one stub file per qapi/<foo>-<target>.json.
>
> That's what I started doing but I forgot that doesn't work out well
> with the linker.
>
> The linker's granularity for dropping stubs is per-file, not per-symbol.
>
> If /any/ method in a stub/nnn.c file is needed, the linker will pull in
> all methods. This results in duplicate symbol errors if only a subset
> of stub methods were actually needed.
>
> This forces us to have a separate stub file per build configuration
> scenario that can affect use of stubs.
You're right.
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 02/10] qapi: expand docs for SEV commands
2025-05-08 13:58 [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Daniel P. Berrangé
2025-05-08 13:58 ` [PATCH 01/10] qapi: expose rtc-reset-reinjection command unconditionally Daniel P. Berrangé
@ 2025-05-08 13:58 ` Daniel P. Berrangé
2025-05-13 12:06 ` Markus Armbruster
2025-05-08 13:58 ` [PATCH 03/10] qapi: make SEV commands unconditionally available Daniel P. Berrangé
` (10 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-08 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini,
Markus Armbruster, Pierrick Bouvier, Richard Henderson,
Daniel P. Berrangé
This gives some more context about the behaviour of the commands in
unsupported guest configuration or platform scenarios.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
qapi/misc-target.json | 43 ++++++++++++++++++++++++++++++++++++-------
1 file changed, 36 insertions(+), 7 deletions(-)
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 5d0ffb0164..ae55e437a5 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -110,7 +110,11 @@
##
# @query-sev:
#
-# Returns information about SEV
+# Returns information about SEV/SEV-ES/SEV-SNP.
+#
+# If unavailable due to an incompatible configuration the
+# returned @enabled field will be set to 'false' and the
+# state of all other fields is undefined.
#
# Returns: @SevInfo
#
@@ -141,7 +145,16 @@
##
# @query-sev-launch-measure:
#
-# Query the SEV guest launch information.
+# Query the SEV/SEV-ES guest launch information.
+#
+# This is only valid on x86 machines configured with KVM and the
+# 'sev-guest' confidential virtualization object. The launch
+# measurement for SEV-SNP guests is only available within
+# the guest.
+#
+# This will return an error if the launch measurement is
+# unavailable, either due to an invalid guest configuration
+# or if the guest has not reached the required SEV state.
#
# Returns: The @SevLaunchMeasureInfo for the guest
#
@@ -185,8 +198,9 @@
##
# @query-sev-capabilities:
#
-# This command is used to get the SEV capabilities, and is supported
-# on AMD X86 platforms only.
+# This command is used to get the SEV capabilities, and is only
+# supported on AMD X86 platforms with KVM enabled. If SEV is not
+# available on the platform an error will be returned.
#
# Returns: SevCapability objects.
#
@@ -205,7 +219,15 @@
##
# @sev-inject-launch-secret:
#
-# This command injects a secret blob into memory of SEV guest.
+# This command injects a secret blob into memory of a SEV/SEV-ES guest.
+#
+# This is only valid on x86 machines configured with KVM and the
+# 'sev-guest' confidential virtualization object. SEV-SNP guests
+# do not support launch secret injection
+#
+# This will return an error if launch secret injection is not possible,
+# either due to an invalid guest configuration, or if the guest has not
+# reached the required SEV state.
#
# @packet-header: the launch secret packet header encoded in base64
#
@@ -236,8 +258,15 @@
##
# @query-sev-attestation-report:
#
-# This command is used to get the SEV attestation report, and is
-# supported on AMD X86 platforms only.
+# This command is used to get the SEV attestation report.
+#
+# This is only valid on x86 machines configured with KVM and the
+# 'sev-guest' confidential virtualization object. The attestation
+# report for SEV-SNP guests is only available within the guest.
+#
+# This will return an error if the attestation report is
+# unavailable, either due to an invalid guest configuration
+# or if the guest has not reached the required SEV state.
#
# @mnonce: a random 16 bytes value encoded in base64 (it will be
# included in report)
--
2.49.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 02/10] qapi: expand docs for SEV commands
2025-05-08 13:58 ` [PATCH 02/10] qapi: expand docs for SEV commands Daniel P. Berrangé
@ 2025-05-13 12:06 ` Markus Armbruster
2025-05-13 12:21 ` Daniel P. Berrangé
0 siblings, 1 reply; 41+ messages in thread
From: Markus Armbruster @ 2025-05-13 12:06 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini, Pierrick Bouvier, Richard Henderson
Daniel P. Berrangé <berrange@redhat.com> writes:
> This gives some more context about the behaviour of the commands in
> unsupported guest configuration or platform scenarios.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> qapi/misc-target.json | 43 ++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index 5d0ffb0164..ae55e437a5 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -110,7 +110,11 @@
> ##
> # @query-sev:
> #
> -# Returns information about SEV
> +# Returns information about SEV/SEV-ES/SEV-SNP.
> +#
> +# If unavailable due to an incompatible configuration the
> +# returned @enabled field will be set to 'false' and the
> +# state of all other fields is undefined.
That's awful. Not this patch's fault.
What's "incompatible configuration"?
Actual behavior as far as I can tell:
* If !CONFIG_SEV: GenericError "SEV is not available in this QEMU".
* If CONFIG_SEV and !sev_enabled(): SevInfo filled with zero bytes
* If CONFIG_SEV and sev_enabled(): SevInfo filled properly
sev_enabled() is true when the machine's cgs member is an instance of
"sev-common".
> #
> # Returns: @SevInfo
> #
> @@ -141,7 +145,16 @@
> ##
> # @query-sev-launch-measure:
> #
> -# Query the SEV guest launch information.
> +# Query the SEV/SEV-ES guest launch information.
> +#
> +# This is only valid on x86 machines configured with KVM and the
> +# 'sev-guest' confidential virtualization object. The launch
> +# measurement for SEV-SNP guests is only available within
> +# the guest.
> +#
> +# This will return an error if the launch measurement is
> +# unavailable, either due to an invalid guest configuration
> +# or if the guest has not reached the required SEV state.
> #
> # Returns: The @SevLaunchMeasureInfo for the guest
> #
> @@ -185,8 +198,9 @@
> ##
> # @query-sev-capabilities:
> #
> -# This command is used to get the SEV capabilities, and is supported
> -# on AMD X86 platforms only.
> +# This command is used to get the SEV capabilities, and is only
> +# supported on AMD X86 platforms with KVM enabled. If SEV is not
> +# available on the platform an error will be returned.
What does "not supported" mean here?
> #
> # Returns: SevCapability objects.
> #
> @@ -205,7 +219,15 @@
> ##
> # @sev-inject-launch-secret:
> #
> -# This command injects a secret blob into memory of SEV guest.
> +# This command injects a secret blob into memory of a SEV/SEV-ES guest.
> +#
> +# This is only valid on x86 machines configured with KVM and the
> +# 'sev-guest' confidential virtualization object. SEV-SNP guests
> +# do not support launch secret injection
> +#
> +# This will return an error if launch secret injection is not possible,
> +# either due to an invalid guest configuration, or if the guest has not
> +# reached the required SEV state.
> #
> # @packet-header: the launch secret packet header encoded in base64
> #
> @@ -236,8 +258,15 @@
> ##
> # @query-sev-attestation-report:
> #
> -# This command is used to get the SEV attestation report, and is
> -# supported on AMD X86 platforms only.
> +# This command is used to get the SEV attestation report.
> +#
> +# This is only valid on x86 machines configured with KVM and the
> +# 'sev-guest' confidential virtualization object. The attestation
> +# report for SEV-SNP guests is only available within the guest.
> +#
> +# This will return an error if the attestation report is
> +# unavailable, either due to an invalid guest configuration
> +# or if the guest has not reached the required SEV state.
> #
> # @mnonce: a random 16 bytes value encoded in base64 (it will be
> # included in report)
Strict improvemement already, so
Acked-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 02/10] qapi: expand docs for SEV commands
2025-05-13 12:06 ` Markus Armbruster
@ 2025-05-13 12:21 ` Daniel P. Berrangé
0 siblings, 0 replies; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-13 12:21 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini, Pierrick Bouvier, Richard Henderson
On Tue, May 13, 2025 at 02:06:40PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
> > This gives some more context about the behaviour of the commands in
> > unsupported guest configuration or platform scenarios.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > qapi/misc-target.json | 43 ++++++++++++++++++++++++++++++++++++-------
> > 1 file changed, 36 insertions(+), 7 deletions(-)
> >
> > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > index 5d0ffb0164..ae55e437a5 100644
> > --- a/qapi/misc-target.json
> > +++ b/qapi/misc-target.json
> > @@ -110,7 +110,11 @@
> > ##
> > # @query-sev:
> > #
> > -# Returns information about SEV
> > +# Returns information about SEV/SEV-ES/SEV-SNP.
> > +#
> > +# If unavailable due to an incompatible configuration the
> > +# returned @enabled field will be set to 'false' and the
> > +# state of all other fields is undefined.
>
> That's awful. Not this patch's fault.
Yep, IMHO, all the fields except 'enabled' should have been
optional, and omitted when @enabled==false. Probably too
later
> What's "incompatible configuration"?
Essentially it'll only set values for the extra fields
beyond @enabled when a configuration includes the
following:
'-object sev-guest,id=sev -machine ...,confidential-guest-support=sev"
(or sev-snp-guest object)
> Actual behavior as far as I can tell:
>
> * If !CONFIG_SEV: GenericError "SEV is not available in this QEMU".
>
> * If CONFIG_SEV and !sev_enabled(): SevInfo filled with zero bytes
Having these two scenarios be different feels wrong to me - they
are both "SEV not enabled" scenarios IMHO, and whether or not
SEV is enabled should be irrelevant.
A difference is justified in query-sev-capabilities as that's
a feature probing method, where as this one is a runtime state
query method.
> * If CONFIG_SEV and sev_enabled(): SevInfo filled properly
>
> sev_enabled() is true when the machine's cgs member is an instance of
> "sev-common".
Yep.
> > @@ -185,8 +198,9 @@
> > ##
> > # @query-sev-capabilities:
> > #
> > -# This command is used to get the SEV capabilities, and is supported
> > -# on AMD X86 platforms only.
> > +# This command is used to get the SEV capabilities, and is only
> > +# supported on AMD X86 platforms with KVM enabled. If SEV is not
> > +# available on the platform an error will be returned.
>
> What does "not supported" mean here?
Any of at least:
* Not x86 system target
* Not KVM accelerator
* No SEV in host kernel
* No SEV in host CPUs
* SEV not enabled in host UEFI
* /dev/sev device not accessible / not present
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] 41+ messages in thread
* [PATCH 03/10] qapi: make SEV commands unconditionally available
2025-05-08 13:58 [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Daniel P. Berrangé
2025-05-08 13:58 ` [PATCH 01/10] qapi: expose rtc-reset-reinjection command unconditionally Daniel P. Berrangé
2025-05-08 13:58 ` [PATCH 02/10] qapi: expand docs for SEV commands Daniel P. Berrangé
@ 2025-05-08 13:58 ` Daniel P. Berrangé
2025-05-08 13:58 ` [PATCH 04/10] qapi: expose query-gic-capability command unconditionally Daniel P. Berrangé
` (9 subsequent siblings)
12 siblings, 0 replies; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-08 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini,
Markus Armbruster, Pierrick Bouvier, Richard Henderson,
Daniel P. Berrangé
This removes the TARGET_I386 condition from the SEV confidential
virtualization commands, moving them to the recently introduced
misc-i386.json QAPI file, given they are inherantly i386 specific
commands.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
qapi/misc-i386.json | 271 ++++++++++++++++++++++++++++++++
qapi/misc-target.json | 284 ----------------------------------
stubs/meson.build | 1 +
stubs/monitor-i386-sev.c | 36 +++++
target/i386/sev-system-stub.c | 32 ----
target/i386/sev.c | 2 +-
6 files changed, 309 insertions(+), 317 deletions(-)
create mode 100644 stubs/monitor-i386-sev.c
diff --git a/qapi/misc-i386.json b/qapi/misc-i386.json
index d5bfd91405..de0e4ab67f 100644
--- a/qapi/misc-i386.json
+++ b/qapi/misc-i386.json
@@ -22,3 +22,274 @@
# <- { "return": {} }
##
{ 'command': 'rtc-reset-reinjection' }
+
+##
+# @SevState:
+#
+# An enumeration of SEV state information used during @query-sev.
+#
+# @uninit: The guest is uninitialized.
+#
+# @launch-update: The guest is currently being launched; plaintext
+# data and register state is being imported.
+#
+# @launch-secret: The guest is currently being launched; ciphertext
+# data is being imported.
+#
+# @running: The guest is fully launched or migrated in.
+#
+# @send-update: The guest is currently being migrated out to another
+# machine.
+#
+# @receive-update: The guest is currently being migrated from another
+# machine.
+#
+# Since: 2.12
+##
+{ 'enum': 'SevState',
+ 'data': ['uninit', 'launch-update', 'launch-secret', 'running',
+ 'send-update', 'receive-update' ] }
+
+##
+# @SevGuestType:
+#
+# An enumeration indicating the type of SEV guest being run.
+#
+# @sev: The guest is a legacy SEV or SEV-ES guest.
+#
+# @sev-snp: The guest is an SEV-SNP guest.
+#
+# Since: 6.2
+##
+{ 'enum': 'SevGuestType',
+ 'data': [ 'sev', 'sev-snp' ] }
+
+##
+# @SevGuestInfo:
+#
+# Information specific to legacy SEV/SEV-ES guests.
+#
+# @policy: SEV policy value
+#
+# @handle: SEV firmware handle
+#
+# Since: 2.12
+##
+{ 'struct': 'SevGuestInfo',
+ 'data': { 'policy': 'uint32',
+ 'handle': 'uint32' } }
+
+##
+# @SevSnpGuestInfo:
+#
+# Information specific to SEV-SNP guests.
+#
+# @snp-policy: SEV-SNP policy value
+#
+# Since: 9.1
+##
+{ 'struct': 'SevSnpGuestInfo',
+ 'data': { 'snp-policy': 'uint64' } }
+
+##
+# @SevInfo:
+#
+# Information about Secure Encrypted Virtualization (SEV) support
+#
+# @enabled: true if SEV is active
+#
+# @api-major: SEV API major version
+#
+# @api-minor: SEV API minor version
+#
+# @build-id: SEV FW build id
+#
+# @state: SEV guest state
+#
+# @sev-type: Type of SEV guest being run
+#
+# Since: 2.12
+##
+{ 'union': 'SevInfo',
+ 'base': { 'enabled': 'bool',
+ 'api-major': 'uint8',
+ 'api-minor' : 'uint8',
+ 'build-id' : 'uint8',
+ 'state' : 'SevState',
+ 'sev-type' : 'SevGuestType' },
+ 'discriminator': 'sev-type',
+ 'data': {
+ 'sev': 'SevGuestInfo',
+ 'sev-snp': 'SevSnpGuestInfo' } }
+
+
+##
+# @query-sev:
+#
+# Returns information about SEV/SEV-ES/SEV-SNP.
+#
+# If unavailable due to an incompatible configuration the
+# returned @enabled field will be set to 'false' and the
+# state of all other fields is undefined.
+#
+# Returns: @SevInfo
+#
+# Since: 2.12
+#
+# .. qmp-example::
+#
+# -> { "execute": "query-sev" }
+# <- { "return": { "enabled": true, "api-major" : 0, "api-minor" : 0,
+# "build-id" : 0, "policy" : 0, "state" : "running",
+# "handle" : 1 } }
+##
+{ 'command': 'query-sev', 'returns': 'SevInfo' }
+
+##
+# @SevLaunchMeasureInfo:
+#
+# SEV Guest Launch measurement information
+#
+# @data: the measurement value encoded in base64
+#
+# Since: 2.12
+##
+{ 'struct': 'SevLaunchMeasureInfo', 'data': {'data': 'str'} }
+
+##
+# @query-sev-launch-measure:
+#
+# Query the SEV/SEV-ES guest launch information.
+#
+# This is only valid on x86 machines configured with KVM and the
+# 'sev-guest' confidential virtualization object. The launch
+# measurement for SEV-SNP guests is only available within
+# the guest.
+#
+# This will return an error if the launch measurement is
+# unavailable, either due to an invalid guest configuration
+# or if the guest has not reached the required SEV state.
+#
+# Returns: The @SevLaunchMeasureInfo for the guest
+#
+# Since: 2.12
+#
+# .. qmp-example::
+#
+# -> { "execute": "query-sev-launch-measure" }
+# <- { "return": { "data": "4l8LXeNlSPUDlXPJG5966/8%YZ" } }
+##
+{ 'command': 'query-sev-launch-measure', 'returns': 'SevLaunchMeasureInfo' }
+
+##
+# @SevCapability:
+#
+# The struct describes capability for a Secure Encrypted
+# Virtualization feature.
+#
+# @pdh: Platform Diffie-Hellman key (base64 encoded)
+#
+# @cert-chain: PDH certificate chain (base64 encoded)
+#
+# @cpu0-id: Unique ID of CPU0 (base64 encoded) (since 7.1)
+#
+# @cbitpos: C-bit location in page table entry
+#
+# @reduced-phys-bits: Number of physical Address bit reduction when
+# SEV is enabled
+#
+# Since: 2.12
+##
+{ 'struct': 'SevCapability',
+ 'data': { 'pdh': 'str',
+ 'cert-chain': 'str',
+ 'cpu0-id': 'str',
+ 'cbitpos': 'int',
+ 'reduced-phys-bits': 'int'} }
+
+##
+# @query-sev-capabilities:
+#
+# This command is used to get the SEV capabilities, and is only
+# supported on AMD X86 platforms with KVM enabled. If SEV is not
+# available on the platform an error will be returned.
+#
+# Returns: SevCapability objects.
+#
+# Since: 2.12
+#
+# .. qmp-example::
+#
+# -> { "execute": "query-sev-capabilities" }
+# <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
+# "cpu0-id": "2lvmGwo+...61iEinw==",
+# "cbitpos": 47, "reduced-phys-bits": 1}}
+##
+{ 'command': 'query-sev-capabilities', 'returns': 'SevCapability' }
+
+##
+# @sev-inject-launch-secret:
+#
+# This command injects a secret blob into memory of a SEV/SEV-ES guest.
+#
+# This is only valid on x86 machines configured with KVM and the
+# 'sev-guest' confidential virtualization object. SEV-SNP guests
+# do not support launch secret injection
+#
+# This will return an error if launch secret injection is not possible,
+# either due to an invalid guest configuration, or if the guest has not
+# reached the required SEV state.
+#
+# @packet-header: the launch secret packet header encoded in base64
+#
+# @secret: the launch secret data to be injected encoded in base64
+#
+# @gpa: the guest physical address where secret will be injected.
+#
+# Since: 6.0
+##
+{ 'command': 'sev-inject-launch-secret',
+ 'data': { 'packet-header': 'str', 'secret': 'str', '*gpa': 'uint64' } }
+
+##
+# @SevAttestationReport:
+#
+# The struct describes attestation report for a Secure Encrypted
+# Virtualization feature.
+#
+# @data: guest attestation report (base64 encoded)
+#
+# Since: 6.1
+##
+{ 'struct': 'SevAttestationReport',
+ 'data': { 'data': 'str'} }
+
+##
+# @query-sev-attestation-report:
+#
+# This command is used to get the SEV attestation report.
+#
+# This is only valid on x86 machines configured with KVM and the
+# 'sev-guest' confidential virtualization object. The attestation
+# report for SEV-SNP guests is only available within the guest.
+#
+# This will return an error if the attestation report is
+# unavailable, either due to an invalid guest configuration
+# or if the guest has not reached the required SEV state.
+#
+# @mnonce: a random 16 bytes value encoded in base64 (it will be
+# included in report)
+#
+# Returns: SevAttestationReport objects.
+#
+# Since: 6.1
+#
+# .. qmp-example::
+#
+# -> { "execute" : "query-sev-attestation-report",
+# "arguments": { "mnonce": "aaaaaaa" } }
+# <- { "return" : { "data": "aaaaaaaabbbddddd"} }
+##
+{ 'command': 'query-sev-attestation-report',
+ 'data': { 'mnonce': 'str' },
+ 'returns': 'SevAttestationReport' }
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index ae55e437a5..ba4403a924 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -2,290 +2,6 @@
# vim: filetype=python
#
-##
-# @SevState:
-#
-# An enumeration of SEV state information used during @query-sev.
-#
-# @uninit: The guest is uninitialized.
-#
-# @launch-update: The guest is currently being launched; plaintext
-# data and register state is being imported.
-#
-# @launch-secret: The guest is currently being launched; ciphertext
-# data is being imported.
-#
-# @running: The guest is fully launched or migrated in.
-#
-# @send-update: The guest is currently being migrated out to another
-# machine.
-#
-# @receive-update: The guest is currently being migrated from another
-# machine.
-#
-# Since: 2.12
-##
-{ 'enum': 'SevState',
- 'data': ['uninit', 'launch-update', 'launch-secret', 'running',
- 'send-update', 'receive-update' ],
- 'if': 'TARGET_I386' }
-
-##
-# @SevGuestType:
-#
-# An enumeration indicating the type of SEV guest being run.
-#
-# @sev: The guest is a legacy SEV or SEV-ES guest.
-#
-# @sev-snp: The guest is an SEV-SNP guest.
-#
-# Since: 6.2
-##
-{ 'enum': 'SevGuestType',
- 'data': [ 'sev', 'sev-snp' ],
- 'if': 'TARGET_I386' }
-
-##
-# @SevGuestInfo:
-#
-# Information specific to legacy SEV/SEV-ES guests.
-#
-# @policy: SEV policy value
-#
-# @handle: SEV firmware handle
-#
-# Since: 2.12
-##
-{ 'struct': 'SevGuestInfo',
- 'data': { 'policy': 'uint32',
- 'handle': 'uint32' },
- 'if': 'TARGET_I386' }
-
-##
-# @SevSnpGuestInfo:
-#
-# Information specific to SEV-SNP guests.
-#
-# @snp-policy: SEV-SNP policy value
-#
-# Since: 9.1
-##
-{ 'struct': 'SevSnpGuestInfo',
- 'data': { 'snp-policy': 'uint64' },
- 'if': 'TARGET_I386' }
-
-##
-# @SevInfo:
-#
-# Information about Secure Encrypted Virtualization (SEV) support
-#
-# @enabled: true if SEV is active
-#
-# @api-major: SEV API major version
-#
-# @api-minor: SEV API minor version
-#
-# @build-id: SEV FW build id
-#
-# @state: SEV guest state
-#
-# @sev-type: Type of SEV guest being run
-#
-# Since: 2.12
-##
-{ 'union': 'SevInfo',
- 'base': { 'enabled': 'bool',
- 'api-major': 'uint8',
- 'api-minor' : 'uint8',
- 'build-id' : 'uint8',
- 'state' : 'SevState',
- 'sev-type' : 'SevGuestType' },
- 'discriminator': 'sev-type',
- 'data': {
- 'sev': 'SevGuestInfo',
- 'sev-snp': 'SevSnpGuestInfo' },
- 'if': 'TARGET_I386' }
-
-
-##
-# @query-sev:
-#
-# Returns information about SEV/SEV-ES/SEV-SNP.
-#
-# If unavailable due to an incompatible configuration the
-# returned @enabled field will be set to 'false' and the
-# state of all other fields is undefined.
-#
-# Returns: @SevInfo
-#
-# Since: 2.12
-#
-# .. qmp-example::
-#
-# -> { "execute": "query-sev" }
-# <- { "return": { "enabled": true, "api-major" : 0, "api-minor" : 0,
-# "build-id" : 0, "policy" : 0, "state" : "running",
-# "handle" : 1 } }
-##
-{ 'command': 'query-sev', 'returns': 'SevInfo',
- 'if': 'TARGET_I386' }
-
-##
-# @SevLaunchMeasureInfo:
-#
-# SEV Guest Launch measurement information
-#
-# @data: the measurement value encoded in base64
-#
-# Since: 2.12
-##
-{ 'struct': 'SevLaunchMeasureInfo', 'data': {'data': 'str'},
- 'if': 'TARGET_I386' }
-
-##
-# @query-sev-launch-measure:
-#
-# Query the SEV/SEV-ES guest launch information.
-#
-# This is only valid on x86 machines configured with KVM and the
-# 'sev-guest' confidential virtualization object. The launch
-# measurement for SEV-SNP guests is only available within
-# the guest.
-#
-# This will return an error if the launch measurement is
-# unavailable, either due to an invalid guest configuration
-# or if the guest has not reached the required SEV state.
-#
-# Returns: The @SevLaunchMeasureInfo for the guest
-#
-# Since: 2.12
-#
-# .. qmp-example::
-#
-# -> { "execute": "query-sev-launch-measure" }
-# <- { "return": { "data": "4l8LXeNlSPUDlXPJG5966/8%YZ" } }
-##
-{ 'command': 'query-sev-launch-measure', 'returns': 'SevLaunchMeasureInfo',
- 'if': 'TARGET_I386' }
-
-##
-# @SevCapability:
-#
-# The struct describes capability for a Secure Encrypted
-# Virtualization feature.
-#
-# @pdh: Platform Diffie-Hellman key (base64 encoded)
-#
-# @cert-chain: PDH certificate chain (base64 encoded)
-#
-# @cpu0-id: Unique ID of CPU0 (base64 encoded) (since 7.1)
-#
-# @cbitpos: C-bit location in page table entry
-#
-# @reduced-phys-bits: Number of physical Address bit reduction when
-# SEV is enabled
-#
-# Since: 2.12
-##
-{ 'struct': 'SevCapability',
- 'data': { 'pdh': 'str',
- 'cert-chain': 'str',
- 'cpu0-id': 'str',
- 'cbitpos': 'int',
- 'reduced-phys-bits': 'int'},
- 'if': 'TARGET_I386' }
-
-##
-# @query-sev-capabilities:
-#
-# This command is used to get the SEV capabilities, and is only
-# supported on AMD X86 platforms with KVM enabled. If SEV is not
-# available on the platform an error will be returned.
-#
-# Returns: SevCapability objects.
-#
-# Since: 2.12
-#
-# .. qmp-example::
-#
-# -> { "execute": "query-sev-capabilities" }
-# <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
-# "cpu0-id": "2lvmGwo+...61iEinw==",
-# "cbitpos": 47, "reduced-phys-bits": 1}}
-##
-{ 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
- 'if': 'TARGET_I386' }
-
-##
-# @sev-inject-launch-secret:
-#
-# This command injects a secret blob into memory of a SEV/SEV-ES guest.
-#
-# This is only valid on x86 machines configured with KVM and the
-# 'sev-guest' confidential virtualization object. SEV-SNP guests
-# do not support launch secret injection
-#
-# This will return an error if launch secret injection is not possible,
-# either due to an invalid guest configuration, or if the guest has not
-# reached the required SEV state.
-#
-# @packet-header: the launch secret packet header encoded in base64
-#
-# @secret: the launch secret data to be injected encoded in base64
-#
-# @gpa: the guest physical address where secret will be injected.
-#
-# Since: 6.0
-##
-{ 'command': 'sev-inject-launch-secret',
- 'data': { 'packet-header': 'str', 'secret': 'str', '*gpa': 'uint64' },
- 'if': 'TARGET_I386' }
-
-##
-# @SevAttestationReport:
-#
-# The struct describes attestation report for a Secure Encrypted
-# Virtualization feature.
-#
-# @data: guest attestation report (base64 encoded)
-#
-# Since: 6.1
-##
-{ 'struct': 'SevAttestationReport',
- 'data': { 'data': 'str'},
- 'if': 'TARGET_I386' }
-
-##
-# @query-sev-attestation-report:
-#
-# This command is used to get the SEV attestation report.
-#
-# This is only valid on x86 machines configured with KVM and the
-# 'sev-guest' confidential virtualization object. The attestation
-# report for SEV-SNP guests is only available within the guest.
-#
-# This will return an error if the attestation report is
-# unavailable, either due to an invalid guest configuration
-# or if the guest has not reached the required SEV state.
-#
-# @mnonce: a random 16 bytes value encoded in base64 (it will be
-# included in report)
-#
-# Returns: SevAttestationReport objects.
-#
-# Since: 6.1
-#
-# .. qmp-example::
-#
-# -> { "execute" : "query-sev-attestation-report",
-# "arguments": { "mnonce": "aaaaaaa" } }
-# <- { "return" : { "data": "aaaaaaaabbbddddd"} }
-##
-{ 'command': 'query-sev-attestation-report',
- 'data': { 'mnonce': 'str' },
- 'returns': 'SevAttestationReport',
- 'if': 'TARGET_I386' }
-
##
# @GICCapability:
#
diff --git a/stubs/meson.build b/stubs/meson.build
index 9907b54c1e..9922ec7b88 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -78,6 +78,7 @@ if have_system
stub_ss.add(files('win32-kbd-hook.c'))
stub_ss.add(files('xen-hw-stub.c'))
stub_ss.add(files('monitor-i386-rtc.c'))
+ stub_ss.add(files('monitor-i386-sev.c'))
endif
if have_system or have_user
diff --git a/stubs/monitor-i386-sev.c b/stubs/monitor-i386-sev.c
new file mode 100644
index 0000000000..d4f024128c
--- /dev/null
+++ b/stubs/monitor-i386-sev.c
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-misc-i386.h"
+
+SevInfo *qmp_query_sev(Error **errp)
+{
+ error_setg(errp, "SEV is not available in this QEMU");
+ return NULL;
+}
+
+SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
+{
+ error_setg(errp, "SEV is not available in this QEMU");
+ return NULL;
+}
+
+SevCapability *qmp_query_sev_capabilities(Error **errp)
+{
+ error_setg(errp, "SEV is not available in this QEMU");
+ return NULL;
+}
+
+void qmp_sev_inject_launch_secret(const char *packet_header, const char *secret,
+ bool has_gpa, uint64_t gpa, Error **errp)
+{
+ error_setg(errp, "SEV is not available in this QEMU");
+}
+
+SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce,
+ Error **errp)
+{
+ error_setg(errp, "SEV is not available in this QEMU");
+ return NULL;
+}
diff --git a/target/i386/sev-system-stub.c b/target/i386/sev-system-stub.c
index d5bf886e79..7c5c02a565 100644
--- a/target/i386/sev-system-stub.c
+++ b/target/i386/sev-system-stub.c
@@ -14,34 +14,9 @@
#include "qemu/osdep.h"
#include "monitor/monitor.h"
#include "monitor/hmp-target.h"
-#include "qapi/qapi-commands-misc-target.h"
#include "qapi/error.h"
#include "sev.h"
-SevInfo *qmp_query_sev(Error **errp)
-{
- error_setg(errp, "SEV is not available in this QEMU");
- return NULL;
-}
-
-SevLaunchMeasureInfo *qmp_query_sev_launch_measure(Error **errp)
-{
- error_setg(errp, "SEV is not available in this QEMU");
- return NULL;
-}
-
-SevCapability *qmp_query_sev_capabilities(Error **errp)
-{
- error_setg(errp, "SEV is not available in this QEMU");
- return NULL;
-}
-
-void qmp_sev_inject_launch_secret(const char *packet_header, const char *secret,
- bool has_gpa, uint64_t gpa, Error **errp)
-{
- error_setg(errp, "SEV is not available in this QEMU");
-}
-
int sev_encrypt_flash(hwaddr gpa, uint8_t *ptr, uint64_t len, Error **errp)
{
g_assert_not_reached();
@@ -56,13 +31,6 @@ int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
g_assert_not_reached();
}
-SevAttestationReport *qmp_query_sev_attestation_report(const char *mnonce,
- Error **errp)
-{
- error_setg(errp, "SEV is not available in this QEMU");
- return NULL;
-}
-
void hmp_info_sev(Monitor *mon, const QDict *qdict)
{
monitor_printf(mon, "SEV is not available in this QEMU\n");
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 7ee700d6a3..56dd64e659 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -37,7 +37,7 @@
#include "qom/object.h"
#include "monitor/monitor.h"
#include "monitor/hmp-target.h"
-#include "qapi/qapi-commands-misc-target.h"
+#include "qapi/qapi-commands-misc-i386.h"
#include "confidential-guest.h"
#include "hw/i386/pc.h"
#include "system/address-spaces.h"
--
2.49.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 04/10] qapi: expose query-gic-capability command unconditionally
2025-05-08 13:58 [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Daniel P. Berrangé
` (2 preceding siblings ...)
2025-05-08 13:58 ` [PATCH 03/10] qapi: make SEV commands unconditionally available Daniel P. Berrangé
@ 2025-05-08 13:58 ` Daniel P. Berrangé
2025-05-08 13:58 ` [PATCH 05/10] qapi: make SGX commands unconditionally available Daniel P. Berrangé
` (8 subsequent siblings)
12 siblings, 0 replies; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-08 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini,
Markus Armbruster, Pierrick Bouvier, Richard Henderson,
Daniel P. Berrangé
This removes the TARGET_ARM condition from the query-gic-capability
command. This requires providing a QMP command stub for non-ARM targets.
This in turn requires moving the command out of misc-target.json, since
that will trigger symbol poisoning errors when built from target
independent code.
Following the earlier precedent, this creates a misc-arm.json file to
hold this ARM specific command.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
qapi/meson.build | 1 +
qapi/misc-arm.json | 49 +++++++++++++++++++++++++++++++++++++++
qapi/misc-target.json | 44 -----------------------------------
qapi/qapi-schema.json | 1 +
stubs/meson.build | 1 +
stubs/monitor-arm-gic.c | 12 ++++++++++
target/arm/arm-qmp-cmds.c | 2 +-
7 files changed, 65 insertions(+), 45 deletions(-)
create mode 100644 qapi/misc-arm.json
create mode 100644 stubs/monitor-arm-gic.c
diff --git a/qapi/meson.build b/qapi/meson.build
index 3a9bd06104..5e93e6b8cf 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -64,6 +64,7 @@ if have_system
'qdev',
'pci',
'rocker',
+ 'misc-arm',
'misc-i386',
'tpm',
'uefi',
diff --git a/qapi/misc-arm.json b/qapi/misc-arm.json
new file mode 100644
index 0000000000..f5341372f5
--- /dev/null
+++ b/qapi/misc-arm.json
@@ -0,0 +1,49 @@
+# -*- Mode: Python -*-
+# vim: filetype=python
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+##
+# @GICCapability:
+#
+# The struct describes capability for a specific GIC (Generic
+# Interrupt Controller) version. These bits are not only decided by
+# QEMU/KVM software version, but also decided by the hardware that the
+# program is running upon.
+#
+# @version: version of GIC to be described. Currently, only 2 and 3
+# are supported.
+#
+# @emulated: whether current QEMU/hardware supports emulated GIC
+# device in user space.
+#
+# @kernel: whether current QEMU/hardware supports hardware accelerated
+# GIC device in kernel.
+#
+# Since: 2.6
+##
+{ 'struct': 'GICCapability',
+ 'data': { 'version': 'int',
+ 'emulated': 'bool',
+ 'kernel': 'bool' } }
+
+##
+# @query-gic-capabilities:
+#
+# It will return a list of GICCapability objects that describe its
+# capability bits.
+#
+# On non-ARM targets this command will report an error as the GIC
+# technology is not applicable.
+#
+# Returns: a list of GICCapability objects.
+#
+# Since: 2.6
+#
+# .. qmp-example::
+#
+# -> { "execute": "query-gic-capabilities" }
+# <- { "return": [{ "version": 2, "emulated": true, "kernel": false },
+# { "version": 3, "emulated": false, "kernel": true } ] }
+##
+{ 'command': 'query-gic-capabilities', 'returns': ['GICCapability'] }
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index ba4403a924..d36292b332 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -2,50 +2,6 @@
# vim: filetype=python
#
-##
-# @GICCapability:
-#
-# The struct describes capability for a specific GIC (Generic
-# Interrupt Controller) version. These bits are not only decided by
-# QEMU/KVM software version, but also decided by the hardware that the
-# program is running upon.
-#
-# @version: version of GIC to be described. Currently, only 2 and 3
-# are supported.
-#
-# @emulated: whether current QEMU/hardware supports emulated GIC
-# device in user space.
-#
-# @kernel: whether current QEMU/hardware supports hardware accelerated
-# GIC device in kernel.
-#
-# Since: 2.6
-##
-{ 'struct': 'GICCapability',
- 'data': { 'version': 'int',
- 'emulated': 'bool',
- 'kernel': 'bool' },
- 'if': 'TARGET_ARM' }
-
-##
-# @query-gic-capabilities:
-#
-# This command is ARM-only. It will return a list of GICCapability
-# objects that describe its capability bits.
-#
-# Returns: a list of GICCapability objects.
-#
-# Since: 2.6
-#
-# .. qmp-example::
-#
-# -> { "execute": "query-gic-capabilities" }
-# <- { "return": [{ "version": 2, "emulated": true, "kernel": false },
-# { "version": 3, "emulated": false, "kernel": true } ] }
-##
-{ 'command': 'query-gic-capabilities', 'returns': ['GICCapability'],
- 'if': 'TARGET_ARM' }
-
##
# @SGXEPCSection:
#
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 96f6aa4413..e96bff8d38 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -61,6 +61,7 @@
{ 'include': 'replay.json' }
{ 'include': 'yank.json' }
{ 'include': 'misc.json' }
+{ 'include': 'misc-arm.json' }
{ 'include': 'misc-i386.json' }
{ 'include': 'misc-target.json' }
{ 'include': 'audio.json' }
diff --git a/stubs/meson.build b/stubs/meson.build
index 9922ec7b88..07e9d3799a 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -77,6 +77,7 @@ if have_system
stub_ss.add(files('target-monitor-defs.c'))
stub_ss.add(files('win32-kbd-hook.c'))
stub_ss.add(files('xen-hw-stub.c'))
+ stub_ss.add(files('monitor-arm-gic.c'))
stub_ss.add(files('monitor-i386-rtc.c'))
stub_ss.add(files('monitor-i386-sev.c'))
endif
diff --git a/stubs/monitor-arm-gic.c b/stubs/monitor-arm-gic.c
new file mode 100644
index 0000000000..b3429243ef
--- /dev/null
+++ b/stubs/monitor-arm-gic.c
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-misc-arm.h"
+
+
+GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
+{
+ error_setg(errp, "GIC hardware is not available on this target");
+ return NULL;
+}
diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c
index a1a944adb4..ef18c867ca 100644
--- a/target/arm/arm-qmp-cmds.c
+++ b/target/arm/arm-qmp-cmds.c
@@ -27,7 +27,7 @@
#include "qapi/visitor.h"
#include "qapi/qobject-input-visitor.h"
#include "qapi/qapi-commands-machine-target.h"
-#include "qapi/qapi-commands-misc-target.h"
+#include "qapi/qapi-commands-misc-arm.h"
#include "qobject/qdict.h"
#include "qom/qom-qobject.h"
--
2.49.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 05/10] qapi: make SGX commands unconditionally available
2025-05-08 13:58 [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Daniel P. Berrangé
` (3 preceding siblings ...)
2025-05-08 13:58 ` [PATCH 04/10] qapi: expose query-gic-capability command unconditionally Daniel P. Berrangé
@ 2025-05-08 13:58 ` Daniel P. Berrangé
2025-05-08 13:58 ` [PATCH 06/10] qapi: make Xen event " Daniel P. Berrangé
` (7 subsequent siblings)
12 siblings, 0 replies; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-08 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini,
Markus Armbruster, Pierrick Bouvier, Richard Henderson,
Daniel P. Berrangé
This removes the TARGET_I386 condition from the SGX confidential
virtualization commands, moving them to the recently introduced
misc-i386.json QAPI file, given they are inherantly i386 specific
commands.
Observe a pre-existing bug that the "SGXEPCSection" struct lacked
a TARGET_I386 condition, despite its only usage being behind a
TARGET_I386 condition.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hw/i386/sgx-stub.c | 13 -------
hw/i386/sgx.c | 2 +-
qapi/misc-i386.json | 77 +++++++++++++++++++++++++++++++++++++++
qapi/misc-target.json | 78 ----------------------------------------
stubs/meson.build | 1 +
stubs/monitor-i386-sgx.c | 17 +++++++++
6 files changed, 96 insertions(+), 92 deletions(-)
create mode 100644 stubs/monitor-i386-sgx.c
diff --git a/hw/i386/sgx-stub.c b/hw/i386/sgx-stub.c
index 38ff75e9f3..10ffcdd154 100644
--- a/hw/i386/sgx-stub.c
+++ b/hw/i386/sgx-stub.c
@@ -4,24 +4,11 @@
#include "hw/i386/pc.h"
#include "hw/i386/sgx-epc.h"
#include "qapi/error.h"
-#include "qapi/qapi-commands-misc-target.h"
void sgx_epc_build_srat(GArray *table_data)
{
}
-SGXInfo *qmp_query_sgx(Error **errp)
-{
- error_setg(errp, "SGX support is not compiled in");
- return NULL;
-}
-
-SGXInfo *qmp_query_sgx_capabilities(Error **errp)
-{
- error_setg(errp, "SGX support is not compiled in");
- return NULL;
-}
-
void hmp_info_sgx(Monitor *mon, const QDict *qdict)
{
monitor_printf(mon, "SGX is not available in this QEMU\n");
diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
index 5685c4fb80..2b10a8d144 100644
--- a/hw/i386/sgx.c
+++ b/hw/i386/sgx.c
@@ -19,7 +19,7 @@
#include "monitor/hmp-target.h"
#include "qapi/error.h"
#include "qemu/error-report.h"
-#include "qapi/qapi-commands-misc-target.h"
+#include "qapi/qapi-commands-misc-i386.h"
#include "system/address-spaces.h"
#include "system/hw_accel.h"
#include "system/reset.h"
diff --git a/qapi/misc-i386.json b/qapi/misc-i386.json
index de0e4ab67f..0ddc297ccb 100644
--- a/qapi/misc-i386.json
+++ b/qapi/misc-i386.json
@@ -293,3 +293,80 @@
{ 'command': 'query-sev-attestation-report',
'data': { 'mnonce': 'str' },
'returns': 'SevAttestationReport' }
+
+##
+# @SGXEPCSection:
+#
+# Information about intel SGX EPC section info
+#
+# @node: the numa node
+#
+# @size: the size of EPC section
+#
+# Since: 7.0
+##
+{ 'struct': 'SGXEPCSection',
+ 'data': { 'node': 'int',
+ 'size': 'uint64'}}
+
+##
+# @SGXInfo:
+#
+# Information about intel Safe Guard eXtension (SGX) support
+#
+# @sgx: true if SGX is supported
+#
+# @sgx1: true if SGX1 is supported
+#
+# @sgx2: true if SGX2 is supported
+#
+# @flc: true if FLC is supported
+#
+# @sections: The EPC sections info for guest (Since: 7.0)
+#
+# Since: 6.2
+##
+{ 'struct': 'SGXInfo',
+ 'data': { 'sgx': 'bool',
+ 'sgx1': 'bool',
+ 'sgx2': 'bool',
+ 'flc': 'bool',
+ 'sections': ['SGXEPCSection']} }
+
+##
+# @query-sgx:
+#
+# Returns information about SGX
+#
+# Returns: @SGXInfo
+#
+# Since: 6.2
+#
+# .. qmp-example::
+#
+# -> { "execute": "query-sgx" }
+# <- { "return": { "sgx": true, "sgx1" : true, "sgx2" : true,
+# "flc": true,
+# "sections": [{"node": 0, "size": 67108864},
+# {"node": 1, "size": 29360128}]} }
+##
+{ 'command': 'query-sgx', 'returns': 'SGXInfo' }
+
+##
+# @query-sgx-capabilities:
+#
+# Returns information from host SGX capabilities
+#
+# Returns: @SGXInfo
+#
+# Since: 6.2
+#
+# .. qmp-example::
+#
+# -> { "execute": "query-sgx-capabilities" }
+# <- { "return": { "sgx": true, "sgx1" : true, "sgx2" : true,
+# "flc": true,
+# "section" : [{"node": 0, "size": 67108864},
+# {"node": 1, "size": 29360128}]} }
+##
+{ 'command': 'query-sgx-capabilities', 'returns': 'SGXInfo' }
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index d36292b332..d9368a1f21 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -2,84 +2,6 @@
# vim: filetype=python
#
-##
-# @SGXEPCSection:
-#
-# Information about intel SGX EPC section info
-#
-# @node: the numa node
-#
-# @size: the size of EPC section
-#
-# Since: 7.0
-##
-{ 'struct': 'SGXEPCSection',
- 'data': { 'node': 'int',
- 'size': 'uint64'}}
-
-##
-# @SGXInfo:
-#
-# Information about intel Safe Guard eXtension (SGX) support
-#
-# @sgx: true if SGX is supported
-#
-# @sgx1: true if SGX1 is supported
-#
-# @sgx2: true if SGX2 is supported
-#
-# @flc: true if FLC is supported
-#
-# @sections: The EPC sections info for guest (Since: 7.0)
-#
-# Since: 6.2
-##
-{ 'struct': 'SGXInfo',
- 'data': { 'sgx': 'bool',
- 'sgx1': 'bool',
- 'sgx2': 'bool',
- 'flc': 'bool',
- 'sections': ['SGXEPCSection']},
- 'if': 'TARGET_I386' }
-
-##
-# @query-sgx:
-#
-# Returns information about SGX
-#
-# Returns: @SGXInfo
-#
-# Since: 6.2
-#
-# .. qmp-example::
-#
-# -> { "execute": "query-sgx" }
-# <- { "return": { "sgx": true, "sgx1" : true, "sgx2" : true,
-# "flc": true,
-# "sections": [{"node": 0, "size": 67108864},
-# {"node": 1, "size": 29360128}]} }
-##
-{ 'command': 'query-sgx', 'returns': 'SGXInfo', 'if': 'TARGET_I386' }
-
-##
-# @query-sgx-capabilities:
-#
-# Returns information from host SGX capabilities
-#
-# Returns: @SGXInfo
-#
-# Since: 6.2
-#
-# .. qmp-example::
-#
-# -> { "execute": "query-sgx-capabilities" }
-# <- { "return": { "sgx": true, "sgx1" : true, "sgx2" : true,
-# "flc": true,
-# "section" : [{"node": 0, "size": 67108864},
-# {"node": 1, "size": 29360128}]} }
-##
-{ 'command': 'query-sgx-capabilities', 'returns': 'SGXInfo', 'if': 'TARGET_I386' }
-
##
# @EvtchnPortType:
diff --git a/stubs/meson.build b/stubs/meson.build
index 07e9d3799a..f2eb488018 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -80,6 +80,7 @@ if have_system
stub_ss.add(files('monitor-arm-gic.c'))
stub_ss.add(files('monitor-i386-rtc.c'))
stub_ss.add(files('monitor-i386-sev.c'))
+ stub_ss.add(files('monitor-i386-sgx.c'))
endif
if have_system or have_user
diff --git a/stubs/monitor-i386-sgx.c b/stubs/monitor-i386-sgx.c
new file mode 100644
index 0000000000..55a4f194f3
--- /dev/null
+++ b/stubs/monitor-i386-sgx.c
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-misc-i386.h"
+
+SGXInfo *qmp_query_sgx(Error **errp)
+{
+ error_setg(errp, "SGX support is not compiled in");
+ return NULL;
+}
+
+SGXInfo *qmp_query_sgx_capabilities(Error **errp)
+{
+ error_setg(errp, "SGX support is not compiled in");
+ return NULL;
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 06/10] qapi: make Xen event commands unconditionally available
2025-05-08 13:58 [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Daniel P. Berrangé
` (4 preceding siblings ...)
2025-05-08 13:58 ` [PATCH 05/10] qapi: make SGX commands unconditionally available Daniel P. Berrangé
@ 2025-05-08 13:58 ` Daniel P. Berrangé
2025-05-08 15:01 ` Philippe Mathieu-Daudé
2025-05-08 13:58 ` [PATCH 07/10] qapi: remove the misc-target.json file Daniel P. Berrangé
` (6 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-08 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini,
Markus Armbruster, Pierrick Bouvier, Richard Henderson,
Daniel P. Berrangé
This removes the TARGET_I386 condition from the Xen event channel
commands, moving them to the recently introduced misc-i386.json
QAPI file, given they are inherantly i386 specific commands.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hw/i386/kvm/xen-stubs.c | 13 -----
hw/i386/kvm/xen_evtchn.c | 2 +-
qapi/misc-i386.json | 107 +++++++++++++++++++++++++++++++++++++
qapi/misc-target.json | 110 ---------------------------------------
stubs/meson.build | 1 +
stubs/monitor-i386-xen.c | 16 ++++++
6 files changed, 125 insertions(+), 124 deletions(-)
create mode 100644 stubs/monitor-i386-xen.c
diff --git a/hw/i386/kvm/xen-stubs.c b/hw/i386/kvm/xen-stubs.c
index d03131e686..ce73119ee7 100644
--- a/hw/i386/kvm/xen-stubs.c
+++ b/hw/i386/kvm/xen-stubs.c
@@ -12,7 +12,6 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
-#include "qapi/qapi-commands-misc-target.h"
#include "xen_evtchn.h"
#include "xen_primary_console.h"
@@ -38,15 +37,3 @@ void xen_primary_console_create(void)
void xen_primary_console_set_be_port(uint16_t port)
{
}
-#ifdef TARGET_I386
-EvtchnInfoList *qmp_xen_event_list(Error **errp)
-{
- error_setg(errp, "Xen event channel emulation not enabled");
- return NULL;
-}
-
-void qmp_xen_event_inject(uint32_t port, Error **errp)
-{
- error_setg(errp, "Xen event channel emulation not enabled");
-}
-#endif
diff --git a/hw/i386/kvm/xen_evtchn.c b/hw/i386/kvm/xen_evtchn.c
index b5190549a8..dd566c4967 100644
--- a/hw/i386/kvm/xen_evtchn.c
+++ b/hw/i386/kvm/xen_evtchn.c
@@ -19,7 +19,7 @@
#include "monitor/monitor.h"
#include "monitor/hmp.h"
#include "qapi/error.h"
-#include "qapi/qapi-commands-misc-target.h"
+#include "qapi/qapi-commands-misc-i386.h"
#include "qobject/qdict.h"
#include "qom/object.h"
#include "exec/target_page.h"
diff --git a/qapi/misc-i386.json b/qapi/misc-i386.json
index 0ddc297ccb..3d3f373a6b 100644
--- a/qapi/misc-i386.json
+++ b/qapi/misc-i386.json
@@ -370,3 +370,110 @@
# {"node": 1, "size": 29360128}]} }
##
{ 'command': 'query-sgx-capabilities', 'returns': 'SGXInfo' }
+
+##
+# @EvtchnPortType:
+#
+# An enumeration of Xen event channel port types.
+#
+# @closed: The port is unused.
+#
+# @unbound: The port is allocated and ready to be bound.
+#
+# @interdomain: The port is connected as an interdomain interrupt.
+#
+# @pirq: The port is bound to a physical IRQ (PIRQ).
+#
+# @virq: The port is bound to a virtual IRQ (VIRQ).
+#
+# @ipi: The post is an inter-processor interrupt (IPI).
+#
+# Since: 8.0
+##
+{ 'enum': 'EvtchnPortType',
+ 'data': ['closed', 'unbound', 'interdomain', 'pirq', 'virq', 'ipi'] }
+
+##
+# @EvtchnInfo:
+#
+# Information about a Xen event channel port
+#
+# @port: the port number
+#
+# @vcpu: target vCPU for this port
+#
+# @type: the port type
+#
+# @remote-domain: remote domain for interdomain ports
+#
+# @target: remote port ID, or virq/pirq number
+#
+# @pending: port is currently active pending delivery
+#
+# @masked: port is masked
+#
+# Since: 8.0
+##
+{ 'struct': 'EvtchnInfo',
+ 'data': {'port': 'uint16',
+ 'vcpu': 'uint32',
+ 'type': 'EvtchnPortType',
+ 'remote-domain': 'str',
+ 'target': 'uint16',
+ 'pending': 'bool',
+ 'masked': 'bool'} }
+
+
+##
+# @xen-event-list:
+#
+# Query the Xen event channels opened by the guest.
+#
+# Returns: list of open event channel ports.
+#
+# Since: 8.0
+#
+# .. qmp-example::
+#
+# -> { "execute": "xen-event-list" }
+# <- { "return": [
+# {
+# "pending": false,
+# "port": 1,
+# "vcpu": 1,
+# "remote-domain": "qemu",
+# "masked": false,
+# "type": "interdomain",
+# "target": 1
+# },
+# {
+# "pending": false,
+# "port": 2,
+# "vcpu": 0,
+# "remote-domain": "",
+# "masked": false,
+# "type": "virq",
+# "target": 0
+# }
+# ]
+# }
+##
+{ 'command': 'xen-event-list',
+ 'returns': ['EvtchnInfo'] }
+
+##
+# @xen-event-inject:
+#
+# Inject a Xen event channel port (interrupt) to the guest.
+#
+# @port: The port number
+#
+# Since: 8.0
+#
+# .. qmp-example::
+#
+# -> { "execute": "xen-event-inject", "arguments": { "port": 1 } }
+# <- { "return": { } }
+##
+{ 'command': 'xen-event-inject',
+ 'data': { 'port': 'uint32' } }
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index d9368a1f21..6c2f4e71d7 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -3,113 +3,3 @@
#
-##
-# @EvtchnPortType:
-#
-# An enumeration of Xen event channel port types.
-#
-# @closed: The port is unused.
-#
-# @unbound: The port is allocated and ready to be bound.
-#
-# @interdomain: The port is connected as an interdomain interrupt.
-#
-# @pirq: The port is bound to a physical IRQ (PIRQ).
-#
-# @virq: The port is bound to a virtual IRQ (VIRQ).
-#
-# @ipi: The post is an inter-processor interrupt (IPI).
-#
-# Since: 8.0
-##
-{ 'enum': 'EvtchnPortType',
- 'data': ['closed', 'unbound', 'interdomain', 'pirq', 'virq', 'ipi'],
- 'if': 'TARGET_I386' }
-
-##
-# @EvtchnInfo:
-#
-# Information about a Xen event channel port
-#
-# @port: the port number
-#
-# @vcpu: target vCPU for this port
-#
-# @type: the port type
-#
-# @remote-domain: remote domain for interdomain ports
-#
-# @target: remote port ID, or virq/pirq number
-#
-# @pending: port is currently active pending delivery
-#
-# @masked: port is masked
-#
-# Since: 8.0
-##
-{ 'struct': 'EvtchnInfo',
- 'data': {'port': 'uint16',
- 'vcpu': 'uint32',
- 'type': 'EvtchnPortType',
- 'remote-domain': 'str',
- 'target': 'uint16',
- 'pending': 'bool',
- 'masked': 'bool'},
- 'if': 'TARGET_I386' }
-
-
-##
-# @xen-event-list:
-#
-# Query the Xen event channels opened by the guest.
-#
-# Returns: list of open event channel ports.
-#
-# Since: 8.0
-#
-# .. qmp-example::
-#
-# -> { "execute": "xen-event-list" }
-# <- { "return": [
-# {
-# "pending": false,
-# "port": 1,
-# "vcpu": 1,
-# "remote-domain": "qemu",
-# "masked": false,
-# "type": "interdomain",
-# "target": 1
-# },
-# {
-# "pending": false,
-# "port": 2,
-# "vcpu": 0,
-# "remote-domain": "",
-# "masked": false,
-# "type": "virq",
-# "target": 0
-# }
-# ]
-# }
-##
-{ 'command': 'xen-event-list',
- 'returns': ['EvtchnInfo'],
- 'if': 'TARGET_I386' }
-
-##
-# @xen-event-inject:
-#
-# Inject a Xen event channel port (interrupt) to the guest.
-#
-# @port: The port number
-#
-# Since: 8.0
-#
-# .. qmp-example::
-#
-# -> { "execute": "xen-event-inject", "arguments": { "port": 1 } }
-# <- { "return": { } }
-##
-{ 'command': 'xen-event-inject',
- 'data': { 'port': 'uint32' },
- 'if': 'TARGET_I386' }
diff --git a/stubs/meson.build b/stubs/meson.build
index f2eb488018..0ef11976a2 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -81,6 +81,7 @@ if have_system
stub_ss.add(files('monitor-i386-rtc.c'))
stub_ss.add(files('monitor-i386-sev.c'))
stub_ss.add(files('monitor-i386-sgx.c'))
+ stub_ss.add(files('monitor-i386-xen.c'))
endif
if have_system or have_user
diff --git a/stubs/monitor-i386-xen.c b/stubs/monitor-i386-xen.c
new file mode 100644
index 0000000000..95b826f979
--- /dev/null
+++ b/stubs/monitor-i386-xen.c
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-misc-i386.h"
+
+EvtchnInfoList *qmp_xen_event_list(Error **errp)
+{
+ error_setg(errp, "Xen event channel emulation not enabled");
+ return NULL;
+}
+
+void qmp_xen_event_inject(uint32_t port, Error **errp)
+{
+ error_setg(errp, "Xen event channel emulation not enabled");
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 06/10] qapi: make Xen event commands unconditionally available
2025-05-08 13:58 ` [PATCH 06/10] qapi: make Xen event " Daniel P. Berrangé
@ 2025-05-08 15:01 ` Philippe Mathieu-Daudé
2025-05-08 17:48 ` David Woodhouse
0 siblings, 1 reply; 41+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-05-08 15:01 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Peter Maydell, Paolo Bonzini, Markus Armbruster, Pierrick Bouvier,
Richard Henderson, David Woodhouse, Zhao Liu
Cc'ing Zhao
On 8/5/25 15:58, Daniel P. Berrangé wrote:
> This removes the TARGET_I386 condition from the Xen event channel
> commands, moving them to the recently introduced misc-i386.json
> QAPI file, given they are inherantly i386 specific commands.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> hw/i386/kvm/xen-stubs.c | 13 -----
> hw/i386/kvm/xen_evtchn.c | 2 +-
> qapi/misc-i386.json | 107 +++++++++++++++++++++++++++++++++++++
> qapi/misc-target.json | 110 ---------------------------------------
> stubs/meson.build | 1 +
> stubs/monitor-i386-xen.c | 16 ++++++
> 6 files changed, 125 insertions(+), 124 deletions(-)
> create mode 100644 stubs/monitor-i386-xen.c
>
> diff --git a/hw/i386/kvm/xen-stubs.c b/hw/i386/kvm/xen-stubs.c
> index d03131e686..ce73119ee7 100644
> --- a/hw/i386/kvm/xen-stubs.c
> +++ b/hw/i386/kvm/xen-stubs.c
> @@ -12,7 +12,6 @@
> #include "qemu/osdep.h"
>
> #include "qapi/error.h"
> -#include "qapi/qapi-commands-misc-target.h"
>
> #include "xen_evtchn.h"
> #include "xen_primary_console.h"
> @@ -38,15 +37,3 @@ void xen_primary_console_create(void)
> void xen_primary_console_set_be_port(uint16_t port)
> {
> }
> -#ifdef TARGET_I386
> -EvtchnInfoList *qmp_xen_event_list(Error **errp)
> -{
> - error_setg(errp, "Xen event channel emulation not enabled");
> - return NULL;
> -}
> -
> -void qmp_xen_event_inject(uint32_t port, Error **errp)
> -{
> - error_setg(errp, "Xen event channel emulation not enabled");
> -}
> -#endif
> diff --git a/hw/i386/kvm/xen_evtchn.c b/hw/i386/kvm/xen_evtchn.c
> index b5190549a8..dd566c4967 100644
> --- a/hw/i386/kvm/xen_evtchn.c
> +++ b/hw/i386/kvm/xen_evtchn.c
> @@ -19,7 +19,7 @@
> #include "monitor/monitor.h"
> #include "monitor/hmp.h"
> #include "qapi/error.h"
> -#include "qapi/qapi-commands-misc-target.h"
> +#include "qapi/qapi-commands-misc-i386.h"
> #include "qobject/qdict.h"
> #include "qom/object.h"
> #include "exec/target_page.h"
> diff --git a/qapi/misc-i386.json b/qapi/misc-i386.json
> index 0ddc297ccb..3d3f373a6b 100644
> --- a/qapi/misc-i386.json
> +++ b/qapi/misc-i386.json
> @@ -370,3 +370,110 @@
> # {"node": 1, "size": 29360128}]} }
> ##
> { 'command': 'query-sgx-capabilities', 'returns': 'SGXInfo' }
> +
> +##
> +# @EvtchnPortType:
> +#
> +# An enumeration of Xen event channel port types.
> +#
> +# @closed: The port is unused.
> +#
> +# @unbound: The port is allocated and ready to be bound.
> +#
> +# @interdomain: The port is connected as an interdomain interrupt.
> +#
> +# @pirq: The port is bound to a physical IRQ (PIRQ).
> +#
> +# @virq: The port is bound to a virtual IRQ (VIRQ).
> +#
> +# @ipi: The post is an inter-processor interrupt (IPI).
> +#
> +# Since: 8.0
> +##
> +{ 'enum': 'EvtchnPortType',
> + 'data': ['closed', 'unbound', 'interdomain', 'pirq', 'virq', 'ipi'] }
> +
> +##
> +# @EvtchnInfo:
> +#
> +# Information about a Xen event channel port
> +#
> +# @port: the port number
> +#
> +# @vcpu: target vCPU for this port
> +#
> +# @type: the port type
> +#
> +# @remote-domain: remote domain for interdomain ports
> +#
> +# @target: remote port ID, or virq/pirq number
> +#
> +# @pending: port is currently active pending delivery
> +#
> +# @masked: port is masked
> +#
> +# Since: 8.0
> +##
> +{ 'struct': 'EvtchnInfo',
> + 'data': {'port': 'uint16',
> + 'vcpu': 'uint32',
> + 'type': 'EvtchnPortType',
> + 'remote-domain': 'str',
> + 'target': 'uint16',
> + 'pending': 'bool',
> + 'masked': 'bool'} }
> +
> +
> +##
> +# @xen-event-list:
> +#
> +# Query the Xen event channels opened by the guest.
> +#
> +# Returns: list of open event channel ports.
> +#
> +# Since: 8.0
> +#
> +# .. qmp-example::
> +#
> +# -> { "execute": "xen-event-list" }
> +# <- { "return": [
> +# {
> +# "pending": false,
> +# "port": 1,
> +# "vcpu": 1,
> +# "remote-domain": "qemu",
> +# "masked": false,
> +# "type": "interdomain",
> +# "target": 1
> +# },
> +# {
> +# "pending": false,
> +# "port": 2,
> +# "vcpu": 0,
> +# "remote-domain": "",
> +# "masked": false,
> +# "type": "virq",
> +# "target": 0
> +# }
> +# ]
> +# }
> +##
> +{ 'command': 'xen-event-list',
> + 'returns': ['EvtchnInfo'] }
> +
> +##
> +# @xen-event-inject:
> +#
> +# Inject a Xen event channel port (interrupt) to the guest.
> +#
> +# @port: The port number
> +#
> +# Since: 8.0
> +#
> +# .. qmp-example::
> +#
> +# -> { "execute": "xen-event-inject", "arguments": { "port": 1 } }
> +# <- { "return": { } }
> +##
> +{ 'command': 'xen-event-inject',
> + 'data': { 'port': 'uint32' } }
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index d9368a1f21..6c2f4e71d7 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -3,113 +3,3 @@
> #
>
>
> -##
> -# @EvtchnPortType:
> -#
> -# An enumeration of Xen event channel port types.
> -#
> -# @closed: The port is unused.
> -#
> -# @unbound: The port is allocated and ready to be bound.
> -#
> -# @interdomain: The port is connected as an interdomain interrupt.
> -#
> -# @pirq: The port is bound to a physical IRQ (PIRQ).
> -#
> -# @virq: The port is bound to a virtual IRQ (VIRQ).
> -#
> -# @ipi: The post is an inter-processor interrupt (IPI).
> -#
> -# Since: 8.0
> -##
> -{ 'enum': 'EvtchnPortType',
> - 'data': ['closed', 'unbound', 'interdomain', 'pirq', 'virq', 'ipi'],
> - 'if': 'TARGET_I386' }
Interesting, this ought to be "all {TARGET_I386, CONFIG_KVM}".
> -
> -##
> -# @EvtchnInfo:
> -#
> -# Information about a Xen event channel port
> -#
> -# @port: the port number
> -#
> -# @vcpu: target vCPU for this port
> -#
> -# @type: the port type
> -#
> -# @remote-domain: remote domain for interdomain ports
> -#
> -# @target: remote port ID, or virq/pirq number
> -#
> -# @pending: port is currently active pending delivery
> -#
> -# @masked: port is masked
> -#
> -# Since: 8.0
> -##
> -{ 'struct': 'EvtchnInfo',
> - 'data': {'port': 'uint16',
> - 'vcpu': 'uint32',
> - 'type': 'EvtchnPortType',
> - 'remote-domain': 'str',
> - 'target': 'uint16',
> - 'pending': 'bool',
> - 'masked': 'bool'},
> - 'if': 'TARGET_I386' }
> -
> -
> -##
> -# @xen-event-list:
> -#
> -# Query the Xen event channels opened by the guest.
> -#
> -# Returns: list of open event channel ports.
> -#
> -# Since: 8.0
> -#
> -# .. qmp-example::
> -#
> -# -> { "execute": "xen-event-list" }
> -# <- { "return": [
> -# {
> -# "pending": false,
> -# "port": 1,
> -# "vcpu": 1,
> -# "remote-domain": "qemu",
> -# "masked": false,
> -# "type": "interdomain",
> -# "target": 1
> -# },
> -# {
> -# "pending": false,
> -# "port": 2,
> -# "vcpu": 0,
> -# "remote-domain": "",
> -# "masked": false,
> -# "type": "virq",
> -# "target": 0
> -# }
> -# ]
> -# }
> -##
> -{ 'command': 'xen-event-list',
> - 'returns': ['EvtchnInfo'],
> - 'if': 'TARGET_I386' }
> -
> -##
> -# @xen-event-inject:
> -#
> -# Inject a Xen event channel port (interrupt) to the guest.
> -#
> -# @port: The port number
> -#
> -# Since: 8.0
> -#
> -# .. qmp-example::
> -#
> -# -> { "execute": "xen-event-inject", "arguments": { "port": 1 } }
> -# <- { "return": { } }
> -##
> -{ 'command': 'xen-event-inject',
> - 'data': { 'port': 'uint32' },
> - 'if': 'TARGET_I386' }
> diff --git a/stubs/meson.build b/stubs/meson.build
> index f2eb488018..0ef11976a2 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -81,6 +81,7 @@ if have_system
> stub_ss.add(files('monitor-i386-rtc.c'))
> stub_ss.add(files('monitor-i386-sev.c'))
> stub_ss.add(files('monitor-i386-sgx.c'))
> + stub_ss.add(files('monitor-i386-xen.c'))
> endif
>
> if have_system or have_user
> diff --git a/stubs/monitor-i386-xen.c b/stubs/monitor-i386-xen.c
> new file mode 100644
> index 0000000000..95b826f979
> --- /dev/null
> +++ b/stubs/monitor-i386-xen.c
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-misc-i386.h"
> +
> +EvtchnInfoList *qmp_xen_event_list(Error **errp)
> +{
> + error_setg(errp, "Xen event channel emulation not enabled");
> + return NULL;
> +}
> +
> +void qmp_xen_event_inject(uint32_t port, Error **errp)
> +{
> + error_setg(errp, "Xen event channel emulation not enabled");
> +}
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 06/10] qapi: make Xen event commands unconditionally available
2025-05-08 15:01 ` Philippe Mathieu-Daudé
@ 2025-05-08 17:48 ` David Woodhouse
2025-05-08 17:53 ` Daniel P. Berrangé
0 siblings, 1 reply; 41+ messages in thread
From: David Woodhouse @ 2025-05-08 17:48 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Daniel P. Berrangé, qemu-devel
Cc: Peter Maydell, Paolo Bonzini, Markus Armbruster, Pierrick Bouvier,
Richard Henderson, Zhao Liu
[-- Attachment #1: Type: text/plain, Size: 1711 bytes --]
On Thu, 2025-05-08 at 17:01 +0200, Philippe Mathieu-Daudé wrote:
> Cc'ing Zhao
>
> On 8/5/25 15:58, Daniel P. Berrangé wrote:
Hm, what mailer does that? Does it mean August 5th or May 8th? Even in
the original definition of RFC821 they knew not to use dates in those
forms :)
> > This removes the TARGET_I386 condition from the Xen event channel
> > commands, moving them to the recently introduced misc-i386.json
> > QAPI file, given they are inherantly i386 specific commands.
'inherently'
>
> > ---
> > -##
> > -# @EvtchnPortType:
> > -#
> > -# An enumeration of Xen event channel port types.
> > -#
> > -# @closed: The port is unused.
> > -#
> > -# @unbound: The port is allocated and ready to be bound.
> > -#
> > -# @interdomain: The port is connected as an interdomain interrupt.
> > -#
> > -# @pirq: The port is bound to a physical IRQ (PIRQ).
> > -#
> > -# @virq: The port is bound to a virtual IRQ (VIRQ).
> > -#
> > -# @ipi: The post is an inter-processor interrupt (IPI).
> > -#
> > -# Since: 8.0
> > -##
> > -{ 'enum': 'EvtchnPortType',
> > - 'data': ['closed', 'unbound', 'interdomain', 'pirq', 'virq', 'ipi'],
> > - 'if': 'TARGET_I386' }
>
> Interesting, this ought to be "all {TARGET_I386, CONFIG_KVM}".
Isn't it CONFIG_XEN_EMU? But ISTR it wasn't possible to depend on those
target-specific options in generic code? I forget the precise details.
So we ended up just ensuring the stubs were provided in the right
cases.
This patch moves those stubs from hw/i386/kvm/xen-stubs.c which was
built if !CONFIG_XEN_EMU, and moves them elsewhere if !CONFIG_KVM? So
at first glance I think it might fail for KVM && !XEN_EMU builds... ?
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 06/10] qapi: make Xen event commands unconditionally available
2025-05-08 17:48 ` David Woodhouse
@ 2025-05-08 17:53 ` Daniel P. Berrangé
2025-05-08 19:08 ` David Woodhouse
0 siblings, 1 reply; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-08 17:53 UTC (permalink / raw)
To: David Woodhouse
Cc: Philippe Mathieu-Daudé, qemu-devel, Peter Maydell,
Paolo Bonzini, Markus Armbruster, Pierrick Bouvier,
Richard Henderson, Zhao Liu
On Thu, May 08, 2025 at 10:48:22AM -0700, David Woodhouse wrote:
> On Thu, 2025-05-08 at 17:01 +0200, Philippe Mathieu-Daudé wrote:
> > Cc'ing Zhao
> >
> > On 8/5/25 15:58, Daniel P. Berrangé wrote:
>
> Hm, what mailer does that? Does it mean August 5th or May 8th? Even in
> the original definition of RFC821 they knew not to use dates in those
> forms :)
>
> > > This removes the TARGET_I386 condition from the Xen event channel
> > > commands, moving them to the recently introduced misc-i386.json
> > > QAPI file, given they are inherantly i386 specific commands.
>
> 'inherently'
>
> >
> > > ---
> > > -##
> > > -# @EvtchnPortType:
> > > -#
> > > -# An enumeration of Xen event channel port types.
> > > -#
> > > -# @closed: The port is unused.
> > > -#
> > > -# @unbound: The port is allocated and ready to be bound.
> > > -#
> > > -# @interdomain: The port is connected as an interdomain interrupt.
> > > -#
> > > -# @pirq: The port is bound to a physical IRQ (PIRQ).
> > > -#
> > > -# @virq: The port is bound to a virtual IRQ (VIRQ).
> > > -#
> > > -# @ipi: The post is an inter-processor interrupt (IPI).
> > > -#
> > > -# Since: 8.0
> > > -##
> > > -{ 'enum': 'EvtchnPortType',
> > > - 'data': ['closed', 'unbound', 'interdomain', 'pirq', 'virq', 'ipi'],
> > > - 'if': 'TARGET_I386' }
> >
> > Interesting, this ought to be "all {TARGET_I386, CONFIG_KVM}".
>
> Isn't it CONFIG_XEN_EMU? But ISTR it wasn't possible to depend on those
> target-specific options in generic code? I forget the precise details.
>
> So we ended up just ensuring the stubs were provided in the right
> cases.
>
> This patch moves those stubs from hw/i386/kvm/xen-stubs.c which was
> built if !CONFIG_XEN_EMU, and moves them elsewhere if !CONFIG_KVM? So
> at first glance I think it might fail for KVM && !XEN_EMU builds... ?
The files in the top level 'stubs/' directory get built into
libqemuutil.a, and thus those stubs are available to every
system binary, in every configuration scenario.
If the real impl of these methods is present in any binary, the stubs
file impl should get discarded by the linker automatically, avoiding
a duplicate symbols problem.
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] 41+ messages in thread* Re: [PATCH 06/10] qapi: make Xen event commands unconditionally available
2025-05-08 17:53 ` Daniel P. Berrangé
@ 2025-05-08 19:08 ` David Woodhouse
0 siblings, 0 replies; 41+ messages in thread
From: David Woodhouse @ 2025-05-08 19:08 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Philippe Mathieu-Daudé, qemu-devel, Peter Maydell,
Paolo Bonzini, Markus Armbruster, Pierrick Bouvier,
Richard Henderson, Zhao Liu
[-- Attachment #1: Type: text/plain, Size: 731 bytes --]
On Thu, 2025-05-08 at 18:53 +0100, Daniel P. Berrangé wrote:
>
> > This patch moves those stubs from hw/i386/kvm/xen-stubs.c which was
> > built if !CONFIG_XEN_EMU, and moves them elsewhere if !CONFIG_KVM? So
> > at first glance I think it might fail for KVM && !XEN_EMU builds... ?
>
> The files in the top level 'stubs/' directory get built into
> libqemuutil.a, and thus those stubs are available to every
> system binary, in every configuration scenario.
>
> If the real impl of these methods is present in any binary, the stubs
> file impl should get discarded by the linker automatically, avoiding
> a duplicate symbols problem.
Thanks.
In that case,
Acked-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 07/10] qapi: remove the misc-target.json file
2025-05-08 13:58 [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Daniel P. Berrangé
` (5 preceding siblings ...)
2025-05-08 13:58 ` [PATCH 06/10] qapi: make Xen event " Daniel P. Berrangé
@ 2025-05-08 13:58 ` Daniel P. Berrangé
2025-05-08 13:58 ` [PATCH 08/10] qapi: Make CpuModelExpansionInfo::deprecated-props optional and generic Daniel P. Berrangé
` (5 subsequent siblings)
12 siblings, 0 replies; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-08 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini,
Markus Armbruster, Pierrick Bouvier, Richard Henderson,
Daniel P. Berrangé
This file is now empty and can thus be removed.
Observe the pre-existing bug with s390-skeys.c and target/i386/monitor.c
both including qapi-commands-misc-target.h despite not requiring it.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hw/s390x/s390-skeys.c | 1 -
qapi/meson.build | 1 -
qapi/qapi-schema.json | 1 -
target/i386/monitor.c | 1 -
4 files changed, 4 deletions(-)
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index aedb62b2d3..8eeecfd58f 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -17,7 +17,6 @@
#include "hw/s390x/storage-keys.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-machine.h"
-#include "qapi/qapi-commands-misc-target.h"
#include "qobject/qdict.h"
#include "qemu/error-report.h"
#include "system/memory_mapping.h"
diff --git a/qapi/meson.build b/qapi/meson.build
index 5e93e6b8cf..ffe44f9e0b 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -42,7 +42,6 @@ qapi_all_modules = [
'machine-target',
'migration',
'misc',
- 'misc-target',
'net',
'pragma',
'qom',
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index e96bff8d38..d8eb79cfda 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -63,7 +63,6 @@
{ 'include': 'misc.json' }
{ 'include': 'misc-arm.json' }
{ 'include': 'misc-i386.json' }
-{ 'include': 'misc-target.json' }
{ 'include': 'audio.json' }
{ 'include': 'acpi.json' }
{ 'include': 'pci.json' }
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 3ea92b066e..3c9b6ca62f 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -29,7 +29,6 @@
#include "monitor/hmp.h"
#include "qobject/qdict.h"
#include "qapi/error.h"
-#include "qapi/qapi-commands-misc-target.h"
#include "qapi/qapi-commands-misc.h"
/* Perform linear address sign extension */
--
2.49.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 08/10] qapi: Make CpuModelExpansionInfo::deprecated-props optional and generic
2025-05-08 13:58 [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Daniel P. Berrangé
` (6 preceding siblings ...)
2025-05-08 13:58 ` [PATCH 07/10] qapi: remove the misc-target.json file Daniel P. Berrangé
@ 2025-05-08 13:58 ` Daniel P. Berrangé
2025-05-13 12:38 ` Markus Armbruster
2025-05-08 13:58 ` [PATCH 09/10] qapi: make most CPU commands unconditionally available Daniel P. Berrangé
` (4 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-08 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini,
Markus Armbruster, Pierrick Bouvier, Richard Henderson,
Daniel P. Berrangé
From: Philippe Mathieu-Daudé <philmd@linaro.org>
We'd like to have some unified QAPI schema. Having a structure field
conditional to a target being built in is not very practical.
While @deprecated-props is only used by s390x target, it is generic
enough and could be used by other targets (assuming we expand
CpuModelExpansionType enum values).
Let's always include this field, regardless of the target, but
make it optional.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
qapi/machine-target.json | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 541f93eeb7..3b109b4af8 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -244,19 +244,18 @@
#
# @model: the expanded CpuModelInfo.
#
-# @deprecated-props: a list of properties that are flagged as
+# @deprecated-props: an optional list of properties that are flagged as
# deprecated by the CPU vendor. The list depends on the
# CpuModelExpansionType: "static" properties are a subset of the
# enabled-properties for the expanded model; "full" properties are
# a set of properties that are deprecated across all models for
-# the architecture. (since: 9.1).
+# the architecture. (since: 10.1 -- since 9.1 on s390x --).
#
# Since: 2.8
##
{ 'struct': 'CpuModelExpansionInfo',
'data': { 'model': 'CpuModelInfo',
- 'deprecated-props' : { 'type': ['str'],
- 'if': 'TARGET_S390X' } },
+ '*deprecated-props' : { 'type': ['str'] } },
'if': { 'any': [ 'TARGET_S390X',
'TARGET_I386',
'TARGET_ARM',
--
2.49.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 08/10] qapi: Make CpuModelExpansionInfo::deprecated-props optional and generic
2025-05-08 13:58 ` [PATCH 08/10] qapi: Make CpuModelExpansionInfo::deprecated-props optional and generic Daniel P. Berrangé
@ 2025-05-13 12:38 ` Markus Armbruster
2025-05-13 12:41 ` Daniel P. Berrangé
0 siblings, 1 reply; 41+ messages in thread
From: Markus Armbruster @ 2025-05-13 12:38 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini, Pierrick Bouvier, Richard Henderson
Copying my review from the initial posting:
Daniel P. Berrangé <berrange@redhat.com> writes:
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> We'd like to have some unified QAPI schema. Having a structure field
> conditional to a target being built in is not very practical.
>
> While @deprecated-props is only used by s390x target, it is generic
> enough and could be used by other targets (assuming we expand
> CpuModelExpansionType enum values).
>
> Let's always include this field, regardless of the target, but
> make it optional.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> qapi/machine-target.json | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index 541f93eeb7..3b109b4af8 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -244,19 +244,18 @@
> #
> # @model: the expanded CpuModelInfo.
> #
> -# @deprecated-props: a list of properties that are flagged as
> +# @deprecated-props: an optional list of properties that are flagged as
> # deprecated by the CPU vendor. The list depends on the
> # CpuModelExpansionType: "static" properties are a subset of the
> # enabled-properties for the expanded model; "full" properties are
> # a set of properties that are deprecated across all models for
> -# the architecture. (since: 9.1).
> +# the architecture. (since: 10.1 -- since 9.1 on s390x --).
> #
> # Since: 2.8
> ##
> { 'struct': 'CpuModelExpansionInfo',
> 'data': { 'model': 'CpuModelInfo',
> - 'deprecated-props' : { 'type': ['str'],
> - 'if': 'TARGET_S390X' } },
> + '*deprecated-props' : { 'type': ['str'] } },
Make this
'*deprecated-props' : ['str'] },
please.
When I see "optional array", I wonder about the difference between
"absent" and "present and empty". The doc comment doesn't quite explain
it. I figure "present and empty" means empty, while "absent" means we
don't know / not implemented.
Is the difference useful?
Philippe is happy to implement either variant.
> 'if': { 'any': [ 'TARGET_S390X',
> 'TARGET_I386',
> 'TARGET_ARM',
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 08/10] qapi: Make CpuModelExpansionInfo::deprecated-props optional and generic
2025-05-13 12:38 ` Markus Armbruster
@ 2025-05-13 12:41 ` Daniel P. Berrangé
0 siblings, 0 replies; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-13 12:41 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini, Pierrick Bouvier, Richard Henderson
On Tue, May 13, 2025 at 02:38:40PM +0200, Markus Armbruster wrote:
> Copying my review from the initial posting:
>
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
> > From: Philippe Mathieu-Daudé <philmd@linaro.org>
> >
> > We'd like to have some unified QAPI schema. Having a structure field
> > conditional to a target being built in is not very practical.
> >
> > While @deprecated-props is only used by s390x target, it is generic
> > enough and could be used by other targets (assuming we expand
> > CpuModelExpansionType enum values).
> >
> > Let's always include this field, regardless of the target, but
> > make it optional.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> > qapi/machine-target.json | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> > index 541f93eeb7..3b109b4af8 100644
> > --- a/qapi/machine-target.json
> > +++ b/qapi/machine-target.json
> > @@ -244,19 +244,18 @@
> > #
> > # @model: the expanded CpuModelInfo.
> > #
> > -# @deprecated-props: a list of properties that are flagged as
> > +# @deprecated-props: an optional list of properties that are flagged as
> > # deprecated by the CPU vendor. The list depends on the
> > # CpuModelExpansionType: "static" properties are a subset of the
> > # enabled-properties for the expanded model; "full" properties are
> > # a set of properties that are deprecated across all models for
> > -# the architecture. (since: 9.1).
> > +# the architecture. (since: 10.1 -- since 9.1 on s390x --).
> > #
> > # Since: 2.8
> > ##
> > { 'struct': 'CpuModelExpansionInfo',
> > 'data': { 'model': 'CpuModelInfo',
> > - 'deprecated-props' : { 'type': ['str'],
> > - 'if': 'TARGET_S390X' } },
> > + '*deprecated-props' : { 'type': ['str'] } },
>
> Make this
>
> '*deprecated-props' : ['str'] },
>
> please.
>
> When I see "optional array", I wonder about the difference between
> "absent" and "present and empty". The doc comment doesn't quite explain
> it. I figure "present and empty" means empty, while "absent" means we
> don't know / not implemented.
>
> Is the difference useful?
I'm doubtful that the difference is useful.
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] 41+ messages in thread
* [PATCH 09/10] qapi: make most CPU commands unconditionally available
2025-05-08 13:58 [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Daniel P. Berrangé
` (7 preceding siblings ...)
2025-05-08 13:58 ` [PATCH 08/10] qapi: Make CpuModelExpansionInfo::deprecated-props optional and generic Daniel P. Berrangé
@ 2025-05-08 13:58 ` Daniel P. Berrangé
2025-05-08 20:55 ` Pierrick Bouvier
2025-05-13 12:44 ` Markus Armbruster
2025-05-08 13:58 ` [PATCH 10/10] qapi: make s390x specific " Daniel P. Berrangé
` (3 subsequent siblings)
12 siblings, 2 replies; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-08 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini,
Markus Armbruster, Pierrick Bouvier, Richard Henderson,
Daniel P. Berrangé
This removes the TARGET_* conditions from all the CPU commands
that are conceptually target independent. Top level stubs are
provided to cope with targets which do not currently implement
all of the commands.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
qapi/machine-target.json | 397 --------------------------
qapi/machine.json | 363 +++++++++++++++++++++++
stubs/meson.build | 2 +
stubs/monitor-cpu-s390x.c | 23 ++
stubs/monitor-cpu.c | 21 ++
target/arm/arm-qmp-cmds.c | 2 +-
target/i386/cpu-system.c | 2 +-
target/i386/cpu.c | 2 +-
target/loongarch/loongarch-qmp-cmds.c | 2 +-
target/mips/system/mips-qmp-cmds.c | 12 +-
target/ppc/ppc-qmp-cmds.c | 12 +-
target/riscv/riscv-qmp-cmds.c | 2 +-
target/s390x/cpu_models_system.c | 2 +-
13 files changed, 437 insertions(+), 405 deletions(-)
create mode 100644 stubs/monitor-cpu-s390x.c
create mode 100644 stubs/monitor-cpu.c
diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 3b109b4af8..f19e34adaf 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -6,403 +6,6 @@
{ 'include': 'machine-common.json' }
-##
-# @CpuModelInfo:
-#
-# Virtual CPU model.
-#
-# A CPU model consists of the name of a CPU definition, to which delta
-# changes are applied (e.g. features added/removed). Most magic
-# values that an architecture might require should be hidden behind
-# the name. However, if required, architectures can expose relevant
-# properties.
-#
-# @name: the name of the CPU definition the model is based on
-#
-# @props: a dictionary of QOM properties to be applied
-#
-# Since: 2.8
-##
-{ 'struct': 'CpuModelInfo',
- 'data': { 'name': 'str',
- '*props': 'any' } }
-
-##
-# @CpuModelExpansionType:
-#
-# An enumeration of CPU model expansion types.
-#
-# @static: Expand to a static CPU model, a combination of a static
-# base model name and property delta changes. As the static base
-# model will never change, the expanded CPU model will be the
-# same, independent of QEMU version, machine type, machine
-# options, and accelerator options. Therefore, the resulting
-# model can be used by tooling without having to specify a
-# compatibility machine - e.g. when displaying the "host" model.
-# The @static CPU models are migration-safe.
-#
-# @full: Expand all properties. The produced model is not guaranteed
-# to be migration-safe, but allows tooling to get an insight and
-# work with model details.
-#
-# .. note:: When a non-migration-safe CPU model is expanded in static
-# mode, some features enabled by the CPU model may be omitted,
-# because they can't be implemented by a static CPU model
-# definition (e.g. cache info passthrough and PMU passthrough in
-# x86). If you need an accurate representation of the features
-# enabled by a non-migration-safe CPU model, use @full. If you
-# need a static representation that will keep ABI compatibility
-# even when changing QEMU version or machine-type, use @static (but
-# keep in mind that some features may be omitted).
-#
-# Since: 2.8
-##
-{ 'enum': 'CpuModelExpansionType',
- 'data': [ 'static', 'full' ] }
-
-##
-# @CpuModelCompareResult:
-#
-# An enumeration of CPU model comparison results. The result is
-# usually calculated using e.g. CPU features or CPU generations.
-#
-# @incompatible: If model A is incompatible to model B, model A is not
-# guaranteed to run where model B runs and the other way around.
-#
-# @identical: If model A is identical to model B, model A is
-# guaranteed to run where model B runs and the other way around.
-#
-# @superset: If model A is a superset of model B, model B is
-# guaranteed to run where model A runs. There are no guarantees
-# about the other way.
-#
-# @subset: If model A is a subset of model B, model A is guaranteed to
-# run where model B runs. There are no guarantees about the other
-# way.
-#
-# Since: 2.8
-##
-{ 'enum': 'CpuModelCompareResult',
- 'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }
-
-##
-# @CpuModelBaselineInfo:
-#
-# The result of a CPU model baseline.
-#
-# @model: the baselined CpuModelInfo.
-#
-# Since: 2.8
-##
-{ 'struct': 'CpuModelBaselineInfo',
- 'data': { 'model': 'CpuModelInfo' },
- 'if': 'TARGET_S390X' }
-
-##
-# @CpuModelCompareInfo:
-#
-# The result of a CPU model comparison.
-#
-# @result: The result of the compare operation.
-#
-# @responsible-properties: List of properties that led to the
-# comparison result not being identical.
-#
-# @responsible-properties is a list of QOM property names that led to
-# both CPUs not being detected as identical. For identical models,
-# this list is empty. If a QOM property is read-only, that means
-# there's no known way to make the CPU models identical. If the
-# special property name "type" is included, the models are by
-# definition not identical and cannot be made identical.
-#
-# Since: 2.8
-##
-{ 'struct': 'CpuModelCompareInfo',
- 'data': { 'result': 'CpuModelCompareResult',
- 'responsible-properties': ['str'] },
- 'if': 'TARGET_S390X' }
-
-##
-# @query-cpu-model-comparison:
-#
-# Compares two CPU models, @modela and @modelb, returning how they
-# compare in a specific configuration. The results indicates how
-# both models compare regarding runnability. This result can be
-# used by tooling to make decisions if a certain CPU model will
-# run in a certain configuration or if a compatible CPU model has
-# to be created by baselining.
-#
-# Usually, a CPU model is compared against the maximum possible CPU
-# model of a certain configuration (e.g. the "host" model for KVM).
-# If that CPU model is identical or a subset, it will run in that
-# configuration.
-#
-# The result returned by this command may be affected by:
-#
-# * QEMU version: CPU models may look different depending on the QEMU
-# version. (Except for CPU models reported as "static" in
-# query-cpu-definitions.)
-# * machine-type: CPU model may look different depending on the
-# machine-type. (Except for CPU models reported as "static" in
-# query-cpu-definitions.)
-# * machine options (including accelerator): in some architectures,
-# CPU models may look different depending on machine and accelerator
-# options. (Except for CPU models reported as "static" in
-# query-cpu-definitions.)
-# * "-cpu" arguments and global properties: arguments to the -cpu
-# option and global properties may affect expansion of CPU models.
-# Using query-cpu-model-expansion while using these is not advised.
-#
-# Some architectures may not support comparing CPU models. s390x
-# supports comparing CPU models.
-#
-# @modela: description of the first CPU model to compare, referred to
-# as "model A" in CpuModelCompareResult
-#
-# @modelb: description of the second CPU model to compare, referred to
-# as "model B" in CpuModelCompareResult
-#
-# Returns: a CpuModelCompareInfo describing how both CPU models
-# compare
-#
-# Errors:
-# - if comparing CPU models is not supported
-# - if a model cannot be used
-# - if a model contains an unknown cpu definition name, unknown
-# properties or properties with wrong types.
-#
-# .. note:: This command isn't specific to s390x, but is only
-# implemented on this architecture currently.
-#
-# Since: 2.8
-##
-{ 'command': 'query-cpu-model-comparison',
- 'data': { 'modela': 'CpuModelInfo', 'modelb': 'CpuModelInfo' },
- 'returns': 'CpuModelCompareInfo',
- 'if': 'TARGET_S390X' }
-
-##
-# @query-cpu-model-baseline:
-#
-# Baseline two CPU models, @modela and @modelb, creating a compatible
-# third model. The created model will always be a static,
-# migration-safe CPU model (see "static" CPU model expansion for
-# details).
-#
-# This interface can be used by tooling to create a compatible CPU
-# model out two CPU models. The created CPU model will be identical
-# to or a subset of both CPU models when comparing them. Therefore,
-# the created CPU model is guaranteed to run where the given CPU
-# models run.
-#
-# The result returned by this command may be affected by:
-#
-# * QEMU version: CPU models may look different depending on the QEMU
-# version. (Except for CPU models reported as "static" in
-# query-cpu-definitions.)
-# * machine-type: CPU model may look different depending on the
-# machine-type. (Except for CPU models reported as "static" in
-# query-cpu-definitions.)
-# * machine options (including accelerator): in some architectures,
-# CPU models may look different depending on machine and accelerator
-# options. (Except for CPU models reported as "static" in
-# query-cpu-definitions.)
-# * "-cpu" arguments and global properties: arguments to the -cpu
-# option and global properties may affect expansion of CPU models.
-# Using query-cpu-model-expansion while using these is not advised.
-#
-# Some architectures may not support baselining CPU models. s390x
-# supports baselining CPU models.
-#
-# @modela: description of the first CPU model to baseline
-#
-# @modelb: description of the second CPU model to baseline
-#
-# Returns: a CpuModelBaselineInfo describing the baselined CPU model
-#
-# Errors:
-# - if baselining CPU models is not supported
-# - if a model cannot be used
-# - if a model contains an unknown cpu definition name, unknown
-# properties or properties with wrong types.
-#
-# .. note:: This command isn't specific to s390x, but is only
-# implemented on this architecture currently.
-#
-# Since: 2.8
-##
-{ 'command': 'query-cpu-model-baseline',
- 'data': { 'modela': 'CpuModelInfo',
- 'modelb': 'CpuModelInfo' },
- 'returns': 'CpuModelBaselineInfo',
- 'if': 'TARGET_S390X' }
-
-##
-# @CpuModelExpansionInfo:
-#
-# The result of a cpu model expansion.
-#
-# @model: the expanded CpuModelInfo.
-#
-# @deprecated-props: an optional list of properties that are flagged as
-# deprecated by the CPU vendor. The list depends on the
-# CpuModelExpansionType: "static" properties are a subset of the
-# enabled-properties for the expanded model; "full" properties are
-# a set of properties that are deprecated across all models for
-# the architecture. (since: 10.1 -- since 9.1 on s390x --).
-#
-# Since: 2.8
-##
-{ 'struct': 'CpuModelExpansionInfo',
- 'data': { 'model': 'CpuModelInfo',
- '*deprecated-props' : { 'type': ['str'] } },
- 'if': { 'any': [ 'TARGET_S390X',
- 'TARGET_I386',
- 'TARGET_ARM',
- 'TARGET_LOONGARCH64',
- 'TARGET_RISCV' ] } }
-
-##
-# @query-cpu-model-expansion:
-#
-# Expands a given CPU model, @model, (or a combination of CPU model +
-# additional options) to different granularities, specified by @type,
-# allowing tooling to get an understanding what a specific CPU model
-# looks like in QEMU under a certain configuration.
-#
-# This interface can be used to query the "host" CPU model.
-#
-# The data returned by this command may be affected by:
-#
-# * QEMU version: CPU models may look different depending on the QEMU
-# version. (Except for CPU models reported as "static" in
-# query-cpu-definitions.)
-# * machine-type: CPU model may look different depending on the
-# machine-type. (Except for CPU models reported as "static" in
-# query-cpu-definitions.)
-# * machine options (including accelerator): in some architectures,
-# CPU models may look different depending on machine and accelerator
-# options. (Except for CPU models reported as "static" in
-# query-cpu-definitions.)
-# * "-cpu" arguments and global properties: arguments to the -cpu
-# option and global properties may affect expansion of CPU models.
-# Using query-cpu-model-expansion while using these is not advised.
-#
-# Some architectures may not support all expansion types. s390x
-# supports "full" and "static". Arm only supports "full".
-#
-# @model: description of the CPU model to expand
-#
-# @type: expansion type, specifying how to expand the CPU model
-#
-# Returns: a CpuModelExpansionInfo describing the expanded CPU model
-#
-# Errors:
-# - if expanding CPU models is not supported
-# - if the model cannot be expanded
-# - if the model contains an unknown CPU definition name, unknown
-# properties or properties with a wrong type
-# - if an expansion type is not supported
-#
-# Since: 2.8
-##
-{ 'command': 'query-cpu-model-expansion',
- 'data': { 'type': 'CpuModelExpansionType',
- 'model': 'CpuModelInfo' },
- 'returns': 'CpuModelExpansionInfo',
- 'if': { 'any': [ 'TARGET_S390X',
- 'TARGET_I386',
- 'TARGET_ARM',
- 'TARGET_LOONGARCH64',
- 'TARGET_RISCV' ] } }
-
-##
-# @CpuDefinitionInfo:
-#
-# Virtual CPU definition.
-#
-# @name: the name of the CPU definition
-#
-# @migration-safe: whether a CPU definition can be safely used for
-# migration in combination with a QEMU compatibility machine when
-# migrating between different QEMU versions and between hosts with
-# different sets of (hardware or software) capabilities. If not
-# provided, information is not available and callers should not
-# assume the CPU definition to be migration-safe. (since 2.8)
-#
-# @static: whether a CPU definition is static and will not change
-# depending on QEMU version, machine type, machine options and
-# accelerator options. A static model is always migration-safe.
-# (since 2.8)
-#
-# @unavailable-features: List of properties that prevent the CPU model
-# from running in the current host. (since 2.8)
-#
-# @typename: Type name that can be used as argument to
-# @device-list-properties, to introspect properties configurable
-# using -cpu or -global. (since 2.9)
-#
-# @alias-of: Name of CPU model this model is an alias for. The target
-# of the CPU model alias may change depending on the machine type.
-# Management software is supposed to translate CPU model aliases
-# in the VM configuration, because aliases may stop being
-# migration-safe in the future (since 4.1)
-#
-# @deprecated: If true, this CPU model is deprecated and may be
-# removed in in some future version of QEMU according to the QEMU
-# deprecation policy. (since 5.2)
-#
-# @unavailable-features is a list of QOM property names that represent
-# CPU model attributes that prevent the CPU from running. If the QOM
-# property is read-only, that means there's no known way to make the
-# CPU model run in the current host. Implementations that choose not
-# to provide specific information return the property name "type". If
-# the property is read-write, it means that it MAY be possible to run
-# the CPU model in the current host if that property is changed.
-# Management software can use it as hints to suggest or choose an
-# alternative for the user, or just to generate meaningful error
-# messages explaining why the CPU model can't be used. If
-# @unavailable-features is an empty list, the CPU model is runnable
-# using the current host and machine-type. If @unavailable-features
-# is not present, runnability information for the CPU is not
-# available.
-#
-# Since: 1.2
-##
-{ 'struct': 'CpuDefinitionInfo',
- 'data': { 'name': 'str',
- '*migration-safe': 'bool',
- 'static': 'bool',
- '*unavailable-features': [ 'str' ],
- 'typename': 'str',
- '*alias-of' : 'str',
- 'deprecated' : 'bool' },
- 'if': { 'any': [ 'TARGET_PPC',
- 'TARGET_ARM',
- 'TARGET_I386',
- 'TARGET_S390X',
- 'TARGET_MIPS',
- 'TARGET_LOONGARCH64',
- 'TARGET_RISCV' ] } }
-
-##
-# @query-cpu-definitions:
-#
-# Return a list of supported virtual CPU definitions
-#
-# Returns: a list of CpuDefinitionInfo
-#
-# Since: 1.2
-##
-{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
- 'if': { 'any': [ 'TARGET_PPC',
- 'TARGET_ARM',
- 'TARGET_I386',
- 'TARGET_S390X',
- 'TARGET_MIPS',
- 'TARGET_LOONGARCH64',
- 'TARGET_RISCV' ] } }
-
##
# @S390CpuPolarization:
#
diff --git a/qapi/machine.json b/qapi/machine.json
index c8feb9fe17..31e8be7f44 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1916,3 +1916,366 @@
##
{ 'command': 'dump-skeys',
'data': { 'filename': 'str' } }
+
+##
+# @CpuModelInfo:
+#
+# Virtual CPU model.
+#
+# A CPU model consists of the name of a CPU definition, to which delta
+# changes are applied (e.g. features added/removed). Most magic
+# values that an architecture might require should be hidden behind
+# the name. However, if required, architectures can expose relevant
+# properties.
+#
+# @name: the name of the CPU definition the model is based on
+#
+# @props: a dictionary of QOM properties to be applied
+#
+# Since: 2.8
+##
+{ 'struct': 'CpuModelInfo',
+ 'data': { 'name': 'str',
+ '*props': 'any' } }
+
+##
+# @CpuModelExpansionType:
+#
+# An enumeration of CPU model expansion types.
+#
+# @static: Expand to a static CPU model, a combination of a static
+# base model name and property delta changes. As the static base
+# model will never change, the expanded CPU model will be the
+# same, independent of QEMU version, machine type, machine
+# options, and accelerator options. Therefore, the resulting
+# model can be used by tooling without having to specify a
+# compatibility machine - e.g. when displaying the "host" model.
+# The @static CPU models are migration-safe.
+#
+# @full: Expand all properties. The produced model is not guaranteed
+# to be migration-safe, but allows tooling to get an insight and
+# work with model details.
+#
+# .. note:: When a non-migration-safe CPU model is expanded in static
+# mode, some features enabled by the CPU model may be omitted,
+# because they can't be implemented by a static CPU model
+# definition (e.g. cache info passthrough and PMU passthrough in
+# x86). If you need an accurate representation of the features
+# enabled by a non-migration-safe CPU model, use @full. If you
+# need a static representation that will keep ABI compatibility
+# even when changing QEMU version or machine-type, use @static (but
+# keep in mind that some features may be omitted).
+#
+# Since: 2.8
+##
+{ 'enum': 'CpuModelExpansionType',
+ 'data': [ 'static', 'full' ] }
+
+##
+# @CpuModelCompareResult:
+#
+# An enumeration of CPU model comparison results. The result is
+# usually calculated using e.g. CPU features or CPU generations.
+#
+# @incompatible: If model A is incompatible to model B, model A is not
+# guaranteed to run where model B runs and the other way around.
+#
+# @identical: If model A is identical to model B, model A is
+# guaranteed to run where model B runs and the other way around.
+#
+# @superset: If model A is a superset of model B, model B is
+# guaranteed to run where model A runs. There are no guarantees
+# about the other way.
+#
+# @subset: If model A is a subset of model B, model A is guaranteed to
+# run where model B runs. There are no guarantees about the other
+# way.
+#
+# Since: 2.8
+##
+{ 'enum': 'CpuModelCompareResult',
+ 'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }
+
+##
+# @CpuModelBaselineInfo:
+#
+# The result of a CPU model baseline.
+#
+# @model: the baselined CpuModelInfo.
+#
+# Since: 2.8
+##
+{ 'struct': 'CpuModelBaselineInfo',
+ 'data': { 'model': 'CpuModelInfo' } }
+
+##
+# @CpuModelCompareInfo:
+#
+# The result of a CPU model comparison.
+#
+# @result: The result of the compare operation.
+#
+# @responsible-properties: List of properties that led to the
+# comparison result not being identical.
+#
+# @responsible-properties is a list of QOM property names that led to
+# both CPUs not being detected as identical. For identical models,
+# this list is empty. If a QOM property is read-only, that means
+# there's no known way to make the CPU models identical. If the
+# special property name "type" is included, the models are by
+# definition not identical and cannot be made identical.
+#
+# Since: 2.8
+##
+{ 'struct': 'CpuModelCompareInfo',
+ 'data': { 'result': 'CpuModelCompareResult',
+ 'responsible-properties': ['str'] } }
+
+##
+# @query-cpu-model-comparison:
+#
+# Compares two CPU models, @modela and @modelb, returning how they
+# compare in a specific configuration. The results indicates how
+# both models compare regarding runnability. This result can be
+# used by tooling to make decisions if a certain CPU model will
+# run in a certain configuration or if a compatible CPU model has
+# to be created by baselining.
+#
+# Usually, a CPU model is compared against the maximum possible CPU
+# model of a certain configuration (e.g. the "host" model for KVM).
+# If that CPU model is identical or a subset, it will run in that
+# configuration.
+#
+# The result returned by this command may be affected by:
+#
+# * QEMU version: CPU models may look different depending on the QEMU
+# version. (Except for CPU models reported as "static" in
+# query-cpu-definitions.)
+# * machine-type: CPU model may look different depending on the
+# machine-type. (Except for CPU models reported as "static" in
+# query-cpu-definitions.)
+# * machine options (including accelerator): in some architectures,
+# CPU models may look different depending on machine and accelerator
+# options. (Except for CPU models reported as "static" in
+# query-cpu-definitions.)
+# * "-cpu" arguments and global properties: arguments to the -cpu
+# option and global properties may affect expansion of CPU models.
+# Using query-cpu-model-expansion while using these is not advised.
+#
+# Some architectures may not support comparing CPU models. s390x
+# supports comparing CPU models.
+#
+# @modela: description of the first CPU model to compare, referred to
+# as "model A" in CpuModelCompareResult
+#
+# @modelb: description of the second CPU model to compare, referred to
+# as "model B" in CpuModelCompareResult
+#
+# Returns: a CpuModelCompareInfo describing how both CPU models
+# compare
+#
+# Errors:
+# - if comparing CPU models is not supported by the target
+# - if a model cannot be used
+# - if a model contains an unknown cpu definition name, unknown
+# properties or properties with wrong types.
+#
+# Since: 2.8
+##
+{ 'command': 'query-cpu-model-comparison',
+ 'data': { 'modela': 'CpuModelInfo', 'modelb': 'CpuModelInfo' },
+ 'returns': 'CpuModelCompareInfo' }
+
+##
+# @query-cpu-model-baseline:
+#
+# Baseline two CPU models, @modela and @modelb, creating a compatible
+# third model. The created model will always be a static,
+# migration-safe CPU model (see "static" CPU model expansion for
+# details).
+#
+# This interface can be used by tooling to create a compatible CPU
+# model out two CPU models. The created CPU model will be identical
+# to or a subset of both CPU models when comparing them. Therefore,
+# the created CPU model is guaranteed to run where the given CPU
+# models run.
+#
+# The result returned by this command may be affected by:
+#
+# * QEMU version: CPU models may look different depending on the QEMU
+# version. (Except for CPU models reported as "static" in
+# query-cpu-definitions.)
+# * machine-type: CPU model may look different depending on the
+# machine-type. (Except for CPU models reported as "static" in
+# query-cpu-definitions.)
+# * machine options (including accelerator): in some architectures,
+# CPU models may look different depending on machine and accelerator
+# options. (Except for CPU models reported as "static" in
+# query-cpu-definitions.)
+# * "-cpu" arguments and global properties: arguments to the -cpu
+# option and global properties may affect expansion of CPU models.
+# Using query-cpu-model-expansion while using these is not advised.
+#
+# Some architectures may not support baselining CPU models. s390x
+# supports baselining CPU models.
+#
+# @modela: description of the first CPU model to baseline
+#
+# @modelb: description of the second CPU model to baseline
+#
+# Returns: a CpuModelBaselineInfo describing the baselined CPU model
+#
+# Errors:
+# - if baselining CPU models is not supported by the target
+# - if a model cannot be used
+# - if a model contains an unknown cpu definition name, unknown
+# properties or properties with wrong types.
+#
+# Since: 2.8
+##
+{ 'command': 'query-cpu-model-baseline',
+ 'data': { 'modela': 'CpuModelInfo',
+ 'modelb': 'CpuModelInfo' },
+ 'returns': 'CpuModelBaselineInfo' }
+
+##
+# @CpuModelExpansionInfo:
+#
+# The result of a cpu model expansion.
+#
+# @model: the expanded CpuModelInfo.
+#
+# @deprecated-props: an optional list of properties that are flagged as
+# deprecated by the CPU vendor. The list depends on the
+# CpuModelExpansionType: "static" properties are a subset of the
+# enabled-properties for the expanded model; "full" properties are
+# a set of properties that are deprecated across all models for
+# the architecture. (since: 10.1 -- since 9.1 on s390x --).
+#
+# Since: 2.8
+##
+{ 'struct': 'CpuModelExpansionInfo',
+ 'data': { 'model': 'CpuModelInfo',
+ '*deprecated-props' : { 'type': ['str'] } } }
+
+##
+# @query-cpu-model-expansion:
+#
+# Expands a given CPU model, @model, (or a combination of CPU model +
+# additional options) to different granularities, specified by @type,
+# allowing tooling to get an understanding what a specific CPU model
+# looks like in QEMU under a certain configuration.
+#
+# This interface can be used to query the "host" CPU model.
+#
+# The data returned by this command may be affected by:
+#
+# * QEMU version: CPU models may look different depending on the QEMU
+# version. (Except for CPU models reported as "static" in
+# query-cpu-definitions.)
+# * machine-type: CPU model may look different depending on the
+# machine-type. (Except for CPU models reported as "static" in
+# query-cpu-definitions.)
+# * machine options (including accelerator): in some architectures,
+# CPU models may look different depending on machine and accelerator
+# options. (Except for CPU models reported as "static" in
+# query-cpu-definitions.)
+# * "-cpu" arguments and global properties: arguments to the -cpu
+# option and global properties may affect expansion of CPU models.
+# Using query-cpu-model-expansion while using these is not advised.
+#
+# Some architectures may not support all expansion types. s390x
+# supports "full" and "static". Arm only supports "full".
+#
+# @model: description of the CPU model to expand
+#
+# @type: expansion type, specifying how to expand the CPU model
+#
+# Returns: a CpuModelExpansionInfo describing the expanded CPU model
+#
+# Errors:
+# - if expanding CPU models is not supported
+# - if the model cannot be expanded
+# - if the model contains an unknown CPU definition name, unknown
+# properties or properties with a wrong type
+# - if an expansion type is not supported
+#
+# Since: 2.8
+##
+{ 'command': 'query-cpu-model-expansion',
+ 'data': { 'type': 'CpuModelExpansionType',
+ 'model': 'CpuModelInfo' },
+ 'returns': 'CpuModelExpansionInfo' }
+
+##
+# @CpuDefinitionInfo:
+#
+# Virtual CPU definition.
+#
+# @name: the name of the CPU definition
+#
+# @migration-safe: whether a CPU definition can be safely used for
+# migration in combination with a QEMU compatibility machine when
+# migrating between different QEMU versions and between hosts with
+# different sets of (hardware or software) capabilities. If not
+# provided, information is not available and callers should not
+# assume the CPU definition to be migration-safe. (since 2.8)
+#
+# @static: whether a CPU definition is static and will not change
+# depending on QEMU version, machine type, machine options and
+# accelerator options. A static model is always migration-safe.
+# (since 2.8)
+#
+# @unavailable-features: List of properties that prevent the CPU model
+# from running in the current host. (since 2.8)
+#
+# @typename: Type name that can be used as argument to
+# @device-list-properties, to introspect properties configurable
+# using -cpu or -global. (since 2.9)
+#
+# @alias-of: Name of CPU model this model is an alias for. The target
+# of the CPU model alias may change depending on the machine type.
+# Management software is supposed to translate CPU model aliases
+# in the VM configuration, because aliases may stop being
+# migration-safe in the future (since 4.1)
+#
+# @deprecated: If true, this CPU model is deprecated and may be
+# removed in in some future version of QEMU according to the QEMU
+# deprecation policy. (since 5.2)
+#
+# @unavailable-features is a list of QOM property names that represent
+# CPU model attributes that prevent the CPU from running. If the QOM
+# property is read-only, that means there's no known way to make the
+# CPU model run in the current host. Implementations that choose not
+# to provide specific information return the property name "type". If
+# the property is read-write, it means that it MAY be possible to run
+# the CPU model in the current host if that property is changed.
+# Management software can use it as hints to suggest or choose an
+# alternative for the user, or just to generate meaningful error
+# messages explaining why the CPU model can't be used. If
+# @unavailable-features is an empty list, the CPU model is runnable
+# using the current host and machine-type. If @unavailable-features
+# is not present, runnability information for the CPU is not
+# available.
+#
+# Since: 1.2
+##
+{ 'struct': 'CpuDefinitionInfo',
+ 'data': { 'name': 'str',
+ '*migration-safe': 'bool',
+ 'static': 'bool',
+ '*unavailable-features': [ 'str' ],
+ 'typename': 'str',
+ '*alias-of' : 'str',
+ 'deprecated' : 'bool' } }
+
+##
+# @query-cpu-definitions:
+#
+# Return a list of supported virtual CPU definitions
+#
+# Returns: a list of CpuDefinitionInfo
+#
+# Since: 1.2
+##
+{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
diff --git a/stubs/meson.build b/stubs/meson.build
index 0ef11976a2..3b2fad0824 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -82,6 +82,8 @@ if have_system
stub_ss.add(files('monitor-i386-sev.c'))
stub_ss.add(files('monitor-i386-sgx.c'))
stub_ss.add(files('monitor-i386-xen.c'))
+ stub_ss.add(files('monitor-cpu.c'))
+ stub_ss.add(files('monitor-cpu-s390x.c'))
endif
if have_system or have_user
diff --git a/stubs/monitor-cpu-s390x.c b/stubs/monitor-cpu-s390x.c
new file mode 100644
index 0000000000..71e794482b
--- /dev/null
+++ b/stubs/monitor-cpu-s390x.c
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-machine.h"
+
+CpuModelCompareInfo *
+qmp_query_cpu_model_comparison(CpuModelInfo *infoa,
+ CpuModelInfo *infob,
+ Error **errp)
+{
+ error_setg(errp, "CPU model comparison is not supported on this target");
+ return NULL;
+}
+
+CpuModelBaselineInfo *
+qmp_query_cpu_model_baseline(CpuModelInfo *infoa,
+ CpuModelInfo *infob,
+ Error **errp)
+{
+ error_setg(errp, "CPU model baseline is not supported on this target");
+ return NULL;
+}
diff --git a/stubs/monitor-cpu.c b/stubs/monitor-cpu.c
new file mode 100644
index 0000000000..a8c7ee89b9
--- /dev/null
+++ b/stubs/monitor-cpu.c
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-machine.h"
+
+CpuModelExpansionInfo *
+qmp_query_cpu_model_expansion(CpuModelExpansionType type,
+ CpuModelInfo *model,
+ Error **errp)
+{
+ error_setg(errp, "CPU model expansion is not supported on this target");
+ return NULL;
+}
+
+CpuDefinitionInfoList *
+qmp_query_cpu_definitions(Error **errp)
+{
+ error_setg(errp, "CPU model definitions are not supported on this target");
+ return NULL;
+}
diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c
index ef18c867ca..cca6b9722b 100644
--- a/target/arm/arm-qmp-cmds.c
+++ b/target/arm/arm-qmp-cmds.c
@@ -26,7 +26,7 @@
#include "qapi/error.h"
#include "qapi/visitor.h"
#include "qapi/qobject-input-visitor.h"
-#include "qapi/qapi-commands-machine-target.h"
+#include "qapi/qapi-commands-machine.h"
#include "qapi/qapi-commands-misc-arm.h"
#include "qobject/qdict.h"
#include "qom/qom-qobject.h"
diff --git a/target/i386/cpu-system.c b/target/i386/cpu-system.c
index 55f192e819..b1494aa674 100644
--- a/target/i386/cpu-system.c
+++ b/target/i386/cpu-system.c
@@ -24,7 +24,7 @@
#include "qobject/qdict.h"
#include "qapi/qobject-input-visitor.h"
#include "qom/qom-qobject.h"
-#include "qapi/qapi-commands-machine-target.h"
+#include "qapi/qapi-commands-machine.h"
#include "cpu-internal.h"
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 6f21d5ed22..4933676dc5 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -38,7 +38,7 @@
#include "exec/watchpoint.h"
#ifndef CONFIG_USER_ONLY
#include "system/reset.h"
-#include "qapi/qapi-commands-machine-target.h"
+#include "qapi/qapi-commands-machine.h"
#include "system/address-spaces.h"
#include "hw/boards.h"
#include "hw/i386/sgx-epc.h"
diff --git a/target/loongarch/loongarch-qmp-cmds.c b/target/loongarch/loongarch-qmp-cmds.c
index 6f732d80f3..f5f1cd0009 100644
--- a/target/loongarch/loongarch-qmp-cmds.c
+++ b/target/loongarch/loongarch-qmp-cmds.c
@@ -8,7 +8,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
-#include "qapi/qapi-commands-machine-target.h"
+#include "qapi/qapi-commands-machine.h"
#include "cpu.h"
#include "qobject/qdict.h"
#include "qapi/qobject-input-visitor.h"
diff --git a/target/mips/system/mips-qmp-cmds.c b/target/mips/system/mips-qmp-cmds.c
index 7340ac70ba..d98d6623f2 100644
--- a/target/mips/system/mips-qmp-cmds.c
+++ b/target/mips/system/mips-qmp-cmds.c
@@ -7,9 +7,19 @@
*/
#include "qemu/osdep.h"
-#include "qapi/qapi-commands-machine-target.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-machine.h"
#include "cpu.h"
+CpuModelExpansionInfo *
+qmp_query_cpu_model_expansion(CpuModelExpansionType type,
+ CpuModelInfo *model,
+ Error **errp)
+{
+ error_setg(errp, "CPU model expansion is not supported on this target");
+ return NULL;
+}
+
static void mips_cpu_add_definition(gpointer data, gpointer user_data)
{
ObjectClass *oc = data;
diff --git a/target/ppc/ppc-qmp-cmds.c b/target/ppc/ppc-qmp-cmds.c
index a25d86a8d1..7022564604 100644
--- a/target/ppc/ppc-qmp-cmds.c
+++ b/target/ppc/ppc-qmp-cmds.c
@@ -28,7 +28,8 @@
#include "qemu/ctype.h"
#include "monitor/hmp-target.h"
#include "monitor/hmp.h"
-#include "qapi/qapi-commands-machine-target.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-machine.h"
#include "cpu-models.h"
#include "cpu-qom.h"
@@ -175,6 +176,15 @@ int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval)
return -EINVAL;
}
+CpuModelExpansionInfo *
+qmp_query_cpu_model_expansion(CpuModelExpansionType type,
+ CpuModelInfo *model,
+ Error **errp)
+{
+ error_setg(errp, "CPU model expansion is not supported on this target");
+ return NULL;
+}
+
static void ppc_cpu_defs_entry(gpointer data, gpointer user_data)
{
ObjectClass *oc = data;
diff --git a/target/riscv/riscv-qmp-cmds.c b/target/riscv/riscv-qmp-cmds.c
index d0a324364d..8ba8aa0d5f 100644
--- a/target/riscv/riscv-qmp-cmds.c
+++ b/target/riscv/riscv-qmp-cmds.c
@@ -25,7 +25,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
-#include "qapi/qapi-commands-machine-target.h"
+#include "qapi/qapi-commands-machine.h"
#include "qobject/qbool.h"
#include "qobject/qdict.h"
#include "qapi/qobject-input-visitor.h"
diff --git a/target/s390x/cpu_models_system.c b/target/s390x/cpu_models_system.c
index 4351182f72..9d84faa3c9 100644
--- a/target/s390x/cpu_models_system.c
+++ b/target/s390x/cpu_models_system.c
@@ -19,7 +19,7 @@
#include "qapi/visitor.h"
#include "qapi/qobject-input-visitor.h"
#include "qobject/qdict.h"
-#include "qapi/qapi-commands-machine-target.h"
+#include "qapi/qapi-commands-machine.h"
static void list_add_feat(const char *name, void *opaque);
--
2.49.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 09/10] qapi: make most CPU commands unconditionally available
2025-05-08 13:58 ` [PATCH 09/10] qapi: make most CPU commands unconditionally available Daniel P. Berrangé
@ 2025-05-08 20:55 ` Pierrick Bouvier
2025-05-13 12:44 ` Markus Armbruster
1 sibling, 0 replies; 41+ messages in thread
From: Pierrick Bouvier @ 2025-05-08 20:55 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini,
Markus Armbruster, Richard Henderson
On 5/8/25 6:58 AM, Daniel P. Berrangé wrote:
> This removes the TARGET_* conditions from all the CPU commands
> that are conceptually target independent. Top level stubs are
> provided to cope with targets which do not currently implement
> all of the commands.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> qapi/machine-target.json | 397 --------------------------
> qapi/machine.json | 363 +++++++++++++++++++++++
> stubs/meson.build | 2 +
> stubs/monitor-cpu-s390x.c | 23 ++
> stubs/monitor-cpu.c | 21 ++
> target/arm/arm-qmp-cmds.c | 2 +-
> target/i386/cpu-system.c | 2 +-
> target/i386/cpu.c | 2 +-
> target/loongarch/loongarch-qmp-cmds.c | 2 +-
> target/mips/system/mips-qmp-cmds.c | 12 +-
> target/ppc/ppc-qmp-cmds.c | 12 +-
> target/riscv/riscv-qmp-cmds.c | 2 +-
> target/s390x/cpu_models_system.c | 2 +-
> 13 files changed, 437 insertions(+), 405 deletions(-)
> create mode 100644 stubs/monitor-cpu-s390x.c
> create mode 100644 stubs/monitor-cpu.c
>
This sounds good to me, as long we are ok with having commands returning
a "this command is not supported here" paradigm.
I thought it was the complex case we were trying to deal with, precisely
because we didn't want to introduce the "command here, but it returns an
error" shell.
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 09/10] qapi: make most CPU commands unconditionally available
2025-05-08 13:58 ` [PATCH 09/10] qapi: make most CPU commands unconditionally available Daniel P. Berrangé
2025-05-08 20:55 ` Pierrick Bouvier
@ 2025-05-13 12:44 ` Markus Armbruster
2025-05-13 16:37 ` Daniel P. Berrangé
1 sibling, 1 reply; 41+ messages in thread
From: Markus Armbruster @ 2025-05-13 12:44 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini, Pierrick Bouvier, Richard Henderson
Daniel P. Berrangé <berrange@redhat.com> writes:
> This removes the TARGET_* conditions from all the CPU commands
> that are conceptually target independent. Top level stubs are
> provided to cope with targets which do not currently implement
> all of the commands.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> qapi/machine-target.json | 397 --------------------------
> qapi/machine.json | 363 +++++++++++++++++++++++
> stubs/meson.build | 2 +
> stubs/monitor-cpu-s390x.c | 23 ++
> stubs/monitor-cpu.c | 21 ++
> target/arm/arm-qmp-cmds.c | 2 +-
> target/i386/cpu-system.c | 2 +-
> target/i386/cpu.c | 2 +-
> target/loongarch/loongarch-qmp-cmds.c | 2 +-
> target/mips/system/mips-qmp-cmds.c | 12 +-
> target/ppc/ppc-qmp-cmds.c | 12 +-
> target/riscv/riscv-qmp-cmds.c | 2 +-
> target/s390x/cpu_models_system.c | 2 +-
> 13 files changed, 437 insertions(+), 405 deletions(-)
> create mode 100644 stubs/monitor-cpu-s390x.c
> create mode 100644 stubs/monitor-cpu.c
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index 3b109b4af8..f19e34adaf 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -6,403 +6,6 @@
>
> { 'include': 'machine-common.json' }
>
> -##
> -# @CpuModelInfo:
> -#
> -# Virtual CPU model.
> -#
> -# A CPU model consists of the name of a CPU definition, to which delta
> -# changes are applied (e.g. features added/removed). Most magic
> -# values that an architecture might require should be hidden behind
> -# the name. However, if required, architectures can expose relevant
> -# properties.
> -#
> -# @name: the name of the CPU definition the model is based on
> -#
> -# @props: a dictionary of QOM properties to be applied
> -#
> -# Since: 2.8
> -##
> -{ 'struct': 'CpuModelInfo',
> - 'data': { 'name': 'str',
> - '*props': 'any' } }
> -
> -##
> -# @CpuModelExpansionType:
> -#
> -# An enumeration of CPU model expansion types.
> -#
> -# @static: Expand to a static CPU model, a combination of a static
> -# base model name and property delta changes. As the static base
> -# model will never change, the expanded CPU model will be the
> -# same, independent of QEMU version, machine type, machine
> -# options, and accelerator options. Therefore, the resulting
> -# model can be used by tooling without having to specify a
> -# compatibility machine - e.g. when displaying the "host" model.
> -# The @static CPU models are migration-safe.
> -#
> -# @full: Expand all properties. The produced model is not guaranteed
> -# to be migration-safe, but allows tooling to get an insight and
> -# work with model details.
> -#
> -# .. note:: When a non-migration-safe CPU model is expanded in static
> -# mode, some features enabled by the CPU model may be omitted,
> -# because they can't be implemented by a static CPU model
> -# definition (e.g. cache info passthrough and PMU passthrough in
> -# x86). If you need an accurate representation of the features
> -# enabled by a non-migration-safe CPU model, use @full. If you
> -# need a static representation that will keep ABI compatibility
> -# even when changing QEMU version or machine-type, use @static (but
> -# keep in mind that some features may be omitted).
> -#
> -# Since: 2.8
> -##
> -{ 'enum': 'CpuModelExpansionType',
> - 'data': [ 'static', 'full' ] }
> -
> -##
> -# @CpuModelCompareResult:
> -#
> -# An enumeration of CPU model comparison results. The result is
> -# usually calculated using e.g. CPU features or CPU generations.
> -#
> -# @incompatible: If model A is incompatible to model B, model A is not
> -# guaranteed to run where model B runs and the other way around.
> -#
> -# @identical: If model A is identical to model B, model A is
> -# guaranteed to run where model B runs and the other way around.
> -#
> -# @superset: If model A is a superset of model B, model B is
> -# guaranteed to run where model A runs. There are no guarantees
> -# about the other way.
> -#
> -# @subset: If model A is a subset of model B, model A is guaranteed to
> -# run where model B runs. There are no guarantees about the other
> -# way.
> -#
> -# Since: 2.8
> -##
> -{ 'enum': 'CpuModelCompareResult',
> - 'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }
> -
> -##
> -# @CpuModelBaselineInfo:
> -#
> -# The result of a CPU model baseline.
> -#
> -# @model: the baselined CpuModelInfo.
> -#
> -# Since: 2.8
> -##
> -{ 'struct': 'CpuModelBaselineInfo',
> - 'data': { 'model': 'CpuModelInfo' },
> - 'if': 'TARGET_S390X' }
> -
> -##
> -# @CpuModelCompareInfo:
> -#
> -# The result of a CPU model comparison.
> -#
> -# @result: The result of the compare operation.
> -#
> -# @responsible-properties: List of properties that led to the
> -# comparison result not being identical.
> -#
> -# @responsible-properties is a list of QOM property names that led to
> -# both CPUs not being detected as identical. For identical models,
> -# this list is empty. If a QOM property is read-only, that means
> -# there's no known way to make the CPU models identical. If the
> -# special property name "type" is included, the models are by
> -# definition not identical and cannot be made identical.
> -#
> -# Since: 2.8
> -##
> -{ 'struct': 'CpuModelCompareInfo',
> - 'data': { 'result': 'CpuModelCompareResult',
> - 'responsible-properties': ['str'] },
> - 'if': 'TARGET_S390X' }
> -
> -##
> -# @query-cpu-model-comparison:
> -#
> -# Compares two CPU models, @modela and @modelb, returning how they
> -# compare in a specific configuration. The results indicates how
> -# both models compare regarding runnability. This result can be
> -# used by tooling to make decisions if a certain CPU model will
> -# run in a certain configuration or if a compatible CPU model has
> -# to be created by baselining.
> -#
> -# Usually, a CPU model is compared against the maximum possible CPU
> -# model of a certain configuration (e.g. the "host" model for KVM).
> -# If that CPU model is identical or a subset, it will run in that
> -# configuration.
> -#
> -# The result returned by this command may be affected by:
> -#
> -# * QEMU version: CPU models may look different depending on the QEMU
> -# version. (Except for CPU models reported as "static" in
> -# query-cpu-definitions.)
> -# * machine-type: CPU model may look different depending on the
> -# machine-type. (Except for CPU models reported as "static" in
> -# query-cpu-definitions.)
> -# * machine options (including accelerator): in some architectures,
> -# CPU models may look different depending on machine and accelerator
> -# options. (Except for CPU models reported as "static" in
> -# query-cpu-definitions.)
> -# * "-cpu" arguments and global properties: arguments to the -cpu
> -# option and global properties may affect expansion of CPU models.
> -# Using query-cpu-model-expansion while using these is not advised.
> -#
> -# Some architectures may not support comparing CPU models. s390x
> -# supports comparing CPU models.
> -#
> -# @modela: description of the first CPU model to compare, referred to
> -# as "model A" in CpuModelCompareResult
> -#
> -# @modelb: description of the second CPU model to compare, referred to
> -# as "model B" in CpuModelCompareResult
> -#
> -# Returns: a CpuModelCompareInfo describing how both CPU models
> -# compare
> -#
> -# Errors:
> -# - if comparing CPU models is not supported
> -# - if a model cannot be used
> -# - if a model contains an unknown cpu definition name, unknown
> -# properties or properties with wrong types.
> -#
> -# .. note:: This command isn't specific to s390x, but is only
> -# implemented on this architecture currently.
> -#
> -# Since: 2.8
> -##
> -{ 'command': 'query-cpu-model-comparison',
> - 'data': { 'modela': 'CpuModelInfo', 'modelb': 'CpuModelInfo' },
> - 'returns': 'CpuModelCompareInfo',
> - 'if': 'TARGET_S390X' }
> -
> -##
> -# @query-cpu-model-baseline:
> -#
> -# Baseline two CPU models, @modela and @modelb, creating a compatible
> -# third model. The created model will always be a static,
> -# migration-safe CPU model (see "static" CPU model expansion for
> -# details).
> -#
> -# This interface can be used by tooling to create a compatible CPU
> -# model out two CPU models. The created CPU model will be identical
> -# to or a subset of both CPU models when comparing them. Therefore,
> -# the created CPU model is guaranteed to run where the given CPU
> -# models run.
> -#
> -# The result returned by this command may be affected by:
> -#
> -# * QEMU version: CPU models may look different depending on the QEMU
> -# version. (Except for CPU models reported as "static" in
> -# query-cpu-definitions.)
> -# * machine-type: CPU model may look different depending on the
> -# machine-type. (Except for CPU models reported as "static" in
> -# query-cpu-definitions.)
> -# * machine options (including accelerator): in some architectures,
> -# CPU models may look different depending on machine and accelerator
> -# options. (Except for CPU models reported as "static" in
> -# query-cpu-definitions.)
> -# * "-cpu" arguments and global properties: arguments to the -cpu
> -# option and global properties may affect expansion of CPU models.
> -# Using query-cpu-model-expansion while using these is not advised.
> -#
> -# Some architectures may not support baselining CPU models. s390x
> -# supports baselining CPU models.
> -#
> -# @modela: description of the first CPU model to baseline
> -#
> -# @modelb: description of the second CPU model to baseline
> -#
> -# Returns: a CpuModelBaselineInfo describing the baselined CPU model
> -#
> -# Errors:
> -# - if baselining CPU models is not supported
> -# - if a model cannot be used
> -# - if a model contains an unknown cpu definition name, unknown
> -# properties or properties with wrong types.
> -#
> -# .. note:: This command isn't specific to s390x, but is only
> -# implemented on this architecture currently.
> -#
> -# Since: 2.8
> -##
> -{ 'command': 'query-cpu-model-baseline',
> - 'data': { 'modela': 'CpuModelInfo',
> - 'modelb': 'CpuModelInfo' },
> - 'returns': 'CpuModelBaselineInfo',
> - 'if': 'TARGET_S390X' }
> -
> -##
> -# @CpuModelExpansionInfo:
> -#
> -# The result of a cpu model expansion.
> -#
> -# @model: the expanded CpuModelInfo.
> -#
> -# @deprecated-props: an optional list of properties that are flagged as
> -# deprecated by the CPU vendor. The list depends on the
> -# CpuModelExpansionType: "static" properties are a subset of the
> -# enabled-properties for the expanded model; "full" properties are
> -# a set of properties that are deprecated across all models for
> -# the architecture. (since: 10.1 -- since 9.1 on s390x --).
> -#
> -# Since: 2.8
> -##
> -{ 'struct': 'CpuModelExpansionInfo',
> - 'data': { 'model': 'CpuModelInfo',
> - '*deprecated-props' : { 'type': ['str'] } },
> - 'if': { 'any': [ 'TARGET_S390X',
> - 'TARGET_I386',
> - 'TARGET_ARM',
> - 'TARGET_LOONGARCH64',
> - 'TARGET_RISCV' ] } }
> -
> -##
> -# @query-cpu-model-expansion:
> -#
> -# Expands a given CPU model, @model, (or a combination of CPU model +
> -# additional options) to different granularities, specified by @type,
> -# allowing tooling to get an understanding what a specific CPU model
> -# looks like in QEMU under a certain configuration.
> -#
> -# This interface can be used to query the "host" CPU model.
> -#
> -# The data returned by this command may be affected by:
> -#
> -# * QEMU version: CPU models may look different depending on the QEMU
> -# version. (Except for CPU models reported as "static" in
> -# query-cpu-definitions.)
> -# * machine-type: CPU model may look different depending on the
> -# machine-type. (Except for CPU models reported as "static" in
> -# query-cpu-definitions.)
> -# * machine options (including accelerator): in some architectures,
> -# CPU models may look different depending on machine and accelerator
> -# options. (Except for CPU models reported as "static" in
> -# query-cpu-definitions.)
> -# * "-cpu" arguments and global properties: arguments to the -cpu
> -# option and global properties may affect expansion of CPU models.
> -# Using query-cpu-model-expansion while using these is not advised.
> -#
> -# Some architectures may not support all expansion types. s390x
> -# supports "full" and "static". Arm only supports "full".
> -#
> -# @model: description of the CPU model to expand
> -#
> -# @type: expansion type, specifying how to expand the CPU model
> -#
> -# Returns: a CpuModelExpansionInfo describing the expanded CPU model
> -#
> -# Errors:
> -# - if expanding CPU models is not supported
> -# - if the model cannot be expanded
> -# - if the model contains an unknown CPU definition name, unknown
> -# properties or properties with a wrong type
> -# - if an expansion type is not supported
> -#
> -# Since: 2.8
> -##
> -{ 'command': 'query-cpu-model-expansion',
> - 'data': { 'type': 'CpuModelExpansionType',
> - 'model': 'CpuModelInfo' },
> - 'returns': 'CpuModelExpansionInfo',
> - 'if': { 'any': [ 'TARGET_S390X',
> - 'TARGET_I386',
> - 'TARGET_ARM',
> - 'TARGET_LOONGARCH64',
> - 'TARGET_RISCV' ] } }
> -
> -##
> -# @CpuDefinitionInfo:
> -#
> -# Virtual CPU definition.
> -#
> -# @name: the name of the CPU definition
> -#
> -# @migration-safe: whether a CPU definition can be safely used for
> -# migration in combination with a QEMU compatibility machine when
> -# migrating between different QEMU versions and between hosts with
> -# different sets of (hardware or software) capabilities. If not
> -# provided, information is not available and callers should not
> -# assume the CPU definition to be migration-safe. (since 2.8)
> -#
> -# @static: whether a CPU definition is static and will not change
> -# depending on QEMU version, machine type, machine options and
> -# accelerator options. A static model is always migration-safe.
> -# (since 2.8)
> -#
> -# @unavailable-features: List of properties that prevent the CPU model
> -# from running in the current host. (since 2.8)
> -#
> -# @typename: Type name that can be used as argument to
> -# @device-list-properties, to introspect properties configurable
> -# using -cpu or -global. (since 2.9)
> -#
> -# @alias-of: Name of CPU model this model is an alias for. The target
> -# of the CPU model alias may change depending on the machine type.
> -# Management software is supposed to translate CPU model aliases
> -# in the VM configuration, because aliases may stop being
> -# migration-safe in the future (since 4.1)
> -#
> -# @deprecated: If true, this CPU model is deprecated and may be
> -# removed in in some future version of QEMU according to the QEMU
> -# deprecation policy. (since 5.2)
> -#
> -# @unavailable-features is a list of QOM property names that represent
> -# CPU model attributes that prevent the CPU from running. If the QOM
> -# property is read-only, that means there's no known way to make the
> -# CPU model run in the current host. Implementations that choose not
> -# to provide specific information return the property name "type". If
> -# the property is read-write, it means that it MAY be possible to run
> -# the CPU model in the current host if that property is changed.
> -# Management software can use it as hints to suggest or choose an
> -# alternative for the user, or just to generate meaningful error
> -# messages explaining why the CPU model can't be used. If
> -# @unavailable-features is an empty list, the CPU model is runnable
> -# using the current host and machine-type. If @unavailable-features
> -# is not present, runnability information for the CPU is not
> -# available.
> -#
> -# Since: 1.2
> -##
> -{ 'struct': 'CpuDefinitionInfo',
> - 'data': { 'name': 'str',
> - '*migration-safe': 'bool',
> - 'static': 'bool',
> - '*unavailable-features': [ 'str' ],
> - 'typename': 'str',
> - '*alias-of' : 'str',
> - 'deprecated' : 'bool' },
> - 'if': { 'any': [ 'TARGET_PPC',
> - 'TARGET_ARM',
> - 'TARGET_I386',
> - 'TARGET_S390X',
> - 'TARGET_MIPS',
> - 'TARGET_LOONGARCH64',
> - 'TARGET_RISCV' ] } }
> -
> -##
> -# @query-cpu-definitions:
> -#
> -# Return a list of supported virtual CPU definitions
> -#
> -# Returns: a list of CpuDefinitionInfo
> -#
> -# Since: 1.2
> -##
> -{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
> - 'if': { 'any': [ 'TARGET_PPC',
> - 'TARGET_ARM',
> - 'TARGET_I386',
> - 'TARGET_S390X',
> - 'TARGET_MIPS',
> - 'TARGET_LOONGARCH64',
> - 'TARGET_RISCV' ] } }
> -
> ##
> # @S390CpuPolarization:
> #
> diff --git a/qapi/machine.json b/qapi/machine.json
> index c8feb9fe17..31e8be7f44 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1916,3 +1916,366 @@
> ##
> { 'command': 'dump-skeys',
> 'data': { 'filename': 'str' } }
> +
> +##
> +# @CpuModelInfo:
> +#
> +# Virtual CPU model.
> +#
> +# A CPU model consists of the name of a CPU definition, to which delta
> +# changes are applied (e.g. features added/removed). Most magic
> +# values that an architecture might require should be hidden behind
> +# the name. However, if required, architectures can expose relevant
> +# properties.
> +#
> +# @name: the name of the CPU definition the model is based on
> +#
> +# @props: a dictionary of QOM properties to be applied
> +#
> +# Since: 2.8
> +##
> +{ 'struct': 'CpuModelInfo',
> + 'data': { 'name': 'str',
> + '*props': 'any' } }
> +
> +##
> +# @CpuModelExpansionType:
> +#
> +# An enumeration of CPU model expansion types.
> +#
> +# @static: Expand to a static CPU model, a combination of a static
> +# base model name and property delta changes. As the static base
> +# model will never change, the expanded CPU model will be the
> +# same, independent of QEMU version, machine type, machine
> +# options, and accelerator options. Therefore, the resulting
> +# model can be used by tooling without having to specify a
> +# compatibility machine - e.g. when displaying the "host" model.
> +# The @static CPU models are migration-safe.
> +#
> +# @full: Expand all properties. The produced model is not guaranteed
> +# to be migration-safe, but allows tooling to get an insight and
> +# work with model details.
> +#
> +# .. note:: When a non-migration-safe CPU model is expanded in static
> +# mode, some features enabled by the CPU model may be omitted,
> +# because they can't be implemented by a static CPU model
> +# definition (e.g. cache info passthrough and PMU passthrough in
> +# x86). If you need an accurate representation of the features
> +# enabled by a non-migration-safe CPU model, use @full. If you
> +# need a static representation that will keep ABI compatibility
> +# even when changing QEMU version or machine-type, use @static (but
> +# keep in mind that some features may be omitted).
> +#
> +# Since: 2.8
> +##
> +{ 'enum': 'CpuModelExpansionType',
> + 'data': [ 'static', 'full' ] }
> +
> +##
> +# @CpuModelCompareResult:
> +#
> +# An enumeration of CPU model comparison results. The result is
> +# usually calculated using e.g. CPU features or CPU generations.
> +#
> +# @incompatible: If model A is incompatible to model B, model A is not
> +# guaranteed to run where model B runs and the other way around.
> +#
> +# @identical: If model A is identical to model B, model A is
> +# guaranteed to run where model B runs and the other way around.
> +#
> +# @superset: If model A is a superset of model B, model B is
> +# guaranteed to run where model A runs. There are no guarantees
> +# about the other way.
> +#
> +# @subset: If model A is a subset of model B, model A is guaranteed to
> +# run where model B runs. There are no guarantees about the other
> +# way.
> +#
> +# Since: 2.8
> +##
> +{ 'enum': 'CpuModelCompareResult',
> + 'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }
> +
> +##
> +# @CpuModelBaselineInfo:
> +#
> +# The result of a CPU model baseline.
> +#
> +# @model: the baselined CpuModelInfo.
> +#
> +# Since: 2.8
> +##
> +{ 'struct': 'CpuModelBaselineInfo',
> + 'data': { 'model': 'CpuModelInfo' } }
> +
> +##
> +# @CpuModelCompareInfo:
> +#
> +# The result of a CPU model comparison.
> +#
> +# @result: The result of the compare operation.
> +#
> +# @responsible-properties: List of properties that led to the
> +# comparison result not being identical.
> +#
> +# @responsible-properties is a list of QOM property names that led to
> +# both CPUs not being detected as identical. For identical models,
> +# this list is empty. If a QOM property is read-only, that means
> +# there's no known way to make the CPU models identical. If the
> +# special property name "type" is included, the models are by
> +# definition not identical and cannot be made identical.
> +#
> +# Since: 2.8
> +##
> +{ 'struct': 'CpuModelCompareInfo',
> + 'data': { 'result': 'CpuModelCompareResult',
> + 'responsible-properties': ['str'] } }
> +
> +##
> +# @query-cpu-model-comparison:
> +#
> +# Compares two CPU models, @modela and @modelb, returning how they
> +# compare in a specific configuration. The results indicates how
> +# both models compare regarding runnability. This result can be
> +# used by tooling to make decisions if a certain CPU model will
> +# run in a certain configuration or if a compatible CPU model has
> +# to be created by baselining.
> +#
> +# Usually, a CPU model is compared against the maximum possible CPU
> +# model of a certain configuration (e.g. the "host" model for KVM).
> +# If that CPU model is identical or a subset, it will run in that
> +# configuration.
> +#
> +# The result returned by this command may be affected by:
> +#
> +# * QEMU version: CPU models may look different depending on the QEMU
> +# version. (Except for CPU models reported as "static" in
> +# query-cpu-definitions.)
> +# * machine-type: CPU model may look different depending on the
> +# machine-type. (Except for CPU models reported as "static" in
> +# query-cpu-definitions.)
> +# * machine options (including accelerator): in some architectures,
> +# CPU models may look different depending on machine and accelerator
> +# options. (Except for CPU models reported as "static" in
> +# query-cpu-definitions.)
> +# * "-cpu" arguments and global properties: arguments to the -cpu
> +# option and global properties may affect expansion of CPU models.
> +# Using query-cpu-model-expansion while using these is not advised.
> +#
> +# Some architectures may not support comparing CPU models. s390x
> +# supports comparing CPU models.
> +#
> +# @modela: description of the first CPU model to compare, referred to
> +# as "model A" in CpuModelCompareResult
> +#
> +# @modelb: description of the second CPU model to compare, referred to
> +# as "model B" in CpuModelCompareResult
> +#
> +# Returns: a CpuModelCompareInfo describing how both CPU models
> +# compare
> +#
> +# Errors:
> +# - if comparing CPU models is not supported by the target
You add "by the target", and ...
> +# - if a model cannot be used
> +# - if a model contains an unknown cpu definition name, unknown
> +# properties or properties with wrong types.
delete this note:
# .. note:: This command isn't specific to s390x, but is only
# implemented on this architecture currently.
Lost: command currently works just for s390x targets. Intentional?
Same for query-cpu-model-baseline below.
> +#
> +# Since: 2.8
> +##
> +{ 'command': 'query-cpu-model-comparison',
> + 'data': { 'modela': 'CpuModelInfo', 'modelb': 'CpuModelInfo' },
> + 'returns': 'CpuModelCompareInfo' }
> +
> +##
> +# @query-cpu-model-baseline:
> +#
> +# Baseline two CPU models, @modela and @modelb, creating a compatible
> +# third model. The created model will always be a static,
> +# migration-safe CPU model (see "static" CPU model expansion for
> +# details).
> +#
> +# This interface can be used by tooling to create a compatible CPU
> +# model out two CPU models. The created CPU model will be identical
> +# to or a subset of both CPU models when comparing them. Therefore,
> +# the created CPU model is guaranteed to run where the given CPU
> +# models run.
> +#
> +# The result returned by this command may be affected by:
> +#
> +# * QEMU version: CPU models may look different depending on the QEMU
> +# version. (Except for CPU models reported as "static" in
> +# query-cpu-definitions.)
> +# * machine-type: CPU model may look different depending on the
> +# machine-type. (Except for CPU models reported as "static" in
> +# query-cpu-definitions.)
> +# * machine options (including accelerator): in some architectures,
> +# CPU models may look different depending on machine and accelerator
> +# options. (Except for CPU models reported as "static" in
> +# query-cpu-definitions.)
> +# * "-cpu" arguments and global properties: arguments to the -cpu
> +# option and global properties may affect expansion of CPU models.
> +# Using query-cpu-model-expansion while using these is not advised.
> +#
> +# Some architectures may not support baselining CPU models. s390x
> +# supports baselining CPU models.
> +#
> +# @modela: description of the first CPU model to baseline
> +#
> +# @modelb: description of the second CPU model to baseline
> +#
> +# Returns: a CpuModelBaselineInfo describing the baselined CPU model
> +#
> +# Errors:
> +# - if baselining CPU models is not supported by the target
> +# - if a model cannot be used
> +# - if a model contains an unknown cpu definition name, unknown
> +# properties or properties with wrong types.
> +#
> +# Since: 2.8
> +##
> +{ 'command': 'query-cpu-model-baseline',
> + 'data': { 'modela': 'CpuModelInfo',
> + 'modelb': 'CpuModelInfo' },
> + 'returns': 'CpuModelBaselineInfo' }
> +
> +##
> +# @CpuModelExpansionInfo:
> +#
> +# The result of a cpu model expansion.
> +#
> +# @model: the expanded CpuModelInfo.
> +#
> +# @deprecated-props: an optional list of properties that are flagged as
> +# deprecated by the CPU vendor. The list depends on the
> +# CpuModelExpansionType: "static" properties are a subset of the
> +# enabled-properties for the expanded model; "full" properties are
> +# a set of properties that are deprecated across all models for
> +# the architecture. (since: 10.1 -- since 9.1 on s390x --).
> +#
> +# Since: 2.8
> +##
> +{ 'struct': 'CpuModelExpansionInfo',
> + 'data': { 'model': 'CpuModelInfo',
> + '*deprecated-props' : { 'type': ['str'] } } }
> +
> +##
> +# @query-cpu-model-expansion:
> +#
> +# Expands a given CPU model, @model, (or a combination of CPU model +
> +# additional options) to different granularities, specified by @type,
> +# allowing tooling to get an understanding what a specific CPU model
> +# looks like in QEMU under a certain configuration.
> +#
> +# This interface can be used to query the "host" CPU model.
> +#
> +# The data returned by this command may be affected by:
> +#
> +# * QEMU version: CPU models may look different depending on the QEMU
> +# version. (Except for CPU models reported as "static" in
> +# query-cpu-definitions.)
> +# * machine-type: CPU model may look different depending on the
> +# machine-type. (Except for CPU models reported as "static" in
> +# query-cpu-definitions.)
> +# * machine options (including accelerator): in some architectures,
> +# CPU models may look different depending on machine and accelerator
> +# options. (Except for CPU models reported as "static" in
> +# query-cpu-definitions.)
> +# * "-cpu" arguments and global properties: arguments to the -cpu
> +# option and global properties may affect expansion of CPU models.
> +# Using query-cpu-model-expansion while using these is not advised.
> +#
> +# Some architectures may not support all expansion types. s390x
> +# supports "full" and "static". Arm only supports "full".
> +#
> +# @model: description of the CPU model to expand
> +#
> +# @type: expansion type, specifying how to expand the CPU model
> +#
> +# Returns: a CpuModelExpansionInfo describing the expanded CPU model
> +#
> +# Errors:
> +# - if expanding CPU models is not supported
> +# - if the model cannot be expanded
> +# - if the model contains an unknown CPU definition name, unknown
> +# properties or properties with a wrong type
> +# - if an expansion type is not supported
> +#
> +# Since: 2.8
> +##
> +{ 'command': 'query-cpu-model-expansion',
> + 'data': { 'type': 'CpuModelExpansionType',
> + 'model': 'CpuModelInfo' },
> + 'returns': 'CpuModelExpansionInfo' }
> +
> +##
> +# @CpuDefinitionInfo:
> +#
> +# Virtual CPU definition.
> +#
> +# @name: the name of the CPU definition
> +#
> +# @migration-safe: whether a CPU definition can be safely used for
> +# migration in combination with a QEMU compatibility machine when
> +# migrating between different QEMU versions and between hosts with
> +# different sets of (hardware or software) capabilities. If not
> +# provided, information is not available and callers should not
> +# assume the CPU definition to be migration-safe. (since 2.8)
> +#
> +# @static: whether a CPU definition is static and will not change
> +# depending on QEMU version, machine type, machine options and
> +# accelerator options. A static model is always migration-safe.
> +# (since 2.8)
> +#
> +# @unavailable-features: List of properties that prevent the CPU model
> +# from running in the current host. (since 2.8)
> +#
> +# @typename: Type name that can be used as argument to
> +# @device-list-properties, to introspect properties configurable
> +# using -cpu or -global. (since 2.9)
> +#
> +# @alias-of: Name of CPU model this model is an alias for. The target
> +# of the CPU model alias may change depending on the machine type.
> +# Management software is supposed to translate CPU model aliases
> +# in the VM configuration, because aliases may stop being
> +# migration-safe in the future (since 4.1)
> +#
> +# @deprecated: If true, this CPU model is deprecated and may be
> +# removed in in some future version of QEMU according to the QEMU
> +# deprecation policy. (since 5.2)
> +#
> +# @unavailable-features is a list of QOM property names that represent
> +# CPU model attributes that prevent the CPU from running. If the QOM
> +# property is read-only, that means there's no known way to make the
> +# CPU model run in the current host. Implementations that choose not
> +# to provide specific information return the property name "type". If
> +# the property is read-write, it means that it MAY be possible to run
> +# the CPU model in the current host if that property is changed.
> +# Management software can use it as hints to suggest or choose an
> +# alternative for the user, or just to generate meaningful error
> +# messages explaining why the CPU model can't be used. If
> +# @unavailable-features is an empty list, the CPU model is runnable
> +# using the current host and machine-type. If @unavailable-features
> +# is not present, runnability information for the CPU is not
> +# available.
> +#
> +# Since: 1.2
> +##
> +{ 'struct': 'CpuDefinitionInfo',
> + 'data': { 'name': 'str',
> + '*migration-safe': 'bool',
> + 'static': 'bool',
> + '*unavailable-features': [ 'str' ],
> + 'typename': 'str',
> + '*alias-of' : 'str',
> + 'deprecated' : 'bool' } }
> +
> +##
> +# @query-cpu-definitions:
> +#
> +# Return a list of supported virtual CPU definitions
> +#
> +# Returns: a list of CpuDefinitionInfo
> +#
> +# Since: 1.2
> +##
> +{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
> diff --git a/stubs/meson.build b/stubs/meson.build
> index 0ef11976a2..3b2fad0824 100644
[...]
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 09/10] qapi: make most CPU commands unconditionally available
2025-05-13 12:44 ` Markus Armbruster
@ 2025-05-13 16:37 ` Daniel P. Berrangé
0 siblings, 0 replies; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-13 16:37 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini, Pierrick Bouvier, Richard Henderson
On Tue, May 13, 2025 at 02:44:40PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
> > This removes the TARGET_* conditions from all the CPU commands
> > that are conceptually target independent. Top level stubs are
> > provided to cope with targets which do not currently implement
> > all of the commands.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > qapi/machine-target.json | 397 --------------------------
> > qapi/machine.json | 363 +++++++++++++++++++++++
> > stubs/meson.build | 2 +
> > stubs/monitor-cpu-s390x.c | 23 ++
> > stubs/monitor-cpu.c | 21 ++
> > target/arm/arm-qmp-cmds.c | 2 +-
> > target/i386/cpu-system.c | 2 +-
> > target/i386/cpu.c | 2 +-
> > target/loongarch/loongarch-qmp-cmds.c | 2 +-
> > target/mips/system/mips-qmp-cmds.c | 12 +-
> > target/ppc/ppc-qmp-cmds.c | 12 +-
> > target/riscv/riscv-qmp-cmds.c | 2 +-
> > target/s390x/cpu_models_system.c | 2 +-
> > 13 files changed, 437 insertions(+), 405 deletions(-)
> > create mode 100644 stubs/monitor-cpu-s390x.c
> > create mode 100644 stubs/monitor-cpu.c
snip
> > +##
> > +# @query-cpu-model-comparison:
> > +#
> > +# Compares two CPU models, @modela and @modelb, returning how they
> > +# compare in a specific configuration. The results indicates how
> > +# both models compare regarding runnability. This result can be
> > +# used by tooling to make decisions if a certain CPU model will
> > +# run in a certain configuration or if a compatible CPU model has
> > +# to be created by baselining.
> > +#
> > +# Usually, a CPU model is compared against the maximum possible CPU
> > +# model of a certain configuration (e.g. the "host" model for KVM).
> > +# If that CPU model is identical or a subset, it will run in that
> > +# configuration.
> > +#
> > +# The result returned by this command may be affected by:
> > +#
> > +# * QEMU version: CPU models may look different depending on the QEMU
> > +# version. (Except for CPU models reported as "static" in
> > +# query-cpu-definitions.)
> > +# * machine-type: CPU model may look different depending on the
> > +# machine-type. (Except for CPU models reported as "static" in
> > +# query-cpu-definitions.)
> > +# * machine options (including accelerator): in some architectures,
> > +# CPU models may look different depending on machine and accelerator
> > +# options. (Except for CPU models reported as "static" in
> > +# query-cpu-definitions.)
> > +# * "-cpu" arguments and global properties: arguments to the -cpu
> > +# option and global properties may affect expansion of CPU models.
> > +# Using query-cpu-model-expansion while using these is not advised.
> > +#
> > +# Some architectures may not support comparing CPU models. s390x
> > +# supports comparing CPU models.
> > +#
> > +# @modela: description of the first CPU model to compare, referred to
> > +# as "model A" in CpuModelCompareResult
> > +#
> > +# @modelb: description of the second CPU model to compare, referred to
> > +# as "model B" in CpuModelCompareResult
> > +#
> > +# Returns: a CpuModelCompareInfo describing how both CPU models
> > +# compare
> > +#
> > +# Errors:
> > +# - if comparing CPU models is not supported by the target
>
> You add "by the target", and ...
>
> > +# - if a model cannot be used
> > +# - if a model contains an unknown cpu definition name, unknown
> > +# properties or properties with wrong types.
>
> delete this note:
>
> # .. note:: This command isn't specific to s390x, but is only
> # implemented on this architecture currently.
>
> Lost: command currently works just for s390x targets. Intentional?
Kind of.
Previously, the whole command would be tagged for TARGET_S390X
so the command was not available at all.
Now its always available, but may return an error at runtime
for some targets.
Since we're not providing any mechanism for apps to detect
which targets will work and which won't, the implication
is that apps are expected to try it if they need it, and
cope with any error.
From that POV I thought it might be better not to call
out a point-in-time limitation of the impl. I'm not too
fussed either way though, so we could re-instate the
note.
> Same for query-cpu-model-baseline below.
Likewise
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] 41+ messages in thread
* [PATCH 10/10] qapi: make s390x specific CPU commands unconditionally available
2025-05-08 13:58 [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Daniel P. Berrangé
` (8 preceding siblings ...)
2025-05-08 13:58 ` [PATCH 09/10] qapi: make most CPU commands unconditionally available Daniel P. Berrangé
@ 2025-05-08 13:58 ` Daniel P. Berrangé
2025-05-08 14:56 ` [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Philippe Mathieu-Daudé
` (2 subsequent siblings)
12 siblings, 0 replies; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-08 13:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini,
Markus Armbruster, Pierrick Bouvier, Richard Henderson,
Daniel P. Berrangé
This removes the TARGET_S390X and CONFIG_KVM conditions from the
CPU commands that are conceptually specific to s390x. Top level
stubs are provided to cope with non-s390x targets, or builds
without KVM.
The removal of CONFIG_KVM is justified by the fact there is no
conceptual difference between running 'qemu-system-s390x -accel tcg'
on a build with and without KVM built-in, so apps only using TCG
can't rely on the CONFIG_KVM in the schema.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hw/s390x/cpu-topology.c | 4 ++--
include/hw/s390x/cpu-topology.h | 2 +-
...machine-target.json => machine-s390x.json} | 15 +++++--------
qapi/meson.build | 2 +-
qapi/qapi-schema.json | 2 +-
stubs/meson.build | 1 +
stubs/monitor-cpu-s390x-kvm.c | 22 +++++++++++++++++++
tests/qtest/qmp-cmd-test.c | 1 +
8 files changed, 34 insertions(+), 15 deletions(-)
rename qapi/{machine-target.json => machine-s390x.json} (85%)
create mode 100644 stubs/monitor-cpu-s390x-kvm.c
diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c
index 7d4e1f5472..b513f8936e 100644
--- a/hw/s390x/cpu-topology.c
+++ b/hw/s390x/cpu-topology.c
@@ -23,8 +23,8 @@
#include "target/s390x/cpu.h"
#include "hw/s390x/s390-virtio-ccw.h"
#include "hw/s390x/cpu-topology.h"
-#include "qapi/qapi-commands-machine-target.h"
-#include "qapi/qapi-events-machine-target.h"
+#include "qapi/qapi-commands-machine-s390x.h"
+#include "qapi/qapi-events-machine-s390x.h"
/*
* s390_topology is used to keep the topology information.
diff --git a/include/hw/s390x/cpu-topology.h b/include/hw/s390x/cpu-topology.h
index 9283c948e3..d5e9aa43f8 100644
--- a/include/hw/s390x/cpu-topology.h
+++ b/include/hw/s390x/cpu-topology.h
@@ -13,7 +13,7 @@
#include "qemu/queue.h"
#include "hw/boards.h"
-#include "qapi/qapi-types-machine-target.h"
+#include "qapi/qapi-types-machine-s390x.h"
#define S390_TOPOLOGY_CPU_IFL 0x03
diff --git a/qapi/machine-target.json b/qapi/machine-s390x.json
similarity index 85%
rename from qapi/machine-target.json
rename to qapi/machine-s390x.json
index f19e34adaf..5bd77c12ae 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-s390x.json
@@ -15,8 +15,7 @@
# Since: 8.2
##
{ 'enum': 'S390CpuPolarization',
- 'data': [ 'horizontal', 'vertical' ],
- 'if': 'TARGET_S390X'
+ 'data': [ 'horizontal', 'vertical' ]
}
##
@@ -54,8 +53,7 @@
'*entitlement': 'S390CpuEntitlement',
'*dedicated': 'bool'
},
- 'features': [ 'unstable' ],
- 'if': { 'all': [ 'TARGET_S390X' , 'CONFIG_KVM' ] }
+ 'features': [ 'unstable' ]
}
##
@@ -90,8 +88,7 @@
##
{ 'event': 'CPU_POLARIZATION_CHANGE',
'data': { 'polarization': 'S390CpuPolarization' },
- 'features': [ 'unstable' ],
- 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] }
+ 'features': [ 'unstable' ]
}
##
@@ -104,8 +101,7 @@
# Since: 8.2
##
{ 'struct': 'CpuPolarizationInfo',
- 'data': { 'polarization': 'S390CpuPolarization' },
- 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] }
+ 'data': { 'polarization': 'S390CpuPolarization' }
}
##
@@ -120,6 +116,5 @@
# Since: 8.2
##
{ 'command': 'query-s390x-cpu-polarization', 'returns': 'CpuPolarizationInfo',
- 'features': [ 'unstable' ],
- 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] }
+ 'features': [ 'unstable' ]
}
diff --git a/qapi/meson.build b/qapi/meson.build
index ffe44f9e0b..e038b636c9 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -39,7 +39,7 @@ qapi_all_modules = [
'job',
'machine-common',
'machine',
- 'machine-target',
+ 'machine-s390x',
'migration',
'misc',
'net',
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index d8eb79cfda..a8f66163cb 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -57,7 +57,7 @@
{ 'include': 'qdev.json' }
{ 'include': 'machine-common.json' }
{ 'include': 'machine.json' }
-{ 'include': 'machine-target.json' }
+{ 'include': 'machine-s390x.json' }
{ 'include': 'replay.json' }
{ 'include': 'yank.json' }
{ 'include': 'misc.json' }
diff --git a/stubs/meson.build b/stubs/meson.build
index 3b2fad0824..cef046e685 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -84,6 +84,7 @@ if have_system
stub_ss.add(files('monitor-i386-xen.c'))
stub_ss.add(files('monitor-cpu.c'))
stub_ss.add(files('monitor-cpu-s390x.c'))
+ stub_ss.add(files('monitor-cpu-s390x-kvm.c'))
endif
if have_system or have_user
diff --git a/stubs/monitor-cpu-s390x-kvm.c b/stubs/monitor-cpu-s390x-kvm.c
new file mode 100644
index 0000000000..8683dd2d4c
--- /dev/null
+++ b/stubs/monitor-cpu-s390x-kvm.c
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-machine-s390x.h"
+
+void qmp_set_cpu_topology(uint16_t core,
+ bool has_socket, uint16_t socket,
+ bool has_book, uint16_t book,
+ bool has_drawer, uint16_t drawer,
+ bool has_entitlement, S390CpuEntitlement entitlement,
+ bool has_dedicated, bool dedicated,
+ Error **errp)
+{
+ error_setg(errp, "CPU topology change is not supported on this target");
+}
+
+CpuPolarizationInfo *qmp_query_s390x_cpu_polarization(Error **errp)
+{
+ error_setg(errp, "CPU polarization is not supported on this target");
+ return NULL;
+}
diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
index 15c88248b7..040d042810 100644
--- a/tests/qtest/qmp-cmd-test.c
+++ b/tests/qtest/qmp-cmd-test.c
@@ -100,6 +100,7 @@ static bool query_is_ignored(const char *cmd)
/* Success depends on target arch: */
"query-cpu-definitions", /* arm, i386, ppc, s390x */
"query-gic-capabilities", /* arm */
+ "query-s390x-cpu-polarization", /* s390x */
/* Success depends on target-specific build configuration: */
"query-pci", /* CONFIG_PCI */
"x-query-virtio", /* CONFIG_VIRTIO */
--
2.49.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema
2025-05-08 13:58 [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Daniel P. Berrangé
` (9 preceding siblings ...)
2025-05-08 13:58 ` [PATCH 10/10] qapi: make s390x specific " Daniel P. Berrangé
@ 2025-05-08 14:56 ` Philippe Mathieu-Daudé
2025-05-08 14:58 ` Daniel P. Berrangé
2025-05-08 21:09 ` Pierrick Bouvier
2025-05-10 9:28 ` Markus Armbruster
12 siblings, 1 reply; 41+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-05-08 14:56 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Peter Maydell, Paolo Bonzini, Markus Armbruster, Pierrick Bouvier,
Richard Henderson
On 8/5/25 15:58, Daniel P. Berrangé wrote:
> Pierrick has proposed a series that introduces a concept of runtime
> conditionals to the QAPI schema, in order to adapt the TARGET_*
> conditionals currently used at build time:
>
> https://lists.nongnu.org/archive/html/qemu-devel/2025-05/msg01699.html
>
> For the sake of comparison & evaluation, this series illustrates the
> alternative approach that we've discussed of entirely removing any
> concept of TARGET_* conditionals.
>
> With this the QAPI schema varies solely based on CONFIG_* conditionals,
> and is thus invariant across different target emulators.
>
> In this PoC I've taken the minimal effort approach to the problem.
>
> The QAPI schema has removed the TARGET_* conditionals and in order to
> make all the emulators then compile, the stubs/ directory is populated
> with a bunch of files to provide dummy impls of the target specific QMP
> commands.
>
> This is sufficient to make the current QEMU binaries build successfully.
>
> To make the "single binary" concept work, however, would require
> additional followup work to eliminate the stubs.
>
> Instead of having stubs we would need to de-couple the QMP command
> impl from the machine internals. This would likely require greater
> use of interfaces and/or virtual method dispatchers on the machine
> class. This would enable the 'qmp_XXXXX' command impls to exist
> once. Then they call out to virtual methods on the machine to provide
> the real impl, and/or generate an error if the virtual method is not
> implemented for the machine.
Note, I don't see commands "per machine", but per device (including
CPU). When not specified, we iterate over device implementations / CPUs.
>
> Daniel P. Berrangé (9):
> qapi: expose rtc-reset-reinjection command unconditionally
> qapi: expand docs for SEV commands
> qapi: make SEV commands unconditionally available
> qapi: expose query-gic-capability command unconditionally
> qapi: make SGX commands unconditionally available
> qapi: make Xen event commands unconditionally available
> qapi: remove the misc-target.json file
> qapi: make most CPU commands unconditionally available
> qapi: make s390x specific CPU commands unconditionally available
>
> Philippe Mathieu-Daudé (1):
> qapi: Make CpuModelExpansionInfo::deprecated-props optional and
> generic
>
> hw/i386/kvm/xen-stubs.c | 13 -
> hw/i386/kvm/xen_evtchn.c | 2 +-
> hw/i386/monitor.c | 2 +-
> hw/i386/sgx-stub.c | 13 -
> hw/i386/sgx.c | 2 +-
> hw/s390x/cpu-topology.c | 4 +-
> hw/s390x/s390-skeys.c | 1 -
> include/hw/s390x/cpu-topology.h | 2 +-
> qapi/machine-s390x.json | 120 ++++++
> qapi/machine-target.json | 523 --------------------------
> qapi/machine.json | 363 ++++++++++++++++++
> qapi/meson.build | 5 +-
> qapi/misc-arm.json | 49 +++
> qapi/misc-i386.json | 479 +++++++++++++++++++++++
> qapi/misc-target.json | 504 -------------------------
> qapi/qapi-schema.json | 5 +-
> stubs/meson.build | 8 +
> stubs/monitor-arm-gic.c | 12 +
> stubs/monitor-cpu-s390x-kvm.c | 22 ++
> stubs/monitor-cpu-s390x.c | 23 ++
> stubs/monitor-cpu.c | 21 ++
> stubs/monitor-i386-rtc.c | 10 +
> stubs/monitor-i386-sev.c | 36 ++
> stubs/monitor-i386-sgx.c | 17 +
> stubs/monitor-i386-xen.c | 16 +
> target/arm/arm-qmp-cmds.c | 4 +-
> target/i386/cpu-system.c | 2 +-
> target/i386/cpu.c | 2 +-
> target/i386/monitor.c | 1 -
> target/i386/sev-system-stub.c | 32 --
> target/i386/sev.c | 2 +-
> target/loongarch/loongarch-qmp-cmds.c | 2 +-
> target/mips/system/mips-qmp-cmds.c | 12 +-
> target/ppc/ppc-qmp-cmds.c | 12 +-
> target/riscv/riscv-qmp-cmds.c | 2 +-
> target/s390x/cpu_models_system.c | 2 +-
> tests/qtest/qmp-cmd-test.c | 1 +
Eh I did a very similar series last month which didn't pass local
review, I need to remember what was the issue...
$ git diff --stat eb3020b6ed2baca63..32235d621871df0ccc
MAINTAINERS | 3 ++
qapi/machine-arm-target.json | 47 +++++++++++++++++
qapi/machine-s390x-target.json | 301
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
qapi/machine-target.json | 295
--------------------------------------------------------------------------------------------------------
qapi/{misc-target.json => machine-x86-target.json} | 94
++++++++++++++++++----------------
qapi/machine.json | 44 ----------------
qapi/qapi-schema.json | 4 +-
include/hw/s390x/cpu-topology.h | 2 +-
hw/i386/kvm/xen-stubs.c | 2 +-
hw/i386/kvm/xen_evtchn.c | 2 +-
hw/i386/monitor.c | 2 +-
hw/i386/sgx-stub.c | 2 +-
hw/i386/sgx.c | 2 +-
hw/s390x/s390-skeys.c | 1 -
target/arm/arm-qmp-cmds.c | 3 +-
target/i386/cpu-system.c | 1 +
target/i386/cpu.c | 3 ++
target/i386/monitor.c | 2 +-
target/i386/sev-system-stub.c | 2 +-
target/i386/sev.c | 2 +-
target/s390x/cpu_models_system.c | 3 +-
qapi/meson.build | 4 +-
22 files changed, 423 insertions(+), 398 deletions(-)
$ git log --oneline eb3020b6ed2baca63..32235d621871df0ccc | tac -r
c92b1454ef1 qapi: Restrict ARM specific schemas to ARM
9c708185fe4 qapi: Restrict CpuModelCompareResult to s390x target
c50cc3c438c qapi: Restrict s390x-specific schemas to s390x target
221ec7acf18 qapi: Restrict SGXEPCSection to x86 targets
a3606c5b23c qapi: Restrict x86-specific schemas to x86 targets
32235d62187 qapi: Guard x86 specific schemas with conditional
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema
2025-05-08 14:56 ` [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Philippe Mathieu-Daudé
@ 2025-05-08 14:58 ` Daniel P. Berrangé
0 siblings, 0 replies; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-08 14:58 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Peter Maydell, Paolo Bonzini, Markus Armbruster,
Pierrick Bouvier, Richard Henderson
On Thu, May 08, 2025 at 04:56:03PM +0200, Philippe Mathieu-Daudé wrote:
> On 8/5/25 15:58, Daniel P. Berrangé wrote:
> > Pierrick has proposed a series that introduces a concept of runtime
> > conditionals to the QAPI schema, in order to adapt the TARGET_*
> > conditionals currently used at build time:
> >
> > https://lists.nongnu.org/archive/html/qemu-devel/2025-05/msg01699.html
> >
> > For the sake of comparison & evaluation, this series illustrates the
> > alternative approach that we've discussed of entirely removing any
> > concept of TARGET_* conditionals.
> >
> > With this the QAPI schema varies solely based on CONFIG_* conditionals,
> > and is thus invariant across different target emulators.
> >
> > In this PoC I've taken the minimal effort approach to the problem.
> >
> > The QAPI schema has removed the TARGET_* conditionals and in order to
> > make all the emulators then compile, the stubs/ directory is populated
> > with a bunch of files to provide dummy impls of the target specific QMP
> > commands.
> >
> > This is sufficient to make the current QEMU binaries build successfully.
> >
> > To make the "single binary" concept work, however, would require
> > additional followup work to eliminate the stubs.
> >
> > Instead of having stubs we would need to de-couple the QMP command
> > impl from the machine internals. This would likely require greater
> > use of interfaces and/or virtual method dispatchers on the machine
> > class. This would enable the 'qmp_XXXXX' command impls to exist
> > once. Then they call out to virtual methods on the machine to provide
> > the real impl, and/or generate an error if the virtual method is not
> > implemented for the machine.
>
> Note, I don't see commands "per machine", but per device (including
> CPU). When not specified, we iterate over device implementations / CPUs.
Yep, that makes sense. If its not abstracted via machines, we'll need
to abstract it in some other place in our object model. The right answer
probably varies depending on each feature/command we're considering
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] 41+ messages in thread
* Re: [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema
2025-05-08 13:58 [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Daniel P. Berrangé
` (10 preceding siblings ...)
2025-05-08 14:56 ` [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Philippe Mathieu-Daudé
@ 2025-05-08 21:09 ` Pierrick Bouvier
2025-05-09 9:02 ` Daniel P. Berrangé
2025-05-10 9:28 ` Markus Armbruster
12 siblings, 1 reply; 41+ messages in thread
From: Pierrick Bouvier @ 2025-05-08 21:09 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini,
Markus Armbruster, Richard Henderson
On 5/8/25 6:58 AM, Daniel P. Berrangé wrote:
> Pierrick has proposed a series that introduces a concept of runtime
> conditionals to the QAPI schema, in order to adapt the TARGET_*
> conditionals currently used at build time:
>
> https://lists.nongnu.org/archive/html/qemu-devel/2025-05/msg01699.html
>
> For the sake of comparison & evaluation, this series illustrates the
> alternative approach that we've discussed of entirely removing any
> concept of TARGET_* conditionals.
>
> With this the QAPI schema varies solely based on CONFIG_* conditionals,
> and is thus invariant across different target emulators.
>
> In this PoC I've taken the minimal effort approach to the problem.
>
> The QAPI schema has removed the TARGET_* conditionals and in order to
> make all the emulators then compile, the stubs/ directory is populated
> with a bunch of files to provide dummy impls of the target specific QMP
> commands.
>
> This is sufficient to make the current QEMU binaries build successfully.
>
> To make the "single binary" concept work, however, would require
> additional followup work to eliminate the stubs.
>
> Instead of having stubs we would need to de-couple the QMP command
> impl from the machine internals. This would likely require greater
> use of interfaces and/or virtual method dispatchers on the machine
> class. This would enable the 'qmp_XXXXX' command impls to exist
> once. Then they call out to virtual methods on the machine to provide
> the real impl, and/or generate an error if the virtual method is not
> implemented for the machine.
>
Thanks for posting it Daniel.
I think your approach is pretty neat, and yes, it's much simpler than
having any compile time or runtime conditional to deal with that.
When we talked about that on previous thread, I thought the idea was to
expose *all* the commands to *all* the targets, which I didn't really
understand, considering we have target specific commands by design.
I understand better where you wanted to go, by extracting concerned
commands in dedicated files.
The only downside I can see is that some commands have to be there, but
return an "error, not implemented" at runtime. Fine for me, but some
people may argue against it.
A concern I might have as well is about how we'll deal if we want to
hide some commands in the future, based on various criterias
(is_heterogenenous()?). The mantra "define all, and let the build system
hide things" mantra means you can only have a single definition existing
in the binary, by design. But maybe it's not even a real concern, and I
definitely prefer to see problems before fixing them, so it's definitely
not blocking this approach.
Overall, if I had to choose, I think I would pick your approach instead
of the "runtime conditional" route, it's just simpler and saner overall.
I would be happy to see you take ownership on that, as long as we can
deliver something in the next weeks. I can help on anything as well.
@Markus, @Michael, which approach would you prefer to be followed for QAPI?
Thanks,
Pierrick
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema
2025-05-08 21:09 ` Pierrick Bouvier
@ 2025-05-09 9:02 ` Daniel P. Berrangé
2025-05-09 13:43 ` Markus Armbruster
0 siblings, 1 reply; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-09 9:02 UTC (permalink / raw)
To: Pierrick Bouvier
Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini, Markus Armbruster, Richard Henderson
On Thu, May 08, 2025 at 02:09:37PM -0700, Pierrick Bouvier wrote:
> On 5/8/25 6:58 AM, Daniel P. Berrangé wrote:
> > Pierrick has proposed a series that introduces a concept of runtime
> > conditionals to the QAPI schema, in order to adapt the TARGET_*
> > conditionals currently used at build time:
> >
> > https://lists.nongnu.org/archive/html/qemu-devel/2025-05/msg01699.html
> >
> > For the sake of comparison & evaluation, this series illustrates the
> > alternative approach that we've discussed of entirely removing any
> > concept of TARGET_* conditionals.
> >
> > With this the QAPI schema varies solely based on CONFIG_* conditionals,
> > and is thus invariant across different target emulators.
> >
> > In this PoC I've taken the minimal effort approach to the problem.
> >
> > The QAPI schema has removed the TARGET_* conditionals and in order to
> > make all the emulators then compile, the stubs/ directory is populated
> > with a bunch of files to provide dummy impls of the target specific QMP
> > commands.
> >
> > This is sufficient to make the current QEMU binaries build successfully.
> >
> > To make the "single binary" concept work, however, would require
> > additional followup work to eliminate the stubs.
> >
> > Instead of having stubs we would need to de-couple the QMP command
> > impl from the machine internals. This would likely require greater
> > use of interfaces and/or virtual method dispatchers on the machine
> > class. This would enable the 'qmp_XXXXX' command impls to exist
> > once. Then they call out to virtual methods on the machine to provide
> > the real impl, and/or generate an error if the virtual method is not
> > implemented for the machine.
> >
>
> Thanks for posting it Daniel.
>
> I think your approach is pretty neat, and yes, it's much simpler than having
> any compile time or runtime conditional to deal with that.
>
> When we talked about that on previous thread, I thought the idea was to
> expose *all* the commands to *all* the targets, which I didn't really
> understand, considering we have target specific commands by design.
> I understand better where you wanted to go, by extracting concerned commands
> in dedicated files.
>
> The only downside I can see is that some commands have to be there, but
> return an "error, not implemented" at runtime. Fine for me, but some people
> may argue against it.
>
> A concern I might have as well is about how we'll deal if we want to hide
> some commands in the future, based on various criterias
> (is_heterogenenous()?). The mantra "define all, and let the build system
> hide things" mantra means you can only have a single definition existing in
> the binary, by design. But maybe it's not even a real concern, and I
> definitely prefer to see problems before fixing them, so it's definitely not
> blocking this approach.
I think we have to distinguish between what made sense in the context
of our original design, from what makes sense for our future design(s)
and plans.
I can understand why we took the direction we did historically, but I
think we have unwittingly created problems for ourselves that are
looking increasingly worse than the problems we considered solved.
In the other thread I pointed out my dislike for QAPI schema not being
fully self-describing when we have conditionals present, even today,
but there are other aspects that trouble me more wrt conditionals.
Since the schema is currently target specific, a mgmt app has to probe
for QEMU capabilities once for every target binary. QEMU has ~30 system
binaries, and if querying capabilities takes 250 milliseconds, then
querying all binaries is ~ 7 seconds of work. Libvirt mitigates this
overhead by caching the results of capabilities probes. We periodically
suffer delays when we must invalidate our cache though, typically on
software upgrades, and this is unpleasant for users who think we've
got stuck or just have bad slow code.
Even if we had a QAPI schema that didn't vary per target, this is
repeated probing is tricky to avoid when we have completely independant
binaries. We would need QEMU to have some internal "build id", so that
we could detect that all binaries came from the same build, to let us
avoid re-probing each binary.
With your work to have a single binary that can impersonate any of our
target specific binaries, *if* we had a QAPI schema that didn't vary
per target, we would stand a real chance of being able to eliminate
the need to probe capabilities once for every target.
eg if we assume qemu-system-x86_64 is a symlink to "/usr/bin/qemu-system"
(or whatever you call the common binary), then we only need to probe one
binary - the target of the symlink - we can easily identify that all
the legacy binary symlinks resolve to the same real binary with no special
effort.
Conversely if we want the QAPI schema to be varying at runtime per target,
it means we have to select a target before we can probe the schema in the
binary. If we assume an install with only the new binary and none of the
legacy target specific symlinks, we have a chicken & egg problem though.
How do we know what targets are supported ? We should be able to use QAPI
to query the available targets, but before we can use QAPI we need to
select a target ?
We could make QAPI only expose the pieces of schema that are *not*
specific to a target initially, and then refine the schema once a
target is selected. That would solve the immediate problem, but I
think that kind of cleverness is just postponing the pain
Consider that we eventually want a QEMU binary that can run CPUs from
multiple targets concurrently. That implies that a dynamic QAPI schema
needs to turn on the target specific pieces of multiple targets at once.
At that point a mgmt app fundamentally has to understand ahead of time
that some features are target specific - the existance/absence of
features in the QAPI schema can no longer express what is target
specific, since we must expose the union of features all targets
that are in use.
So as we look to the future of QEMU, IMHO, the concept of using the QAPI
schema definitions to dynamically express what is available per-target
is unhelpful and likely even to be harmful.
Assuming a schema that is fixed at build time ought to simplify QEMU
maintainers' life, and while mgmt apps loose a little info, they will
gain in other ways such as not having to query QEMU capabilities
30 times over.
> Overall, if I had to choose, I think I would pick your approach instead of
> the "runtime conditional" route, it's just simpler and saner overall.
> I would be happy to see you take ownership on that, as long as we can
> deliver something in the next weeks. I can help on anything as well.
I'm fine with you driving this forward, not least because I can't commit
to having any predictable amount of time to write code myself for this.
> @Markus, @Michael, which approach would you prefer to be followed for QAPI?
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema
2025-05-09 9:02 ` Daniel P. Berrangé
@ 2025-05-09 13:43 ` Markus Armbruster
2025-05-09 13:56 ` Daniel P. Berrangé
0 siblings, 1 reply; 41+ messages in thread
From: Markus Armbruster @ 2025-05-09 13:43 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Pierrick Bouvier, qemu-devel, Peter Maydell,
Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Thu, May 08, 2025 at 02:09:37PM -0700, Pierrick Bouvier wrote:
>> On 5/8/25 6:58 AM, Daniel P. Berrangé wrote:
>> > Pierrick has proposed a series that introduces a concept of runtime
>> > conditionals to the QAPI schema, in order to adapt the TARGET_*
>> > conditionals currently used at build time:
>> >
>> > https://lists.nongnu.org/archive/html/qemu-devel/2025-05/msg01699.html
>> >
>> > For the sake of comparison & evaluation, this series illustrates the
>> > alternative approach that we've discussed of entirely removing any
>> > concept of TARGET_* conditionals.
>> >
>> > With this the QAPI schema varies solely based on CONFIG_* conditionals,
>> > and is thus invariant across different target emulators.
>> >
>> > In this PoC I've taken the minimal effort approach to the problem.
>> >
>> > The QAPI schema has removed the TARGET_* conditionals and in order to
>> > make all the emulators then compile, the stubs/ directory is populated
>> > with a bunch of files to provide dummy impls of the target specific QMP
>> > commands.
>> >
>> > This is sufficient to make the current QEMU binaries build successfully.
>> >
>> > To make the "single binary" concept work, however, would require
>> > additional followup work to eliminate the stubs.
>> >
>> > Instead of having stubs we would need to de-couple the QMP command
>> > impl from the machine internals. This would likely require greater
>> > use of interfaces and/or virtual method dispatchers on the machine
>> > class. This would enable the 'qmp_XXXXX' command impls to exist
>> > once. Then they call out to virtual methods on the machine to provide
>> > the real impl, and/or generate an error if the virtual method is not
>> > implemented for the machine.
>> >
>>
>> Thanks for posting it Daniel.
>>
>> I think your approach is pretty neat, and yes, it's much simpler than having
>> any compile time or runtime conditional to deal with that.
>>
>> When we talked about that on previous thread, I thought the idea was to
>> expose *all* the commands to *all* the targets, which I didn't really
>> understand, considering we have target specific commands by design.
>> I understand better where you wanted to go, by extracting concerned commands
>> in dedicated files.
>>
>> The only downside I can see is that some commands have to be there, but
>> return an "error, not implemented" at runtime. Fine for me, but some people
>> may argue against it.
>>
>> A concern I might have as well is about how we'll deal if we want to hide
>> some commands in the future, based on various criterias
>> (is_heterogenenous()?). The mantra "define all, and let the build system
>> hide things" mantra means you can only have a single definition existing in
>> the binary, by design. But maybe it's not even a real concern, and I
>> definitely prefer to see problems before fixing them, so it's definitely not
>> blocking this approach.
>
> I think we have to distinguish between what made sense in the context
> of our original design, from what makes sense for our future design(s)
> and plans.
No argument.
The original design idea is simple: #ifdef CONFIG_FOO for QAPI schema,
to not generate dead code, and to not have silly stubs. Even if you
don't care about wasting disk and address space, you probably care about
wasting developer time on writing silly stubs and waiting for dead code
to compile.
Initially, target-specific macros did not work in conditions. That was
easy enough to fix, so we did.
However, target-specific conditions turned out less useful than
anticipated. Adding a target-specific conditional anywhere in a
definition makes the entire definition and its users target-specific.
This can make handwritten code artificially target-specific, which is
undesirable.
Target-specific conditions now also complicate the single binary work.
> I can understand why we took the direction we did historically, but I
> think we have unwittingly created problems for ourselves that are
> looking increasingly worse than the problems we considered solved.
>
>
> In the other thread I pointed out my dislike for QAPI schema not being
> fully self-describing when we have conditionals present, even today,
> but there are other aspects that trouble me more wrt conditionals.
I'm not sure I have all this present in my mind... Can you summarize
what troubles you? Or point me to the message(s)?
> Since the schema is currently target specific, a mgmt app has to probe
> for QEMU capabilities once for every target binary. QEMU has ~30 system
> binaries, and if querying capabilities takes 250 milliseconds, then
> querying all binaries is ~ 7 seconds of work. Libvirt mitigates this
> overhead by caching the results of capabilities probes. We periodically
> suffer delays when we must invalidate our cache though, typically on
> software upgrades, and this is unpleasant for users who think we've
> got stuck or just have bad slow code.
How does cache invalidation work? Timestamp of binary?
> Even if we had a QAPI schema that didn't vary per target, this is
> repeated probing is tricky to avoid when we have completely independant
> binaries. We would need QEMU to have some internal "build id", so that
> we could detect that all binaries came from the same build, to let us
> avoid re-probing each binary.
Back when I created QAPI/QMP introspection, I floated the idea to put
something into the QMP greeting to enable safe caching. Libvirt
developers told me they didn't need that. I don't remember the details,
but I guess the cache invalidation they already had was deemed good
enough.
> With your work to have a single binary that can impersonate any of our
> target specific binaries, *if* we had a QAPI schema that didn't vary
> per target, we would stand a real chance of being able to eliminate
> the need to probe capabilities once for every target.
This is actually independent of "single binary".
query-qmp-schema's value will be the same for all qemu-system-FOO built
from same QAPI schema source and configuration.
If there are no target-dependent conditionals, schema configuration is
the same regardless of target.
Of course, capability probing can go beyond query-qmp-schema, but you
get the idea.
> eg if we assume qemu-system-x86_64 is a symlink to "/usr/bin/qemu-system"
> (or whatever you call the common binary), then we only need to probe one
> binary - the target of the symlink - we can easily identify that all
> the legacy binary symlinks resolve to the same real binary with no special
> effort.
Have a message digest of query-qmp-schema's value in the QMP greeting.
Management application uses it as key to cache probe results.
> Conversely if we want the QAPI schema to be varying at runtime per target,
> it means we have to select a target before we can probe the schema in the
> binary. If we assume an install with only the new binary and none of the
> legacy target specific symlinks, we have a chicken & egg problem though.
>
> How do we know what targets are supported ? We should be able to use QAPI
> to query the available targets, but before we can use QAPI we need to
> select a target ?
>
> We could make QAPI only expose the pieces of schema that are *not*
> specific to a target initially, and then refine the schema once a
> target is selected. That would solve the immediate problem, but I
> think that kind of cleverness is just postponing the pain
>
> Consider that we eventually want a QEMU binary that can run CPUs from
> multiple targets concurrently. That implies that a dynamic QAPI schema
> needs to turn on the target specific pieces of multiple targets at once.
> At that point a mgmt app fundamentally has to understand ahead of time
> that some features are target specific - the existance/absence of
> features in the QAPI schema can no longer express what is target
> specific, since we must expose the union of features all targets
> that are in use.
>
> So as we look to the future of QEMU, IMHO, the concept of using the QAPI
> schema definitions to dynamically express what is available per-target
> is unhelpful and likely even to be harmful.
>
> Assuming a schema that is fixed at build time ought to simplify QEMU
> maintainers' life, and while mgmt apps loose a little info, they will
> gain in other ways such as not having to query QEMU capabilities
> 30 times over.
Schema fixed at build time is a fundamental design choice we made for
QAPI.
It limits the questions schema introspection can answer. Can affect
valid questions, but that doesn't mean we have to answer them via schema
introspection. We've been tempted to answer them via schema
introspection because schema introspection has been such a success.
We may need to think about replacements better than "add yet another ad
hoc query command" and "QOM tree spelunking". Not a blocker.
>> Overall, if I had to choose, I think I would pick your approach instead of
>> the "runtime conditional" route, it's just simpler and saner overall.
>> I would be happy to see you take ownership on that, as long as we can
>> deliver something in the next weeks. I can help on anything as well.
>
> I'm fine with you driving this forward, not least because I can't commit
> to having any predictable amount of time to write code myself for this.
>
>> @Markus, @Michael, which approach would you prefer to be followed for QAPI?
>
> With regards,
> Daniel
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema
2025-05-09 13:43 ` Markus Armbruster
@ 2025-05-09 13:56 ` Daniel P. Berrangé
2025-05-10 6:08 ` Markus Armbruster
0 siblings, 1 reply; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-09 13:56 UTC (permalink / raw)
To: Markus Armbruster
Cc: Pierrick Bouvier, qemu-devel, Peter Maydell,
Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson
On Fri, May 09, 2025 at 03:43:30PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
> > On Thu, May 08, 2025 at 02:09:37PM -0700, Pierrick Bouvier wrote:
> >> On 5/8/25 6:58 AM, Daniel P. Berrangé wrote:
> >> > Pierrick has proposed a series that introduces a concept of runtime
> >> > conditionals to the QAPI schema, in order to adapt the TARGET_*
> >> > conditionals currently used at build time:
> >> >
> >> > https://lists.nongnu.org/archive/html/qemu-devel/2025-05/msg01699.html
> >> >
> >> > For the sake of comparison & evaluation, this series illustrates the
> >> > alternative approach that we've discussed of entirely removing any
> >> > concept of TARGET_* conditionals.
> >> >
> >> > With this the QAPI schema varies solely based on CONFIG_* conditionals,
> >> > and is thus invariant across different target emulators.
> >> >
> >> > In this PoC I've taken the minimal effort approach to the problem.
> >> >
> >> > The QAPI schema has removed the TARGET_* conditionals and in order to
> >> > make all the emulators then compile, the stubs/ directory is populated
> >> > with a bunch of files to provide dummy impls of the target specific QMP
> >> > commands.
> >> >
> >> > This is sufficient to make the current QEMU binaries build successfully.
> >> >
> >> > To make the "single binary" concept work, however, would require
> >> > additional followup work to eliminate the stubs.
> >> >
> >> > Instead of having stubs we would need to de-couple the QMP command
> >> > impl from the machine internals. This would likely require greater
> >> > use of interfaces and/or virtual method dispatchers on the machine
> >> > class. This would enable the 'qmp_XXXXX' command impls to exist
> >> > once. Then they call out to virtual methods on the machine to provide
> >> > the real impl, and/or generate an error if the virtual method is not
> >> > implemented for the machine.
> >> >
> >>
> >> Thanks for posting it Daniel.
> >>
> >> I think your approach is pretty neat, and yes, it's much simpler than having
> >> any compile time or runtime conditional to deal with that.
> >>
> >> When we talked about that on previous thread, I thought the idea was to
> >> expose *all* the commands to *all* the targets, which I didn't really
> >> understand, considering we have target specific commands by design.
> >> I understand better where you wanted to go, by extracting concerned commands
> >> in dedicated files.
> >>
> >> The only downside I can see is that some commands have to be there, but
> >> return an "error, not implemented" at runtime. Fine for me, but some people
> >> may argue against it.
> >>
> >> A concern I might have as well is about how we'll deal if we want to hide
> >> some commands in the future, based on various criterias
> >> (is_heterogenenous()?). The mantra "define all, and let the build system
> >> hide things" mantra means you can only have a single definition existing in
> >> the binary, by design. But maybe it's not even a real concern, and I
> >> definitely prefer to see problems before fixing them, so it's definitely not
> >> blocking this approach.
> >
> > I think we have to distinguish between what made sense in the context
> > of our original design, from what makes sense for our future design(s)
> > and plans.
>
> No argument.
>
> The original design idea is simple: #ifdef CONFIG_FOO for QAPI schema,
> to not generate dead code, and to not have silly stubs. Even if you
> don't care about wasting disk and address space, you probably care about
> wasting developer time on writing silly stubs and waiting for dead code
> to compile.
>
> Initially, target-specific macros did not work in conditions. That was
> easy enough to fix, so we did.
Ah, yes, forgot that bit of history.
> > I can understand why we took the direction we did historically, but I
> > think we have unwittingly created problems for ourselves that are
> > looking increasingly worse than the problems we considered solved.
> >
> >
> > In the other thread I pointed out my dislike for QAPI schema not being
> > fully self-describing when we have conditionals present, even today,
> > but there are other aspects that trouble me more wrt conditionals.
>
> I'm not sure I have all this present in my mind... Can you summarize
> what troubles you? Or point me to the message(s)?
Oppps, sorry, should have cross-linked to:
https://lists.nongnu.org/archive/html/qemu-devel/2025-05/msg01947.html
> > Since the schema is currently target specific, a mgmt app has to probe
> > for QEMU capabilities once for every target binary. QEMU has ~30 system
> > binaries, and if querying capabilities takes 250 milliseconds, then
> > querying all binaries is ~ 7 seconds of work. Libvirt mitigates this
> > overhead by caching the results of capabilities probes. We periodically
> > suffer delays when we must invalidate our cache though, typically on
> > software upgrades, and this is unpleasant for users who think we've
> > got stuck or just have bad slow code.
>
> How does cache invalidation work? Timestamp of binary?
ctime of libvirt itself and ctime of QEMU binary are the two
primary factors.
We had to throw in other factors on top for various edge cases
over time. So we also check the mtime of the directory containing
QEMU loadable modules, as features reported can vary if the user
installs new device/backend modules. Also check kernel version,
microcode version, CPUID signature, because that can affect
availability of some features.
NB, this is caching more than just the QMP schema - we issue
many 'query-xxxx' commands when probing QEMU.
https://gitlab.com/libvirt/libvirt/-/blame/master/src/qemu/qemu_capabilities.c#L5406
> > Even if we had a QAPI schema that didn't vary per target, this is
> > repeated probing is tricky to avoid when we have completely independant
> > binaries. We would need QEMU to have some internal "build id", so that
> > we could detect that all binaries came from the same build, to let us
> > avoid re-probing each binary.
>
> Back when I created QAPI/QMP introspection, I floated the idea to put
> something into the QMP greeting to enable safe caching. Libvirt
> developers told me they didn't need that. I don't remember the details,
> but I guess the cache invalidation they already had was deemed good
> enough.
I don't recall that discussion, but I would think the problem is
that we probe much more than just QMP schema. Actually thinking
about it, the fact that we probe more than just QMP schema means
my idea of probing once and getting the answer for all targets
may not be practical. Some of the query-xxx commands we run will
likely need to know the target.
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] 41+ messages in thread
* Re: [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema
2025-05-09 13:56 ` Daniel P. Berrangé
@ 2025-05-10 6:08 ` Markus Armbruster
2025-05-12 18:38 ` Daniel P. Berrangé
0 siblings, 1 reply; 41+ messages in thread
From: Markus Armbruster @ 2025-05-10 6:08 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Pierrick Bouvier, qemu-devel, Peter Maydell,
Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Fri, May 09, 2025 at 03:43:30PM +0200, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> > I think we have to distinguish between what made sense in the context
>> > of our original design, from what makes sense for our future design(s)
>> > and plans.
>>
>> No argument.
>>
>> The original design idea is simple: #ifdef CONFIG_FOO for QAPI schema,
>> to not generate dead code, and to not have silly stubs. Even if you
>> don't care about wasting disk and address space, you probably care about
>> wasting developer time on writing silly stubs and waiting for dead code
>> to compile.
>>
>> Initially, target-specific macros did not work in conditions. That was
>> easy enough to fix, so we did.
>
> Ah, yes, forgot that bit of history.
>
>> > I can understand why we took the direction we did historically, but I
>> > think we have unwittingly created problems for ourselves that are
>> > looking increasingly worse than the problems we considered solved.
>> >
>> >
>> > In the other thread I pointed out my dislike for QAPI schema not being
>> > fully self-describing when we have conditionals present, even today,
>> > but there are other aspects that trouble me more wrt conditionals.
>>
>> I'm not sure I have all this present in my mind... Can you summarize
>> what troubles you? Or point me to the message(s)?
>
> Oppps, sorry, should have cross-linked to:
>
> https://lists.nongnu.org/archive/html/qemu-devel/2025-05/msg01947.html
Hadn't gotten to that thread... Thanks!
>> > Since the schema is currently target specific, a mgmt app has to probe
>> > for QEMU capabilities once for every target binary. QEMU has ~30 system
>> > binaries, and if querying capabilities takes 250 milliseconds, then
>> > querying all binaries is ~ 7 seconds of work. Libvirt mitigates this
>> > overhead by caching the results of capabilities probes. We periodically
>> > suffer delays when we must invalidate our cache though, typically on
>> > software upgrades, and this is unpleasant for users who think we've
>> > got stuck or just have bad slow code.
>>
>> How does cache invalidation work? Timestamp of binary?
>
> ctime of libvirt itself and ctime of QEMU binary are the two
> primary factors.
>
> We had to throw in other factors on top for various edge cases
> over time. So we also check the mtime of the directory containing
> QEMU loadable modules, as features reported can vary if the user
> installs new device/backend modules. Also check kernel version,
> microcode version, CPUID signature, because that can affect
> availability of some features.
>
> NB, this is caching more than just the QMP schema - we issue
> many 'query-xxxx' commands when probing QEMU.
>
> https://gitlab.com/libvirt/libvirt/-/blame/master/src/qemu/qemu_capabilities.c#L5406
>
>> > Even if we had a QAPI schema that didn't vary per target, this is
>> > repeated probing is tricky to avoid when we have completely independant
>> > binaries. We would need QEMU to have some internal "build id", so that
>> > we could detect that all binaries came from the same build, to let us
>> > avoid re-probing each binary.
>>
>> Back when I created QAPI/QMP introspection, I floated the idea to put
>> something into the QMP greeting to enable safe caching. Libvirt
>> developers told me they didn't need that. I don't remember the details,
>> but I guess the cache invalidation they already had was deemed good
>> enough.
>
> I don't recall that discussion, but I would think the problem is
> that we probe much more than just QMP schema. Actually thinking
> about it, the fact that we probe more than just QMP schema means
> my idea of probing once and getting the answer for all targets
> may not be practical. Some of the query-xxx commands we run will
> likely need to know the target.
Yes.
We could split the cache along validity conditions. Possibly worth the
extra complexity if expensive probes then live longer in their cache.
I'd love to have a list of probes libvirt runs and why.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema
2025-05-10 6:08 ` Markus Armbruster
@ 2025-05-12 18:38 ` Daniel P. Berrangé
0 siblings, 0 replies; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-12 18:38 UTC (permalink / raw)
To: Markus Armbruster
Cc: Pierrick Bouvier, qemu-devel, Peter Maydell,
Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson
On Sat, May 10, 2025 at 08:08:02AM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
> > On Fri, May 09, 2025 at 03:43:30PM +0200, Markus Armbruster wrote:
> >> Daniel P. Berrangé <berrange@redhat.com> writes:
> >> > Even if we had a QAPI schema that didn't vary per target, this is
> >> > repeated probing is tricky to avoid when we have completely independant
> >> > binaries. We would need QEMU to have some internal "build id", so that
> >> > we could detect that all binaries came from the same build, to let us
> >> > avoid re-probing each binary.
> >>
> >> Back when I created QAPI/QMP introspection, I floated the idea to put
> >> something into the QMP greeting to enable safe caching. Libvirt
> >> developers told me they didn't need that. I don't remember the details,
> >> but I guess the cache invalidation they already had was deemed good
> >> enough.
> >
> > I don't recall that discussion, but I would think the problem is
> > that we probe much more than just QMP schema. Actually thinking
> > about it, the fact that we probe more than just QMP schema means
> > my idea of probing once and getting the answer for all targets
> > may not be practical. Some of the query-xxx commands we run will
> > likely need to know the target.
>
> Yes.
>
> We could split the cache along validity conditions. Possibly worth the
> extra complexity if expensive probes then live longer in their cache.
>
> I'd love to have a list of probes libvirt runs and why.
We don't record the 'why' I'm afraid, so answer the 'why' is pretty
much a case of having to read the code :-(
The list of probes is also somewhat metadata driven, starting
from:
https://gitlab.com/libvirt/libvirt/-/blob/6ff8d08777ebbcb9a1e11534c3a3341fbf0343e8/src/qemu/qemu_capabilities.c?page=6#L5718
you would have to then work backwards for each virQMEUCapsProbeQMP
method we call.
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] 41+ messages in thread
* Re: [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema
2025-05-08 13:58 [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema Daniel P. Berrangé
` (11 preceding siblings ...)
2025-05-08 21:09 ` Pierrick Bouvier
@ 2025-05-10 9:28 ` Markus Armbruster
2025-05-12 18:39 ` Daniel P. Berrangé
12 siblings, 1 reply; 41+ messages in thread
From: Markus Armbruster @ 2025-05-10 9:28 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini, Pierrick Bouvier, Richard Henderson
PATCH 10 empties out and deletes qapi/machine-target.json. PATCH 04
empties out qapi/misc-target.json without deleting it. Missing:
* Delete qapi/misc-target.json
* Delete entry F: qapi/machine-target.json in MAINTAINERS
* Delete dead logic around qapi_specific_outputs in qapi/meson.build
The latter deserves its own PATCH 11. Whether to delete the
qapi/*-target.json in the patch that empties the file or in PATCH 11 is
a matter of taste.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema
2025-05-10 9:28 ` Markus Armbruster
@ 2025-05-12 18:39 ` Daniel P. Berrangé
2025-05-12 20:09 ` Pierrick Bouvier
0 siblings, 1 reply; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-12 18:39 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini, Pierrick Bouvier, Richard Henderson
On Sat, May 10, 2025 at 11:28:59AM +0200, Markus Armbruster wrote:
> PATCH 10 empties out and deletes qapi/machine-target.json. PATCH 04
> empties out qapi/misc-target.json without deleting it. Missing:
>
> * Delete qapi/misc-target.json
>
> * Delete entry F: qapi/machine-target.json in MAINTAINERS
>
> * Delete dead logic around qapi_specific_outputs in qapi/meson.build
>
> The latter deserves its own PATCH 11. Whether to delete the
> qapi/*-target.json in the patch that empties the file or in PATCH 11 is
> a matter of taste.
FYI, after discussing with Pierrick, I'm going to let him take over
work on this patch series to drive it forward to something viable
to submit as a non-RFC.
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] 41+ messages in thread
* Re: [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema
2025-05-12 18:39 ` Daniel P. Berrangé
@ 2025-05-12 20:09 ` Pierrick Bouvier
2025-05-13 7:59 ` Markus Armbruster
0 siblings, 1 reply; 41+ messages in thread
From: Pierrick Bouvier @ 2025-05-12 20:09 UTC (permalink / raw)
To: Daniel P. Berrangé, Markus Armbruster
Cc: qemu-devel, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini, Richard Henderson
On 5/12/25 11:39 AM, Daniel P. Berrangé wrote:
> On Sat, May 10, 2025 at 11:28:59AM +0200, Markus Armbruster wrote:
>> PATCH 10 empties out and deletes qapi/machine-target.json. PATCH 04
>> empties out qapi/misc-target.json without deleting it. Missing:
>>
>> * Delete qapi/misc-target.json
>>
>> * Delete entry F: qapi/machine-target.json in MAINTAINERS
>>
>> * Delete dead logic around qapi_specific_outputs in qapi/meson.build
>>
>> The latter deserves its own PATCH 11. Whether to delete the
>> qapi/*-target.json in the patch that empties the file or in PATCH 11 is
>> a matter of taste.
>
> FYI, after discussing with Pierrick, I'm going to let him take over
> work on this patch series to drive it forward to something viable
> to submit as a non-RFC.
>
Thanks for your work Daniel, I'll continue the effort based on your
approach.
Just for information, I'll apply a Signed-off-by with my name to all
patches posted (including the ones I won't touch) simply because I use
git rebase --signoff by default, and don't want to have to do this
manually and selectively, as I work with a single master, with stacked
branches and update-refs.
Regards,
Pierrick
> With regards,
> Daniel
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema
2025-05-12 20:09 ` Pierrick Bouvier
@ 2025-05-13 7:59 ` Markus Armbruster
2025-05-13 14:36 ` Pierrick Bouvier
0 siblings, 1 reply; 41+ messages in thread
From: Markus Armbruster @ 2025-05-13 7:59 UTC (permalink / raw)
To: Pierrick Bouvier
Cc: Daniel P. Berrangé, qemu-devel, Peter Maydell,
Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson
Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> On 5/12/25 11:39 AM, Daniel P. Berrangé wrote:
>> On Sat, May 10, 2025 at 11:28:59AM +0200, Markus Armbruster wrote:
>>> PATCH 10 empties out and deletes qapi/machine-target.json. PATCH 04
>>> empties out qapi/misc-target.json without deleting it. Missing:
>>>
>>> * Delete qapi/misc-target.json
>>>
>>> * Delete entry F: qapi/machine-target.json in MAINTAINERS
>>>
>>> * Delete dead logic around qapi_specific_outputs in qapi/meson.build
>>>
>>> The latter deserves its own PATCH 11. Whether to delete the
>>> qapi/*-target.json in the patch that empties the file or in PATCH 11 is
>>> a matter of taste.
>>
>> FYI, after discussing with Pierrick, I'm going to let him take over
>> work on this patch series to drive it forward to something viable
>> to submit as a non-RFC.
No worries, you're passing them into capable hands.
> Thanks for your work Daniel, I'll continue the effort based on your
> approach.
>
> Just for information, I'll apply a Signed-off-by with my name to all
> patches posted (including the ones I won't touch) simply because I use
> git rebase --signoff by default, and don't want to have to do this
> manually and selectively, as I work with a single master, with stacked
> branches and update-refs.
We actually expect you to add your S-o-B to patches that go through your
tree whether you changed them or not.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema
2025-05-13 7:59 ` Markus Armbruster
@ 2025-05-13 14:36 ` Pierrick Bouvier
2025-05-13 14:55 ` Daniel P. Berrangé
0 siblings, 1 reply; 41+ messages in thread
From: Pierrick Bouvier @ 2025-05-13 14:36 UTC (permalink / raw)
To: Markus Armbruster
Cc: Daniel P. Berrangé, qemu-devel, Peter Maydell,
Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson
On 5/13/25 12:59 AM, Markus Armbruster wrote:
>> Just for information, I'll apply a Signed-off-by with my name to all
>> patches posted (including the ones I won't touch) simply because I use
>> git rebase --signoff by default, and don't want to have to do this
>> manually and selectively, as I work with a single master, with stacked
>> branches and update-refs.
>
> We actually expect you to add your S-o-B to patches that go through your
> tree whether you changed them or not.
>
Oh ok, it's good then!
I was thinking it was deserved to "I'm the author of this patch".
Thanks,
Pierrick
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 00/10] qapi: remove all TARGET_* conditionals from the schema
2025-05-13 14:36 ` Pierrick Bouvier
@ 2025-05-13 14:55 ` Daniel P. Berrangé
0 siblings, 0 replies; 41+ messages in thread
From: Daniel P. Berrangé @ 2025-05-13 14:55 UTC (permalink / raw)
To: Pierrick Bouvier
Cc: Markus Armbruster, qemu-devel, Peter Maydell,
Philippe Mathieu-Daudé, Paolo Bonzini, Richard Henderson
On Tue, May 13, 2025 at 07:36:03AM -0700, Pierrick Bouvier wrote:
> On 5/13/25 12:59 AM, Markus Armbruster wrote:
> > > Just for information, I'll apply a Signed-off-by with my name to all
> > > patches posted (including the ones I won't touch) simply because I use
> > > git rebase --signoff by default, and don't want to have to do this
> > > manually and selectively, as I work with a single master, with stacked
> > > branches and update-refs.
> >
> > We actually expect you to add your S-o-B to patches that go through your
> > tree whether you changed them or not.
> >
>
> Oh ok, it's good then!
> I was thinking it was deserved to "I'm the author of this patch".
The first one means that.
Essentially the S-o-B lines form a chain tracing the flow of patches.
The first S-o-B will be the original author. Additional S-o-Bs will
be anyone who queued or modified the patch on its way into qemu.git.
Each additional one says that they trusted the person they got it
from and thus they're asserting compliance with the DCO at each step
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] 41+ messages in thread