[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/5] target-i386: Move user-mode exception actio
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 5/5] target-i386: Move user-mode exception actions out of user-exec.c |
Date: |
Tue, 17 May 2016 14:47:31 +0100 |
On 16 May 2016 at 18:54, Sergey Fedorov <address@hidden> wrote:
> 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().
I looked a bit closer at this, and I think that we're actually
OK to just not set exception_next_eip (or to set it wrongly,
according to taste). In the softmmu equivalent code path,
tlb_fill() will call raise_exception_err_ra() for a page
fault. This code path also ends up doing
env->exception_next_eip = env->eip
before it has done a cpu_restore_state() [in this case the
restore-state happens in cpu_loop_exit_restore()]. But this
is OK because we only use env->exception_next_eip as the
value to pass into do_interrupt_all(), which states that
next_eip is only valid if is_int is true (ie we got here from
an int instruction or syscall).
For do_interrupt_user() it is also true that next_eip only
matters if is_int (or if into is EXCP_SYSCALL), so we can
safely just not update exception_next_eip here in
x86_cpu_handle_mmu_fault().
So I think the best approach is:
(1) in this patch, set exception_next_eip to -1 (clearly
indicating that it is not supposed to be valid)
(2) add a comment to do_interrupt_user() like the one for
do_interrupt_all() noting that next_eip is only valid in
some cases.
thanks
-- PMM
[Qemu-devel] [PATCH 2/5] user-exec: Push resume-from-signal code out to handle_cpu_signal(), Peter Maydell, 2016/05/16