Peter Krempa <pkre...@redhat.com> writes:
> On Fri, Apr 25, 2025 at 17:38:44 +0200, Markus Armbruster via Devel wrote:
>> Pierrick Bouvier <pierrick.bouv...@linaro.org> writes:
>
> [...]
>
>> 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.
>>
>> 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
Commands and events:
CPU introspection: query-cpu-model-baseline, query-cpu-model-comparison,
query-cpu-model-expansion, query-cpu-definitions
S390 KVM CPU stuff: set-cpu-topology, CPU_POLARIZATION_CHANGE,
query-s390x-cpu-polarization.
GIC: query-gic-capabilities
SEV: query-sev, query-sev-launch-measure, query-sev-capabilities,
sev-inject-launch-secret, query-sev-attestation-report
SGX: query-sgx, query-sgx-capabilities
Xen: xen-event-list, xen-event-inject
An odd duck: rtc-reset-reinjection
> I've had a look at what bits of the QMP schema are depending on the
> above defines which libvirt uses.
>
> In many cases libvirt could restrict the use of given command/property
> to only supported architectures. We decided to simply probe the presence
> of the command because it's convenient to not have to filter them any
> more
>
> - query-gic-capabilities
> - libvirt already calls this only for ARM guests based on the
> definition
>
> - query-sev and friends
> - libvirt uses presence of 'query-sev' to decide if the binary
> supports it; patching in a platofrm check is possible although
> inconvenient
>
> - query-sgx and friends
> - similar to sev
>
> -query-cpu-definitions and friends
> - see below
Large subset of my list.
>> > What we try to do here is to build them only
>> > once
>> instead.
>>
>> You're trying to eliminate target-specific QAPI conditionals. Correct?
>>
>> > 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.
>
> Indeed and libvirt already uses this in few cases as noded above.
>
>>
>> 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.
>>
>> 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."
>>
>> 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.
>
> I wonder how this will work if libvirt is probing a binary. Libvirt does
> not look at the filename. It can't because it can be a
> user-specified/compiled binary, override script, or a distro that chose
> to rename the binary.
>
> The second thing that libvirt does after 'query-version' is
> 'query-target'.
>
> So what should libvirt do once multiple targets are supported?
>
> How do we query CPUs for each of the supported targets?
>
> Will the result be the same if we query them one at a time or all at
> once?
Pierrick's stated goal is to have no noticable differences between the
single binary and the qemu-system-<target> it covers. This is obviously
impossible if we can interact with the single binary before the target
is fixed.
>> 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.
>
> As noted filename will not work. Users can specify any filename and
> create override scripts or rename the binary.
True.
>> 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.
>
> Ah I see you had a similar question :D
>
>>
>> Of course, "fixing the target" stops making sense once we move to
>> heterogeneous machines with multiple 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
