| Message ID | 20211015041053.2769193-4-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: > This is the major portion of handle_cpu_signal which is specific > to tcg, handling the page protections for the translations. > Most of the rest will migrate to linux-user/ shortly. > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > v2: Pass guest address to handle_sigsegv_accerr_write. > --- > include/exec/exec-all.h | 12 +++++ > accel/tcg/user-exec.c | 103 ++++++++++++++++++++++++---------------- > 2 files changed, 74 insertions(+), 41 deletions(-) > Reviewed-by: Warner Losh <imp@bsdimp.com> > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h > index e54f8e5d65..5f94d799aa 100644 > --- a/include/exec/exec-all.h > +++ b/include/exec/exec-all.h > @@ -673,6 +673,18 @@ static inline tb_page_addr_t > get_page_addr_code_hostp(CPUArchState *env, > */ > MMUAccessType adjust_signal_pc(uintptr_t *pc, bool is_write); > > +/** > + * handle_sigsegv_accerr_write: > + * @cpu: the cpu context > + * @old_set: the sigset_t from the signal ucontext_t > + * @host_pc: the host pc, adjusted for the signal > + * @host_addr: the host address of the fault > + * > + * Return true if the write fault has been handled, and should be > re-tried. > + */ > +bool handle_sigsegv_accerr_write(CPUState *cpu, sigset_t *old_set, > + uintptr_t host_pc, abi_ptr guest_addr); > + > /** > * cpu_signal_handler > * @signum: host signal number > diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c > index 3f3e793b7b..cb63e528c5 100644 > --- a/accel/tcg/user-exec.c > +++ b/accel/tcg/user-exec.c > @@ -114,6 +114,54 @@ MMUAccessType adjust_signal_pc(uintptr_t *pc, bool > is_write) > return is_write ? MMU_DATA_STORE : MMU_DATA_LOAD; > } > > +/** > + * handle_sigsegv_accerr_write: > + * @cpu: the cpu context > + * @old_set: the sigset_t from the signal ucontext_t > + * @host_pc: the host pc, adjusted for the signal > + * @guest_addr: the guest address of the fault > + * > + * Return true if the write fault has been handled, and should be > re-tried. > + * > + * Note that it is important that we don't call page_unprotect() unless > + * this is really a "write to nonwriteable page" fault, because > + * page_unprotect() assumes that if it is called for an access to > + * a page that's writeable this means we had two threads racing and > + * another thread got there first and already made the page writeable; > + * so we will retry the access. If we were to call page_unprotect() > + * for some other kind of fault that should really be passed to the > + * guest, we'd end up in an infinite loop of retrying the faulting access. > + */ > +bool handle_sigsegv_accerr_write(CPUState *cpu, sigset_t *old_set, > + uintptr_t host_pc, abi_ptr guest_addr) > +{ > + switch (page_unprotect(guest_addr, host_pc)) { > + case 0: > + /* > + * Fault not caused by a page marked unwritable to protect > + * cached translations, must be the guest binary's problem. > + */ > + return false; > + case 1: > + /* > + * Fault caused by protection of cached translation; TBs > + * invalidated, so resume execution. Retain helper_retaddr > + * for a possible second fault. > + */ > + return true; > + case 2: > + /* > + * Fault caused by protection of cached translation, and the > + * currently executing TB was modified and must be exited > + * immediately. Clear helper_retaddr for next execution. > + */ > + cpu_exit_tb_from_sighandler(cpu, old_set); > + /* NORETURN */ > + default: > + g_assert_not_reached(); > + } > +} > + > /* > * 'pc' is the host PC at which the exception was raised. > * 'address' is the effective address of the memory exception. > @@ -125,8 +173,9 @@ static inline int handle_cpu_signal(uintptr_t pc, > siginfo_t *info, > { > CPUState *cpu = current_cpu; > CPUClass *cc; > - unsigned long address = (unsigned long)info->si_addr; > + unsigned long host_addr = (unsigned long)info->si_addr; > MMUAccessType access_type = adjust_signal_pc(&pc, is_write); > + abi_ptr guest_addr; > > /* For synchronous signals we expect to be coming from the vCPU > * thread (so current_cpu should be valid) and either from running > @@ -143,49 +192,21 @@ static inline int handle_cpu_signal(uintptr_t pc, > siginfo_t *info, > > #if defined(DEBUG_SIGNAL) > printf("qemu: SIGSEGV pc=0x%08lx address=%08lx w=%d oldset=0x%08lx\n", > - pc, address, is_write, *(unsigned long *)old_set); > + pc, host_addr, is_write, *(unsigned long *)old_set); > #endif > - /* XXX: locking issue */ > - /* Note that it is important that we don't call page_unprotect() > unless > - * this is really a "write to nonwriteable page" fault, because > - * page_unprotect() assumes that if it is called for an access to > - * a page that's writeable this means we had two threads racing and > - * another thread got there first and already made the page writeable; > - * so we will retry the access. If we were to call page_unprotect() > - * for some other kind of fault that should really be passed to the > - * guest, we'd end up in an infinite loop of retrying the faulting > - * access. > - */ > - if (is_write && info->si_signo == SIGSEGV && info->si_code == > SEGV_ACCERR && > - h2g_valid(address)) { > - switch (page_unprotect(h2g(address), pc)) { > - case 0: > - /* Fault not caused by a page marked unwritable to protect > - * cached translations, must be the guest binary's problem. > - */ > - break; > - case 1: > - /* Fault caused by protection of cached translation; TBs > - * invalidated, so resume execution. Retain helper_retaddr > - * for a possible second fault. > - */ > - return 1; > - case 2: > - /* Fault caused by protection of cached translation, and the > - * currently executing TB was modified and must be exited > - * immediately. Clear helper_retaddr for next execution. > - */ > - cpu_exit_tb_from_sighandler(cpu, old_set); > - /* NORETURN */ > - > - default: > - g_assert_not_reached(); > - } > - } > > /* Convert forcefully to guest address space, invalid addresses > are still valid segv ones */ > - address = h2g_nocheck(address); > + guest_addr = h2g_nocheck(host_addr); > + > + /* XXX: locking issue */ > + if (is_write && > + info->si_signo == SIGSEGV && > + info->si_code == SEGV_ACCERR && > + h2g_valid(host_addr) && > + handle_sigsegv_accerr_write(cpu, old_set, pc, guest_addr)) { > + return 1; > + } > > /* > * There is no way the target can handle this other than raising > @@ -194,7 +215,7 @@ static inline int handle_cpu_signal(uintptr_t pc, > siginfo_t *info, > sigprocmask(SIG_SETMASK, old_set, NULL); > > cc = CPU_GET_CLASS(cpu); > - cc->tcg_ops->tlb_fill(cpu, address, 0, access_type, > + cc->tcg_ops->tlb_fill(cpu, guest_addr, 0, access_type, > MMU_USER_IDX, false, pc); > g_assert_not_reached(); > } > -- > 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">This is the major portion of handle_cpu_signal which is specific<br> to tcg, handling the page protections for the translations.<br> Most of the rest will migrate to linux-user/ shortly.<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: Pass guest address to handle_sigsegv_accerr_write.<br> ---<br> include/exec/exec-all.h | 12 +++++<br> accel/tcg/user-exec.c | 103 ++++++++++++++++++++++++----------------<br> 2 files changed, 74 insertions(+), 41 deletions(-)<br></blockquote><div><br></div><div><div>Reviewed-by: Warner Losh <<a href="mailto:imp@bsdimp.com">imp@bsdimp.com</a>></div></div><div> </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 e54f8e5d65..5f94d799aa 100644<br> --- a/include/exec/exec-all.h<br> +++ b/include/exec/exec-all.h<br> @@ -673,6 +673,18 @@ static inline tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env,<br> */<br> MMUAccessType adjust_signal_pc(uintptr_t *pc, bool is_write);<br> <br> +/**<br> + * handle_sigsegv_accerr_write:<br> + * @cpu: the cpu context<br> + * @old_set: the sigset_t from the signal ucontext_t<br> + * @host_pc: the host pc, adjusted for the signal<br> + * @host_addr: the host address of the fault<br> + *<br> + * Return true if the write fault has been handled, and should be re-tried.<br> + */<br> +bool handle_sigsegv_accerr_write(CPUState *cpu, sigset_t *old_set,<br> + uintptr_t host_pc, abi_ptr guest_addr);<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 3f3e793b7b..cb63e528c5 100644<br> --- a/accel/tcg/user-exec.c<br> +++ b/accel/tcg/user-exec.c<br> @@ -114,6 +114,54 @@ MMUAccessType adjust_signal_pc(uintptr_t *pc, bool is_write)<br> return is_write ? MMU_DATA_STORE : MMU_DATA_LOAD;<br> }<br> <br> +/**<br> + * handle_sigsegv_accerr_write:<br> + * @cpu: the cpu context<br> + * @old_set: the sigset_t from the signal ucontext_t<br> + * @host_pc: the host pc, adjusted for the signal<br> + * @guest_addr: the guest address of the fault<br> + *<br> + * Return true if the write fault has been handled, and should be re-tried.<br> + *<br> + * Note that it is important that we don't call page_unprotect() unless<br> + * this is really a "write to nonwriteable page" fault, because<br> + * page_unprotect() assumes that if it is called for an access to<br> + * a page that's writeable this means we had two threads racing and<br> + * another thread got there first and already made the page writeable;<br> + * so we will retry the access. If we were to call page_unprotect()<br> + * for some other kind of fault that should really be passed to the<br> + * guest, we'd end up in an infinite loop of retrying the faulting access.<br> + */<br> +bool handle_sigsegv_accerr_write(CPUState *cpu, sigset_t *old_set,<br> + uintptr_t host_pc, abi_ptr guest_addr)<br> +{<br> + switch (page_unprotect(guest_addr, host_pc)) {<br> + case 0:<br> + /*<br> + * Fault not caused by a page marked unwritable to protect<br> + * cached translations, must be the guest binary's problem.<br> + */<br> + return false;<br> + case 1:<br> + /*<br> + * Fault caused by protection of cached translation; TBs<br> + * invalidated, so resume execution. Retain helper_retaddr<br> + * for a possible second fault.<br> + */<br> + return true;<br> + case 2:<br> + /*<br> + * Fault caused by protection of cached translation, and the<br> + * currently executing TB was modified and must be exited<br> + * immediately. Clear helper_retaddr for next execution.<br> + */<br> + cpu_exit_tb_from_sighandler(cpu, old_set);<br> + /* NORETURN */<br> + default:<br> + g_assert_not_reached();<br> + }<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> @@ -125,8 +173,9 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info,<br> {<br> CPUState *cpu = current_cpu;<br> CPUClass *cc;<br> - unsigned long address = (unsigned long)info->si_addr;<br> + unsigned long host_addr = (unsigned long)info->si_addr;<br> MMUAccessType access_type = adjust_signal_pc(&pc, is_write);<br> + abi_ptr guest_addr;<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> @@ -143,49 +192,21 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info,<br> <br> #if defined(DEBUG_SIGNAL)<br> printf("qemu: SIGSEGV pc=0x%08lx address=%08lx w=%d oldset=0x%08lx\n",<br> - pc, address, is_write, *(unsigned long *)old_set);<br> + pc, host_addr, is_write, *(unsigned long *)old_set);<br> #endif<br> - /* XXX: locking issue */<br> - /* Note that it is important that we don't call page_unprotect() unless<br> - * this is really a "write to nonwriteable page" fault, because<br> - * page_unprotect() assumes that if it is called for an access to<br> - * a page that's writeable this means we had two threads racing and<br> - * another thread got there first and already made the page writeable;<br> - * so we will retry the access. If we were to call page_unprotect()<br> - * for some other kind of fault that should really be passed to the<br> - * guest, we'd end up in an infinite loop of retrying the faulting<br> - * access.<br> - */<br> - if (is_write && info->si_signo == SIGSEGV && info->si_code == SEGV_ACCERR &&<br> - h2g_valid(address)) {<br> - switch (page_unprotect(h2g(address), pc)) {<br> - case 0:<br> - /* Fault not caused by a page marked unwritable to protect<br> - * cached translations, must be the guest binary's problem.<br> - */<br> - break;<br> - case 1:<br> - /* Fault caused by protection of cached translation; TBs<br> - * invalidated, so resume execution. Retain helper_retaddr<br> - * for a possible second fault.<br> - */<br> - return 1;<br> - case 2:<br> - /* Fault caused by protection of cached translation, and the<br> - * currently executing TB was modified and must be exited<br> - * immediately. Clear helper_retaddr for next execution.<br> - */<br> - cpu_exit_tb_from_sighandler(cpu, old_set);<br> - /* NORETURN */<br> -<br> - default:<br> - g_assert_not_reached();<br> - }<br> - }<br> <br> /* Convert forcefully to guest address space, invalid addresses<br> are still valid segv ones */<br> - address = h2g_nocheck(address);<br> + guest_addr = h2g_nocheck(host_addr);<br> +<br> + /* XXX: locking issue */<br> + if (is_write &&<br> + info->si_signo == SIGSEGV &&<br> + info->si_code == SEGV_ACCERR &&<br> + h2g_valid(host_addr) &&<br> + handle_sigsegv_accerr_write(cpu, old_set, pc, guest_addr)) {<br> + return 1;<br> + }<br> <br> /*<br> * There is no way the target can handle this other than raising<br> @@ -194,7 +215,7 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info,<br> sigprocmask(SIG_SETMASK, old_set, NULL);<br> <br> cc = CPU_GET_CLASS(cpu);<br> - cc->tcg_ops->tlb_fill(cpu, address, 0, access_type,<br> + cc->tcg_ops->tlb_fill(cpu, guest_addr, 0, access_type,<br> MMU_USER_IDX, false, pc);<br> g_assert_not_reached();<br> }<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 e54f8e5d65..5f94d799aa 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -673,6 +673,18 @@ static inline tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, */ MMUAccessType adjust_signal_pc(uintptr_t *pc, bool is_write); +/** + * handle_sigsegv_accerr_write: + * @cpu: the cpu context + * @old_set: the sigset_t from the signal ucontext_t + * @host_pc: the host pc, adjusted for the signal + * @host_addr: the host address of the fault + * + * Return true if the write fault has been handled, and should be re-tried. + */ +bool handle_sigsegv_accerr_write(CPUState *cpu, sigset_t *old_set, + uintptr_t host_pc, abi_ptr guest_addr); + /** * cpu_signal_handler * @signum: host signal number diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c index 3f3e793b7b..cb63e528c5 100644 --- a/accel/tcg/user-exec.c +++ b/accel/tcg/user-exec.c @@ -114,6 +114,54 @@ MMUAccessType adjust_signal_pc(uintptr_t *pc, bool is_write) return is_write ? MMU_DATA_STORE : MMU_DATA_LOAD; } +/** + * handle_sigsegv_accerr_write: + * @cpu: the cpu context + * @old_set: the sigset_t from the signal ucontext_t + * @host_pc: the host pc, adjusted for the signal + * @guest_addr: the guest address of the fault + * + * Return true if the write fault has been handled, and should be re-tried. + * + * Note that it is important that we don't call page_unprotect() unless + * this is really a "write to nonwriteable page" fault, because + * page_unprotect() assumes that if it is called for an access to + * a page that's writeable this means we had two threads racing and + * another thread got there first and already made the page writeable; + * so we will retry the access. If we were to call page_unprotect() + * for some other kind of fault that should really be passed to the + * guest, we'd end up in an infinite loop of retrying the faulting access. + */ +bool handle_sigsegv_accerr_write(CPUState *cpu, sigset_t *old_set, + uintptr_t host_pc, abi_ptr guest_addr) +{ + switch (page_unprotect(guest_addr, host_pc)) { + case 0: + /* + * Fault not caused by a page marked unwritable to protect + * cached translations, must be the guest binary's problem. + */ + return false; + case 1: + /* + * Fault caused by protection of cached translation; TBs + * invalidated, so resume execution. Retain helper_retaddr + * for a possible second fault. + */ + return true; + case 2: + /* + * Fault caused by protection of cached translation, and the + * currently executing TB was modified and must be exited + * immediately. Clear helper_retaddr for next execution. + */ + cpu_exit_tb_from_sighandler(cpu, old_set); + /* NORETURN */ + default: + g_assert_not_reached(); + } +} + /* * 'pc' is the host PC at which the exception was raised. * 'address' is the effective address of the memory exception. @@ -125,8 +173,9 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, { CPUState *cpu = current_cpu; CPUClass *cc; - unsigned long address = (unsigned long)info->si_addr; + unsigned long host_addr = (unsigned long)info->si_addr; MMUAccessType access_type = adjust_signal_pc(&pc, is_write); + abi_ptr guest_addr; /* For synchronous signals we expect to be coming from the vCPU * thread (so current_cpu should be valid) and either from running @@ -143,49 +192,21 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, #if defined(DEBUG_SIGNAL) printf("qemu: SIGSEGV pc=0x%08lx address=%08lx w=%d oldset=0x%08lx\n", - pc, address, is_write, *(unsigned long *)old_set); + pc, host_addr, is_write, *(unsigned long *)old_set); #endif - /* XXX: locking issue */ - /* Note that it is important that we don't call page_unprotect() unless - * this is really a "write to nonwriteable page" fault, because - * page_unprotect() assumes that if it is called for an access to - * a page that's writeable this means we had two threads racing and - * another thread got there first and already made the page writeable; - * so we will retry the access. If we were to call page_unprotect() - * for some other kind of fault that should really be passed to the - * guest, we'd end up in an infinite loop of retrying the faulting - * access. - */ - if (is_write && info->si_signo == SIGSEGV && info->si_code == SEGV_ACCERR && - h2g_valid(address)) { - switch (page_unprotect(h2g(address), pc)) { - case 0: - /* Fault not caused by a page marked unwritable to protect - * cached translations, must be the guest binary's problem. - */ - break; - case 1: - /* Fault caused by protection of cached translation; TBs - * invalidated, so resume execution. Retain helper_retaddr - * for a possible second fault. - */ - return 1; - case 2: - /* Fault caused by protection of cached translation, and the - * currently executing TB was modified and must be exited - * immediately. Clear helper_retaddr for next execution. - */ - cpu_exit_tb_from_sighandler(cpu, old_set); - /* NORETURN */ - - default: - g_assert_not_reached(); - } - } /* Convert forcefully to guest address space, invalid addresses are still valid segv ones */ - address = h2g_nocheck(address); + guest_addr = h2g_nocheck(host_addr); + + /* XXX: locking issue */ + if (is_write && + info->si_signo == SIGSEGV && + info->si_code == SEGV_ACCERR && + h2g_valid(host_addr) && + handle_sigsegv_accerr_write(cpu, old_set, pc, guest_addr)) { + return 1; + } /* * There is no way the target can handle this other than raising @@ -194,7 +215,7 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, sigprocmask(SIG_SETMASK, old_set, NULL); cc = CPU_GET_CLASS(cpu); - cc->tcg_ops->tlb_fill(cpu, address, 0, access_type, + cc->tcg_ops->tlb_fill(cpu, guest_addr, 0, access_type, MMU_USER_IDX, false, pc); g_assert_not_reached(); }