From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:51909)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mtosatti@redhat.com>) id 1QIMCl-0006aQ-CW
	for qemu-devel@nongnu.org; Fri, 06 May 2011 10:40:32 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mtosatti@redhat.com>) id 1QIMCk-00068R-5I
	for qemu-devel@nongnu.org; Fri, 06 May 2011 10:40:31 -0400
Received: from mx1.redhat.com ([209.132.183.28]:5974)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mtosatti@redhat.com>) id 1QIMCj-000688-UU
	for qemu-devel@nongnu.org; Fri, 06 May 2011 10:40:30 -0400
Date: Fri, 6 May 2011 11:39:49 -0300
From: Marcelo Tosatti <mtosatti@redhat.com>
Message-ID: <20110506143948.GA7452@amt.cnet>
References: <20110503190357.GA10617@amt.cnet>
	<388629436.349154.1304582839092.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <388629436.349154.1304582839092.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com>
Subject: Re: [Qemu-devel] [PATCH v3 5/5] hpet 'driftfix': add code in
 hpet_timer() to compensate delayed callbacks and coalesced interrupts
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Ulrich Obergfell <uobergfe@redhat.com>
Cc: aliguori@us.ibm.com, kvm@vger.kernel.org, jan kiszka <jan.kiszka@siemens.com>, qemu-devel@nongnu.org, gcosta@redhat.com, avi@redhat.com

On Thu, May 05, 2011 at 04:07:19AM -0400, Ulrich Obergfell wrote:
> 
> Hi Marcelo,
>  
> > Other than that, shouldnt reset accounting variables to init state on
> > write to GLOBAL_ENABLE_CFG / writes to main counter?
> 
> I'd suggest to initialize/reset the driftfix-related fields in the
> 'HPETTimer' structure (including the backlog of unaccounted ticks)
> in the following situations.
> 
> 
> - When the guest o/s sets the 'CFG_ENABLE' bit (overall enable) in
>   the General Configuration Register.
> 
>   This is the place in hpet_ram_writel():
> 
>         case HPET_CFG:
>             ...
>             if (activating_bit(old_val, new_val, HPET_CFG_ENABLE)) {
>                 ...
>                 for (i = 0; i < s->num_timers; i++) {
>                     if ((&s->timer[i])->cmp != ~0ULL) {
>                         // initialize / reset fields here
>                         hpet_set_timer(&s->timer[i]);
> 
> 
> - When the guest o/s sets the 'TN_ENABLE' bit (timer N interrupt
>   enable) in the Timer N Configuration and Capabilities Register.
> 
>   This is the place in hpet_ram_writel():
> 
>         case HPET_TN_CFG:
>             ...
>             if (activating_bit(old_val, new_val, HPET_TN_ENABLE)) {
>                 // initialize / reset fields here
>                 hpet_set_timer(timer);
> 
> 
> This should cover cases such as ...
> 
> - first time initialization of HPET & timers during guest o/s boot.
>   (includes kexec and reboot)
> 
> - if a guest o/s stops and restarts the timer.
>   (for example, to change the comparator register value or
>    to switch a timer between periodic mode and non-periodic mode)
> 
> 
> Regards,
> 
> Uli

Uli, looks good.