[PATCH 1/4] BIOS changes for configuring irq0->inti2 override (v2)

[PATCH 1/4] BIOS changes for configuring irq0->inti2 override (v2)

[Beth Kon]

These patches resolve the irq0->inti2 override issue, and get the hpet working
on kvm. 

They are dependent on Jes Sorensen's recent 0006-qemu-kvm-irq-routing.patch.

Override and HPET changes are sent as a series because HPET depends on the 
override. Win2k8 expects the HPET interrupt on inti2, regardless of whether 
an override exists in the BIOS. And the HPET spec states that in legacy mode, 
timer interrupt is on inti2.

The irq0->inti2 override will always be used unless the kernel cannot do irq 
routing (i.e., compatibility with old kernels). So if the kernel is capable, 
userspace sets up irq0->inti2 via the irq routing interface, and adds the 
irq0->inti2 override to the MADT interrupt source override table, 
and the mp table (for the no-acpi case).

A couple of months ago, Marcelo was seeing RHEL5 guests complain of invalid
checksum with these patches, but later he couldn't reproduce it, and I'm not 
seeing it now. While all guests still need to be fully tested, everything 
appears to be in order.  I've tested on win2k864, win2k832, RHEL5.3 32 bit, 
and ubuntu 8.10 64 bit. 

Signed-off-by: Beth Kon <eak@us.ibm.com>

diff --git a/kvm/bios/rombios32.c b/kvm/bios/rombios32.c
index 8684987..07dda73 100755
--- a/kvm/bios/rombios32.c
+++ b/kvm/bios/rombios32.c
@@ -445,6 +445,9 @@ uint32_t cpuid_ext_features;
 unsigned long ram_size;
 uint64_t ram_end;
 uint8_t bios_uuid[16];
+#ifdef BX_QEMU
+uint8_t irq0_override;
+#endif
 #ifdef BX_USE_EBDA_TABLES
 unsigned long ebda_cur_addr;
 #endif
@@ -477,6 +480,7 @@ void wrmsr_smp(uint32_t index, uint64_t val)
 #define QEMU_CFG_SIGNATURE  0x00
 #define QEMU_CFG_ID         0x01
 #define QEMU_CFG_UUID       0x02
+#define QEMU_CFG_IRQ0_OVERRIDE 0x0e
 
 int qemu_cfg_port;
 
@@ -518,6 +522,18 @@ void uuid_probe(void)
     memset(bios_uuid, 0, 16);
 }
 
+#ifdef BX_QEMU
+void irq0_override_probe(void)
+{
+    if(qemu_cfg_port) {
+        qemu_cfg_select(QEMU_CFG_IRQ0_OVERRIDE);
+        qemu_cfg_read(&irq0_override, 1);
+        return;
+    }
+    memset(&irq0_override, 0, 1);
+}
+#endif
+
 void cpu_probe(void)
 {
     uint32_t eax, ebx, ecx, edx;
@@ -1160,6 +1176,13 @@ static void mptable_init(void)
 
     /* irqs */
     for(i = 0; i < 16; i++) {
+#ifdef BX_QEMU
+        /* One entry per ioapic interrupt destination. Destination 2 is covered
+         * by irq0->inti2 override (i == 0). Source IRQ 2 is unused 
+         */
+        if (irq0_override && i == 2)
+            continue;
+#endif
         putb(&q, 3); /* entry type = I/O interrupt */
         putb(&q, 0); /* interrupt type = vectored interrupt */
         putb(&q, 0); /* flags: po=0, el=0 */
@@ -1167,7 +1190,12 @@ static void mptable_init(void)
         putb(&q, 0); /* source bus ID = ISA */
         putb(&q, i); /* source bus IRQ */
         putb(&q, ioapic_id); /* dest I/O APIC ID */
-        putb(&q, i); /* dest I/O APIC interrupt in */
+#ifdef BX_QEMU
+        if (irq0_override && i == 0)
+            putb(&q, 2); /* dest I/O APIC interrupt in */
+        else
+#endif
+            putb(&q, i); /* dest I/O APIC interrupt in */
     }
     /* patch length */
     len = q - mp_config_table;
@@ -1550,16 +1578,18 @@ void acpi_bios_init(void)
 
     addr = (addr + 7) & ~7;
     madt_addr = addr;
+    madt = (void *)(addr);
     madt_size = sizeof(*madt) +
         sizeof(struct madt_processor_apic) * MAX_CPUS +
-#ifdef BX_QEMU
-        sizeof(struct madt_io_apic) /* + sizeof(struct madt_int_override) */;
-#else
         sizeof(struct madt_io_apic);
+#ifdef BX_QEMU
+    for (i = 0; i < 16; i++)
+        if (PCI_ISA_IRQ_MASK & (1U << i))
+            madt_size += sizeof(struct madt_int_override);
+    if (irq0_override)
+        madt_size += sizeof(struct madt_int_override);
 #endif
-    madt = (void *)(addr);
     addr += madt_size;
-
 #ifdef BX_QEMU
 #ifdef HPET_WORKS_IN_KVM
     addr = (addr + 7) & ~7;
@@ -1660,23 +1690,20 @@ void acpi_bios_init(void)
         io_apic->io_apic_id = smp_cpus;
         io_apic->address = cpu_to_le32(0xfec00000);
         io_apic->interrupt = cpu_to_le32(0);
+        int_override = (struct madt_int_override*)(io_apic + 1);
 #ifdef BX_QEMU
-#ifdef HPET_WORKS_IN_KVM
-        io_apic++;
-
-        int_override = (void *)io_apic;
-        int_override->type = APIC_XRUPT_OVERRIDE;
-        int_override->length = sizeof(*int_override);
-        int_override->bus = cpu_to_le32(0);
-        int_override->source = cpu_to_le32(0);
-        int_override->gsi = cpu_to_le32(2);
-        int_override->flags = cpu_to_le32(0);
-#endif
+        if (irq0_override) {
+            memset(int_override, 0, sizeof(*int_override));
+            int_override->type = APIC_XRUPT_OVERRIDE;
+            int_override->length = sizeof(*int_override);
+            int_override->source = 0;
+            int_override->gsi = 2;
+            int_override->flags = 0; /* conforms to bus specifications */
+            int_override++;
+        }
 #endif
-
-        int_override = (struct madt_int_override*)(io_apic + 1);
         for ( i = 0; i < 16; i++ ) {
-            if ( PCI_ISA_IRQ_MASK & (1U << i) ) {
+            if (PCI_ISA_IRQ_MASK & (1U << i)) {
                 memset(int_override, 0, sizeof(*int_override));
                 int_override->type   = APIC_XRUPT_OVERRIDE;
                 int_override->length = sizeof(*int_override);
@@ -1688,7 +1715,6 @@ void acpi_bios_init(void)
                 continue;
             }
             int_override++;
-            madt_size += sizeof(struct madt_int_override);
         }
         acpi_build_table_header((struct acpi_table_header *)madt,
                                 "APIC", madt_size, 1);
@@ -2325,6 +2351,9 @@ void rombios32_init(uint32_t *s3_resume_vector, uint8_t *shutdown_flag)
 
     if (bios_table_cur_addr != 0) {
 
+#ifdef BX_QEMU
+        irq0_override_probe();
+#endif        
         mptable_init();
 
         uuid_probe();

[PATCH 2/4] Userspace changes for configuring irq0->inti2 override (v2)

[Beth Kon]

Signed-off-by: Beth Kon <eak@us.ibm.com>

diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index e1b19d7..bb74f38 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -279,6 +279,7 @@ void *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
     fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
     fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)nographic);
     fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
+    fw_cfg_add_i16(s, FW_CFG_IRQ0_OVERRIDE, (uint16_t)irq0override);
 
     register_savevm("fw_cfg", -1, 1, fw_cfg_save, fw_cfg_load, s);
     qemu_register_reset(fw_cfg_reset, s);
diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
index f616ed2..498c1e3 100644
--- a/hw/fw_cfg.h
+++ b/hw/fw_cfg.h
@@ -15,6 +15,7 @@
 #define FW_CFG_INITRD_SIZE      0x0b
 #define FW_CFG_BOOT_DEVICE      0x0c
 #define FW_CFG_NUMA             0x0d
+#define FW_CFG_IRQ0_OVERRIDE    0x0e
 #define FW_CFG_MAX_ENTRY        0x10
 
 #define FW_CFG_WRITE_CHANNEL    0x4000
diff --git a/hw/ioapic.c b/hw/ioapic.c
index 0b70cf6..2d77a2c 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -23,6 +23,7 @@
 
 #include "hw.h"
 #include "pc.h"
+#include "sysemu.h"
 #include "qemu-timer.h"
 #include "host-utils.h"
 
@@ -95,14 +96,13 @@ void ioapic_set_irq(void *opaque, int vector, int level)
 {
     IOAPICState *s = opaque;
 
-#if 0
     /* ISA IRQs map to GSI 1-1 except for IRQ0 which maps
      * to GSI 2.  GSI maps to ioapic 1-1.  This is not
      * the cleanest way of doing it but it should work. */
 
-    if (vector == 0)
+    if (vector == 0 && irq0override) {
         vector = 2;
-#endif
+    }
 
     if (vector >= 0 && vector < IOAPIC_NUM_PINS) {
         uint32_t mask = 1 << vector;
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 8cb6faa..2e52c87 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -879,7 +879,11 @@ int kvm_arch_init_irq_routing(void)
                 return r;
         }
         for (i = 0; i < 24; ++i) {
-            r = kvm_add_irq_route(kvm_context, i, KVM_IRQCHIP_IOAPIC, i);
+            if (i == 0) {
+                r = kvm_add_irq_route(kvm_context, i, KVM_IRQCHIP_IOAPIC, 2);
+            } else if (i != 2) {
+                r = kvm_add_irq_route(kvm_context, i, KVM_IRQCHIP_IOAPIC, i);
+            }
             if (r < 0)
                 return r;
         }
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 8226001..c64718c 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -165,6 +165,7 @@ void qemu_kvm_cpu_stop(CPUState *env);
 #define kvm_enabled() (kvm_allowed)
 #define qemu_kvm_irqchip_in_kernel() kvm_irqchip_in_kernel(kvm_context)
 #define qemu_kvm_pit_in_kernel() kvm_pit_in_kernel(kvm_context)
+#define qemu_kvm_has_gsi_routing() kvm_has_gsi_routing(kvm_context)
 #define kvm_has_sync_mmu() qemu_kvm_has_sync_mmu()
 void kvm_init_vcpu(CPUState *env);
 void kvm_load_tsc(CPUState *env);
@@ -173,6 +174,7 @@ void kvm_load_tsc(CPUState *env);
 #define kvm_nested 0
 #define qemu_kvm_irqchip_in_kernel() (0)
 #define qemu_kvm_pit_in_kernel() (0)
+#define qemu_kvm_has_gsi_routing() (0)
 #define kvm_has_sync_mmu() (0)
 #define kvm_load_registers(env) do {} while(0)
 #define kvm_save_registers(env) do {} while(0)
diff --git a/sysemu.h b/sysemu.h
index 1f45fd6..292bbc3 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -93,6 +93,7 @@ extern int graphic_width;
 extern int graphic_height;
 extern int graphic_depth;
 extern int nographic;
+extern int irq0override;
 extern const char *keyboard_layout;
 extern int win2k_install_hack;
 extern int rtc_td_hack;
diff --git a/vl.c b/vl.c
index 6b4b7d2..f9a72b3 100644
--- a/vl.c
+++ b/vl.c
@@ -207,6 +207,7 @@ static int vga_ram_size;
 enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
 static DisplayState *display_state;
 int nographic;
+int irq0override;
 static int curses;
 static int sdl;
 const char* keyboard_layout = NULL;
@@ -5039,6 +5040,7 @@ int main(int argc, char **argv, char **envp)
     vga_ram_size = VGA_RAM_SIZE;
     snapshot = 0;
     nographic = 0;
+    irq0override = 1;
     curses = 0;
     kernel_filename = NULL;
     kernel_cmdline = "";
@@ -6135,8 +6137,14 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
-    if (kvm_enabled())
-	kvm_init_ap();
+    if (kvm_enabled()) {
+       kvm_init_ap();
+#ifdef USE_KVM
+        if (kvm_irqchip && !qemu_kvm_has_gsi_routing()) {
+            irq0override = 0;
+        }
+#endif
+    }
 
     machine->init(ram_size, vga_ram_size, boot_devices,
                   kernel_filename, kernel_cmdline, initrd_filename, cpu_model);

[PATCH 3/4] BIOS changes for KVM HPET (v2)

[Beth Kon]

Just a note here... 
The number of table_offset_entry entries for the non BX_QEMU case doesn't make  
sense here. There are only 2 entries. I left it as is, since it does not 
impact HPET's interraction with it. Actually it seems like dead code since 
this is in kvm code but with BX_QEMU undefined. It doesn't seem to be a
problem. 


Signed-off-by: Beth Kon <eak@us.ibm.com>


diff --git a/kvm/bios/acpi-dsdt.dsl b/kvm/bios/acpi-dsdt.dsl
index c756fed..0e142be 100755
--- a/kvm/bios/acpi-dsdt.dsl
+++ b/kvm/bios/acpi-dsdt.dsl
@@ -308,7 +308,6 @@ DefinitionBlock (
             })
         }
 #ifdef BX_QEMU
-#ifdef HPET_WORKS_IN_KVM
         Device(HPET) {
             Name(_HID,  EISAID("PNP0103"))
             Name(_UID, 0)
@@ -328,7 +327,6 @@ DefinitionBlock (
             })
         }
 #endif
-#endif
     }
 
     Scope(\_SB.PCI0) {
diff --git a/kvm/bios/rombios32.c b/kvm/bios/rombios32.c
index ddfa828..7441cd7 100755
--- a/kvm/bios/rombios32.c
+++ b/kvm/bios/rombios32.c
@@ -1293,7 +1293,7 @@ struct rsdt_descriptor_rev1
 {
 	ACPI_TABLE_HEADER_DEF                           /* ACPI common table header */
 #ifdef BX_QEMU
-	uint32_t                             table_offset_entry [2]; /* Array of pointers to other */
+	uint32_t                             table_offset_entry [3]; /* Array of pointers to other */
 //	uint32_t                             table_offset_entry [4]; /* Array of pointers to other */
 #else
 	uint32_t                             table_offset_entry [3]; /* Array of pointers to other */
@@ -1450,8 +1450,8 @@ struct acpi_20_generic_address {
 } __attribute__((__packed__));
 
 /*
- *  * HPET Description Table
- *   */
+ *  HPET Description Table
+ */
 struct acpi_20_hpet {
     ACPI_TABLE_HEADER_DEF                           /* ACPI common table header */
     uint32_t           timer_block_id;
@@ -1591,13 +1591,11 @@ void acpi_bios_init(void)
 #endif
     addr += madt_size;
 #ifdef BX_QEMU
-#ifdef HPET_WORKS_IN_KVM
     addr = (addr + 7) & ~7;
     hpet_addr = addr;
     hpet = (void *)(addr);
     addr += sizeof(*hpet);
 #endif
-#endif
 
     acpi_tables_size = addr - base_addr;
 
@@ -1620,10 +1618,10 @@ void acpi_bios_init(void)
     memset(rsdt, 0, sizeof(*rsdt));
     rsdt->table_offset_entry[0] = cpu_to_le32(fadt_addr);
     rsdt->table_offset_entry[1] = cpu_to_le32(madt_addr);
-    //rsdt->table_offset_entry[2] = cpu_to_le32(ssdt_addr);
 #ifdef BX_QEMU
-    //rsdt->table_offset_entry[3] = cpu_to_le32(hpet_addr);
+    rsdt->table_offset_entry[2] = cpu_to_le32(hpet_addr);
 #endif
+    //rsdt->table_offset_entry[3] = cpu_to_le32(ssdt_addr);
     acpi_build_table_header((struct acpi_table_header *)rsdt,
                             "RSDT", sizeof(*rsdt), 1);
 
@@ -1723,7 +1721,6 @@ void acpi_bios_init(void)
 
 #ifdef BX_QEMU
     /* HPET */
-#ifdef HPET_WORKS_IN_KVM
     memset(hpet, 0, sizeof(*hpet));
     /* Note timer_block_id value must be kept in sync with value advertised by
      * emulated hpet
@@ -1733,7 +1730,6 @@ void acpi_bios_init(void)
     acpi_build_table_header((struct  acpi_table_header *)hpet,
                              "HPET", sizeof(*hpet), 1);
 #endif
-#endif
 
 }
 

[PATCH 4/4] Userspace changes for KVM HPET (v2)

[Beth Kon]

Signed-off-by: Beth Kon <eak@us.ibm.com>


diff --git a/hw/hpet.c b/hw/hpet.c
index c7945ec..47c9f89 100644
--- a/hw/hpet.c
+++ b/hw/hpet.c
@@ -30,6 +30,7 @@
 #include "console.h"
 #include "qemu-timer.h"
 #include "hpet_emul.h"
+#include "qemu-kvm.h"
 
 //#define HPET_DEBUG
 #ifdef HPET_DEBUG
@@ -48,6 +49,43 @@ uint32_t hpet_in_legacy_mode(void)
         return 0;
 }
 
+static void hpet_kpit_enable(void)
+{
+    struct kvm_pit_state ps;
+    kvm_get_pit(kvm_context, &ps);
+    kvm_set_pit(kvm_context, &ps);
+}
+
+static void hpet_kpit_disable(void)
+{
+    struct kvm_pit_state ps;
+    kvm_get_pit(kvm_context, &ps);
+    ps.channels[0].mode = 0xff;
+    kvm_set_pit(kvm_context, &ps);
+}
+
+static void hpet_legacy_enable(void)
+{
+    if (qemu_kvm_pit_in_kernel()) {
+       hpet_kpit_disable();
+       dprintf("qemu: hpet disabled kernel pit\n");
+    } else {
+       hpet_pit_disable();
+       dprintf("qemu: hpet disabled userspace pit\n");
+    }
+}
+
+static void hpet_legacy_disable(void)
+{
+    if (qemu_kvm_pit_in_kernel()) {
+       hpet_kpit_enable();
+       dprintf("qemu: hpet enabled kernel pit\n");
+    } else {
+       hpet_pit_enable();
+       dprintf("qemu: hpet enabled userspace pit\n");
+    }
+}
+
 static uint32_t timer_int_route(struct HPETTimer *timer)
 {
     uint32_t route;
@@ -475,9 +513,9 @@ static void hpet_ram_writel(void *opaque, target_phys_addr_t addr,
                 }
                 /* i8254 and RTC are disabled when HPET is in legacy mode */
                 if (activating_bit(old_val, new_val, HPET_CFG_LEGACY)) {
-                    hpet_pit_disable();
+                    hpet_legacy_enable();
                 } else if (deactivating_bit(old_val, new_val, HPET_CFG_LEGACY)) {
-                    hpet_pit_enable();
+                    hpet_legacy_disable();
                 }
                 break;
             case HPET_CFG + 4:
@@ -560,7 +598,7 @@ static void hpet_reset(void *opaque) {
          * hpet_reset is called due to system reset. At this point control must
          * be returned to pit until SW reenables hpet.
          */
-        hpet_pit_enable();
+        hpet_legacy_disable();
     count = 1;
 }
 
diff --git a/vl.c b/vl.c
index f9a72b3..b860b82 100644
--- a/vl.c
+++ b/vl.c
@@ -6138,10 +6138,15 @@ int main(int argc, char **argv, char **envp)
     }
 
     if (kvm_enabled()) {
-       kvm_init_ap();
+        kvm_init_ap();
 #ifdef USE_KVM
         if (kvm_irqchip && !qemu_kvm_has_gsi_routing()) {
             irq0override = 0;
+            /* if kernel can't do irq routing, interrupt source
+             * override 0->2 can not be set up as required by hpet,
+             * so disable hpet.
+             */
+            no_hpet=1;
         }
 #endif
     }

Re: [PATCH 1/4] BIOS changes for configuring irq0->inti2 override (v2)

[Avi Kivity]

Beth Kon wrote:
> These patches resolve the irq0->inti2 override issue, and get the hpet working
> on kvm. 
>
> They are dependent on Jes Sorensen's recent 0006-qemu-kvm-irq-routing.patch.
>
> Override and HPET changes are sent as a series because HPET depends on the 
> override. Win2k8 expects the HPET interrupt on inti2, regardless of whether 
> an override exists in the BIOS. And the HPET spec states that in legacy mode, 
> timer interrupt is on inti2.
>
> The irq0->inti2 override will always be used unless the kernel cannot do irq 
> routing (i.e., compatibility with old kernels). So if the kernel is capable, 
> userspace sets up irq0->inti2 via the irq routing interface, and adds the 
> irq0->inti2 override to the MADT interrupt source override table, 
> and the mp table (for the no-acpi case).
>
> A couple of months ago, Marcelo was seeing RHEL5 guests complain of invalid
> checksum with these patches, but later he couldn't reproduce it, and I'm not 
> seeing it now. While all guests still need to be fully tested, everything 
> appears to be in order.  I've tested on win2k864, win2k832, RHEL5.3 32 bit, 
> and ubuntu 8.10 64 bit. 
>
>   

What are the changes relative to v1?

> @@ -477,6 +480,7 @@ void wrmsr_smp(uint32_t index, uint64_t val)
>  #define QEMU_CFG_SIGNATURE  0x00
>  #define QEMU_CFG_ID         0x01
>  #define QEMU_CFG_UUID       0x02
> +#define QEMU_CFG_IRQ0_OVERRIDE 0x0e
>   

As noted, this should be in the arch local space.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/4] BIOS changes for configuring irq0->inti2 override(v2)

[Beth Kon]

Avi Kivity wrote:
> Beth Kon wrote:
>> These patches resolve the irq0->inti2 override issue, and get the 
>> hpet working
>> on kvm.
>> They are dependent on Jes Sorensen's recent 
>> 0006-qemu-kvm-irq-routing.patch.
>>
>> Override and HPET changes are sent as a series because HPET depends 
>> on the override. Win2k8 expects the HPET interrupt on inti2, 
>> regardless of whether an override exists in the BIOS. And the HPET 
>> spec states that in legacy mode, timer interrupt is on inti2.
>>
>> The irq0->inti2 override will always be used unless the kernel cannot 
>> do irq routing (i.e., compatibility with old kernels). So if the 
>> kernel is capable, userspace sets up irq0->inti2 via the irq routing 
>> interface, and adds the irq0->inti2 override to the MADT interrupt 
>> source override table, and the mp table (for the no-acpi case).
>>
>> A couple of months ago, Marcelo was seeing RHEL5 guests complain of 
>> invalid
>> checksum with these patches, but later he couldn't reproduce it, and 
>> I'm not seeing it now. While all guests still need to be fully 
>> tested, everything appears to be in order.  I've tested on win2k864, 
>> win2k832, RHEL5.3 32 bit, and ubuntu 8.10 64 bit.
>>   
>
> What are the changes relative to v1?
Just merge issues with the changes you put in when moving to the newer 
bios. I submitted prematurely, incorrectly thinking I was done testing. 
When I finished, some problems surfaced.
>
>> @@ -477,6 +480,7 @@ void wrmsr_smp(uint32_t index, uint64_t val)
>>  #define QEMU_CFG_SIGNATURE  0x00
>>  #define QEMU_CFG_ID         0x01
>>  #define QEMU_CFG_UUID       0x02
>> +#define QEMU_CFG_IRQ0_OVERRIDE 0x0e
>>   
>
> As noted, this should be in the arch local space.
>
The base changes were not in the code yet. As we discussed on IRC, I'll 
resubmit once they're there.