Re: [Qemu-devel] [RESEND][PATCH 0/3] Fix guest time drift under heavy load.

[andrzej zaborowski]

2008/11/6 Gleb Natapov <address@hidden>:
> 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