Patchwork ARM semihosting improvements

login
register
mail settings
Submitter Daniel Jacobowitz
Date Dec. 30, 2009, 5:23 p.m.
Message ID <20091230172351.GA2326@caradoc.them.org>
Download mbox | patch
Permalink /patch/41914/
State New
Headers show

Comments

Daniel Jacobowitz - Dec. 30, 2009, 5:23 p.m.
From: Daniel Jacobowitz <dan@codesourcery.com>

This patch improves ARM semihosting to the point where qemu-system-arm
can simulate cc1 from GCC.  It can't simulate GCC itself, which
requires POSIXy bits like execve, but the backend works, including the
preprocessor.

* Use -kernel and -append for SYS_GET_CMDLINE.  This lets semihosted
programs receive command line options.

* Correctly return errno values without gdb attached.  Previously
most system calls discarded errno.

* Set errno == ENOENT after SYS_FLEN of a directory.  This is a
workaround for the absence of stat in the ARM semihosting protocol.
Now stat on directories will report that they do not exist, which
causes most applications to skip the missing directory.

Signed-off-by: Daniel Jacobowitz <dan@codesourcery.com>
Laurent Desnogues - Dec. 30, 2009, 5:32 p.m.
On Wed, Dec 30, 2009 at 6:23 PM, Daniel Jacobowitz <drow@false.org> wrote:
> From: Daniel Jacobowitz <dan@codesourcery.com>
>
> This patch improves ARM semihosting to the point where qemu-system-arm
> can simulate cc1 from GCC.  It can't simulate GCC itself, which
> requires POSIXy bits like execve, but the backend works, including the
> preprocessor.
>
> * Use -kernel and -append for SYS_GET_CMDLINE.  This lets semihosted
> programs receive command line options.
>
> * Correctly return errno values without gdb attached.  Previously
> most system calls discarded errno.
>
> * Set errno == ENOENT after SYS_FLEN of a directory.  This is a
> workaround for the absence of stat in the ARM semihosting protocol.
> Now stat on directories will report that they do not exist, which
> causes most applications to skip the missing directory.
>
> Signed-off-by: Daniel Jacobowitz <dan@codesourcery.com>
>
> diff --git a/arm-semi.c b/arm-semi.c
> index 5239ffc..e4d1ae5 100644
> --- a/arm-semi.c
> +++ b/arm-semi.c
> @@ -35,6 +35,7 @@
>  #include "qemu-common.h"
>  #include "sysemu.h"
>  #include "gdbstub.h"
> +#include "hw/arm-misc.h"
>  #endif
>
>  #define SYS_OPEN        0x01
> @@ -108,8 +109,12 @@ static inline uint32_t set_swi_errno(TaskState *ts, uint32_t code)
>     return code;
>  }
>  #else
> +static target_ulong syscall_err;
> +
>  static inline uint32_t set_swi_errno(CPUState *env, uint32_t code)
>  {
> +    if (code == (uint32_t)-1)
> +        syscall_err = errno;
>     return code;
>  }
>
> @@ -118,10 +123,6 @@ static inline uint32_t set_swi_errno(CPUState *env, uint32_t code)
>
>  static target_ulong arm_semi_syscall_len;
>
> -#if !defined(CONFIG_USER_ONLY)
> -static target_ulong syscall_err;
> -#endif
> -
>  static void arm_semi_cb(CPUState *env, target_ulong ret, target_ulong err)
>  {
>  #ifdef CONFIG_USER_ONLY
> @@ -156,8 +157,17 @@ static void arm_semi_flen_cb(CPUState *env, 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;
> +    uint32_t size, mode;
>     cpu_memory_rw_debug(env, env->regs[13]-64+32, (uint8_t *)&size, 4, 0);
> +
> +    /* Report that all directories do not exist.  */
> +    cpu_memory_rw_debug(env, env->regs[13]-64+8, (uint8_t *)&mode, 4, 0);
> +    mode = be32_to_cpu(mode);
> +    if (mode & 040000) {
> +        err = 2;
> +        size = -1;
> +    }
> +
>     env->regs[0] = be32_to_cpu(size);
>  #ifdef CONFIG_USER_ONLY
>     ((TaskState *)env->opaque)->swi_errno = err;
> @@ -310,6 +320,11 @@ uint32_t do_arm_semihosting(CPUState *env)
>             ret = set_swi_errno(ts, fstat(ARG(0), &buf));
>             if (ret == (uint32_t)-1)
>                 return -1;
> +            if (S_ISDIR (buf.st_mode)) {
> +                errno = ENOENT;
> +                set_swi_errno(ts, -1);
> +                return -1;
> +            }
>             return buf.st_size;
>         }
>     case SYS_TMPNAM:
> @@ -370,13 +385,21 @@ uint32_t do_arm_semihosting(CPUState *env)
>         return syscall_err;
>  #endif
>     case SYS_GET_CMDLINE:
> -#ifdef CONFIG_USER_ONLY
> -        /* Build a commandline from the original argv.  */
>         {
> -            char **arg = ts->info->host_argv;
>             int len = ARG(1);
>             /* lock the buffer on the ARM side */
>             char *cmdline_buffer = (char*)lock_user(VERIFY_WRITE, ARG(0), len, 0);
> +#ifdef CONFIG_USER_ONLY
> +            /* Build a commandline from the original argv.  */
> +            char **arg = ts->info->host_argv;

Did you check this works?  I think it doesn't since host_argv
field is being assigned target_argv, defined in main.  And
target_argv is freed in main before starting simulation...


Laurent

> +#else
> +            /* Build a commandline from -kernel and -append.  */
> +            /* This is simple but only because we do no escaping.  */
> +            const char *arglist[3] = { 0 }, **arg = arglist;
> +
> +            arglist[0] = env->boot_info->kernel_filename;
> +            arglist[1] = env->boot_info->kernel_cmdline;
> +#endif
>
>             if (!cmdline_buffer)
>                 /* FIXME - should this error code be -TARGET_EFAULT ? */
> @@ -410,9 +433,6 @@ uint32_t do_arm_semihosting(CPUState *env)
>             /* Return success if commandline fit into buffer.  */
>             return *arg ? -1 : 0;
>         }
> -#else
> -      return -1;
> -#endif
>     case SYS_HEAPINFO:
>         {
>             uint32_t *ptr;
>
> --
> Daniel Jacobowitz
> CodeSourcery
>
>
>
Daniel Jacobowitz - Dec. 30, 2009, 5:37 p.m.
On Wed, Dec 30, 2009 at 06:32:14PM +0100, Laurent Desnogues wrote:
> > @@ -370,13 +385,21 @@ uint32_t do_arm_semihosting(CPUState *env)
> >         return syscall_err;
> >  #endif
> >     case SYS_GET_CMDLINE:
> > -#ifdef CONFIG_USER_ONLY
> > -        /* Build a commandline from the original argv.  */
> >         {
> > -            char **arg = ts->info->host_argv;
> >             int len = ARG(1);
> >             /* lock the buffer on the ARM side */
> >             char *cmdline_buffer = (char*)lock_user(VERIFY_WRITE, ARG(0), len, 0);
> > +#ifdef CONFIG_USER_ONLY
> > +            /* Build a commandline from the original argv.  */
> > +            char **arg = ts->info->host_argv;
> 
> Did you check this works?  I think it doesn't since host_argv
> field is being assigned target_argv, defined in main.  And
> target_argv is freed in main before starting simulation...

Well, that's possible - but that code was there already; I only moved
the CONFIG_USER_ONLY case down a couple of lines.

I don't recall why there's user-mode support in this file.
Jamie Lokier - Dec. 30, 2009, 6:38 p.m.
Daniel Jacobowitz wrote:
> From: Daniel Jacobowitz <dan@codesourcery.com>
> 
> This patch improves ARM semihosting to the point where qemu-system-arm
> can simulate cc1 from GCC.  It can't simulate GCC itself, which
> requires POSIXy bits like execve, but the backend works, including the
> preprocessor.

I see that you didn't start the semihosting support, but what's the
purpose of it?  Why would you use it to run programs like cc1 instead
of qemu-arm, the user mode simulation?

Thanks,
-- Jamie
Daniel Jacobowitz - Dec. 30, 2009, 6:43 p.m.
On Wed, Dec 30, 2009 at 06:38:16PM +0000, Jamie Lokier wrote:
> I see that you didn't start the semihosting support, but what's the
> purpose of it?  Why would you use it to run programs like cc1 instead
> of qemu-arm, the user mode simulation?

You use it to run an arm-none-eabi cc1, not an arm-none-linux-gnueabi
cc1.  In my case that was useful because there was no dynamic memory
allocation.  We also rely on semihosting for GCC testing.

Patch

diff --git a/arm-semi.c b/arm-semi.c
index 5239ffc..e4d1ae5 100644
--- a/arm-semi.c
+++ b/arm-semi.c
@@ -35,6 +35,7 @@ 
 #include "qemu-common.h"
 #include "sysemu.h"
 #include "gdbstub.h"
+#include "hw/arm-misc.h"
 #endif
 
 #define SYS_OPEN        0x01
@@ -108,8 +109,12 @@  static inline uint32_t set_swi_errno(TaskState *ts, uint32_t code)
     return code;
 }
 #else
+static target_ulong syscall_err;
+
 static inline uint32_t set_swi_errno(CPUState *env, uint32_t code)
 {
+    if (code == (uint32_t)-1)
+        syscall_err = errno;
     return code;
 }
 
@@ -118,10 +123,6 @@  static inline uint32_t set_swi_errno(CPUState *env, uint32_t code)
 
 static target_ulong arm_semi_syscall_len;
 
-#if !defined(CONFIG_USER_ONLY)
-static target_ulong syscall_err;
-#endif
-
 static void arm_semi_cb(CPUState *env, target_ulong ret, target_ulong err)
 {
 #ifdef CONFIG_USER_ONLY
@@ -156,8 +157,17 @@  static void arm_semi_flen_cb(CPUState *env, 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;
+    uint32_t size, mode;
     cpu_memory_rw_debug(env, env->regs[13]-64+32, (uint8_t *)&size, 4, 0);
+
+    /* Report that all directories do not exist.  */
+    cpu_memory_rw_debug(env, env->regs[13]-64+8, (uint8_t *)&mode, 4, 0);
+    mode = be32_to_cpu(mode);
+    if (mode & 040000) {
+        err = 2;
+        size = -1;
+    }
+
     env->regs[0] = be32_to_cpu(size);
 #ifdef CONFIG_USER_ONLY
     ((TaskState *)env->opaque)->swi_errno = err;
@@ -310,6 +320,11 @@  uint32_t do_arm_semihosting(CPUState *env)
             ret = set_swi_errno(ts, fstat(ARG(0), &buf));
             if (ret == (uint32_t)-1)
                 return -1;
+            if (S_ISDIR (buf.st_mode)) {
+                errno = ENOENT;
+                set_swi_errno(ts, -1);
+                return -1;
+            }
             return buf.st_size;
         }
     case SYS_TMPNAM:
@@ -370,13 +385,21 @@  uint32_t do_arm_semihosting(CPUState *env)
         return syscall_err;
 #endif
     case SYS_GET_CMDLINE:
-#ifdef CONFIG_USER_ONLY
-        /* Build a commandline from the original argv.  */
         {
-            char **arg = ts->info->host_argv;
             int len = ARG(1);
             /* lock the buffer on the ARM side */
             char *cmdline_buffer = (char*)lock_user(VERIFY_WRITE, ARG(0), len, 0);
+#ifdef CONFIG_USER_ONLY
+            /* Build a commandline from the original argv.  */
+            char **arg = ts->info->host_argv;
+#else
+            /* Build a commandline from -kernel and -append.  */
+            /* This is simple but only because we do no escaping.  */
+            const char *arglist[3] = { 0 }, **arg = arglist;
+
+            arglist[0] = env->boot_info->kernel_filename;
+            arglist[1] = env->boot_info->kernel_cmdline;
+#endif
 
             if (!cmdline_buffer)
                 /* FIXME - should this error code be -TARGET_EFAULT ? */
@@ -410,9 +433,6 @@  uint32_t do_arm_semihosting(CPUState *env)
             /* Return success if commandline fit into buffer.  */
             return *arg ? -1 : 0;
         }
-#else
-      return -1;
-#endif
     case SYS_HEAPINFO:
         {
             uint32_t *ptr;