qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] virtio-gpu: build pci and vga bits modular too.
@ 2020-10-23  6:46 Gerd Hoffmann
  2020-10-23  6:46 ` [PATCH 1/2] virtio-gpu: add virtio-gpu-pci module Gerd Hoffmann
  2020-10-23  6:46 ` [PATCH 2/2] virtio-gpu: add virtio-vga module Gerd Hoffmann
  0 siblings, 2 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2020-10-23  6:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann



Gerd Hoffmann (2):
  virtio-gpu: add virtio-gpu-pci module
  virtio-gpu: add virtio-vga module

 util/module.c          |  6 ++++++
 hw/display/meson.build | 21 +++++++++++++++++----
 2 files changed, 23 insertions(+), 4 deletions(-)

-- 
2.27.0




^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] virtio-gpu: add virtio-gpu-pci module
  2020-10-23  6:46 [PATCH 0/2] virtio-gpu: build pci and vga bits modular too Gerd Hoffmann
@ 2020-10-23  6:46 ` Gerd Hoffmann
  2020-10-23  8:05   ` Marc-André Lureau
  2020-10-23  6:46 ` [PATCH 2/2] virtio-gpu: add virtio-vga module Gerd Hoffmann
  1 sibling, 1 reply; 8+ messages in thread
From: Gerd Hoffmann @ 2020-10-23  6:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

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(-)

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



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] virtio-gpu: add virtio-vga module
  2020-10-23  6:46 [PATCH 0/2] virtio-gpu: build pci and vga bits modular too Gerd Hoffmann
  2020-10-23  6:46 ` [PATCH 1/2] virtio-gpu: add virtio-gpu-pci module Gerd Hoffmann
@ 2020-10-23  6:46 ` Gerd Hoffmann
  2020-10-23  8:07   ` Marc-André Lureau
  1 sibling, 1 reply; 8+ messages in thread
From: Gerd Hoffmann @ 2020-10-23  6:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Build virtio-gpu vga devices modular.  Must be a separate module because
not all qemu softmmu variants come with VGA support.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 util/module.c          |  3 +++
 hw/display/meson.build | 10 ++++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/util/module.c b/util/module.c
index 9490f975d303..503c399421c5 100644
--- a/util/module.c
+++ b/util/module.c
@@ -304,6 +304,9 @@ static struct {
     { "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" },
+    { "virtio-vga-base",       "hw-", "display-virtio-vga"    },
+    { "virtio-vga",            "hw-", "display-virtio-vga"    },
+    { "vhost-user-vga",        "hw-", "display-virtio-vga"    },
     { "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 669935371335..11ea2895c5fe 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -71,8 +71,14 @@ if config_all_devices.has_key('CONFIG_VIRTIO_PCI')
   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'))
+if config_all_devices.has_key('CONFIG_VIRTIO_VGA')
+  virtio_vga_ss = ss.source_set()
+  virtio_vga_ss.add(when: 'CONFIG_VIRTIO_VGA',
+                    if_true: [files('virtio-vga.c'), pixman])
+  virtio_vga_ss.add(when: 'CONFIG_VHOST_USER_VGA',
+                    if_true: files('vhost-user-vga.c'))
+  hw_display_modules += {'virtio-vga': virtio_vga_ss}
+endif
 
 specific_ss.add(when: [x11, opengl, 'CONFIG_MILKYMIST_TMU2'], if_true: files('milkymist-tmu2.c'))
 specific_ss.add(when: 'CONFIG_OMAP', if_true: files('omap_lcdc.c'))
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] virtio-gpu: add virtio-gpu-pci module
  2020-10-23  6:46 ` [PATCH 1/2] virtio-gpu: add virtio-gpu-pci module Gerd Hoffmann
@ 2020-10-23  8:05   ` Marc-André Lureau
  2020-10-23 11:01     ` Gerd Hoffmann
  0 siblings, 1 reply; 8+ messages in thread
From: Marc-André Lureau @ 2020-10-23  8:05 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: QEMU

[-- 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 --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] virtio-gpu: add virtio-vga module
  2020-10-23  6:46 ` [PATCH 2/2] virtio-gpu: add virtio-vga module Gerd Hoffmann
@ 2020-10-23  8:07   ` Marc-André Lureau
  0 siblings, 0 replies; 8+ messages in thread
From: Marc-André Lureau @ 2020-10-23  8:07 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: QEMU

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

On Fri, Oct 23, 2020 at 10:48 AM Gerd Hoffmann <kraxel@redhat.com> wrote:

> Build virtio-gpu vga devices modular.  Must be a separate module because
> not all qemu softmmu variants come with VGA support.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

---
>  util/module.c          |  3 +++
>  hw/display/meson.build | 10 ++++++++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/util/module.c b/util/module.c
> index 9490f975d303..503c399421c5 100644
> --- a/util/module.c
> +++ b/util/module.c
> @@ -304,6 +304,9 @@ static struct {
>      { "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" },
> +    { "virtio-vga-base",       "hw-", "display-virtio-vga"    },
> +    { "virtio-vga",            "hw-", "display-virtio-vga"    },
> +    { "vhost-user-vga",        "hw-", "display-virtio-vga"    },
>      { "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 669935371335..11ea2895c5fe 100644
> --- a/hw/display/meson.build
> +++ b/hw/display/meson.build
> @@ -71,8 +71,14 @@ if config_all_devices.has_key('CONFIG_VIRTIO_PCI')
>    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'))
> +if config_all_devices.has_key('CONFIG_VIRTIO_VGA')
> +  virtio_vga_ss = ss.source_set()
> +  virtio_vga_ss.add(when: 'CONFIG_VIRTIO_VGA',
> +                    if_true: [files('virtio-vga.c'), pixman])
> +  virtio_vga_ss.add(when: 'CONFIG_VHOST_USER_VGA',
> +                    if_true: files('vhost-user-vga.c'))
> +  hw_display_modules += {'virtio-vga': virtio_vga_ss}
> +endif
>
>  specific_ss.add(when: [x11, opengl, 'CONFIG_MILKYMIST_TMU2'], if_true:
> files('milkymist-tmu2.c'))
>  specific_ss.add(when: 'CONFIG_OMAP', if_true: files('omap_lcdc.c'))
> --
> 2.27.0
>
>
>

-- 
Marc-André Lureau

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] virtio-gpu: add virtio-gpu-pci module
  2020-10-23  8:05   ` Marc-André Lureau
@ 2020-10-23 11:01     ` Gerd Hoffmann
  2020-10-23 11:26       ` Marc-André Lureau
  0 siblings, 1 reply; 8+ messages in thread
From: Gerd Hoffmann @ 2020-10-23 11:01 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: QEMU

  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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] virtio-gpu: add virtio-gpu-pci module
  2020-10-23 11:01     ` Gerd Hoffmann
@ 2020-10-23 11:26       ` Marc-André Lureau
  2020-10-26  6:21         ` Gerd Hoffmann
  0 siblings, 1 reply; 8+ messages in thread
From: Marc-André Lureau @ 2020-10-23 11:26 UTC (permalink / raw)
  To: Gerd Hoffmann, Paolo Bonzini; +Cc: QEMU

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

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?

-- 
Marc-André Lureau

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] virtio-gpu: add virtio-gpu-pci module
  2020-10-23 11:26       ` Marc-André Lureau
@ 2020-10-26  6:21         ` Gerd Hoffmann
  0 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2020-10-26  6:21 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: Paolo Bonzini, QEMU

  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



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-10-26  6:23 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-23  6:46 [PATCH 0/2] virtio-gpu: build pci and vga bits modular too Gerd Hoffmann
2020-10-23  6:46 ` [PATCH 1/2] virtio-gpu: add virtio-gpu-pci module Gerd Hoffmann
2020-10-23  8:05   ` Marc-André Lureau
2020-10-23 11:01     ` Gerd Hoffmann
2020-10-23 11:26       ` Marc-André Lureau
2020-10-26  6:21         ` Gerd Hoffmann
2020-10-23  6:46 ` [PATCH 2/2] virtio-gpu: add virtio-vga module Gerd Hoffmann
2020-10-23  8:07   ` Marc-André Lureau

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).