Re: [RFC PATCH v2 02/11] qemu: Convert target_name() to TargetInfo API

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 4/17/25 17:50, Philippe Mathieu-Daudé wrote:
> Have target_name() be a target-agnostic method, dispatching
> to a per-target TargetInfo singleton structure.
> By default a stub singleton is used. No logical change
> expected.
> 
> Inspired-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   meson.build                     |  3 +++
>   include/hw/core/cpu.h           |  2 --
>   include/qemu/target_info-impl.h | 23 +++++++++++++++++++++++
>   include/qemu/target_info.h      | 19 +++++++++++++++++++
>   cpu-target.c                    |  5 -----
>   hw/core/machine-qmp-cmds.c      |  1 +
>   plugins/loader.c                |  2 +-
>   system/vl.c                     |  2 +-
>   target_info-stub.c              | 19 +++++++++++++++++++
>   target_info.c                   | 16 ++++++++++++++++
>   10 files changed, 83 insertions(+), 9 deletions(-)
>   create mode 100644 include/qemu/target_info-impl.h
>   create mode 100644 include/qemu/target_info.h
>   create mode 100644 target_info-stub.c
>   create mode 100644 target_info.c
> 
> diff --git a/meson.build b/meson.build
> index bcb9d39a387..49a050a28a3 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3807,6 +3807,9 @@ endif
>   common_ss.add(pagevary)
>   specific_ss.add(files('page-target.c', 'page-vary-target.c'))
>   
> +common_ss.add(files('target_info.c'))
> +specific_ss.add(files('target_info-stub.c'))
> +
>   subdir('backends')
>   subdir('disas')
>   subdir('migration')
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 5b645df59f5..9d9448341d1 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -1115,8 +1115,6 @@ bool cpu_exec_realizefn(CPUState *cpu, Error **errp);
>   void cpu_exec_unrealizefn(CPUState *cpu);
>   void cpu_exec_reset_hold(CPUState *cpu);
>   
> -const char *target_name(void);
> -
>   #ifdef COMPILING_PER_TARGET
>   
>   extern const VMStateDescription vmstate_cpu_common;
> diff --git a/include/qemu/target_info-impl.h b/include/qemu/target_info-impl.h
> new file mode 100644
> index 00000000000..d5c94ed5296
> --- /dev/null
> +++ b/include/qemu/target_info-impl.h
> @@ -0,0 +1,23 @@
> +/*
> + * QEMU binary/target API ...
> + *
> + *  Copyright (c) Linaro
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef QEMU_TARGET_INFO_IMPL_H
> +#define QEMU_TARGET_INFO_IMPL_H
> +
> +#include "qemu/target_info.h"
> +
> +typedef struct TargetInfo {
> +
> +    /* runtime equivalent of TARGET_NAME definition */
> +    const char *const name;
> +
> +} TargetInfo;
> +
> +const TargetInfo *target_info(void);
> +
> +#endif
> diff --git a/include/qemu/target_info.h b/include/qemu/target_info.h
> new file mode 100644
> index 00000000000..3f6cbbbd300
> --- /dev/null
> +++ b/include/qemu/target_info.h
> @@ -0,0 +1,19 @@
> +/*
> + * QEMU binary/target API
> + *
> + *  Copyright (c) Linaro
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef QEMU_TARGET_INFO_H
> +#define QEMU_TARGET_INFO_H
> +
> +/**
> + * target_name:
> + *
> + * Returns: Canonical target name (i.e. "i386").
> + */
> +const char *target_name(void);
> +
> +#endif
> diff --git a/cpu-target.c b/cpu-target.c
> index c99d208a7c4..3f82d3ea444 100644
> --- a/cpu-target.c
> +++ b/cpu-target.c
> @@ -165,8 +165,3 @@ bool target_words_bigendian(void)
>   {
>       return TARGET_BIG_ENDIAN;
>   }
> -
> -const char *target_name(void)
> -{
> -    return TARGET_NAME;
> -}
> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
> index 408994b67d7..b317aec234f 100644
> --- a/hw/core/machine-qmp-cmds.c
> +++ b/hw/core/machine-qmp-cmds.c
> @@ -19,6 +19,7 @@
>   #include "qapi/qobject-input-visitor.h"
>   #include "qapi/type-helpers.h"
>   #include "qemu/uuid.h"
> +#include "qemu/target_info.h"
>   #include "qom/qom-qobject.h"
>   #include "system/hostmem.h"
>   #include "system/hw_accel.h"
> diff --git a/plugins/loader.c b/plugins/loader.c
> index 7523d554f03..36a4e88d4db 100644
> --- a/plugins/loader.c
> +++ b/plugins/loader.c
> @@ -29,7 +29,7 @@
>   #include "qemu/xxhash.h"
>   #include "qemu/plugin.h"
>   #include "qemu/memalign.h"
> -#include "hw/core/cpu.h"
> +#include "qemu/target_info.h"
>   #include "exec/tb-flush.h"
>   
>   #include "plugin.h"
> diff --git a/system/vl.c b/system/vl.c
> index c17945c4939..d8a0fe713c9 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -40,6 +40,7 @@
>   #include "qemu/help_option.h"
>   #include "qemu/hw-version.h"
>   #include "qemu/uuid.h"
> +#include "qemu/target_info.h"
>   #include "system/reset.h"
>   #include "system/runstate.h"
>   #include "system/runstate-action.h"
> @@ -79,7 +80,6 @@
>   #include "hw/block/block.h"
>   #include "hw/i386/x86.h"
>   #include "hw/i386/pc.h"
> -#include "hw/core/cpu.h"
>   #include "migration/cpr.h"
>   #include "migration/misc.h"
>   #include "migration/snapshot.h"
> diff --git a/target_info-stub.c b/target_info-stub.c
> new file mode 100644
> index 00000000000..1e44bb6f6fb
> --- /dev/null
> +++ b/target_info-stub.c
> @@ -0,0 +1,19 @@
> +/*
> + * QEMU target info stubs
> + *
> + *  Copyright (c) Linaro
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/target_info-impl.h"
> +
> +static const TargetInfo target_info_stub = {
> +    .name = TARGET_NAME,
> +};
> +
> +const TargetInfo *target_info(void)
> +{
> +    return &target_info_stub;
> +}
> diff --git a/target_info.c b/target_info.c
> new file mode 100644
> index 00000000000..877a6a15014
> --- /dev/null
> +++ b/target_info.c
> @@ -0,0 +1,16 @@
> +/*
> + * QEMU binary/target helpers
> + *
> + *  Copyright (c) Linaro
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/target_info-impl.h"
> +#include "qemu/target_info.h"
> +
> +const char *target_name(void)
> +{
> +    return target_info()->name;
> +}

What is the benefit to have two different files (common and specific)?
target_name() can be inline in the same header, returning the matching 
field in existing target_info, which does not need any specialization 
per target.