Re: [PATCH 3/3] stop the periodic RTC update timer

[Paolo Bonzini <pbonzini@redhat.com>]

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