Re: [PULL v3 49/81] acpi: pc: vga: use AcpiDevAmlIf interface to build VGA device descriptors

[Michael S. Tsirkin]

On Mon, Nov 07, 2022 at 10:07:52PM +0000, Bernhard Beschow wrote:
> Am 7. November 2022 13:00:36 UTC schrieb "Michael S. Tsirkin" 
> <mst@redhat.com>:
> >On Mon, Nov 07, 2022 at 06:16:25PM +0530, Ani Sinha wrote:
> >> On Mon, Nov 7, 2022 at 6:03 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >> >
> >> > On Sun, Nov 06, 2022 at 10:16:41PM +0100, Bernhard Beschow wrote:
> >> > >
> >> > >
> >> > > On Sat, Nov 5, 2022 at 6:45 PM Michael S. Tsirkin <mst@redhat.com> 
> >> > > wrote:
> >> > >
> >> > >     From: Igor Mammedov <imammedo@redhat.com>
> >> > >
> >> > >     Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> >> > >     Message-Id: <20221017102146.2254096-2-imammedo@redhat.com>
> >> > >     Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> >> > >     Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >> > >     NB: we do not expect any functional change in
> >> > >     any ACPI tables with this change. It's only a refactoring.
> >> > >
> >> > >     Reviewed-by: Ani Sinha <ani@anisinha.ca>
> >> > >     ---
> >> > >      hw/display/vga_int.h       |  2 ++
> >> > >      hw/display/acpi-vga-stub.c |  7 +++++++
> >> > >      hw/display/acpi-vga.c      | 26 ++++++++++++++++++++++++++
> >> > >      hw/display/vga-pci.c       |  4 ++++
> >> > >      hw/i386/acpi-build.c       | 26 +-------------------------
> >> > >      hw/display/meson.build     | 17 +++++++++++++++++
> >> > >      6 files changed, 57 insertions(+), 25 deletions(-)
> >> > >      create mode 100644 hw/display/acpi-vga-stub.c
> >> > >      create mode 100644 hw/display/acpi-vga.c
> >> > >
> >> > >
> >> > > With this "qemu:qtest+qtest-hppa / qtest-hppa/display-vga-test" fails 
> >> > > due to
> >> > > the symbol "aml_return" being undefined:
> >> > >
> >> > > # starting QEMU: exec ./qemu-system-hppa -qtest 
> >> > > unix:/tmp/qtest-515650.sock
> >> > > -qtest-log /dev/null -chardev 
> >> > > socket,path=/tmp/qtest-515650.qmp,id=char0 -mon
> >> > > chardev=char0,mode=control -display none -vga none -device virtio-vga 
> >> > > -accel
> >> > > qtest
> >> > > ----------------------------------- stderr 
> >> > > -----------------------------------
> >> > > Failed to open module: qemu/build/qemu-bundle/usr/lib/qemu/
> >> > > hw-display-virtio-vga.so: undefined symbol: aml_return
> >> > > qemu-system-hppa: -device virtio-vga: 'virtio-vga' is not a valid 
> >> > > device model
> >> > > name
> >> > > Broken pipe
> >> > > ../src/tests/qtest/libqtest.c:179: kill_qemu() tried to terminate QEMU 
> >> > > process
> >> > > but encountered exit status 1 (expected 0)
> >> > >
> >> > > (test program exited with status code -6)
> >> > >
> >> > > Best regards,
> >> > > Bernhard
> >> >
> >> > It's unfortunate that it doesn't reproduce for me :(
> >> 
> >> To reproduce:
> >> 
> >> - docker pull registry.gitlab.com/qemu-project/qemu/qemu/centos8:latest
> >> - configure line:
> >> 
> >> ../configure --enable-werror --disable-docs --disable-nettle
> >> --enable-gcrypt --enable-fdt=system --enable-modules
> >> --enable-trace-backends=dtrace --enable-docs --enable-vfio-user-server
> >> --target-list="ppc64-softmmu or1k-softmmu s390x-softmmu x86_64-softmmu
> >> rx-softmmu sh4-softmmu nios2-softmmu"
> >
> >
> >I suspect --enable-modules is the difference.
> 
> Indeed, I'm building with --enable-modules and got the undefined symbol.
> 
> Perhaps we could use the -Wl,--no-undefined linker option when building with 
> modules to catch these errors at link time already?
> 
> Best regards,
> Bernhard

I don't see how it can work but sure, send a patch :)

> >
> >> - # make
> >> - # QTEST_QEMU_BINARY=./qemu-system-or1k  ./tests/qtest/qos-test
> >> failed to open module:
> >> /build/qemu/qemu/build/qemu-bundle/usr/local/lib64/qemu/hw-display-virtio-vga.so:
> >> undefined symbol: aml_return
> >> qemu-system-or1k: ../util/error.c:59: error_setv: Assertion `*errp ==
> >> NULL' failed.
> >> Broken pipe
> >> ../tests/qtest/libqtest.c:188: kill_qemu() detected QEMU death from
> >> signal 6 (Aborted) (core dumped)
> >> Aborted (core dumped)
> >