| Message ID | 20250305153929.43687-5-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/microblaze: Quick single binary proof of concept | expand |
On 3/5/25 07:39, Philippe Mathieu-Daudé wrote: > Introduce an API to get information specific to a binary > from the binary name (argv[0]). > > Initialize it from qemu_init() on system emulation. > What we want here is more a include/qemu/target_info.h, which will allow to query the name of it, and helper for every architecture: target_is_aarch64() target_is_ppc64() ... Eventually, we can add combined getters like: target_is_64bit() ... Naming "legacy" something that will be present in the long term is not the best move I think. > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > meson.build | 2 +- > include/qemu/legacy_binary_info.h | 14 +++ > legacy_binary_info.c | 160 ++++++++++++++++++++++++++++++ > system/vl.c | 2 + > 4 files changed, 177 insertions(+), 1 deletion(-) > create mode 100644 include/qemu/legacy_binary_info.h > create mode 100644 legacy_binary_info.c > > diff --git a/meson.build b/meson.build > index eaae1da2e92..e4ede6ba06f 100644 > --- a/meson.build > +++ b/meson.build > @@ -3767,7 +3767,7 @@ if have_block > endif > endif > > -common_ss.add(files('cpu-common.c')) > +common_ss.add(files('cpu-common.c', 'legacy_binary_info.c')) > specific_ss.add(files('cpu-target.c', 'arch_info-target.c')) > > subdir('system') > diff --git a/include/qemu/legacy_binary_info.h b/include/qemu/legacy_binary_info.h > new file mode 100644 > index 00000000000..ae67399ebf2 > --- /dev/null > +++ b/include/qemu/legacy_binary_info.h > @@ -0,0 +1,14 @@ > +/* > + * QEMU legacy binary helpers > + * > + * Copyright (c) Linaro > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + */ > + > +#ifndef QEMU_LEGACY_BINARY_INFO_H > +#define QEMU_LEGACY_BINARY_INFO_H > + > +void legacy_binary_info_init(const char *argv0); > + > +#endif > diff --git a/legacy_binary_info.c b/legacy_binary_info.c > new file mode 100644 > index 00000000000..0c50fc9248a > --- /dev/null > +++ b/legacy_binary_info.c > @@ -0,0 +1,160 @@ > +/* > + * QEMU legacy binary helpers > + * > + * Copyright (c) Linaro > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + */ > + > +#include "qemu/osdep.h" > +#include "qemu/arch_info.h" > +#include "qemu/legacy_binary_info.h" > + > +typedef struct LegacyBinaryInfo { > + const char *binary_name; > + QemuArchBit arch_bit; > +} LegacyBinaryInfo; > + > +/* List alphabetically sorted by binary_name */ > +static const LegacyBinaryInfo legacy_binary_infos[] = { > + { > + .binary_name = "qemu-system-aarch64", > + .arch_bit = QEMU_ARCH_BIT_ARM, > + }, > + { > + .binary_name = "qemu-system-alpha", > + .arch_bit = QEMU_ARCH_BIT_ALPHA, > + }, > + { > + .binary_name = "qemu-system-arm", > + .arch_bit = QEMU_ARCH_BIT_ARM, > + }, > + { > + .binary_name = "qemu-system-avr", > + .arch_bit = QEMU_ARCH_BIT_AVR, > + }, > + { > + .binary_name = "qemu-system-hppa", > + .arch_bit = QEMU_ARCH_BIT_HPPA, > + }, > + { > + .binary_name = "qemu-system-i386", > + .arch_bit = QEMU_ARCH_BIT_I386, > + }, > + { > + .binary_name = "qemu-system-loongarch64", > + .arch_bit = QEMU_ARCH_BIT_LOONGARCH, > + }, > + { > + .binary_name = "qemu-system-m68k", > + .arch_bit = QEMU_ARCH_BIT_M68K, > + }, > + { > + .binary_name = "qemu-system-microblaze", > + .arch_bit = QEMU_ARCH_BIT_MICROBLAZE, > + }, > + { > + .binary_name = "qemu-system-microblazeel", > + .arch_bit = QEMU_ARCH_BIT_MICROBLAZE, > + }, > + { > + .binary_name = "qemu-system-mips", > + .arch_bit = QEMU_ARCH_BIT_MIPS, > + }, > + { > + .binary_name = "qemu-system-mips64", > + .arch_bit = QEMU_ARCH_BIT_MIPS, > + }, > + { > + .binary_name = "qemu-system-mips64el", > + .arch_bit = QEMU_ARCH_BIT_MIPS, > + }, > + { > + .binary_name = "qemu-system-mipsel", > + .arch_bit = QEMU_ARCH_BIT_MIPS, > + }, > + { > + .binary_name = "qemu-system-or1k", > + .arch_bit = QEMU_ARCH_BIT_OPENRISC, > + }, > + { > + .binary_name = "qemu-system-ppc", > + .arch_bit = QEMU_ARCH_BIT_PPC, > + }, > + { > + .binary_name = "qemu-system-ppc64", > + .arch_bit = QEMU_ARCH_BIT_PPC, > + }, > + { > + .binary_name = "qemu-system-riscv32", > + .arch_bit = QEMU_ARCH_BIT_RISCV, > + }, > + { > + .binary_name = "qemu-system-riscv64", > + .arch_bit = QEMU_ARCH_BIT_RISCV, > + }, > + { > + .binary_name = "qemu-system-rx", > + .arch_bit = QEMU_ARCH_BIT_RX, > + }, > + { > + .binary_name = "qemu-system-s390x", > + .arch_bit = QEMU_ARCH_BIT_S390X, > + }, > + { > + .binary_name = "qemu-system-sh4", > + .arch_bit = QEMU_ARCH_BIT_SH4, > + }, > + { > + .binary_name = "qemu-system-sh4eb", > + .arch_bit = QEMU_ARCH_BIT_SH4, > + }, > + { > + .binary_name = "qemu-system-sparc", > + .arch_bit = QEMU_ARCH_BIT_SPARC, > + }, > + { > + .binary_name = "qemu-system-sparc64", > + .arch_bit = QEMU_ARCH_BIT_SPARC, > + }, > + { > + .binary_name = "qemu-system-tricore", > + .arch_bit = QEMU_ARCH_BIT_TRICORE, > + }, > + { > + .binary_name = "qemu-system-x86_64", > + .arch_bit = QEMU_ARCH_BIT_I386, > + }, > + { > + .binary_name = "qemu-system-xtensa", > + .arch_bit = QEMU_ARCH_BIT_XTENSA, > + }, > + { > + .binary_name = "qemu-system-xtensaeb", > + .arch_bit = QEMU_ARCH_BIT_XTENSA, > + }, > +}; > + > +static int current_index = -1; > + > +void legacy_binary_info_init(const char *argv0) > +{ > + g_auto(GStrv) tokens = g_strsplit(argv0, G_DIR_SEPARATOR_S, -1); > + unsigned count = 0; > + const char *binary_name; > + > + while (tokens[count]) { > + count++; > + } > + assert(count > 0); > + binary_name = tokens[count - 1]; > + > + for (size_t i = 0; i < ARRAY_SIZE(legacy_binary_infos); i++) { > + if (!strcmp(legacy_binary_infos[i].binary_name, binary_name)) { > + current_index = i; > + return; > + } > + } > + fprintf(stderr, "Missing legacy info for '%s' binary.\n", binary_name); > + abort(); > +} > diff --git a/system/vl.c b/system/vl.c > index a41ba4a2d5f..74a062c7fff 100644 > --- a/system/vl.c > +++ b/system/vl.c > @@ -28,6 +28,7 @@ > #include "qemu/units.h" > #include "qemu/module.h" > #include "qemu/arch_info.h" > +#include "qemu/legacy_binary_info.h" > #include "exec/cpu-common.h" > #include "exec/page-vary.h" > #include "hw/qdev-properties.h" > @@ -2883,6 +2884,7 @@ void qemu_init(int argc, char **argv) > > error_init(argv[0]); > qemu_init_exec_dir(argv[0]); > + legacy_binary_info_init(argv[0]); > > os_setup_limits(); >
Il mer 5 mar 2025, 16:39 Philippe Mathieu-Daudé <philmd@linaro.org> ha scritto: > + for (size_t i = 0; i < ARRAY_SIZE(legacy_binary_infos); i++) { > + if (!strcmp(legacy_binary_infos[i].binary_name, binary_name)) { > + current_index = i; > + return; > + } > + } > + fprintf(stderr, "Missing legacy info for '%s' binary.\n", > binary_name); > Wouldn't this crash if a binary is renamed to qemu-kvm or anything else but its original name? There should be a default target in the binary, and this function should only be called it there's none; but it should also use the normal Error** interface instead of aborting. Also do not call things legacy, as Pierrick also said and explained. Paolo + abort(); > +} > diff --git a/system/vl.c b/system/vl.c > index a41ba4a2d5f..74a062c7fff 100644 > --- a/system/vl.c > +++ b/system/vl.c > @@ -28,6 +28,7 @@ > #include "qemu/units.h" > #include "qemu/module.h" > #include "qemu/arch_info.h" > +#include "qemu/legacy_binary_info.h" > #include "exec/cpu-common.h" > #include "exec/page-vary.h" > #include "hw/qdev-properties.h" > @@ -2883,6 +2884,7 @@ void qemu_init(int argc, char **argv) > > error_init(argv[0]); > qemu_init_exec_dir(argv[0]); > + legacy_binary_info_init(argv[0]); > > os_setup_limits(); > > -- > 2.47.1 > >
On 3/5/25 07:39, Philippe Mathieu-Daudé wrote: > +void legacy_binary_info_init(const char *argv0) > +{ > + g_auto(GStrv) tokens = g_strsplit(argv0, G_DIR_SEPARATOR_S, -1); > + unsigned count = 0; > + const char *binary_name; > + > + while (tokens[count]) { > + count++; > + } > + assert(count > 0); > + binary_name = tokens[count - 1]; This is g_path_get_basename(). > + > + for (size_t i = 0; i < ARRAY_SIZE(legacy_binary_infos); i++) { > + if (!strcmp(legacy_binary_infos[i].binary_name, binary_name)) { > + current_index = i; > + return; > + } > + } > + fprintf(stderr, "Missing legacy info for '%s' binary.\n", binary_name); > + abort(); > +} I'm with Paolo that this should not abort here; Error is better. Even if the caller supplies error_fatal. When testing for errors before and after a patch, I often rename the binary, e.g. qemu-system-aarch64-good / qemu-system-aarch64-bad. Leaving it in the same build directory is required in order to let it find the uninstalled rom images. Is there a way we can preserve something akin to this? Do we need to add the -target command-line option that Pierrick mooted? r~
On 05/03/2025 17.59, Pierrick Bouvier wrote: > On 3/5/25 07:39, Philippe Mathieu-Daudé wrote: >> Introduce an API to get information specific to a binary >> from the binary name (argv[0]). >> >> Initialize it from qemu_init() on system emulation. >> > > What we want here is more a include/qemu/target_info.h, which will allow to > query the name of it, and helper for every architecture: > > target_is_aarch64() > target_is_ppc64() > ... > > Eventually, we can add combined getters like: > target_is_64bit() > ... > > Naming "legacy" something that will be present in the long term is not the > best move I think. FWIW, I agree, this should rather be target_is_64bit() or something similar, like target_words_bigendian() ? Thomas
On 6/3/25 08:26, Thomas Huth wrote: > On 05/03/2025 17.59, Pierrick Bouvier wrote: >> On 3/5/25 07:39, Philippe Mathieu-Daudé wrote: >>> Introduce an API to get information specific to a binary >>> from the binary name (argv[0]). >>> >>> Initialize it from qemu_init() on system emulation. >>> >> >> What we want here is more a include/qemu/target_info.h, which will >> allow to query the name of it, and helper for every architecture: >> >> target_is_aarch64() >> target_is_ppc64() >> ... >> >> Eventually, we can add combined getters like: >> target_is_64bit() >> ... >> >> Naming "legacy" something that will be present in the long term is not >> the best move I think. > > FWIW, I agree, this should rather be target_is_64bit() or something > similar, like target_words_bigendian() ? This API is to allow refactoring code for heterogeneous emulation, without changing user-facing behavior of current qemu-system binaries, which I now consider as 'legacy'. Once all current restrictions removed, the new qemu-system-heterogeneous binary is expected to run any combination of targets. qemu-system-$target will be a call to qemu-system-heterogeneous with a restricted subset, possibly in the form of: $ qemu-system-heterogeneous --target aarch64-softmmu ^ equivalent of today's qemu-system-aarch64 If you don't like 'qemu_legacy_binary_' prefix, I can use 'qemu_single_binary_' instead. target_is_64bit() is misleading, for example in: $ qemu-system-aarch64 -M zynqmp we create 64-bit and 32-bit ARM cores.
Il gio 6 mar 2025, 10:27 Philippe Mathieu-Daudé <philmd@linaro.org> ha scritto: > This API is to allow refactoring code for heterogeneous emulation, > without changing user-facing behavior of current qemu-system binaries, > which I now consider as 'legacy'. > > Once all current restrictions removed, the new qemu-system-heterogeneous > binary is expected to run any combination of targets. > > qemu-system-$target will be a call to qemu-system-heterogeneous with > a restricted subset, possibly in the form of: > > $ qemu-system-heterogeneous --target aarch64-softmmu > Or just qemu-system I guess. ^ equivalent of today's qemu-system-aarch64 > > If you don't like 'qemu_legacy_binary_' prefix, I can use > 'qemu_single_binary_' instead. > Still there is a problem with renaming binaries (both the "qemu-kvm" case and the good/bad case that Richard pointed out). I think you should try creating two versions of system/arch_init.c, so that it has a separate implementation for heterogeneous vs. single-target binaries. Then you can keep separate linking steps for single-target binaries and you naturally get the right target info from either the target-specific arch_init-single.c, or the --target option for arch_init-multi.c. (Is --target even necessary? As long as you have a way disambiguate same-named machines like -M virt, and have no default machine in the multi-target binary, you shouldn't need it). target_is_64bit() is misleading, for example in: > > $ qemu-system-aarch64 -M zynqmp > > we create 64-bit and 32-bit ARM cores. > Agreed, bit size and endianness should only matter in the CPU code. Paolo >
On Thu, Mar 06, 2025 at 12:34:13PM +0100, Paolo Bonzini wrote: > Il gio 6 mar 2025, 10:27 Philippe Mathieu-Daudé <philmd@linaro.org> ha > scritto: > > > This API is to allow refactoring code for heterogeneous emulation, > > without changing user-facing behavior of current qemu-system binaries, > > which I now consider as 'legacy'. > > > > Once all current restrictions removed, the new qemu-system-heterogeneous > > binary is expected to run any combination of targets. > > > > qemu-system-$target will be a call to qemu-system-heterogeneous with > > a restricted subset, possibly in the form of: > > > > $ qemu-system-heterogeneous --target aarch64-softmmu > > > > Or just qemu-system I guess. > > ^ equivalent of today's qemu-system-aarch64 > > > > If you don't like 'qemu_legacy_binary_' prefix, I can use > > 'qemu_single_binary_' instead. > > > > Still there is a problem with renaming binaries (both the "qemu-kvm" case > and the good/bad case that Richard pointed out). We could special case the '-kvm' suffix, because by its nature it implies the current binary build target. > > I think you should try creating two versions of system/arch_init.c, so that > it has a separate implementation for heterogeneous vs. single-target > binaries. Then you can keep separate linking steps for single-target > binaries and you naturally get the right target info from either the > target-specific arch_init-single.c, or the --target option for > arch_init-multi.c. > > (Is --target even necessary? As long as you have a way disambiguate > same-named machines like -M virt, and have no default machine in the > multi-target binary, you shouldn't need it). If we did 'query-machines' on qemu-system-heterogeneous, it would return all machines from all targets. To disambiguate naming there are various options * The query-machines command would have to gain a new 'target' field and we would have to document that uniqness is across the tuple (name, target), not merely name. That's a semantic change. We would still need a way to express the 'target' when asking to instantiate a machine * The query-machines command would have to gain a new 'target' paramter so callers can restrict the data they receive back We would still need a way to express the 'target' when asking to instantiate a machine * Rename all machine types so they are '<target>-<machine>' The query-machines command doesn't change. Apps would have to "parse" the machine name to see what 'target' each is associated with, or we include an explicit 'target' field in the returned data. Instianting a machine would not need changing * Require --target CLI arg, meaning query-machines remains unchanged, as does instantiating machines Any other options ? The last is the simplest option if we just make --target be defaulted based on the binary name. With regards, Daniel
On Wed, Mar 05, 2025 at 05:56:46PM -0800, Richard Henderson wrote: > On 3/5/25 07:39, Philippe Mathieu-Daudé wrote: > > +void legacy_binary_info_init(const char *argv0) > > +{ > > + g_auto(GStrv) tokens = g_strsplit(argv0, G_DIR_SEPARATOR_S, -1); > > + unsigned count = 0; > > + const char *binary_name; > > + > > + while (tokens[count]) { > > + count++; > > + } > > + assert(count > 0); > > + binary_name = tokens[count - 1]; > > This is g_path_get_basename(). > > > + > > + for (size_t i = 0; i < ARRAY_SIZE(legacy_binary_infos); i++) { > > + if (!strcmp(legacy_binary_infos[i].binary_name, binary_name)) { > > + current_index = i; > > + return; > > + } > > + } > > + fprintf(stderr, "Missing legacy info for '%s' binary.\n", binary_name); > > + abort(); > > +} > > I'm with Paolo that this should not abort here; Error is better. > Even if the caller supplies error_fatal. > > When testing for errors before and after a patch, I often rename > the binary, e.g. qemu-system-aarch64-good / qemu-system-aarch64-bad. > Leaving it in the same build directory is required in order to let > it find the uninstalled rom images. > > Is there a way we can preserve something akin to this? Replacign !strcmp with g_str_has_prefix in the above code would be an simple option to preseve the use of custom suffixes, along with special casing on existence of 'kvm' in the name. With regards, Daniel
On Thu, 6 Mar 2025, Daniel P. Berrangé wrote: > On Thu, Mar 06, 2025 at 12:34:13PM +0100, Paolo Bonzini wrote: >> Il gio 6 mar 2025, 10:27 Philippe Mathieu-Daudé <philmd@linaro.org> ha >> scritto: >> >>> This API is to allow refactoring code for heterogeneous emulation, >>> without changing user-facing behavior of current qemu-system binaries, >>> which I now consider as 'legacy'. >>> >>> Once all current restrictions removed, the new qemu-system-heterogeneous >>> binary is expected to run any combination of targets. >>> >>> qemu-system-$target will be a call to qemu-system-heterogeneous with >>> a restricted subset, possibly in the form of: >>> >>> $ qemu-system-heterogeneous --target aarch64-softmmu >>> >> >> Or just qemu-system I guess. >> >> ^ equivalent of today's qemu-system-aarch64 >>> >>> If you don't like 'qemu_legacy_binary_' prefix, I can use >>> 'qemu_single_binary_' instead. >>> >> >> Still there is a problem with renaming binaries (both the "qemu-kvm" case >> and the good/bad case that Richard pointed out). > > We could special case the '-kvm' suffix, because by its nature it > implies the current binary build target. > >> >> I think you should try creating two versions of system/arch_init.c, so that >> it has a separate implementation for heterogeneous vs. single-target >> binaries. Then you can keep separate linking steps for single-target >> binaries and you naturally get the right target info from either the >> target-specific arch_init-single.c, or the --target option for >> arch_init-multi.c. >> >> (Is --target even necessary? As long as you have a way disambiguate >> same-named machines like -M virt, and have no default machine in the >> multi-target binary, you shouldn't need it). > > If we did 'query-machines' on qemu-system-heterogeneous, it would > return all machines from all targets. To disambiguate naming there > are various options > > * The query-machines command would have to gain a new 'target' > field and we would have to document that uniqness is across > the tuple (name, target), not merely name. That's a semantic > change. > > We would still need a way to express the 'target' when asking > to instantiate a machine > > * The query-machines command would have to gain a new 'target' > paramter so callers can restrict the data they receive back > > We would still need a way to express the 'target' when asking > to instantiate a machine > > * Rename all machine types so they are '<target>-<machine>' > The query-machines command doesn't change. Apps would have > to "parse" the machine name to see what 'target' each is > associated with, or we include an explicit 'target' field > in the returned data. Instianting a machine would not need > changing I think -machine m68k:virt could work, -M help would list machines like: arm:raspi i386:pc etc. Management apps could easily find : to separate arch but those that don't care about arch would just work and list more possible machines. Some machines like pc or mac99 that may appear differently in different single arch binary might need to get resolved first. Maybe need a way to search machine list by pattern e.g. as -machine x86_64:help. I tend to agree with Peter that if a multi binary qemu-system-arm would be able to also create the 64 bit machines that's not a problem as long as all the 32 bit machines still work the same. This would just make qemu-system-arm and qemu-system-aarch64 the same or maybe you can set the search pattern from command name so qemu-system-arm -M help would be the same as qemu-system -M arm:help. Allowing renaming binaries and still keep single arch behaviour probably needs to keep a way to build single arch binaries so you can't convert everything to runtime check and drop #ifdefs. Regards, BALATON Zoltan > * Require --target CLI arg, meaning query-machines remains > unchanged, as does instantiating machines > > Any other options ? > > The last is the simplest option if we just make --target be defaulted > based on the binary name. > > With regards, > Daniel >
On Thu, Mar 06, 2025 at 02:45:52PM +0100, BALATON Zoltan wrote: > On Thu, 6 Mar 2025, Daniel P. Berrangé wrote: > > On Thu, Mar 06, 2025 at 12:34:13PM +0100, Paolo Bonzini wrote: > > > Il gio 6 mar 2025, 10:27 Philippe Mathieu-Daudé <philmd@linaro.org> ha > > > scritto: > > > > > > > This API is to allow refactoring code for heterogeneous emulation, > > > > without changing user-facing behavior of current qemu-system binaries, > > > > which I now consider as 'legacy'. > > > > > > > > Once all current restrictions removed, the new qemu-system-heterogeneous > > > > binary is expected to run any combination of targets. > > > > > > > > qemu-system-$target will be a call to qemu-system-heterogeneous with > > > > a restricted subset, possibly in the form of: > > > > > > > > $ qemu-system-heterogeneous --target aarch64-softmmu > > > > > > > > > > Or just qemu-system I guess. > > > > > > ^ equivalent of today's qemu-system-aarch64 > > > > > > > > If you don't like 'qemu_legacy_binary_' prefix, I can use > > > > 'qemu_single_binary_' instead. > > > > > > > > > > Still there is a problem with renaming binaries (both the "qemu-kvm" case > > > and the good/bad case that Richard pointed out). > > > > We could special case the '-kvm' suffix, because by its nature it > > implies the current binary build target. > > > > > > > > I think you should try creating two versions of system/arch_init.c, so that > > > it has a separate implementation for heterogeneous vs. single-target > > > binaries. Then you can keep separate linking steps for single-target > > > binaries and you naturally get the right target info from either the > > > target-specific arch_init-single.c, or the --target option for > > > arch_init-multi.c. > > > > > > (Is --target even necessary? As long as you have a way disambiguate > > > same-named machines like -M virt, and have no default machine in the > > > multi-target binary, you shouldn't need it). > > > > If we did 'query-machines' on qemu-system-heterogeneous, it would > > return all machines from all targets. To disambiguate naming there > > are various options > > > > * The query-machines command would have to gain a new 'target' > > field and we would have to document that uniqness is across > > the tuple (name, target), not merely name. That's a semantic > > change. > > > > We would still need a way to express the 'target' when asking > > to instantiate a machine > > > > * The query-machines command would have to gain a new 'target' > > paramter so callers can restrict the data they receive back > > > > We would still need a way to express the 'target' when asking > > to instantiate a machine > > > > * Rename all machine types so they are '<target>-<machine>' > > The query-machines command doesn't change. Apps would have > > to "parse" the machine name to see what 'target' each is > > associated with, or we include an explicit 'target' field > > in the returned data. Instianting a machine would not need > > changing > > I think -machine m68k:virt could work, -M help would list machines like: > > arm:raspi > i386:pc > etc. > > Management apps could easily find : to separate arch but those that don't > care about arch would just work and list more possible machines. Some > machines like pc or mac99 that may appear differently in different single > arch binary might need to get resolved first. Maybe need a way to search > machine list by pattern e.g. as -machine x86_64:help. ...except that custom structures/formats in command line args is something we've tried very hard to eliminate in Qemu, and instead model everything as a distinct fields, using QAPI, so... .. if you're meaning "arm:raspi" as a short hand for "target:machine" that would be a design anti-pattern, b ...if you're meaning that "arm:raspi" is the full machine name, to be strictly treated as an opaque string that would be acceptable. I rather think the latter would not end up being treated as an opaque string though - the tempetation to parse it & assign semantics to the pieces is just too great. So I'm not a fan of that approach. From a QAPI design best pratice POV, the requirement would be for -machine target=arm,name=raspi With regards, Daniel
On Thu, 6 Mar 2025, Daniel P. Berrangé wrote: > On Thu, Mar 06, 2025 at 02:45:52PM +0100, BALATON Zoltan wrote: >> On Thu, 6 Mar 2025, Daniel P. Berrangé wrote: >>> On Thu, Mar 06, 2025 at 12:34:13PM +0100, Paolo Bonzini wrote: >>>> Il gio 6 mar 2025, 10:27 Philippe Mathieu-Daudé <philmd@linaro.org> ha >>>> scritto: >>>> >>>>> This API is to allow refactoring code for heterogeneous emulation, >>>>> without changing user-facing behavior of current qemu-system binaries, >>>>> which I now consider as 'legacy'. >>>>> >>>>> Once all current restrictions removed, the new qemu-system-heterogeneous >>>>> binary is expected to run any combination of targets. >>>>> >>>>> qemu-system-$target will be a call to qemu-system-heterogeneous with >>>>> a restricted subset, possibly in the form of: >>>>> >>>>> $ qemu-system-heterogeneous --target aarch64-softmmu >>>>> >>>> >>>> Or just qemu-system I guess. >>>> >>>> ^ equivalent of today's qemu-system-aarch64 >>>>> >>>>> If you don't like 'qemu_legacy_binary_' prefix, I can use >>>>> 'qemu_single_binary_' instead. >>>>> >>>> >>>> Still there is a problem with renaming binaries (both the "qemu-kvm" case >>>> and the good/bad case that Richard pointed out). >>> >>> We could special case the '-kvm' suffix, because by its nature it >>> implies the current binary build target. >>> >>>> >>>> I think you should try creating two versions of system/arch_init.c, so that >>>> it has a separate implementation for heterogeneous vs. single-target >>>> binaries. Then you can keep separate linking steps for single-target >>>> binaries and you naturally get the right target info from either the >>>> target-specific arch_init-single.c, or the --target option for >>>> arch_init-multi.c. >>>> >>>> (Is --target even necessary? As long as you have a way disambiguate >>>> same-named machines like -M virt, and have no default machine in the >>>> multi-target binary, you shouldn't need it). >>> >>> If we did 'query-machines' on qemu-system-heterogeneous, it would >>> return all machines from all targets. To disambiguate naming there >>> are various options >>> >>> * The query-machines command would have to gain a new 'target' >>> field and we would have to document that uniqness is across >>> the tuple (name, target), not merely name. That's a semantic >>> change. >>> >>> We would still need a way to express the 'target' when asking >>> to instantiate a machine >>> >>> * The query-machines command would have to gain a new 'target' >>> paramter so callers can restrict the data they receive back >>> >>> We would still need a way to express the 'target' when asking >>> to instantiate a machine >>> >>> * Rename all machine types so they are '<target>-<machine>' >>> The query-machines command doesn't change. Apps would have >>> to "parse" the machine name to see what 'target' each is >>> associated with, or we include an explicit 'target' field >>> in the returned data. Instianting a machine would not need >>> changing >> >> I think -machine m68k:virt could work, -M help would list machines like: >> >> arm:raspi >> i386:pc >> etc. >> >> Management apps could easily find : to separate arch but those that don't >> care about arch would just work and list more possible machines. Some >> machines like pc or mac99 that may appear differently in different single >> arch binary might need to get resolved first. Maybe need a way to search >> machine list by pattern e.g. as -machine x86_64:help. > > ...except that custom structures/formats in command line args is > something we've tried very hard to eliminate in Qemu, and instead > model everything as a distinct fields, using QAPI, so... > > .. if you're meaning "arm:raspi" as a short hand for "target:machine" > that would be a design anti-pattern, b > > ...if you're meaning that "arm:raspi" is the full machine name, to be > strictly treated as an opaque string that would be acceptable. > > I rather think the latter would not end up being treated as an opaque > string though - the tempetation to parse it & assign semantics to the > pieces is just too great. So I'm not a fan of that approach. > > From a QAPI design best pratice POV, the requirement would be for > > -machine target=arm,name=raspi As long as I don't have to type that and can use -M arm:raspi as a shorthand that's OK but then we could just make an exception for this and combine target and machine name here for simplicity. Unless it's simpler to internally use separate name and target attributes and implement a command line shorthand. You'll also need a way to display machine list with target and name in a way that's easy to parse for tools for which the target:name format seems like a trivial way. So I don't mind how you rationalise it and call all of it the machine name or if it's implemented as separate name and target attributes but please try to keep the command line something a human can use too. Regards, BALATON Zoltan
diff --git a/meson.build b/meson.build index eaae1da2e92..e4ede6ba06f 100644 --- a/meson.build +++ b/meson.build @@ -3767,7 +3767,7 @@ if have_block endif endif -common_ss.add(files('cpu-common.c')) +common_ss.add(files('cpu-common.c', 'legacy_binary_info.c')) specific_ss.add(files('cpu-target.c', 'arch_info-target.c')) subdir('system') diff --git a/include/qemu/legacy_binary_info.h b/include/qemu/legacy_binary_info.h new file mode 100644 index 00000000000..ae67399ebf2 --- /dev/null +++ b/include/qemu/legacy_binary_info.h @@ -0,0 +1,14 @@ +/* + * QEMU legacy binary helpers + * + * Copyright (c) Linaro + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#ifndef QEMU_LEGACY_BINARY_INFO_H +#define QEMU_LEGACY_BINARY_INFO_H + +void legacy_binary_info_init(const char *argv0); + +#endif diff --git a/legacy_binary_info.c b/legacy_binary_info.c new file mode 100644 index 00000000000..0c50fc9248a --- /dev/null +++ b/legacy_binary_info.c @@ -0,0 +1,160 @@ +/* + * QEMU legacy binary helpers + * + * Copyright (c) Linaro + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" +#include "qemu/arch_info.h" +#include "qemu/legacy_binary_info.h" + +typedef struct LegacyBinaryInfo { + const char *binary_name; + QemuArchBit arch_bit; +} LegacyBinaryInfo; + +/* List alphabetically sorted by binary_name */ +static const LegacyBinaryInfo legacy_binary_infos[] = { + { + .binary_name = "qemu-system-aarch64", + .arch_bit = QEMU_ARCH_BIT_ARM, + }, + { + .binary_name = "qemu-system-alpha", + .arch_bit = QEMU_ARCH_BIT_ALPHA, + }, + { + .binary_name = "qemu-system-arm", + .arch_bit = QEMU_ARCH_BIT_ARM, + }, + { + .binary_name = "qemu-system-avr", + .arch_bit = QEMU_ARCH_BIT_AVR, + }, + { + .binary_name = "qemu-system-hppa", + .arch_bit = QEMU_ARCH_BIT_HPPA, + }, + { + .binary_name = "qemu-system-i386", + .arch_bit = QEMU_ARCH_BIT_I386, + }, + { + .binary_name = "qemu-system-loongarch64", + .arch_bit = QEMU_ARCH_BIT_LOONGARCH, + }, + { + .binary_name = "qemu-system-m68k", + .arch_bit = QEMU_ARCH_BIT_M68K, + }, + { + .binary_name = "qemu-system-microblaze", + .arch_bit = QEMU_ARCH_BIT_MICROBLAZE, + }, + { + .binary_name = "qemu-system-microblazeel", + .arch_bit = QEMU_ARCH_BIT_MICROBLAZE, + }, + { + .binary_name = "qemu-system-mips", + .arch_bit = QEMU_ARCH_BIT_MIPS, + }, + { + .binary_name = "qemu-system-mips64", + .arch_bit = QEMU_ARCH_BIT_MIPS, + }, + { + .binary_name = "qemu-system-mips64el", + .arch_bit = QEMU_ARCH_BIT_MIPS, + }, + { + .binary_name = "qemu-system-mipsel", + .arch_bit = QEMU_ARCH_BIT_MIPS, + }, + { + .binary_name = "qemu-system-or1k", + .arch_bit = QEMU_ARCH_BIT_OPENRISC, + }, + { + .binary_name = "qemu-system-ppc", + .arch_bit = QEMU_ARCH_BIT_PPC, + }, + { + .binary_name = "qemu-system-ppc64", + .arch_bit = QEMU_ARCH_BIT_PPC, + }, + { + .binary_name = "qemu-system-riscv32", + .arch_bit = QEMU_ARCH_BIT_RISCV, + }, + { + .binary_name = "qemu-system-riscv64", + .arch_bit = QEMU_ARCH_BIT_RISCV, + }, + { + .binary_name = "qemu-system-rx", + .arch_bit = QEMU_ARCH_BIT_RX, + }, + { + .binary_name = "qemu-system-s390x", + .arch_bit = QEMU_ARCH_BIT_S390X, + }, + { + .binary_name = "qemu-system-sh4", + .arch_bit = QEMU_ARCH_BIT_SH4, + }, + { + .binary_name = "qemu-system-sh4eb", + .arch_bit = QEMU_ARCH_BIT_SH4, + }, + { + .binary_name = "qemu-system-sparc", + .arch_bit = QEMU_ARCH_BIT_SPARC, + }, + { + .binary_name = "qemu-system-sparc64", + .arch_bit = QEMU_ARCH_BIT_SPARC, + }, + { + .binary_name = "qemu-system-tricore", + .arch_bit = QEMU_ARCH_BIT_TRICORE, + }, + { + .binary_name = "qemu-system-x86_64", + .arch_bit = QEMU_ARCH_BIT_I386, + }, + { + .binary_name = "qemu-system-xtensa", + .arch_bit = QEMU_ARCH_BIT_XTENSA, + }, + { + .binary_name = "qemu-system-xtensaeb", + .arch_bit = QEMU_ARCH_BIT_XTENSA, + }, +}; + +static int current_index = -1; + +void legacy_binary_info_init(const char *argv0) +{ + g_auto(GStrv) tokens = g_strsplit(argv0, G_DIR_SEPARATOR_S, -1); + unsigned count = 0; + const char *binary_name; + + while (tokens[count]) { + count++; + } + assert(count > 0); + binary_name = tokens[count - 1]; + + for (size_t i = 0; i < ARRAY_SIZE(legacy_binary_infos); i++) { + if (!strcmp(legacy_binary_infos[i].binary_name, binary_name)) { + current_index = i; + return; + } + } + fprintf(stderr, "Missing legacy info for '%s' binary.\n", binary_name); + abort(); +} diff --git a/system/vl.c b/system/vl.c index a41ba4a2d5f..74a062c7fff 100644 --- a/system/vl.c +++ b/system/vl.c @@ -28,6 +28,7 @@ #include "qemu/units.h" #include "qemu/module.h" #include "qemu/arch_info.h" +#include "qemu/legacy_binary_info.h" #include "exec/cpu-common.h" #include "exec/page-vary.h" #include "hw/qdev-properties.h" @@ -2883,6 +2884,7 @@ void qemu_init(int argc, char **argv) error_init(argv[0]); qemu_init_exec_dir(argv[0]); + legacy_binary_info_init(argv[0]); os_setup_limits();
Introduce an API to get information specific to a binary from the binary name (argv[0]). Initialize it from qemu_init() on system emulation. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- meson.build | 2 +- include/qemu/legacy_binary_info.h | 14 +++ legacy_binary_info.c | 160 ++++++++++++++++++++++++++++++ system/vl.c | 2 + 4 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 include/qemu/legacy_binary_info.h create mode 100644 legacy_binary_info.c