From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45974) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1StCvI-0000L6-0l for qemu-devel@nongnu.org; Mon, 23 Jul 2012 03:19:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1StCvD-0003TB-OZ for qemu-devel@nongnu.org; Mon, 23 Jul 2012 03:19:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:9973) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1StCvD-0003T4-G6 for qemu-devel@nongnu.org; Mon, 23 Jul 2012 03:19:15 -0400 Message-ID: <500CFAED.9070903@redhat.com> Date: Mon, 23 Jul 2012 09:19:09 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1342781633-7288-1-git-send-email-pbonzini@redhat.com> <1342781633-7288-5-git-send-email-pbonzini@redhat.com> <87mx2rjbte.fsf@elfo.mitica> In-Reply-To: <87mx2rjbte.fsf@elfo.mitica> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/7] RTC: Update the RTC clock only when reading it List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: quintela@redhat.com Cc: "Zhang, Yang Z" , qemu-devel@nongnu.org Il 23/07/2012 07:17, Juan Quintela ha scritto: > Paolo Bonzini wrote: >> From: "Zhang, Yang Z" >> >> Calculate guest RTC based on the time of the last update, instead of >> using timers. The formula is >> >> (base_rtc + guest_time_now - guest_time_last_update + offset) >> >> Base_rtc is the RTC value when the RTC was last updated. >> Guest_time_now is the guest time when the access happens. >> Guest_time_last_update was the guest time when the RTC was last updated. >> Offset is used when divider reset happens or the set bit is toggled. >> >> The timer is kept in order to signal interrupts, but it only needs to >> run when either UF or AF is cleared. When the bits are both set, the >> timer does not run. >> >> UIP is now synthesized when reading register A. If the timer is not set, >> or if there is more than one second before it (as is the case at the >> end of this series), the leading edge of UIP is computed and the rising >> edge occurs 220us later. If the update timer occurs within one second, >> however, the rising edge of the AF and UF bits should coincide withe >> the falling edge of UIP. We do not know exactly when this will happen >> because there could be delays in the servicing of the timer. Hence, in >> this case reading register A only computes for the rising edge of UIP, >> and latches the bit until the timer is fired and clears it. >> >> Signed-off-by: Yang Zhang >> Signed-off-by: Paolo Bonzini >> @@ -540,11 +593,12 @@ static const VMStateDescription vmstate_rtc = { >> VMSTATE_INT32(current_tm.tm_mday, RTCState), >> VMSTATE_INT32(current_tm.tm_mon, RTCState), >> VMSTATE_INT32(current_tm.tm_year, RTCState), >> + VMSTATE_UINT64(base_rtc, RTCState), >> + VMSTATE_UINT64(last_update, RTCState), >> + VMSTATE_INT64(offset, RTCState), > VMSTATE_UINT64_V(base_rtc, RTCState, 3) > same ofr the others. > > Normally, new fields are added at the end of the structure. Doesn't really matter if you bump the minimum version but you're right that it is more correct. > >> VMSTATE_TIMER(periodic_timer, RTCState), >> VMSTATE_INT64(next_periodic_time, RTCState), >> - VMSTATE_INT64(next_second_time, RTCState), >> - VMSTATE_TIMER(second_timer, RTCState), >> - VMSTATE_TIMER(second_timer2, RTCState), >> + VMSTATE_TIMER(update_timer, RTCState), > > I have to read the rest of the patch to know what is the relation of > this 4 fields, to see if there is any way to create this in any sane > way that is compatible. next_second_time is computed like this (see check_update_timer): guest_nsec = get_guest_rtc_ns(s) % NSEC_PER_SEC; next_update_time = qemu_get_clock_ns(rtc_clock) + NSEC_PER_SEC - guest_nsec; One of second_timer and second_timer2 is unset, depending on whether UIP=1 or UIP=0 respectively. second_timer if set is next_second_time. second_timer2 if set is next_second_time + get_ticks_per_sec/100. > The new fields can go in a different subsection. But then you would transmit the subsection always, and old QEMU doesn't know how to skip it. So it doesn't really help. This is because base_rtc, last_update and offset need to be transmitted always. next_alarm_time only if no alarm fired already, but the common case is yes. Only for update_timer we can hack and transmit it in the slot that was used for next_second_time. Unless you want to transmit the subsection only for the new machine version, which is never done in the QEMU tree. Paolo