On Mon, Jan 22, 2024 at 2:40 PM Thomas Huth <th...@redhat.com> wrote:
>
> 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 <pbonz...@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:
xtensa is covered by the other patch I sent, but I (incorrectly)
thought these two would be independent and didn't send them as a
single thread.
> /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?
You can use it; it's compiled once, and then (based on the per-target
configuration) the executable will or will not include the files that
depend on it, similar to devices.
Perhaps however it makes sense to turn it into a Kconfig symbol, as
that would make the use more clear. Or based on Phillippe's
suggestion, enabling device tree everywhere might make sense too.
Paolo
