| Message ID | 20210918184527.408540-20-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user: Streamline handling of SIGSEGV | expand |
On 9/18/21 20:45, Richard Henderson wrote: > Add a new user-only interface for updating cpu state before > raising a signal. This will replace tlb_fill for user-only > and should result in less boilerplate for each guest. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > include/hw/core/tcg-cpu-ops.h | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h > index 4a4c4053e3..e229a40772 100644 > --- a/include/hw/core/tcg-cpu-ops.h > +++ b/include/hw/core/tcg-cpu-ops.h > @@ -114,6 +114,32 @@ struct TCGCPUOps { > */ > bool (*io_recompile_replay_branch)(CPUState *cpu, > const TranslationBlock *tb); > +#else > + /** > + * record_sigsegv: > + * @cpu: cpu context > + * @addr: faulting guest address > + * @access_type: access was read/write/execute > + * @maperr: true for invalid page, false for permission fault > + * @ra: host pc for unwinding > + * > + * We are about to raise SIGSEGV with si_code set for @maperr, > + * and si_addr set for @addr. Record anything further needed > + * for the signal ucontext_t. > + * > + * If the emulated kernel does not provide anything to the signal > + * handler with anything besides the user context registers, and > + * the siginfo_t, then this hook need do nothing and may be omitted. > + * Otherwise, record the data and return; the caller will raise > + * the signal, unwind the cpu state, and return to the main loop. > + * > + * If it is simpler to re-use the sysemu tlb_fill code, @ra is provided > + * so that a "normal" cpu exception can be raised. In this case, > + * the signal must be raised by the architecture cpu_loop. > + */ Shouldn't it have the QEMU_NORETURN attribute? > + void (*record_sigsegv)(CPUState *cpu, vaddr addr, > + MMUAccessType access_type, > + bool maperr, uintptr_t ra); > #endif /* CONFIG_SOFTMMU */ > #endif /* NEED_CPU_H */ > >
On 9/19/21 20:22, Philippe Mathieu-Daudé wrote: > On 9/18/21 20:45, Richard Henderson wrote: >> Add a new user-only interface for updating cpu state before >> raising a signal. This will replace tlb_fill for user-only >> and should result in less boilerplate for each guest. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> include/hw/core/tcg-cpu-ops.h | 26 ++++++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h >> index 4a4c4053e3..e229a40772 100644 >> --- a/include/hw/core/tcg-cpu-ops.h >> +++ b/include/hw/core/tcg-cpu-ops.h >> @@ -114,6 +114,32 @@ struct TCGCPUOps { >> */ >> bool (*io_recompile_replay_branch)(CPUState *cpu, >> const TranslationBlock *tb); >> +#else >> + /** >> + * record_sigsegv: >> + * @cpu: cpu context >> + * @addr: faulting guest address >> + * @access_type: access was read/write/execute >> + * @maperr: true for invalid page, false for permission fault >> + * @ra: host pc for unwinding >> + * >> + * We are about to raise SIGSEGV with si_code set for @maperr, >> + * and si_addr set for @addr. Record anything further needed >> + * for the signal ucontext_t. >> + * >> + * If the emulated kernel does not provide anything to the signal >> + * handler with anything besides the user context registers, and >> + * the siginfo_t, then this hook need do nothing and may be omitted. >> + * Otherwise, record the data and return; the caller will raise >> + * the signal, unwind the cpu state, and return to the main loop. >> + * >> + * If it is simpler to re-use the sysemu tlb_fill code, @ra is provided >> + * so that a "normal" cpu exception can be raised. In this case, >> + * the signal must be raised by the architecture cpu_loop. >> + */ > > Shouldn't it have the QEMU_NORETURN attribute? Eh now I saw the next patch and understood raise_sigsegv() is where QEMU_NORETURN belong :) Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > >> + void (*record_sigsegv)(CPUState *cpu, vaddr addr, >> + MMUAccessType access_type, >> + bool maperr, uintptr_t ra); >> #endif /* CONFIG_SOFTMMU */ >> #endif /* NEED_CPU_H */ >> >> > >
On 9/19/21 11:24 AM, Philippe Mathieu-Daudé wrote: > On 9/19/21 20:22, Philippe Mathieu-Daudé wrote: >> On 9/18/21 20:45, Richard Henderson wrote: >>> Add a new user-only interface for updating cpu state before >>> raising a signal. This will replace tlb_fill for user-only >>> and should result in less boilerplate for each guest. >>> >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>> --- >>> include/hw/core/tcg-cpu-ops.h | 26 ++++++++++++++++++++++++++ >>> 1 file changed, 26 insertions(+) >>> >>> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h >>> index 4a4c4053e3..e229a40772 100644 >>> --- a/include/hw/core/tcg-cpu-ops.h >>> +++ b/include/hw/core/tcg-cpu-ops.h >>> @@ -114,6 +114,32 @@ struct TCGCPUOps { >>> */ >>> bool (*io_recompile_replay_branch)(CPUState *cpu, >>> const TranslationBlock *tb); >>> +#else >>> + /** >>> + * record_sigsegv: >>> + * @cpu: cpu context >>> + * @addr: faulting guest address >>> + * @access_type: access was read/write/execute >>> + * @maperr: true for invalid page, false for permission fault >>> + * @ra: host pc for unwinding >>> + * >>> + * We are about to raise SIGSEGV with si_code set for @maperr, >>> + * and si_addr set for @addr. Record anything further needed >>> + * for the signal ucontext_t. >>> + * >>> + * If the emulated kernel does not provide anything to the signal >>> + * handler with anything besides the user context registers, and >>> + * the siginfo_t, then this hook need do nothing and may be omitted. >>> + * Otherwise, record the data and return; the caller will raise >>> + * the signal, unwind the cpu state, and return to the main loop. >>> + * >>> + * If it is simpler to re-use the sysemu tlb_fill code, @ra is provided >>> + * so that a "normal" cpu exception can be raised. In this case, >>> + * the signal must be raised by the architecture cpu_loop. >>> + */ >> >> Shouldn't it have the QEMU_NORETURN attribute? > > Eh now I saw the next patch and understood raise_sigsegv() is > where QEMU_NORETURN belong :) Well, I had intended the hook to be able to return, so that the hook can merely set some env->fields, and return to raise the signal. But in the end, all 4 targets that use the hook raise the exception themselves -- sometimes because env->exception_index is part of the data to be passed to the signal handler. r~
diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h index 4a4c4053e3..e229a40772 100644 --- a/include/hw/core/tcg-cpu-ops.h +++ b/include/hw/core/tcg-cpu-ops.h @@ -114,6 +114,32 @@ struct TCGCPUOps { */ bool (*io_recompile_replay_branch)(CPUState *cpu, const TranslationBlock *tb); +#else + /** + * record_sigsegv: + * @cpu: cpu context + * @addr: faulting guest address + * @access_type: access was read/write/execute + * @maperr: true for invalid page, false for permission fault + * @ra: host pc for unwinding + * + * We are about to raise SIGSEGV with si_code set for @maperr, + * and si_addr set for @addr. Record anything further needed + * for the signal ucontext_t. + * + * If the emulated kernel does not provide anything to the signal + * handler with anything besides the user context registers, and + * the siginfo_t, then this hook need do nothing and may be omitted. + * Otherwise, record the data and return; the caller will raise + * the signal, unwind the cpu state, and return to the main loop. + * + * If it is simpler to re-use the sysemu tlb_fill code, @ra is provided + * so that a "normal" cpu exception can be raised. In this case, + * the signal must be raised by the architecture cpu_loop. + */ + void (*record_sigsegv)(CPUState *cpu, vaddr addr, + MMUAccessType access_type, + bool maperr, uintptr_t ra); #endif /* CONFIG_SOFTMMU */ #endif /* NEED_CPU_H */
Add a new user-only interface for updating cpu state before raising a signal. This will replace tlb_fill for user-only and should result in less boilerplate for each guest. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- include/hw/core/tcg-cpu-ops.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)