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

[Anthony Liguori]

Paolo Bonzini <address@hidden> writes:

> From: Yang Zhang <address@hidden>
>
> 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 <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  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