Hi Paolo,

On 30/5/24 15:58, Philippe Mathieu-Daudé wrote:
On 30/5/24 09:31, Paolo Bonzini wrote:
On Thu, May 30, 2024 at 9:22 AM Philippe Mathieu-Daudé
<phi...@linaro.org> wrote:

On 30/5/24 08:02, Paolo Bonzini wrote:
On Wed, May 29, 2024 at 5:56 PM Philippe Mathieu-Daudé
<phi...@linaro.org> wrote:
It is pointless to build semihosting when TCG is not available.

Why? I would have naively assumed that a suitable semihosting API
could be implemented by KVM. The justification (and thus the commit
message) needs to be different for each architecture if it's a matter
of instruction set or insufficient KVM userspace API.

I wasn't sure where semihosting could be used so asked on IRC and
Alex told me TCG only. Maybe the current implementation is TCG
only, and I can reword. It certainly need some refactor to work
on KVM, because currently semihosting end calling the TCG probe_access
API, which I'm trying to restrict to TCG in order to ease linking
multiple libtcg for the single binary (see
https://lore.kernel.org/qemu-devel/20240529155918.6221-1-phi...@linaro.org/).

Ok, that goes in the commit message though.

"Semihosting currently uses the TCG probe_access API. It is pointless
to have it in the binary when TCG isn't".

and in the first two patches:

"Semihosting currently uses the TCG probe_access API. To prepare for
encoding the TCG dependency in Kconfig, do not enable it unless TCG is
available".

But then, "select FOO if TCG" mean that it can be compiled out; so
perhaps "imply SEMIHOSTING if TCG" is better? Same for RISC-V's
"select ARM_COMPATIBLE_SEMIHOSTING if TCG".

Building qemu-system-mips configured with --without-default-devices:

Undefined symbols for architecture arm64:
  "_qemu_semihosting_console_write", referenced from:
      _mips_semihosting in target_mips_tcg_sysemu_mips-semi.c.o
  "_semihost_sys_close", referenced from:
      _mips_semihosting in target_mips_tcg_sysemu_mips-semi.c.o
  "_uaccess_strlen_user", referenced from:
      _mips_semihosting in target_mips_tcg_sysemu_mips-semi.c.o
  ...

So this one has to use "select".

Similarly m68k:

Undefined symbols for architecture arm64:
  "_semihost_sys_close", referenced from:
      _do_m68k_semihosting in target_m68k_m68k-semi.c.o
  ...

I can link m68k using semihosting stubs but I'm not sure it is right:

-- >8 --
diff --git a/semihosting/stubs-target-all.c b/semihosting/stubs-target-all.c
new file mode 100644
index 0000000000..1f33173f43
--- /dev/null
+++ b/semihosting/stubs-target-all.c
@@ -0,0 +1,97 @@
+/*
+ * Semihosting Stubs
+ *
+ * Copyright (c) 2024 Linaro Ltd
+ *
+ * Stubs for semihosting targets that don't actually do semihosting.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "exec/exec-all.h"
+#include "semihosting/syscalls.h"
+
+void semihost_sys_open(CPUState *cs, gdb_syscall_complete_cb complete,
+                       target_ulong fname, target_ulong fname_len,
+                       int gdb_flags, int mode)
+{
+}
+
+void semihost_sys_close(CPUState *cs, gdb_syscall_complete_cb complete, int fd)
+{
+}
+
+void semihost_sys_read_gf(CPUState *cs, gdb_syscall_complete_cb complete,
+                          GuestFD *gf, target_ulong buf, target_ulong len)
+{
+}
+
+void semihost_sys_read(CPUState *cs, gdb_syscall_complete_cb complete,
+                       int fd, target_ulong buf, target_ulong len)
+{
+}
+
+void semihost_sys_write_gf(CPUState *cs, gdb_syscall_complete_cb complete,
+                           GuestFD *gf, target_ulong buf, target_ulong len)
+{
+}
+
+void semihost_sys_write(CPUState *cs, gdb_syscall_complete_cb complete,
+                        int fd, target_ulong buf, target_ulong len)
+{
+}
+
+void semihost_sys_lseek(CPUState *cs, gdb_syscall_complete_cb complete,
+                        int fd, int64_t off, int gdb_whence)
+{
+}
+
+void semihost_sys_isatty(CPUState *cs, gdb_syscall_complete_cb complete, int fd)
+{
+}
+
+void semihost_sys_flen(CPUState *cs, gdb_syscall_complete_cb fstat_cb,
+                       gdb_syscall_complete_cb flen_cb, int fd,
+                       target_ulong fstat_addr)
+{
+}
+
+void semihost_sys_fstat(CPUState *cs, gdb_syscall_complete_cb complete,
+                        int fd, target_ulong addr)
+{
+}
+
+void semihost_sys_stat(CPUState *cs, gdb_syscall_complete_cb complete,
+                       target_ulong fname, target_ulong fname_len,
+                       target_ulong addr)
+{
+}
+
+void semihost_sys_remove(CPUState *cs, gdb_syscall_complete_cb complete,
+                         target_ulong fname, target_ulong fname_len)
+{
+}
+
+void semihost_sys_rename(CPUState *cs, gdb_syscall_complete_cb complete,
+                         target_ulong oname, target_ulong oname_len,
+                         target_ulong nname, target_ulong nname_len)
+{
+}
+
+void semihost_sys_system(CPUState *cs, gdb_syscall_complete_cb complete,
+                         target_ulong cmd, target_ulong cmd_len)
+{
+}
+
+void semihost_sys_gettimeofday(CPUState *cs, gdb_syscall_complete_cb complete,
+                               target_ulong tv_addr, target_ulong tz_addr)
+{
+}
+
+#ifndef CONFIG_USER_ONLY
+void semihost_sys_poll_one(CPUState *cs, gdb_syscall_complete_cb complete,
+                           int fd, GIOCondition cond, int timeout)
+{
+}
+#endif
diff --git a/semihosting/meson.build b/semihosting/meson.build
index 34933e5a19..aa8b7a9913 100644
--- a/semihosting/meson.build
+++ b/semihosting/meson.build
@@ -7,7 +7,7 @@ specific_ss.add(when: ['CONFIG_SEMIHOSTING', 'CONFIG_SYSTEM_ONLY'], if_true: fil
   'config.c',
   'console.c',
   'uaccess.c',
-))
+), if_false: files('stubs-target-all.c'))

common_ss.add(when: ['CONFIG_SEMIHOSTING', 'CONFIG_SYSTEM_ONLY'], if_false: files('stubs-all.c')) system_ss.add(when: ['CONFIG_SEMIHOSTING'], if_false: files('stubs-system.c'))
---

For mips more stubs are needed:

Undefined symbols for architecture arm64:
  "_qemu_semihosting_console_write", referenced from:
      _mips_semihosting in target_mips_tcg_sysemu_mips-semi.c.o
  "_uaccess_lock_user", referenced from:
      _mips_semihosting in target_mips_tcg_sysemu_mips-semi.c.o
      _mips_semihosting in target_mips_tcg_sysemu_mips-semi.c.o
      _mips_semihosting in target_mips_tcg_sysemu_mips-semi.c.o
      _uhi_fstat_cb in target_mips_tcg_sysemu_mips-semi.c.o
  "_uaccess_lock_user_string", referenced from:
      _mips_semihosting in target_mips_tcg_sysemu_mips-semi.c.o
      _mips_semihosting.cold.6 in target_mips_tcg_sysemu_mips-semi.c.o
      _mips_semihosting.cold.6 in target_mips_tcg_sysemu_mips-semi.c.o
  "_uaccess_strlen_user", referenced from:
      _mips_semihosting in target_mips_tcg_sysemu_mips-semi.c.o
  "_uaccess_unlock_user", referenced from:
      _mips_semihosting in target_mips_tcg_sysemu_mips-semi.c.o
      _mips_semihosting in target_mips_tcg_sysemu_mips-semi.c.o
      _mips_semihosting in target_mips_tcg_sysemu_mips-semi.c.o
      _mips_semihosting in target_mips_tcg_sysemu_mips-semi.c.o
      _mips_semihosting in target_mips_tcg_sysemu_mips-semi.c.o
      _mips_semihosting in target_mips_tcg_sysemu_mips-semi.c.o
      _uhi_fstat_cb in target_mips_tcg_sysemu_mips-semi.c.o
      ...


Reply via email to