Jan Beulich
2012-Aug-14 09:51 UTC
[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);
tupeng212
2012-Aug-15 14:07 UTC
Re: [PATCH, RFC v2] x86/HVM: assorted RTC emulation adjustments (was Re: Big Bug:Time in VM goes slower...)
Hi, Jan: I am sorry I really don't have much time to try a test of your patch, and it is not convenient for me to have a try. For the version I have been using is xen4.0.x, and your patch is based on the latest version xen4.2.x.(I have never complied the unstable one), so I merged your patch to my xen4.0.x, still couldn't find the two functions below: static void rtc_update_timer2(void *opaque) static void rtc_alarm_cb(void *opaque) so I didn't merge the two functions which contains a rtc_toggle_irq() . The results for me were these: 1 In my real application environment, it worked very well in the former 5mins, much better than before, but at last it lagged again. I don't know whether it belongs to the two missed functions. I lack the ability to figure them out. 2 When I tested my test program which I provided days before, it worked very well, maybe the program doesn't emulate the real environment due to the same setting rate, so I modified this program as which in the attachment. if you are more convenient, you can help me to have a look of it. And I have a opinion, because our product is based on Version Xen4.0.x, if you have enough time, can you write another patch based http://xenbits.xen.org/hg/xen-4.0-testing.hg/ for me, thank you very much! 3 I also have a thought that can we have some detecting methods to find the lagging time earlier to adjust time back to normal value in the code? best regards, tupeng212 Second draft of a patch posted; no test results so far for first draft. Jan From: Jan Beulich Date: 2012-08-14 17:51 To: Yang Z Zhang; Keir Fraser; Tim Deegan CC: tupeng212; xen-devel Subject: [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@lists.xen.org http://lists.xen.org/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
tupeng212
2012-Aug-15 14:12 UTC
Re: [PATCH, RFC v2] x86/HVM: assorted RTC emulation adjustments (was Re: Big Bug:Time in VM goes slower...)
Hi, Jan: I am sorry I really don't have much time to try a test of your patch, and it is not convenient for me to have a try. For the version I have been using is xen4.0.x, and your patch is based on the latest version xen4.2.x.(I have never complied the unstable one), so I merged your patch to my xen4.0.x, still couldn't find the two functions below: static void rtc_update_timer2(void *opaque) static void rtc_alarm_cb(void *opaque) so I didn't merge the two functions which contains a rtc_toggle_irq() . The results for me were these: 1 In my real application environment, it worked very well in the former 5mins, much better than before, but at last it lagged again. I don't know whether it belongs to the two missed functions. I lack the ability to figure them out. 2 When I tested my test program which I provided days before, it worked very well, maybe the program doesn't emulate the real environment due to the same setting rate, so I modified this program as which in the attachment. if you are more convenient, you can help me to have a look of it. -------------------------------------------------------------------- #include <stdio.h> #include <windows.h> typedef int (__stdcall *NTSETTIMER)(IN ULONG RequestedResolution, IN BOOLEAN Set, OUT PULONG ActualResolution ); typedef int (__stdcall *NTQUERYTIMER)(OUT PULONG MinimumResolution, OUT PULONG MaximumResolution, OUT PULONG CurrentResolution ); int main() { DWORD min_res = 0, max_res = 0, cur_res = 0, ret = 0; HMODULE hdll = NULL; hdll = GetModuleHandle("ntdll.dll"); NTSETTIMER AddrNtSetTimer = 0; NTQUERYTIMER AddrNtQueyTimer = 0; AddrNtSetTimer = (NTSETTIMER) GetProcAddress(hdll, "NtSetTimerResolution"); AddrNtQueyTimer = (NTQUERYTIMER)GetProcAddress(hdll, "NtQueryTimerResolution"); while (1) { ret = AddrNtQueyTimer(&min_res, &max_res, &cur_res); printf("min_res = %d, max_res = %d, cur_res = %d\n",min_res, max_res, cur_res); Sleep(5); ret = AddrNtSetTimer(10000, 1, &cur_res); Sleep(5); ret = AddrNtSetTimer(10000, 0, &cur_res); Sleep(5); ret = AddrNtSetTimer(50000, 1, &cur_res); Sleep(5); ret = AddrNtSetTimer(50000, 0, &cur_res); Sleep(5); ret = AddrNtSetTimer(100000, 1, &cur_res); Sleep(5); ret = AddrNtSetTimer(100000, 0, &cur_res); Sleep(5); } return 0; } -------------------------------------------------------------------- And I have a opinion, because our product is based on Version Xen4.0.x, if you have enough time, can you write another patch based http://xenbits.xen.org/hg/xen-4.0-testing.hg/ for me, thank you very much! 3 I also have a thought that can we have some detecting methods to find the lagging time earlier to adjust time back to normal value in the code? best regards, tupeng212 Second draft of a patch posted; no test results so far for first draft. Jan From: Jan Beulich Date: 2012-08-14 17:51 To: Yang Z Zhang; Keir Fraser; Tim Deegan CC: tupeng212; xen-devel Subject: [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@lists.xen.org http://lists.xen.org/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Jan Beulich
2012-Aug-15 15:00 UTC
Re: [PATCH, RFC v2] x86/HVM: assorted RTC emulation adjustments (was Re: Big Bug:Time in VM goes slower...)
>>> On 15.08.12 at 16:07, tupeng212 <tupeng212@gmail.com> wrote: > Hi, Jan: > I am sorry I really don''t have much time to try a test of your patch, and it > is not convenient > for me to have a try. For the version I have been using is xen4.0.x, and > your patch is based on > the latest version xen4.2.x.(I have never complied the unstable one), so I > merged your patch to my > xen4.0.x, still couldn''t find the two functions below: > static void rtc_update_timer2(void *opaque) > static void rtc_alarm_cb(void *opaque) > so I didn''t merge the two functions which contains a rtc_toggle_irq() .Which looks quite right for the older tree.> The results for me were these: > 1 In my real application environment, it worked very well in the former > 5mins, much better than before, > but at last it lagged again. I don''t know whether it belongs to the two > missed functions. I lack the > ability to figure them out.Unlikely.> 2 When I tested my test program which I provided days before, it worked very > well, maybe the program doesn''t > emulate the real environment due to the same setting rate, so I modified > this program as which in the attachment.Okay, so we''re at least moving in the right direction.> if you are more convenient, you can help me to have a look of it. > And I have a opinion, because our product is based on Version Xen4.0.x, if > you have enough time, can you write > another patch based http://xenbits.xen.org/hg/xen-4.0-testing.hg/ for me, > thank you very much!You seem to have done the right thing, so I don''t see an immediate need.> 3 I also have a thought that can we have some detecting methods to find the > lagging time earlier to adjust time > back to normal value in the code?For that we''d first need to understand what the reason for the remaining mis-behavior is. Jan
Jan Beulich
2012-Aug-16 08:22 UTC
Re: [PATCH, RFC v2] x86/HVM: assorted RTC emulation adjustments (was Re: Big Bug:Time in VM goes slower...)
>>> On 15.08.12 at 16:12, tupeng212 <tupeng212@gmail.com> wrote: > The results for me were these: > 1 In my real application environment, it worked very well in the former > 5mins, much better than before, > but at last it lagged again. I don''t know whether it belongs to the two > missed functions. I lack the > ability to figure them out.Did you check whether possibly the guest kernel started flipping between to rate values? If so, that is something that we could deal with too (yet it might be a bit involved, so I''d like to avoid going that route if it would lead nowhere). Jan
tupeng212
2012-Aug-16 13:43 UTC
Re: [PATCH, RFC v2] x86/HVM: assorted RTC emulation adjustments (was Re: Big Bug:Time in VM goes slower...)
Dear Jan: I didn't check the flipping in JVM, but from the printing period in xen, this setting happened(unit: ns): 976562, 976562, 976562, 976562, 15625000, 976562, 976562, 976562, 976562, 15625000 ..... and someone told me windows use these two rate/period only. besides, I checked my former simple tester this morning after it ran for a whole night, it lagged much. so there exists difference between virtualization and reality. tupeng212 From: Jan Beulich Date: 2012-08-16 16:22 To: tupeng212 CC: Yang Z Zhang; xen-devel; Keir Fraser; Tim Deegan Subject: Re: [Xen-devel] [PATCH, RFC v2] x86/HVM: assorted RTC emulation adjustments (was Re: Big Bug:Time in VM goes slower...)>>> On 15.08.12 at 16:12, tupeng212 <tupeng212@gmail.com> wrote: > The results for me were these: > 1 In my real application environment, it worked very well in the former > 5mins, much better than before, > but at last it lagged again. I don't know whether it belongs to the two > missed functions. I lack the > ability to figure them out.Did you check whether possibly the guest kernel started flipping between to rate values? If so, that is something that we could deal with too (yet it might be a bit involved, so I'd like to avoid going that route if it would lead nowhere). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Jan Beulich
2012-Aug-16 14:15 UTC
Re: [PATCH, RFC v2] x86/HVM: assorted RTC emulation adjustments (was Re: Big Bug:Time in VM goes slower...)
>>> On 16.08.12 at 15:43, tupeng212 <tupeng212@gmail.com> wrote: > Dear Jan: > I didn''t check the flipping in JVM, but from the printing period in xen, > this setting happened(unit: ns): > 976562, 976562, 976562, 976562, 15625000, 976562, 976562, 976562, 976562, > 15625000 ..... > and someone told me windows use these two rate/period only.Okay, some back and forth is there in any case, but without knowing the time distance between the individual instances it''s hard to tell whether what I''m thinking of might help.> besides, I checked my former simple tester this morning after it ran for a > whole night, it lagged much.Could you btw quantify the lagging? Your tester sets only a single, constant rate repeatedly, right? If so, then the thought of adjustment likely won''t help. Jan
tupeng212
2012-Aug-16 14:27 UTC
Re: [PATCH, RFC v2] x86/HVM: assorted RTC emulation adjustments (was Re: Big Bug:Time in VM goes slower...)
Okay, some back and forth is there in any case, but without knowing the time distance between the individual instances it's hard to tell whether what I'm thinking of might help.> besides, I checked my former simple tester this morning after it ran for a > whole night, it lagged much.Could you btw quantify the lagging? // I didn't pay attention to it, but about half an hour's lagging certainly exist. when you will go home, you can also have a try to see result tomorrow morning. Your tester sets only a single, constant rate repeatedly, right? //yes, the simplest test just setting a single 10000 down repeatly. If so, then the thought of adjustment likely won't help. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel