Re: [Qemu-arm] [PATCH 05/13] target/arm/arm-semi: Factor out implementation of SYS_CLOSE

["Alex Bennée" <alex.bennee@linaro.org>]

Peter Maydell <peter.maydell@linaro.org> writes:

> Currently for the semihosting calls which take a file descriptor
> (SYS_CLOSE, SYS_WRITE, SYS_READ, SYS_ISTTY, SYS_SEEK, SYS_FLEN)
> we have effectively two implementations, one for real host files
> and one for when we indirect via the gdbstub. We want to add a
> third one to deal with the magic :semihosting-features file.
>
> Instead of having a three-way if statement in each of these
> cases, factor out the implementation of the calls to separate
> functions which we dispatch to via function pointers selected
> via the GuestFDType for the guest fd.
>
> In this commit, we set up the framework for the dispatch,
> and convert the SYS_CLOSE call to use it.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  target/arm/arm-semi.c | 42 +++++++++++++++++++++++++++++++++++-------
>  1 file changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
> index ce3ba554bef..f3e0bf77cd3 100644
> --- a/target/arm/arm-semi.c
> +++ b/target/arm/arm-semi.c
> @@ -116,6 +116,7 @@ static int open_modeflags[12] = {
>  typedef enum GuestFDType {
>      GuestFDUnused = 0,
>      GuestFDHost = 1,
> +    GuestFDGDB = 2,
>  } GuestFDType;
>
>  /*
> @@ -179,14 +180,14 @@ static GuestFD *do_get_guestfd(int guestfd)
>  /*
>   * Associate the specified guest fd (which must have been
>   * allocated via alloc_fd() and not previously used) with
> - * the specified host fd.
> + * the specified host/gdb fd.
>   */
>  static void associate_guestfd(int guestfd, int hostfd)
>  {
>      GuestFD *gf = do_get_guestfd(guestfd);
>
>      assert(gf);
> -    gf->type = GuestFDHost;
> +    gf->type = use_gdb_syscalls() ? GuestFDGDB : GuestFDHost;
>      gf->hostfd = hostfd;
>  }
>
> @@ -337,6 +338,37 @@ static target_ulong arm_gdb_syscall(ARMCPU *cpu, gdb_syscall_complete_cb cb,
>      return is_a64(env) ? env->xregs[0] : env->regs[0];
>  }
>
> +/*
> + * Types for functions implementing various semihosting calls
> + * for specific types of guest file descriptor. These must all
> + * do the work and return the required return value for the guest,
> + * setting the guest errno if appropriate.
> + */
> +typedef uint32_t sys_closefn(TaskState *ts, ARMCPU *cpu, GuestFD *gf);
> +
> +static uint32_t host_closefn(TaskState *ts, ARMCPU *cpu, GuestFD *gf)
> +{
> +    return set_swi_errno(ts, close(gf->hostfd));
> +}
> +
> +static uint32_t gdb_closefn(TaskState *ts, ARMCPU *cpu, GuestFD *gf)
> +{
> +    return arm_gdb_syscall(cpu, arm_semi_cb, "close,%x", gf->hostfd);
> +}
> +
> +typedef struct GuestFDFunctions {
> +    sys_closefn *closefn;
> +} GuestFDFunctions;
> +
> +static const GuestFDFunctions guestfd_fns[] = {
> +    [GuestFDHost] = {
> +        .closefn = host_closefn,
> +    },
> +    [GuestFDGDB] = {
> +        .closefn = gdb_closefn,
> +    },
> +};
> +
>  /* Read the input value from the argument block; fail the semihosting
>   * call if the memory read fails.
>   */
> @@ -452,11 +484,7 @@ target_ulong do_arm_semihosting(CPUARMState *env)
>              return set_swi_errno(ts, -1);
>          }
>
> -        if (use_gdb_syscalls()) {
> -            ret = arm_gdb_syscall(cpu, arm_semi_cb, "close,%x", gf->hostfd);
> -        } else {
> -            ret = set_swi_errno(ts, close(gf->hostfd));
> -        }
> +        ret = guestfd_fns[gf->type].closefn(ts, cpu, gf);
>          dealloc_guestfd(arg0);
>          return ret;
>      case TARGET_SYS_WRITEC:


--
Alex Bennée