[Qemu-devel] [PATCH 0/9] target-arm: Implement A64 semihosting

[Qemu-devel] [PATCH 0/9] target-arm: Implement A64 semihosting

[Peter Maydell]

This patch series implements support for semihosting for the
64-bit ARM instruction set.

It owes a significant debt to the patches sent earlier
by Christopher Covington (and with code written by Derek Hower).
However, it is a full from-scratch rewrite (since there were
several things which I felt those patches didn't take the
right approach on). I mostly just looked at the earlier
patches to check I hadn't missed anything.

The changes in the A64 API compared to the A32/T32 one are:
 * input syscall number is in register W0
 * return result is in register X0
 * all argument parameter blocks are 64 bits wide, not 32
 * there is a new SyncCacheRange syscall
 * the SYS_EXIT syscall takes a parameter block and is able
   to pass a guest exit status out
 * the insn used to trigger semihosting is a HLT, not an
   SVC or BKPT.

I've tested this for A32, T32 and A64 semihosting, for
both usermode and system emulation, with and without gdb
remote syscalls.

The test code I wrote to do the testing is here:
https://git.linaro.org/people/peter.maydell/semihosting-tests.git/
(not very exciting, but might be handy if anybody needs a
basic "how to run C code starting with bare metal system
emulation" template.)

The test series also includes a bugfix: we haven't correctly
forwarded SYS_WRITE0 (print string to terminal) to gdb since
the gdb hosted syscall support was added to QEMU back in 2007...

Christopher Covington (1):
  target-arm: Improve semihosting debug prints

Peter Maydell (8):
  target-arm/arm-semi.c: Fix broken SYS_WRITE0 via gdb
  gdbstub: Implement gdb_do_syscallv()
  target-arm/arm-semi.c: Factor out repeated 'return env->regs[0]'
  include/exec/softmmu-semi.h: Add support for 64-bit values
  target-arm/arm-semi.c: Support widening APIs to 64 bits
  target-arm/arm-semi.c: Implement A64 specific SyncCacheRange call
  target-arm/arm-semi.c: SYS_EXIT on A64 takes a parameter block
  target-arm: Wire up HLT 0xf000 as the A64 semihosting instruction

 gdbstub.c                   |  14 ++--
 include/exec/gdbstub.h      |  27 +++++++
 include/exec/softmmu-semi.h |  18 +++++
 linux-user/main.c           |   3 +
 target-arm/arm-semi.c       | 171 +++++++++++++++++++++++++++++++++-----------
 target-arm/cpu.h            |   3 +-
 target-arm/helper-a64.c     |   6 ++
 target-arm/helper.c         |  12 +++-
 target-arm/internals.h      |   2 +
 target-arm/translate-a64.c  |  14 +++-
 10 files changed, 217 insertions(+), 53 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 1/9] target-arm/arm-semi.c: Fix broken SYS_WRITE0 via gdb

[Peter Maydell]

A spurious trailing "\n" in the gdb syscall format string used
for SYS_WRITE0 meant that gdb would reject the remote syscall,
with the effect that the output from the guest was silently dropped.
Remove the newline so that gdb accepts the packet.

Cc: qemu-stable@nongnu.org
---
 target-arm/arm-semi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c
index a2a7369..42522a7 100644
--- a/target-arm/arm-semi.c
+++ b/target-arm/arm-semi.c
@@ -260,7 +260,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
             return (uint32_t)-1;
         len = strlen(s);
         if (use_gdb_syscalls()) {
-            gdb_do_syscall(arm_semi_cb, "write,2,%x,%x\n", args, len);
+            gdb_do_syscall(arm_semi_cb, "write,2,%x,%x", args, len);
             ret = env->regs[0];
         } else {
             ret = write(STDERR_FILENO, s, len);
-- 
1.9.1

[Qemu-devel] [PATCH 2/9] target-arm: Improve semihosting debug prints

[Peter Maydell]

From: Christopher Covington <christopher.covington@linaro.org>

Print semihosting debugging information before the
do_arm_semihosting() call so that angel_SWIreason_ReportException,
which causes the function to not return, gets the same debug prints as
other semihosting calls. Also print out the semihosting call number.

Signed-off-by: Christopher Covington <christopher.covington@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 01f0d0d..9e0ca49 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -4561,8 +4561,10 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
             nr = arm_lduw_code(env, env->regs[15], env->bswap_code) & 0xff;
             if (nr == 0xab) {
                 env->regs[15] += 2;
+                qemu_log_mask(CPU_LOG_INT,
+                              "...handling as semihosting call 0x%x\n",
+                              env->regs[0]);
                 env->regs[0] = do_arm_semihosting(env);
-                qemu_log_mask(CPU_LOG_INT, "...handled as semihosting call\n");
                 return;
             }
         }
@@ -4882,8 +4884,10 @@ void arm_cpu_do_interrupt(CPUState *cs)
             if (((mask == 0x123456 && !env->thumb)
                     || (mask == 0xab && env->thumb))
                   && (env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR) {
+                qemu_log_mask(CPU_LOG_INT,
+                              "...handling as semihosting call 0x%x\n",
+                              env->regs[0]);
                 env->regs[0] = do_arm_semihosting(env);
-                qemu_log_mask(CPU_LOG_INT, "...handled as semihosting call\n");
                 return;
             }
         }
@@ -4900,8 +4904,10 @@ void arm_cpu_do_interrupt(CPUState *cs)
             if (mask == 0xab
                   && (env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR) {
                 env->regs[15] += 2;
+                qemu_log_mask(CPU_LOG_INT,
+                              "...handling as semihosting call 0x%x\n",
+                              env->regs[0]);
                 env->regs[0] = do_arm_semihosting(env);
-                qemu_log_mask(CPU_LOG_INT, "...handled as semihosting call\n");
                 return;
             }
         }
-- 
1.9.1

[Qemu-devel] [PATCH 3/9] gdbstub: Implement gdb_do_syscallv()

[Peter Maydell]

Implement a variant of the existing gdb_do_syscall() which
takes a va_list.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 gdbstub.c              | 14 ++++++++++----
 include/exec/gdbstub.h | 27 +++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index ffe7e6e..eee9b25 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1301,9 +1301,8 @@ send_packet:
     %x  - target_ulong argument printed in hex.
     %lx - 64-bit argument printed in hex.
     %s  - string pointer (target_ulong) and length (int) pair.  */
-void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...)
+void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va)
 {
-    va_list va;
     char *p;
     char *p_end;
     target_ulong addr;
@@ -1317,7 +1316,6 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...)
 #ifndef CONFIG_USER_ONLY
     vm_stop(RUN_STATE_DEBUG);
 #endif
-    va_start(va, fmt);
     p = s->syscall_buf;
     p_end = &s->syscall_buf[sizeof(s->syscall_buf)];
     *(p++) = 'F';
@@ -1351,7 +1349,6 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...)
         }
     }
     *p = 0;
-    va_end(va);
 #ifdef CONFIG_USER_ONLY
     put_packet(s, s->syscall_buf);
     gdb_handlesig(s->c_cpu, 0);
@@ -1366,6 +1363,15 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...)
 #endif
 }
 
+void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...)
+{
+    va_list va;
+
+    va_start(va, fmt);
+    gdb_do_syscallv(cb, fmt, va);
+    va_end(va);
+}
+
 static void gdb_read_byte(GDBState *s, int ch)
 {
     int i, csum;
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 05f57c2..d9e8cf7 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -14,7 +14,34 @@
 typedef void (*gdb_syscall_complete_cb)(CPUState *cpu,
                                         target_ulong ret, target_ulong err);
 
+/**
+ * gdb_do_syscall:
+ * @cb: function to call when the system call has completed
+ * @fmt: gdb syscall format string
+ * ...: list of arguments to interpolate into @fmt
+ *
+ * Send a GDB syscall request. This function will return immediately;
+ * the callback function will be called later when the remote system
+ * call has completed.
+ *
+ * @fmt should be in the 'call-id,parameter,parameter...' format documented
+ * for the F request packet in the GDB remote protocol. A limited set of
+ * printf-style format specifiers is supported:
+ *   %x  - target_ulong argument printed in hex
+ *   %lx - 64-bit argument printed in hex
+ *   %s  - string pointer (target_ulong) and length (int) pair
+ */
 void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...);
+/**
+ * gdb_do_syscallv:
+ * @cb: function to call when the system call has completed
+ * @fmt: gdb syscall format string
+ * @va: arguments to interpolate into @fmt
+ *
+ * As gdb_do_syscall, but taking a va_list rather than a variable
+ * argument list.
+ */
+void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va);
 int use_gdb_syscalls(void);
 void gdb_set_stop_cpu(CPUState *cpu);
 void gdb_exit(CPUArchState *, int);
-- 
1.9.1

[Qemu-devel] [PATCH 4/9] target-arm/arm-semi.c: Factor out repeated 'return env->regs[0]'

[Peter Maydell]

Factor out a repeated pattern in the semihosting code:

    gdb_do_syscall(arm_semi_cb, "system,%s", arg0, (int)arg1+1);
    /* arm_semi_cb sets env->regs[0] to the syscall return value */
    return env->regs[0];

For A64 the return value will go in a different register; pull
the sequence out into its own function that passes the return
value in a static variable rather than overloading regs[0]
for the purpose, so the code will work on both A32/T32 and A64.

Note that the lack-of-synchronization bug noted in the FIXME
comment is not introduced by this commit, but was already present.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/arm-semi.c | 79 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 47 insertions(+), 32 deletions(-)

diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c
index 42522a7..dbdc211 100644
--- a/target-arm/arm-semi.c
+++ b/target-arm/arm-semi.c
@@ -134,6 +134,7 @@ static void arm_semi_cb(CPUState *cs, target_ulong ret, target_ulong err)
 #ifdef CONFIG_USER_ONLY
     TaskState *ts = cs->opaque;
 #endif
+    target_ulong reg0 = env->regs[0];
 
     if (ret == (target_ulong)-1) {
 #ifdef CONFIG_USER_ONLY
@@ -141,22 +142,23 @@ static void arm_semi_cb(CPUState *cs, target_ulong ret, target_ulong err)
 #else
 	syscall_err = err;
 #endif
-        env->regs[0] = ret;
+        reg0 = ret;
     } else {
         /* Fixup syscalls that use nonstardard return conventions.  */
-        switch (env->regs[0]) {
+        switch (reg0) {
         case TARGET_SYS_WRITE:
         case TARGET_SYS_READ:
-            env->regs[0] = arm_semi_syscall_len - ret;
+            reg0 = arm_semi_syscall_len - ret;
             break;
         case TARGET_SYS_SEEK:
-            env->regs[0] = 0;
+            reg0 = 0;
             break;
         default:
-            env->regs[0] = ret;
+            reg0 = ret;
             break;
         }
     }
+    env->regs[0] = reg0;
 }
 
 static void arm_semi_flen_cb(CPUState *cs, target_ulong ret, target_ulong err)
@@ -175,6 +177,25 @@ static void arm_semi_flen_cb(CPUState *cs, target_ulong ret, target_ulong err)
 #endif
 }
 
+static target_ulong arm_gdb_syscall(ARMCPU *cpu, gdb_syscall_complete_cb cb,
+                                    const char *fmt, ...)
+{
+    va_list va;
+    CPUARMState *env = &cpu->env;
+
+    va_start(va, fmt);
+    gdb_do_syscallv(cb, fmt, va);
+    va_end(va);
+
+    /* FIXME: we are implicitly relying on the syscall completing
+     * before this point, which is not guaranteed. We should
+     * put in an explicit synchronization between this and
+     * the callback function.
+     */
+
+    return env->regs[0];
+}
+
 /* Read the input value from the argument block; fail the semihosting
  * call if the memory read fails.
  */
@@ -223,9 +244,8 @@ uint32_t do_arm_semihosting(CPUARMState *env)
             return result_fileno;
         }
         if (use_gdb_syscalls()) {
-            gdb_do_syscall(arm_semi_cb, "open,%s,%x,1a4", arg0,
-                           (int)arg2+1, gdb_open_modeflags[arg1]);
-            ret = env->regs[0];
+            ret = arm_gdb_syscall(cpu, arm_semi_cb, "open,%s,%x,1a4", arg0,
+                                  (int)arg2+1, gdb_open_modeflags[arg1]);
         } else {
             ret = set_swi_errno(ts, open(s, open_modeflags[arg1], 0644));
         }
@@ -234,8 +254,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
     case TARGET_SYS_CLOSE:
         GET_ARG(0);
         if (use_gdb_syscalls()) {
-            gdb_do_syscall(arm_semi_cb, "close,%x", arg0);
-            return env->regs[0];
+            return arm_gdb_syscall(cpu, arm_semi_cb, "close,%x", arg0);
         } else {
             return set_swi_errno(ts, close(arg0));
         }
@@ -248,8 +267,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
               return (uint32_t)-1;
           /* Write to debug console.  stderr is near enough.  */
           if (use_gdb_syscalls()) {
-                gdb_do_syscall(arm_semi_cb, "write,2,%x,1", args);
-                return env->regs[0];
+                return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,1", args);
           } else {
                 return write(STDERR_FILENO, &c, 1);
           }
@@ -260,8 +278,8 @@ uint32_t do_arm_semihosting(CPUARMState *env)
             return (uint32_t)-1;
         len = strlen(s);
         if (use_gdb_syscalls()) {
-            gdb_do_syscall(arm_semi_cb, "write,2,%x,%x", args, len);
-            ret = env->regs[0];
+            return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,%x",
+                                   args, len);
         } else {
             ret = write(STDERR_FILENO, s, len);
         }
@@ -274,8 +292,8 @@ uint32_t do_arm_semihosting(CPUARMState *env)
         len = arg2;
         if (use_gdb_syscalls()) {
             arm_semi_syscall_len = len;
-            gdb_do_syscall(arm_semi_cb, "write,%x,%x,%x", arg0, arg1, len);
-            return env->regs[0];
+            return arm_gdb_syscall(cpu, arm_semi_cb, "write,%x,%x,%x",
+                                   arg0, arg1, len);
         } else {
             s = lock_user(VERIFY_READ, arg1, len, 1);
             if (!s) {
@@ -295,8 +313,8 @@ uint32_t do_arm_semihosting(CPUARMState *env)
         len = arg2;
         if (use_gdb_syscalls()) {
             arm_semi_syscall_len = len;
-            gdb_do_syscall(arm_semi_cb, "read,%x,%x,%x", arg0, arg1, len);
-            return env->regs[0];
+            return arm_gdb_syscall(cpu, arm_semi_cb, "read,%x,%x,%x",
+                                   arg0, arg1, len);
         } else {
             s = lock_user(VERIFY_WRITE, arg1, len, 0);
             if (!s) {
@@ -317,8 +335,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
     case TARGET_SYS_ISTTY:
         GET_ARG(0);
         if (use_gdb_syscalls()) {
-            gdb_do_syscall(arm_semi_cb, "isatty,%x", arg0);
-            return env->regs[0];
+            return arm_gdb_syscall(cpu, arm_semi_cb, "isatty,%x", arg0);
         } else {
             return isatty(arg0);
         }
@@ -326,8 +343,8 @@ uint32_t do_arm_semihosting(CPUARMState *env)
         GET_ARG(0);
         GET_ARG(1);
         if (use_gdb_syscalls()) {
-            gdb_do_syscall(arm_semi_cb, "lseek,%x,%x,0", arg0, arg1);
-            return env->regs[0];
+            return arm_gdb_syscall(cpu, arm_semi_cb, "lseek,%x,%x,0",
+                                   arg0, arg1);
         } else {
             ret = set_swi_errno(ts, lseek(arg0, arg1, SEEK_SET));
             if (ret == (uint32_t)-1)
@@ -337,9 +354,8 @@ uint32_t do_arm_semihosting(CPUARMState *env)
     case TARGET_SYS_FLEN:
         GET_ARG(0);
         if (use_gdb_syscalls()) {
-            gdb_do_syscall(arm_semi_flen_cb, "fstat,%x,%x",
-                           arg0, env->regs[13]-64);
-            return env->regs[0];
+            return arm_gdb_syscall(cpu, arm_semi_flen_cb, "fstat,%x,%x",
+                                   arg0, env->regs[13]-64);
         } else {
             struct stat buf;
             ret = set_swi_errno(ts, fstat(arg0, &buf));
@@ -354,8 +370,8 @@ uint32_t do_arm_semihosting(CPUARMState *env)
         GET_ARG(0);
         GET_ARG(1);
         if (use_gdb_syscalls()) {
-            gdb_do_syscall(arm_semi_cb, "unlink,%s", arg0, (int)arg1+1);
-            ret = env->regs[0];
+            ret = arm_gdb_syscall(cpu, arm_semi_cb, "unlink,%s",
+                                  arg0, (int)arg1+1);
         } else {
             s = lock_user_string(arg0);
             if (!s) {
@@ -372,9 +388,8 @@ uint32_t do_arm_semihosting(CPUARMState *env)
         GET_ARG(2);
         GET_ARG(3);
         if (use_gdb_syscalls()) {
-            gdb_do_syscall(arm_semi_cb, "rename,%s,%s",
-                           arg0, (int)arg1+1, arg2, (int)arg3+1);
-            return env->regs[0];
+            return arm_gdb_syscall(cpu, arm_semi_cb, "rename,%s,%s",
+                                   arg0, (int)arg1+1, arg2, (int)arg3+1);
         } else {
             char *s2;
             s = lock_user_string(arg0);
@@ -398,8 +413,8 @@ uint32_t do_arm_semihosting(CPUARMState *env)
         GET_ARG(0);
         GET_ARG(1);
         if (use_gdb_syscalls()) {
-            gdb_do_syscall(arm_semi_cb, "system,%s", arg0, (int)arg1+1);
-            return env->regs[0];
+            return arm_gdb_syscall(cpu, arm_semi_cb, "system,%s",
+                                   arg0, (int)arg1+1);
         } else {
             s = lock_user_string(arg0);
             if (!s) {
-- 
1.9.1

[Qemu-devel] [PATCH 5/9] include/exec/softmmu-semi.h: Add support for 64-bit values

[Peter Maydell]

Add support for getting and setting 64-bit values in the
softmmu semihosting support functions. This will be needed
for 64-bit ARM semihosting.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/exec/softmmu-semi.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/exec/softmmu-semi.h b/include/exec/softmmu-semi.h
index 1819cc2..3a58c3f 100644
--- a/include/exec/softmmu-semi.h
+++ b/include/exec/softmmu-semi.h
@@ -9,6 +9,14 @@
 #ifndef SOFTMMU_SEMI_H
 #define SOFTMMU_SEMI_H 1
 
+static inline uint64_t softmmu_tget64(CPUArchState *env, target_ulong addr)
+{
+    uint64_t val;
+
+    cpu_memory_rw_debug(ENV_GET_CPU(env), addr, (uint8_t *)&val, 8, 0);
+    return tswap64(val);
+}
+
 static inline uint32_t softmmu_tget32(CPUArchState *env, target_ulong addr)
 {
     uint32_t val;
@@ -16,6 +24,7 @@ static inline uint32_t softmmu_tget32(CPUArchState *env, target_ulong addr)
     cpu_memory_rw_debug(ENV_GET_CPU(env), addr, (uint8_t *)&val, 4, 0);
     return tswap32(val);
 }
+
 static inline uint32_t softmmu_tget8(CPUArchState *env, target_ulong addr)
 {
     uint8_t val;
@@ -24,16 +33,25 @@ static inline uint32_t softmmu_tget8(CPUArchState *env, target_ulong addr)
     return val;
 }
 
+#define get_user_u64(arg, p) ({ arg = softmmu_tget64(env, p); 0; })
 #define get_user_u32(arg, p) ({ arg = softmmu_tget32(env, p) ; 0; })
 #define get_user_u8(arg, p) ({ arg = softmmu_tget8(env, p) ; 0; })
 #define get_user_ual(arg, p) get_user_u32(arg, p)
 
+static inline void softmmu_tput64(CPUArchState *env,
+                                  target_ulong addr, uint64_t val)
+{
+    val = tswap64(val);
+    cpu_memory_rw_debug(ENV_GET_CPU(env), addr, (uint8_t *)&val, 8, 1);
+}
+
 static inline void softmmu_tput32(CPUArchState *env,
                                   target_ulong addr, uint32_t val)
 {
     val = tswap32(val);
     cpu_memory_rw_debug(ENV_GET_CPU(env), addr, (uint8_t *)&val, 4, 1);
 }
+#define put_user_u64(arg, p) ({ softmmu_tput64(env, p, arg) ; 0; })
 #define put_user_u32(arg, p) ({ softmmu_tput32(env, p, arg) ; 0; })
 #define put_user_ual(arg, p) put_user_u32(arg, p)
 
-- 
1.9.1

[Qemu-devel] [PATCH 6/9] target-arm/arm-semi.c: Support widening APIs to 64 bits

[Peter Maydell]

The 64-bit A64 semihosting API has some pervasive changes from
the 32-bit version:
 * all parameter blocks are arrays of 64-bit values, not 32-bit
 * the semihosting call number is passed in W0
 * the return value is a 64-bit value in X0

Implement the necessary handling for this widening.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/arm-semi.c | 69 ++++++++++++++++++++++++++++++++++++++++++---------
 target-arm/cpu.h      |  2 +-
 2 files changed, 58 insertions(+), 13 deletions(-)

diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c
index dbdc211..1d4cc59 100644
--- a/target-arm/arm-semi.c
+++ b/target-arm/arm-semi.c
@@ -134,7 +134,7 @@ static void arm_semi_cb(CPUState *cs, target_ulong ret, target_ulong err)
 #ifdef CONFIG_USER_ONLY
     TaskState *ts = cs->opaque;
 #endif
-    target_ulong reg0 = env->regs[0];
+    target_ulong reg0 = is_a64(env) ? env->xregs[0] : env->regs[0];
 
     if (ret == (target_ulong)-1) {
 #ifdef CONFIG_USER_ONLY
@@ -158,7 +158,30 @@ static void arm_semi_cb(CPUState *cs, target_ulong ret, target_ulong err)
             break;
         }
     }
-    env->regs[0] = reg0;
+    if (is_a64(env)) {
+        env->xregs[0] = reg0;
+    } else {
+        env->regs[0] = reg0;
+    }
+}
+
+static target_ulong arm_flen_buf(ARMCPU *cpu)
+{
+    /* Return an address in target memory of 64 bytes where the remote
+     * gdb should write its stat struct. (The format of this structure
+     * is defined by GDB's remote protocol and is not target-specific.)
+     * We put this on the guest's stack just below SP.
+     */
+    CPUARMState *env = &cpu->env;
+    target_ulong sp;
+
+    if (is_a64(env)) {
+        sp = env->xregs[31];
+    } else {
+        sp = env->regs[13];
+    }
+
+    return sp - 64;
 }
 
 static void arm_semi_flen_cb(CPUState *cs, target_ulong ret, target_ulong err)
@@ -168,8 +191,13 @@ static void arm_semi_flen_cb(CPUState *cs, target_ulong ret, target_ulong err)
     /* The size is always stored in big-endian order, extract
        the value. We assume the size always fit in 32 bits.  */
     uint32_t size;
-    cpu_memory_rw_debug(cs, env->regs[13]-64+32, (uint8_t *)&size, 4, 0);
-    env->regs[0] = be32_to_cpu(size);
+    cpu_memory_rw_debug(cs, arm_flen_buf(cpu) + 32, (uint8_t *)&size, 4, 0);
+    size = be32_to_cpu(size);
+    if (is_a64(env)) {
+        env->xregs[0] = size;
+    } else {
+        env->regs[0] = size;
+    }
 #ifdef CONFIG_USER_ONLY
     ((TaskState *)cs->opaque)->swi_errno = err;
 #else
@@ -193,20 +221,30 @@ static target_ulong arm_gdb_syscall(ARMCPU *cpu, gdb_syscall_complete_cb cb,
      * the callback function.
      */
 
-    return env->regs[0];
+    return is_a64(env) ? env->xregs[0] : env->regs[0];
 }
 
 /* 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)) {       \
-        return (uint32_t)-1;                            \
+    if (is_a64(env)) {                                  \
+        if (get_user_u64(arg ## n, args + (n) * 8)) {   \
+            return -1;                                  \
+        }                                               \
+    } else {                                            \
+        if (get_user_u32(arg ## n, args + (n) * 4)) {   \
+            return -1;                                  \
+        }                                               \
     }                                                   \
 } while (0)
 
-#define SET_ARG(n, val) put_user_ual(val, args + (n) * 4)
-uint32_t do_arm_semihosting(CPUARMState *env)
+#define SET_ARG(n, val)                                 \
+    (is_a64(env) ?                                      \
+     put_user_u64(val, args + (n) * 8) :                \
+     put_user_u32(val, args + (n) * 4))
+
+target_ulong do_arm_semihosting(CPUARMState *env)
 {
     ARMCPU *cpu = arm_env_get_cpu(env);
     CPUState *cs = CPU(cpu);
@@ -222,8 +260,15 @@ uint32_t do_arm_semihosting(CPUARMState *env)
     CPUARMState *ts = env;
 #endif
 
-    nr = env->regs[0];
-    args = env->regs[1];
+    if (is_a64(env)) {
+        /* Note that the syscall number is in W0, not X0 */
+        nr = env->xregs[0] & 0xffffffffU;
+        args = env->xregs[1];
+    } else {
+        nr = env->regs[0];
+        args = env->regs[1];
+    }
+
     switch (nr) {
     case TARGET_SYS_OPEN:
         GET_ARG(0);
@@ -355,7 +400,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
         GET_ARG(0);
         if (use_gdb_syscalls()) {
             return arm_gdb_syscall(cpu, arm_semi_flen_cb, "fstat,%x,%x",
-                                   arg0, env->regs[13]-64);
+                                   arg0, arm_flen_buf(cpu));
         } else {
             struct stat buf;
             ret = set_swi_errno(ts, fstat(arg0, &buf));
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 7e89152..e067faa 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -500,7 +500,7 @@ typedef struct CPUARMState {
 
 ARMCPU *cpu_arm_init(const char *cpu_model);
 int cpu_arm_exec(CPUState *cpu);
-uint32_t do_arm_semihosting(CPUARMState *env);
+target_ulong do_arm_semihosting(CPUARMState *env);
 void aarch64_sync_32_to_64(CPUARMState *env);
 void aarch64_sync_64_to_32(CPUARMState *env);
 
-- 
1.9.1

[Qemu-devel] [PATCH 7/9] target-arm/arm-semi.c: Implement A64 specific SyncCacheRange call

[Peter Maydell]

The A64 semihosting ABI defines a new call SyncCacheRange
for doing a 'clean D-cache and invalidate I-cache' sequence.
Since QEMU doesn't implement caches, we can implement this as a nop.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/arm-semi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c
index 1d4cc59..1d0d7aa 100644
--- a/target-arm/arm-semi.c
+++ b/target-arm/arm-semi.c
@@ -58,6 +58,7 @@
 #define TARGET_SYS_GET_CMDLINE 0x15
 #define TARGET_SYS_HEAPINFO    0x16
 #define TARGET_SYS_EXIT        0x18
+#define TARGET_SYS_SYNCCACHE   0x19
 
 /* ADP_Stopped_ApplicationExit is used for exit(0),
  * anything else is implemented as exit(1) */
@@ -623,6 +624,15 @@ target_ulong do_arm_semihosting(CPUARMState *env)
         ret = (args == ADP_Stopped_ApplicationExit) ? 0 : 1;
         gdb_exit(env, ret);
         exit(ret);
+    case TARGET_SYS_SYNCCACHE:
+        /* Clean the D-cache and invalidate the I-cache for the specified
+         * virtual address range. This is a nop for us since we don't
+         * implement caches. This is only present on A64.
+         */
+        if (is_a64(env)) {
+            return 0;
+        }
+        /* fall through -- invalid for A32/T32 */
     default:
         fprintf(stderr, "qemu: Unsupported SemiHosting SWI 0x%02x\n", nr);
         cpu_dump_state(cs, stderr, fprintf, 0);
-- 
1.9.1

[Qemu-devel] [PATCH 8/9] target-arm/arm-semi.c: SYS_EXIT on A64 takes a parameter block

[Peter Maydell]

The A64 semihosting API changes the interface for SYS_EXIT so
that instead of taking a single exception type in a register,
it takes a parameter block containing the exception type and
a sub-code. Implement this.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/arm-semi.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c
index 1d0d7aa..d7cff3d 100644
--- a/target-arm/arm-semi.c
+++ b/target-arm/arm-semi.c
@@ -619,9 +619,24 @@ target_ulong do_arm_semihosting(CPUARMState *env)
             return 0;
         }
     case TARGET_SYS_EXIT:
-        /* ARM specifies only Stopped_ApplicationExit as normal
-         * exit, everything else is considered an error */
-        ret = (args == ADP_Stopped_ApplicationExit) ? 0 : 1;
+        if (is_a64(env)) {
+            /* The A64 version of this call takes a parameter block,
+             * so the application-exit type can return a subcode which
+             * is the exit status code from the application.
+             */
+            GET_ARG(0);
+            GET_ARG(1);
+
+            if (arg0 == ADP_Stopped_ApplicationExit) {
+                ret = arg1;
+            } else {
+                ret = 1;
+            }
+        } else {
+            /* ARM specifies only Stopped_ApplicationExit as normal
+             * exit, everything else is considered an error */
+            ret = (args == ADP_Stopped_ApplicationExit) ? 0 : 1;
+        }
         gdb_exit(env, ret);
         exit(ret);
     case TARGET_SYS_SYNCCACHE:
-- 
1.9.1

[Qemu-devel] [PATCH 9/9] target-arm: Wire up HLT 0xf000 as the A64 semihosting instruction

[Peter Maydell]

For the A64 instruction set, the semihosting call instruction
is 'HLT 0xf000'. Wire this up to call do_arm_semihosting()
if semihosting is enabled.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/main.c          |  3 +++
 target-arm/cpu.h           |  1 +
 target-arm/helper-a64.c    |  6 ++++++
 target-arm/internals.h     |  2 ++
 target-arm/translate-a64.c | 14 ++++++++++++--
 5 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index fdee981..56f452e 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1054,6 +1054,9 @@ void cpu_loop(CPUARMState *env)
                 queue_signal(env, info.si_signo, &info);
             }
             break;
+        case EXCP_SEMIHOST:
+            env->xregs[0] = do_arm_semihosting(env);
+            break;
         default:
             fprintf(stderr, "qemu: unhandled CPU exception 0x%x - aborting\n",
                     trapnr);
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index e067faa..fd91288 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -56,6 +56,7 @@
 #define EXCP_SMC            13   /* Secure Monitor Call */
 #define EXCP_VIRQ           14
 #define EXCP_VFIQ           15
+#define EXCP_SEMIHOST       16   /* semihosting call (A64 only) */
 
 #define ARMV7M_EXCP_RESET   1
 #define ARMV7M_EXCP_NMI     2
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 08c95a3..02fc9b4 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -514,6 +514,12 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
     case EXCP_VFIQ:
         addr += 0x100;
         break;
+    case EXCP_SEMIHOST:
+        qemu_log_mask(CPU_LOG_INT,
+                      "...handling as semihosting call 0x%" PRIx64 "\n",
+                      env->xregs[0]);
+        env->xregs[0] = do_arm_semihosting(env);
+        return;
     default:
         cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
     }
diff --git a/target-arm/internals.h b/target-arm/internals.h
index 924aff9..36a56aa 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -36,6 +36,7 @@ static inline bool excp_is_internal(int excp)
         || excp == EXCP_HALTED
         || excp == EXCP_EXCEPTION_EXIT
         || excp == EXCP_KERNEL_TRAP
+        || excp == EXCP_SEMIHOST
         || excp == EXCP_STREX;
 }
 
@@ -58,6 +59,7 @@ static const char * const excnames[] = {
     [EXCP_SMC] = "Secure Monitor Call",
     [EXCP_VIRQ] = "Virtual IRQ",
     [EXCP_VFIQ] = "Virtual FIQ",
+    [EXCP_SEMIHOST] = "Semihosting call",
 };
 
 static inline void arm_log_exception(int idx)
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 689f2be..8810760 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -30,6 +30,7 @@
 #include "internals.h"
 #include "qemu/host-utils.h"
 
+#include "exec/semihost.h"
 #include "exec/gen-icount.h"
 
 #include "exec/helper-proto.h"
@@ -1553,8 +1554,17 @@ static void disas_exc(DisasContext *s, uint32_t insn)
             unallocated_encoding(s);
             break;
         }
-        /* HLT */
-        unsupported_encoding(s, insn);
+        /* HLT. This has two purposes.
+         * Architecturally, it is an external halting debug instruction.
+         * Since QEMU doesn't implement external debug, we treat this as
+         * it is required for halting debug disabled: it will UNDEF.
+         * Secondly, "HLT 0xf000" is the A64 semihosting syscall instruction.
+         */
+        if (semihosting_enabled() && imm16 == 0xf000) {
+            gen_exception_internal_insn(s, 0, EXCP_SEMIHOST);
+        } else {
+            unsupported_encoding(s, insn);
+        }
         break;
     case 5:
         if (op2_ll < 1 || op2_ll > 3) {
-- 
1.9.1

Re: [Qemu-devel] [PATCH 4/9] target-arm/arm-semi.c: Factor out repeated 'return env->regs[0]'

[Christopher Covington]

On Thu, Aug 13, 2015 at 9:35 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Factor out a repeated pattern in the semihosting code:
>
>     gdb_do_syscall(arm_semi_cb, "system,%s", arg0, (int)arg1+1);
>     /* arm_semi_cb sets env->regs[0] to the syscall return value */
>     return env->regs[0];
>
> For A64 the return value will go in a different register; pull
> the sequence out into its own function that passes the return
> value in a static variable rather than overloading regs[0]
> for the purpose, so the code will work on both A32/T32 and A64.
>
> Note that the lack-of-synchronization bug noted in the FIXME
> comment is not introduced by this commit, but was already present.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Christopher Covington <christopher.covington@linaro.org>

Re: [Qemu-devel] [PATCH 9/9] target-arm: Wire up HLT 0xf000 as the A64 semihosting instruction

[Christopher Covington]

[-- Attachment #1: Type: text/plain, Size: 362 bytes --]

On Aug 13, 2015 9:35 AM, "Peter Maydell" <peter.maydell@linaro.org> wrote:
>
> For the A64 instruction set, the semihosting call instruction
> is 'HLT 0xf000'. Wire this up to call do_arm_semihosting()
> if semihosting is enabled.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Christopher Covington <christopher.covington@linaro.org>

[-- Attachment #2: Type: text/html, Size: 624 bytes --]

Re: [Qemu-devel] [PATCH 6/9] target-arm/arm-semi.c: Support widening APIs to 64 bits

[Christopher Covington]

On Thu, Aug 13, 2015 at 9:35 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> The 64-bit A64 semihosting API has some pervasive changes from
> the 32-bit version:
>  * all parameter blocks are arrays of 64-bit values, not 32-bit
>  * the semihosting call number is passed in W0
>  * the return value is a 64-bit value in X0
>
> Implement the necessary handling for this widening.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Christopher Covington <christopher.covington@linaro.org>

Re: [Qemu-devel] [PATCH 7/9] target-arm/arm-semi.c: Implement A64 specific SyncCacheRange call

[Christopher Covington]

On Thu, Aug 13, 2015 at 9:35 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> The A64 semihosting ABI defines a new call SyncCacheRange
> for doing a 'clean D-cache and invalidate I-cache' sequence.
> Since QEMU doesn't implement caches, we can implement this as a nop.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Christopher Covington <christopher.covington@linaro.org>

Re: [Qemu-devel] [PATCH 0/9] target-arm: Implement A64 semihosting

[Christopher Covington]

Hi Peter,

On 08/13/2015 12:35 PM, Peter Maydell wrote:
> This patch series implements support for semihosting for the
> 64-bit ARM instruction set.
> 
> It owes a significant debt to the patches sent earlier
> by Christopher Covington (and with code written by Derek Hower).
> However, it is a full from-scratch rewrite (since there were
> several things which I felt those patches didn't take the
> right approach on). I mostly just looked at the earlier
> patches to check I hadn't missed anything.
> 
> The changes in the A64 API compared to the A32/T32 one are:
>  * input syscall number is in register W0
>  * return result is in register X0
>  * all argument parameter blocks are 64 bits wide, not 32
>  * there is a new SyncCacheRange syscall
>  * the SYS_EXIT syscall takes a parameter block and is able
>    to pass a guest exit status out
>  * the insn used to trigger semihosting is a HLT, not an
>    SVC or BKPT.
> 
> I've tested this for A32, T32 and A64 semihosting, for
> both usermode and system emulation, with and without gdb
> remote syscalls.
> 
> The test code I wrote to do the testing is here:
> https://git.linaro.org/people/peter.maydell/semihosting-tests.git/
> (not very exciting, but might be handy if anybody needs a
> basic "how to run C code starting with bare metal system
> emulation" template.)
> 
> The test series also includes a bugfix: we haven't correctly
> forwarded SYS_WRITE0 (print string to terminal) to gdb since
> the gdb hosted syscall support was added to QEMU back in 2007...

Your work on this is greatly appreciated.

Tested-by: Christopher Covington <cov@codeaurora.org>

This works for simple Linux userspace angel-load, angel-store, and angel-exit
utilities as well as at least one newlib/libgloss test. Some more complicated
newlib/libgloss binaries don't run, but that appears to be because of
attempted vbar_el3 accesses.

If it's not much trouble, adding your semihosting tests to kvm-unit-tests
might be nice.

Thanks,
Christopher Covington

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [Qemu-devel] [PATCH 9/9] target-arm: Wire up HLT 0xf000 as the A64 semihosting instruction

[Peter Maydell]

On 13 August 2015 at 17:35, Peter Maydell <peter.maydell@linaro.org> wrote:
> For the A64 instruction set, the semihosting call instruction
> is 'HLT 0xf000'. Wire this up to call do_arm_semihosting()
> if semihosting is enabled.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---

> @@ -1553,8 +1554,17 @@ static void disas_exc(DisasContext *s, uint32_t insn)
>              unallocated_encoding(s);
>              break;
>          }
> -        /* HLT */
> -        unsupported_encoding(s, insn);
> +        /* HLT. This has two purposes.
> +         * Architecturally, it is an external halting debug instruction.
> +         * Since QEMU doesn't implement external debug, we treat this as
> +         * it is required for halting debug disabled: it will UNDEF.
> +         * Secondly, "HLT 0xf000" is the A64 semihosting syscall instruction.
> +         */
> +        if (semihosting_enabled() && imm16 == 0xf000) {
> +            gen_exception_internal_insn(s, 0, EXCP_SEMIHOST);
> +        } else {
> +            unsupported_encoding(s, insn);
> +        }

Christopher pointed out to me at KVM Forum that this isn't
consistent with how we do 32-bit ARM semihosting, which has a
check to prevent its use from userspace in system emulation.
(The idea is that semihosting is basically a "guest can pwn
your host" API, so giving access to it to guest userspace is
kind of brave.)

I propose to squash the following change into this patch as I
put it into target-arm.next.


--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1561,6 +1561,16 @@ static void disas_exc(DisasContext *s, uint32_t insn)
          * Secondly, "HLT 0xf000" is the A64 semihosting syscall instruction.
          */
         if (semihosting_enabled() && imm16 == 0xf000) {
+#ifndef CONFIG_USER_ONLY
+            /* In system mode, don't allow userspace access to semihosting,
+             * to provide some semblance of security (and for consistency
+             * with our 32-bit semihosting).
+             */
+            if (s->current_el == 0) {
+                unsupported_encoding(s, insn);
+                break;
+            }
+#endif
             gen_exception_internal_insn(s, 0, EXCP_SEMIHOST);
         } else {
             unsupported_encoding(s, insn);


This brings it into line with the 32-bit code.

There is a usecase for allowing unfettered access to semihosting
in system emulation mode (basically, running bare metal test
binaries). I think we should deal with that by having a separate
command line option for "userspace semihosting access is OK",
which changes the behaviour for both 32-bit and 64-bit semihosting
APIs. Alternatively, we could instead allow userspace to use
"safe" parts of the semihosting API, like "print to stdout",
but not the less safe parts like "open and write to arbitrary
host files". Or we could decide that this safety check isn't
actually very useful (no other model/debug environment has it
that I know of) and drop it entirely; but that makes me a little
nervous.

(It would actually be nice to be able to say "I'd like the
guest kernel to be able to do early printk via semihosting
without trusting it to open files etc", for that matter.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 9/9] target-arm: Wire up HLT 0xf000 as the A64 semihosting instruction

[Christopher Covington]

Hi Peter,

On 08/27/2015 02:35 PM, Peter Maydell wrote:
> On 13 August 2015 at 17:35, Peter Maydell <peter.maydell@linaro.org> wrote:
>> For the A64 instruction set, the semihosting call instruction
>> is 'HLT 0xf000'. Wire this up to call do_arm_semihosting()
>> if semihosting is enabled.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
> 
>> @@ -1553,8 +1554,17 @@ static void disas_exc(DisasContext *s, uint32_t insn)
>>              unallocated_encoding(s);
>>              break;
>>          }
>> -        /* HLT */
>> -        unsupported_encoding(s, insn);
>> +        /* HLT. This has two purposes.
>> +         * Architecturally, it is an external halting debug instruction.
>> +         * Since QEMU doesn't implement external debug, we treat this as
>> +         * it is required for halting debug disabled: it will UNDEF.
>> +         * Secondly, "HLT 0xf000" is the A64 semihosting syscall instruction.
>> +         */
>> +        if (semihosting_enabled() && imm16 == 0xf000) {
>> +            gen_exception_internal_insn(s, 0, EXCP_SEMIHOST);
>> +        } else {
>> +            unsupported_encoding(s, insn);
>> +        }
> 
> Christopher pointed out to me at KVM Forum that this isn't
> consistent with how we do 32-bit ARM semihosting, which has a
> check to prevent its use from userspace in system emulation.
> (The idea is that semihosting is basically a "guest can pwn
> your host" API, so giving access to it to guest userspace is
> kind of brave.)

> There is a usecase for allowing unfettered access to semihosting
> in system emulation mode (basically, running bare metal test
> binaries). I think we should deal with that by having a separate
> command line option for "userspace semihosting access is OK",
> which changes the behaviour for both 32-bit and 64-bit semihosting
> APIs. Alternatively, we could instead allow userspace to use
> "safe" parts of the semihosting API, like "print to stdout",
> but not the less safe parts like "open and write to arbitrary
> host files". Or we could decide that this safety check isn't
> actually very useful (no other model/debug environment has it
> that I know of) and drop it entirely; but that makes me a little
> nervous.

I find allowing trusted guests to access host files to be a very useful
feature. To me it is very similar to passing through / (root) via VirtIO-9P.
Perhaps a useful way of making sure the user knows what files their guest is
gaining access to would be to have a semihosting path prefix option. That way
access could be allowed nowhere; clearly allow everywhere (/); or clearly be
restricted to, and relative to, a certain sysroot directory
(/home/user/my-sysroot).

Christopher Covington

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project