* [Qemu-devel] [PATCH v4 1/5] linux-user: Implement aarch64 PR_SVE_SET/GET_VL
2018-03-03 14:38 [Qemu-devel] [PATCH v4 0/5] target/arm linux-user changes for sve Richard Henderson
@ 2018-03-03 14:38 ` Richard Henderson
2018-03-06 12:28 ` Alex Bennée
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 2/5] aarch64-linux-user: Split out helpers for guest signal handling Richard Henderson
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Richard Henderson @ 2018-03-03 14:38 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
As an implementation choice, widening VL has zeroed the
previously inaccessible portion of the sve registers.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/aarch64/target_syscall.h | 3 +++
target/arm/cpu.h | 1 +
linux-user/syscall.c | 27 ++++++++++++++++++++++++
target/arm/cpu64.c | 41 +++++++++++++++++++++++++++++++++++++
4 files changed, 72 insertions(+)
diff --git a/linux-user/aarch64/target_syscall.h b/linux-user/aarch64/target_syscall.h
index 604ab99b14..205265e619 100644
--- a/linux-user/aarch64/target_syscall.h
+++ b/linux-user/aarch64/target_syscall.h
@@ -19,4 +19,7 @@ struct target_pt_regs {
#define TARGET_MLOCKALL_MCL_CURRENT 1
#define TARGET_MLOCKALL_MCL_FUTURE 2
+#define TARGET_PR_SVE_SET_VL 50
+#define TARGET_PR_SVE_GET_VL 51
+
#endif /* AARCH64_TARGET_SYSCALL_H */
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 8dd6b788df..5f4566f017 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -861,6 +861,7 @@ int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
#ifdef TARGET_AARCH64
int aarch64_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
+void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq);
#endif
target_ulong do_arm_semihosting(CPUARMState *env);
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index e24f43c4a2..38f40e2692 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10670,6 +10670,33 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
break;
}
#endif
+#ifdef TARGET_AARCH64
+ case TARGET_PR_SVE_SET_VL:
+ /* We cannot support either PR_SVE_SET_VL_ONEXEC
+ or PR_SVE_VL_INHERIT. Therefore, anything above
+ ARM_MAX_VQ results in EINVAL. */
+ ret = -TARGET_EINVAL;
+ if (arm_feature(cpu_env, ARM_FEATURE_SVE)
+ && arg2 >= 0 && arg2 <= ARM_MAX_VQ * 16 && !(arg2 & 15)) {
+ CPUARMState *env = cpu_env;
+ int old_vq = (env->vfp.zcr_el[1] & 0xf) + 1;
+ int vq = MAX(arg2 / 16, 1);
+
+ if (vq < old_vq) {
+ aarch64_sve_narrow_vq(env, vq);
+ }
+ env->vfp.zcr_el[1] = vq - 1;
+ ret = vq * 16;
+ }
+ break;
+ case TARGET_PR_SVE_GET_VL:
+ ret = -TARGET_EINVAL;
+ if (arm_feature(cpu_env, ARM_FEATURE_SVE)) {
+ CPUARMState *env = cpu_env;
+ ret = ((env->vfp.zcr_el[1] & 0xf) + 1) * 16;
+ }
+ break;
+#endif /* AARCH64 */
case PR_GET_SECCOMP:
case PR_SET_SECCOMP:
/* Disable seccomp to prevent the target disabling syscalls we
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 4228713b19..74b485b382 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -366,3 +366,44 @@ static void aarch64_cpu_register_types(void)
}
type_init(aarch64_cpu_register_types)
+
+/* The manual says that when SVE is enabled and VQ is widened the
+ * implementation is allowed to zero the previously inaccessible
+ * portion of the registers. The corollary to that is that when
+ * SVE is enabled and VQ is narrowed we are also allowed to zero
+ * the now inaccessible portion of the registers.
+ *
+ * The intent of this is that no predicate bit beyond VQ is ever set.
+ * Which means that some operations on predicate registers themselves
+ * may operate on full uint64_t or even unrolled across the maximum
+ * uint64_t[4]. Performing 4 bits of host arithmetic unconditionally
+ * may well be cheaper than conditionals to restrict the operation
+ * to the relevant portion of a uint16_t[16].
+ *
+ * TODO: Need to call this for changes to the real system registers
+ * and EL state changes.
+ */
+void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq)
+{
+ int i, j;
+ uint64_t pmask;
+
+ assert(vq >= 1 && vq <= ARM_MAX_VQ);
+
+ /* Zap the high bits of the zregs. */
+ for (i = 0; i < 32; i++) {
+ memset(&env->vfp.zregs[i].d[2 * vq], 0, 16 * (ARM_MAX_VQ - vq));
+ }
+
+ /* Zap the high bits of the pregs and ffr. */
+ pmask = 0;
+ if (vq & 3) {
+ pmask = ~(-1ULL << (16 * (vq & 3)));
+ }
+ for (j = vq / 4; j < ARM_MAX_VQ / 4; j++) {
+ for (i = 0; i < 17; ++i) {
+ env->vfp.pregs[i].p[j] &= pmask;
+ }
+ pmask = 0;
+ }
+}
--
2.14.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v4 1/5] linux-user: Implement aarch64 PR_SVE_SET/GET_VL
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 1/5] linux-user: Implement aarch64 PR_SVE_SET/GET_VL Richard Henderson
@ 2018-03-06 12:28 ` Alex Bennée
2018-03-06 12:58 ` [Qemu-devel] [Qemu-arm] " Peter Maydell
0 siblings, 1 reply; 15+ messages in thread
From: Alex Bennée @ 2018-03-06 12:28 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
Richard Henderson <richard.henderson@linaro.org> writes:
> As an implementation choice, widening VL has zeroed the
> previously inaccessible portion of the sve registers.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/aarch64/target_syscall.h | 3 +++
> target/arm/cpu.h | 1 +
> linux-user/syscall.c | 27 ++++++++++++++++++++++++
> target/arm/cpu64.c | 41 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 72 insertions(+)
>
> diff --git a/linux-user/aarch64/target_syscall.h b/linux-user/aarch64/target_syscall.h
> index 604ab99b14..205265e619 100644
> --- a/linux-user/aarch64/target_syscall.h
> +++ b/linux-user/aarch64/target_syscall.h
> @@ -19,4 +19,7 @@ struct target_pt_regs {
> #define TARGET_MLOCKALL_MCL_CURRENT 1
> #define TARGET_MLOCKALL_MCL_FUTURE 2
>
> +#define TARGET_PR_SVE_SET_VL 50
> +#define TARGET_PR_SVE_GET_VL 51
For some reason I thought we might get this from our copy of
linux-headers but it seems we only do that for KVM bits.
> +
> #endif /* AARCH64_TARGET_SYSCALL_H */
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 8dd6b788df..5f4566f017 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -861,6 +861,7 @@ int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
> #ifdef TARGET_AARCH64
> int aarch64_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
> int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
> +void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq);
> #endif
>
> target_ulong do_arm_semihosting(CPUARMState *env);
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index e24f43c4a2..38f40e2692 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -10670,6 +10670,33 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
> break;
> }
> #endif
> +#ifdef TARGET_AARCH64
> + case TARGET_PR_SVE_SET_VL:
> + /* We cannot support either PR_SVE_SET_VL_ONEXEC
> + or PR_SVE_VL_INHERIT. Therefore, anything above
> + ARM_MAX_VQ results in EINVAL. */
> + ret = -TARGET_EINVAL;
> + if (arm_feature(cpu_env, ARM_FEATURE_SVE)
> + && arg2 >= 0 && arg2 <= ARM_MAX_VQ * 16 && !(arg2 & 15)) {
> + CPUARMState *env = cpu_env;
The kernel code splits the arg2 up into VL and flags. We don't seem to
be doing that here.
vl = arg & PR_SVE_VL_LEN_MASK;
flags = arg & ~vl;
I'm not sure what && !(arg2 & 15) is doing but PR_SVE_VL_LEN_MASK is
0xffff, Perhaps some defines would be useful to make it clearer.
> + int old_vq = (env->vfp.zcr_el[1] & 0xf) + 1;
> + int vq = MAX(arg2 / 16, 1);
> +
> + if (vq < old_vq) {
> + aarch64_sve_narrow_vq(env, vq);
> + }
> + env->vfp.zcr_el[1] = vq - 1;
It seems odd not to have setting this inside cpu64.c. Won't a similar
manipulation need to be made for system mode? I'd keep all the logic
together in aarch64_sve_narrow_vq (or maybe call it aarch64_sve_set_vq
and pass it the current exception level).
> + ret = vq * 16;
> + }
> + break;
> + case TARGET_PR_SVE_GET_VL:
> + ret = -TARGET_EINVAL;
> + if (arm_feature(cpu_env, ARM_FEATURE_SVE)) {
> + CPUARMState *env = cpu_env;
> + ret = ((env->vfp.zcr_el[1] & 0xf) + 1) * 16;
> + }
> + break;
> +#endif /* AARCH64 */
> case PR_GET_SECCOMP:
> case PR_SET_SECCOMP:
> /* Disable seccomp to prevent the target disabling syscalls we
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 4228713b19..74b485b382 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -366,3 +366,44 @@ static void aarch64_cpu_register_types(void)
> }
>
> type_init(aarch64_cpu_register_types)
> +
> +/* The manual says that when SVE is enabled and VQ is widened the
> + * implementation is allowed to zero the previously inaccessible
> + * portion of the registers. The corollary to that is that when
> + * SVE is enabled and VQ is narrowed we are also allowed to zero
> + * the now inaccessible portion of the registers.
> + *
> + * The intent of this is that no predicate bit beyond VQ is ever set.
> + * Which means that some operations on predicate registers themselves
> + * may operate on full uint64_t or even unrolled across the maximum
> + * uint64_t[4]. Performing 4 bits of host arithmetic unconditionally
> + * may well be cheaper than conditionals to restrict the operation
> + * to the relevant portion of a uint16_t[16].
> + *
> + * TODO: Need to call this for changes to the real system registers
> + * and EL state changes.
> + */
> +void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq)
> +{
> + int i, j;
> + uint64_t pmask;
> +
> + assert(vq >= 1 && vq <= ARM_MAX_VQ);
> +
> + /* Zap the high bits of the zregs. */
> + for (i = 0; i < 32; i++) {
> + memset(&env->vfp.zregs[i].d[2 * vq], 0, 16 * (ARM_MAX_VQ - vq));
> + }
> +
> + /* Zap the high bits of the pregs and ffr. */
> + pmask = 0;
> + if (vq & 3) {
> + pmask = ~(-1ULL << (16 * (vq & 3)));
> + }
The kernel defines SVE_VQ_BYTES for clarity, perhaps we should do so to
here.
> + for (j = vq / 4; j < ARM_MAX_VQ / 4; j++) {
> + for (i = 0; i < 17; ++i) {
> + env->vfp.pregs[i].p[j] &= pmask;
> + }
> + pmask = 0;
> + }
> +}
--
Alex Bennée
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v4 1/5] linux-user: Implement aarch64 PR_SVE_SET/GET_VL
2018-03-06 12:28 ` Alex Bennée
@ 2018-03-06 12:58 ` Peter Maydell
2018-03-06 14:03 ` Alex Bennée
0 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2018-03-06 12:58 UTC (permalink / raw)
To: Alex Bennée; +Cc: Richard Henderson, qemu-arm, QEMU Developers
On 6 March 2018 at 12:28, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> As an implementation choice, widening VL has zeroed the
>> previously inaccessible portion of the sve registers.
>>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> + int old_vq = (env->vfp.zcr_el[1] & 0xf) + 1;
>> + int vq = MAX(arg2 / 16, 1);
>> +
>> + if (vq < old_vq) {
>> + aarch64_sve_narrow_vq(env, vq);
>> + }
>> + env->vfp.zcr_el[1] = vq - 1;
>
> It seems odd not to have setting this inside cpu64.c. Won't a similar
> manipulation need to be made for system mode? I'd keep all the logic
> together in aarch64_sve_narrow_vq (or maybe call it aarch64_sve_set_vq
> and pass it the current exception level).
I think I asked Richard to put it into linux-user because it was
in target/arm in an earlier version of this series. The manipulation
that's happening here is kind of linux-specific (if it were for
system mode we'd need to think about ZCR_EL2 and ZCR_EL3 as well),
and the analogy is with cpu_set_tls/cpu_get_tls which are in
linux-user/arm/target_cpu.h.
NB: I've already put this series in target-arm.next -- do you want
me to drop them ? (That would mean they won't go in 2.12, given
RTH is away.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v4 1/5] linux-user: Implement aarch64 PR_SVE_SET/GET_VL
2018-03-06 12:58 ` [Qemu-devel] [Qemu-arm] " Peter Maydell
@ 2018-03-06 14:03 ` Alex Bennée
0 siblings, 0 replies; 15+ messages in thread
From: Alex Bennée @ 2018-03-06 14:03 UTC (permalink / raw)
To: Peter Maydell; +Cc: Richard Henderson, qemu-arm, QEMU Developers
Peter Maydell <peter.maydell@linaro.org> writes:
> On 6 March 2018 at 12:28, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> As an implementation choice, widening VL has zeroed the
>>> previously inaccessible portion of the sve registers.
>>>
>>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>
>>> + int old_vq = (env->vfp.zcr_el[1] & 0xf) + 1;
>>> + int vq = MAX(arg2 / 16, 1);
>>> +
>>> + if (vq < old_vq) {
>>> + aarch64_sve_narrow_vq(env, vq);
>>> + }
>>> + env->vfp.zcr_el[1] = vq - 1;
>>
>> It seems odd not to have setting this inside cpu64.c. Won't a similar
>> manipulation need to be made for system mode? I'd keep all the logic
>> together in aarch64_sve_narrow_vq (or maybe call it aarch64_sve_set_vq
>> and pass it the current exception level).
>
> I think I asked Richard to put it into linux-user because it was
> in target/arm in an earlier version of this series. The manipulation
> that's happening here is kind of linux-specific (if it were for
> system mode we'd need to think about ZCR_EL2 and ZCR_EL3 as well),
> and the analogy is with cpu_set_tls/cpu_get_tls which are in
> linux-user/arm/target_cpu.h.
Fair enough.
>
> NB: I've already put this series in target-arm.next -- do you want
> me to drop them ? (That would mean they won't go in 2.12, given
> RTH is away.)
No it's fine. We can always fix up minor nits later when system mode is
done.
Acked-by: Alex Bennée <alex.bennee@linaro.org>
>
> thanks
> -- PMM
--
Alex Bennée
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v4 2/5] aarch64-linux-user: Split out helpers for guest signal handling
2018-03-03 14:38 [Qemu-devel] [PATCH v4 0/5] target/arm linux-user changes for sve Richard Henderson
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 1/5] linux-user: Implement aarch64 PR_SVE_SET/GET_VL Richard Henderson
@ 2018-03-03 14:38 ` Richard Henderson
2018-03-06 14:11 ` Alex Bennée
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 3/5] aarch64-linux-user: Remove struct target_aux_context Richard Henderson
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Richard Henderson @ 2018-03-03 14:38 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Split out helpers from target_setup_frame and target_restore_sigframe
for dealing with general registers, fpsimd registers, and the end record.
When we add support for sve registers, the relative positions of
these will change.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/signal.c | 120 ++++++++++++++++++++++++++++++----------------------
1 file changed, 69 insertions(+), 51 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 9a380b9e31..25c9743aed 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1462,16 +1462,17 @@ struct target_rt_sigframe {
uint32_t tramp[2];
};
-static int target_setup_sigframe(struct target_rt_sigframe *sf,
- CPUARMState *env, target_sigset_t *set)
+static void target_setup_general_frame(struct target_rt_sigframe *sf,
+ CPUARMState *env, target_sigset_t *set)
{
int i;
- struct target_aux_context *aux =
- (struct target_aux_context *)sf->uc.tuc_mcontext.__reserved;
- /* set up the stack frame for unwinding */
- __put_user(env->xregs[29], &sf->fp);
- __put_user(env->xregs[30], &sf->lr);
+ __put_user(0, &sf->uc.tuc_flags);
+ __put_user(0, &sf->uc.tuc_link);
+
+ __put_user(target_sigaltstack_used.ss_sp, &sf->uc.tuc_stack.ss_sp);
+ __put_user(sas_ss_flags(env->xregs[31]), &sf->uc.tuc_stack.ss_flags);
+ __put_user(target_sigaltstack_used.ss_size, &sf->uc.tuc_stack.ss_size);
for (i = 0; i < 31; i++) {
__put_user(env->xregs[i], &sf->uc.tuc_mcontext.regs[i]);
@@ -1485,39 +1486,42 @@ static int target_setup_sigframe(struct target_rt_sigframe *sf,
for (i = 0; i < TARGET_NSIG_WORDS; i++) {
__put_user(set->sig[i], &sf->uc.tuc_sigmask.sig[i]);
}
+}
+
+static void target_setup_fpsimd_record(struct target_fpsimd_context *fpsimd,
+ CPUARMState *env)
+{
+ int i;
+
+ __put_user(TARGET_FPSIMD_MAGIC, &fpsimd->head.magic);
+ __put_user(sizeof(struct target_fpsimd_context), &fpsimd->head.size);
+ __put_user(vfp_get_fpsr(env), &fpsimd->fpsr);
+ __put_user(vfp_get_fpcr(env), &fpsimd->fpcr);
for (i = 0; i < 32; i++) {
uint64_t *q = aa64_vfp_qreg(env, i);
#ifdef TARGET_WORDS_BIGENDIAN
- __put_user(q[0], &aux->fpsimd.vregs[i * 2 + 1]);
- __put_user(q[1], &aux->fpsimd.vregs[i * 2]);
+ __put_user(q[0], &fpsimd->vregs[i * 2 + 1]);
+ __put_user(q[1], &fpsimd->vregs[i * 2]);
#else
- __put_user(q[0], &aux->fpsimd.vregs[i * 2]);
- __put_user(q[1], &aux->fpsimd.vregs[i * 2 + 1]);
+ __put_user(q[0], &fpsimd->vregs[i * 2]);
+ __put_user(q[1], &fpsimd->vregs[i * 2 + 1]);
#endif
}
- __put_user(vfp_get_fpsr(env), &aux->fpsimd.fpsr);
- __put_user(vfp_get_fpcr(env), &aux->fpsimd.fpcr);
- __put_user(TARGET_FPSIMD_MAGIC, &aux->fpsimd.head.magic);
- __put_user(sizeof(struct target_fpsimd_context),
- &aux->fpsimd.head.size);
-
- /* set the "end" magic */
- __put_user(0, &aux->end.magic);
- __put_user(0, &aux->end.size);
-
- return 0;
}
-static int target_restore_sigframe(CPUARMState *env,
- struct target_rt_sigframe *sf)
+static void target_setup_end_record(struct target_aarch64_ctx *end)
+{
+ __put_user(0, &end->magic);
+ __put_user(0, &end->size);
+}
+
+static void target_restore_general_frame(CPUARMState *env,
+ struct target_rt_sigframe *sf)
{
sigset_t set;
- int i;
- struct target_aux_context *aux =
- (struct target_aux_context *)sf->uc.tuc_mcontext.__reserved;
- uint32_t magic, size, fpsr, fpcr;
uint64_t pstate;
+ int i;
target_to_host_sigset(&set, &sf->uc.tuc_sigmask);
set_sigmask(&set);
@@ -1530,30 +1534,48 @@ static int target_restore_sigframe(CPUARMState *env,
__get_user(env->pc, &sf->uc.tuc_mcontext.pc);
__get_user(pstate, &sf->uc.tuc_mcontext.pstate);
pstate_write(env, pstate);
+}
- __get_user(magic, &aux->fpsimd.head.magic);
- __get_user(size, &aux->fpsimd.head.size);
+static void target_restore_fpsimd_record(CPUARMState *env,
+ struct target_fpsimd_context *fpsimd)
+{
+ uint32_t fpsr, fpcr;
+ int i;
- if (magic != TARGET_FPSIMD_MAGIC
- || size != sizeof(struct target_fpsimd_context)) {
- return 1;
- }
+ __get_user(fpsr, &fpsimd->fpsr);
+ vfp_set_fpsr(env, fpsr);
+ __get_user(fpcr, &fpsimd->fpcr);
+ vfp_set_fpcr(env, fpcr);
for (i = 0; i < 32; i++) {
uint64_t *q = aa64_vfp_qreg(env, i);
#ifdef TARGET_WORDS_BIGENDIAN
- __get_user(q[0], &aux->fpsimd.vregs[i * 2 + 1]);
- __get_user(q[1], &aux->fpsimd.vregs[i * 2]);
+ __get_user(q[0], &fpsimd->vregs[i * 2 + 1]);
+ __get_user(q[1], &fpsimd->vregs[i * 2]);
#else
- __get_user(q[0], &aux->fpsimd.vregs[i * 2]);
- __get_user(q[1], &aux->fpsimd.vregs[i * 2 + 1]);
+ __get_user(q[0], &fpsimd->vregs[i * 2]);
+ __get_user(q[1], &fpsimd->vregs[i * 2 + 1]);
#endif
}
- __get_user(fpsr, &aux->fpsimd.fpsr);
- vfp_set_fpsr(env, fpsr);
- __get_user(fpcr, &aux->fpsimd.fpcr);
- vfp_set_fpcr(env, fpcr);
+}
+static int target_restore_sigframe(CPUARMState *env,
+ struct target_rt_sigframe *sf)
+{
+ struct target_aux_context *aux
+ = (struct target_aux_context *)sf->uc.tuc_mcontext.__reserved;
+ uint32_t magic, size;
+
+ target_restore_general_frame(env, sf);
+
+ __get_user(magic, &aux->fpsimd.head.magic);
+ __get_user(size, &aux->fpsimd.head.size);
+ if (magic == TARGET_FPSIMD_MAGIC
+ && size == sizeof(struct target_fpsimd_context)) {
+ target_restore_fpsimd_record(env, &aux->fpsimd);
+ } else {
+ return 1;
+ }
return 0;
}
@@ -1580,6 +1602,7 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
CPUARMState *env)
{
struct target_rt_sigframe *frame;
+ struct target_aux_context *aux;
abi_ulong frame_addr, return_addr;
frame_addr = get_sigframe(ka, env);
@@ -1587,17 +1610,12 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
goto give_sigsegv;
}
+ aux = (struct target_aux_context *)frame->uc.tuc_mcontext.__reserved;
- __put_user(0, &frame->uc.tuc_flags);
- __put_user(0, &frame->uc.tuc_link);
+ target_setup_general_frame(frame, env, set);
+ target_setup_fpsimd_record(&aux->fpsimd, env);
+ target_setup_end_record(&aux->end);
- __put_user(target_sigaltstack_used.ss_sp,
- &frame->uc.tuc_stack.ss_sp);
- __put_user(sas_ss_flags(env->xregs[31]),
- &frame->uc.tuc_stack.ss_flags);
- __put_user(target_sigaltstack_used.ss_size,
- &frame->uc.tuc_stack.ss_size);
- target_setup_sigframe(frame, env, set);
if (ka->sa_flags & TARGET_SA_RESTORER) {
return_addr = ka->sa_restorer;
} else {
--
2.14.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/5] aarch64-linux-user: Split out helpers for guest signal handling
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 2/5] aarch64-linux-user: Split out helpers for guest signal handling Richard Henderson
@ 2018-03-06 14:11 ` Alex Bennée
0 siblings, 0 replies; 15+ messages in thread
From: Alex Bennée @ 2018-03-06 14:11 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
Richard Henderson <richard.henderson@linaro.org> writes:
> Split out helpers from target_setup_frame and target_restore_sigframe
> for dealing with general registers, fpsimd registers, and the end record.
>
> When we add support for sve registers, the relative positions of
> these will change.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> linux-user/signal.c | 120 ++++++++++++++++++++++++++++++----------------------
> 1 file changed, 69 insertions(+), 51 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 9a380b9e31..25c9743aed 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -1462,16 +1462,17 @@ struct target_rt_sigframe {
> uint32_t tramp[2];
> };
>
> -static int target_setup_sigframe(struct target_rt_sigframe *sf,
> - CPUARMState *env, target_sigset_t *set)
> +static void target_setup_general_frame(struct target_rt_sigframe *sf,
> + CPUARMState *env, target_sigset_t *set)
> {
> int i;
> - struct target_aux_context *aux =
> - (struct target_aux_context *)sf->uc.tuc_mcontext.__reserved;
>
> - /* set up the stack frame for unwinding */
> - __put_user(env->xregs[29], &sf->fp);
> - __put_user(env->xregs[30], &sf->lr);
> + __put_user(0, &sf->uc.tuc_flags);
> + __put_user(0, &sf->uc.tuc_link);
> +
> + __put_user(target_sigaltstack_used.ss_sp, &sf->uc.tuc_stack.ss_sp);
> + __put_user(sas_ss_flags(env->xregs[31]), &sf->uc.tuc_stack.ss_flags);
> + __put_user(target_sigaltstack_used.ss_size, &sf->uc.tuc_stack.ss_size);
>
> for (i = 0; i < 31; i++) {
> __put_user(env->xregs[i], &sf->uc.tuc_mcontext.regs[i]);
> @@ -1485,39 +1486,42 @@ static int target_setup_sigframe(struct target_rt_sigframe *sf,
> for (i = 0; i < TARGET_NSIG_WORDS; i++) {
> __put_user(set->sig[i], &sf->uc.tuc_sigmask.sig[i]);
> }
> +}
> +
> +static void target_setup_fpsimd_record(struct target_fpsimd_context *fpsimd,
> + CPUARMState *env)
> +{
> + int i;
> +
> + __put_user(TARGET_FPSIMD_MAGIC, &fpsimd->head.magic);
> + __put_user(sizeof(struct target_fpsimd_context), &fpsimd->head.size);
> + __put_user(vfp_get_fpsr(env), &fpsimd->fpsr);
> + __put_user(vfp_get_fpcr(env), &fpsimd->fpcr);
>
> for (i = 0; i < 32; i++) {
> uint64_t *q = aa64_vfp_qreg(env, i);
> #ifdef TARGET_WORDS_BIGENDIAN
> - __put_user(q[0], &aux->fpsimd.vregs[i * 2 + 1]);
> - __put_user(q[1], &aux->fpsimd.vregs[i * 2]);
> + __put_user(q[0], &fpsimd->vregs[i * 2 + 1]);
> + __put_user(q[1], &fpsimd->vregs[i * 2]);
> #else
> - __put_user(q[0], &aux->fpsimd.vregs[i * 2]);
> - __put_user(q[1], &aux->fpsimd.vregs[i * 2 + 1]);
> + __put_user(q[0], &fpsimd->vregs[i * 2]);
> + __put_user(q[1], &fpsimd->vregs[i * 2 + 1]);
> #endif
> }
> - __put_user(vfp_get_fpsr(env), &aux->fpsimd.fpsr);
> - __put_user(vfp_get_fpcr(env), &aux->fpsimd.fpcr);
> - __put_user(TARGET_FPSIMD_MAGIC, &aux->fpsimd.head.magic);
> - __put_user(sizeof(struct target_fpsimd_context),
> - &aux->fpsimd.head.size);
> -
> - /* set the "end" magic */
> - __put_user(0, &aux->end.magic);
> - __put_user(0, &aux->end.size);
> -
> - return 0;
> }
>
> -static int target_restore_sigframe(CPUARMState *env,
> - struct target_rt_sigframe *sf)
> +static void target_setup_end_record(struct target_aarch64_ctx *end)
> +{
> + __put_user(0, &end->magic);
> + __put_user(0, &end->size);
> +}
> +
> +static void target_restore_general_frame(CPUARMState *env,
> + struct target_rt_sigframe *sf)
> {
> sigset_t set;
> - int i;
> - struct target_aux_context *aux =
> - (struct target_aux_context *)sf->uc.tuc_mcontext.__reserved;
> - uint32_t magic, size, fpsr, fpcr;
> uint64_t pstate;
> + int i;
>
> target_to_host_sigset(&set, &sf->uc.tuc_sigmask);
> set_sigmask(&set);
> @@ -1530,30 +1534,48 @@ static int target_restore_sigframe(CPUARMState *env,
> __get_user(env->pc, &sf->uc.tuc_mcontext.pc);
> __get_user(pstate, &sf->uc.tuc_mcontext.pstate);
> pstate_write(env, pstate);
> +}
>
> - __get_user(magic, &aux->fpsimd.head.magic);
> - __get_user(size, &aux->fpsimd.head.size);
> +static void target_restore_fpsimd_record(CPUARMState *env,
> + struct target_fpsimd_context *fpsimd)
> +{
> + uint32_t fpsr, fpcr;
> + int i;
>
> - if (magic != TARGET_FPSIMD_MAGIC
> - || size != sizeof(struct target_fpsimd_context)) {
> - return 1;
> - }
> + __get_user(fpsr, &fpsimd->fpsr);
> + vfp_set_fpsr(env, fpsr);
> + __get_user(fpcr, &fpsimd->fpcr);
> + vfp_set_fpcr(env, fpcr);
>
> for (i = 0; i < 32; i++) {
> uint64_t *q = aa64_vfp_qreg(env, i);
> #ifdef TARGET_WORDS_BIGENDIAN
> - __get_user(q[0], &aux->fpsimd.vregs[i * 2 + 1]);
> - __get_user(q[1], &aux->fpsimd.vregs[i * 2]);
> + __get_user(q[0], &fpsimd->vregs[i * 2 + 1]);
> + __get_user(q[1], &fpsimd->vregs[i * 2]);
> #else
> - __get_user(q[0], &aux->fpsimd.vregs[i * 2]);
> - __get_user(q[1], &aux->fpsimd.vregs[i * 2 + 1]);
> + __get_user(q[0], &fpsimd->vregs[i * 2]);
> + __get_user(q[1], &fpsimd->vregs[i * 2 + 1]);
> #endif
> }
> - __get_user(fpsr, &aux->fpsimd.fpsr);
> - vfp_set_fpsr(env, fpsr);
> - __get_user(fpcr, &aux->fpsimd.fpcr);
> - vfp_set_fpcr(env, fpcr);
> +}
>
> +static int target_restore_sigframe(CPUARMState *env,
> + struct target_rt_sigframe *sf)
> +{
> + struct target_aux_context *aux
> + = (struct target_aux_context *)sf->uc.tuc_mcontext.__reserved;
> + uint32_t magic, size;
> +
> + target_restore_general_frame(env, sf);
> +
> + __get_user(magic, &aux->fpsimd.head.magic);
> + __get_user(size, &aux->fpsimd.head.size);
> + if (magic == TARGET_FPSIMD_MAGIC
> + && size == sizeof(struct target_fpsimd_context)) {
> + target_restore_fpsimd_record(env, &aux->fpsimd);
> + } else {
> + return 1;
> + }
> return 0;
> }
>
> @@ -1580,6 +1602,7 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
> CPUARMState *env)
> {
> struct target_rt_sigframe *frame;
> + struct target_aux_context *aux;
> abi_ulong frame_addr, return_addr;
>
> frame_addr = get_sigframe(ka, env);
> @@ -1587,17 +1610,12 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
> if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
> goto give_sigsegv;
> }
> + aux = (struct target_aux_context *)frame->uc.tuc_mcontext.__reserved;
>
> - __put_user(0, &frame->uc.tuc_flags);
> - __put_user(0, &frame->uc.tuc_link);
> + target_setup_general_frame(frame, env, set);
> + target_setup_fpsimd_record(&aux->fpsimd, env);
> + target_setup_end_record(&aux->end);
>
> - __put_user(target_sigaltstack_used.ss_sp,
> - &frame->uc.tuc_stack.ss_sp);
> - __put_user(sas_ss_flags(env->xregs[31]),
> - &frame->uc.tuc_stack.ss_flags);
> - __put_user(target_sigaltstack_used.ss_size,
> - &frame->uc.tuc_stack.ss_size);
> - target_setup_sigframe(frame, env, set);
> if (ka->sa_flags & TARGET_SA_RESTORER) {
> return_addr = ka->sa_restorer;
> } else {
--
Alex Bennée
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v4 3/5] aarch64-linux-user: Remove struct target_aux_context
2018-03-03 14:38 [Qemu-devel] [PATCH v4 0/5] target/arm linux-user changes for sve Richard Henderson
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 1/5] linux-user: Implement aarch64 PR_SVE_SET/GET_VL Richard Henderson
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 2/5] aarch64-linux-user: Split out helpers for guest signal handling Richard Henderson
@ 2018-03-03 14:38 ` Richard Henderson
2018-03-06 14:16 ` Alex Bennée
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 4/5] aarch64-linux-user: Add support for EXTRA signal frame records Richard Henderson
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Richard Henderson @ 2018-03-03 14:38 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
This changes the qemu signal frame layout to be more like the kernel's,
in that the various records are dynamically allocated rather than fixed
in place by a structure.
For now, all of the allocation is out of uc.tuc_mcontext.__reserved,
so the allocation is actually trivial. That will change with SVE support.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/signal.c | 89 ++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 61 insertions(+), 28 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 25c9743aed..f9eef3d753 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1443,20 +1443,12 @@ 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;
-};
-
struct target_rt_sigframe {
struct target_siginfo info;
struct target_ucontext uc;
+};
+
+struct target_rt_frame_record {
uint64_t fp;
uint64_t lr;
uint32_t tramp[2];
@@ -1562,20 +1554,47 @@ static void target_restore_fpsimd_record(CPUARMState *env,
static int target_restore_sigframe(CPUARMState *env,
struct target_rt_sigframe *sf)
{
- struct target_aux_context *aux
- = (struct target_aux_context *)sf->uc.tuc_mcontext.__reserved;
- uint32_t magic, size;
+ struct target_aarch64_ctx *ctx;
+ struct target_fpsimd_context *fpsimd = NULL;
target_restore_general_frame(env, sf);
- __get_user(magic, &aux->fpsimd.head.magic);
- __get_user(size, &aux->fpsimd.head.size);
- if (magic == TARGET_FPSIMD_MAGIC
- && size == sizeof(struct target_fpsimd_context)) {
- target_restore_fpsimd_record(env, &aux->fpsimd);
- } else {
+ ctx = (struct target_aarch64_ctx *)sf->uc.tuc_mcontext.__reserved;
+ while (ctx) {
+ uint32_t magic, size;
+
+ __get_user(magic, &ctx->magic);
+ __get_user(size, &ctx->size);
+ switch (magic) {
+ case 0:
+ if (size != 0) {
+ return 1;
+ }
+ ctx = NULL;
+ continue;
+
+ case TARGET_FPSIMD_MAGIC:
+ if (fpsimd || size != sizeof(struct target_fpsimd_context)) {
+ return 1;
+ }
+ fpsimd = (struct target_fpsimd_context *)ctx;
+ break;
+
+ default:
+ /* Unknown record -- we certainly didn't generate it.
+ * Did we in fact get out of sync?
+ */
+ return 1;
+ }
+ ctx = (void *)ctx + size;
+ }
+
+ /* Require FPSIMD always. */
+ if (!fpsimd) {
return 1;
}
+ target_restore_fpsimd_record(env, fpsimd);
+
return 0;
}
@@ -1601,20 +1620,33 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
target_siginfo_t *info, target_sigset_t *set,
CPUARMState *env)
{
+ int size = offsetof(struct target_rt_sigframe, uc.tuc_mcontext.__reserved);
+ int fpsimd_ofs, end1_ofs, fr_ofs;
struct target_rt_sigframe *frame;
- struct target_aux_context *aux;
+ struct target_rt_frame_record *fr;
abi_ulong frame_addr, return_addr;
+ fpsimd_ofs = size;
+ size += sizeof(struct target_fpsimd_context);
+ end1_ofs = size;
+ size += sizeof(struct target_aarch64_ctx);
+ fr_ofs = size;
+ size += sizeof(struct target_rt_frame_record);
+
frame_addr = get_sigframe(ka, env);
trace_user_setup_frame(env, frame_addr);
if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
goto give_sigsegv;
}
- aux = (struct target_aux_context *)frame->uc.tuc_mcontext.__reserved;
target_setup_general_frame(frame, env, set);
- target_setup_fpsimd_record(&aux->fpsimd, env);
- target_setup_end_record(&aux->end);
+ target_setup_fpsimd_record((void *)frame + fpsimd_ofs, env);
+ target_setup_end_record((void *)frame + end1_ofs);
+
+ /* Set up the stack frame for unwinding. */
+ fr = (void *)frame + fr_ofs;
+ __put_user(env->xregs[29], &fr->fp);
+ __put_user(env->xregs[30], &fr->lr);
if (ka->sa_flags & TARGET_SA_RESTORER) {
return_addr = ka->sa_restorer;
@@ -1624,13 +1656,14 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
* Since these are instructions they need to be put as little-endian
* regardless of target default or current CPU endianness.
*/
- __put_user_e(0xd2801168, &frame->tramp[0], le);
- __put_user_e(0xd4000001, &frame->tramp[1], le);
- return_addr = frame_addr + offsetof(struct target_rt_sigframe, tramp);
+ __put_user_e(0xd2801168, &fr->tramp[0], le);
+ __put_user_e(0xd4000001, &fr->tramp[1], le);
+ return_addr = frame_addr + fr_ofs
+ + offsetof(struct target_rt_frame_record, tramp);
}
env->xregs[0] = usig;
env->xregs[31] = frame_addr;
- env->xregs[29] = env->xregs[31] + offsetof(struct target_rt_sigframe, fp);
+ env->xregs[29] = frame_addr + fr_ofs;
env->pc = ka->_sa_handler;
env->xregs[30] = return_addr;
if (info) {
--
2.14.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/5] aarch64-linux-user: Remove struct target_aux_context
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 3/5] aarch64-linux-user: Remove struct target_aux_context Richard Henderson
@ 2018-03-06 14:16 ` Alex Bennée
0 siblings, 0 replies; 15+ messages in thread
From: Alex Bennée @ 2018-03-06 14:16 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
Richard Henderson <richard.henderson@linaro.org> writes:
> This changes the qemu signal frame layout to be more like the kernel's,
> in that the various records are dynamically allocated rather than fixed
> in place by a structure.
>
> For now, all of the allocation is out of uc.tuc_mcontext.__reserved,
> so the allocation is actually trivial. That will change with SVE support.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> linux-user/signal.c | 89 ++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 61 insertions(+), 28 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 25c9743aed..f9eef3d753 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -1443,20 +1443,12 @@ 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;
> -};
> -
> struct target_rt_sigframe {
> struct target_siginfo info;
> struct target_ucontext uc;
> +};
> +
> +struct target_rt_frame_record {
> uint64_t fp;
> uint64_t lr;
> uint32_t tramp[2];
> @@ -1562,20 +1554,47 @@ static void target_restore_fpsimd_record(CPUARMState *env,
> static int target_restore_sigframe(CPUARMState *env,
> struct target_rt_sigframe *sf)
> {
> - struct target_aux_context *aux
> - = (struct target_aux_context *)sf->uc.tuc_mcontext.__reserved;
> - uint32_t magic, size;
> + struct target_aarch64_ctx *ctx;
> + struct target_fpsimd_context *fpsimd = NULL;
>
> target_restore_general_frame(env, sf);
>
> - __get_user(magic, &aux->fpsimd.head.magic);
> - __get_user(size, &aux->fpsimd.head.size);
> - if (magic == TARGET_FPSIMD_MAGIC
> - && size == sizeof(struct target_fpsimd_context)) {
> - target_restore_fpsimd_record(env, &aux->fpsimd);
> - } else {
> + ctx = (struct target_aarch64_ctx *)sf->uc.tuc_mcontext.__reserved;
> + while (ctx) {
> + uint32_t magic, size;
> +
> + __get_user(magic, &ctx->magic);
> + __get_user(size, &ctx->size);
> + switch (magic) {
> + case 0:
> + if (size != 0) {
> + return 1;
> + }
> + ctx = NULL;
> + continue;
> +
> + case TARGET_FPSIMD_MAGIC:
> + if (fpsimd || size != sizeof(struct target_fpsimd_context)) {
> + return 1;
> + }
> + fpsimd = (struct target_fpsimd_context *)ctx;
> + break;
> +
> + default:
> + /* Unknown record -- we certainly didn't generate it.
> + * Did we in fact get out of sync?
> + */
> + return 1;
> + }
> + ctx = (void *)ctx + size;
> + }
> +
> + /* Require FPSIMD always. */
> + if (!fpsimd) {
> return 1;
> }
> + target_restore_fpsimd_record(env, fpsimd);
> +
> return 0;
> }
>
> @@ -1601,20 +1620,33 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
> target_siginfo_t *info, target_sigset_t *set,
> CPUARMState *env)
> {
> + int size = offsetof(struct target_rt_sigframe, uc.tuc_mcontext.__reserved);
> + int fpsimd_ofs, end1_ofs, fr_ofs;
> struct target_rt_sigframe *frame;
> - struct target_aux_context *aux;
> + struct target_rt_frame_record *fr;
> abi_ulong frame_addr, return_addr;
>
> + fpsimd_ofs = size;
> + size += sizeof(struct target_fpsimd_context);
> + end1_ofs = size;
> + size += sizeof(struct target_aarch64_ctx);
> + fr_ofs = size;
> + size += sizeof(struct target_rt_frame_record);
> +
> frame_addr = get_sigframe(ka, env);
> trace_user_setup_frame(env, frame_addr);
> if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
> goto give_sigsegv;
> }
> - aux = (struct target_aux_context *)frame->uc.tuc_mcontext.__reserved;
>
> target_setup_general_frame(frame, env, set);
> - target_setup_fpsimd_record(&aux->fpsimd, env);
> - target_setup_end_record(&aux->end);
> + target_setup_fpsimd_record((void *)frame + fpsimd_ofs, env);
> + target_setup_end_record((void *)frame + end1_ofs);
> +
> + /* Set up the stack frame for unwinding. */
> + fr = (void *)frame + fr_ofs;
> + __put_user(env->xregs[29], &fr->fp);
> + __put_user(env->xregs[30], &fr->lr);
>
> if (ka->sa_flags & TARGET_SA_RESTORER) {
> return_addr = ka->sa_restorer;
> @@ -1624,13 +1656,14 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
> * Since these are instructions they need to be put as little-endian
> * regardless of target default or current CPU endianness.
> */
> - __put_user_e(0xd2801168, &frame->tramp[0], le);
> - __put_user_e(0xd4000001, &frame->tramp[1], le);
> - return_addr = frame_addr + offsetof(struct target_rt_sigframe, tramp);
> + __put_user_e(0xd2801168, &fr->tramp[0], le);
> + __put_user_e(0xd4000001, &fr->tramp[1], le);
> + return_addr = frame_addr + fr_ofs
> + + offsetof(struct target_rt_frame_record, tramp);
> }
> env->xregs[0] = usig;
> env->xregs[31] = frame_addr;
> - env->xregs[29] = env->xregs[31] + offsetof(struct target_rt_sigframe, fp);
> + env->xregs[29] = frame_addr + fr_ofs;
> env->pc = ka->_sa_handler;
> env->xregs[30] = return_addr;
> if (info) {
--
Alex Bennée
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v4 4/5] aarch64-linux-user: Add support for EXTRA signal frame records
2018-03-03 14:38 [Qemu-devel] [PATCH v4 0/5] target/arm linux-user changes for sve Richard Henderson
` (2 preceding siblings ...)
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 3/5] aarch64-linux-user: Remove struct target_aux_context Richard Henderson
@ 2018-03-03 14:38 ` Richard Henderson
2018-03-06 14:26 ` [Qemu-devel] [Qemu-arm] " Alex Bennée
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 5/5] aarch64-linux-user: Add support for SVE " Richard Henderson
2018-03-05 15:42 ` [Qemu-devel] [PATCH v4 0/5] target/arm linux-user changes for sve Peter Maydell
5 siblings, 1 reply; 15+ messages in thread
From: Richard Henderson @ 2018-03-03 14:38 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
The EXTRA record allows for additional space to be allocated
beyon what is currently reserved. Add code to emit and read
this record type.
Nothing uses extra space yet.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/signal.c | 74 +++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 63 insertions(+), 11 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index f9eef3d753..c31cf0601d 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1443,6 +1443,15 @@ struct target_fpsimd_context {
uint64_t vregs[32 * 2]; /* really uint128_t vregs[32] */
};
+#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];
+};
+
struct target_rt_sigframe {
struct target_siginfo info;
struct target_ucontext uc;
@@ -1502,6 +1511,15 @@ static void target_setup_fpsimd_record(struct target_fpsimd_context *fpsimd,
}
}
+static void target_setup_extra_record(struct target_extra_context *extra,
+ uint64_t datap, uint32_t extra_size)
+{
+ __put_user(TARGET_EXTRA_MAGIC, &extra->head.magic);
+ __put_user(sizeof(struct target_extra_context), &extra->head.size);
+ __put_user(datap, &extra->datap);
+ __put_user(extra_size, &extra->size);
+}
+
static void target_setup_end_record(struct target_aarch64_ctx *end)
{
__put_user(0, &end->magic);
@@ -1554,48 +1572,74 @@ static void target_restore_fpsimd_record(CPUARMState *env,
static int target_restore_sigframe(CPUARMState *env,
struct target_rt_sigframe *sf)
{
- struct target_aarch64_ctx *ctx;
+ struct target_aarch64_ctx *ctx, *extra = NULL;
struct target_fpsimd_context *fpsimd = NULL;
+ uint64_t extra_datap = 0;
+ bool used_extra = false;
+ bool err = false;
target_restore_general_frame(env, sf);
ctx = (struct target_aarch64_ctx *)sf->uc.tuc_mcontext.__reserved;
while (ctx) {
- uint32_t magic, size;
+ uint32_t magic, size, extra_size;
__get_user(magic, &ctx->magic);
__get_user(size, &ctx->size);
switch (magic) {
case 0:
if (size != 0) {
- return 1;
+ err = true;
+ goto exit;
+ }
+ if (used_extra) {
+ ctx = NULL;
+ } else {
+ ctx = extra;
+ used_extra = true;
}
- ctx = NULL;
continue;
case TARGET_FPSIMD_MAGIC:
if (fpsimd || size != sizeof(struct target_fpsimd_context)) {
- return 1;
+ err = true;
+ goto exit;
}
fpsimd = (struct target_fpsimd_context *)ctx;
break;
+ case TARGET_EXTRA_MAGIC:
+ if (extra || size != sizeof(struct target_extra_context)) {
+ err = true;
+ goto exit;
+ }
+ __get_user(extra_datap,
+ &((struct target_extra_context *)ctx)->datap);
+ __get_user(extra_size,
+ &((struct target_extra_context *)ctx)->size);
+ extra = lock_user(VERIFY_READ, extra_datap, extra_size, 0);
+ break;
+
default:
/* Unknown record -- we certainly didn't generate it.
* Did we in fact get out of sync?
*/
- return 1;
+ err = true;
+ goto exit;
}
ctx = (void *)ctx + size;
}
/* Require FPSIMD always. */
- if (!fpsimd) {
- return 1;
+ if (fpsimd) {
+ target_restore_fpsimd_record(env, fpsimd);
+ } else {
+ err = true;
}
- target_restore_fpsimd_record(env, fpsimd);
- return 0;
+ exit:
+ unlock_user(extra, extra_datap, 0);
+ return err;
}
static abi_ulong get_sigframe(struct target_sigaction *ka, CPUARMState *env)
@@ -1621,7 +1665,8 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
CPUARMState *env)
{
int size = offsetof(struct target_rt_sigframe, uc.tuc_mcontext.__reserved);
- int fpsimd_ofs, end1_ofs, fr_ofs;
+ int fpsimd_ofs, end1_ofs, fr_ofs, end2_ofs = 0;
+ int extra_ofs = 0, extra_base = 0, extra_size = 0;
struct target_rt_sigframe *frame;
struct target_rt_frame_record *fr;
abi_ulong frame_addr, return_addr;
@@ -1641,7 +1686,14 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
target_setup_general_frame(frame, env, set);
target_setup_fpsimd_record((void *)frame + fpsimd_ofs, env);
+ if (extra_ofs) {
+ target_setup_extra_record((void *)frame + extra_ofs,
+ frame_addr + extra_base, extra_size);
+ }
target_setup_end_record((void *)frame + end1_ofs);
+ if (end2_ofs) {
+ target_setup_end_record((void *)frame + end2_ofs);
+ }
/* Set up the stack frame for unwinding. */
fr = (void *)frame + fr_ofs;
--
2.14.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v4 4/5] aarch64-linux-user: Add support for EXTRA signal frame records
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 4/5] aarch64-linux-user: Add support for EXTRA signal frame records Richard Henderson
@ 2018-03-06 14:26 ` Alex Bennée
0 siblings, 0 replies; 15+ messages in thread
From: Alex Bennée @ 2018-03-06 14:26 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
Richard Henderson <richard.henderson@linaro.org> writes:
> The EXTRA record allows for additional space to be allocated
> beyon what is currently reserved. Add code to emit and read
> this record type.
>
> Nothing uses extra space yet.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> linux-user/signal.c | 74 +++++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 63 insertions(+), 11 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index f9eef3d753..c31cf0601d 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -1443,6 +1443,15 @@ struct target_fpsimd_context {
> uint64_t vregs[32 * 2]; /* really uint128_t vregs[32] */
> };
>
> +#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];
> +};
> +
> struct target_rt_sigframe {
> struct target_siginfo info;
> struct target_ucontext uc;
> @@ -1502,6 +1511,15 @@ static void target_setup_fpsimd_record(struct target_fpsimd_context *fpsimd,
> }
> }
>
> +static void target_setup_extra_record(struct target_extra_context *extra,
> + uint64_t datap, uint32_t extra_size)
> +{
> + __put_user(TARGET_EXTRA_MAGIC, &extra->head.magic);
> + __put_user(sizeof(struct target_extra_context), &extra->head.size);
> + __put_user(datap, &extra->datap);
> + __put_user(extra_size, &extra->size);
> +}
> +
> static void target_setup_end_record(struct target_aarch64_ctx *end)
> {
> __put_user(0, &end->magic);
> @@ -1554,48 +1572,74 @@ static void target_restore_fpsimd_record(CPUARMState *env,
> static int target_restore_sigframe(CPUARMState *env,
> struct target_rt_sigframe *sf)
> {
> - struct target_aarch64_ctx *ctx;
> + struct target_aarch64_ctx *ctx, *extra = NULL;
> struct target_fpsimd_context *fpsimd = NULL;
> + uint64_t extra_datap = 0;
> + bool used_extra = false;
> + bool err = false;
>
> target_restore_general_frame(env, sf);
>
> ctx = (struct target_aarch64_ctx *)sf->uc.tuc_mcontext.__reserved;
> while (ctx) {
> - uint32_t magic, size;
> + uint32_t magic, size, extra_size;
>
> __get_user(magic, &ctx->magic);
> __get_user(size, &ctx->size);
> switch (magic) {
> case 0:
> if (size != 0) {
> - return 1;
> + err = true;
> + goto exit;
> + }
> + if (used_extra) {
> + ctx = NULL;
> + } else {
> + ctx = extra;
> + used_extra = true;
> }
> - ctx = NULL;
> continue;
>
> case TARGET_FPSIMD_MAGIC:
> if (fpsimd || size != sizeof(struct target_fpsimd_context)) {
> - return 1;
> + err = true;
> + goto exit;
> }
> fpsimd = (struct target_fpsimd_context *)ctx;
> break;
>
> + case TARGET_EXTRA_MAGIC:
> + if (extra || size != sizeof(struct target_extra_context)) {
> + err = true;
> + goto exit;
> + }
> + __get_user(extra_datap,
> + &((struct target_extra_context *)ctx)->datap);
> + __get_user(extra_size,
> + &((struct target_extra_context *)ctx)->size);
> + extra = lock_user(VERIFY_READ, extra_datap, extra_size, 0);
> + break;
> +
> default:
> /* Unknown record -- we certainly didn't generate it.
> * Did we in fact get out of sync?
> */
> - return 1;
> + err = true;
> + goto exit;
> }
> ctx = (void *)ctx + size;
> }
>
> /* Require FPSIMD always. */
> - if (!fpsimd) {
> - return 1;
> + if (fpsimd) {
> + target_restore_fpsimd_record(env, fpsimd);
> + } else {
> + err = true;
> }
> - target_restore_fpsimd_record(env, fpsimd);
>
> - return 0;
> + exit:
> + unlock_user(extra, extra_datap, 0);
> + return err;
> }
>
> static abi_ulong get_sigframe(struct target_sigaction *ka, CPUARMState *env)
> @@ -1621,7 +1665,8 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
> CPUARMState *env)
> {
> int size = offsetof(struct target_rt_sigframe, uc.tuc_mcontext.__reserved);
> - int fpsimd_ofs, end1_ofs, fr_ofs;
> + int fpsimd_ofs, end1_ofs, fr_ofs, end2_ofs = 0;
> + int extra_ofs = 0, extra_base = 0, extra_size = 0;
> struct target_rt_sigframe *frame;
> struct target_rt_frame_record *fr;
> abi_ulong frame_addr, return_addr;
> @@ -1641,7 +1686,14 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
>
> target_setup_general_frame(frame, env, set);
> target_setup_fpsimd_record((void *)frame + fpsimd_ofs, env);
> + if (extra_ofs) {
> + target_setup_extra_record((void *)frame + extra_ofs,
> + frame_addr + extra_base, extra_size);
> + }
> target_setup_end_record((void *)frame + end1_ofs);
> + if (end2_ofs) {
> + target_setup_end_record((void *)frame + end2_ofs);
> + }
>
> /* Set up the stack frame for unwinding. */
> fr = (void *)frame + fr_ofs;
--
Alex Bennée
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v4 5/5] aarch64-linux-user: Add support for SVE signal frame records
2018-03-03 14:38 [Qemu-devel] [PATCH v4 0/5] target/arm linux-user changes for sve Richard Henderson
` (3 preceding siblings ...)
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 4/5] aarch64-linux-user: Add support for EXTRA signal frame records Richard Henderson
@ 2018-03-03 14:38 ` Richard Henderson
2018-03-05 15:44 ` [Qemu-devel] [Qemu-arm] " Peter Maydell
2018-03-06 14:27 ` Alex Bennée
2018-03-05 15:42 ` [Qemu-devel] [PATCH v4 0/5] target/arm linux-user changes for sve Peter Maydell
5 siblings, 2 replies; 15+ messages in thread
From: Richard Henderson @ 2018-03-03 14:38 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Depending on the currently selected size of the SVE vector registers,
we can either store the data within the "standard" allocation, or we
may beedn to allocate additional space with an EXTRA record.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/signal.c | 210 +++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 192 insertions(+), 18 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index c31cf0601d..92513f655c 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1452,6 +1452,34 @@ struct target_extra_context {
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];
+ /* The actual SVE data immediately follows. It is layed out
+ * according to TARGET_SVE_SIG_{Z,P}REG_OFFSET, based off of
+ * the original struct pointer.
+ */
+};
+
+#define TARGET_SVE_VQ_BYTES 16
+
+#define TARGET_SVE_SIG_ZREG_SIZE(VQ) ((VQ) * TARGET_SVE_VQ_BYTES)
+#define TARGET_SVE_SIG_PREG_SIZE(VQ) ((VQ) * (TARGET_SVE_VQ_BYTES / 8))
+
+#define TARGET_SVE_SIG_REGS_OFFSET \
+ QEMU_ALIGN_UP(sizeof(struct target_sve_context), TARGET_SVE_VQ_BYTES)
+#define TARGET_SVE_SIG_ZREG_OFFSET(VQ, N) \
+ (TARGET_SVE_SIG_REGS_OFFSET + TARGET_SVE_SIG_ZREG_SIZE(VQ) * (N))
+#define TARGET_SVE_SIG_PREG_OFFSET(VQ, N) \
+ (TARGET_SVE_SIG_ZREG_OFFSET(VQ, 32) + TARGET_SVE_SIG_PREG_SIZE(VQ) * (N))
+#define TARGET_SVE_SIG_FFR_OFFSET(VQ) \
+ (TARGET_SVE_SIG_PREG_OFFSET(VQ, 16))
+#define TARGET_SVE_SIG_CONTEXT_SIZE(VQ) \
+ (TARGET_SVE_SIG_PREG_OFFSET(VQ, 17))
+
struct target_rt_sigframe {
struct target_siginfo info;
struct target_ucontext uc;
@@ -1526,6 +1554,34 @@ static void target_setup_end_record(struct target_aarch64_ctx *end)
__put_user(0, &end->size);
}
+static void target_setup_sve_record(struct target_sve_context *sve,
+ CPUARMState *env, int vq, int size)
+{
+ int i, j;
+
+ __put_user(TARGET_SVE_MAGIC, &sve->head.magic);
+ __put_user(size, &sve->head.size);
+ __put_user(vq * TARGET_SVE_VQ_BYTES, &sve->vl);
+
+ /* Note that SVE regs are stored as a byte stream, with each byte element
+ * at a subsequent address. This corresponds to a little-endian store
+ * of our 64-bit hunks.
+ */
+ for (i = 0; i < 32; ++i) {
+ uint64_t *z = (void *)sve + TARGET_SVE_SIG_ZREG_OFFSET(vq, i);
+ for (j = 0; j < vq * 2; ++j) {
+ __put_user_e(env->vfp.zregs[i].d[j], z + j, le);
+ }
+ }
+ for (i = 0; i <= 16; ++i) {
+ uint16_t *p = (void *)sve + TARGET_SVE_SIG_PREG_OFFSET(vq, i);
+ for (j = 0; j < vq; ++j) {
+ uint64_t r = env->vfp.pregs[i].p[j >> 2];
+ __put_user_e(r >> ((j & 3) * 16), p + j, le);
+ }
+ }
+}
+
static void target_restore_general_frame(CPUARMState *env,
struct target_rt_sigframe *sf)
{
@@ -1569,14 +1625,45 @@ static void target_restore_fpsimd_record(CPUARMState *env,
}
}
+static void target_restore_sve_record(CPUARMState *env,
+ struct target_sve_context *sve, int vq)
+{
+ int i, j;
+
+ /* Note that SVE regs are stored as a byte stream, with each byte element
+ * at a subsequent address. This corresponds to a little-endian load
+ * of our 64-bit hunks.
+ */
+ for (i = 0; i < 32; ++i) {
+ uint64_t *z = (void *)sve + TARGET_SVE_SIG_ZREG_OFFSET(vq, i);
+ for (j = 0; j < vq * 2; ++j) {
+ __get_user_e(env->vfp.zregs[i].d[j], z + j, le);
+ }
+ }
+ for (i = 0; i <= 16; ++i) {
+ uint16_t *p = (void *)sve + TARGET_SVE_SIG_PREG_OFFSET(vq, i);
+ for (j = 0; j < vq; ++j) {
+ uint16_t r;
+ __get_user_e(r, p + j, le);
+ if (j & 3) {
+ env->vfp.pregs[i].p[j >> 2] |= (uint64_t)r << ((j & 3) * 16);
+ } else {
+ env->vfp.pregs[i].p[j >> 2] = r;
+ }
+ }
+ }
+}
+
static int target_restore_sigframe(CPUARMState *env,
struct target_rt_sigframe *sf)
{
struct target_aarch64_ctx *ctx, *extra = NULL;
struct target_fpsimd_context *fpsimd = NULL;
+ struct target_sve_context *sve = NULL;
uint64_t extra_datap = 0;
bool used_extra = false;
bool err = false;
+ int vq = 0, sve_size = 0;
target_restore_general_frame(env, sf);
@@ -1608,6 +1695,18 @@ static int target_restore_sigframe(CPUARMState *env,
fpsimd = (struct target_fpsimd_context *)ctx;
break;
+ case TARGET_SVE_MAGIC:
+ if (arm_feature(env, ARM_FEATURE_SVE)) {
+ vq = (env->vfp.zcr_el[1] & 0xf) + 1;
+ sve_size = QEMU_ALIGN_UP(TARGET_SVE_SIG_CONTEXT_SIZE(vq), 16);
+ if (!sve && size == sve_size) {
+ sve = (struct target_sve_context *)ctx;
+ break;
+ }
+ }
+ err = true;
+ goto exit;
+
case TARGET_EXTRA_MAGIC:
if (extra || size != sizeof(struct target_extra_context)) {
err = true;
@@ -1637,12 +1736,18 @@ static int target_restore_sigframe(CPUARMState *env,
err = true;
}
+ /* SVE data, if present, overwrites FPSIMD data. */
+ if (sve) {
+ target_restore_sve_record(env, sve, vq);
+ }
+
exit:
unlock_user(extra, extra_datap, 0);
return err;
}
-static abi_ulong get_sigframe(struct target_sigaction *ka, CPUARMState *env)
+static abi_ulong get_sigframe(struct target_sigaction *ka,
+ CPUARMState *env, int size)
{
abi_ulong sp;
@@ -1655,30 +1760,97 @@ static abi_ulong get_sigframe(struct target_sigaction *ka, CPUARMState *env)
sp = target_sigaltstack_used.ss_sp + target_sigaltstack_used.ss_size;
}
- sp = (sp - sizeof(struct target_rt_sigframe)) & ~15;
+ sp = (sp - size) & ~15;
return sp;
}
+typedef struct {
+ int total_size;
+ int extra_base;
+ int extra_size;
+ int std_end_ofs;
+ int extra_ofs;
+ int extra_end_ofs;
+} target_sigframe_layout;
+
+static int alloc_sigframe_space(int this_size, target_sigframe_layout *l)
+{
+ /* Make sure there will always be space for the end marker. */
+ const int std_size = sizeof(struct target_rt_sigframe)
+ - sizeof(struct target_aarch64_ctx);
+ int this_loc = l->total_size;
+
+ if (l->extra_base) {
+ /* Once we have begun an extra space, all allocations go there. */
+ l->extra_size += this_size;
+ } else if (this_size + this_loc > std_size) {
+ /* This allocation does not fit in the standard space. */
+ /* Allocate the extra record. */
+ l->extra_ofs = this_loc;
+ l->total_size += sizeof(struct target_extra_context);
+
+ /* Allocate the standard end record. */
+ l->std_end_ofs = l->total_size;
+ l->total_size += sizeof(struct target_aarch64_ctx);
+
+ /* Allocate the requested record. */
+ l->extra_base = this_loc = l->total_size;
+ l->extra_size = this_size;
+ }
+ l->total_size += this_size;
+
+ return this_loc;
+}
+
static void target_setup_frame(int usig, struct target_sigaction *ka,
target_siginfo_t *info, target_sigset_t *set,
CPUARMState *env)
{
- int size = offsetof(struct target_rt_sigframe, uc.tuc_mcontext.__reserved);
- int fpsimd_ofs, end1_ofs, fr_ofs, end2_ofs = 0;
- int extra_ofs = 0, extra_base = 0, extra_size = 0;
+ target_sigframe_layout layout = {
+ /* Begin with the size pointing to the reserved space. */
+ .total_size = offsetof(struct target_rt_sigframe,
+ uc.tuc_mcontext.__reserved),
+ };
+ int fpsimd_ofs, fr_ofs, sve_ofs = 0, vq = 0, sve_size = 0;
struct target_rt_sigframe *frame;
struct target_rt_frame_record *fr;
abi_ulong frame_addr, return_addr;
- fpsimd_ofs = size;
- size += sizeof(struct target_fpsimd_context);
- end1_ofs = size;
- size += sizeof(struct target_aarch64_ctx);
- fr_ofs = size;
- size += sizeof(struct target_rt_frame_record);
+ /* FPSIMD record is always in the standard space. */
+ fpsimd_ofs = alloc_sigframe_space(sizeof(struct target_fpsimd_context),
+ &layout);
- frame_addr = get_sigframe(ka, env);
+ /* SVE state needs saving only if it exists. */
+ if (arm_feature(env, ARM_FEATURE_SVE)) {
+ vq = (env->vfp.zcr_el[1] & 0xf) + 1;
+ sve_size = QEMU_ALIGN_UP(TARGET_SVE_SIG_CONTEXT_SIZE(vq), 16);
+ sve_ofs = alloc_sigframe_space(sve_size, &layout);
+ }
+
+ if (layout.extra_ofs) {
+ /* Reserve space for the extra end marker. The standard end marker
+ * will have been allocated when we allocated the extra record.
+ */
+ layout.extra_end_ofs
+ = alloc_sigframe_space(sizeof(struct target_aarch64_ctx), &layout);
+ } else {
+ /* Reserve space for the standard end marker.
+ * Do not use alloc_sigframe_space because we cheat
+ * std_size therein to reserve space for this.
+ */
+ layout.std_end_ofs = layout.total_size;
+ layout.total_size += sizeof(struct target_aarch64_ctx);
+ }
+
+ /* Reserve space for the return code. On a real system this would
+ * be within the VDSO. So, despite the name this is not a "real"
+ * record within the frame.
+ */
+ fr_ofs = layout.total_size;
+ layout.total_size += sizeof(struct target_rt_frame_record);
+
+ frame_addr = get_sigframe(ka, env, layout.total_size);
trace_user_setup_frame(env, frame_addr);
if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
goto give_sigsegv;
@@ -1686,13 +1858,15 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
target_setup_general_frame(frame, env, set);
target_setup_fpsimd_record((void *)frame + fpsimd_ofs, env);
- if (extra_ofs) {
- target_setup_extra_record((void *)frame + extra_ofs,
- frame_addr + extra_base, extra_size);
+ target_setup_end_record((void *)frame + layout.std_end_ofs);
+ if (layout.extra_ofs) {
+ target_setup_extra_record((void *)frame + layout.extra_ofs,
+ frame_addr + layout.extra_base,
+ layout.extra_size);
+ target_setup_end_record((void *)frame + layout.extra_end_ofs);
}
- target_setup_end_record((void *)frame + end1_ofs);
- if (end2_ofs) {
- target_setup_end_record((void *)frame + end2_ofs);
+ if (sve_ofs) {
+ target_setup_sve_record((void *)frame + sve_ofs, env, vq, sve_size);
}
/* Set up the stack frame for unwinding. */
--
2.14.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v4 5/5] aarch64-linux-user: Add support for SVE signal frame records
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 5/5] aarch64-linux-user: Add support for SVE " Richard Henderson
@ 2018-03-05 15:44 ` Peter Maydell
2018-03-06 14:27 ` Alex Bennée
1 sibling, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2018-03-05 15:44 UTC (permalink / raw)
To: Richard Henderson; +Cc: QEMU Developers, qemu-arm
On 3 March 2018 at 14:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
> Depending on the currently selected size of the SVE vector registers,
> we can either store the data within the "standard" allocation, or we
> may beedn to allocate additional space with an EXTRA record.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/signal.c | 210 +++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 192 insertions(+), 18 deletions(-)
> +static int alloc_sigframe_space(int this_size, target_sigframe_layout *l)
> +{
> + /* Make sure there will always be space for the end marker. */
> + const int std_size = sizeof(struct target_rt_sigframe)
> + - sizeof(struct target_aarch64_ctx);
> + int this_loc = l->total_size;
> +
> + if (l->extra_base) {
> + /* Once we have begun an extra space, all allocations go there. */
> + l->extra_size += this_size;
> + } else if (this_size + this_loc > std_size) {
> + /* This allocation does not fit in the standard space. */
I think strictly this runs into trouble if it lets us put something
in the standard space that runs right up to the end marker and
doesn't leave space for the extra marker, and then the next thing
can't put in the extra marker. But this won't happen in practice,
so I don't think we really need to try to handle that.
> + /* Allocate the extra record. */
> + l->extra_ofs = this_loc;
> + l->total_size += sizeof(struct target_extra_context);
> +
> + /* Allocate the standard end record. */
> + l->std_end_ofs = l->total_size;
> + l->total_size += sizeof(struct target_aarch64_ctx);
> +
> + /* Allocate the requested record. */
> + l->extra_base = this_loc = l->total_size;
> + l->extra_size = this_size;
> + }
> + l->total_size += this_size;
> +
> + return this_loc;
> +}
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v4 5/5] aarch64-linux-user: Add support for SVE signal frame records
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 5/5] aarch64-linux-user: Add support for SVE " Richard Henderson
2018-03-05 15:44 ` [Qemu-devel] [Qemu-arm] " Peter Maydell
@ 2018-03-06 14:27 ` Alex Bennée
1 sibling, 0 replies; 15+ messages in thread
From: Alex Bennée @ 2018-03-06 14:27 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
Richard Henderson <richard.henderson@linaro.org> writes:
> Depending on the currently selected size of the SVE vector registers,
> we can either store the data within the "standard" allocation, or we
> may beedn to allocate additional space with an EXTRA record.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/signal.c | 210 +++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 192 insertions(+), 18 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index c31cf0601d..92513f655c 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -1452,6 +1452,34 @@ struct target_extra_context {
> 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];
> + /* The actual SVE data immediately follows. It is layed out
> + * according to TARGET_SVE_SIG_{Z,P}REG_OFFSET, based off of
> + * the original struct pointer.
> + */
> +};
> +
> +#define TARGET_SVE_VQ_BYTES 16
Ahh there it is. I guess if we do a later clean-up we can use this
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> +
> +#define TARGET_SVE_SIG_ZREG_SIZE(VQ) ((VQ) * TARGET_SVE_VQ_BYTES)
> +#define TARGET_SVE_SIG_PREG_SIZE(VQ) ((VQ) * (TARGET_SVE_VQ_BYTES / 8))
> +
> +#define TARGET_SVE_SIG_REGS_OFFSET \
> + QEMU_ALIGN_UP(sizeof(struct target_sve_context), TARGET_SVE_VQ_BYTES)
> +#define TARGET_SVE_SIG_ZREG_OFFSET(VQ, N) \
> + (TARGET_SVE_SIG_REGS_OFFSET + TARGET_SVE_SIG_ZREG_SIZE(VQ) * (N))
> +#define TARGET_SVE_SIG_PREG_OFFSET(VQ, N) \
> + (TARGET_SVE_SIG_ZREG_OFFSET(VQ, 32) + TARGET_SVE_SIG_PREG_SIZE(VQ) * (N))
> +#define TARGET_SVE_SIG_FFR_OFFSET(VQ) \
> + (TARGET_SVE_SIG_PREG_OFFSET(VQ, 16))
> +#define TARGET_SVE_SIG_CONTEXT_SIZE(VQ) \
> + (TARGET_SVE_SIG_PREG_OFFSET(VQ, 17))
> +
> struct target_rt_sigframe {
> struct target_siginfo info;
> struct target_ucontext uc;
> @@ -1526,6 +1554,34 @@ static void target_setup_end_record(struct target_aarch64_ctx *end)
> __put_user(0, &end->size);
> }
>
> +static void target_setup_sve_record(struct target_sve_context *sve,
> + CPUARMState *env, int vq, int size)
> +{
> + int i, j;
> +
> + __put_user(TARGET_SVE_MAGIC, &sve->head.magic);
> + __put_user(size, &sve->head.size);
> + __put_user(vq * TARGET_SVE_VQ_BYTES, &sve->vl);
> +
> + /* Note that SVE regs are stored as a byte stream, with each byte element
> + * at a subsequent address. This corresponds to a little-endian store
> + * of our 64-bit hunks.
> + */
> + for (i = 0; i < 32; ++i) {
> + uint64_t *z = (void *)sve + TARGET_SVE_SIG_ZREG_OFFSET(vq, i);
> + for (j = 0; j < vq * 2; ++j) {
> + __put_user_e(env->vfp.zregs[i].d[j], z + j, le);
> + }
> + }
> + for (i = 0; i <= 16; ++i) {
> + uint16_t *p = (void *)sve + TARGET_SVE_SIG_PREG_OFFSET(vq, i);
> + for (j = 0; j < vq; ++j) {
> + uint64_t r = env->vfp.pregs[i].p[j >> 2];
> + __put_user_e(r >> ((j & 3) * 16), p + j, le);
> + }
> + }
> +}
> +
> static void target_restore_general_frame(CPUARMState *env,
> struct target_rt_sigframe *sf)
> {
> @@ -1569,14 +1625,45 @@ static void target_restore_fpsimd_record(CPUARMState *env,
> }
> }
>
> +static void target_restore_sve_record(CPUARMState *env,
> + struct target_sve_context *sve, int vq)
> +{
> + int i, j;
> +
> + /* Note that SVE regs are stored as a byte stream, with each byte element
> + * at a subsequent address. This corresponds to a little-endian load
> + * of our 64-bit hunks.
> + */
> + for (i = 0; i < 32; ++i) {
> + uint64_t *z = (void *)sve + TARGET_SVE_SIG_ZREG_OFFSET(vq, i);
> + for (j = 0; j < vq * 2; ++j) {
> + __get_user_e(env->vfp.zregs[i].d[j], z + j, le);
> + }
> + }
> + for (i = 0; i <= 16; ++i) {
> + uint16_t *p = (void *)sve + TARGET_SVE_SIG_PREG_OFFSET(vq, i);
> + for (j = 0; j < vq; ++j) {
> + uint16_t r;
> + __get_user_e(r, p + j, le);
> + if (j & 3) {
> + env->vfp.pregs[i].p[j >> 2] |= (uint64_t)r << ((j & 3) * 16);
> + } else {
> + env->vfp.pregs[i].p[j >> 2] = r;
> + }
> + }
> + }
> +}
> +
> static int target_restore_sigframe(CPUARMState *env,
> struct target_rt_sigframe *sf)
> {
> struct target_aarch64_ctx *ctx, *extra = NULL;
> struct target_fpsimd_context *fpsimd = NULL;
> + struct target_sve_context *sve = NULL;
> uint64_t extra_datap = 0;
> bool used_extra = false;
> bool err = false;
> + int vq = 0, sve_size = 0;
>
> target_restore_general_frame(env, sf);
>
> @@ -1608,6 +1695,18 @@ static int target_restore_sigframe(CPUARMState *env,
> fpsimd = (struct target_fpsimd_context *)ctx;
> break;
>
> + case TARGET_SVE_MAGIC:
> + if (arm_feature(env, ARM_FEATURE_SVE)) {
> + vq = (env->vfp.zcr_el[1] & 0xf) + 1;
> + sve_size = QEMU_ALIGN_UP(TARGET_SVE_SIG_CONTEXT_SIZE(vq), 16);
> + if (!sve && size == sve_size) {
> + sve = (struct target_sve_context *)ctx;
> + break;
> + }
> + }
> + err = true;
> + goto exit;
> +
> case TARGET_EXTRA_MAGIC:
> if (extra || size != sizeof(struct target_extra_context)) {
> err = true;
> @@ -1637,12 +1736,18 @@ static int target_restore_sigframe(CPUARMState *env,
> err = true;
> }
>
> + /* SVE data, if present, overwrites FPSIMD data. */
> + if (sve) {
> + target_restore_sve_record(env, sve, vq);
> + }
> +
> exit:
> unlock_user(extra, extra_datap, 0);
> return err;
> }
>
> -static abi_ulong get_sigframe(struct target_sigaction *ka, CPUARMState *env)
> +static abi_ulong get_sigframe(struct target_sigaction *ka,
> + CPUARMState *env, int size)
> {
> abi_ulong sp;
>
> @@ -1655,30 +1760,97 @@ static abi_ulong get_sigframe(struct target_sigaction *ka, CPUARMState *env)
> sp = target_sigaltstack_used.ss_sp + target_sigaltstack_used.ss_size;
> }
>
> - sp = (sp - sizeof(struct target_rt_sigframe)) & ~15;
> + sp = (sp - size) & ~15;
>
> return sp;
> }
>
> +typedef struct {
> + int total_size;
> + int extra_base;
> + int extra_size;
> + int std_end_ofs;
> + int extra_ofs;
> + int extra_end_ofs;
> +} target_sigframe_layout;
> +
> +static int alloc_sigframe_space(int this_size, target_sigframe_layout *l)
> +{
> + /* Make sure there will always be space for the end marker. */
> + const int std_size = sizeof(struct target_rt_sigframe)
> + - sizeof(struct target_aarch64_ctx);
> + int this_loc = l->total_size;
> +
> + if (l->extra_base) {
> + /* Once we have begun an extra space, all allocations go there. */
> + l->extra_size += this_size;
> + } else if (this_size + this_loc > std_size) {
> + /* This allocation does not fit in the standard space. */
> + /* Allocate the extra record. */
> + l->extra_ofs = this_loc;
> + l->total_size += sizeof(struct target_extra_context);
> +
> + /* Allocate the standard end record. */
> + l->std_end_ofs = l->total_size;
> + l->total_size += sizeof(struct target_aarch64_ctx);
> +
> + /* Allocate the requested record. */
> + l->extra_base = this_loc = l->total_size;
> + l->extra_size = this_size;
> + }
> + l->total_size += this_size;
> +
> + return this_loc;
> +}
> +
> static void target_setup_frame(int usig, struct target_sigaction *ka,
> target_siginfo_t *info, target_sigset_t *set,
> CPUARMState *env)
> {
> - int size = offsetof(struct target_rt_sigframe, uc.tuc_mcontext.__reserved);
> - int fpsimd_ofs, end1_ofs, fr_ofs, end2_ofs = 0;
> - int extra_ofs = 0, extra_base = 0, extra_size = 0;
> + target_sigframe_layout layout = {
> + /* Begin with the size pointing to the reserved space. */
> + .total_size = offsetof(struct target_rt_sigframe,
> + uc.tuc_mcontext.__reserved),
> + };
> + int fpsimd_ofs, fr_ofs, sve_ofs = 0, vq = 0, sve_size = 0;
> struct target_rt_sigframe *frame;
> struct target_rt_frame_record *fr;
> abi_ulong frame_addr, return_addr;
>
> - fpsimd_ofs = size;
> - size += sizeof(struct target_fpsimd_context);
> - end1_ofs = size;
> - size += sizeof(struct target_aarch64_ctx);
> - fr_ofs = size;
> - size += sizeof(struct target_rt_frame_record);
> + /* FPSIMD record is always in the standard space. */
> + fpsimd_ofs = alloc_sigframe_space(sizeof(struct target_fpsimd_context),
> + &layout);
>
> - frame_addr = get_sigframe(ka, env);
> + /* SVE state needs saving only if it exists. */
> + if (arm_feature(env, ARM_FEATURE_SVE)) {
> + vq = (env->vfp.zcr_el[1] & 0xf) + 1;
> + sve_size = QEMU_ALIGN_UP(TARGET_SVE_SIG_CONTEXT_SIZE(vq), 16);
> + sve_ofs = alloc_sigframe_space(sve_size, &layout);
> + }
> +
> + if (layout.extra_ofs) {
> + /* Reserve space for the extra end marker. The standard end marker
> + * will have been allocated when we allocated the extra record.
> + */
> + layout.extra_end_ofs
> + = alloc_sigframe_space(sizeof(struct target_aarch64_ctx), &layout);
> + } else {
> + /* Reserve space for the standard end marker.
> + * Do not use alloc_sigframe_space because we cheat
> + * std_size therein to reserve space for this.
> + */
> + layout.std_end_ofs = layout.total_size;
> + layout.total_size += sizeof(struct target_aarch64_ctx);
> + }
> +
> + /* Reserve space for the return code. On a real system this would
> + * be within the VDSO. So, despite the name this is not a "real"
> + * record within the frame.
> + */
> + fr_ofs = layout.total_size;
> + layout.total_size += sizeof(struct target_rt_frame_record);
> +
> + frame_addr = get_sigframe(ka, env, layout.total_size);
> trace_user_setup_frame(env, frame_addr);
> if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
> goto give_sigsegv;
> @@ -1686,13 +1858,15 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
>
> target_setup_general_frame(frame, env, set);
> target_setup_fpsimd_record((void *)frame + fpsimd_ofs, env);
> - if (extra_ofs) {
> - target_setup_extra_record((void *)frame + extra_ofs,
> - frame_addr + extra_base, extra_size);
> + target_setup_end_record((void *)frame + layout.std_end_ofs);
> + if (layout.extra_ofs) {
> + target_setup_extra_record((void *)frame + layout.extra_ofs,
> + frame_addr + layout.extra_base,
> + layout.extra_size);
> + target_setup_end_record((void *)frame + layout.extra_end_ofs);
> }
> - target_setup_end_record((void *)frame + end1_ofs);
> - if (end2_ofs) {
> - target_setup_end_record((void *)frame + end2_ofs);
> + if (sve_ofs) {
> + target_setup_sve_record((void *)frame + sve_ofs, env, vq, sve_size);
> }
>
> /* Set up the stack frame for unwinding. */
--
Alex Bennée
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v4 0/5] target/arm linux-user changes for sve
2018-03-03 14:38 [Qemu-devel] [PATCH v4 0/5] target/arm linux-user changes for sve Richard Henderson
` (4 preceding siblings ...)
2018-03-03 14:38 ` [Qemu-devel] [PATCH v4 5/5] aarch64-linux-user: Add support for SVE " Richard Henderson
@ 2018-03-05 15:42 ` Peter Maydell
5 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2018-03-05 15:42 UTC (permalink / raw)
To: Richard Henderson; +Cc: QEMU Developers, qemu-arm
On 3 March 2018 at 14:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
> Changes since v3:
> * Review comments applied.
> * Frame allocation generalized in patch 5; hopefully this
> eliminates some of the confusion seen during review.
>
> Changes since v2:
> * 5 patches merged,
> * The PR_SVE_SET/GET_VL patch is more specifically user-only.
> * Split the signal frame patch into 4 parts.
>
Applied to target-arm.next, thanks.
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread