Re: [Qemu-devel] [PATCH 5/5] target-i386: Move user-mode exception actions out of user-exec.c

[Sergey Fedorov]

On 16/05/16 19:09, Peter Maydell wrote:
> The exception_action() function in user-exec.c is just a call to
> cpu_loop_exit() for every target CPU except i386.  Since this
> function is only called if the target's handle_mmu_fault() hook has
> indicated an MMU fault, and that hook is only called from the
> handle_cpu_signal() code path, we can simply move the x86-specific
> setup into that hook, which allows us to remove the TARGET_I386
> ifdef from user-exec.c.
>
> Of the actions that were done by the call to raise_interrupt_err():
>  * cpu_svm_check_intercept_param() is a no-op in user mode
>  * check_exception() is a no-op since double faults are impossible
>    for user-mode
>  * assignments to cs->exception_index and env->error_code are no-ops
>  * cpu_loop_exit_restore() is equivalent to cpu_loop_exit() since
>    pc is 0
> which leaves just setting env_>exception_is_int and
> env->exception_next_eip as actions that need to be added to
> x86_cpu_handle_mmu_fault().
>
> Signed-off-by: Peter Maydell <address@hidden>
> ---
>  target-i386/helper.c |  2 ++
>  user-exec.c          | 16 +---------------
>  2 files changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index bf3e762..e1dde46 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -700,6 +700,8 @@ int x86_cpu_handle_mmu_fault(CPUState *cs, vaddr addr,
>      env->error_code = (is_write << PG_ERROR_W_BIT);
>      env->error_code |= PG_ERROR_U_MASK;
>      cs->exception_index = EXCP0E_PAGE;
> +    env->exception_is_int = 0;
> +    env->exception_next_eip = env->eip;

'env->eip' was updated by restore_state_to_opc() from
cpu_restore_state_from_tb() from cpu_restore_state() from
handle_cpu_signal() _after_ calling 'handle_mmu_fault' hook but _before_
calling exception_action().

Kind regards,
Sergey

>      return 1;
>  }
>  
> diff --git a/user-exec.c b/user-exec.c
> index ad669f4..439bb37 100644
> --- a/user-exec.c
> +++ b/user-exec.c
> @@ -39,18 +39,6 @@
>  
>  //#define DEBUG_SIGNAL
>  
> -static void exception_action(CPUState *cpu)
> -{
> -#if defined(TARGET_I386)
> -    X86CPU *x86_cpu = X86_CPU(cpu);
> -    CPUX86State *env1 = &x86_cpu->env;
> -
> -    raise_exception_err(env1, cpu->exception_index, env1->error_code);
> -#else
> -    cpu_loop_exit(cpu);
> -#endif
> -}
> -
>  /* exit the current TB from a signal handler. The host registers are
>     restored in a state compatible with the CPU emulator
>   */
> @@ -119,10 +107,8 @@ static inline int handle_cpu_signal(uintptr_t pc, 
> unsigned long address,
>      /* now we have a real cpu fault */
>      cpu_restore_state(cpu, pc);
>  
> -    /* we restore the process signal mask as the sigreturn should
> -       do it (XXX: use sigsetjmp) */
>      sigprocmask(SIG_SETMASK, old_set, NULL);
> -    exception_action(cpu);
> +    cpu_loop_exit(cpu);
>  
>      /* never comes here */
>      return 1;