| Message ID | 20201023064618.21409-2-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-gpu: build pci and vga bits modular too. | expand |
On Fri, Oct 23, 2020 at 10:46 AM Gerd Hoffmann <kraxel@redhat.com> wrote: > Build virtio-gpu pci devices modular. Must be a separate module because > not all qemu softmmu variants come with PCI support. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > Hmm, I realize we have a different behaviour when building devices as modules shared by different qemu system binaries. It will attempt to load any kind of modules: ./qemu-system-m68k -kernel ~/Downloads/vmlinux-5.8.0-3-m68k -device virtio-gpu-pci Failed to open module: /home/elmarco/src/qemu/buildnodoc/hw-display-virtio-gpu-pci.so: undefined symbol: virtio_instance_init_common qemu-system-m68k: -device virtio-gpu-pci: 'virtio-gpu-pci' is not a valid device model name And this is not a new problem, for example with qemu 5.1.0-5.fc33: $ qemu-system-m68k -device help Failed to open module: /usr/lib64/qemu/hw-usb-smartcard.so: undefined symbol: ccid_card_send_apdu_to_guest Failed to open module: /usr/lib64/qemu/hw-display-qxl.so: undefined symbol: vga_ioport_read ... What would be the solution? load modules from an arch-specific directory? link to a common shared library location? Ex: /usr/lib64/qemu/x86_64/hw-usb-smartcard.so -> /usr/lib64/qemu/hw-usb-smartcard.so --- > util/module.c | 3 +++ > hw/display/meson.build | 11 +++++++++-- > 2 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/util/module.c b/util/module.c > index fe3b82dd4d37..9490f975d303 100644 > --- a/util/module.c > +++ b/util/module.c > @@ -301,6 +301,9 @@ static struct { > { "qxl", "hw-", "display-qxl" }, > { "virtio-gpu-device", "hw-", "display-virtio-gpu" }, > { "vhost-user-gpu", "hw-", "display-virtio-gpu" }, > + { "virtio-gpu-pci-base", "hw-", "display-virtio-gpu-pci" }, > + { "virtio-gpu-pci", "hw-", "display-virtio-gpu-pci" }, > + { "vhost-user-gpu-pci", "hw-", "display-virtio-gpu-pci" }, > { "chardev-braille", "chardev-", "baum" }, > { "chardev-spicevmc", "chardev-", "spice" }, > { "chardev-spiceport", "chardev-", "spice" }, > diff --git a/hw/display/meson.build b/hw/display/meson.build > index 0d5ddecd6503..669935371335 100644 > --- a/hw/display/meson.build > +++ b/hw/display/meson.build > @@ -62,8 +62,15 @@ if config_all_devices.has_key('CONFIG_VIRTIO_GPU') > hw_display_modules += {'virtio-gpu': virtio_gpu_ss} > endif > > -softmmu_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRTIO_PCI'], if_true: > files('virtio-gpu-pci.c')) > -softmmu_ss.add(when: ['CONFIG_VHOST_USER_GPU', 'CONFIG_VIRTIO_PCI'], > if_true: files('vhost-user-gpu-pci.c')) > +if config_all_devices.has_key('CONFIG_VIRTIO_PCI') > + virtio_gpu_pci_ss = ss.source_set() > + virtio_gpu_pci_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRTIO_PCI'], > + if_true: [files('virtio-gpu-pci.c'), pixman]) > + virtio_gpu_pci_ss.add(when: ['CONFIG_VHOST_USER_GPU', > 'CONFIG_VIRTIO_PCI'], > + if_true: files('vhost-user-gpu-pci.c')) > + hw_display_modules += {'virtio-gpu-pci': virtio_gpu_pci_ss} > +endif > + > softmmu_ss.add(when: 'CONFIG_VIRTIO_VGA', if_true: files('virtio-vga.c')) > softmmu_ss.add(when: 'CONFIG_VHOST_USER_VGA', if_true: > files('vhost-user-vga.c')) > > -- > 2.27.0 > > > Otherwise patch works well: Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Hi, > Hmm, I realize we have a different behaviour when building devices as > modules shared by different qemu system binaries. > > It will attempt to load any kind of modules: > > ./qemu-system-m68k -kernel ~/Downloads/vmlinux-5.8.0-3-m68k -device > virtio-gpu-pci > Failed to open module: > /home/elmarco/src/qemu/buildnodoc/hw-display-virtio-gpu-pci.so: undefined > symbol: virtio_instance_init_common > qemu-system-m68k: -device virtio-gpu-pci: 'virtio-gpu-pci' is not a valid > device model name Yes. The last line is printed for non-modular builds too. The module load error can obviously only happen on modular builds. > $ qemu-system-m68k -device help > Failed to open module: /usr/lib64/qemu/hw-usb-smartcard.so: undefined > symbol: ccid_card_send_apdu_to_guest > Failed to open module: /usr/lib64/qemu/hw-display-qxl.so: undefined symbol: > vga_ioport_read That one is fixed meanwhile: commit 501093207eb1ed4845e0a65ee1ce7db7b9676e0b Author: Gerd Hoffmann <kraxel@redhat.com> Date: Wed Sep 23 11:12:17 2020 +0200 module: silence errors for module_load_qom_all(). take care, Gerd
HI On Fri, Oct 23, 2020 at 3:01 PM Gerd Hoffmann <kraxel@redhat.com> wrote: > Hi, > > > Hmm, I realize we have a different behaviour when building devices as > > modules shared by different qemu system binaries. > > > > It will attempt to load any kind of modules: > > > > ./qemu-system-m68k -kernel ~/Downloads/vmlinux-5.8.0-3-m68k -device > > virtio-gpu-pci > > Failed to open module: > > /home/elmarco/src/qemu/buildnodoc/hw-display-virtio-gpu-pci.so: undefined > > symbol: virtio_instance_init_common > > qemu-system-m68k: -device virtio-gpu-pci: 'virtio-gpu-pci' is not a valid > > device model name > > Yes. The last line is printed for non-modular builds too. > The module load error can obviously only happen on modular builds. > > > $ qemu-system-m68k -device help > > Failed to open module: /usr/lib64/qemu/hw-usb-smartcard.so: undefined > > symbol: ccid_card_send_apdu_to_guest > > Failed to open module: /usr/lib64/qemu/hw-display-qxl.so: undefined > symbol: > > vga_ioport_read > > That one is fixed meanwhile: > > commit 501093207eb1ed4845e0a65ee1ce7db7b9676e0b > Author: Gerd Hoffmann <kraxel@redhat.com> > Date: Wed Sep 23 11:12:17 2020 +0200 > > module: silence errors for module_load_qom_all(). > > Ok, but that could hide real errors, couldn't it? What about the proposal to have a subdir per arch with symlinks?
Hi, > > commit 501093207eb1ed4845e0a65ee1ce7db7b9676e0b > > Author: Gerd Hoffmann <kraxel@redhat.com> > > Date: Wed Sep 23 11:12:17 2020 +0200 > > > > module: silence errors for module_load_qom_all(). > > > Ok, but that could hide real errors, couldn't it? It should not. If you explicitly ask for an module and it doesn't load you'll get an error no matter what. This only skips the error message in case loading all qom modules (for '-device help' & friends) was requested. > What about the proposal to have a subdir per arch with symlinks? The modules are not per-arch. They just depend on pci or vga or usb being present in core qemu, and some qemu-system-$arch variants don't have that. So -- for example -- s390x has no vga support, therefore qxl doesn't load. qxl wasn't available before, so nothing fundamental changed. The only difference is that you get an additional error message line from the attempt to load the qxl module. Why is this a problem? take care, Gerd
diff --git a/util/module.c b/util/module.c index fe3b82dd4d37..9490f975d303 100644 --- a/util/module.c +++ b/util/module.c @@ -301,6 +301,9 @@ static struct { { "qxl", "hw-", "display-qxl" }, { "virtio-gpu-device", "hw-", "display-virtio-gpu" }, { "vhost-user-gpu", "hw-", "display-virtio-gpu" }, + { "virtio-gpu-pci-base", "hw-", "display-virtio-gpu-pci" }, + { "virtio-gpu-pci", "hw-", "display-virtio-gpu-pci" }, + { "vhost-user-gpu-pci", "hw-", "display-virtio-gpu-pci" }, { "chardev-braille", "chardev-", "baum" }, { "chardev-spicevmc", "chardev-", "spice" }, { "chardev-spiceport", "chardev-", "spice" }, diff --git a/hw/display/meson.build b/hw/display/meson.build index 0d5ddecd6503..669935371335 100644 --- a/hw/display/meson.build +++ b/hw/display/meson.build @@ -62,8 +62,15 @@ if config_all_devices.has_key('CONFIG_VIRTIO_GPU') hw_display_modules += {'virtio-gpu': virtio_gpu_ss} endif -softmmu_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRTIO_PCI'], if_true: files('virtio-gpu-pci.c')) -softmmu_ss.add(when: ['CONFIG_VHOST_USER_GPU', 'CONFIG_VIRTIO_PCI'], if_true: files('vhost-user-gpu-pci.c')) +if config_all_devices.has_key('CONFIG_VIRTIO_PCI') + virtio_gpu_pci_ss = ss.source_set() + virtio_gpu_pci_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRTIO_PCI'], + if_true: [files('virtio-gpu-pci.c'), pixman]) + virtio_gpu_pci_ss.add(when: ['CONFIG_VHOST_USER_GPU', 'CONFIG_VIRTIO_PCI'], + if_true: files('vhost-user-gpu-pci.c')) + hw_display_modules += {'virtio-gpu-pci': virtio_gpu_pci_ss} +endif + softmmu_ss.add(when: 'CONFIG_VIRTIO_VGA', if_true: files('virtio-vga.c')) softmmu_ss.add(when: 'CONFIG_VHOST_USER_VGA', if_true: files('vhost-user-vga.c'))
Build virtio-gpu pci devices modular. Must be a separate module because not all qemu softmmu variants come with PCI support. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- util/module.c | 3 +++ hw/display/meson.build | 11 +++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-)