[RFC PATCH 0/2] hw/s390x: modularize virtio-gpu-ccw

[RFC PATCH 0/2] hw/s390x: modularize virtio-gpu-ccw

[Halil Pasic]

This series explores Daniels idea he proposed in [1], and
is a successor to the v3.

I decided to go with the RFC tag because there are several
things I have doubts about, and I hope somebody more familiar
with the topic can help me out. I kept running into situations
where meson felt like something I have to cheat and defeat.

One thing that required special attention is keeping things functional
from the build tree (i.e. not requiring an install).  First I failed to
generate a meson output form a custom_target() in a qemu target specific
folder. So I decided to generate scripts that can be used both for
installation and during compilation to massage the build tree so that
goal is accomplished.

The solution for running the scripts as a part of the build is kind of
ugly, because for custom_target() output is mandatory, but my output is
invalid. So I lied.

When I thought, I was done with an RFC quality solution, I tested it from
scratch (deleting my builddir), and it turns out for
meson.add_install_script(script_name) script_name is supposed to be
around, and executable already at configure time. So my generated
scripts won't do because not around in the beginning. I decided to work
that around quick and dirty by introducing a wrapper script.

One alternative to this would be to have the logic in a single script in
the source-tree, and just generate the list of modules for each target,
which would be a parameter of this single script along with the target
name. The downside of this is that we require 2 new in source tree
scripts this way as well, but it is less flexible, because it can't be
used for other generated scripts.

Another thing I'm not really super satisfied with is this
modules_restricted interface, but for my use case it does the job, and
when something new emerges we can re-evaluate.

But before I continue agonizing on these matters, I would like to get
some feedback regarding, is this approach to the problem what the
community wants (or is the whole symlinks idea controversial).

[1] https://mail.gnu.org/archive/html/qemu-s390x/2021-03/msg00206.html

Changelog v3 --> RFC:
* switch to the symlinks approach (Daniel)
* split s390x and common in separate patches (Eduardo)


Halil Pasic (2):
  modules: introduce target specific modules
  hw/s390x: modularize virtio-gpu-ccw

 hw/s390x/meson.build               |  8 +++++-
 include/hw/s390x/css.h             |  7 -----
 include/hw/s390x/s390_flic.h       |  3 +++
 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 +
 target/s390x/cpu.h                 |  9 ++++---
 util/module.c                      | 14 ++++++++--
 12 files changed, 112 insertions(+), 16 deletions(-)
 create mode 100755 scripts/call_generated_script.sh
 create mode 100755 scripts/modules/target-symlinks.sh


base-commit: 2615a5e433aeb812c300d3a48e1a88e1303e2339
-- 
2.25.1

[RFC PATCH 1/2] modules: introduce target specific modules

[Halil Pasic]

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/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;
-- 
2.25.1

[RFC PATCH 2/2] hw/s390x: modularize virtio-gpu-ccw

[Halil Pasic]

Since the virtio-gpu-ccw device depends on the hw-display-virtio-gpu
module, which provides the type virtio-gpu-device, packaging the
hw-display-virtio-gpu module as a separate package that may or may not
be installed along with the qemu package leads to problems. Namely if
the hw-display-virtio-gpu is absent, qemu continues to advertise
virtio-gpu-ccw, but it aborts not only when one attempts using
virtio-gpu-ccw, but also when libvirtd's capability probing tries
to instantiate the type to introspect it.

Let us thus introduce a module named hw-s390x-virtio-gpu-ccw that
is going to provide the virtio-gpu-ccw device. The hw-s390x prefix
was chosen because it is not a portable device.

With virtio-gpu-ccw built as a module, the correct way to package a
modularized qemu is to require that hw-display-virtio-gpu must be
installed whenever the module hw-s390x-virtio-gpu-ccw.

The definition S390_ADAPTER_SUPPRESSIBLE was moved to "cpu.h", per
suggestion of Thomas Huth. From interface design perspective, IMHO, not
a good thing as it belongs to the public interface of
css_register_io_adapters(). We did this because CONFIG_KVM requeires
NEED_CPU_H and Thomas, and other commenters did not like the
consequences of that.

Moving the interrupt related declarations to s390_flic.h was suggested
by Cornelia Huck.

Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
---
 hw/s390x/meson.build         | 7 +++++++
 include/hw/s390x/css.h       | 7 -------
 include/hw/s390x/s390_flic.h | 3 +++
 target/s390x/cpu.h           | 9 ++++++---
 util/module.c                | 1 +
 5 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
index 0cee605f0a..ad2941f7ca 100644
--- a/hw/s390x/meson.build
+++ b/hw/s390x/meson.build
@@ -47,3 +47,10 @@ virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: files('vhost-user-fs-ccw.c'
 s390x_ss.add_all(when: 'CONFIG_VIRTIO_CCW', if_true: virtio_ss)
 
 hw_arch += {'s390x': s390x_ss}
+
+hw_s390x_modules = {}
+virtio_gpu_ccw_ss = ss.source_set()
+virtio_gpu_ccw_ss.add(when: 'CONFIG_VIRTIO_GPU', if_true: [files('virtio-ccw-gpu.c'), pixman])
+hw_s390x_modules += {'virtio-gpu-ccw': virtio_gpu_ccw_ss}
+modules += {'hw-s390x': hw_s390x_modules}
+modules_restricted += {'hw-s390x-virtio-gpu-ccw': ['s390x-softmmu']}
diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
index 7901ab276c..bba7593d2e 100644
--- a/include/hw/s390x/css.h
+++ b/include/hw/s390x/css.h
@@ -12,7 +12,6 @@
 #ifndef CSS_H
 #define CSS_H
 
-#include "cpu.h"
 #include "hw/s390x/adapter.h"
 #include "hw/s390x/s390_flic.h"
 #include "hw/s390x/ioinst.h"
@@ -233,12 +232,6 @@ uint32_t css_get_adapter_id(CssIoAdapterType type, uint8_t isc);
 void css_register_io_adapters(CssIoAdapterType type, bool swap, bool maskable,
                               uint8_t flags, Error **errp);
 
-#ifndef CONFIG_KVM
-#define S390_ADAPTER_SUPPRESSIBLE 0x01
-#else
-#define S390_ADAPTER_SUPPRESSIBLE KVM_S390_ADAPTER_SUPPRESSIBLE
-#endif
-
 #ifndef CONFIG_USER_ONLY
 SubchDev *css_find_subch(uint8_t m, uint8_t cssid, uint8_t ssid,
                          uint16_t schid);
diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h
index e91b15d2d6..3907a13d07 100644
--- a/include/hw/s390x/s390_flic.h
+++ b/include/hw/s390x/s390_flic.h
@@ -134,6 +134,9 @@ void s390_flic_init(void);
 S390FLICState *s390_get_flic(void);
 QEMUS390FLICState *s390_get_qemu_flic(S390FLICState *fs);
 S390FLICStateClass *s390_get_flic_class(S390FLICState *fs);
+void s390_crw_mchk(void);
+void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr,
+                       uint32_t io_int_parm, uint32_t io_int_word);
 bool ais_needed(void *opaque);
 
 #endif /* HW_S390_FLIC_H */
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 60d434d5ed..b434b905c0 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -40,6 +40,12 @@
 
 #define S390_MAX_CPUS 248
 
+#ifndef CONFIG_KVM
+#define S390_ADAPTER_SUPPRESSIBLE 0x01
+#else
+#define S390_ADAPTER_SUPPRESSIBLE KVM_S390_ADAPTER_SUPPRESSIBLE
+#endif
+
 typedef struct PSW {
     uint64_t mask;
     uint64_t addr;
@@ -806,9 +812,6 @@ int cpu_s390x_signal_handler(int host_signum, void *pinfo, void *puc);
 
 
 /* interrupt.c */
-void s390_crw_mchk(void);
-void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr,
-                       uint32_t io_int_parm, uint32_t io_int_word);
 #define RA_IGNORED                  0
 void s390_program_interrupt(CPUS390XState *env, uint32_t code, uintptr_t ra);
 /* service interrupts are floating therefore we must not pass an cpustate */
diff --git a/util/module.c b/util/module.c
index 26fc893d98..5638202357 100644
--- a/util/module.c
+++ b/util/module.c
@@ -313,6 +313,7 @@ static struct {
     { "virtio-gpu-pci-base",   "hw-", "display-virtio-gpu-pci" },
     { "virtio-gpu-pci",        "hw-", "display-virtio-gpu-pci" },
     { "vhost-user-gpu-pci",    "hw-", "display-virtio-gpu-pci" },
+    { "virtio-gpu-ccw",        "hw-", "s390x-virtio-gpu-ccw"   },
     { "virtio-vga-base",       "hw-", "display-virtio-vga"    },
     { "virtio-vga",            "hw-", "display-virtio-vga"    },
     { "vhost-user-vga",        "hw-", "display-virtio-vga"    },
-- 
2.25.1

Re: [RFC PATCH 1/2] modules: introduce target specific modules

[Philippe Mathieu-Daudé]

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?

Re: [RFC PATCH 1/2] modules: introduce target specific modules

[Halil Pasic]

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