[Qemu-devel] [PATCH] hw/ptimer: Don't wrap around counter for expired timer that uses tick handler

[Qemu-devel] [PATCH] hw/ptimer: Don't wrap around counter for expired timer that uses tick handler

[Dmitry Osipenko]

Software should see timer counter wrap around only after IRQ being triggered.
Change returned counter value to "1" for the expired timer and avoid returning
wrapped around counter value in periodic mode for the timer that has bottom-half
handler setup, assuming it is IRQ handler.

This fixes regression introduced by the commit 5a50307 ("hw/ptimer: Perform
counter wrap around if timer already expired") on SPARC emulated machine as
reported by Mark Cave-Ayland.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 hw/core/ptimer.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index 7f89001..620ac2e 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -98,10 +98,10 @@ uint64_t ptimer_get_count(ptimer_state *s)
         bool oneshot = (s->enabled == 2);
 
         /* Figure out the current counter value.  */
-        if (expired && (oneshot || use_icount)) {
+        if (expired && (oneshot || use_icount || s->bh != NULL)) {
             /* Prevent timer underflowing if it should already have
                triggered.  */
-            counter = 0;
+            counter = 1;
         } else {
             uint64_t rem;
             uint64_t div;
@@ -148,7 +148,9 @@ uint64_t ptimer_get_count(ptimer_state *s)
 
             if (expired && counter != 0) {
                 /* Wrap around periodic counter.  */
-                counter = s->limit - (counter - 1) % s->limit;
+                counter = s->delta = s->limit - (counter - 1) % s->limit;
+                /* Re-arm timer according to the wrapped around value.  */
+                ptimer_reload(s);
             }
         }
     } else {
-- 
2.9.0

Re: [Qemu-devel] [PATCH] hw/ptimer: Don't wrap around counter for expired timer that uses tick handler

[Mark Cave-Ayland]

On 24/06/16 21:29, Dmitry Osipenko wrote:

> Software should see timer counter wrap around only after IRQ being triggered.
> Change returned counter value to "1" for the expired timer and avoid returning
> wrapped around counter value in periodic mode for the timer that has bottom-half
> handler setup, assuming it is IRQ handler.
>
> This fixes regression introduced by the commit 5a50307 ("hw/ptimer: Perform
> counter wrap around if timer already expired") on SPARC emulated machine as
> reported by Mark Cave-Ayland.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  hw/core/ptimer.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
> index 7f89001..620ac2e 100644
> --- a/hw/core/ptimer.c
> +++ b/hw/core/ptimer.c
> @@ -98,10 +98,10 @@ uint64_t ptimer_get_count(ptimer_state *s)
>          bool oneshot = (s->enabled == 2);
>
>          /* Figure out the current counter value.  */
> -        if (expired && (oneshot || use_icount)) {
> +        if (expired && (oneshot || use_icount || s->bh != NULL)) {
>              /* Prevent timer underflowing if it should already have
>                 triggered.  */
> -            counter = 0;
> +            counter = 1;
>          } else {
>              uint64_t rem;
>              uint64_t div;
> @@ -148,7 +148,9 @@ uint64_t ptimer_get_count(ptimer_state *s)
>
>              if (expired && counter != 0) {
>                  /* Wrap around periodic counter.  */
> -                counter = s->limit - (counter - 1) % s->limit;
> +                counter = s->delta = s->limit - (counter - 1) % s->limit;
> +                /* Re-arm timer according to the wrapped around value.  */
> +                ptimer_reload(s);
>              }
>          }
>      } else {
>

Hi Dmitry,

Thanks for the patch, however I am unable to apply this to git master 
with git am or directly using patch:

$ patch -p1 < timer.eml
(Stripping trailing CRs from patch.)
patching file hw/core/ptimer.c
Hunk #1 FAILED at 98.
Hunk #2 succeeded at 143 (offset -5 lines).
1 out of 2 hunks FAILED -- saving rejects to file hw/core/ptimer.c.rej

Does it need to be rebased before it can be applied?


ATB,

Mark.

Re: [Qemu-devel] [PATCH] hw/ptimer: Don't wrap around counter for expired timer that uses tick handler

[Dmitry Osipenko]

On 25.06.2016 13:04, Mark Cave-Ayland wrote:
> On 24/06/16 21:29, Dmitry Osipenko wrote:
> 
>> Software should see timer counter wrap around only after IRQ being triggered.
>> Change returned counter value to "1" for the expired timer and avoid returning
>> wrapped around counter value in periodic mode for the timer that has bottom-half
>> handler setup, assuming it is IRQ handler.
>>
>> This fixes regression introduced by the commit 5a50307 ("hw/ptimer: Perform
>> counter wrap around if timer already expired") on SPARC emulated machine as
>> reported by Mark Cave-Ayland.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  hw/core/ptimer.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
>> index 7f89001..620ac2e 100644
>> --- a/hw/core/ptimer.c
>> +++ b/hw/core/ptimer.c
>> @@ -98,10 +98,10 @@ uint64_t ptimer_get_count(ptimer_state *s)
>>          bool oneshot = (s->enabled == 2);
>>
>>          /* Figure out the current counter value.  */
>> -        if (expired && (oneshot || use_icount)) {
>> +        if (expired && (oneshot || use_icount || s->bh != NULL)) {
>>              /* Prevent timer underflowing if it should already have
>>                 triggered.  */
>> -            counter = 0;
>> +            counter = 1;
>>          } else {
>>              uint64_t rem;
>>              uint64_t div;
>> @@ -148,7 +148,9 @@ uint64_t ptimer_get_count(ptimer_state *s)
>>
>>              if (expired && counter != 0) {
>>                  /* Wrap around periodic counter.  */
>> -                counter = s->limit - (counter - 1) % s->limit;
>> +                counter = s->delta = s->limit - (counter - 1) % s->limit;
>> +                /* Re-arm timer according to the wrapped around value.  */
>> +                ptimer_reload(s);
>>              }
>>          }
>>      } else {
>>
> 
> Hi Dmitry,
> 
> Thanks for the patch, however I am unable to apply this to git master with git
> am or directly using patch:
> 
> $ patch -p1 < timer.eml
> (Stripping trailing CRs from patch.)
> patching file hw/core/ptimer.c
> Hunk #1 FAILED at 98.
> Hunk #2 succeeded at 143 (offset -5 lines).
> 1 out of 2 hunks FAILED -- saving rejects to file hw/core/ptimer.c.rej
> 
> Does it need to be rebased before it can be applied?
> 
> 
> ATB,
> 
> Mark.
> 

Yes, but it's my overlook. I made it on top of tainted branch, will send V2.
Sorry and thanks for trying it and letting me know.

-- 
Dmitry