[Qemu-devel] implementing architectural timers using QEMU timers

[Qemu-devel] implementing architectural timers using QEMU timers

[Max Filippov]

Hello,

I'm trying to reimplement xtensa CCOUNT (cycle counter) and
CCOMPARE (CCOUNT-based timer interrupts) using QEMU
timers. That is CCOUNT value is derived from the
QEMU_CLOCK_VIRTUAL clock and CCOMPARE interrupts are
generated from the QEMU_CLOCK_VIRTUAL timer callbacks.
The code is here:
  https://github.com/OSLL/qemu-xtensa/commits/xtensa-ccount

I've got the following issues doing that:

- in non-icount mode I can often read CCOUNT and get a value
  that is greater than programmed CCOMPARE value, which
  means that QEMU timer must have been fired at that point, but
  no sign of timer callback being called. That is timer callback
  invocation lags behind due time.

  Is my understanding correct that there's no hard expectations
  that firing of QEMU timers will be correctly sequenced with
  readings of QEMU clock?

- I thought that could be improved in -icount mode, so I tried that.
  It is better with -icount, but it's still not 100% accurate. That is
  I was able to observe guest reading QEMU clock value that is
  past QEMU timer deadline before that timer callback was
  invoked.

  That sounds like a bug to me, is it?

- when guest sets a timer and halts itself waiting for timer
  interrupt with waiti opcode QEMU behaviour is very strange with
  -icount: regardless of the programmed timeout QEMU waits for
  about a second before it delivers interrupt, and, AFAICT,
  interrupt delivery it is not caused by the corresponding CCOUNT
  timer. I was able to track this issue down to the
  qemu_clock_use_for_deadline function, i.e. always returning true
  'fixes' that unwanted delay, but looking around the timer code
  I've got the impression that that's not the correct fix.

  Any suggestions on how to fix that?

-- 
Thanks.
-- Max

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

[Alex Bligh]

> On 9 Jan 2017, at 15:18, Max Filippov <jcmvbkbc@gmail.com> wrote:
> 
> Hello,
> 
> I'm trying to reimplement xtensa CCOUNT (cycle counter) and
> CCOMPARE (CCOUNT-based timer interrupts) using QEMU
> timers. That is CCOUNT value is derived from the
> QEMU_CLOCK_VIRTUAL clock and CCOMPARE interrupts are
> generated from the QEMU_CLOCK_VIRTUAL timer callbacks.
> The code is here:
>  https://github.com/OSLL/qemu-xtensa/commits/xtensa-ccount
> 
> I've got the following issues doing that:
> 
> - in non-icount mode I can often read CCOUNT and get a value
>  that is greater than programmed CCOMPARE value, which
>  means that QEMU timer must have been fired at that point, but
>  no sign of timer callback being called. That is timer callback
>  invocation lags behind due time.
> 
>  Is my understanding correct that there's no hard expectations
>  that firing of QEMU timers will be correctly sequenced with
>  readings of QEMU clock?

My understanding is that the intent of the API contract is that

* The timer will not fire earlier than the time requested (but
  can fire later)

* Timers on the same clock and context will fire in the order
  of their expiry time (with the ordering being undefined in
  the event the expiry times are equal).

Whether in practice any users make use of the second guarantee
above I don't know.

> 
> - I thought that could be improved in -icount mode, so I tried that.
>  It is better with -icount, but it's still not 100% accurate. That is
>  I was able to observe guest reading QEMU clock value that is
>  past QEMU timer deadline before that timer callback was
>  invoked.
> 
>  That sounds like a bug to me, is it?

Hmm... that would be a bug if it were guaranteed that the
guest reads are always perfectly in sync with the timer
itself. I don't know whether that is the case.

Even though I authored a large timer patch, I found the icount
stuff confusing.

> - when guest sets a timer and halts itself waiting for timer
>  interrupt with waiti opcode QEMU behaviour is very strange with
>  -icount: regardless of the programmed timeout QEMU waits for
>  about a second before it delivers interrupt, and, AFAICT,
>  interrupt delivery it is not caused by the corresponding CCOUNT
>  timer. I was able to track this issue down to the
>  qemu_clock_use_for_deadline function, i.e. always returning true
>  'fixes' that unwanted delay, but looking around the timer code
>  I've got the impression that that's not the correct fix.
> 
>  Any suggestions on how to fix that?

This could be someone using timerlistgroup_deadline_ns rather than
qemu_clock_deadline_ns_all.

The comment at the top of qemu_clock_deadline_ns_all says:

/* Calculate the soonest deadline across all timerlists attached
 * to the clock. This is used for the icount timeout so we
 * ignore whether or not the clock should be used in deadline
 * calculations.
 */

I'm wondering whether there is something up with that logic.

-- 
Alex Bligh

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

[Pavel Dovgalyuk]

> From: Max Filippov [mailto:jcmvbkbc@gmail.com]

> I'm trying to reimplement xtensa CCOUNT (cycle counter) and
> CCOMPARE (CCOUNT-based timer interrupts) using QEMU
> timers. That is CCOUNT value is derived from the
> QEMU_CLOCK_VIRTUAL clock and CCOMPARE interrupts are
> generated from the QEMU_CLOCK_VIRTUAL timer callbacks.
> The code is here:
>   https://github.com/OSLL/qemu-xtensa/commits/xtensa-ccount
> 
> I've got the following issues doing that:
> 
> - I thought that could be improved in -icount mode, so I tried that.
>   It is better with -icount, but it's still not 100% accurate. That is
>   I was able to observe guest reading QEMU clock value that is
>   past QEMU timer deadline before that timer callback was
>   invoked.

icount is meant to be 100% accurate.
tcg_get_icount_limit function calculates the deadline before the soonest
virtual timer and executes number of instructions that will fit this
timeout.

Maybe you need the accuracy that is higher than the precision of the virtual clock?
Every executed instruction in icount mode takes fixed period of virtual time.

Pavel Dovgalyuk

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

[Frederic Konrad]

On 01/09/2017 04:18 PM, Max Filippov wrote:
> Hello,
> 
> I'm trying to reimplement xtensa CCOUNT (cycle counter) and
> CCOMPARE (CCOUNT-based timer interrupts) using QEMU
> timers. That is CCOUNT value is derived from the
> QEMU_CLOCK_VIRTUAL clock and CCOMPARE interrupts are
> generated from the QEMU_CLOCK_VIRTUAL timer callbacks.
> The code is here:
>   https://github.com/OSLL/qemu-xtensa/commits/xtensa-ccount
> 
> I've got the following issues doing that:
> 
> - in non-icount mode I can often read CCOUNT and get a value
>   that is greater than programmed CCOMPARE value, which
>   means that QEMU timer must have been fired at that point, but
>   no sign of timer callback being called. That is timer callback
>   invocation lags behind due time.
> 
>   Is my understanding correct that there's no hard expectations
>   that firing of QEMU timers will be correctly sequenced with
>   readings of QEMU clock?
> 
> - I thought that could be improved in -icount mode, so I tried that.
>   It is better with -icount, but it's still not 100% accurate. That is
>   I was able to observe guest reading QEMU clock value that is
>   past QEMU timer deadline before that timer callback was
>   invoked.
> 
>   That sounds like a bug to me, is it?

Did you try "sleep" icount option?

eg:
-icount 1,sleep=off

Fred

> 
> - when guest sets a timer and halts itself waiting for timer
>   interrupt with waiti opcode QEMU behaviour is very strange with
>   -icount: regardless of the programmed timeout QEMU waits for
>   about a second before it delivers interrupt, and, AFAICT,
>   interrupt delivery it is not caused by the corresponding CCOUNT
>   timer. I was able to track this issue down to the
>   qemu_clock_use_for_deadline function, i.e. always returning true
>   'fixes' that unwanted delay, but looking around the timer code
>   I've got the impression that that's not the correct fix.
> 
>   Any suggestions on how to fix that?
> 

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

[Max Filippov]

On Mon, Jan 9, 2017 at 7:41 AM, Alex Bligh <alex@alex.org.uk> wrote:
>> On 9 Jan 2017, at 15:18, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> I'm trying to reimplement xtensa CCOUNT (cycle counter) and
>> CCOMPARE (CCOUNT-based timer interrupts) using QEMU
>> timers. That is CCOUNT value is derived from the
>> QEMU_CLOCK_VIRTUAL clock and CCOMPARE interrupts are
>> generated from the QEMU_CLOCK_VIRTUAL timer callbacks.
>> The code is here:
>>  https://github.com/OSLL/qemu-xtensa/commits/xtensa-ccount
>>
>> I've got the following issues doing that:
>>
>> - in non-icount mode I can often read CCOUNT and get a value
>>  that is greater than programmed CCOMPARE value, which
>>  means that QEMU timer must have been fired at that point, but
>>  no sign of timer callback being called. That is timer callback
>>  invocation lags behind due time.
>>
>>  Is my understanding correct that there's no hard expectations
>>  that firing of QEMU timers will be correctly sequenced with
>>  readings of QEMU clock?
>
> My understanding is that the intent of the API contract is that
>
> * The timer will not fire earlier than the time requested (but
>   can fire later)
>
> * Timers on the same clock and context will fire in the order
>   of their expiry time (with the ordering being undefined in
>   the event the expiry times are equal).
>
> Whether in practice any users make use of the second guarantee
> above I don't know.

Ok.

>> - I thought that could be improved in -icount mode, so I tried that.
>>  It is better with -icount, but it's still not 100% accurate. That is
>>  I was able to observe guest reading QEMU clock value that is
>>  past QEMU timer deadline before that timer callback was
>>  invoked.
>>
>>  That sounds like a bug to me, is it?
>
> Hmm... that would be a bug if it were guaranteed that the
> guest reads are always perfectly in sync with the timer
> itself. I don't know whether that is the case.

I guess that's my intuitive expectation of the determinism
offered by the -icount. Because if QEMU_CLOCK_VIRTUAL
timers are allowed to lag they cannot be used for deterministic
events, right? If that's intentional, then the question is what
can be used to generate an event at specific virtual time?

> Even though I authored a large timer patch, I found the icount
> stuff confusing.
>
>> - when guest sets a timer and halts itself waiting for timer
>>  interrupt with waiti opcode QEMU behaviour is very strange with
>>  -icount: regardless of the programmed timeout QEMU waits for
>>  about a second before it delivers interrupt, and, AFAICT,
>>  interrupt delivery it is not caused by the corresponding CCOUNT
>>  timer. I was able to track this issue down to the
>>  qemu_clock_use_for_deadline function, i.e. always returning true
>>  'fixes' that unwanted delay, but looking around the timer code
>>  I've got the impression that that's not the correct fix.
>>
>>  Any suggestions on how to fix that?
>
> This could be someone using timerlistgroup_deadline_ns rather than
> qemu_clock_deadline_ns_all.

Well, main_loop_wait uses timerlistgroup_deadline_ns. If I only
change this place to take all timers into account it works without
the unwanted timeout. But still the following comment above the
qemu_clock_use_for_deadline makes me wonder if such change
is legitimate:

 * Determine whether a clock should be used for deadline
 * calculations. Some clocks, for instance vm_clock with
 * use_icount set, do not count in nanoseconds. Such clocks
 * are not used for deadline calculations, and are presumed
 * to interrupt any poll using qemu_notify/aio_notify
 * etc.

> The comment at the top of qemu_clock_deadline_ns_all says:
>
> /* Calculate the soonest deadline across all timerlists attached
>  * to the clock. This is used for the icount timeout so we
>  * ignore whether or not the clock should be used in deadline
>  * calculations.
>  */
>
> I'm wondering whether there is something up with that logic.

-- 
Thanks.
-- Max

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

[Max Filippov]

On Tue, Jan 10, 2017 at 12:31 AM, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
>> From: Max Filippov [mailto:jcmvbkbc@gmail.com]
>
>> I'm trying to reimplement xtensa CCOUNT (cycle counter) and
>> CCOMPARE (CCOUNT-based timer interrupts) using QEMU
>> timers. That is CCOUNT value is derived from the
>> QEMU_CLOCK_VIRTUAL clock and CCOMPARE interrupts are
>> generated from the QEMU_CLOCK_VIRTUAL timer callbacks.
>> The code is here:
>>   https://github.com/OSLL/qemu-xtensa/commits/xtensa-ccount
>>
>> I've got the following issues doing that:
>>
>> - I thought that could be improved in -icount mode, so I tried that.
>>   It is better with -icount, but it's still not 100% accurate. That is
>>   I was able to observe guest reading QEMU clock value that is
>>   past QEMU timer deadline before that timer callback was
>>   invoked.
>
> icount is meant to be 100% accurate.
> tcg_get_icount_limit function calculates the deadline before the soonest
> virtual timer and executes number of instructions that will fit this
> timeout.

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?

> Maybe you need the accuracy that is higher than the precision of the virtual clock?
> Every executed instruction in icount mode takes fixed period of virtual time.

-- 
Thanks.
-- Max

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

[Max Filippov]

On Tue, Jan 10, 2017 at 1:10 AM, Frederic Konrad
<fred.konrad@greensocs.com> wrote:
> On 01/09/2017 04:18 PM, Max Filippov wrote:
>> I'm trying to reimplement xtensa CCOUNT (cycle counter) and
>> CCOMPARE (CCOUNT-based timer interrupts) using QEMU
>> timers. That is CCOUNT value is derived from the
>> QEMU_CLOCK_VIRTUAL clock and CCOMPARE interrupts are
>> generated from the QEMU_CLOCK_VIRTUAL timer callbacks.
>> The code is here:
>>   https://github.com/OSLL/qemu-xtensa/commits/xtensa-ccount
>>
>> I've got the following issues doing that:
>>
>> - I thought that could be improved in -icount mode, so I tried that.
>>   It is better with -icount, but it's still not 100% accurate. That is
>>   I was able to observe guest reading QEMU clock value that is
>>   past QEMU timer deadline before that timer callback was
>>   invoked.
>>
>>   That sounds like a bug to me, is it?
>
> Did you try "sleep" icount option?
>
> eg:
> -icount 1,sleep=off

That doesn't seem to change anything.

-- 
Thanks.
-- Max

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

[Peter Maydell]

On 10 January 2017 at 18:39, Max Filippov <jcmvbkbc@gmail.com> wrote:
> 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?

Is your code calling gen_io_start() and gen_io_end() in the right
places around where it generates code to do the CCOMPARE accesses ?
(the rules for getting icount to work right are a bit fiddly)

thanks
-- PMM

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

[Max Filippov]

On Tue, Jan 10, 2017 at 10:47 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 10 January 2017 at 18:39, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> 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?
>
> Is your code calling gen_io_start() and gen_io_end() in the right
> places around where it generates code to do the CCOMPARE accesses ?
> (the rules for getting icount to work right are a bit fiddly)

Yes, it does. Without these calls qemu aborts if I actually try to
access timers. But these functions themselves don't guarantee
anything, as they just generate code that sets can_do_io in the
env to 1 and to 0.

-- 
Thanks.
-- Max

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

[Pavel Dovgalyuk]

> From: Max Filippov [mailto:jcmvbkbc@gmail.com]
> On Tue, Jan 10, 2017 at 12:31 AM, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
> >> From: Max Filippov [mailto:jcmvbkbc@gmail.com]
> >
> >> I'm trying to reimplement xtensa CCOUNT (cycle counter) and
> >> CCOMPARE (CCOUNT-based timer interrupts) using QEMU
> >> timers. That is CCOUNT value is derived from the
> >> QEMU_CLOCK_VIRTUAL clock and CCOMPARE interrupts are
> >> generated from the QEMU_CLOCK_VIRTUAL timer callbacks.
> >> The code is here:
> >>   https://github.com/OSLL/qemu-xtensa/commits/xtensa-ccount
> >>
> >> I've got the following issues doing that:
> >>
> >> - I thought that could be improved in -icount mode, so I tried that.
> >>   It is better with -icount, but it's still not 100% accurate. That is
> >>   I was able to observe guest reading QEMU clock value that is
> >>   past QEMU timer deadline before that timer callback was
> >>   invoked.
> >
> > icount is meant to be 100% accurate.
> > tcg_get_icount_limit function calculates the deadline before the soonest
> > virtual timer and executes number of instructions that will fit this
> > timeout.
> 
> 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.

Pavel Dovgalyuk

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

[Peter Maydell]

On 12 January 2017 at 11:28, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
>> From: Max Filippov [mailto:jcmvbkbc@gmail.com]
>> 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).

thanks
-- PMM

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

[Pavel Dovgalyuk]

> From: Peter Maydell [mailto:peter.maydell@linaro.org]
> On 12 January 2017 at 11:28, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
> >> From: Max Filippov [mailto:jcmvbkbc@gmail.com]
> >> 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.

Pavel Dovgalyuk

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

[Max Filippov]

On Thu, Jan 12, 2017 at 3:28 AM, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
>> From: Max Filippov [mailto:jcmvbkbc@gmail.com]
>> On Tue, Jan 10, 2017 at 12:31 AM, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
>> >> From: Max Filippov [mailto:jcmvbkbc@gmail.com]
>> >
>> >> I'm trying to reimplement xtensa CCOUNT (cycle counter) and
>> >> CCOMPARE (CCOUNT-based timer interrupts) using QEMU
>> >> timers. That is CCOUNT value is derived from the
>> >> QEMU_CLOCK_VIRTUAL clock and CCOMPARE interrupts are
>> >> generated from the QEMU_CLOCK_VIRTUAL timer callbacks.
>> >> The code is here:
>> >>   https://github.com/OSLL/qemu-xtensa/commits/xtensa-ccount
>> >>
>> >> I've got the following issues doing that:
>> >>
>> >> - I thought that could be improved in -icount mode, so I tried that.
>> >>   It is better with -icount, but it's still not 100% accurate. That is
>> >>   I was able to observe guest reading QEMU clock value that is
>> >>   past QEMU timer deadline before that timer callback was
>> >>   invoked.
>> >
>> > icount is meant to be 100% accurate.
>> > tcg_get_icount_limit function calculates the deadline before the soonest
>> > virtual timer and executes number of instructions that will fit this
>> > timeout.
>>
>> 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.

I've added raising EXCP_YIELD in the next TB:
  http://lists.nongnu.org/archive/html/qemu-devel/2017-01/msg02811.html
With that icount timers work perfectly for me.

-- 
Thanks.
-- Max

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

[Paolo Bonzini]

On 12/01/2017 13:19, Pavel Dovgalyuk wrote:
>> From: Peter Maydell [mailto:peter.maydell@linaro.org]
>> On 12 January 2017 at 11:28, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
>>>> From: Max Filippov [mailto:jcmvbkbc@gmail.com]
>>>> 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);
 }
 

?

Paolo

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

[Pavel Dovgalyuk]

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> On 12/01/2017 13:19, Pavel Dovgalyuk wrote:
> >> From: Peter Maydell [mailto:peter.maydell@linaro.org]
> >> On 12 January 2017 at 11:28, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
> >>>> From: Max Filippov [mailto:jcmvbkbc@gmail.com]
> >>>> 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

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

[Max Filippov]

On Mon, Jan 16, 2017 at 9:19 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 12/01/2017 13:19, Pavel Dovgalyuk wrote:
>>> From: Peter Maydell [mailto:peter.maydell@linaro.org]
>>> On 12 January 2017 at 11:28, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote:
>>>>> From: Max Filippov [mailto:jcmvbkbc@gmail.com]
>>>>> 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);
>  }

With -1 moved into TCG temporary that works for me as well.

-- 
Thanks.
-- Max