From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ulrich Obergfell Subject: Re: [PATCH v3 5/5] hpet 'driftfix': add code in hpet_timer() to compensate delayed callbacks and coalesced interrupts Date: Wed, 4 May 2011 04:06:59 -0400 (EDT) Message-ID: <541075535.332616.1304496419890.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> References: <20110503190357.GA10617@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: aliguori@us.ibm.com, kvm@vger.kernel.org, jan kiszka , qemu-devel@nongnu.org, gcosta@redhat.com, avi@redhat.com To: Marcelo Tosatti Return-path: In-Reply-To: <20110503190357.GA10617@amt.cnet> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org List-Id: kvm.vger.kernel.org Hi Marcelo, > Whats prev_period for, since in practice the period will not change > between interrupts (OS programs comparator once, or perhaps twice > during bootup) ? 'prev_period' is needed if a guest o/s changes the comparator period 'on the fly' (without stopping and restarting the timer). guest o/s changes period | ti(n-1) | ti(n) ti(n+1) | v | | +---------------------+------------------------------+ <--- prev_period ---> <---------- period ----------> The idea is that each timer interrupt represents a certain quantum of time (the comparator period). If a guest o/s changes the period between timer interrupt 'n-1' and timer interrupt 'n', I think the new value should not take effect before timer interrupt 'n'. Timer interrupt 'n' still represents the old/previous quantum, and timer interrupt 'n+1' represents the new quantum. Hence, the patch decrements 'ticks_not_accounted' by 'prev_period' and sets 'prev_period' to 'period' when an interrupt was delivered to the guest o/s. + irq_delivered = update_irq(t, 1); + if (irq_delivered) { + t->ticks_not_accounted -= t->prev_period; + t->prev_period = t->period; + } else { Most of the time 'prev_period' is equal to 'period'. It should only be different in the scenario shown above. Regards, Uli