[RFC,04/18] qemu: Introduce 'qemu/legacy_binary_info.h'

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

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