[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 1/1] hw/s390x: modularize virtio-gpu-ccw
From: |
Cornelia Huck |
Subject: |
Re: [PATCH v3 1/1] hw/s390x: modularize virtio-gpu-ccw |
Date: |
Fri, 5 Mar 2021 12:54:42 +0100 |
On Tue, 2 Mar 2021 18:35:44 +0100
Halil Pasic <pasic@linux.ibm.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. Because registering
> virtio-gpu-ccw would make non-s390x emulator fail due to a missing
> parent type, if built as a module, before registering it, we check
> if the ancestor types are already registered.
>
> 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.
>
> The definition S390_ADAPTER_SUPPRESSIBLE was moved to "cpu.h", per
> suggestion of Thomas Huth. From interface design perspective, IMHO, not
> a good thing as it belongs to the public interface of
> css_register_io_adapters(). We did this because CONFIG_KVM requeires
> NEED_CPU_H and Thomas, and other commenters did not like the
> consequences of that.
>
> Moving the interrupt related declarations to s390_flic.h was suggested
> by Cornelia Huck.
>
> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> ---
>
> As explained in [1] the previous idea of type_register_mayfail() does
> not work. The next best thing is to check if all types we need are
> already registered before registering virtio-gpu-ccw from the module. It
> is reasonable to assume that when the module is loaded, the ancestors
> are already registered (which is not the case if the device is a
> built in one).
>
> The alternatives to this approch I could identify are:
> * A poor mans version of this which checks for the parent
> * Emulator specific modules:
> * An emulator specific directory within the modules directory which
> is ignored by the other emulators.
> * A way to tell the shared util library the name of this directory,
> and the code to check it if set.
> * Build hw-s390x-virtio-gpu-ccw so it lands in this special directory
> in the build tree, and install it there as well.
> I've spend some time with looking into this, but I came to the
> conclusion that the two latter points look hairy.
>
> [1]
> https://lore.kernel.org/qemu-devel/20210222125548.346166-1-pasic@linux.ibm.com/T/#maf0608df5479f87b23606f01f732740d2617b458
> ---
> hw/s390x/meson.build | 7 ++++-
> hw/s390x/virtio-ccw-gpu.c | 5 ++++
> include/hw/s390x/css.h | 7 -----
> include/hw/s390x/s390_flic.h | 3 +++
> include/qom/object.h | 10 ++++++++
> qom/object.c | 50 ++++++++++++++++++++++++++++++++++++
> target/s390x/cpu.h | 9 ++++---
> util/module.c | 1 +
> 8 files changed, 81 insertions(+), 11 deletions(-)
The s390x part looks fine, but I'm not that well versed in the object
and module stuff...