Re: [Qemu-devel] implementing architectural timers using QEMU timers

[Pavel Dovgalyuk]

> From: Paolo Bonzini [mailto:address@hidden On Behalf Of Paolo Bonzini
> On 12/01/2017 13:19, Pavel Dovgalyuk wrote:
> >> From: Peter Maydell [mailto:address@hidden
> >> On 12 January 2017 at 11:28, Pavel Dovgalyuk <address@hidden> wrote:
> >>>> From: Max Filippov [mailto:address@hidden
> >>>> Ok, looks like what happens in my case is that instruction that
> >>>> sets CCOMPARE and thus changes remaining icount does not
> >>>> cause exit from the cpu_exec. So merely ending TB on
> >>>> QEMU_CLOCK_VIRTUAL timer update is not enough, I need to
> >>>> throw an exception of some kind? Or does the timer code need
> >>>> to take care of that?
> >>>
> >>> Yes, it seems that you should end the block with an exception,
> >>> to allow icount loop recalculate the timeouts.
> >>
> >> Really? The ARM translate.c doesn't generate an exception.
> >> It just does
> >>  gen_io_end();
> >>  gen_lookup_tb();
> >>
> >> (so we force a lookup of the next TB, but don't throw an
> >> exception of any kind).
> >
> > Maybe I missing something. As far as I understand, changing the virtual 
> > timer
> > notifies the iothread and os_host_main_loop_wait kicks the CPU thread.
> >
> > But within that period of time before changing the timer and kicking the 
> > thread
> > CPU may proceed some instructions and the timer will be expired if it was 
> > set
> > to one of the soonest instructions.
> 
> My understanding (which may be wrong!) was that after gen_io_end you
> would exit with TB_EXIT_ICOUNT_EXPIRED and cpu->icount_decr.u16.high =
> -1, but indeed I don't see anything that calls cpu_interrupt in that
> path.
> 
> Maybe something like this:
> 
> diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h
> index 050de59..c20d193 100644
> --- a/include/exec/gen-icount.h
> +++ b/include/exec/gen-icount.h
> @@ -73,6 +73,9 @@ static inline void gen_io_end(void)
>  {
>      TCGv_i32 tmp = tcg_const_i32(0);
>      tcg_gen_st_i32(tmp, cpu_env, -ENV_OFFSET + offsetof(CPUState, 
> can_do_io));
> +    /* Make the next TB exit immediately with TB_EXIT_ICOUNT_EXPIRED.  */
> +    tcg_gen_st16_i32(-1, cpu_env,
> +                     -ENV_OFFSET + offsetof(CPUState, icount_decr.u16.high));
>      tcg_temp_free_i32(tmp);
>  }

I guess it will help, but this seems too general.
There are some IO cases that do not need breaking the execution.
E.g., rdtsc in x86 does not change any timers, but is translated as IO 
operation.
However, I think it doesn't called too often, therefore it will not affect on 
performance too much.

Pavel Dovgalyuk