Re: [Qemu-devel] [PATCH v5 2/2] Add Nios II semihosting support.

2019-03-14 Thread Peter Maydell
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.

2019-03-08 Thread Peter Maydell
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.

2019-03-08 Thread Sandra Loosemore

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.

2019-03-08 Thread Peter Maydell
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.

2019-03-07 Thread Sandra Loosemore

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 

Re: [Qemu-devel] [PATCH v5 2/2] Add Nios II semihosting support.

2019-03-07 Thread Peter Maydell
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



[Qemu-devel] [PATCH v5 2/2] Add Nios II semihosting support.

2019-02-12 Thread Sandra Loosemore
This patch adds support for libgloss semihosting to Nios II bare-metal
emulation.

Signed-off-by: Sandra Loosemore 
Signed-off-by: Julian Brown 
---
 qemu-options.hx|   8 +-
 target/nios2/Makefile.objs |   2 +-
 target/nios2/cpu.h |   4 +-
 target/nios2/helper.c  |  11 ++
 target/nios2/nios2-semi.c  | 446 +
 5 files changed, 465 insertions(+), 6 deletions(-)
 create mode 100644 target/nios2/nios2-semi.c

diff --git a/qemu-options.hx b/qemu-options.hx
index 06ef1a7..5019ede 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3712,21 +3712,21 @@ ETEXI
 DEF("semihosting", 0, QEMU_OPTION_semihosting,
 "-semihostingsemihosting mode\n",
 QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32 |
-QEMU_ARCH_MIPS)
+QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2)
 STEXI
 @item -semihosting
 @findex -semihosting
-Enable semihosting mode (ARM, M68K, Xtensa, MIPS only).
+Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II only).
 ETEXI
 DEF("semihosting-config", HAS_ARG, QEMU_OPTION_semihosting_config,
 "-semihosting-config 
[enable=on|off][,target=native|gdb|auto][,arg=str[,...]]\n" \
 "semihosting configuration\n",
 QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32 |
-QEMU_ARCH_MIPS)
+QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2)
 STEXI
 @item -semihosting-config 
[enable=on|off][,target=native|gdb|auto][,arg=str[,...]]
 @findex -semihosting-config
-Enable and configure semihosting (ARM, M68K, Xtensa, MIPS only).
+Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II only).
 @table @option
 @item target=@code{native|gdb|auto}
 Defines where the semihosting calls will be addressed, to QEMU (@code{native})
diff --git a/target/nios2/Makefile.objs b/target/nios2/Makefile.objs
index 2a11c5c..010de0e 100644
--- a/target/nios2/Makefile.objs
+++ b/target/nios2/Makefile.objs
@@ -1,4 +1,4 @@
-obj-y += translate.o op_helper.o helper.o cpu.o mmu.o
+obj-y += translate.o op_helper.o helper.o cpu.o mmu.o nios2-semi.o
 obj-$(CONFIG_SOFTMMU) += monitor.o
 
 $(obj)/op_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h
index 047f376..afd30d5 100644
--- a/target/nios2/cpu.h
+++ b/target/nios2/cpu.h
@@ -141,7 +141,7 @@ typedef struct Nios2CPUClass {
 #define R_PC 64
 
 /* Exceptions */
-#define EXCP_BREAK-1
+#define EXCP_BREAK0x1000
 #define EXCP_RESET0
 #define EXCP_PRESET   1
 #define EXCP_IRQ  2
@@ -223,6 +223,8 @@ void nios2_cpu_do_unaligned_access(CPUState *cpu, vaddr 
addr,
 qemu_irq *nios2_cpu_pic_init(Nios2CPU *cpu);
 void nios2_check_interrupts(CPUNios2State *env);
 
+void do_nios2_semihosting(CPUNios2State *env);
+
 #define TARGET_PHYS_ADDR_SPACE_BITS 32
 #ifdef CONFIG_USER_ONLY
 # define TARGET_VIRT_ADDR_SPACE_BITS 31
diff --git a/target/nios2/helper.c b/target/nios2/helper.c
index a8b8ec6..ca3b087 100644
--- a/target/nios2/helper.c
+++ b/target/nios2/helper.c
@@ -25,6 +25,7 @@
 #include "exec/exec-all.h"
 #include "exec/log.h"
 #include "exec/helper-proto.h"
+#include "exec/semihost.h"
 
 #if defined(CONFIG_USER_ONLY)
 
@@ -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;
+}
+
 if ((env->regs[CR_STATUS] & CR_STATUS_EH) == 0) {
 env->regs[CR_BSTATUS] = env->regs[CR_STATUS];
 env->regs[R_BA] = env->regs[R_PC] + 4;
diff --git a/target/nios2/nios2-semi.c b/target/nios2/nios2-semi.c
new file mode 100644
index 000..9db518a
--- /dev/null
+++ b/target/nios2/nios2-semi.c
@@ -0,0 +1,446 @@
+/*
+ *  Nios II Semihosting syscall interface.
+ *  This code is derived from m68k-semi.c.
+ *
+ *  Copyright (c) 2017-2019 Mentor Graphics
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#include "qemu/osdep.h"
+
+#include "cpu.h"
+#if defined(CONFIG_USER_ONLY)
+#include "qemu.h"
+#else
+#include "qemu-common.h"
+#include "exec/gdbstub.h"
+#include "exec/softmmu-semi.h"
+#endif
+#include "qemu/log.h"
+#include "sysemu/sysemu.h"
+