Re: [PATCH] monitor: add dumpdtb command only in device-tree-enabled targets

[Thomas Huth]

On 22/01/2024 10.24, Paolo Bonzini wrote:
> Remove the command altogether from targets that do not have device tree support,
> instead of leaving it nonfunctional.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   meson.build        | 2 --
>   qapi/machine.json  | 2 +-
>   hmp-commands.hx    | 2 +-
>   system/meson.build | 2 +-
>   4 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index de8e5ad67dc..6bf7f3eb685 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3126,8 +3126,6 @@ if fdt_required.length() > 0 or 
> get_option('fdt').enabled()
>     endif
>   endif
>   
> -config_host_data.set('CONFIG_FDT', fdt.found())
> -
>   vhost_user = not_found
>   if host_os == 'linux' and have_vhost_user
>     libvhost_user = subproject('libvhost-user')
> diff --git a/qapi/machine.json b/qapi/machine.json
> index b6d634b30d5..c5e40857ba4 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1828,4 +1828,4 @@
>   ##
>   { 'command': 'dumpdtb',
>     'data': { 'filename': 'str' },
> -  'if': 'CONFIG_FDT' }
> +  'if': 'TARGET_NEEDS_FDT' }
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 17b5ea839d9..f7263957240 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1819,7 +1819,7 @@ ERST
>           .flags      = "p",
>       },
>   
> -#if defined(CONFIG_FDT)
> +#if defined(TARGET_NEED_FDT)
>       {
>           .name       = "dumpdtb",
>           .args_type  = "filename:F",
> diff --git a/system/meson.build b/system/meson.build
> index 25e21172505..4e54f5d1a4b 100644
> --- a/system/meson.build
> +++ b/system/meson.build
> @@ -32,7 +32,7 @@ if have_tpm
>   endif
>   
>   system_ss.add(when: seccomp, if_true: files('qemu-seccomp.c'))
> -system_ss.add(when: fdt, if_true: files('device_tree.c'))
> +system_ss.add(when: 'TARGET_NEED_FDT', if_true: [fdt, files('device_tree.c')])

IIUC this does not work. For example, I applied your patch and tried to 
"make qemu-system-xtensa", I got:

/usr/bin/ld: libcommon.fa.p/hw_core_guest-loader.c.o: in function 
`loader_insert_platform_data':
/home/thuth/tmp/qemu-build/../../devel/qemu/hw/core/guest-loader.c:55: 
undefined reference to `qemu_fdt_add_subnode'
/usr/bin/ld: 
/home/thuth/tmp/qemu-build/../../devel/qemu/hw/core/guest-loader.c:56: 
undefined reference to `qemu_fdt_setprop'
/usr/bin/ld: 
/home/thuth/tmp/qemu-build/../../devel/qemu/hw/core/guest-loader.c:60: 
undefined reference to `qemu_fdt_setprop_string_array'
/usr/bin/ld: 
/home/thuth/tmp/qemu-build/../../devel/qemu/hw/core/guest-loader.c:67: 
undefined reference to `qemu_fdt_setprop_string'
/usr/bin/ld: 
/home/thuth/tmp/qemu-build/../../devel/qemu/hw/core/guest-loader.c:73: 
undefined reference to `qemu_fdt_setprop_string_array'
collect2: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.

system_ss is a common set, so you must not use a TARGET_* switch for 
deciding what goes into that set, or did I get that wrong? I.e. I think you 
have to drop the system/meson.build change here?

Also, there is another #ifdef CONFIG_FDT in hw/xtensa/xtfpga.c which likely 
needs some treatment, too?

 Thomas