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: Wed, 11 Jan 2012 08:24:45 +0100 Message-ID: <4F0D393D.5090909@redhat.com> References: <4F0AA2F6.20005@redhat.com> <4F0C03D0.1010801@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: "qemu-devel@nongnu.org" , "avi@redhat.com" , "aliguori@us.ibm.com" , "Zhang, Xiantao" , "Shan, Haitao" , "kvm@vger.kernel.org" To: "Zhang, Yang Z" Return-path: Received: from mail-we0-f174.google.com ([74.125.82.174]:35820 "EHLO mail-we0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752933Ab2AKHYt (ORCPT ); Wed, 11 Jan 2012 02:24:49 -0500 Received: by werm1 with SMTP id m1so287146wer.19 for ; Tue, 10 Jan 2012 23:24:48 -0800 (PST) In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On 01/11/2012 01:56 AM, Zhang, Yang Z wrote: >>>> Clients are supposed to wait for UIP=0 before reading the RTC, >>>> and an update is supposed to be at least 220 microseconds away >>>> when UIP=0. >>> >>> Hardware need a period time to update clock and it would not >>> provide the right value during the update. So it uses UIP to >>> notify the software doesn't believe the value if the UIP is set. >>> For emulation, you can read RTC at any time and it always gives >>> you the right value. So there is no need to emulate UIP. >> >> This is incorrect, for two reasons. First, the UIP is in the spec, >> and we have to implement it. Second, reading the clock is not >> atomic, and waiting for UIP=0 gives you 220 microseconds during >> which you know that the read will appear atomic. > > For a simulator, we need to follow the spec strictly and simulate > hardware as precisely as possible. But QEMU is a generic machine > emulator and virtualizer. It's not a hardware simulator. If there is > an easy way we can provide the same function, why we chose the > complicated one? Because it's not in the spec because some engineer thought it was cool. It's in the spec because it gives you a way to do atomic reads. 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. >> 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? Note that this case is mentioned in drivers/rtc/rtc-cmos.c in the Linux source code, even though it is not used. Paolo