Menu
▾
▴
Re: [kvm-devel] [RFC] qemu: simulate edge-triggered interrupt in master PIC for time-drift-fix
|
From: Dor L. <dor...@gm...> - 2007-11-10 20:17:17
|
Carlo Marcelo Arenas Belon wrote:
> On Sat, Nov 10, 2007 at 12:35:34AM +0200, Dor Laor wrote:
>
>> Carlo Marcelo Arenas Belon wrote:
>>
>> wrong patch, this was meant to go to the slave PIC, so it will use s->pics[1]
>> instead of s->pics[0].
>>
>> Why to the slave pic? Isn't the pit connected to line 0 of the master?
>>
>
> yes, you are right; 8254 should be connected to line 0 of the master 8259 and
> using the slave PIC is not correct.
>
> sadly, with the patch and while trying to test if it make a difference or not,
> I found that when using -no-kvm-irqchip -tdf with an ACPI enabled guest
> (Fedora 7 x86_64) it was dying with the following kernel panic :
>
> MP-BIOS bug: 8254 timer not connected to IO-APIC
> Kernel panic - not syncing: IO-APIC + timer doesn't work! Try using the
> 'noapic' kernel parameter.
>
>
It was only tested with non acpi/apic guests, mainly with windows.
If you'll use windows with stadard hal as a guest you'll see the difference.
For acpi/apic guest the -tdf is not relevant because they dont use the
pit as timesource and use apic/acpi timers.
> and pressummed I misread the code and got the wrong PIC; when that problem was
> already happenning even without the patch.
>
> so to resume, the original patch should be right, and probably closer to what
> the original implementation of tdf was doing; but that original implementation
> has a bug with ACPI and the current code works eventhough is obviously broken,
> just because the compiler is packing the PicState2 struct in a way that makes
> "s" and "&s->pics[0]" point to the same address.
>
>
You're right about this and we should apply the patch.
>> also from my tests it might seem that tdf is irrelevant anyway with the new
>> clock work and haven't been able to find a case where enabling it (so
>> triggering this buggy code path) migh be needed.
>>
>> any one care to comment on any current users of tdf? and if there are none in
>> the viability for removing it?
>>
>>
>>
>> It does work but only for non-acpi guest that has the -no-kvm-irqchip
>> parameter.
>> To test it you can load your host and see what happens to the clock when
>> you run
>> a 1000HZ guest (use taskset to pin the guest with other cpu intensive
>> tasks.)
>> We decided not to fix it in the in-kernel pic since once the tpr
>> optimization enable
>> running acpi/apic guests and thus the pic is not used as time source.
>> Dor.
>>
>
> not sure if I follow what you meant here, but I had been able to reproduce the
> time drifts at least when using -no-acpi
>
> Carlo
>
>> On Fri, Nov 09, 2007 at 11:22:10AM -0600, Carlo Marcelo Arenas Belon wrote:
>>
>>
>> The following patch fixes 1a483ef4040ed380bf69d684783d06a617073256 so that the
>> parent PIC pointer is used to send the edge irq0 instead of the PIC pair and
>> that is an incompatible pointer type as reported in :
>>
>> /var/tmp/portage/app-emulation/kvm-51/work/kvm-51/qemu/hw/i8259.c: In function
>> `
>> pic_read_irq':
>> /var/tmp/portage/app-emulation/kvm-51/work/kvm-51/qemu/hw/i8259.c:248:
>> warning: passing arg 1 of `pic_set_irq1' from incompatible pointer type
>> /var/tmp/portage/app-emulation/kvm-51/work/kvm-51/qemu/hw/i8259.c:249:
>> warning: passing arg 1 of `pic_set_irq1' from incompatible pointer type
>>
>> Signed-off-by: Carlo Marcelo Arenas Belon <ca...@sa...>
>> ---
>> qemu/hw/i8259.c | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/qemu/hw/i8259.c b/qemu/hw/i8259.c
>> index 01447d7..60063d4 100644
>> --- a/qemu/hw/i8259.c
>> +++ b/qemu/hw/i8259.c
>> @@ -245,8 +245,8 @@ int pic_read_irq(PicState2 *s)
>> if (timer_ints_to_push > 0) {
>> timer_ints_to_push--;
>> /* simulate an edge irq0, like the one generated by i8254 */
>> - pic_set_irq1(s, 0, 0);
>> - pic_set_irq1(s, 0, 1);
>> + pic_set_irq1(&s->pics[0], 0, 0);
>> + pic_set_irq1(&s->pics[0], 0, 1);
>> }
>> }
>>
>> --
>> 1.5.2.5
>>
>>
>>
>> -------------------------------------------------------------------------
>> This SF.net email is sponsored by: Splunk Inc.
>> Still grepping through log files to find problems? Stop.
>> Now Search log events and configuration files using AJAX and a browser.
>> Download your FREE copy of Splunk now >> http://get.splunk.com/
>> _______________________________________________
>> kvm-devel mailing list
>> kvm...@li...
>> https://lists.sourceforge.net/lists/listinfo/kvm-devel
>>
>>
>>
>
>
|
