Philippe Mathieu-Daudé <phi...@linaro.org> writes: > ping :) > > On 12/6/24 15:12, Philippe Mathieu-Daudé wrote: >> 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 >> ... >>
Oh sorry I was waiting for a re-spin. I should just apply the above to 3/3? -- Alex Bennée Virtualisation Tech Lead @ Linaro
