[RFC PATCH] target/mips: Allow building without Inter-Thread Communication hardware

[RFC PATCH] target/mips: Allow building without Inter-Thread Communication hardware

[Philippe Mathieu-Daudé]

The Inter-Thread Communication unit (TYPE_MIPS_ITU) is an optional
device that is only selected by a few machines. However it goes
deep into the translation code, as the MTC0/MTHC0 SAAR helpers
call itc_reconfigure().

When building with no machine selecting the ITU component (which
is implemented in hw/misc/mips_itu.c), we get the following link
failure:

  /usr/bin/ld: target_mips_cp0_helper.c.o: in function `helper_mtc0_saar':
  target/mips/cp0_helper.c:1118: undefined reference to `itc_reconfigure'
  /usr/bin/ld: target_mips_cp0_helper.c.o: in function `helper_mthc0_saar':
  target/mips/cp0_helper.c:1135: undefined reference to `itc_reconfigure'

Fix by adding a stub, built when the ITU isn't selected.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
RFC because too much Meson machinery to my taste.
But how to deal with such architectural devices else?

To reproduce:

$ echo CONFIG_JAZZ=y > default-configs/devices/mips64el-softmmu.mak
$ echo CONFIG_SEMIHOSTING=y >> default-configs/devices/mips64el-softmmu.mak
$ configure --without-default-devices
$ ninja qemu-system-mips64el
$ ./qemu-system-mips64el -M magnum -S
---
 target/mips/cp0_itu-stub.c | 15 +++++++++++++++
 target/mips/meson.build    |  3 +++
 2 files changed, 18 insertions(+)
 create mode 100644 target/mips/cp0_itu-stub.c

diff --git a/target/mips/cp0_itu-stub.c b/target/mips/cp0_itu-stub.c
new file mode 100644
index 00000000000..995b5a09ff8
--- /dev/null
+++ b/target/mips/cp0_itu-stub.c
@@ -0,0 +1,15 @@
+/*
+ * QEMU Inter-Thread Communication Unit emulation stubs
+ *
+ *  Copyright (c) 2021 Philippe Mathieu-Daudé
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "hw/misc/mips_itu.h"
+
+void itc_reconfigure(MIPSITUState *tag)
+{
+    /* nothing? */
+}
diff --git a/target/mips/meson.build b/target/mips/meson.build
index 3b131c4a7f6..a631688fae0 100644
--- a/target/mips/meson.build
+++ b/target/mips/meson.build
@@ -45,6 +45,9 @@
   'cp0_helper.c',
   'mips-semi.c',
 ))
+mips_softmmu_ss.add(when: 'CONFIG_MIPS_ITU', if_false: files(
+  'cp0_itu-stub.c',
+))
 
 mips_ss.add_all(when: 'CONFIG_TCG', if_true: [mips_tcg_ss])
 
-- 
2.26.3

Re: [RFC PATCH] target/mips: Allow building without Inter-Thread Communication hardware

[Richard Henderson]

On 4/27/21 12:11 PM, Philippe Mathieu-Daudé wrote:
> The Inter-Thread Communication unit (TYPE_MIPS_ITU) is an optional
> device that is only selected by a few machines. However it goes
> deep into the translation code, as the MTC0/MTHC0 SAAR helpers
> call itc_reconfigure().
> 
> When building with no machine selecting the ITU component (which
> is implemented in hw/misc/mips_itu.c), we get the following link
> failure:
> 
>    /usr/bin/ld: target_mips_cp0_helper.c.o: in function `helper_mtc0_saar':
>    target/mips/cp0_helper.c:1118: undefined reference to `itc_reconfigure'
>    /usr/bin/ld: target_mips_cp0_helper.c.o: in function `helper_mthc0_saar':
>    target/mips/cp0_helper.c:1135: undefined reference to `itc_reconfigure'
> 
> Fix by adding a stub, built when the ITU isn't selected.
> 
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
> RFC because too much Meson machinery to my taste.
> But how to deal with such architectural devices else?
> 
> To reproduce:
> 
> $ echo CONFIG_JAZZ=y > default-configs/devices/mips64el-softmmu.mak
> $ echo CONFIG_SEMIHOSTING=y >> default-configs/devices/mips64el-softmmu.mak
> $ configure --without-default-devices
> $ ninja qemu-system-mips64el
> $ ./qemu-system-mips64el -M magnum -S
> ---
>   target/mips/cp0_itu-stub.c | 15 +++++++++++++++
>   target/mips/meson.build    |  3 +++
>   2 files changed, 18 insertions(+)
>   create mode 100644 target/mips/cp0_itu-stub.c

Perhaps use __attribute__((weak)) on itc_reconfigure?  Then you don't need the 
stub at all.  You're already protecting the actual call, so there should be no 
change needed there.

We're not using weak so far, but as far as I can tell this is supported by gcc 
on windows as well.


r~

Re: [RFC PATCH] target/mips: Allow building without Inter-Thread Communication hardware

[Philippe Mathieu-Daudé]

On Wed, Apr 28, 2021 at 12:13 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
> On 4/27/21 12:11 PM, Philippe Mathieu-Daudé wrote:
> > The Inter-Thread Communication unit (TYPE_MIPS_ITU) is an optional
> > device that is only selected by a few machines. However it goes
> > deep into the translation code, as the MTC0/MTHC0 SAAR helpers
> > call itc_reconfigure().
> >
> > When building with no machine selecting the ITU component (which
> > is implemented in hw/misc/mips_itu.c), we get the following link
> > failure:
> >
> >    /usr/bin/ld: target_mips_cp0_helper.c.o: in function `helper_mtc0_saar':
> >    target/mips/cp0_helper.c:1118: undefined reference to `itc_reconfigure'
> >    /usr/bin/ld: target_mips_cp0_helper.c.o: in function `helper_mthc0_saar':
> >    target/mips/cp0_helper.c:1135: undefined reference to `itc_reconfigure'
> >
> > Fix by adding a stub, built when the ITU isn't selected.
> >
> > Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> > ---
> > RFC because too much Meson machinery to my taste.
> > But how to deal with such architectural devices else?
> >
> > To reproduce:
> >
> > $ echo CONFIG_JAZZ=y > default-configs/devices/mips64el-softmmu.mak
> > $ echo CONFIG_SEMIHOSTING=y >> default-configs/devices/mips64el-softmmu.mak
> > $ configure --without-default-devices
> > $ ninja qemu-system-mips64el
> > $ ./qemu-system-mips64el -M magnum -S
> > ---
> >   target/mips/cp0_itu-stub.c | 15 +++++++++++++++
> >   target/mips/meson.build    |  3 +++
> >   2 files changed, 18 insertions(+)
> >   create mode 100644 target/mips/cp0_itu-stub.c
>
> Perhaps use __attribute__((weak)) on itc_reconfigure?  Then you don't need the
> stub at all.  You're already protecting the actual call, so there should be no
> change needed there.
>
> We're not using weak so far, but as far as I can tell this is supported by gcc
> on windows as well.

Apparently we are:

$ git grep attribute.*weak
softmmu/memory.c:3286:void __attribute__((weak)) fuzz_dma_read_cb(size_t addr,
tests/qtest/libqtest-single.h:16:QTestState *global_qtest
__attribute__((common, weak));

I think we should use either stubs or attribute weak, not both.