From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [Qemu-devel] Re: [RFC: 0/2] patch for QEMU HPET periodic timer emulation to alleviate time drift Date: Mon, 07 Feb 2011 16:08:55 +0100 Message-ID: <4D500B07.1070305@siemens.com> References: <480481933.225059.1296734409954.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> <1375835067.226263.1296740625327.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> <4D4AC99A.2070803@siemens.com> <4D4B0B07.2040904@codemonkey.ws> <4D4B1CF8.8040800@web.de> <4D4B5F23.7040801@codemonkey.ws> <4D4BBF55.9060000@web.de> <4D4FE6BF.5080502@redhat.com> <4D4FEF81.1040603@codemonkey.ws> <4D4FF02F.2030309@redhat.com> <4D4FF24A.7000004@codemonkey.ws> <4D4FFD3B.2030903@siemens.com> <4D5001A0.8020503@codemonkey.ws> <4D5004FC.80000@siemens.com> <4D5007B9.7060806@codemonkey.ws> <4D500872.3070506@siemens.com> <4D50092C.1080109@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: qemu-devel , Glauber Costa , Ulrich Obergfell , kvm , Avi Kivity To: Anthony Liguori Return-path: Received: from thoth.sbs.de ([192.35.17.2]:30776 "EHLO thoth.sbs.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753309Ab1BGPJM (ORCPT ); Mon, 7 Feb 2011 10:09:12 -0500 In-Reply-To: <4D50092C.1080109@codemonkey.ws> Sender: kvm-owner@vger.kernel.org List-ID: On 2011-02-07 16:01, Anthony Liguori wrote: > On 02/07/2011 08:57 AM, Jan Kiszka wrote: >> There should rather be a special vmstate struct for PeriodicTimer, just >> like we already have for normal timers. >> > > Agreed. > >>> It's convenient because then if we lose ticks in the PeriodicTimer >>> layer, the devices have instance access to that info. When you do a >>> read() from timerfd, it returns the number of coalesced events. That's >>> the interface I had in my mind. >>> >>> We could just add a getter for PeriodicTimer and it would serve the same >>> purpose. >>> >> I'm still not sure what the device model is supposed to do with that >> information. I think at could remain private to the PeriodicTimer >> implementation (unless we want to dump some stats or such). >> > > Yeah, I've been thinking about it too and I think I agree with you. > > So here's the new proposal: > > typedef struct PeriodicTimer PeriodicTimer; > > /** > * @accumulated_ticks: the number of unacknowledged ticks in total > since the creation of the timer > **/ > typedef void (PeriodicTimerFunc)(void *opaque); Or just use QEMUTimerCB directly. BTW, QEMUPeriodicTimer* would probably be more consistent with the existing naming scheme. > > PeriodicTimer *periodic_timer_new(PeriodicTimerFunc *cb, void *opaque); > > void periodic_timer_mod(PeriodicTimer *timer, int64_t interval, TimeUnit > unit); > > /** > * @policy: the drift catch-up policy > * DRIFT_COMP_FAST, deliver next tick as soon as any > tick is acknowledged if accumulated_ticks > 1 > * DRIFT_COMP_NONE, do not change interval regardless of > accumulated ticks > * DRIFT_COMP_GRADUAL, shorten interval by half until > accumulated_ticks <= 1 > */ > void periodic_timer_set_policy(PeriodicTimer *timer, > DriftCompensationPolicy policy); > > /** > * @ticks: number of ticks to acknowledge that are currently outstanding. > **/ > void periodic_timer_ack(PeriodicTimer *timer, int ticks); > > int periodic_timer_get_accumulated_ticks(PeriodicTimer *timer); > Looks like a plan. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=41544 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PmSi4-0001zQ-61 for qemu-devel@nongnu.org; Mon, 07 Feb 2011 10:09:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PmSi3-0001Zc-5I for qemu-devel@nongnu.org; Mon, 07 Feb 2011 10:09:00 -0500 Received: from thoth.sbs.de ([192.35.17.2]:16559) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PmSi2-0001ZS-T5 for qemu-devel@nongnu.org; Mon, 07 Feb 2011 10:08:59 -0500 Message-ID: <4D500B07.1070305@siemens.com> Date: Mon, 07 Feb 2011 16:08:55 +0100 From: Jan Kiszka MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [RFC: 0/2] patch for QEMU HPET periodic timer emulation to alleviate time drift References: <480481933.225059.1296734409954.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> <1375835067.226263.1296740625327.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> <4D4AC99A.2070803@siemens.com> <4D4B0B07.2040904@codemonkey.ws> <4D4B1CF8.8040800@web.de> <4D4B5F23.7040801@codemonkey.ws> <4D4BBF55.9060000@web.de> <4D4FE6BF.5080502@redhat.com> <4D4FEF81.1040603@codemonkey.ws> <4D4FF02F.2030309@redhat.com> <4D4FF24A.7000004@codemonkey.ws> <4D4FFD3B.2030903@siemens.com> <4D5001A0.8020503@codemonkey.ws> <4D5004FC.80000@siemens.com> <4D5007B9.7060806@codemonkey.ws> <4D500872.3070506@siemens.com> <4D50092C.1080109@codemonkey.ws> In-Reply-To: <4D50092C.1080109@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Glauber Costa , Avi Kivity , qemu-devel , kvm , Ulrich Obergfell On 2011-02-07 16:01, Anthony Liguori wrote: > On 02/07/2011 08:57 AM, Jan Kiszka wrote: >> There should rather be a special vmstate struct for PeriodicTimer, just >> like we already have for normal timers. >> > > Agreed. > >>> It's convenient because then if we lose ticks in the PeriodicTimer >>> layer, the devices have instance access to that info. When you do a >>> read() from timerfd, it returns the number of coalesced events. That's >>> the interface I had in my mind. >>> >>> We could just add a getter for PeriodicTimer and it would serve the same >>> purpose. >>> >> I'm still not sure what the device model is supposed to do with that >> information. I think at could remain private to the PeriodicTimer >> implementation (unless we want to dump some stats or such). >> > > Yeah, I've been thinking about it too and I think I agree with you. > > So here's the new proposal: > > typedef struct PeriodicTimer PeriodicTimer; > > /** > * @accumulated_ticks: the number of unacknowledged ticks in total > since the creation of the timer > **/ > typedef void (PeriodicTimerFunc)(void *opaque); Or just use QEMUTimerCB directly. BTW, QEMUPeriodicTimer* would probably be more consistent with the existing naming scheme. > > PeriodicTimer *periodic_timer_new(PeriodicTimerFunc *cb, void *opaque); > > void periodic_timer_mod(PeriodicTimer *timer, int64_t interval, TimeUnit > unit); > > /** > * @policy: the drift catch-up policy > * DRIFT_COMP_FAST, deliver next tick as soon as any > tick is acknowledged if accumulated_ticks > 1 > * DRIFT_COMP_NONE, do not change interval regardless of > accumulated ticks > * DRIFT_COMP_GRADUAL, shorten interval by half until > accumulated_ticks <= 1 > */ > void periodic_timer_set_policy(PeriodicTimer *timer, > DriftCompensationPolicy policy); > > /** > * @ticks: number of ticks to acknowledge that are currently outstanding. > **/ > void periodic_timer_ack(PeriodicTimer *timer, int ticks); > > int periodic_timer_get_accumulated_ticks(PeriodicTimer *timer); > Looks like a plan. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux