Hi Halil,
On 3/16/21 1:26 PM, Halil Pasic wrote:
> After some back-and-forth in [1] Daniel suggested that
> we could/should make qemu modules per-target by introducing a
> dedicated modules directory for each target, which can symlink the
> modules that do work with and do make sense for the given target.
>
> That way we can avoid trying to load modules we know won't work and
> coming up with convoluted ways for making subsequent failures graceful.
> The topic of per-target modules was discussed before [1] but, the
> idea with the symlinks originates from [1].
>
> This patch introduces this new scheme of loading modules without
> actually introducing any changes to what modules are available to what
> targets. For the exploitation have look at 'hw/s390x: modularize
> virtio-gpu-ccw'.
>
> [1] https://mail.gnu.org/archive/html/qemu-s390x/2021-03/msg00206.html
>
> Signed-off-by: Halil Pasic <pa...@linux.ibm.com>
> Suggested-by: "Daniel P. Berrangé" <berra...@redhat.com>
> ---
> hw/s390x/meson.build | 1 -
> include/qemu/module.h | 2 ++
> meson.build | 43 +++++++++++++++++++++++++++++-
> roms/SLOF | 2 +-
> roms/opensbi | 2 +-
> scripts/call_generated_script.sh | 6 +++++
> scripts/modules/target-symlinks.sh | 31 +++++++++++++++++++++
> softmmu/runstate.c | 1 +
> util/module.c | 13 +++++++--
> 9 files changed, 95 insertions(+), 6 deletions(-)
> create mode 100755 scripts/call_generated_script.sh
> create mode 100755 scripts/modules/target-symlinks.sh
> diff --git a/include/qemu/module.h b/include/qemu/module.h
> index 944d403cbd..85a59fde81 100644
> --- a/include/qemu/module.h
> +++ b/include/qemu/module.h
> @@ -73,4 +73,6 @@ 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 set_emulator_modules_dir(char const *dir_name);
> +
> #endif
> diff --git a/meson.build b/meson.build
> index a7d2dd429d..8926968182 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1749,6 +1749,7 @@ user_ss = ss.source_set()
> util_ss = ss.source_set()
>
> modules = {}
> +modules_restricted = {}
> hw_arch = {}
> target_arch = {}
> target_softmmu_arch = {}
> @@ -2092,14 +2093,54 @@ common_ss.add(hwcore)
> # Targets #
> ###########
>
> +module_targets = []
> foreach m : block_mods + softmmu_mods
> - shared_module(m.name(),
> + module_targets += shared_module(m.name(),
> name_prefix: '',
> link_whole: m,
> install: true,
> install_dir: qemu_moddir)
> endforeach
>
> +foreach target : target_dirs
> + if not target.endswith('-softmmu')
> + continue
> + endif
> + filtered_module_targets = []
> + foreach m : module_targets
> + restricted_to = modules_restricted.get(m.name(), [])
> + if restricted_to.length() == 0 or restricted_to.contains(target)
> + filtered_module_targets += m
> + endif
> + endforeach
> + s = custom_target('Make symbolic links script for ' + target + ' modules' ,
> + input: filtered_module_targets,
> + output: 'make_mod_symlinks_'+target+'.sh',
> + install: false,
> + depends: filtered_module_targets,
> + build_by_default: true,
> + build_always_stale: true,
> + command: [
> + meson.current_source_dir() /
> 'scripts/modules/target-symlinks.sh',
> + '@OUTPUT@',
> + target,
> + '@INPUT@'
> + ])
> +
> + # We run the script as a part of the build so things keep working form the
> + # build tree (without requiring an instalation). I couldn't find a nicer
> way.
> + custom_target('Run symbolic links script for ' + target + ' modules' ,
> + depends: s,
> + output: 'make_mod_symlinks_'+target+'.sh.dummy',
> + install: false,
> + build_by_default: true,
> + build_always_stale: true,
> + command: [
> + s.full_path(),
> + meson.current_build_dir()
> + ])
> + meson.add_install_script(meson.current_source_dir() /
> 'scripts/call_generated_script.sh', s.full_path(), qemu_moddir)
> +endforeach
> softmmu_ss.add(authz, blockdev, chardev, crypto, io, qmp)
> common_ss.add(qom, qemuutil)
>
> diff --git a/roms/SLOF b/roms/SLOF
> index 33a7322de1..e18ddad851 160000
> --- a/roms/SLOF
> +++ b/roms/SLOF
> @@ -1 +1 @@
> -Subproject commit 33a7322de13e9dca4b38851a345a58d37e7a441d
> +Subproject commit e18ddad8516ff2cfe36ec130200318f7251aa78c
> diff --git a/roms/opensbi b/roms/opensbi
> index 234ed8e427..a98258d0b5 160000
> --- a/roms/opensbi
> +++ b/roms/opensbi
> @@ -1 +1 @@
> -Subproject commit 234ed8e427f4d92903123199f6590d144e0d9351
> +Subproject commit a98258d0b537a295f517bbc8d813007336731fa9
While your patch deals with "target modules", the 2 submodule
changes are unrelated, right?
