[Qemu-devel] [PATCH 0/3] mips: Handle late r4k timers

[Qemu-devel] [PATCH 0/3] mips: Handle late r4k timers

[edgar.iglesias]

From: Edgar E. Iglesias <edgar.iglesias@gmail.com>

Linux runs a test to check if the internal r4k timer is usable prior
to enabling it. The test basically starts a timer, repeatedly
compares cp0_count with cp0_compare and finally when the
timer should have hit it verifies that the irq line is asserted.
Depending on host timing, qemu may fail this test. On my x86_64 host,
we basically always fail it.

The following set of patches makes qemu pass the test reliably.

Edgar E. Iglesias (3):
  mips: Break TBs after mfc0_count
  mips: Break out cpu_mips_timer_expire
  mips: Expire late timers when reading cp0_count

 hw/mips_timer.c         |   43 +++++++++++++++++++++++++++++--------------
 target-mips/translate.c |    4 +++-
 2 files changed, 32 insertions(+), 15 deletions(-)

-- 
1.7.2.2

[Qemu-devel] [PATCH 1/3] mips: Break TBs after mfc0_count

[edgar.iglesias]

From: Edgar E. Iglesias <edgar.iglesias@gmail.com>

Break the TB after reading the count register. This makes it
possible to take timer interrupts immediately after a read of
a possibly expired timer.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
---
 target-mips/translate.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index cce77be..313cc29 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -3410,8 +3410,10 @@ static void gen_mfc0 (CPUState *env, DisasContext *ctx, TCGv arg, int reg, int s
             gen_helper_mfc0_count(arg);
             if (use_icount) {
                 gen_io_end();
-                ctx->bstate = BS_STOP;
             }
+            /* Break the TB to be able to take timer interrupts immediately
+               after reading count.  */
+            ctx->bstate = BS_STOP;
             rn = "Count";
             break;
         /* 6,7 are implementation dependent */
-- 
1.7.2.2

[Qemu-devel] [PATCH 2/3] mips: Break out cpu_mips_timer_expire

[edgar.iglesias]

From: Edgar E. Iglesias <edgar.iglesias@gmail.com>

Reorganize for future patches, no functional change.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
---
 hw/mips_timer.c |   36 ++++++++++++++++++++++--------------
 1 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/hw/mips_timer.c b/hw/mips_timer.c
index e3beee8..8c32087 100644
--- a/hw/mips_timer.c
+++ b/hw/mips_timer.c
@@ -42,16 +42,6 @@ uint32_t cpu_mips_get_random (CPUState *env)
 }
 
 /* MIPS R4K timer */
-uint32_t cpu_mips_get_count (CPUState *env)
-{
-    if (env->CP0_Cause & (1 << CP0Ca_DC))
-        return env->CP0_Count;
-    else
-        return env->CP0_Count +
-            (uint32_t)muldiv64(qemu_get_clock(vm_clock),
-                               TIMER_FREQ, get_ticks_per_sec());
-}
-
 static void cpu_mips_timer_update(CPUState *env)
 {
     uint64_t now, next;
@@ -64,6 +54,27 @@ static void cpu_mips_timer_update(CPUState *env)
     qemu_mod_timer(env->timer, next);
 }
 
+/* Expire the timer.  */
+static void cpu_mips_timer_expire(CPUState *env)
+{
+    cpu_mips_timer_update(env);
+    if (env->insn_flags & ISA_MIPS32R2) {
+        env->CP0_Cause |= 1 << CP0Ca_TI;
+    }
+    qemu_irq_raise(env->irq[(env->CP0_IntCtl >> CP0IntCtl_IPTI) & 0x7]);
+}
+
+uint32_t cpu_mips_get_count (CPUState *env)
+{
+    if (env->CP0_Cause & (1 << CP0Ca_DC)) {
+        return env->CP0_Count;
+    } else {
+        return env->CP0_Count +
+            (uint32_t)muldiv64(qemu_get_clock(vm_clock),
+                               TIMER_FREQ, get_ticks_per_sec());
+    }
+}
+
 void cpu_mips_store_count (CPUState *env, uint32_t count)
 {
     if (env->CP0_Cause & (1 << CP0Ca_DC))
@@ -116,11 +127,8 @@ static void mips_timer_cb (void *opaque)
        the comparator value.  Offset the count by one to avoid immediately
        retriggering the callback before any virtual time has passed.  */
     env->CP0_Count++;
-    cpu_mips_timer_update(env);
+    cpu_mips_timer_expire(env);
     env->CP0_Count--;
-    if (env->insn_flags & ISA_MIPS32R2)
-        env->CP0_Cause |= 1 << CP0Ca_TI;
-    qemu_irq_raise(env->irq[(env->CP0_IntCtl >> CP0IntCtl_IPTI) & 0x7]);
 }
 
 void cpu_mips_clock_init (CPUState *env)
-- 
1.7.2.2

[Qemu-devel] [PATCH 3/3] mips: Expire late timers when reading cp0_count

[edgar.iglesias]

From: Edgar E. Iglesias <edgar.iglesias@gmail.com>

When reading cp0_count from a timer with a late trigger that should
already have expired, expire it and raise the timer irq.

This makes it possible for guest code (e.g, Linux) that first read
cp0_count, then compare it with cp0_compare and check for raised
timer interrupt lines to run reliably.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
---
 hw/mips_timer.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/mips_timer.c b/hw/mips_timer.c
index 8c32087..4e5c0b5 100644
--- a/hw/mips_timer.c
+++ b/hw/mips_timer.c
@@ -69,9 +69,16 @@ uint32_t cpu_mips_get_count (CPUState *env)
     if (env->CP0_Cause & (1 << CP0Ca_DC)) {
         return env->CP0_Count;
     } else {
+        uint64_t now;
+
+        now = qemu_get_clock(vm_clock);
+        if (qemu_timer_expired(env->timer, now)) {
+            /* The timer has already expired.  */
+            cpu_mips_timer_expire(env);
+        }
+
         return env->CP0_Count +
-            (uint32_t)muldiv64(qemu_get_clock(vm_clock),
-                               TIMER_FREQ, get_ticks_per_sec());
+            (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
     }
 }
 
-- 
1.7.2.2

[Qemu-devel] Re: [PATCH 3/3] mips: Expire late timers when reading cp0_count

[Edgar E. Iglesias]

On Tue, Jan 18, 2011 at 12:29:42AM +0100, edgar.iglesias@gmail.com wrote:
> From: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> 
> When reading cp0_count from a timer with a late trigger that should
> already have expired, expire it and raise the timer irq.
> 
> This makes it possible for guest code (e.g, Linux) that first read
> cp0_count, then compare it with cp0_compare and check for raised
> timer interrupt lines to run reliably.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>

Sorry sent the wrong version of this one. It's supposed to be the
following:

commit 139330de404209528712fd703952c0b5ad4459a1
Author: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Date:   Tue Jan 18 00:12:22 2011 +0100

    mips: Expire late timers when reading cp0_count
    
    When reading cp0_count from a timer with a late trigger that should
    already have expired, expire it and raise the timer irq.
    
    This makes it possible for guest code (e.g, Linux) that first read
    cp0_count, then compare it with cp0_compare and check for raised
    timer interrupt lines to run reliably.
    
    Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>

diff --git a/hw/mips_timer.c b/hw/mips_timer.c
index 8c32087..9c95f28 100644
--- a/hw/mips_timer.c
+++ b/hw/mips_timer.c
@@ -69,9 +69,17 @@ uint32_t cpu_mips_get_count (CPUState *env)
     if (env->CP0_Cause & (1 << CP0Ca_DC)) {
         return env->CP0_Count;
     } else {
+        uint64_t now;
+
+        now = qemu_get_clock(vm_clock);
+        if (qemu_timer_pending(env->timer)
+            && qemu_timer_expired(env->timer, now)) {
+            /* The timer has already expired.  */
+            cpu_mips_timer_expire(env);
+        }
+
         return env->CP0_Count +
-            (uint32_t)muldiv64(qemu_get_clock(vm_clock),
-                               TIMER_FREQ, get_ticks_per_sec());
+            (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
     }
 }
 
 

Re: [Qemu-devel] [PATCH 1/3] mips: Break TBs after mfc0_count

[Aurelien Jarno]

On Tue, Jan 18, 2011 at 12:29:40AM +0100, edgar.iglesias@gmail.com wrote:
> From: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> 
> Break the TB after reading the count register. This makes it
> possible to take timer interrupts immediately after a read of
> a possibly expired timer.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> ---
>  target-mips/translate.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index cce77be..313cc29 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -3410,8 +3410,10 @@ static void gen_mfc0 (CPUState *env, DisasContext *ctx, TCGv arg, int reg, int s
>              gen_helper_mfc0_count(arg);
>              if (use_icount) {
>                  gen_io_end();
> -                ctx->bstate = BS_STOP;
>              }
> +            /* Break the TB to be able to take timer interrupts immediately
> +               after reading count.  */
> +            ctx->bstate = BS_STOP;
>              rn = "Count";
>              break;
>          /* 6,7 are implementation dependent */

This looks fine, however it should probably be done the same way for
dmfc0 on 64-bit MIPS.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] Re: [PATCH 2/3] mips: Break out cpu_mips_timer_expire

[Aurelien Jarno]

On Tue, Jan 18, 2011 at 12:29:41AM +0100, edgar.iglesias@gmail.com wrote:
> From: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> 
> Reorganize for future patches, no functional change.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> ---
>  hw/mips_timer.c |   36 ++++++++++++++++++++++--------------
>  1 files changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/mips_timer.c b/hw/mips_timer.c
> index e3beee8..8c32087 100644
> --- a/hw/mips_timer.c
> +++ b/hw/mips_timer.c
> @@ -42,16 +42,6 @@ uint32_t cpu_mips_get_random (CPUState *env)
>  }
>  
>  /* MIPS R4K timer */
> -uint32_t cpu_mips_get_count (CPUState *env)
> -{
> -    if (env->CP0_Cause & (1 << CP0Ca_DC))
> -        return env->CP0_Count;
> -    else
> -        return env->CP0_Count +
> -            (uint32_t)muldiv64(qemu_get_clock(vm_clock),
> -                               TIMER_FREQ, get_ticks_per_sec());
> -}
> -
>  static void cpu_mips_timer_update(CPUState *env)
>  {
>      uint64_t now, next;
> @@ -64,6 +54,27 @@ static void cpu_mips_timer_update(CPUState *env)
>      qemu_mod_timer(env->timer, next);
>  }
>  
> +/* Expire the timer.  */
> +static void cpu_mips_timer_expire(CPUState *env)
> +{
> +    cpu_mips_timer_update(env);
> +    if (env->insn_flags & ISA_MIPS32R2) {
> +        env->CP0_Cause |= 1 << CP0Ca_TI;
> +    }
> +    qemu_irq_raise(env->irq[(env->CP0_IntCtl >> CP0IntCtl_IPTI) & 0x7]);
> +}
> +
> +uint32_t cpu_mips_get_count (CPUState *env)
> +{
> +    if (env->CP0_Cause & (1 << CP0Ca_DC)) {
> +        return env->CP0_Count;
> +    } else {
> +        return env->CP0_Count +
> +            (uint32_t)muldiv64(qemu_get_clock(vm_clock),
> +                               TIMER_FREQ, get_ticks_per_sec());
> +    }
> +}
> +
>  void cpu_mips_store_count (CPUState *env, uint32_t count)
>  {
>      if (env->CP0_Cause & (1 << CP0Ca_DC))
> @@ -116,11 +127,8 @@ static void mips_timer_cb (void *opaque)
>         the comparator value.  Offset the count by one to avoid immediately
>         retriggering the callback before any virtual time has passed.  */
>      env->CP0_Count++;
> -    cpu_mips_timer_update(env);
> +    cpu_mips_timer_expire(env);
>      env->CP0_Count--;
> -    if (env->insn_flags & ISA_MIPS32R2)
> -        env->CP0_Cause |= 1 << CP0Ca_TI;
> -    qemu_irq_raise(env->irq[(env->CP0_IntCtl >> CP0IntCtl_IPTI) & 0x7]);
>  }
>  
>  void cpu_mips_clock_init (CPUState *env)
> -- 
> 1.7.2.2
> 
> 

Acked-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] Re: [PATCH 3/3] mips: Expire late timers when reading cp0_count

[Aurelien Jarno]

On Tue, Jan 18, 2011 at 01:33:00AM +0100, Edgar E. Iglesias wrote:
> On Tue, Jan 18, 2011 at 12:29:42AM +0100, edgar.iglesias@gmail.com wrote:
> > From: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > 
> > When reading cp0_count from a timer with a late trigger that should
> > already have expired, expire it and raise the timer irq.
> > 
> > This makes it possible for guest code (e.g, Linux) that first read
> > cp0_count, then compare it with cp0_compare and check for raised
> > timer interrupt lines to run reliably.
> > 
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> 
> Sorry sent the wrong version of this one. It's supposed to be the
> following:
> 
> commit 139330de404209528712fd703952c0b5ad4459a1
> Author: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> Date:   Tue Jan 18 00:12:22 2011 +0100
> 
>     mips: Expire late timers when reading cp0_count
>     
>     When reading cp0_count from a timer with a late trigger that should
>     already have expired, expire it and raise the timer irq.
>     
>     This makes it possible for guest code (e.g, Linux) that first read
>     cp0_count, then compare it with cp0_compare and check for raised
>     timer interrupt lines to run reliably.
>     
>     Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> 
> diff --git a/hw/mips_timer.c b/hw/mips_timer.c
> index 8c32087..9c95f28 100644
> --- a/hw/mips_timer.c
> +++ b/hw/mips_timer.c
> @@ -69,9 +69,17 @@ uint32_t cpu_mips_get_count (CPUState *env)
>      if (env->CP0_Cause & (1 << CP0Ca_DC)) {
>          return env->CP0_Count;
>      } else {
> +        uint64_t now;
> +
> +        now = qemu_get_clock(vm_clock);
> +        if (qemu_timer_pending(env->timer)
> +            && qemu_timer_expired(env->timer, now)) {
> +            /* The timer has already expired.  */
> +            cpu_mips_timer_expire(env);
> +        }
> +
>          return env->CP0_Count +
> -            (uint32_t)muldiv64(qemu_get_clock(vm_clock),
> -                               TIMER_FREQ, get_ticks_per_sec());
> +            (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
>      }
>  }
>  

Given the TB is now ended after this instruction (due to patch 1), isn't
the interrupt handled before starting the next TB, where the interrupt
line (I guess CP0_Cause) read?

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] Re: [PATCH 3/3] mips: Expire late timers when reading cp0_count

[Edgar E. Iglesias]

On Tue, Jan 18, 2011 at 11:36:25AM +0100, Aurelien Jarno wrote:
> On Tue, Jan 18, 2011 at 01:33:00AM +0100, Edgar E. Iglesias wrote:
> > On Tue, Jan 18, 2011 at 12:29:42AM +0100, edgar.iglesias@gmail.com wrote:
> > > From: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > > 
> > > When reading cp0_count from a timer with a late trigger that should
> > > already have expired, expire it and raise the timer irq.
> > > 
> > > This makes it possible for guest code (e.g, Linux) that first read
> > > cp0_count, then compare it with cp0_compare and check for raised
> > > timer interrupt lines to run reliably.
> > > 
> > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > 
> > Sorry sent the wrong version of this one. It's supposed to be the
> > following:
> > 
> > commit 139330de404209528712fd703952c0b5ad4459a1
> > Author: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > Date:   Tue Jan 18 00:12:22 2011 +0100
> > 
> >     mips: Expire late timers when reading cp0_count
> >     
> >     When reading cp0_count from a timer with a late trigger that should
> >     already have expired, expire it and raise the timer irq.
> >     
> >     This makes it possible for guest code (e.g, Linux) that first read
> >     cp0_count, then compare it with cp0_compare and check for raised
> >     timer interrupt lines to run reliably.
> >     
> >     Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > 
> > diff --git a/hw/mips_timer.c b/hw/mips_timer.c
> > index 8c32087..9c95f28 100644
> > --- a/hw/mips_timer.c
> > +++ b/hw/mips_timer.c
> > @@ -69,9 +69,17 @@ uint32_t cpu_mips_get_count (CPUState *env)
> >      if (env->CP0_Cause & (1 << CP0Ca_DC)) {
> >          return env->CP0_Count;
> >      } else {
> > +        uint64_t now;
> > +
> > +        now = qemu_get_clock(vm_clock);
> > +        if (qemu_timer_pending(env->timer)
> > +            && qemu_timer_expired(env->timer, now)) {
> > +            /* The timer has already expired.  */
> > +            cpu_mips_timer_expire(env);
> > +        }
> > +
> >          return env->CP0_Count +
> > -            (uint32_t)muldiv64(qemu_get_clock(vm_clock),
> > -                               TIMER_FREQ, get_ticks_per_sec());
> > +            (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
> >      }
> >  }
> >  
> 
> Given the TB is now ended after this instruction (due to patch 1), isn't
> the interrupt handled before starting the next TB, where the interrupt
> line (I guess CP0_Cause) read?

Hi,

The problem here is different. Due to host timing granularity, the
timer might expire later than it's precise scheduled time. If that
happens, get_count will return a count value that goes beyond the
trigger time but the interrupt may come later (when the host timer
expires).

This patch catches that case and expires the timer in-band, raising
the timer interrupt if needed.

Cheers

Re: [Qemu-devel] [PATCH 1/3] mips: Break TBs after mfc0_count

[Edgar E. Iglesias]

On Tue, Jan 18, 2011 at 11:34:28AM +0100, Aurelien Jarno wrote:
> On Tue, Jan 18, 2011 at 12:29:40AM +0100, edgar.iglesias@gmail.com wrote:
> > From: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > 
> > Break the TB after reading the count register. This makes it
> > possible to take timer interrupts immediately after a read of
> > a possibly expired timer.
> > 
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > ---
> >  target-mips/translate.c |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> > 
> > diff --git a/target-mips/translate.c b/target-mips/translate.c
> > index cce77be..313cc29 100644
> > --- a/target-mips/translate.c
> > +++ b/target-mips/translate.c
> > @@ -3410,8 +3410,10 @@ static void gen_mfc0 (CPUState *env, DisasContext *ctx, TCGv arg, int reg, int s
> >              gen_helper_mfc0_count(arg);
> >              if (use_icount) {
> >                  gen_io_end();
> > -                ctx->bstate = BS_STOP;
> >              }
> > +            /* Break the TB to be able to take timer interrupts immediately
> > +               after reading count.  */
> > +            ctx->bstate = BS_STOP;
> >              rn = "Count";
> >              break;
> >          /* 6,7 are implementation dependent */
> 
> This looks fine, however it should probably be done the same way for
> dmfc0 on 64-bit MIPS.

You're right, I'll fix that. Thanks.

Re: [Qemu-devel] Re: [PATCH 3/3] mips: Expire late timers when reading cp0_count

[Aurelien Jarno]

On Tue, Jan 18, 2011 at 11:41:54AM +0100, Edgar E. Iglesias wrote:
> On Tue, Jan 18, 2011 at 11:36:25AM +0100, Aurelien Jarno wrote:
> > On Tue, Jan 18, 2011 at 01:33:00AM +0100, Edgar E. Iglesias wrote:
> > > On Tue, Jan 18, 2011 at 12:29:42AM +0100, edgar.iglesias@gmail.com wrote:
> > > > From: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > > > 
> > > > When reading cp0_count from a timer with a late trigger that should
> > > > already have expired, expire it and raise the timer irq.
> > > > 
> > > > This makes it possible for guest code (e.g, Linux) that first read
> > > > cp0_count, then compare it with cp0_compare and check for raised
> > > > timer interrupt lines to run reliably.
> > > > 
> > > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > > 
> > > Sorry sent the wrong version of this one. It's supposed to be the
> > > following:
> > > 
> > > commit 139330de404209528712fd703952c0b5ad4459a1
> > > Author: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > > Date:   Tue Jan 18 00:12:22 2011 +0100
> > > 
> > >     mips: Expire late timers when reading cp0_count
> > >     
> > >     When reading cp0_count from a timer with a late trigger that should
> > >     already have expired, expire it and raise the timer irq.
> > >     
> > >     This makes it possible for guest code (e.g, Linux) that first read
> > >     cp0_count, then compare it with cp0_compare and check for raised
> > >     timer interrupt lines to run reliably.
> > >     
> > >     Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > > 
> > > diff --git a/hw/mips_timer.c b/hw/mips_timer.c
> > > index 8c32087..9c95f28 100644
> > > --- a/hw/mips_timer.c
> > > +++ b/hw/mips_timer.c
> > > @@ -69,9 +69,17 @@ uint32_t cpu_mips_get_count (CPUState *env)
> > >      if (env->CP0_Cause & (1 << CP0Ca_DC)) {
> > >          return env->CP0_Count;
> > >      } else {
> > > +        uint64_t now;
> > > +
> > > +        now = qemu_get_clock(vm_clock);
> > > +        if (qemu_timer_pending(env->timer)
> > > +            && qemu_timer_expired(env->timer, now)) {
> > > +            /* The timer has already expired.  */
> > > +            cpu_mips_timer_expire(env);
> > > +        }
> > > +
> > >          return env->CP0_Count +
> > > -            (uint32_t)muldiv64(qemu_get_clock(vm_clock),
> > > -                               TIMER_FREQ, get_ticks_per_sec());
> > > +            (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
> > >      }
> > >  }
> > >  
> > 
> > Given the TB is now ended after this instruction (due to patch 1), isn't
> > the interrupt handled before starting the next TB, where the interrupt
> > line (I guess CP0_Cause) read?
> 
> Hi,
> 
> The problem here is different. Due to host timing granularity, the
> timer might expire later than it's precise scheduled time. If that
> happens, get_count will return a count value that goes beyond the
> trigger time but the interrupt may come later (when the host timer
> expires).
> 
> This patch catches that case and expires the timer in-band, raising
> the timer interrupt if needed.
> 

Ok, thanks for the explanation.

Acked-by: Aurelien Jarno <aurelien@aurel32.net>


-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 1/3] mips: Break TBs after mfc0_count

[Edgar E. Iglesias]

On Tue, Jan 18, 2011 at 11:34:28AM +0100, Aurelien Jarno wrote:
> On Tue, Jan 18, 2011 at 12:29:40AM +0100, edgar.iglesias@gmail.com wrote:
> > From: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > 
> > Break the TB after reading the count register. This makes it
> > possible to take timer interrupts immediately after a read of
> > a possibly expired timer.
> > 
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> > ---
> >  target-mips/translate.c |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> > 
> > diff --git a/target-mips/translate.c b/target-mips/translate.c
> > index cce77be..313cc29 100644
> > --- a/target-mips/translate.c
> > +++ b/target-mips/translate.c
> > @@ -3410,8 +3410,10 @@ static void gen_mfc0 (CPUState *env, DisasContext *ctx, TCGv arg, int reg, int s
> >              gen_helper_mfc0_count(arg);
> >              if (use_icount) {
> >                  gen_io_end();
> > -                ctx->bstate = BS_STOP;
> >              }
> > +            /* Break the TB to be able to take timer interrupts immediately
> > +               after reading count.  */
> > +            ctx->bstate = BS_STOP;
> >              rn = "Count";
> >              break;
> >          /* 6,7 are implementation dependent */
> 
> This looks fine, however it should probably be done the same way for
> dmfc0 on 64-bit MIPS.

Thanks for the quick review, I've pushed the series with this
suggested change.

Cheers