| Message ID | 20230822170209.1130173-2-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user/aarch64: Add ESR signal frame record | expand |
On Tue, 22 Aug 2023 at 18:03, Richard Henderson <richard.henderson@linaro.org> wrote: > > These are all synchronous exceptions for which the kernel > passes on ESR to the user signal handler. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > @@ -191,6 +199,14 @@ static void target_setup_end_record(struct target_aarch64_ctx *end) > __put_user(0, &end->size); > } > > +static void target_setup_esr_record(struct target_esr_context *esr, > + CPUARMState *env) > +{ > + __put_user(TARGET_ESR_MAGIC, &esr->head.magic); > + __put_user(sizeof(struct target_esr_context), &esr->head.size); > + __put_user(env->exception.syndrome, &esr->esr); > +} > + > static void target_setup_sve_record(struct target_sve_context *sve, > CPUARMState *env, int size) > { > @@ -443,6 +459,10 @@ static int target_restore_sigframe(CPUARMState *env, > fpsimd = (struct target_fpsimd_context *)ctx; > break; > > + case TARGET_ESR_MAGIC: > + /* ignore */ > + break; > + > case TARGET_SVE_MAGIC: > if (sve || size < sizeof(struct target_sve_context)) { > goto err; > @@ -558,6 +578,23 @@ static int alloc_sigframe_space(int this_size, target_sigframe_layout *l) > return this_loc; > } > > +static bool need_save_esr(target_siginfo_t *info, CPUARMState *env) > +{ > + int sig = info->si_signo; > + int type = info->si_code >> 16; > + > + if (type != QEMU_SI_FAULT) { > + return false; > + } > + > + /* See arch/arm64/mm/fault.c, set_thread_esr. */ > + if (sig == TARGET_SIGSEGV || sig == TARGET_SIGBUS) { > + return true; > + } It's possible to get here without env->exception.syndrome being set correctly, I think, if we take a host SIGSEGV or SIGBUS and host_signal_handler() calls either cpu_loop_exit_sigsegv() or cpu_loop_exit_sigbus(). Can also happen for other places that call one of those two functions, like allocation_tag_mem(). At least, I can't see where we would be setting syndrome in that code path. > + > + return false; > +} Maybe we should do the "sanitize ESR for fault addresses in the upper half of guest address space" logic that the kernel set_thread_esr() does? thanks -- PMM
On 8/29/23 07:35, Peter Maydell wrote: >> + /* See arch/arm64/mm/fault.c, set_thread_esr. */ >> + if (sig == TARGET_SIGSEGV || sig == TARGET_SIGBUS) { >> + return true; >> + } > > It's possible to get here without env->exception.syndrome > being set correctly, I think, if we take a host > SIGSEGV or SIGBUS and host_signal_handler() calls either > cpu_loop_exit_sigsegv() or cpu_loop_exit_sigbus(). Can also > happen for other places that call one of those two functions, > like allocation_tag_mem(). At least, I can't see where we > would be setting syndrome in that code path. cpu_loop_exit_sig* go through arm_cpu_record_sigsegv and arm_cpu_record_sigbus, which use the normal fault processing paths to populate FAR_EL1 and ESR_EL1. > Maybe we should do the "sanitize ESR for fault addresses in > the upper half of guest address space" logic that the kernel > set_thread_esr() does? I guess we could, though I'm not sure how such an address could occur. r~
diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c index b265cfd470..b2280fa9e3 100644 --- a/linux-user/aarch64/signal.c +++ b/linux-user/aarch64/signal.c @@ -21,6 +21,7 @@ #include "user-internals.h" #include "signal-common.h" #include "linux-user/trace.h" +#include "target/arm/syndrome.h" struct target_sigcontext { uint64_t fault_address; @@ -64,6 +65,13 @@ struct target_fpsimd_context { uint64_t vregs[32 * 2]; /* really uint128_t vregs[32] */ }; +#define TARGET_ESR_MAGIC 0x45535201 + +struct target_esr_context { + struct target_aarch64_ctx head; + uint64_t esr; +}; + #define TARGET_EXTRA_MAGIC 0x45585401 struct target_extra_context { @@ -191,6 +199,14 @@ static void target_setup_end_record(struct target_aarch64_ctx *end) __put_user(0, &end->size); } +static void target_setup_esr_record(struct target_esr_context *esr, + CPUARMState *env) +{ + __put_user(TARGET_ESR_MAGIC, &esr->head.magic); + __put_user(sizeof(struct target_esr_context), &esr->head.size); + __put_user(env->exception.syndrome, &esr->esr); +} + static void target_setup_sve_record(struct target_sve_context *sve, CPUARMState *env, int size) { @@ -443,6 +459,10 @@ static int target_restore_sigframe(CPUARMState *env, fpsimd = (struct target_fpsimd_context *)ctx; break; + case TARGET_ESR_MAGIC: + /* ignore */ + break; + case TARGET_SVE_MAGIC: if (sve || size < sizeof(struct target_sve_context)) { goto err; @@ -558,6 +578,23 @@ static int alloc_sigframe_space(int this_size, target_sigframe_layout *l) return this_loc; } +static bool need_save_esr(target_siginfo_t *info, CPUARMState *env) +{ + int sig = info->si_signo; + int type = info->si_code >> 16; + + if (type != QEMU_SI_FAULT) { + return false; + } + + /* See arch/arm64/mm/fault.c, set_thread_esr. */ + if (sig == TARGET_SIGSEGV || sig == TARGET_SIGBUS) { + return true; + } + + return false; +} + static void target_setup_frame(int usig, struct target_sigaction *ka, target_siginfo_t *info, target_sigset_t *set, CPUARMState *env) @@ -567,7 +604,7 @@ static void target_setup_frame(int usig, struct target_sigaction *ka, .total_size = offsetof(struct target_rt_sigframe, uc.tuc_mcontext.__reserved), }; - int fpsimd_ofs, fr_ofs, sve_ofs = 0, za_ofs = 0; + int fpsimd_ofs, fr_ofs, esr_ofs = 0, sve_ofs = 0, za_ofs = 0; int sve_size = 0, za_size = 0; struct target_rt_sigframe *frame; struct target_rt_frame_record *fr; @@ -577,6 +614,12 @@ static void target_setup_frame(int usig, struct target_sigaction *ka, fpsimd_ofs = alloc_sigframe_space(sizeof(struct target_fpsimd_context), &layout); + /* ESR state needs saving only for certain signals. */ + if (need_save_esr(info, env)) { + esr_ofs = alloc_sigframe_space(sizeof(struct target_esr_context), + &layout); + } + /* SVE state needs saving only if it exists. */ if (cpu_isar_feature(aa64_sve, env_archcpu(env)) || cpu_isar_feature(aa64_sme, env_archcpu(env))) { @@ -637,6 +680,9 @@ static void target_setup_frame(int usig, struct target_sigaction *ka, layout.extra_size); target_setup_end_record((void *)frame + layout.extra_end_ofs); } + if (esr_ofs) { + target_setup_esr_record((void *)frame + esr_ofs, env); + } if (sve_ofs) { target_setup_sve_record((void *)frame + sve_ofs, env, sve_size); }
These are all synchronous exceptions for which the kernel passes on ESR to the user signal handler. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- linux-user/aarch64/signal.c | 48 ++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-)