Re: [PATCH 1/2] virtio-gpu: add virtio-gpu-pci module - Marc-André Lureau

From: "Marc-André Lureau" <marcandre.lureau@gmail.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: QEMU <qemu-devel@nongnu.org>
Subject: Re: [PATCH 1/2] virtio-gpu: add virtio-gpu-pci module
Date: Fri, 23 Oct 2020 12:05:48 +0400	[thread overview]
Message-ID: <CAJ+F1CK0A1+vOMZq55guJi9OPg1zyVyRQ_azYA7NNVg5Kx5hDA@mail.gmail.com> (raw)
In-Reply-To: <20201023064618.21409-2-kraxel@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 3563 bytes --]

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>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 5139 bytes --]