> From: Aurelien Jarno [mailto:aurel...@aurel32.net] > On 2015-06-15 07:53, Pavel Dovgaluk wrote: > > > From: Aurelien Jarno [mailto:aurel...@aurel32.net] > > > On 2015-06-10 11:33, Pavel Dovgalyuk wrote: > > > > This patch fixes exception handling in MIPS. > > > > MIPS instructions generate several types of exceptions. > > > > When exception is generated, it breaks the execution of the current > > > > translation > > > > block. Implementation of the exceptions handling in MIPS does not > > > > correctly > > > > restore icount for the instruction which caused the exception. In most > > > > cases > > > > icount will be decreased by the value equal to the size of TB. > > > > > > I don't think it is correct. There is no real point of always doing > > > retranslation for an exception triggered from the helpers, especially > > > when the CPU state has been saved before anyway? > > > > As you know, icount is processed as follows: > > > > TB: > > if icount < n then exit > > icount -= n > > instr1 > > instr2 > > ... > > instrn > > exit > > > > When one of the instructions initiates an exception, then icount should be > > restored > > and adjusted number of instructions should be subtracted instead of initial > > n. > > > > E.g., 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 > > helper_raise_exception_err into TB), then PC is not passed as retaddr and > > correct icount is not recovered. > > > > This behavior leads to incorrect values of timers and non-deterministic > > execution > > of the code. > > Ok, this therefore doesn't looks something MIPS specific, but rather a > flaw in the icount design. Instead of fixing blindly one target, we > should try to fix it globally, or if not possible at least agree on a > way to fix that for all target and provide the infrastructure for that > (for example provide load/store functions which accept a return > address). Paolo any opinion on that? > > Also retranslation has a cost (actually on MIPS we spend more time in > *retranslation* than in translation due to the way the MMU works), we > should avoid it if we already have to save the CPU state for another > reason. At least your patch should remove the code saving the CPU state > when possible if the helper uses retranslation instead.
Ok, I'll remove CPU saving where it is not actually used because of the exception. > > > > This patch passes pointer to the translation block internals to the > > > > exception > > > > handler. It allows correct restoring of the icount value. > > > > > > Your patch doesn't do that for all the helpers, for example all the > > > memory access helpers. It probably improves the situation but therefore > > > doesn't fix it. > > > > Right, I missed these helpers. I'll add them in the next version. > > Except we currently don't provide a way in the softmmu code to provide a > return address... Softmmu passes return address into tlb_fill function, which passes it to raise_exception. I also fixed HELPER_LD_ATOMIC and HELPER_ST_ATOMIC to recover PC value. Do you mean something different? Pavel Dovgalyuk