Re: [Qemu-devel] [PATCH v2 6/7] linux-user: Support SVE in aarch64 signal frames

[Peter Maydell]

On 11 February 2018 at 20:58, Richard Henderson
<address@hidden> wrote:
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  linux-user/signal.c | 348 
> ++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 283 insertions(+), 65 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 9a380b9e31..af953175db 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -1443,35 +1443,61 @@ struct target_fpsimd_context {
>      uint64_t vregs[32 * 2]; /* really uint128_t vregs[32] */
>  };
>
> -/*
> - * Auxiliary context saved in the sigcontext.__reserved array. Not exported 
> to
> - * user space as it will change with the addition of new context. User space
> - * should check the magic/size information.
> - */
> -struct target_aux_context {
> -    struct target_fpsimd_context fpsimd;
> -    /* additional context to be added before "end" */
> -    struct target_aarch64_ctx end;
> +#define TARGET_EXTRA_MAGIC  0x45585401
> +
> +struct target_extra_context {
> +    struct target_aarch64_ctx head;
> +    uint64_t datap; /* 16-byte aligned pointer to extra space cast to __u64 
> */
> +    uint32_t size; /* size in bytes of the extra space */
> +    uint32_t reserved[3];
> +};
> +
> +#define TARGET_SVE_MAGIC    0x53564501
> +
> +struct target_sve_context {
> +    struct target_aarch64_ctx head;
> +    uint16_t vl;
> +    uint16_t reserved[3];
>  };

I found this patch too hard to review. It looks like you've combined
a refactoring of how we handle the AArch64 "bunch of extra auxiliary
context records" sigframe information with the addition of the SVE
context record. Could you split that into separate patches, please?
A description of why the current code isn't sufficient would also
be helpful.

thanks
-- PMM