On Fri, May 31, 2019 at 12:42 PM Philippe Mathieu-Daudé
<phi...@redhat.com> wrote:
>
> Hi Miroslav,
>
> On 5/31/19 11:12 AM, Miroslav Rezanina wrote:
> > On Tue, May 14, 2019 at 04:52:56PM +0100, Alex Bennée wrote:
> >> Now we have a common semihosting console interface use that for our
> >> string output. However ARM is currently unique in also supporting
> >> semihosting for linux-user so we need to replicate the API in
> >> linux-user. If other architectures gain this support we can move the
> >> file later.
> >>
> >> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
> >> ---
> >> linux-user/Makefile.objs | 2 ++
> >> linux-user/arm/semihost.c | 24 ++++++++++++++++++++++++
> >> target/arm/arm-semi.c | 31 ++++++-------------------------
> >> 3 files changed, 32 insertions(+), 25 deletions(-)
> >> create mode 100644 linux-user/arm/semihost.c
> >>
> >> diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
> >> index 769b8d83362..285c5dfa17a 100644
> >> --- a/linux-user/Makefile.objs
> >> +++ b/linux-user/Makefile.objs
> >> @@ -6,4 +6,6 @@ obj-y = main.o syscall.o strace.o mmap.o signal.o \
> >> obj-$(TARGET_HAS_BFLT) += flatload.o
> >> obj-$(TARGET_I386) += vm86.o
> >> obj-$(TARGET_ARM) += arm/nwfpe/
> >> +obj-$(TARGET_ARM) += arm/semihost.o
> >> +obj-$(TARGET_AARCH64) += arm/semihost.o
> >> obj-$(TARGET_M68K) += m68k-sim.o
> >> diff --git a/linux-user/arm/semihost.c b/linux-user/arm/semihost.c
> >> new file mode 100644
> >> index 00000000000..9554102a855
> >> --- /dev/null
> >> +++ b/linux-user/arm/semihost.c
> >> @@ -0,0 +1,24 @@
> >> +/*
> >> + * ARM Semihosting Console Support
> >> + *
> >> + * Copyright (c) 2019 Linaro Ltd
> >> + *
> >> + * Currently ARM is unique in having support for semihosting support
> >> + * in linux-user. So for now we implement the common console API but
> >> + * just for arm linux-user.
> >> + *
> >> + * SPDX-License-Identifier: GPL-2.0-or-later
> >> + */
> >> +
> >> +#include "qemu/osdep.h"
> >> +#include "cpu.h"
> >> +#include "hw/semihosting/console.h"
> >> +#include "qemu.h"
> >> +
> >> +int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr,
> >> int len)
> >> +{
> >> + void *s = lock_user_string(addr);
> >> + len = write(STDERR_FILENO, s, len ? len : strlen(s));
> >> + unlock_user(s, addr, 0);
> >> + return len;
> >> +}
> >> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
> >> index 9e5a414dd89..253c66b172a 100644
> >> --- a/target/arm/arm-semi.c
> >> +++ b/target/arm/arm-semi.c
> >> @@ -27,6 +27,7 @@
> >>
> >> #include "cpu.h"
> >> #include "hw/semihosting/semihost.h"
> >> +#include "hw/semihosting/console.h"
> >> #ifdef CONFIG_USER_ONLY
> >> #include "qemu.h"
> >>
> >> @@ -314,32 +315,12 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> >> return set_swi_errno(ts, close(arg0));
> >> }
> >> case TARGET_SYS_WRITEC:
> >> - {
> >> - char c;
> >> -
> >> - if (get_user_u8(c, args))
> >> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> >> - return (uint32_t)-1;
> >> - /* Write to debug console. stderr is near enough. */
> >> - if (use_gdb_syscalls()) {
> >> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,1",
> >> args);
> >> - } else {
> >> - return write(STDERR_FILENO, &c, 1);
> >> - }
> >> - }
> >> + {
> >> + qemu_semihosting_console_out(env, args, 1);
> >> + return 0xdeadbeef;
> >> + }
> >> case TARGET_SYS_WRITE0:
> >> - if (!(s = lock_user_string(args)))
> >> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> >> - return (uint32_t)-1;
> >> - len = strlen(s);
> >> - if (use_gdb_syscalls()) {
> >> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,%x",
> >> - args, len);
> >> - } else {
> >> - ret = write(STDERR_FILENO, s, len);
> >> - }
> >> - unlock_user(s, args, 0);
> >> - return ret;
> >> + return qemu_semihosting_console_out(env, args, 0);
> >> case TARGET_SYS_WRITE:
> >> GET_ARG(0);
> >> GET_ARG(1);
> >> --
> >> 2.20.1
> >>
> >>
> >
> > Hi Alex,
> >
> > this patch breaks build for softmmu target when CONFIG_SEMIHOSTING is not
> > enabled as qemu_semihosting_console_out
> > is not defined in such case - neither linux-user/arm/semihost.c nor
> > hw/semihosting/console.c compiled and function
> > is not in stubs/semihost.c
linux-user/arm/semihost.c is not built on softmmu-only build.
> Kinda funny, I noticed the same issue at the same time, and was chatting
> with Alex about it.
>
> I prepared a patch expliciting we can not disable CONFIG_SEMIHOSTING on
> the MIPS arch. Would that work for you?
>
> Regards,
>
> Phil.
