On 10/31/2011 06:54 AM, Liu Yu-B13201 wrote:


-----Original Message-----
From: Alexander Graf [mailto:ag...@suse.de]
Sent: Monday, October 31, 2011 12:21 AM
To: Bhushan Bharat-R65777
Cc: Liu Yu-B13201; kvm-ppc@vger.kernel.org; bharatb.ya...@gmail.com
Subject: Re: [PATCH v2] Fix DEC truncation for greater than
0xffff_ffff/1000


On 20.10.2011, at 09:18, Bhushan Bharat-R65777 wrote:


-----Original Message-----
From: Liu Yu-B13201
Sent: Thursday, October 20, 2011 12:35 PM
To: Bhushan Bharat-R65777; ag...@suse.de
Cc: kvm-ppc@vger.kernel.org; bharatb.ya...@gmail.com
Subject: RE: [PATCH v2] Fix DEC truncation for greater than
0xffff_ffff/1000



-----Original Message-----
From: Bhushan Bharat-R65777
Sent: Thursday, October 20, 2011 2:41 PM
To: Liu Yu-B13201; ag...@suse.de
Cc: kvm-ppc@vger.kernel.org; bharatb.ya...@gmail.com
Subject: RE: [PATCH v2] Fix DEC truncation for greater than
0xffff_ffff/1000



-----Original Message-----
From: Liu Yu-B13201
Sent: Wednesday, October 19, 2011 4:28 PM
To: Bhushan Bharat-R65777; ag...@suse.de
Cc: kvm-ppc@vger.kernel.org; bharatb.ya...@gmail.com;
Bhushan Bharat-
R65777
Subject: RE: [PATCH v2] Fix DEC truncation for greater than
0xffff_ffff/1000



-----Original Message-----
From: kvm-ppc-ow...@vger.kernel.org
[mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of
Bharat Bhushan
Sent: Wednesday, October 19, 2011 12:16 PM
To: ag...@suse.de
Cc: kvm-ppc@vger.kernel.org; bharatb.ya...@gmail.com; Bhushan
Bharat-R65777
Subject: [PATCH v2] Fix DEC truncation for greater than
0xffff_ffff/1000

kvmppc_emulate_dec() uses dec_nsec of type unsigned
long and does
below calculation:

        dec_nsec = vcpu->arch.dec;
        dec_nsec *= 1000;
This will truncate if DEC value "vcpu->arch.dec" is greater than
0xffff_ffff/1000.
For example : For tb_ticks_per_usec = 4a, we can not set
decrementer
more than ~58ms.

Signed-off-by: Bharat Bhushan<bharat.bhus...@freescale.com>
---
arch/powerpc/kvm/emulate.c |   12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kvm/emulate.c
b/arch/powerpc/kvm/emulate.c
index 8af3bad..e7f3da4 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -84,6 +84,7 @@ static bool kvmppc_dec_enabled(struct kvm_vcpu
*vcpu)  void kvmppc_emulate_dec(struct kvm_vcpu *vcpu)  {
        unsigned long dec_nsec;
+       unsigned long long dec_time;

        pr_debug("mtDEC: %x\n", vcpu->arch.dec);  #ifdef
CONFIG_PPC_BOOK3S @@ -103,11 +104,12 @@ void
kvmppc_emulate_dec(struct
kvm_vcpu *vcpu)
                * host ticks. */

                hrtimer_try_to_cancel(&vcpu->arch.dec_timer);
-               dec_nsec = vcpu->arch.dec;
-               dec_nsec *= 1000;
-               dec_nsec /= tb_ticks_per_usec;
-               hrtimer_start(&vcpu->arch.dec_timer,
ktime_set(0, dec_nsec),
-                             HRTIMER_MODE_REL);
+               dec_time = vcpu->arch.dec;
+               dec_time *= 1000;
+               do_div(dec_time, tb_ticks_per_usec);
+               dec_nsec = do_div(dec_time, NSEC_PER_SEC);
+               hrtimer_start(&vcpu->arch.dec_timer,
+                       ktime_set(dec_time, dec_nsec),
HRTIMER_MODE_REL);
                vcpu->arch.dec_jiffies = get_tb();
        } else {
                hrtimer_try_to_cancel(&vcpu->arch.dec_timer);
--
1.7.0.4

How does this impact performance?
64bits multiplication and division looks slow.

I tried running below test as guest, with and without
this patch and
tried to find latency added by this patch. Also I run
this for a list
of timeouts (1, 2 , 4, 8, 16, 32ms) one by one.

- get TB (say a).
- set decrementer in auto reload mode.
- wait for 1000 timebase interrupts.
- Get timebase delta (b = get_tb - a).

                (b1     -   b2)<=>  b1 with this patch and b2
without this patch. And roughly I found any impact. For example:
For 1ms =  ( 48a19d8 -  48a1459)  = 0x57f  = .0018% For 32ms =
(90fdfa23 - 90fdfe79)  = -(0x456)
Doesn't (b1 - b2) mean difference of the last one
interrupt between have
patch and havenot patch?
The time of previous 999 interrupts is hidden in the cpu idle time.


Probably I have not described properly. b1 and b2 are
delta, not timestamp. In this case I run this test with patch
        Print on console the total time (in TB tick) for which
this test runs. Which includes time of all 1000 interrupts.
Then I exit and rerun the above test case without patch
Then mannualy calculated difference/percentage etc.

Also if you see timebase delta, it suggest that it is not
timebase difference of one decrementer.

So Yu are you ok with this patch? If so, please ack.

Acked-by: Liu Yu<yu....@freescale.com>

Thanks, applied to kvm-ppc-next.


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to