From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Date: Thu, 10 Nov 2011 17:11:22 +0000 Subject: Re: [PATCH v2] Fix DEC truncation for greater than 0xffff_ffff/1000 Message-Id: <4EBC05BA.5080201@suse.de> List-Id: References: <1318997766-28951-1-git-send-email-bharat.bhushan@freescale.com> In-Reply-To: <1318997766-28951-1-git-send-email-bharat.bhushan@freescale.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: kvm-ppc@vger.kernel.org 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 >>>>>>> --- >>>>>>> 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 Thanks, applied to kvm-ppc-next. Alex