All of lore.kernel.org
 help / color / mirror / Atom feed
From: Halil Pasic <pasic@linux.ibm.com>
To: "Cornelia Huck" <cohuck@redhat.com>,
	"Halil Pasic" <pasic@linux.ibm.com>,
	"Christian Borntraeger" <borntraeger@de.ibm.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"David Hildenbrand" <david@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	qemu-s390x@nongnu.org, qemu-devel@nongnu.org
Cc: Boris Fiuczynski <fiuczy@linux.ibm.com>, Bruce Rogers <brogers@suse.com>
Subject: [PATCH v2 1/1] hw/s390x: modularize virtio-gpu-ccw
Date: Mon, 22 Feb 2021 13:55:48 +0100	[thread overview]
Message-ID: <20210222125548.346166-1-pasic@linux.ibm.com> (raw)

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.

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.

Introducing type_register_mayfail() was suggested by Gerd Hoffmann.

Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
---
 hw/s390x/meson.build         |  7 +++++-
 hw/s390x/virtio-ccw-gpu.c    |  4 ++++
 include/hw/s390x/css.h       |  7 ------
 include/hw/s390x/s390_flic.h |  3 +++
 include/qom/object.h         | 22 +++++++++++++++++++
 qom/object.c                 | 42 ++++++++++++++++++++++++------------
 target/s390x/cpu.h           |  9 +++++---
 util/module.c                |  1 +
 8 files changed, 70 insertions(+), 25 deletions(-)

diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
index 2a7818d94b..7ac972afcf 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,9 @@ 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}
+
+hw_s390x_modules = {}
+virtio_gpu_ccw_ss = ss.source_set()
+virtio_gpu_ccw_ss.add(when: 'CONFIG_VIRTIO_GPU', if_true: [files('virtio-ccw-gpu.c'), pixman])
+hw_s390x_modules += {'virtio-gpu-ccw': virtio_gpu_ccw_ss}
+modules += {'hw-s390x': hw_s390x_modules}
diff --git a/hw/s390x/virtio-ccw-gpu.c b/hw/s390x/virtio-ccw-gpu.c
index c301e2586b..5ac9b6b2a6 100644
--- a/hw/s390x/virtio-ccw-gpu.c
+++ b/hw/s390x/virtio-ccw-gpu.c
@@ -62,7 +62,11 @@ static const TypeInfo virtio_ccw_gpu = {
 
 static void virtio_ccw_gpu_register(void)
 {
+#ifdef CONFIG_MODULES
+    type_register_static_mayfail(&virtio_ccw_gpu);
+#else
     type_register_static(&virtio_ccw_gpu);
+#endif
 }
 
 type_init(virtio_ccw_gpu_register)
diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
index 08c869ab0a..7858666307 100644
--- a/include/hw/s390x/css.h
+++ b/include/hw/s390x/css.h
@@ -12,7 +12,6 @@
 #ifndef CSS_H
 #define CSS_H
 
-#include "cpu.h"
 #include "hw/s390x/adapter.h"
 #include "hw/s390x/s390_flic.h"
 #include "hw/s390x/ioinst.h"
@@ -233,12 +232,6 @@ uint32_t css_get_adapter_id(CssIoAdapterType type, uint8_t isc);
 void css_register_io_adapters(CssIoAdapterType type, bool swap, bool maskable,
                               uint8_t flags, Error **errp);
 
-#ifndef CONFIG_KVM
-#define S390_ADAPTER_SUPPRESSIBLE 0x01
-#else
-#define S390_ADAPTER_SUPPRESSIBLE KVM_S390_ADAPTER_SUPPRESSIBLE
-#endif
-
 #ifndef CONFIG_USER_ONLY
 SubchDev *css_find_subch(uint8_t m, uint8_t cssid, uint8_t ssid,
                          uint16_t schid);
diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h
index e91b15d2d6..3907a13d07 100644
--- a/include/hw/s390x/s390_flic.h
+++ b/include/hw/s390x/s390_flic.h
@@ -134,6 +134,9 @@ void s390_flic_init(void);
 S390FLICState *s390_get_flic(void);
 QEMUS390FLICState *s390_get_qemu_flic(S390FLICState *fs);
 S390FLICStateClass *s390_get_flic_class(S390FLICState *fs);
+void s390_crw_mchk(void);
+void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr,
+                       uint32_t io_int_parm, uint32_t io_int_word);
 bool ais_needed(void *opaque);
 
 #endif /* HW_S390_FLIC_H */
diff --git a/include/qom/object.h b/include/qom/object.h
index 6721cd312e..3428546d91 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -826,6 +826,17 @@ const char *object_get_typename(const Object *obj);
  */
 Type type_register_static(const TypeInfo *info);
 
+/**
+ * type_register_static_mayfail:
+ * @info: The #TypeInfo of the new type.
+ *
+ * @info and all of the strings it points to should exist for the life time
+ * that the type is registered.
+ *
+ * Returns: the new #Type or NULL if missing a parent type.
+ */
+Type type_register_static_mayfail(const TypeInfo *info);
+
 /**
  * type_register:
  * @info: The #TypeInfo of the new type
@@ -837,6 +848,17 @@ Type type_register_static(const TypeInfo *info);
  */
 Type type_register(const TypeInfo *info);
 
+/**
+ * type_register_mayfail:
+ * @info: The #TypeInfo of the new type
+ *
+ * Unlike type_register_static(), this call does not require @info or its
+ * string members to continue to exist after the call returns.
+ *
+ * Returns: the new #Type or NULL if missing a parent type.
+ */
+Type type_register_mayfail(const TypeInfo *info);
+
 /**
  * type_register_static_array:
  * @infos: The array of the new type #TypeInfo structures.
diff --git a/qom/object.c b/qom/object.c
index 491823db4a..ed217cbfb0 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -135,7 +135,7 @@ static TypeImpl *type_new(const TypeInfo *info)
     return ti;
 }
 
-static TypeImpl *type_register_internal(const TypeInfo *info)
+static TypeImpl *type_register_internal(const TypeInfo *info, bool mayfail)
 {
     TypeImpl *ti;
     ti = type_new(info);
@@ -147,7 +147,13 @@ static TypeImpl *type_register_internal(const TypeInfo *info)
 TypeImpl *type_register(const TypeInfo *info)
 {
     assert(info->parent);
-    return type_register_internal(info);
+    return type_register_internal(info, false);
+}
+
+TypeImpl *type_register_mayfail(const TypeInfo *info)
+{
+    assert(info->parent);
+    return type_register_internal(info, true);
 }
 
 TypeImpl *type_register_static(const TypeInfo *info)
@@ -155,6 +161,11 @@ TypeImpl *type_register_static(const TypeInfo *info)
     return type_register(info);
 }
 
+TypeImpl *type_register_static_mayfail(const TypeInfo *info)
+{
+    return type_register_mayfail(info);
+}
+
 void type_register_static_array(const TypeInfo *infos, int nr_infos)
 {
     int i;
@@ -173,13 +184,16 @@ static TypeImpl *type_get_by_name(const char *name)
     return type_table_lookup(name);
 }
 
-static TypeImpl *type_get_parent(TypeImpl *type)
+static TypeImpl *type_get_parent(TypeImpl *type, bool mayfail)
 {
     if (!type->parent_type && type->parent) {
         type->parent_type = type_get_by_name(type->parent);
         if (!type->parent_type) {
             fprintf(stderr, "Type '%s' is missing its parent '%s'\n",
                     type->name, type->parent);
+            if (mayfail) {
+                return NULL;
+            }
             abort();
         }
     }
@@ -199,7 +213,7 @@ static size_t type_class_get_size(TypeImpl *ti)
     }
 
     if (type_has_parent(ti)) {
-        return type_class_get_size(type_get_parent(ti));
+        return type_class_get_size(type_get_parent(ti, false));
     }
 
     return sizeof(ObjectClass);
@@ -212,7 +226,7 @@ static size_t type_object_get_size(TypeImpl *ti)
     }
 
     if (type_has_parent(ti)) {
-        return type_object_get_size(type_get_parent(ti));
+        return type_object_get_size(type_get_parent(ti, false));
     }
 
     return 0;
@@ -236,7 +250,7 @@ static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type)
             return true;
         }
 
-        type = type_get_parent(type);
+        type = type_get_parent(type, false);
     }
 
     return false;
@@ -307,7 +321,7 @@ static void type_initialize(TypeImpl *ti)
     }
     ti->class = g_malloc0(ti->class_size);
 
-    parent = type_get_parent(ti);
+    parent = type_get_parent(ti, false);
     if (parent) {
         type_initialize(parent);
         GSList *e;
@@ -357,7 +371,7 @@ static void type_initialize(TypeImpl *ti)
         if (parent->class_base_init) {
             parent->class_base_init(ti->class, ti->class_data);
         }
-        parent = type_get_parent(parent);
+        parent = type_get_parent(parent, false);
     }
 
     if (ti->class_init) {
@@ -368,7 +382,7 @@ static void type_initialize(TypeImpl *ti)
 static void object_init_with_type(Object *obj, TypeImpl *ti)
 {
     if (type_has_parent(ti)) {
-        object_init_with_type(obj, type_get_parent(ti));
+        object_init_with_type(obj, type_get_parent(ti, false));
     }
 
     if (ti->instance_init) {
@@ -383,7 +397,7 @@ static void object_post_init_with_type(Object *obj, TypeImpl *ti)
     }
 
     if (type_has_parent(ti)) {
-        object_post_init_with_type(obj, type_get_parent(ti));
+        object_post_init_with_type(obj, type_get_parent(ti, false));
     }
 }
 
@@ -674,7 +688,7 @@ static void object_deinit(Object *obj, TypeImpl *type)
     }
 
     if (type_has_parent(type)) {
-        object_deinit(obj, type_get_parent(type));
+        object_deinit(obj, type_get_parent(type, false));
     }
 }
 
@@ -1040,7 +1054,7 @@ ObjectClass *module_object_class_by_name(const char *typename)
 
 ObjectClass *object_class_get_parent(ObjectClass *class)
 {
-    TypeImpl *type = type_get_parent(class->type);
+    TypeImpl *type = type_get_parent(class->type, false);
 
     if (!type) {
         return NULL;
@@ -2791,8 +2805,8 @@ static void register_types(void)
         .abstract = true,
     };
 
-    type_interface = type_register_internal(&interface_info);
-    type_register_internal(&object_info);
+    type_interface = type_register_internal(&interface_info, false);
+    type_register_internal(&object_info, false);
 }
 
 type_init(register_types)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 60d434d5ed..b434b905c0 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -40,6 +40,12 @@
 
 #define S390_MAX_CPUS 248
 
+#ifndef CONFIG_KVM
+#define S390_ADAPTER_SUPPRESSIBLE 0x01
+#else
+#define S390_ADAPTER_SUPPRESSIBLE KVM_S390_ADAPTER_SUPPRESSIBLE
+#endif
+
 typedef struct PSW {
     uint64_t mask;
     uint64_t addr;
@@ -806,9 +812,6 @@ int cpu_s390x_signal_handler(int host_signum, void *pinfo, void *puc);
 
 
 /* interrupt.c */
-void s390_crw_mchk(void);
-void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr,
-                       uint32_t io_int_parm, uint32_t io_int_word);
 #define RA_IGNORED                  0
 void s390_program_interrupt(CPUS390XState *env, uint32_t code, uintptr_t ra);
 /* service interrupts are floating therefore we must not pass an cpustate */
diff --git a/util/module.c b/util/module.c
index c65060c167..cbe89fede6 100644
--- a/util/module.c
+++ b/util/module.c
@@ -304,6 +304,7 @@ 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-gpu-ccw",        "hw-", "s390x-virtio-gpu-ccw"   },
     { "virtio-vga-base",       "hw-", "display-virtio-vga"    },
     { "virtio-vga",            "hw-", "display-virtio-vga"    },
     { "vhost-user-vga",        "hw-", "display-virtio-vga"    },

base-commit: 1af5629673bb5c1592d993f9fb6119a62845f576
-- 
2.25.1



             reply	other threads:[~2021-02-22 13:31 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-22 12:55 Halil Pasic [this message]
2021-02-22 17:18 ` [PATCH v2 1/1] hw/s390x: modularize virtio-gpu-ccw Boris Fiuczynski
2021-02-22 17:30   ` Daniel P. Berrangé
2021-02-23 12:14     ` Halil Pasic
2021-02-24 11:36 ` Gerd Hoffmann
2021-02-24 16:46   ` Halil Pasic
2021-02-25  8:05     ` Gerd Hoffmann
2021-02-25 21:14     ` Halil Pasic
2021-03-03  7:07       ` Gerd Hoffmann
2021-03-03 14:36         ` Halil Pasic

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210222125548.346166-1-pasic@linux.ibm.com \
    --to=pasic@linux.ibm.com \
    --cc=berrange@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=brogers@suse.com \
    --cc=cohuck@redhat.com \
    --cc=david@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=fiuczy@linux.ibm.com \
    --cc=kraxel@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-s390x@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.