Re: [Qemu-devel] [PATCH] ARM semihosting improvements

[Laurent Desnogues]

On Wed, Dec 30, 2009 at 6:23 PM, Daniel Jacobowitz <address@hidden> wrote:
> From: Daniel Jacobowitz <address@hidden>
>
> 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 <address@hidden>
>
> 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
>
>
>