From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43567) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bL8IK-0001rm-HY for qemu-devel@nongnu.org; Thu, 07 Jul 2016 08:20:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bL8IJ-00083L-DO for qemu-devel@nongnu.org; Thu, 07 Jul 2016 08:20:40 -0400 References: <20160625123521.16752-1-digetx@gmail.com> <5814242b-8a8c-852a-51e8-268033b51200@gmail.com> <121dd825-9c64-b3b9-bca8-41bcb984ec0f@gmail.com> <340f08ea-ee9b-1284-7e7f-17d33b314885@gmail.com> From: Dmitry Osipenko Message-ID: <6493a684-c571-c8d8-3b2f-f8da719a1b54@gmail.com> Date: Thu, 7 Jul 2016 15:20:21 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] hw/ptimer: Don't wrap around counter for expired timer that uses tick handler List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: QEMU Developers , qemu-arm , Peter Crosthwaite , Mark Cave-Ayland On 07.07.2016 13:53, Peter Maydell wrote: > On 4 July 2016 at 10:55, Peter Maydell wrote: >> On 1 July 2016 at 18:49, Dmitry Osipenko wrote: >>> On 01.07.2016 19:36, Peter Maydell wrote: >>>> On 30 June 2016 at 20:01, Dmitry Osipenko wrote: >>>>> On 30.06.2016 18:02, Peter Maydell wrote: >>>>>> What I meant was: ptimer_get_count() is typically called to generate >>>>>> a value to return from a register. That's a separate thing, conceptually, >>>>>> from whether the device happens to also trigger an interrupt on timer >>>>>> expiry by passing a bh to ptimer_init(). So it's very odd for a detail >>>>>> of interrupt-on-timer-expiry (that there is a bottom half) to affect >>>>>> the value returned when you read the timer count register. >>>> >>>>> In order to handle wraparound correctly, software needs to track the moment of >>>>> the wraparound - the interrupt. If software reads wrapped around counter value >>>>> before IRQ triggered (ptimer expired), then it would assume that no wraparound >>>>> happened and won't perform counter value correction, resulting in periodic >>>>> counter "jumping" backwards. >>>> >>>> That just says you need particular behaviour between counter reads >>>> and IRQ triggers; it doesn't say that you need the behaviour to be >>>> different if the ptimer code doesn't know about the IRQ trigger. >>>> >>> >>> Okay, I already explained the reason for having two different behaviours - to >>> make polled counter value more distributed when possible. If I understand you >>> correctly, you don't like it because it is "odd" and I agree that it's a bit clumsy. >> >>> So, what we are going to do now? Would you just revert the offending commit or >>> you have some other suggestions? >> >> Well, we need to fix the regression, but basically I'm kind of >> confused at the moment. I haven't invested a lot of time in >> trying to understand the timer code, so all I can really do >> is say "this does not look like the right thing" and ask you >> to come up with a different fix for it. > I'm currently leaning to the revert solution. Unfortunately, I haven't had a chance to look at it yet, will do it today and send patch after. > My current best guess is that this condition should simply be > "if (expired) {". > Since you are insisted that wraparound isn't a needed feature - yes, that's nearly the same what the original code did :) -- Dmitry