Re: [Qemu-devel] regression in timer code?

[Max Filippov]

On Wed, Mar 14, 2018 at 2:07 AM, Pavel Dovgalyuk <address@hidden> wrote:
>> From: Max Filippov [mailto:address@hidden
>> the commit b39e3f34c9de7ead6a11a74aa2de78baf41d81a7
>> ("icount: fixed saving/restoring of icount warp timers") has changed
>> something that made timers test for target/xtensa unstable.
>> Specifically ccount_write case in the tests/tcg/xtensa/test_timer.S
>> now fails for me about half of the times it is run. Given that it is run
>> under -icount I guess this is a bug. Could you please take a look?
>>
>> The minimal test case is available here:
>>   http://jcmvbkbc.spb.ru/~dumb/tmp/201803131306/test_timer.tst
>> It is run as
>>   qemu-system-xtensa -M sim -cpu dc232b -nographic -semihosting
>> -icount 7  -kernel ./test_timer.tst
>
> I investigated your test and concluded the following.
> First, update_ccount is inaccurate opration because of the division
> with the remainder:
>     env->sregs[CCOUNT] = env->ccount_base +
>         (uint32_t)((now - env->time_base) *
>                    env->config->clock_freq_khz / 1000000);
>
> Therefore, the following sequence in the test may give different result 
> depending
> of the actual value of "now" variable.

But the expression above depends on the difference between the now
and env->time_base, both these values are read from
 qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) and, most importantly,
the test is 100% deterministic, i.e. it runs under -icount and there's no
external factors involved, no waiting for anything, not even interrupts.
Thus the difference must be constant, i.e. I must see consistent results.
Am I wrong somewhere?

>     rsr     a3, ccount
>     rsr     a4, ccount
>     sub     a4, a4, a3
>
> Consider the code:
>
> test ccount_write
>     rsr     a3, ccount
>     rsr     a4, ccount
>     sub     a4, a4, a3 ; usually 1, but sometimes 2 because of rounding
>     movi    a2, 0x12345678
>     wsr     a2, ccount
>     esync
>     rsr     a3, ccount
>     sub     a3, a3, a2 ; usually 3 (esync + yield + rsr), but sometimes 4 
> because of rounding
>     slli    a4, a4, 2  ; 4 or 8
>     assert  ltu, a3, a4 ; (3 or 4) < (4 or 8) ?
> test_end
>
> Therefore in some cases we get a4=4 and a3=4 that forces the test to fail.

-- 
Thanks.
-- Max