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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Paolo Bonzini [mailto:paolo.bonzini@xxxxxxxxx] 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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux