| Message ID | 20240315130910.15750-1-philmd@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | qapi: Make @query-cpu-definitions command target-agnostic | expand |
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > Hi Alex, Markus, > > Markus mentioned QAPI problems with the heterogeneous emulation > binary. My understanding is, while QAPI can use host-specific > conditional (OS, library available, configure option), it > shouldn't use target-specific ones. "Shouldn't" is too strong in the current state of things. It can be awkward, though. Target-specific macros may be used only in target-specific code, i.e. code that is compiled per target. To catch uses in target-independent code, we poison them there; see exec/poison.h. QAPI-generated .c are not normally compiled per target. To enable use of target-specific macros in conditionals, we compile .c generated QAPI submodules named *-target.json per target. This is a bit of a hack. Since the same conditionals will also appear in the .h generated from them, these headers can only be used in target-specific code. Sometimes, these headers "infect" target-independent code: we need to compile per target just for the headers. Awkward. I can dig out an example if there's interest. But what about possible future state of things? The QAPI schema is compile-time static by design. QAPI conditionals permit adjusting the schema for build configuration. Target-specific conditionals adjust it for the build configuration of the target-specific part. Each QEMU binary contains just one target's target-specific part. However, a single QEMU binary will contain several target-specific parts, one per target it supports. The targets' QAPI schema adjustments may conflict. For a single binary, we need to resolve these conflicts. Special case: QAPI definitions that exist only for some targets. Example: command query-sev exists only for TARGET_I386. It's actually a stub when CONFIG_SEV is off. Obvious solution: make it exist if it needs to exist for any target compiled into the binary. Requires command stubs for the other targets. Example: query-sev now exists when the single binary supports x86. It's a stub when CONFIG_SEV is off. It behaves like a stub when CONFIG_SEV is on, but the machine isn't x86. Drawback: introspection with query-qmp-schema becomes less precise. When the binary supports just one target, introspection can answer target-specific questions like "does this target support SEV?" With a single binary, that's no longer possible. Harmless enough for SEV, but consider query-cpu-model-expansion. The command may support expansion types "full" and "static". Currently, s390x supports both, ARM supports only "full", Loongarch only "static", RISC-V only "full", and x86 supports both. (I think. The documentation is incomplete: # Some architectures may not support all expansion types. s390x # supports "full" and "static". Arm only supports "full". ) A management application may want to find out which expansion type is supported. Right now, it can't. But we could improve the schema so it can find out via introspection: define the expansion types only when they're actually supported. With a single binary, that's no longer possible: we have to define an expansion type when *any* target supports it. Such loss of introspection power is not a show-stopper, just something we need to keep in mind. > This series is an example on how to remove target specific > bits from the @query-cpu-definitions command. This is an instance of the special case with an additional twist: each target defines its own QMP command handler. > Target specific > code is registered as CPUClass handlers, then a generic method > is used, iterating over all targets built in. > > The first set of patches were already posted / reviewed last > year. > > The PPC and S390X targets still need work (help welcomed), > however the code is useful enough to be tested and see if this > is a good approach. > > The only drawback is a change in QAPI introspection, because > targets not implementing @query-cpu-definitions were returning > "CommandNotFound". The change is introspection is actually something else. Before the series, query-qmp-schema returns a description of command query-cpu-definitions exactly when the (single) target supports it. Afterwards, it always returns one (I think). The CommandNotFound change is a change in behavior when you try to execute query-cpu-definitions, but the target doesn't support it. Before, the command doesn't exist, and the QMP core duly replies CommandNotFound. Afterwards, it does exist, but the target doesn't implement the call back, so the command fails. I guess it fails with GenericError. We could make it fail with CommandNotFound if we care. > My view is this was an incomplete > implementation, rather than a feature. Feels fair (but I'm not an expert in this area).
The general approach looks sane enough to me.
Hi Alex, Markus, Markus mentioned QAPI problems with the heterogeneous emulation binary. My understanding is, while QAPI can use host-specific conditional (OS, library available, configure option), it shouldn't use target-specific ones. This series is an example on how to remove target specific bits from the @query-cpu-definitions command. Target specific code is registered as CPUClass handlers, then a generic method is used, iterating over all targets built in. The first set of patches were already posted / reviewed last year. The PPC and S390X targets still need work (help welcomed), however the code is useful enough to be tested and see if this is a good approach. The only drawback is a change in QAPI introspection, because targets not implementing @query-cpu-definitions were returning "CommandNotFound". My view is this was an incomplete implementation, rather than a feature. Regards, Phil. Philippe Mathieu-Daudé (21): target/i386: Declare CPU QOM types using DEFINE_TYPES() macro target/mips: Declare CPU QOM types using DEFINE_TYPES() macro target/ppc: Declare CPU QOM types using DEFINE_TYPES() macro target/sparc: Declare CPU QOM types using DEFINE_TYPES() macro cpus: Open code OBJECT_DECLARE_TYPE() in OBJECT_DECLARE_CPU_TYPE() target/i386: Make X86_CPU common to new I386_CPU / X86_64_CPU types target/mips: Make MIPS_CPU common to new MIPS32_CPU / MIPS64_CPU types target/sparc: Make SPARC_CPU common to new SPARC32_CPU/SPARC64_CPU types qapi: Merge machine-common.json with qapi/machine.json qapi: Make CpuModel* definitions target agnostic qapi: Make CpuDefinitionInfo target agnostic system: Introduce QemuArchBit enum system: Introduce cpu_typename_by_arch_bit() system: Introduce QMP generic_query_cpu_definitions() target/arm: Use QMP generic_query_cpu_definitions() target/loongarch: Use QMP generic_query_cpu_definitions() target/riscv: Use QMP generic_query_cpu_definitions() target/i386: Use QMP generic_query_cpu_definitions() target/ppc: Factor ppc_add_alias_definitions() out target/ppc: Use QMP generic_query_cpu_definitions() qapi: Make @query-cpu-definitions target-agnostic MAINTAINERS | 3 +- qapi/machine-common.json | 21 ---- qapi/machine-target.json | 167 +------------------------- qapi/machine.json | 166 ++++++++++++++++++++++++- qapi/qapi-schema.json | 1 - include/hw/core/cpu.h | 7 +- include/hw/core/sysemu-cpu-ops.h | 14 +++ include/sysemu/arch_init.h | 71 +++++++---- target/i386/cpu-qom.h | 16 ++- target/mips/cpu-qom.h | 13 +- target/ppc/cpu-models.h | 4 + target/riscv/cpu.h | 2 + target/s390x/cpu.h | 2 +- target/sparc/cpu-qom.h | 9 +- system/cpu-qmp-cmds.c | 71 +++++++++++ system/cpu-qom-helpers.c | 58 +++++++++ target/arm/arm-qmp-cmds.c | 27 ----- target/i386/cpu.c | 77 ++++++------ target/loongarch/loongarch-qmp-cmds.c | 25 ---- target/mips/cpu.c | 34 ++++-- target/mips/sysemu/mips-qmp-cmds.c | 31 ----- target/ppc/cpu_init.c | 53 ++++---- target/ppc/ppc-qmp-cmds.c | 31 +---- target/riscv/cpu.c | 1 + target/riscv/riscv-qmp-cmds.c | 13 +- target/sparc/cpu.c | 35 ++++-- tests/qtest/cpu-plug-test.c | 2 +- qapi/meson.build | 1 - system/meson.build | 2 + 29 files changed, 515 insertions(+), 442 deletions(-) delete mode 100644 qapi/machine-common.json create mode 100644 system/cpu-qmp-cmds.c create mode 100644 system/cpu-qom-helpers.c