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

[Pavel Dovgalyuk]

On 28.10.2021 22:26, Richard Henderson wrote:
> On 10/28/21 4:48 AM, Pavel Dovgalyuk wrote:
> > +        if (cpu->cflags_next_tb == -1
> > +            && (!use_icount || !(tb->cflags & CF_USE_ICOUNT)
> > +                || cpu_neg(cpu)->icount_decr.u16.low >= tb->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;
> > +        }
>
> I can't see that this will work.

The issue was the following:
- icount is enabled
- watchpoint hit
- next block should contain a single instruction to stop the execution 
after memory access
- tb with one instruction is translated, cflags_next_tb is reset
- main loop breaks the execution for some reason
- after resuming the execution, we are trying to find new tb without any 
filter in cflags
- tb with multiple instructions is executed (instead of previously 
generated)

> We've been asked to exit to the main loop; probably for an interrupt.  
> The next thing that's likely to happen is that cpu_cc->do_interrupt will 
> adjust cpu state to continue in the guest interrupt handler.  The 
> cflags_next_tb flag that you're setting up is not relevant to that context.
>
> This seems related to Phil's reported problem
>
>    https://gitlab.com/qemu-project/qemu/-/issues/245
>
> in which an interrupt arrives before we finish processing the watchpoint.

Right, this is similar.

> I *think* we need to make cflags_next_tb != -1 be treated like any other 
> interrupt disable bit, and delay delivery of the interrupt.  Which also 
> means that we should not check for exit_tb at the beginning of any TB we 
> generate for the watchpoint step.
>
> I simply haven't taken the time to investigate this properly.
>
>
> r~