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

2016-06-25 Thread 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 
>> ---
>>  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



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

2016-06-25 Thread 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 
---
 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.




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

2016-06-24 Thread 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 
---
 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