[PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000

[PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000

[Bharat Bhushan]

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.bhushan@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

RE: [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000

[Liu Yu-B13201]

 

> -----Original Message-----
> From: kvm-ppc-owner@vger.kernel.org 
> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Bharat Bhushan
> Sent: Wednesday, October 19, 2011 12:16 PM
> To: agraf@suse.de
> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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.bhushan@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.

Thanks,
Yu

RE: [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000

[Bhushan Bharat-R65777]

> -----Original Message-----
> From: Liu Yu-B13201
> Sent: Wednesday, October 19, 2011 4:28 PM
> To: Bhushan Bharat-R65777; agraf@suse.de
> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@gmail.com; Bhushan Bharat-
> R65777
> Subject: RE: [PATCH v2] Fix DEC truncation for greater than
> 0xffff_ffff/1000
> 
> 
> 
> > -----Original Message-----
> > From: kvm-ppc-owner@vger.kernel.org
> > [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Bharat Bhushan
> > Sent: Wednesday, October 19, 2011 12:16 PM
> > To: agraf@suse.de
> > Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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.bhushan@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)
 
Above values are not consistent but always in a delta of ~+-.002%.

Thanks
-Bharat

RE: [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000

[Liu Yu-B13201]

 

> -----Original Message-----
> From: Bhushan Bharat-R65777 
> Sent: Thursday, October 20, 2011 2:41 PM
> To: Liu Yu-B13201; agraf@suse.de
> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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; agraf@suse.de
> > Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@gmail.com; 
> Bhushan Bharat-
> > R65777
> > Subject: RE: [PATCH v2] Fix DEC truncation for greater than
> > 0xffff_ffff/1000
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: kvm-ppc-owner@vger.kernel.org
> > > [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Bharat Bhushan
> > > Sent: Wednesday, October 19, 2011 12:16 PM
> > > To: agraf@suse.de
> > > Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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.bhushan@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.

Thanks,
Yu

RE: [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000

[Bhushan Bharat-R65777]

> -----Original Message-----
> From: Liu Yu-B13201
> Sent: Thursday, October 20, 2011 12:35 PM
> To: Bhushan Bharat-R65777; agraf@suse.de
> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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; agraf@suse.de
> > Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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; agraf@suse.de
> > > Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@gmail.com;
> > Bhushan Bharat-
> > > R65777
> > > Subject: RE: [PATCH v2] Fix DEC truncation for greater than
> > > 0xffff_ffff/1000
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: kvm-ppc-owner@vger.kernel.org
> > > > [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Bharat Bhushan
> > > > Sent: Wednesday, October 19, 2011 12:16 PM
> > > > To: agraf@suse.de
> > > > Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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.bhushan@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.

Thanks
-Bharat

Re: [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000

[Alexander Graf]

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; agraf@suse.de
>> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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; agraf@suse.de
>>> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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; agraf@suse.de
>>>> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@gmail.com;
>>> Bhushan Bharat-
>>>> R65777
>>>> Subject: RE: [PATCH v2] Fix DEC truncation for greater than
>>>> 0xffff_ffff/1000
>>>> 
>>>> 
>>>> 
>>>>> -----Original Message-----
>>>>> From: kvm-ppc-owner@vger.kernel.org
>>>>> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Bharat Bhushan
>>>>> Sent: Wednesday, October 19, 2011 12:16 PM
>>>>> To: agraf@suse.de
>>>>> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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.bhushan@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.


Alex

RE: [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000

[Liu Yu-B13201]

 

> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de] 
> Sent: Monday, October 31, 2011 12:21 AM
> To: Bhushan Bharat-R65777
> Cc: Liu Yu-B13201; kvm-ppc@vger.kernel.org; bharatb.yadav@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; agraf@suse.de
> >> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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; agraf@suse.de
> >>> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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; agraf@suse.de
> >>>> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@gmail.com;
> >>> Bhushan Bharat-
> >>>> R65777
> >>>> Subject: RE: [PATCH v2] Fix DEC truncation for greater than
> >>>> 0xffff_ffff/1000
> >>>> 
> >>>> 
> >>>> 
> >>>>> -----Original Message-----
> >>>>> From: kvm-ppc-owner@vger.kernel.org
> >>>>> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of 
> Bharat Bhushan
> >>>>> Sent: Wednesday, October 19, 2011 12:16 PM
> >>>>> To: agraf@suse.de
> >>>>> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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.bhushan@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.liu@freescale.com>

Thanks,
Yu

Re: [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000

[Alexander Graf]

On 10/31/2011 06:54 AM, Liu Yu-B13201 wrote:
>
>
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Monday, October 31, 2011 12:21 AM
>> To: Bhushan Bharat-R65777
>> Cc: Liu Yu-B13201; kvm-ppc@vger.kernel.org; bharatb.yadav@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; agraf@suse.de
>>>> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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; agraf@suse.de
>>>>> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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; agraf@suse.de
>>>>>> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@gmail.com;
>>>>> Bhushan Bharat-
>>>>>> R65777
>>>>>> Subject: RE: [PATCH v2] Fix DEC truncation for greater than
>>>>>> 0xffff_ffff/1000
>>>>>>
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: kvm-ppc-owner@vger.kernel.org
>>>>>>> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of
>> Bharat Bhushan
>>>>>>> Sent: Wednesday, October 19, 2011 12:16 PM
>>>>>>> To: agraf@suse.de
>>>>>>> Cc: kvm-ppc@vger.kernel.org; bharatb.yadav@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.bhushan@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.liu@freescale.com>

Thanks, applied to kvm-ppc-next.


Alex