qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/1] hw/s390x: modularize virtio-gpu-ccw


From: Thomas Huth
Subject: Re: [PATCH 1/1] hw/s390x: modularize virtio-gpu-ccw
Date: Thu, 18 Feb 2021 15:44:47 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0

On 18/02/2021 11.34, Halil Pasic wrote:
On Thu, 18 Feb 2021 10:23:16 +0100
Thomas Huth <thuth@redhat.com> wrote:

Since the virtio-gpu-ccw device depends on the hw-display-virtio-gpu
module, which provides the type virtio-gpu-device, packaging the
hw-display-virtio-gpu module as a separate package that may or may not
be installed along with the qemu package leads to problems. Namely if
the hw-display-virtio-gpu is absent, qemu continues to advertise
virtio-gpu-ccw, but it aborts not only when one attempts using
virtio-gpu-ccw, but also when libvirtd's capability probing tries
to instantiate the type to introspect it.

Let us thus introduce a module named hw-s390x-virtio-gpu-ccw that
is going to provide the virtio-gpu-ccw device. The hw-s390x prefix
was chosen because it is not a portable device.

With virtio-gpu-ccw built as a module, the correct way to package a
modularized qemu is to require that hw-display-virtio-gpu must be
installed whenever the module hw-s390x-virtio-gpu-ccw.

Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
---
   hw/s390x/meson.build | 17 ++++++++++++++++-
   util/module.c        |  1 +
   2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
index 2a7818d94b..153b1309fb 100644
--- a/hw/s390x/meson.build
+++ b/hw/s390x/meson.build
@@ -34,7 +34,6 @@ virtio_ss.add(files('virtio-ccw.c'))
   virtio_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: 
files('virtio-ccw-balloon.c'))
   virtio_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-ccw-blk.c'))
   virtio_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: 
files('virtio-ccw-crypto.c'))
-virtio_ss.add(when: 'CONFIG_VIRTIO_GPU', if_true: files('virtio-ccw-gpu.c'))
   virtio_ss.add(when: 'CONFIG_VIRTIO_INPUT', if_true: 
files('virtio-ccw-input.c'))
   virtio_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('virtio-ccw-net.c'))
   virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-ccw-rng.c'))
@@ -46,3 +45,19 @@ virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: 
files('vhost-user-fs-ccw.c'
   s390x_ss.add_all(when: 'CONFIG_VIRTIO_CCW', if_true: virtio_ss)
hw_arch += {'s390x': s390x_ss}
+
+if target.startswith('s390x')
+  hw_s390x_modules = {}
+
+  hw_s390x_modules_c_args = ['-DNEED_CPU_H',
+             '-DCONFIG_TARGET="@0@-config-target.h"'.format(target)]
+  hw_s390x_modules_inc = [include_directories('../../target' / 
config_target['TARGET_BASE_ARCH'])]
+  hw_s390x_modules_dependencies = declare_dependency(
+              include_directories: hw_s390x_modules_inc, compile_args: 
hw_s390x_modules_c_args)

Basically the patch looks fine to me, but I wonder why all that above lines
(related to hw_s390x_modules_dependencies) are requred at all? The other
display modules in hw/display/meson.build also do not need to re-define
c_args for example?

The explanation is simple. Unlike most devices, the ccw devices aren't
portable. In particular both css.c and css.h includes "cpu.h", and
virtio-ccw-gpu.c includes "qemu/osdep.h". Furthermore osdep.h contains:
#ifdef NEED_CPU_H
#include CONFIG_TARGET
#else
#include "exec/poison.h"
#endif
so if we don't have NEED_CPU_H, among others CONFIG_KVM is poisoned, and
CONFIG_KVM is used in "css.h". Frankly, I can't tell under what circumstances
does css need "cpu.h".
As far as I can see, the only real reason right now is the CONFIG_KVM section in css.h. I think you could simply move that into another header file instead (cpu.h ?)

 Thomas




reply via email to

[Prev in Thread] Current Thread [Next in Thread]