[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH, RFC v2] x86/HVM: assorted RTC emulation adjustments (was Re: Big Bug:Time in VM goes slower...)
Below/attached a second draft of a patch to fix not only this issue, but a few more with the RTC emulation. Keir, Tim, Yang, others - the change to xen/arch/x86/hvm/vpt.c really looks more like a hack than a solution, but I don't see another way without much more intrusive changes. The point is that we want the RTC code to decide whether to generate an interrupt (so that RTC_PF can become set correctly even without RTC_PIE getting enabled by the guest). Additionally I wonder whether alarm_timer_update() shouldn't bail on non-conforming RTC_*_ALARM values (as those would never match the values they get compared against, whereas with the current way of handling this they would appear to match - i.e. set RTC_AF and possibly generate an interrupt - some other point in time). I realize the behavior here may not be precisely specified, but the specification saying "the current time has matched the alarm time" means to me a value by value comparison, which implies that non-conforming values would never match (since non-conforming current time values could get replaced at any time by the hardware due to overflow detection). Jan - don't call rtc_timer_update() on REG_A writes when the value didn't change (doing the call always was reported to cause wall clock time lagging with the JVM running on Windows) - don't call rtc_timer_update() on REG_B writes at all - only call alarm_timer_update() on REG_B writes when relevant bits change - only call check_update_timer() on REG_B writes when SET changes - 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) - raise the RTC IRQ not only when UIE gets set while UF was already set, but generalize this to cover AIE and PIE as well - properly mask off bit 7 when retrieving the hour values in alarm_timer_update(), and properly use RTC_HOURS_ALARM's bit 7 when converting from 12- to 24-hour value - also handle the two other possible clock bases - use RTC_* names in a couple of places where literal numbers were used so far --- a/xen/arch/x86/hvm/rtc.c +++ b/xen/arch/x86/hvm/rtc.c @@ -50,11 +50,24 @@ static void rtc_set_time(RTCState *s); static inline int from_bcd(RTCState *s, int a); static inline int convert_hour(RTCState *s, int hour); -static void rtc_periodic_cb(struct vcpu *v, void *opaque) +static void rtc_toggle_irq(RTCState *s) +{ + struct domain *d = vrtc_domain(s); + + ASSERT(spin_is_locked(&s->lock)); + s->hw.cmos_data[RTC_REG_C] |= RTC_IRQF; + hvm_isa_irq_deassert(d, RTC_IRQ); + hvm_isa_irq_assert(d, RTC_IRQ); +} + +void rtc_periodic_interrupt(void *opaque) { RTCState *s = opaque; + spin_lock(&s->lock); - s->hw.cmos_data[RTC_REG_C] |= 0xc0; + 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); } @@ -68,19 +81,25 @@ static void rtc_timer_update(RTCState *s ASSERT(spin_is_locked(&s->lock)); period_code = s->hw.cmos_data[RTC_REG_A] & RTC_RATE_SELECT; - if ( (period_code != 0) && (s->hw.cmos_data[RTC_REG_B] & RTC_PIE) ) + switch ( s->hw.cmos_data[RTC_REG_A] & RTC_DIV_CTL ) { - if ( period_code <= 2 ) + case RTC_REF_CLCK_32KHZ: + if ( (period_code != 0) && (period_code <= 2) ) period_code += 7; - - period = 1 << (period_code - 1); /* period in 32 Khz cycles */ - period = DIV_ROUND((period * 1000000000ULL), 32768); /* period in ns */ - create_periodic_time(v, &s->pt, period, period, RTC_IRQ, - rtc_periodic_cb, s); - } - else - { + /* fall through */ + case RTC_REF_CLCK_1MHZ: + case RTC_REF_CLCK_4MHZ: + if ( period_code != 0 ) + { + 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, NULL, s); + break; + } + /* fall through */ + default: destroy_periodic_time(&s->pt); + break; } } @@ -102,7 +121,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; @@ -144,7 +163,6 @@ static void rtc_update_timer(void *opaqu static void rtc_update_timer2(void *opaque) { RTCState *s = opaque; - struct domain *d = vrtc_domain(s); spin_lock(&s->lock); if (!(s->hw.cmos_data[RTC_REG_B] & RTC_SET)) @@ -152,11 +170,7 @@ static void rtc_update_timer2(void *opaq s->hw.cmos_data[RTC_REG_C] |= RTC_UF; s->hw.cmos_data[RTC_REG_A] &= ~RTC_UIP; if ((s->hw.cmos_data[RTC_REG_B] & RTC_UIE)) - { - s->hw.cmos_data[RTC_REG_C] |= RTC_IRQF; - hvm_isa_irq_deassert(d, RTC_IRQ); - hvm_isa_irq_assert(d, RTC_IRQ); - } + rtc_toggle_irq(s); check_update_timer(s); } spin_unlock(&s->lock); @@ -175,21 +189,18 @@ static void alarm_timer_update(RTCState stop_timer(&s->alarm_timer); - if ((s->hw.cmos_data[RTC_REG_B] & RTC_AIE) && - !(s->hw.cmos_data[RTC_REG_B] & RTC_SET)) + if ( !(s->hw.cmos_data[RTC_REG_B] & RTC_SET) ) { s->current_tm = gmtime(get_localtime(d)); rtc_copy_date(s); alarm_sec = from_bcd(s, s->hw.cmos_data[RTC_SECONDS_ALARM]); alarm_min = from_bcd(s, s->hw.cmos_data[RTC_MINUTES_ALARM]); - alarm_hour = from_bcd(s, s->hw.cmos_data[RTC_HOURS_ALARM]); - alarm_hour = convert_hour(s, alarm_hour); + alarm_hour = convert_hour(s, s->hw.cmos_data[RTC_HOURS_ALARM]); cur_sec = from_bcd(s, s->hw.cmos_data[RTC_SECONDS]); cur_min = from_bcd(s, s->hw.cmos_data[RTC_MINUTES]); - cur_hour = from_bcd(s, s->hw.cmos_data[RTC_HOURS]); - cur_hour = convert_hour(s, cur_hour); + cur_hour = convert_hour(s, s->hw.cmos_data[RTC_HOURS]); next_update_time = USEC_PER_SEC - (get_localtime_us(d) % USEC_PER_SEC); next_update_time = next_update_time * NS_PER_USEC + NOW(); @@ -343,7 +354,6 @@ static void alarm_timer_update(RTCState static void rtc_alarm_cb(void *opaque) { RTCState *s = opaque; - struct domain *d = vrtc_domain(s); spin_lock(&s->lock); if (!(s->hw.cmos_data[RTC_REG_B] & RTC_SET)) @@ -351,11 +361,7 @@ static void rtc_alarm_cb(void *opaque) s->hw.cmos_data[RTC_REG_C] |= RTC_AF; /* alarm interrupt */ if (s->hw.cmos_data[RTC_REG_B] & RTC_AIE) - { - s->hw.cmos_data[RTC_REG_C] |= RTC_IRQF; - hvm_isa_irq_deassert(d, RTC_IRQ); - hvm_isa_irq_assert(d, RTC_IRQ); - } + rtc_toggle_irq(s); alarm_timer_update(s); } spin_unlock(&s->lock); @@ -365,6 +371,7 @@ static int rtc_ioport_write(void *opaque { RTCState *s = opaque; struct domain *d = vrtc_domain(s); + uint32_t orig, mask; spin_lock(&s->lock); @@ -382,6 +389,7 @@ static int rtc_ioport_write(void *opaque return 0; } + orig = s->hw.cmos_data[s->hw.cmos_index]; switch ( s->hw.cmos_index ) { case RTC_SECONDS_ALARM: @@ -405,9 +413,9 @@ static int rtc_ioport_write(void *opaque break; case RTC_REG_A: /* UIP bit is read only */ - s->hw.cmos_data[RTC_REG_A] = (data & ~RTC_UIP) | - (s->hw.cmos_data[RTC_REG_A] & RTC_UIP); - rtc_timer_update(s); + s->hw.cmos_data[RTC_REG_A] = (data & ~RTC_UIP) | (orig & RTC_UIP); + if ( (data ^ orig) & (RTC_RATE_SELECT | RTC_DIV_CTL) ) + rtc_timer_update(s); break; case RTC_REG_B: if ( data & RTC_SET ) @@ -415,7 +423,7 @@ static int rtc_ioport_write(void *opaque /* set mode: reset UIP mode */ s->hw.cmos_data[RTC_REG_A] &= ~RTC_UIP; /* adjust cmos before stopping */ - if (!(s->hw.cmos_data[RTC_REG_B] & RTC_SET)) + if (!(orig & RTC_SET)) { s->current_tm = gmtime(get_localtime(d)); rtc_copy_date(s); @@ -424,21 +432,26 @@ static int rtc_ioport_write(void *opaque else { /* if disabling set mode, update the time */ - if ( s->hw.cmos_data[RTC_REG_B] & RTC_SET ) + if ( orig & RTC_SET ) rtc_set_time(s); } - /* if the interrupt is already set when the interrupt become - * enabled, raise an interrupt immediately*/ - if ((data & RTC_UIE) && !(s->hw.cmos_data[RTC_REG_B] & RTC_UIE)) - if (s->hw.cmos_data[RTC_REG_C] & RTC_UF) + /* + * If the interrupt is already set when the interrupt becomes + * enabled, raise an interrupt immediately. + * NB: RTC_{A,P,U}IE == RTC_{A,P,U}F respectively. + */ + for ( mask = RTC_UIE; mask <= RTC_PIE; mask <<= 1 ) + if ( (data & mask) && !(orig & mask) && + (s->hw.cmos_data[RTC_REG_C] & mask) ) { - hvm_isa_irq_deassert(d, RTC_IRQ); - hvm_isa_irq_assert(d, RTC_IRQ); + rtc_toggle_irq(s); + break; } s->hw.cmos_data[RTC_REG_B] = data; - rtc_timer_update(s); - 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: @@ -453,7 +466,7 @@ static int rtc_ioport_write(void *opaque static inline int to_bcd(RTCState *s, int a) { - if ( s->hw.cmos_data[RTC_REG_B] & 0x04 ) + if ( s->hw.cmos_data[RTC_REG_B] & RTC_DM_BINARY ) return a; else return ((a / 10) << 4) | (a % 10); @@ -461,7 +474,7 @@ static inline int to_bcd(RTCState *s, in static inline int from_bcd(RTCState *s, int a) { - if ( s->hw.cmos_data[RTC_REG_B] & 0x04 ) + if ( s->hw.cmos_data[RTC_REG_B] & RTC_DM_BINARY ) return a; else return ((a >> 4) * 10) + (a & 0x0f); @@ -469,12 +482,14 @@ static inline int from_bcd(RTCState *s, /* Hours in 12 hour mode are in 1-12 range, not 0-11. * So we need convert it before using it*/ -static inline int convert_hour(RTCState *s, int hour) +static inline int convert_hour(RTCState *s, int raw) { + int hour = from_bcd(s, raw & 0x7f); + if (!(s->hw.cmos_data[RTC_REG_B] & RTC_24H)) { hour %= 12; - if (s->hw.cmos_data[RTC_HOURS] & 0x80) + if (raw & 0x80) hour += 12; } return hour; @@ -493,8 +508,7 @@ static void rtc_set_time(RTCState *s) tm->tm_sec = from_bcd(s, s->hw.cmos_data[RTC_SECONDS]); tm->tm_min = from_bcd(s, s->hw.cmos_data[RTC_MINUTES]); - tm->tm_hour = from_bcd(s, s->hw.cmos_data[RTC_HOURS] & 0x7f); - tm->tm_hour = convert_hour(s, tm->tm_hour); + tm->tm_hour = convert_hour(s, s->hw.cmos_data[RTC_HOURS]); tm->tm_wday = from_bcd(s, s->hw.cmos_data[RTC_DAY_OF_WEEK]); tm->tm_mday = from_bcd(s, s->hw.cmos_data[RTC_DAY_OF_MONTH]); tm->tm_mon = from_bcd(s, s->hw.cmos_data[RTC_MONTH]) - 1; --- 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 @@ void 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 @@ void 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 ) + 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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |