Re: [Qemu-devel] [PATCH 3/3] stop the periodic RTC update timer

[Marcelo Tosatti <mtosatti@redhat.com>]

On Fri, Jan 06, 2012 at 07:37:31AM +0000, Zhang, Yang Z wrote:
> change the RTC update logic to use host time with offset to calculate RTC clock.
> 	There have no need to use two periodic timers to maintain an internal timer for RTC clock update and alarm check. Instead, we calculate the real RTC time by the host time with an offset. For alarm and updated-end interrupt, if guest enabled it, then we setup a timer, or else, stop it.
> 
> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> 
> diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
> index 9cbd052..ac1854e 100644
> --- a/hw/mc146818rtc.c
> +++ b/hw/mc146818rtc.c
> @@ -84,7 +84,7 @@ typedef struct RTCState {
>      MemoryRegion io;
>      uint8_t cmos_data[128];
>      uint8_t cmos_index;
> -    struct tm current_tm;
> +    int64_t offset;
>      int32_t base_year;
>      qemu_irq irq;
>      qemu_irq sqw_irq;
> @@ -93,19 +93,18 @@ typedef struct RTCState {
>      QEMUTimer *periodic_timer;
>      int64_t next_periodic_time;
>      /* second update */
> -    int64_t next_second_time;
> +    QEMUTimer *update_timer;
> +    int64_t next_update_time;
> +    /* alarm  */
> +    QEMUTimer *alarm_timer;
> +    int64_t next_alarm_time;
>      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;
>  } RTCState;
> 
> -static void rtc_set_time(RTCState *s);
> -static void rtc_copy_date(RTCState *s);
> -
>  #ifdef TARGET_I386
>  static void rtc_coalesced_timer_update(RTCState *s)
>  {
> @@ -140,6 +139,72 @@ static void rtc_coalesced_timer(void *opaque)
>  }
>  #endif
> 
> +static inline int rtc_to_bcd(RTCState *s, int a)
> +{
> +    if (s->cmos_data[RTC_REG_B] & REG_B_DM) {
> +        return a;
> +    } else {
> +        return ((a / 10) << 4) | (a % 10);
> +    }
> +}
> +
> +static inline int rtc_from_bcd(RTCState *s, int a)
> +{
> +    if (s->cmos_data[RTC_REG_B] & REG_B_DM) {
> +        return a;
> +    } else {
> +        return ((a >> 4) * 10) + (a & 0x0f);
> +    }
> +}
> +
> +static void rtc_set_time(RTCState *s)
> +{
> +    struct tm tm ;
> +
> +    tm.tm_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS]);
> +    tm.tm_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES]);
> +    tm.tm_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS] & 0x7f);
> +    if (!(s->cmos_data[RTC_REG_B] & REG_B_24H) &&
> +        (s->cmos_data[RTC_HOURS] & 0x80)) {
> +        tm.tm_hour += 12;
> +    }
> +    tm.tm_wday = rtc_from_bcd(s, s->cmos_data[RTC_DAY_OF_WEEK]) - 1;
> +    tm.tm_mday = rtc_from_bcd(s, s->cmos_data[RTC_DAY_OF_MONTH]);
> +    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->offset = qemu_timedate_diff(&tm);
> +
> +    rtc_change_mon_event(&tm);
> +}
> +
> +static void rtc_update_time(RTCState *s)
> +{
> +    struct tm tm;
> +    int year;
> +
> +    qemu_get_timedate(&tm, s->offset);
> +
> +    s->cmos_data[RTC_SECONDS] = rtc_to_bcd(s, tm.tm_sec);
> +    s->cmos_data[RTC_MINUTES] = rtc_to_bcd(s, tm.tm_min);
> +    if (s->cmos_data[RTC_REG_B] & REG_B_24H) {
> +        /* 24 hour format */
> +        s->cmos_data[RTC_HOURS] = rtc_to_bcd(s, tm.tm_hour);
> +    } else {
> +        /* 12 hour format */
> +        s->cmos_data[RTC_HOURS] = rtc_to_bcd(s, tm.tm_hour % 12);
> +        if (tm.tm_hour >= 12)
> +            s->cmos_data[RTC_HOURS] |= 0x80;
> +    }
> +    s->cmos_data[RTC_DAY_OF_WEEK] = rtc_to_bcd(s, tm.tm_wday + 1);
> +    s->cmos_data[RTC_DAY_OF_MONTH] = rtc_to_bcd(s, tm.tm_mday);
> +    s->cmos_data[RTC_MONTH] = rtc_to_bcd(s, tm.tm_mon + 1);
> +    year = (tm.tm_year - s->base_year) % 100;
> +    if (year < 0)
> +        year += 100;
> +    s->cmos_data[RTC_YEAR] = rtc_to_bcd(s, year);
> +}
> +

Please have this code move in a separate, earlier patch.

>  static void rtc_timer_update(RTCState *s, int64_t current_time)
>  {
>      int period_code, period;
> @@ -174,7 +239,7 @@ static void rtc_timer_update(RTCState *s, int64_t current_time)
>      }
>  }
> 
> -static void rtc_periodic_timer(void *opaque)
> +static void rtc_periodic_interrupt(void *opaque)
>  {
>      RTCState *s = opaque;
> 
> @@ -204,6 +269,92 @@ static void rtc_periodic_timer(void *opaque)
>      }
>  }
> 
> +static void rtc_enable_update_interrupt(void *opaque)
> +{
> +    RTCState *s = opaque;
> +
> +    s->next_update_time = qemu_get_clock_ns(rtc_clock) + get_ticks_per_sec();
> +    qemu_mod_timer(s->update_timer, s->next_update_time);
> +}
> +
> +static void rtc_disable_update_interrupt(void *opaque)
> +{
> +    RTCState *s = opaque;
> +
> +    qemu_del_timer(s->update_timer);
> +}
> +
> +static void rtc_update_interrupt(void *opaque)
> +{
> +    RTCState *s = opaque;
> +
> +    /* 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);
> +
> +        s->next_update_time += get_ticks_per_sec();
> +       qemu_mod_timer(s->update_timer, s->next_update_time);
> +    } else
> +       rtc_disable_update_interrupt(s);
> +}
> +
> +static void rtc_enable_alarm(void *opaque)
> +{
> +    RTCState *s = opaque;
> +
> +    s->next_alarm_time = qemu_get_clock_ns(rtc_clock) + get_ticks_per_sec();
> +    s->next_alarm_time /= get_ticks_per_sec();
> +    s->next_alarm_time *= get_ticks_per_sec();
> +
> +    qemu_mod_timer(s->alarm_timer, s->next_alarm_time);
> +}
> +
> +static void rtc_disable_alarm(void *opaque)
> +{
> +    RTCState *s = opaque;
> +
> +    qemu_del_timer(s->alarm_timer);
> +}
> +
> +static void rtc_alarm_interrupt(void *opaque)
> +{
> +    RTCState *s = opaque;
> +    struct tm tm;
> +    int hour = 0;
> +
> +    qemu_get_timedate(&tm, s->offset);
> +
> +    if ((s->cmos_data[RTC_HOURS_ALARM] & 0xc0) != 0xc0) {
> +       hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS_ALARM] & 0x7f);
> +       if (!(s->cmos_data[RTC_REG_B] & REG_B_24H) &&
> +            (s->cmos_data[RTC_HOURS_ALARM] & 0x80)) {
> +                   hour += 12;
> +       }
> +    }
> +
> +    /* check alarm */
> +    if (s->cmos_data[RTC_REG_B] & REG_B_AIE) {
> +        if (((s->cmos_data[RTC_SECONDS_ALARM] & 0xc0) == 0xc0 ||
> +             rtc_from_bcd(s, s->cmos_data[RTC_SECONDS_ALARM]) == tm.tm_sec) &&
> +            ((s->cmos_data[RTC_MINUTES_ALARM] & 0xc0) == 0xc0 ||
> +             rtc_from_bcd(s, s->cmos_data[RTC_MINUTES_ALARM]) == tm.tm_min) &&
> +            ((s->cmos_data[RTC_HOURS_ALARM] & 0xc0) == 0xc0 ||
> +             hour == tm.tm_hour)) {
> +
> +           printf("raise irq\n");

printf.

> +            s->cmos_data[RTC_REG_C] |= 0xa0;
> +            qemu_irq_raise(s->irq);
> +        }
> +
> +       s->next_alarm_time += get_ticks_per_sec();
> +       qemu_mod_timer(s->alarm_timer, s->next_alarm_time);
> +    } else
> +       rtc_disable_alarm(s);
> +
> +}
> +
>  static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
>  {
>      RTCState *s = opaque;
> @@ -239,26 +390,37 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
>              rtc_timer_update(s, qemu_get_clock_ns(rtc_clock));
>              break;
>          case RTC_REG_B:
> -            if (data & REG_B_SET) {
> -                /* set mode: reset UIP mode */
> -                s->cmos_data[RTC_REG_A] &= ~REG_A_UIP;
> -                data &= ~REG_B_UIE;
> -            } else {
> +            if (data & REG_B_SET)
> +                rtc_update_time(s);
> +            else {
>                  /* if disabling set mode, update the time */
>                  if (s->cmos_data[RTC_REG_B] & REG_B_SET) {
>                      rtc_set_time(s);
>                  }
>              }
> +
>              if (((s->cmos_data[RTC_REG_B] ^ data) & (REG_B_DM | REG_B_24H)) &&
>                  !(data & REG_B_SET)) {
>                  /* If the time format has changed and not in set mode,
>                     update the registers immediately. */
>                  s->cmos_data[RTC_REG_B] = data;
> -                rtc_copy_date(s);
> -            } else {
> +                rtc_update_time(s);
> +            } else
>                  s->cmos_data[RTC_REG_B] = data;
> -            }
 +
> +           /* check alarm interrupt */
> +           if ((s->cmos_data[RTC_REG_B] & REG_B_AIE) &&
> +               !(s->cmos_data[RTC_REG_B] & REG_B_SET) )
> +               rtc_enable_alarm(s);
> +
> +           /* check update-ended interrupt */
> +           if ((s->cmos_data[RTC_REG_B] & REG_B_UIE) &&
> +               !(s->cmos_data[RTC_REG_B] & REG_B_SET))
> +               rtc_enable_update_interrupt(s);
> +
> +           /* check periodic interrupt or square-wave */
>              rtc_timer_update(s, qemu_get_clock_ns(rtc_clock));
> +

Regarding the UIP bit, a guest could read it in a loop and wait for the
value to change. But you can emulate it in cmos_ioport_read by reading
the host time, that is, return 1 during 244us, 0 for remaining of the
second, and have that in sync with update-cycle-ended interrupt if its
enabled.