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

[Paolo Bonzini]

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