| Message ID | 20210316122648.3372459-2-pasic@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/s390x: modularize virtio-gpu-ccw | expand |
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 <pasic@linux.ibm.com> > Suggested-by: "Daniel P. Berrangé" <berrange@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?
On Thu, 18 Mar 2021 12:36:48 +0100 Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > 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? Hi Philippe! Not only unrelated but also unintentional. Seems I was not careful enough with "git add -u". Should we decide to go in this direction (symlinks) I will make sure to drop these changes next time. Regards, Halil
diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build index 91495b5631..0cee605f0a 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')) 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 diff --git a/scripts/call_generated_script.sh b/scripts/call_generated_script.sh new file mode 100755 index 0000000000..1743b39d7c --- /dev/null +++ b/scripts/call_generated_script.sh @@ -0,0 +1,6 @@ +#!/bin/bash + +SCRIPT=$1 +shift + +${SCRIPT} "$@" diff --git a/scripts/modules/target-symlinks.sh b/scripts/modules/target-symlinks.sh new file mode 100755 index 0000000000..25a402a27f --- /dev/null +++ b/scripts/modules/target-symlinks.sh @@ -0,0 +1,31 @@ +#!/bin/bash + +TARGET_FILE="$1" +shift +TARGET_DIR="$1" +shift + +cat > "${TARGET_FILE}" <<EOF +#!/bin/bash + +test \$# -eq 1 || exit 1 +if [ -z \${MESON_INSTALL_DESTDIR_PREFIX+N} ]; then + INSTALL_DIR="\${1}" +else + INSTALL_DIR="\${MESON_INSTALL_DESTDIR_PREFIX}/\${1}" +fi +test -d "\${INSTALL_DIR}" || exit 1 +LINKS_DIR="\${INSTALL_DIR}/${TARGET_DIR}" +mkdir -p \${LINKS_DIR} +# clean up old symbolic links +find \${LINKS_DIR} -iname '*.so' -type l -delete +cd "\${LINKS_DIR}" + +EOF +chmod u+x "${TARGET_FILE}" + +while test $# -gt 0 +do + echo ln -sfrt \"\$\{LINKS_DIR\}\" \"../"$1"\" >> "${TARGET_FILE}" + shift +done diff --git a/softmmu/runstate.c b/softmmu/runstate.c index ce8977c6a2..6842827ad5 100644 --- a/softmmu/runstate.c +++ b/softmmu/runstate.c @@ -758,6 +758,7 @@ void qemu_init_subsystems(void) atexit(qemu_run_exit_notifiers); + set_emulator_modules_dir(g_strconcat(TARGET_NAME, "-softmmu")); module_call_init(MODULE_INIT_QOM); module_call_init(MODULE_INIT_MIGRATION); diff --git a/util/module.c b/util/module.c index c65060c167..26fc893d98 100644 --- a/util/module.c +++ b/util/module.c @@ -64,6 +64,15 @@ static ModuleTypeList *find_type(module_init_type type) return &init_type_list[type]; } +static char const *emulator_modules_dir; + +void set_emulator_modules_dir(char const *dir_name) +{ + assert(dir_name); + assert(!emulator_modules_dir); + emulator_modules_dir = dir_name; +} + void register_module_init(void (*fn)(void), module_init_type type) { ModuleEntry *e; @@ -252,8 +261,8 @@ bool module_load_one(const char *prefix, const char *lib_name, bool mayfail) assert(n_dirs <= ARRAY_SIZE(dirs)); for (i = 0; i < n_dirs; i++) { - fname = g_strdup_printf("%s/%s%s", - dirs[i], module_name, CONFIG_HOST_DSOSUF); + fname = g_strdup_printf("%s/%s/%s%s", dirs[i], emulator_modules_dir, + module_name, CONFIG_HOST_DSOSUF); ret = module_load_file(fname, mayfail, export_symbols); g_free(fname); fname = NULL;
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 <pasic@linux.ibm.com> Suggested-by: "Daniel P. Berrangé" <berrange@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