[Qemu-devel] [PATCH 1.7 0/2] PPC: Fix BookE timer performance regression

[Qemu-devel] [PATCH 1.7 0/2] PPC: Fix BookE timer performance regression

[Alexander Graf]

Thanks to the new timer infrastructure we are now able to trigger timer events
and ridiculous granularities in sub-microsecond orders.

However, the BookE targets were quite unhappy about that change, showing up
to x10 slowdown on a simple Linux guest bootup test.

This patch set makes the constant timer facilities in BookE more lazy and less
fine grained than they could be. That way we're at least as good as we were in
QEMU 1.6.

Alexander Graf (2):
  PPC: Make BookE FIT/WDT timers more lazy
  PPC: BookE: Make FIT/WDT timers at best millisecond grained

 hw/ppc/ppc_booke.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 44 insertions(+), 5 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH 1/2] PPC: Make BookE FIT/WDT timers more lazy

[Alexander Graf]

Today we fire FIT and WDT timer events every time the respective bit
position in TB flips from 0 -> 1.

However, there is no need to do this if the end result would be that
we're changing a TSR bit that is set to 1 to 1 again. No guest visible
change would have occured.

So whenever we see that the TSR bit to our timer is already set, don't
even bother to update the timer that would potentially fire it off.

However, we do need to make sure that we update our timer that notifies
us of the TB flip when the respective TSR bit gets unset. In that case
we do care about the flip and need to notify the guest again. So add
a callback into our timer handlers when TSR bits get unset.

This improves performance for me when the guest is busy processing things.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppc/ppc_booke.c | 43 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/ppc_booke.c b/hw/ppc/ppc_booke.c
index 8bbfc72..28cf9fd 100644
--- a/hw/ppc/ppc_booke.c
+++ b/hw/ppc/ppc_booke.c
@@ -128,7 +128,8 @@ static uint8_t booke_get_wdt_target(CPUPPCState *env, ppc_tb_t *tb_env)
 static void booke_update_fixed_timer(CPUPPCState         *env,
                                      uint8_t           target_bit,
                                      uint64_t          *next,
-                                     struct QEMUTimer *timer)
+                                     struct QEMUTimer *timer,
+                                     int               tsr_bit)
 {
     ppc_tb_t *tb_env = env->tb_env;
     uint64_t delta_tick, ticks = 0;
@@ -136,6 +137,14 @@ static void booke_update_fixed_timer(CPUPPCState         *env,
     uint64_t period;
     uint64_t now;
 
+    if (!env->spr[SPR_BOOKE_TSR] & tsr_bit) {
+        /*
+         * Don't arm the timer again when the guest has the current
+         * interrupt still pending. Wait for it to ack it.
+         */
+        return;
+    }
+
     now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     tb  = cpu_ppc_get_tb(tb_env, now, tb_env->tb_offset);
     period = 1ULL << target_bit;
@@ -167,6 +176,7 @@ static void booke_update_fixed_timer(CPUPPCState         *env,
         (*next)++;
     }
 
+    /* Fire the next timer */
     timer_mod(timer, *next);
 }
 
@@ -200,7 +210,8 @@ static void booke_fit_cb(void *opaque)
     booke_update_fixed_timer(env,
                              booke_get_fit_target(env, tb_env),
                              &booke_timer->fit_next,
-                             booke_timer->fit_timer);
+                             booke_timer->fit_timer,
+                             TSR_FIS);
 }
 
 static void booke_wdt_cb(void *opaque)
@@ -220,15 +231,35 @@ static void booke_wdt_cb(void *opaque)
     booke_update_fixed_timer(env,
                              booke_get_wdt_target(env, tb_env),
                              &booke_timer->wdt_next,
-                             booke_timer->wdt_timer);
+                             booke_timer->wdt_timer,
+                             TSR_WIS);
 }
 
 void store_booke_tsr(CPUPPCState *env, target_ulong val)
 {
     PowerPCCPU *cpu = ppc_env_get_cpu(env);
+    ppc_tb_t *tb_env = env->tb_env;
+    booke_timer_t *booke_timer = tb_env->opaque;
 
     env->spr[SPR_BOOKE_TSR] &= ~val;
     kvmppc_clear_tsr_bits(cpu, val);
+
+    if (val & TSR_FIS) {
+        booke_update_fixed_timer(env,
+                                 booke_get_fit_target(env, tb_env),
+                                 &booke_timer->fit_next,
+                                 booke_timer->fit_timer,
+                                 TSR_FIS);
+    }
+
+    if (val & TSR_WIS) {
+        booke_update_fixed_timer(env,
+                                 booke_get_wdt_target(env, tb_env),
+                                 &booke_timer->wdt_next,
+                                 booke_timer->wdt_timer,
+                                 TSR_WIS);
+    }
+
     booke_update_irq(cpu);
 }
 
@@ -247,12 +278,14 @@ void store_booke_tcr(CPUPPCState *env, target_ulong val)
     booke_update_fixed_timer(env,
                              booke_get_fit_target(env, tb_env),
                              &booke_timer->fit_next,
-                             booke_timer->fit_timer);
+                             booke_timer->fit_timer,
+                             TSR_FIS);
 
     booke_update_fixed_timer(env,
                              booke_get_wdt_target(env, tb_env),
                              &booke_timer->wdt_next,
-                             booke_timer->wdt_timer);
+                             booke_timer->wdt_timer,
+                             TSR_WIS);
 }
 
 static void ppc_booke_timer_reset_handle(void *opaque)
-- 
1.7.12.4

[Qemu-devel] [PATCH 2/2] PPC: BookE: Make FIT/WDT timers at best microsecond grained

[Alexander Graf]

The default granularity for the FIT timer on 440 is on every 0x1000th
transition of TB from 0 to 1. Translated that means 48828 times a second.

Since interrupts are quite expensive for 440 and we don't really care
about the accuracy of the FIT to that significance, let's force FIT and
WDT to at best microsecond granularity.

This basically restores behavior as it was in QEMU 1.6, where timers
could only deal with microsecond granularities at all.

This patch greatly improves performance with the 440 target and restores
roughly the same performance level that QEMU 1.6 had for me.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppc/ppc_booke.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/ppc/ppc_booke.c b/hw/ppc/ppc_booke.c
index 28cf9fd..3038acc 100644
--- a/hw/ppc/ppc_booke.c
+++ b/hw/ppc/ppc_booke.c
@@ -174,6 +174,12 @@ static void booke_update_fixed_timer(CPUPPCState         *env,
 
     if (*next == now) {
         (*next)++;
+    } else {
+        /*
+         * There's no point to fake any granularity that's more fine grained
+         * than microseconds. Anything beyond that just overloads the system.
+         */
+        *next = MAX(*next, now + SCALE_MS);
     }
 
     /* Fire the next timer */
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 1.7 0/2] PPC: Fix BookE timer performance regression

[Stefan Weil]

Am 23.11.2013 04:53, schrieb Alexander Graf:
> Thanks to the new timer infrastructure we are now able to trigger timer events
> and ridiculous granularities in sub-microsecond orders.
>
> However, the BookE targets were quite unhappy about that change, showing up
> to x10 slowdown on a simple Linux guest bootup test.
>
> This patch set makes the constant timer facilities in BookE more lazy and less
> fine grained than they could be. That way we're at least as good as we were in
> QEMU 1.6.
>
> Alexander Graf (2):
>   PPC: Make BookE FIT/WDT timers more lazy
>   PPC: BookE: Make FIT/WDT timers at best millisecond grained
>
>  hw/ppc/ppc_booke.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 44 insertions(+), 5 deletions(-)

Good morning Alexander,

I have two small remarks:

Patch 1: Could you please use 'QemuTimer' instead of 'struct QEMUTimer'
in the modified lines of code?

Patch 2: Milliseconds instead of microseconds (commit message and code
comment)?

Cheers,
Stefan

Re: [Qemu-devel] [PATCH 1.7 0/2] PPC: Fix BookE timer performance regression

[Stefan Weil]

Am 23.11.2013 05:08, schrieb Stefan Weil:
> Good morning Alexander,
>
> I have two small remarks:
>
> Patch 1: Could you please use 'QemuTimer' instead of 'struct QEMUTimer'
> in the modified lines of code?

Sorry, QEMUTimer, of course. The 'struct' keyword is redundant and
should be avoided.

Re: [Qemu-devel] [PATCH 1.7 0/2] PPC: Fix BookE timer performance regression

[Alexander Graf]

On 23.11.2013, at 05:08, Stefan Weil <sw@weilnetz.de> wrote:

> Am 23.11.2013 04:53, schrieb Alexander Graf:
>> Thanks to the new timer infrastructure we are now able to trigger timer events
>> and ridiculous granularities in sub-microsecond orders.
>> 
>> However, the BookE targets were quite unhappy about that change, showing up
>> to x10 slowdown on a simple Linux guest bootup test.
>> 
>> This patch set makes the constant timer facilities in BookE more lazy and less
>> fine grained than they could be. That way we're at least as good as we were in
>> QEMU 1.6.
>> 
>> Alexander Graf (2):
>>  PPC: Make BookE FIT/WDT timers more lazy
>>  PPC: BookE: Make FIT/WDT timers at best millisecond grained
>> 
>> hw/ppc/ppc_booke.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 44 insertions(+), 5 deletions(-)
> 
> Good morning Alexander,
> 
> I have two small remarks:
> 
> Patch 1: Could you please use 'QemuTimer' instead of 'struct QEMUTimer'
> in the modified lines of code?
> 
> Patch 2: Milliseconds instead of microseconds (commit message and code
> comment)?

Alrighty :). Changed both and will resend. Thanks a lot for the review!


Alex