* RE: [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000
2011-10-19 4:28 [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000 Bharat Bhushan
@ 2011-10-19 10:57 ` Liu Yu-B13201
2011-10-20 6:40 ` Bhushan Bharat-R65777
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Liu Yu-B13201 @ 2011-10-19 10:57 UTC (permalink / raw)
To: kvm-ppc
> -----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
^ permalink raw reply [flat|nested] 8+ messages in thread* RE: [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000
2011-10-19 4:28 [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000 Bharat Bhushan
2011-10-19 10:57 ` Liu Yu-B13201
@ 2011-10-20 6:40 ` Bhushan Bharat-R65777
2011-10-20 7:05 ` Liu Yu-B13201
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Bhushan Bharat-R65777 @ 2011-10-20 6:40 UTC (permalink / raw)
To: kvm-ppc
> -----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
^ permalink raw reply [flat|nested] 8+ messages in thread* RE: [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000
2011-10-19 4:28 [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000 Bharat Bhushan
2011-10-19 10:57 ` Liu Yu-B13201
2011-10-20 6:40 ` Bhushan Bharat-R65777
@ 2011-10-20 7:05 ` Liu Yu-B13201
2011-10-20 7:18 ` Bhushan Bharat-R65777
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Liu Yu-B13201 @ 2011-10-20 7:05 UTC (permalink / raw)
To: kvm-ppc
> -----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
^ permalink raw reply [flat|nested] 8+ messages in thread* RE: [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000
2011-10-19 4:28 [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000 Bharat Bhushan
` (2 preceding siblings ...)
2011-10-20 7:05 ` Liu Yu-B13201
@ 2011-10-20 7:18 ` Bhushan Bharat-R65777
2011-10-30 16:20 ` Alexander Graf
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Bhushan Bharat-R65777 @ 2011-10-20 7:18 UTC (permalink / raw)
To: kvm-ppc
> -----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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000
2011-10-19 4:28 [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000 Bharat Bhushan
` (3 preceding siblings ...)
2011-10-20 7:18 ` Bhushan Bharat-R65777
@ 2011-10-30 16:20 ` Alexander Graf
2011-10-31 5:54 ` Liu Yu-B13201
2011-11-10 17:11 ` Alexander Graf
6 siblings, 0 replies; 8+ messages in thread
From: Alexander Graf @ 2011-10-30 16:20 UTC (permalink / raw)
To: kvm-ppc
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
^ permalink raw reply [flat|nested] 8+ messages in thread* RE: [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000
2011-10-19 4:28 [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000 Bharat Bhushan
` (4 preceding siblings ...)
2011-10-30 16:20 ` Alexander Graf
@ 2011-10-31 5:54 ` Liu Yu-B13201
2011-11-10 17:11 ` Alexander Graf
6 siblings, 0 replies; 8+ messages in thread
From: Liu Yu-B13201 @ 2011-10-31 5:54 UTC (permalink / raw)
To: kvm-ppc
> -----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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000
2011-10-19 4:28 [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000 Bharat Bhushan
` (5 preceding siblings ...)
2011-10-31 5:54 ` Liu Yu-B13201
@ 2011-11-10 17:11 ` Alexander Graf
6 siblings, 0 replies; 8+ messages in thread
From: Alexander Graf @ 2011-11-10 17:11 UTC (permalink / raw)
To: kvm-ppc
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
^ permalink raw reply [flat|nested] 8+ messages in thread