[PATCH v4 0/7] build some devices as modules.

[PATCH v4 0/7] build some devices as modules.

[Gerd Hoffmann]

Specifically devices which depend on shared libraries,
to reduce the runtime dependencies of core qemu.

v2:
 - better commit messages.
 - add some more devices.
 - general tidy up.

v3:
 - rebase, solve stubs conflict.
 - fix -vga $name
 - fix -device $name,help

v4:
 - rebase to latest master
 - adapt to armbru's device init changes

Gerd Hoffmann (7):
  qdev: add support for device module loading
  build: fix device module builds
  ccid: build smartcard as module
  usb: build usb-redir as module
  vga: build qxl as module
  vga: build virtio-gpu only once
  vga: build virtio-gpu as module

 Makefile.objs            |  1 +
 Makefile.target          |  7 +++++
 include/hw/qdev-core.h   |  3 ++
 include/qemu/module.h    |  1 +
 hw/core/qdev.c           | 66 ++++++++++++++++++++++++++++++++++++++++
 qdev-monitor.c           |  5 +++
 qom/qom-qmp-cmds.c       |  5 +++
 softmmu/vl.c             |  4 +++
 stubs/hw-module.c        | 10 ++++++
 hw/Makefile.objs         |  2 ++
 hw/display/Makefile.objs | 28 ++++++++++-------
 hw/usb/Makefile.objs     | 13 +++++---
 stubs/Makefile.objs      |  1 +
 13 files changed, 131 insertions(+), 15 deletions(-)
 create mode 100644 stubs/hw-module.c

-- 
2.18.4

[PATCH v4 1/7] qdev: add support for device module loading

[Gerd Hoffmann]

When compiling devices as modules we'll need some infrastrtucture to
actually load those modules if needed.  This patch adds it.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/hw/qdev-core.h |  3 +++
 include/qemu/module.h  |  1 +
 hw/core/qdev.c         | 55 ++++++++++++++++++++++++++++++++++++++++++
 qdev-monitor.c         |  5 ++++
 qom/qom-qmp-cmds.c     |  5 ++++
 softmmu/vl.c           |  4 +++
 stubs/hw-module.c      | 10 ++++++++
 stubs/Makefile.objs    |  1 +
 8 files changed, 84 insertions(+)
 create mode 100644 stubs/hw-module.c

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 7dc10be46f8c..0a33939f4855 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -557,4 +557,7 @@ void device_listener_unregister(DeviceListener *listener);
  */
 bool qdev_should_hide_device(QemuOpts *opts);
 
+void qdev_module_load_type(const char *type);
+void qdev_module_load_all(void);
+
 #endif
diff --git a/include/qemu/module.h b/include/qemu/module.h
index 011ae1ae7605..077a6b09bca7 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -64,6 +64,7 @@ typedef enum {
 #define block_module_load_one(lib) module_load_one("block-", lib)
 #define ui_module_load_one(lib) module_load_one("ui-", lib)
 #define audio_module_load_one(lib) module_load_one("audio-", lib)
+#define hw_module_load_one(lib) module_load_one("hw-", lib)
 
 void register_module_init(void (*fn)(void), module_init_type type);
 void register_dso_module_init(void (*fn)(void), module_init_type type);
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 2131c7f951dd..6119e25e0eeb 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -137,6 +137,9 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
  */
 DeviceState *qdev_new(const char *name)
 {
+    if (!object_class_by_name(name)) {
+        qdev_module_load_type(name);
+    }
     return DEVICE(object_new(name));
 }
 
@@ -147,6 +150,9 @@ DeviceState *qdev_new(const char *name)
  */
 DeviceState *qdev_try_new(const char *name)
 {
+    if (!object_class_by_name(name)) {
+        qdev_module_load_type(name);
+    }
     if (!object_class_by_name(name)) {
         return NULL;
     }
@@ -154,6 +160,55 @@ DeviceState *qdev_try_new(const char *name)
     return DEVICE(object_new(name));
 }
 
+/*
+ * Building devices modular is mostly useful in case they have
+ * dependencies to external shared libraries, so we can cut down the
+ * core qemu library dependencies.  Which is the case for only a very
+ * few devices.  So with the expectation that this will be rather the
+ * exception than to rule go with a simple hardcoded list for now
+ * (instead of generating it automatically somehow).
+ */
+static struct {
+    const char *type;
+    const char *mod;
+} const hwmodules[] = {
+};
+
+static bool qdev_module_loaded_all;
+
+void qdev_module_load_type(const char *type)
+{
+    int i;
+
+    if (qdev_module_loaded_all) {
+        return;
+    }
+    for (i = 0; i < ARRAY_SIZE(hwmodules); i++) {
+        if (strcmp(hwmodules[i].type, type) == 0) {
+            hw_module_load_one(hwmodules[i].mod);
+            return;
+        }
+    }
+}
+
+void qdev_module_load_all(void)
+{
+    int i;
+
+    if (qdev_module_loaded_all) {
+        return;
+    }
+    for (i = 0; i < ARRAY_SIZE(hwmodules); i++) {
+        if (i > 0 && strcmp(hwmodules[i - 1].mod,
+                            hwmodules[i].mod) == 0) {
+            /* one module implementing multiple devices -> load only once */
+            continue;
+        }
+        hw_module_load_one(hwmodules[i].mod);
+    }
+    qdev_module_loaded_all = true;
+}
+
 static QTAILQ_HEAD(, DeviceListener) device_listeners
     = QTAILQ_HEAD_INITIALIZER(device_listeners);
 
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 22da107484c5..86f29daa7469 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -147,6 +147,7 @@ static void qdev_print_devinfos(bool show_no_user)
     int i;
     bool cat_printed;
 
+    qdev_module_load_all();
     list = object_class_get_list_sorted(TYPE_DEVICE, false);
 
     for (i = 0; i <= DEVICE_CATEGORY_MAX; i++) {
@@ -224,6 +225,10 @@ static DeviceClass *qdev_get_device_class(const char **driver, Error **errp)
             oc = object_class_by_name(*driver);
         }
     }
+    if (!oc) {
+        qdev_module_load_type(*driver);
+        oc = object_class_by_name(*driver);
+    }
 
     if (!object_class_dynamic_cast(oc, TYPE_DEVICE)) {
         if (*driver != original_name) {
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index c5249e44d020..4e0d7e53cd16 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -116,6 +116,7 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements,
 {
     ObjectTypeInfoList *ret = NULL;
 
+    qdev_module_load_all();
     object_class_foreach(qom_list_types_tramp, implements, abstract, &ret);
 
     return ret;
@@ -131,6 +132,10 @@ ObjectPropertyInfoList *qmp_device_list_properties(const char *typename,
     ObjectPropertyInfoList *prop_list = NULL;
 
     klass = object_class_by_name(typename);
+    if (klass == NULL) {
+        qdev_module_load_type(typename);
+        klass = object_class_by_name(typename);
+    }
     if (klass == NULL) {
         error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
                   "Device '%s' not found", typename);
diff --git a/softmmu/vl.c b/softmmu/vl.c
index f669c06ede4a..8d753ead5983 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1770,6 +1770,10 @@ static bool vga_interface_available(VGAInterfaceType t)
 {
     const VGAInterfaceInfo *ti = &vga_interfaces[t];
 
+    if (ti->class_names[0] && !object_class_by_name(ti->class_names[0])) {
+        qdev_module_load_type(ti->class_names[0]);
+    }
+
     assert(t < VGA_TYPE_MAX);
     return !ti->class_names[0] ||
            object_class_by_name(ti->class_names[0]) ||
diff --git a/stubs/hw-module.c b/stubs/hw-module.c
new file mode 100644
index 000000000000..58b2160b97fe
--- /dev/null
+++ b/stubs/hw-module.c
@@ -0,0 +1,10 @@
+#include "qemu/osdep.h"
+#include "hw/qdev-core.h"
+
+void qdev_module_load_all(void)
+{
+}
+
+void qdev_module_load_type(const char *type)
+{
+}
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 28e48171d1f3..7a9f38f60b78 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -5,6 +5,7 @@ stub-obj-y += dump.o
 stub-obj-y += error-printf.o
 stub-obj-y += fdset.o
 stub-obj-y += gdbstub.o
+stub-obj-y += hw-module.o
 stub-obj-y += iothread-lock.o
 stub-obj-y += is-daemonized.o
 stub-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
-- 
2.18.4

[PATCH v4 2/7] build: fix device module builds

[Gerd Hoffmann]

See comment.  Feels quite hackish.  Better ideas anyone?

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 Makefile.target | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Makefile.target b/Makefile.target
index 8ed1eba95b9c..c70325df5796 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -179,6 +179,13 @@ endif # CONFIG_SOFTMMU
 dummy := $(call unnest-vars,,obj-y)
 all-obj-y := $(obj-y)
 
+#
+# common-obj-m has some crap here, probably as side effect from
+# filling obj-y.  Clear it.  Fixes suspious dependency errors when
+# building devices as modules.
+#
+common-obj-m :=
+
 include $(SRC_PATH)/Makefile.objs
 dummy := $(call unnest-vars,.., \
                authz-obj-y \
-- 
2.18.4

[PATCH v4 3/7] ccid: build smartcard as module

[Gerd Hoffmann]

Drops libcacard.so dependency from core qemu.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 Makefile.objs        | 1 +
 hw/core/qdev.c       | 2 ++
 hw/Makefile.objs     | 1 +
 hw/usb/Makefile.objs | 4 +++-
 4 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Makefile.objs b/Makefile.objs
index 7ce2588b89a3..ca555ede0710 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -59,6 +59,7 @@ common-obj-y += migration/
 common-obj-y += audio/
 common-obj-m += audio/
 common-obj-y += hw/
+common-obj-m += hw/
 
 common-obj-y += replay/
 
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6119e25e0eeb..d6459e4aa8e8 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -172,6 +172,8 @@ static struct {
     const char *type;
     const char *mod;
 } const hwmodules[] = {
+    { .type = "ccid-card-passthru",    .mod = "usb-smartcard"         },
+    { .type = "ccid-card-emulated",    .mod = "usb-smartcard"         },
 };
 
 static bool qdev_module_loaded_all;
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 4cbe5e4e57d6..af8fd9a510ed 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -43,4 +43,5 @@ devices-dirs-y += smbios/
 endif
 
 common-obj-y += $(devices-dirs-y)
+common-obj-m += usb/
 obj-y += $(devices-dirs-y)
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index fa5c3fa1b877..3c5b3d4fadd3 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -29,11 +29,13 @@ common-obj-$(CONFIG_USB_NETWORK)      += dev-network.o
 
 ifeq ($(CONFIG_USB_SMARTCARD),y)
 common-obj-y                          += dev-smartcard-reader.o
-common-obj-$(CONFIG_SMARTCARD)        += smartcard.mo
+ifeq ($(CONFIG_SMARTCARD),y)
+common-obj-m                          += smartcard.mo
 smartcard.mo-objs := ccid-card-passthru.o ccid-card-emulated.o
 smartcard.mo-cflags := $(SMARTCARD_CFLAGS)
 smartcard.mo-libs := $(SMARTCARD_LIBS)
 endif
+endif
 
 ifeq ($(CONFIG_POSIX),y)
 common-obj-$(CONFIG_USB_STORAGE_MTP)  += dev-mtp.o
-- 
2.18.4

[PATCH v4 4/7] usb: build usb-redir as module

[Gerd Hoffmann]

Drops libusbredirparser.so dependency from core qemu.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/core/qdev.c       | 1 +
 hw/usb/Makefile.objs | 9 ++++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index d6459e4aa8e8..e77f40db675c 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -174,6 +174,7 @@ static struct {
 } const hwmodules[] = {
     { .type = "ccid-card-passthru",    .mod = "usb-smartcard"         },
     { .type = "ccid-card-emulated",    .mod = "usb-smartcard"         },
+    { .type = "usb-redir",             .mod = "usb-redirect"          },
 };
 
 static bool qdev_module_loaded_all;
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 3c5b3d4fadd3..3bb8dd53421a 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -43,9 +43,12 @@ endif
 
 # usb redirection
 ifeq ($(CONFIG_USB),y)
-common-obj-$(CONFIG_USB_REDIR) += redirect.o quirks.o
-redirect.o-cflags = $(USB_REDIR_CFLAGS)
-redirect.o-libs = $(USB_REDIR_LIBS)
+ifeq ($(CONFIG_USB-REDIR),y)
+common-obj-m += redirect.mo
+redirect.mo-objs = redirect.o quirks.o
+redirect.mo-cflags = $(USB_REDIR_CFLAGS)
+redirect.mo-libs = $(USB_REDIR_LIBS)
+endif
 endif
 
 # usb pass-through
-- 
2.18.4

[PATCH v4 5/7] vga: build qxl as module

[Gerd Hoffmann]

First step in making spice support modular.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/core/qdev.c           | 2 ++
 hw/Makefile.objs         | 1 +
 hw/display/Makefile.objs | 5 ++++-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index e77f40db675c..fe3dec9595cd 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -175,6 +175,8 @@ static struct {
     { .type = "ccid-card-passthru",    .mod = "usb-smartcard"         },
     { .type = "ccid-card-emulated",    .mod = "usb-smartcard"         },
     { .type = "usb-redir",             .mod = "usb-redirect"          },
+    { .type = "qxl-vga",               .mod = "display-qxl"           },
+    { .type = "qxl",                   .mod = "display-qxl"           },
 };
 
 static bool qdev_module_loaded_all;
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index af8fd9a510ed..14b7ea4eb62e 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -43,5 +43,6 @@ devices-dirs-y += smbios/
 endif
 
 common-obj-y += $(devices-dirs-y)
+common-obj-m += display/
 common-obj-m += usb/
 obj-y += $(devices-dirs-y)
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index 77a7d622bd2d..76b3571e4902 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -44,7 +44,10 @@ common-obj-$(CONFIG_ARTIST) += artist.o
 
 obj-$(CONFIG_VGA) += vga.o
 
-common-obj-$(CONFIG_QXL) += qxl.o qxl-logger.o qxl-render.o
+ifeq ($(CONFIG_QXL),y)
+common-obj-m += qxl.mo
+qxl.mo-objs = qxl.o qxl-logger.o qxl-render.o
+endif
 
 obj-$(CONFIG_VIRTIO_GPU) += virtio-gpu-base.o virtio-gpu.o virtio-gpu-3d.o
 obj-$(CONFIG_VHOST_USER_GPU) += vhost-user-gpu.o
-- 
2.18.4

[PATCH v4 6/7] vga: build virtio-gpu only once

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/display/Makefile.objs | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index 76b3571e4902..d619594ad4d3 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -49,12 +49,12 @@ common-obj-m += qxl.mo
 qxl.mo-objs = qxl.o qxl-logger.o qxl-render.o
 endif
 
-obj-$(CONFIG_VIRTIO_GPU) += virtio-gpu-base.o virtio-gpu.o virtio-gpu-3d.o
-obj-$(CONFIG_VHOST_USER_GPU) += vhost-user-gpu.o
-obj-$(call land,$(CONFIG_VIRTIO_GPU),$(CONFIG_VIRTIO_PCI)) += virtio-gpu-pci.o
-obj-$(call land,$(CONFIG_VHOST_USER_GPU),$(CONFIG_VIRTIO_PCI)) += vhost-user-gpu-pci.o
-obj-$(CONFIG_VIRTIO_VGA) += virtio-vga.o
-obj-$(CONFIG_VHOST_USER_VGA) += vhost-user-vga.o
+common-obj-$(CONFIG_VIRTIO_GPU) += virtio-gpu-base.o virtio-gpu.o virtio-gpu-3d.o
+common-obj-$(CONFIG_VHOST_USER_GPU) += vhost-user-gpu.o
+common-obj-$(call land,$(CONFIG_VIRTIO_GPU),$(CONFIG_VIRTIO_PCI)) += virtio-gpu-pci.o
+common-obj-$(call land,$(CONFIG_VHOST_USER_GPU),$(CONFIG_VIRTIO_PCI)) += vhost-user-gpu-pci.o
+common-obj-$(CONFIG_VIRTIO_VGA) += virtio-vga.o
+common-obj-$(CONFIG_VHOST_USER_VGA) += vhost-user-vga.o
 virtio-gpu.o-cflags := $(VIRGL_CFLAGS)
 virtio-gpu.o-libs += $(VIRGL_LIBS)
 virtio-gpu-3d.o-cflags := $(VIRGL_CFLAGS)
-- 
2.18.4

[PATCH v4 7/7] vga: build virtio-gpu as module

[Gerd Hoffmann]

Drops libvirglrenderer.so dependency from core qemu.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/core/qdev.c           |  6 ++++++
 hw/display/Makefile.objs | 23 +++++++++++++----------
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index fe3dec9595cd..59b9c45e607f 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -177,6 +177,12 @@ static struct {
     { .type = "usb-redir",             .mod = "usb-redirect"          },
     { .type = "qxl-vga",               .mod = "display-qxl"           },
     { .type = "qxl",                   .mod = "display-qxl"           },
+    { .type = "virtio-gpu-device",     .mod = "display-virtio-gpu"    },
+    { .type = "virtio-gpu-pci",        .mod = "display-virtio-gpu"    },
+    { .type = "virtio-vga",            .mod = "display-virtio-gpu"    },
+    { .type = "vhost-user-gpu-device", .mod = "display-virtio-gpu"    },
+    { .type = "vhost-user-gpu-pci",    .mod = "display-virtio-gpu"    },
+    { .type = "vhost-user-vga",        .mod = "display-virtio-gpu"    },
 };
 
 static bool qdev_module_loaded_all;
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index d619594ad4d3..e907f3182b0c 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -49,16 +49,19 @@ common-obj-m += qxl.mo
 qxl.mo-objs = qxl.o qxl-logger.o qxl-render.o
 endif
 
-common-obj-$(CONFIG_VIRTIO_GPU) += virtio-gpu-base.o virtio-gpu.o virtio-gpu-3d.o
-common-obj-$(CONFIG_VHOST_USER_GPU) += vhost-user-gpu.o
-common-obj-$(call land,$(CONFIG_VIRTIO_GPU),$(CONFIG_VIRTIO_PCI)) += virtio-gpu-pci.o
-common-obj-$(call land,$(CONFIG_VHOST_USER_GPU),$(CONFIG_VIRTIO_PCI)) += vhost-user-gpu-pci.o
-common-obj-$(CONFIG_VIRTIO_VGA) += virtio-vga.o
-common-obj-$(CONFIG_VHOST_USER_VGA) += vhost-user-vga.o
-virtio-gpu.o-cflags := $(VIRGL_CFLAGS)
-virtio-gpu.o-libs += $(VIRGL_LIBS)
-virtio-gpu-3d.o-cflags := $(VIRGL_CFLAGS)
-virtio-gpu-3d.o-libs += $(VIRGL_LIBS)
+ifeq ($(CONFIG_VIRTIO_GPU),y)
+common-obj-m += virtio-gpu.mo
+virtio-gpu-obj-$(CONFIG_VIRTIO_GPU) += virtio-gpu-base.o virtio-gpu.o virtio-gpu-3d.o
+virtio-gpu-obj-$(CONFIG_VHOST_USER_GPU) += vhost-user-gpu.o
+virtio-gpu-obj-$(call land,$(CONFIG_VIRTIO_GPU),$(CONFIG_VIRTIO_PCI)) += virtio-gpu-pci.o
+virtio-gpu-obj-$(call land,$(CONFIG_VHOST_USER_GPU),$(CONFIG_VIRTIO_PCI)) += vhost-user-gpu-pci.o
+virtio-gpu-obj-$(CONFIG_VIRTIO_VGA) += virtio-vga.o
+virtio-gpu-obj-$(CONFIG_VHOST_USER_VGA) += vhost-user-vga.o
+virtio-gpu.mo-objs := $(virtio-gpu-obj-y)
+virtio-gpu.mo-cflags := $(VIRGL_CFLAGS)
+virtio-gpu.mo-libs := $(VIRGL_LIBS)
+endif
+
 common-obj-$(CONFIG_DPCD) += dpcd.o
 common-obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx_dp.o
 
-- 
2.18.4

Re: [PATCH v4 0/7] build some devices as modules.

[Stefan Hajnoczi]

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

On Mon, Jun 22, 2020 at 03:55:54PM +0200, Gerd Hoffmann wrote:
> Specifically devices which depend on shared libraries,
> to reduce the runtime dependencies of core qemu.

Just wanted to say great that you are doing this! Nice that progress is
being made on QEMU modularization.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 3/7] ccid: build smartcard as module

[Philippe Mathieu-Daudé]

Hi Gerd,

On 6/22/20 3:55 PM, Gerd Hoffmann wrote:
> Drops libcacard.so dependency from core qemu.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  Makefile.objs        | 1 +
>  hw/core/qdev.c       | 2 ++
>  hw/Makefile.objs     | 1 +
>  hw/usb/Makefile.objs | 4 +++-
>  4 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 7ce2588b89a3..ca555ede0710 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -59,6 +59,7 @@ common-obj-y += migration/
>  common-obj-y += audio/
>  common-obj-m += audio/
>  common-obj-y += hw/
> +common-obj-m += hw/
>  
>  common-obj-y += replay/
>  
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 6119e25e0eeb..d6459e4aa8e8 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -172,6 +172,8 @@ static struct {
>      const char *type;
>      const char *mod;
>  } const hwmodules[] = {
> +    { .type = "ccid-card-passthru",    .mod = "usb-smartcard"         },
> +    { .type = "ccid-card-emulated",    .mod = "usb-smartcard"         },

We want to use type definitions here (such TYPE_CCID_PASSTHRU),
as we don't guaranty them stable.

We don't want to include various "hw/x/y.h" in this core file.

Since there is a relation between QOM type and the module,
can we store/use the module name in the TypeInfo declaration?

  static const TypeInfo passthru_card_info = {
      .name          = TYPE_CCID_PASSTHRU,
      .parent        = TYPE_CCID_CARD,
      .instance_size = sizeof(PassthruState),
      .class_init    = passthru_class_initfn,
      .module_name   = "usb-smartcard",        <=====
  };

Actually this modularization is not specific to QDEV
and can be used to all QOM right? I.e:

  static const TypeInfo qcrypto_tls_creds_x509_info = {
      .parent = TYPE_QCRYPTO_TLS_CREDS,
      .name = TYPE_QCRYPTO_TLS_CREDS_X509,
      .module_name = "gnu-tls",
      ...
  }

>  };
>  
>  static bool qdev_module_loaded_all;
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 4cbe5e4e57d6..af8fd9a510ed 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -43,4 +43,5 @@ devices-dirs-y += smbios/
>  endif
>  
>  common-obj-y += $(devices-dirs-y)
> +common-obj-m += usb/
>  obj-y += $(devices-dirs-y)
> diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
> index fa5c3fa1b877..3c5b3d4fadd3 100644
> --- a/hw/usb/Makefile.objs
> +++ b/hw/usb/Makefile.objs
> @@ -29,11 +29,13 @@ common-obj-$(CONFIG_USB_NETWORK)      += dev-network.o
>  
>  ifeq ($(CONFIG_USB_SMARTCARD),y)
>  common-obj-y                          += dev-smartcard-reader.o
> -common-obj-$(CONFIG_SMARTCARD)        += smartcard.mo
> +ifeq ($(CONFIG_SMARTCARD),y)
> +common-obj-m                          += smartcard.mo
>  smartcard.mo-objs := ccid-card-passthru.o ccid-card-emulated.o
>  smartcard.mo-cflags := $(SMARTCARD_CFLAGS)
>  smartcard.mo-libs := $(SMARTCARD_LIBS)
>  endif
> +endif
>  
>  ifeq ($(CONFIG_POSIX),y)
>  common-obj-$(CONFIG_USB_STORAGE_MTP)  += dev-mtp.o
> 

Re: [PATCH v4 3/7] ccid: build smartcard as module

[Gerd Hoffmann]

  Hi,

> > +    { .type = "ccid-card-passthru",    .mod = "usb-smartcard"         },
> > +    { .type = "ccid-card-emulated",    .mod = "usb-smartcard"         },
> 
> We want to use type definitions here (such TYPE_CCID_PASSTHRU),
> as we don't guaranty them stable.

Hmm?  I'm pretty sure '-device ccid-card-passthru' *is* stable ABI.

> Since there is a relation between QOM type and the module,
> can we store/use the module name in the TypeInfo declaration?
> 
>   static const TypeInfo passthru_card_info = {
>       .name          = TYPE_CCID_PASSTHRU,
>       .parent        = TYPE_CCID_CARD,
>       .instance_size = sizeof(PassthruState),
>       .class_init    = passthru_class_initfn,
>       .module_name   = "usb-smartcard",        <=====
>   };

That doesn't buy us much, the TypeInfo ends up in the module not qemu.
So qemu can't access it without loading the module.

We do *not* want load all modules on startup though.  Which means we
need a such list in qemu.  The struct above is just that.  There
certainly is room for improvement, building that list automatically
somehow for example.

Given that most devices don't depend on external shared libraries I
expect the list of device modules will stay relatively short.  So I've
decided to start with something simple and see how it goes (see also
patch 1/7).

> Actually this modularization is not specific to QDEV
> and can be used to all QOM right? I.e:
> 
>   static const TypeInfo qcrypto_tls_creds_x509_info = {
>       .parent = TYPE_QCRYPTO_TLS_CREDS,
>       .name = TYPE_QCRYPTO_TLS_CREDS_X509,
>       .module_name = "gnu-tls",
>       ...
>   }

Not as-is.  You'll need module load hooks in more places then and some
code tweaks to move it from qdev level (loading hw-* module only) to qom
level.

But, yes, moving the infrastructure to some qom-module.c file might be
useful when modularizing non-device objects.  Do you have any candidates
in mind?

take care,
  Gerd

Re: [PATCH v4 1/7] qdev: add support for device module loading

[Richard Henderson]

On 6/22/20 6:55 AM, Gerd Hoffmann wrote:
> @@ -147,6 +150,9 @@ DeviceState *qdev_new(const char *name)
>   */
>  DeviceState *qdev_try_new(const char *name)
>  {
> +    if (!object_class_by_name(name)) {
> +        qdev_module_load_type(name);
> +    }
>      if (!object_class_by_name(name)) {
>          return NULL;
>      }

The second if can be nested inside the first.
Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v4 3/7] ccid: build smartcard as module

[Philippe Mathieu-Daudé]

On 6/23/20 7:12 PM, Gerd Hoffmann wrote:
>   Hi,
> 
>>> +    { .type = "ccid-card-passthru",    .mod = "usb-smartcard"         },
>>> +    { .type = "ccid-card-emulated",    .mod = "usb-smartcard"         },
>>
>> We want to use type definitions here (such TYPE_CCID_PASSTHRU),
>> as we don't guaranty them stable.
> 
> Hmm?  I'm pretty sure '-device ccid-card-passthru' *is* stable ABI.

Asking on IRC, there is no explicit contract.

But as you remarked, doing so would break the CLI, so we should
some day clarify that objects implementing TYPE_USER_CREATABLE
can not have their typename changed. For the rest, there is no
restriction.

>> Since there is a relation between QOM type and the module,
>> can we store/use the module name in the TypeInfo declaration?
>>
>>   static const TypeInfo passthru_card_info = {
>>       .name          = TYPE_CCID_PASSTHRU,
>>       .parent        = TYPE_CCID_CARD,
>>       .instance_size = sizeof(PassthruState),
>>       .class_init    = passthru_class_initfn,
>>       .module_name   = "usb-smartcard",        <=====
>>   };
> 
> That doesn't buy us much, the TypeInfo ends up in the module not qemu.
> So qemu can't access it without loading the module.
> 
> We do *not* want load all modules on startup though.  Which means we
> need a such list in qemu.  The struct above is just that.  There
> certainly is room for improvement, building that list automatically
> somehow for example.

OK.

> Given that most devices don't depend on external shared libraries I
> expect the list of device modules will stay relatively short.  So I've
> decided to start with something simple and see how it goes (see also
> patch 1/7).
> 
>> Actually this modularization is not specific to QDEV
>> and can be used to all QOM right? I.e:
>>
>>   static const TypeInfo qcrypto_tls_creds_x509_info = {
>>       .parent = TYPE_QCRYPTO_TLS_CREDS,
>>       .name = TYPE_QCRYPTO_TLS_CREDS_X509,
>>       .module_name = "gnu-tls",
>>       ...
>>   }
> 
> Not as-is.  You'll need module load hooks in more places then and some
> code tweaks to move it from qdev level (loading hw-* module only) to qom
> level.
> 
> But, yes, moving the infrastructure to some qom-module.c file might be
> useful when modularizing non-device objects.  Do you have any candidates
> in mind?

So far I was only thinking of gnutls.

Re: [PATCH v4 3/7] ccid: build smartcard as module

[Gerd Hoffmann]

> >>   static const TypeInfo qcrypto_tls_creds_x509_info = {
> >>       .parent = TYPE_QCRYPTO_TLS_CREDS,
> >>       .name = TYPE_QCRYPTO_TLS_CREDS_X509,
> >>       .module_name = "gnu-tls",
> >>       ...
> >>   }
> > 
> > Not as-is.  You'll need module load hooks in more places then and some
> > code tweaks to move it from qdev level (loading hw-* module only) to qom
> > level.

[ note: v5 of the series does this ]

> > But, yes, moving the infrastructure to some qom-module.c file might be
> > useful when modularizing non-device objects.  Do you have any candidates
> > in mind?
> 
> So far I was only thinking of gnutls.

Looks challenging on a quick glance ...

take care,
  Gerd

Re: [PATCH v4 3/7] ccid: build smartcard as module

[Daniel P. Berrangé]

On Tue, Jun 30, 2020 at 06:07:51PM +0200, Gerd Hoffmann wrote:
> > >>   static const TypeInfo qcrypto_tls_creds_x509_info = {
> > >>       .parent = TYPE_QCRYPTO_TLS_CREDS,
> > >>       .name = TYPE_QCRYPTO_TLS_CREDS_X509,
> > >>       .module_name = "gnu-tls",
> > >>       ...
> > >>   }
> > > 
> > > Not as-is.  You'll need module load hooks in more places then and some
> > > code tweaks to move it from qdev level (loading hw-* module only) to qom
> > > level.
> 
> [ note: v5 of the series does this ]
> 
> > > But, yes, moving the infrastructure to some qom-module.c file might be
> > > useful when modularizing non-device objects.  Do you have any candidates
> > > in mind?
> > 
> > So far I was only thinking of gnutls.
> 
> Looks challenging on a quick glance ...

Yeah, I'm not convinced modularizing that is a good use of time. It is
plumbed in across multiple backends (chardev, migration, block, ui)
and also providing the secure RNG used across QEMU.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|