[05/10] RTC: Update the RTC clock only when reading it

From: Yang Zhang <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>
---
 hw/mc146818rtc.c |  321 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 186 insertions(+), 135 deletions(-)

Paolo Bonzini <pbonzini@redhat.com> writes:

> From: Yang Zhang <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>
> ---
>  hw/mc146818rtc.c |  321 +++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 186 insertions(+), 135 deletions(-)
>
> diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
> index 293f174..2c34a82 100644
> --- a/hw/mc146818rtc.c
> +++ b/hw/mc146818rtc.c
> @@ -45,6 +45,8 @@
>  # define DPRINTF_C(format, ...)      do { } while (0)
>  #endif
>  
> +#define NSEC_PER_SEC    1000000000LL
> +
>  #define RTC_REINJECT_ON_ACK_COUNT 20
>  
>  typedef struct RTCState {
> @@ -54,27 +56,40 @@ typedef struct RTCState {
>      uint8_t cmos_index;
>      struct tm current_tm;
>      int32_t base_year;
> +    uint64_t base_rtc;
> +    uint64_t last_update;
> +    int64_t offset;
>      qemu_irq irq;
>      qemu_irq sqw_irq;
>      int it_shift;
>      /* periodic timer */
>      QEMUTimer *periodic_timer;
>      int64_t next_periodic_time;
> -    /* second update */
> -    int64_t next_second_time;
> +    /* update-ended timer */
> +    QEMUTimer *update_timer;
>      uint16_t irq_reinject_on_ack_count;
>      uint32_t irq_coalesced;
>      uint32_t period;
>      QEMUTimer *coalesced_timer;
> -    QEMUTimer *second_timer;
> -    QEMUTimer *second_timer2;
>      Notifier clock_reset_notifier;
>      LostTickPolicy lost_tick_policy;
>      Notifier suspend_notifier;
>  } RTCState;
>  
>  static void rtc_set_time(RTCState *s);
> -static void rtc_copy_date(RTCState *s);
> +static void rtc_update_time(RTCState *s);
> +static void rtc_set_cmos(RTCState *s);
> +static inline int rtc_from_bcd(RTCState *s, int a);
> +
> +static uint64_t get_guest_rtc_ns(RTCState *s)
> +{
> +    uint64_t guest_rtc;
> +    uint64_t guest_clock = qemu_get_clock_ns(rtc_clock);
> +
> +    guest_rtc = s->base_rtc * NSEC_PER_SEC
> +                 + guest_clock - s->last_update + s->offset;
> +    return guest_rtc;
> +}
>  
>  #ifdef TARGET_I386
>  static void rtc_coalesced_timer_update(RTCState *s)
> @@ -110,6 +125,7 @@ static void rtc_coalesced_timer(void *opaque)
>  }
>  #endif
>  
> +/* handle periodic timer */
>  static void periodic_timer_update(RTCState *s, int64_t current_time)
>  {
>      int period_code, period;
> @@ -175,6 +191,100 @@ static void rtc_periodic_timer(void *opaque)
>      }
>  }
>  
> +/* handle update-ended timer */
> +static void check_update_timer(RTCState *s)
> +{
> +    uint64_t next_update_time;
> +    uint64_t guest_nsec;
> +
> +    /* From the data sheet: setting the SET bit does not prevent
> +     * interrupts from occurring!  However, it will prevent an
> +     * alarm interrupt from occurring, because the time of day is
> +     * not updated.
> +     */
> +    if ((s->cmos_data[RTC_REG_C] & REG_C_UF) &&
> +        (s->cmos_data[RTC_REG_B] & REG_B_SET)) {
> +        qemu_del_timer(s->update_timer);
> +        return;
> +    }
> +    if ((s->cmos_data[RTC_REG_C] & REG_C_UF) &&
> +        (s->cmos_data[RTC_REG_C] & REG_C_AF)) {
> +        qemu_del_timer(s->update_timer);
> +        return;
> +    }
> +
> +    guest_nsec = get_guest_rtc_ns(s) % NSEC_PER_SEC;
> +    /* reprogram to next second */
> +    next_update_time = qemu_get_clock_ns(rtc_clock)
> +        + NSEC_PER_SEC - guest_nsec;
> +    if (next_update_time != qemu_timer_expire_time_ns(s->update_timer)) {
> +        qemu_mod_timer(s->update_timer, next_update_time);
> +    }
> +}
> +
> +static inline uint8_t convert_hour(RTCState *s, uint8_t hour)
> +{
> +    if (!(s->cmos_data[RTC_REG_B] & REG_B_24H)) {
> +        hour %= 12;
> +        if (s->cmos_data[RTC_HOURS] & 0x80) {
> +            hour += 12;
> +        }
> +    }
> +    return hour;
> +}
> +
> +static uint32_t check_alarm(RTCState *s)
> +{
> +    uint8_t alarm_hour, alarm_min, alarm_sec;
> +    uint8_t cur_hour, cur_min, cur_sec;
> +
> +    alarm_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS_ALARM]);
> +    alarm_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES_ALARM]);
> +    alarm_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS_ALARM]);
> +    alarm_hour = convert_hour(s, alarm_hour);
> +
> +    cur_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS]);
> +    cur_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES]);
> +    cur_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS]);
> +    cur_hour = convert_hour(s, cur_hour);
> +
> +    if (((s->cmos_data[RTC_SECONDS_ALARM] & 0xc0) == 0xc0
> +                || alarm_sec == cur_sec) &&
> +            ((s->cmos_data[RTC_MINUTES_ALARM] & 0xc0) == 0xc0
> +             || alarm_min == cur_min) &&
> +            ((s->cmos_data[RTC_HOURS_ALARM] & 0xc0) == 0xc0
> +             || alarm_hour == cur_hour)) {
> +        return 1;
> +    }
> +    return 0;
> +
> +}
> +
> +static void rtc_update_timer(void *opaque)
> +{
> +    RTCState *s = opaque;
> +    int32_t irqs = REG_C_UF;
> +    int32_t new_irqs;
> +
> +    /* UIP might have been latched, update time and clear it.  */
> +    rtc_update_time(s);
> +    s->cmos_data[RTC_REG_A] &= ~REG_A_UIP;
> +
> +    if (check_alarm(s)) {
> +        irqs |= REG_C_AF;
> +        if (s->cmos_data[RTC_REG_B] & REG_B_AIE) {
> +            qemu_system_wakeup_request(QEMU_WAKEUP_REASON_RTC);
> +        }
> +    }
> +    new_irqs = irqs & ~s->cmos_data[RTC_REG_C];
> +    s->cmos_data[RTC_REG_C] |= irqs;
> +    if ((new_irqs & s->cmos_data[RTC_REG_B]) != 0) {
> +        s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
> +        qemu_irq_raise(s->irq);
> +    }
> +    check_update_timer(s);
> +}
> +
>  static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
>  {
>      RTCState *s = opaque;
> @@ -189,6 +299,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
>          case RTC_MINUTES_ALARM:
>          case RTC_HOURS_ALARM:
>              s->cmos_data[s->cmos_index] = data;
> +            check_update_timer(s);
>              break;
>          case RTC_SECONDS:
>          case RTC_MINUTES:
> @@ -201,6 +312,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
>              /* if in set mode, do not update the time */
>              if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
>                  rtc_set_time(s);
> +                check_update_timer(s);
>              }
>              break;
>          case RTC_REG_A:
> @@ -208,15 +320,21 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
>              s->cmos_data[RTC_REG_A] = (data & ~REG_A_UIP) |
>                  (s->cmos_data[RTC_REG_A] & REG_A_UIP);
>              periodic_timer_update(s, qemu_get_clock_ns(rtc_clock));
> +            check_update_timer(s);
>              break;
>          case RTC_REG_B:
>              if (data & REG_B_SET) {
> +                /* update cmos to when the rtc was stopping */
> +                if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
> +                    rtc_update_time(s);
> +                }
>                  /* set mode: reset UIP mode */
>                  s->cmos_data[RTC_REG_A] &= ~REG_A_UIP;
>                  data &= ~REG_B_UIE;
>              } else {
>                  /* if disabling set mode, update the time */
>                  if (s->cmos_data[RTC_REG_B] & REG_B_SET) {
> +                    s->offset = get_guest_rtc_ns(s) % NSEC_PER_SEC;
>                      rtc_set_time(s);
>                  }
>              }
> @@ -231,6 +349,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
>              }
>              s->cmos_data[RTC_REG_B] = data;
>              periodic_timer_update(s, qemu_get_clock_ns(rtc_clock));
> +            check_update_timer(s);
>              break;
>          case RTC_REG_C:
>          case RTC_REG_D:
> @@ -279,10 +398,13 @@ static void rtc_set_time(RTCState *s)
>      tm->tm_mon = rtc_from_bcd(s, s->cmos_data[RTC_MONTH]) - 1;
>      tm->tm_year = rtc_from_bcd(s, s->cmos_data[RTC_YEAR]) + s->base_year - 1900;
>  
> +    s->base_rtc = mktimegm(tm);
> +    s->last_update = qemu_get_clock_ns(rtc_clock);
> +
>      rtc_change_mon_event(tm);
>  }
>  
> -static void rtc_copy_date(RTCState *s)
> +static void rtc_set_cmos(RTCState *s)
>  {
>      const struct tm *tm = &s->current_tm;
>      int year;
> @@ -308,122 +430,41 @@ static void rtc_copy_date(RTCState *s)
>      s->cmos_data[RTC_YEAR] = rtc_to_bcd(s, year);
>  }
>  
> -/* month is between 0 and 11. */
> -static int get_days_in_month(int month, int year)
> +static void rtc_update_time(RTCState *s)
>  {
> -    static const int days_tab[12] = {
> -        31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
> -    };
> -    int d;
> -    if ((unsigned )month >= 12)
> -        return 31;
> -    d = days_tab[month];
> -    if (month == 1) {
> -        if ((year % 4) == 0 && ((year % 100) != 0 || (year % 400) == 0))
> -            d++;
> -    }
> -    return d;
> +    struct tm *ret;
> +    time_t guest_sec;
> +    int64_t guest_nsec;
> +
> +    guest_nsec = get_guest_rtc_ns(s);
> +    guest_sec = guest_nsec / NSEC_PER_SEC;
> +    ret = gmtime(&guest_sec);

gmtime_r() ?

> +    s->current_tm = *ret;
> +    rtc_set_cmos(s);
>  }
>  
> -/* update 'tm' to the next second */
> -static void rtc_next_second(struct tm *tm)
> +static int update_in_progress(RTCState *s)
>  {
> -    int days_in_month;
> -
> -    tm->tm_sec++;
> -    if ((unsigned)tm->tm_sec >= 60) {
> -        tm->tm_sec = 0;
> -        tm->tm_min++;
> -        if ((unsigned)tm->tm_min >= 60) {
> -            tm->tm_min = 0;
> -            tm->tm_hour++;
> -            if ((unsigned)tm->tm_hour >= 24) {
> -                tm->tm_hour = 0;
> -                /* next day */
> -                tm->tm_wday++;
> -                if ((unsigned)tm->tm_wday >= 7)
> -                    tm->tm_wday = 0;
> -                days_in_month = get_days_in_month(tm->tm_mon,
> -                                                  tm->tm_year + 1900);
> -                tm->tm_mday++;
> -                if (tm->tm_mday < 1) {
> -                    tm->tm_mday = 1;
> -                } else if (tm->tm_mday > days_in_month) {
> -                    tm->tm_mday = 1;
> -                    tm->tm_mon++;
> -                    if (tm->tm_mon >= 12) {
> -                        tm->tm_mon = 0;
> -                        tm->tm_year++;
> -                    }
> -                }
> -            }
> -        }
> -    }
> -}
> -
> -
> -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) {
> -        s->next_second_time += get_ticks_per_sec();
> -        qemu_mod_timer(s->second_timer, s->next_second_time);
> -    } else {
> -        rtc_next_second(&s->current_tm);
> -
> -        if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
> -            /* update in progress bit */
> -            s->cmos_data[RTC_REG_A] |= REG_A_UIP;
> -        }
> -        /* should be 244 us = 8 / 32768 seconds, but currently the
> -           timers do not have the necessary resolution. */
> -        delay = (get_ticks_per_sec() * 1) / 100;
> -        if (delay < 1)
> -            delay = 1;
> -        qemu_mod_timer(s->second_timer2,
> -                       s->next_second_time + delay);
> -    }
> -}
> -
> -static void rtc_update_second2(void *opaque)
> -{
> -    RTCState *s = opaque;
> +    int64_t guest_nsec;
>  
> -    if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
> -        rtc_copy_date(s);
> +    if (s->cmos_data[RTC_REG_B] & REG_B_SET) {
> +        return 0;
>      }
> -
> -    /* check alarm */
> -    if (((s->cmos_data[RTC_SECONDS_ALARM] & 0xc0) == 0xc0 ||
> -         rtc_from_bcd(s, s->cmos_data[RTC_SECONDS_ALARM]) == s->current_tm.tm_sec) &&
> -        ((s->cmos_data[RTC_MINUTES_ALARM] & 0xc0) == 0xc0 ||
> -         rtc_from_bcd(s, s->cmos_data[RTC_MINUTES_ALARM]) == s->current_tm.tm_min) &&
> -        ((s->cmos_data[RTC_HOURS_ALARM] & 0xc0) == 0xc0 ||
> -         rtc_from_bcd(s, s->cmos_data[RTC_HOURS_ALARM]) == s->current_tm.tm_hour)) {
> -
> -        s->cmos_data[RTC_REG_C] |= REG_C_AF;
> -        if (s->cmos_data[RTC_REG_B] & REG_B_AIE) {
> -            qemu_system_wakeup_request(QEMU_WAKEUP_REASON_RTC);
> -            qemu_irq_raise(s->irq);
> -            s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
> +    if (qemu_timer_pending(s->update_timer)) {
> +        int64_t next_update_time = qemu_timer_expire_time_ns(s->update_timer);
> +        /* Latch UIP until the timer expires.  */
> +        if (qemu_get_clock_ns(rtc_clock) >= (next_update_time - 244000)) {
> +            s->cmos_data[RTC_REG_A] |= REG_A_UIP;
> +            return 1;
>          }
>      }
>  
> -    /* update ended interrupt */
> -    s->cmos_data[RTC_REG_C] |= REG_C_UF;
> -    if (s->cmos_data[RTC_REG_B] & REG_B_UIE) {
> -        s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
> -        qemu_irq_raise(s->irq);
> +    guest_nsec = get_guest_rtc_ns(s);
> +    /* UIP bit will be set at last 244us of every second. */
> +    if ((guest_nsec % NSEC_PER_SEC) >= (NSEC_PER_SEC - 244000)) {
> +        return 1;
>      }

244us isn't quite right, 8 / 32khz is closer to 245us but it makes more
sense to me to:

/* UIP gets held for 8 cycles */
#define RTC_CLOCK_RATE   (32768)
#define UIP_HOLD_LENGTH  ((8 * NSEC_PER_SEC) / RTC_CLOCK_RATE)

Regards,

Anthony Liguori

Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: Yang Zhang <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>

>  
>  static const VMStateDescription vmstate_rtc = {
>      .name = "mc146818rtc",
> -    .version_id = 2,
> -    .minimum_version_id = 1,
> -    .minimum_version_id_old = 1,
> +    .version_id = 3,
> +    .minimum_version_id = 3,
> +    .minimum_version_id_old = 3,
>      .post_load = rtc_post_load,
>      .fields      = (VMStateField []) {
>          VMSTATE_BUFFER(cmos_data, RTCState),
> @@ -542,11 +595,12 @@ static const VMStateDescription vmstate_rtc = {
>          VMSTATE_INT32(current_tm.tm_year, RTCState),
>          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_UINT32_V(irq_coalesced, RTCState, 2),
>          VMSTATE_UINT32_V(period, RTCState, 2),
> +        VMSTATE_UINT64_V(base_rtc, RTCState, 3),
> +        VMSTATE_UINT64_V(last_update, RTCState, 3),
> +        VMSTATE_INT64_V(offset, RTCState, 3),
> +        VMSTATE_TIMER_V(update_timer, RTCState, 3),
>          VMSTATE_END_OF_LIST()
>      }
>  };

Why did you remove all the migration from previous versions?
You can't migrate now from version{1,2}, and we used to be able to do
it?

Why did you remove it?

Later, Juan.

Il 02/08/2012 11:09, Juan Quintela ha scritto:
> Why did you remove all the migration from previous versions?
> You can't migrate now from version{1,2}, and we used to be able to do
> it?
> 
> Why did you remove it?

Because it won't work; we removed three fields.  You need to add
rtc_load_old which is done later in the series.  But I guess I can use
VMSTATE_UNUSED instead.

Paolo

Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 02/08/2012 11:09, Juan Quintela ha scritto:
>> Why did you remove all the migration from previous versions?
>> You can't migrate now from version{1,2}, and we used to be able to do
>> it?
>> 
>> Why did you remove it?
>
> Because it won't work; we removed three fields.  You need to add
> rtc_load_old which is done later in the series.  But I guess I can use
> VMSTATE_UNUSED instead.

Something like (completely untested):

static bool version_less_3(void *opaque, int version_id)
{
    return version_id < 3;
}

static const VMStateDescription vmstate_rtc = {
    .name = "mc146818rtc",
    .version_id = 3,
    .minimum_version_id = 1,
    .minimum_version_id_old = 1,
    .post_load = rtc_post_load,
    .fields      = (VMStateField []) {
         VMSTATE_BUFFER(cmos_data, RTCState),
 @@ -542,11 +595,12 @@ static const VMStateDescription vmstate_rtc = {
         VMSTATE_INT32(current_tm.tm_year, RTCState),
         VMSTATE_TIMER(periodic_timer, RTCState),
         VMSTATE_INT64(next_periodic_time, RTCState),
         VMSTATE_UNUSED_TEST(8*3, v_less_3); //* whatever space */
-        VMSTATE_INT64(next_second_time, RTCState),
-        VMSTATE_TIMER(second_timer, RTCState),
-        VMSTATE_TIMER(second_timer2, RTCState),
         VMSTATE_UINT32_V(irq_coalesced, RTCState, 2),
         VMSTATE_UINT32_V(period, RTCState, 2),
+        VMSTATE_UINT64_V(base_rtc, RTCState, 3),
+        VMSTATE_UINT64_V(last_update, RTCState, 3),
+        VMSTATE_INT64_V(offset, RTCState, 3),
+        VMSTATE_TIMER_V(update_timer, RTCState, 3),
         VMSTATE_END_OF_LIST()
     }
 };


This will make "migration protocol" work,  I have zero clue if
"obviating" the value of next_second_time, and the two second_timers*
can work, that depends on how rtc works.  Perhaps some extra magic on
post_load() is needed, though.

Could you tell me if you need anything else?

Later, Juan.