Re: [PATCH] mos6522: fix linking error when CONFIG_MOS6522 is not set

[Murilo Opsfelder Araújo]

Hi, Mark.

Thanks for reviewing.  Comments below.

On 5/2/22 06:43, Mark Cave-Ayland wrote:
> On 30/04/2022 00:31, Murilo Opsfelder Araujo wrote:
>
> > When CONFIG_MOS6522 is not set, building ppc64-softmmu target fails:
> >
> >      /usr/bin/ld: libqemu-ppc64-softmmu.fa.p/monitor_misc.c.o:(.data+0x1158): 
> > undefined reference to `hmp_info_via'
> >      clang-13: error: linker command failed with exit code 1 (use -v to see 
> > invocation)
> >
> > Add CONFIG_MOS6522 check for hmp_info_via in hmp-commands-info.hx to fix
> > such linking error.
> >
> > Fixes: 409e9f7131e5 (mos6522: add "info via" HMP command for debugging)
> > Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
> > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > Cc: Fabiano Rosas <farosas@linux.ibm.com>
> > ---
> >   hmp-commands-info.hx | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> > index adfa085a9b..9ad784dd9f 100644
> > --- a/hmp-commands-info.hx
> > +++ b/hmp-commands-info.hx
> > @@ -881,6 +881,7 @@ SRST
> >   ERST
> >   #if defined(TARGET_M68K) || defined(TARGET_PPC)
> > +#if defined(CONFIG_MOS6522)
> >       {
> >           .name         = "via",
> >           .args_type    = "",
> > @@ -889,6 +890,7 @@ ERST
> >           .cmd          = hmp_info_via,
> >       },
> >   #endif
> > +#endif
> >   SRST
> >     ``info via``
>
> Hmmm. The patch in its proposed form isn't correct, since device CONFIG_* 
> defines aren't declared when processing hmp-commands-info.hx. This was 
> something that was discovered and discussed in the original thread for which 
> the current workaround is to use the per-target TARGET_* defines instead.

So my proposed fix worked just by coincidence.  Thanks for providing the 
background.

> Given that the g3beige and mac99 machines are included by default in 
> qemu-system-ppc64 which both contain the MOS6522 device, I can't quite 
> understand how CONFIG_MOS6522 isn't being selected.
>
> Can you give more information about how you are building QEMU including your 
> configure command line?

Here is a reproducer adapted from CentOS 9 Stream qemu-kvm[0] package
(build failed on c9s ppc64le with QEMU at commit 
f5643914a9e8f79c606a76e6a9d7ea82a3fc3e65):

$ cat > configs/devices/rh-virtio.mak <<"EOF"
CONFIG_VIRTIO=y
CONFIG_VIRTIO_BALLOON=y
CONFIG_VIRTIO_BLK=y
CONFIG_VIRTIO_GPU=y
CONFIG_VIRTIO_INPUT=y
CONFIG_VIRTIO_INPUT_HOST=y
CONFIG_VIRTIO_NET=y
CONFIG_VIRTIO_RNG=y
CONFIG_VIRTIO_SCSI=y
CONFIG_VIRTIO_SERIAL=y
EOF

$ cat > configs/devices/ppc64-softmmu/ppc64-rh-devices.mak <<"EOF"
include ../rh-virtio.mak
CONFIG_DIMM=y
CONFIG_MEM_DEVICE=y
CONFIG_NVDIMM=y
CONFIG_PCI=y
CONFIG_PCI_DEVICES=y
CONFIG_PCI_TESTDEV=y
CONFIG_PCI_EXPRESS=y
CONFIG_PSERIES=y
CONFIG_SCSI=y
CONFIG_SPAPR_VSCSI=y
CONFIG_TEST_DEVICES=y
CONFIG_USB=y
CONFIG_USB_OHCI=y
CONFIG_USB_OHCI_PCI=y
CONFIG_USB_SMARTCARD=y
CONFIG_USB_STORAGE_CORE=y
CONFIG_USB_STORAGE_CLASSIC=y
CONFIG_USB_XHCI=y
CONFIG_USB_XHCI_NEC=y
CONFIG_USB_XHCI_PCI=y
CONFIG_VFIO=y
CONFIG_VFIO_PCI=y
CONFIG_VGA=y
CONFIG_VGA_PCI=y
CONFIG_VHOST_USER=y
CONFIG_VIRTIO_PCI=y
CONFIG_VIRTIO_VGA=y
CONFIG_WDT_IB6300ESB=y
CONFIG_XICS=y
CONFIG_XIVE=y
CONFIG_TPM=y
CONFIG_TPM_SPAPR=y
CONFIG_TPM_EMULATOR=y
EOF

$ mkdir build
$ cd build

$ ../configure --cc=clang --cxx=/bin/false --prefix=/usr --libdir=/usr/lib64 
--datadir=/usr/share --sysconfdir=/etc --interp-prefix=/usr/qemu-%M 
--localstatedir=/var --docdir=/usr/share/doc --libexecdir=/usr/libexec 
'--extra-ldflags=-Wl,-z,relro -Wl,--as-needed  -Wl,-z,now   ' 
'--extra-cflags=-O2  -fexceptions -g -grecord-gcc-switches -pipe -Wall 
-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS 
--config /usr/lib/rpm/redhat/redhat-hardened-clang.cfg -fstack-protector-strong 
  -m64 -mcpu=power9 -mtune=power9 -fasynchronous-unwind-tables 
-fstack-clash-protection -Wno-string-plus-int' 
--with-pkgversion=qemu-kvm-7.0.0-1.el9 --with-suffix=qemu-kvm 
--firmwarepath=/usr/share/qemu-firmware:/usr/share/ipxe/qemu:/usr/share/seavgabios:/usr/share/seabios
 --meson=internal --enable-trace-backend=dtrace --with-coroutine=ucontext 
--with-git=git --tls-priority=@QEMU,SYSTEM --audio-drv-list= --disable-alsa 
--disable-attr --disable-auth-pam --disable-avx2 --disable-avx512f 
--disable-block-drv-whitelist-in-tools --disable-bochs --disable-bpf 
--disable-brlapi --disable-bsd-user --disable-bzip2 --disable-cap-ng 
--disable-capstone --disable-cfi --disable-cfi-debug --disable-cloop 
--disable-cocoa --disable-coreaudio --disable-coroutine-pool 
--disable-crypto-afalg --disable-curl --disable-curses --disable-dbus-display 
--disable-debug-info --disable-debug-mutex --disable-debug-tcg --disable-dmg 
--disable-docs --disable-dsound --disable-fdt --disable-fuse 
--disable-fuse-lseek --disable-gcrypt --disable-gettext --disable-gio 
--disable-glusterfs --disable-gnutls --disable-gtk --disable-guest-agent 
--disable-guest-agent-msi --disable-hax --disable-hvf --disable-iconv 
--disable-jack --disable-kvm --disable-l2tpv3 --disable-libdaxctl 
--disable-libiscsi --disable-libnfs --disable-libpmem --disable-libssh 
--disable-libudev --disable-libusb --disable-linux-aio --disable-linux-io-uring 
--disable-linux-user --disable-live-block-migration --disable-lto 
--disable-lzfse --disable-lzo --disable-malloc-trim --disable-membarrier 
--disable-modules --disable-module-upgrades --disable-mpath 
--disable-multiprocess --disable-netmap --disable-nettle --disable-numa 
--disable-nvmm --disable-opengl --disable-oss --disable-pa --disable-parallels 
--disable-pie --disable-pvrdma --disable-qcow1 --disable-qed --disable-qga-vss 
--disable-qom-cast-debug --disable-rbd --disable-rdma --disable-replication 
--disable-rng-none --disable-safe-stack --disable-sanitizers --disable-sdl 
--disable-sdl-image --disable-seccomp --disable-selinux --disable-slirp 
--disable-slirp-smbd --disable-smartcard --disable-snappy --disable-sparse 
--disable-spice --disable-spice-protocol --disable-strip --disable-system 
--disable-tcg --disable-tools --disable-tpm --disable-u2f --disable-usb-redir 
--disable-user --disable-vde --disable-vdi --disable-vhost-crypto 
--disable-vhost-kernel --disable-vhost-net --disable-vhost-scsi 
--disable-vhost-user --disable-vhost-user-blk-server --disable-vhost-vdpa 
--disable-vhost-vsock --disable-virglrenderer --disable-virtfs 
--disable-virtiofsd --disable-vnc --disable-vnc-jpeg --disable-vnc-sasl 
--disable-vte --disable-vvfat --disable-werror --disable-whpx --disable-xen 
--disable-xen-pci-passthrough --disable-xkbcommon --disable-zstd 
--with-git-submodules=ignore --without-default-devices 
--with-devices-ppc64=ppc64-rh-devices --target-list=ppc64-softmmu 
--block-drv-rw-whitelist=qcow2,raw,file,host_device,nbd,iscsi,rbd,blkdebug,luks,null-co,nvme,copy-on-read,throttle,compress
 --block-drv-ro-whitelist=vdi,vmdk,vhdx,vpc,https --enable-attr --enable-cap-ng 
--enable-capstone=internal --enable-coroutine-pool --enable-curl 
--enable-debug-info --enable-docs --enable-fdt=system --enable-gnutls 
--enable-guest-agent --enable-iconv --enable-kvm --enable-libusb 
--enable-libudev --enable-linux-aio --enable-lzo --enable-malloc-trim 
--enable-modules --enable-mpath --enable-numa --enable-pa --enable-pie 
--enable-rbd --enable-rdma --enable-seccomp --enable-selinux 
--enable-slirp=system --enable-snappy --enable-spice-protocol --enable-system 
--enable-tcg --enable-tools --enable-tpm --enable-vdi --enable-virtiofsd 
--enable-vhost-kernel --enable-vhost-net --enable-vhost-user 
--enable-vhost-user-blk-server --enable-vhost-vdpa --enable-vhost-vsock 
--enable-vnc --enable-vnc-sasl --enable-werror --enable-xkbcommon

$ make -O -j$(nproc) V=1 VERBOSE=1
...
/usr/bin/ld: libqemu-ppc64-softmmu.fa.p/monitor_misc.c.o:(.data+0x1158): 
undefined reference to `hmp_info_via'
clang-13: error: linker command failed with exit code 1 (use -v to see 
invocation)

I have figured that it also fails with this minimal set of configure options
(in addition to the devices CONFIG_* options above):

$ ../configure --without-default-devices --with-devices-ppc64=ppc64-rh-devices 
--target-list=ppc64-softmmu
$ make -j
...
/usr/bin/ld: libqemu-ppc64-softmmu.fa.p/monitor_misc.c.o:(.data.rel+0x3228): 
undefined reference to `hmp_info_via'
collect2: error: ld returned 1 exit status

Since TARGET_PPC is defined when building target ppc64-softmmu, the 
hmp_info_via will be referenced when processing the hmp-commands-info.hx.
However, hmp_info_via implementation resides on hw/misc/mos6522.c, which is 
built only if CONFIG_MOS6522 is defined, as per hw/misc/meson.build.

If hmp_info_via is generic enough and not device-specific, it could be moved 
out of mos6522.c to somewhere else.

What do you think?

[0] 
https://gitlab.com/redhat/centos-stream/rpms/qemu-kvm/-/blob/c9s/qemu-kvm.spec#L686

--
Murilo