From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LNYaI-0006G7-0a for qemu-devel@nongnu.org; Thu, 15 Jan 2009 15:12:58 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LNYaG-0006FH-67 for qemu-devel@nongnu.org; Thu, 15 Jan 2009 15:12:57 -0500 Received: from [199.232.76.173] (port=40046 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LNYaG-0006F8-2E for qemu-devel@nongnu.org; Thu, 15 Jan 2009 15:12:56 -0500 Received: from mx20.gnu.org ([199.232.41.8]:37920) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1LNYaF-000243-JO for qemu-devel@nongnu.org; Thu, 15 Jan 2009 15:12:55 -0500 Received: from mail-qy0-f20.google.com ([209.85.221.20]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LNYaE-0005y8-NB for qemu-devel@nongnu.org; Thu, 15 Jan 2009 15:12:54 -0500 Received: by qyk13 with SMTP id 13so1899593qyk.10 for ; Thu, 15 Jan 2009 12:12:48 -0800 (PST) Message-ID: <496F98B3.70900@codemonkey.ws> Date: Thu, 15 Jan 2009 14:12:35 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH][RESEND] Time drift again. References: <20090114115910.GR3267@redhat.com> In-Reply-To: <20090114115910.GR3267@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Gleb Natapov wrote: > Hello, > > After my last patch to fix interrupt coalescing was rejected > on the basis that it is too intrusive we decided to make the > fix much more localized and only fix the problem for RTC time > source. Unfortunately it is impossible to fix the problem entirely > inside RTC code like Andrzej proposed since Windows reads RTC > register C more then once on each time interrupt so it is impossible > to count reliably how many interrupt windows actually handled. > Proposed solution is localized to I386 target and is disabled by > default. To enable it "-rtc-td-hack" flag should be used. > > Signed-off-by: Gleb Natapov > Applied. Thanks for all your work on this. I know this has gone through a lot of iterations to come up with something that made everyone happy and the persistence is appreciated :-) Regards, Anthony Liguori > diff --git a/hw/apic.c b/hw/apic.c > index bad137f..3052843 100644 > --- a/hw/apic.c > +++ b/hw/apic.c > @@ -100,6 +100,8 @@ struct IOAPICState { > static int apic_io_memory; > static APICState *local_apics[MAX_APICS + 1]; > static int last_apic_id = 0; > +static int apic_irq_delivered; > + > > static void apic_init_ipi(APICState *s); > static void apic_set_irq(APICState *s, int vector_num, int trigger_mode); > @@ -133,6 +135,14 @@ static inline void reset_bit(uint32_t *tab, int index) > tab[i] &= ~mask; > } > > +static inline int get_bit(uint32_t *tab, int index) > +{ > + int i, mask; > + i = index >> 5; > + mask = 1 << (index & 0x1f); > + return !!(tab[i] & mask); > +} > + > static void apic_local_deliver(CPUState *env, int vector) > { > APICState *s = env->apic_state; > @@ -349,8 +359,20 @@ static void apic_update_irq(APICState *s) > cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD); > } > > +void apic_reset_irq_delivered(void) > +{ > + apic_irq_delivered = 0; > +} > + > +int apic_get_irq_delivered(void) > +{ > + return apic_irq_delivered; > +} > + > static void apic_set_irq(APICState *s, int vector_num, int trigger_mode) > { > + apic_irq_delivered += !get_bit(s->irr, vector_num); > + > set_bit(s->irr, vector_num); > if (trigger_mode) > set_bit(s->tmr, vector_num); > diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c > index faf847d..1c71bda 100644 > --- a/hw/mc146818rtc.c > +++ b/hw/mc146818rtc.c > @@ -66,6 +66,10 @@ struct RTCState { > int64_t next_periodic_time; > /* second update */ > int64_t next_second_time; > +#ifdef TARGET_I386 > + uint32_t irq_coalesced; > + uint32_t period; > +#endif > QEMUTimer *second_timer; > QEMUTimer *second_timer2; > }; > @@ -103,12 +107,20 @@ static void rtc_timer_update(RTCState *s, int64_t current_time) > period_code += 7; > /* period in 32 Khz cycles */ > period = 1 << (period_code - 1); > +#ifdef TARGET_I386 > + if(period != s->period) > + s->irq_coalesced = (s->irq_coalesced * s->period) / period; > + s->period = period; > +#endif > /* compute 32 khz clock */ > cur_clock = muldiv64(current_time, 32768, ticks_per_sec); > next_irq_clock = (cur_clock & ~(period - 1)) + period; > s->next_periodic_time = muldiv64(next_irq_clock, ticks_per_sec, 32768) + 1; > qemu_mod_timer(s->periodic_timer, s->next_periodic_time); > } else { > +#ifdef TARGET_I386 > + s->irq_coalesced = 0; > +#endif > qemu_del_timer(s->periodic_timer); > } > } > @@ -118,6 +130,12 @@ static void rtc_periodic_timer(void *opaque) > RTCState *s = opaque; > > rtc_timer_update(s, s->next_periodic_time); > +#ifdef TARGET_I386 > + if ((s->cmos_data[RTC_REG_C] & 0xc0) && rtc_td_hack) { > + s->irq_coalesced++; > + return; > + } > +#endif > s->cmos_data[RTC_REG_C] |= 0xc0; > rtc_irq_raise(s->irq); > } > @@ -378,6 +396,15 @@ static uint32_t cmos_ioport_read(void *opaque, uint32_t addr) > case RTC_REG_C: > ret = s->cmos_data[s->cmos_index]; > qemu_irq_lower(s->irq); > +#ifdef TARGET_I386 > + if(s->irq_coalesced) { > + apic_reset_irq_delivered(); > + qemu_irq_raise(s->irq); > + if (apic_get_irq_delivered()) > + s->irq_coalesced--; > + break; > + } > +#endif > s->cmos_data[RTC_REG_C] = 0x00; > break; > default: > @@ -472,6 +499,28 @@ static int rtc_load(QEMUFile *f, void *opaque, int version_id) > return 0; > } > > +#ifdef TARGET_I386 > +static void rtc_save_td(QEMUFile *f, void *opaque) > +{ > + RTCState *s = opaque; > + > + qemu_put_be32(f, s->irq_coalesced); > + qemu_put_be32(f, s->period); > +} > + > +static int rtc_load_td(QEMUFile *f, void *opaque, int version_id) > +{ > + RTCState *s = opaque; > + > + if (version_id != 1) > + return -EINVAL; > + > + s->irq_coalesced = qemu_get_be32(f); > + s->period = qemu_get_be32(f); > + return 0; > +} > +#endif > + > RTCState *rtc_init(int base, qemu_irq irq) > { > RTCState *s; > @@ -502,6 +551,10 @@ RTCState *rtc_init(int base, qemu_irq irq) > register_ioport_read(base, 2, 1, cmos_ioport_read, s); > > register_savevm("mc146818rtc", base, 1, rtc_save, rtc_load, s); > +#ifdef TARGET_I386 > + if (rtc_td_hack) > + register_savevm("mc146818rtc-td", base, 1, rtc_save_td, rtc_load_td, s); > +#endif > return s; > } > > @@ -608,5 +661,9 @@ RTCState *rtc_mm_init(target_phys_addr_t base, int it_shift, qemu_irq irq) > cpu_register_physical_memory(base, 2 << it_shift, io_memory); > > register_savevm("mc146818rtc", base, 1, rtc_save, rtc_load, s); > +#ifdef TARGET_I386 > + if (rtc_td_hack) > + register_savevm("mc146818rtc-td", base, 1, rtc_save_td, rtc_load_td, s); > +#endif > return s; > } > diff --git a/hw/pc.h b/hw/pc.h > index a60bddf..5dc7709 100644 > --- a/hw/pc.h > +++ b/hw/pc.h > @@ -46,6 +46,8 @@ void apic_deliver_pic_intr(CPUState *env, int level); > int apic_get_interrupt(CPUState *env); > IOAPICState *ioapic_init(void); > void ioapic_set_irq(void *opaque, int vector, int level); > +void apic_reset_irq_delivered(void); > +int apic_get_irq_delivered(void); > > /* i8254.c */ > > diff --git a/qemu-doc.texi b/qemu-doc.texi > index 5a12dc9..f61ae34 100644 > --- a/qemu-doc.texi > +++ b/qemu-doc.texi > @@ -420,6 +420,11 @@ Use it when installing Windows 2000 to avoid a disk full bug. After > Windows 2000 is installed, you no longer need this option (this option > slows down the IDE transfers). > > +@item -rtc-td-hack > +Use it if you experience time drift problem in Windows with ACPI HAL. > +This option will try to figure out how many timer interrupts were not > +processed by the Windows guest and will re-inject them. > + > @item -option-rom @var{file} > Load the contents of @var{file} as an option ROM. > This option is useful to load things like EtherBoot. > diff --git a/sysemu.h b/sysemu.h > index 47c1fea..a9e156c 100644 > --- a/sysemu.h > +++ b/sysemu.h > @@ -90,6 +90,7 @@ extern int graphic_depth; > extern int nographic; > extern const char *keyboard_layout; > extern int win2k_install_hack; > +extern int rtc_td_hack; > extern int alt_grab; > extern int usb_enabled; > extern int smp_cpus; > diff --git a/vl.c b/vl.c > index e29072b..244d1c3 100644 > --- a/vl.c > +++ b/vl.c > @@ -211,6 +211,7 @@ CharDriverState *serial_hds[MAX_SERIAL_PORTS]; > CharDriverState *parallel_hds[MAX_PARALLEL_PORTS]; > #ifdef TARGET_I386 > int win2k_install_hack = 0; > +int rtc_td_hack = 0; > #endif > int usb_enabled = 0; > int smp_cpus = 1; > @@ -3877,6 +3878,7 @@ static void help(int exitcode) > "-full-screen start in full screen\n" > #ifdef TARGET_I386 > "-win2k-hack use it when installing Windows 2000 to avoid a disk full bug\n" > + "-rtc-td-hack use it to fix time drift in Windows ACPI HAL\n" > #endif > "-usb enable the USB driver (will be the default soon)\n" > "-usbdevice name add the host or guest USB device 'name'\n" > @@ -4072,6 +4074,7 @@ enum { > QEMU_OPTION_kernel_kqemu, > QEMU_OPTION_enable_kvm, > QEMU_OPTION_win2k_hack, > + QEMU_OPTION_rtc_td_hack, > QEMU_OPTION_usb, > QEMU_OPTION_usbdevice, > QEMU_OPTION_smp, > @@ -4180,6 +4183,7 @@ static const QEMUOption qemu_options[] = { > #endif > { "pidfile", HAS_ARG, QEMU_OPTION_pidfile }, > { "win2k-hack", 0, QEMU_OPTION_win2k_hack }, > + { "rtc-td-hack", 0, QEMU_OPTION_rtc_td_hack }, > { "usbdevice", HAS_ARG, QEMU_OPTION_usbdevice }, > { "smp", HAS_ARG, QEMU_OPTION_smp }, > { "vnc", HAS_ARG, QEMU_OPTION_vnc }, > @@ -4993,6 +4997,9 @@ int main(int argc, char **argv, char **envp) > case QEMU_OPTION_win2k_hack: > win2k_install_hack = 1; > break; > + case QEMU_OPTION_rtc_td_hack: > + rtc_td_hack = 1; > + break; > #endif > #ifdef USE_KQEMU > case QEMU_OPTION_no_kqemu: > -- > Gleb. > > >