From: "Pavel Dovgaluk" <Pavel.Dovgaluk@ispras.ru>
To: 'Aurelien Jarno' <aurelien@aurel32.net>
Cc: pbonzini@redhat.com, rth7680@gmail.com, leon.alrae@imgtec.com,
qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2 0/3] Fix exceptions handling for MIPS and i386
Date: Thu, 18 Jun 2015 09:18:38 +0300 [thread overview]
Message-ID: <001001d0a98e$9dd07940$d9716bc0$@Dovgaluk@ispras.ru> (raw)
In-Reply-To: <20150617132446.GI931@aurel32.net>
> From: Aurelien Jarno [mailto:aurelien@aurel32.net]
> On 2015-06-17 15:41, Pavel Dovgalyuk wrote:
> > This set of patches fixes exception handling for MIPS and i386 targets.
> > These targets contain instructions that break correct execution in
> > icount/TCG modes (MIPS) and in regular TCG mode (i386).
>
> Just to be clear, this is not something specific to MIPS and i386.
> Every target which call cpu_loop_exit without doing a retranslation is
> affected by the icount bug.
True. But I haven't checked other platforms yet.
> > 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.
>
> Also just to be clear, the way you propose is just one way to fix the
> issue. The other way (which is used for all similar instructions) is to
> call just before the helper:
> gen_update_cc_op(s);
> gen_jmp_im(pc_start - s->cs_base);
This would work only for regular execution, not for icount one.
> > 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.
>
> While I think it's the correct way for load/stores or FPU exceptions, I
> am not convinced we should do that in all cases. Retranslation has a
> cost, and when the exception is likely to occur, it's better to save the
> CPU state instead of going for a retranslation. Actually we had to
> rollback such a change on SH4, as it has some performances issues. See
> commit 1012740098d0307ce5d957ebbe9a7f020da7f574.
Ok, I'll double check the patch to make translation to stop when exception
is likely to occur after current instruction.
> One way to fix that would be to reduce the cost of retranslation, for
> example by using a kind of exception table that we generate at the same
> time of the TB.
What exactly do you mean?
Pavel Dovgalyuk
next prev parent reply other threads:[~2015-06-18 6:18 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-17 12:41 [Qemu-devel] [PATCH v2 0/3] Fix exceptions handling for MIPS and i386 Pavel Dovgalyuk
2015-06-17 12:42 ` [Qemu-devel] [PATCH v2 1/3] softmmu: add helper function to pass through retaddr Pavel Dovgalyuk
2015-06-17 12:53 ` Paolo Bonzini
2015-06-18 5:17 ` Pavel Dovgaluk
2015-06-18 8:16 ` Paolo Bonzini
2015-06-18 8:20 ` Aurelien Jarno
2015-06-18 9:24 ` Pavel Dovgaluk
2015-06-18 9:30 ` Paolo Bonzini
2015-06-18 9:33 ` Pavel Dovgaluk
2015-06-18 9:35 ` Paolo Bonzini
2015-06-17 12:42 ` [Qemu-devel] [PATCH v2 2/3] target-mips: exceptions handling in icount mode Pavel Dovgalyuk
2015-06-17 13:05 ` Aurelien Jarno
2015-06-17 12:42 ` [Qemu-devel] [PATCH v2 3/3] target-i386: fix memory operations in helpers Pavel Dovgalyuk
2015-06-17 13:27 ` Aurelien Jarno
2015-06-17 13:24 ` [Qemu-devel] [PATCH v2 0/3] Fix exceptions handling for MIPS and i386 Aurelien Jarno
2015-06-18 6:18 ` Pavel Dovgaluk [this message]
2015-06-17 14:19 ` Aurelien Jarno
2015-06-18 7:12 ` Pavel Dovgaluk
2015-06-18 8:16 ` Aurelien Jarno
2015-06-18 8:58 ` Pavel Dovgaluk
2015-06-18 9:08 ` Aurelien Jarno
2015-06-18 9:29 ` Paolo Bonzini
2015-06-18 9:42 ` Aurelien Jarno
2015-06-18 10:02 ` Paolo Bonzini
2015-06-18 17:42 ` Aurelien Jarno
2015-06-19 5:09 ` Pavel Dovgaluk
2015-06-19 8:22 ` Aurelien Jarno
[not found] ` <55826f70.2215370a.4634.ffff91b2SMTPIN_ADDED_BROKEN@mx.google.com>
2015-06-18 7:51 ` Peter Maydell
2015-06-18 7:56 ` Pavel Dovgaluk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='001001d0a98e$9dd07940$d9716bc0$@Dovgaluk@ispras.ru' \
--to=pavel.dovgaluk@ispras.ru \
--cc=aurelien@aurel32.net \
--cc=leon.alrae@imgtec.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=rth7680@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.