| Message ID | 20211015041053.2769193-2-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | user-only: Cleanup SIGSEGV and SIGBUS handling | expand |
On Thu, Oct 14, 2021 at 10:10 PM Richard Henderson < richard.henderson@linaro.org> wrote: > Split out a function to adjust the raw signal pc into a > value that could be passed to cpu_restore_state. > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > v2: Adjust pc in place; return MMUAccessType. > --- > include/exec/exec-all.h | 10 ++++++++++ > accel/tcg/user-exec.c | 41 +++++++++++++++++++++++++---------------- > 2 files changed, 35 insertions(+), 16 deletions(-) > Reviewed-by: Warner Losh <imp@bsdimp.com> > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h > index 9d5987ba04..e54f8e5d65 100644 > --- a/include/exec/exec-all.h > +++ b/include/exec/exec-all.h > @@ -663,6 +663,16 @@ static inline tb_page_addr_t > get_page_addr_code_hostp(CPUArchState *env, > return addr; > } > > +/** > + * adjust_signal_pc: > + * @pc: raw pc from the host signal ucontext_t. > + * @is_write: host memory operation was write, or read-modify-write. > + * > + * Alter @pc as required for unwinding. Return the type of the > + * guest memory access -- host reads may be for guest execution. > + */ > +MMUAccessType adjust_signal_pc(uintptr_t *pc, bool is_write); > + > /** > * cpu_signal_handler > * @signum: host signal number > diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c > index e6bb29b42d..c02d509ec6 100644 > --- a/accel/tcg/user-exec.c > +++ b/accel/tcg/user-exec.c > @@ -57,18 +57,11 @@ static void QEMU_NORETURN > cpu_exit_tb_from_sighandler(CPUState *cpu, > cpu_loop_exit_noexc(cpu); > } > > -/* 'pc' is the host PC at which the exception was raised. 'address' is > - the effective address of the memory exception. 'is_write' is 1 if a > - write caused the exception and otherwise 0'. 'old_set' is the > - signal set which should be restored */ > -static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, > - int is_write, sigset_t *old_set) > +/* > + * Adjust the pc to pass to cpu_restore_state; return the memop type. > + */ > +MMUAccessType adjust_signal_pc(uintptr_t *pc, bool is_write) > { > - CPUState *cpu = current_cpu; > - CPUClass *cc; > - unsigned long address = (unsigned long)info->si_addr; > - MMUAccessType access_type = is_write ? MMU_DATA_STORE : MMU_DATA_LOAD; > - > switch (helper_retaddr) { > default: > /* > @@ -77,7 +70,7 @@ static inline int handle_cpu_signal(uintptr_t pc, > siginfo_t *info, > * pointer into the generated code that will unwind to the > * correct guest pc. > */ > - pc = helper_retaddr; > + *pc = helper_retaddr; > break; > > case 0: > @@ -97,7 +90,7 @@ static inline int handle_cpu_signal(uintptr_t pc, > siginfo_t *info, > * Therefore, adjust to compensate for what will be done later > * by cpu_restore_state_from_tb. > */ > - pc += GETPC_ADJ; > + *pc += GETPC_ADJ; > break; > > case 1: > @@ -113,12 +106,28 @@ static inline int handle_cpu_signal(uintptr_t pc, > siginfo_t *info, > * > * Like tb_gen_code, release the memory lock before cpu_loop_exit. > */ > - pc = 0; > - access_type = MMU_INST_FETCH; > mmap_unlock(); > - break; > + *pc = 0; > + return MMU_INST_FETCH; > } > > + return is_write ? MMU_DATA_STORE : MMU_DATA_LOAD; > +} > + > +/* > + * 'pc' is the host PC at which the exception was raised. > + * 'address' is the effective address of the memory exception. > + * 'is_write' is 1 if a write caused the exception and otherwise 0. > + * 'old_set' is the signal set which should be restored. > + */ > +static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, > + int is_write, sigset_t *old_set) > +{ > + CPUState *cpu = current_cpu; > + CPUClass *cc; > + unsigned long address = (unsigned long)info->si_addr; > + MMUAccessType access_type = adjust_signal_pc(&pc, is_write); > + > /* For synchronous signals we expect to be coming from the vCPU > * thread (so current_cpu should be valid) and either from running > * code or during translation which can fault as we cross pages. > -- > 2.25.1 > > <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 14, 2021 at 10:10 PM Richard Henderson <<a href="mailto:richard.henderson@linaro.org">richard.henderson@linaro.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Split out a function to adjust the raw signal pc into a<br> value that could be passed to cpu_restore_state.<br> <br> Reviewed-by: Philippe Mathieu-Daudé <<a href="mailto:f4bug@amsat.org" target="_blank">f4bug@amsat.org</a>><br> Signed-off-by: Richard Henderson <<a href="mailto:richard.henderson@linaro.org" target="_blank">richard.henderson@linaro.org</a>><br> ---<br> v2: Adjust pc in place; return MMUAccessType.<br> ---<br> include/exec/exec-all.h | 10 ++++++++++<br> accel/tcg/user-exec.c | 41 +++++++++++++++++++++++++----------------<br> 2 files changed, 35 insertions(+), 16 deletions(-)<br></blockquote><div><br></div><div>Reviewed-by: Warner Losh <<a href="mailto:imp@bsdimp.com">imp@bsdimp.com</a>></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h<br> index 9d5987ba04..e54f8e5d65 100644<br> --- a/include/exec/exec-all.h<br> +++ b/include/exec/exec-all.h<br> @@ -663,6 +663,16 @@ static inline tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env,<br> return addr;<br> }<br> <br> +/**<br> + * adjust_signal_pc:<br> + * @pc: raw pc from the host signal ucontext_t.<br> + * @is_write: host memory operation was write, or read-modify-write.<br> + *<br> + * Alter @pc as required for unwinding. Return the type of the<br> + * guest memory access -- host reads may be for guest execution.<br> + */<br> +MMUAccessType adjust_signal_pc(uintptr_t *pc, bool is_write);<br> +<br> /**<br> * cpu_signal_handler<br> * @signum: host signal number<br> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c<br> index e6bb29b42d..c02d509ec6 100644<br> --- a/accel/tcg/user-exec.c<br> +++ b/accel/tcg/user-exec.c<br> @@ -57,18 +57,11 @@ static void QEMU_NORETURN cpu_exit_tb_from_sighandler(CPUState *cpu,<br> cpu_loop_exit_noexc(cpu);<br> }<br> <br> -/* 'pc' is the host PC at which the exception was raised. 'address' is<br> - the effective address of the memory exception. 'is_write' is 1 if a<br> - write caused the exception and otherwise 0'. 'old_set' is the<br> - signal set which should be restored */<br> -static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info,<br> - int is_write, sigset_t *old_set)<br> +/*<br> + * Adjust the pc to pass to cpu_restore_state; return the memop type.<br> + */<br> +MMUAccessType adjust_signal_pc(uintptr_t *pc, bool is_write)<br> {<br> - CPUState *cpu = current_cpu;<br> - CPUClass *cc;<br> - unsigned long address = (unsigned long)info->si_addr;<br> - MMUAccessType access_type = is_write ? MMU_DATA_STORE : MMU_DATA_LOAD;<br> -<br> switch (helper_retaddr) {<br> default:<br> /*<br> @@ -77,7 +70,7 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info,<br> * pointer into the generated code that will unwind to the<br> * correct guest pc.<br> */<br> - pc = helper_retaddr;<br> + *pc = helper_retaddr;<br> break;<br> <br> case 0:<br> @@ -97,7 +90,7 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info,<br> * Therefore, adjust to compensate for what will be done later<br> * by cpu_restore_state_from_tb.<br> */<br> - pc += GETPC_ADJ;<br> + *pc += GETPC_ADJ;<br> break;<br> <br> case 1:<br> @@ -113,12 +106,28 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info,<br> *<br> * Like tb_gen_code, release the memory lock before cpu_loop_exit.<br> */<br> - pc = 0;<br> - access_type = MMU_INST_FETCH;<br> mmap_unlock();<br> - break;<br> + *pc = 0;<br> + return MMU_INST_FETCH;<br> }<br> <br> + return is_write ? MMU_DATA_STORE : MMU_DATA_LOAD;<br> +}<br> +<br> +/*<br> + * 'pc' is the host PC at which the exception was raised.<br> + * 'address' is the effective address of the memory exception.<br> + * 'is_write' is 1 if a write caused the exception and otherwise 0.<br> + * 'old_set' is the signal set which should be restored.<br> + */<br> +static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info,<br> + int is_write, sigset_t *old_set)<br> +{<br> + CPUState *cpu = current_cpu;<br> + CPUClass *cc;<br> + unsigned long address = (unsigned long)info->si_addr;<br> + MMUAccessType access_type = adjust_signal_pc(&pc, is_write);<br> +<br> /* For synchronous signals we expect to be coming from the vCPU<br> * thread (so current_cpu should be valid) and either from running<br> * code or during translation which can fault as we cross pages.<br> -- <br> 2.25.1<br> <br> </blockquote></div></div>
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 9d5987ba04..e54f8e5d65 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -663,6 +663,16 @@ static inline tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, return addr; } +/** + * adjust_signal_pc: + * @pc: raw pc from the host signal ucontext_t. + * @is_write: host memory operation was write, or read-modify-write. + * + * Alter @pc as required for unwinding. Return the type of the + * guest memory access -- host reads may be for guest execution. + */ +MMUAccessType adjust_signal_pc(uintptr_t *pc, bool is_write); + /** * cpu_signal_handler * @signum: host signal number diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c index e6bb29b42d..c02d509ec6 100644 --- a/accel/tcg/user-exec.c +++ b/accel/tcg/user-exec.c @@ -57,18 +57,11 @@ static void QEMU_NORETURN cpu_exit_tb_from_sighandler(CPUState *cpu, cpu_loop_exit_noexc(cpu); } -/* 'pc' is the host PC at which the exception was raised. 'address' is - the effective address of the memory exception. 'is_write' is 1 if a - write caused the exception and otherwise 0'. 'old_set' is the - signal set which should be restored */ -static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, - int is_write, sigset_t *old_set) +/* + * Adjust the pc to pass to cpu_restore_state; return the memop type. + */ +MMUAccessType adjust_signal_pc(uintptr_t *pc, bool is_write) { - CPUState *cpu = current_cpu; - CPUClass *cc; - unsigned long address = (unsigned long)info->si_addr; - MMUAccessType access_type = is_write ? MMU_DATA_STORE : MMU_DATA_LOAD; - switch (helper_retaddr) { default: /* @@ -77,7 +70,7 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, * pointer into the generated code that will unwind to the * correct guest pc. */ - pc = helper_retaddr; + *pc = helper_retaddr; break; case 0: @@ -97,7 +90,7 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, * Therefore, adjust to compensate for what will be done later * by cpu_restore_state_from_tb. */ - pc += GETPC_ADJ; + *pc += GETPC_ADJ; break; case 1: @@ -113,12 +106,28 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, * * Like tb_gen_code, release the memory lock before cpu_loop_exit. */ - pc = 0; - access_type = MMU_INST_FETCH; mmap_unlock(); - break; + *pc = 0; + return MMU_INST_FETCH; } + return is_write ? MMU_DATA_STORE : MMU_DATA_LOAD; +} + +/* + * 'pc' is the host PC at which the exception was raised. + * 'address' is the effective address of the memory exception. + * 'is_write' is 1 if a write caused the exception and otherwise 0. + * 'old_set' is the signal set which should be restored. + */ +static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, + int is_write, sigset_t *old_set) +{ + CPUState *cpu = current_cpu; + CPUClass *cc; + unsigned long address = (unsigned long)info->si_addr; + MMUAccessType access_type = adjust_signal_pc(&pc, is_write); + /* For synchronous signals we expect to be coming from the vCPU * thread (so current_cpu should be valid) and either from running * code or during translation which can fault as we cross pages.