From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH 3/3] stop the periodic RTC update timer Date: Thu, 12 Jan 2012 10:41:04 +0100 Message-ID: References: <4F0AA2F6.20005@redhat.com> <4F0C03D0.1010801@redhat.com> <4F0D393D.5090909@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: qemu-devel@nongnu.org To: kvm@vger.kernel.org Return-path: Received: from lo.gmane.org ([80.91.229.12]:54778 "EHLO lo.gmane.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752859Ab2ALJlY (ORCPT ); Thu, 12 Jan 2012 04:41:24 -0500 Received: from list by lo.gmane.org with local (Exim 4.69) (envelope-from ) id 1RlH9r-0006EF-Fu for kvm@vger.kernel.org; Thu, 12 Jan 2012 10:41:21 +0100 Received: from 93-34-200-238.ip51.fastwebnet.it ([93.34.200.238]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Thu, 12 Jan 2012 10:41:19 +0100 Received: from pbonzini by 93-34-200-238.ip51.fastwebnet.it with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Thu, 12 Jan 2012 10:41:19 +0100 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On 01/12/2012 01:51 AM, Zhang, Yang Z wrote: >> QEMU not being a simulator means that we always assume that the RTC is >> programmed for a 32768 Hz clock, for example, because any other setting would >> not make sense on a PC. We can use a 1-second (or higher, as in your patches) >> timer, rather than a 32768 Hz timer which anyway would not work well. >> >> So we're taking shortcuts, but each of them must be evaluated separately, and >> _this_ shortcut is not acceptable. >> >>> Also, is there an actual case that break with my patch? >> >> Any decent unit test for the RTC would break. > > Any decent unit test break the following logic too. The spec provide > three ways for you to program, why we only focus on 0x20? Because this > is for emulation not for hardware simulation. Because no real OS set it > to other value. No, because those bits are supposed to be 0/1/2 depending on the frequency of the crystal that is attached to the RTC. The RTC will not operate correctly unless the value and the actual frequency are the same. A parameter to the unit test would be the frequency of the crystal. It would always be 32768 Hz, thus the unit test will always write 0x20 (or 0x60/0x70 to reset). Testing with a different oscillator frequency would be possible and indeed would fail, but it would also be kind of useless. >>>> It means that the (not externally visible) millisecond value is set >>>> to 500 when you modify the current time of the RTC. The next update >>>> of the clock will happen exactly 500 ms after you reset bit >>>> 7 of register B. >>> >>> Same question, any reason need to complicate the current logic? Or any >>> actual usage model need to add this? >> >> Is it really so difficult to implement? > > I think what we are talking is do we really need it? Not how difficult to add it. Sure; but since you're overhauling the whole model, it seems like a good occasion to make things better, too. >> Note that this case is mentioned in drivers/rtc/rtc-cmos.c in the Linux source >> code, even though it is not used. > > Yes, it just mentioned the next update will happen in 500ms later. > What's wrong with this? The highest resolution of RTC is 1 second, if > any software intend to use RTC do some check within 1 second, it should > be wrong. The highest resolution of the RTC is 1 second, but synchronizing the updates with other external clock sources could be a valuable thing. Knowing the time of the next update lets you do this kind of synchronization. Paolo