Re: [Qemu-devel] [PATCH] i386: fix breakpoints handling in icount mode

[Pavel Dovgaluk]

> From: Frederic Konrad [mailto:address@hidden
> On 22/10/2014 13:38, Pavel Dovgalyuk wrote:
> 
> Hi Pavel,
> > This patch fixes instructions counting when execution is stopped on
> > breakpoint (e.g. set from gdb). Without a patch extra instruction is 
> > translated
> > and icount is incremented by invalid value (which equals to number of
> > executed instructions + 1).
> >
> > Signed-off-by: Pavel Dovgalyuk <address@hidden>
> > ---
> >   target-i386/translate.c |    3 ++-
> >   1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/target-i386/translate.c b/target-i386/translate.c
> > index 1284173..193cf9f 100644
> > --- a/target-i386/translate.c
> > +++ b/target-i386/translate.c
> > @@ -8000,7 +8000,7 @@ static inline void 
> > gen_intermediate_code_internal(X86CPU *cpu,
> >                   if (bp->pc == pc_ptr &&
> >                       !((bp->flags & BP_CPU) && (tb->flags & HF_RF_MASK))) {
> >                       gen_debug(dc, pc_ptr - dc->cs_base);
> > -                    break;
> > +                    goto done_generating;
> This makes sense to me.
> But I don't see why you don't just "break" like the other instruction in
> this loop?

Single break will just exit the breakpoints iteration loop. I'll need an 
additional flag
to break the translation loop. ARM does the same thing, anyway :)

> >                   }
> >               }
> >           }
> > @@ -8049,6 +8049,7 @@ static inline void 
> > gen_intermediate_code_internal(X86CPU *cpu,
> >               break;
> >           }
> >       }
> > +done_generating:
> >       if (tb->cflags & CF_LAST_IO)
> >           gen_io_end();
> Is there any reason why you don't jump over this two lines in case of a
> breakpoint?

Shouldn't we switch off can_do_io flag if it was switched on?

> 
> >       gen_tb_end(tb, num_insns);
> >
> >
> 
> I'll give it a try later and I'll let you know.

Thanks.

Pavel Dovgalyuk