From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Ky5x9-0003J2-So for qemu-devel@nongnu.org; Thu, 06 Nov 2008 09:35:19 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Ky5x8-0003IZ-9M for qemu-devel@nongnu.org; Thu, 06 Nov 2008 09:35:19 -0500 Received: from [199.232.76.173] (port=40598 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ky5x8-0003IW-5q for qemu-devel@nongnu.org; Thu, 06 Nov 2008 09:35:18 -0500 Received: from rv-out-0708.google.com ([209.85.198.241]:48020) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Ky5x7-0005M4-NQ for qemu-devel@nongnu.org; Thu, 06 Nov 2008 09:35:18 -0500 Received: by rv-out-0708.google.com with SMTP id f25so620353rvb.22 for ; Thu, 06 Nov 2008 06:35:16 -0800 (PST) Message-ID: Date: Thu, 6 Nov 2008 15:35:16 +0100 From: "andrzej zaborowski" Subject: Re: [Qemu-devel] [RESEND][PATCH 0/3] Fix guest time drift under heavy load. In-Reply-To: <20081106141821.GG3820@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <20081029152236.14831.15193.stgit@dhcp-1-237.local> <490B59BF.3000205@codemonkey.ws> <20081102130441.GD16809@redhat.com> <49119551.2070704@redhat.com> <20081106071624.GC3820@redhat.com> <20081106100821.GF3820@redhat.com> <20081106141821.GG3820@redhat.com> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gleb Natapov Cc: dlaor@redhat.com, qemu-devel@nongnu.org 2008/11/6 Gleb Natapov : > On Thu, Nov 06, 2008 at 02:21:16PM +0100, andrzej zaborowski wrote: >> >> > It is nothing like a -win2k-hack since there is no any guest "bad >> >> > behaviour" that cause the problem. Yes real hardware doesn't do this, >> >> > but real hardware also provides OS with enough CPU power to handle every >> >> > single timer interrupt. >> >> >> >> A guest that counts on having enough CPU for something is >> >> timing-depenent (buggy). >> >> >> > Tell this to RT developers who count each CPU cycle. >> >> They don't usually use qemu (they certainly shouldn't). >> > It seems to me that you were saying that counting on CPU availability > is buggy in general and since Windows doing this it is buggy. And what I > am saying is that for certain thing it is valid thing to do. RT people > shouldn't use qemu in production of cause, should people use qemu to run > Windows in production? People shouldn't use buggy software in general.. if they do, they have to accept its bugs ;) Or they can have a nasty, but local, workaround. > >> > >> >> > And even if _some_ interrupts are dropped the >> >> > drift is easily fixed with NTP. Try to run Windows XP on very slow machine >> >> > and I am sure you'll see very noticeable time drift. >> >> >> >> Exactly. You'll find the drift on real hardware, so you should find >> >> it in the emulator too. You're trying to hack around it. >> >> >> > If I'll try to run windows XP on 486 then yes, I'll see the time drift. >> > After analyzing the problem I, most certainly, will decide that HW is >> > too old will buy modern CPU and will solve the time drift problem. What do >> > you propose for QEMU users? To use real HW? No, I didn't say we should not have a workaround for this specific case. I propose we use it. That said, even on a modern cpu you have to assume from the start that an emulator will give you about the speed of an 486. >> > > You ignored this question, but it was not rhetorical at all. Can you > answer it please? > >> >> Linux doesn't see this because the clocksource and the >> >> clockevents-device come from separate clks there. It is a windows' >> >> problem. It *is* "bad behaviour". >> > OK we will call the flag -win-time-drift-hack. >> >> I'm not saying it needs a flag, but that's it's a hack so it should stay local. >> > The fix is already local. It is local to PIT and RTC. And to make it > local I change an infrastructure in the first patch. Unnecessarily and invasively. How's that different from an invasive ugly hack.. > >> > >> >> >> >> > >> >> >> Instead you can have the interrupt sources register a callback in the >> >> >> PIC that the PIC calls when the interrupt wasn't delivered. Or.. in >> >> > It requires the mapping from interrupt vector inside the PIC to >> >> > interrupt source. >> >> >> >> Of course. >> >> >> >> > This approach was rejected long time ago. >> >> >> >> Then you'll have to find a different one. >> >> >> > >> > I found one. Here it is, implemented by this patch series. >> > >> >> qemu_irq is the wrong place. >> > Why? Don't give me "that is not how real HW works". Real HW, if properly >> >> I explained in a previous mail, but let me reiterate: qemu_irq >> abstracts a pin on whose one end a device can set a voltage level and >> the other end read it. That's it - there's communication in one way >> only. If you want to send a notification the other direction use a >> second qemu_irq or a callback. It's a quite simple change in your >> PIC. >> > And why is this abstract is set in stone? Why can't we extend it to be: Because we don't need to and because it's so backwards.. > qemu_irq abstracts a pin on whose one end a device can set a voltage > level and the other end read it. If the receiving end "excepts" a signal > (for whatever definition of "except" appropriate for a receiving device) > the function returns positive number, if receiving device loose > information about the signal it returns zero. > >> > configured, will behave more or less deterministically. I.e if timer >> > interrupt is configured to generate highest priority interrupt vector >> > and IRQ handler is fast enough (can be calculated knowing CPU freq) the >> > chances of loosing interrupt will be minimal. And those few that are >> > lost due to SMM or NMI can be compensated by NTP. >> > >> >> > >> >> >> the case of mc146818rtc.c wouldn't it be enough to check if the irq >> >> >> has been acked by reading RTC_REG_C? e.g. >> >> >> >> >> >> static void rtc_periodic_timer(void *opaque) >> >> >> { >> >> >> RTCState *s = opaque; >> >> >> >> >> >> rtc_timer_update(s, s->next_periodic_time); >> >> >> + if (s->cmos_data[RTC_REG_C] & 0xc0) >> >> >> + s->irq_coalesced++; >> >> >> s->cmos_data[RTC_REG_C] |= 0xc0; >> >> >> qemu_irq_raise(s->irq); >> >> >> } >> >> >> >> >> > PIC/APIC in effect has a queue of one interrupt. This means that if >> >> > timer tick is still not acknowledged it doesn't mean that interrupt >> >> > was not queued for delivery inside a PIC. >> >> >> >> This doesn't matter, the tick that arrived while a previous interrupt >> >> was not acked yet, is lost anyway, >> > Not it is not. Not necessary. It can be queued inside PIC and delivered >> > by PIC itself immediately after interrupt acknowledgement. >> >> You can argue that it's the new irq that's lost or it's the first one >> that was lost, either way the PIC only sees one time the irq rising, >> instead of two. That means they were coalesced. > Nothing is lost and PIC sees two irq rising. Example: > - RTC triggers first interrupt. > - It is delivered to PIC. PIC sets corespondent bit in IRR. > - CPU picks up RTC interrupt and it's bit is cleared from IRR bitmap. > - CPU jumps to RTC IRQ routing but before it gets a chance to acknowledge > IRQ to PIC new timer is triggered. > - With your patch you increment irq_coalesced in that case. > - Interrupt is delivered to PIC. No, it isn't (unless the PIC is poorly implemented). We raise the irq, but since it's already high, nothing happens, there's no rising edge. > PIC sets corespondent bit in IRR > but don't deliver the interrupt to the CPU since one is already > in progress. > - CPU acks first IRQ. Only here it's lowered. > - PIC delivers second IRQ from IRR. > > The result is that two interrupts are delivered to CPU and > irq_coalesced == 1 so one more will be needlessly signaled. > >> >> i.e. had been coalesced. So >> >> this'll give you the right number of interrupts to re-inject. >> > No it will not. You'll inject more interrupt then needed and clock will >> > drift forwards. >> >> The PIC won't see more interrupts (rising edges) than times the timer >> had ticked to given moment. Thus the guest OS won't see them either. >> > See above. > >> > >> >> >> >> Ofcourse this, as well as your approach are both wrong because the >> >> guest may be intentionally ignoring the irq and expecting the >> >> interrupts to coalesce. Once it starts processing the RTC interrupts >> >> it will get an unexpected storm. >> >> >> > This is one more thing which is broken in your suggestion, not mine. If a >> > guest wants to ignore interrupt it will mask it and my patch don't report >> > interrupts delivered while masked as been coalesced. >> >> This is moot, it may choose to mask it or not, it can also mask it >> lower down the path. >> > Are we talking about real OSes that we want to run as guests or we give > purely theoretical examples? We're talking about emulating the hw. We have to make compromises sometimes, but we try to avoid them. Cheers, Andrzej