diff mbox

hw/ptimer: Don't wrap around counter for expired timer that uses tick handler

Message ID 20160624202934.702-1-digetx@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dmitry Osipenko June 24, 2016, 8:29 p.m. UTC
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(-)

Comments

Mark Cave-Ayland June 25, 2016, 10:04 a.m. UTC | #1
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.
Dmitry Osipenko June 25, 2016, 12:20 p.m. UTC | #2
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.
diff mbox

Patch

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 {