| Message ID | 20200622135601.12433-4-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | build some devices as modules. | expand |
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 >
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
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.
> >> 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
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
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
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(-)