thr3ads.net - Xen devel - [PATCH v2] x86/HVM: assorted RTC emulation adjustments [Feb 2013]

If this information is useful, please help other people find it:
Share via:

Jan Beulich

2013-Feb-04 15:53 UTC

[PATCH v2] x86/HVM: assorted RTC emulation adjustments

- 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@suse.com>
Tested-by: Olaf Hering <olaf@aepfle.de>
---
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@lists.xen.org
http://lists.xen.org/xen-devel

Keir Fraser

2013-Feb-04 16:51 UTC

head link

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

On 04/02/2013 15:53, "Jan Beulich" <JBeulich@suse.com> 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@suse.com>
> Tested-by: Olaf Hering <olaf@aepfle.de>
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@lists.xen.org
> http://lists.xen.org/xen-devel

Jan Beulich

2013-Feb-04 17:03 UTC

head link

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

>>> On 04.02.13 at 17:51, Keir Fraser <keir@xen.org> wrote:
> On 04/02/2013 15:53, "Jan Beulich" <JBeulich@suse.com>
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@suse.com>
>> Tested-by: Olaf Hering <olaf@aepfle.de>
> 
> 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!
Will do, not the least because it appears to fix a problem for Olaf.

Jan

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

[PATCH v2] x86/HVM: assorted RTC emulation adjustments

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

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