Re: [PATCH v4 46/53] semihosting: Add GuestFDConsole

[Luc Michel]

On 13:45 Tue 07 Jun     , Richard Henderson wrote:
> Add a GuestFDType for connecting to the semihosting console.
> Hook up to read, write, isatty, and fstat syscalls.
> 
> Note that the arm-specific syscall flen cannot be applied
> to the console, because the console is not a descriptor
> exposed to the guest.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Luc Michel <lmichel@kalray.eu>

> ---
>  include/semihosting/guestfd.h |  7 ++--
>  semihosting/syscalls.c        | 68 +++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+), 3 deletions(-)
> 
> diff --git a/include/semihosting/guestfd.h b/include/semihosting/guestfd.h
> index ef268abe85..a7ea1041ea 100644
> --- a/include/semihosting/guestfd.h
> +++ b/include/semihosting/guestfd.h
> @@ -13,9 +13,10 @@
>  
>  typedef enum GuestFDType {
>      GuestFDUnused = 0,
> -    GuestFDHost = 1,
> -    GuestFDGDB = 2,
> -    GuestFDStatic = 3,
> +    GuestFDHost,
> +    GuestFDGDB,
> +    GuestFDStatic,
> +    GuestFDConsole,
>  } GuestFDType;
>  
>  /*
> diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c
> index 13a9bdeda6..9e499b1751 100644
> --- a/semihosting/syscalls.c
> +++ b/semihosting/syscalls.c
> @@ -10,6 +10,7 @@
>  #include "exec/gdbstub.h"
>  #include "semihosting/guestfd.h"
>  #include "semihosting/syscalls.h"
> +#include "semihosting/console.h"
>  #ifdef CONFIG_USER_ONLY
>  #include "qemu.h"
>  #else
> @@ -577,6 +578,56 @@ static void staticfile_flen(CPUState *cs, 
> gdb_syscall_complete_cb complete,
>      complete(cs, gf->staticfile.len, 0);
>  }
>  
> +/*
> + * Console semihosting syscall implementations.
> + */
> +
> +static void console_read(CPUState *cs, gdb_syscall_complete_cb complete,
> +                         GuestFD *gf, target_ulong buf, target_ulong len)
> +{
> +    CPUArchState *env G_GNUC_UNUSED = cs->env_ptr;
> +    char *ptr;
> +    int ret;
> +
> +    ptr = lock_user(VERIFY_WRITE, buf, len, 0);
> +    if (!ptr) {
> +        complete(cs, -1, EFAULT);
> +        return;
> +    }
> +    ret = qemu_semihosting_console_read(cs, ptr, len);
> +    complete(cs, ret, 0);
> +    unlock_user(ptr, buf, ret);
> +}
> +
> +static void console_write(CPUState *cs, gdb_syscall_complete_cb complete,
> +                          GuestFD *gf, target_ulong buf, target_ulong len)
> +{
> +    CPUArchState *env G_GNUC_UNUSED = cs->env_ptr;
> +    char *ptr = lock_user(VERIFY_READ, buf, len, 1);
> +    int ret;
> +
> +    if (!ptr) {
> +        complete(cs, -1, EFAULT);
> +        return;
> +    }
> +    ret = qemu_semihosting_console_write(ptr, len);
> +    complete(cs, ret ? ret : -1, ret ? 0 : EIO);
> +    unlock_user(ptr, buf, ret);
> +}
> +
> +static void console_fstat(CPUState *cs, gdb_syscall_complete_cb complete,
> +                          GuestFD *gf, target_ulong addr)
> +{
> +    static const struct stat tty_buf = {
> +        .st_mode = 020666,  /* S_IFCHR, ugo+rw */
> +        .st_rdev = 5,       /* makedev(5, 0) -- linux /dev/tty */
> +    };
> +    int ret;
> +
> +    ret = copy_stat_to_user(cs, addr, &tty_buf);
> +    complete(cs, ret ? -1 : 0, ret ? -ret : 0);
> +}
> +
>  /*
>   * Syscall entry points.
>   */
> @@ -608,6 +659,7 @@ void semihost_sys_close(CPUState *cs, 
> gdb_syscall_complete_cb complete, int fd)
>          host_close(cs, complete, gf);
>          break;
>      case GuestFDStatic:
> +    case GuestFDConsole:
>          complete(cs, 0, 0);
>          break;
>      default:
> @@ -637,6 +689,9 @@ void semihost_sys_read_gf(CPUState *cs, 
> gdb_syscall_complete_cb complete,
>      case GuestFDStatic:
>          staticfile_read(cs, complete, gf, buf, len);
>          break;
> +    case GuestFDConsole:
> +        console_read(cs, complete, gf, buf, len);
> +        break;
>      default:
>          g_assert_not_reached();
>      }
> @@ -672,6 +727,9 @@ void semihost_sys_write_gf(CPUState *cs, 
> gdb_syscall_complete_cb complete,
>      case GuestFDHost:
>          host_write(cs, complete, gf, buf, len);
>          break;
> +    case GuestFDConsole:
> +        console_write(cs, complete, gf, buf, len);
> +        break;
>      case GuestFDStatic:
>          /* Static files are never open for writing: EBADF. */
>          complete(cs, -1, EBADF);
> @@ -712,6 +770,9 @@ void semihost_sys_lseek(CPUState *cs, 
> gdb_syscall_complete_cb complete,
>      case GuestFDStatic:
>          staticfile_lseek(cs, complete, gf, off, gdb_whence);
>          break;
> +    case GuestFDConsole:
> +        complete(cs, -1, ESPIPE);
> +        break;
>      default:
>          g_assert_not_reached();
>      }
> @@ -735,6 +796,9 @@ void semihost_sys_isatty(CPUState *cs, 
> gdb_syscall_complete_cb complete, int fd)
>      case GuestFDStatic:
>          complete(cs, 0, ENOTTY);
>          break;
> +    case GuestFDConsole:
> +        complete(cs, 1, 0);
> +        break;
>      default:
>          g_assert_not_reached();
>      }
> @@ -760,6 +824,7 @@ void semihost_sys_flen(CPUState *cs, 
> gdb_syscall_complete_cb fstat_cb,
>      case GuestFDStatic:
>          staticfile_flen(cs, flen_cb, gf);
>          break;
> +    case GuestFDConsole:
>      default:
>          g_assert_not_reached();
>      }
> @@ -781,6 +846,9 @@ void semihost_sys_fstat(CPUState *cs, 
> gdb_syscall_complete_cb complete,
>      case GuestFDHost:
>          host_fstat(cs, complete, gf, addr);
>          break;
> +    case GuestFDConsole:
> +        console_fstat(cs, complete, gf, addr);
> +        break;
>      case GuestFDStatic:
>      default:
>          g_assert_not_reached();
> -- 
> 2.34.1
> 
> 
> 
> 
> To declare a filtering error, please use the following link : 
> https://www.security-mail.net/reporter.php?mid=9e96.629fd636.cb46b.0&r=lmichel%40kalrayinc.com&s=qemu-devel-bounces%2Blmichel%3Dkalrayinc.com%40nongnu.org&o=%5BPATCH+v4+46%2F53%5D+semihosting%3A+Add+GuestFDConsole&verdict=C&c=3841184167a9542876ea3c22372ea8e4d69b4137
> 

--