| Message ID | 20250424183350.1798746-1-pierrick.bouvier@linaro.org |
|---|---|
| Headers | show |
| Series | single-binary: make QAPI generated files common | expand |
On 24/4/25 20:33, Pierrick Bouvier wrote: > QAPI > ==== > > QAPI generated files contain conditional clauses to define various structures, > enums, and commands only for specific targets. This forces files to be > compiled for every target. What we try to do here is to build them only once > instead. > > In the past, we identied that the best approach to solve this is to expose code > for all targets (thus removing all #if clauses), and stub missing > symbols for concerned targets. > > This series build QAPI generated code once, by removing all TARGET_{arch} and > CONFIG_KVM clauses. What it does *not* at the moment is: > - prevent target specific commands to be visible for all targets > (see TODO comment on patch 2 explaining how to address this) + # "#if TARGET_S390X && CONFIG_KVM" will become: + # "if (target_s390x() || kvm_enabled()) {" I like it. > - nothing was done to hide all this from generated documentation > > From what I understood, the only thing that matters is to limit qmp commands > visible. Exposing enums, structure, or events is not a problem, since they > won't be used/triggered for non concerned targets. Please correct me if this is > wrong, and if there are unexpected consequences for libvirt or other consumers. What about function name clashes? I.e.: 389 ## 390 # @query-cpu-definitions: 391 # 392 # Return a list of supported virtual CPU definitions 393 # 394 # Returns: a list of CpuDefinitionInfo 395 # 396 # Since: 1.2 397 ## 398 { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'], 399 'if': { 'any': [ 'TARGET_PPC', 400 'TARGET_ARM', 401 'TARGET_I386', 402 'TARGET_S390X', 403 'TARGET_MIPS', 404 'TARGET_LOONGARCH64', 405 'TARGET_RISCV' ] } } $ git grep qmp.query.cpu.definitions target/arm/arm-qmp-cmds.c:238:CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) target/i386/cpu.c:6418:CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) target/loongarch/loongarch-qmp-cmds.c:30:CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) target/mips/system/mips-qmp-cmds.c:28:CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) target/ppc/ppc-qmp-cmds.c:192:CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) target/riscv/riscv-qmp-cmds.c:56:CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) target/s390x/cpu_models_system.c:85:CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) Prepend target name to these functions and dispatch generated code?
+Marc-André, Daniel & Dave On 24/4/25 20:33, Pierrick Bouvier wrote: > Note: This RFC was posted to trigger a discussion around this topic, and it's > not expected to merge it as it is. > > Context > ======= > > Linaro is working towards heterogeneous emulation, mixing several architectures > in a single QEMU process. The first prerequisite is to be able to build such a > binary, which we commonly name "single-binary" in our various series. > An (incomplete) list of series is available here: > https://patchew.org/search?q=project%3AQEMU+single-binary > > We don't expect to change existing command line interface or any observable > behaviour, it should be identical to existing binaries. If anyone notices a > difference, it will be a bug. > > The first objective we target is to combine qemu-system-arm and > qemu-system-aarch64 in a single binary, showing that we can build and link such > a thing. While being useless from a feature point of view, it allows us to make > good progress towards the goal, and unify two "distinct" architectures, and gain > experience on problems met. > > Our current approach is to remove compilation units duplication to be able to > link all object files together. One of the concerned subsystem is QAPI. > > QAPI > ==== > > QAPI generated files contain conditional clauses to define various structures, > enums, and commands only for specific targets. This forces files to be > compiled for every target. What we try to do here is to build them only once > instead. > > In the past, we identied that the best approach to solve this is to expose code > for all targets (thus removing all #if clauses), and stub missing > symbols for concerned targets. > > This series build QAPI generated code once, by removing all TARGET_{arch} and > CONFIG_KVM clauses. What it does *not* at the moment is: > - prevent target specific commands to be visible for all targets > (see TODO comment on patch 2 explaining how to address this) > - nothing was done to hide all this from generated documentation > > From what I understood, the only thing that matters is to limit qmp commands > visible. Exposing enums, structure, or events is not a problem, since they > won't be used/triggered for non concerned targets. Please correct me if this is > wrong, and if there are unexpected consequences for libvirt or other consumers. > > Impact on code size > =================== > > There is a strong focus on keeping QEMU fast and small. Concerning performance, > there is no impact, as the only thing that would change is to conditionally > check current target to register some commands. > Concerning code size, you can find the impact on various qemu-system binaries > with optimized and stripped build. > > upstream: > 12588 ./build/qemu-system-s390x > 83992 ./build/qemu-system-x86_64 > 31884 ./build/qemu-system-aarch64 > upstream + this series: > 12644 ./build/qemu-system-s390x (+56kB, +0.004%) > 84076 ./build/qemu-system-x86_64 (+84kB, +0.001%) > 31944 ./build/qemu-system-aarch64 (+60kB, +0.001%) > > Feedback > ======== > > The goal of this series is to be spark a conversation around following topics: > > - Would you be open to such an approach? (expose all code, and restrict commands > registered at runtime only for specific targets) > > - Are there unexpected consequences for libvirt or other consumers to expose > more definitions than what we have now? > > - Would you recommend another approach instead? I experimented with having per > target generated files, but we still need to expose quite a lot in headers, so > my opinion is that it's much more complicated for zero benefit. As well, the > code size impact is more than negligible, so the simpler, the better. > > Feel free to add anyone I could have missed in CC. > > Regards, > Pierrick > > Pierrick Bouvier (3): > qapi: add weak stubs for target specific commands > qapi: always expose TARGET_* or CONFIG_KVM code > qapi: make all generated files common > > qapi/commands-weak-stubs.c | 38 ++++++++++++++++++++++++++++++++++++++ > qapi/meson.build | 5 ++++- > scripts/qapi/commands.py | 4 ++++ > scripts/qapi/common.py | 4 +++- > 4 files changed, 49 insertions(+), 2 deletions(-) > create mode 100644 qapi/commands-weak-stubs.c >
On 4/24/25 13:43, Philippe Mathieu-Daudé wrote: > What about function name clashes? I.e.: > > 389 ## > 390 # @query-cpu-definitions: > 391 # > 392 # Return a list of supported virtual CPU definitions > 393 # > 394 # Returns: a list of CpuDefinitionInfo > 395 # > 396 # Since: 1.2 > 397 ## > 398 { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'], > 399 'if': { 'any': [ 'TARGET_PPC', > 400 'TARGET_ARM', > 401 'TARGET_I386', > 402 'TARGET_S390X', > 403 'TARGET_MIPS', > 404 'TARGET_LOONGARCH64', > 405 'TARGET_RISCV' ] } } > > $ git grep qmp.query.cpu.definitions > target/arm/arm-qmp-cmds.c:238:CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) > target/i386/cpu.c:6418:CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) > target/loongarch/loongarch-qmp-cmds.c:30:CpuDefinitionInfoList > *qmp_query_cpu_definitions(Error **errp) > target/mips/system/mips-qmp-cmds.c:28:CpuDefinitionInfoList > *qmp_query_cpu_definitions(Error **errp) > target/ppc/ppc-qmp-cmds.c:192:CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) > target/riscv/riscv-qmp-cmds.c:56:CpuDefinitionInfoList *qmp_query_cpu_definitions(Error > **errp) > target/s390x/cpu_models_system.c:85:CpuDefinitionInfoList *qmp_query_cpu_definitions(Error > **errp) > > Prepend target name to these functions and dispatch generated code? I expect we'll make this function 100% generic based on the TargetInfo API. r~
On 4/24/25 13:43, Philippe Mathieu-Daudé wrote: > What about function name clashes? I.e.: > > 389 ## > 390 # @query-cpu-definitions: > 391 # > 392 # Return a list of supported virtual CPU definitions > 393 # > 394 # Returns: a list of CpuDefinitionInfo > 395 # > 396 # Since: 1.2 > 397 ## > 398 { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'], > 399 'if': { 'any': [ 'TARGET_PPC', > 400 'TARGET_ARM', > 401 'TARGET_I386', > 402 'TARGET_S390X', > 403 'TARGET_MIPS', > 404 'TARGET_LOONGARCH64', > 405 'TARGET_RISCV' ] } } > > $ git grep qmp.query.cpu.definitions > target/arm/arm-qmp-cmds.c:238:CpuDefinitionInfoList > *qmp_query_cpu_definitions(Error **errp) > target/i386/cpu.c:6418:CpuDefinitionInfoList > *qmp_query_cpu_definitions(Error **errp) > target/loongarch/loongarch-qmp-cmds.c:30:CpuDefinitionInfoList > *qmp_query_cpu_definitions(Error **errp) > target/mips/system/mips-qmp-cmds.c:28:CpuDefinitionInfoList > *qmp_query_cpu_definitions(Error **errp) > target/ppc/ppc-qmp-cmds.c:192:CpuDefinitionInfoList > *qmp_query_cpu_definitions(Error **errp) > target/riscv/riscv-qmp-cmds.c:56:CpuDefinitionInfoList > *qmp_query_cpu_definitions(Error **errp) > target/s390x/cpu_models_system.c:85:CpuDefinitionInfoList > *qmp_query_cpu_definitions(Error **errp) > > Prepend target name to these functions and dispatch generated code? In general, either we'll: - unify implementations - create a dispatcher function (based on TargetInfo target_arch()) + renaming existing symbols with suffix _{arch} - we'll create a specific interface for the concerned symbol if needed In this case, given the implementations that are very similar, maybe we can unify them in a single function using target_info()->target_cpu_type(). It's not a problem at the moment, and not directly related to QAPI generated code. We'll have to deal with symbol clashes when we have deduplicated all compilation units. QAPI code generator does not have to solve this.
On Thu, Apr 24, 2025 at 11:33:47AM -0700, Pierrick Bouvier wrote: > Feedback > ======== > > The goal of this series is to be spark a conversation around following topics: > > - Would you be open to such an approach? (expose all code, and restrict commands > registered at runtime only for specific targets) QMP defines a public API between QEMU and external mgmt apps, and personally I like the idea that the API exposed is identical across all binaries and thus the API becomes independent of the impl choice of combined vs separate binaries,. > - Are there unexpected consequences for libvirt or other consumers to expose > more definitions than what we have now? QEMU used the selective hiding of commands in the QMP schema as a mechanism to allow mgmt apps to probe for supported features. We need to check usage of each QMP API feature that's behind a TARGET_* condition and identify which libvirt uses as a feature probe, then come up with a strategy for how best to handle each case in libvirt in future. We might need some additional runtime mechanism to probe for certain things, but we won't know until we look at things in more detail. > - Would you recommend another approach instead? I experimented with having per > target generated files, but we still need to expose quite a lot in headers, so > my opinion is that it's much more complicated for zero benefit. As well, the > code size impact is more than negligible, so the simpler, the better. IMHO it is unfortunate that the API we currently expose has a dependency on a specific impl choice that mgmt apps are expected to rely on for feature probing. An ideal API design is not so closely coupled to impl choice (separate vs combined binaries), and would expose enough functionality such that mgmt apps continue to work regardless of the impl choices. We thought the conditionals were a good thing when we first designed QMP this way. We ended up using two distinct classes of conditionals, one reflecting build time features and one reflecting which target binary is used. I don't think we fully contemplated the implications that the latter set of conditionals would have on our ability to change our impl approach in future. I think the proposal here is taking us in a good direction given what we now know. With regards, Daniel
On 4/25/25 00:35, Daniel P. Berrangé wrote: > On Thu, Apr 24, 2025 at 11:33:47AM -0700, Pierrick Bouvier wrote: >> Feedback >> ======== >> >> The goal of this series is to be spark a conversation around following topics: >> >> - Would you be open to such an approach? (expose all code, and restrict commands >> registered at runtime only for specific targets) > > QMP defines a public API between QEMU and external mgmt apps, and personally I > like the idea that the API exposed is identical across all binaries and thus > the API becomes independent of the impl choice of combined vs separate binaries,. > >> - Are there unexpected consequences for libvirt or other consumers to expose >> more definitions than what we have now? > > QEMU used the selective hiding of commands in the QMP schema as a mechanism > to allow mgmt apps to probe for supported features. We need to check usage > of each QMP API feature that's behind a TARGET_* condition and identify > which libvirt uses as a feature probe, then come up with a strategy for how > best to handle each case in libvirt in future. We might need some additional > runtime mechanism to probe for certain things, but we won't know until we > look at things in more detail. > Could we consider to hide the concerned commands from introspection related commands as well? The same way we prevent those commands to be registered, we can probably prevent them from being visible for libvirt. The code would still be there, and compiled once, but based on runtime target_X() value, it would not appear in introspected schema. I'm not sure how all this is implemented from QAPI code generator, maybe it's hard to do something like this, if we build the schema at compile time for instance. >> - Would you recommend another approach instead? I experimented with having per >> target generated files, but we still need to expose quite a lot in headers, so >> my opinion is that it's much more complicated for zero benefit. As well, the >> code size impact is more than negligible, so the simpler, the better. > > IMHO it is unfortunate that the API we currently expose has a dependency on > a specific impl choice that mgmt apps are expected to rely on for feature > probing. An ideal API design is not so closely coupled to impl choice > (separate vs combined binaries), and would expose enough functionality > such that mgmt apps continue to work regardless of the impl choices. > At this point, do we know which kind of "feature" gets probed? Is it only the list of commands available, or is there probes based on enum/struct definition? If yes, the latter seems to be a wrong way to identify a target, when it could simply use query-target. > We thought the conditionals were a good thing when we first designed QMP > this way. We ended up using two distinct classes of conditionals, one > reflecting build time features and one reflecting which target binary is > used. I don't think we fully contemplated the implications that the latter > set of conditionals would have on our ability to change our impl approach > in future. I think the proposal here is taking us in a good direction > given what we now know. > Thanks for considering an alternative way given the new needs, that's appreciated. Would that possible to get some help from people from libvirt or QAPI developers for this? > With regards, > Daniel Thanks, Pierrick
On 4/25/25 08:38, Markus Armbruster wrote: > Pierrick Bouvier <pierrick.bouvier@linaro.org> writes: > >> Note: This RFC was posted to trigger a discussion around this topic, and it's >> not expected to merge it as it is. >> >> Context >> ======= >> >> Linaro is working towards heterogeneous emulation, mixing several architectures >> in a single QEMU process. The first prerequisite is to be able to build such a >> binary, which we commonly name "single-binary" in our various series. >> An (incomplete) list of series is available here: >> https://patchew.org/search?q=project%3AQEMU+single-binary >> >> We don't expect to change existing command line interface or any observable >> behaviour, it should be identical to existing binaries. If anyone notices a >> difference, it will be a bug. > > Define "notice a difference" :) More on that below. > Given a single-binary *named* exactly like an existing qemu-system-X binary, any user or QEMU management layer should not be able to distinguish it from the real qemu-system-X binary. The new potential things will be: - introduction of an (optional) -target option, which allows to override/disambiguate default target detected. - potentially more boards/cpus/devices visible, once we start developing heterogeneous emulation. See it as a new CONFIG_{new_board} present. Out of that, once the current target is identified, based on argv[0], there should be absolutely no difference, whether in the behaviour, UI, command line, or the monitor interfaces. Maybe (i.e. probably) one day people will be interested to create a new shiny command line for heteregenous scenarios, but for now, this is *not* the goal we pursue. We just want to be able to manually define a board mixing two different cpu architectures, without reinventing all the wheels coming with that. Once everything is ready (and not before), it will be a good time to revisit the command line interface to reflect this. Definitely a small task compared to all we have left to do now. Finally, even if we decide to do those changes, I think they should be reflected on both existing binaries and the new single-binary. It would be a mistake to create "yet another" way to use QEMU, just because we have N architectures available instead of one. >> The first objective we target is to combine qemu-system-arm and >> qemu-system-aarch64 in a single binary, showing that we can build and link such >> a thing. While being useless from a feature point of view, it allows us to make >> good progress towards the goal, and unify two "distinct" architectures, and gain >> experience on problems met. > > Makes sense to me. > >> Our current approach is to remove compilation units duplication to be able to >> link all object files together. One of the concerned subsystem is QAPI. >> >> QAPI >> ==== >> >> QAPI generated files contain conditional clauses to define various structures, >> enums, and commands only for specific targets. This forces files to be >> compiled for every target. > > To be precise: conditionals that use macros restricted to > target-specific code, i.e. the ones poisoned by exec/poison.h. Let's > call them target-specific QAPI conditionals. > > The QAPI generator is blissfully unaware of all this. > Indeed, the only thing QAPI generaor is aware of is that it's a compile time definition, since it implements those with #if, compared to a runtime check. > The build system treats QAPI modules qapi/*-target.json as > target-specific. The .c files generated for them are compiled per > target. See qapi/meson.build. > > Only such target-specific modules can can use target-specific QAPI > conditionals. Use in target-independent modules will generate C that > won't compile. > > Poisoned macros used in qapi/*-target.json: > > CONFIG_KVM > TARGET_ARM > TARGET_I386 > TARGET_LOONGARCH64 > TARGET_MIPS > TARGET_PPC > TARGET_RISCV > TARGET_S390X > >> What we try to do here is to build them only once >> instead. > > You're trying to eliminate target-specific QAPI conditionals. Correct? > Yes, but without impacting the list of commands exposed. Thus, it would be needed to select at runtime to expose/register commands. >> In the past, we identied that the best approach to solve this is to expose code >> for all targets (thus removing all #if clauses), and stub missing >> symbols for concerned targets. > > This affects QAPI/QMP introspection, i.e. the value of query-qmp-schema. > > Management applications can no longer use introspection to find out > whether target-specific things are available. > As asked on my previous email answering Daniel, would that be possible to build the schema dynamically, so we can decide what to expose or not introspection wise? > For instance, query-cpu-definitions is implemented for targets arm, > i386, loongarch, mips, ppc, riscv, and s390x. It initially was for > fewer targets, and more targets followed one by one. Still more may > follow in the future. Right now, management applications can use > introspection to find out whether it is available. That stops working > when you make it available for all targets, stubbed out for the ones > that don't (yet) implement it. > I will repeat, just to be clear, I don't think exposing all commands is a good idea. The current series *does not* do this, simply because I didn't want to huge work for nothing. > Management applications may have to be adjusted for this. > > This is not an attempt to shoot down your approach. I'm merely > demonstrating limitations of your promise "if anyone notices a > difference, it will be a bug." > I stick to this promise :). > Now, we could get really fancy and try to keep introspection the same by > applying conditionals dynamically somehow. I.e. have the single binary > return different introspection values depending on the actual guest's > target. > > This requires fixing the target before introspection. Unless this is > somehow completely transparent (wrapper scripts, or awful hacks based on > the binary's filename, perhaps), management applications may have to be > adjusted to actually do that. > > Applies not just to introspection. Consider query-cpu-definitions > again. It currently returns CPU definitions for *the* target. What > would a single binary's query-cpu-definitions return? The CPU > definitions for *all* its targets? Management applications then receive > CPUs that won't work, which may upset them. To avoid noticable > difference, we again have to fix the target before we look. > > Of course, "fixing the target" stops making sense once we move to > heterogeneous machines with multiple targets. > At this point, I don't have think about what should be the semantic when we'll have multiple targets running simultaneously (expose the union, restrict to the main arch, choose a third way). >> This series build QAPI generated code once, by removing all TARGET_{arch} and >> CONFIG_KVM clauses. What it does *not* at the moment is: >> - prevent target specific commands to be visible for all targets >> (see TODO comment on patch 2 explaining how to address this) >> - nothing was done to hide all this from generated documentation > > For better or worse, generated documentation always contains everything. > Fine for me, it makes sense, as the official documentation published, which is what people will consume primarily, is for all targets. > An argument could be made for stripping out documentation for the stuff > that isn't included in this build. > >> From what I understood, the only thing that matters is to limit qmp commands >> visible. Exposing enums, structure, or events is not a problem, since they >> won't be used/triggered for non concerned targets. Please correct me if this is >> wrong, and if there are unexpected consequences for libvirt or other consumers. > > I'm not sure what you mean by "to limit qmp commands visible". > > QAPI/QMP introspection has all commands and events, and all types > reachable from them. query-qmp-schema returns an array, where each > array element describes one command, event, or type. When a command, > event, or type is conditional in the schema, the element is wrapped in > the #if generated for the condition. > After reading and answering to your valuable email, I definitely think the introspection schema we expose should be adapted, independently of how we build QAPI code (i.e. using #ifdef TARGET or not). Is it something technically hard to achieve? >> >> Impact on code size >> =================== >> >> There is a strong focus on keeping QEMU fast and small. Concerning performance, >> there is no impact, as the only thing that would change is to conditionally >> check current target to register some commands. >> Concerning code size, you can find the impact on various qemu-system binaries >> with optimized and stripped build. >> >> upstream: >> 12588 ./build/qemu-system-s390x >> 83992 ./build/qemu-system-x86_64 >> 31884 ./build/qemu-system-aarch64 >> upstream + this series: >> 12644 ./build/qemu-system-s390x (+56kB, +0.004%) >> 84076 ./build/qemu-system-x86_64 (+84kB, +0.001%) >> 31944 ./build/qemu-system-aarch64 (+60kB, +0.001%) >> >> Feedback >> ======== >> >> The goal of this series is to be spark a conversation around following topics: >> >> - Would you be open to such an approach? (expose all code, and restrict commands >> registered at runtime only for specific targets) > > Yes, if we can find acceptable solutions for the problems that come with > it. > >> - Are there unexpected consequences for libvirt or other consumers to expose >> more definitions than what we have now? > > Maybe. > >> - Would you recommend another approach instead? I experimented with having per >> target generated files, but we still need to expose quite a lot in headers, so >> my opinion is that it's much more complicated for zero benefit. As well, the >> code size impact is more than negligible, so the simpler, the better. >> >> Feel free to add anyone I could have missed in CC. > > I'm throwing in devel@lists.libvirt.org. > >> Regards, >> Pierrick >> >> Pierrick Bouvier (3): >> qapi: add weak stubs for target specific commands >> qapi: always expose TARGET_* or CONFIG_KVM code >> qapi: make all generated files common >> >> qapi/commands-weak-stubs.c | 38 ++++++++++++++++++++++++++++++++++++++ >> qapi/meson.build | 5 ++++- >> scripts/qapi/commands.py | 4 ++++ >> scripts/qapi/common.py | 4 +++- >> 4 files changed, 49 insertions(+), 2 deletions(-) >> create mode 100644 qapi/commands-weak-stubs.c > Thanks, Pierrick
On 4/25/25 14:07, Pierrick Bouvier wrote: >> QAPI/QMP introspection has all commands and events, and all types >> reachable from them. query-qmp-schema returns an array, where each >> array element describes one command, event, or type. When a command, >> event, or type is conditional in the schema, the element is wrapped in >> the #if generated for the condition. >> > > After reading and answering to your valuable email, I definitely think > the introspection schema we expose should be adapted, independently of > how we build QAPI code (i.e. using #ifdef TARGET or not). > > Is it something technically hard to achieve? > From existing json format, we could imagine to change semantic of "if" field to mean "expose in the schema, and register associated commands", instead "introduce ifdefs around this". QAPI generator would not have to know about what is inside the ifs, simply calling expression passed as value, and condition command registering and schema exposition with that.
On 25/4/25 09:35, Daniel P. Berrangé wrote: > On Thu, Apr 24, 2025 at 11:33:47AM -0700, Pierrick Bouvier wrote: >> Feedback >> ======== >> >> The goal of this series is to be spark a conversation around following topics: >> >> - Would you be open to such an approach? (expose all code, and restrict commands >> registered at runtime only for specific targets) > > QMP defines a public API between QEMU and external mgmt apps, and personally I > like the idea that the API exposed is identical across all binaries and thus > the API becomes independent of the impl choice of combined vs separate binaries,. I tried to expose all structures / unions as a first step (not yet commands) but realized even structure fields can be conditional, see @deprecated-props: ## # @CpuModelExpansionInfo: # # The result of a cpu model expansion. # # @model: the expanded CpuModelInfo. # # @deprecated-props: a 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). # # Since: 2.8 ## { 'struct': 'CpuModelExpansionInfo', 'data': { 'model': 'CpuModelInfo', 'deprecated-props' : { 'type': ['str'], 'if': 'TARGET_S390X' } }, 'if': { 'any': [ 'TARGET_S390X', 'TARGET_I386', 'TARGET_ARM', 'TARGET_LOONGARCH64', 'TARGET_RISCV' ] } }
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > On 25/4/25 09:35, Daniel P. Berrangé wrote: >> On Thu, Apr 24, 2025 at 11:33:47AM -0700, Pierrick Bouvier wrote: >>> Feedback >>> ======== >>> >>> The goal of this series is to be spark a conversation around following topics: >>> >>> - Would you be open to such an approach? (expose all code, and restrict commands >>> registered at runtime only for specific targets) >> QMP defines a public API between QEMU and external mgmt apps, and personally I >> like the idea that the API exposed is identical across all binaries and thus >> the API becomes independent of the impl choice of combined vs separate binaries,. > > I tried to expose all structures / unions as a first step (not yet > commands) but realized even structure fields can be conditional, Correct. See docs/devel/qapi-code-gen.rst section "Configuring the schema". > see @deprecated-props: > > ## > # @CpuModelExpansionInfo: > # > # The result of a cpu model expansion. > # > # @model: the expanded CpuModelInfo. > # > # @deprecated-props: a 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). > # > # Since: 2.8 > ## > { 'struct': 'CpuModelExpansionInfo', > 'data': { 'model': 'CpuModelInfo', > 'deprecated-props' : { 'type': ['str'], > 'if': 'TARGET_S390X' } }, > 'if': { 'any': [ 'TARGET_S390X', > 'TARGET_I386', > 'TARGET_ARM', > 'TARGET_LOONGARCH64', > 'TARGET_RISCV' ] } }
Note: This RFC was posted to trigger a discussion around this topic, and it's not expected to merge it as it is. Context ======= Linaro is working towards heterogeneous emulation, mixing several architectures in a single QEMU process. The first prerequisite is to be able to build such a binary, which we commonly name "single-binary" in our various series. An (incomplete) list of series is available here: https://patchew.org/search?q=project%3AQEMU+single-binary We don't expect to change existing command line interface or any observable behaviour, it should be identical to existing binaries. If anyone notices a difference, it will be a bug. The first objective we target is to combine qemu-system-arm and qemu-system-aarch64 in a single binary, showing that we can build and link such a thing. While being useless from a feature point of view, it allows us to make good progress towards the goal, and unify two "distinct" architectures, and gain experience on problems met. Our current approach is to remove compilation units duplication to be able to link all object files together. One of the concerned subsystem is QAPI. QAPI ==== QAPI generated files contain conditional clauses to define various structures, enums, and commands only for specific targets. This forces files to be compiled for every target. What we try to do here is to build them only once instead. In the past, we identied that the best approach to solve this is to expose code for all targets (thus removing all #if clauses), and stub missing symbols for concerned targets. This series build QAPI generated code once, by removing all TARGET_{arch} and CONFIG_KVM clauses. What it does *not* at the moment is: - prevent target specific commands to be visible for all targets (see TODO comment on patch 2 explaining how to address this) - nothing was done to hide all this from generated documentation From what I understood, the only thing that matters is to limit qmp commands visible. Exposing enums, structure, or events is not a problem, since they won't be used/triggered for non concerned targets. Please correct me if this is wrong, and if there are unexpected consequences for libvirt or other consumers. Impact on code size =================== There is a strong focus on keeping QEMU fast and small. Concerning performance, there is no impact, as the only thing that would change is to conditionally check current target to register some commands. Concerning code size, you can find the impact on various qemu-system binaries with optimized and stripped build. upstream: 12588 ./build/qemu-system-s390x 83992 ./build/qemu-system-x86_64 31884 ./build/qemu-system-aarch64 upstream + this series: 12644 ./build/qemu-system-s390x (+56kB, +0.004%) 84076 ./build/qemu-system-x86_64 (+84kB, +0.001%) 31944 ./build/qemu-system-aarch64 (+60kB, +0.001%) Feedback ======== The goal of this series is to be spark a conversation around following topics: - Would you be open to such an approach? (expose all code, and restrict commands registered at runtime only for specific targets) - Are there unexpected consequences for libvirt or other consumers to expose more definitions than what we have now? - Would you recommend another approach instead? I experimented with having per target generated files, but we still need to expose quite a lot in headers, so my opinion is that it's much more complicated for zero benefit. As well, the code size impact is more than negligible, so the simpler, the better. Feel free to add anyone I could have missed in CC. Regards, Pierrick Pierrick Bouvier (3): qapi: add weak stubs for target specific commands qapi: always expose TARGET_* or CONFIG_KVM code qapi: make all generated files common qapi/commands-weak-stubs.c | 38 ++++++++++++++++++++++++++++++++++++++ qapi/meson.build | 5 ++++- scripts/qapi/commands.py | 4 ++++ scripts/qapi/common.py | 4 +++- 4 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 qapi/commands-weak-stubs.c