Re: [PATCH v2 1/3] icount: preserve cflags when custom tb is about to execute

[Pavel Dovgalyuk]

On 11.11.2021 15:20, Alex Bennée wrote:
> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes:
>
> > When debugging with the watchpoints, qemu may need to create
> > TB with single instruction. This is achieved by setting cpu->cflags_next_tb.
> > But when this block is about to execute, it may be interrupted by another
> > thread. In this case cflags will be lost and next executed TB will not
> > be the special one.
> > This patch checks TB exit reason and restores cflags_next_tb to allow
> > finding the interrupted block.
> >
> > Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> > ---
> >   accel/tcg/cpu-exec.c |   10 ++++++++++
> >   1 file changed, 10 insertions(+)
> >
> > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> > index 2d14d02f6c..df12452b8f 100644
> > --- a/accel/tcg/cpu-exec.c
> > +++ b/accel/tcg/cpu-exec.c
> > @@ -846,6 +846,16 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, 
> > TranslationBlock *tb,
> >            * cpu_handle_interrupt.  cpu_handle_interrupt will also
> >            * clear cpu->icount_decr.u16.high.
> >            */
> > +        if (cpu->cflags_next_tb == -1
> > +            && (!use_icount || !(tb->cflags & CF_USE_ICOUNT)
>
> Why check use_icount here? The cflags should always have CF_USE_ICOUNT
> set when icount is enabled. Lets not over complicate the inverted ||
> tests we have here.

Not really. Sometimes we use non-icount blocks in icount mode.
But AFAIR they are used only for triggering the exeptions, but not for 
real execution.

> > +                || cpu_neg(cpu)->icount_decr.u16.low >= tb->icount))
> > {
>
> Is u16.low ever set when icount isn't enabled?

This condition is checked for icount mode only.
u16.low is not used without icount.

> > +            /*
> > +             * icount is disabled or there are enough instructions
> > +             * in the budget, do not retranslate this block with
> > +             * different parameters.
> > +             */
> > +            cpu->cflags_next_tb = tb->cflags;
> > +        }
> >           return;
> >       }
> >