Re: [Qemu-devel] [PATCH 2/2] target-arm/arm-semi.c: Fix SYS_HEAPINFO for 64-bit guests

[Laurent Desnogues]

On Fri, Jun 24, 2016 at 5:49 PM, Peter Maydell <address@hidden> wrote:
> SYS_HEAPINFO is one of the few semihosting calls which has to write
> values back into a parameter block in memory.  When we added
> support for 64-bit semihosting we updated the code which reads from
> the parameter block to read 64-bit words but forgot to change the
> code that writes back into the block. Update it to treat the
> block as a set of words of the appropriate width for the guest.
>
> Signed-off-by: Peter Maydell <address@hidden>
> ---
>  target-arm/arm-semi.c | 47 ++++++++++++++++++++++++++---------------------
>  1 file changed, 26 insertions(+), 21 deletions(-)
>
> diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c
> index 8be0645..d50726f 100644
> --- a/target-arm/arm-semi.c
> +++ b/target-arm/arm-semi.c
> @@ -564,8 +564,10 @@ target_ulong do_arm_semihosting(CPUARMState *env)
>          }
>      case TARGET_SYS_HEAPINFO:
>          {
> -            uint32_t *ptr;
> +            target_ulong retvals[4];
>              uint32_t limit;

I think limit should also be converted to target_ulong.

Thanks,

Laurent

> +            int i;
> +
>              GET_ARG(0);
>
>  #ifdef CONFIG_USER_ONLY
> @@ -587,30 +589,33 @@ target_ulong do_arm_semihosting(CPUARMState *env)
>                  ts->heap_limit = limit;
>              }
>
> -            ptr = lock_user(VERIFY_WRITE, arg0, 16, 0);
> -            if (!ptr) {
> -                /* FIXME - should this error code be -TARGET_EFAULT ? */
> -                return (uint32_t)-1;
> -            }
> -            ptr[0] = tswap32(ts->heap_base);
> -            ptr[1] = tswap32(ts->heap_limit);
> -            ptr[2] = tswap32(ts->stack_base);
> -            ptr[3] = tswap32(0); /* Stack limit.  */
> -            unlock_user(ptr, arg0, 16);
> +            retvals[0] = ts->heap_base;
> +            retvals[1] = ts->heap_limit;
> +            retvals[2] = ts->stack_base;
> +            retvals[3] = 0; /* Stack limit.  */
>  #else
>              limit = ram_size;
> -            ptr = lock_user(VERIFY_WRITE, arg0, 16, 0);
> -            if (!ptr) {
> -                /* FIXME - should this error code be -TARGET_EFAULT ? */
> -                return (uint32_t)-1;
> -            }
>              /* TODO: Make this use the limit of the loaded application.  */
> -            ptr[0] = tswap32(limit / 2);
> -            ptr[1] = tswap32(limit);
> -            ptr[2] = tswap32(limit); /* Stack base */
> -            ptr[3] = tswap32(0); /* Stack limit.  */
> -            unlock_user(ptr, arg0, 16);
> +            retvals[0] = limit / 2;
> +            retvals[1] = limit;
> +            retvals[2] = limit; /* Stack base */
> +            retvals[3] = 0; /* Stack limit.  */
>  #endif
> +
> +            for (i = 0; i < ARRAY_SIZE(retvals); i++) {
> +                bool fail;
> +
> +                if (is_a64(env)) {
> +                    fail = put_user_u64(retvals[i], arg0 + i * 8);
> +                } else {
> +                    fail = put_user_u32(retvals[i], arg0 + i * 4);
> +                }
> +
> +                if (fail) {
> +                    /* Couldn't write back to argument block */
> +                    return -1;
> +                }
> +            }
>              return 0;
>          }
>      case TARGET_SYS_EXIT:
> --
> 1.9.1
>