Re: [Qemu-devel] [PATCH v4 0/5] Fix exceptions handling for MIPS and i386

[Pavel Dovgaluk]

Forgot to fix the subject.
These patches also fix exceptions handling for PowerPC.

Pavel Dovgalyuk


> -----Original Message-----
> From: Pavel Dovgalyuk [mailto:address@hidden
> Sent: Monday, June 29, 2015 10:23 AM
> To: address@hidden
> Cc: address@hidden; address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden
> Subject: [PATCH v4 0/5] Fix exceptions handling for MIPS and i386
> 
> QEMU targets ISAs contain instruction that can break the execution
> flow with exceptions. When exception breaks the execution of the translation
> block it may corrupt PC and icount values.
> 
> This set of patches fixes exception handling for MIPS and i386 targets.
> 
> Incorrect execution for i386 is causes by exceptions raised by MMU functions.
> MMU helper functions are called from generated code and other helper
> functions. In both cases they try to get function's return address for
> restoring virtual CPU state.
> 
> When MMU helper is called from some other helper function
> (like helper_maskmov_xmm) through cpu_st* function, the return address
> will point to that helper. That is why CPU state cannot be restored in
> the case of MMU fault.
> 
> This bug can occur when maskmov instruction is located in the middle of the
> translation block.
> 
> Execution sequence for this example:
> 
> TB start:
> PC1: instr1
>      instr2
> PC2: maskmov <page fault>
>      <page fault processing>
> PC1: instr1
>      instr2
>      maskmov
> 
> At the start of TB execution guest PC points to instr1. When page fault occurs
> QEMU tries to restore guest PC (which should be equal to PC2). It reads host 
> PC
> from the call stack and checks whether it points to TB or not. Bug in ldst
> helpers implementation provides incorrect host PC, which is not located within
> the TB. That's why QEMU cannot recover guest PC and it remains the same (PC1).
> After page fault processing QEMU restarts TB and executes instr1 and instr2
> for the second time, because guest PC was not recovered.
> 
> Bugs in MIPS helper functions do not break the execution in regular TCG mode,
> because PC value is updated before calling the functions that can raise
> an exception. But icount value cannot be updated this way. Therefore
> exceptions make execution in icount mode non-determinisic.
> In icount mode every translation block looks as follows:
> 
> if icount < n then exit
> icount -= n
> instr1
> instr2
> ...
> instrn
> exit
> 
> When one of these instructions initiates an exception, icount should be
> restored and adjusted number of instructions should be subtracted from icount
> instead of initial n.
> 
> tlb_fill function passes retaddr to raise_exception, which allows restoring
> current instructions in TB and correct icount calculation.
> 
> When exception triggered with other function (e.g. by embedding call to
> exception raising helper into TB), then PC is not passed as retaddr and
> correct icount is not recovered. In such cases icount will be decreased
> by the value equal to the size of TB.
> 
> This behavior leads to incorrect values of virtual clock and
> non-deterministic execution of the code.
> 
> These patches passes pointer to the translation block code to the exception
> handler. It allows correct restoring of PC and icount values.
> 
> v4 changes:
>  * Fixed exceptions handling for PowerPC
>  * Fixed passing of mmu_idx into helpers (as suggested by Aurelien Jarno)
>  * Added cpu_loop_exit_restore function (as suggested by Aurelien Jarno)
>  * Fixed exceptions handling in compare helper functions for MIPS (as 
> suggested by Aurelien
> Jarno)
>  * Removed several CPU state saving calls for MIPS (as suggested by Aurelien 
> Jarno)
> 
> v3 changes:
>  * Modified exception handling for syscall (as suggested by Aurelien Jarno)
>  * Removed redundant calls to save_cpu_state (as suggested by Aurelien Jarno)
>  * Removed helper_call* functions from softmmu (as suggested by Paolo Bonzini)
> 
> v2 changes:
>  * Added softmmu functions to pass TB return value into memory operations 
> handlers
>  * Fixed memory operations handling for MIPS
>  * Disabled updates of the PC that are overridden with cpu_restore_state
>  * Fixed memory operations and exceptions invoked by i386 helpers
> 
> ---
> 
> Pavel Dovgalyuk (5):
>       softmmu: add helper function to pass through retaddr
>       cpu-exec: introduce loop exit with restore function
>       target-mips: improve exceptions handling
>       target-i386: fix memory operations in helpers
>       target-ppc: exceptions handling in icount mode
> 
> 
>  cpu-exec.c                       |    9
>  include/exec/cpu_ldst_template.h |   59 +++
>  include/exec/exec-all.h          |    1
>  softmmu_template.h               |    6
>  target-i386/cc_helper.c          |    2
>  target-i386/cpu.h                |    5
>  target-i386/excp_helper.c        |   21 +
>  target-i386/fpu_helper.c         |  146 ++++----
>  target-i386/helper.c             |    4
>  target-i386/int_helper.c         |   32 +-
>  target-i386/mem_helper.c         |   39 +-
>  target-i386/misc_helper.c        |   12 -
>  target-i386/ops_sse.h            |    2
>  target-i386/seg_helper.c         |  712 
> +++++++++++++++++++-------------------
>  target-i386/svm_helper.c         |    4
>  target-i386/translate.c          |   25 -
>  target-mips/cpu.h                |   23 +
>  target-mips/helper.h             |    1
>  target-mips/msa_helper.c         |  158 +++++---
>  target-mips/op_helper.c          |  179 ++++------
>  target-mips/translate.c          |  372 +++++++++-----------
>  target-ppc/cpu.h                 |    3
>  target-ppc/excp_helper.c         |   38 ++
>  target-ppc/fpu_helper.c          |  191 +++++-----
>  target-ppc/helper.h              |    1
>  target-ppc/mem_helper.c          |    6
>  target-ppc/misc_helper.c         |    8
>  target-ppc/mmu-hash64.c          |   12 -
>  target-ppc/mmu_helper.c          |   18 -
>  target-ppc/timebase_helper.c     |   20 +
>  target-ppc/translate.c           |   84 ----
>  tcg/tcg.h                        |   23 +
>  32 files changed, 1122 insertions(+), 1094 deletions(-)
> 
> --
> Pavel Dovgalyuk