Re: [Xen-devel] [PATCH v2] x86/HVM: assorted RTC emulation adjustments

[Keir Fraser <keir@xxxxxxx>]

On 04/02/2013 15:53, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:

> - only call check_update_timer() on REG_B writes when SET changes
> - only call alarm_timer_update() on REG_B writes when relevant bits
>   change
> - instead properly handle AF and PF when the guest is not also setting
>   AIE/PIE respectively (for UF this was already the case, only a
>   comment was slightly inaccurate), including calling the respective
>   update functions upon REG_C reads
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> Tested-by: Olaf Hering <olaf@xxxxxxxxx>

Without spending some quality time with the RTC data sheet and our emulation
code, I don't think I can usefully comment on this. Superficially it all
sounds reasonable. So it's hard for me to Ack it, but you can feel free to
check it in anyway!

 -- Keir

> ---
> v2: This is the (fixed) remaining part of a similarly named patch sent
>     about half a year ago. The originally missing piece are the added
>     calls from the REG_C read code (which back then I had taken note of
>     only as being a possible future enhancement).
> 
> --- a/xen/arch/x86/hvm/rtc.c
> +++ b/xen/arch/x86/hvm/rtc.c
> @@ -60,12 +60,19 @@ static void rtc_toggle_irq(RTCState *s)
>      hvm_isa_irq_assert(d, RTC_IRQ);
>  }
>  
> -static void rtc_periodic_cb(struct vcpu *v, void *opaque)
> +void rtc_periodic_interrupt(void *opaque)
>  {
>      RTCState *s = opaque;
>  
>      spin_lock(&s->lock);
> -    s->hw.cmos_data[RTC_REG_C] |= RTC_PF | RTC_IRQF;
> +    if ( s->hw.cmos_data[RTC_REG_C] & RTC_PF )
> +        destroy_periodic_time(&s->pt);
> +    else
> +    {
> +        s->hw.cmos_data[RTC_REG_C] |= RTC_PF;
> +        if ( s->hw.cmos_data[RTC_REG_B] & RTC_PIE )
> +            rtc_toggle_irq(s);
> +    }
>      spin_unlock(&s->lock);
>  }
>  
> @@ -91,8 +98,7 @@ static void rtc_timer_update(RTCState *s
>          {
>              period = 1 << (period_code - 1); /* period in 32 Khz cycles */
>              period = DIV_ROUND(period * 1000000000ULL, 32768); /* in ns */
> -            create_periodic_time(v, &s->pt, period, period, RTC_IRQ,
> -                                 rtc_periodic_cb, s);
> +            create_periodic_time(v, &s->pt, period, period, RTC_IRQ, NULL,
> s);
>              break;
>          }
>          /* fall through */
> @@ -120,7 +126,7 @@ static void check_update_timer(RTCState
>          guest_usec = get_localtime_us(d) % USEC_PER_SEC;
>          if (guest_usec >= (USEC_PER_SEC - 244))
>          {
> -            /* RTC is in update cycle when enabling UIE */
> +            /* RTC is in update cycle */
>              s->hw.cmos_data[RTC_REG_A] |= RTC_UIP;
>              next_update_time = (USEC_PER_SEC - guest_usec) * NS_PER_USEC;
>              expire_time = NOW() + next_update_time;
> @@ -188,7 +194,7 @@ static void alarm_timer_update(RTCState
>  
>      stop_timer(&s->alarm_timer);
>  
> -    if ((s->hw.cmos_data[RTC_REG_B] & RTC_AIE) &&
> +    if (!(s->hw.cmos_data[RTC_REG_C] & RTC_AF) &&
>              !(s->hw.cmos_data[RTC_REG_B] & RTC_SET))
>      {
>          s->current_tm = gmtime(get_localtime(d));
> @@ -455,8 +461,10 @@ static int rtc_ioport_write(void *opaque
>                  break;
>              }
>          s->hw.cmos_data[RTC_REG_B] = data;
> -        check_update_timer(s);
> -        alarm_timer_update(s);
> +        if ( (data ^ orig) & RTC_SET )
> +            check_update_timer(s);
> +        if ( (data ^ orig) & (RTC_24H | RTC_DM_BINARY | RTC_SET) )
> +            alarm_timer_update(s);
>          break;
>      case RTC_REG_C:
>      case RTC_REG_D:
> @@ -610,6 +618,8 @@ static uint32_t rtc_ioport_read(RTCState
>          hvm_isa_irq_deassert(vrtc_domain(s), RTC_IRQ);
>          s->hw.cmos_data[RTC_REG_C] = 0x00;
>          check_update_timer(s);
> +        alarm_timer_update(s);
> +        rtc_timer_update(s);
>          break;
>      default:
>          ret = s->hw.cmos_data[s->hw.cmos_index];
> --- a/xen/arch/x86/hvm/vpt.c
> +++ b/xen/arch/x86/hvm/vpt.c
> @@ -22,6 +22,7 @@
>  #include <asm/hvm/vpt.h>
>  #include <asm/event.h>
>  #include <asm/apic.h>
> +#include <asm/mc146818rtc.h>
>  
>  #define mode_is(d, name) \
>      ((d)->arch.hvm_domain.params[HVM_PARAM_TIMER_MODE] == HVMPTM_##name)
> @@ -218,6 +219,7 @@ int pt_update_irq(struct vcpu *v)
>      struct periodic_time *pt, *temp, *earliest_pt = NULL;
>      uint64_t max_lag = -1ULL;
>      int irq, is_lapic;
> +    void *pt_priv;
>  
>      spin_lock(&v->arch.hvm_vcpu.tm_lock);
>  
> @@ -251,13 +253,14 @@ int pt_update_irq(struct vcpu *v)
>      earliest_pt->irq_issued = 1;
>      irq = earliest_pt->irq;
>      is_lapic = (earliest_pt->source == PTSRC_lapic);
> +    pt_priv = earliest_pt->priv;
>  
>      spin_unlock(&v->arch.hvm_vcpu.tm_lock);
>  
>      if ( is_lapic )
> -    {
>          vlapic_set_irq(vcpu_vlapic(v), irq, 0);
> -    }
> +    else if ( irq == RTC_IRQ && pt_priv )
> +        rtc_periodic_interrupt(pt_priv);
>      else
>      {
>          hvm_isa_irq_deassert(v->domain, irq);
> --- a/xen/include/asm-x86/hvm/vpt.h
> +++ b/xen/include/asm-x86/hvm/vpt.h
> @@ -181,6 +181,7 @@ void rtc_migrate_timers(struct vcpu *v);
>  void rtc_deinit(struct domain *d);
>  void rtc_reset(struct domain *d);
>  void rtc_update_clock(struct domain *d);
> +void rtc_periodic_interrupt(void *);
>  
>  void pmtimer_init(struct vcpu *v);
>  void pmtimer_deinit(struct domain *d);
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel