Re: [Qemu-devel] [PATCH v5 2/2] Add Nios II semihosting support.
On Wed, 13 Feb 2019 at 04:02, Sandra Loosemore wrote: > > This patch adds support for libgloss semihosting to Nios II bare-metal > emulation. > > Signed-off-by: Sandra Loosemore > Signed-off-by: Julian Brown Hi; here are some more detailed code review comments based on your spec document. They're all fairly minor; otherwise the code looks good. > @@ -169,6 +170,16 @@ void nios2_cpu_do_interrupt(CPUState *cs) > break; > > case EXCP_BREAK: > +qemu_log_mask(CPU_LOG_INT, "BREAK exception at pc=%x\n", > + env->regs[R_PC]); > + > +if (semihosting_enabled()) { > +qemu_log_mask(CPU_LOG_INT, "Entering semihosting\n"); > +env->regs[R_PC] += 4; > +do_nios2_semihosting(env); > +break; > +} The spec says that "break 1" instructions are semihosting. This looks like it's treating any kind of break instruction as semihosting. Is the check for "only 1" being done elsewhere, or should it happen here ? > +static void translate_stat(CPUNios2State *env, target_ulong addr, > + struct stat *s) > +{ > +struct nios2_gdb_stat *p; > + > +p = lock_user(VERIFY_WRITE, addr, sizeof(struct nios2_gdb_stat), 0); > + > +if (!p) { > +/* FIXME - should this return an error code? */ Should it ? :-) I would suggest yes. (Alternatively if the semihosting spec for this architecture specifically forbids it we should comment that here as we do for the "no way to report errors" cases in nios2_semi_return_u32 and _u64, and drop the fixme.) > +return; > +} > +/* Read the input value from the argument block; fail the semihosting > + * call if the memory read fails. > + */ > +#define GET_ARG(n) do { \ > +if (get_user_ual(arg ## n, args + (n) * 4)) { \ > +result = -1;\ > +errno = EFAULT; \ > +goto failed;\ > +} \ > +} while (0) > + > +void do_nios2_semihosting(CPUNios2State *env) > +{ > +int nr; > +uint32_t args; > +target_ulong arg0, arg1, arg2, arg3; > +void *p; > +void *q; > +uint32_t len; > +uint32_t result; > + > +nr = env->regs[R_ARG0]; > +args = env->regs[R_ARG1]; > +switch (nr) { > +case HOSTED_EXIT: > +gdb_exit(env, env->regs[R_ARG0]); > +exit(env->regs[R_ARG0]); > +case HOSTED_OPEN: > +GET_ARG(0); > +GET_ARG(1); > +GET_ARG(2); > +GET_ARG(3); > +if (use_gdb_syscalls()) { > +gdb_do_syscall(nios2_semi_cb, "open,%s,%x,%x", arg0, (int)arg1, > + arg2, arg3); > +return; > +} else { > +p = lock_user_string(arg0); > +if (!p) { > +/* FIXME - check error code? */ > +result = -1; Yes, we should set errno here, right? (and also in the other FIXME comments below). > +} else { > +result = open(p, translate_openflags(arg2), arg3); > +unlock_user(p, arg0, 0); > +} > +} > +break; > +case HOSTED_LSEEK: > +{ > +uint64_t off; > +GET_ARG(0); > +GET_ARG(1); > +GET_ARG(2); > +GET_ARG(3); > +off = (uint32_t)arg2 | ((uint64_t)arg1 << 32); > +if (use_gdb_syscalls()) { > +nios2_semi_is_fseek = 1; Shouldn't this flag be called '...is_lseek' ? > +gdb_do_syscall(nios2_semi_cb, "lseek,%x,%lx,%x", > + arg0, off, arg3); > +} else { > +off = lseek(arg0, off, arg3); > +nios2_semi_return_u64(env, off, errno); > +} > +return; > +} > +default: > +cpu_abort(CPU(nios2_env_get_cpu(env)), > + "Unsupported semihosting syscall %d\n", nr); > +result = 0; We should never cpu_abort() for things the guest can provoke: we should just log this as LOG_GUEST_ERROR and continue. > +} > +failed: > +nios2_semi_return_u32(env, result, errno); thanks -- PMM
Re: [Qemu-devel] [PATCH v5 2/2] Add Nios II semihosting support.
On Fri, 8 Mar 2019 at 17:01, Sandra Loosemore wrote: > > On 3/8/19 9:04 AM, Peter Maydell wrote: > > Thanks. So who "owns" this ABI (ie has the authority to change > > it and should be the end documenting it)? How many projects or > > bits of software are implementing either end of it? > > Going back in ancient history, I implemented the m68k version in > libgloss in 2006 to support a hardware debug stub that CodeSourcery was > also providing at that time. We later moved the runtime side of it into > target-agnostic code in an internal library, so when it came time to do > a similar JTAG debug stub for bare-metal Nios II hardware testing in > 2012, we re-used our existing code for both library and debug stub. > Later Altera implemented the same protocol in some proprietary > simulators they provided to us, and more recently we wrote these patches > to add it to QEMU. We've shifted away from hardware testing and no > longer use the original debug stub now. > > > If we decide that QEMU owns the spec we can put the documentation > > into docs/specs/. > > Making QEMU the "owner" of the ABI seems a little peculiar to me since > it is only one client among several, and is a latecomer too. I think > libgloss would make a little more sense. OTOH, I have no problem with > making the documentation part of QEMU. Thanks for the backstory. I agree that if QEMU is just one of multiple implementations then it's not a great place to hold the specification. Either libgloss, or I suppose in theory Altera as the owner of the architecture could bless the specification and host it... -- PMM
Re: [Qemu-devel] [PATCH v5 2/2] Add Nios II semihosting support.
On 3/8/19 9:04 AM, Peter Maydell wrote: On Fri, 8 Mar 2019 at 03:13, Sandra Loosemore wrote: On 3/7/19 7:58 AM, Peter Maydell wrote: On Wed, 13 Feb 2019 at 04:02, Sandra Loosemore wrote: This patch adds support for libgloss semihosting to Nios II bare-metal emulation. Signed-off-by: Sandra Loosemore Signed-off-by: Julian Brown Do you have a link to the spec that defines this semihosting ABI, please ? Well, I just wrote some derived from the comments in the libgloss sources; see the attachment. :-) Thanks. So who "owns" this ABI (ie has the authority to change it and should be the end documenting it)? How many projects or bits of software are implementing either end of it? Going back in ancient history, I implemented the m68k version in libgloss in 2006 to support a hardware debug stub that CodeSourcery was also providing at that time. We later moved the runtime side of it into target-agnostic code in an internal library, so when it came time to do a similar JTAG debug stub for bare-metal Nios II hardware testing in 2012, we re-used our existing code for both library and debug stub. Later Altera implemented the same protocol in some proprietary simulators they provided to us, and more recently we wrote these patches to add it to QEMU. We've shifted away from hardware testing and no longer use the original debug stub now. If we decide that QEMU owns the spec we can put the documentation into docs/specs/. Making QEMU the "owner" of the ABI seems a little peculiar to me since it is only one client among several, and is a latecomer too. I think libgloss would make a little more sense. OTOH, I have no problem with making the documentation part of QEMU. -Sandra
Re: [Qemu-devel] [PATCH v5 2/2] Add Nios II semihosting support.
On Fri, 8 Mar 2019 at 03:13, Sandra Loosemore wrote: > > On 3/7/19 7:58 AM, Peter Maydell wrote: > > On Wed, 13 Feb 2019 at 04:02, Sandra Loosemore > > wrote: > >> > >> This patch adds support for libgloss semihosting to Nios II bare-metal > >> emulation. > >> > >> Signed-off-by: Sandra Loosemore > >> Signed-off-by: Julian Brown > > > > Do you have a link to the spec that defines this semihosting > > ABI, please ? > > Well, I just wrote some derived from the comments in the libgloss > sources; see the attachment. :-) Thanks. So who "owns" this ABI (ie has the authority to change it and should be the end documenting it)? How many projects or bits of software are implementing either end of it? If we decide that QEMU owns the spec we can put the documentation into docs/specs/. > FWIW this is pretty much a direct copy of the m68k semihosting protocol, > which CodeSourcery contributed ages ago to both libgloss and qemu. Yeah, and that seems to be undocumented too :-( I'm not a huge fan of "this is just some random thing that's private to QEMU and libgloss". thanks -- PMM
Re: [Qemu-devel] [PATCH v5 2/2] Add Nios II semihosting support.
On 3/7/19 7:58 AM, Peter Maydell wrote: On Wed, 13 Feb 2019 at 04:02, Sandra Loosemore wrote: This patch adds support for libgloss semihosting to Nios II bare-metal emulation. Signed-off-by: Sandra Loosemore Signed-off-by: Julian Brown Do you have a link to the spec that defines this semihosting ABI, please ? Well, I just wrote some derived from the comments in the libgloss sources; see the attachment. :-) FWIW this is pretty much a direct copy of the m68k semihosting protocol, which CodeSourcery contributed ages ago to both libgloss and qemu. -Sandra Nios II Semihosting Protocol The runtime (libgloss) indicates a semihosting request to the debug agent by issuing a "break 1" instruction. r4 and r5 are used to pass parameters per the normal C ABI on nios2. r4 contains a request code. r5 is typically a pointer to a 4-word parameter block, except for the exit operation where it is an immediate integer value. The result of the operation is returned in the first word of the parameter block. The second word is used to return an errno value, encoded per the "Errno Values" section of the RSP documentation in the GDB User Manual. The supported r4 request codes are: #define HOSTED_EXIT 0 Terminate program execution; send a 'W' stop reply to GDB. r5 contains the exit code, as an immediate integer rather than indirectly in a parameter block. This semihosting request isn't expected to return. #define HOSTED_INIT_SIM 1 Reserved/unimplemented. #define HOSTED_OPEN 2 Open file; 'Fopen' GDB fileio request. r5 points to a parameter block containing: [0] pointer to filename [1] filename length [2] open flags, encoded per the GDB RSP documentation [3] mode, encoded per the GDB RSP documentation Return values in parameter block: [0] file descriptor or -1 on error [1] errno, encoded per the GDB RSP documentation #define HOSTED_CLOSE 3 Close file; 'Fclose' GDB fileio request. r5 points to a parameter block containing: [0] file descriptor Return values in parameter block: [0] return status [1] errno, encoded per the GDB RSP documentation #define HOSTED_READ 4 Read from file; 'Fread' GDB fileio request. r5 points to a parameter block containing: [0] file descriptor [1] pointer to buffer [2] buffer size Return values in parameter block: [0] number of bytes read [1] errno, encoded per the GDB RSP documentation #define HOSTED_WRITE 5 Write to file; 'Fwrite' GDB fileio request. r5 points to a parameter block containing: [0] file descriptor [1] pointer to buffer [2] byte count Return values in parameter block: [0] number of bytes written [1] errno, encoded per the GDB RSP documentation #define HOSTED_LSEEK 6 File seek; 'Flseek' GDB fileio request. r5 points to a parameter block containing: [0] file descriptor [1] high word of 64-bit offset [2] low word of 64-bit offset [3] seek flag, encoded per the GDB RSP documentation Return values in parameter block: [0] high word of 64-bit result [1] low word of 64-bit result [2] errno, encoded per the GDB RSP documentation #define HOSTED_RENAME 7 File rename; 'Frename' GDB fileio request. r5 points to a parameter block containing: [0] oldname pointer [1] oldname length [2] newname pointer [3] newname length Return values in parameter block: [0] return status [1] errno, encoded per the GDB RSP documentation #define HOSTED_UNLINK 8 File unlink/delete; 'Funlink' GDB fileio request. r5 points to a parameter block containing: [0] filename pointer [1] filename length Return values in parameter block: [0] return status [1] errno, encoded per the GDB RSP documentation #define HOSTED_STAT 9 File information; 'Fstat' GDB fileio request. r5 points to a parameter block containing: [0] filename pointer [1] filename length [2] pointer to stat buf, using the structure definition in the GDB RSP documentation Return values in parameter block: [0] return status [1] errno, encoded per the GDB RSP documentation #define HOSTED_FSTAT 10 File information; 'Ffstat' GDB fileio request. r5 points to a parameter block containing: [0] file descriptor [1] pointer to stat buf, using the structure definition in the GDB RSP documentation Return values in parameter block: [0] return status [1] errno, encoded per the GDB RSP documentation #define HOSTED_GETTIMEOFDAY 11 Get current time; 'Fgettimeofday' GDB fileio request. r5 points to a parameter block containing: [0] timeval pointer, using the structure definition in the GDB RSP documentation Return values in parameter block: [0] return status [1] errno, encoded per the GDB RSP documentation #define HOSTED_ISATTY 12 Return true if the file descriptor is the GDB console; 'Fisatty' GDB fileio request. r5 points to a parameter block containing: [0] file descriptor Return values in par
Re: [Qemu-devel] [PATCH v5 2/2] Add Nios II semihosting support.
On Wed, 13 Feb 2019 at 04:02, Sandra Loosemore wrote: > > This patch adds support for libgloss semihosting to Nios II bare-metal > emulation. > > Signed-off-by: Sandra Loosemore > Signed-off-by: Julian Brown Do you have a link to the spec that defines this semihosting ABI, please ? thanks -- PMM