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

[Zhang, Yang Z]

> -----Original Message-----
> From: Paolo Bonzini [mailto:address@hidden On Behalf Of Paolo
> Bonzini
> Because it's not in the spec because some engineer thought it was cool.
It not cool. We need to do some optimizations to get Better Performance.

> 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.
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.
static void rtc_update_second(void *opaque)
{
    RTCState *s = opaque;
    int64_t delay;

    /* if the oscillator is not in normal operation, we do not update */
    if ((s->cmos_data[RTC_REG_A] & 0x70) != 0x20) {
        .....
}


> >> 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. 

> 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.

Anyway, I agree with your point. If we really need to add those features, I 
will add it in next version. Before it, we need figure out whether it is 
necessary.

Best regards
yang