On Thu, Jun 10, 2021 at 07:57:54AM +0200, Gerd Hoffmann wrote: > Add module_allow_arch() to set the target architecture. > In case a module is limited to some arch verify arches > match and ignore the module if not. > > Signed-off-by: Gerd Hoffmann <kra...@redhat.com> > --- > include/qemu/module.h | 1 + > softmmu/vl.c | 3 +++ > util/module.c | 15 +++++++++++++++ > 3 files changed, 19 insertions(+) > > diff --git a/include/qemu/module.h b/include/qemu/module.h > index d3cab3c25a2f..7825f6d8c847 100644 > --- a/include/qemu/module.h > +++ b/include/qemu/module.h > @@ -72,6 +72,7 @@ void module_call_init(module_init_type type); > bool module_load_one(const char *prefix, const char *lib_name, bool mayfail); > void module_load_qom_one(const char *type); > void module_load_qom_all(void); > +void module_allow_arch(const char *arch); > > /* > * macros to store module metadata in a .modinfo section. > diff --git a/softmmu/vl.c b/softmmu/vl.c > index ba26a042b284..96316774fcc9 100644 > --- a/softmmu/vl.c > +++ b/softmmu/vl.c > @@ -126,6 +126,8 @@ > #include "sysemu/iothread.h" > #include "qemu/guest-random.h" > > +#include "config-host.h" > + > #define MAX_VIRTIO_CONSOLES 1 > > typedef struct BlockdevOptionsQueueEntry { > @@ -2723,6 +2725,7 @@ void qemu_init(int argc, char **argv, char **envp) > error_init(argv[0]); > qemu_init_exec_dir(argv[0]); > > + module_allow_arch(TARGET_NAME); > qemu_init_subsystems(); > > /* first pass of option parsing */ > diff --git a/util/module.c b/util/module.c > index 6e4199169c41..564b8e3da760 100644 > --- a/util/module.c > +++ b/util/module.c > @@ -122,6 +122,12 @@ void module_call_init(module_init_type type) > static Modules *modinfo; > static char *module_dirs[5]; > static int module_ndirs; > +static const char *module_arch; > + > +void module_allow_arch(const char *arch) > +{ > + module_arch = arch; > +} > > static void module_load_path_init(void) > { > @@ -295,6 +301,14 @@ bool module_load_one(const char *prefix, const char > *lib_name, bool mayfail) > module_load_modinfo(); > > for (modlist = modinfo->list; modlist != NULL; modlist = modlist->next) { > + if (modlist->value->has_arch) { > + if (strcmp(modlist->value->name, module_name) == 0) { > + if (!module_arch || > + strcmp(modlist->value->arch, module_arch) != 0) { > + return false; > + } > + } > + }
I have a little hard time following the code paths, but IIUC, with this change, instead of "module.so" we would have multiple copies of something like "module-$arch.so" ? Then we load them all, read their modinfo section and discard the ones with non-matching arch ? If that is a correct understanding, then I wonder about the scalability of it. We have 30 system emulator targets right now, so if we get even a few arch specific modues, that's going to be alot of modules we're loading, checking and discarding. Wouldn't it be simpler if we just made use of the directory layout /usr/lib64/qemu/$mod.so for common modules and /usr/lib64/qemu/$arch/$mod.so for arch specific modules. That would let us load only the modules we know are applicable for the system target arch and not need this post-load filtering from metadata. 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 :|