On Thu, Oct 14, 2021 at 10:10 PM Richard Henderson < [email protected]> 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é <[email protected]> > Signed-off-by: Richard Henderson <[email protected]> > --- > 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 <[email protected]> > 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 > >
