diff mbox series

target/i386: fix translation for icount mode

Message ID 20180920071702.22477.43980.stgit@pasha-VirtualBox (mailing list archive)
State New, archived
Headers show
Series target/i386: fix translation for icount mode | expand

Commit Message

Pavel Dovgalyuk Sept. 20, 2018, 7:17 a.m. UTC
This patch fixes the checking of boundary crossing instructions.
In icount mode only first instruction of the block may cross
the page boundary to keep the translation deterministic.
These conditions already existed, but compared the wrong variable.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 target/i386/translate.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Paolo Bonzini Sept. 20, 2018, 2:03 p.m. UTC | #1
On 20/09/2018 09:17, Pavel Dovgalyuk wrote:
> This patch fixes the checking of boundary crossing instructions.
> In icount mode only first instruction of the block may cross
> the page boundary to keep the translation deterministic.
> These conditions already existed, but compared the wrong variable.
> 
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> ---
>  target/i386/translate.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index 1f9d1d9..c946bc4 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -8510,10 +8510,10 @@ static void i386_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
>             chance to happen */
>          dc->base.is_jmp = DISAS_TOO_MANY;
>      } else if ((tb_cflags(dc->base.tb) & CF_USE_ICOUNT)
> -               && ((dc->base.pc_next & TARGET_PAGE_MASK)
> -                   != ((dc->base.pc_next + TARGET_MAX_INSN_SIZE - 1)
> +               && ((pc_next & TARGET_PAGE_MASK)
> +                   != ((pc_next + TARGET_MAX_INSN_SIZE - 1)
>                         & TARGET_PAGE_MASK)
> -                   || (dc->base.pc_next & ~TARGET_PAGE_MASK) == 0)) {
> +                   || (pc_next & ~TARGET_PAGE_MASK) == 0)) {
>          /* Do not cross the boundary of the pages in icount mode,
>             it can cause an exception. Do it only when boundary is
>             crossed by the first instruction in the block.
> 

Queued, but perhaps this check should be applied to the generic code?...

Paolo
Pavel Dovgalyuk Sept. 21, 2018, 5:19 a.m. UTC | #2
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> On 20/09/2018 09:17, Pavel Dovgalyuk wrote:
> > This patch fixes the checking of boundary crossing instructions.
> > In icount mode only first instruction of the block may cross
> > the page boundary to keep the translation deterministic.
> > These conditions already existed, but compared the wrong variable.
> >
> > Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> > ---
> >  target/i386/translate.c |    6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/target/i386/translate.c b/target/i386/translate.c
> > index 1f9d1d9..c946bc4 100644
> > --- a/target/i386/translate.c
> > +++ b/target/i386/translate.c
> > @@ -8510,10 +8510,10 @@ static void i386_tr_translate_insn(DisasContextBase *dcbase,
> CPUState *cpu)
> >             chance to happen */
> >          dc->base.is_jmp = DISAS_TOO_MANY;
> >      } else if ((tb_cflags(dc->base.tb) & CF_USE_ICOUNT)
> > -               && ((dc->base.pc_next & TARGET_PAGE_MASK)
> > -                   != ((dc->base.pc_next + TARGET_MAX_INSN_SIZE - 1)
> > +               && ((pc_next & TARGET_PAGE_MASK)
> > +                   != ((pc_next + TARGET_MAX_INSN_SIZE - 1)
> >                         & TARGET_PAGE_MASK)
> > -                   || (dc->base.pc_next & ~TARGET_PAGE_MASK) == 0)) {
> > +                   || (pc_next & ~TARGET_PAGE_MASK) == 0)) {
> >          /* Do not cross the boundary of the pages in icount mode,
> >             it can cause an exception. Do it only when boundary is
> >             crossed by the first instruction in the block.
> >
> 
> Queued, but perhaps this check should be applied to the generic code?...

It maybe target-specific. Here is the ARM code:

    if (dc->base.is_jmp == DISAS_NEXT
        && (dc->pc - dc->page_start >= TARGET_PAGE_SIZE
            || (dc->pc - dc->page_start >= TARGET_PAGE_SIZE - 3
                && insn_crosses_page(env, dc)))) {
        dc->base.is_jmp = DISAS_TOO_MANY;
    }

Pavel Dovgalyuk
diff mbox series

Patch

diff --git a/target/i386/translate.c b/target/i386/translate.c
index 1f9d1d9..c946bc4 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -8510,10 +8510,10 @@  static void i386_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
            chance to happen */
         dc->base.is_jmp = DISAS_TOO_MANY;
     } else if ((tb_cflags(dc->base.tb) & CF_USE_ICOUNT)
-               && ((dc->base.pc_next & TARGET_PAGE_MASK)
-                   != ((dc->base.pc_next + TARGET_MAX_INSN_SIZE - 1)
+               && ((pc_next & TARGET_PAGE_MASK)
+                   != ((pc_next + TARGET_MAX_INSN_SIZE - 1)
                        & TARGET_PAGE_MASK)
-                   || (dc->base.pc_next & ~TARGET_PAGE_MASK) == 0)) {
+                   || (pc_next & ~TARGET_PAGE_MASK) == 0)) {
         /* Do not cross the boundary of the pages in icount mode,
            it can cause an exception. Do it only when boundary is
            crossed by the first instruction in the block.