Re: [Qemu-devel] [PATCH 4/7] RTC: Update the RTC clock only when reading it

[Juan Quintela <quintela@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: "Zhang, Yang Z" <yang.z.zhang@intel.com>
>
> 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 <yang.z.zhang@intel.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> @@ -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.


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

The new fields can go in a different subsection.  The three ones that
are removed, I don't know if that values can be calculated.


>          VMSTATE_UINT32_V(irq_coalesced, RTCState, 2),
>          VMSTATE_UINT32_V(period, RTCState, 2),
>          VMSTATE_END_OF_LIST()

Later, Juan.