From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dor Laor Subject: Re: [PATCH 2/2] Remove -tdf Date: Wed, 23 Jul 2008 01:03:32 +0300 Message-ID: <48865934.8070007@qumranet.com> References: <1216756217-21888-1-git-send-email-aliguori@us.ibm.com> <1216756217-21888-2-git-send-email-aliguori@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, Avi Kivity To: Anthony Liguori Return-path: Received: from il.qumranet.com ([212.179.150.194]:47320 "EHLO il.qumranet.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751079AbYGVWFI (ORCPT ); Tue, 22 Jul 2008 18:05:08 -0400 In-Reply-To: <1216756217-21888-2-git-send-email-aliguori@us.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: Anthony Liguori wrote: > The last time I posted the KVM patch series to qemu-devel, the -tdf patch met with > some opposition. Since today we implement timer catch-up in the in-kernel PIT and > the in-kernel PIT is used by default, it doesn't seem all that valuable to have > timer catch-up in userspace too. > > Removing it will reduce our divergence from QEMU. > > IMHO the in kernel PIT should go away, there is no reason to keep it except that userspace PIT drifts. Currently both in-kernel PIT and even the in kernel irqchips are not 100% bullet proof. Of course this code is a hack, Gleb Natapov has send better fix for PIT/RTC to qemu list. Can you look into them: http://www.mail-archive.com/kvm@vger.kernel.org/msg01181.html Thanks, Dor > Signed-off-by: Anthony Liguori > > diff --git a/qemu/hw/i8254.c b/qemu/hw/i8254.c > index 69eb889..d0394c0 100644 > --- a/qemu/hw/i8254.c > +++ b/qemu/hw/i8254.c > @@ -332,11 +332,6 @@ static uint32_t pit_ioport_read(void *opaque, uint32_t addr) > return ret; > } > > -/* global counters for time-drift fix */ > -int64_t timer_acks=0, timer_interrupts=0, timer_ints_to_push=0; > - > -extern int time_drift_fix; > - > static void pit_irq_timer_update(PITChannelState *s, int64_t current_time) > { > int64_t expire_time; > @@ -347,24 +342,6 @@ static void pit_irq_timer_update(PITChannelState *s, int64_t current_time) > expire_time = pit_get_next_transition_time(s, current_time); > irq_level = pit_get_out1(s, current_time); > qemu_set_irq(s->irq, irq_level); > - if (time_drift_fix && irq_level==1) { > - /* FIXME: fine tune timer_max_fix (max fix per tick). > - * Should it be 1 (double time), 2 , 4, 10 ? > - * Currently setting it to 5% of PIT-ticks-per-second (per PIT-tick) > - */ > - const long pit_ticks_per_sec = (s->count>0) ? (PIT_FREQ/s->count) : 0; > - const long timer_max_fix = pit_ticks_per_sec/20; > - const long delta = timer_interrupts - timer_acks; > - const long max_delta = pit_ticks_per_sec * 60; /* one minute */ > - if ((delta > max_delta) && (pit_ticks_per_sec > 0)) { > - printf("time drift is too long, %ld seconds were lost\n", delta/pit_ticks_per_sec); > - timer_acks = timer_interrupts; > - timer_ints_to_push = 0; > - } else if (delta > 0) { > - timer_ints_to_push = MIN(delta, timer_max_fix); > - } > - timer_interrupts++; > - } > #ifdef DEBUG_PIT > printf("irq_level=%d next_delay=%f\n", > irq_level, > diff --git a/qemu/hw/i8259.c b/qemu/hw/i8259.c > index b266119..1707434 100644 > --- a/qemu/hw/i8259.c > +++ b/qemu/hw/i8259.c > @@ -221,35 +221,18 @@ static inline void pic_intack(PicState *s, int irq) > } else { > s->isr |= (1 << irq); > } > - > /* We don't clear a level sensitive interrupt here */ > if (!(s->elcr & (1 << irq))) > s->irr &= ~(1 << irq); > - > } > > -extern int time_drift_fix; > - > int pic_read_irq(PicState2 *s) > { > int irq, irq2, intno; > > irq = pic_get_irq(&s->pics[0]); > if (irq >= 0) { > - > pic_intack(&s->pics[0], irq); > -#ifndef TARGET_IA64 > - if (time_drift_fix && irq == 0) { > - extern int64_t timer_acks, timer_ints_to_push; > - timer_acks++; > - if (timer_ints_to_push > 0) { > - timer_ints_to_push--; > - /* simulate an edge irq0, like the one generated by i8254 */ > - pic_set_irq1(&s->pics[0], 0, 0); > - pic_set_irq1(&s->pics[0], 0, 1); > - } > - } > -#endif > if (irq == 2) { > irq2 = pic_get_irq(&s->pics[1]); > if (irq2 >= 0) { > diff --git a/qemu/vl.c b/qemu/vl.c > index 19c8bbf..d6877cd 100644 > --- a/qemu/vl.c > +++ b/qemu/vl.c > @@ -229,7 +229,6 @@ const char *option_rom[MAX_OPTION_ROMS]; > int nb_option_roms; > int semihosting_enabled = 0; > int autostart = 1; > -int time_drift_fix = 0; > unsigned int kvm_shadow_memory = 0; > const char *mem_path = NULL; > int hpagesize = 0; > @@ -7968,7 +7967,6 @@ static void help(int exitcode) > #ifndef _WIN32 > "-daemonize daemonize QEMU after initializing\n" > #endif > - "-tdf inject timer interrupts that got lost\n" > "-kvm-shadow-memory megs set the amount of shadow pages to be allocated\n" > "-mem-path set the path to hugetlbfs/tmpfs mounted directory, also enables allocation of guest memory with huge pages\n" > "-option-rom rom load a file, rom, into the option ROM space\n" > @@ -8089,7 +8087,6 @@ enum { > QEMU_OPTION_tb_size, > QEMU_OPTION_icount, > QEMU_OPTION_incoming, > - QEMU_OPTION_tdf, > QEMU_OPTION_kvm_shadow_memory, > QEMU_OPTION_mempath, > }; > @@ -8202,7 +8199,6 @@ const QEMUOption qemu_options[] = { > #if defined(TARGET_ARM) || defined(TARGET_M68K) > { "semihosting", 0, QEMU_OPTION_semihosting }, > #endif > - { "tdf", 0, QEMU_OPTION_tdf }, /* enable time drift fix */ > { "kvm-shadow-memory", HAS_ARG, QEMU_OPTION_kvm_shadow_memory }, > { "name", HAS_ARG, QEMU_OPTION_name }, > #if defined(TARGET_SPARC) > @@ -9092,9 +9088,6 @@ int main(int argc, char **argv) > case QEMU_OPTION_semihosting: > semihosting_enabled = 1; > break; > - case QEMU_OPTION_tdf: > - time_drift_fix = 1; > - break; > case QEMU_OPTION_kvm_shadow_memory: > kvm_shadow_memory = (int64_t)atoi(optarg) * 1024 * 1024 / 4096; > break; >