Re: [Xen-devel] [PATCH 1/6] x86/HVM: fix processing of RTC REG_B writes

[Tim Deegan <tim@xxxxxxx>]

At 10:40 +0100 on 02 May (1367491221), Jan Beulich wrote:
> >>> On 02.05.13 at 11:21, Tim Deegan <tim@xxxxxxx> wrote:
> > At 14:51 +0100 on 29 Apr (1367247101), Jan Beulich wrote:
> >> We must store the new values before calling rtc_update_irq(), and we
> >> need to call rtc_timer_update() when PIE transitions from 0 to 1 (as we
> >> may have previously turned off the periodic timer due to the guest not
> >> reading REG_C, and hence may have to re-enable it in order to start
> >> IRQs getting delivered to the guest).
> >> 
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> >> 
> >> --- a/xen/arch/x86/hvm/rtc.c
> >> +++ b/xen/arch/x86/hvm/rtc.c
> >> @@ -468,12 +468,14 @@ static int rtc_ioport_write(void *opaque
> >>              if ( orig & RTC_SET )
> >>                  rtc_set_time(s);
> >>          }
> >> +        s->hw.cmos_data[RTC_REG_B] = data;
> >>          /*
> >>           * If the interrupt is already set when the interrupt becomes
> >>           * enabled, raise an interrupt immediately.
> >>           */
> >>          rtc_update_irq(s);
> >> -        s->hw.cmos_data[RTC_REG_B] = data;
> >> +        if ( (data & RTC_PIE) && !(orig & RTC_PIE) )
> >> +            rtc_timer_update(s);
> > 
> > Shouldn't this be 'if ( (data ^ orig) & RTC_PIE )'? You'll want to
> > disable the timer if the interrupt's been turned off.
> 
> No, in the spirit of the other involved code we'll want to keep it
> running until reaching dead_ticks.

To get the benefit of VPT processing if the interrupt's only briefly
disabled?  Fair enough.  If you add a comment to that effect, then

Reviewed-by: Tim Deegan <tim@xxxxxxx>

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