Re: i8259 defaults wrong?

[Herbert Poetzl <herbert@13thfloor.at>]

>On Mon, Aug 17, 2009 at 04:20:24AM +0200, Herbert Poetzl wrote:
>> 
>> Greetings!
>> 
>> looking at the i8259 implementation found in qemu
>> as well as in the in-kernel kvm implementation, I
>> see that on pic_reset() special_fully_nested_mode
>> is set to zero, but the intel(r) 8259A manual says
>> on page 15:
>> 
>>   Fully Nested Mode
>>  
>>   This mode is entered after initialization unless
>>   another mode is programmed.
>> 
>> any comments are appreciated, a patch should be
>> trivial, but I'd gladly submit one if requested.
>
> Well, does the BIOS set fully nested mode on reset? 

no idea, but as far as I tested, it doesn't matter
for Linux guests, but makes some other operating
systems - which seem to heavily rely on this default 
(e.g. OpenStep, darwin) - work like expected.

> If not, a kvm change is difficult, since the bios 
> and kvm are updated independently.

nevertheless IMHO it should be fixed ASAP, and the 
required change is trivial, i.e. only a single line 
in the kvm/qemu code needs to be changed to make it
work correctly (see patches below)

best,
Herbert

please CC me as I'm not subscribed to either list


; fix i8259 reset behaviour 
; qemu/kvm (outside the kernel)
;
--- ./hw/i8259.c.orig	2009-08-12 19:45:45.000000000 +0200
+++ ./hw/i8259.c	2009-08-17 03:48:15.000000000 +0200
@@ -295,7 +295,7 @@ static void pic_reset(void *opaque)
     s->init_state = 0;
     s->auto_eoi = 0;
     s->rotate_on_auto_eoi = 0;
-    s->special_fully_nested_mode = 0;
+    s->special_fully_nested_mode = 1;
     s->init4 = 0;
     s->single_mode = 0;
     /* Note: ELCR is not reset */


; fix i8259 reset behaviour
; kvm (inside the kernel)

--- ./arch/x86/kvm/i8259.c.orig	2009-06-11 17:12:23.000000000 +0200
+++ ./arch/x86/kvm/i8259.c	2009-08-18 02:50:00.000000000 +0200
@@ -279,7 +279,7 @@ void kvm_pic_reset(struct kvm_kpic_state
 	s->init_state = 0;
 	s->auto_eoi = 0;
 	s->rotate_on_auto_eoi = 0;
-	s->special_fully_nested_mode = 0;
+	s->special_fully_nested_mode = 1;
 	s->init4 = 0;
 }
 

>> TIA,
>> Herbert